From a26b2de454ae942dd6dfd16ab267fd59090c2201 Mon Sep 17 00:00:00 2001 From: fluxis Date: Tue, 7 Jul 2026 09:42:41 -0400 Subject: [PATCH] fix(metadata): stop specialist providers hijacking new library chains (#294) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(metadata): seed specialist providers off and scope chains to declared levels New library provider chains were seeded from every enabled metadata provider, ordered purely by each plugin's declared default_priority and enabled whenever that priority was > 0. Two consequences: - A specialist provider (e.g. silo.sportarr, which declares series/season/ episode) could out-rank the general providers and land at position 1, enabled, on every new TV series library. - Single-purpose providers that declare only their own level (audiobook / ebook / manga metadata) were still attached as disabled rows to series and movie libraries, cluttering the chain editor with providers that cannot serve that content. Introduce a `default_enabled` capability-metadata flag (defaults to true, so every existing plugin is unaffected). A provider sets it false to be seeded installed-but-disabled while keeping its declared priority, so a user can opt in per-library and it slots in where the manifest intends instead of jumping to the top. At the same time, seedDefaultChain and AppendProviderToAllChains now drop providers that do not declare a content level, reusing the same providerSupportsLevel rule as the chain-less fallback (issue #106). LookupSeedPlacement resolves support/priority/enabled with a single metadata fetch. buildSeededChainEntries is extracted as a pure, unit-tested helper. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): standardize metadata provider slug casing in library chain editor The library provider-chain editor showed the same provider differently depending on where the chain came from: a freshly defaulted chain used the capability display name ("TMDB"), while a chain loaded from the server used the capability id ("tmdb", which the API returns as provider_slug). So a provider read one way before saving and another after, and differed between library types depending on which levels already had a saved chain. Standardize on the capability id everywhere (matches the server's provider_slug and the mono/slug styling). Extract the provider mapping into a pure, unit-tested metadataProvidersFromInstallations helper. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(web): mirror server seeding rules in the library form's default chain The form builds its own default chain client-side, and any touch (including changing the library type on create, the normal path for a series library) marks it dirty and POSTs it after create — replacing the server-seeded chain. That chain still enabled every provider with a declared priority and listed unsupported providers as disabled rows, so the server-side fix evaporated on the UI create path. buildDefaultLevelChains now applies the same rules as buildSeededChainEntries: providers that don't declare the level are dropped, a declaring provider is enabled only if it doesn't opt out via default_enabled, and a legacy catch-all (no declared levels) is parked last, disabled. Co-Authored-By: Claude Fable 5 * refactor(web,api): serve default provider chains from the server Replace the form's client-side reimplementation of the seeding rules with a new additive endpoint, GET /api/v1/libraries/provider-defaults?library_type=X, which returns the exact chain seedDefaultChain would write for that type. The create form now renders those server-computed defaults, changing the library type just refetches them (no longer marking the chain dirty), and a create with an untouched chain lets the server-seeded chain stand instead of writing one back. Editing an existing library uses the same defaults to fill levels its saved chain doesn't cover. Types the server seeds no metadata levels for (e.g. podcasts) return an empty levels map rather than an error. This removes buildDefaultLevelChains / metadataProvidersFromInstallations and the default_priority/default_enabled manifest parsing from the frontend — one source of truth for default ordering and enablement. Co-Authored-By: Claude Fable 5 * feat(web): show a loading state in the provider chain editor While the server chain (for an existing library) or the type's defaults are still in flight, the editor rendered empty provider lists for a moment. Show a spinner row instead; local edits always render immediately. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com> --- internal/api/handlers/libraries.go | 139 +++++++++++----- .../libraries_provider_defaults_test.go | 49 ++++++ .../api/handlers/libraries_seed_chain_test.go | 100 ++++++++++++ internal/api/handlers/plugins.go | 4 +- internal/api/router.go | 1 + internal/metadata/chain.go | 80 +++++++++- internal/metadata/chain_test.go | 36 +++++ .../admin/libraries/LibraryForm.test.tsx | 128 ++++++++++++++- .../admin/libraries/LibraryFormSections.tsx | 8 +- .../admin/libraries/useLibraryForm.ts | 148 ++++++++---------- web/src/hooks/queries/admin/libraries.ts | 15 ++ web/src/hooks/queries/keys.ts | 2 + 12 files changed, 575 insertions(+), 135 deletions(-) create mode 100644 internal/api/handlers/libraries_provider_defaults_test.go create mode 100644 internal/api/handlers/libraries_seed_chain_test.go diff --git a/internal/api/handlers/libraries.go b/internal/api/handlers/libraries.go index 65dd81c6..e6e8d985 100644 --- a/internal/api/handlers/libraries.go +++ b/internal/api/handlers/libraries.go @@ -2005,6 +2005,44 @@ func (h *LibraryHandler) HandleGetLibraryProviders(w http.ResponseWriter, r *htt writeJSON(w, http.StatusOK, map[string]any{"levels": levels}) } +// HandleGetLibraryProviderDefaults handles GET /libraries/provider-defaults. +// It returns the provider chain that would be seeded for a new library of the +// given type, grouped by content level and in seeded order. The admin UI +// renders this while creating a library instead of re-deriving the chain from +// plugin manifests client-side, so the displayed defaults and the chain the +// server seeds on create can never disagree. +func (h *LibraryHandler) HandleGetLibraryProviderDefaults(w http.ResponseWriter, r *http.Request) { + libraryType := r.URL.Query().Get("library_type") + levels := metadataContentLevelsForLibraryType(libraryType) + if len(levels) == 0 { + // A type the server doesn't seed chains for (unknown, or one like + // podcasts with no metadata content levels) simply has no defaults. + writeJSON(w, http.StatusOK, map[string]any{"levels": map[string][]chainLevelEntry{}}) + return + } + + if h.ChainRepo == nil { + writeError(w, http.StatusServiceUnavailable, "unavailable", "Provider chain management is not configured") + return + } + + out := make(map[string][]chainLevelEntry, len(levels)) + for _, level := range levels { + out[level] = []chainLevelEntry{} + } + for _, e := range h.seedDefaultChain(r.Context(), libraryType) { + out[e.ContentLevel] = append(out[e.ContentLevel], chainLevelEntry{ + PluginInstallationID: e.PluginInstallationID, + CapabilityID: e.CapabilityID, + ProviderSlug: e.CapabilityID, + Priority: e.Priority, + Enabled: e.Enabled, + }) + } + + writeJSON(w, http.StatusOK, map[string]any{"levels": out}) +} + // HandleSetLibraryProviders handles PUT /libraries/{id}/providers. // It replaces the entire provider chain for the given library. func (h *LibraryHandler) HandleSetLibraryProviders(w http.ResponseWriter, r *http.Request) { @@ -2082,50 +2120,73 @@ func (h *LibraryHandler) seedDefaultChain(ctx context.Context, libraryType strin var entries []metadata.ChainEntry for _, level := range levels { - type candidate struct { - installationID int - capabilityID string - priority int - enabled bool - } - var candidates []candidate - + candidates := make([]seedCandidate, 0, len(caps)) for _, c := range caps { - defaultPriority := metadata.LookupDefaultPriority(ctx, h.ChainRepo.Pool(), c.PluginInstallationID, c.CapabilityID, level) - if defaultPriority > 0 { - candidates = append(candidates, candidate{ - installationID: c.PluginInstallationID, - capabilityID: c.CapabilityID, - priority: defaultPriority, - enabled: true, - }) - } else { - // Plugin doesn't declare this level — include but disabled. - candidates = append(candidates, candidate{ - installationID: c.PluginInstallationID, - capabilityID: c.CapabilityID, - priority: 999, - enabled: false, - }) - } - } - - sort.Slice(candidates, func(i, j int) bool { - return candidates[i].priority < candidates[j].priority - }) - - for i, cand := range candidates { - entries = append(entries, metadata.ChainEntry{ - PluginInstallationID: cand.installationID, - CapabilityID: cand.capabilityID, - CapabilityType: "metadata_provider.v1", - ContentLevel: level, - Priority: i, - Enabled: cand.enabled, + p := metadata.LookupSeedPlacement(ctx, h.ChainRepo.Pool(), c.PluginInstallationID, c.CapabilityID, level) + candidates = append(candidates, seedCandidate{ + installationID: c.PluginInstallationID, + capabilityID: c.CapabilityID, + supportsLevel: p.SupportsLevel, + declaredPriority: p.DefaultPriority, + defaultEnabled: p.DefaultEnabled, }) } + entries = append(entries, buildSeededChainEntries(level, candidates)...) + } + + return entries +} + +// seedCandidate is a metadata provider under consideration for a freshly seeded +// chain at one content level, carrying the manifest-declared values that decide +// its placement. +type seedCandidate struct { + installationID int + capabilityID string + supportsLevel bool // provider handles this content level (declared it, or is a legacy catch-all) + declaredPriority int // manifest default_priority for this level; 0 = level not declared + defaultEnabled bool // manifest default_enabled; false = specialist opts out of auto-enable +} + +// buildSeededChainEntries orders the providers for one content level and assigns +// positional priorities. Providers that do not handle this level are dropped +// outright — a single-purpose provider (e.g. audiobook/ebook/manga metadata) +// never clutters a library type it cannot serve. Of the remaining providers, one +// that declares this level (declaredPriority>0) is placed by that priority and +// seeded enabled unless it opted out via default_enabled; a legacy provider that +// declares no levels at all is parked last and disabled. Keeping an opted-out +// provider at its declared priority (rather than forcing it last) means that when +// a user does enable it, it slots in where the manifest intends instead of +// jumping to the top of the chain. +func buildSeededChainEntries(level string, candidates []seedCandidate) []metadata.ChainEntry { + ranked := make([]seedCandidate, 0, len(candidates)) + for _, c := range candidates { + if !c.supportsLevel { + continue + } + if c.declaredPriority > 0 { + ranked = append(ranked, c) + } else { + // Legacy provider that declares no levels — park last, disabled. + ranked = append(ranked, seedCandidate{installationID: c.installationID, capabilityID: c.capabilityID, supportsLevel: true, declaredPriority: 999, defaultEnabled: false}) + } } + sort.SliceStable(ranked, func(i, j int) bool { + return ranked[i].declaredPriority < ranked[j].declaredPriority + }) + + entries := make([]metadata.ChainEntry, len(ranked)) + for i, c := range ranked { + entries[i] = metadata.ChainEntry{ + PluginInstallationID: c.installationID, + CapabilityID: c.capabilityID, + CapabilityType: "metadata_provider.v1", + ContentLevel: level, + Priority: i, + Enabled: c.declaredPriority > 0 && c.defaultEnabled, + } + } return entries } diff --git a/internal/api/handlers/libraries_provider_defaults_test.go b/internal/api/handlers/libraries_provider_defaults_test.go new file mode 100644 index 00000000..4ad011e8 --- /dev/null +++ b/internal/api/handlers/libraries_provider_defaults_test.go @@ -0,0 +1,49 @@ +package handlers + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// A library type the server seeds no metadata levels for (unknown types, or +// ones like podcasts without metadata content levels) has no defaults — the +// endpoint answers with an empty levels map rather than an error, so the UI +// can treat "no defaults" and "defaults" uniformly. +func TestHandleGetLibraryProviderDefaults_NoLevelsForType(t *testing.T) { + h := &LibraryHandler{} + + for _, libraryType := range []string{"podcasts", "bogus", ""} { + req := httptest.NewRequest(http.MethodGet, "/libraries/provider-defaults?library_type="+libraryType, nil) + rec := httptest.NewRecorder() + h.HandleGetLibraryProviderDefaults(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("type %q: expected 200, got %d", libraryType, rec.Code) + } + var body struct { + Levels map[string][]chainLevelEntry `json:"levels"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("type %q: decoding body: %v", libraryType, err) + } + if len(body.Levels) != 0 { + t.Errorf("type %q: expected empty levels, got %v", libraryType, body.Levels) + } + } +} + +// A seedable type still needs the chain repository; without one the endpoint +// reports unavailable like the other provider-chain handlers. +func TestHandleGetLibraryProviderDefaults_NoChainRepo(t *testing.T) { + h := &LibraryHandler{} + + req := httptest.NewRequest(http.MethodGet, "/libraries/provider-defaults?library_type=series", nil) + rec := httptest.NewRecorder() + h.HandleGetLibraryProviderDefaults(rec, req) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("expected 503, got %d", rec.Code) + } +} diff --git a/internal/api/handlers/libraries_seed_chain_test.go b/internal/api/handlers/libraries_seed_chain_test.go new file mode 100644 index 00000000..f051c765 --- /dev/null +++ b/internal/api/handlers/libraries_seed_chain_test.go @@ -0,0 +1,100 @@ +package handlers + +import ( + "testing" +) + +// A specialist provider that opts out of default-enable (default_enabled=false) +// must be seeded installed-but-off and ranked by its declared priority — never +// jumping ahead of the general providers just because a user might enable it. +// A provider that does not declare the content level at all must be excluded +// entirely, not shown as a disabled row. Together these are the regressions that +// let silo.sportarr land at position 1 enabled, and let the audiobook/ebook/manga +// providers clutter every TV series and movie library's provider chain. +func TestBuildSeededChainEntries_OptOutAndLevelScoping(t *testing.T) { + candidates := []seedCandidate{ + {installationID: 1, capabilityID: "tmdb", supportsLevel: true, declaredPriority: 3, defaultEnabled: true}, + {installationID: 2, capabilityID: "tvdb", supportsLevel: true, declaredPriority: 2, defaultEnabled: true}, + {installationID: 22, capabilityID: "sportarr", supportsLevel: true, declaredPriority: 50, defaultEnabled: false}, + // Declares only {audiobook} — does not support the series level, so it + // must not appear in a series library's chain at all. + {installationID: 3, capabilityID: "audiobook-metadata", supportsLevel: false, declaredPriority: 0, defaultEnabled: true}, + {installationID: 8, capabilityID: "ebook-metadata", supportsLevel: false, declaredPriority: 0, defaultEnabled: true}, + } + + entries := buildSeededChainEntries("series", candidates) + + if len(entries) != 3 { + t.Fatalf("expected 3 entries (unsupported providers excluded), got %d", len(entries)) + } + + // Ordering: general providers by declared priority (tvdb 2, tmdb 3), then + // the opted-out specialist by its declared priority (50). + type want struct { + installationID int + enabled bool + } + wants := []want{ + {2, true}, // tvdb + {1, true}, // tmdb + {22, false}, // sportarr — declared but opted out + } + for i, w := range wants { + e := entries[i] + if e.PluginInstallationID != w.installationID { + t.Errorf("position %d: got installation %d, want %d", i, e.PluginInstallationID, w.installationID) + } + if e.Enabled != w.enabled { + t.Errorf("position %d (installation %d): enabled=%v, want %v", i, e.PluginInstallationID, e.Enabled, w.enabled) + } + if e.Priority != i { + t.Errorf("position %d: got priority %d, want %d", i, e.Priority, i) + } + if e.ContentLevel != "series" { + t.Errorf("position %d: got content level %q, want series", i, e.ContentLevel) + } + if e.CapabilityType != "metadata_provider.v1" { + t.Errorf("position %d: got capability type %q", i, e.CapabilityType) + } + } + + // The audiobook/ebook providers must be entirely absent. + for _, e := range entries { + if e.PluginInstallationID == 3 || e.PluginInstallationID == 8 { + t.Errorf("provider %d does not declare series and must not be seeded", e.PluginInstallationID) + } + } +} + +// A provider with default_enabled=true (the default) that declares the level is +// seeded enabled, unchanged from the pre-flag behavior. +func TestBuildSeededChainEntries_DefaultsEnabled(t *testing.T) { + entries := buildSeededChainEntries("movie", []seedCandidate{ + {installationID: 1, capabilityID: "tmdb", supportsLevel: true, declaredPriority: 2, defaultEnabled: true}, + }) + if len(entries) != 1 { + t.Fatalf("expected 1 entry, got %d", len(entries)) + } + if !entries[0].Enabled { + t.Errorf("provider with default_enabled=true should be seeded enabled") + } +} + +// A legacy provider that declares no default_priority map at all makes no claim +// and stays eligible for every level (parked last, disabled) — preserving the +// pre-existing catch-all behavior for providers that never enumerated levels. +func TestBuildSeededChainEntries_LegacyCatchAllParkedLast(t *testing.T) { + entries := buildSeededChainEntries("series", []seedCandidate{ + {installationID: 1, capabilityID: "tmdb", supportsLevel: true, declaredPriority: 3, defaultEnabled: true}, + {installationID: 9, capabilityID: "legacy", supportsLevel: true, declaredPriority: 0, defaultEnabled: true}, + }) + if len(entries) != 2 { + t.Fatalf("expected 2 entries, got %d", len(entries)) + } + if entries[0].PluginInstallationID != 1 || !entries[0].Enabled { + t.Errorf("tmdb should be first and enabled, got %+v", entries[0]) + } + if entries[1].PluginInstallationID != 9 || entries[1].Enabled { + t.Errorf("legacy catch-all should be parked last and disabled, got %+v", entries[1]) + } +} diff --git a/internal/api/handlers/plugins.go b/internal/api/handlers/plugins.go index e6dc79fe..47c749f1 100644 --- a/internal/api/handlers/plugins.go +++ b/internal/api/handlers/plugins.go @@ -690,8 +690,8 @@ func (h *PluginHandler) syncMetadataProviders(ctx context.Context, installation if cap.Type != "metadata_provider.v1" { continue } - if err := h.chainRepo.AppendProviderToAllChains(ctx, installation.ID, cap.ID, func(level string) int { - return metadata.LookupDefaultPriority(ctx, h.chainRepo.Pool(), installation.ID, cap.ID, level) + if err := h.chainRepo.AppendProviderToAllChains(ctx, installation.ID, cap.ID, func(level string) metadata.SeedPlacement { + return metadata.LookupSeedPlacement(ctx, h.chainRepo.Pool(), installation.ID, cap.ID, level) }); err != nil { slog.Warn("failed to append provider to library chains", "installation_id", installation.ID, diff --git a/internal/api/router.go b/internal/api/router.go index c5ba8588..969ed2f6 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -1851,6 +1851,7 @@ func NewRouter(deps Dependencies) chi.Router { r.Post("/{id}/metadata-match-queue/retry", libraryHandler.HandleRetryMetadataMatchQueue) r.Post("/{id}/metadata-match-queue/cancel", libraryHandler.HandleCancelMetadataMatchQueue) r.Post("/{id}/refresh-metadata", libraryHandler.HandleRefreshLibraryMetadata) + r.Get("/provider-defaults", libraryHandler.HandleGetLibraryProviderDefaults) r.Get("/{id}/providers", libraryHandler.HandleGetLibraryProviders) r.Put("/{id}/providers", libraryHandler.HandleSetLibraryProviders) r.Put("/{id}/poster", libraryHandler.HandleUploadPoster) diff --git a/internal/metadata/chain.go b/internal/metadata/chain.go index fc437daa..79e4cf2d 100644 --- a/internal/metadata/chain.go +++ b/internal/metadata/chain.go @@ -175,14 +175,15 @@ func (r *ChainRepository) DeleteChain(ctx context.Context, folderID int) error { } // AppendProviderToAllChains adds a provider to every existing library chain -// (per content level) that doesn't already include it. The defaultPriority -// callback returns the plugin's declared priority for a content level (0 means -// the plugin doesn't declare that level — the entry is still added but disabled). +// (per content level) that it serves and doesn't already include it. The +// placement callback resolves the plugin's manifest intent for a content level: +// levels it does not support are skipped entirely, and it is appended enabled +// only when it declares the level and opts into default_enabled. func (r *ChainRepository) AppendProviderToAllChains( ctx context.Context, pluginInstallationID int, capabilityID string, - defaultPriority func(contentLevel string) int, + placement func(contentLevel string) SeedPlacement, ) error { // Find every distinct (folder, level) pair that has chain entries. rows, err := r.pool.Query(ctx, @@ -210,6 +211,15 @@ func (r *ChainRepository) AppendProviderToAllChains( } for _, g := range groups { + // Only attach the provider to levels it actually serves. A single-purpose + // provider (e.g. audiobook/ebook/manga metadata, or a sports provider that + // only declares series levels) must not clutter every library's chain with + // a disabled row for content it cannot handle. + p := placement(g.contentLevel) + if !p.SupportsLevel { + continue + } + // Check if the provider is already in this chain. var exists bool err := r.pool.QueryRow(ctx, @@ -238,8 +248,11 @@ func (r *ChainRepository) AppendProviderToAllChains( return fmt.Errorf("getting max priority: %w", err) } - dp := defaultPriority(g.contentLevel) - enabled := dp > 0 + // A provider joins an existing library disabled unless it declares this + // level and opts into being enabled by default. This keeps a specialist + // (default_enabled=false) one click away instead of silently taking over + // established libraries the moment it is installed. + enabled := p.DefaultPriority > 0 && p.DefaultEnabled _, err = r.pool.Exec(ctx, `INSERT INTO library_provider_chains (media_folder_id, plugin_installation_id, capability_id, capability_type, content_level, priority, enabled) @@ -442,6 +455,61 @@ func extractDefaultPriority(metadataJSON []byte, contentLevel string) int { return 0 } +// extractDefaultEnabled reports whether a provider should be enabled by default +// when a chain is first seeded for a new library. It defaults to true when the +// flag is absent or unparseable, so every plugin predating this flag keeps its +// original seeded-enabled behavior. A specialist provider (e.g. a sports +// metadata source that should not compete with general providers on every +// library) sets metadata.default_enabled=false to be seeded installed-but-off, +// leaving it one click away for users who want it. The flag lives in the same +// "metadata" envelope as default_priority. +func extractDefaultEnabled(metadataJSON []byte) bool { + var meta map[string]json.RawMessage + if err := json.Unmarshal(metadataJSON, &meta); err != nil { + return true + } + raw, ok := meta["default_enabled"] + if !ok { + if innerRaw, innerOK := meta["metadata"]; innerOK { + var inner map[string]json.RawMessage + if err := json.Unmarshal(innerRaw, &inner); err == nil { + raw, ok = inner["default_enabled"] + } + } + } + if !ok { + return true + } + var enabled bool + if err := json.Unmarshal(raw, &enabled); err != nil { + return true + } + return enabled +} + +// SeedPlacement captures how a provider's manifest wants it placed when a chain +// is first seeded for a content level: whether it handles the level at all, its +// declared priority, and whether it should be enabled by default. +type SeedPlacement struct { + SupportsLevel bool + DefaultPriority int + DefaultEnabled bool +} + +// LookupSeedPlacement resolves a provider's seed placement for one content level +// from its capability manifest with a single metadata fetch. SupportsLevel uses +// the same rule as the chain-less fallback (providerSupportsLevel): a provider +// that enumerates levels is eligible only for the ones it lists; a provider that +// declares none stays eligible everywhere. +func LookupSeedPlacement(ctx context.Context, pool *pgxpool.Pool, pluginInstallationID int, capabilityID, contentLevel string) SeedPlacement { + md := lookupCapabilityMetadata(ctx, pool, pluginInstallationID, capabilityID) + return SeedPlacement{ + SupportsLevel: providerSupportsLevel(md, contentLevel), + DefaultPriority: extractDefaultPriority(md, contentLevel), + DefaultEnabled: extractDefaultEnabled(md), + } +} + // declaredPriorityLevels parses a capability's default_priority map. The map may // sit at the top level or inside a "metadata" envelope (plugin capability // metadata wraps plugin-declared fields in a "metadata" sub-object). The second diff --git a/internal/metadata/chain_test.go b/internal/metadata/chain_test.go index 2f439723..7b674073 100644 --- a/internal/metadata/chain_test.go +++ b/internal/metadata/chain_test.go @@ -57,3 +57,39 @@ func TestProviderSupportsLevel(t *testing.T) { }) } } + +func TestExtractDefaultEnabled(t *testing.T) { + cases := []struct { + name string + metadataJSON string + want bool + }{ + // Absent flag defaults to true so every existing plugin is seeded + // enabled exactly as before this flag existed. + {"no metadata defaults enabled", ``, true}, + {"empty object defaults enabled", `{}`, true}, + {"tmdb without flag defaults enabled", tmdbCapMetadata, true}, + {"malformed json defaults enabled (fail-open)", `{not json`, true}, + + // A specialist provider opts out of being auto-enabled. The flag lives + // in the same "metadata" envelope as default_priority. + {"opt-out inside envelope", `{"metadata":{"default_priority":{"series":50},"default_enabled":false}}`, false}, + {"opt-out at top level", `{"default_enabled":false}`, false}, + + // An explicit true is honored (and is the default anyway). + {"explicit true inside envelope", `{"metadata":{"default_enabled":true}}`, true}, + + // A non-boolean value is ignored, falling back to the enabled default. + {"non-boolean value defaults enabled", `{"metadata":{"default_enabled":"nope"}}`, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := extractDefaultEnabled([]byte(tc.metadataJSON)) + if got != tc.want { + t.Fatalf("extractDefaultEnabled(%q) = %v, want %v", + tc.metadataJSON, got, tc.want) + } + }) + } +} diff --git a/web/src/components/admin/libraries/LibraryForm.test.tsx b/web/src/components/admin/libraries/LibraryForm.test.tsx index 328454bd..155eb450 100644 --- a/web/src/components/admin/libraries/LibraryForm.test.tsx +++ b/web/src/components/admin/libraries/LibraryForm.test.tsx @@ -1,6 +1,14 @@ import { describe, expect, it } from "vitest"; -import { contentLevelsForType } from "./useLibraryForm"; +import type { PluginInstallation } from "@/api/types"; + +import type { LevelChainItem } from "./useLibraryForm"; +import { + contentLevelsForType, + hasMetadataProviderCapability, + levelChainsFromResponse, + mergeChainWithDefaults, +} from "./useLibraryForm"; describe("contentLevelsForType", () => { it("maps ebook libraries to the ebook metadata content level", () => { @@ -19,3 +27,121 @@ describe("contentLevelsForType", () => { ]); }); }); + +describe("levelChainsFromResponse", () => { + it("orders entries by server priority and keeps the server's slug and enabled state", () => { + // The same converter handles a saved chain and the provider-defaults + // response — including a specialist the server seeded disabled. + const chains = levelChainsFromResponse({ + levels: { + series: [ + { + plugin_installation_id: 22, + capability_id: "sportarr", + provider_slug: "sportarr", + priority: 2, + enabled: false, + }, + { + plugin_installation_id: 2, + capability_id: "tvdb", + provider_slug: "tvdb", + priority: 0, + enabled: true, + }, + { + plugin_installation_id: 1, + capability_id: "tmdb", + provider_slug: "tmdb", + priority: 1, + enabled: true, + }, + ], + }, + }); + + expect(chains["series"]!.map((e) => [e.provider_slug, e.enabled])).toEqual([ + ["tvdb", true], + ["tmdb", true], + ["sportarr", false], + ]); + }); + + it("returns an empty record for a missing or empty response", () => { + expect(levelChainsFromResponse(undefined)).toEqual({}); + expect(levelChainsFromResponse({ levels: {} })).toEqual({}); + }); +}); + +describe("mergeChainWithDefaults", () => { + const item = (slug: string, over: Partial = {}): LevelChainItem => ({ + plugin_installation_id: 1, + capability_id: slug, + provider_slug: slug, + enabled: true, + ...over, + }); + + it("fills only the levels the saved chain does not cover", () => { + const merged = mergeChainWithDefaults( + { series: [item("tvdb")], season: [] }, + { series: [item("tmdb")], season: [item("tmdb")], episode: [item("tmdb")] }, + "series", + ); + + expect(merged["series"]!.map((e) => e.provider_slug)).toEqual(["tvdb"]); + expect(merged["season"]!.map((e) => e.provider_slug)).toEqual(["tmdb"]); + expect(merged["episode"]!.map((e) => e.provider_slug)).toEqual(["tmdb"]); + }); + + it("leaves levels empty when the defaults have nothing for them either", () => { + const merged = mergeChainWithDefaults({}, {}, "movies"); + expect(merged["movie"]).toEqual([]); + }); +}); + +describe("hasMetadataProviderCapability", () => { + const installation = (over: Partial): PluginInstallation => + ({ + id: 1, + plugin_id: "silo.tmdb", + version: "1.0.0", + install_path: "/x", + enabled: true, + capabilities: [], + global_config_schema: [], + user_config_schema: [], + routes: [], + assets: [], + global_configs: [], + auth_bindings: [], + task_bindings: [], + update_policy: "manual", + ...over, + }) as PluginInstallation; + + it("ignores disabled installations and non-metadata capabilities", () => { + expect( + hasMetadataProviderCapability([ + installation({ + id: 1, + enabled: false, + capabilities: [{ type: "metadata_provider.v1", id: "tvdb", display_name: "TVDB" }], + }), + installation({ + id: 2, + capabilities: [{ type: "request_router.v1", id: "seerr", display_name: "Seerr" }], + }), + ]), + ).toBe(false); + + expect( + hasMetadataProviderCapability([ + installation({ + id: 3, + capabilities: [{ type: "metadata_provider.v1", id: "tmdb", display_name: "TMDB" }], + }), + ]), + ).toBe(true); + }); +}); diff --git a/web/src/components/admin/libraries/LibraryFormSections.tsx b/web/src/components/admin/libraries/LibraryFormSections.tsx index 23a7a454..26749b3d 100644 --- a/web/src/components/admin/libraries/LibraryFormSections.tsx +++ b/web/src/components/admin/libraries/LibraryFormSections.tsx @@ -7,6 +7,7 @@ import { ChevronRight, FolderOpen, FolderSearch, + Loader2, Plus, Trash2, } from "lucide-react"; @@ -345,7 +346,12 @@ export function MetadataFields({ form }: { form: LibraryFormController }) { Providers are asked in order from top to bottom. Uncheck a provider to skip it for that level.

- {form.hasMetadataProviders ? ( + {form.chainLoading ? ( +
+ + Loading providers… +
+ ) : form.hasMetadataProviders ? ( form.contentLevels.map((level) => ( ; -}; +// hasMetadataProviderCapability reports whether any enabled plugin installation +// exposes a metadata provider — used only to decide between the chain editor +// and the "install a plugin" empty state. The chain contents themselves come +// from the server. +export function hasMetadataProviderCapability(installations: PluginInstallation[]): boolean { + return installations.some( + (inst) => + inst.enabled && (inst.capabilities ?? []).some((cap) => cap.type === "metadata_provider.v1"), + ); +} export interface LibraryFormErrors { name?: string; @@ -65,64 +75,43 @@ export function contentLevelLabel(level: string): string { .join(" "); } -function buildDefaultLevelChains( - metadataProviders: MetadataProvider[], - libraryType: string, -): Record { - const defaultChain: Record = {}; - for (const level of contentLevelsForType(libraryType)) { - const sorted = [...metadataProviders].sort((a, b) => { - const pa = a.defaultPriority[level] ?? 0; - const pb = b.defaultPriority[level] ?? 0; - if ((pa === 0) !== (pb === 0)) return pa === 0 ? 1 : -1; - return pa - pb; - }); - defaultChain[level] = sorted.map((provider) => ({ - plugin_installation_id: provider.plugin_installation_id, - capability_id: provider.capability_id, - provider_slug: provider.slug, - enabled: (provider.defaultPriority[level] ?? 0) > 0, - })); - } - return defaultChain; -} - -function buildLevelChainsFromServer( - currentChain: { - levels?: Record< - string, - Array<{ - plugin_installation_id: number; - capability_id: string; - provider_slug: string; - enabled: boolean; - }> - >; - } | null, - metadataProviders: MetadataProvider[], - libraryType: string, +// levelChainsFromResponse converts a provider-chain API response (a library's +// saved chain, or the server-computed defaults for a library type) into the +// editor's per-level item lists, preserving server priority order. +export function levelChainsFromResponse( + response: LibraryProviderChainResponse | null | undefined, ): Record { const mapped: Record = {}; - if (currentChain?.levels) { - for (const [level, entries] of Object.entries(currentChain.levels)) { - mapped[level] = entries.map((entry) => ({ + for (const [level, entries] of Object.entries(response?.levels ?? {})) { + mapped[level] = [...entries] + .sort((a, b) => a.priority - b.priority) + .map((entry) => ({ plugin_installation_id: entry.plugin_installation_id, capability_id: entry.capability_id, provider_slug: entry.provider_slug, enabled: entry.enabled, })); - } - } - - const defaults = buildDefaultLevelChains(metadataProviders, libraryType); - for (const level of contentLevelsForType(libraryType)) { - if (!mapped[level] || mapped[level].length === 0) { - mapped[level] = defaults[level] ?? []; - } } return mapped; } +// mergeChainWithDefaults fills content levels the saved chain doesn't cover +// (e.g. a library created before a level existed) with the server-computed +// defaults, without touching levels the chain already defines. +export function mergeChainWithDefaults( + chain: Record, + defaults: Record, + libraryType: string, +): Record { + const merged = { ...chain }; + for (const level of contentLevelsForType(libraryType)) { + if (!merged[level] || merged[level].length === 0) { + merged[level] = defaults[level] ?? []; + } + } + return merged; +} + function buildProviderChainBody(activeLevelChains: Record) { return { levels: Object.fromEntries( @@ -171,38 +160,16 @@ export function useLibraryForm({ const setChainMutation = useSetLibraryProviders(); const { installations } = useAdminPlugins(); const { data: currentChain } = useLibraryProviders(library?.id ?? null); - - const metadataProviders = useMemo(() => { - const result: MetadataProvider[] = []; - for (const inst of installations) { - if (!inst.enabled) continue; - for (const cap of inst.capabilities ?? []) { - if (cap.type === "metadata_provider.v1") { - const dp = - (cap.metadata?.default_priority as Record) ?? - ((cap.metadata?.metadata as Record)?.default_priority as Record< - string, - number - >) ?? - {}; - result.push({ - plugin_installation_id: inst.id, - capability_id: cap.id, - slug: cap.display_name || cap.id, - defaultPriority: dp, - }); - } - } - } - return result; - }, [installations]); + // The server computes default chains (same logic that seeds them on create), + // so the form never re-derives defaults from plugin manifests client-side. + const { data: providerDefaults, isLoading: defaultsLoading } = useLibraryProviderDefaults(type); const isPending = createMutation.isPending || updateMutation.isPending || setChainMutation.isPending; const defaultLevelChains = useMemo( - () => buildDefaultLevelChains(metadataProviders, type), - [metadataProviders, type], + () => levelChainsFromResponse(providerDefaults), + [providerDefaults], ); const resolvedLevelChains = useMemo(() => { if (!library) { @@ -211,9 +178,14 @@ export function useLibraryForm({ if (currentChain === undefined) { return levelChains; } - return buildLevelChainsFromServer(currentChain, metadataProviders, type); - }, [currentChain, defaultLevelChains, levelChains, library, metadataProviders, type]); + return mergeChainWithDefaults(levelChainsFromResponse(currentChain), defaultLevelChains, type); + }, [currentChain, defaultLevelChains, levelChains, library, type]); const activeLevelChains = chainDirty ? levelChains : resolvedLevelChains; + // The chain editor has nothing truthful to show until the server chain (for + // an existing library) and the type's defaults have arrived; local edits + // always render immediately. + const chainLoading = + !chainDirty && (defaultsLoading || (library !== null && currentChain === undefined)); const allErrors = useMemo(() => { const next: LibraryFormErrors = {}; @@ -250,8 +222,11 @@ export function useLibraryForm({ function handleTypeChange(newType: string) { setType(newType); if (!library) { - setLevelChains(buildDefaultLevelChains(metadataProviders, newType)); - setChainDirty(true); + // Drop any local chain edits: the new type's defaults come from the + // server, and with a clean chain the create flow lets the server-seeded + // chain stand instead of writing one back. + setLevelChains({}); + setChainDirty(false); } } @@ -368,9 +343,10 @@ export function useLibraryForm({ toggleTrailerKind, contentLevels: contentLevelsForType(type), activeLevelChains, + chainLoading, reorderLevel, toggleLevelProvider, - hasMetadataProviders: metadataProviders.length > 0, + hasMetadataProviders: hasMetadataProviderCapability(installations), errors, isPending, submit, diff --git a/web/src/hooks/queries/admin/libraries.ts b/web/src/hooks/queries/admin/libraries.ts index 41f6d358..406d9b88 100644 --- a/web/src/hooks/queries/admin/libraries.ts +++ b/web/src/hooks/queries/admin/libraries.ts @@ -485,6 +485,21 @@ export function useLibraryProviders(libraryId: number | null) { }); } +// useLibraryProviderDefaults fetches the provider chain the server would seed +// for a new library of the given type — the single source of truth the create +// form renders instead of re-deriving defaults from plugin manifests. +export function useLibraryProviderDefaults(libraryType: string) { + return useQuery({ + queryKey: adminKeys.libraryProviderDefaults(libraryType), + queryFn: () => + api( + `/libraries/provider-defaults?library_type=${encodeURIComponent(libraryType)}`, + ).then((d) => d ?? { levels: {} }), + enabled: libraryType !== "", + staleTime: ADMIN_STALE_TIME, + }); +} + export function useSetLibraryProviders() { const queryClient = useQueryClient(); return useMutation({ diff --git a/web/src/hooks/queries/keys.ts b/web/src/hooks/queries/keys.ts index ca82427d..74850e10 100644 --- a/web/src/hooks/queries/keys.ts +++ b/web/src/hooks/queries/keys.ts @@ -364,6 +364,8 @@ export const adminKeys = { collectionTemplates: () => ["admin", "collections", "templates"] as const, collectionTemplateBundles: () => ["admin", "collections", "templateBundles"] as const, libraryProviders: (id: number) => ["admin", "libraries", id, "providers"] as const, + libraryProviderDefaults: (libraryType: string) => + ["admin", "libraries", "provider-defaults", libraryType] as const, nodes: () => ["admin", "nodes"] as const, stats: () => ["admin", "stats"] as const, sessions: () => ["admin", "sessions"] as const,