Wait & show spinner when associated libraries still loading (PP-5030)

[tdilauro]

Description

• The Libraries panel on integration edit forms now waits for the sitewide library list, showing an accessible "Loading libraries..." status line, then renders once with the final linked labels.
• A shared utility now handles the "loading" vs. "settled" distinction and guards the fetch. The list is requested only when not already loaded or in flight, and a failed request is retried.
• If the list fails to load, the panel shows an alert instead of degrading silently, and the failure stays visible while a retry runs.
• The pre-auth setup (when we're adding a new PM instance) page no longer issues a libraries request that cannot succeed.

Note:

• Known follow-ups: Header, QuicksightDashboard, and CustomLists still read the libraries slice directly; migrating them to settledAllLibraries / fetchLibrariesIfNeeded is deferred for future work.
• Scope creep control for AI reviews: findings that do not show data loss or a broken guarantee may be reported once as suggested future work, but will not be added to this branch.

Motivation and Context

The library list loads separately from the integration data and, on large sites, may arrive noticeably later. The Libraries section rendered bare short names first, then visibly rewrote itself with linked labels.

[Jira PP-5030]

How Has This Been Tested?

• Manual testing in local dev environment using dev-server against a PM CM with many libraries.
• New and updated tests cover the new behavior.
• CI tests pass.

Checklist:

• N/A - I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @tdilauro's task in 8m 51s —— View job

---

Code review

• Gather context (diff vs origin/main, CLAUDE.md)
• Review src/utils/allLibraries.ts and the libraries reducer wrapper
• Review components (LibrariesLoadStatus, LibrariesRefreshWarning, ServiceEditForm, IndividualAdminEditForm, EditableConfigList, LibraryRegistration, the six config lists)
• Review tests
• Post review

---

Summary

This holds up well. I traced the reducer's state machine through every ordering I could construct — first failure → retry → SUCCESS → LOAD, overlapping header/tab requests, a FAILURE landing beside current data, EDIT_LIBRARY_* clearing fetchError out from under a pending list failure — and settledAllLibraries reports the right one of loading / clean / blocking-error / stale-with-refresh-error in each. The data-loss paths are clean too: IndividualAdminEditForm.handleData serializes state.admin.roles (seeded from item.roles), and ServiceEditForm.handleData serializes state.libraries (seeded from item.libraries), so hiding the roles table or the Libraries panel never drops what a submit posts, and the per-library toggles whose un-check branches rebuild roles from the list are the exact set locked under allLibrariesRefreshError. Two open bot findings look incorrect to me and I'd dismiss both: react-bootstrap 0.32.4's Alert does emit role="alert" — ErrorMessage renders a bare <Alert> and tests/jest/components/ErrorMessage.test.tsx:11 finds it with getByRole("alert") — so the "Failures Are Not Announced" premise doesn't hold; and LibrariesLoadStatus already names what it produces (a role="status" line), which is what the naming convention is after. I could not run npm test or npm run lint here — node_modules is absent and the install was not permitted — so this is static review only; CI and Codecov (100% patch coverage) cover that.

Details

Minor: src/components/EditableConfigList.tsx:114-118

usesLibraryList has to be kept in sync by hand with two other per-page edits — settledAllLibraries(state) in mapStateToProps and fetchLibrariesIfNeeded(dispatch, actions) in mapDispatchToProps — and nothing ties the three together. A page that sets the flag but misses the dispatch wiring renders a permanent "Loading libraries…" live region and permanently suppresses its association toggles, since getAssociatedEntries returns undefined for as long as allLibraries is undefined; a page that merges the data but forgets the flag shows the failure alert and refresh warning with no status line, because those two are not gated on it. Neither drift produces a type error or a test failure. Six pages now repeat the identical triple, so a single helper applied at connect time (merging the state slice and wrapping fetchData) would remove the flag entirely and make the coupling unforgettable. Given the scope note in the PR description, reporting this once as suggested future work rather than as something to add here.

circulation-admin/src/components/EditableConfigList.tsx

Lines 114 to 118 in 13cfa8e

	adminLevel?: number;
	limitOne = false;
	/** True on lists whose data merges settledAllLibraries; gates the
	* library-list status line in list mode. */
	usesLibraryList = false;

---

· branch chore/associated-libraries-load-lazily

[greptile-apps]

Confidence Score: 4/5

The functional changes appear safe, but the explicit component-naming requirement remains unsatisfied before merge.

Findings

1.  Failures Are Not Announced ▶
2.  Component Name Hides DOM Role ▶

Summary

The PR coordinates loading of the sitewide library list across configuration pages so association labels do not render in an intermediate state.

• Adds shared helpers for distinguishing loading, loaded, failed, and stale library-list states and for avoiding duplicate requests.
• Retains stale library data during refreshes and exposes initial-load and refresh failures separately.
• Adds loading, empty-state, and failure feedback to service and administrator forms and lists.
• Protects administrator roles and service associations from unsafe edits when complete library data is unavailable.
• Skips the unavailable libraries request during initial administrator setup.
• Adds focused component, reducer, utility, and connected-wiring coverage.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Configuration page mounts] --> B{Library list available or in flight?}
  B -- Loaded --> C[Render final library labels and links]
  B -- In flight --> D[Show Loading libraries status]
  B -- Missing or failed --> E[Fetch or retry library list]
  E --> F{Request result}
  F -- Success --> C
  F -- Initial failure --> G[Show failure alert and restrict unsafe edits]
  F -- Refresh failure --> H[Retain stale list and show warning]
  H --> I[Allow only edits that do not rebuild data from the stale list]

Loading

_{Reviews (26) · Last reviewed commit: "CI AI code review feedback"}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.82%. Comparing base (c77e8ca) to head (13cfa8e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #352      +/-   ##
==========================================
+ Coverage   91.50%   91.82%   +0.31%     
==========================================
  Files         206      209       +3     
  Lines        7593     7716     +123     
  Branches     1662     1696      +34     
==========================================
+ Hits         6948     7085     +137     
+ Misses        357      353       -4     
+ Partials      288      278      -10     

Flag	Coverage Δ
jest	91.82% <100.00%> (+0.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[tdilauro]

Minor: src/utils/allLibraries.ts:47-56

fetchLibrariesIfNeeded only runs from each list's componentDidMount, and TabContainer mounts every config tab once per ConfigPage mount (inactive tabs are hidden with display: none, not unmounted), so switching tabs never re-runs it — and Header likewise requests the list only when it mounts. After a failed /admin/libraries request the new alert is therefore a dead end for as long as the admin stays on the configuration page: the only recovery is navigating off /admin/web/config and back, or reloading. A "Try again" button in the alert that dispatches fetchLibraries() would make the failure the PR now surfaces actually recoverable.

circulation-admin/src/utils/allLibraries.ts

Lines 47 to 56 in 619422d

	export function fetchLibrariesIfNeeded(dispatch, actions: ActionCreator): void {
	dispatch((thunkDispatch, getState) => {
	const libraries = getState().editor.libraries;
	if (
	!libraries?.isFetching &&
	(!libraries?.isLoaded || libraries?.fetchError)
	) {
	thunkDispatch(actions.fetchLibraries()).catch(() => {});
	}
	});

That is not quite true, but even if it were, this change is out of proportion to its value. It would be an absolute mess to do. If an issue like this ever becomes a problem in real life, there are easier ways to handle this.

[greptile-apps]

Failures Are Not Announced

When either library request fails, this component clears the persistent status region because it assumes the adjacent Alert has role="alert". This repository uses react-bootstrap 0.32.4, whose Alert renders a plain div unless a role is explicitly supplied. As a result, screen-reader users are not notified when loading transitions to a failure or refresh failure. The same unsupported assumption appears in the alerts rendered by LibrariesRefreshWarning, IndividualAdminEditForm, and ServiceEditForm.

[greptile-apps]

Component Name Hides DOM Role

LibrariesLoadStatus directly renders a <p>, but its name does not identify that DOM role. This violates the repository directive that component names should reflect the element they produce, so the requirement must be satisfied before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[tdilauro]

Re: Claude feedback...

Minor: src/components/IndividualAdminEditForm.tsx:189-191
While the list is loading (allLibraries undefined), the whole roles table is withheld — including the manager-all / librarian-all header checkboxes, which are not list-dependent. That contradicts the reasoning stated in isDisabled ("The sitewide toggles ... stay assignable, so the form cannot be reduced to submitting a roleless admin"): on a fresh load of /admin/web/config/individualAdmins/create, the admins fetch typically settles before the slow libraries fetch, so there is a window where Submit is enabled and "System Admin" is the only role control on screen; submitting there creates an admin with roles: []. Hoisting the two sitewide checkboxes (or the table's ) outside the gate would make the loading path match the stale-list path, where they already stay enabled.

circulation-admin/src/components/IndividualAdminEditForm.tsx

Lines 189 to 191 in 1df9580

	{allLibraries &&
	!(this.props.item && allLibrariesError) &&
	this.renderRolesTable(allLibraries, allLibrariesError)}

Not going to do this. This would perpetuate the same kind of "has everything loaded yet" confusion I opened this PR to fix. We have the loading state from the libraries, so I think it's better to just display everything once the libraries have arrived.

[PoorBillyPilgrim]

LGTM 🚀