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

Fix duplication of uri when the value of a format already contains one #1033

Merged
merged 5 commits into from Dec 12, 2019

Conversation

Luwangel
Copy link
Contributor

@Luwangel Luwangel commented Dec 12, 2019

Trello Card #

Ref. #986

The AsterPlot feature introduced a bug in some formats due to a duplication of the URI in the query URL.

It's the case with the LodexField format, which has a value equals to:
https://publication-language.data.istex.fr/api/export/jsonallvalue?uri=ark:/67375/KM1-FNPPMN4Q-S

Due to the similar resources feature introduced by the AsterPlot format, we needed to pass the current resource URI to get similar resources. And we decided to do it for all formats.

But as a result, when the base URL already contains a URI, we have the following duplication:
https://publication-language.data.istex.fr/api/export/jsonallvalue?uri=ark:/67375/KM1-FNPPMN4Q-S?uri=uid%3A%2F12WDH7ZW

So we need to change the default behaviour to inject the uri only when it's necessary (and so currently only in the AsterPlot format).

Todo

  • Add options to the inject data method to include the URI only when it's needed
  • Update the load format saga to not send the URI when it's not needed
  • Fix tests

Screenshots

Sélection_003

@Luwangel Luwangel added Trello 👷‍♂️ Ready For Review PR en attente de relecture et de validation labels Dec 12, 2019
@mchaffotte mchaffotte merged commit 47fdc24 into master Dec 12, 2019
@mchaffotte mchaffotte deleted the format-lodex-champ branch December 12, 2019 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👷‍♂️ Ready For Review PR en attente de relecture et de validation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants