fix: replace hideArchived param with showArchived, adjust actions#788
fix: replace hideArchived param with showArchived, adjust actions#788
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRenames archived-items flag Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Popup as AddSponsorPageTemplatePopup
participant Redux as Redux Action
participant API as Backend API
User->>Popup: open modal / interact (search/sort/page)
Popup->>Redux: dispatch getSponsorPages(term,page,perPage,order,orderDir,showArchived)
Redux->>API: GET /sponsor-pages?term=...&page=...&filter[]=is_archived==${showArchived ? 1 : 0}
API-->>Redux: sponsor pages payload
Redux-->>Popup: receive pages via props
User->>Popup: select pages + add_ons → submit
Popup->>User: call onSubmit({ pages, add_ons })
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/pages/sponsors/sponsor-forms-tab/index.js (1)
124-167:⚠️ Potential issue | 🔴 CriticalFix leftover
hideArchivedreferences (runtime error).
hideArchivedis undefined and will throw when these branches execute.🛠️ Suggested fix
- hideArchived + showArchived ... - hideArchived + showArchivedsrc/actions/sponsor-forms-actions.js (1)
253-301:⚠️ Potential issue | 🟡 MinorAdd
showArchivedto the request payload for consistency with similar actions.
getGlobalTemplatesusesshowArchivedin the API filter but omits it from the payload object passed tocreateAction. Other similar actions likegetSponsorManagedForms,getSponsorForms, andgetFormTemplatesincludeshowArchivedin their payload, making it available in reducer state. While the reducer doesn't currently use it, this inconsistency should be corrected for maintainability and to align with the established pattern.Suggested fix
- { order, orderDir, page, term } + { order, orderDir, page, term, showArchived }src/pages/sponsors-global/form-templates/form-template-item-list-page.js (1)
258-271:⚠️ Potential issue | 🟡 MinorMissing
checkedprop on Checkbox - checkbox state won't reflect current filter.The Checkbox is missing the
checked={showArchived}prop, so it won't visually reflect the current archived filter state from Redux. Other similar pages in this PR (e.g.,form-template-list-page.jsat line 258,inventory-list-page.jsat line 233) correctly bindchecked={showArchived}.Proposed fix
<Checkbox onChange={handleShowArchivedForms} + checked={showArchived} inputProps={{ "aria-label": T.translate( "form_template_item_list.show_archived" ) }} />src/pages/sponsors/sponsor-form-item-list-page/index.js (1)
237-250:⚠️ Potential issue | 🟡 MinorMissing
checkedprop on Checkbox - checkbox state won't reflect current filter.Same issue as in
form-template-item-list-page.js: the Checkbox is missingchecked={showArchived}, so it won't visually reflect the current archived filter state.Proposed fix
<Checkbox onChange={handleShowArchivedForms} + checked={showArchived} inputProps={{ "aria-label": T.translate( "sponsor_form_item_list.show_archived" ) }} />
🤖 Fix all issues with AI agents
In `@src/pages/sponsors-global/page-templates/page-template-list-page.js`:
- Around line 207-212: The Checkbox inside FormControlLabel is missing the
controlled checked prop so its UI won't reflect the current filter; update the
JSX where FormControlLabel/Checkbox are rendered to pass checked={showArchived}
to the Checkbox (keeping the existing onChange={handleShowArchived}) so the
component is controlled and stays in sync with the showArchived state.
In `@src/pages/sponsors/sponsor-forms-list-page/index.js`:
- Around line 186-196: The Checkbox is uncontrolled and can drift from app
state; bind its checked prop to the showArchived state so the UI reflects the
reducer value. Update the Checkbox inside FormControlLabel to pass
checked={showArchived} (while keeping onChange={handleShowArchivedForms} and
aria-label) so the component stays in sync with the showArchived reducer value.
In `@src/pages/sponsors/sponsor-forms-tab/index.js`:
- Around line 286-296: The Checkbox is using the wrong prop for control: replace
the prop named value with checked so the component reflects the showArchived
state; update the JSX where Checkbox is rendered (the element using showArchived
and handleShowArchivedForms) to pass checked={showArchived} instead of
value={showArchived} and keep onChange={handleShowArchivedForms} and existing
inputProps/label as-is so the checkbox becomes a controlled component.
In
`@src/pages/sponsors/sponsor-pages-tab/components/sponsor-page-template-popup.js`:
- Around line 301-304: AddSponsorPageTemplatePopup's PropTypes are incomplete;
update AddSponsorPageTemplatePopup.propTypes to declare onSubmit
(PropTypes.func.isRequired), sponsor (PropTypes.object.isRequired or a more
specific PropTypes.shape describing sponsorships), and summitId
(PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired) so the
component won't crash when it accesses sponsor.sponsorships or calls onSubmit;
optionally add AddSponsorPageTemplatePopup.defaultProps = { sponsor: {} } if you
prefer to allow missing sponsor and avoid runtime map errors.
- Around line 206-221: The Yup schema defines errors for add_ons but the UI
doesn't render them; update the form to show validation feedback for the field
named "add_ons" by reading formik.touched.add_ons and formik.errors.add_ons (via
the formik prop passed into MuiFormikSelectGroup) and render the validation
message beneath the select — either by passing error and helperText props into
MuiFormikSelectGroup or by adding a FormHelperText (with error styling) directly
under the MuiFormikSelectGroup when formik.touched.add_ons &&
formik.errors.add_ons is truthy so users see the validation error on submit.
🧹 Nitpick comments (3)
src/pages/sponsors/sponsor-pages-tab/components/sponsor-page-template-popup.js (3)
229-229: Hardcoded strings should be internationalized.Two UI strings are not using the i18n translation system:
- Line 229:
"items selected"- Line 241:
"sort by"Proposed fix for i18n
- <Grid2 size={4}>{selectedPages.length} items selected</Grid2> + <Grid2 size={4}> + {T.translate("edit_sponsor.pages_tab.items_selected", { count: selectedPages.length })} + </Grid2>- <SwapVertIcon fontSize="large" sx={{ mr: 1 }} /> sort by + <SwapVertIcon fontSize="large" sx={{ mr: 1 }} /> + {T.translate("edit_sponsor.pages_tab.sort_by")}Remember to add corresponding translation keys to your i18n files.
Also applies to: 241-241
78-90: Consider resetting local state when dialog opens.The
searchTermstate is not reset when the dialog reopens, which could leave stale search text from a previous session. Also,selectedPagesshould likely be cleared when the dialog opens to prevent stale selections.Proposed fix to reset state on open
useEffect(() => { if (open) { + setSearchTerm(""); + setSelectedPages([]); getSponsorPages( term, currentPage, FIVE_PER_PAGE, order, orderDir, false, sponsorshipTypeIds ); } }, [open]);
269-282: Empty state feedback when no sponsor pages exist.The table only renders when
sponsorPages.length > 0, but there's no feedback message when no pages are available. Users won't know if the list is empty vs. still loading.Proposed fix to add empty state
{sponsorPages.length > 0 && ( <Box sx={{ p: 2 }}> <MuiTable columns={columns} data={sponsorPages} options={tableOptions} currentPage={currentPage} perPage={perPage} totalRows={totalCount} onSort={handleSort} onPageChange={handlePageChange} /> </Box> )} + {sponsorPages.length === 0 && ( + <Box sx={{ p: 2, textAlign: "center" }}> + <Typography color="text.secondary"> + {T.translate("edit_sponsor.pages_tab.no_pages_found")} + </Typography> + </Box> + )}
| <DialogContent sx={{ p: 0 }}> | ||
| <Grid2 container spacing={2} size={12} sx={{ p: 2 }}> | ||
| <MuiFormikSelectGroup | ||
| name="add_ons" | ||
| formik={formik} | ||
| queryFunction={querySponsorAddons} | ||
| // params for function, except input | ||
| queryParams={[summitId, sponsor.id, sponsorshipIds]} | ||
| showSelectAll | ||
| getGroupId={(addon) => addon.sponsorship.type.id} | ||
| getGroupLabel={(addon) => addon.sponsorship.type.type.name} | ||
| placeholder={T.translate( | ||
| "edit_sponsor.placeholders.select_add_ons" | ||
| )} | ||
| /> | ||
| </Grid2> |
There was a problem hiding this comment.
No validation error display for add_ons field.
The Yup validation schema defines an error message for add_ons, but there's no UI element to display this error. If a user selects pages but no add-ons, the form submission will fail silently.
Proposed fix to show validation error
<Grid2 container spacing={2} size={12} sx={{ p: 2 }}>
<MuiFormikSelectGroup
name="add_ons"
formik={formik}
queryFunction={querySponsorAddons}
// params for function, except input
queryParams={[summitId, sponsor.id, sponsorshipIds]}
showSelectAll
getGroupId={(addon) => addon.sponsorship.type.id}
getGroupLabel={(addon) => addon.sponsorship.type.type.name}
placeholder={T.translate(
"edit_sponsor.placeholders.select_add_ons"
)}
+ error={formik.touched.add_ons && Boolean(formik.errors.add_ons)}
+ helperText={formik.touched.add_ons && formik.errors.add_ons}
/>
</Grid2>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <DialogContent sx={{ p: 0 }}> | |
| <Grid2 container spacing={2} size={12} sx={{ p: 2 }}> | |
| <MuiFormikSelectGroup | |
| name="add_ons" | |
| formik={formik} | |
| queryFunction={querySponsorAddons} | |
| // params for function, except input | |
| queryParams={[summitId, sponsor.id, sponsorshipIds]} | |
| showSelectAll | |
| getGroupId={(addon) => addon.sponsorship.type.id} | |
| getGroupLabel={(addon) => addon.sponsorship.type.type.name} | |
| placeholder={T.translate( | |
| "edit_sponsor.placeholders.select_add_ons" | |
| )} | |
| /> | |
| </Grid2> | |
| <DialogContent sx={{ p: 0 }}> | |
| <Grid2 container spacing={2} size={12} sx={{ p: 2 }}> | |
| <MuiFormikSelectGroup | |
| name="add_ons" | |
| formik={formik} | |
| queryFunction={querySponsorAddons} | |
| // params for function, except input | |
| queryParams={[summitId, sponsor.id, sponsorshipIds]} | |
| showSelectAll | |
| getGroupId={(addon) => addon.sponsorship.type.id} | |
| getGroupLabel={(addon) => addon.sponsorship.type.type.name} | |
| placeholder={T.translate( | |
| "edit_sponsor.placeholders.select_add_ons" | |
| )} | |
| error={formik.touched.add_ons && Boolean(formik.errors.add_ons)} | |
| helperText={formik.touched.add_ons && formik.errors.add_ons} | |
| /> | |
| </Grid2> |
🤖 Prompt for AI Agents
In
`@src/pages/sponsors/sponsor-pages-tab/components/sponsor-page-template-popup.js`
around lines 206 - 221, The Yup schema defines errors for add_ons but the UI
doesn't render them; update the form to show validation feedback for the field
named "add_ons" by reading formik.touched.add_ons and formik.errors.add_ons (via
the formik prop passed into MuiFormikSelectGroup) and render the validation
message beneath the select — either by passing error and helperText props into
MuiFormikSelectGroup or by adding a FormHelperText (with error styling) directly
under the MuiFormikSelectGroup when formik.touched.add_ons &&
formik.errors.add_ons is truthy so users see the validation error on submit.
| AddSponsorPageTemplatePopup.propTypes = { | ||
| open: PropTypes.bool.isRequired, | ||
| onClose: PropTypes.func.isRequired | ||
| }; |
There was a problem hiding this comment.
Incomplete PropTypes definition.
The component uses onSubmit, sponsor, and summitId without default values, but these are not declared in PropTypes. This will cause runtime errors if the parent component doesn't provide them (e.g., sponsor.sponsorships.map on lines 50/52 will throw if sponsor is undefined).
Proposed fix to complete PropTypes
AddSponsorPageTemplatePopup.propTypes = {
open: PropTypes.bool.isRequired,
- onClose: PropTypes.func.isRequired
+ onClose: PropTypes.func.isRequired,
+ onSubmit: PropTypes.func.isRequired,
+ sponsor: PropTypes.shape({
+ id: PropTypes.number.isRequired,
+ sponsorships: PropTypes.arrayOf(
+ PropTypes.shape({
+ id: PropTypes.number.isRequired,
+ type: PropTypes.shape({
+ id: PropTypes.number.isRequired
+ }).isRequired
+ })
+ ).isRequired
+ }).isRequired,
+ summitId: PropTypes.number.isRequired
};📝 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.
| AddSponsorPageTemplatePopup.propTypes = { | |
| open: PropTypes.bool.isRequired, | |
| onClose: PropTypes.func.isRequired | |
| }; | |
| AddSponsorPageTemplatePopup.propTypes = { | |
| open: PropTypes.bool.isRequired, | |
| onClose: PropTypes.func.isRequired, | |
| onSubmit: PropTypes.func.isRequired, | |
| sponsor: PropTypes.shape({ | |
| id: PropTypes.number.isRequired, | |
| sponsorships: PropTypes.arrayOf( | |
| PropTypes.shape({ | |
| id: PropTypes.number.isRequired, | |
| type: PropTypes.shape({ | |
| id: PropTypes.number.isRequired | |
| }).isRequired | |
| }) | |
| ).isRequired | |
| }).isRequired, | |
| summitId: PropTypes.number.isRequired | |
| }; |
🤖 Prompt for AI Agents
In
`@src/pages/sponsors/sponsor-pages-tab/components/sponsor-page-template-popup.js`
around lines 301 - 304, AddSponsorPageTemplatePopup's PropTypes are incomplete;
update AddSponsorPageTemplatePopup.propTypes to declare onSubmit
(PropTypes.func.isRequired), sponsor (PropTypes.object.isRequired or a more
specific PropTypes.shape describing sponsorships), and summitId
(PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired) so the
component won't crash when it accesses sponsor.sponsorships or calls onSubmit;
optionally add AddSponsorPageTemplatePopup.defaultProps = { sponsor: {} } if you
prefer to allow missing sponsor and avoid runtime map errors.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/sponsors/sponsor-forms-tab/index.js (1)
100-103:⚠️ Potential issue | 🟡 MinorSearch resets archive filter state.
When searching,
showArchivedis not passed, causing the filter to reset to default (false). If a user has "show archived" enabled and then searches, they'll unexpectedly stop seeing archived items.🔧 Proposed fix to preserve archive filter during search
const handleSearch = (searchTerm) => { - getSponsorManagedForms(searchTerm); - getSponsorCustomizedForms(searchTerm); + getSponsorManagedForms( + searchTerm, + DEFAULT_CURRENT_PAGE, + managedForms.perPage, + managedForms.order, + managedForms.orderDir, + showArchived + ); + getSponsorCustomizedForms( + searchTerm, + DEFAULT_CURRENT_PAGE, + customizedForms.perPage, + customizedForms.order, + customizedForms.orderDir, + showArchived + ); };
🧹 Nitpick comments (1)
src/pages/sponsors/sponsor-forms-tab/index.js (1)
105-107: Remove debug console.log.This appears to be a leftover debug statement.
🧹 Proposed cleanup
const handleCustomizeForm = (item) => { - console.log("CUSTOMIZE : ", item); + // TODO: Implement customization logic };
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/sponsors/sponsor-forms-tab/index.js (1)
100-103:⚠️ Potential issue | 🟡 MinorSearch resets the archived filter unexpectedly.
handleSearchdoesn't passshowArchived, so searching while "Show Archived" is checked will reset the filter tofalseand hide archived items. This is inconsistent with other handlers that preserve the filter state.🐛 Proposed fix
const handleSearch = (searchTerm) => { - getSponsorManagedForms(searchTerm); - getSponsorCustomizedForms(searchTerm); + getSponsorManagedForms( + searchTerm, + DEFAULT_CURRENT_PAGE, + managedForms.perPage, + managedForms.order, + managedForms.orderDir, + showArchived + ); + getSponsorCustomizedForms( + searchTerm, + DEFAULT_CURRENT_PAGE, + customizedForms.perPage, + customizedForms.order, + customizedForms.orderDir, + showArchived + ); };
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/sponsors-global/page-templates/page-template-list-page.js (1)
92-101:⚠️ Potential issue | 🟡 MinorReset pagination when toggling the archived filter.
KeepingcurrentPagecan land users on an empty page if the filtered result set shrinks.🔧 Suggested tweak
const handleShowArchived = (ev) => { getPageTemplates( term, - currentPage, + DEFAULT_CURRENT_PAGE, perPage, order, orderDir, ev.target.checked ); };
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
feature/show_archive_items_instead_of_hide
branch
from
d51a2ae to
a30dd13
Compare
1 participant
ref: https://app.clickup.com/t/86b7v230m
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Refactor