chooser
chooser copied to clipboard
Added regex checks for 'Link to Work' and 'Link to Creator Profile'
Fixes
- Fixes #452 by @soustab10 (myself)
Description
- This PR adds regex checks for the 'Link to Work' and 'Link to Creator Profile' fields under Attribution details section.
- It adds a numeric validation for the year of creation to be a 4 digit number.
Tests
This PR can be tested by putting an invalid URL to the mentioned sections. A prompt Please enter a valid URL
is presented.
Screenshots
Leaving the field empty or putting the correct URL gives no error.
Checklist
- [x] My pull request has a descriptive title (not a vague title like
Update index.md
). - [x] My pull request targets the default branch of the repository (
main
ormaster
). - [x] My commit messages follow best practices.
- [x] My code follows the established code style of the repository.
- [x] I added or updated tests for the changes I made (if applicable).
- [x] I added or updated documentation (if applicable).
- [x] I tried running the project locally and verified that there are no visible errors.
Developer Certificate of Origin
For the purposes of this DCO, "license" is equivalent to "license or public domain dedication," and "open source license" is equivalent to "open content license or public domain dedication."
Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
@possumbilities please review this PR. Thank you.
@soustab10 Thanks for working on this! I see you used a new dependency vee-validate
. This project already needs some pairing back on how many dependencies its utilizing. So I'm going to look into it further. The preference here would be accomplishing validation without introducing new dependencies into the chain first, and then IF it's necessary outlining why, what the cost/benefit analysis is, etc.
@soustab10 Thanks for working on this! I see you used a new dependency
vee-validate
. This project already needs some pairing back on how many dependencies its utilizing. So I'm going to look into it further. The preference here would be accomplishing validation without introducing new dependencies into the chain first, and then IF it's necessary outlining why, what the cost/benefit analysis is, etc.
@possumbilities vee-validate isn't necessary at all. I was just trying during the development phase to see what works best. I will commit soon with the necessary changes. Thank you.
@soustab10 It looks like your project isn't passing the checks, can you investigate and verify?
@soustab10 It looks like your project isn't passing the checks, can you investigate and verify?
yes, absolutely.
@possumbilities the module @creativecommons/cc-assets
is being read as undefined, causing the build to fail. I am trying to fix this asap.
@soustab10 Can't we completely rid of the vee-validate dependency?
@soustab10 Can't we completely rid of the vee-validate dependency?
@soustab10 do you mind if we work this out together?
@Cronus1007 I have removed the vee-validate dependency completely.
@soustab10 Please revert the changes made in package-lock.json. The workflows are still failing. Please have a look upon them as well. Are you able to run it locally?
@soustab10 I have resolved the package-lock.json conflict for you. As much as I understood Ig you used --force
flag thats why ci/cd was failing or your npm version which you used differ from 6.14.8 .
@possumbilities I think we should move package-lock.json into .gitignore so that no such issue will occur in future. What your take upon it?
Thank you @Cronus1007 . What did you change?
@soustab10 I just used the package-lock.json that is into the main
branch of Chooser codebase.
@soustab10 I just used the package-lock.json that is into the
main
branch of Chooser codebase.
Okay thanks a lot! Do I need to make any further changes?
Lets wait for @possumbilities to review the PR as well.
@Cronus1007 I think there's value in keeping the package-lock.json
file tracked so we can make changes to it in the future, but people should be careful not committing changes to it when not needed. It does have its uses and I'm not sure its worth removing outright going forward.
@soustab10 looks like there's still changes requested pending?
@soustab10 looks like there's still changes requested pending?
I have made the changes requested via this commit
@Cronus1007 it's waiting on you to sign off on the review request I think.
@Cronus1007 I think there's value in keeping the
package-lock.json
file tracked so we can make changes to it in the future, but people should be careful not committing changes to it when not needed.
Makes sense.
@soustab10 Thanks for the contribution 💯 @possumbilities LGTM
Thanks a lot! Hoping to make more contributions!
Hi! @soustab10 thank you so much for all your work on this, it's greatly appreciated. Apologies for the lengthy delay in response here.
I tested this and there's still at least one unresolved issued with the regex. The URL check seems to be capping the validation of the domain extension to 6 characters. As a result valid TLDs will throw an "invalid URL" UX error.
For example:
https://something.channel
There's a lengthy list of valid domains that are over the 6 character limit in the IANA list.
I'd suggest adjusting the regex in the checker to not be bound by the 6character limit on the extension, so the error doesn't throw incorrectly.
Hi again @soustab10 just letting you know there's still a minor error here that needs resolving before it can be approved. Thanks so much for all your work so far!
Deploy Preview for creativecommons-chooser ready!
Name | Link |
---|---|
Latest commit | 68c5962625c9793b44045b70b4293671a1734554 |
Latest deploy log | https://app.netlify.com/sites/creativecommons-chooser/deploys/66197341d002040008279b70 |
Deploy Preview | https://deploy-preview-456--creativecommons-chooser.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.