Merged
Conversation
📝 WalkthroughWalkthroughReplaces the legacy EditSponsorPage and legacy SponsorGeneralForm with a new SponsorPage router, TabNav, and tabDefs; moves SponsorGeneralForm under sponsor-page/tabs; adjusts routing in sponsor-id-layout; rewires many imports, Redux mappings, and access restrictions across sponsor-related tabs and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant TabNav
participant SponsorPage
participant Store as Redux
participant API
User->>Browser: click sponsor tab
Browser->>TabNav: tab click event
TabNav->>Browser: history.push(/app/summits/:sid/sponsors/:id/<tab>)
Browser->>SponsorPage: route change → mount tab route
SponsorPage->>Redux: dispatch fetch actions (ads, tiers, forms, purchases, badge scans...)
Redux->>API: HTTP requests
API-->>Redux: responses
Redux-->>SponsorPage: updated props (entity, lists)
SponsorPage-->>Browser: render selected tab component
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/index.js (1)
324-328:⚠️ Potential issue | 🟡 MinorGuard
ItemSettingsModalrender to satisfy required prop contract.The
itemprop is declared asPropTypes.object.isRequiredinItemSettingsModal, but the current code passesnullwhensettingsItemisnull, violating the contract. Render the modal only whensettingsItemexists.Minimal fix
- <ItemSettingsModal - item={settingsItem} - open={!!settingsItem} - onClose={() => setSettingsItem(null)} - /> + {settingsItem && ( + <ItemSettingsModal + item={settingsItem} + open + onClose={() => setSettingsItem(null)} + /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/index.js` around lines 324 - 328, The ItemSettingsModal is being rendered with item={settingsItem} even when settingsItem is null which violates its PropTypes.object.isRequired; change the render so ItemSettingsModal is only mounted when settingsItem is truthy (e.g. wrap the component in a conditional that checks settingsItem) and keep the existing props (open, onClose using setSettingsItem(null)) so onClose still clears state but the modal is never instantiated with a null item.src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js (1)
44-56:⚠️ Potential issue | 🔴 CriticalFix inconsistent
jest.requireActualpath causing pipeline failure.The
jest.mockpath on line 44 correctly uses../../../../../../actions/sponsor-pages-actions(6 directory levels up), but thejest.requireActualcall on line 45 still uses the old path../../../../actions/sponsor-pages-actions(4 levels up). This path inconsistency causes the Jest test suite failure shown in the pipeline logs.🐛 Proposed fix
jest.mock("../../../../../../actions/sponsor-pages-actions", () => ({ - ...jest.requireActual("../../../../actions/sponsor-pages-actions"), + ...jest.requireActual("../../../../../../actions/sponsor-pages-actions"), getSponsorManagedPages: jest.fn(() => ({ type: "MOCK_ACTION" })), getSponsorCustomizedPages: jest.fn(() => ({ type: "MOCK_ACTION" })), getSponsorCustomizedPage: jest.fn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js` around lines 44 - 56, The jest mock uses a 6-level path but the nested jest.requireActual call uses a 4-level path, causing resolution failures; update the jest.requireActual call inside the jest.mock block so it imports the same module path as the outer jest.mock (i.e., use the identical "../../../../../../actions/sponsor-pages-actions" path) ensuring the mocked exports like getSponsorManagedPages, getSponsorCustomizedPages, getSponsorCustomizedPage, saveSponsorCustomizedPage, saveSponsorManagedPage, resetSponsorPage, archiveCustomizedPage, and unarchiveCustomizedPage reference the real module correctly.
🧹 Nitpick comments (3)
src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-form-item-from-inventory.js (1)
23-32: Consider replacing deep relative imports with aliases to reduce fragility.Using path aliases (e.g.,
src/...or@/...) would make future sponsor-page moves safer and reduce churn like this across files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-form-item-from-inventory.js` around lines 23 - 32, The imports in sponsor-form-item-from-inventory.js use fragile deep relative paths (e.g., "../../../../../../../utils/rate-helpers") — replace them with your project path aliases (e.g., "@/utils/rate-helpers" or "src/utils/rate-helpers") for all referenced symbols (formatRateFromCents, SearchInput, DEFAULT_CURRENT_PAGE, DEFAULT_PER_PAGE, getInventoryItems, MuiTable, MenuButton) so the file can move without breaking imports; update the import statements to use the configured alias and, if not already configured, add the alias to your build/tsconfig/jsconfig and eslint resolver so the new paths resolve correctly.src/pages/sponsors/sponsor-page/index.js (1)
76-81: Remove unused props passed toTabNav.
TabNavderivescurrentSummitandsponsorfrom Redux state via its ownmapStateToProps. ThesummitIdandsponsorIdprops passed here are not used by the component.♻️ Proposed fix
<Box sx={{ borderBottom: 1, borderColor: "divider", mt: 2 }}> <TabNav history={history} location={location} - summitId={currentSummit.id} - sponsorId={entity.id} /> </Box>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/index.js` around lines 76 - 81, The TabNav component is receiving unused props summitId and sponsorId (they are derived from Redux in TabNav's mapStateToProps); remove the summitId={currentSummit.id} and sponsorId={entity.id} props from the TabNav JSX here while keeping the necessary history and location props so TabNav continues to receive routing props only.src/pages/sponsors/sponsor-page/components/tab-nav.js (1)
54-54: Consider handling whenselectedTabdoesn't match any visible tab.If a user navigates directly to a URL for a tab they don't have access to (e.g.,
/app/summits/1/sponsors/42/usersas a non-admin),selectedTabwill be"users"but that tab won't be in the filtered list. MUITabswithvalueprop set to a non-existent tab value can behave unexpectedly (no tab selected, or console warning).♻️ Suggested improvement
+ const visibleTabs = tabs.filter((t) => memberObj.hasAccess(t.accessRoute)); + const selectedTab = + visibleTabs.find((t) => t.value === location.pathname.split("/")[6])?.value || + visibleTabs[0]?.value; - const selectedTab = location.pathname.split("/")[6] || tabs[0].value; // ... return ( <Tabs value={selectedTab} onChange={(ev, val) => handleTabChange(val)} sx={{ minHeight: "36px" }} > - {tabs - .filter((t) => memberObj.hasAccess(t.accessRoute)) - .map((tab) => ( + {visibleTabs.map((tab) => (Also applies to: 62-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/components/tab-nav.js` at line 54, The current computation of selectedTab using location.pathname.split("/")[6] can yield a value that doesn't exist in the filtered/visible tabs list, causing MUI Tabs to misbehave; update the logic that determines selectedTab (the variable named selectedTab in this module/component) to check whether the derived value exists in the current tabs array (after any filtering) and if not, fall back to the first visible tab's value (e.g., tabs[0].value or the first element of the filtered list) before passing it to the Tabs value prop so the component always receives a valid tab value.
🤖 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/layouts/sponsor-id-layout.js`:
- Line 144: The catch-all Route (Route path={match.url} component={SponsorPage})
lets legacy /sponsor-forms/:form_id/items requests reach SponsorPage but they no
longer match internal /forms/:form_id/items routes; add a Redirect before that
catch-all that rewrites legacy /sponsor-forms/:form_id/* to /forms/:form_id/* to
preserve deep links; place this Redirect immediately above the Route that
renders SponsorPage so it runs first and references the same match.url base and
SponsorPage routing behavior.
In `@src/pages/sponsors/sponsor-page/tabs/sponsor-general-form/index.js`:
- Around line 41-62: The component SponsorGeneralForm is calling getSponsorTiers
in handleSponsorshipPaginate but getSponsorTiers was not destructured from the
component props; update the SponsorGeneralForm parameter destructuring to
include getSponsorTiers so handleSponsorshipPaginate can call it, i.e., add
getSponsorTiers to the list of props (alongside sponsor, member, currentSummit,
etc.) and ensure any usages like handleSponsorshipPaginate(sponsor.id, ...)
reference the newly-destructured getSponsorTiers.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js`:
- Around line 58-61: The jest.mock call for sponsor-forms-actions uses an
inconsistent requireActual path; update the jest.requireActual argument to match
the mock module path ("../../../../../../actions/sponsor-forms-actions") so the
real module is correctly referenced, and keep the getSponsorships mock
(jest.fn(() => () => Promise.resolve())) as is; verify the change in the
jest.mock block that defines getSponsorships to ensure imports resolve
correctly.
In `@src/pages/sponsors/sponsor-users-list-per-sponsor/index.js`:
- Around line 163-166: The component now receives sponsor from mapStateToProps
(currentSponsorState.entity) which may have company: null and causes the
dereference sponsor.company.id to crash; update either mapStateToProps to ensure
sponsor has a safe default shape (e.g., merge entity with company: {} when
undefined) or change all dereferences to use a safe check (e.g., guard with
sponsor && sponsor.company && sponsor.company.id or optional chaining
sponsor.company?.id) so accesses in the component no longer throw when
currentSponsorState.entity.company is null; locate mapStateToProps, the sponsor
variable, and the places using sponsor.company.id and apply one of these fixes
consistently.
---
Outside diff comments:
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/index.js`:
- Around line 324-328: The ItemSettingsModal is being rendered with
item={settingsItem} even when settingsItem is null which violates its
PropTypes.object.isRequired; change the render so ItemSettingsModal is only
mounted when settingsItem is truthy (e.g. wrap the component in a conditional
that checks settingsItem) and keep the existing props (open, onClose using
setSettingsItem(null)) so onClose still clears state but the modal is never
instantiated with a null item.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js`:
- Around line 44-56: The jest mock uses a 6-level path but the nested
jest.requireActual call uses a 4-level path, causing resolution failures; update
the jest.requireActual call inside the jest.mock block so it imports the same
module path as the outer jest.mock (i.e., use the identical
"../../../../../../actions/sponsor-pages-actions" path) ensuring the mocked
exports like getSponsorManagedPages, getSponsorCustomizedPages,
getSponsorCustomizedPage, saveSponsorCustomizedPage, saveSponsorManagedPage,
resetSponsorPage, archiveCustomizedPage, and unarchiveCustomizedPage reference
the real module correctly.
---
Nitpick comments:
In `@src/pages/sponsors/sponsor-page/components/tab-nav.js`:
- Line 54: The current computation of selectedTab using
location.pathname.split("/")[6] can yield a value that doesn't exist in the
filtered/visible tabs list, causing MUI Tabs to misbehave; update the logic that
determines selectedTab (the variable named selectedTab in this module/component)
to check whether the derived value exists in the current tabs array (after any
filtering) and if not, fall back to the first visible tab's value (e.g.,
tabs[0].value or the first element of the filtered list) before passing it to
the Tabs value prop so the component always receives a valid tab value.
In `@src/pages/sponsors/sponsor-page/index.js`:
- Around line 76-81: The TabNav component is receiving unused props summitId and
sponsorId (they are derived from Redux in TabNav's mapStateToProps); remove the
summitId={currentSummit.id} and sponsorId={entity.id} props from the TabNav JSX
here while keeping the necessary history and location props so TabNav continues
to receive routing props only.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-form-item-from-inventory.js`:
- Around line 23-32: The imports in sponsor-form-item-from-inventory.js use
fragile deep relative paths (e.g., "../../../../../../../utils/rate-helpers") —
replace them with your project path aliases (e.g., "@/utils/rate-helpers" or
"src/utils/rate-helpers") for all referenced symbols (formatRateFromCents,
SearchInput, DEFAULT_CURRENT_PAGE, DEFAULT_PER_PAGE, getInventoryItems,
MuiTable, MenuButton) so the file can move without breaking imports; update the
import statements to use the configured alias and, if not already configured,
add the alias to your build/tsconfig/jsconfig and eslint resolver so the new
paths resolve correctly.
🪄 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: 97924e19-a7ad-4759-be77-ec64b83f563a
📒 Files selected for processing (37)
src/components/forms/sponsor-general-form/index.jssrc/layouts/sponsor-id-layout.jssrc/pages/sponsors/edit-sponsor-page.jssrc/pages/sponsors/sponsor-page/components/tab-nav.jssrc/pages/sponsors/sponsor-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-badge-scans/edit-badge-scan-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-badge-scans/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/cart-note/__tests__/cart-note.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/cart-note/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/cart-view.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/__tests__/edit-cart-form.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/edit-cart-form.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/edit-form/new-cart-form.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/select-form-dialog/__tests__/select-form-dialog.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/components/select-form-dialog/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/add-sponsor-form-template-popup/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-form-item-from-inventory.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/add-extra-question-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/add-tier-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/badge-scan-settings.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/extra-questions.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/manage-tier-addons-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/sponsor-header.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/sponsorship.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-media-upload-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/components/add-sponsor-page-template-popup/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-purchases-tab/index.jssrc/pages/sponsors/sponsor-users-list-per-sponsor/index.js
💤 Files with no reviewable changes (2)
- src/components/forms/sponsor-general-form/index.js
- src/pages/sponsors/edit-sponsor-page.js
| )} | ||
| /> | ||
| </Switch> | ||
| <Route path={match.url} component={SponsorPage} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether legacy route strings are still referenced (docs, links, navigation code).
rg -n --iglob '*.{js,jsx,ts,tsx,md}' 'sponsor-forms/.*/items|/sponsor-forms/'Repository: fntechgit/summit-admin
Length of output: 2522
🏁 Script executed:
# First, let's examine the sponsor-id-layout.js file to understand the routing structure
wc -l src/layouts/sponsor-id-layout.jsRepository: fntechgit/summit-admin
Length of output: 102
🏁 Script executed:
# Read the sponsor-id-layout.js file to see the routing context
cat -n src/layouts/sponsor-id-layout.jsRepository: fntechgit/summit-admin
Length of output: 6364
🏁 Script executed:
# Check if there are any historical references to the old sponsor-forms route in git
git log --oneline --all --grep="sponsor-forms" | head -20Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Search for any route definitions or removals related to sponsor-forms in the layout file
rg -n "sponsor-forms|SponsorPage" src/layouts/sponsor-id-layout.jsRepository: fntechgit/summit-admin
Length of output: 813
🏁 Script executed:
# Check the test file context to understand those old route references
sed -n '120,175p' src/pages/sponsors/__tests__/edit-sponsor-page.test.jsRepository: fntechgit/summit-admin
Length of output: 1637
🏁 Script executed:
# Search for any navigation or link generation code that might reference sponsor-forms routes
rg -n "sponsor-forms|/items" --type js -B 2 -A 2 src/pages/sponsors/ | head -80Repository: fntechgit/summit-admin
Length of output: 8994
🏁 Script executed:
# Check if SponsorPage component has any special handling for sponsor-forms paths
head -100 src/pages/sponsors/sponsor-page.jsRepository: fntechgit/summit-admin
Length of output: 159
🏁 Script executed:
# Find SponsorPage component
find src/pages/sponsors -name "*sponsor-page*" -type fRepository: fntechgit/summit-admin
Length of output: 369
🏁 Script executed:
# Check if SponsorPage is a directory with index
ls -la src/pages/sponsors/sponsor-page/Repository: fntechgit/summit-admin
Length of output: 353
🏁 Script executed:
# Look for any route configuration related to forms that might show the old pattern
rg -n "forms.*items|sponsor-forms" src/layouts/ src/routes/ 2>/dev/null || trueRepository: fntechgit/summit-admin
Length of output: 176
🏁 Script executed:
# Check if there's a forms route handler or if forms are nested under sponsor-page
rg -n "Route.*forms" src/pages/sponsors/sponsor-page/ -A 2Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Check sponsor-layout.js to see how forms are routed
cat -n src/layouts/sponsor-layout.jsRepository: fntechgit/summit-admin
Length of output: 5567
🏁 Script executed:
# Look for any route that might handle the legacy sponsor-forms pattern
rg -n "sponsor-forms|/forms/" src/layouts/sponsor-layout.jsRepository: fntechgit/summit-admin
Length of output: 146
🏁 Script executed:
# Check git history to see if sponsor-forms routes were explicitly removed
git log --oneline --all | grep -i "sponsor-form\|route" | head -20Repository: fntechgit/summit-admin
Length of output: 430
🏁 Script executed:
# Search for any comments or TODOs about route changes
rg -n "TODO|FIXME|deprecated|legacy|backward" src/layouts/sponsor-id-layout.js src/layouts/sponsor-layout.jsRepository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Final check: are there any references to SponsorPage handling forms routes?
rg -n "forms|items" src/pages/sponsors/sponsor-page/index.jsRepository: fntechgit/summit-admin
Length of output: 353
🏁 Script executed:
# Check the specific git commit about sponsor page routes refactoring
git show 674196c --stat | head -50Repository: fntechgit/summit-admin
Length of output: 2984
🏁 Script executed:
# Verify if SponsorPage's /forms route properly handles the form_id/items path
sed -n '95,130p' src/pages/sponsors/sponsor-page/index.jsRepository: fntechgit/summit-admin
Length of output: 1176
Add backward compatibility redirect for legacy sponsor-forms routes.
The recent route refactoring changed the pattern from sponsor-forms to forms. With the catch-all <Route path={match.url} component={SponsorPage} /> at line 144, legacy links using /sponsor-forms/:form_id/items will reach SponsorPage but fail to match its internal route (/forms/:form_id/items), breaking existing bookmarks and deep links.
Add a redirect route before the catch-all to preserve backward compatibility:
Suggested redirect
-import { Switch, Route } from "react-router-dom";
+import { Switch, Route, Redirect } from "react-router-dom";
...
+ <Route
+ exact
+ path={`${match.url}/sponsor-forms/:form_id/items`}
+ render={(props) => (
+ <Redirect to={`${match.url}/forms/${props.match.params.form_id}/items`} />
+ )}
+ />
<Route path={match.url} component={SponsorPage} />📝 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.
Suggested change
| <Route path={match.url} component={SponsorPage} /> | |
| <Route | |
| exact | |
| path={`${match.url}/sponsor-forms/:form_id/items`} | |
| render={(props) => ( | |
| <Redirect to={`${match.url}/forms/${props.match.params.form_id}/items`} /> | |
| )} | |
| /> | |
| <Route path={match.url} component={SponsorPage} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/layouts/sponsor-id-layout.js` at line 144, The catch-all Route (Route
path={match.url} component={SponsorPage}) lets legacy
/sponsor-forms/:form_id/items requests reach SponsorPage but they no longer
match internal /forms/:form_id/items routes; add a Redirect before that
catch-all that rewrites legacy /sponsor-forms/:form_id/* to /forms/:form_id/* to
preserve deep links; place this Redirect immediately above the Route that
renders SponsorPage so it runs first and references the same match.url base and
SponsorPage routing behavior.
src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/sponsors/__tests__/edit-sponsor-page.test.js (1)
84-86: Remove the explicitactwrapper.The project uses
@testing-library/user-eventv14.5.2, which automatically wraps user interactions inact(). The explicit wrapper on lines 84-86 is redundant.await userEvent.click(formsTab);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/__tests__/edit-sponsor-page.test.js` around lines 84 - 86, Remove the redundant explicit act wrapper around the user interaction: locate the test that calls await act(async () => { await userEvent.click(formsTab); }); and replace it by directly awaiting the user event call (await userEvent.click(formsTab)); specifically remove the outer act(...) so userEvent.click(formsTab) runs by itself since `@testing-library/user-event` v14.5.2 already wraps interactions in act.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/sponsors/__tests__/edit-sponsor-page.test.js`:
- Around line 84-86: Remove the redundant explicit act wrapper around the user
interaction: locate the test that calls await act(async () => { await
userEvent.click(formsTab); }); and replace it by directly awaiting the user
event call (await userEvent.click(formsTab)); specifically remove the outer
act(...) so userEvent.click(formsTab) runs by itself since
`@testing-library/user-event` v14.5.2 already wraps interactions in act.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4a87744-2fd2-4e9e-a424-cf65276f6b1d
📒 Files selected for processing (2)
src/pages/sponsors/__tests__/edit-sponsor-page.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/tests/sponsor-pages-tab.test.js
smarcet
reviewed
Apr 7, 2026
smarcet
reviewed
Apr 7, 2026
smarcet
reviewed
Apr 7, 2026
smarcet
reviewed
Apr 7, 2026
smarcet
requested changes
Apr 7, 2026
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@santipalenque please review comments
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sponsors/sponsor-page/tabDefs.js`:
- Around line 18-23: The Breadcrumb title is hardcoded as "Forms", breaking
i18n; change it to use the same i18n pattern as the tabDefs (e.g., use the
translation key used by this tab or call the translation function) so Breadcrumb
receives a localized string instead of a literal. Update the Breadcrumb
data.title to derive from the tab's labelKey or invoke the translation helper
used across the file (referencing Breadcrumb and the existing labelKey entries)
so the title is consistent with the other tab labels.
🪄 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: 90381f99-ac21-43f0-926e-88923a7ed338
📒 Files selected for processing (12)
src/pages/sponsors/sponsor-page/components/tab-nav.jssrc/pages/sponsors/sponsor-page/index.jssrc/pages/sponsors/sponsor-page/tabDefs.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-badge-scans/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-general-form/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-media-upload-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-purchases-tab/index.jssrc/pages/sponsors/sponsor-users-list-per-sponsor/index.js
✅ Files skipped from review due to trivial changes (2)
- src/pages/sponsors/sponsor-page/tabs/sponsor-purchases-tab/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-media-upload-tab/index.js
🚧 Files skipped from review as they are similar to previous changes (9)
- src/pages/sponsors/sponsor-page/tabs/sponsor-badge-scans/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/tests/sponsor-pages-tab.test.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-cart-tab/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-general-form/index.js
- src/pages/sponsors/sponsor-page/index.js
- src/pages/sponsors/sponsor-page/components/tab-nav.js
- src/pages/sponsors/sponsor-users-list-per-sponsor/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js
- src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js
Comment on lines
+18
to
+23
| <Breadcrumb | ||
| data={{ | ||
| title: "Forms", | ||
| pathname: match.url | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Hardcoded breadcrumb title breaks i18n consistency.
The "Forms" string is hardcoded while all tab definitions use labelKey for internationalization. Consider using a translation function or pattern consistent with the rest of the file.
Suggested fix
+import T from "i18n-react"; // or your i18n utility
+
const SponsorFormsRoute = ({ match }) => (
<div>
<Breadcrumb
data={{
- title: "Forms",
+ title: T.translate("edit_sponsor.tab.forms"),
pathname: match.url
}}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-page/tabDefs.js` around lines 18 - 23, The
Breadcrumb title is hardcoded as "Forms", breaking i18n; change it to use the
same i18n pattern as the tabDefs (e.g., use the translation key used by this tab
or call the translation function) so Breadcrumb receives a localized string
instead of a literal. Update the Breadcrumb data.title to derive from the tab's
labelKey or invoke the translation helper used across the file (referencing
Breadcrumb and the existing labelKey entries) so the title is consistent with
the other tab labels.
This was referenced Apr 7, 2026
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
2 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.
https://app.clickup.com/t/86b99kqg1
Summary by CodeRabbit
New Features
Refactor
Chores
Tests