Feat: sponsor pages CRUD #720
Conversation
tomrndom
force-pushed
the
feature/sponsor-pages
branch
from
269ffce to
de9802b
Compare
santipalenque
force-pushed
the
feature/sponsor-pages
branch
from
de9802b to
a765852
Compare
📝 WalkthroughWalkthroughThis pull request introduces a new page template management feature for sponsor inventory. It adds environment configuration for the Sponsor Pages API, creates Redux actions and reducers for page template CRUD operations, builds list and edit UI components, establishes new routing and menu entries, and includes translations. Additionally, it refactors import paths across the form templates section to accommodate the new page-templates directory structure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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: 9
Fix all issues with AI Agents 🤖
In @.env.example:
- Around line 20-21: The SPONSOR_PAGES_SCOPES environment variable contains
space-separated scopes and should be wrapped in quotes for proper parsing and
consistency; update the .env.example by adding quotes around the
SPONSOR_PAGES_SCOPES value (and optionally SPONSOR_PAGES_API_URL for
consistency) so the SPONSOR_PAGES_SCOPES="page-template/read
page-template/write" is used and matches quoting style used elsewhere in the
file.
In @src/actions/page-template-actions.js:
- Around line 214-225: archivePageTemplate is missing a startLoading() call so
the UI shows no loading feedback; update the archivePageTemplate async action to
invoke startLoading() (like unarchivePageTemplate does) before making the
putRequest, then proceed to call putRequest with the same params and dispatch
flow (ensure you import or reference startLoading and keep
PAGE_TEMPLATE_ARCHIVED as the success action).
In @src/layouts/page-template-layout.js:
- Around line 38-43: The route is declaring :page_template_id but
EditPageTemplatePage reads match.params.form_template_id, so the ID is
undefined; fix by making the parameter names consistent—either change the Route
path `${match.url}/:page_template_id(\\d+)` to
`${match.url}/:form_template_id(\\d+)` so it matches EditPageTemplatePage, or
update EditPageTemplatePage to read match.params.page_template_id (or fallback
to match.params.form_template_id) where it sets formTemplateId.
In @src/pages/sponsors-global/page-templates/page-template-list-page.js:
- Line 2: Update the file header copyright year from 2024 to 2025 to match the
rest of the PR and project headers; locate the top-of-file copyright comment in
page-template-list-page.js and change the year token "2024" to "2025".
In @src/pages/sponsors-global/page-templates/page-template-popup.js:
- Around line 144-148: PageTemplatePopup is missing the pageTemplate prop in its
PropTypes declaration; update the PageTemplatePopup.propTypes block to include a
pageTemplate entry (use PropTypes.object or a more specific PropTypes.shape
describing expected fields, and append .isRequired if the Redux selector always
provides it) so the component's received Redux prop is properly validated.
- Line 89: The Divider component is using an unsupported prop `gutterBottom`;
remove the `gutterBottom` attribute from the Divider element and replace spacing
by applying an sx margin-bottom style (e.g., sx with mb or marginBottom) on that
Divider or its surrounding container so visual spacing is preserved; locate the
Divider instance (Divider) in page-template-popup.js and update its props
accordingly.
- Around line 24-26: The handleClose handler currently only calls onClose() and
leaves form state intact; update it to call formik.resetForm() before onClose()
so the popup resets validation and values on close, and move the handleClose
declaration to after the useFormik() call so it can reference formik; ensure you
call formik.resetForm() (or formik.resetForm({ values: initialValues }) if
needed) then onClose() inside the updated handleClose function.
In @src/reducers/sponsors_inventory/page-template-list-reducer.js:
- Line 2: Update the file header copyright year from 2024 to 2025 in
src/reducers/sponsors_inventory/page-template-list-reducer.js by editing the top
comment block (the copyright comment) so it matches the other files in the PR.
🧹 Nitpick comments (4)
src/pages/sponsors-global/page-templates/edit-page-template-page.js (1)
58-60: Translation key inconsistency with page template context.This is
EditPageTemplatePagebut it usesedit_form_template.form_templatetranslation. Consider using a page template-specific translation key for consistency, or document that page templates intentionally share form template UI strings.src/pages/sponsors-global/page-templates/page-template-popup.js (1)
28-38: Placeholder handlers need implementation.These
console.logstatements indicate incomplete functionality for adding info, documents, and media. Consider adding TODO comments or opening issues to track this work.Would you like me to open an issue to track implementation of these handlers?
src/actions/page-template-actions.js (2)
99-117: Inconsistent parameter naming:formTemplateIdshould bepageTemplateId.For clarity and consistency with the page template domain, rename the parameter and references.
🔎 Proposed fix
-export const getPageTemplate = (formTemplateId) => async (dispatch) => { +export const getPageTemplate = (pageTemplateId) => async (dispatch) => { const accessToken = await getAccessTokenSafely(); dispatch(startLoading()); const params = { access_token: accessToken, expand: "materials,meta_fields,meta_fields.values" }; return getRequest( null, createAction(RECEIVE_PAGE_TEMPLATE), - `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${formTemplateId}`, + `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}`, authErrorHandler )(params)(dispatch).then(() => { dispatch(stopLoading()); }); };Apply the same rename to
deletePageTemplate(line 119) and update the payload at line 130.
177-177: Non-idiomatic thunk dispatch pattern.
getPageTemplates()(dispatch, getState)manually invokes the thunk, bypassing Redux middleware. Usedispatch(getPageTemplates())for consistency and to ensure middleware (e.g., logging, error tracking) is applied.🔎 Proposed fix
- getPageTemplates()(dispatch, getState); + dispatch(getPageTemplates());Apply the same fix at line 202.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.env.examplesrc/actions/page-template-actions.jssrc/app.jssrc/components/menu/index.jssrc/i18n/en.jsonsrc/layouts/form-template-item-layout.jssrc/layouts/form-template-layout.jssrc/layouts/inventory-item-layout.jssrc/layouts/page-template-layout.jssrc/layouts/primary-layout.jssrc/pages/sponsors-global/form-templates/add-form-template-item-popup.jssrc/pages/sponsors-global/form-templates/edit-form-template-item-page.jssrc/pages/sponsors-global/form-templates/edit-form-template-page.jssrc/pages/sponsors-global/form-templates/form-template-from-duplicate-popup.jssrc/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/form-templates/form-template-popup.jssrc/pages/sponsors-global/form-templates/sponsor-inventory-popup.jssrc/pages/sponsors-global/inventory/edit-inventory-item-page.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/edit-page-template-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors-global/page-templates/page-template-popup.jssrc/pages/sponsors-global/shared/meta-field-values.jssrc/pages/sponsors/components/additional-input.jssrc/reducers/sponsors_inventory/page-template-list-reducer.jssrc/store.js
🧰 Additional context used
🧬 Code graph analysis (6)
src/store.js (2)
src/reducers/sponsor_settings/sponsor-settings-reducer.js (1)
sponsorSettingsReducer(48-106)src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
pageTemplateListReducer(35-122)
src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
src/actions/page-template-actions.js (10)
REQUEST_PAGE_TEMPLATES(40-40)REQUEST_PAGE_TEMPLATES(40-40)RECEIVE_PAGE_TEMPLATES(39-39)RECEIVE_PAGE_TEMPLATES(39-39)PAGE_TEMPLATE_DELETED(36-36)PAGE_TEMPLATE_DELETED(36-36)PAGE_TEMPLATE_ARCHIVED(43-43)PAGE_TEMPLATE_ARCHIVED(43-43)PAGE_TEMPLATE_UNARCHIVED(44-44)PAGE_TEMPLATE_UNARCHIVED(44-44)
src/pages/sponsors-global/page-templates/page-template-popup.js (3)
src/pages/sponsors-global/form-templates/form-template-popup.js (2)
handleClose(126-129)formik(43-98)src/components/mui/formik-inputs/mui-formik-textfield.js (1)
MuiFormikTextField(6-35)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
mapStateToProps(259-261)
src/pages/sponsors-global/page-templates/edit-page-template-page.js (7)
src/pages/sponsors-global/form-templates/edit-form-template-page.js (5)
props(29-39)formTemplateId(40-40)title(50-52)breadcrumb(53-53)mapStateToProps(74-76)src/pages/sponsors-global/inventory/edit-inventory-item-page.js (4)
props(29-39)title(50-52)breadcrumb(53-53)mapStateToProps(74-76)src/pages/sponsors-global/form-templates/form-template-item-list-page.js (1)
mapStateToProps(322-331)src/pages/sponsors-global/form-templates/form-template-list-page.js (1)
mapStateToProps(367-374)src/pages/sponsors-global/inventory/inventory-list-page.js (1)
mapStateToProps(312-319)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
mapStateToProps(259-261)src/pages/sponsors-global/page-templates/page-template-popup.js (1)
mapStateToProps(150-152)
src/layouts/primary-layout.js (1)
src/layouts/page-template-layout.js (1)
PageTemplateLayout(23-53)
src/actions/page-template-actions.js (3)
src/pages/sponsors-global/form-templates/edit-form-template-page.js (1)
formTemplateId(40-40)src/pages/sponsors-global/page-templates/edit-page-template-page.js (1)
formTemplateId(40-40)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
pageTemplateId(52-52)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 21-21: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ 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 (24)
src/pages/sponsors-global/form-templates/sponsor-inventory-popup.js (1)
31-31: LGTM! Import path correctly updated to shared location.The MetaFieldValues component is now imported from the shared module, which is appropriate for a component used across multiple form templates.
src/pages/sponsors/components/additional-input.js (1)
17-17: LGTM! Consolidated to shared MetaFieldValues module.The import path correctly points to the centralized shared location for MetaFieldValues.
src/pages/sponsors-global/form-templates/form-template-popup.js (1)
23-23: LGTM! Import path refactored to shared module.Consistent with the broader refactoring to centralize MetaFieldValues in the shared directory.
src/layouts/form-template-layout.js (1)
19-20: LGTM! Import paths updated to new form template page locations.The imports correctly reference the reorganized form template pages under sponsors-global/form-templates.
src/pages/sponsors-global/form-templates/edit-form-template-page.js (1)
18-26: Import paths correctly adjusted for file location.Both imports properly use
../../../to navigate from the file's location three levels deep (src/pages/sponsors-global/form-templates/) back to thesrc/root, correctly resolving tosrc/components/forms/form-template-form.jsandsrc/actions/form-template-actions.js.src/app.js (1)
84-84: LGTM!The new global exposure of
SPONSOR_PAGES_API_URLfollows the established pattern for other API URLs in the application and is correctly placed alongside similar environment variable assignments.src/pages/sponsors-global/form-templates/edit-form-template-item-page.js (1)
18-18: Import paths have been verified and resolve correctly.Both referenced modules exist at their expected locations:
src/components/forms/inventory-item-form.js✓src/actions/form-template-item-actions.js✓The relative import paths correctly resolve from the file's location.
src/layouts/inventory-item-layout.js (1)
19-20: Import paths correctly reference relocated files.The new file locations have been verified:
inventory-list-page.jsexists atsrc/pages/sponsors-global/inventory/edit-inventory-item-page.jsexists atsrc/pages/sponsors-global/inventory/No remaining references to old paths were found in the codebase.
src/components/menu/index.js (1)
65-68: Correct the translation key reference in the menu item.The menu item has been properly added and is functional, but the translation key path needs correction:
- The translation key is
menu.page_templates(notmenu.inventory.page_templates). The key exists insrc/i18n/en.jsonline 191 with value "Pages".- The route for
page-templatesis properly configured at/app/page-templatesinsrc/layouts/primary-layout.js.- Access controls are in place via
Restrict(withRouter(PageTemplateLayout), "page-template")insrc/layouts/page-template-layout.js.All infrastructure (layout, pages, reducers, and actions) is properly implemented.
Likely an incorrect or invalid review comment.
src/layouts/primary-layout.js (1)
32-32: LGTM! New page templates route properly integrated.The import and route for
PageTemplateLayoutfollow the established pattern and correctly enable the new page templates feature at/app/page-templates.Also applies to: 69-69
src/pages/sponsors-global/form-templates/form-template-list-page.js (1)
40-45: LGTM! Import paths correctly updated.The import paths have been consistently updated to reflect the reorganized directory structure, with no changes to functionality.
src/pages/sponsors-global/inventory/inventory-list-page.js (1)
42-45: LGTM! Import paths correctly updated.The import paths have been consistently adjusted for the new directory structure with no functional changes.
src/store.js (1)
161-161: LGTM! Page template reducer properly integrated.The new
pageTemplateListReduceris correctly imported and wired into the Redux store, following established patterns for other list reducers.Also applies to: 326-327
src/pages/sponsors-global/inventory/edit-inventory-item-page.js (1)
18-18: LGTM! Import paths correctly updated.The import paths have been appropriately adjusted to reflect the reorganized directory structure.
Also applies to: 26-26
src/layouts/form-template-item-layout.js (1)
19-20: LGTM! Clean refactoring of import paths.The import paths have been correctly updated to reflect the new directory structure under
sponsors-global/form-templates. This aligns with the PR's objective to reorganize the form template files.src/i18n/en.json (2)
190-191: LGTM! Menu entries added for the new page templates feature.The new menu items "Inventory" and "Pages" are properly placed within the existing menu structure to support the sponsor pages CRUD functionality.
3837-3866: LGTM! Comprehensive translation keys for page templates.The translation structure includes:
- List view labels (code, name, module types)
- UI controls (hide_archived, add buttons)
- CRUD dialog translations (page_crud nested object)
- Informational messages and placeholders
The translations are well-organized and follow the existing patterns in the codebase.
src/pages/sponsors-global/form-templates/form-template-item-list-page.js (1)
30-30: LGTM! Import paths correctly updated for directory restructuring.All import paths have been adjusted from
../../to../../../to account for the new file location depth within thesponsors-global/form-templatesdirectory structure. The changes are consistent and maintain all necessary imports.Also applies to: 42-47
src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
23-122: LGTM! Well-structured reducer implementation.The reducer properly handles:
- State initialization with sensible defaults
- Pagination, sorting, and filtering state
- Response data transformation (lines 72-80) that counts modules by kind for efficient list display
- Archive/unarchive operations with immutable updates
- User logout cleanup
The logic is clear and follows Redux best practices.
src/layouts/page-template-layout.js (1)
23-53: LGTM on layout structure.The component follows the existing layout patterns with proper breadcrumb integration, route guarding, and NoMatch fallback.
src/pages/sponsors-global/page-templates/edit-page-template-page.js (2)
74-76: Verify shared Redux state doesn't cause conflicts.This page template component shares
currentFormTemplateStatewith the form template feature. If both pages can be accessed simultaneously (e.g., via browser tabs or quick navigation), edits on one may inadvertently affect the other's displayed state.
42-48: LGTM on useEffect logic.The effect correctly handles both create (reset) and edit (fetch) flows with appropriate dependencies.
src/pages/sponsors-global/page-templates/page-template-popup.js (1)
40-52: LGTM on Formik form setup.The form correctly uses
enableReinitializefor prop-driven updates, applies validation schema, and delegates submission to the parent viaonSave.src/actions/page-template-actions.js (1)
46-97: LGTM ongetPageTemplatesimplementation.The action correctly handles search term escaping, filter building, pagination, ordering, and archive filtering with proper loading state management.
| deleteFormTemplateMetaFieldTypeValue, | ||
| deleteFormTemplateMaterial | ||
| } = props; | ||
| const formTemplateId = match.params.form_template_id; |
There was a problem hiding this comment.
@santipalenque this param does not match with the one declare at layout
see https://github.com/fntechgit/summit-admin/pull/720/changes#diff-9285a04c58bedb479ca8afc90b000fc9489927de21dd8c27713ab105c0ff9c93R41
|
|
||
| const PageTemplatePopup = ({ pageTemplate, open, onClose, onSave }) => { | ||
| const handleClose = () => { | ||
| onClose(); |
There was a problem hiding this comment.
@santipalenque are we missing here a formik.resetForm() ?
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@santipalenque please review comments
santipalenque
force-pushed
the
feature/sponsor-pages
branch
from
403e726 to
e6ce3e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/sponsors-global/form-templates/form-template-popup.js (2)
202-208: Use a stable key instead of the object reference.Using
key={field}with an object reference can cause React reconciliation issues, especially when fields are reordered or modified. Use the field's unique identifier or index as a fallback.Proposed fix
{formik.values.meta_fields.map((field, fieldIndex) => ( <Grid2 container spacing={2} sx={{ alignItems: "center" }} - key={field} + key={field.id ?? fieldIndex} >
254-258: Add missingkeyprop to MenuItem elements.React requires a
keyprop for elements rendered in a list. This will trigger a console warning.Proposed fix
{METAFIELD_TYPES.map((field_type) => ( - <MenuItem value={field_type}> + <MenuItem key={field_type} value={field_type}> {field_type} </MenuItem> ))}src/pages/sponsors-global/form-templates/sponsor-inventory-popup.js (1)
433-437: Add missingkeyprop to MenuItem elements.React requires a
keyprop for elements rendered in a list.Proposed fix
{METAFIELD_TYPES.map((field_type) => ( - <MenuItem value={field_type}> + <MenuItem key={field_type} value={field_type}> {field_type} </MenuItem> ))}
🤖 Fix all issues with AI agents
In @.env.example:
- Line 21: The SPONSOR_PAGES_SCOPES environment variable value is unquoted;
update the .env.example so the SPONSOR_PAGES_SCOPES entry uses quotes around the
scope string (consistent with other scope variables like the ones on lines with
similar scope entries) to satisfy the linter and maintain formatting
consistency.
In @src/actions/page-template-actions.js:
- Around line 233-236: Reducer handling PAGE_TEMPLATE_UNARCHIVED expects the
payload to be the ID but the action dispatches payload = { pageTemplateId };
update the reducer to extract the ID from the action payload (e.g., const
updatedFormTemplateId = payload.pageTemplateId || payload) so it supports the
current action shape (or alternatively change the action to dispatch
createAction(PAGE_TEMPLATE_UNARCHIVED)(pageTemplateId)); ensure references to
PAGE_TEMPLATE_UNARCHIVED and pageTemplateId are used when locating the reducer
and action.
- Around line 214-225: The archivePageTemplate action is missing loading state
updates; update archivePageTemplate to dispatch startLoading() before making the
async putRequest and ensure stopLoading() is dispatched after completion or on
error (use try/finally or promise .finally), keeping the existing putRequest
call (with createAction(PAGE_TEMPLATE_ARCHIVED) and snackbarErrorHandler) and
returning its result; reference the archivePageTemplate function, dispatch
startLoading() immediately and dispatch stopLoading() in the finally block so
loading state is consistent with unarchivePageTemplate/deletePageTemplate.
In @src/i18n/en.json:
- Around line 3879-3882: Update the i18n entry for page_template_list.alert_info
to correct the typo: replace "botton" with "button" in the string value for the
"alert_info" key so it reads "To edit a Page click on the item's Edit button."
In @src/pages/sponsors-global/page-templates/edit-page-template-page.js:
- Around line 14-26: The component is wrongly wired to FormTemplate
actions/state; replace imports of getFormTemplate, resetFormTemplateForm,
saveFormTemplate, deleteFormTemplateMetaFieldType,
deleteFormTemplateMetaFieldTypeValue, deleteFormTemplateMaterial with the
corresponding PageTemplate actions (e.g., getPageTemplate,
resetPageTemplateForm, savePageTemplate, deletePageTemplateMetaFieldType,
deletePageTemplateMetaFieldTypeValue, deletePageTemplateMaterial) and update
mapStateToProps/mapDispatchToProps to use currentPageTemplateState and the
PageTemplate action creators (the block around the functions mapped at lines
~74-85). Also update the translation key used in the Breadcrumb/heading from
"edit_form_template.form_template" to the page-template specific key (e.g.,
"edit_page_template.page_template") so the UI reflects PageTemplate instead of
FormTemplate.
In @src/pages/sponsors-global/page-templates/page-template-list-page.js:
- Around line 190-195: The checkbox is uncontrolled and doesn't reflect Redux
state; update the Checkbox in the FormControlLabel to be controlled by passing
checked={hideArchived} and ensure handleHideArchived uses the event value
(e.target.checked) to dispatch the Redux update (or accept the new boolean
directly) so the UI always matches the hideArchived state; locate
FormControlLabel/Checkbox and the handleHideArchived function to make these
changes.
- Around line 77-86: The handleSearch handler should reset pagination before
fetching results so a new search starts on page 1; update handleSearch to set
currentPage to 1 (call setCurrentPage(1) or otherwise assign currentPage = 1)
and then call getPageTemplates with page 1 (use the 1 value instead of
currentPage) so the search always fetches from the first page; reference
handleSearch, getPageTemplates and currentPage (and setCurrentPage if present)
when making the change.
In @src/pages/sponsors-global/page-templates/page-template-popup.js:
- Line 89: The Divider components in page-template-popup.js are being passed a
non-existent gutterBottom prop (e.g., the Divider instance around the Divider
gutterBottom line and the other occurrence at the later Divider) which is a
Typography prop and causes a React warning; remove the gutterBottom prop from
those Divider usages (or if spacing under a Typography was intended, move
gutterBottom to the relevant Typography element or apply appropriate spacing via
className/style) so Divider is rendered without the invalid prop.
- Around line 150-152: The mapStateToProps uses currentPageTemplateState but the
reducer isn't registered in the Redux store; add a currentPageTemplateState
reducer entry to the combined reducers in src/store.js following the existing
pattern (e.g., like currentCompanyState and currentFormTemplateState), import
the reducer implementation (e.g., from reducers/currentPageTemplateReducer or
the file that defines currentPageTemplateState) and register it under the key
currentPageTemplateState so components (including page-template-popup.js)
receive the expected state slice.
In @src/reducers/sponsors_inventory/page-template-list-reducer.js:
- Around line 90-97: The reducer handling PAGE_TEMPLATE_DELETED is reading
payload.pageTemplateId but the action dispatches payload.formTemplateId, causing
deletes to be ignored; update the action creator that dispatches
PAGE_TEMPLATE_DELETED to send { pageTemplateId } instead of { formTemplateId }
(or alternatively update the reducer to read payload.formTemplateId), ensuring
the PAGE_TEMPLATE_DELETED handler uses the same property when filtering
state.pageTemplates by id.
🧹 Nitpick comments (8)
src/i18n/en.json (1)
3891-3894: Consider renaming keys to reflect page templates.These keys appear to reference "form_template" but are under
page_template_list. If they're intended for page templates, consider renaming for consistency:
delete_form_template_warning→delete_page_template_warningadd_form_template→add_page_templateIf these are intentionally reusing form template terminology, this can be ignored.
src/store.js (1)
330-331: Consider naming consistency for the state key.The new
pageTemplateListStatekey omits thecurrentprefix, while similar list reducers use the prefix (e.g.,currentFormTemplateListState,currentInventoryItemListState). However, I see other reducers in the store also omit it (e.g.,sponsorFormsListState), so this is acceptable given the mixed conventions already in the codebase.src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
54-56: Missing dependency in useEffect.The
useEffecthook usesgetPageTemplatesbut doesn't include it in the dependency array. While this works since the action is stable from Redux, consider adding it to satisfy ESLint's exhaustive-deps rule, or add a disable comment if intentional.🔧 Suggested fix
useEffect(() => { getPageTemplates(); - }, []); + }, [getPageTemplates]);src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
99-117: Inconsistent payload handling and variable naming.
PAGE_TEMPLATE_ARCHIVEDusespayload.responsewhilePAGE_TEMPLATE_UNARCHIVEDusespayload.pageTemplateIddirectly. This asymmetry could cause confusion.Variable
updatedFormTemplateat Line 100 should beupdatedPageTemplateto match the domain context.🔧 Proposed fix for naming
case PAGE_TEMPLATE_ARCHIVED: { - const updatedFormTemplate = payload.response; + const updatedPageTemplate = payload.response; const updatedPageTemplates = state.pageTemplates.map((item) => - item.id === updatedFormTemplate.id + item.id === updatedPageTemplate.id ? { ...item, is_archived: true } : item ); return { ...state, pageTemplates: updatedPageTemplates }; } case PAGE_TEMPLATE_UNARCHIVED: { - const updatedFormTemplateId = payload; + const { pageTemplateId } = payload; const updatedPageTemplates = state.pageTemplates.map((item) => - item.id === updatedFormTemplateId + item.id === pageTemplateId ? { ...item, is_archived: false } : item ); return { ...state, pageTemplates: updatedPageTemplates }; }src/pages/sponsors-global/page-templates/page-template-popup.js (3)
24-26: Form should reset when closing the dialog.The form state persists after closing, which could show stale data when reopening. Following the pattern from
form-template-popup.js, reset the form before callingonClose.🔧 Proposed fix
const handleClose = () => { + formik.resetForm(); onClose(); };Note: Move
handleClosebelow theformikdeclaration or extract the reset logic.
40-52: Handle undefinedpageTemplatein initial values.If
pageTemplateis undefined (e.g., when creating new), spreading it directly could cause issues. Add a fallback.🔧 Proposed fix
const formik = useFormik({ initialValues: { - ...pageTemplate + code: "", + name: "", + ...pageTemplate },
144-148: Missing PropType forpageTemplate.The component uses
pageTemplatefrom props but it's not declared in PropTypes.🔧 Proposed fix
PageTemplatePopup.propTypes = { + pageTemplate: PropTypes.object, open: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, onSave: PropTypes.func.isRequired }; + +PageTemplatePopup.defaultProps = { + pageTemplate: {} +};src/actions/page-template-actions.js (1)
99-117: Inconsistent parameter naming:formTemplateIdshould bepageTemplateId.These functions use
formTemplateIdas the parameter name, which appears to be carried over from form templates. For clarity and consistency with the rest of the page template domain, rename topageTemplateId.🔧 Proposed fix
-export const getPageTemplate = (formTemplateId) => async (dispatch) => { +export const getPageTemplate = (pageTemplateId) => async (dispatch) => { const accessToken = await getAccessTokenSafely(); // ... return getRequest( null, createAction(RECEIVE_PAGE_TEMPLATE), - `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${formTemplateId}`, + `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}`, authErrorHandler )(params)(dispatch).then(() => { dispatch(stopLoading()); }); }; -export const deletePageTemplate = (formTemplateId) => async (dispatch) => { +export const deletePageTemplate = (pageTemplateId) => async (dispatch) => { // ... return deleteRequest( null, - createAction(PAGE_TEMPLATE_DELETED)({ formTemplateId }), - `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${formTemplateId}`, + createAction(PAGE_TEMPLATE_DELETED)({ pageTemplateId }), + `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}`, null, authErrorHandler )(params)(dispatch).then(() => { dispatch(stopLoading()); }); };Also applies to: 119-137
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.env.examplesrc/actions/page-template-actions.jssrc/app.jssrc/components/menu/index.jssrc/i18n/en.jsonsrc/layouts/form-template-item-layout.jssrc/layouts/form-template-layout.jssrc/layouts/inventory-item-layout.jssrc/layouts/page-template-layout.jssrc/layouts/primary-layout.jssrc/pages/sponsors-global/form-templates/add-form-template-item-popup.jssrc/pages/sponsors-global/form-templates/form-template-from-duplicate-popup.jssrc/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/form-templates/form-template-popup.jssrc/pages/sponsors-global/form-templates/sponsor-inventory-popup.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/edit-page-template-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors-global/page-templates/page-template-popup.jssrc/pages/sponsors-global/shared/meta-field-values.jssrc/pages/sponsors/components/additional-input.jssrc/reducers/sponsors_inventory/page-template-list-reducer.jssrc/store.js
🚧 Files skipped from review as they are similar to previous changes (5)
- src/layouts/form-template-item-layout.js
- src/pages/sponsors-global/form-templates/form-template-item-list-page.js
- src/layouts/primary-layout.js
- src/layouts/page-template-layout.js
- src/app.js
🧰 Additional context used
🧬 Code graph analysis (6)
src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
src/actions/page-template-actions.js (10)
REQUEST_PAGE_TEMPLATES(40-40)REQUEST_PAGE_TEMPLATES(40-40)RECEIVE_PAGE_TEMPLATES(39-39)RECEIVE_PAGE_TEMPLATES(39-39)PAGE_TEMPLATE_DELETED(36-36)PAGE_TEMPLATE_DELETED(36-36)PAGE_TEMPLATE_ARCHIVED(43-43)PAGE_TEMPLATE_ARCHIVED(43-43)PAGE_TEMPLATE_UNARCHIVED(44-44)PAGE_TEMPLATE_UNARCHIVED(44-44)
src/pages/sponsors-global/page-templates/edit-page-template-page.js (2)
src/pages/sponsors-global/form-templates/form-template-list-page.js (1)
mapStateToProps(367-374)src/pages/sponsors-global/page-templates/page-template-popup.js (1)
mapStateToProps(150-152)
src/store.js (2)
src/reducers/sponsor_settings/sponsor-settings-reducer.js (1)
sponsorSettingsReducer(48-106)src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
pageTemplateListReducer(35-122)
src/pages/sponsors-global/page-templates/page-template-list-page.js (3)
src/actions/page-template-actions.js (2)
getPageTemplates(46-97)getPageTemplates(46-97)src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
pageTemplates(72-80)src/components/mui/table/mui-table.js (1)
MuiTable(31-298)
src/pages/sponsors-global/page-templates/page-template-popup.js (3)
src/pages/sponsors-global/form-templates/form-template-popup.js (2)
handleClose(126-129)formik(43-98)src/pages/sponsors-global/form-templates/form-template-list-page.js (1)
mapStateToProps(367-374)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
mapStateToProps(259-261)
src/actions/page-template-actions.js (3)
src/components/audit-logs/index.js (1)
page(37-37)src/pages/sponsors-global/page-templates/edit-page-template-page.js (1)
pageTemplateId(40-40)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
pageTemplateId(52-52)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 21-21: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ 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 (15)
.env.example (1)
20-21: Verify whether SPONSOR_PAGES_SCOPES should be included in the SCOPES variable.The new
SPONSOR_PAGES_SCOPESvariable is defined but not included in the comprehensiveSCOPESvariable on line 19. Other API-specific scopes (SPONSOR_USERS_API_SCOPES,PURCHASES_API_SCOPES,EMAIL_SCOPES,FILE_UPLOAD_SCOPES) are included there. Confirm whether this exclusion is intentional or if the sponsor pages scopes should also be added to the OAuth2 scope list.src/pages/sponsors/components/additional-input.js (1)
17-17: LGTM!The import path update aligns with the PR's reorganization to centralize
MetaFieldValuesin the shared location.src/pages/sponsors-global/form-templates/form-template-popup.js (1)
23-23: LGTM!The import path update aligns with the shared module reorganization.
src/pages/sponsors-global/form-templates/sponsor-inventory-popup.js (1)
31-31: LGTM!The import path update is consistent with the shared module reorganization.
src/components/menu/index.js (1)
64-68: LGTM!The new
page_templatesmenu item follows the existing structure and naming conventions of sibling items undersponsors_inventory.src/i18n/en.json (1)
190-191: LGTM!Menu translations added correctly for the new page templates feature.
src/layouts/form-template-layout.js (1)
19-19: LGTM!The import path update correctly reflects the reorganization of form template pages under the
sponsors-global/form-templatesdirectory.src/layouts/inventory-item-layout.js (1)
18-18: LGTM!The import path update correctly reflects the reorganization of inventory pages under the
sponsors-global/inventorydirectory.src/pages/sponsors-global/inventory/inventory-list-page.js (1)
42-45: LGTM!The import paths are correctly updated to reflect the file's new location under
sponsors-global/inventory. The relative paths (../../../for root-level modules and../form-templates/for the sibling popup) are accurate.src/store.js (1)
161-161: LGTM!The import for the new page template list reducer follows the established pattern for sponsor inventory reducers.
src/pages/sponsors-global/form-templates/form-template-list-page.js (1)
40-45: LGTM!The import paths are correctly updated for the file's new location under
sponsors-global/form-templates. The relative paths properly reference both root-level modules (../../../) and co-located popup components (./).src/pages/sponsors-global/page-templates/page-template-list-page.js (2)
103-118: Incomplete handler implementations.
handleClonePageTemplate,handleEdit, andhandleDeleteonly log to console. If these are intentionally deferred, consider adding TODO comments or disabling the corresponding UI elements until implemented.
38-51: LGTM - Component structure and Redux integration.The component is well-structured with clear separation of concerns. The Redux connection properly maps state and actions, and the table integration with pagination, sorting, and archive functionality follows established patterns.
Also applies to: 259-268
src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
23-33: LGTM - Default state and LOGOUT_USER handling.The default state is well-defined with sensible defaults, and the LOGOUT_USER handler properly resets state.
Also applies to: 35-40
src/actions/page-template-actions.js (1)
46-97: LGTM -getPageTemplatesaction.Well-structured async action with proper filter building, pagination, sorting support, and loading state management.
| export const archivePageTemplate = (pageTemplateId) => async (dispatch) => { | ||
| const accessToken = await getAccessTokenSafely(); | ||
| const params = { access_token: accessToken }; | ||
|
|
||
| return putRequest( | ||
| null, | ||
| createAction(PAGE_TEMPLATE_ARCHIVED), | ||
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, | ||
| null, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch); | ||
| }; |
There was a problem hiding this comment.
Missing loading state management in archivePageTemplate.
Unlike other async actions (unarchivePageTemplate, deletePageTemplate, etc.), this action doesn't dispatch startLoading() / stopLoading(). This creates an inconsistent UX.
🔧 Proposed fix
export const archivePageTemplate = (pageTemplateId) => async (dispatch) => {
const accessToken = await getAccessTokenSafely();
const params = { access_token: accessToken };
+ dispatch(startLoading());
+
return putRequest(
null,
createAction(PAGE_TEMPLATE_ARCHIVED),
`${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`,
null,
snackbarErrorHandler
- )(params)(dispatch);
+ )(params)(dispatch).then(() => {
+ 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.
| export const archivePageTemplate = (pageTemplateId) => async (dispatch) => { | |
| const accessToken = await getAccessTokenSafely(); | |
| const params = { access_token: accessToken }; | |
| return putRequest( | |
| null, | |
| createAction(PAGE_TEMPLATE_ARCHIVED), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, | |
| null, | |
| snackbarErrorHandler | |
| )(params)(dispatch); | |
| }; | |
| export const archivePageTemplate = (pageTemplateId) => async (dispatch) => { | |
| const accessToken = await getAccessTokenSafely(); | |
| const params = { access_token: accessToken }; | |
| dispatch(startLoading()); | |
| return putRequest( | |
| null, | |
| createAction(PAGE_TEMPLATE_ARCHIVED), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, | |
| null, | |
| snackbarErrorHandler | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| }).catch((error) => { | |
| dispatch(stopLoading()); | |
| throw error; | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In @src/actions/page-template-actions.js around lines 214 - 225, The
archivePageTemplate action is missing loading state updates; update
archivePageTemplate to dispatch startLoading() before making the async
putRequest and ensure stopLoading() is dispatched after completion or on error
(use try/finally or promise .finally), keeping the existing putRequest call
(with createAction(PAGE_TEMPLATE_ARCHIVED) and snackbarErrorHandler) and
returning its result; reference the archivePageTemplate function, dispatch
startLoading() immediately and dispatch stopLoading() in the finally block so
loading state is consistent with unarchivePageTemplate/deletePageTemplate.
| return deleteRequest( | ||
| null, | ||
| createAction(PAGE_TEMPLATE_UNARCHIVED)({ pageTemplateId }), | ||
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, |
There was a problem hiding this comment.
Payload structure mismatch with reducer for unarchive.
The action dispatches createAction(PAGE_TEMPLATE_UNARCHIVED)({ pageTemplateId }) which creates payload = { pageTemplateId }. However, the reducer at Line 110 does const updatedFormTemplateId = payload, treating the entire payload as the ID instead of extracting pageTemplateId from it.
This will cause the unarchive to fail silently (no template will match).
Either fix the reducer (recommended - see reducer review) or change the action:
- createAction(PAGE_TEMPLATE_UNARCHIVED)({ pageTemplateId }),
+ createAction(PAGE_TEMPLATE_UNARCHIVED)(pageTemplateId),📝 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 deleteRequest( | |
| null, | |
| createAction(PAGE_TEMPLATE_UNARCHIVED)({ pageTemplateId }), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, | |
| return deleteRequest( | |
| null, | |
| createAction(PAGE_TEMPLATE_UNARCHIVED)(pageTemplateId), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/page-templates/${pageTemplateId}/archive`, |
🤖 Prompt for AI Agents
In @src/actions/page-template-actions.js around lines 233 - 236, Reducer
handling PAGE_TEMPLATE_UNARCHIVED expects the payload to be the ID but the
action dispatches payload = { pageTemplateId }; update the reducer to extract
the ID from the action payload (e.g., const updatedFormTemplateId =
payload.pageTemplateId || payload) so it supports the current action shape (or
alternatively change the action to dispatch
createAction(PAGE_TEMPLATE_UNARCHIVED)(pageTemplateId)); ensure references to
PAGE_TEMPLATE_UNARCHIVED and pageTemplateId are used when locating the reducer
and action.
| "page_template_list": { | ||
| "page_templates": "Page Templates", | ||
| "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit botton.", | ||
| "code": "Code", |
There was a problem hiding this comment.
Fix typo in alert_info.
"botton" should be "button".
Proposed fix
"page_template_list": {
"page_templates": "Page Templates",
- "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit botton.",
+ "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit button.",
"code": "Code",📝 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.
| "page_template_list": { | |
| "page_templates": "Page Templates", | |
| "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit botton.", | |
| "code": "Code", | |
| "page_template_list": { | |
| "page_templates": "Page Templates", | |
| "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit button.", | |
| "code": "Code", |
🤖 Prompt for AI Agents
In @src/i18n/en.json around lines 3879 - 3882, Update the i18n entry for
page_template_list.alert_info to correct the typo: replace "botton" with
"button" in the string value for the "alert_info" key so it reads "To edit a
Page click on the item's Edit button."
| import React, { useEffect } from "react"; | ||
| import { connect } from "react-redux"; | ||
| import { Breadcrumb } from "react-breadcrumbs"; | ||
| import T from "i18n-react/dist/i18n-react"; | ||
| import FormTemplateForm from "../../../components/forms/form-template-form"; | ||
| import { | ||
| getFormTemplate, | ||
| resetFormTemplateForm, | ||
| saveFormTemplate, | ||
| deleteFormTemplateMetaFieldType, | ||
| deleteFormTemplateMetaFieldTypeValue, | ||
| deleteFormTemplateMaterial | ||
| } from "../../../actions/form-template-actions"; |
There was a problem hiding this comment.
Component uses FormTemplate actions/state instead of PageTemplate.
This component is named EditPageTemplatePage but imports and uses FormTemplate actions (getFormTemplate, saveFormTemplate, etc.) and maps currentFormTemplateState. This appears to be a copy-paste error - it should use the corresponding PageTemplate actions and state.
🐛 Proposed fix outline
-import FormTemplateForm from "../../../components/forms/form-template-form";
import {
- getFormTemplate,
- resetFormTemplateForm,
- saveFormTemplate,
- deleteFormTemplateMetaFieldType,
- deleteFormTemplateMetaFieldTypeValue,
- deleteFormTemplateMaterial
-} from "../../../actions/form-template-actions";
+ getPageTemplate,
+ resetPageTemplateForm,
+ savePageTemplate,
+ // ... other page template actions as needed
+} from "../../../actions/page-template-actions";
// Update component to use page template actions and form
// ...
-const mapStateToProps = ({ currentFormTemplateState }) => ({
- ...currentFormTemplateState
+const mapStateToProps = ({ currentPageTemplateState }) => ({
+ ...currentPageTemplateState
});
-export default connect(mapStateToProps, {
- getFormTemplate,
- resetFormTemplateForm,
- saveFormTemplate,
- deleteFormTemplateMetaFieldType,
- deleteFormTemplateMetaFieldTypeValue,
- deleteFormTemplateMaterial
-})(EditPageTemplatePage);
+export default connect(mapStateToProps, {
+ getPageTemplate,
+ resetPageTemplateForm,
+ savePageTemplate,
+ // ... other actions
+})(EditPageTemplatePage);Also update the translation key at Line 59 from edit_form_template.form_template to a page template specific key.
Also applies to: 74-85
🤖 Prompt for AI Agents
In @src/pages/sponsors-global/page-templates/edit-page-template-page.js around
lines 14 - 26, The component is wrongly wired to FormTemplate actions/state;
replace imports of getFormTemplate, resetFormTemplateForm, saveFormTemplate,
deleteFormTemplateMetaFieldType, deleteFormTemplateMetaFieldTypeValue,
deleteFormTemplateMaterial with the corresponding PageTemplate actions (e.g.,
getPageTemplate, resetPageTemplateForm, savePageTemplate,
deletePageTemplateMetaFieldType, deletePageTemplateMetaFieldTypeValue,
deletePageTemplateMaterial) and update mapStateToProps/mapDispatchToProps to use
currentPageTemplateState and the PageTemplate action creators (the block around
the functions mapped at lines ~74-85). Also update the translation key used in
the Breadcrumb/heading from "edit_form_template.form_template" to the
page-template specific key (e.g., "edit_page_template.page_template") so the UI
reflects PageTemplate instead of FormTemplate.
| const handleSearch = (searchTerm) => { | ||
| getPageTemplates( | ||
| searchTerm, | ||
| currentPage, | ||
| perPage, | ||
| order, | ||
| orderDir, | ||
| hideArchived | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Search should reset pagination to page 1.
When initiating a new search, the current page should reset to avoid displaying an empty page if the filtered results have fewer pages than the current position.
🔧 Proposed fix
const handleSearch = (searchTerm) => {
getPageTemplates(
searchTerm,
- currentPage,
+ DEFAULT_CURRENT_PAGE,
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 handleSearch = (searchTerm) => { | |
| getPageTemplates( | |
| searchTerm, | |
| currentPage, | |
| perPage, | |
| order, | |
| orderDir, | |
| hideArchived | |
| ); | |
| }; | |
| const handleSearch = (searchTerm) => { | |
| getPageTemplates( | |
| searchTerm, | |
| DEFAULT_CURRENT_PAGE, | |
| perPage, | |
| order, | |
| orderDir, | |
| hideArchived | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In @src/pages/sponsors-global/page-templates/page-template-list-page.js around
lines 77 - 86, The handleSearch handler should reset pagination before fetching
results so a new search starts on page 1; update handleSearch to set currentPage
to 1 (call setCurrentPage(1) or otherwise assign currentPage = 1) and then call
getPageTemplates with page 1 (use the 1 value instead of currentPage) so the
search always fetches from the first page; reference handleSearch,
getPageTemplates and currentPage (and setCurrentPage if present) when making the
change.
| <FormGroup> | ||
| <FormControlLabel | ||
| control={<Checkbox onChange={handleHideArchived} />} | ||
| label={T.translate("page_template_list.hide_archived")} | ||
| /> | ||
| </FormGroup> |
There was a problem hiding this comment.
Checkbox is uncontrolled and doesn't reflect Redux state.
The checkbox doesn't use the checked prop, so it won't reflect the hideArchived value from Redux state on initial render or after state updates. This causes a mismatch between UI and state.
🔧 Proposed fix
<FormGroup>
<FormControlLabel
- control={<Checkbox onChange={handleHideArchived} />}
+ control={<Checkbox checked={hideArchived} onChange={handleHideArchived} />}
label={T.translate("page_template_list.hide_archived")}
/>
</FormGroup>📝 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.
| <FormGroup> | |
| <FormControlLabel | |
| control={<Checkbox onChange={handleHideArchived} />} | |
| label={T.translate("page_template_list.hide_archived")} | |
| /> | |
| </FormGroup> | |
| <FormGroup> | |
| <FormControlLabel | |
| control={<Checkbox checked={hideArchived} onChange={handleHideArchived} />} | |
| label={T.translate("page_template_list.hide_archived")} | |
| /> | |
| </FormGroup> |
🤖 Prompt for AI Agents
In @src/pages/sponsors-global/page-templates/page-template-list-page.js around
lines 190 - 195, The checkbox is uncontrolled and doesn't reflect Redux state;
update the Checkbox in the FormControlLabel to be controlled by passing
checked={hideArchived} and ensure handleHideArchived uses the event value
(e.target.checked) to dispatch the Redux update (or accept the new boolean
directly) so the UI always matches the hideArchived state; locate
FormControlLabel/Checkbox and the handleHideArchived function to make these
changes.
| /> | ||
| </Grid2> | ||
| </Grid2> | ||
| <Divider gutterBottom /> |
There was a problem hiding this comment.
Invalid gutterBottom prop on Divider.
gutterBottom is a Typography prop, not a Divider prop. This will have no effect and may cause a React warning.
🔧 Proposed fix
-<Divider gutterBottom />
+<Divider sx={{ mb: 2 }} />Also applies to: 122-122
🤖 Prompt for AI Agents
In @src/pages/sponsors-global/page-templates/page-template-popup.js at line 89,
The Divider components in page-template-popup.js are being passed a non-existent
gutterBottom prop (e.g., the Divider instance around the Divider gutterBottom
line and the other occurrence at the later Divider) which is a Typography prop
and causes a React warning; remove the gutterBottom prop from those Divider
usages (or if spacing under a Typography was intended, move gutterBottom to the
relevant Typography element or apply appropriate spacing via className/style) so
Divider is rendered without the invalid prop.
| const mapStateToProps = ({ currentPageTemplateState }) => ({ | ||
| ...currentPageTemplateState | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for currentPageTemplateState references throughout the codebase
rg -n "currentPageTemplateState" --type=jsRepository: fntechgit/summit-admin
Length of output: 292
🏁 Script executed:
# Also search for store configuration and reducer registrations
fd -e "store" -e "reducer" --type f | head -20Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Look for index files that might register reducers
fd -n "index" --type f | grep -E "(store|reducer)" | head -20Repository: fntechgit/summit-admin
Length of output: 296
🏁 Script executed:
# Find store configuration files
find . -type f -name "*store*" | grep -E "\.(js|ts)$" | head -20Repository: fntechgit/summit-admin
Length of output: 80
🏁 Script executed:
# Search for pageTemplateListState to understand existing reducer pattern
rg -n "pageTemplateListState" --type=js -A 3Repository: fntechgit/summit-admin
Length of output: 763
🏁 Script executed:
# Search for reducer registrations or combinReducers
rg -n "combineReducers\|reducer.*=" --type=js -B 2 -A 5 | head -100Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Read store.js to see reducer configuration
cat -n src/store.js | head -350 | tail -50Repository: fntechgit/summit-admin
Length of output: 2740
🏁 Script executed:
# Check if currentPageTemplateState is defined anywhere in store.js
rg -n "currentPageTemplateState\|Reducer" src/store.jsRepository: fntechgit/summit-admin
Length of output: 48
Add missing currentPageTemplateState reducer.
The mapStateToProps in page-template-popup.js references currentPageTemplateState, but it is not registered in the Redux store (src/store.js:295-331). Only pageTemplateListState exists. Add the corresponding currentPageTemplateState reducer to store configuration following the naming convention pattern used for other single-item state (e.g., currentCompanyState, currentFormTemplateState).
🤖 Prompt for AI Agents
In @src/pages/sponsors-global/page-templates/page-template-popup.js around lines
150 - 152, The mapStateToProps uses currentPageTemplateState but the reducer
isn't registered in the Redux store; add a currentPageTemplateState reducer
entry to the combined reducers in src/store.js following the existing pattern
(e.g., like currentCompanyState and currentFormTemplateState), import the
reducer implementation (e.g., from reducers/currentPageTemplateReducer or the
file that defines currentPageTemplateState) and register it under the key
currentPageTemplateState so components (including page-template-popup.js)
receive the expected state slice.
| case PAGE_TEMPLATE_DELETED: { | ||
| const { pageTemplateId } = payload; | ||
| return { | ||
| ...state, | ||
| pageTemplates: state.pageTemplates.filter( | ||
| (a) => a.id !== pageTemplateId | ||
| ) | ||
| }; |
There was a problem hiding this comment.
Payload property name mismatch with action.
The reducer expects payload.pageTemplateId but the action in page-template-actions.js (Line 130) dispatches { formTemplateId }. This will cause the delete to fail silently.
🐛 Proposed fix - align property names
Either update the reducer to match the action:
case PAGE_TEMPLATE_DELETED: {
- const { pageTemplateId } = payload;
+ const { formTemplateId } = payload;
return {
...state,
pageTemplates: state.pageTemplates.filter(
- (a) => a.id !== pageTemplateId
+ (a) => a.id !== formTemplateId
)
};
}Or better, update the action to use consistent naming (pageTemplateId).
🤖 Prompt for AI Agents
In @src/reducers/sponsors_inventory/page-template-list-reducer.js around lines
90 - 97, The reducer handling PAGE_TEMPLATE_DELETED is reading
payload.pageTemplateId but the action dispatches payload.formTemplateId, causing
deletes to be ignored; update the action creator that dispatches
PAGE_TEMPLATE_DELETED to send { pageTemplateId } instead of { formTemplateId }
(or alternatively update the reducer to read payload.formTemplateId), ensuring
the PAGE_TEMPLATE_DELETED handler uses the same property when filtering
state.pageTemplates by id.
.env.example
Outdated
| FILE_UPLOAD_SCOPES="files/upload" | ||
| SCOPES="profile openid offline_access ${SPONSOR_USERS_API_SCOPES} ${PURCHASES_API_SCOPES} ${EMAIL_SCOPES} ${FILE_UPLOAD_SCOPES} ${SCOPES_BASE_REALM}/summits/delete-event ${SCOPES_BASE_REALM}/summits/write ${SCOPES_BASE_REALM}/summits/write-event ${SCOPES_BASE_REALM}/summits/read/all ${SCOPES_BASE_REALM}/summits/read ${SCOPES_BASE_REALM}/summits/publish-event ${SCOPES_BASE_REALM}/members/read ${SCOPES_BASE_REALM}/members/read/me ${SCOPES_BASE_REALM}/speakers/write ${SCOPES_BASE_REALM}/attendees/write ${SCOPES_BASE_REALM}/members/write ${SCOPES_BASE_REALM}/organizations/write ${SCOPES_BASE_REALM}/organizations/read ${SCOPES_BASE_REALM}/summits/write-presentation-materials ${SCOPES_BASE_REALM}/summits/registration-orders/update ${SCOPES_BASE_REALM}/summits/registration-orders/delete ${SCOPES_BASE_REALM}/summits/registration-orders/create/offline ${SCOPES_BASE_REALM}/summits/badge-scans/read entity-updates/publish ${SCOPES_BASE_REALM}/audit-logs/read" | ||
| SPONSOR_PAGES_API_URL=https://sponsor-pages-api.dev.fnopen.com | ||
| SPONSOR_PAGES_SCOPES=page-template/read page-template/write |
.env.example
Outdated
| SPONSOR_USERS_API_SCOPES="show-medata/read show-medata/write access-requests/read access-requests/write sponsor-users/read sponsor-users/write groups/read groups/write" | ||
| EMAIL_SCOPES="clients/read templates/read templates/write emails/read" | ||
| FILE_UPLOAD_SCOPES="files/upload" | ||
| SCOPES="profile openid offline_access ${SPONSOR_USERS_API_SCOPES} ${PURCHASES_API_SCOPES} ${EMAIL_SCOPES} ${FILE_UPLOAD_SCOPES} ${SCOPES_BASE_REALM}/summits/delete-event ${SCOPES_BASE_REALM}/summits/write ${SCOPES_BASE_REALM}/summits/write-event ${SCOPES_BASE_REALM}/summits/read/all ${SCOPES_BASE_REALM}/summits/read ${SCOPES_BASE_REALM}/summits/publish-event ${SCOPES_BASE_REALM}/members/read ${SCOPES_BASE_REALM}/members/read/me ${SCOPES_BASE_REALM}/speakers/write ${SCOPES_BASE_REALM}/attendees/write ${SCOPES_BASE_REALM}/members/write ${SCOPES_BASE_REALM}/organizations/write ${SCOPES_BASE_REALM}/organizations/read ${SCOPES_BASE_REALM}/summits/write-presentation-materials ${SCOPES_BASE_REALM}/summits/registration-orders/update ${SCOPES_BASE_REALM}/summits/registration-orders/delete ${SCOPES_BASE_REALM}/summits/registration-orders/create/offline ${SCOPES_BASE_REALM}/summits/badge-scans/read entity-updates/publish ${SCOPES_BASE_REALM}/audit-logs/read" |
santipalenque
force-pushed
the
feature/sponsor-pages
branch
from
74727ec to
43ee702
Compare
|
LGTM |
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 88-97: When toggling the hide-archived filter in
handleHideArchived, reset pagination to page 1 before fetching: set current page
state to 1 (call setCurrentPage(1) if available) and call getPageTemplates with
page argument 1 (e.g., getPageTemplates(term, 1, perPage, order, orderDir,
ev.target.checked)) so the UI never stays on a stale/empty page after filtering.
In `@src/pages/sponsors-global/page-templates/page-template-popup.js`:
- Around line 144-148: The PropTypes for PageTemplatePopup are missing the
pageTemplate prop and should be added to PageTemplatePopup.propTypes (e.g.,
pageTemplate: PropTypes.object.isRequired or a more specific shape if known) so
the component's expected data is validated; also ensure the popup/dialog
component includes proper accessibility attributes like aria-labelledby and
aria-describedby (or role="dialog") tied to the dialog title/description to
improve screen reader support.
♻️ Duplicate comments (8)
src/actions/page-template-actions.js (2)
214-225: Missing loading state management inarchivePageTemplate.Unlike other async actions, this action doesn't dispatch
startLoading()/stopLoading(), causing inconsistent UX where the loading indicator won't appear during archive operations.
233-236: Payload structure mismatch with reducer.The action dispatches
{ pageTemplateId }as payload, but the reducer atpage-template-list-reducer.jsline 110 doesconst updatedFormTemplateId = payload, treating the entire payload as the ID. This will cause unarchive to silently fail.Either change the action to dispatch the ID directly:
- createAction(PAGE_TEMPLATE_UNARCHIVED)({ pageTemplateId }), + createAction(PAGE_TEMPLATE_UNARCHIVED)(pageTemplateId),Or update the reducer to extract
pageTemplateIdfrom the payload object.src/pages/sponsors-global/page-templates/page-template-list-page.js (2)
77-86: Search should reset pagination to page 1.When initiating a new search, the current page should reset to avoid displaying empty results if filtered data has fewer pages.
🔧 Proposed fix
const handleSearch = (searchTerm) => { getPageTemplates( searchTerm, - currentPage, + DEFAULT_CURRENT_PAGE, perPage, order, orderDir, hideArchived ); };
190-195: Checkbox is uncontrolled and doesn't reflect Redux state.The checkbox doesn't use the
checkedprop, so it won't reflect thehideArchivedvalue from Redux state on initial render or after state updates.🔧 Proposed fix
<FormGroup> <FormControlLabel - control={<Checkbox onChange={handleHideArchived} />} + control={<Checkbox checked={hideArchived} onChange={handleHideArchived} />} label={T.translate("page_template_list.hide_archived")} /> </FormGroup>src/pages/sponsors-global/page-templates/page-template-popup.js (3)
24-26: Reset the form on close (prevents stale/dirty values when reopening).
Right now closing the dialog doesn’t clear edits unlesspageTemplatechanges.One option
const PageTemplatePopup = ({ pageTemplate, open, onClose, onSave }) => { - const handleClose = () => { - onClose(); - }; + const handleClose = () => { + formik.resetForm(); + onClose(); + };Also applies to: 40-52
144-152: Redux wiring: ensurecurrentPageTemplateStateexists in the store and containspageTemplate.
Otherwise this component will mount with missing data (and the form will render with empty initial values).#!/bin/bash set -euo pipefail # Does the state slice exist anywhere? rg -n --type=js --type=jsx 'currentPageTemplateState' -S # Is there a reducer registered under that key in store setup? rg -n --type=js --type=jsx 'combineReducers\(|currentPageTemplateState\s*:' -S src/store.js src/**/store*.js src/**/reducers*.js 2>/dev/null || true # What prop name does the reducer expose? (pageTemplate vs currentPageTemplate vs entity, etc.) rg -n --type=js --type=jsx 'pageTemplate\s*:' -S src/reducers src/pages | head -n 50
89-89: InvalidgutterBottomprop onDivider(no-op + possible warning).Proposed fix
- <Divider gutterBottom /> + <Divider sx={{ mb: 2 }} />Also applies to: 122-122
src/i18n/en.json (1)
3884-3914: Fix typo (“botton” → “button”) + sanity-check key naming/copy.
The typo is still present. Also, keys likedelete_form_template_warninginsidepage_template_listlook copy/paste and may be incorrect for “pages” UX.Proposed fix (typo)
- "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit botton.", + "alert_info": "You can create or archive Pages from the list. To edit a Page click on the item's Edit button.",#!/bin/bash set -euo pipefail # Ensure `page_template_list` is not duplicated in en.json (duplicate JSON keys overwrite silently) rg -n '"page_template_list"\s*:' src/i18n/en.json # Check whether the page templates UI actually uses these "form_template" warning keys rg -n --type=js --type=jsx 'page_template_list\.delete_form_template_warning|delete_form_template_warning' -S
🧹 Nitpick comments (1)
src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
103-118: Incomplete handler implementations.These handlers only log to the console and don't perform any actual operations:
handleClonePageTemplate- logs "CLONE PAGE"handleEdit- logs "EDIT"handleDelete- logs "DELETE"If these are intentional placeholders for future work, consider adding TODO comments or disabling the associated UI elements until implementation is complete.
Would you like me to help implement these handlers or create issues to track this work?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.env.examplesrc/actions/page-template-actions.jssrc/app.jssrc/components/menu/index.jssrc/i18n/en.jsonsrc/layouts/form-template-item-layout.jssrc/layouts/form-template-layout.jssrc/layouts/inventory-item-layout.jssrc/layouts/page-template-layout.jssrc/layouts/primary-layout.jssrc/pages/sponsors-global/form-templates/add-form-template-item-popup.jssrc/pages/sponsors-global/form-templates/form-template-from-duplicate-popup.jssrc/pages/sponsors-global/form-templates/form-template-item-list-page.jssrc/pages/sponsors-global/form-templates/form-template-list-page.jssrc/pages/sponsors-global/form-templates/form-template-popup.jssrc/pages/sponsors-global/form-templates/sponsor-inventory-popup.jssrc/pages/sponsors-global/inventory/inventory-list-page.jssrc/pages/sponsors-global/page-templates/edit-page-template-page.jssrc/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors-global/page-templates/page-template-popup.jssrc/pages/sponsors/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.jssrc/reducers/sponsors_inventory/page-template-list-reducer.jssrc/store.js
🚧 Files skipped from review as they are similar to previous changes (10)
- src/layouts/form-template-item-layout.js
- src/layouts/inventory-item-layout.js
- src/app.js
- src/layouts/page-template-layout.js
- src/pages/sponsors-global/page-templates/edit-page-template-page.js
- src/reducers/sponsors_inventory/page-template-list-reducer.js
- src/pages/sponsors/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js
- src/pages/sponsors-global/form-templates/form-template-list-page.js
- src/components/menu/index.js
- src/layouts/primary-layout.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/store.js (2)
src/reducers/sponsor_settings/sponsor-settings-reducer.js (1)
sponsorSettingsReducer(48-106)src/reducers/sponsors_inventory/page-template-list-reducer.js (1)
pageTemplateListReducer(35-122)
src/pages/sponsors-global/page-templates/page-template-popup.js (3)
src/pages/sponsors-global/form-templates/form-template-popup.js (1)
formik(37-77)src/components/mui/formik-inputs/mui-formik-textfield.js (1)
MuiFormikTextField(6-35)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
mapStateToProps(259-261)
src/actions/page-template-actions.js (3)
src/components/audit-logs/index.js (1)
page(37-37)src/pages/sponsors-global/page-templates/edit-page-template-page.js (1)
pageTemplateId(40-40)src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
pageTemplateId(52-52)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 17-17: [UnorderedKey] The EMAIL_SCOPES key should go before the PURCHASES_API_SCOPES key
(UnorderedKey)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The FILE_UPLOAD_SCOPES key should go before the PURCHASES_API_SCOPES key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The SPONSOR_PAGES_API_URL key should go before the SPONSOR_USERS_API_SCOPES key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The SPONSOR_PAGES_SCOPES key should go before the SPONSOR_USERS_API_SCOPES key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The ALLOWED_USER_GROUPS key should go before the GOOGLE_API_KEY key
(UnorderedKey)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 24-24: [UnorderedKey] The APP_CLIENT_NAME key should go before the GOOGLE_API_KEY key
(UnorderedKey)
⏰ 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 (12)
src/layouts/form-template-layout.js (1)
19-19: ✓ Import path verified and correct.The file exists at the new location
src/pages/sponsors-global/form-templates/form-template-list-page.js. The import path change aligns with the directory restructuring and is properly formatted. No issues found.src/pages/sponsors-global/form-templates/form-template-item-list-page.js (1)
30-47: Import paths correctly updated for new directory structure.All imports resolve properly. The file move into the
form-templatessubdirectory is handled correctly with adjusted relative paths (../../../for shared components and actions) and local popup components imported from the same directory.src/pages/sponsors-global/inventory/inventory-list-page.js (1)
42-45: LGTM!The import paths have been correctly updated to reflect the new directory structure where form templates and shared components are organized under
sponsors-global/form-templates. No functional changes..env.example (1)
19-21: LGTM!The new
SPONSOR_PAGES_API_URLandSPONSOR_PAGES_SCOPESenvironment variables are properly added, andSCOPEScorrectly includes${SPONSOR_PAGES_SCOPES}to authorize the new page template API endpoints.src/actions/page-template-actions.js (2)
46-97: LGTM!The
getPageTemplatesaction properly manages loading state, constructs filter parameters with proper escaping, and handles pagination and sorting correctly.
151-210: LGTM!The
savePageTemplateaction correctly handles both create (POST) and update (PUT) flows with proper loading state management, success notifications, and automatic list refresh.src/store.js (2)
161-161: LGTM!The new
pageTemplateListReducerimport is correctly added.
330-331: LGTM!The
pageTemplateListReduceris properly wired into the Redux store underpageTemplateListState, following the existing naming conventions.src/pages/sponsors-global/page-templates/page-template-list-page.js (2)
250-254: PageTemplatePopup may need entity data for editing.The popup receives
openandonSavebut no entity data. WhenpageTemplateIdis set for editing (not "new"), the popup likely needs the template data to pre-populate the form. Currently, the popup only opens for new templates (setPageTemplateId("new")), buthandleEditexists and doesn't utilize this state.Consider whether
handleEditshould set thepageTemplateIdand pass the entity to the popup:const handleEdit = (row) => { - console.log("EDIT", row); + setPageTemplateId(row.id); };And pass entity data to the popup if needed for edit mode.
259-268: LGTM!The Redux connection is properly configured with
mapStateToPropsspreading thepageTemplateListStateand connecting the necessary action creators.src/pages/sponsors-global/page-templates/page-template-popup.js (1)
5-16: No action needed—the Grid2 import and sizing props are correct for MUI v6.4.3.The code already uses the correct MUI v6 Grid2 API:
- Importing
Grid2from@mui/materialis valid (both direct import and namespace import work).- The
sizeprop (e.g.,size={12},size={4},size={8}) is the correct sizing API in MUI v6 (the oldxs,sm,md, etc. props were replaced bysizein Grid2 v6).No build break will occur; the code will build and function correctly.
Likely an incorrect or invalid review comment.
src/i18n/en.json (1)
190-192: The i18n path structure is correct. Bothinventoryandpage_templatesshould be flat keys undermenu, which matches how all other menu items are structured in the file and how the menu code references them. The menu code definesinventoryandpage_templatesas direct child items under thesponsors_inventorymenu item, and translation calls likeT.translate("menu.inventory")andT.translate("menu.page_templates")will resolve correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const handleHideArchived = (ev) => { | ||
| getPageTemplates( | ||
| term, | ||
| currentPage, | ||
| perPage, | ||
| order, | ||
| orderDir, | ||
| ev.target.checked | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Filter change should reset pagination to page 1.
Similar to search, toggling the "hide archived" filter should reset to page 1 to prevent displaying an empty page when filtered results have fewer pages.
🔧 Proposed fix
const handleHideArchived = (ev) => {
getPageTemplates(
term,
- currentPage,
+ DEFAULT_CURRENT_PAGE,
perPage,
order,
orderDir,
ev.target.checked
);
};📝 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 handleHideArchived = (ev) => { | |
| getPageTemplates( | |
| term, | |
| currentPage, | |
| perPage, | |
| order, | |
| orderDir, | |
| ev.target.checked | |
| ); | |
| }; | |
| const handleHideArchived = (ev) => { | |
| getPageTemplates( | |
| term, | |
| DEFAULT_CURRENT_PAGE, | |
| perPage, | |
| order, | |
| orderDir, | |
| ev.target.checked | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-list-page.js` around
lines 88 - 97, When toggling the hide-archived filter in handleHideArchived,
reset pagination to page 1 before fetching: set current page state to 1 (call
setCurrentPage(1) if available) and call getPageTemplates with page argument 1
(e.g., getPageTemplates(term, 1, perPage, order, orderDir, ev.target.checked))
so the UI never stays on a stale/empty page after filtering.
| const handleAddInfo = () => { | ||
| console.log("ADD INFO"); | ||
| }; | ||
|
|
||
| const handleAddDocument = () => { | ||
| console.log("ADD DOCUMENT"); | ||
| }; | ||
|
|
||
| const handleAddMedia = () => { | ||
| console.log("ADD MEDIA"); | ||
| }; |
There was a problem hiding this comment.
Remove console.log placeholders (or gate behind a dev-only logger).
These button handlers currently do nothing user-visible besides logging.
Minimal cleanup
const handleAddInfo = () => {
- console.log("ADD INFO");
+ // TODO: implement module add flow
};
const handleAddDocument = () => {
- console.log("ADD DOCUMENT");
+ // TODO: implement module add flow
};
const handleAddMedia = () => {
- console.log("ADD MEDIA");
+ // TODO: implement module add flow
};📝 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 handleAddInfo = () => { | |
| console.log("ADD INFO"); | |
| }; | |
| const handleAddDocument = () => { | |
| console.log("ADD DOCUMENT"); | |
| }; | |
| const handleAddMedia = () => { | |
| console.log("ADD MEDIA"); | |
| }; | |
| const handleAddInfo = () => { | |
| // TODO: implement module add flow | |
| }; | |
| const handleAddDocument = () => { | |
| // TODO: implement module add flow | |
| }; | |
| const handleAddMedia = () => { | |
| // TODO: implement module add flow | |
| }; |
| PageTemplatePopup.propTypes = { | ||
| open: PropTypes.bool.isRequired, | ||
| onClose: PropTypes.func.isRequired, | ||
| onSave: PropTypes.func.isRequired | ||
| }; |
There was a problem hiding this comment.
Add missing pageTemplate PropType (and consider dialog accessibility attrs).
Proposed patch
PageTemplatePopup.propTypes = {
+ pageTemplate: PropTypes.object, // or PropTypes.shape({ code: ..., name: ... })
open: PropTypes.bool.isRequired,
onClose: PropTypes.func.isRequired,
onSave: PropTypes.func.isRequired
};🤖 Prompt for AI Agents
In `@src/pages/sponsors-global/page-templates/page-template-popup.js` around lines
144 - 148, The PropTypes for PageTemplatePopup are missing the pageTemplate prop
and should be added to PageTemplatePopup.propTypes (e.g., pageTemplate:
PropTypes.object.isRequired or a more specific shape if known) so the
component's expected data is validated; also ensure the popup/dialog component
includes proper accessibility attributes like aria-labelledby and
aria-describedby (or role="dialog") tied to the dialog title/description to
improve screen reader support.
ref: https://app.clickup.com/t/86b799157
ref: https://app.clickup.com/t/86b7t3uu3
Summary by CodeRabbit
New Features
Configuration
✏️ Tip: You can customize this high-level summary in your review settings.