fix: migrate tier list to mui components#890
Open
tomrndom wants to merge 7 commits into
Open
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:
📝 WalkthroughWalkthroughActions now use snackbar handlers and consistently stop loading in finally; getSponsorships accepts a search term with escaped filter[] and debounced calls; list page migrated to hooks with MUI MuiTable and in-page Formik SponsorshipDialog; i18n strings switched to “Tier”; new unit tests for list page and dialog added. ChangesSponsorship/Tier updates
Sequence Diagram(s)sequenceDiagram
participant User
participant List as SponsorshipListPage
participant Redux as sponsorship-actions
participant API as Sponsorships API
participant Dialog as SponsorshipDialog
User->>List: open page / enter search / click Add/Edit
List->>Redux: dispatch getSponsorships(term,page,perPage,order,orderDir)
Redux->>API: GET /sponsorships?filter[]...
API-->>Redux: sponsorships list
Redux-->>List: update sponsorships state / stopLoading
List->>Dialog: open (Add) or dispatch getSponsorship(id) (Edit)
Redux->>API: GET /sponsorships/{id}
API-->>Redux: sponsorship entity
Redux-->>List: currentSponsorship updated
User->>Dialog: submit form
Dialog->>List: onSave(values)
List->>Redux: dispatch saveSponsorship(values)
Redux->>API: POST/PUT /sponsorships
API-->>Redux: saved sponsorship
Redux-->>List: dispatch snackbarSuccess / stopLoading
List->>Dialog: close
List->>Redux: dispatch getSponsorships(currentTerm,...)
Redux->>API: GET /sponsorships?...
API-->>Redux: refreshed list
Redux-->>List: updated sponsorships
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/en.json`:
- Line 2895: The i18n key "remove_warning" currently reads "Are you sure you
want to delete sponsorship " and must be updated to use the new terminology;
locate the "remove_warning" entry in src/i18n/en.json and replace the value with
the corrected delete prompt (for example: "Are you sure you want to delete this
Tier?") so the confirmation dialog on the Tier page uses "Tier" instead of
"sponsorship".
In `@src/pages/sponsorship-types/components/sponsorship-dialog.js`:
- Around line 59-72: The Dialog component has had MUI focus-management disabled
and the close IconButton lacks an accessible name; restore accessibility by
removing disableEnforceFocus, disableAutoFocus, and disableRestoreFocus from the
Dialog (unless you have a documented, justified reason) so focus is
trapped/returned properly, and add an aria-label (or localized label) to the
IconButton that renders the CloseIcon (the close control tied to handleClose) so
screen readers can announce it; after changes, verify keyboard-only navigation
enters the Dialog on open and returns focus to the trigger on close.
In `@src/pages/sponsorship-types/sponsorship-list-page.js`:
- Around line 45-47: The current implementation uses searchTerm (const
[searchTerm, setSearchTerm]) as both the live input value and the applied
filter, causing the fetch via getSponsorships to use an unsubmitted draft;
change this by introducing a separate state (e.g., draftSearch and
setDraftSearch) bound to the input onChange, and an applied state (e.g.,
appliedSearch or searchFilter with setSearchFilter) that you update only on
submit/Enter; then pass appliedSearch/searchFilter to getSponsorships and keep
searchTerm (or rename) for the applied value used by paging/sorting to prevent
unexpected dataset changes when typing.
- Around line 196-210: The UI hides the table when sponsorships.length === 0,
making the empty-state copy sponsorship_list.no_sponsorships unreachable; update
the render so MuiTable is always rendered (pass an empty array when there are no
rows) or explicitly render the empty-state message when sponsorships.length ===
0. Locate the block that conditionally renders <MuiTable> and change it to
either always render <MuiTable columns={columns} data={sponsorships} ... /> so
the component can show its built-in empty state, or replace the false branch
with a simple element that displays the sponsorship_list.no_sponsorships message
(using current translation utilities) when sponsorships.length === 0.
- Around line 97-108: The handleSave function currently closes the dialog in the
finally block so setOpen(false) runs even when saveSponsorship rejects; change
the flow so setOpen(false) is called only after a successful save: call
saveSponsorship(entity).then(() => { await/return getSponsorships(...);
setOpen(false); }).catch(...) to leave the dialog open on errors (and optionally
handle/log the rejection) — adjust the promise chain around saveSponsorship,
getSponsorships, and setOpen to move setOpen(false) into the success (.then)
path and remove it from finally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec3c6f78-30a0-4f69-958c-ba13cc3c508a
📒 Files selected for processing (7)
src/actions/sponsorship-actions.jssrc/i18n/en.jsonsrc/pages/sponsorship-types/__tests__/sponsorship-list-page.test.jssrc/pages/sponsorship-types/components/__tests__/sponsorship-dialog.test.jssrc/pages/sponsorship-types/components/sponsorship-dialog.jssrc/pages/sponsorship-types/sponsorship-list-page.jssrc/reducers/sponsorships/sponsorship-list-reducer.js
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/pages/sponsorship-types/sponsorship-list-page.js (1)
45-67:⚠️ Potential issue | 🟠 MajorKeep a separate applied search filter.
searchTermis still acting as both the live input state and the applied backend filter. Typing a draft term and then paging, sorting, saving, or deleting will refresh the table with an unsubmitted filter and unexpectedly change the dataset.Also applies to: 70-79, 97-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 45 - 67, searchTerm is being used as both the live input value and the applied backend filter, causing unsubmitted draft input to affect calls to getSponsorships; introduce a separate state (e.g., appliedSearch or appliedFilter) and keep searchTerm as the controlled input, update appliedSearch only when the user submits/commits the search, then change all calls to getSponsorships (including in useEffect, handlePageChange, handlePerPageChange, handleSort, and any save/delete flows referenced) to pass appliedSearch instead of searchTerm so paging/sorting/deletes use the last applied filter rather than the live input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/sponsorship-types/sponsorship-list-page.js`:
- Around line 168-176: The TextField for search (component TextField using
value={searchTerm}, onChange={setSearchTerm}, onKeyDown={handleSearch}) lacks an
accessible name; add a visible label prop or an aria-label/aria-labelledby to
the TextField so assistive tech can identify it (e.g., provide label="Search
sponsorships" or aria-label="Search sponsorships"), ensuring the label text is
localized via T.translate if needed and matches the placeholder intent.
- Around line 155-156: Replace the hardcoded "tiers" suffix with a translation
call so the count label uses the app's i18n and pluralization; locate the JSX
near Grid2 / Box where totalSponsorships is rendered and change the content to
use T.translate (or the project's pluralization helper) instead of the fixed
string, e.g. call T.translate with a key like "sponsorships.tiers" and pass the
count (totalSponsorships) so the label localizes and pluralizes correctly.
- Around line 17-20: The file imports MUI v6 packages but the project uses React
16.13.1; either upgrade React to ^17+ in package.json and reinstall, or revert
these components to Material-UI v4 equivalents: replace imports of Box, Button,
Grid2, TextField from "@mui/material" with the v4 packages (e.g., import { Box,
Button, Grid, TextField } from "@material-ui/core" and replace Grid2 usage with
Grid) and replace AddIcon/SearchIcon imports from "@mui/icons-material" with
"@material-ui/icons"; update any prop/API differences (e.g., Grid2 -> Grid) and
keep the existing MuiTable import (MuiTable) intact so runtime compatibility is
restored.
---
Duplicate comments:
In `@src/pages/sponsorship-types/sponsorship-list-page.js`:
- Around line 45-67: searchTerm is being used as both the live input value and
the applied backend filter, causing unsubmitted draft input to affect calls to
getSponsorships; introduce a separate state (e.g., appliedSearch or
appliedFilter) and keep searchTerm as the controlled input, update appliedSearch
only when the user submits/commits the search, then change all calls to
getSponsorships (including in useEffect, handlePageChange, handlePerPageChange,
handleSort, and any save/delete flows referenced) to pass appliedSearch instead
of searchTerm so paging/sorting/deletes use the last applied filter rather than
the live input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37ba9be6-1947-43d8-adf3-7f07f36d3629
📒 Files selected for processing (2)
src/pages/sponsorship-types/components/sponsorship-dialog.jssrc/pages/sponsorship-types/sponsorship-list-page.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/sponsorship-types/components/sponsorship-dialog.js
Comment on lines
+17
to
+20
| import { Box, Button, Grid2, TextField } from "@mui/material"; | ||
| import AddIcon from "@mui/icons-material/Add"; | ||
| import SearchIcon from "@mui/icons-material/Search"; | ||
| import MuiTable from "openstack-uicore-foundation/lib/components/mui/table"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for package.json in root
cd /
find . -maxdepth 2 -name "package.json" -type f 2>/dev/null | head -5Repository: fntechgit/summit-admin
Length of output: 1891
🏁 Script executed:
# Look for lock files to determine installed versions
git ls-files | grep -E "(package\.json|yarn\.lock|package-lock\.json)" | head -10Repository: fntechgit/summit-admin
Length of output: 88
🏁 Script executed:
# Read the package.json to check React and MUI versions
cat package.json | head -100Repository: fntechgit/summit-admin
Length of output: 3536
🏁 Script executed:
# Search specifically for React and MUI versions in package.json
rg '"react":|"@mui/material":|"@mui/icons-material":' package.json -A 1Repository: fntechgit/summit-admin
Length of output: 242
MUI 6 requires React 17+ but the project uses React 16.13.1.
This page imports from @mui/material and @mui/icons-material (currently ^6.4.3 in package.json), which officially support only React ^17 || ^18 || ^19. The repo remains on React ^16.13.1, creating an unsupported dependency combination that will break at runtime.
Either upgrade React to 17+, or revert to a MUI version compatible with React 16.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 17 - 20,
The file imports MUI v6 packages but the project uses React 16.13.1; either
upgrade React to ^17+ in package.json and reinstall, or revert these components
to Material-UI v4 equivalents: replace imports of Box, Button, Grid2, TextField
from "@mui/material" with the v4 packages (e.g., import { Box, Button, Grid,
TextField } from "@material-ui/core" and replace Grid2 usage with Grid) and
replace AddIcon/SearchIcon imports from "@mui/icons-material" with
"@material-ui/icons"; update any prop/API differences (e.g., Grid2 -> Grid) and
keep the existing MuiTable import (MuiTable) intact so runtime compatibility is
restored.
Comment on lines
+155
to
+156
| <Grid2 size={2}> | ||
| <Box component="span">{totalSponsorships} tiers</Box> |
There was a problem hiding this comment.
Avoid hardcoded tiers in the count badge.
Everything else on this screen goes through T.translate, but this suffix is fixed English, so it will not localize or pluralize correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 155 - 156,
Replace the hardcoded "tiers" suffix with a translation call so the count label
uses the app's i18n and pluralization; locate the JSX near Grid2 / Box where
totalSponsorships is rendered and change the content to use T.translate (or the
project's pluralization helper) instead of the fixed string, e.g. call
T.translate with a key like "sponsorships.tiers" and pass the count
(totalSponsorships) so the label localizes and pluralizes correctly.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/pages/sponsorship-types/sponsorship-list-page.js (3)
171-179:⚠️ Potential issue | 🟠 MajorAdd an accessible name to the search input.
Line 171 defines a
TextFieldwith only a placeholder; please add a localizedlabeloraria-labelso assistive tech can identify the field reliably.♿ Minimal patch
<TextField variant="outlined" + aria-label={T.translate("general.search")} value={searchTerm} placeholder={T.translate("sponsorship_list.placeholders.search")}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 171 - 179, The search TextField currently only has a placeholder which is not sufficient for assistive tech; add an accessible name by supplying either a localized label prop or an aria-label prop (e.g., use T.translate("sponsorship_list.labels.search") or similar) on the TextField component that uses the same localization namespace as the placeholder so screen readers can identify the input (target the TextField where searchTerm, setSearchTerm, and handleSearch are used).
17-20:⚠️ Potential issue | 🔴 CriticalMUI v6 + React 16 is an unsupported dependency combination.
Line 17 imports MUI v6 components while the repo is on React 16.13.1. This can break rendering/runtime behavior. Please either upgrade React to a supported version or downgrade MUI packages to a React-16-compatible line.
#!/bin/bash set -euo pipefail echo "React version:" rg -n '"react"\s*:' package.json echo "MUI versions:" rg -n '"@mui/material"\s*:|"@mui/icons-material"\s*:' package.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 17 - 20, The file sponsorship-list-page.js currently imports MUI v6 symbols (Box, Button, Grid2, TextField, AddIcon, SearchIcon) which is incompatible with the repo's React 16.13.1; fix by either upgrading React in package.json to a version supported by MUI v6 or replacing/downgrading the MUI packages to versions compatible with React 16 (and update the imports accordingly), then run the supplied verification script to confirm "react" and "@mui/material"/"@mui/icons-material" versions match supported compatibility; ensure the import lines in sponsorship-list-page.js reflect the chosen package versions.
45-80:⚠️ Potential issue | 🟠 MajorDecouple draft input from the applied search filter.
Line 53 and Line 67 use
searchTermfor paging/sorting even though the fetch is only intended on Enter (Line 72). This means unsubmitted typing can still alter later requests. Also, Line 74 should not rely on state during the Enter event path.💡 Suggested fix
- const [searchTerm, setSearchTerm] = useState(""); + const [draftSearchTerm, setDraftSearchTerm] = useState(""); + const [appliedSearchTerm, setAppliedSearchTerm] = useState(""); const handlePageChange = (page) => { - getSponsorships(searchTerm, page, perPage, order, orderDir); + getSponsorships(appliedSearchTerm, page, perPage, order, orderDir); }; const handlePerPageChange = (newPerPage) => { getSponsorships( - searchTerm, + appliedSearchTerm, DEFAULT_CURRENT_PAGE, newPerPage, order, orderDir ); }; const handleSort = (key, dir) => { - getSponsorships(searchTerm, currentPage, perPage, key, dir); + getSponsorships(appliedSearchTerm, currentPage, perPage, key, dir); }; const handleSearch = (ev) => { - setSearchTerm(ev.target.value); if (ev.key === "Enter") { + const term = ev.target.value; + setAppliedSearchTerm(term); getSponsorships( - searchTerm, + term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir ); } }; ... - value={searchTerm} + value={draftSearchTerm} ... - onChange={(e) => setSearchTerm(e.target.value)} + onChange={(e) => setDraftSearchTerm(e.target.value)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsorship-types/sponsorship-list-page.js` around lines 45 - 80, The current handlers (handlePageChange, handlePerPageChange, handleSort, and the Enter path in handleSearch) wrongly use the live draft state searchTerm, causing unsubmitted typing to affect requests; introduce a separate draft state (e.g., searchInput) and an applied state (e.g., appliedSearch) or at minimum use the event value when Enter is pressed: update handleSearch to setSearchInput(ev.target.value) for normal typing and on Enter either setAppliedSearch(ev.target.value) then call getSponsorships(ev.target.value, DEFAULT_CURRENT_PAGE, perPage, order, orderDir) (or call getSponsorships using appliedSearch after setting it), and change handlePageChange/handlePerPageChange/handleSort to call getSponsorships using appliedSearch (not searchTerm) so paging/sorting use the last applied search filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/pages/sponsorship-types/sponsorship-list-page.js`:
- Around line 171-179: The search TextField currently only has a placeholder
which is not sufficient for assistive tech; add an accessible name by supplying
either a localized label prop or an aria-label prop (e.g., use
T.translate("sponsorship_list.labels.search") or similar) on the TextField
component that uses the same localization namespace as the placeholder so screen
readers can identify the input (target the TextField where searchTerm,
setSearchTerm, and handleSearch are used).
- Around line 17-20: The file sponsorship-list-page.js currently imports MUI v6
symbols (Box, Button, Grid2, TextField, AddIcon, SearchIcon) which is
incompatible with the repo's React 16.13.1; fix by either upgrading React in
package.json to a version supported by MUI v6 or replacing/downgrading the MUI
packages to versions compatible with React 16 (and update the imports
accordingly), then run the supplied verification script to confirm "react" and
"@mui/material"/"@mui/icons-material" versions match supported compatibility;
ensure the import lines in sponsorship-list-page.js reflect the chosen package
versions.
- Around line 45-80: The current handlers (handlePageChange,
handlePerPageChange, handleSort, and the Enter path in handleSearch) wrongly use
the live draft state searchTerm, causing unsubmitted typing to affect requests;
introduce a separate draft state (e.g., searchInput) and an applied state (e.g.,
appliedSearch) or at minimum use the event value when Enter is pressed: update
handleSearch to setSearchInput(ev.target.value) for normal typing and on Enter
either setAppliedSearch(ev.target.value) then call
getSponsorships(ev.target.value, DEFAULT_CURRENT_PAGE, perPage, order, orderDir)
(or call getSponsorships using appliedSearch after setting it), and change
handlePageChange/handlePerPageChange/handleSort to call getSponsorships using
appliedSearch (not searchTerm) so paging/sorting use the last applied search
filter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b0042a0-1666-4859-a14e-96da5e9ff17b
📒 Files selected for processing (2)
src/i18n/en.jsonsrc/pages/sponsorship-types/sponsorship-list-page.js
smarcet
reviewed
May 19, 2026
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
Deep risk-focused review — 6 findings (2 HIGH, 3 MEDIUM, 1 LOW). See inline comments for details.
| name: initialEntity?.name ?? "", | ||
| label: initialEntity?.label ?? "", | ||
| size: initialEntity?.size ?? "", | ||
| order: initialEntity?.order ?? 0 |
There was a problem hiding this comment.
[LOW] order in initialValues with no rendered input — silently sent in the API body
order is included in initialValues (defaulting to 0 for new entities) but there is no <input> for it in the dialog. normalizeSponsorship in the action is a no-op identity function, so order: 0 is sent in every POST/PUT body. If the API rejects unexpected fields or if 0 is treated as "move to top", this causes silent mis-ordering of existing tiers.
Fix: if order is not user-editable in this dialog, remove it from initialValues and strip it in normalizeSponsorship. Confirm with the API spec whether order should be included in create/update payloads.
smarcet
requested changes
May 19, 2026
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
feature/sponsorship-type-list-mui
branch
from
6e64c7f to
72a2f9e
Compare
| {T.translate("edit_sponsorship.size")} | ||
| </InputLabel> | ||
| <Box> | ||
| <MuiFormikSelect |
There was a problem hiding this comment.
use MuiformikSelectV2 , is much cleaner
Author
There was a problem hiding this comment.
Current version is not exporting MuiformikSelectV2, I'll create a PR adding the component into the webpack.common and index file so it can be imported. Thank you
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/86b9j5rez
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Localization