-
Notifications
You must be signed in to change notification settings - Fork 4
Feat: sponsor pages CRUD #720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
269ffce to
de9802b
Compare
de9802b to
a765852
Compare
📝 WalkthroughWalkthroughAdds a new Page Templates feature: Redux actions and reducer, React pages (list, edit, popup), a layout and route at /app/page-templates, UI wiring (search/pagination/sort/archive), i18n entries, store integration, environment variable exposure, and multiple import-path restructures for sponsors-global modules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as PageTemplateListPage (React)
participant Redux as Redux (Actions/Reducer)
participant API as Sponsor Pages API
participant Toast as UI Toast/Snackbar
User->>UI: open list / search / paginate / toggle archived
UI->>Redux: dispatch getPageTemplates(term,page,perPage,order,dir,hideArchived)
Redux->>Redux: dispatch REQUEST_PAGE_TEMPLATES
Redux->>API: GET /page-templates?filters...&page...
API-->>Redux: 200 {data, meta}
Redux->>Redux: dispatch RECEIVE_PAGE_TEMPLATES (normalize)
Redux-->>UI: update props (pageTemplates, pagination)
UI->>User: render table
User->>UI: click Save / Archive / Delete
UI->>Redux: dispatch savePageTemplate / archivePageTemplate / deletePageTemplate
Redux->>API: POST/PUT/DELETE /page-templates/...
API-->>Redux: 200/204 success
Redux->>Redux: dispatch PAGE_TEMPLATE_ADDED/UPDATED/DELETED/ARCHIVED/UNARCHIVED
Redux->>Toast: show success snackbar
Redux->>Redux: optionally dispatch getPageTemplates (refresh)
Redux-->>UI: update props -> UI updates table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque are we missing here a formik.resetForm() ?
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque please review comments
ref: https://app.clickup.com/t/86b799157
ref: https://app.clickup.com/t/86b7t3uu3
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.