Open
Conversation
|
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:
📝 WalkthroughWalkthroughIntroduces managed sponsor page customization workflow by adding Redux action types and thunk for single-page fetch, refactoring save logic with conditional update/create flows and distinct normalization, updating the sponsor pages tab UI to support customize actions with separate managed/customized popups, and adding corresponding i18n keys and test coverage. Changes
Sequence DiagramsequenceDiagram
actor User
participant Tab as Sponsor Pages Tab
participant Redux as Redux Store
participant API as Backend API
participant Popup as PageTemplatePopup
User->>Tab: Click customize on managed page
activate Tab
Tab->>Redux: dispatch getSponsorManagedPage(pageId)
activate Redux
Redux->>API: GET /show-pages/{pageId}?expand=modules
activate API
API-->>Redux: Page data + modules
deactivate API
Redux->>Redux: Handle RECEIVE_SPONSOR_MANAGED_PAGE<br/>(normalize upload_deadline)
Redux-->>Tab: Updated state
deactivate Redux
Tab->>Tab: Open managedPagePopup
Tab->>Popup: Render with managed page data + title
deactivate Tab
activate Popup
User->>Popup: Modify page settings and save
Popup->>Redux: dispatch saveSponsorManagedPage(entity)
activate Redux
Redux->>API: PUT /show-pages/{pageId}/customization<br/>(with normalized payload)
activate API
API-->>Redux: Success response
deactivate API
Redux->>Redux: Dispatch SPONSOR_MANAGED_PAGE_UPDATED
Redux-->>Popup: Success notification
deactivate Redux
Popup->>Tab: Close popup
deactivate Popup
activate Tab
Tab->>Redux: dispatch getSponsorManagedPages(hideArchived)
Tab->>Redux: dispatch getSponsorCustomizedPages(hideArchived)
activate Redux
Redux->>API: Fetch refreshed lists
activate API
API-->>Redux: Updated pages
deactivate API
Redux-->>Tab: Updated grids
deactivate Redux
Tab->>User: Display updated pages
deactivate Tab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-pages-actions.js (1)
282-293:⚠️ Potential issue | 🔴 Critical
normalizeSponsorManagedPagenow throws before the create request.Line 287 calls
includesonnormalizedEntity.allowed_add_ons, but that field is never initialized here. The “add managed page from template” path will fail with aTypeErrorbefore the POST is sent.Proposed fix
const normalizeSponsorManagedPage = (entity) => { const normalizedEntity = { show_page_ids: entity.pages }; + const selectedAddOns = entity.add_ons ?? []; - if (normalizedEntity.allowed_add_ons.includes("all")) { + if (selectedAddOns.includes("all")) { normalizedEntity.apply_to_all_add_ons = true; normalizedEntity.allowed_add_ons = []; } else { - normalizedEntity.allowed_add_ons = entity.add_ons.map((a) => a.id); + normalizedEntity.allowed_add_ons = selectedAddOns.map((a) => a.id ?? a); normalizedEntity.apply_to_all_add_ons = false; } return normalizedEntity; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-pages-actions.js` around lines 282 - 293, normalizeSponsorManagedPage currently calls normalizedEntity.allowed_add_ons.includes(...) before allowed_add_ons is initialized; fix by initializing allowed_add_ons from entity.add_ons (e.g., normalizedEntity.allowed_add_ons = (entity.add_ons || []).map(a => a.id)) before the if-check, then perform the includes("all") test and set apply_to_all_add_ons and allowed_add_ons (to [] when "all" is present) accordingly; ensure you handle a missing/null entity.add_ons by defaulting to an empty array so the add-managed-page flow doesn’t throw.
🤖 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/actions/sponsor-pages-actions.js`:
- Around line 298-305: The call to normalizeSelectAllField inside
normalizeSponsorManagedPageToCustomize is passing the wrong flag name
("apply_to_all_types"), causing the companion flag for allowed_add_ons to be
emitted incorrectly; change the third argument to "apply_to_all_add_ons" so
normalizeSelectAllField(serialization of allowed_add_ons) writes the expected
apply_to_all_add_ons flag (update the call in
normalizeSponsorManagedPageToCustomize that currently references
allowed_add_ons).
In `@src/pages/sponsors/sponsor-pages-tab/index.js`:
- Around line 274-292: The popup is being closed in the .finally block causing
edits to be lost on save failure and skipping cleanup; change the flow in
handleSaveManagedPage so setOpenPopup(null) is called only on successful save
(inside the .then chain after refresh calls) and also invoke
handleClosePagePopup() there to clear currentEditPage, while leaving the popup
open on .catch so the user can correct errors; reference saveSponsorManagedPage,
getSponsorManagedPages, getSponsorCustomizedPages, setOpenPopup, and
handleClosePagePopup when making this change.
---
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 282-293: normalizeSponsorManagedPage currently calls
normalizedEntity.allowed_add_ons.includes(...) before allowed_add_ons is
initialized; fix by initializing allowed_add_ons from entity.add_ons (e.g.,
normalizedEntity.allowed_add_ons = (entity.add_ons || []).map(a => a.id)) before
the if-check, then perform the includes("all") test and set apply_to_all_add_ons
and allowed_add_ons (to [] when "all" is present) accordingly; ensure you handle
a missing/null entity.add_ons by defaulting to an empty array so the
add-managed-page flow doesn’t throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76ed2285-9141-418e-bda7-94bd694fbb28
📒 Files selected for processing (4)
src/actions/sponsor-pages-actions.jssrc/i18n/en.jsonsrc/pages/sponsors/sponsor-pages-tab/index.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.js
Comment on lines
+274
to
+292
| const handleSaveManagedPage = (entity) => { | ||
| saveSponsorManagedPage(entity) | ||
| .then(() => { | ||
| getSponsorManagedPages( | ||
| term, | ||
| DEFAULT_CURRENT_PAGE, | ||
| managedPages.perPage, | ||
| managedPages.order, | ||
| managedPages.orderDir | ||
| ); | ||
| getSponsorCustomizedPages( | ||
| term, | ||
| DEFAULT_CURRENT_PAGE, | ||
| customizedPages.perPage, | ||
| customizedPages.order, | ||
| customizedPages.orderDir | ||
| ); | ||
| }) | ||
| .finally(() => setOpenPopup(null)); |
There was a problem hiding this comment.
Don't close the managed-page editor in finally.
Line 292 closes the dialog even when saveSponsorManagedPage fails, which drops the user's edits. It also skips handleClosePagePopup, so currentEditPage stays populated and can leak into the next “New Page” open.
Proposed fix
const handleSaveManagedPage = (entity) => {
saveSponsorManagedPage(entity)
.then(() => {
getSponsorManagedPages(
term,
DEFAULT_CURRENT_PAGE,
managedPages.perPage,
managedPages.order,
managedPages.orderDir
);
getSponsorCustomizedPages(
term,
DEFAULT_CURRENT_PAGE,
customizedPages.perPage,
customizedPages.order,
customizedPages.orderDir
);
- })
- .finally(() => setOpenPopup(null));
+ handleClosePagePopup();
+ });
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-pages-tab/index.js` around lines 274 - 292, The
popup is being closed in the .finally block causing edits to be lost on save
failure and skipping cleanup; change the flow in handleSaveManagedPage so
setOpenPopup(null) is called only on successful save (inside the .then chain
after refresh calls) and also invoke handleClosePagePopup() there to clear
currentEditPage, while leaving the popup open on .catch so the user can correct
errors; reference saveSponsorManagedPage, getSponsorManagedPages,
getSponsorCustomizedPages, setOpenPopup, and handleClosePagePopup when making
this change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/pages/sponsors/sponsor-pages-tab/index.js (1)
199-200: Remove the temporary console log.
console.log("CHECK!", item)will run on every Customize click and dump the row payload into the browser console.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-pages-tab/index.js` around lines 199 - 200, Remove the temporary console.log("CHECK!", item) that prints the row payload on every Customize click; inside the click handler where getSponsorManagedPage(item.id).then(() => setOpenPopup("managedPagePopup")) is invoked, delete the console.log statement (or replace it with a controlled debug/logging call if persistent logging is required) so only getSponsorManagedPage and setOpenPopup remain.src/pages/sponsors/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js (1)
305-307: Assert the refresh arguments, not only the call counts.These expectations still pass if the save handler rebuilds either grid with the wrong filters or pagination. Verifying the last
getSponsorManagedPages/getSponsorCustomizedPagesarguments here would catch the missinghideArchivedregression inhandleSaveManagedPage.Example assertion upgrade
await waitFor(() => { - expect(getSponsorManagedPages).toHaveBeenCalledTimes(2); - expect(getSponsorCustomizedPages).toHaveBeenCalledTimes(2); + expect(getSponsorManagedPages).toHaveBeenLastCalledWith( + "", + 1, + 10, + "id", + 1, + false + ); + expect(getSponsorCustomizedPages).toHaveBeenLastCalledWith( + "", + 1, + 10, + "id", + 1, + false + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js` around lines 305 - 307, The test currently only checks call counts for getSponsorManagedPages and getSponsorCustomizedPages; change it to assert the last call arguments to ensure the correct filters/pagination are passed (e.g., verify that the last call to getSponsorManagedPages and getSponsorCustomizedPages includes the expected hideArchived flag and other filter/pagination params), and update the assertions around the save flow (handleSaveManagedPage) to use toHaveBeenLastCalledWith-style checks or expect.objectContaining on the last call args rather than relying solely on toHaveBeenCalledTimes.
🤖 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/pages/sponsors/sponsor-pages-tab/index.js`:
- Around line 279-292: The post-save refresh calls to getSponsorManagedPages and
getSponsorCustomizedPages are missing the current hideArchived flag, causing
grids to repopulate incorrectly when "Hide archived" is enabled; update the two
calls in the post-save refresh to pass the appropriate hideArchived value (e.g.,
managedPages.hideArchived and customizedPages.hideArchived or the shared
hideArchived state) as an additional parameter to getSponsorManagedPages(...)
and getSponsorCustomizedPages(...) so the refresh respects the archived filter.
---
Nitpick comments:
In `@src/pages/sponsors/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js`:
- Around line 305-307: The test currently only checks call counts for
getSponsorManagedPages and getSponsorCustomizedPages; change it to assert the
last call arguments to ensure the correct filters/pagination are passed (e.g.,
verify that the last call to getSponsorManagedPages and
getSponsorCustomizedPages includes the expected hideArchived flag and other
filter/pagination params), and update the assertions around the save flow
(handleSaveManagedPage) to use toHaveBeenLastCalledWith-style checks or
expect.objectContaining on the last call args rather than relying solely on
toHaveBeenCalledTimes.
In `@src/pages/sponsors/sponsor-pages-tab/index.js`:
- Around line 199-200: Remove the temporary console.log("CHECK!", item) that
prints the row payload on every Customize click; inside the click handler where
getSponsorManagedPage(item.id).then(() => setOpenPopup("managedPagePopup")) is
invoked, delete the console.log statement (or replace it with a controlled
debug/logging call if persistent logging is required) so only
getSponsorManagedPage and setOpenPopup remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35d93b55-7a8e-486d-b8ac-1a682644832e
📒 Files selected for processing (4)
src/i18n/en.jsonsrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-pages-tab/index.js
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-pages-actions.js (1)
282-293:⚠️ Potential issue | 🔴 CriticalRestore the
add_onssource for the “all add-ons” check.
normalizedEntityonly containsshow_page_idshere, so Line 287 dereferencesundefined.includes("all"). Every create save will throw before the POST. Read fromentity.add_ons(or initializenormalizedEntity.allowed_add_onsfirst) before mapping IDs.Suggested fix
- if (normalizedEntity.allowed_add_ons.includes("all")) { + if (entity.add_ons.includes("all")) { normalizedEntity.apply_to_all_add_ons = true; normalizedEntity.allowed_add_ons = []; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-pages-actions.js` around lines 282 - 293, In normalizeSponsorManagedPage, the code checks normalizedEntity.allowed_add_ons.includes("all") before allowed_add_ons is set, causing a crash; change the logic to read from entity.add_ons (or initialize normalizedEntity.allowed_add_ons first) — e.g., build allowed_add_ons from entity.add_ons (mapping to ids) or set normalizedEntity.allowed_add_ons = entity.add_ons.map(a => a.id) before the includes("all") check, then set apply_to_all_add_ons and clear allowed_add_ons when "all" is present; update the branches inside normalizeSponsorManagedPage to reference entity.add_ons or the initialized normalizedEntity.allowed_add_ons consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 282-293: In normalizeSponsorManagedPage, the code checks
normalizedEntity.allowed_add_ons.includes("all") before allowed_add_ons is set,
causing a crash; change the logic to read from entity.add_ons (or initialize
normalizedEntity.allowed_add_ons first) — e.g., build allowed_add_ons from
entity.add_ons (mapping to ids) or set normalizedEntity.allowed_add_ons =
entity.add_ons.map(a => a.id) before the includes("all") check, then set
apply_to_all_add_ons and clear allowed_add_ons when "all" is present; update the
branches inside normalizeSponsorManagedPage to reference entity.add_ons or the
initialized normalizedEntity.allowed_add_ons consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1092a026-c347-4b5d-8b5c-4f658b2517ed
📒 Files selected for processing (1)
src/actions/sponsor-pages-actions.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
…oad tables after save Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
feature/sponsor-managed-page-customize
branch
from
2b437c0 to
7d785d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-pages-actions.js (1)
282-293:⚠️ Potential issue | 🔴 CriticalInitialize the add-on source before checking for
"all".
normalizedEntityonly containsshow_page_idshere, so Line 287 will throw on the create flow before the request is even sent.Proposed fix
const normalizeSponsorManagedPage = (entity) => { + const selectedAddOns = entity.add_ons ?? entity.allowed_add_ons ?? []; const normalizedEntity = { show_page_ids: entity.pages }; - if (normalizedEntity.allowed_add_ons.includes("all")) { + if (selectedAddOns.includes("all")) { normalizedEntity.apply_to_all_add_ons = true; normalizedEntity.allowed_add_ons = []; } else { - normalizedEntity.allowed_add_ons = entity.add_ons.map((a) => a.id); + normalizedEntity.allowed_add_ons = selectedAddOns.map((a) => + typeof a === "object" ? a.id : a + ); normalizedEntity.apply_to_all_add_ons = false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-pages-actions.js` around lines 282 - 293, The normalizeSponsorManagedPage function builds normalizedEntity without initializing allowed_add_ons, causing normalizedEntity.allowed_add_ons.includes("all") to throw; fix by first populating normalizedEntity.allowed_add_ons from the source (e.g., set normalizedEntity.allowed_add_ons = entity.add_ons.map(a => a.id) or entity.allowed_add_ons if present) and then check for includes("all"), setting normalizedEntity.apply_to_all_add_ons = true and normalizedEntity.allowed_add_ons = [] when "all" is present, otherwise set apply_to_all_add_ons = false; ensure show_page_ids remains assigned from entity.pages so the rest of the function uses the initialized allowed_add_ons correctly.
♻️ Duplicate comments (1)
src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js (1)
262-275:⚠️ Potential issue | 🟠 MajorOnly close the page editor after a successful save.
Both save handlers call
setOpenPopup(null)in.finally(). A rejected save drops the user's edits, and this path also skipshandleClosePagePopup(), socurrentEditPagecan leak into the next open.Proposed fix
const handleSaveCustomizedPage = (entity) => { saveSponsorCustomizedPage(entity) .then(() => { const { perPage, order, orderDir } = customizedPages; getSponsorCustomizedPages( term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir, hideArchived ); - }) - .finally(() => setOpenPopup(null)); + handleClosePagePopup(); + }); }; const handleSaveManagedPage = (entity) => { saveSponsorManagedPage(entity) .then(() => { getSponsorManagedPages( term, DEFAULT_CURRENT_PAGE, managedPages.perPage, managedPages.order, managedPages.orderDir, hideArchived ); getSponsorCustomizedPages( term, DEFAULT_CURRENT_PAGE, customizedPages.perPage, customizedPages.order, customizedPages.orderDir, hideArchived ); - }) - .finally(() => setOpenPopup(null)); + handleClosePagePopup(); + }); };Also applies to: 278-299
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js` around lines 262 - 275, The handler handleSaveCustomizedPage currently closes the editor in a .finally() which hides the popup even on failed saves and prevents handleClosePagePopup() from running; change this so setOpenPopup(null) and the call to handleClosePagePopup() (or the same cleanup that clears currentEditPage) occur only in the successful branch (.then()), not in .finally(), so that failed save keeps the editor open and currentEditPage is cleared only on success; apply the same change to the other save handler referenced around lines 278-299 (the save handler that calls saveSponsorCustomizedPage/saveSponsorPage).
🧹 Nitpick comments (1)
src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js (1)
198-200: Remove the debug log from the customize path.Clicking “Customize” will dump the whole row to the browser console on every edit with no user-facing value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js` around lines 198 - 200, The handleManagedEdit function currently logs the entire item to the console (console.log("CHECK!", item)); remove that debug statement and keep the remaining logic: call getSponsorManagedPage(item.id) and then setOpenPopup("managedPagePopup") so the customize path no longer dumps row data to the browser console.
🤖 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/actions/sponsor-pages-actions.js`:
- Around line 311-315: normalizeSponsorManagedPageToCustomize currently
serializes media module deadlines with moment.utc(), causing timezone shifts for
non-UTC summits; update the function signature to accept
currentSummit.time_zone.name and replace
moment.utc(module.upload_deadline).unix() with moment.tz(module.upload_deadline,
currentSummit.time_zone.name).unix() when setting
normalizedModule.upload_deadline for PAGES_MODULE_KINDS.MEDIA so it matches
normalizeSponsorCustomPage and the reducer behavior.
---
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 282-293: The normalizeSponsorManagedPage function builds
normalizedEntity without initializing allowed_add_ons, causing
normalizedEntity.allowed_add_ons.includes("all") to throw; fix by first
populating normalizedEntity.allowed_add_ons from the source (e.g., set
normalizedEntity.allowed_add_ons = entity.add_ons.map(a => a.id) or
entity.allowed_add_ons if present) and then check for includes("all"), setting
normalizedEntity.apply_to_all_add_ons = true and
normalizedEntity.allowed_add_ons = [] when "all" is present, otherwise set
apply_to_all_add_ons = false; ensure show_page_ids remains assigned from
entity.pages so the rest of the function uses the initialized allowed_add_ons
correctly.
---
Duplicate comments:
In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js`:
- Around line 262-275: The handler handleSaveCustomizedPage currently closes the
editor in a .finally() which hides the popup even on failed saves and prevents
handleClosePagePopup() from running; change this so setOpenPopup(null) and the
call to handleClosePagePopup() (or the same cleanup that clears currentEditPage)
occur only in the successful branch (.then()), not in .finally(), so that failed
save keeps the editor open and currentEditPage is cleared only on success; apply
the same change to the other save handler referenced around lines 278-299 (the
save handler that calls saveSponsorCustomizedPage/saveSponsorPage).
---
Nitpick comments:
In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js`:
- Around line 198-200: The handleManagedEdit function currently logs the entire
item to the console (console.log("CHECK!", item)); remove that debug statement
and keep the remaining logic: call getSponsorManagedPage(item.id) and then
setOpenPopup("managedPagePopup") so the customize path no longer dumps row data
to the browser console.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31420c60-7981-477b-a819-51927d9934d1
📒 Files selected for processing (6)
src/actions/sponsor-pages-actions.jssrc/i18n/en.jsonsrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/sponsors-global/page-templates/page-template-popup/index.js
- src/reducers/sponsors/sponsor-page-pages-list-reducer.js
Comment on lines
+311
to
+315
| if (module.kind === PAGES_MODULE_KINDS.MEDIA && module.upload_deadline) { | ||
| normalizedModule.upload_deadline = moment | ||
| .utc(module.upload_deadline) | ||
| .unix(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
sed -n '298,339p' src/actions/sponsor-pages-actions.js
printf '\n---\n'
sed -n '556,590p' src/actions/sponsor-pages-actions.js
printf '\n---\n'
sed -n '177,195p' src/reducers/sponsors/sponsor-page-pages-list-reducer.jsRepository: fntechgit/summit-admin
Length of output: 2818
🏁 Script executed:
rg "normalizeSponsorManagedPageToCustomize" src/actions/sponsor-pages-actions.js -B 3 -A 3Repository: fntechgit/summit-admin
Length of output: 384
🏁 Script executed:
sed -n '1,330p' src/actions/sponsor-pages-actions.js | tail -n 50Repository: fntechgit/summit-admin
Length of output: 1415
🏁 Script executed:
rg -B 20 "const normalizedEntity = normalizeSponsorManagedPageToCustomize" src/actions/sponsor-pages-actions.js | head -n 40Repository: fntechgit/summit-admin
Length of output: 662
🏁 Script executed:
rg "normalizeSponsorManagedPageToCustomize" --type js -lRepository: fntechgit/summit-admin
Length of output: 102
🏁 Script executed:
rg "normalizeSponsorManagedPageToCustomize\(" src/actions/sponsor-pages-actions.jsRepository: fntechgit/summit-admin
Length of output: 144
🏁 Script executed:
rg "currentSummit.time_zone" src/ -A 1 -B 1Repository: fntechgit/summit-admin
Length of output: 42265
Fix timezone inconsistency in managed page deadline serialization.
The normalizeSponsorManagedPageToCustomize function serializes deadlines using moment.utc(), but the parallel normalizeSponsorCustomPage function uses moment.tz(..., summitTZ), and the reducer rehydrates managed pages in summit timezone. For non-UTC summits, this inconsistency can shift stored deadlines when saving a managed page.
Pass currentSummit.time_zone.name to normalizeSponsorManagedPageToCustomize and use moment.tz() instead of moment.utc() to match the existing customized-page serialization path and reducer behavior.
Proposed fix
- const normalizedEntity = normalizeSponsorManagedPageToCustomize(entity);
+ const normalizedEntity = normalizeSponsorManagedPageToCustomize(
+ entity,
+ currentSummit.time_zone.name
+ );-const normalizeSponsorManagedPageToCustomize = (entity) => {
+const normalizeSponsorManagedPageToCustomize = (entity, summitTZ) => {
const normalizedEntity = {
...entity,
...normalizeSelectAllField(
entity.allowed_add_ons,
"apply_to_all_add_ons",
"allowed_add_ons"
)
};
normalizedEntity.modules = entity.modules.map((module) => {
const normalizedModule = { ...module };
if (module.kind === PAGES_MODULE_KINDS.MEDIA && module.upload_deadline) {
normalizedModule.upload_deadline = moment
- .utc(module.upload_deadline)
+ .tz(module.upload_deadline, summitTZ)
.unix();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/sponsor-pages-actions.js` around lines 311 - 315,
normalizeSponsorManagedPageToCustomize currently serializes media module
deadlines with moment.utc(), causing timezone shifts for non-UTC summits; update
the function signature to accept currentSummit.time_zone.name and replace
moment.utc(module.upload_deadline).unix() with moment.tz(module.upload_deadline,
currentSummit.time_zone.name).unix() when setting
normalizedModule.upload_deadline for PAGES_MODULE_KINDS.MEDIA so it matches
normalizeSponsorCustomPage and the reducer behavior.
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.
ref: https://app.clickup.com/t/86b7991c7
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Chores
Tests