Conversation
Co-authored-by: Artic0din <9603319+Artic0din@users.noreply.github.com>
…s schema Co-authored-by: Artic0din <9603319+Artic0din@users.noreply.github.com>
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a stable library_id (media server section ID) as part of media-item identity across backend, database schema/migrations, and frontend typing/deduping so that library renames no longer orphan saved/ignored data or collapse cross-library copies.
Changes:
- Add
library_idto backend/TSMediaItem, and populate it in Plex + Emby/Jellyfin scanners. - Update SQLite schema + queries to key identity on
(tmdb_id, library_id, edition)and add a v6→v7 migration to backfill existing rows. - Update routing/handlers and frontend dedupe logic to use
library_idwhile keepinglibrary_titlefor display/backward compatibility.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/types/media-and-posters/media-item-and-library.ts | Add library_id to the frontend MediaItem interface. |
| frontend/src/lib/stores/global-store-library-sections.ts | Include library_id in frontend media-item dedupe key. |
| backend/routing/database/update.go | Use library_id when deleting poster sets from a media item. |
| backend/routing/database/resolve_library_id.go | New helpers to resolve stable library_id from query params/title. |
| backend/routing/database/ignore.go | Accept/resolve library_id for ignore + stop-ignore routes. |
| backend/routing/database/delete.go | Accept/resolve library_id for delete route. |
| backend/routing/database/add.go | Ensure library_id is resolved/populated before DB upsert. |
| backend/models/media_item.go | Add LibraryID field to backend MediaItem JSON model. |
| backend/mediaserver/plex/get_media_item_details.go | Populate/use LibraryID for DB existence checks and on-server updates. |
| backend/mediaserver/plex/get_library_section_items.go | Populate/use LibraryID during section scans and DB checks. |
| backend/mediaserver/handle_temp_ignored_items.go | Use library_id when stopping ignore for temp-ignored items. |
| backend/mediaserver/ej/get_media_item_details.go | Use LibraryID for DB checks and on-server updates. |
| backend/mediaserver/ej/get_library_section_items.go | Populate/use LibraryID; propagate through boxset split path. |
| backend/mediaserver/check_for_media_item_changes.go | Use library_id when deleting/updating DB rows for missing cache items. |
| backend/database/sqlite-update-media-item.go | Update MediaItems by (tmdb_id, library_id, edition). |
| backend/database/sqlite_upsert_saved_item.go | Upsert MediaItems/SavedItems using library_id identity and FK. |
| backend/database/sqlite_update_on_server.go | Update on_server by (tmdb_id, library_id, edition). |
| backend/database/sqlite_ignore_stop.go | Stop ignoring by (tmdb_id, library_id, edition). |
| backend/database/sqlite_ignore_item.go | Store/query ignored items with library_id; propagate into cached items. |
| backend/database/sqlite_get_saved_sets_count.go | Count unique saved sets by library_id. |
| backend/database/sqlite_get_saved_sets_all.go | Include library_id in saved-sets query joins and JSON. |
| backend/database/sqlite_get_media_items.go | Include library_id in MediaItems queries and flags join keys. |
| backend/database/sqlite_delete.go | Delete/unlink poster sets using library_id keys. |
| backend/database/sqlite_create_tables.go | New-install schema updates for library_id and new constraints/indexes. |
| backend/database/sqlite_check_media_item.go | Existence checks keyed by (tmdb_id, library_id, edition). |
| backend/database/migration/sqlite_migration_v6_v7.go | Add v6→v7 migration rebuilding tables + backfilling library_id. |
| backend/database/migration/migrate.go | Register v6→v7 migration path. |
| backend/database/db.go | Bump DB version to 7; update DB interface signatures to libraryID. |
Suppressed comments (2)
backend/database/sqlite_ignore_item.go:73
- IgnoreMediaItem trims tmdb_id/library_id/edition but no longer trims library_title, so the DB can end up storing titles with leading/trailing whitespace (and returning inconsistent display values).
tmdbID = strings.TrimSpace(tmdbID)
libraryID = strings.TrimSpace(libraryID)
edition = strings.TrimSpace(edition)
mode = strings.ToLower(strings.TrimSpace(mode))
backend/routing/database/ignore.go:116
- StopIgnoringItemInDB now accepts
library_id(and falls back tolibrary_title), but the swagger docs still marklibrary_titleas required and omitlibrary_id.
// Get query parameters
tmdbID := r.URL.Query().Get("tmdb_id")
libraryID := resolveLibraryID(r)
libraryTitle := r.URL.Query().Get("library_title")
edition := r.URL.Query().Get("edition")
if tmdbID == "" || libraryID == "" {
logAction.SetError("Missing required query parameters", "TMDB ID and Library ID (or Title) are required",
map[string]any{
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| logAction.AppendResult("complete", req.Complete) | ||
|
|
||
| for _, ps := range req.UpdateItem.PosterSets { | ||
| if ps.ToDelete { | ||
| // Delete the poster set | ||
| Err := database.DeletePosterSetForMediaItem(ctx, req.UpdateItem.MediaItem.TMDB_ID, req.UpdateItem.MediaItem.LibraryTitle, req.UpdateItem.MediaItem.Edition, ps.ID) | ||
| Err := database.DeletePosterSetForMediaItem(ctx, req.UpdateItem.MediaItem.TMDB_ID, req.UpdateItem.MediaItem.LibraryID, req.UpdateItem.MediaItem.Edition, ps.ID) | ||
| if Err.Message != "" { |
| if _, err := conn.ExecContext(ctx, | ||
| `UPDATE IgnoredItems SET library_id = ? WHERE library_id = ?`, | ||
| lib.ID, lib.Title, | ||
| ); err != nil { |
| // Get the query parameters | ||
| tmdbID := r.URL.Query().Get("tmdb_id") | ||
| libraryTitle := r.URL.Query().Get("library_title") | ||
| libraryID := resolveLibraryID(r) | ||
| edition := r.URL.Query().Get("edition") | ||
|
|
||
| // Validate the parameters | ||
| if tmdbID == "" || libraryTitle == "" { | ||
| if tmdbID == "" || libraryID == "" { |
| // Get query parameters | ||
| tmdbID := r.URL.Query().Get("tmdb_id") | ||
| libraryID := resolveLibraryID(r) | ||
| libraryTitle := r.URL.Query().Get("library_title") | ||
| edition := r.URL.Query().Get("edition") | ||
| mode := r.URL.Query().Get("mode") // e.g., "always", "until-set-available", "until-new-set-available" | ||
| currentSets := r.URL.Query().Get("current_sets") // comma-separated list of current sets for the item, used for temporary ignore modes | ||
|
|
||
| if tmdbID == "" || libraryTitle == "" || mode == "" { | ||
| logAction.SetError("Missing required query parameters", "TMDB ID, Library Title, and Mode are required", | ||
| if tmdbID == "" || libraryID == "" || mode == "" { |
| Msg("Temp ignored item not found in cache") | ||
| continue | ||
| } | ||
| cachedItem.LibraryID = libraryID | ||
| cachedItem.IgnoredMode = mode | ||
| cachedItem.IgnoredSets = strings.Split(currentSets, ",") |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f030ba9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| action = "This item will be removed from the database since it is set to be ignored temporarily" | ||
| moreInfo = "This may indicate that the Media Item was removed or there is an issue with the media server. Please double check if this item exists. If it does exist and you want to keep it as ignored temporarily, please ignore it again." | ||
| database.DeleteMediaItemAndIgnoredStatus(ctx, dbItem.TMDB_ID, dbItem.LibraryTitle, dbItem.Edition) | ||
| database.DeleteMediaItemAndIgnoredStatus(ctx, dbItem.TMDB_ID, dbItem.LibraryID, dbItem.Edition) |
There was a problem hiding this comment.
When a media-server library is renamed, the database retains the old library_title, so the title-based cache lookup on line 31 reports the item missing even though dbItem.LibraryID still matches the scanned item. This sends false not-found notifications, marks saved items off-server, and this branch deletes temporarily ignored entries; locate the cache section by library_id before taking these destructive actions.
Useful? React with 👍 / 👎.
|
|
||
| export interface MediaItem { | ||
| tmdb_id: string; | ||
| library_id: string; |
There was a problem hiding this comment.
The frontend now receives library_id but its delete and ignore services still send only library_title. After a library rename, a saved row can retain the old display title, causing resolveLibraryID to fall back to that title and silently target no row; duplicate configured titles can instead resolve to the first library and mutate the wrong item. Pass this stable ID through the delete, ignore, and stop-ignore requests.
Useful? React with 👍 / 👎.
| if ps.ToDelete { | ||
| // Delete the poster set | ||
| Err := database.DeletePosterSetForMediaItem(ctx, req.UpdateItem.MediaItem.TMDB_ID, req.UpdateItem.MediaItem.LibraryTitle, req.UpdateItem.MediaItem.Edition, ps.ID) | ||
| Err := database.DeletePosterSetForMediaItem(ctx, req.UpdateItem.MediaItem.TMDB_ID, req.UpdateItem.MediaItem.LibraryID, req.UpdateItem.MediaItem.Edition, ps.ID) |
There was a problem hiding this comment.
For pre-v7 clients whose update payload contains only library_title, LibraryID decodes as empty here. A deletion then affects zero rows while the endpoint returns success, whereas a non-delete update can upsert a duplicate identity under library_id = ''; resolve the ID from the title after decoding, as the add handler already does.
Useful? React with 👍 / 👎.
| // Rebuild the index that query paths use for fast library_id look-ups. | ||
| if _, err := conn.ExecContext(ctx, `DROP INDEX IF EXISTS idx_mediaitems_library_title`); err != nil { | ||
| logAction.SetError("Failed to drop old library_title index on MediaItems", "", map[string]any{"error": err.Error()}) | ||
| return *logAction.Error | ||
| } | ||
| if _, err := conn.ExecContext(ctx, `CREATE INDEX IF NOT EXISTS idx_mediaitems_library_id ON MediaItems (library_id)`); err != nil { |
There was a problem hiding this comment.
On every v6 database, renaming and dropping SavedItems_old and IgnoredItems_old also drops idx_saveditems_poster_set_id, idx_saveditems_item, and idx_ignoreditems_mode, but this migration recreates only a new MediaItems index. Upgraded installations therefore lose indexes that fresh v7 databases still create, forcing full scans for poster-set reference checks and temporary-ignore queries; recreate the affected SavedItems and IgnoredItems indexes before completing the migration.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| // --- MediaItems --- | ||
| if _, err = tx.ExecContext(ctx, `ALTER TABLE MediaItems RENAME TO MediaItems_old;`); err != nil { |
There was a problem hiding this comment.
SQLite rewrites the existing Movies and Series foreign keys to reference MediaItems_old when this table is renamed, but the migration subsequently drops that table without rebuilding either dependent table. Every migrated schema is therefore left with foreign keys targeting a nonexistent table; if foreign-key enforcement is enabled, later child writes fail and media-item cascades cannot work, while enforcement during the migration can cascade-delete the child rows when MediaItems_old is dropped. Rebuild the dependent tables or use a migration sequence that preserves their references to MediaItems.
Useful? React with 👍 / 👎.
| if _, err := conn.ExecContext(ctx, | ||
| `UPDATE MediaItems SET library_id = ? WHERE library_title = ? AND library_id = library_title`, | ||
| lib.ID, lib.Title, |
There was a problem hiding this comment.
When upgrading from v1, migrate_1_to_2 calls the now-v7 database.CreateTables, but media items reconstructed directly from legacy movie JSON set only LibraryTitle, so the updated upsert stores library_id = ''. The later edition migration skips its rebuild because the current table already has edition, this migration skips its rebuild because library_id exists, and this predicate excludes those empty IDs; their saved sets consequently remain unreachable by scanners using the real section ID, and equal TMDB IDs from different libraries can collide under the empty key. Populate LibraryID during legacy conversion or include empty IDs in this backfill.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| Err := database.IgnoreMediaItem(ctx, tmdbID, libraryTitle, edition, mode, currentSets) | ||
| Err := database.IgnoreMediaItem(ctx, tmdbID, libraryID, libraryTitle, edition, mode, currentSets) |
There was a problem hiding this comment.
A caller may now supply library_id without library_title, because the validation explicitly accepts either, but this call then inserts an empty display title into IgnoredItems. Temporary-ignore processing later locates the cached item by the stored library_title, so these rows are never processed or automatically cleared; resolve the configured title from the ID before storing the ignore row, or continue requiring both values.
Useful? React with 👍 / 👎.
Library titles are mutable display names — renaming a library currently creates a new media identity and orphans saved/ignored data. This PR introduces a stable
library_id(the media server's section ID) as the true identity key, while retaininglibrary_titlefor display and backward-compatible API responses.Data model
models.MediaItemgainsLibraryID string(library_idin JSON), populated beforeLibraryTitleMediaIteminterface gainslibrary_id: string${tmdb_id}|${library_id}|${edition}— two copies of the same TMDB item in different libraries now remain distinctDatabase schema (new installations)
MediaItems,SavedItems,IgnoredItemsall gain alibrary_id TEXT NOT NULL DEFAULT ''column. Identity constraints change from(tmdb_id, library_title, edition)→(tmdb_id, library_id, edition):Migration (v6 → v7)
Recreates all three tables with the new schema. Backfill strategy: seed
library_id = library_titlefor all existing rows, then run a per-configured-libraryUPDATEto replace the title value with the real section ID where a match exists. Rows with no configured match keep the title as ID rather than being orphaned.Scanners
Plex sets
item.LibraryID = strconv.Itoa(metadata.LibrarySectionID)in both the section scan and the single-item details path. Emby/Jellyfin setsitem.LibraryID = section.IDduring the section scan.Routing (backward-compatible)
New
resolveLibraryID(r)helper: readslibrary_idquery param first, falls back to looking uplibrary_titleagainst the configured library list. Old clients sending onlylibrary_titlecontinue to work.ignore,delete,add, andupdatehandlers updated accordingly.IgnoreMediaItemnow also accepts and storeslibraryTitleso the display column is populated on newly ignored items.