feat(metadata): implement top poster api option #583
Conversation
WalkthroughAdds TopPoster as an alternative poster provider alongside RPDB: new TopPoster utility, server /top-poster route, UI selector and inputs, schema/config fields and migrations, unified poster-service resolution and fallback while preserving RPDB compatibility. Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"noteFill":"#f6fbff","actorBorder":"#6b7280","actorBkg":"#eef2ff"}}}%%
sequenceDiagram
participant User as User / Client
participant Frontend as Frontend UI
participant Server as API Server
participant TopPoster as TOP Poster API
participant RPDB as RPDB Backend
participant Cache as Cache (24h)
rect rgb(245,250,255)
Note over Frontend: Configure poster service & API key
User->>Frontend: Select posterService and enter API key
Frontend->>Server: Persist userData (posterService + key)
end
rect rgb(240,255,240)
Note over User,Frontend: Poster retrieval
User->>Frontend: Request item with poster
Frontend->>Server: GET /api/top-poster (or unified poster endpoint)
Server->>Cache: Check cached poster URL
alt Cache hit
Cache-->>Server: Return cached URL
else Cache miss
alt posterService == "top-poster"
Server->>TopPoster: Request poster URL (with API key)
TopPoster-->>Server: Poster URL or null
else posterService == "rpdb"
Server->>RPDB: Request poster URL
RPDB-->>Server: Poster URL or null
end
Server->>Cache: Store result on success
end
alt URL found
Server-->>Frontend: 301 redirect or poster URL
Frontend->>User: Render poster
else URL missing
Server-->>Frontend: 404 structured error
Frontend->>User: Render fallback poster
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/frontend/src/components/menu/addons.tsx (1)
2016-2017: UI copy now matches generalised enhanced-poster behaviourRenaming the RPDB toggle/tooltip to “Enhanced Posters” and explicitly mentioning “RPDB or Top Poster” brings the catalogue settings UI in line with the new dual‑service poster flow, while continuing to drive the existing
rpdbboolean flag. That’s a sensible way to evolve the feature without breaking existing configs.Also applies to: 2175-2178
packages/server/src/routes/api/top-poster.ts (4)
13-13: Unused logger declaration.The
loggeris instantiated but never used in this route. Either remove it or add appropriate logging for debugging/monitoring.🔎 Suggested fix
-const logger = createLogger('server');Or add logging for successful/failed requests:
+ logger.info(`Top Poster request`, { id, type, season, episode }); const topPoster = new TopPoster(apiKey);
43-52: Partial season/episode parameters may cause unexpected behaviour.When only
seasonis provided withoutepisode(or vice versa), the conditionseason && episodeevaluates to false, falling through togetPosterUrlinstead of returning an error. This could be confusing for API consumers expecting episode thumbnail functionality.🔎 Suggested fix
const { id, type, fallback, apiKey, season, episode } = data; const topPoster = new TopPoster(apiKey); let posterUrl: string | null; - if (season && episode) { + if (season !== undefined || episode !== undefined) { + if (season === undefined || episode === undefined) { + res.status(400).json( + createResponse({ + success: false, + detail: 'Both season and episode are required for episode thumbnails', + error: { + code: constants.ErrorCode.BAD_REQUEST, + message: 'Both season and episode must be provided together', + }, + }) + ); + return; + } posterUrl = await topPoster.getEpisodeThumbnailUrl( type, id, season, episode ); } else { posterUrl = await topPoster.getPosterUrl(type, id); }
64-64: Redundant non-null assertion.The
posterUrl!assertion is unnecessary since the precedingif (!posterUrl)block returns early, guaranteeingposterUrlis non-null at this point.🔎 Suggested fix
- res.redirect(301, posterUrl!); + res.redirect(301, posterUrl);
15-22: Consider adding stricter validation fortypeparameter.The
typefield accepts any string, but the TopPoster utility likely only supports specific values (e.g., 'movie', 'series'). Adding enum validation would provide better error messages.🔎 Suggested fix
const searchParams = z.object({ id: z.string(), - type: z.string(), + type: z.enum(['movie', 'series']), fallback: z.string().optional(), apiKey: z.string(), season: z.coerce.number().optional(), episode: z.coerce.number().optional(), });packages/core/src/main.ts (1)
405-408: Consider renaming therpdbproperty in catalog modifications.The condition
modification?.rpdbis misleading now that the feature supports multiple poster services. Consider renaming toposterServiceEnabledor similar in a follow-up change for clarity.- modification?.rpdb && activePosterService + modification?.posterServiceEnabled && activePosterServiceThis would require updating the schema and existing configurations.
top_poster_api_openapi.json (2)
1-1: OpenAPI specification lacks security definitions.Static analysis flagged missing security configuration. While this documents an external API (Top Poster), the specification should include security schemes for completeness:
- CKV_OPENAPI_4: No global security field defined
- CKV_OPENAPI_5: No security operations defined
The API uses API keys in the path (
{api_key}), which should be documented in thesecuritySchemessection.Consider adding security scheme documentation:
"components": { "securitySchemes": { "ApiKeyPath": { "type": "apiKey", "in": "path", "name": "api_key", "description": "API key passed in URL path" } } }And applying it globally:
"security": [{ "ApiKeyPath": [] }]
1-1: Consider relocating OpenAPI specification.The OpenAPI JSON file is placed at the repository root. Consider moving it to a more appropriate location such as
docs/orpackages/server/openapi/to maintain a cleaner project structure.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/core/src/utils/constants.tspackages/core/src/utils/index.tspackages/core/src/utils/top-poster.tspackages/frontend/src/components/menu/addons.tsxpackages/frontend/src/components/menu/services.tsxpackages/frontend/src/components/shared/config-templates-modal.tsxpackages/server/src/app.tspackages/server/src/routes/api/index.tspackages/server/src/routes/api/top-poster.tstop_poster_api_openapi.json
🧰 Additional context used
🧬 Code graph analysis (3)
packages/server/src/routes/api/top-poster.ts (4)
packages/server/src/utils/responses.ts (1)
createResponse(26-35)packages/core/src/utils/config.ts (1)
formatZodError(37-39)packages/core/src/utils/top-poster.ts (1)
TopPoster(11-175)packages/core/src/utils/constants.ts (1)
APIError(104-113)
packages/frontend/src/components/menu/services.tsx (2)
packages/core/src/main.ts (1)
setUserData(116-118)packages/frontend/src/components/ui/password-input/password-input.tsx (1)
PasswordInput(40-146)
packages/core/src/main.ts (3)
packages/core/src/utils/rpdb.ts (1)
RPDB(11-150)packages/core/src/utils/top-poster.ts (1)
TopPoster(11-175)packages/core/src/utils/env.ts (1)
Env(318-2071)
🪛 Checkov (3.2.334)
top_poster_api_openapi.json
[high] 1: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 1: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (10)
packages/core/src/utils/constants.ts (1)
717-747: TopPoster top-level option is wired consistently
topPosterApiKeyis correctly added to both the type union andTOP_LEVEL_OPTION_DETAILS, so consumers (e.g. templates UI) can surface the new key without type gaps. No issues from my side here.packages/core/src/db/schemas.ts (1)
431-435: Poster service wiring and validation schema look coherentAdding
topPosterApiKeyand a constrainedposterServiceenum toUserDataSchema, plus theTopPosterIsValidResponsezod schema/type, lines up with the new Top Poster flow and is type-safe. Just ensure all callers use the exact literals'rpdb'and'top-poster'when reading/writingposterService, to avoid subtle mismatches.Also applies to: 1067-1070
packages/server/src/routes/api/index.ts (1)
7-7: Server API barrel cleanly exposes Top Poster routeRe‑exporting
topPosterApialongside the existing route modules keeps the API surface consistent and makes wiring inapp.tsstraightforward. Looks good.packages/frontend/src/components/shared/config-templates-modal.tsx (1)
862-887: Template input support for Top Poster API key is consistentAdding
topPosterApiKeytotopLevelFieldsand driving its label/description fromTOP_LEVEL_OPTION_DETAILSmatches the existing pattern for TMDB/RPDB/TVDB keys, so imported templates can now prompt for the Top Poster key without extra bespoke logic.packages/core/src/utils/index.ts (1)
16-16: TopPoster utility correctly re-exported from utils barrelRe‑exporting
./top-poster.jshere is the right move so callers can accessTopPostervia the central utils entrypoint, mirroring the existing RPDB helper.packages/server/src/app.ts (1)
9-9: Top Poster API route is wired consistently with existing APIsImporting
topPosterApiand mounting it at/top-posteron the versionedapiRoutermirrors the existing RPDB route pattern, so the new endpoint will be reachable at/api/v{API_VERSION}/top-posterwithout special handling. No concerns here.Also applies to: 95-95
packages/frontend/src/components/menu/services.tsx (2)
303-326: LGTM! Clean implementation of the poster service selector.The TemplateOption component is appropriately configured with the select type and the two service options. The default value of 'rpdb' correctly aligns with the backward compatibility logic in the core layer.
327-378: LGTM! Conditional API key inputs are well-structured.Both PasswordInput components:
- Use
autoComplete="new-password"to prevent browser autofill issues- Include helpful links to obtain API keys
- Follow the existing input patterns in the codebase
packages/core/src/main.ts (2)
400-421: LGTM! Good backward compatibility handling.The logic correctly:
- Falls back to 'rpdb' when
posterServiceis not set butrpdbApiKeyexists- Selects the appropriate API key based on the active service
- Instantiates the correct poster service class
428-454: LGTM! Unified poster URL construction.The code correctly:
- Detects existing poster service URLs to avoid double-processing
- Constructs the appropriate API path based on the selected service
- Falls back to direct API calls when
BASE_URLis not available
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/core/src/utils/top-poster.ts (2)
19-49: Cache check doesn't handle false values, and error differentiation is missing.The issues identified in the previous review are still present:
- Line 21:
if (cached)won't work iffalseis cached. Useif (cached !== undefined).- Lines 32-36: All non-OK HTTP responses are treated as "Invalid Top Poster API key", including 5xx server errors and 429 rate limits. This makes diagnosis difficult—users can't tell if their key is wrong or if the service is temporarily unavailable.
- Line 38: No try-catch around
response.json()or the schema parse—malformed responses will throw unhandled errors.As noted in the previous review, you should:
- Treat 4xx auth errors (400/401/403) or
valid: falseas an invalid key.- Treat 5xx/429/network/parse failures as "service unavailable" with a distinct error message.
54-104: Remove@ts-ignoreby properly initialising variables.The
// @ts-ignoreon line 101 suppresses TypeScript's legitimate concern about potentially uninitialised variables. As highlighted in the previous review, you should:
- Initialise
idTypeandidValueasnullon lines 61-62- Update the check on line 102 to explicitly narrow the types
- Remove the
@ts-ignoredirectiveThis ensures type safety without suppressing compiler warnings.
Recommended fix
- let idType: 'tmdb' | 'imdb' | 'tvdb'; - let idValue: string; + let idType: 'tmdb' | 'imdb' | 'tvdb' | null = null; + let idValue: string | null = null; switch (parsedId.type) { // ... switch cases ... } - // @ts-ignore if (!idType || !idValue) return null; return { idType, idValue };
🧹 Nitpick comments (1)
packages/core/src/utils/top-poster.ts (1)
139-174: LGTM with minor observation.The episode thumbnail URL retrieval follows the same sound pattern as
getPosterUrl. While there's some code duplication between the two methods, the different URL construction and cache keys justify keeping them separate rather than abstracting.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/core/src/utils/constants.tspackages/core/src/utils/index.tspackages/core/src/utils/top-poster.tspackages/frontend/src/components/menu/addons.tsxpackages/frontend/src/components/menu/services.tsxpackages/frontend/src/components/shared/config-templates-modal.tsxpackages/server/src/app.tspackages/server/src/routes/api/index.tspackages/server/src/routes/api/top-poster.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/server/src/routes/api/index.ts
- packages/frontend/src/components/shared/config-templates-modal.tsx
- packages/server/src/routes/api/top-poster.ts
- packages/frontend/src/components/menu/services.tsx
- packages/frontend/src/components/menu/addons.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/utils/top-poster.ts (5)
packages/core/src/utils/cache.ts (1)
Cache(24-306)packages/core/src/db/schemas.ts (2)
TopPosterIsValidResponse(1067-1069)TopPosterIsValidResponse(1070-1070)packages/core/src/utils/env.ts (1)
Env(318-2071)packages/core/src/utils/id-parser.ts (1)
IdParser(66-199)packages/core/src/utils/anime-database.ts (1)
AnimeDatabase(502-1054)
🔇 Additional comments (11)
packages/core/src/utils/index.ts (1)
16-16: LGTM!The export follows the established pattern and is appropriately positioned alongside the RPDB export.
packages/core/src/utils/constants.ts (1)
717-747: LGTM!The Top Poster API key configuration follows the established pattern for RPDB and provides clear guidance to users.
packages/server/src/app.ts (2)
9-9: LGTM!The Top Poster API import and route registration follow the established pattern for API routes.
95-95: LGTM!Route registration is consistent with other API routes like RPDB.
packages/core/src/db/schemas.ts (2)
433-434: LGTM!The schema additions for Top Poster support are well-structured and follow Zod v4 conventions. Making both fields optional ensures backward compatibility.
1067-1070: LGTM!The TopPosterIsValidResponse schema correctly mirrors the RPDBIsValidResponse pattern and provides proper type safety for API validation responses.
packages/core/src/main.ts (3)
34-34: LGTM!The TopPoster import is correctly placed alongside the RPDB import.
400-421: Good backward compatibility handling.The logic properly defaults to RPDB when
posterServiceisn't explicitly set butrpdbApiKeyis present, ensuring existing configurations continue to work. The conditional selection of API key and instantiation of the appropriate poster service is correctly implemented.
426-455: LGTM!The poster modification logic correctly:
- Detects existing poster service URLs (both RPDB and Top Poster)
- Routes requests through the appropriate API endpoint based on
posterService- Falls back to direct API calls when
BASE_URLis not configuredpackages/core/src/utils/top-poster.ts (2)
11-18: LGTM!The constructor provides essential validation that the API key is set before allowing instantiation.
105-138: LGTM!The poster URL retrieval logic is well-structured:
- Properly caches results to reduce API calls
- Optional existence check via HEAD request
- Gracefully handles errors by returning null when poster doesn't exist
- Cache key includes API key for proper multi-tenant support
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/core/src/utils/top-poster.ts (3)
19-49: Address error handling and cache checking issues from previous review.The issues flagged in the previous review remain unaddressed:
- Cache checking (line 21): Using
if (cached)will incorrectly skip cachedfalsevalues. Useif (cached !== undefined)instead.- Error differentiation (lines 32-36): All HTTP errors are treated as "Invalid API key", including transient 5xx and 429 errors. This misleads users when the service is temporarily unavailable.
- JSON parsing (line 38): No error handling for malformed responses.
Please refer to the detailed fix suggestions in the previous review comment.
54-104: Remove @ts-ignore by initializing variables explicitly.As noted in the previous review, the
@ts-ignoreat lines 101-102 should be removed by initializingidTypeandidValuetonulland narrowing their types explicitly. This avoids suppressing TypeScript's type checking.Refer to the previous review comment for the suggested fix.
105-174: Fix cache checking in poster URL methods.Both
getPosterUrl(line 116) andgetEpisodeThumbnailUrl(line 152) useif (cached)which will fail to return cached empty strings or other falsy values. Useif (cached !== undefined)instead.🔎 Proposed fix
public async getPosterUrl( type: string, id: string, checkExists: boolean = true ): Promise<string | null> { const parsed = this.parseId(type, id); if (!parsed) return null; const { idType, idValue } = parsed; const cacheKey = `${type}-${id}-${this.apiKey}`; const cached = await posterCheckCache.get(cacheKey); - if (cached) { + if (cached !== undefined) { return cached; } // ... rest of method public async getEpisodeThumbnailUrl( type: string, id: string, season: number, episode: number, checkExists: boolean = true ): Promise<string | null> { const parsed = this.parseId(type, id); if (!parsed) return null; const { idType, idValue } = parsed; const cacheKey = `${type}-${id}-S${season}E${episode}-${this.apiKey}`; const cached = await posterCheckCache.get(cacheKey); - if (cached) { + if (cached !== undefined) { return cached; } // ... rest of method
🧹 Nitpick comments (2)
packages/core/src/utils/top-poster.ts (1)
13-17: Consider trimming the API key input.The empty string check is good, but it doesn't catch whitespace-only API keys. Consider adding
.trim()to handle accidental whitespace.🔎 Suggested enhancement
constructor(apiKey: string) { - this.apiKey = apiKey; + this.apiKey = apiKey.trim(); if (!this.apiKey) { throw new Error('Top Poster API key is not set'); } }packages/core/src/main.ts (1)
400-408: Backward compatibility logic is sound.The default to 'rpdb' when
rpdbApiKeyis present butposterServiceis not set ensures existing users' configurations continue to work. However, note thatmodification?.rpdbis now a misnomer since it applies to both RPDB and Top Poster services.Consider renaming
modification.rpdbtomodification.enhancePosterormodification.usePosterServicein a future refactor to better reflect its dual purpose.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/core/src/utils/constants.tspackages/core/src/utils/index.tspackages/core/src/utils/top-poster.tspackages/frontend/src/components/menu/addons.tsxpackages/frontend/src/components/menu/services.tsxpackages/frontend/src/components/shared/config-templates-modal.tsxpackages/server/src/app.tspackages/server/src/routes/api/index.tspackages/server/src/routes/api/top-poster.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/server/src/routes/api/top-poster.ts
- packages/frontend/src/components/menu/services.tsx
- packages/server/src/routes/api/index.ts
- packages/frontend/src/components/menu/addons.tsx
- packages/frontend/src/components/shared/config-templates-modal.tsx
- packages/core/src/utils/index.ts
🔇 Additional comments (6)
packages/core/src/db/schemas.ts (2)
446-447: LGTM! Schema additions follow existing patterns.The new fields integrate well with the existing RPDB fields and maintain backward compatibility through optional typing.
1081-1084: LGTM! Validation schema mirrors RPDB pattern.The
TopPosterIsValidResponseschema is correctly defined and follows the established pattern for API key validation responses.packages/server/src/app.ts (1)
9-9: LGTM! Route registration follows established patterns.The Top Poster API route is correctly registered alongside the RPDB route, maintaining consistency with the existing API structure.
Also applies to: 95-95
packages/core/src/utils/constants.ts (1)
717-747: LGTM! Constant definition follows existing patterns.The
topPosterApiKeyentry is well-structured and includes a helpful registration URL in the description, following the same pattern as other API key constants.packages/core/src/main.ts (2)
410-421: API instance creation correctly handles both poster services.The logic properly maps the poster service to the corresponding API key and instantiates the correct class. The code correctly handles the case where an API key might be undefined (posterApi will be undefined, and the subsequent check at line 426 will skip poster modification).
426-455: Poster URL modification logic correctly handles multiple scenarios.The logic properly:
- Skips modification for posters already from rating or top-streaming services
- Routes through server API when BASE_URL is available (enabling caching/fallback)
- Calls poster API directly when BASE_URL is not configured
- Uses
checkExists: falseas an optimisation when calling the API directlyThe non-null assertion on
posterApiKeyat line 441 is safe because we're inside theif (posterApi...)block, andposterApican only be defined ifposterApiKeyis defined.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/components/menu/addons.tsx (1)
1570-1576: Consider both poster services when initialisingusePosterServicefor new catalogs.Currently,
usePosterServicedefaults totrueonly whenuserData.rpdbApiKeyis present. However, users may configuretopPosterApiKeywithposterService: 'top-poster'instead. New catalogs should default tousePosterService: truewhen either poster service is configured.🔎 Proposed fix
modifications.push({ id: catalog.id, name: catalog.name, type: catalog.type, enabled: true, shuffle: false, - usePosterService: userData.rpdbApiKey ? true : false, + usePosterService: !!(userData.rpdbApiKey || userData.topPosterApiKey), hideable: catalog.hideable, searchable: catalog.searchable, addonName: catalog.addonName, });
🧹 Nitpick comments (1)
packages/core/src/utils/top-poster.ts (1)
59-68: Consider caching invalid API key results to prevent repeated validation requests.When
data.validisfalse, an error is thrown before the result is cached. This means repeated calls with the same invalid key will continue hitting the API. If intentional (to allow retries after fixing the key), this is acceptable, but cachingfalsecould reduce unnecessary API load.🔎 Proposed fix to cache invalid keys
+ apiKeyValidationCache.set( + this.apiKey, + data.valid, + Env.RPDB_API_KEY_VALIDITY_CACHE_TTL + ); + if (!data.valid) { throw new Error('Invalid Top Poster API key'); } - apiKeyValidationCache.set( - this.apiKey, - data.valid, - Env.RPDB_API_KEY_VALIDITY_CACHE_TTL - ); return data.valid;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/core/src/utils/top-poster.tspackages/frontend/src/components/menu/addons.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/main.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/frontend/src/components/menu/addons.tsx (1)
packages/core/src/main.ts (1)
setUserData(116-118)
🔇 Additional comments (10)
packages/core/src/utils/top-poster.ts (4)
11-18: LGTM!Constructor properly validates and trims the API key, throwing a clear error when empty.
74-123: LGTM!The
parseIdmethod properly handles type narrowing with explicit null initialisation, and the ID mapping logic covers all expected cases with appropriate fallbacks viaAnimeDatabase.
124-157: LGTM!The
getPosterUrlmethod follows a sensible pattern with proper caching, timeout handling, and optional existence checking.
158-193: LGTM!The
getEpisodeThumbnailUrlmethod is consistent withgetPosterUrland properly includes season/episode in the cache key.packages/frontend/src/components/menu/addons.tsx (3)
91-91: LGTM!The interface field rename from
rpdbtousePosterServicealigns with the schema changes and provides a more generic name for supporting multiple poster services.
1998-2025: LGTM!The icon button toggle and tooltip correctly use the renamed
usePosterServicefield with service-agnostic terminology.
2184-2199: LGTM!The switch control correctly binds to
usePosterServicewith appropriate label and help text updates describing both RPDB and Top Poster support.packages/core/src/db/schemas.ts (3)
296-305: LGTM!The schema transform provides excellent backward compatibility by migrating the legacy
rpdbfield tousePosterService. The nullish coalescing (??) ensures existing configurations continue working whilst new ones use the renamed field.
450-451: LGTM!The new
topPosterApiKeyandposterServicefields are correctly defined as optional, allowing users to configure either RPDB or Top Poster without breaking existing configurations.
1085-1088: LGTM!The
TopPosterIsValidResponseschema is consistent with the existingRPDBIsValidResponsepattern, maintaining API parity between the two poster services.
Viren070
force-pushed
the
top-poster-api
branch
from
37e0706 to
b8fd229
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/main.ts (1)
925-941: Consider simplifying nested ternary operators for readability.The poster service and API resolution logic is correct, including the fallback to RPDB when
posterServiceis undefined butrpdbApiKeyexists. However, the deeply nested ternaries make this harder to maintain.🔎 Suggested refactor for clarity
- const posterService = applyPosterService - ? this.userData.posterService || - (this.userData.rpdbApiKey ? 'rpdb' : undefined) - : undefined; - const posterApiKey = - posterService === 'rpdb' - ? this.userData.rpdbApiKey - : posterService === 'top-poster' - ? this.userData.topPosterApiKey - : undefined; - const posterApi = posterApiKey - ? posterService === 'rpdb' - ? new RPDB(posterApiKey) - : posterService === 'top-poster' - ? new TopPoster(posterApiKey) - : undefined - : undefined; + let posterService: 'rpdb' | 'top-poster' | undefined; + if (applyPosterService) { + posterService = this.userData.posterService || + (this.userData.rpdbApiKey ? 'rpdb' : undefined); + } + + const posterApiKey = posterService === 'rpdb' + ? this.userData.rpdbApiKey + : posterService === 'top-poster' + ? this.userData.topPosterApiKey + : undefined; + + let posterApi: RPDB | TopPoster | undefined; + if (posterApiKey) { + posterApi = posterService === 'rpdb' + ? new RPDB(posterApiKey) + : new TopPoster(posterApiKey); + }packages/core/src/utils/top-poster.ts (1)
67-71: Consider using a dedicated TTL constant for Top Poster.The code reuses
Env.RPDB_API_KEY_VALIDITY_CACHE_TTLfor Top Poster API key caching. While functionally equivalent, consider adding aTOP_POSTER_API_KEY_VALIDITY_CACHE_TTLconstant for clarity and independent configuration.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/core/src/db/schemas.tspackages/core/src/main.tspackages/core/src/utils/config.tspackages/core/src/utils/constants.tspackages/core/src/utils/index.tspackages/core/src/utils/top-poster.tspackages/frontend/src/components/menu/addons.tsxpackages/frontend/src/components/menu/services.tsxpackages/frontend/src/components/shared/config-templates-modal.tsxpackages/frontend/src/context/userData.tsxpackages/server/src/app.tspackages/server/src/routes/api/index.tspackages/server/src/routes/api/top-poster.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/server/src/routes/api/top-poster.ts
- packages/core/src/utils/index.ts
- packages/server/src/app.ts
- packages/server/src/routes/api/index.ts
- packages/frontend/src/components/shared/config-templates-modal.tsx
- packages/core/src/utils/constants.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/frontend/src/components/menu/addons.tsx (1)
packages/core/src/main.ts (1)
setUserData(125-127)
packages/core/src/main.ts (4)
packages/core/src/db/schemas.ts (1)
MetaPreview(936-936)packages/core/src/utils/rpdb.ts (1)
RPDB(11-150)packages/core/src/utils/top-poster.ts (1)
TopPoster(13-162)packages/core/src/utils/env.ts (1)
Env(318-2096)
🔇 Additional comments (17)
packages/core/src/utils/config.ts (1)
662-669: LGTM! Migration logic for poster redirect API is correct.The migration safely transfers the legacy
rpdbUseRedirectApivalue to the newusePosterRedirectApifield and cleans up the old property. This aligns with the frontend migration inuserData.tsx.packages/frontend/src/context/userData.tsx (1)
146-152: LGTM! Frontend migration mirrors backend logic.The migration is consistent with
packages/core/src/utils/config.ts, ensuring both client-side and server-side handle the legacyrpdbUseRedirectApifield identically.packages/frontend/src/components/menu/services.tsx (2)
303-326: LGTM! Poster service selector implementation is clean.The TemplateOption-based selector and conditional API key inputs work well together. Consider whether both API keys should coexist when a user switches services, or if the unused key should be cleared for a cleaner user experience.
381-401: Redirect toggle correctly unified for both poster services.The disabled condition properly checks for the presence of either API key, ensuring the toggle is only active when a poster service is configured.
packages/core/src/main.ts (3)
35-36: LGTM! TopPoster import added correctly.
463-469: LGTM! Poster service flag correctly derived from catalog modification.The renamed parameter
applyPosterServiceproperly reflects the broader scope of the feature.
945-971: LGTM! Poster URL construction handles both services correctly.The logic properly:
- Skips items already using poster service URLs
- Constructs redirect URLs with the correct service endpoint
- Falls back to direct poster API calls when redirect is disabled
packages/core/src/utils/top-poster.ts (4)
13-20: LGTM! Constructor validates API key presence.The constructor correctly trims and validates the API key before use.
22-73: LGTM! API key validation now differentiates error types.The implementation correctly distinguishes between:
- Network/connection errors
- Invalid API keys (401)
- Rate limiting (429)
- Unexpected responses
- Malformed JSON
This addresses the concerns from the previous review.
78-127: LGTM! ID parsing handles various formats correctly.The method properly:
- Parses TMDB, IMDB, and TVDB IDs
- Falls back to AnimeDatabase mappings for unsupported ID types
- Correctly excludes TVDB for movies (line 98)
- Uses null initialisation for
idTypeandidValueaddressing the previous review's TypeScript concern
128-161: LGTM! Poster URL retrieval with appropriate caching.The implementation correctly:
- Caches results using a composite key including the API key
- Performs HEAD requests for existence checking when required
- Returns early without checking existence when
checkExistsis falsepackages/core/src/db/schemas.ts (2)
290-312: LGTM! CatalogModification schema correctly handles backward compatibility.The transform merges the legacy
rpdbfield intousePosterService, ensuring existing configurations continue to work whilst enabling the new dual-service approach.
1126-1129: LGTM! TopPosterIsValidResponse schema added.The schema correctly mirrors the RPDBIsValidResponse structure for consistency.
packages/frontend/src/components/menu/addons.tsx (4)
1563-1565: LGTM! Backward-compatible initialization logic.The initialization correctly enables
usePosterServicewhen eitherrpdbApiKeyortopPosterApiKeyis present, ensuring smooth migration for existing RPDB users whilst supporting the new Top Poster service.
2722-2749: LGTM! Consistent UI updates for poster service toggle.The icon rendering and tooltip text have been correctly updated to reflect the new
usePosterServicefield. The generic "Poster Services" label appropriately accommodates both RPDB and Top Poster.
2935-2950: LGTM! Clear and consistent switch control implementation.The label, help text, and state management have been properly updated. The help text clearly mentions both RPDB and Top Poster, making it easy for users to understand the available options.
1563-1565: All references to the oldrpdbfield have been successfully updated.Verification confirms that the renaming from
rpdbtousePosterServiceis consistent across the frontend. The code at lines 1563-1565, 2722-2749, and 2935-2950 correctly usesusePosterService. The legacyrpdbfield is intentionally preserved in the type schema (packages/core/src/db/schemas.ts) for backward compatibility, with a migration layer that maps the old field to the new one. Other references to "rpdb" in the codebase are legitimate and refer to the RPDB poster service class, API keys, or configuration—not the renamed field.
2 participants
Implements top poster api as an additional option for posters alongside RPDB
Closes #555
Summary by CodeRabbit
New Features
Improvements
Compatibility
✏️ Tip: You can customize this high-level summary in your review settings.