Skip to content

Show short names and link library associations (PP-5014) - #344

Open
tdilauro wants to merge 3 commits into
mainfrom
feature/show-library-short-name
Open

Show short names and link library associations (PP-5014)#344
tdilauro wants to merge 3 commits into
mainfrom
feature/show-library-short-name

Conversation

@tdilauro

Copy link
Copy Markdown
Contributor

Description

Show each library's short name alongside its display name, in the format <name> - <short-name>, wherever libraries are listed in the admin UI:

  • Associated library rows in integration edit forms (which now also link to the library's configuration page)
  • The associated-library disclosure panels on the integrations, collections, discovery services, and admins lists
  • The top-level Libraries list

The new links in edit forms open in a new tab, so an accidental click won't navigate away from a form with unsaved edits.

Motivation and Context

  • Having the short name handy in displays is helpful when doing various configuration tasks.
  • Having a link to an associated library makes it easier to navigate to that library's configuration for additional information or update.

[Jira PP-5014]

How Has This Been Tested?

  • Manual testing in local dev environment.
  • New and updated to tests to cover the new funtionality.
  • All tests pass locally and in CI.

Checklist:

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Claude finished @tdilauro's task in 4m 12s —— View job


Code review

  • Gather context (diff, changed files)
  • Review source changes
  • Review test changes
  • Post review

Summary

The refactor is a clean win: libraryLabel / libraryConfigHref collapse four hand-rolled fallback chains into one place, LibraryRegistration.name no longer emits …/edit/undefined when a library has no uuid, and ServiceEditForm now renders something for a library missing from allLibraries instead of nothing. Test coverage tracks the behavior change well, including the no-uuid and unknown-library paths. My findings are all about coverage and consistency of the new label, not correctness. Separately, several unrelated re-formats in DiscoveryServices.tsx, Libraries.tsx, and LibraryRegistration.tsx produce >80-column interface … extends … lines that the repo's Prettier 3.8.4 would not emit — worth re-running npx prettier --write on the changed files so the next contributor's pre-commit hook doesn't churn them back.

Details

Minor: src/components/ServiceEditForm.tsx:463

The "Add Library" select in this same form still labels options with library.name alone, so the associated-library rows immediately above now read "New York Public Library - nypl" while the picker below reads "New York Public Library" — and two similarly-named libraries remain indistinguishable at exactly the moment the admin has to choose between them. {libraryLabel(library.name, library.short_name)} here would make the form self-consistent.

Minor: src/components/IndividualAdminEditForm.tsx:194

The per-library role table in the admin edit form still shows bare library.name, even though the IndividualAdmins list that links into it now shows "Alpha Library - alpha - Manager". Since the checkbox aria-labels in the same rows already key off library.short_name, the visible column is the one place the short name is missing:

<td>{libraryLabel(library.name, library.short_name)}</td>

Minor: src/components/ServiceEditForm.tsx:614-631

This anchor — href/target/rel/title/aria-label, the (opens in a new tab) suffix, and the fall-back-to-plain-text branch — is byte-for-byte the same as LibraryRegistration.name (src/components/LibraryRegistration.tsx:127-147). Extracting a LibraryConfigLink component that takes {name, short_name, uuid} and returns either the link or the label would keep the new-tab policy and the accessible-name wording in one place, and would give the third call site (EditableConfigList.renderAssociatedSection, which deliberately stays same-tab) something explicit to differ from.

renderLibraryLabel(shortName: string): JSX.Element | string {
const library = this.getLibrary(shortName);
const label = libraryLabel(library?.name, shortName);
const href = libraryConfigHref(library?.uuid);
return href ? (
<a
href={href}
target="_blank"
rel="noopener noreferrer"
title={`${label} (opens in a new tab)`}
aria-label={`${label} (opens in a new tab)`}
>
{label}
</a>
) : (
label
);
}

Minor: src/components/DiscoveryServices.tsx:102-103

The label and the suffix now use the same " - " separator, so a registered library renders as Alpha Library - alpha - registered - production — four segments with no visual cue for which is the short name (IndividualAdmins has the same shape: Alpha Library - alpha - Manager). When the entry is linked the anchor boundary disambiguates it, but the no-uuid rows are plain text end to end. Parenthesizing the short name — libraryLabel emitting Alpha Library (alpha) — or wrapping the suffix in a styled <span> would keep these readable.

label: libraryLabel(meta?.name ?? lib.name, lib.short_name),
suffix: lib.stage ? ` - registered - ${lib.stage}` : " - registered",

Nit: src/components/ServiceEditForm.tsx:623

title duplicates the aria-label verbatim, so hovering any associated library pops a tooltip that just repeats the link text already on screen. Dropping title and keeping aria-label gives the same accessible name without the redundant tooltip (same at src/components/LibraryRegistration.tsx:137).

title={`${label} (opens in a new tab)`}

feature/show-library-short-name

@tdilauro
tdilauro requested a review from a team August 21, 2026 14:00
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown

Greptile Summary

The PR displays each library's short name alongside its display name and adds configuration-page links where UUIDs are available.

  • Centralizes library label and configuration URL generation in shared helpers.
  • Updates library lists, association disclosures, and service edit forms.
  • Adds and updates component and utility test coverage.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
src/utils/sharedFunctions.ts Adds shared helpers for library labels and optional configuration URLs, including fallbacks for missing fields.
src/components/ServiceEditForm.tsx Displays associated libraries using the shared label helper and opens available configuration links in a new tab.
src/components/LibraryRegistration.tsx Updates registration rows to show combined library labels and optional configuration links.
src/components/Libraries.tsx Updates top-level library rows to include short names while retaining UUID fallback behavior.
src/components/DiscoveryServices.tsx Updates registered-library disclosures to use shared labels and configuration URLs.

Reviews (2): Last reviewed commit: "AI CI code review feedback" | Re-trigger Greptile

Comment thread src/components/ServiceEditForm.tsx Outdated
Comment thread src/components/DiscoveryServices.tsx
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.24%. Comparing base (4f789a8) to head (30293ef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/components/DiscoveryServices.tsx 50.00% 0 Missing and 2 partials ⚠️
src/components/IndividualAdmins.tsx 71.42% 0 Missing and 2 partials ⚠️
src/components/Libraries.tsx 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #344      +/-   ##
==========================================
- Coverage   91.25%   91.24%   -0.01%     
==========================================
  Files         205      205              
  Lines        7606     7622      +16     
  Branches     1663     1667       +4     
==========================================
+ Hits         6941     6955      +14     
- Misses        366      367       +1     
- Partials      299      300       +1     
Flag Coverage Δ
jest 91.24% <83.78%> (-0.01%) ⬇️

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 tdilauro changed the title Feature/show library short name Show short names and link library associations (PP-5014) Aug 21, 2026

@PoorBillyPilgrim PoorBillyPilgrim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 🚀

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