Added regex checks for 'Link to Work' and 'Link to Creator Profile'

soustab10 commented 1 year ago

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 or master).
[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. ```

soustab10 commented 1 year ago

@possumbilities please review this PR. Thank you.

possumbilities commented 1 year ago

@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 commented 1 year ago

@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.

possumbilities commented 1 year ago

@soustab10 It looks like your project isn't passing the checks, can you investigate and verify?

soustab10 commented 1 year ago

@soustab10 It looks like your project isn't passing the checks, can you investigate and verify?

yes, absolutely.

soustab10 commented 1 year ago

@possumbilities the module @creativecommons/cc-assets is being read as undefined, causing the build to fail. I am trying to fix this asap.

Cronus1007 commented 1 year ago

@soustab10 Can't we completely rid of the vee-validate dependency?

codewithmide commented 1 year ago

@soustab10 Can't we completely rid of the vee-validate dependency?

@soustab10 do you mind if we work this out together?

soustab10 commented 1 year ago

@Cronus1007 I have removed the vee-validate dependency completely.

Cronus1007 commented 1 year ago

@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?

Cronus1007 commented 1 year ago

@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?

soustab10 commented 1 year ago

Thank you @Cronus1007 . What did you change?

Cronus1007 commented 1 year ago

@soustab10 I just used the package-lock.json that is into the main branch of Chooser codebase.

soustab10 commented 1 year ago

@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?

Cronus1007 commented 1 year ago

Lets wait for @possumbilities to review the PR as well.

possumbilities commented 1 year ago

@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.

possumbilities commented 1 year ago

@soustab10 looks like there's still changes requested pending?

soustab10 commented 1 year ago

@soustab10 looks like there's still changes requested pending?

I have made the changes requested via this commit

possumbilities commented 1 year ago

@Cronus1007 it's waiting on you to sign off on the review request I think.

Cronus1007 commented 1 year ago

@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 commented 1 year ago

@soustab10 Thanks for the contribution 💯 @possumbilities LGTM

Thanks a lot! Hoping to make more contributions!

possumbilities commented 1 year ago

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.

possumbilities commented 1 year ago

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!

netlify[bot] commented 4 months ago

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.

creativecommons / chooser