feat(voting): land new feature to enable voting interested/not interested on titles#2642
Open
regexb wants to merge 8 commits intoseerr-team:developfrom
Open
feat(voting): land new feature to enable voting interested/not interested on titles#2642regexb wants to merge 8 commits intoseerr-team:developfrom
regexb wants to merge 8 commits intoseerr-team:developfrom
Conversation
…oting on media requests
…g, and new UI components
…displaying popular media
📝 WalkthroughWalkthroughIntroduces a comprehensive voting feature enabling users to cast "interested" or "not interested" votes on movies and TV shows. Adds Vote entity with database migrations, REST API endpoints for vote creation/deletion/retrieval, vote history tracking, server-wide voting settings, vote-based popularity ranking, and UI components for voting and viewing vote history across media details and user profiles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant API as Vote API
participant DB as Database
User->>Client: Click vote button (interested)
Client->>API: POST /vote {tmdbId, mediaType, actionType}
activate API
API->>DB: Query existing vote
alt Vote exists
DB-->>API: Return existing vote
API->>DB: Update actionType & updatedAt
else No vote found
DB-->>API: Null
API->>DB: Create new Vote
end
DB-->>API: Vote saved
deactivate API
API-->>Client: VoteUpsertResponse {created: bool}
Client->>Client: Revalidate data
Client-->>User: Update UI (vote state)
sequenceDiagram
participant User
participant Client
participant API as Discover API
participant DB as Database
participant TMDB as TMDB
User->>Client: Browse Popular On Server
Client->>API: GET /discover/popular?take=20&skip=0
activate API
API->>DB: Query votes grouped by tmdbId/mediaType
DB-->>API: Vote aggregates (score = interested - 0.5*not_interested)
API->>DB: Fetch media rows filtered by score threshold
DB-->>API: Paginated vote groups
API->>TMDB: Fetch movie/TV details for each result
TMDB-->>API: Media details
API->>API: Map to MovieResult/TvResult
deactivate API
API-->>Client: Paginated results
Client-->>User: Display popular items
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
regexb
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
server/entity/MediaRequest.ts (1)
410-429: Consider extracting duplicated voting logic.The try/catch block wrapping
upsertInterestedVoteis identical in both the movie and TV request branches. While acceptable for now, consider extracting this into a helper to reduce duplication if the voting logic evolves.Also applies to: 561-580
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/entity/MediaRequest.ts` around lines 410 - 429, The duplicated try/catch voting block around MediaRequest.upsertInterestedVote (guarded by settings.main.enableVoting and using requestUser, requestBody.mediaId, requestBody.mediaType) should be extracted into a single helper function (e.g., upsertInterestedVoteForRequest or handleInterestedVote) that performs the await MediaRequest.upsertInterestedVote(...) and the logger.warn call on error (preserving the same log payload including request.id, tmdbId, mediaType and error message). Replace the duplicated blocks in both the movie and TV branches with a call to this helper to centralize error handling and reduce duplication.src/components/UserProfile/VoteHistory/index.tsx (1)
49-120: N+1 fetch pattern for vote item details.Each
VoteHistoryItemtriggers a separate SWR request to fetch movie/TV details. With 20 items per page, this means 20 additional API calls. While SWR's caching and deduplication help with repeated items, this could still cause noticeable loading delays.Consider either:
- Enriching the
/vote/historyAPI response with basic title/poster data- Batching the detail fetches on the client side
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/VoteHistory/index.tsx` around lines 49 - 120, The VoteHistoryItem component causes N+1 requests by calling useSWR(detailsPath) per item; instead stop per-item fetches and either (A) enrich the /vote/history response with title/poster fields and pass them into VoteHistoryItem (add props like details or posterPath/title to VoteHistoryItem and remove useSWR/detailsPath), or (B) perform a single batched details fetch in the parent (e.g., fetchDetailsForTmdbIds or getBatchMediaDetails) that returns a map of tmdbId→MovieDetails|TvDetails, then pass the resolved data into VoteHistoryItem (again removing useSWR/detailsPath). Update VoteHistoryItem to use the provided props (title, posterPath or details) and delete the internal useSWR call.server/routes/discover.ts (1)
800-839: Sequential TMDB API calls may cause latency at scale.Each vote row triggers an individual TMDB API call. With 20 items per page, this means 20 external requests per
/popularcall. WhilePromise.allparallelizes them, this could still cause noticeable latency and TMDB rate limiting under load. Consider caching TMDB responses or using a batch endpoint if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 800 - 839, The current mapping creates one TMDB request per voteRow inside the voteRows.map (using tmdb.getMovie and tmdb.getTvShow) which causes many external calls; refactor mappedResults to first dedupe and group tmdbIds by mediaType (from voteRows), fetch details for each unique id in parallel (using tmdb.getMovie/getTvShow) and build a lookup map, then map voteRows to results by looking up details and calling mapMovieResult/mapTvResult (keep the existing error logging for individual lookup failures); this reduces requests, enables caching on the lookup map, and centralizes external calls instead of calling TMDB per voteRow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 259-261: Update the OpenAPI contract to document that the
enableVoting boolean and the new voting permission together gate all voting
endpoints: add to the enableVoting property description (symbol: enableVoting)
that voting is only available when enableVoting is true and the caller has the
voting permission, and update all /vote* endpoint descriptions (referenced as
/vote endpoints) to state the same constraint and list possible rejection
responses (403 Forbidden when feature disabled or permission missing, plus 401
for unauthenticated), including example response bodies and status codes; ensure
the permission name used in text matches the codebase's permission symbol (the
"voting" or similarly named permission) so consumers can map it to their auth
checks.
- Around line 721-781: The OpenAPI schemas use type: number for discrete fields
causing float generation; update the Vote schema (properties id and tmdbId) and
VoteUpsertRequest (tmdbId) to use type: integer (and ensure examples are
integers), and also change all pagination-related fields (PageInfo and any
schemas with page, take, skip) from type: number to type: integer so IDs and
pagination params are modeled as integers across the contract.
In `@server/entity/DiscoverSlider.ts`:
- Around line 26-32: DiscoverSlider.bootstrapSliders() currently overwrites
persisted admin changes by setting existingSlider.order and calling
sliderRepository.save when a built-in slider's order differs; stop mutating
persisted built-in order during startup: remove the branch that assigns
existingSlider.order = slider.order and the subsequent sliderRepository.save
call in bootstrapSliders(), and instead implement a separate one-time
migration/backfill routine (not in bootstrapSliders) that shifts existing rows
to accommodate new built-in insertions; alternatively, if immediate change is
required, only set order when existingSlider.order is null/undefined (leave
non-null values untouched) and perform any shifting logic in a dedicated
migration function.
In `@server/routes/discover.ts`:
- Around line 845-850: The response currently returns totalResults that can
differ from the length of results when TMDB lookups are filtered out; update the
handler that builds the response (variables page, itemsPerPage, totalResults,
results) to ensure pagination reflects the actual returned items — either
recompute totalResults = results.length (and totalPages =
Math.ceil(results.length / itemsPerPage)) after filtering, or return both
originalTotal and filteredTotal so consumers can distinguish them; make the
change where results are filtered (the block that filters TMDB lookups) and
adjust the final JSON construction to use the corrected count for
totalResults/totalPages or add a filteredCount field.
- Line 784: The code currently computes totalResults by materializing all
grouped rows with groupedVotes.clone().getRawMany().length which is inefficient;
replace this with a COUNT query instead: build a subquery from groupedVotes (or
reuse groupedVotes.clone()) and select COUNT(*) as total (or use QueryBuilder's
getRawOne on that COUNT) so the database returns a single number rather than all
rows; update the variable totalResults to use that COUNT result (referencing
groupedVotes and totalResults) and ensure pagination still uses the original
groupedVotes query for fetching the page.
In `@server/routes/vote.ts`:
- Around line 114-166: The POST upsert handler in voteRoutes.post uses findOne
then save which creates a TOCTOU race that can surface a unique-constraint
error; replace the two-step logic in voteRoutes.post (the findOne +
voteRepository.save paths) with a single atomic upsert (e.g.,
voteRepository.upsert or an INSERT...ON CONFLICT equivalent) to create-or-update
the Vote, or if you prefer to keep save(), catch the unique constraint DB error
from voteRepository.save, then query for the existing Vote (by tmdbId,
mediaType, user.id) and return that with created: false; update the handler’s
error path to distinguish constraint violations from generic failures
(referencing voteCreate.parse, existingVote, voteRepository.save, and the Vote
constructor).
In `@src/i18n/locale/en.json`:
- Around line 458-459: Update the "components.PermissionEdit.voteDescription"
translation value to mention that the permission covers both expressing interest
and marking items as "not interested" (i.e., both positive and negative vote
actions); locate the JSON key "components.PermissionEdit.voteDescription" (and
related key "components.PermissionEdit.vote") and replace the current
description string with wording that explicitly includes both types of votes.
---
Nitpick comments:
In `@server/entity/MediaRequest.ts`:
- Around line 410-429: The duplicated try/catch voting block around
MediaRequest.upsertInterestedVote (guarded by settings.main.enableVoting and
using requestUser, requestBody.mediaId, requestBody.mediaType) should be
extracted into a single helper function (e.g., upsertInterestedVoteForRequest or
handleInterestedVote) that performs the await
MediaRequest.upsertInterestedVote(...) and the logger.warn call on error
(preserving the same log payload including request.id, tmdbId, mediaType and
error message). Replace the duplicated blocks in both the movie and TV branches
with a call to this helper to centralize error handling and reduce duplication.
In `@server/routes/discover.ts`:
- Around line 800-839: The current mapping creates one TMDB request per voteRow
inside the voteRows.map (using tmdb.getMovie and tmdb.getTvShow) which causes
many external calls; refactor mappedResults to first dedupe and group tmdbIds by
mediaType (from voteRows), fetch details for each unique id in parallel (using
tmdb.getMovie/getTvShow) and build a lookup map, then map voteRows to results by
looking up details and calling mapMovieResult/mapTvResult (keep the existing
error logging for individual lookup failures); this reduces requests, enables
caching on the lookup map, and centralizes external calls instead of calling
TMDB per voteRow.
In `@src/components/UserProfile/VoteHistory/index.tsx`:
- Around line 49-120: The VoteHistoryItem component causes N+1 requests by
calling useSWR(detailsPath) per item; instead stop per-item fetches and either
(A) enrich the /vote/history response with title/poster fields and pass them
into VoteHistoryItem (add props like details or posterPath/title to
VoteHistoryItem and remove useSWR/detailsPath), or (B) perform a single batched
details fetch in the parent (e.g., fetchDetailsForTmdbIds or
getBatchMediaDetails) that returns a map of tmdbId→MovieDetails|TvDetails, then
pass the resolved data into VoteHistoryItem (again removing useSWR/detailsPath).
Update VoteHistoryItem to use the provided props (title, posterPath or details)
and delete the internal useSWR call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fffb4cf0-d79f-4a19-b090-b3d1da6db8da
📒 Files selected for processing (33)
seerr-api.ymlserver/constants/discover.tsserver/entity/DiscoverSlider.tsserver/entity/MediaRequest.tsserver/entity/User.tsserver/entity/Vote.tsserver/interfaces/api/settingsInterfaces.tsserver/interfaces/api/voteInterfaces.tsserver/lib/settings/index.tsserver/migration/postgres/1772153885378-AddVoting.tsserver/migration/sqlite/1772153871569-AddVoting.tsserver/models/Movie.tsserver/models/Tv.tsserver/routes/discover.tsserver/routes/index.tsserver/routes/movie.tsserver/routes/tv.tsserver/routes/vote.tssrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/PopularOnServer.tsxsrc/components/Discover/constants.tssrc/components/Discover/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PermissionEdit/index.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserProfile/VoteHistory/index.tsxsrc/components/UserProfile/index.tsxsrc/context/SettingsContext.tsxsrc/i18n/locale/en.jsonsrc/pages/_app.tsxsrc/pages/discover/popular.tsxsrc/pages/profile/votes.tsx
Comment on lines
+259
to
+261
| enableVoting: | ||
| type: boolean | ||
| example: false |
There was a problem hiding this comment.
Document the voting permission and feature gate in the contract.
These new endpoints/settings are missing the key runtime constraint from the PR: voting depends on both enableVoting and the new voting permission. As written, /vote* looks like a normal authenticated surface, so API consumers have no documented reason for a 403/disabled response. Please add that requirement to the property/endpoint descriptions and document the expected rejection response(s).
📝 Suggested contract shape
enableVoting:
type: boolean
example: false
+ description: Enables the voting feature server-wide. Users must still have the `VOTE` permission.
/vote:
post:
summary: Create or update vote
- description: Create a vote for the current user, or update an existing vote for the same media.
+ description: |
+ Create a vote for the current user, or update an existing vote for the same media.
+ Requires the `VOTE` permission and `enableVoting` to be enabled.
responses:
'200':
description: Existing vote updated
'201':
description: Vote created
+ '403':
+ description: Voting is disabled or the user lacks the `VOTE` permissionAlso applies to: 718-720, 4865-4982
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@seerr-api.yml` around lines 259 - 261, Update the OpenAPI contract to
document that the enableVoting boolean and the new voting permission together
gate all voting endpoints: add to the enableVoting property description (symbol:
enableVoting) that voting is only available when enableVoting is true and the
caller has the voting permission, and update all /vote* endpoint descriptions
(referenced as /vote endpoints) to state the same constraint and list possible
rejection responses (403 Forbidden when feature disabled or permission missing,
plus 401 for unauthenticated), including example response bodies and status
codes; ensure the permission name used in text matches the codebase's permission
symbol (the "voting" or similarly named permission) so consumers can map it to
their auth checks.
Comment on lines
+721
to
+781
| Vote: | ||
| type: object | ||
| properties: | ||
| id: | ||
| type: number | ||
| example: 1 | ||
| tmdbId: | ||
| type: number | ||
| example: 872585 | ||
| mediaType: | ||
| type: string | ||
| enum: [movie, tv] | ||
| actionType: | ||
| type: string | ||
| enum: [interested, not_interested] | ||
| createdAt: | ||
| type: string | ||
| example: '2020-09-12T10:00:27.000Z' | ||
| updatedAt: | ||
| type: string | ||
| example: '2020-09-12T10:05:27.000Z' | ||
| VoteLookupResponse: | ||
| type: object | ||
| properties: | ||
| vote: | ||
| nullable: true | ||
| allOf: | ||
| - $ref: '#/components/schemas/Vote' | ||
| VoteHistoryResponse: | ||
| type: object | ||
| properties: | ||
| pageInfo: | ||
| $ref: '#/components/schemas/PageInfo' | ||
| results: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/Vote' | ||
| VoteUpsertRequest: | ||
| type: object | ||
| properties: | ||
| tmdbId: | ||
| type: number | ||
| example: 872585 | ||
| mediaType: | ||
| type: string | ||
| enum: [movie, tv] | ||
| actionType: | ||
| type: string | ||
| enum: [interested, not_interested] | ||
| required: | ||
| - tmdbId | ||
| - mediaType | ||
| - actionType | ||
| VoteUpsertResponse: | ||
| allOf: | ||
| - $ref: '#/components/schemas/Vote' | ||
| - type: object | ||
| properties: | ||
| created: | ||
| type: boolean | ||
| example: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l seerr-api.ymlRepository: seerr-team/seerr
Length of output: 78
🏁 Script executed:
# Check the main range (721-781)
sed -n '721,781p' seerr-api.ymlRepository: seerr-team/seerr
Length of output: 1516
🏁 Script executed:
# Check other ranges mentioned
sed -n '4896,4908p' seerr-api.ymlRepository: seerr-team/seerr
Length of output: 345
🏁 Script executed:
# Check path params ranges
sed -n '4953,4956p' seerr-api.yml
sed -n '4976,4979p' seerr-api.ymlRepository: seerr-team/seerr
Length of output: 241
🏁 Script executed:
# Check discover/popular range
sed -n '6202,6207p' seerr-api.ymlRepository: seerr-team/seerr
Length of output: 189
Use integer for IDs and pagination fields.
The new contract models discrete values like id, tmdbId, take, skip, and page as number. In OpenAPI, this generates floating-point types in SDKs and permits invalid inputs like 1.5 for TMDB IDs or page numbers. Change these to integer.
Applies to lines 724-725, 761-763, 4896-4908, 4953-4956, 4976-4979, and 6202-6207.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@seerr-api.yml` around lines 721 - 781, The OpenAPI schemas use type: number
for discrete fields causing float generation; update the Vote schema (properties
id and tmdbId) and VoteUpsertRequest (tmdbId) to use type: integer (and ensure
examples are integers), and also change all pagination-related fields (PageInfo
and any schemas with page, take, skip) from type: number to type: integer so IDs
and pagination params are modeled as integers across the contract.
Comment on lines
+26
to
+32
| } else if ( | ||
| existingSlider.isBuiltIn && | ||
| typeof slider.order === 'number' && | ||
| existingSlider.order !== slider.order | ||
| ) { | ||
| existingSlider.order = slider.order; | ||
| await sliderRepository.save(existingSlider); |
There was a problem hiding this comment.
Don't reset persisted built-in slider order during bootstrap.
DiscoverSlider.bootstrapSliders() runs on every startup, and the discover settings route already persists admin-defined order for built-in sliders. This branch will silently overwrite those customizations on every boot. The new built-in insertion needs a one-time migration/backfill that shifts existing rows instead of mutating order here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/entity/DiscoverSlider.ts` around lines 26 - 32,
DiscoverSlider.bootstrapSliders() currently overwrites persisted admin changes
by setting existingSlider.order and calling sliderRepository.save when a
built-in slider's order differs; stop mutating persisted built-in order during
startup: remove the branch that assigns existingSlider.order = slider.order and
the subsequent sliderRepository.save call in bootstrapSliders(), and instead
implement a separate one-time migration/backfill routine (not in
bootstrapSliders) that shifts existing rows to accommodate new built-in
insertions; alternatively, if immediate change is required, only set order when
existingSlider.order is null/undefined (leave non-null values untouched) and
perform any shifting logic in a dedicated migration function.
| END) > 0` | ||
| ); | ||
|
|
||
| const totalResults = (await groupedVotes.clone().getRawMany()).length; |
There was a problem hiding this comment.
Inefficient total count calculation.
Fetching all grouped rows just to count them (getRawMany().length) is wasteful, especially as the vote table grows. Consider using a COUNT query or wrapping the grouped query as a subquery.
💡 More efficient counting approach
- const totalResults = (await groupedVotes.clone().getRawMany()).length;
+ // Use a subquery count instead of fetching all rows
+ const countResult = await voteRepository
+ .createQueryBuilder()
+ .select('COUNT(*)', 'count')
+ .from(`(${groupedVotes.clone().getQuery()})`, 'grouped')
+ .setParameters(groupedVotes.getParameters())
+ .getRawOne();
+ const totalResults = Number(countResult?.count ?? 0);Alternatively, if the above doesn't work cleanly with TypeORM, you could cache/paginate differently or accept the current approach for now given expected vote volumes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const totalResults = (await groupedVotes.clone().getRawMany()).length; | |
| // Use a subquery count instead of fetching all rows | |
| const countResult = await voteRepository | |
| .createQueryBuilder() | |
| .select('COUNT(*)', 'count') | |
| .from(`(${groupedVotes.clone().getQuery()})`, 'grouped') | |
| .setParameters(groupedVotes.getParameters()) | |
| .getRawOne(); | |
| const totalResults = Number(countResult?.count ?? 0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/discover.ts` at line 784, The code currently computes
totalResults by materializing all grouped rows with
groupedVotes.clone().getRawMany().length which is inefficient; replace this with
a COUNT query instead: build a subquery from groupedVotes (or reuse
groupedVotes.clone()) and select COUNT(*) as total (or use QueryBuilder's
getRawOne on that COUNT) so the database returns a single number rather than all
rows; update the variable totalResults to use that COUNT result (referencing
groupedVotes and totalResults) and ensure pagination still uses the original
groupedVotes query for fetching the page.
Comment on lines
+845
to
+850
| return res.status(200).json({ | ||
| page, | ||
| totalPages: Math.ceil(totalResults / itemsPerPage), | ||
| totalResults, | ||
| results, | ||
| }); |
There was a problem hiding this comment.
totalResults may not match actual results count.
If TMDB lookups fail for some items (filtered out at line 841-843), the returned results array will have fewer items than totalResults indicates. This could cause pagination issues where users see fewer items than expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/discover.ts` around lines 845 - 850, The response currently
returns totalResults that can differ from the length of results when TMDB
lookups are filtered out; update the handler that builds the response (variables
page, itemsPerPage, totalResults, results) to ensure pagination reflects the
actual returned items — either recompute totalResults = results.length (and
totalPages = Math.ceil(results.length / itemsPerPage)) after filtering, or
return both originalTotal and filteredTotal so consumers can distinguish them;
make the change where results are filtered (the block that filters TMDB lookups)
and adjust the final JSON construction to use the corrected count for
totalResults/totalPages or add a filteredCount field.
Comment on lines
+114
to
+166
| voteRoutes.post<never, VoteUpsertResponse>('/', async (req, res, next) => { | ||
| // Satisfy typescript here. User is set by router middleware. | ||
| if (!req.user) { | ||
| return next({ status: 500, message: 'User missing from request.' }); | ||
| } | ||
|
|
||
| try { | ||
| const { tmdbId, mediaType, actionType } = voteCreate.parse(req.body); | ||
| const voteRepository = getRepository(Vote); | ||
| const existingVote = await voteRepository.findOne({ | ||
| where: { | ||
| user: { id: req.user.id }, | ||
| tmdbId, | ||
| mediaType, | ||
| }, | ||
| }); | ||
|
|
||
| if (existingVote) { | ||
| existingVote.actionType = actionType; | ||
| existingVote.updatedAt = new Date(); | ||
| const updatedVote = await voteRepository.save(existingVote); | ||
|
|
||
| return res.status(200).json({ | ||
| ...updatedVote, | ||
| created: false, | ||
| }); | ||
| } | ||
|
|
||
| const vote = await voteRepository.save( | ||
| new Vote({ | ||
| user: req.user, | ||
| tmdbId, | ||
| mediaType, | ||
| actionType, | ||
| }) | ||
| ); | ||
|
|
||
| return res.status(201).json({ | ||
| ...vote, | ||
| created: true, | ||
| }); | ||
| } catch (e) { | ||
| if (e instanceof ZodError) { | ||
| return next({ status: 400, message: 'Invalid vote payload.' }); | ||
| } | ||
| logger.debug('Something went wrong creating or updating a vote.', { | ||
| label: 'API', | ||
| errorMessage: e instanceof Error ? e.message : undefined, | ||
| }); | ||
|
|
||
| return next({ status: 500, message: 'Unable to submit vote.' }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Race condition in POST upsert could return 500 instead of handling gracefully.
The findOne followed by save pattern has a TOCTOU window. If two concurrent requests attempt to create the same vote, one will hit the unique constraint and return a generic 500 error. Consider using an INSERT ... ON CONFLICT pattern via upsert() or catching the unique constraint violation specifically to return the existing vote.
💡 Alternative approach using TypeORM upsert
try {
const { tmdbId, mediaType, actionType } = voteCreate.parse(req.body);
const voteRepository = getRepository(Vote);
- const existingVote = await voteRepository.findOne({
- where: {
- user: { id: req.user.id },
- tmdbId,
- mediaType,
- },
- });
-
- if (existingVote) {
- existingVote.actionType = actionType;
- existingVote.updatedAt = new Date();
- const updatedVote = await voteRepository.save(existingVote);
-
- return res.status(200).json({
- ...updatedVote,
- created: false,
- });
- }
-
- const vote = await voteRepository.save(
- new Vote({
- user: req.user,
+
+ // Use upsert to handle race conditions atomically
+ await voteRepository.upsert(
+ {
+ user: { id: req.user.id },
tmdbId,
mediaType,
actionType,
- })
+ updatedAt: new Date(),
+ },
+ ['user', 'tmdbId', 'mediaType']
);
-
- return res.status(201).json({
- ...vote,
- created: true,
- });
+
+ const vote = await voteRepository.findOneOrFail({
+ where: { user: { id: req.user.id }, tmdbId, mediaType },
+ });
+
+ return res.status(200).json({ ...vote, created: false });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/vote.ts` around lines 114 - 166, The POST upsert handler in
voteRoutes.post uses findOne then save which creates a TOCTOU race that can
surface a unique-constraint error; replace the two-step logic in voteRoutes.post
(the findOne + voteRepository.save paths) with a single atomic upsert (e.g.,
voteRepository.upsert or an INSERT...ON CONFLICT equivalent) to create-or-update
the Vote, or if you prefer to keep save(), catch the unique constraint DB error
from voteRepository.save, then query for the existing Vote (by tmdbId,
mediaType, user.id) and return that with created: false; update the handler’s
error path to distinguish constraint violations from generic failures
(referencing voteCreate.parse, existingVote, voteRepository.save, and the Vote
constructor).
Comment on lines
+458
to
+459
| "components.PermissionEdit.vote": "Vote", | ||
| "components.PermissionEdit.voteDescription": "Grant permission to express interest in media without creating requests.", |
There was a problem hiding this comment.
Update the permission description to cover both vote actions.
This permission also enables “not interested” votes, but the current copy only mentions expressing interest.
✏️ Suggested copy
- "components.PermissionEdit.voteDescription": "Grant permission to express interest in media without creating requests.",
+ "components.PermissionEdit.voteDescription": "Grant permission to express interest or disinterest in media without creating requests.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "components.PermissionEdit.vote": "Vote", | |
| "components.PermissionEdit.voteDescription": "Grant permission to express interest in media without creating requests.", | |
| "components.PermissionEdit.vote": "Vote", | |
| "components.PermissionEdit.voteDescription": "Grant permission to express interest or disinterest in media without creating requests.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/locale/en.json` around lines 458 - 459, Update the
"components.PermissionEdit.voteDescription" translation value to mention that
the permission covers both expressing interest and marking items as "not
interested" (i.e., both positive and negative vote actions); locate the JSON key
"components.PermissionEdit.voteDescription" (and related key
"components.PermissionEdit.vote") and replace the current description string
with wording that explicitly includes both types of votes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
New feature proposal: Voting on media
Background
I had a need on my server to start purging media in order to save space but I didn't know what was going to be safe to remove. I have access to some watch data through tautulli, but it didn't give me the whole picture of what my users actually were interested in watching collectively. I started building something standalone, but realized it might actually be something more community members want to enable. The long term goal is: 1) Add voting support and a discover page row for popular media by votes. 2) Add a "Swipe to discover" feed that provides a fast UI/UX for users to quickly cast votes with a feed that's configurable by the admin. 3) Expose endpoints for the API key to be used to fetch aggregated voting information to be fed into other systems or scripts for automation.
Features
Use of AI
AI through Cursor Agents was heavily used for this development but not without careful hand holding, planning, and using existing architecture decisions seen through other feature implementations. I want to be honest, I typed hardly any of the code myself, but every decision and line was reviewed. I understand and guided each structural detail and how and why each piece of this feature works and is implemented the way it is. That being said, please review with an extra dose of skepticism.
Known quirks
This adds a new permission. I noticed that even if this is set to true for the default, it will not update existing users. Looking through existing commits where permissions were added, it seems like this is always the case and there isn't a way to initialize a new permission on existing users. This makes turning on this feature for existing users a pain (mitigated slightly by "Bulk Edit"). After turning on voting, every user needs to have the "voting" permission also enabled.
How Has This Been Tested?
Screenshots / Logs (if applicable)
Vote Buttons
Recent Votes (user profile)
Vote History (user)
Popular On This Server
Not Included but fast followup work:
Code already started and being tested: https://github.com/regexb/seerr/tree/feature-discover-swipe
Swipe to vote low profile CTA
Swipe to vote page
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Release Notes