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

#7009 Removed one of the redundant encoding functions. #7010

Merged
merged 1 commit into from Jul 2, 2020
Merged

#7009 Removed one of the redundant encoding functions. #7010

merged 1 commit into from Jul 2, 2020

Conversation

JingMa87
Copy link
Contributor

@JingMa87 JingMa87 commented Jun 23, 2020

What this PR does / why we need it: Fixes double set name encoding in the URL which creates a faulty URL

Which issue(s) this PR closes: 7009

Closes #7009

Special notes for your reviewer: It's an amazing fix.

Suggestions on how to test this: You can try harvesting the repo https://easy.dans.knaw.nl/oai with the set name D20000:D26000 and the prefix oai_dc. The archive type should be Generic OAI archive. The faulty URL returned 0 results and the fix will minimally return 8 failed results. I'm looking into the failures for a possible next commit.

Does this PR introduce a user interface change? If mockups are available, please link/include them here: No

Is there a release notes update needed for this change?: Yes, apparently there's quite some repo's that have issues so this will possibly solve quite some of these.

Additional documentation:

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0008%) to 19.558% when pulling 1da6cc6 on JingMa87:7009-double-set-name-encoding into b97185a on IQSS:develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

This is a small change that can easily be reversed and seems to fix a problem @JingMa87 is having.

I'm not sure why the encoding code is in there. It's interesting that removing it fixes it.

I'm moving this to QA but @landreev might want to take a look as well.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Code Review 🦁 to QA 🔎✅ Jun 24, 2020
@kcondon kcondon self-assigned this Jul 2, 2020
@kcondon kcondon merged commit bd736d2 into IQSS:develop Jul 2, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jul 2, 2020
@JingMa87 JingMa87 deleted the 7009-double-set-name-encoding branch July 2, 2020 17:46
@djbrooke djbrooke added this to the Dataverse 5 milestone Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

The set param in the OAI-PMH request URL is wrongfully encoded twice
5 participants