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

soustab10 · 2023-03-08T07:45:11Z

Fixes

Fixes [Bug] Add regex to 'Link to Work' , 'Link to Creator Profile' and add numeric validation to 'Year of Creation' #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

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the default branch of the repository (main or master).
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
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 · 2023-03-08T07:45:34Z

@possumbilities please review this PR. Thank you.

possumbilities · 2023-03-10T20:08:34Z

@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 · 2023-03-10T20:12:57Z

@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 · 2023-03-21T20:27:51Z

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

soustab10 · 2023-03-21T20:29:17Z

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

yes, absolutely.

soustab10 · 2023-03-21T20:36:02Z

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

Cronus1007 · 2023-03-21T21:01:35Z

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

codewithmide · 2023-03-22T13:58:42Z

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

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

soustab10 · 2023-03-23T07:25:48Z

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

Cronus1007 · 2023-03-23T08:02:10Z

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

src/store/index.js

src/components/AttributionDetailsStep.vue

Cronus1007 · 2023-03-23T13:04:55Z

@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 · 2023-03-23T13:26:19Z

Thank you @Cronus1007 . What did you change?

Cronus1007 · 2023-03-23T13:27:34Z

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

soustab10 · 2023-03-23T13:29:11Z

@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 · 2023-03-23T13:30:47Z

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

possumbilities · 2023-03-31T20:37:31Z

@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 · 2023-03-31T20:39:28Z

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

soustab10 · 2023-03-31T20:44:35Z

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

I have made the changes requested via this commit

possumbilities · 2023-03-31T20:46:34Z

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

Cronus1007 · 2023-03-31T21:26:45Z

@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

@soustab10 Thanks for the contribution 💯
@possumbilities LGTM

soustab10 · 2023-04-01T20:58:29Z

@soustab10 Thanks for the contribution 💯 @possumbilities LGTM

Thanks a lot! Hoping to make more contributions!

possumbilities · 2023-06-23T19:45:56Z

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 · 2023-07-11T19:26:59Z

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 · 2024-04-12T17:12:59Z

✅ Deploy Preview for creativecommons-chooser ready!

Name	Link
🔨 Latest commit	`68c5962`
🔍 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.

TimidRobot

In addition to the requested changes, I think there needs to be more consideration to the UX and user stories:

I think help text should be a different color (not red, as validation is non-blocking)
Users of the Chooser interested in print or media may not want the protocol
- (this is an issue in the current design, not just this PR)
As the validation is non-blocking, it needs to be worded as a suggestion

TimidRobot · 2024-04-12T17:16:43Z

src/components/AttributionDetailsStep.vue

+        this.attributionErrorMsg.yearOfCreationError = '';
+      }
+      else {
+        this.attributionErrorMsg.yearOfCreationError = 'Please enter a valid year';


Two digit years are valid, even if imprecise. Also the text is nonblocking.

Please update text to something like: For clarity, please consider a four digit year

TimidRobot · 2024-04-12T17:39:18Z

src/components/AttributionDetailsStep.vue

@@ -58,7 +62,7 @@ import { FontAwesomeIcon } from '@fortawesome/vue-fontawesome';
 import { faInfoCircle } from '@fortawesome/free-solid-svg-icons';
 import { library } from '@fortawesome/fontawesome-svg-core';
 library.add(faInfoCircle);
-
+const regexTestString= /[(http(s)?):(www)?a-zA-Z0-9@:%._~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_+.~#?&//=]*)/;


This regex has a number of problems (including #456 (comment)).

Maybe use The Perfect URL Regular Expression - Perfect URL Regex:

const regexTestString= /((([A-Za-z]{3,9}:(?:\/\/)?)(?:[\-;:&=\+\$,\w]+@)?[A-Za-z0-9\.\-]+|(?:www\.|[\-;:&=\+\$,\w]+@)[A-Za-z0-9\.\-]+)((?:\/[\+~%\/\.\w\-_]*)?\??(?:[\-\+=&;%@\.\w_]*)#?(?:[\.\!\/\\\w]*))?)/;

soustab10 added 2 commits March 6, 2023 14:56

added regex checks to url

279a582

added year validation

02e1da3

soustab10 requested a review from a team as a code owner March 8, 2023 07:45

cc-open-source-bot added this to Code Review in Active Sprint Mar 8, 2023

possumbilities mentioned this pull request Mar 8, 2023

[Bug] Add regex to 'Link to Work' , 'Link to Creator Profile' and add numeric validation to 'Year of Creation' #452

Open

1 task

Merge branch 'creativecommons:main' into regexcheck

8ded40d

soustab10 requested review from a team as code owners March 10, 2023 19:22

soustab10 requested review from TimidRobot, possumbilities and akmadian and removed request for a team March 10, 2023 19:22

soustab10 added 2 commits March 11, 2023 20:39

removed vee-validate and updated year validation

788954a

removed errors

618a241

soustab10 and others added 2 commits March 23, 2023 12:53

removed vee-validate dependency

213592d

Merge branch 'creativecommons:main' into regexcheck

6953a35

Cronus1007 requested changes Mar 23, 2023

View reviewed changes

src/store/index.js Outdated Show resolved Hide resolved

src/components/AttributionDetailsStep.vue Outdated Show resolved Hide resolved

deleted unused lines and store regex check in variable

5d73a61

resolved package-lock.json

9afc23c

possumbilities removed request for TimidRobot and akmadian March 31, 2023 20:35

Cronus1007 approved these changes Apr 1, 2023

View reviewed changes

Merge branch 'main' into regexcheck

da328de

Merge branch 'main' into regexcheck

cc92078

TimidRobot requested changes Apr 12, 2024

View reviewed changes

Merge branch 'main' into regexcheck

68c5962

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

soustab10 commented Mar 8, 2023 •

edited by TimidRobot

Loading

soustab10 commented Mar 8, 2023

possumbilities commented Mar 10, 2023

soustab10 commented Mar 10, 2023

possumbilities commented Mar 21, 2023

soustab10 commented Mar 21, 2023

soustab10 commented Mar 21, 2023

Cronus1007 commented Mar 21, 2023

codewithmide commented Mar 22, 2023 •

edited

Loading

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023 •

edited

Loading

Cronus1007 commented Mar 23, 2023 •

edited

Loading

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023

possumbilities commented Mar 31, 2023

possumbilities commented Mar 31, 2023

soustab10 commented Mar 31, 2023

possumbilities commented Mar 31, 2023

Cronus1007 commented Mar 31, 2023

Cronus1007 left a comment •

edited

Loading

soustab10 commented Apr 1, 2023

possumbilities commented Jun 23, 2023 •

edited

Loading

possumbilities commented Jul 11, 2023

netlify bot commented Apr 12, 2024 •

edited

Loading

TimidRobot left a comment

TimidRobot Apr 12, 2024 •

edited by possumbilities

Loading

TimidRobot Apr 12, 2024

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

Are you sure you want to change the base?

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

Conversation

soustab10 commented Mar 8, 2023 • edited by TimidRobot Loading

Fixes

Description

Tests

Screenshots

Checklist

Developer Certificate of Origin

soustab10 commented Mar 8, 2023

possumbilities commented Mar 10, 2023

soustab10 commented Mar 10, 2023

possumbilities commented Mar 21, 2023

soustab10 commented Mar 21, 2023

soustab10 commented Mar 21, 2023

Cronus1007 commented Mar 21, 2023

codewithmide commented Mar 22, 2023 • edited Loading

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023 • edited Loading

Cronus1007 commented Mar 23, 2023 • edited Loading

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023

soustab10 commented Mar 23, 2023

Cronus1007 commented Mar 23, 2023

possumbilities commented Mar 31, 2023

possumbilities commented Mar 31, 2023

soustab10 commented Mar 31, 2023

possumbilities commented Mar 31, 2023

Cronus1007 commented Mar 31, 2023

Cronus1007 left a comment • edited Loading

Choose a reason for hiding this comment

soustab10 commented Apr 1, 2023

possumbilities commented Jun 23, 2023 • edited Loading

possumbilities commented Jul 11, 2023

netlify bot commented Apr 12, 2024 • edited Loading

✅ Deploy Preview for creativecommons-chooser ready!

TimidRobot left a comment

Choose a reason for hiding this comment

TimidRobot Apr 12, 2024 • edited by possumbilities Loading

Choose a reason for hiding this comment

TimidRobot Apr 12, 2024

Choose a reason for hiding this comment

soustab10 commented Mar 8, 2023 •

edited by TimidRobot

Loading

codewithmide commented Mar 22, 2023 •

edited

Loading

Cronus1007 commented Mar 23, 2023 •

edited

Loading

Cronus1007 commented Mar 23, 2023 •

edited

Loading

Cronus1007 left a comment •

edited

Loading

possumbilities commented Jun 23, 2023 •

edited

Loading

netlify bot commented Apr 12, 2024 •

edited

Loading

TimidRobot Apr 12, 2024 •

edited by possumbilities

Loading