Drop customlist_sharedlibraries (PP-4773)

[jonathangreen]

Description

Drops the customlist_sharedlibraries table and removes its Table definition from the model.

Do not merge until a release containing #3694 (the first PR in this stack) has shipped. Until then the Backwards compatibility test on this branch is expected to fail: it tests against the last published release, which still reads the table.

The drop deliberately does not re-run the previous PR's backfill. That release stopped writing to the table entirely, including when a list is unshared, so any list unshared since then still has its rows; repeating the backfill would silently re-share every one of them. The deploy-window race it would otherwise catch is minutes long and self-correcting.

The downgrade recreates and repopulates the table rather than leaving it out. The preceding revision's upgrade reads it, so pytest-alembic's test_up_down_consistency walks back through a schema where it has to exist.

Motivation and Context

PP-4773, release 2 of the online-migration split described in CLAUDE.md. Sharing state moved to customlists.shared_locally in the previous release, which left the table unused but still in Base.metadata so the release before it kept working while that migration ran.

How Has This Been Tested?

• tox -e py312-docker -- tests/migration — 19 passed, including up/down consistency across the drop and recreate.
• tox -e py312-docker -- tests/manager/sqlalchemy tests/manager/api/admin — 1128 passed.
• mypy clean.
• ./docker/ci/test_backwards_compatibility.sh 149127422 run locally with this branch as the current side, proving the previous PR genuinely stopped using the table. CI cannot check this until PR 1 has been released, which is why it was verified locally first.

Checklist

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

[claude]

Claude finished @jonathangreen's task in 3m 59s —— View job

---

Code review

• Gather context (read diff, migration, model change)
• Check for remaining references to the dropped table
• Review migration up/down correctness
• Post review

---

Summary

The drop is clean and the release-2 mechanics check out: no reference to customlist_sharedlibraries survives anywhere outside the two migrations and the share_customlist helper in tests/migration/conftest.py (which only runs below revision 912c566f3383, where the table still exists); the recreated DDL in downgrade matches what create_all built column-for-column, including the ix_customlist_sharedlibraries_* index names and the unnamed unique constraint (Base.metadata has no naming convention, so alembic and Postgres agree on the auto-generated name); and the recreate really is load-bearing — test_20260901_912c566f3383_add_shared_locally_to_customlists.py does migrate_down_to(REVISION) then migrate_down_one() and inserts into the join table, which only works because this revision's downgrade puts it back. Two small things below. On the automated findings above: the cross-join's breadth isn't fixable here (the per-library subset was already collapsed into a boolean by 912c566f3383, whose own downgrade restores nothing), and # revision identifiers, used by Alembic. is the standard template header carried by every migration in alembic/versions/ — neither needs a change.

Details

Minor: alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py:61-70

No companion test file was added for this revision, so the INSERT ... SELECT is the only piece of logic in this stack with no assertion behind it — test_up_down_consistency proves it doesn't raise, and the 912c566f3383 test only depends on the table existing, not on its contents. CLAUDE.md asks for a companion test for new code, and a rollback backfill is exactly the code you want covered before you need it. A tests/migration/test_20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py in the shape of its sibling would do it:

alembic_runner.migrate_down_to("5b1f4f3c7979")
owner_id, other_id = alembic_database.library(), alembic_database.library()
shared_id = alembic_database.customlist(library_id=owner_id)  # + UPDATE shared_locally = true
unshared_id = alembic_database.customlist(library_id=owner_id)
alembic_runner.migrate_down_one()
# exactly one row: (shared_id, other_id) -- not the owner, and nothing for unshared_id

circulation/alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py

Lines 61 to 70 in ef8bd5f

	op.execute(
	sa.text(
	"""
	INSERT INTO customlist_sharedlibraries (customlist_id, library_id)
	SELECT c.id, l.id
	FROM customlists c CROSS JOIN libraries l
	WHERE c.shared_locally AND c.library_id IS DISTINCT FROM l.id
	"""
	)
	)

Nit: alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py:20-26

"Deliberately not re-run here" has no antecedent in this comment — the backfill it refers to lives in the previous revision and is never named, so a reader hitting this file cold has to go find 912c566f3383 to work out what isn't being re-run. Naming it fixes the paragraph: # The previous revision's backfill of shared_locally is deliberately not re-run here: ....

circulation/alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py

Lines 20 to 26 in ef8bd5f

	# Sharing moved to customlists.shared_locally one release ago, so no running
	# code (current or previous release) reads or writes this table.
	#
	# Deliberately not re-run here: the previous release stopped writing to this
	# table entirely, including when a list is unshared. Any list unshared since
	# that release still has its rows, so repeating the backfill would silently
	# re-share every one of them.

• chore/drop-customlist-sharedlibraries

[greptile-apps]

Greptile Summary

The PR removes the legacy custom-list sharing association table after sharing state moved to customlists.shared_locally.

• Adds an Alembic upgrade that drops customlist_sharedlibraries.
• Adds downgrade logic that recreates its constraints, indexes, and association rows.
• Removes the legacy table from SQLAlchemy metadata.

Confidence Score: 4/5

The PR is not yet safe to merge because rollback can expose shared lists to libraries that lacked the corresponding historical association.

The downgrade still reconstructs every shared list’s legacy rows using an unconditional cross join, although the preceding migration discarded which individual libraries were historically authorized.

Files Needing Attention: alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py

Important Files Changed

Filename	Overview
alembic/versions/20260901_5b1f4f3c7979_drop_customlist_sharedlibraries.py	Drops the obsolete table on upgrade, but its downgrade still broadens historical per-library sharing when reconstructing associations.
src/palace/manager/sqlalchemy/model/customlist.py	Removes the obsolete association-table definition from SQLAlchemy metadata consistently with the upgrade migration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Current schema] -->|upgrade| B[Drop customlist_sharedlibraries]
  B --> C[Sharing represented by customlists.shared_locally]
  C -->|downgrade| D[Recreate legacy association table]
  D --> E[Populate associations for every non-owning library]

Loading

_{Reviews (2): Last reviewed commit: "Drop customlist_sharedlibraries" | Re-trigger Greptile}

[greptile-apps]

Downgrade broadens list sharing

When a shared list was historically restricted because one or more libraries failed the former per-library validation, this cross join recreates associations for every non-owning library, causing the downgraded application to expose the list or a thin or empty lane to libraries that were previously excluded.

Knowledge Base Used:

• Domain data and persistence
• Database and schema infrastructure

[greptile-apps]

Autogenerated comment remains

The generated Alembic revision-identifiers comment remains in the committed migration, adding template noise contrary to the repository’s migration convention.

Suggested change

# revision identifiers, used by Alembic.

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!