UIQM-716: Consolidate bib and authority routes to avoid page refresh after redirecting from the create/derive page to the edit one.

[Dmytro-Melnyshyn]

Purpose

Add a "Save & keep editing" button to the bibliographic and authority record creation and deriving pages. After hitting the button, redirect to the editing page without refreshing it and retain the focus.

Approach

• consolidate bibliographic and authority routes to avoid refreshing the page;
• delete QuickMarcCreateWrapper.js, QuickMarcEdiitWrapper.js and QuickMarcDeriveWrapper.js. Their functionality has been carried over to a new useSaveRecord hook. They use the same UI that should be used directly in QuickMarcEditorContainer to avoid remounting fields after redirecting to the edit page, which prevents having the same ref for focusing;
• add a "Save & keep editing" button for bibliographic and authority records, update page data as soon as the record is created/derived, and then redirect to the edit page.

Issues

UIQM-716

TODOS and Open Questions

Screencasts

2024-11-12_10h34_17.mp4

Learning

Pre-Merge Checklist

Before merging this PR, please go through the following list and take appropriate actions.

• I've added appropriate record to the CHANGELOG.md
• Does this PR meet or exceed the expected quality standards?
  • Code coverage on new code is 80% or greater
  • Duplications on new code is 3% or less
  • There are no major code smells or security issues
• Does this introduce breaking changes?
  • If any API-related changes - okapi interfaces and permissions are reviewed/changed correspondingly
  • There are no breaking changes in this PR.

If there are breaking changes, please STOP and consider the following:

• What other modules will these changes impact?
• Do JIRAs exist to update the impacted modules?
  • If not, please create them
  • Do they contain the appropriate level of detail? Which endpoints/schemas changed, etc.
  • Do they have all they appropriate links to blocked/related issues?
• Are the JIRAs under active development?
  • If not, contact the project's PO and make sure they're aware of the urgency.
• Do PRs exist for these changes?
  • If so, have they been approved?

Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.

While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.

[github-actions]

Jest Unit Test Results

  1 files  ±0   46 suites   - 2   2m 30s ⏱️ -15s
479 tests  - 7  479 ✅  - 7  0 💤 ±0  0 ❌ ±0 
482 runs   - 7  482 ✅  - 7  0 💤 ±0  0 ❌ ±0 

Results for commit 0faccc5. ± Comparison against base commit 311d033.

This pull request removes 34 and adds 27 tests. Note that renamed tests count towards both.

Given Quick Marc When visiting "duplicate" route should display correct route ‑ Given Quick Marc When visiting "duplicate" route should display correct route
Given QuickMarcCreateWrapper should render with no axe errors ‑ Given QuickMarcCreateWrapper should render with no axe errors
Given QuickMarcCreateWrapper when click on cancel pane button should handle onClose action ‑ Given QuickMarcCreateWrapper when click on cancel pane button should handle onClose action
Given QuickMarcCreateWrapper when click on save button should actualize links ‑ Given QuickMarcCreateWrapper when click on save button should actualize links
Given QuickMarcCreateWrapper when click on save button should create authority record with correct payload and call onSave ‑ Given QuickMarcCreateWrapper when click on save button should create authority record with correct payload and call onSave
Given QuickMarcCreateWrapper when click on save button should create bib record with correct payload ‑ Given QuickMarcCreateWrapper when click on save button should create bib record with correct payload
Given QuickMarcCreateWrapper when click on save button should show on save message and redirect on load page ‑ Given QuickMarcCreateWrapper when click on save button should show on save message and redirect on load page
Given QuickMarcCreateWrapper when click on save button when click on save button in a bib record should wait for links saving and redirection in onSubmit function ‑ Given QuickMarcCreateWrapper when click on save button when click on save button in a bib record should wait for links saving and redirection in onSubmit function
Given QuickMarcCreateWrapper when click on save button when click on save button in a holding record should wait for redirection in onSubmit function ‑ Given QuickMarcCreateWrapper when click on save button when click on save button in a holding record should wait for redirection in onSubmit function
Given QuickMarcCreateWrapper when click on save button when click on save button in an authority record should wait for redirection in onSubmit function ‑ Given QuickMarcCreateWrapper when click on save button when click on save button in an authority record should wait for redirection in onSubmit function
…

Given Quick Marc When visiting "derive" route should display correct route ‑ Given Quick Marc When visiting "derive" route should display correct route
useSaveRecord when creating should actualize links ‑ useSaveRecord when creating should actualize links
useSaveRecord when creating should create authority record with correct payload and call onSave ‑ useSaveRecord when creating should create authority record with correct payload and call onSave
useSaveRecord when creating should create bib record with correct payload ‑ useSaveRecord when creating should create bib record with correct payload
useSaveRecord when creating when hitting save&close should call onSave with `externalId` ‑ useSaveRecord when creating when hitting save&close should call onSave with `externalId`
useSaveRecord when creating when hitting save&continue should call refreshPageData ‑ useSaveRecord when creating when hitting save&continue should call refreshPageData
useSaveRecord when creating when hitting save&continue should redirect to the edit page ‑ useSaveRecord when creating when hitting save&continue should redirect to the edit page
useSaveRecord when creating when marc type is not a bib should not call actualizeLinks ‑ useSaveRecord when creating when marc type is not a bib should not call actualizeLinks
useSaveRecord when creating when there is a linked field should call the saveLinksToNewRecord ‑ useSaveRecord when creating when there is a linked field should call the saveLinksToNewRecord
useSaveRecord when creating when there is a validation error should stop submitting ‑ useSaveRecord when creating when there is a validation error should stop submitting
…

♻️ This comment has been updated with latest results.

[BogdanDenis]

Hey @Dmytro-Melnyshyn, regarding this point:

delete QuickMarcCreateWrapper.js, QuickMarcEdiitWrapper.js and QuickMarcDeriveWrapper.js. Their functionality has been carried over to a new useSaveRecord hook. They use the same UI that should be used directly in QuickMarcEditorContainer to avoid remounting fields after redirecting to the edit page, which prevents having the same ref for focusing;

Can we save a reference to a field name, instead of the field input itself? Field name will stay constant between renders and mounts/unmounts and we won't need to many changes.
Plus, I'd like too keep separate files for Create, Edit and Derive functionality. Right now we only have slightly different logic to save the records, but in future there could be other differences. With separate files we can add some code to one of them, but in a suggested approach we'd need to create another hook with if conditions inside and that can get difficult to maintain

[Dmytro-Melnyshyn]

Can we save a reference to a field name, instead of the field input itself? Field name will stay constant between renders and mounts/unmounts and we won't need to many changes. Plus, I'd like too keep separate files for Create, Edit and Derive functionality. Right now we only have slightly different logic to save the records, but in future there could be other differences. With separate files we can add some code to one of them, but in a suggested approach we'd need to create another hook with if conditions inside and that can get difficult to maintain

Hey @BogdanDenis, I choose to delete files for the following reasons:

1. UI flickering. Here is the video recording. The flickering does not look good and can be requested (maybe not now) to be fixed, which would require moving the functionality to hooks in place of components.
2. Loss of focus, since the timing of mounting and updating components is different, as can also be seen in the video.

I can also create one hook that will have three hooks, each responsible for different actions (Create, Edit, Derive), like we have in deleted files.

[BogdanDenis]

@Dmytro-Melnyshyn ok let's continue with your approach.
Just having separate hooks for Create, Edit and Derive won't be enough, because they'd have to be still called in one place and then conditionally pass the results to <QuickMarcEditor>.
Instead maybe we could pass these hooks as props, just like the wrappers
Or alternatively, use a component factory to pass a hook like a regular function argument
Here's an article with some details and examples https://careers.wolt.com/en/blog/engineering/injecting-hooks-into-react-components , "Passing Hooks in Props" section
So, three separate hooks useMarcCreate, useMarcEdit and useMarcDerive and pass one of them as props or via component factory approach to <QuickMarcEditorContainer>. Then <QuickMarcEditorContainer> will call a hook and pass the results to <QuickMarcEditor>

[Dmytro-Melnyshyn]

@BogdanDenis, it doesn't work, React detects the order of hooks and crashes the page. The useMarcEdit contains useMarcRecordMutation, but useMarcCreate doesn't (React doesn't crash the page if the only difference is in hooks like history, useParams, ...).

I get the following error:

StripesContext.js:11 Warning: React has detected a change in the order of Hooks called by QuickMarcEditorContainer. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://reactjs.org/link/rules-of-hooks

[Terala-Priyanka]

is this comment intentional?

[Dmytro-Melnyshyn]

This permission was commented in the previous implementation, and I decided to leave it too.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
91.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud