Feature/summit sponsor pages #755
Conversation
…opup Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
…750) * feat: page modules generic component, test, adjusting formik inputs Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> * fix: add implementation for pages modules on sponsor global Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com> --------- Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughAdds a sponsor pages feature: new Redux actions/reducer and store slice; sponsor pages listing UI with clone-from-template multi-stage dialogs; new page-template modules (Info/Document/Media) and related module subcomponents and tests; Formik input tweaks; routing/menu entry; new constants, translations, and small normalizeEntity changes (module mapping, upload_deadline conversion). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SponsorPagesListPage
participant Redux
participant API
participant Store
User->>SponsorPagesListPage: mount / request pages
SponsorPagesListPage->>Redux: dispatch getSponsorPages(filters)
Redux->>Redux: dispatch startLoading
Redux->>API: GET /sponsor_pages with params
API-->>Redux: pages response
Redux->>Store: dispatch RECEIVE_SPONSOR_PAGES (normalize)
Store-->>SponsorPagesListPage: updated props
Redux->>Redux: dispatch stopLoading
SponsorPagesListPage-->>User: render pages table
sequenceDiagram
participant User
participant GlobalPagePopup
participant SelectPagesDialog
participant SelectSponsorshipsDialog
participant Redux
participant API
User->>GlobalPagePopup: open popup
GlobalPagePopup->>SelectPagesDialog: show templates stage
User->>SelectPagesDialog: select templates, click Add
SelectPagesDialog-->>GlobalPagePopup: onSelectTemplates(ids)
GlobalPagePopup->>SelectSponsorshipsDialog: show sponsorships stage
User->>SelectSponsorshipsDialog: choose sponsorships, click Save
SelectSponsorshipsDialog-->>GlobalPagePopup: onSave(tiers)
GlobalPagePopup->>Redux: dispatch cloneGlobalPage(templateIds, sponsorIds/allSponsors)
Redux->>API: POST /sponsor_pages/clone with payload
API-->>Redux: success
Redux->>Redux: dispatch GLOBAL_PAGE_CLONED and getSponsorForms
Redux-->>GlobalPagePopup: success
GlobalPagePopup-->>User: show snackbar, close popup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/mui/formik-inputs/mui-formik-datepicker.js (1)
8-8:propsis undefined — this will throw a ReferenceError at runtime.The component destructures only
{ name, label, required }but line 40 spreads{...props}which doesn't exist in scope.Additionally,
marginis not a valid prop on MUI'sDatePickercomponent — it should remain inslotProps.textField.🐛 Proposed fix
-const MuiFormikDatepicker = ({ name, label, required }) => { +const MuiFormikDatepicker = ({ name, label, required, ...props }) => {And move
marginback intoslotProps.textField:textField: { name, label: required ? requiredLabel : label, error: meta.touched && Boolean(meta.error), helperText: meta.touched && meta.error, - fullWidth: true + fullWidth: true, + margin: "normal" },}} - margin="normal" // eslint-disable-next-line react/jsx-props-no-spreading {...props}Also applies to: 38-40
src/components/mui/formik-inputs/mui-formik-select.js (1)
74-79: Missing PropTypes definition forlabelprop.The component accepts a
labelprop (line 16) but it's not declared in PropTypes.Suggested fix
MuiFormikSelect.propTypes = { name: PropTypes.string.isRequired, children: PropTypes.node.isRequired, placeholder: PropTypes.string, - isClearable: PropTypes.bool + isClearable: PropTypes.bool, + label: PropTypes.string };
🤖 Fix all issues with AI agents
In `@src/actions/sponsor-pages-actions.js`:
- Around line 95-133: The cloneGlobalPage action dispatches startLoading() but
never dispatches stopLoading(), leaving the loading state active; update
cloneGlobalPage to dispatch stopLoading() in both the success path (before/after
dispatching getSponsorForms() and snackbarSuccessHandler) and in the
catch/failure path (before handling errors via snackbarErrorHandler) so that
stopLoading() is always called regardless of postRequest resolving or rejecting;
locate the postRequest(...) promise chain in cloneGlobalPage and add
dispatch(stopLoading()) in both the .then and .catch handlers (or use finally)
to guarantee loading is cleared.
- Around line 124-125: The call to getSponsorForms() inside the cloneGlobalPage
promise chain will throw because getSponsorForms is not imported; fix this by
adding an import for getSponsorForms at the top of the file from the module that
exports it (the same module where sponsor form actions live) and keep the
dispatch(getSponsorForms()) call, or if the intent was to refresh sponsor pages
instead, replace that call with the correct exported action (e.g.,
dispatch(getSponsorPages()) )—ensure the chosen function is imported and
referenced (symbols: getSponsorForms, cloneGlobalPage, dispatch).
In `@src/components/mui/formik-inputs/mui-formik-select.js`:
- Around line 34-45: The label can overlap the placeholder because hasValue only
reflects a selected value; update the hasValue logic in the component that
renders InputLabel and Select so it treats a provided placeholder (and
displayEmpty=true) as a value — e.g., compute hasValue = Boolean(field.value) ||
(displayEmpty && Boolean(placeholder)) — then continue using hasValue for
InputLabel's shrink and Select's notched props (symbols: InputLabel, Select,
hasValue, placeholder, displayEmpty, field).
In `@src/components/mui/formik-inputs/mui-formik-upload.js`:
- Line 65: Remove the debug console.log in the UploadFormik component (the line
logging "CHJECK RESPONSE" in mui-formik-upload.js); delete that console.log
statement (or replace it with a proper logger call without the typo if
intentional) so no stray debug output remains in the upload handling code.
- Line 16: Remove the debug console.log statement currently logging "images: "
and field.value inside the MuiFormikUpload component; delete the line
console.log("images: ", field.value) (or replace it with a proper debug-level
logger if persistent logging is required) so no console output remains in
production.
- Around line 93-101: The canAddMore function references mediaType.is_editable
but mediaType lacks that property; update the mediaType object used by this
component (the mediaType declaration near the top of mui-formik-upload.js) to
include is_editable: true or derive it from props, so canAddMore correctly
evaluates editable media types; ensure changes touch the mediaType definition
(not canAddMore) and keep the singleFile and field.value logic intact.
In `@src/layouts/sponsor-layout.js`:
- Line 28: The import for the React component is using camelCase; change the
import identifier from sponsorPagesListPage to PascalCase SponsorPagesListPage
and update all places where sponsorPagesListPage is referenced (e.g., the
component usage currently referencing it) to use SponsorPagesListPage so the
import and usage match the React component naming convention.
In `@src/pages/sponsors-global/page-templates/page-template-modules-form.js`:
- Around line 182-193: InputLabel's htmlFor is incorrectly targeting
buildFieldName("max_file_size") while the actual form control is
MuiFormikTextField with name buildFieldName("description"); update the
InputLabel so htmlFor uses buildFieldName("description") to match the
MuiFormikTextField name and restore proper accessibility (locate the InputLabel
and adjust the buildFieldName argument from "max_file_size" to "description").
- Around line 92-98: The MuiFormikUpload instance inside the
DocumentDownloadModule uses a hardcoded id ("document-module-upload") which will
duplicate across multiple modules; change the id to be unique per module by
deriving it from the module instance (e.g., include the module index, module id
prop, or reuse buildFieldName("file") as part of the id) so each MuiFormikUpload
has a distinct id attribute; update the id prop on the MuiFormikUpload in
page-template-modules-form.js accordingly and ensure any label/for references
use the same generated id.
- Around line 167-178: The InputLabel uses the wrong htmlFor value; change
htmlFor in the InputLabel surrounding the MuiFormikSelect to use
buildFieldName("file_type_id") instead of buildFieldName("max_file_size") so the
label correctly targets the select; update the InputLabel that wraps the
MuiFormikSelect (referencing buildFieldName and the file_type_id field name) to
restore proper label-input association for accessibility.
In
`@src/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.js`:
- Around line 31-43: The Dialog currently passes the external onClose prop
directly which prevents resetting internal state; change the Dialog's onClose to
call the local handleClose function (the handler that resets stage and
selectedTemplates) instead of the external onClose, so backdrop clicks/Escape
trigger resetting internal state; keep using the external onClose (prop named
onClose) inside handleClose if it needs to notify parents after internal reset.
In `@src/pages/sponsors/sponsor-pages-list-page/index.js`:
- Around line 143-154: The Checkbox is uncontrolled because it lacks a checked
prop tied to the Redux state; update the Checkbox in the FormControlLabel to
pass checked={hideArchived} (ensure hideArchived is derived from Redux state in
this component) so the visual state matches the filter on initial render, and
keep using handleHideArchivedForms to dispatch/toggle the hideArchived value; if
hideArchived can be undefined ensure it is coerced to a boolean (e.g.,
!!hideArchived) before passing to checked.
- Around line 31-42: The component SponsorPagesListPage destructures
getSponsorForm but it isn’t wired to Redux; either add getSponsorForm to the
connect(...) mapDispatchToProps alongside getSponsorPages so the prop is
provided, or remove getSponsorForm from SponsorPagesListPage’s parameter list
and any usages (e.g., handleRowEdit) if the action isn’t implemented; ensure any
call sites like handleRowEdit reference the correctly connected action name.
In `@src/reducers/sponsors/sponsor-pages-list-reducer.js`:
- Around line 61-70: The reducer currently builds sponsorPages with module
counts using hardcoded kinds ("Upload"/"Download") causing
upload_mod/download_mod to be zero; update the modules.filter calls in the
sponsorPages mapping to use the PAGES_MODULE_KINDS constants (e.g.,
PAGES_MODULE_KINDS.DOCUMENT and PAGES_MODULE_KINDS.MEDIA) when computing those
counts, rename the fields from upload_mod/download_mod to document_mod/media_mod
to match the API constants, and propagate those new field names to the
corresponding table column and UI references so they consume document_mod and
media_mod instead of the old names.
🧹 Nitpick comments (15)
src/components/mui/formik-inputs/mui-formik-radio-group.js (1)
13-19: LGTM! Clean addition of configurable margin prop.The implementation correctly adds an optional
marginprop with a sensible default, preserving backward compatibility while enabling layout customization. This aligns with the PR-wide pattern of exposing more customization options on Formik inputs.Optional: Consider using
PropTypes.oneOfto restrict values to MUI's valid options:- margin: PropTypes.string, + margin: PropTypes.oneOf(["none", "dense", "normal"]),Also applies to: 25-25, 65-65
src/actions/page-template-actions.js (1)
145-160: Consider adding null-safety for the modules array.The
normalizeEntityfunction assumesentity.modulesis always a valid array. Ifentity.modulesisnullorundefined, the.map()call will throw a runtime error.Suggested defensive check
const normalizeEntity = (entity) => { const normalizedEntity = { ...entity }; - normalizedEntity.modules = entity.modules.map((module) => { + normalizedEntity.modules = (entity.modules || []).map((module) => { const normalizedModule = { ...module }; if (module.kind === PAGES_MODULE_KINDS.MEDIA && module.upload_deadline) { normalizedModule.upload_deadline = moment(module.upload_deadline).unix(); } delete normalizedModule._tempId; return normalizedModule; }); return normalizedEntity; };src/pages/sponsors/sponsor-pages-list-page/components/global-page/select-pages-dialog.js (3)
140-143: Hardcoded text should use i18n translation.The text "items selected" is hardcoded in English. For consistency with the rest of the component, this should use
T.translate().Suggested fix
<Grid2 size={4} sx={{ mt: 1 }}> - {selectedRows.length} items selected + {T.translate("sponsor_pages.global_page_popup.items_selected", { count: selectedRows.length })} </Grid2>
180-183: Consider adding PropTypes for Redux-injected props.The component receives several props from Redux (
pageTemplates,items,currentPage,term,order,orderDir,total,getPageTemplates) but onlyonCloseandonSaveare declared in PropTypes. Adding PropTypes for all props improves documentation and development-time validation.
38-40: AddgetPageTemplatesto useEffect dependency array.The effect references
getPageTemplatesbut the dependency array is empty. While this works because the function reference is stable (from Redux), adding it satisfies the exhaustive-deps rule and makes the intent clearer.Suggested fix
useEffect(() => { getPageTemplates("", 1, DEFAULT_PER_PAGE, "id", 1, true); - }, []); + }, [getPageTemplates]);src/components/mui/formik-inputs/mui-formik-upload.js (1)
125-128: Missingidin PropTypes.The
idprop is used (line 109) but not declared in PropTypes.Suggested fix
MuiFormikUpload.propTypes = { + id: PropTypes.string, name: PropTypes.string.isRequired, singleFile: PropTypes.bool };src/actions/sponsor-pages-actions.js (1)
133-133: Silent catch block may hide errors.The empty catch block swallows all errors silently. While the comment explains the intent to catch promise rejections, consider at minimum logging the error for debugging purposes.
Suggested fix
- .catch(() => {}); // need to catch promise reject + .catch((err) => { + console.error("cloneGlobalPage failed:", err); + });src/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.js (1)
46-49: Consider addingcloneGlobalPageto PropTypes.The
cloneGlobalPageprop is injected byconnectbut not declared in PropTypes. While optional, documenting injected props improves maintainability.Proposed fix
GlobalPagePopup.propTypes = { open: PropTypes.bool.isRequired, - onClose: PropTypes.func.isRequired + onClose: PropTypes.func.isRequired, + cloneGlobalPage: PropTypes.func.isRequired };src/pages/sponsors-global/page-templates/page-template-popup.js (1)
160-162: Remove unnecessaryformikprop from PageModules.The
PageModulescomponent usesuseFormikContext()internally to access formik state (as seen in the relevant code snippet). Theformikprop passed here is unused.Proposed fix
<Box sx={{ py: 2 }}> - <PageModules formik={formik} /> + <PageModules /> </Box>src/pages/sponsors-global/page-templates/page-template-module-form.test.js (2)
64-64: Minor: Comment in Spanish should be in English for consistency.The comment "Mock DragAndDropList que captura onReorder" should be in English to maintain consistency with the rest of the codebase.
Suggested fix
-// Mock DragAndDropList que captura onReorder +// Mock DragAndDropList that captures onReorder
303-316: Consider: Tests rely on MUI implementation details.Tests use
.closest(".MuiAccordionSummary-root")which couples them to MUI's class naming. This could break if MUI changes its class names in a future version. Consider addingdata-testidattributes to theAccordionSummarycomponents in the actual implementation for more resilient testing.src/pages/sponsors/sponsor-pages-list-page/index.js (3)
45-47: Missing dependency inuseEffect.The
useEffecthook usesgetSponsorPagesbut doesn't include it in the dependency array. While this works becausegetSponsorPagesis a stable reference from Redux, ESLint'sreact-hooks/exhaustive-depsrule would flag this.Suggested fix
useEffect(() => { getSponsorPages(); - }, []); + }, [getSponsorPages]);
63-71: Stubbed handlers with console.log statements.
handleRowDeleteandhandleArchiveItemcontainconsole.logstatements and commented-out code. These appear to be work-in-progress placeholders. Consider adding TODO comments or removing the console.log before merging.Suggested change
const handleRowDelete = (itemId) => { - console.log("DELETE ITEM ID...", itemId); - // deleteSponsorForm(itemId); + // TODO: Implement delete functionality + // deleteSponsorPage(itemId); }; - const handleArchiveItem = (item) => console.log("archive ITEM...", item); - // item.is_archived - // ? unarchiveSponsorForm(item.id) - // : archiveSponsorForm(item.id); + const handleArchiveItem = (item) => { + // TODO: Implement archive functionality + // item.is_archived ? unarchiveSponsorPage(item.id) : archiveSponsorPage(item.id); + };
205-212: Commented code for FormPagePopup.The
FormPagePopupcomponent is commented out, indicating incomplete functionality. Consider adding a TODO comment to track this work item.src/i18n/en.json (1)
2644-2658: Minor: Inconsistent casing in translation value.Line 2648 has
"name": "name"(lowercase) while the corresponding column header on line 2632 has"name_column_label": "Name"(capitalized). The popup'snamefield label should likely be capitalized for consistency.Suggested fix
"global_page_popup": { "title": "Add Page Template", "items_selected": "items selected", "code": "Code", - "name": "name", + "name": "Name", "info_mod": "Info Mod",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/actions/page-template-actions.jssrc/actions/sponsor-pages-actions.jssrc/components/menu/index.jssrc/components/mui/formik-inputs/mui-formik-datepicker.jssrc/components/mui/formik-inputs/mui-formik-radio-group.jssrc/components/mui/formik-inputs/mui-formik-select.jssrc/components/mui/formik-inputs/mui-formik-upload.jssrc/i18n/en.jsonsrc/layouts/sponsor-layout.jssrc/pages/sponsors-global/page-templates/page-template-module-form.test.jssrc/pages/sponsors-global/page-templates/page-template-modules-form.jssrc/pages/sponsors-global/page-templates/page-template-popup.jssrc/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.jssrc/pages/sponsors/sponsor-pages-list-page/components/global-page/select-pages-dialog.jssrc/pages/sponsors/sponsor-pages-list-page/index.jssrc/reducers/sponsors/sponsor-pages-list-reducer.jssrc/store.jssrc/utils/constants.js
🧰 Additional context used
🧬 Code graph analysis (8)
src/store.js (1)
src/reducers/sponsors/sponsor-pages-list-reducer.js (1)
sponsorPagesListReducer(33-83)
src/components/menu/index.js (1)
src/reducers/notes/notes-reducer.js (1)
summitId(48-48)
src/pages/sponsors-global/page-templates/page-template-popup.js (2)
src/pages/sponsors-global/page-templates/page-template-modules-form.js (2)
modules(200-200)PageModules(198-343)src/utils/constants.js (4)
PAGES_MODULE_KINDS(245-249)PAGES_MODULE_KINDS(245-249)PAGE_MODULES_MEDIA_TYPES(251-254)PAGE_MODULES_MEDIA_TYPES(251-254)
src/components/mui/formik-inputs/mui-formik-select.js (6)
src/components/mui/formik-inputs/mui-formik-datepicker.js (1)
field(9-9)src/components/mui/formik-inputs/mui-formik-upload.js (1)
field(14-14)src/components/mui/formik-inputs/mui-formik-textfield.js (1)
field(7-7)src/components/mui/formik-inputs/sponsorship-input-mui.js (2)
field(31-31)field(37-37)src/components/mui/formik-inputs/mui-formik-checkbox.js (1)
field(12-12)src/components/mui/formik-inputs/company-input-mui.js (2)
field(32-32)field(38-38)
src/actions/page-template-actions.js (1)
src/utils/constants.js (2)
PAGES_MODULE_KINDS(245-249)PAGES_MODULE_KINDS(245-249)
src/components/mui/formik-inputs/mui-formik-upload.js (1)
src/utils/constants.js (6)
MAX_INVENTORY_IMAGE_UPLOAD_SIZE(190-190)MAX_INVENTORY_IMAGE_UPLOAD_SIZE(190-190)MAX_INVENTORY_IMAGES_UPLOAD_QTY(191-191)MAX_INVENTORY_IMAGES_UPLOAD_QTY(191-191)ALLOWED_INVENTORY_IMAGE_FORMATS(192-192)ALLOWED_INVENTORY_IMAGE_FORMATS(192-192)
src/pages/sponsors-global/page-templates/page-template-modules-form.js (3)
src/utils/constants.js (4)
PAGE_MODULES_MEDIA_TYPES(251-254)PAGE_MODULES_MEDIA_TYPES(251-254)PAGES_MODULE_KINDS(245-249)PAGES_MODULE_KINDS(245-249)src/components/mui/showConfirmDialog.js (1)
showConfirmDialog(5-42)src/components/mui/dnd-list.js (1)
DragAndDropList(16-72)
src/reducers/sponsors/sponsor-pages-list-reducer.js (1)
src/actions/sponsor-pages-actions.js (4)
REQUEST_SPONSOR_PAGES(31-31)REQUEST_SPONSOR_PAGES(31-31)RECEIVE_SPONSOR_PAGES(32-32)RECEIVE_SPONSOR_PAGES(32-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (22)
src/components/mui/formik-inputs/mui-formik-select.js (2)
1-12: LGTM!The new imports for
IconButtonandInputLabelare appropriate for the added label and clearable functionality.
29-30: LGTM!The
hasValueguard correctly checks for non-empty values, handling empty string,undefined, andnullcases appropriately.src/utils/constants.js (1)
244-254: LGTM!The new constants are well-organized and follow the existing patterns in this file.
PAGES_MODULE_KINDSandPAGE_MODULES_MEDIA_TYPESprovide clear, centralized definitions for module kinds and media types used throughout the sponsor pages feature.src/actions/page-template-actions.js (1)
14-33: LGTM!The new imports for
moment-timezoneandPAGES_MODULE_KINDSare correctly placed and support the enhanced module normalization logic.src/actions/sponsor-pages-actions.js (1)
68-72: Verify the filter logic for sponsorship types.The filter uses
sponsorship_type_id_not_inwhich appears to exclude rather than include the specified sponsorship types. Confirm this is the intended behavior for the use case.src/components/mui/formik-inputs/mui-formik-upload.js (1)
76-91: Verify the property name of the imageFile parameter passed by UploadInputV2's onRemove callback.The filter on line 84 compares
i.filename !== imageFile.name, but without access to the external UploadInputV2 library (fromopenstack-uicore-foundation), it cannot be definitively confirmed whetherimageFile.nameis correct. Cross-checking with similar code in inventory-item-form.js and form-template-form.js shows identical patterns, but this could indicate either consistent behavior or a systemic issue. Confirm the property name that UploadInputV2 passes to the onRemove callback—if it's notname, the filter will fail silently and files won't be removed from the array.src/reducers/sponsors/sponsor-pages-list-reducer.js (2)
21-31: LGTM!The DEFAULT_STATE structure is well-defined with appropriate defaults for pagination, sorting, and filtering.
36-52: LGTM!Proper handling of state reset on summit change/logout and request state management with cleared data during loading.
src/store.js (2)
170-170: LGTM!Import correctly added for the new sponsor pages list reducer.
254-254: LGTM!New state slice properly registered following the existing naming conventions and placed logically with other sponsor-related state.
src/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.js (1)
25-29: LGTM!Good use of
.finally()to ensure cleanup regardless of success or failure.src/layouts/sponsor-layout.js (1)
67-87: LGTM!The route structure correctly follows the existing pattern with proper breadcrumb integration and nested Switch/Route setup.
src/pages/sponsors-global/page-templates/page-template-popup.js (3)
33-41: LGTM!Good extraction of common module creation logic into
addModulehelper. The temporary ID and order assignment pattern is clean.
43-70: LGTM!Module handlers properly initialize all required fields for each module type using the extracted
addModulehelper and imported constants.
72-89: LGTM!Good approach to normalize module ordering on submit by mapping indices to
custom_order, ensuring consistent ordering regardless of user reordering during editing.src/components/menu/index.js (1)
247-251: LGTM!The new
sponsor_pagesmenu item follows the established pattern for other sponsor-related menu items, with appropriate routing and access control viaadmin-sponsors.src/pages/sponsors-global/page-templates/page-template-module-form.test.js (1)
83-116: LGTM!The test helpers
renderWithFormikandcreateModuleare well-designed and provide good isolation for testing the PageModules component. ThebeforeEachblock properly resets state between tests.src/pages/sponsors-global/page-templates/page-template-modules-form.js (3)
240-243: Consider: Module ordering may have gaps after deletion.After deleting a module, the
custom_ordervalues of remaining modules are not recalculated. For example, if modules have orders [0, 1, 2] and module at index 1 is deleted, the remaining modules will have orders [0, 2]. This may or may not be intentional depending on how the backend handles ordering.Verify if the backend expects contiguous
custom_ordervalues or if gaps are acceptable. If contiguous values are required, consider recalculating orders after deletion:if (isConfirmed) { const updated = modules .filter((_, i) => i !== index) .map((m, i) => ({ ...m, custom_order: i })); setFieldValue(name, updated); }
198-211: LGTM!The auto-scroll functionality using refs and
useEffectis well-implemented. It correctly detects when a new module is added and scrolls to make it visible.
319-342: LGTM!The main render logic correctly handles the empty state, renders the DragAndDropList with appropriate configuration, and includes the scroll target element.
src/i18n/en.json (2)
2626-2643: LGTM!The new
sponsor_pagestranslation object is well-structured with appropriate keys for the list page, column labels, filters, and empty state message.
3950-3964: LGTM!The new module-related translation keys under
page_template_list.page_crudare comprehensive and cover all the module types (Info, Document, Media) and their fields.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <Grid2 size={12}> | ||
| <InputLabel htmlFor={buildFieldName("max_file_size")}> | ||
| {T.translate("page_template_list.page_crud.description")} | ||
| </InputLabel> | ||
| <MuiFormikTextField | ||
| name={buildFieldName("description")} | ||
| fullWidth | ||
| multiline | ||
| margin="none" | ||
| rows={2} | ||
| /> | ||
| </Grid2> |
There was a problem hiding this comment.
htmlFor attribute points to wrong field name.
The InputLabel on line 183 has htmlFor={buildFieldName("max_file_size")} but the associated field is description. This is another copy-paste error affecting accessibility.
Suggested fix
<Grid2 size={12}>
- <InputLabel htmlFor={buildFieldName("max_file_size")}>
+ <InputLabel htmlFor={buildFieldName("description")}>
{T.translate("page_template_list.page_crud.description")}
</InputLabel>
<MuiFormikTextField
name={buildFieldName("description")}
fullWidth
multiline
margin="none"
rows={2}
/>
</Grid2>📝 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.
| <Grid2 size={12}> | |
| <InputLabel htmlFor={buildFieldName("max_file_size")}> | |
| {T.translate("page_template_list.page_crud.description")} | |
| </InputLabel> | |
| <MuiFormikTextField | |
| name={buildFieldName("description")} | |
| fullWidth | |
| multiline | |
| margin="none" | |
| rows={2} | |
| /> | |
| </Grid2> | |
| <Grid2 size={12}> | |
| <InputLabel htmlFor={buildFieldName("description")}> | |
| {T.translate("page_template_list.page_crud.description")} | |
| </InputLabel> | |
| <MuiFormikTextField | |
| name={buildFieldName("description")} | |
| fullWidth | |
| multiline | |
| margin="none" | |
| rows={2} | |
| /> | |
| </Grid2> |
🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-modules-form.js`
around lines 182 - 193, InputLabel's htmlFor is incorrectly targeting
buildFieldName("max_file_size") while the actual form control is
MuiFormikTextField with name buildFieldName("description"); update the
InputLabel so htmlFor uses buildFieldName("description") to match the
MuiFormikTextField name and restore proper accessibility (locate the InputLabel
and adjust the buildFieldName argument from "max_file_size" to "description").
src/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.js
Show resolved
Hide resolved
| <FormControlLabel | ||
| control={ | ||
| <Checkbox | ||
| onChange={handleHideArchivedForms} | ||
| inputProps={{ | ||
| "aria-label": T.translate("sponsor_pages.hide_archived") | ||
| }} | ||
| /> | ||
| } | ||
| label={T.translate("sponsor_pages.hide_archived")} | ||
| /> | ||
| </FormGroup> |
There was a problem hiding this comment.
Checkbox is uncontrolled - missing checked prop.
The "Hide archived" checkbox doesn't have a checked prop bound to the hideArchived state from Redux. This means the checkbox visual state won't reflect the actual filter state, especially on initial render.
Suggested fix
<FormControlLabel
control={
<Checkbox
+ checked={hideArchived}
onChange={handleHideArchivedForms}
inputProps={{
"aria-label": T.translate("sponsor_pages.hide_archived")
}}
/>
}
label={T.translate("sponsor_pages.hide_archived")}
/>📝 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.
| <FormControlLabel | |
| control={ | |
| <Checkbox | |
| onChange={handleHideArchivedForms} | |
| inputProps={{ | |
| "aria-label": T.translate("sponsor_pages.hide_archived") | |
| }} | |
| /> | |
| } | |
| label={T.translate("sponsor_pages.hide_archived")} | |
| /> | |
| </FormGroup> | |
| <FormControlLabel | |
| control={ | |
| <Checkbox | |
| checked={hideArchived} | |
| onChange={handleHideArchivedForms} | |
| inputProps={{ | |
| "aria-label": T.translate("sponsor_pages.hide_archived") | |
| }} | |
| /> | |
| } | |
| label={T.translate("sponsor_pages.hide_archived")} | |
| /> |
🤖 Prompt for AI Agents
In `@src/pages/sponsors/sponsor-pages-list-page/index.js` around lines 143 - 154,
The Checkbox is uncontrolled because it lacks a checked prop tied to the Redux
state; update the Checkbox in the FormControlLabel to pass
checked={hideArchived} (ensure hideArchived is derived from Redux state in this
component) so the visual state matches the filter on initial render, and keep
using handleHideArchivedForms to dispatch/toggle the hideArchived value; if
hideArchived can be undefined ensure it is coerced to a boolean (e.g.,
!!hideArchived) before passing to checked.
| const sponsorPages = payload.response.data.map((a) => ({ | ||
| id: a.id, | ||
| code: a.code, | ||
| name: a.name, | ||
| tier: a.sponsorship_types.map((s) => s.name).join(", "), | ||
| info_mod: a.modules.filter((m) => m.kind === "Info").length, | ||
| upload_mod: a.modules.filter((m) => m.kind === "Upload").length, | ||
| download_mod: a.modules.filter((m) => m.kind === "Download").length, | ||
| is_archived: a.is_archived | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the PAGES_MODULE_KINDS constant definition
rg -n "PAGES_MODULE_KINDS" --type=js -A 5 -B 2Repository: fntechgit/summit-admin
Length of output: 26166
🏁 Script executed:
#!/bin/bash
# Search for module kind string values used in filtering/comparisons
rg -n "kind\s*[=!:]\s*['\"]" --type=js | head -50Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look for module kind references in API responses or types
rg -n "\"kind\":|'kind':" --type=js -A 2 | head -100Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check the sponsor pages file structure for context
fd "sponsor" --type=f -e js | head -20Repository: fntechgit/summit-admin
Length of output: 1011
🏁 Script executed:
#!/bin/bash
# Check the actual sponsor pages list reducer file
cat -n src/reducers/sponsors/sponsor-pages-list-reducer.js | sed -n '55,75p'Repository: fntechgit/summit-admin
Length of output: 865
🏁 Script executed:
#!/bin/bash
# Also check what the property names should be and any usages of these properties
rg -n "upload_mod|download_mod|document_mod|media_mod" --type=jsRepository: fntechgit/summit-admin
Length of output: 2388
🏁 Script executed:
#!/bin/bash
# Look for API response examples or test data
rg -n "kind.*:.*['\"]" src/pages/sponsors-global/page-templates/page-template-module-form.test.js -A 2 | head -30Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for sponsor pages API examples in actions or tests
fd "sponsor-pages" --type=f -e js | xargs rg -l "modules"Repository: fntechgit/summit-admin
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Check page-template-list-reducer for comparison
cat -n src/reducers/sponsors_inventory/page-template-list-reducer.js | sed -n '70,85p'Repository: fntechgit/summit-admin
Length of output: 728
🏁 Script executed:
#!/bin/bash
# Check if there are any test fixtures or mock data for sponsor pages
fd "sponsor-pages" --type=f -e test.js | xargs cat | head -200Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Look for API response mock data in sponsor pages actions
rg -n "modules.*\[" src/actions/sponsor-pages-actions.js -A 5 | head -50Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if "Upload" and "Download" strings appear anywhere else as module kinds
rg -n '"Upload"|"Download"' --type=js | grep -i moduleRepository: fntechgit/summit-admin
Length of output: 605
🏁 Script executed:
#!/bin/bash
# Check the UI components that use these properties to understand intent
rg -n "upload_mod|download_mod" src/pages/sponsors/sponsor-pages-list-page/index.js -B 2 -A 2Repository: fntechgit/summit-admin
Length of output: 387
🏁 Script executed:
#!/bin/bash
# Check what the actual API might return - look at sponsor-pages-actions
cat src/actions/sponsor-pages-actions.js | head -100Repository: fntechgit/summit-admin
Length of output: 3182
🏁 Script executed:
#!/bin/bash
# Look for API response examples, mocks, or fixtures
fd . --type=f -e json | xargs rg -l "modules" | head -10Repository: fntechgit/summit-admin
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Search for test data or fixtures related to sponsor pages
find . -path ./node_modules -prune -o -type f \( -name "*.test.js" -o -name "*.spec.js" -o -name "*fixture*" -o -name "*mock*" \) -print | xargs rg -l "sponsor.*page" 2>/dev/null | head -10Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about module kinds
rg -n "Info|Upload|Download" src/actions/sponsor-pages-actions.js -B 2 -A 2Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check if the API endpoint documentation or swagger is available
fd . --type=f \( -name "swagger*" -o -name "openapi*" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | head -10Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check i18n for module kind labels
rg -n "upload_mod|download_mod|document_module|media_module|Info|Upload|Download" src/i18n/en.json | head -30Repository: fntechgit/summit-admin
Length of output: 1625
🏁 Script executed:
#!/bin/bash
# Check if there's backend API documentation or type definitions
fd . --type=f -e ts | xargs rg -l "modules|kind" 2>/dev/null | head -10Repository: fntechgit/summit-admin
Length of output: 505
🏁 Script executed:
#!/bin/bash
# Look at the complete page-template-list-reducer file structure for comparison
cat -n src/reducers/sponsors_inventory/page-template-list-reducer.js | head -100Repository: fntechgit/summit-admin
Length of output: 3581
🏁 Script executed:
#!/bin/bash
# Verify what properties are accessed from sponsorPages in the UI
rg -n "sponsorPages" src/pages/sponsors/sponsor-pages-list-page/index.js -A 3 -B 3 | head -50Repository: fntechgit/summit-admin
Length of output: 863
🏁 Script executed:
#!/bin/bash
# Check if there are any actual API examples with modules data structure
rg -n "modules.*\[" --type=js -A 10 | grep -A 10 "kind"Repository: fntechgit/summit-admin
Length of output: 4340
Fix module kind filtering to use constants instead of hardcoded strings.
The reducer filters modules using hardcoded strings "Upload" and "Download", but the PAGES_MODULE_KINDS constant defines the actual API values as "Document" and "Media". This mismatch causes upload_mod and download_mod to always be 0.
Update to use the constants for consistency and correctness:
Suggested fix
+import { PAGES_MODULE_KINDS } from "../../utils/constants";
...
- info_mod: a.modules.filter((m) => m.kind === "Info").length,
- upload_mod: a.modules.filter((m) => m.kind === "Upload").length,
- download_mod: a.modules.filter((m) => m.kind === "Download").length,
+ info_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.INFO).length,
+ document_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.DOCUMENT).length,
+ media_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.MEDIA).length,Note: Also update the corresponding table columns and UI references to use document_mod and media_mod instead of upload_mod and download_mod.
📝 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.
| const sponsorPages = payload.response.data.map((a) => ({ | |
| id: a.id, | |
| code: a.code, | |
| name: a.name, | |
| tier: a.sponsorship_types.map((s) => s.name).join(", "), | |
| info_mod: a.modules.filter((m) => m.kind === "Info").length, | |
| upload_mod: a.modules.filter((m) => m.kind === "Upload").length, | |
| download_mod: a.modules.filter((m) => m.kind === "Download").length, | |
| is_archived: a.is_archived | |
| })); | |
| const sponsorPages = payload.response.data.map((a) => ({ | |
| id: a.id, | |
| code: a.code, | |
| name: a.name, | |
| tier: a.sponsorship_types.map((s) => s.name).join(", "), | |
| info_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.INFO).length, | |
| document_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.DOCUMENT).length, | |
| media_mod: a.modules.filter((m) => m.kind === PAGES_MODULE_KINDS.MEDIA).length, | |
| is_archived: a.is_archived | |
| })); |
🤖 Prompt for AI Agents
In `@src/reducers/sponsors/sponsor-pages-list-reducer.js` around lines 61 - 70,
The reducer currently builds sponsorPages with module counts using hardcoded
kinds ("Upload"/"Download") causing upload_mod/download_mod to be zero; update
the modules.filter calls in the sponsorPages mapping to use the
PAGES_MODULE_KINDS constants (e.g., PAGES_MODULE_KINDS.DOCUMENT and
PAGES_MODULE_KINDS.MEDIA) when computing those counts, rename the fields from
upload_mod/download_mod to document_mod/media_mod to match the API constants,
and propagate those new field names to the corresponding table column and UI
references so they consume document_mod and media_mod instead of the old names.
| ); | ||
| }) | ||
| .catch(() => {}); // need to catch promise reject | ||
| }; |
src/actions/page-template-actions.js
Outdated
| const normalizedModule = { ...module }; | ||
|
|
||
| if (module.kind === PAGES_MODULE_KINDS.MEDIA && module.upload_deadline) { | ||
| normalizedModule.upload_deadline = moment(module.upload_deadline).unix(); |
There was a problem hiding this comment.
this seems wrong, moment will parse the date in client's timezone
| const MuiFormikRadioGroup = ({ | ||
| name, | ||
| label, | ||
| margin = "normal", |
There was a problem hiding this comment.
no need to add margin if we are spreading props
| }; | ||
|
|
||
| const hasValue = | ||
| field.value !== "" && field.value !== undefined && field.value !== null; |
There was a problem hiding this comment.
field?.value && field.value !== ""
| const MuiFormikUpload = ({ id, name, onImageDeleted, singleFile = false }) => { | ||
| const [field, meta, helpers] = useField(name); | ||
|
|
||
| console.log("images: ", field.value); |
| filename: response.name | ||
| }; | ||
| helpers.setValue([...field.value, image]); | ||
| console.log("CHJECK RESPONSE", response); |
| console.log("CHJECK RESPONSE", response); | ||
| const image = buildFileObject(response); | ||
| if (singleFile) { | ||
| helpers.setValue(image); |
There was a problem hiding this comment.
we don't we always return an array ? is more consistent
src/layouts/sponsor-layout.js
Outdated
| pathname: props.match.url | ||
| }} | ||
| /> | ||
| <Switch> |
There was a problem hiding this comment.
why a switch ? are we expecting more pages under /pages ?
| import FormikTextEditor from "../../../components/inputs/formik-text-editor"; | ||
| import MuiFormikUpload from "../../../components/mui/formik-inputs/mui-formik-upload"; | ||
|
|
||
| const InfoModule = ({ baseName, index }) => { |
There was a problem hiding this comment.
split modules into files please, lets try to keep files not too long
| code: a.code, | ||
| name: a.name, | ||
| tier: a.sponsorship_types.map((s) => s.name).join(", "), | ||
| info_mod: a.modules.filter((m) => m.kind === "Info").length, |
| </Grid2> | ||
| <Grid2 size={12}> | ||
| <MuiFormikUpload | ||
| id="document-module-upload" |
There was a problem hiding this comment.
this id should be unique dupe id will lead to issues on dom render
id={document-module-upload-${index}}
|
@tomrndom please review comments and do a general second round testing |
…ments on formik components Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/mui/formik-inputs/mui-formik-select.js (1)
74-79: Missinglabelprop in PropTypes.The
labelprop is destructured and used in the component (line 16) but is not declared in PropTypes. This will cause PropTypes validation warnings in development and reduces component self-documentation.Proposed fix
MuiFormikSelect.propTypes = { name: PropTypes.string.isRequired, children: PropTypes.node.isRequired, + label: PropTypes.string, placeholder: PropTypes.string, isClearable: PropTypes.bool };
🤖 Fix all issues with AI agents
In `@src/actions/sponsor-pages-actions.js`:
- Around line 85-93: The current getRequest(...) call in
sponsor-pages-actions.js (using createAction(REQUEST_SPONSOR_PAGES),
createAction(RECEIVE_SPONSOR_PAGES), authErrorHandler, etc.) only dispatches
stopLoading() in the .then path so a rejected request leaves the spinner
running; update the promise handling on the getRequest(...) invocation to catch
rejections and ensure stopLoading() is always dispatched (use .catch(...) to
handle errors and/or .finally(...) to call dispatch(stopLoading())), preserving
existing error handling via authErrorHandler and the original action creators.
In `@src/components/mui/formik-inputs/mui-formik-datepicker.js`:
- Line 22: The DatePicker's margin prop is currently set at the component level
and is ignored; update the mui DatePicker usage in mui-formik-datepicker (the
DatePicker component instance) to move margin="normal" into slotProps.textField
(e.g., slotProps={{ textField: { margin: "normal", ... } }}) so the TextField
receives the margin; ensure any existing slotProps.textField merging preserves
other props.
In `@src/components/mui/formik-inputs/mui-formik-upload.js`:
- Around line 57-65: The removal logic in handleRemove is comparing i.filename
to imageFile.name but uploaded file objects use file_name (created by
buildFileObject), so newly uploaded files never match; update handleRemove to
compare using the correct properties (e.g., compare (i.file_name || i.filename)
to (imageFile.name || imageFile.file_name)) and/or also fall back to matching on
id when available (i.id === imageFile.id) before calling helpers.setValue and
onImageDeleted to ensure both existing and newly uploaded files can be removed.
- Around line 37-52: The buildFileObject function currently maps response.name
to file.file_name but doesn't set file.filename, causing newly uploaded files to
miss the key expected by getInputValue and removal/display logic; update
buildFileObject to also set file.filename (e.g., from response.name ||
response.file_name) so handleUploadComplete will push objects compatible with
getInputValue and existing removal logic.
In
`@src/pages/sponsors-global/page-templates/modules/page-template-document-download-module.js`:
- Around line 48-52: The MuiFormikUpload usage is passing an invalid prop
singleFile; replace it with maxFiles={1} to restrict uploads to a single file.
Locate the MuiFormikUpload component instance
(id={`document-module-upload-${index}`} name={buildFieldName("file")}) and
remove the singleFile prop, adding maxFiles={1} instead so it matches other
usages of MuiFormikUpload across the codebase.
In
`@src/pages/sponsors-global/page-templates/modules/page-template-info-module.js`:
- Around line 18-22: FormikTextEditor currently reads form state with direct
property access (value={values[name]}, touched?.[name], errors?.[name]) which
breaks for nested field paths like "modules[0].content"; update FormikTextEditor
usage to import and use Formik's getIn utility and replace reads with
getIn(values, name) for value and use getIn(touched, name) and getIn(errors,
name) when computing the error string (e.g., error = getIn(touched, name) &&
getIn(errors, name) ? getIn(errors, name) : ""); keep existing setFieldValue and
setFieldTouched calls as they already support nested paths.
In
`@src/pages/sponsors-global/page-templates/modules/page-template-media-request-module.js`:
- Around line 76-87: The InputLabel's htmlFor is incorrectly set to
buildFieldName("max_file_size") rather than the associated select's name; update
the htmlFor to buildFieldName("file_type_id") so the InputLabel correctly
targets the MuiFormikSelect with name buildFieldName("file_type_id") (look for
the InputLabel and the MuiFormikSelect components in
page-template-media-request-module.js).
In `@src/pages/sponsors-global/page-templates/page-template-modules-form.js`:
- Around line 129-134: Add an accessible label to the icon-only delete button:
update the IconButton element that calls handleDeleteModule (the one wrapping
<DeleteIcon />) to include an aria-label using the existing translation key
'general.delete' (e.g., via t('general.delete')) so screen readers can announce
the button purpose while keeping the onClick behavior intact.
♻️ Duplicate comments (1)
src/reducers/sponsors/sponsor-pages-list-reducer.js (1)
67-73: Variable names still don't align with constant names.The past review suggested renaming
upload_modtomedia_modanddownload_modtodocument_modto match thePAGES_MODULE_KINDSconstants being used. While the hardcoded strings have been replaced with constants (good!), the variable naming inconsistency remains.
🧹 Nitpick comments (5)
src/layouts/sponsor-layout.js (1)
67-85: Consider wrapping inner Route in Switch for consistency.The route structure is functional, but unlike the
/formsroute (lines 50-63) which wraps nested routes in a<Switch>, this uses a bare<Route>. If nested routes are planned (e.g.,/:page_idfor editing individual pages), adding a<Switch>now would maintain consistency and simplify future additions.Optional: Add Switch wrapper
<Breadcrumb data={{ title: T.translate("sponsor_pages.pages"), pathname: props.match.url }} /> + <Switch> <Route strict exact path={props.match.url} component={SponsorPagesListPage} /> + </Switch>src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
77-83: Variable names don't align with constant names, which may cause confusion.The variable names
upload_modanddownload_moddon't semantically match the constants they're filtering on (PAGES_MODULE_KINDS.MEDIAandPAGES_MODULE_KINDS.DOCUMENTrespectively). This creates a disconnect that could confuse maintainers.Consider renaming for clarity:
upload_mod→media_mod(since it filtersMEDIAmodules)download_mod→document_mod(since it filtersDOCUMENTmodules)Alternatively, if these names are intentional for UI purposes, add a brief comment explaining the mapping.
src/pages/sponsors-global/page-templates/modules/page-template-media-request-module.js (1)
84-86: Hardcoded file type options may need expansion.The select currently only offers "PDF" as an option. If more file types should be supported, consider making this dynamic via props or constants.
src/pages/sponsors-global/page-templates/page-template-modules-form.js (1)
155-162: Consider makingdroppableIdunique per instance.If multiple PageModules are ever rendered on the same page, a static ID can collide in the DnD layer.
♻️ Proposed refactor
- droppableId="modules-list" + droppableId={`modules-list-${name}`}src/pages/sponsors/sponsor-pages-list-page/components/global-page/global-page-popup.js (1)
25-28: Consider closing the dialog only on success.Closing in
finallyhides errors and forces users to re-open and re-select after a failed clone.♻️ Proposed refactor
- cloneGlobalPage(selectedTemplates, selectedTiers, allTiers).finally(() => { - handleClose(); - }); + cloneGlobalPage(selectedTemplates, selectedTiers, allTiers) + .then(() => { + handleClose(); + }) + .catch(() => {});
| return getRequest( | ||
| createAction(REQUEST_SPONSOR_PAGES), | ||
| createAction(RECEIVE_SPONSOR_PAGES), | ||
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/show-pages`, | ||
| authErrorHandler, | ||
| { order, orderDir, page, term, hideArchived } | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file and lines mentioned
cat -n src/actions/sponsor-pages-actions.js | sed -n '80,100p'Repository: fntechgit/summit-admin
Length of output: 975
🏁 Script executed:
# Find where getRequest is defined
rg "const getRequest|function getRequest|export.*getRequest" --type js -B 2 -A 10Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Check imports in the file to see where getRequest comes from
head -30 src/actions/sponsor-pages-actions.jsRepository: fntechgit/summit-admin
Length of output: 1180
🏁 Script executed:
# Find cloneGlobalPage implementation
sed -n '96,150p' src/actions/sponsor-pages-actions.jsRepository: fntechgit/summit-admin
Length of output: 1204
🏁 Script executed:
# Search for other uses of getRequest to see error handling patterns
rg "getRequest\(" src/actions/ -A 6 -B 1 | head -100Repository: fntechgit/summit-admin
Length of output: 6135
🏁 Script executed:
# Search for authErrorHandler patterns with catch or finally
rg "authErrorHandler.*\)" -A 8 | grep -E "(catch|finally)" -B 5Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Search for postRequest patterns to see if they use different error handling
rg "postRequest\(" src/actions/ -A 6 | grep -E "(catch|finally|\.then)" -B 2 -A 2 | head -60Repository: fntechgit/summit-admin
Length of output: 2526
🏁 Script executed:
# Search for .catch() handlers in actions to understand error handling patterns
rg "\.catch\(" src/actions/ -B 3 -A 3 | head -80Repository: fntechgit/summit-admin
Length of output: 4194
🏁 Script executed:
# Check sponsor-forms-actions.js to see those catch patterns in context
rg "need to catch promise reject" src/actions/sponsor-forms-actions.js -B 10 -A 3 | head -80Repository: fntechgit/summit-admin
Length of output: 2389
🏁 Script executed:
# Let's verify the exact getSponsorPages implementation one more time to be certain
sed -n '60,100p' src/actions/sponsor-pages-actions.jsRepository: fntechgit/summit-admin
Length of output: 1336
Ensure loading state clears on request failure.
stopLoading() runs only on success; if getRequest rejects, the spinner can remain stuck. Similar functions in the codebase explicitly handle rejection with .catch() and .finally() patterns (see sponsor-forms-actions.js with comments "need to catch promise reject").
🐛 Proposed fix
return getRequest(
createAction(REQUEST_SPONSOR_PAGES),
createAction(RECEIVE_SPONSOR_PAGES),
`${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/show-pages`,
authErrorHandler,
{ order, orderDir, page, term, hideArchived }
- )(params)(dispatch).then(() => {
- dispatch(stopLoading());
- });
+ )(params)(dispatch).finally(() => {
+ dispatch(stopLoading());
+ });📝 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.
| return getRequest( | |
| createAction(REQUEST_SPONSOR_PAGES), | |
| createAction(RECEIVE_SPONSOR_PAGES), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/show-pages`, | |
| authErrorHandler, | |
| { order, orderDir, page, term, hideArchived } | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| }); | |
| return getRequest( | |
| createAction(REQUEST_SPONSOR_PAGES), | |
| createAction(RECEIVE_SPONSOR_PAGES), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/show-pages`, | |
| authErrorHandler, | |
| { order, orderDir, page, term, hideArchived } | |
| )(params)(dispatch).finally(() => { | |
| dispatch(stopLoading()); | |
| }); |
🤖 Prompt for AI Agents
In `@src/actions/sponsor-pages-actions.js` around lines 85 - 93, The current
getRequest(...) call in sponsor-pages-actions.js (using
createAction(REQUEST_SPONSOR_PAGES), createAction(RECEIVE_SPONSOR_PAGES),
authErrorHandler, etc.) only dispatches stopLoading() in the .then path so a
rejected request leaves the spinner running; update the promise handling on the
getRequest(...) invocation to catch rejections and ensure stopLoading() is
always dispatched (use .catch(...) to handle errors and/or .finally(...) to call
dispatch(stopLoading())), preserving existing error handling via
authErrorHandler and the original action creators.
| helperText: meta.touched && meta.error, | ||
| fullWidth: true, | ||
| margin: "normal" | ||
| fullWidth: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "mui-formik-datepicker.js" -type fRepository: fntechgit/summit-admin
Length of output: 125
🏁 Script executed:
head -50 src/components/mui/formik-inputs/mui-formik-datepicker.jsRepository: fntechgit/summit-admin
Length of output: 1536
🌐 Web query:
MUI X DatePicker margin prop API documentation
💡 Result:
In MUI X DatePicker, the margin prop is not a direct DatePicker prop; it belongs to the underlying field component (a pickers TextField).
-
API docs for
margin: it’s documented onPickersTextField(and also onDateField) asmargin: 'none' | 'dense' | 'normal'with default'none'. [1], [2] -
How to set it on
DatePicker(current MUI X): pass it to the field viaslotProps.textField:<DatePicker slotProps={{ textField: { margin: 'dense' } }} />
This “Customize the TextField” approach is shown in the Custom field docs. [3]
Sources:
[1] PickersTextField API (margin) (mui.com)
[2] DateField API (margin) (mui.com)
[3] Custom field docs (use slotProps.textField to pass TextField props) (mui.com)
Citations:
- 1: https://mui.com/x/api/date-pickers/pickers-text-field/?utm_source=openai
- 2: https://mui.com/x/api/date-pickers/date-field/?utm_source=openai
- 3: https://mui.com/x/react-date-pickers/custom-field/?utm_source=openai
🌐 Web query:
"DatePicker" "slotProps" "textField" margin MUI X
💡 Result:
To set the margin on the input used by MUI X DatePicker, pass it through the textField slotProps:
<DatePicker
label="Start date"
slotProps={{
textField: { margin: 'dense' }, // or 'normal' | 'none'
}}
/>slotProps.textField forwards props to the underlying picker field TextField, and margin is a supported prop ('dense' | 'normal' | 'none', default 'none'). [1], [2]
margin="normal" won't be applied on DatePicker.
MUI X DatePicker does not accept a margin prop directly—it must be passed through slotProps.textField. Placing it on the component level will be silently ignored. Move margin into slotProps.textField for it to take effect.
🐛 Proposed fix
slotProps={{
textField: {
name,
label: required ? requiredLabel : label,
error: meta.touched && Boolean(meta.error),
helperText: meta.touched && meta.error,
- fullWidth: true
+ fullWidth: true,
+ margin: "normal"
},
day: {
sx: {
fontSize: "1.2rem",
fontWeight: 600
}
},
layout: {
sx: {
"& .MuiDayCalendar-weekDayLabel": {
fontSize: "1rem"
}
}
}
}}
- margin="normal"
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}🤖 Prompt for AI Agents
In `@src/components/mui/formik-inputs/mui-formik-datepicker.js` at line 22, The
DatePicker's margin prop is currently set at the component level and is ignored;
update the mui DatePicker usage in mui-formik-datepicker (the DatePicker
component instance) to move margin="normal" into slotProps.textField (e.g.,
slotProps={{ textField: { margin: "normal", ... } }}) so the TextField receives
the margin; ensure any existing slotProps.textField merging preserves other
props.
| const buildFileObject = (response) => { | ||
| const file = {}; | ||
| if (response.id !== undefined) file.id = response.id; | ||
| if (response.name) file.file_name = response.name; | ||
| if (response.md5) file.md5 = response.md5; | ||
| if (response.mime_type) file.mime_type = response.mime_type; | ||
| if (response.source_bucket) file.bucket = response.source_bucket; | ||
| if (response.path && response.name) | ||
| file.file_path = `${response.path}${response.name}`; | ||
| return file; | ||
| }; | ||
|
|
||
| const handleUploadComplete = (response) => { | ||
| if (response) { | ||
| const image = { | ||
| file_path: `${response.path}${response.name}`, | ||
| filename: response.name | ||
| }; | ||
| helpers.setValue([...field.value, image]); | ||
| const image = buildFileObject(response); | ||
| helpers.setValue([...(field.value || []), image]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consider adding filename property in buildFileObject for consistency.
The getInputValue function expects a filename property to exist for display purposes (lines 32-33 show fallback chain). Adding it in buildFileObject would ensure newly uploaded files are immediately compatible with the removal logic and display.
Suggested fix
const buildFileObject = (response) => {
const file = {};
if (response.id !== undefined) file.id = response.id;
if (response.name) file.file_name = response.name;
+ if (response.name) file.filename = response.name;
if (response.md5) file.md5 = response.md5;
if (response.mime_type) file.mime_type = response.mime_type;
if (response.source_bucket) file.bucket = response.source_bucket;
if (response.path && response.name)
file.file_path = `${response.path}${response.name}`;
return file;
};📝 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.
| const buildFileObject = (response) => { | |
| const file = {}; | |
| if (response.id !== undefined) file.id = response.id; | |
| if (response.name) file.file_name = response.name; | |
| if (response.md5) file.md5 = response.md5; | |
| if (response.mime_type) file.mime_type = response.mime_type; | |
| if (response.source_bucket) file.bucket = response.source_bucket; | |
| if (response.path && response.name) | |
| file.file_path = `${response.path}${response.name}`; | |
| return file; | |
| }; | |
| const handleUploadComplete = (response) => { | |
| if (response) { | |
| const image = { | |
| file_path: `${response.path}${response.name}`, | |
| filename: response.name | |
| }; | |
| helpers.setValue([...field.value, image]); | |
| const image = buildFileObject(response); | |
| helpers.setValue([...(field.value || []), image]); | |
| const buildFileObject = (response) => { | |
| const file = {}; | |
| if (response.id !== undefined) file.id = response.id; | |
| if (response.name) file.file_name = response.name; | |
| if (response.name) file.filename = response.name; | |
| if (response.md5) file.md5 = response.md5; | |
| if (response.mime_type) file.mime_type = response.mime_type; | |
| if (response.source_bucket) file.bucket = response.source_bucket; | |
| if (response.path && response.name) | |
| file.file_path = `${response.path}${response.name}`; | |
| return file; | |
| }; | |
| const handleUploadComplete = (response) => { | |
| if (response) { | |
| const image = buildFileObject(response); | |
| helpers.setValue([...(field.value || []), image]); |
🤖 Prompt for AI Agents
In `@src/components/mui/formik-inputs/mui-formik-upload.js` around lines 37 - 52,
The buildFileObject function currently maps response.name to file.file_name but
doesn't set file.filename, causing newly uploaded files to miss the key expected
by getInputValue and removal/display logic; update buildFileObject to also set
file.filename (e.g., from response.name || response.file_name) so
handleUploadComplete will push objects compatible with getInputValue and
existing removal logic.
| const handleRemove = (imageFile) => { | ||
| const updated = field.value.filter((i) => i.filename !== imageFile.name); | ||
| const updated = (field.value || []).filter( | ||
| (i) => i.filename !== imageFile.name | ||
| ); | ||
| helpers.setValue(updated); | ||
|
|
||
| if (onImageDeleted) { | ||
| onImageDeleted(imageFile.id); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Bug: handleRemove won't work for newly uploaded files due to property name mismatch.
The handleRemove function filters by i.filename !== imageFile.name, but buildFileObject creates objects with file_name (not filename). Newly uploaded files won't have a filename property, so they can never be removed.
Additionally, imageFile.name comes from the UploadInputV2 callback, but the file object stored has file_name. The comparison logic needs alignment.
Suggested fix
const handleRemove = (imageFile) => {
const updated = (field.value || []).filter(
- (i) => i.filename !== imageFile.name
+ (i) => (i.filename || i.file_name) !== imageFile.name
);
helpers.setValue(updated);
if (onImageDeleted) {
onImageDeleted(imageFile.id);
}
};📝 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.
| const handleRemove = (imageFile) => { | |
| const updated = field.value.filter((i) => i.filename !== imageFile.name); | |
| const updated = (field.value || []).filter( | |
| (i) => i.filename !== imageFile.name | |
| ); | |
| helpers.setValue(updated); | |
| if (onImageDeleted) { | |
| onImageDeleted(imageFile.id); | |
| } | |
| }; | |
| const handleRemove = (imageFile) => { | |
| const updated = (field.value || []).filter( | |
| (i) => (i.filename || i.file_name) !== imageFile.name | |
| ); | |
| helpers.setValue(updated); | |
| if (onImageDeleted) { | |
| onImageDeleted(imageFile.id); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@src/components/mui/formik-inputs/mui-formik-upload.js` around lines 57 - 65,
The removal logic in handleRemove is comparing i.filename to imageFile.name but
uploaded file objects use file_name (created by buildFileObject), so newly
uploaded files never match; update handleRemove to compare using the correct
properties (e.g., compare (i.file_name || i.filename) to (imageFile.name ||
imageFile.file_name)) and/or also fall back to matching on id when available
(i.id === imageFile.id) before calling helpers.setValue and onImageDeleted to
ensure both existing and newly uploaded files can be removed.
src/pages/sponsors-global/page-templates/modules/page-template-document-download-module.js
Show resolved
Hide resolved
| <FormikTextEditor | ||
| name={buildFieldName("content")} | ||
| fullWidth | ||
| margin="none" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat src/components/inputs/formik-text-editor.jsRepository: fntechgit/summit-admin
Length of output: 757
Fix FormikTextEditor to handle nested field names correctly.
The component uses direct property access (values[name], touched?.[name], errors?.[name]) which won't work for nested field paths like modules[0].content. It should use Formik's getIn() utility to properly access nested values.
<details>
<summary>Current implementation (broken for nested fields)</summary>
```javascript
value={values[name]}
error={touched?.[name] && errors?.[name] ? errors?.[name] : ""}
onBlur={() => setFieldTouched(name, true)}Update to:
value={getIn(values, name)}
error={getIn(touched, name) && getIn(errors, name) ? getIn(errors, name) : ""}Note: setFieldValue() and setFieldTouched() already handle nested paths correctly, so those calls don't need changes. Only the value reads need fixing.
🤖 Prompt for AI Agents
In
`@src/pages/sponsors-global/page-templates/modules/page-template-info-module.js`
around lines 18 - 22, FormikTextEditor currently reads form state with direct
property access (value={values[name]}, touched?.[name], errors?.[name]) which
breaks for nested field paths like "modules[0].content"; update FormikTextEditor
usage to import and use Formik's getIn utility and replace reads with
getIn(values, name) for value and use getIn(touched, name) and getIn(errors,
name) when computing the error string (e.g., error = getIn(touched, name) &&
getIn(errors, name) ? getIn(errors, name) : ""); keep existing setFieldValue and
setFieldTouched calls as they already support nested paths.
src/pages/sponsors-global/page-templates/modules/page-template-media-request-module.js
Show resolved
Hide resolved
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/pages/sponsors-global/page-templates/page-template-list-page.js`:
- Around line 122-133: handleDelete currently passes the full row object to
deletePageTemplate but that action expects a template ID (formTemplateId);
update handleDelete to extract and pass row.formTemplateId to deletePageTemplate
(and optionally guard against missing formTemplateId by returning early or
throwing), e.g. call deletePageTemplate(row.formTemplateId).then(() =>
getPageTemplates(...)) so the API receives the correct ID.
In `@src/pages/sponsors-global/page-templates/page-template-modules-form.js`:
- Around line 88-143: The Accordion returned by renderModule lacks a React key;
update the renderModule function to add a stable key prop to the top-level
Accordion (for example key={module.id || `module-${index}`}) so React warnings
are avoided and list reconciliation is stable—modify the Accordion element in
renderModule to include this key.
In `@src/pages/sponsors-global/page-templates/page-template-popup.js`:
- Around line 160-162: The PageModules component is being passed an unused
formik prop even though it uses useFormikContext() internally and accepts an
optional name prop (default "modules"); remove the formik={formik} prop from the
PageModules invocation in page-template-popup.js (the JSX inside the Box) so it
is called simply as <PageModules /> or with an explicit name if needed, and
ensure there are no other references expecting a formik prop in the PageModules
usage.
🧹 Nitpick comments (3)
src/pages/sponsors-global/page-templates/modules/page-template-media-request-module.js (1)
80-86: Consider making file type options configurable or fetching from API.The file type select currently only offers "PDF" as a hardcoded option. If more file types are expected in the future, consider fetching available file types from an API or defining them as constants.
src/pages/sponsors-global/page-templates/page-template-modules-form.js (1)
38-49: Consider translating the default "Module" string.The default case returns a hardcoded "Module" string while other cases use
T.translate(). For consistency and i18n support, consider adding a translation key for the default case.Suggested fix
case PAGES_MODULE_KINDS.MEDIA: return T.translate("page_template_list.page_crud.media_module"); default: - return "Module"; + return T.translate("page_template_list.page_crud.module"); }src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
105-107: Stub implementation for clone functionality.
handleClonePageTemplateonly logs to console. Ensure this is intentional placeholder code and will be implemented before release.Would you like me to help implement the clone functionality or create an issue to track this task?
| const handleDelete = (row) => { | ||
| console.log("DELETE", row); | ||
| deletePageTemplate(row).then(() => | ||
| getPageTemplates( | ||
| term, | ||
| currentPage, | ||
| perPage, | ||
| order, | ||
| orderDir, | ||
| hideArchived | ||
| ) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Bug: deletePageTemplate receives wrong argument type.
The deletePageTemplate action expects a template ID (formTemplateId), but it's being called with the entire row object. This will cause the API call to fail or behave unexpectedly.
Suggested fix
const handleDelete = (row) => {
- deletePageTemplate(row).then(() =>
+ deletePageTemplate(row.id).then(() =>
getPageTemplates(
term,
currentPage,
perPage,
order,
orderDir,
hideArchived
)
);
};📝 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.
| const handleDelete = (row) => { | |
| console.log("DELETE", row); | |
| deletePageTemplate(row).then(() => | |
| getPageTemplates( | |
| term, | |
| currentPage, | |
| perPage, | |
| order, | |
| orderDir, | |
| hideArchived | |
| ) | |
| ); | |
| }; | |
| const handleDelete = (row) => { | |
| deletePageTemplate(row.id).then(() => | |
| getPageTemplates( | |
| term, | |
| currentPage, | |
| perPage, | |
| order, | |
| orderDir, | |
| hideArchived | |
| ) | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-list-page.js` around
lines 122 - 133, handleDelete currently passes the full row object to
deletePageTemplate but that action expects a template ID (formTemplateId);
update handleDelete to extract and pass row.formTemplateId to deletePageTemplate
(and optionally guard against missing formTemplateId by returning early or
throwing), e.g. call deletePageTemplate(row.formTemplateId).then(() =>
getPageTemplates(...)) so the API receives the correct ID.
| const renderModule = (module, index) => ( | ||
| <Accordion | ||
| defaultExpanded | ||
| sx={{ | ||
| mb: 1, | ||
| "&:before": { display: "none" }, | ||
| boxShadow: "none", | ||
| border: "1px solid #e0e0e0", | ||
| borderRadius: "0 !important", | ||
| "&:first-of-type": { borderRadius: 0 }, | ||
| "&:last-of-type": { borderRadius: 0 } | ||
| }} | ||
| > | ||
| <AccordionSummary | ||
| expandIcon={<ExpandMoreIcon />} | ||
| sx={{ | ||
| backgroundColor: "#2196F31F", | ||
| flexDirection: "row-reverse", | ||
| "& .MuiAccordionSummary-expandIconWrapper": { | ||
| marginRight: 1, | ||
| marginLeft: 0 | ||
| } | ||
| }} | ||
| > | ||
| <Box | ||
| sx={{ | ||
| display: "flex", | ||
| alignItems: "center", | ||
| width: "100%", | ||
| justifyContent: "space-between" | ||
| }} | ||
| > | ||
| <Typography>{getModuleTitle(module.kind)}</Typography> | ||
|
|
||
| <Box | ||
| sx={{ display: "flex", alignItems: "center" }} | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| <UnfoldMoreIcon | ||
| sx={{ mr: 1, color: "action.active", cursor: "grab" }} | ||
| /> | ||
| <IconButton | ||
| size="small" | ||
| aria-label={T.translate("general.delete")} | ||
| onClick={() => handleDeleteModule(index, module)} | ||
| > | ||
| <DeleteIcon /> | ||
| </IconButton> | ||
| </Box> | ||
| </Box> | ||
| </AccordionSummary> | ||
| <AccordionDetails sx={{ p: 2 }}> | ||
| {renderModuleFields(module, index)} | ||
| </AccordionDetails> | ||
| </Accordion> | ||
| ); |
There was a problem hiding this comment.
Missing key prop on Accordion component.
The renderModule function returns an Accordion without a key prop. While DragAndDropList handles the key via the idKey prop for its wrapper, React may still warn about missing keys. Consider adding a key for clarity and to avoid potential issues.
Suggested fix
- const renderModule = (module, index) => (
- <Accordion
+ const renderModule = (module, index) => (
+ <Accordion
+ key={module._tempId || `module-${index}`}
defaultExpanded
sx={{🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-modules-form.js`
around lines 88 - 143, The Accordion returned by renderModule lacks a React key;
update the renderModule function to add a stable key prop to the top-level
Accordion (for example key={module.id || `module-${index}`}) so React warnings
are avoided and list reconciliation is stable—modify the Accordion element in
renderModule to include this key.
| <Box sx={{ py: 2 }}> | ||
| <PageModules formik={formik} /> | ||
| </Box> |
There was a problem hiding this comment.
Remove unused formik prop from PageModules.
PageModules uses useFormikContext() internally and expects an optional name prop (default: "modules"), not a formik prop. The passed formik prop is unused and should be removed.
Suggested fix
<Box sx={{ py: 2 }}>
- <PageModules formik={formik} />
+ <PageModules />
</Box>📝 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.
| <Box sx={{ py: 2 }}> | |
| <PageModules formik={formik} /> | |
| </Box> | |
| <Box sx={{ py: 2 }}> | |
| <PageModules /> | |
| </Box> |
🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-popup.js` around lines
160 - 162, The PageModules component is being passed an unused formik prop even
though it uses useFormikContext() internally and accepts an optional name prop
(default "modules"); remove the formik={formik} prop from the PageModules
invocation in page-template-popup.js (the JSX inside the Box) so it is called
simply as <PageModules /> or with an explicit name if needed, and ensure there
are no other references expecting a formik prop in the PageModules usage.
ref:
https://app.clickup.com/t/86b7ubdfg
https://app.clickup.com/t/86b79918r
https://app.clickup.com/t/86b79918z
https://app.clickup.com/t/86b7wedvv
https://app.clickup.com/t/86b7wee1f
https://app.clickup.com/t/86b7wee6u
https://app.clickup.com/t/86b7wee9d
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.