feat(requests): allow users to view requester on media details page#2866
Open
danielkinahan wants to merge 4 commits intoseerr-team:developfrom
Open
feat(requests): allow users to view requester on media details page#2866danielkinahan wants to merge 4 commits intoseerr-team:developfrom
danielkinahan wants to merge 4 commits intoseerr-team:developfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a new React component MediaRequestSummary and integrated it into MovieDetails and TvDetails to display requester, request date, status badges (and 4K indicator). Requests passed are either all requests (for users with MANAGE_REQUESTS or REQUEST_VIEW) or filtered to the current user's requests. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(52,152,219,0.5)
participant Browser
end
rect rgba(46,204,113,0.5)
participant MovieDetails/TvDetails
end
rect rgba(155,89,182,0.5)
participant MediaRequestSummary
end
rect rgba(241,196,15,0.5)
participant Auth/Permissions
end
Browser->>MovieDetails/TvDetails: navigate to media page
MovieDetails/TvDetails->>Auth/Permissions: check user permissions (MANAGE_REQUESTS / REQUEST_VIEW)
Auth/Permissions-->>MovieDetails/TvDetails: permission scope
MovieDetails/TvDetails->>MediaRequestSummary: render with `requests` (full or filtered) and `currentUserId`
MediaRequestSummary->>MediaRequestSummary: sort requests, map status to badges, format dates
MediaRequestSummary-->>Browser: render requester links, avatars, badges
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
🧹 Nitpick comments (1)
src/components/MediaRequestSummary/index.tsx (1)
44-70: Consolidate status mapping to prevent future drift.
statusMessageandstatusBadgeTypeare derived from duplicateswitchlogic on the same enum. Centralizing this in one map/helper will keep status text and badge color in sync as statuses evolve.♻️ Suggested refactor
+const requestStatusMeta = { + [MediaRequestStatus.APPROVED]: { + message: globalMessages.approved, + badgeType: 'success', + }, + [MediaRequestStatus.DECLINED]: { + message: globalMessages.declined, + badgeType: 'danger', + }, + [MediaRequestStatus.FAILED]: { + message: globalMessages.failed, + badgeType: 'danger', + }, + [MediaRequestStatus.COMPLETED]: { + message: globalMessages.completed, + badgeType: 'success', + }, + [MediaRequestStatus.PENDING]: { + message: globalMessages.pending, + badgeType: 'warning', + }, +} as const; ... - const statusMessage = (() => { - switch (request.status) { - case MediaRequestStatus.APPROVED: - return intl.formatMessage(globalMessages.approved); - case MediaRequestStatus.DECLINED: - return intl.formatMessage(globalMessages.declined); - case MediaRequestStatus.FAILED: - return intl.formatMessage(globalMessages.failed); - case MediaRequestStatus.COMPLETED: - return intl.formatMessage(globalMessages.completed); - default: - return intl.formatMessage(globalMessages.pending); - } - })(); - - const statusBadgeType = (() => { - switch (request.status) { - case MediaRequestStatus.APPROVED: - case MediaRequestStatus.COMPLETED: - return 'success'; - case MediaRequestStatus.DECLINED: - case MediaRequestStatus.FAILED: - return 'danger'; - default: - return 'warning'; - } - })(); + const statusMeta = + requestStatusMeta[request.status] ?? requestStatusMeta[MediaRequestStatus.PENDING]; + const statusMessage = intl.formatMessage(statusMeta.message); + const statusBadgeType = statusMeta.badgeType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MediaRequestSummary/index.tsx` around lines 44 - 70, The duplicate switch logic for computing statusMessage and statusBadgeType from MediaRequestStatus should be centralized: create a single helper (e.g., getMediaRequestStatusInfo or STATUS_MAP) that accepts a MediaRequestStatus and returns both the localized message (using intl + globalMessages) and the badge type string; replace the current statusMessage and statusBadgeType computed blocks with calls to that helper (use its .message and .badgeType), ensuring you reference MediaRequestStatus, intl, and globalMessages inside the helper so both values stay in sync as statuses change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/MediaRequestSummary/index.tsx`:
- Around line 44-70: The duplicate switch logic for computing statusMessage and
statusBadgeType from MediaRequestStatus should be centralized: create a single
helper (e.g., getMediaRequestStatusInfo or STATUS_MAP) that accepts a
MediaRequestStatus and returns both the localized message (using intl +
globalMessages) and the badge type string; replace the current statusMessage and
statusBadgeType computed blocks with calls to that helper (use its .message and
.badgeType), ensuring you reference MediaRequestStatus, intl, and globalMessages
inside the helper so both values stay in sync as statuses change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f10119d-f175-4952-a1a5-27805527278f
📒 Files selected for processing (3)
src/components/MediaRequestSummary/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/TvDetails/index.tsx
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
This change adds the ability for a user with View Requests permission to view the requests of other users on the media details page (both TV and movies). I added it as a seperate card under the media-facts card, but I'd like feedback on the placement. I will include screenshots.
Disclaimer on AI
I used AI to search the codebase and review this code for feedback.
How Has This Been Tested?
I copied my db of my instance and ran tests with it, so I could have data. I have only run this in dev mode. I have tried this with a piece of media that has no requests, one and multiple (creates new cards for each).
Screenshots / Logs (if applicable)
One request
Multiple requests
No requests (card doesnt show)
Mobile view of multiple requests
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit