Feature: show level purchases view#949
Open
santipalenque wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a sponsor purchases management feature: new Redux actions and CSV export, reducer and store slice, a purchases list page with search/pagination/sort and inline approve/reject, routing/menu/translations, and corresponding UI/tests updates. ChangesSponsor Purchases List Feature
Sequence DiagramsequenceDiagram
participant Page as ShowPurchaseListPage
participant Redux as Redux (actions/store)
participant API as API
Page->>Redux: mount() → getAllSponsorPurchases(term,page,perPage,order,orderDir)
Redux->>Redux: dispatch REQUEST_ALL_SPONSOR_PURCHASES
Redux->>API: GET /api/public/v1/summits/{id}/sponsors/purchases?filters...
API-->>Redux: returns purchases array
Redux->>Redux: dispatch RECEIVE_ALL_SPONSOR_PURCHASES
Redux-->>Page: state update triggers re-render with purchases
Page->>Redux: user selects status → approveSponsorPurchase(sponsorId,paymentId) or rejectSponsorPurchase(sponsorId,paymentId)
Redux->>API: POST /api/public/v1/summits/{id}/sponsors/{sponsorId}/payments/{paymentId}/approve or /cancel
API-->>Redux: purchase status updated
Redux-->>Page: updated purchase status reflected in table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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
🤖 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.
Inline comments:
In `@src/actions/sponsor-purchases-actions.js`:
- Around line 95-103: The current promise chain calling
getRequest(createAction(REQUEST_ALL_SPONSOR_PURCHASES),
createAction(RECEIVE_ALL_SPONSOR_PURCHASES), ...)(params)(dispatch) only
dispatches stopLoading() inside .then(...), so on failures the global loading
state can remain set; change the chain to ensure stopLoading() is dispatched in
all outcomes (use .finally(() => dispatch(stopLoading())) or add a .catch(err =>
{ /* optional handle */; throw err; }).finally(...) ) so that stopLoading() is
always dispatched after the getRequest call, keeping createAction, getRequest,
and stopLoading usage intact.
In `@src/pages/sponsors/show-pages-list-page/index.js`:
- Around line 184-191: The JSX unguarded access to match.url can throw if match
is undefined; update the Breadcrumb data to use a safe fallback (e.g., use
optional chaining and a default: pathname: match?.url || '' or match?.path ||
'') or ensure the component is wrapped with withRouter so match is always
injected; locate the Breadcrumb usage and replace direct match.url with the
guarded expression (or add withRouter to the component export) to prevent the
TypeError.
In `@src/pages/sponsors/show-purchase-list-page/index.js`:
- Around line 90-92: The menu handler handleMenu currently only console.logs the
item, leaving a dead UI control and leaking payloads; replace the debug log with
a real action or disable the control: implement a meaningful handler (e.g., call
a passed prop like onMenuSelect(item), open the purchase details modal, or
trigger navigation to the purchase view) and remove the console.log, and if no
action is available remove the menu rendering or render a disabled menu entry so
users cannot interact with it; also apply the same change to the other menu
handler block that mirrors this behavior.
- Around line 78-80: handleExport currently calls exportAllSponsorPurchases()
with no context so the exported CSV ignores the current term, order, and
orderDir; update handleExport to pass the current filter/sort state (term,
order, orderDir) to exportAllSponsorPurchases (or read them from the same state
object it uses) so the export mirrors the table view—refer to the handleExport
function and the exportAllSponsorPurchases function and ensure they accept and
forward those parameters.
🪄 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: 28b8f85e-d7de-43cf-9bc9-62bd810a332f
📒 Files selected for processing (9)
src/actions/sponsor-purchases-actions.jssrc/components/menu/menu-definition.jssrc/i18n/en.jsonsrc/layouts/sponsor-layout.jssrc/pages/sponsors/show-pages-list-page/index.jssrc/pages/sponsors/show-purchase-list-page/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-purchases-tab/index.jssrc/reducers/sponsors/show-purchase-list-reducer.jssrc/store.js
Comment on lines
+95
to
+103
| return getRequest( | ||
| createAction(REQUEST_ALL_SPONSOR_PURCHASES), | ||
| createAction(RECEIVE_ALL_SPONSOR_PURCHASES), | ||
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/purchases`, | ||
| authErrorHandler, | ||
| { order, orderDir, page, perPage, term } | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); | ||
| }); |
There was a problem hiding this comment.
Ensure loading state is cleared on request failures.
At Line 95, stopLoading() only runs in .then(...). If the request fails, global loading can remain stuck.
💡 Suggested fix
- return getRequest(
+ return getRequest(
createAction(REQUEST_ALL_SPONSOR_PURCHASES),
createAction(RECEIVE_ALL_SPONSOR_PURCHASES),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/purchases`,
authErrorHandler,
{ order, orderDir, page, perPage, term }
- )(params)(dispatch).then(() => {
- dispatch(stopLoading());
- });
+ )(params)(dispatch).finally(() => {
+ dispatch(stopLoading());
+ });📝 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
| return getRequest( | |
| createAction(REQUEST_ALL_SPONSOR_PURCHASES), | |
| createAction(RECEIVE_ALL_SPONSOR_PURCHASES), | |
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/purchases`, | |
| authErrorHandler, | |
| { order, orderDir, page, perPage, term } | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| }); | |
| return getRequest( | |
| createAction(REQUEST_ALL_SPONSOR_PURCHASES), | |
| createAction(RECEIVE_ALL_SPONSOR_PURCHASES), | |
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/purchases`, | |
| authErrorHandler, | |
| { order, orderDir, page, perPage, term } | |
| )(params)(dispatch).finally(() => { | |
| dispatch(stopLoading()); | |
| }); |
🤖 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/sponsor-purchases-actions.js` around lines 95 - 103, The current
promise chain calling getRequest(createAction(REQUEST_ALL_SPONSOR_PURCHASES),
createAction(RECEIVE_ALL_SPONSOR_PURCHASES), ...)(params)(dispatch) only
dispatches stopLoading() inside .then(...), so on failures the global loading
state can remain set; change the chain to ensure stopLoading() is dispatched in
all outcomes (use .finally(() => dispatch(stopLoading())) or add a .catch(err =>
{ /* optional handle */; throw err; }).finally(...) ) so that stopLoading() is
always dispatched after the getRequest call, keeping createAction, getRequest,
and stopLoading usage intact.
Comment on lines
+184
to
+191
| <div> | ||
| <Breadcrumb | ||
| data={{ | ||
| title: T.translate("show_pages.pages"), | ||
| pathname: match.url | ||
| }} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Critical: Unchecked access to match.url causes runtime error.
The code accesses match.url without verifying that match exists, causing test failures as confirmed by the pipeline. If the component is rendered without the match prop, this will throw a TypeError.
🐛 Proposed fix using optional chaining
<Breadcrumb
data={{
title: T.translate("show_pages.pages"),
- pathname: match.url
+ pathname: match?.url
}}
/>Alternatively, if match is always expected from React Router, ensure the component is wrapped with withRouter HOC or verify the parent route passes match correctly.
📝 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
| <div> | |
| <Breadcrumb | |
| data={{ | |
| title: T.translate("show_pages.pages"), | |
| pathname: match.url | |
| }} | |
| /> | |
| </div> | |
| <div> | |
| <Breadcrumb | |
| data={{ | |
| title: T.translate("show_pages.pages"), | |
| pathname: match?.url | |
| }} | |
| /> | |
| </div> |
🧰 Tools
🪛 GitHub Actions: jest / 0_build.txt
[error] 188-188: TypeError: Cannot read properties of undefined (reading 'url') at pathname: match.url (ShowPagesListPage tests fail).
🪛 GitHub Actions: jest / build
[error] 188-188: TypeError: Cannot read properties of undefined (reading 'url'). Error occurs at pathname: match.url (src/pages/sponsors/show-pages-list-page/index.js:188:29), causing ShowPagesListPage tests to fail.
🤖 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/sponsors/show-pages-list-page/index.js` around lines 184 - 191, The
JSX unguarded access to match.url can throw if match is undefined; update the
Breadcrumb data to use a safe fallback (e.g., use optional chaining and a
default: pathname: match?.url || '' or match?.path || '') or ensure the
component is wrapped with withRouter so match is always injected; locate the
Breadcrumb usage and replace direct match.url with the guarded expression (or
add withRouter to the component export) to prevent the TypeError.
Comment on lines
+78
to
+80
| const handleExport = () => { | ||
| exportAllSponsorPurchases(); | ||
| }; |
There was a problem hiding this comment.
Pass current filters/sort when exporting CSV.
At Line 79, export ignores current term, order, and orderDir, so downloaded data can differ from what the table is showing.
💡 Suggested fix
const handleExport = () => {
- exportAllSponsorPurchases();
+ exportAllSponsorPurchases(term, order, orderDir);
};📝 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
| const handleExport = () => { | |
| exportAllSponsorPurchases(); | |
| }; | |
| const handleExport = () => { | |
| exportAllSponsorPurchases(term, order, orderDir); | |
| }; |
🤖 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/sponsors/show-purchase-list-page/index.js` around lines 78 - 80,
handleExport currently calls exportAllSponsorPurchases() with no context so the
exported CSV ignores the current term, order, and orderDir; update handleExport
to pass the current filter/sort state (term, order, orderDir) to
exportAllSponsorPurchases (or read them from the same state object it uses) so
the export mirrors the table view—refer to the handleExport function and the
exportAllSponsorPurchases function and ensure they accept and forward those
parameters.
Comment on lines
+90
to
+92
| const handleMenu = (item) => { | ||
| console.log("MENU : ", item); | ||
| }; |
There was a problem hiding this comment.
Avoid shipping a non-functional menu action with debug logging.
At Line 91 the menu action only logs row data; this leaves a dead UI control and exposes row payloads in the browser console.
💡 Suggested fix
- const handleMenu = (item) => {
- console.log("MENU : ", item);
- };
+ const handleMenu = () => {};- {
- columnKey: "menu",
- header: "",
- width: 100,
- align: "center",
- render: (row) => (
- <IconButton
- size="large"
- sx={{ color: "primary.main" }}
- onClick={() => handleMenu(row)}
- >
- <MenuIcon fontSize="large" />
- </IconButton>
- )
- }
+ // TODO: add this column back when menu actions are implementedAlso applies to: 173-186
🤖 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/sponsors/show-purchase-list-page/index.js` around lines 90 - 92,
The menu handler handleMenu currently only console.logs the item, leaving a dead
UI control and leaking payloads; replace the debug log with a real action or
disable the control: implement a meaningful handler (e.g., call a passed prop
like onMenuSelect(item), open the purchase details modal, or trigger navigation
to the purchase view) and remove the console.log, and if no action is available
remove the menu rendering or render a disabled menu entry so users cannot
interact with it; also apply the same change to the other menu handler block
that mirrors this behavior.
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
1 participant
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/86b66n5gb
Summary by CodeRabbit
New Features
Style
Documentation