feat: move event category groups to mui#978
Conversation
Signed-off-by: Priscila Moneo <priscila_moneo@hotmail.com.ar>
📝 WalkthroughWalkthroughEvent category groups list now supports search, pagination, and sorting. The action fetches groups with parameterized filters; the reducer tracks query state and pagination metadata; the page component migrated from class to hooks; and the table UI switched from legacy components to MUI Table with dynamic column configuration. ChangesEvent Category Groups List: Pagination, Filtering, Sorting Refactor
Sequence Diagram(s)sequenceDiagram
participant Page as EventCategoryGroupListPage
participant Action as getEventCategoryGroups
participant Reducer as event-category-group-list-reducer
participant UI as MuiTable + SearchInput
Page->>Page: useEffect on mount<br/>(currentSummit exists)
Page->>Action: getEventCategoryGroups(term, page, perPage, order, orderDir)
Action->>Reducer: REQUEST_EVENT_CATEGORY_GROUPS<br/>(payload with query params)
Reducer->>Reducer: store term, order, orderDir,<br/>perPage in state
Action->>Reducer: RECEIVE_EVENT_CATEGORY_GROUPS<br/>(groups array + pagination metadata)
Reducer->>Reducer: map groups with color,<br/>set currentPage, lastPage,<br/>totalEventCategoryGroups
Reducer->>Page: updated state with<br/>eventCategoryGroups + pagination
Page->>UI: render MuiTable with rows,<br/>handlers for sort/paginate
UI->>Page: handleSort(column, direction)<br/>or handlePageChange(page, perPage)
Page->>Action: getEventCategoryGroups(term, newPage, newPerPage, column, direction)
Action->>Reducer: REQUEST / RECEIVE cycle
Reducer->>Page: updated rows and pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/pages/events/event-category-group-list-page.js (2)
42-46: ⚡ Quick winMissing dependencies in
useEffecthook.The
useEffecthook has an empty dependency array but usescurrentSummitandgetEventCategoryGroupsinside. According to React hooks rules, all values from the component scope used inside the effect should be included in the dependency array.While the current code may work (since Redux
connectstabilizes action creators and route changes likely remount the component), this violates React hooks best practices and may cause issues if the component behavior changes.♻️ Add missing dependencies
useEffect(() => { if (currentSummit) { getEventCategoryGroups(); } - }, []); + }, [currentSummit, getEventCategoryGroups]);Note: If
currentSummit.idis the actual dependency (not the entire object), you could use:- }, []); + }, [currentSummit?.id, getEventCategoryGroups]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/events/event-category-group-list-page.js` around lines 42 - 46, The useEffect currently reads currentSummit and calls getEventCategoryGroups but has an empty dependency array; update the effect to include the real dependencies (e.g., include getEventCategoryGroups and the summit identifier such as currentSummit or currentSummit.id) so the effect re-runs correctly when the summit changes, and keep the existing null-check (if (currentSummit) ...) to avoid calling the action with no summit; reference the useEffect, currentSummit (or currentSummit.id) and getEventCategoryGroups symbols when making the change.
58-68: ⚡ Quick winConsider adding error handling to the delete operation.
The
handleDeletefunction has no error handling. IfdeleteEventCategoryGroupfails (network error, server error, etc.), the user receives no feedback, and the list won't refresh. This could leave the UI in an inconsistent state.🛡️ Add error handling
const handleDelete = (groupId) => { - deleteEventCategoryGroup(groupId).then(() => + deleteEventCategoryGroup(groupId) + .then(() => getEventCategoryGroups( term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir ) - ); + ) + .catch((err) => { + console.error("Failed to delete group:", err); + // Optionally show a user-facing error message + }); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/events/event-category-group-list-page.js` around lines 58 - 68, handleDelete currently calls deleteEventCategoryGroup without handling failures; update handleDelete to handle errors (e.g., make it async and use try/catch or attach .catch) so that on success you call getEventCategoryGroups(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir) to refresh the list, and on failure you surface an error to the user (toast/alert) and/or log the error (include the caught error from deleteEventCategoryGroup). Ensure getEventCategoryGroups is only invoked after a successful delete and include references to handleDelete, deleteEventCategoryGroup, getEventCategoryGroups, DEFAULT_CURRENT_PAGE, term, perPage, order, and orderDir.src/actions/event-category-actions.js (1)
421-424: 💤 Low valueMinor inconsistency with codebase pattern for order direction.
This code uses an empty string
""for ascending sort (whenorderDir === 1), whereassrc/actions/attendee-actions.jsuses"+"for the same case. Both are typically valid API syntax, but maintaining consistency across the codebase improves readability.♻️ Align with existing pattern
- const orderDirSign = orderDir === 1 ? "" : "-"; + const orderDirSign = orderDir === 1 ? "+" : "-";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/actions/event-category-actions.js` around lines 421 - 424, The sort direction assignment in the block that sets params.order uses an empty string for ascending (orderDir === 1); change it to use "+" to match the codebase pattern used in attendee-actions.js—update the orderDirSign logic (the variable orderDirSign and the params.order assignment) so ascending maps to "+" and descending to "-" before setting params.order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/actions/event-category-actions.js`:
- Around line 421-424: The sort direction assignment in the block that sets
params.order uses an empty string for ascending (orderDir === 1); change it to
use "+" to match the codebase pattern used in attendee-actions.js—update the
orderDirSign logic (the variable orderDirSign and the params.order assignment)
so ascending maps to "+" and descending to "-" before setting params.order.
In `@src/pages/events/event-category-group-list-page.js`:
- Around line 42-46: The useEffect currently reads currentSummit and calls
getEventCategoryGroups but has an empty dependency array; update the effect to
include the real dependencies (e.g., include getEventCategoryGroups and the
summit identifier such as currentSummit or currentSummit.id) so the effect
re-runs correctly when the summit changes, and keep the existing null-check (if
(currentSummit) ...) to avoid calling the action with no summit; reference the
useEffect, currentSummit (or currentSummit.id) and getEventCategoryGroups
symbols when making the change.
- Around line 58-68: handleDelete currently calls deleteEventCategoryGroup
without handling failures; update handleDelete to handle errors (e.g., make it
async and use try/catch or attach .catch) so that on success you call
getEventCategoryGroups(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir) to
refresh the list, and on failure you surface an error to the user (toast/alert)
and/or log the error (include the caught error from deleteEventCategoryGroup).
Ensure getEventCategoryGroups is only invoked after a successful delete and
include references to handleDelete, deleteEventCategoryGroup,
getEventCategoryGroups, DEFAULT_CURRENT_PAGE, term, perPage, order, and
orderDir.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e540225a-5efe-492f-9179-3b3ea7dda4c5
📒 Files selected for processing (4)
src/actions/event-category-actions.jssrc/i18n/en.jsonsrc/pages/events/event-category-group-list-page.jssrc/reducers/events/event-category-group-list-reducer.js
2 participants
ref: https://app.clickup.com/t/9014802374/86badg7ka
Summary by CodeRabbit
New Features
Improvements
UI/Style Updates