Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#9739 - URLValidator now allows two slashes in the path component of the URL #9750

Merged
merged 4 commits into from
May 21, 2024

Conversation

luddaniel
Copy link
Contributor

What this PR does / why we need it:

URLValidator now allows two slashes in the path component of the URL

Which issue(s) this PR closes:

Closes #9739

Special notes for your reviewer:

From what I see, URLValidator is used to :

  • validate url type of field
  • validate url filled via deaccession dialog

As discussed in #9739, a user is able to provide "valid" syntax url that can be "invalid" when used, so allowing two slashes in the path component of the URL seems to not bring any danger.

Suggestions on how to test this:

  • Save a dataset with an url in a url field type
  • Deaccessionate a dataset and specify a forwarding url

Example of allowed url : https://archive.softwareheritage.org/swh:1:dir:561bfe6698ca9e58b552b4eb4e56132cac41c6f9;origin=https://github.com/gem-pasteur/macsyfinder;visit=swh:1:snp:1bde3cb370766b10132c4e004c7cb377979928d1;anchor=swh:1:rev:868637fce184865d8e0436338af66a2648e8f6e1

Does this PR introduce a user interface change?

No

Is there a release notes update needed for this change?

No

Copy link
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. FYI - I did a quick check and UrlValidator is also used to decide whether the forwarding URL for a deaccessioned dataset is OK. I think allowing two slashes there is also OK, but we should be aware that this PR as is will change that as well.

@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label Aug 2, 2023
@pdurbin
Copy link
Member

pdurbin commented Aug 2, 2023

The ec2 instance didn't spin up. I just triggered another run of the API tests: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9750/2/

@scolapasta
Copy link
Contributor

@luddaniel a small release note for this change would ne nice for users, as well.

@scolapasta scolapasta self-assigned this Apr 25, 2024
@luddaniel
Copy link
Contributor Author

@scolapasta release node is added

@coveralls
Copy link

Coverage Status

coverage: 20.603%. remained the same
when pulling fbe950f on Recherche-Data-Gouv:9739-url-validator
into a329f29 on IQSS:develop.

@scolapasta scolapasta removed their assignment May 3, 2024
@stevenwinship stevenwinship self-assigned this May 21, 2024
@stevenwinship stevenwinship merged commit ba0cdaf into IQSS:develop May 21, 2024
11 checks passed
@stevenwinship stevenwinship removed their assignment May 21, 2024
@pdurbin pdurbin added this to the 6.3 milestone May 21, 2024
@luddaniel luddaniel deleted the 9739-url-validator branch September 19, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: 🚀 Done (Recherche Data Gouv)
Development

Successfully merging this pull request may close these issues.

Metadata Customization : url fieldType validator
7 participants