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

Availability bug #1348

Conversation

spaceo
Copy link
Contributor

@spaceo spaceo commented Jul 26, 2024

Link to issue

https://reload.atlassian.net/browse/DDFLSBP-722

Description

The main problem was that because the availability hook can be run twice with the same parameters eg on the material page where the availability labels are presented. The former version of useAvailabilityData could not handle that scenario.

  • Three hooks are introduced instead of one: One main hook and one for
    physical and one for online materials, to get a better overview of
    the rules and the logic.
  • Apparently we cannot make use of onSuccess handlers on queries if
    the request have alredy been performed and the data is delivered the
    second time by the QueryClient cache. And that is the case of the
    availability labels on a material page, where they are used both in the
    header and in the "udgaver" section later on the page.

@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch 3 times, most recently from 0a363ab to 56ab856 Compare July 31, 2024 11:52
@spaceo spaceo marked this pull request as ready for review July 31, 2024 11:52
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch from 56ab856 to 4b4fa49 Compare July 31, 2024 12:01
@spaceo spaceo assigned kasperbirch1 and unassigned kasperbirch1 Jul 31, 2024
spaceo added 2 commits July 31, 2024 15:00
 Two major factors to be aware of here:

 * Three hooks are introduced instead of one: One main hook and one for
 physical and one for online materials, to get a better overview of
 the rules and the logic.
 * Apparently we cannot make use of `onSuccess` handlers on queries if
 the request have alredy been performed and the data is delivered the
 second time by the `QueryClient` cache. And that is the case of the
 availability labels on a material page, where they are used both in the
 header and in the "udagver" section later on the page.
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch from 4b4fa49 to 4280787 Compare July 31, 2024 13:00
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch 3 times, most recently from 5d6484e to 56fc307 Compare August 2, 2024 08:24
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.

I have two comments that I think you should consider before moving on with this :)
Otherwise I can't see any more potential problems :) Fingers crossed that it all works as they intended, since DDF has soooo much business logic in regard to availability 🤯
Luckily, someone from DDF (probably Tue) will be reviewing this too.

tsconfig.json Outdated Show resolved Hide resolved
src/tests/unit/availability.test.tsx Show resolved Hide resolved
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch 6 times, most recently from b4c7275 to f49c496 Compare August 2, 2024 13:52
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch from f49c496 to 9ed98b6 Compare August 3, 2024 16:46
Since we have multiple article types we should consider those in the
various places where we have functionality relying on those types.
@spaceo spaceo force-pushed the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch from 9ed98b6 to db378df Compare August 3, 2024 18:13
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.

Love it ❤️

@spaceo spaceo merged commit a51c07b into develop Aug 6, 2024
20 checks passed
@spaceo spaceo deleted the DDFLSBP-722-availability-labels-viser-udlant-under-udgaver-skont-bogen-er-hjemme branch August 6, 2024 09:11
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.

4 participants