Skip to content

clear collection titles before updating#280

Open
elfkuzco wants to merge 1 commit into
mainfrom
fix-update-title
Open

clear collection titles before updating#280
elfkuzco wants to merge 1 commit into
mainfrom
fix-update-title

Conversation

@elfkuzco
Copy link
Copy Markdown
Contributor

@elfkuzco elfkuzco commented May 12, 2026

Rationale

This PR clears the collection titles before creating the new ones to be used for an update. By clearing the old collection titles in memory, we avoid creating duplicate collection titles when the original collection title is sent in the update reqeust. This is because SQLAlchemy list of collection titles isn't cleared with just session.delete.
As a consequence, having duplicate collection titles leads to creating duplicate book locations which violates location unique constraint.

Changes

  • clear collection titles before adding new ones
  • update tests to model real-world scenario such that title to update has book, collection title, etc.
  • remove redundant second test since updated test covers everything

This fixes #277

@elfkuzco elfkuzco self-assigned this May 12, 2026
@elfkuzco elfkuzco requested a review from benoit74 May 12, 2026 09:39
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.37%. Comparing base (6408a8e) to head (7f495bd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #280      +/-   ##
==========================================
+ Coverage   81.28%   81.37%   +0.09%     
==========================================
  Files          55       55              
  Lines        2362     2363       +1     
  Branches      236      236              
==========================================
+ Hits         1920     1923       +3     
+ Misses        381      380       -1     
+ Partials       61       60       -1     

☔ View full report in Codecov by Sentry.
📢 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.

@elfkuzco
Copy link
Copy Markdown
Contributor Author

@benoit74 , I am wondering if this is not a good time to add a unique constraint to the collectiontitle table too. The keys for the constraint will be title_id, collection_id, and path. WDYT?

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.

Impossible to add a second collection

1 participant