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/link to ereol #654

Merged

Conversation

Adamik10
Copy link
Contributor

@Adamik10 Adamik10 commented Nov 2, 2023

Link to issue

https://reload.atlassian.net/browse/DDFLSBP-160
https://reload.atlassian.net/browse/DDFLSBP-199

Description

This PR links from the active reservation modal for ereol materials to the ereol origin page of the given material.
Eg. a user has a reservation for the Bible from the Ereol platform - upon opening the reservation details modal and clicking on the "Go to Ereolen" external link button, they will be redirected to the Bible work page on the Ereol platform.
In the case of this access data not being available in the FBI API the user is redirected to the Ereol home page instead.

Screenshot of the result

Screen.Recording.2023-11-02.at.11.43.59.mov

Additional comments or questions

n/a

- Fragments for manifestation with only access data
- complexSearchWithPagination query
The LinkButon wraps arount the Link component, and the Link component
supports this.
Eg. a user has a reservation for the Bible from the Ereol platform -
upon opening the reservation details modal and clicking on the "Go to
Ereolen" external link button, they will be redirected to the Bible work
page on the Ereol platform. In the case of this access data not being
available in the FBI API the user is redirected to the Ereol home page
instead.
Copy link
Contributor

@spaceo spaceo left a comment

Choose a reason for hiding this comment

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

There is a cypress test failing. I'll wait until the test has been fixed because I do not know what implications of the changes would be

@Adamik10 Adamik10 requested a review from spaceo November 7, 2023 10:15
@spaceo spaceo removed their assignment Nov 9, 2023
Copy link
Contributor

@spaceo spaceo left a comment

Choose a reason for hiding this comment

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

Nice stuff!
Have a few comments..

This function compares ISBN identifiers, which can have different
formats, some including spaces, and/or dashes, or without them
altogether. Because of this, we remove all non-digit characters first
before making a comparison.
Copy link
Contributor

@spaceo spaceo left a comment

Choose a reason for hiding this comment

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

Great changes.
Sorry I found some other things I missed in my first review...

@spaceo spaceo removed their assignment Nov 15, 2023
Instead of a hard-coded static string, we now use a configurable url in
the reservation details component when redirecting user to ereol's home
page.
We only use the first one anyways. And limiting it to 100 could affect
performance.
@Adamik10 Adamik10 requested a review from spaceo November 15, 2023 10:33
Copy link
Contributor

@spaceo spaceo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@spaceo spaceo removed their assignment Nov 20, 2023
@Adamik10 Adamik10 merged commit 4ed29e7 into danskernesdigitalebibliotek:demo_2023-48-0 Nov 27, 2023
10 checks passed
@Adamik10 Adamik10 deleted the fix/link-to-ereol branch November 27, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants