Skip to content

fix(libraries): prevent adding documents to read-only libraries#845

Merged
thostetler merged 3 commits intoadsabs:masterfrom
thostetler:fix/scix-864-read-only-library-permissions
Apr 15, 2026
Merged

fix(libraries): prevent adding documents to read-only libraries#845
thostetler merged 3 commits intoadsabs:masterfrom
thostetler:fix/scix-864-read-only-library-permissions

Conversation

@thostetler
Copy link
Copy Markdown
Member

@thostetler thostetler commented Apr 13, 2026

  • Read-only libraries are visually disabled (greyed out, aria-disabled) in the Add to Library modal and cannot be selected — closes SCIX-864
  • Switched mutatemutateAsync so Promise.all actually awaits the API call; previously mutate returned void, causing Promise.all to resolve immediately and always show the success banner regardless of the 403 response
  • MSW mock handler now returns 403 when attempting to add/remove documents on a library with permission === 'read'
  • permission column is now visible in the modal table so users can see why some rows are disabled
  • Added disabledIds prop to LibraryListTable for general-purpose row disabling
  • Add tests
image

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 38.88889% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.9%. Comparing base (9b8f252) to head (7029529).

Files with missing lines Patch % Lines
src/components/Libraries/LibraryListTable.tsx 37.0% 63 Missing ⚠️
src/components/Libraries/AddToLibraryModal.tsx 62.5% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #845     +/-   ##
========================================
+ Coverage    62.5%   62.9%   +0.5%     
========================================
  Files         317     323      +6     
  Lines       36576   37917   +1341     
  Branches     1673    1731     +58     
========================================
+ Hits        22830   23830   +1000     
- Misses      13706   14047    +341     
  Partials       40      40             
Files with missing lines Coverage Δ
src/components/Libraries/AddToLibraryModal.tsx 85.4% <62.5%> (ø)
src/components/Libraries/LibraryListTable.tsx 64.5% <37.0%> (ø)

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thostetler thostetler marked this pull request as ready for review April 13, 2026 23:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the “Add to Library” flow so read-only libraries can’t be selected/modified, and aligns UI + mocks + tests with the expected 403 behavior.

Changes:

  • Disable read-only libraries in the Add to Library modal (UI affordances + aria-disabled) and show the permission column for clarity.
  • Fix async behavior by switching to mutateAsync so batch operations actually await API results before showing success.
  • Update MSW handlers and add modal tests covering read-only selection and 403 error handling.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/mocks/handlers/libraries.ts Return 403 when attempting add/remove docs on permission === 'read' libraries.
src/components/Libraries/AddToLibraryModal.tsx Use mutateAsync; compute/read-only IDs; pass disabledIds; show permission column.
src/components/Libraries/LibraryListTable.tsx Add disabledIds prop and apply row-level disabling semantics/styling.
src/components/Libraries/__tests__/AddToLibraryModal.test.tsx Add tests for read-only disabling and 403 behavior.

Comment thread src/components/Libraries/LibraryListTable.tsx Outdated
Comment thread src/components/Libraries/LibraryListTable.tsx Outdated
Comment thread src/components/Libraries/__tests__/AddToLibraryModal.test.tsx Outdated
- Read-only libraries are now visually disabled in the AddToLibraryModal
  table and cannot be selected
- Switch from mutate to mutateAsync so Promise.all correctly waits for
  the API response; previously it resolved immediately (void), always
  triggering the success banner regardless of the 403 response
- Mock handler returns 403 for any add/remove attempt on a read-only
  library
- Add tests covering selection guard and the regression case
- Remove unconditional inline style from Tr that overrode conditional
  backgroundColor/color props, causing disabled rows to appear highlighted
- Fix malformed aria-label template string on selectable Checkbox
- Replace flaky setTimeout in test with waitFor on submit button loading state
@thostetler thostetler force-pushed the fix/scix-864-read-only-library-permissions branch from 314c5ea to 7029529 Compare April 13, 2026 23:53
Copy link
Copy Markdown
Member

@shinyichen shinyichen left a comment

Choose a reason for hiding this comment

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

👍

@thostetler thostetler merged commit d71e9d5 into adsabs:master Apr 15, 2026
5 of 6 checks passed
@thostetler thostetler deleted the fix/scix-864-read-only-library-permissions branch April 15, 2026 17:41
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