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

UIMARCAUTH-438 Do not load MARC source until consortia data is available. #428

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

BogdanDenis
Copy link
Contributor

Description

Do not load MARC source until consortia data is available.
The issue was caused by a delay when loading consortia data. If an authority record is loaded before consortia data is available then we fetch MARC source from a member tenant, which is incorrect for shared records

Screenshots

chrome_rBhys1sGq4.mp4

Issues

UIMARCAUTH-438

Copy link

github-actions bot commented Dec 3, 2024

Jest Unit Test Results

  1 files  ±0   24 suites  ±0   2m 28s ⏱️ +17s
164 tests +2  164 ✅ +2  0 💤 ±0  0 ❌ ±0 
165 runs  +2  165 ✅ +2  0 💤 ±0  0 ❌ ±0 

Results for commit dbd0a15. ± Comparison against base commit 216e2d6.

♻️ This comment has been updated with latest results.

@BogdanDenis BogdanDenis requested review from Dmytro-Melnyshyn and a team December 4, 2024 11:59
@BogdanDenis BogdanDenis requested a review from a team December 5, 2024 14:34
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

Code changes LGTM; consider clearer names for functions and additional documentation.

@@ -26,6 +26,22 @@ import {

import { AuthorityView } from '../../views';

const checkCanLoadMarcSource = (isConsortiaEnv, isShared, isConsortiumDataLoaded, isAuthorityLoaded) => {
Copy link
Member

Choose a reason for hiding this comment

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

It returns a boolean; just call it canLoadMarcSource. Functions named checkFoo or validateFoo should return null/undefined on success and throw an error on failure. Functions named like isValidFoo or canLoadFoo should return true on success and false on failure.

Consider adding the description from the PR as a header comment for this function to answer why these are the relevant conditions to evaluate when deciding if it is possible to load MARC source.

const marcSource = useMarcSource({ recordId: id, tenantId, enabled: Boolean(selectedAuthority) }, {

const isConsortiaEnv = stripes.hasInterface('consortium');
const isShared = selectedAuthority?.shared;
Copy link
Member

Choose a reason for hiding this comment

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

Consider !!(selectedAuthority?.shared) or Boolean(selectedAuthority?.shared) to convert the undefined value returned by optional chaining to boolean false. Related, selectedAuthority.shared should really be ...isShared, but I don't know if you're in charge of that property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you mean selectedAuthority.isShared then we can't do it because this property comes from a BE response

Copy link

sonarcloud bot commented Dec 6, 2024

@BogdanDenis BogdanDenis merged commit 65c17ab into master Dec 6, 2024
15 checks passed
@BogdanDenis BogdanDenis deleted the UIMARCAUTH-438 branch December 6, 2024 10:34
Dmytro-Melnyshyn pushed a commit that referenced this pull request Dec 13, 2024
…ble. (#428)

* UIMARCAUTH-438 Do not load MARC source until consortia data is available.

* UIMARCAUTH-438 added comments to a new function
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.

3 participants