fix: add sponsorship types ddl on show page template popup#799
Open
fix: add sponsorship types ddl on show page template popup#799
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
|
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:
📝 WalkthroughWalkthroughAPI list for show pages now requests apply_to_all_types; normalization exposes an apply_to_all_types boolean and adjusts sponsorship_types. UI mounts PageTemplatePopup conditionally, fetches global sponsorships, and renders a sponsorship-types DropdownCheckbox. Reducer stores global sponsorships and maps "all" to apply_to_all_types. Changes
Sequence Diagram(s)sequenceDiagram
participant Parent as ShowPagesListPage
participant Popup as PageTemplatePopup
participant Action as getSponsorships
participant API as Backend API
participant Reducer as ShowPagesReducer
participant UI as DropdownCheckbox
Parent->>Action: dispatch getSponsorships(page=1, perPage=MAX_PER_PAGE)
Action->>API: fetch sponsorships
API-->>Action: sponsorships response
Action->>Reducer: dispatch RECEIVE_GLOBAL_SPONSORSHIPS(response)
Reducer-->>Parent: sponsorships via props
Parent->>Popup: mount with sponsorships.items
Popup->>UI: render DropdownCheckbox (sponsorships, sponsorship_types, apply_to_all_types)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 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 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: 6
🧹 Nitpick comments (1)
src/pages/sponsors-global/page-templates/page-template-popup/index.js (1)
63-65: TheuseEffectcondition may cause unexpected behavior.The condition
if (sponsorships)will be true even ifsponsorshipsis an empty object. Additionally,getSponsorshipsis called unconditionally on every mount regardless of whether data is already loaded. Consider checking if data needs to be fetched.♻️ Suggested improvement
useEffect(() => { - if (sponsorships) getSponsorships(1, MAX_PER_PAGE); + if (sponsorships && !sponsorships.items?.length) { + getSponsorships(1, MAX_PER_PAGE); + } }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around lines 63 - 65, The current useEffect uses "if (sponsorships)" which is truthy for an empty object and also calls getSponsorships unconditionally on mount; update the effect to only fetch when there is no data (e.g., sponsorships is null/undefined or Object.keys(sponsorships).length === 0) and include getSponsorships and MAX_PER_PAGE in the dependency array to avoid stale closures; in short, change the condition in the useEffect that references sponsorships to a strict "no data" check and add getSponsorships/MAX_PER_PAGE to the deps so getSponsorships(1, MAX_PER_PAGE) runs only when needed.
🤖 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/show-pages-actions.js`:
- Around line 130-139: normalizeShowPage currently calls
entity.sponsorship_types.includes(...) which throws if sponsorship_types is
undefined or not an array; update the function (normalizeShowPage) to
defensively handle sponsorship_types by checking
Array.isArray(entity.sponsorship_types) (or coerce to an empty array) before
calling .includes, set normalizedEntity.sponsorship_types only when
present/valid, and only run the "all" branch (set apply_to_all_types = true and
delete normalizedEntity.sponsorship_types) when the check confirms the array
contains "all".
In `@src/components/mui/formik-inputs/mui-formik-file-size-field.js`:
- Around line 13-15: The current computation of displayValue treats 0 as falsy
and renders an empty string; update the conditional so only null/undefined are
considered empty (e.g. check field.value == null or typeof field.value ===
'undefined') and keep the Math.floor(field.value / BYTES_PER_MB) for numeric
values; modify the expression around displayValue (which uses field.value and
BYTES_PER_MB) to return "" only when field.value is null/undefined so 0 bytes
render as "0".
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 67-68: The constant COLUMN_8 is incorrectly set to 4; update the
constant declaration so COLUMN_8 = 8 (leave COLUMN_4 as 4) to restore correct
responsive grid sizing when sponsorships is absent; locate and change the value
of COLUMN_8 in the file where COLUMN_4 and COLUMN_8 are declared (used in layout
logic that checks sponsorships) so the grid uses 4 and 8 respectively.
- Line 289: The PropTypes for the sponsorships prop are incorrect: change the
declaration for sponsorships from PropTypes.array to a PropTypes.shape that
matches the reducer shape (e.g. sponsorships: PropTypes.shape({ items:
PropTypes.array, currentPage: PropTypes.number, lastPage: PropTypes.number,
total: PropTypes.number })); update the component's PropTypes where sponsorships
is declared so it reflects this object structure (add .isRequired only if the
reducer guarantees it) and ensure PropTypes is imported/used consistently in the
component that defines sponsorships.
- Around line 219-231: DropdownCheckbox is being passed unsupported props
allName and allValue and Formik's apply_to_all_types will fall out of sync
because DropdownCheckbox encodes the "all" selection inside the
sponsorship_types array; remove those props and instead keep state in sync by
wrapping DropdownCheckbox's onChange: in the page-template-popup component
replace passing formik.handleChange directly with a custom handler that receives
the new sponsorship_types value, calls formik.setFieldValue('sponsorship_types',
newValue) and also sets formik.setFieldValue('apply_to_all_types',
newValue.includes('all')), or alternatively remove apply_to_all_types entirely
and only use sponsorship_types (ensuring the rest of the code reads the presence
of 'all' from sponsorship_types); reference DropdownCheckbox, sponsorship_types,
apply_to_all_types, and formik.handleChange when making the change.
In `@src/reducers/sponsors/show-pages-list-reducer.js`:
- Around line 148-151: The map over sponsorships in newSponsorships accesses
s.type.name directly which throws if type is missing; update the mapping in the
data.map call that constructs newSponsorships to safely read the name (e.g., use
optional chaining and a sensible default such as s.type?.name ?? '' or
'Unknown') so missing type objects don't cause a TypeError.
---
Nitpick comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 63-65: The current useEffect uses "if (sponsorships)" which is
truthy for an empty object and also calls getSponsorships unconditionally on
mount; update the effect to only fetch when there is no data (e.g., sponsorships
is null/undefined or Object.keys(sponsorships).length === 0) and include
getSponsorships and MAX_PER_PAGE in the dependency array to avoid stale
closures; in short, change the condition in the useEffect that references
sponsorships to a strict "no data" check and add getSponsorships/MAX_PER_PAGE to
the deps so getSponsorships(1, MAX_PER_PAGE) runs only when needed.
Comment on lines
130
to
+139
| const normalizeShowPage = (entity) => { | ||
| const normalizedEntity = { ...entity }; | ||
|
|
||
| normalizedEntity.apply_to_all_types = false; | ||
| normalizedEntity.sponsorship_types = entity.sponsorship_types; | ||
|
|
||
| if (entity.sponsorship_types.includes("all")) { | ||
| normalizedEntity.apply_to_all_types = true; | ||
| delete normalizedEntity.sponsorship_types; | ||
| } |
There was a problem hiding this comment.
Guard against undefined sponsorship_types before calling .includes().
If entity.sponsorship_types is undefined or not an array, line 136 will throw a TypeError. Consider adding a defensive check.
🛡️ Proposed fix to add defensive check
const normalizeShowPage = (entity) => {
const normalizedEntity = { ...entity };
normalizedEntity.apply_to_all_types = false;
- normalizedEntity.sponsorship_types = entity.sponsorship_types;
+ normalizedEntity.sponsorship_types = entity.sponsorship_types || [];
- if (entity.sponsorship_types.includes("all")) {
+ if (entity.sponsorship_types?.includes("all")) {
normalizedEntity.apply_to_all_types = true;
delete normalizedEntity.sponsorship_types;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/show-pages-actions.js` around lines 130 - 139, normalizeShowPage
currently calls entity.sponsorship_types.includes(...) which throws if
sponsorship_types is undefined or not an array; update the function
(normalizeShowPage) to defensively handle sponsorship_types by checking
Array.isArray(entity.sponsorship_types) (or coerce to an empty array) before
calling .includes, set normalizedEntity.sponsorship_types only when
present/valid, and only run the "all" branch (set apply_to_all_types = true and
delete normalizedEntity.sponsorship_types) when the check confirms the array
contains "all".
Comment on lines
+13
to
+15
| const displayValue = field.value | ||
| ? Math.floor(field.value / BYTES_PER_MB) | ||
| : ""; |
There was a problem hiding this comment.
Avoid treating 0 bytes as empty.
With the falsy check, a real value of 0 MB renders as blank, so users see “empty” while the form state holds 0. If 0 is a valid value, this is a UX mismatch. Consider only treating null/undefined as empty.
Suggested fix
- const displayValue = field.value
- ? Math.floor(field.value / BYTES_PER_MB)
- : "";
+ const displayValue =
+ field.value == null ? "" : Math.floor(field.value / BYTES_PER_MB);📝 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 displayValue = field.value | |
| ? Math.floor(field.value / BYTES_PER_MB) | |
| : ""; | |
| const displayValue = | |
| field.value == null ? "" : Math.floor(field.value / BYTES_PER_MB); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/formik-inputs/mui-formik-file-size-field.js` around lines
13 - 15, The current computation of displayValue treats 0 as falsy and renders
an empty string; update the conditional so only null/undefined are considered
empty (e.g. check field.value == null or typeof field.value === 'undefined') and
keep the Math.floor(field.value / BYTES_PER_MB) for numeric values; modify the
expression around displayValue (which uses field.value and BYTES_PER_MB) to
return "" only when field.value is null/undefined so 0 bytes render as "0".
src/pages/sponsors-global/page-templates/page-template-popup/index.js
Outdated
Show resolved
Hide resolved
Comment on lines
219
to
231
| {sponsorships && ( | ||
| <Grid2 spacing={2} size={4}> | ||
| <DropdownCheckbox | ||
| name="sponsorship_types" | ||
| allName="apply_to_all_types" | ||
| label={T.translate("page_template_list.sponsorship")} | ||
| allLabel={T.translate("page_template_list.all_tiers")} | ||
| value={formik.values.sponsorship_types} | ||
| allValue={formik.values.apply_to_all_types} | ||
| options={sponsorships.items} | ||
| onChange={formik.handleChange} | ||
| /> | ||
| </Grid2> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the DropdownCheckbox component
cat -n src/components/mui/dropdown-checkbox.jsRepository: fntechgit/summit-admin
Length of output: 2691
🏁 Script executed:
# Also check the exact usage in the page template file
sed -n '219,231p' src/pages/sponsors-global/page-templates/page-template-popup/index.jsRepository: fntechgit/summit-admin
Length of output: 692
DropdownCheckbox does not accept allName or allValue props.
The DropdownCheckbox component only accepts name, label, allLabel, value, options, and onChange. The allName (line 223) and allValue (line 227) props will be silently ignored.
The component manages the "all" functionality internally by including the string "all" in the value array, not through separate props. This means the apply_to_all_types state tracked by formik will not stay synchronized with the component's internal selection state. When "all" is selected, the component will store it in the sponsorship_types array, but apply_to_all_types won't be updated, causing state inconsistency.
Either refactor to use only the value array for state management, or extend DropdownCheckbox to support separate allName and allValue props.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around
lines 219 - 231, DropdownCheckbox is being passed unsupported props allName and
allValue and Formik's apply_to_all_types will fall out of sync because
DropdownCheckbox encodes the "all" selection inside the sponsorship_types array;
remove those props and instead keep state in sync by wrapping DropdownCheckbox's
onChange: in the page-template-popup component replace passing
formik.handleChange directly with a custom handler that receives the new
sponsorship_types value, calls formik.setFieldValue('sponsorship_types',
newValue) and also sets formik.setFieldValue('apply_to_all_types',
newValue.includes('all')), or alternatively remove apply_to_all_types entirely
and only use sponsorship_types (ensuring the rest of the code reads the presence
of 'all' from sponsorship_types); reference DropdownCheckbox, sponsorship_types,
apply_to_all_types, and formik.handleChange when making the change.
| onSave: PropTypes.func.isRequired, | ||
| summitTZ: PropTypes.string.isRequired | ||
| summitTZ: PropTypes.string, | ||
| sponsorships: PropTypes.array |
There was a problem hiding this comment.
PropTypes mismatch: sponsorships is an object, not an array.
Based on the reducer (show-pages-list-reducer.js), sponsorships has the shape { items: [], currentPage, lastPage, total }. The PropTypes should reflect this structure.
🛡️ Proposed fix
- sponsorships: PropTypes.array
+ sponsorships: PropTypes.shape({
+ items: PropTypes.array,
+ currentPage: PropTypes.number,
+ lastPage: PropTypes.number,
+ total: PropTypes.number
+ })📝 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
| sponsorships: PropTypes.array | |
| sponsorships: PropTypes.shape({ | |
| items: PropTypes.array, | |
| currentPage: PropTypes.number, | |
| lastPage: PropTypes.number, | |
| total: PropTypes.number | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` at
line 289, The PropTypes for the sponsorships prop are incorrect: change the
declaration for sponsorships from PropTypes.array to a PropTypes.shape that
matches the reducer shape (e.g. sponsorships: PropTypes.shape({ items:
PropTypes.array, currentPage: PropTypes.number, lastPage: PropTypes.number,
total: PropTypes.number })); update the component's PropTypes where sponsorships
is declared so it reflects this object structure (add .isRequired only if the
reducer guarantees it) and ensure PropTypes is imported/used consistently in the
component that defines sponsorships.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-global/page-templates/page-template-popup/index.js`:
- Around line 54-64: mapStateToProps is referencing a non-existent
currentPageTemplateState causing sponsorships to be undefined; update
mapStateToProps to read sponsorships from pageTemplateListState (the actual
reducer key) and in PageTemplatePopup change the useEffect to always call
getSponsorships on mount (remove the if (sponsorships) guard) and include
getSponsorships and MAX_PER_PAGE in the dependency array so the fetch runs
reliably (reference symbols: mapStateToProps, pageTemplateListState,
PageTemplatePopup, useEffect, getSponsorships, sponsorships, MAX_PER_PAGE).
In `@src/reducers/sponsors/show-pages-list-reducer.js`:
- Around line 89-92: The reducer currently hard-codes the user-facing string
"All Tiers" when building the tier field; instead emit data the UI can localize
by preserving the apply_to_all_types flag and the sponsorship names. Change the
tier assignment so it does not inject the literal "All Tiers" (for example set
tier to null or to the array/string of names when a.apply_to_all_types is false)
and include a boolean field like apply_to_all_types: a.apply_to_all_types (or
return an object { apply_to_all_types: a.apply_to_all_types, names:
a.sponsorship_types.map(s=>s.name) }) so the UI layer can render the translated
label; update references to tier and use the a.modules_count.info_modules_count
mapping unchanged.
---
Duplicate comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 286-288: The PropTypes for the component currently declare
sponsorships as PropTypes.object but the reducer/currentPageTemplateState
provides an object with keys { items, currentPage, lastPage, total }; update the
PropTypes to use PropTypes.shape({ items: PropTypes.array, currentPage:
PropTypes.number, lastPage: PropTypes.number, total: PropTypes.number }) (mark
fields as required if appropriate) so the component's sponsorships prop matches
the state shape; locate the PropTypes block in page-template-popup index.js
where sponsorships is declared and replace PropTypes.object with the shape, and
run the referenced rg commands to verify consistency with
currentPageTemplateState and reducer usage.
- Around line 66-68: COLUMN_8 is incorrectly set to 4 (making both constants
identical) which breaks the responsive layout; change the constant declaration
for COLUMN_8 to 8 so you have const COLUMN_4 = 4; const COLUMN_8 = 8; — update
the constant named COLUMN_8 in the module where COLUMN_4 and COLUMN_8 are
defined to the numeric value 8.
- Around line 218-229: The DropdownCheckbox usage passes unsupported props
allName/allValue which causes apply_to_all_types to drift from
sponsorship_types; inspect the DropdownCheckbox component and either (A) remove
allName/allValue from the call in page-template-popup#index (where
DropdownCheckbox is rendered) and instead manage the "apply to all" behavior in
the parent by updating formik.values.sponsorship_types and
formik.values.apply_to_all_types together in the onChange handler, or (B) add
support in src/components/mui/dropdown-checkbox.js for props allName and
allValue (and ensure it emits a combined value change) so the component keeps
sponsorship_types and apply_to_all_types in sync; update the onChange binding to
use a single source of truth (formik.handleChange or a custom handler that calls
formik.setFieldValue for both fields).
In `@src/reducers/sponsors/show-pages-list-reducer.js`:
- Around line 155-158: The mapping that builds newSponsorships uses s.type.name
which throws when type is missing; update the transformation in
show-pages-list-reducer.js (the data.map that produces newSponsorships) to
defensively access the name (e.g. use optional chaining or a default) and
provide a sensible fallback for missing type/name (e.g. empty string or
"Unknown"), and keep id assignment unchanged; ensure the change is applied to
the map that creates newSponsorships so payloads without s.type do not crash the
reducer.
Comment on lines
54
to
64
| const PageTemplatePopup = ({ | ||
| pageTemplate, | ||
| open, | ||
| onClose, | ||
| onSave, | ||
| summitTZ | ||
| summitTZ, | ||
| getSponsorships, | ||
| sponsorships | ||
| }) => { | ||
| useEffect(() => { | ||
| if (sponsorships) getSponsorships(1, MAX_PER_PAGE); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's explore the repository structure to locate relevant files
fd "page-template-popup" -t f
fd "reducer" -t f | head -20Repository: fntechgit/summit-admin
Length of output: 1020
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read the full component
cat src/pages/sponsors-global/page-templates/page-template-popup/index.jsRepository: fntechgit/summit-admin
Length of output: 9341
🏁 Script executed:
#!/bin/bash
# Search for sponsorships state initialization and reducer definitions
rg -n "sponsorships" src/reducers -A 3 -B 1
rg -n "getSponsorships" src/pages/sponsors-global -A 2 -B 1 | head -50Repository: fntechgit/summit-admin
Length of output: 18485
🏁 Script executed:
#!/bin/bash
# Find getSponsorships action definition
rg -n "export.*getSponsorships" src/actions -A 10Repository: fntechgit/summit-admin
Length of output: 1567
🏁 Script executed:
#!/bin/bash
# Check if there are Redux action creators using useCallback or memoization
cat src/actions/sponsor-forms-actions.js 2>/dev/null || echo "File not found, searching..."
fd "sponsor-forms-actions" -type fRepository: fntechgit/summit-admin
Length of output: 46859
🏁 Script executed:
#!/bin/bash
# Check mapStateToProps more carefully to see default state structure
cat src/reducers/sponsors/sponsor-forms-list-reducer.js | head -70Repository: fntechgit/summit-admin
Length of output: 1867
🏁 Script executed:
#!/bin/bash
# Search for currentPageTemplateState reducer definition
rg -n "currentPageTemplateState" src/reducers -A 5 -B 2 | head -50Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for where PageTemplatePopup is connected/used
rg -n "PageTemplatePopup" src/ -B 3 -A 3 | head -80Repository: fntechgit/summit-admin
Length of output: 5520
🏁 Script executed:
#!/bin/bash
# Check the root reducer to see how states are combined
rg -n "currentPageTemplateState\|combineReducers" src/reducers -A 2 | head -80Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for where currentPageTemplateState is defined in Redux
rg -n "currentPageTemplateState" src/Repository: fntechgit/summit-admin
Length of output: 304
🏁 Script executed:
#!/bin/bash
# Look for index.js in reducers to see how state is combined
find src/reducers -name "index.js" -exec cat {} \;Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for page-template reducer
fd "page-template" src/reducers -type fRepository: fntechgit/summit-admin
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Check the store configuration
find src -name "*store*" -o -name "*configure*" | head -10Repository: fntechgit/summit-admin
Length of output: 78
🏁 Script executed:
#!/bin/bash
# Search for currentPageTemplateState definition
rg -n "currentPageTemplateState" src/ -B 5 -A 10Repository: fntechgit/summit-admin
Length of output: 1236
🏁 Script executed:
#!/bin/bash
# Check store configuration
cat src/store.js | head -100Repository: fntechgit/summit-admin
Length of output: 7588
🏁 Script executed:
#!/bin/bash
# Look for sponsor-forms-list-reducer since that's where sponsorships state is
grep -r "export.*sponsorFormsListReducer\|const.*sponsorFormsListReducer" src/reducers -A 5Repository: fntechgit/summit-admin
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Search for the complete store configuration where reducers are combined
cat src/store.js | tail -150Repository: fntechgit/summit-admin
Length of output: 7660
🏁 Script executed:
#!/bin/bash
# Search for where reducers are assigned their keys in the combined state
rg -n "sponsorFormsListReducer\|currentPageTemplateState" src/store.jsRepository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for currentPageTemplateState in the entire codebase
rg -n "currentPageTemplateState" src/ -B 2 -A 2Repository: fntechgit/summit-admin
Length of output: 609
🏁 Script executed:
#!/bin/bash
# Search for pageTemplateListState and what reducer handles it
rg -n "pageTemplateListState\|pageTemplateListReducer" src/Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look for where currentPageTemplateState might be defined
rg -n "currentPageTemplateState\|pageTemplate.*Reducer\|pageTemplate.*State" src/reducersRepository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check how PageTemplatePopup is used in its parent component
cat src/pages/sponsors-global/page-templates/page-template-list-page.js | grep -A 30 "PageTemplatePopup"Repository: fntechgit/summit-admin
Length of output: 1387
🏁 Script executed:
#!/bin/bash
# Check the other parent usage
cat src/pages/sponsors/show-pages-list-page/index.js | grep -A 30 "PageTemplatePopup"Repository: fntechgit/summit-admin
Length of output: 1277
🏁 Script executed:
#!/bin/bash
# Verify if currentPageTemplateState exists anywhere
rg -r "currentPageTemplateState\s*=" src/Repository: fntechgit/summit-admin
Length of output: 747
🏁 Script executed:
#!/bin/bash
# Check what showPagesListState includes - specifically looking for sponsorships
cat src/reducers/sponsors/show-pages-list-reducer.js | head -100Repository: fntechgit/summit-admin
Length of output: 2742
🏁 Script executed:
#!/bin/bash
# Verify PageTemplatePopup mapStateToProps actually works - check if there's a selector
grep -r "currentPageTemplateState" src/reducers src/store.jsRepository: fntechgit/summit-admin
Length of output: 48
mapStateToProps references undefined Redux state, preventing sponsorship fetch.
The component's mapStateToProps tries to destructure currentPageTemplateState, which doesn't exist in the Redux store (only pageTemplateListState is defined). This causes sponsorships to be undefined when the component is used in PageTemplateListPage. Combined with the empty dependency array [] and the conditional guard if (sponsorships), the fetch never executes and the dropdown remains empty.
The sponsorships reducer initializes with { items: [], ... }, so always fetch on mount and add getSponsorships to dependencies to ensure data loads regardless of initial state. Also fix the mapStateToProps reference to use the correct Redux state key.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around
lines 54 - 64, mapStateToProps is referencing a non-existent
currentPageTemplateState causing sponsorships to be undefined; update
mapStateToProps to read sponsorships from pageTemplateListState (the actual
reducer key) and in PageTemplatePopup change the useEffect to always call
getSponsorships on mount (remove the if (sponsorships) guard) and include
getSponsorships and MAX_PER_PAGE in the dependency array so the fetch runs
reliably (reference symbols: mapStateToProps, pageTemplateListState,
PageTemplatePopup, useEffect, getSponsorships, sponsorships, MAX_PER_PAGE).
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 285-286: The PropTypes declaration for the component's
sponsorships prop is too generic (PropTypes.object); update it to match the
reducer shape (e.g., sponsorships: { items: PropTypes.arrayOf(PropTypes.object),
currentPage: PropTypes.number, lastPage: PropTypes.number, total:
PropTypes.number }) in the page-template-popup component so PropTypes reflect
the actual keys produced by the sponsorships reducer; confirm exact keys from
the reducer and replace PropTypes.object with the appropriate PropTypes.shape or
PropTypes.exact on the sponsorships prop used in this component (reference the
sponsorships prop in the component export/propTypes block).
- Around line 62-64: The useEffect that calls getSponsorships is incorrectly
guarded by a truthy sponsorships check and an empty deps array, which prevents
the fetch from running on mount and ties it to a possibly wrong slice key;
remove the sponsorships guard so getSponsorships(1, MAX_PER_PAGE) runs on mount
inside useEffect, and then verify and fix mapStateToProps to reference the
actual Redux slice that holds sponsorships (e.g., replace
currentPageTemplateState with the real slice like pageTemplateListState or
showPagesListState) so the sponsorships prop is wired correctly; update
references to sponsorships, getSponsorships, useEffect, and mapStateToProps
accordingly.
- Around line 220-228: DropdownCheckbox ignores the allName prop so the form
field apply_to_all_types never updates; change the usage around the
DropdownCheckbox in page-template-popup to handle the "all" toggle explicitly by
replacing onChange={formik.handleChange} with a custom handler that uses
formik.setFieldValue to set both "sponsorship_types" (the component value) and
"apply_to_all_types" (true when the selected value represents all tiers, false
otherwise); reference the DropdownCheckbox props (name="sponsorship_types",
allLabel, value={formik.values.sponsorship_types}) and update those formik
fields in the handler so apply_to_all_types stays in sync.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
smarcet
reviewed
Feb 23, 2026
smarcet
reviewed
Feb 23, 2026
src/pages/sponsors-global/page-templates/page-template-popup/index.js
Outdated
Show resolved
Hide resolved
smarcet
reviewed
Feb 23, 2026
| tier: a.sponsorship_types.map((s) => s.name).join(", "), | ||
| tier: a.apply_to_all_types | ||
| ? "All Tiers" | ||
| : a.sponsorship_types.map((s) => s.name).join(", "), |
There was a problem hiding this comment.
showPage.sponsorship_types is not expanded on api call are we sure that is returned by the api ? i will add to expands bc this can cause a type error at runtime
smarcet
reviewed
Feb 23, 2026
src/pages/sponsors-global/page-templates/page-template-popup/index.js
Outdated
Show resolved
Hide resolved
smarcet
reviewed
Feb 23, 2026
src/pages/sponsors-global/page-templates/page-template-popup/index.js
Outdated
Show resolved
Hide resolved
smarcet
reviewed
Feb 23, 2026
src/pages/sponsors-global/page-templates/page-template-popup/index.js
Outdated
Show resolved
Hide resolved
smarcet
requested changes
Feb 23, 2026
… simplify component logic Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/sponsors-global/page-templates/page-template-popup/index.js (1)
214-222:⚠️ Potential issue | 🟠 MajorSync
apply_to_all_typeswith DropdownCheckbox selection.
DropdownCheckboxdoesn’t supportallName, andapply_to_all_typeswon’t update when “All Tiers” is selected. This will cause stale state to be saved.🐛 Suggested fix
+ const handleSponsorshipsChange = (ev) => { + const { value } = ev.target; + formik.setFieldValue("sponsorship_types", value); + formik.setFieldValue("apply_to_all_types", value.includes("all")); + }; ... - <DropdownCheckbox + <DropdownCheckbox name="sponsorship_types" - allName="apply_to_all_types" label={T.translate("page_template_list.sponsorship")} allLabel={T.translate("page_template_list.all_tiers")} value={formik.values.sponsorship_types} options={sponsorships} - onChange={formik.handleChange} + onChange={handleSponsorshipsChange} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around lines 214 - 222, DropdownCheckbox does not handle the special allName prop so the form field apply_to_all_types stays stale; update the component usage to intercept changes and sync both sponsorship_types and apply_to_all_types via Formik (use formik.setFieldValue) instead of relying solely on formik.handleChange. Specifically, replace the onChange prop on DropdownCheckbox with a custom handler that receives the selected options, sets formik.setFieldValue("sponsorship_types", selectedOptions) and also sets formik.setFieldValue("apply_to_all_types", true/false) based on whether the "all tiers" selection was chosen (detect by the option identifier your DropdownCheckbox emits), keeping the label/option list (sponsorships) and field names unchanged.
🤖 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/reducers/sponsors/show-pages-list-reducer.js`:
- Around line 130-137: Normalize sponsorship_types to an array of ids before
putting it into currentShowPage: when computing sponsorshipTypeIds use
showPage.apply_to_all_types ? ["all"] : (showPage.sponsorship_types || []).map(t
=> (typeof t === "object" && t !== null && ("id" in t)) ? t.id : t); then return
currentShowPage: { ...showPage, sponsorship_types: sponsorshipTypeIds } so
Formik/DropdownCheckbox receive ids not objects (references: showPage,
sponsorship_types, currentShowPage in the reducer).
---
Duplicate comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 214-222: DropdownCheckbox does not handle the special allName prop
so the form field apply_to_all_types stays stale; update the component usage to
intercept changes and sync both sponsorship_types and apply_to_all_types via
Formik (use formik.setFieldValue) instead of relying solely on
formik.handleChange. Specifically, replace the onChange prop on DropdownCheckbox
with a custom handler that receives the selected options, sets
formik.setFieldValue("sponsorship_types", selectedOptions) and also sets
formik.setFieldValue("apply_to_all_types", true/false) based on whether the "all
tiers" selection was chosen (detect by the option identifier your
DropdownCheckbox emits), keeping the label/option list (sponsorships) and field
names unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/i18n/en.jsonsrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors/show-pages-list-page/index.jssrc/reducers/sponsors/show-pages-list-reducer.jssrc/utils/constants.js
Comment on lines
+130
to
+137
| const sponsorshipTypeIds = showPage.apply_to_all_types | ||
| ? ["all"] | ||
| : [...showPage.sponsorship_types]; | ||
|
|
||
| return { | ||
| ...state, | ||
| currentShowPage: { ...showPage, sponsorship_types: sponsorshipTypeIds } | ||
| }; |
There was a problem hiding this comment.
Normalize sponsorship_types to ids to avoid value mismatch.
If showPage.sponsorship_types is an array of objects, the current spread will pass objects into Formik, but the DropdownCheckbox expects ids. This will break selection rendering and updates.
🐛 Suggested fix
- const sponsorshipTypeIds = showPage.apply_to_all_types
- ? ["all"]
- : [...showPage.sponsorship_types];
+ const sponsorshipTypeIds = showPage.apply_to_all_types
+ ? ["all"]
+ : (showPage.sponsorship_types || []).map((s) =>
+ typeof s === "object" ? s.id : s
+ );📝 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 sponsorshipTypeIds = showPage.apply_to_all_types | |
| ? ["all"] | |
| : [...showPage.sponsorship_types]; | |
| return { | |
| ...state, | |
| currentShowPage: { ...showPage, sponsorship_types: sponsorshipTypeIds } | |
| }; | |
| const sponsorshipTypeIds = showPage.apply_to_all_types | |
| ? ["all"] | |
| : (showPage.sponsorship_types || []).map((s) => | |
| typeof s === "object" ? s.id : s | |
| ); | |
| return { | |
| ...state, | |
| currentShowPage: { ...showPage, sponsorship_types: sponsorshipTypeIds } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/reducers/sponsors/show-pages-list-reducer.js` around lines 130 - 137,
Normalize sponsorship_types to an array of ids before putting it into
currentShowPage: when computing sponsorshipTypeIds use
showPage.apply_to_all_types ? ["all"] : (showPage.sponsorship_types || []).map(t
=> (typeof t === "object" && t !== null && ("id" in t)) ? t.id : t); then return
currentShowPage: { ...showPage, sponsorship_types: sponsorshipTypeIds } so
Formik/DropdownCheckbox receive ids not objects (references: showPage,
sponsorship_types, currentShowPage in the reducer).
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/86b799198
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit