fix: use denormalize function for modules for sponsor managed pages#940
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughActions now export and call normalizePageTemplateModules(entity.modules); the reducer calls denormalizePageModules(modules, state.summitTZ || "UTC") for RECEIVE_SPONSOR_MANAGED_PAGE and RECEIVE_SPONSOR_CUSTOMIZED_PAGE. Local timezone/epoch and inline per-module reshaping were removed; corresponding tests were added/updated. ChangesModule Normalization Refactoring
Sequence Diagram: sequenceDiagram
participant SponsorPagesAction
participant PageTemplateUtils
participant SponsorPageReducer
SponsorPagesAction->>PageTemplateUtils: normalizePageTemplateModules(entity.modules)
PageTemplateUtils-->>SponsorPagesAction: normalized modules
SponsorPagesAction->>SponsorPageReducer: dispatch page with normalized modules
SponsorPageReducer->>PageTemplateUtils: denormalizePageModules(modules, summitTZ)
PageTemplateUtils-->>SponsorPageReducer: denormalized modules
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
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)
205-206:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
summitTZthrough the managed-page module normalization path.This save path now calls
normalizePageTemplateModules(entity.modules)without the summit timezone, while the customized-page save path passessummitTZand the reducer denormalizes module deadlines with that timezone. In non-UTC summits, managed-page edits can round-tripupload_deadlineincorrectly.Suggested fix
export const saveSponsorManagedPage = (entity) => async (dispatch, getState) => { const { currentSummitState, currentSponsorState } = getState(); const { currentSummit } = currentSummitState; + const summitTZ = currentSummit.time_zone.name; const { entity: { id: sponsorId } } = currentSponsorState; const accessToken = await getAccessTokenSafely(); @@ if (entity.id) { - const normalizedEntity = normalizeSponsorManagedPageToCustomize(entity); + const normalizedEntity = normalizeSponsorManagedPageToCustomize( + entity, + summitTZ + ); @@ -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 = normalizePageTemplateModules(entity.modules); + normalizedEntity.modules = normalizePageTemplateModules( + entity.modules, + summitTZ + );Also applies to: 300-310
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/actions/sponsor-pages-actions.js` around lines 205 - 206, The managed-page save path is calling normalizeSponsorManagedPageToCustomize(entity) which in turn calls normalizePageTemplateModules(entity.modules) without passing summitTZ, causing module deadlines to be denormalized with the wrong timezone; update normalizeSponsorManagedPageToCustomize (and any other managed-page normalization call sites around the other occurrence at ~300-310) to accept and forward summitTZ into normalizePageTemplateModules (i.e., call normalizePageTemplateModules(entity.modules, summitTZ)) so the same summitTZ used by the customized-page path is used for denormalizing upload_deadline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 205-206: The managed-page save path is calling
normalizeSponsorManagedPageToCustomize(entity) which in turn calls
normalizePageTemplateModules(entity.modules) without passing summitTZ, causing
module deadlines to be denormalized with the wrong timezone; update
normalizeSponsorManagedPageToCustomize (and any other managed-page normalization
call sites around the other occurrence at ~300-310) to accept and forward
summitTZ into normalizePageTemplateModules (i.e., call
normalizePageTemplateModules(entity.modules, summitTZ)) so the same summitTZ
used by the customized-page path is used for denormalizing upload_deadline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d99d5b2-8404-4a1e-a24c-d39e3ed3ca79
📒 Files selected for processing (2)
src/actions/sponsor-pages-actions.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
fix/sponsor-managed-pages-modules-denormalize
branch
from
5aa44f9 to
6853676
Compare
smarcet
reviewed
Jun 9, 2026
| state.summitTZ || "UTC" | ||
| ); | ||
|
|
||
| return { ...state, currentEditPage: { ...editPage, modules } }; |
There was a problem hiding this comment.
Missing regression tests for this new delegation to denormalizePageModules.
Suggested test cases in src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js:
- MEDIA module with
upload_deadline→ verify timezone conversion is applied viaepochToMomentTimeZone - DOCUMENT module with
filepresent → verifyfileis array-wrapped andtypeis set toFILE - DOCUMENT module without
file→ verifytypeis set toURL
| state.summitTZ || "UTC" | ||
| ); | ||
|
|
||
| return { ...state, currentEditPage: { ...customizedPage, modules } }; |
There was a problem hiding this comment.
Missing regression test for this new delegation to denormalizePageModules.
Suggested test case in src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js:
- DOCUMENT module with
filepresent → verify the file is array-wrapped withfile_pathandpublic_urlmapped, andtypeis set toFILE
|
|
||
| return normalizedModule; | ||
| }); | ||
| normalizedEntity.modules = normalizePageTemplateModules(entity.modules); |
There was a problem hiding this comment.
normalizePageTemplateModules is now shared with the customized-page flow. The old inline code sent every file unconditionally; this shared helper only sends a file if it lacks an id/file_id (the isNewFile guard) — a silent behavioral change for the managed-page PUT path.
Missing regression tests for normalizeSponsorManagedPageToCustomize in src/actions/__tests__/:
- DOCUMENT module with a new file (no
id/file_id) → verify file is included in the PUT payload - DOCUMENT module with an existing file (
idpresent) → verify file is NOT included in the PUT payload (isNewFile guard fires) - DOCUMENT/URL module → verify no file appears in the PUT payload
smarcet
requested changes
Jun 9, 2026
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js (2)
81-81: 💤 Low valueTest description could be more precise.
The description says "when summitTZ is not set" but the test uses
summitTZ: ""(an empty string). Consider updating to "falls back to UTC when summitTZ is falsy" or "when summitTZ is empty" for accuracy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js` at line 81, Update the test description to accurately reflect the input being asserted: change the it(...) string in sponsor-page-pages-list-reducer.test.js (the test block with description "falls back to UTC when summitTZ is not set") to either "falls back to UTC when summitTZ is falsy" or "falls back to UTC when summitTZ is empty" to match the test data where summitTZ is an empty string; leave the test body and assertions (the reducer call and expected UTC behavior) unchanged.
93-134: 💤 Low valueConsider adding UTC fallback test for consistency.
The
RECEIVE_SPONSOR_CUSTOMIZED_PAGEsuite lacks a UTC fallback test (present in theRECEIVE_SPONSOR_MANAGED_PAGEsuite at lines 81-90). While the reducer code path is identical for both actions, adding it would ensure symmetric test coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js` around lines 93 - 134, Add a new test in the RECEIVE_SPONSOR_CUSTOMIZED_PAGE block that mirrors the UTC-fallback test from the RECEIVE_SPONSOR_MANAGED_PAGE suite: call dispatchReceiveCustomizedPage with a MEDIA module (use PAGES_MODULE_KINDS.MEDIA and upload_deadline: 1700000000) while creating state without a summitTZ (e.g., createInitialState({}) or omit summitTZ), then assert that the resulting module's upload_deadline equals "moment-1700000000-UTC"; place it alongside the existing MEDIA test and reference dispatchReceiveCustomizedPage and PAGES_MODULE_KINDS.MEDIA to locate where to add it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js`:
- Line 81: Update the test description to accurately reflect the input being
asserted: change the it(...) string in sponsor-page-pages-list-reducer.test.js
(the test block with description "falls back to UTC when summitTZ is not set")
to either "falls back to UTC when summitTZ is falsy" or "falls back to UTC when
summitTZ is empty" to match the test data where summitTZ is an empty string;
leave the test body and assertions (the reducer call and expected UTC behavior)
unchanged.
- Around line 93-134: Add a new test in the RECEIVE_SPONSOR_CUSTOMIZED_PAGE
block that mirrors the UTC-fallback test from the RECEIVE_SPONSOR_MANAGED_PAGE
suite: call dispatchReceiveCustomizedPage with a MEDIA module (use
PAGES_MODULE_KINDS.MEDIA and upload_deadline: 1700000000) while creating state
without a summitTZ (e.g., createInitialState({}) or omit summitTZ), then assert
that the resulting module's upload_deadline equals "moment-1700000000-UTC";
place it alongside the existing MEDIA test and reference
dispatchReceiveCustomizedPage and PAGES_MODULE_KINDS.MEDIA to locate where to
add it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 942e4a9c-9ed4-452d-b0a9-e2896b8c0153
📒 Files selected for processing (1)
src/reducers/sponsors/__tests__/sponsor-page-pages-list-reducer.test.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
smarcet
pushed a commit
that referenced
this pull request
Jun 11, 2026
…940) * fix: use denormalize function for modules for sponsor managed pages Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: add test cases for pages modules checking files and dates Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: add sponsor-pages-actions unit test to check file payload Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> --------- Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
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
2 participants
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/86b9x7fbm
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Refactor
Tests