feat(requestmodal): warning for not yet released media - #1739
feat(requestmodal): warning for not yet released media#1739rienafairefr wants to merge 1 commit into
Conversation
9384e81 to
b0ae099
Compare
0ede7c8 to
883f8a2
Compare
883f8a2 to
2823e78
Compare
gauthier-th
left a comment
gauthier-th
left a comment
There was a problem hiding this comment.
- I think this should be available under a setting, disabled by default. Not everyone wants to have this warning displayed.
- The same behavior should be implemented for series as well.
2f364c3 to
8323f75
Compare
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
Hello @rienafairefr could you rebase and update your PR with the review given by gauthier ? |
8323f75 to
ec8d9fa
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new boolean setting ChangesWarnNonReleased end-to-end
Sequence DiagramsequenceDiagram
actor User
participant UI as Request Modal
participant Settings as Settings Context
participant API as Backend API
User->>UI: Open request modal / select media
UI->>Settings: Read currentSettings.warnNonReleased
Settings-->>UI: Return warnNonReleased
UI->>UI: Compute regional release or season air dates
alt warnNonReleased enabled AND item not released
UI->>User: Render non-released warning alert
else
UI->>User: No warning shown
end
User->>UI: Submit request
UI->>API: POST request
API-->>UI: Respond
UI-->>User: Show result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
ec8d9fa to
650ea7a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/RequestModal/MovieRequestModal.tsx (2)
72-76: Consider simplifying with nullish coalescing.The nested ternaries work correctly but could be more concise.
♻️ Optional simplification
- const discoverRegion = user?.settings?.discoverRegion - ? user.settings.discoverRegion - : settings.currentSettings.discoverRegion - ? settings.currentSettings.discoverRegion - : 'US'; + const discoverRegion = + user?.settings?.discoverRegion ?? + settings.currentSettings.discoverRegion ?? + 'US';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RequestModal/MovieRequestModal.tsx` around lines 72 - 76, The nested ternary that computes discoverRegion is verbose; replace it with a concise nullish coalescing expression using the existing symbols (user?.settings?.discoverRegion, settings.currentSettings.discoverRegion) so discoverRegion = user?.settings?.discoverRegion ?? settings.currentSettings.discoverRegion ?? 'US' (keep the same resolution order and fallback).
80-82: Simplify thenonTheatricalInTheFuturelogic.The condition
(!nonTheatricalReleases || nonTheatricalReleases.length > 0)is confusing and partially redundant:
- When
nonTheatricalReleasesisundefined, the?.every()returnsundefinedanyway, making the entire expression falsy.- The
!nonTheatricalReleasescheck is thus unnecessary.The intent appears to be: "releases exist AND all are in the future."
♻️ Proposed simplification
const nonTheatricalInTheFuture = - (!nonTheatricalReleases || nonTheatricalReleases.length > 0) && - nonTheatricalReleases?.every((r) => new Date(r.release_date) > new Date()); + nonTheatricalReleases && + nonTheatricalReleases.length > 0 && + nonTheatricalReleases.every((r) => new Date(r.release_date) > new Date());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RequestModal/MovieRequestModal.tsx` around lines 80 - 82, The conditional for nonTheatricalInTheFuture is using a redundant !nonTheatricalReleases check; change the logic to explicitly require that nonTheatricalReleases is an array with length > 0 and that every release’s release_date is in the future. In other words, replace the current expression for nonTheatricalInTheFuture with a check that nonTheatricalReleases is present/array and has length > 0, then call nonTheatricalReleases.every(r => new Date(r.release_date) > new Date()); reference symbols: nonTheatricalInTheFuture, nonTheatricalReleases, and r.release_date.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/RequestModal/TvRequestModal.tsx`:
- Around line 386-390: Remove the stray console.log and stop hardcoding
airDateInTheFuture; compute it from seasonsAirDates instead by mapping
data?.seasons to their airDate (already done in seasonsAirDates) and then
checking whether every non-null airDate is in the future (or at least one is,
matching MovieRequestModal logic) to derive airDateInTheFuture; update the usage
of airDateInTheFuture in TvRequestModal so the unreleased state reflects actual
season air dates rather than always true.
- Around line 508-516: The conditional is calling airDateInTheFuture as a
function which causes a runtime error; change the JSX to use boolean
conjunction: check settings.currentSettings.warnNonReleased &&
airDateInTheFuture && then render the <div> with the Alert (using Alert,
intl.formatMessage, messages.requesttvNotReleased) so the alert only renders
when both booleans are true.
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 56-58: Update the text for the warnNonReleased setting to indicate
it only shows a warning (does not block requests): change the warnNonReleasedTip
value so it reads that Seerr will display a warning banner to requesters if a
title is not yet released, and ensure the visible label warnNonReleased remains
the same; apply the exact same wording change to the i18n key warnNonReleasedTip
in en.json and mirror the updated copy in the settings documentation
(general.md) so UI, locale, and docs stay aligned.
In `@src/i18n/locale/en.json`:
- Line 569: The added i18n key
components.RequestModal.requestmovieNonTheattricalInTheFuture is unused and
inconsistent with MovieRequestModal which formats
components.RequestModal.requestmovieNotReleased; replace or remove the dead key
and add/update components.RequestModal.requestmovieNotReleased in
src/i18n/locale/en.json with the combined message text that covers both "in the
future" and "absent" cases so MovieRequestModal's calls resolve correctly (refer
to MovieRequestModal and the message id
components.RequestModal.requestmovieNotReleased).
---
Nitpick comments:
In `@src/components/RequestModal/MovieRequestModal.tsx`:
- Around line 72-76: The nested ternary that computes discoverRegion is verbose;
replace it with a concise nullish coalescing expression using the existing
symbols (user?.settings?.discoverRegion,
settings.currentSettings.discoverRegion) so discoverRegion =
user?.settings?.discoverRegion ?? settings.currentSettings.discoverRegion ??
'US' (keep the same resolution order and fallback).
- Around line 80-82: The conditional for nonTheatricalInTheFuture is using a
redundant !nonTheatricalReleases check; change the logic to explicitly require
that nonTheatricalReleases is an array with length > 0 and that every release’s
release_date is in the future. In other words, replace the current expression
for nonTheatricalInTheFuture with a check that nonTheatricalReleases is
present/array and has length > 0, then call nonTheatricalReleases.every(r => new
Date(r.release_date) > new Date()); reference symbols: nonTheatricalInTheFuture,
nonTheatricalReleases, and r.release_date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfb30d86-c0c8-475f-a16c-3c929140bede
📒 Files selected for processing (11)
cypress/config/settings.cypress.jsondocs/using-seerr/settings/general.mdseerr-api.ymlserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/RequestModal/MovieRequestModal.tsxsrc/components/RequestModal/TvRequestModal.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/context/SettingsContext.tsxsrc/i18n/locale/en.jsonsrc/pages/_app.tsx
650ea7a to
1b6e19d
Compare
|
Added a similar feature (governed by the same config flag) for TV requests 
|
rienafairefr
changed the title
1b6e19d to
4367d97
Compare
|
Should be ready now, my thanks to the reviewers |
There was a problem hiding this comment.
The TV and movie warning strings should be consistent with each other and use cleaner phrasing.
Currently requesttvNotReleased says "The Air Date is in the future" while requestmovieNotReleased says "This movie release is in the future". These are different in structure, and both are, imo, very awkward.
Suggested replacements:
"components.RequestModal.requestmovieNotReleased": "This movie has not been released yet",
"components.RequestModal.requesttvNotReleased": "This series has not aired yet",
"components.RequestModal.requesttvNoAirDate": "No air date available for this series",The requesttvNoAirDate value ("The Air Date is absent") also should be cleaned up here.
8363e18
2e96e29 to
8363e18
Compare
updated the wording 👍 |
TheCatLady
left a comment
TheCatLady
left a comment
There was a problem hiding this comment.
Some suggestions, mostly to improve strings.
42d7ac8
b98ad24 to
67a308d
Compare
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
Running into exactly the case this solves. Theatrical-only releases show up as requestable in Discover, users request them, and the requests sit there or fail because there's no digital release to grab yet. A warning at the request modal is the right place to catch it. Looks like the only open thread is @TheCatLady's question about titles with missing release metadata, and the It was in the v3.4.0 milestone in June before it got pulled. Anything blocking beyond the conflict? |
67a308d to
025cde9
Compare
github-actions
Bot
removed
the
merge conflict
|
the PR was rebased conflict in the locale json fixed without issues |
That was enough to block it 😉 |
Description
In order to help users decide if or when to request a movie that is not yet released outside of theaters
This shows a warning in the Movie Request Modal that is shown when a movie has no Digital or Physical release, or when the planned Digital or Physical release is in the future.
apologies js code is probably not great (not my main language)
Screenshot (if UI-related)
id: 1136867
id: 1071585
How Has This Been Tested?
pnpm run devtargeting my seerr server instanceUsing TV shows and movies that fit the warning (not yet released)
Checklist:
pnpm buildpnpm i18n:extractIssues Fixed or Closed
Summary by CodeRabbit
New Features
UI
Localization
Documentation