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

DDFBRA-236 - Logged in user can go from digital loans to material #1565

Conversation

kasperbirch1
Copy link
Contributor

@kasperbirch1 kasperbirch1 commented Nov 28, 2024

Link to issue

https://reload.atlassian.net/browse/DDFBRA-236
danskernesdigitalebibliotek/dpl-cms#1814

Description

This pull request replaces the link from the digital loan list to point to the material page instead of eReolen.

Unfortunately, I also found a bug in our code that causes an infinite re-render. This issue is also present on the develop branch, and I didn't find an easy solution to fix it. Therefore, this should be fixed in a separate ticket.

Test

If you have an online loan, you can view it under the “Digital Loans” section. The button, which was previously labeled “Go to eReolen” and linked to eReolen, has been updated to now correctly link to the work with the appropriate type selected.

https://varnish.pr-1814.dpl-cms.dplplat01.dpl.reload.dk/user/me/loans

Min.film.1.mp4

@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-236-logged-in-user-can-go-from-digital-loans-to-material-to-listen-read branch 2 times, most recently from 7a2cc4d to a23e109 Compare December 2, 2024 11:31
kasperbirch1 added a commit to danskernesdigitalebibliotek/dpl-cms that referenced this pull request Dec 2, 2024
The translation key 'material-details-go-to-ereolen' has been updated to 'material-details-go-to-material' following changes made in: danskernesdigitalebibliotek/dpl-react#1565.

Additionally, the 'ereolen-my-page-url' has been removed, as it no longer links to eReolen from the loans page.
kasperbirch1 added a commit to danskernesdigitalebibliotek/dpl-cms that referenced this pull request Dec 3, 2024
The translation key 'material-details-go-to-ereolen' has been updated to 'material-details-go-to-material' following changes made in: danskernesdigitalebibliotek/dpl-react#1565.

Additionally, the 'ereolen-my-page-url' has been removed, as it no longer links to eReolen from the loans page.
@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-183-anonymous-and-logged-in-user-can-try-ebook branch from 665996b to e8a38e8 Compare December 3, 2024 11:05
@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-236-logged-in-user-can-go-from-digital-loans-to-material-to-listen-read branch from a23e109 to a25ae80 Compare December 3, 2024 11:05
kasperbirch1 added a commit to danskernesdigitalebibliotek/dpl-cms that referenced this pull request Dec 3, 2024
The translation key 'material-details-go-to-ereolen' has been updated to 'material-details-go-to-material' following changes made in: danskernesdigitalebibliotek/dpl-react#1565.

Additionally, the 'ereolen-my-page-url' has been removed, as it no longer links to eReolen from the loans page.
@Adamik10 Adamik10 self-requested a review December 4, 2024 14:22
@Adamik10 Adamik10 removed their assignment Dec 4, 2024
Copy link
Contributor

@Adamik10 Adamik10 left a comment

Choose a reason for hiding this comment

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

Beautiful job. We clap and cheer.
I have 2 little comments for you to consider 🙏 but otherwise approved 😉

@@ -33,6 +34,7 @@ const MaterialDetailsModal: FC<MaterialDetailsModalProps> = ({
children
}) => {
const t = useText();
// console.log("Rendering MaterialDetailsModal:");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should remove this line?

@@ -1,3 +1,4 @@
// Todo: Fix infinite loop in MaterialDetailsModal
Copy link
Contributor

Choose a reason for hiding this comment

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

Bahahaha. How fun.
Try to describe it better what needs to be fixed so that people who don't know about the bug at all would be able to reproduce it 😉

kasperbirch1 added a commit to danskernesdigitalebibliotek/dpl-cms that referenced this pull request Dec 6, 2024
The translation key 'material-details-go-to-ereolen' has been updated to 'material-details-go-to-material' following changes made in: danskernesdigitalebibliotek/dpl-react#1565.

Additionally, the 'ereolen-my-page-url' has been removed, as it no longer links to eReolen from the loans page.
@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-236-logged-in-user-can-go-from-digital-loans-to-material-to-listen-read branch from a25ae80 to c02d269 Compare December 6, 2024 12:12
While investigating https://reload.atlassian.net/browse/DDFBRA-236, I identified an infinite loop in `MaterialDetailsModal`. This issue appears when the modal is opened via "user/loans" and potentially all places it is used
Since all data from loans comes from Publizon, we don’t have a `workId`, which is required to link to the material. Therefore, I use the `useComplexSearchWithPaginationQuery` to find the work using the ISBN (identifier) and construct the `workId` into a URL, e.g., `/work/work-of:870970-basis:62986115`.  Additionally, if it finds the material type, it adds that as a parameter.
@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-183-anonymous-and-logged-in-user-can-try-ebook branch from d01a598 to bd60ac0 Compare December 11, 2024 08:40
@kasperbirch1 kasperbirch1 force-pushed the DDFBRA-236-logged-in-user-can-go-from-digital-loans-to-material-to-listen-read branch from c02d269 to 94c6cd2 Compare December 11, 2024 08:40
kasperbirch1 added a commit to danskernesdigitalebibliotek/dpl-cms that referenced this pull request Dec 12, 2024
The translation key 'material-details-go-to-ereolen' has been updated to 'material-details-go-to-material' following changes made in: danskernesdigitalebibliotek/dpl-react#1565.

Additionally, the 'ereolen-my-page-url' has been removed, as it no longer links to eReolen from the loans page.
Base automatically changed from DDFBRA-183-anonymous-and-logged-in-user-can-try-ebook to reader-player-feature December 13, 2024 10:48
@kasperbirch1 kasperbirch1 merged commit cfb17f0 into reader-player-feature Dec 13, 2024
20 checks passed
@kasperbirch1 kasperbirch1 deleted the DDFBRA-236-logged-in-user-can-go-from-digital-loans-to-material-to-listen-read branch December 13, 2024 10:54
@kasperbirch1 kasperbirch1 mentioned this pull request Dec 13, 2024
kasperbirch1 added a commit that referenced this pull request Dec 17, 2024
…logged-in-user-can-go-from-digital-loans-to-material-to-listen-read

DDFBRA-236 - Logged in user can go from digital loans to material
kasperbirch1 added a commit that referenced this pull request Jan 7, 2025
…logged-in-user-can-go-from-digital-loans-to-material-to-listen-read

DDFBRA-236 - Logged in user can go from digital loans to material
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.

2 participants