chooser icon indicating copy to clipboard operation
chooser copied to clipboard

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

Open soustab10 opened this issue 1 year ago • 24 comments

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

image

Leaving the field empty or putting the correct URL gives no error.

image

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 avatar Mar 08 '23 07:03 soustab10

@possumbilities please review this PR. Thank you.

soustab10 avatar Mar 08 '23 07:03 soustab10

@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 avatar Mar 10 '23 20:03 possumbilities

@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 avatar Mar 10 '23 20:03 soustab10

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

possumbilities avatar Mar 21 '23 20:03 possumbilities

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

yes, absolutely.

soustab10 avatar Mar 21 '23 20:03 soustab10

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

soustab10 avatar Mar 21 '23 20:03 soustab10

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

Cronus1007 avatar Mar 21 '23 21:03 Cronus1007

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

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

codewithmide avatar Mar 22 '23 13:03 codewithmide

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

soustab10 avatar Mar 23 '23 07:03 soustab10

@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 avatar Mar 23 '23 08:03 Cronus1007

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

Cronus1007 avatar Mar 23 '23 13:03 Cronus1007

Thank you @Cronus1007 . What did you change?

soustab10 avatar Mar 23 '23 13:03 soustab10

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

Cronus1007 avatar Mar 23 '23 13:03 Cronus1007

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

soustab10 avatar Mar 23 '23 13:03 soustab10

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

Cronus1007 avatar Mar 23 '23 13:03 Cronus1007

@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 avatar Mar 31 '23 20:03 possumbilities

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

possumbilities avatar Mar 31 '23 20:03 possumbilities

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

I have made the changes requested via this commit

soustab10 avatar Mar 31 '23 20:03 soustab10

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

possumbilities avatar Mar 31 '23 20:03 possumbilities

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

Cronus1007 avatar Mar 31 '23 21:03 Cronus1007

@soustab10 Thanks for the contribution 💯 @possumbilities LGTM

Thanks a lot! Hoping to make more contributions!

soustab10 avatar Apr 01 '23 20:04 soustab10

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 avatar Jun 23 '23 19:06 possumbilities

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!

possumbilities avatar Jul 11 '23 19:07 possumbilities

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

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Apr 12 '24 17:04 netlify[bot]