-
Notifications
You must be signed in to change notification settings - Fork 4
feat: sponsor cart view #748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a sponsor cart feature: new Redux actions and reducer, API calls for fetching/deleting sponsor cart forms, table UI components and children support, a Sponsor Cart tab in sponsor edit page, i18n entries, and expanded PURCHASES_API_SCOPES in .env.example. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as SponsorCartTab
participant Store as Redux Store
participant Action as SponsorCart Actions
participant API as Purchases API
participant Reducer as Cart Reducer
User->>UI: Open tab or type search term
UI->>Store: Dispatch getSponsorCart(term)
Store->>Action: Execute async action
Action->>Action: read summit & sponsor IDs, get token
Action->>Store: Dispatch REQUEST_SPONSOR_CART
Action->>API: GET /purchases/.../cart?filter=term
API-->>Action: Return cart data
Action->>Store: Dispatch RECEIVE_SPONSOR_CART
Store->>Reducer: Convert amounts, compute totals, store cart
Store-->>UI: Updated state
UI->>User: Render cart rows and totals
User->>UI: Click Delete on a form
UI->>Store: Dispatch deleteSponsorCartForm(formId)
Store->>Action: Execute delete action
Action->>API: DELETE /purchases/.../cart/:formId
API-->>Action: Success
Action->>Store: Dispatch SPONSOR_CART_FORM_DELETED
Store->>Reducer: Remove form, update totals
Store-->>UI: Updated state
UI->>User: Show success snackbar
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @src/components/mui/table/mui-table.js:
- Around line 257-259: The children are always rendered even when data.length
=== 0, which can cause a TotalRow (or other child rows) to appear alongside the
"no items" message; update the rendering so children are only injected when
there is data (i.e., wrap the existing {children} with a conditional like render
children only when data.length > 0), or alternatively suppress the "no items"
row when children are present—preferably implement the first option by gating
children on data.length > 0 to avoid showing totals when the table is empty.
In @src/pages/sponsors/sponsor-cart-tab/index.js:
- Around line 47-49: The useEffect call invokes getSponsorCart but omits it from
the dependency array; update the effect to include getSponsorCart (i.e.,
useEffect(() => { getSponsorCart(); }, [getSponsorCart])) or, if getSponsorCart
is not stable, memoize it with useCallback or bind the stable action via
mapDispatchToProps so the reference doesn't change; ensure the effect lists
getSponsorCart in its dependency array to satisfy exhaustive-deps.
- Around line 102-109: Replace the hardcoded "Manage Items" label with the i18n
translation lookup in the SponsorCartTab component (the Button that calls
handleManageItems(row)) by using the translation hook/function used in this
codebase (e.g., t('...') or useTranslation) and reference a new key like
sponsors.manageItems; then add that key and English value to src/i18n/en.json so
the button text is served from translations rather than hardcoded.
- Around line 44-45: The two useState hooks declared as openPopup and formEdit
in the SponsorCartTab component are unused; remove the declarations "const
[openPopup, setOpenPopup] = useState(null);" and "const [formEdit, setFormEdit]
= useState(null);" to clean up dead state, or if they are placeholders, add a
brief TODO above each declaration indicating intended future use and ensure any
setter usage (setFormEdit) is either removed or tied to a consuming piece of UI
logic before keeping the state.
In @src/reducers/sponsors/sponsor-page-cart-list-reducer.js:
- Around line 62-74: The SPONSOR_CART_FORM_DELETED case removes the form from
state.cart.forms but leaves cart.total stale; update the reducer to recalculate
cart.total after filtering by summing the remaining forms' numeric amounts
(e.g., sum form.net_amount or the canonical cent value you store) and set
cart.total to that new sum (converted to the same unit/format used elsewhere),
ensuring you reference state.cart.forms and the filtered forms variable so the
returned state includes the updated cart.total alongside the updated forms.
In @src/utils/constants.js:
- Line 227: The CENTS_FOR_DOLLAR constant is incorrect (set to 1000) causing
monetary conversions to be off by 10x; change CENTS_FOR_DOLLAR to 100 and ensure
any conversion helpers such as centsToDollar use this constant; verify usages in
sponsor-page-cart-list-reducer.js (where net_amount and discount_amount are
converted) still reference CENTS_FOR_DOLLAR and update tests or comments if any
assume the old value.
🧹 Nitpick comments (7)
.env.example (1)
15-15: Wrap the scope value in quotes for consistency.The static analysis tool correctly flags that the
PURCHASES_API_SCOPESvalue should be surrounded in quotes to follow dotenv best practices and avoid potential parsing issues with special characters or spaces.📝 Suggested fix
-PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write my-cart/read my-cart/write +PURCHASES_API_SCOPES="purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write my-cart/read my-cart/write"src/actions/sponsor-cart-actions.js (1)
43-83: Remove unusedsummitTZvariable.The
summitTZvariable is extracted from state at Line 52 and passed to the action payload at Line 75, but it doesn't appear to be used anywhere in thegetSponsorCartfunction logic. Consider removing it unless it's consumed by the reducer for a specific purpose.♻️ Proposed cleanup if summitTZ is truly unused
export const getSponsorCart = (term = "") => async (dispatch, getState) => { const { currentSummitState, currentSponsorState } = getState(); const { currentSummit } = currentSummitState; const { entity: { id: sponsorId } } = currentSponsorState; const accessToken = await getAccessTokenSafely(); - const summitTZ = currentSummit.time_zone.name; const filter = []; dispatch(startLoading()); if (term) { const escapedTerm = escapeFilterValue(term); filter.push(`name=@${escapedTerm},code=@${escapedTerm}`); } const params = { access_token: accessToken }; if (filter.length > 0) { params["filter[]"] = filter; } return getRequest( createAction(REQUEST_SPONSOR_CART), createAction(RECEIVE_SPONSOR_CART), `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/carts/current`, customErrorHandler, - { term, summitTZ } + { term } )(params)(dispatch) .catch((err) => { console.error(err); }) .finally(() => { dispatch(stopLoading()); }); };src/utils/methods.js (1)
536-537: Add input validation to centsToDollar.The function implementation is correct, but it should handle edge cases like null, undefined, or non-numeric values to prevent runtime errors.
🛡️ Proposed defensive implementation
-export const centsToDollar = (cents) => - `$${(cents / CENTS_FOR_DOLLAR).toFixed(DECIMAL_DIGITS)}`; +export const centsToDollar = (cents) => { + if (cents == null || typeof cents !== 'number') { + return '$0.00'; + } + return `$${(cents / CENTS_FOR_DOLLAR).toFixed(DECIMAL_DIGITS)}`; +};Note: This function depends on
CENTS_FOR_DOLLAR, which may have an incorrect value (see comment on constants.js line 227).src/components/mui/table/extra-rows/NotesRow.jsx (1)
1-16: LGTM - Clean component implementation.The NotesRow component is well-structured and follows MUI patterns correctly. The use of
colSpanwithcolCountensures the note spans the full table width.For improved type safety and documentation, consider adding PropTypes:
📝 Optional PropTypes addition
import TableCell from "@mui/material/TableCell"; import TableRow from "@mui/material/TableRow"; import * as React from "react"; import { Typography } from "@mui/material"; +import PropTypes from 'prop-types'; const NotesRow = ({ colCount, note }) => ( <TableRow> <TableCell sx={{ fontWeight: 800 }} colSpan={colCount}> <Typography variant="body2" sx={{ color: "text.secondary" }}> {note} </Typography> </TableCell> </TableRow> ); +NotesRow.propTypes = { + colCount: PropTypes.number.isRequired, + note: PropTypes.string.isRequired +}; + export default NotesRow;src/components/mui/table/extra-rows/TotalRow.jsx (1)
6-30: Add prop validation or defensive checks.The component doesn't validate that
columnsis a non-empty array or thattargetColexists within the columns. While the current usage may guarantee these conditions, adding PropTypes or runtime checks would make the component more robust and easier to debug.🛡️ Suggested improvement with PropTypes
Add PropTypes at the top of the file:
+import PropTypes from 'prop-types';Add validation before the export:
}; + +TotalRow.propTypes = { + columns: PropTypes.arrayOf(PropTypes.shape({ + columnKey: PropTypes.string.isRequired + })).isRequired, + targetCol: PropTypes.string.isRequired, + total: PropTypes.node, + trailing: PropTypes.number +}; export default TotalRow;src/pages/sponsors/sponsor-cart-tab/index.js (2)
55-57: Implement placeholder event handlers.Multiple event handlers (
handleManageItems,handleLock,handlePayCreditCard,handlePayInvoice) only containconsole.logstatements, indicating incomplete functionality. These handlers are wired to UI buttons/actions, so clicking them will have no effect beyond logging.Would you like me to open an issue to track implementing these features, or can you provide the expected behavior for these actions?
Also applies to: 67-69, 71-77
178-184: Consider omitting empty options object.Line 181 passes an empty
options={{}}object toMuiTable. Since options has a default value in the component signature, this could be omitted for cleaner code unless you plan to add options later.♻️ Suggested simplification
<MuiTable columns={tableColumns} data={cart?.forms} - options={{}} onEdit={handleEdit} onDelete={handleDelete} >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.env.examplesrc/actions/sponsor-cart-actions.jssrc/components/mui/table/extra-rows/NotesRow.jsxsrc/components/mui/table/extra-rows/TotalRow.jsxsrc/components/mui/table/extra-rows/index.jssrc/components/mui/table/mui-table.jssrc/i18n/en.jsonsrc/pages/sponsors/edit-sponsor-page.jssrc/pages/sponsors/sponsor-cart-tab/index.jssrc/reducers/sponsors/sponsor-page-cart-list-reducer.jssrc/store.jssrc/utils/constants.jssrc/utils/methods.js
🧰 Additional context used
🧬 Code graph analysis (7)
src/store.js (1)
src/reducers/sponsors/sponsor-page-cart-list-reducer.js (1)
sponsorPageCartListReducer(30-79)
src/reducers/sponsors/sponsor-page-cart-list-reducer.js (3)
src/actions/sponsor-cart-actions.js (6)
REQUEST_SPONSOR_CART(28-28)REQUEST_SPONSOR_CART(28-28)RECEIVE_SPONSOR_CART(29-29)RECEIVE_SPONSOR_CART(29-29)SPONSOR_CART_FORM_DELETED(30-30)SPONSOR_CART_FORM_DELETED(30-30)src/reducers/sponsors/sponsor-forms-list-reducer.js (1)
form(120-120)src/utils/methods.js (2)
centsToDollar(536-537)centsToDollar(536-537)
src/pages/sponsors/sponsor-cart-tab/index.js (5)
src/actions/sponsor-cart-actions.js (4)
getSponsorCart(43-83)getSponsorCart(43-83)deleteSponsorCartForm(86-116)deleteSponsorCartForm(86-116)src/components/mui/table/mui-table.js (1)
handleDelete(76-93)src/reducers/sponsors/sponsor-page-cart-list-reducer.js (1)
cart(49-49)src/components/mui/search-input.js (1)
SearchInput(6-56)src/components/mui/table/extra-rows/TotalRow.jsx (1)
TotalRow(6-30)
src/actions/sponsor-cart-actions.js (2)
src/utils/methods.js (4)
getAccessTokenSafely(130-137)getAccessTokenSafely(130-137)escapeFilterValue(139-143)escapeFilterValue(139-143)src/utils/constants.js (2)
ERROR_CODE_404(124-124)ERROR_CODE_404(124-124)
src/utils/methods.js (1)
src/utils/constants.js (4)
CENTS_FOR_DOLLAR(227-227)CENTS_FOR_DOLLAR(227-227)DECIMAL_DIGITS(138-138)DECIMAL_DIGITS(138-138)
src/pages/sponsors/edit-sponsor-page.js (1)
src/pages/sponsors/sponsor-cart-tab/index.js (1)
SponsorCartTab(36-222)
src/components/mui/table/mui-table.js (2)
src/components/mui/editable-table/mui-table-editable.js (3)
customPerPageOptions(159-161)handleChangePage(145-147)handleChangeRowsPerPage(149-151)src/components/mui/sortable-table/mui-table-sortable.js (3)
customPerPageOptions(62-64)handleChangePage(48-50)handleChangeRowsPerPage(52-54)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 15-15: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (10)
src/store.js (2)
167-167: LGTM!The import follows the existing pattern and correctly wires the new sponsor cart reducer.
253-253: LGTM!The reducer is properly added to the persisted state configuration, maintaining consistency with the existing store structure.
src/i18n/en.json (2)
2437-2449: LGTM!The new cart_tab translation keys are well-structured and provide clear labels for the sponsor cart UI functionality.
3778-3779: LGTM!The addition of the "total" translation key properly supports the new TotalRow component rendering in the table.
src/components/mui/table/extra-rows/index.js (1)
1-2: LGTM!The barrel export pattern is correctly implemented, providing a clean public API for the extra-rows components.
src/utils/methods.js (1)
24-36: LGTM - Import additions.The new constant imports are properly organized and will support the monetary conversion functionality.
src/pages/sponsors/edit-sponsor-page.js (2)
45-45: LGTM - Clean import.The SponsorCartTab import follows the established pattern for other tab components.
189-191: LGTM - Proper tab integration.The new cart tab is correctly integrated with appropriate props (sponsor entity and summitId), consistent with the existing tab structure.
src/reducers/sponsors/sponsor-page-cart-list-reducer.js (1)
48-60: Verify monetary conversion after fixing CENTS_FOR_DOLLAR.The cart processing logic correctly maps form amounts and calculates the total. However, it depends on
centsToDollar, which uses the potentially incorrectCENTS_FOR_DOLLARconstant (see constants.js line 227).After verifying and fixing the constant value, ensure this conversion produces correct dollar amounts.
src/components/mui/table/mui-table.js (1)
271-294: The pagination conditional rendering is correct and working as designed. All existing usages provide bothperPageandcurrentPageprops, with one intentional exception:sponsor-cart-tab/index.js, which omits pagination to display all cart forms in a single non-paginated view. The conditional{perPage && currentPage && (enables this dual-mode behavior without breaking any existing functionality.
| params["filter[]"] = filter; | ||
| } | ||
|
|
||
| return getRequest( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque as stated on ticket
current cart call could return 404 is cart does not exists
how is this managed to meet the acceptance criteria ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is handled in customErrorHandler
| ))} | ||
| {/* Here we inject extra rows passed as children */} | ||
| {children} | ||
| {data.length === 0 && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque we need to review this logic
children are rendered without any check of data.length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I understand, you want to hide the total row if no data ?
| const [formEdit, setFormEdit] = useState(null); | ||
|
|
||
| useEffect(() => { | ||
| getSponsorCart(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque here the 404 is not managed we need to managed it and avoid to show the classic 404 swal popup as stated on acceptance criteria
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404 is handled separately and not showing the swal alert, like stated in acceptance criteria
| size="small" | ||
| onClick={() => handleManageItems(row)} | ||
| > | ||
| Manage Items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque please move this resource to i18n
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque please review comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/i18n/en.json:
- Around line 2437-2450: The cart_tab translation entries use inconsistent
lowercase for some values; update the values for keys "forms", "pay_cc", and
"pay_invoice" inside the cart_tab object to Title Case ("Forms", "Pay with
Credit Card or ACH", "Pay with Invoice") so they match the capitalization style
of the other entries like "New Form", "Code", and "Name".
In @src/pages/sponsors/sponsor-cart-tab/index.js:
- Around line 55-78: Remove the temporary console.log placeholders from the stub
handlers handleManageItems, handleLock, handlePayCreditCard, and
handlePayInvoice; either implement the actual flows (call the appropriate
actions/handlers such as opening a manage modal, toggling lock state, invoking
payment flow, or routing to invoice flow) or replace the logs with
no-op/placeholder logic behind a debug flag or a TODO comment so no sensitive
data is logged in production. Ensure setFormEdit and deleteSponsorCartForm
remain unchanged for edit/delete handlers.
- Around line 89-95: The render fallback currently returns the hardcoded string
"None"; change it to use the i18n helper instead (e.g., replace "None" with
T.translate("general.no") or a dedicated key like
T.translate("edit_sponsor.cart_tab.no_add_ons")) inside the column definition
where columnKey is "allowed_add_ons" and the render function inspects
row.allowed_add_ons so the UI uses localized text.
- Around line 47-53: The useEffect currently calls getSponsorCart but has an
empty dependency array, which causes a stale-closure/hooks-lint issue; fix by
either (A) adding getSponsorCart to the dependency array of the useEffect that
currently calls getSponsorCart, or (B) memoizing getSponsorCart with useCallback
(so useEffect can safely depend on it) and ensure handleSearch uses the same
memoized function, or (C) if the current behavior is intentional, add an
explicit eslint-disable-next-line comment above the useEffect with a brief
justification; update the useEffect line to either useEffect(() => {
getSponsorCart(); }, [getSponsorCart]); or wrap the getSponsorCart definition in
useCallback and reference that name in deps.
- Around line 147-149: The render uses cart?.forms.length which will throw if
cart.forms is null/undefined; change usages (in the sponsor cart tab where cart
and forms are referenced — e.g., the JSX block rendering Box and the expression
cart?.forms.length) to safely check forms as well (use cart?.forms?.length or
guard with cart && cart.forms && cart.forms.length) so the component won't crash
when forms is missing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/i18n/en.jsonsrc/pages/sponsors/sponsor-cart-tab/index.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
src/pages/sponsors/sponsor-cart-tab/index.js (2)
63-66: No changes needed.MuiTable'sonDeletecallback receivesitem.id(line 91 of mui-table.js), not the full row object. ThehandleDeletefunction correctly expects anitemIdparameter and passes it directly todeleteSponsorCartForm.
14-34: No changes needed—Grid2 import is correct for MUI v6.4.3.The named import
import { Grid2 } from "@mui/material"is the valid and documented approach in MUI v6. No build-break risk exists.Likely an incorrect or invalid review comment.
src/i18n/en.json (1)
3775-3781:mui_table.totaladdition is safe; no other locale files exist in this repository.This application uses English only (multi-language support is explicitly disabled in
src/app.js). Only one locale file exists (src/i18n/en.json), and no fallback verification is needed.Likely an incorrect or invalid review comment.
| "cart_tab": { | ||
| "new_form": "New Form", | ||
| "forms": "forms", | ||
| "code": "Code", | ||
| "name": "Name", | ||
| "add_ons": "Add-ons", | ||
| "discount": "Discount", | ||
| "amount": "Amount", | ||
| "manage_items": "Manage Items", | ||
| "add_form": "Add Form", | ||
| "no_cart": "No cart found.", | ||
| "pay_cc": "pay with credit card or ach", | ||
| "pay_invoice": "pay with invoice" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, explore the i18n directory structure
echo "=== Locale files in src/i18n ==="
ls -la src/i18n/ | grep -E "\.(json|js)$"
# Count total locale files
echo "=== Total locale files ==="
fd -e json src/i18n | wc -lRepository: fntechgit/summit-admin
Length of output: 542
🏁 Script executed:
#!/bin/bash
# Read the exact lines around the cart_tab section
echo "=== Content at lines 2437-2450 in en.json ==="
sed -n '2437,2450p' src/i18n/en.jsonRepository: fntechgit/summit-admin
Length of output: 508
🏁 Script executed:
#!/bin/bash
# Search for cart_tab in all locale files
echo "=== Files containing 'cart_tab' ==="
rg "cart_tab" src/i18n -l
# Show content of cart_tab in each file
echo -e "\n=== cart_tab definitions across locales ==="
rg -A 12 '"cart_tab"' src/i18nRepository: fntechgit/summit-admin
Length of output: 774
🏁 Script executed:
#!/bin/bash
# Search for JSON files that might be locale files
echo "=== All JSON files in repo ==="
fd -e json . | grep -iE "(locale|lang|i18n|translation)" | head -20
# Check if there's a locales directory elsewhere
echo -e "\n=== Searching for locale-like directories ==="
fd -type d -iE "(locale|lang|i18n|translation)" .
# Verify all json files in src/i18n
echo -e "\n=== All files in src/i18n ==="
fd . src/i18nRepository: fntechgit/summit-admin
Length of output: 396
🏁 Script executed:
#!/bin/bash
# Check git history to see if this is a new feature
echo "=== Recent changes to src/i18n/en.json ==="
git log --oneline -10 -- src/i18n/en.json
# Check if cart_tab was recently added
echo -e "\n=== Checking cart_tab presence in git ==="
git log -p --all -S "cart_tab" -- "*.json" | head -50Repository: fntechgit/summit-admin
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Check if the application uses i18n/internationalization
echo "=== Checking for i18n usage in code ==="
rg -t js -t ts "i18n|i18next|translation|locale" src --max-count=5
# Check package.json for i18n libraries
echo -e "\n=== I18n related dependencies ==="
rg "i18n|i18next|react-intl" package.jsonRepository: fntechgit/summit-admin
Length of output: 41358
🏁 Script executed:
#!/bin/bash
# Check if there's a build process or config that expects multiple locales
echo "=== Checking for locale config/setup ==="
rg "locale|lang" . --type json -t toml -t yaml -t js --max-count=10
# Look for any comments or documentation about locales
echo -e "\n=== README or docs mentioning locales ==="
fd README . && cat README.md | grep -iA 5 -B 5 "locale\|language\|i18n" || echo "No README found"Repository: fntechgit/summit-admin
Length of output: 50437
Normalize capitalization in cart_tab values for UI consistency.
The cart_tab strings have inconsistent capitalization: "forms", "pay with credit card or ach", and "pay with invoice" are lowercase while other values use Title Case ("New Form", "Code", "Name", etc.). Standardize to Title Case to match the rest of the object.
🤖 Prompt for AI Agents
In @src/i18n/en.json around lines 2437 - 2450, The cart_tab translation entries
use inconsistent lowercase for some values; update the values for keys "forms",
"pay_cc", and "pay_invoice" inside the cart_tab object to Title Case ("Forms",
"Pay with Credit Card or ACH", "Pay with Invoice") so they match the
capitalization style of the other entries like "New Form", "Code", and "Name".
| useEffect(() => { | ||
| getSponsorCart(); | ||
| }, []); | ||
|
|
||
| const handleSearch = (searchTerm) => { | ||
| getSponsorCart(searchTerm); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix useEffect deps to avoid stale closure / hooks-lint failures.
getSponsorCart is used inside an empty-deps effect; include it in the dependency array (or explicitly justify disabling lint) so behavior is stable.
Proposed diff
useEffect(() => {
getSponsorCart();
-}, []);
+}, [getSponsorCart]);📝 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.
| useEffect(() => { | |
| getSponsorCart(); | |
| }, []); | |
| const handleSearch = (searchTerm) => { | |
| getSponsorCart(searchTerm); | |
| }; | |
| useEffect(() => { | |
| getSponsorCart(); | |
| }, [getSponsorCart]); | |
| const handleSearch = (searchTerm) => { | |
| getSponsorCart(searchTerm); | |
| }; |
🤖 Prompt for AI Agents
In @src/pages/sponsors/sponsor-cart-tab/index.js around lines 47 - 53, The
useEffect currently calls getSponsorCart but has an empty dependency array,
which causes a stale-closure/hooks-lint issue; fix by either (A) adding
getSponsorCart to the dependency array of the useEffect that currently calls
getSponsorCart, or (B) memoizing getSponsorCart with useCallback (so useEffect
can safely depend on it) and ensure handleSearch uses the same memoized
function, or (C) if the current behavior is intentional, add an explicit
eslint-disable-next-line comment above the useEffect with a brief justification;
update the useEffect line to either useEffect(() => { getSponsorCart(); },
[getSponsorCart]); or wrap the getSponsorCart definition in useCallback and
reference that name in deps.
| const handleManageItems = (item) => { | ||
| console.log("MANAGE ITEMS : ", item); | ||
| }; | ||
|
|
||
| const handleEdit = (item) => { | ||
| setFormEdit(item); | ||
| }; | ||
|
|
||
| const handleDelete = (itemId) => { | ||
| deleteSponsorCartForm(itemId); | ||
| }; | ||
|
|
||
| const handleLock = (item) => { | ||
| console.log("LOCK : ", item); | ||
| }; | ||
|
|
||
| const handlePayCreditCard = () => { | ||
| console.log("PAY CREDIT CARD"); | ||
| }; | ||
|
|
||
| const handlePayInvoice = () => { | ||
| console.log("PAY INVOICE"); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove console.log placeholders (manage/lock/pay) before merge.
These handlers look like stubs; leaving console logs in production code is noisy and can leak data. Either implement the flows or remove logs / gate behind a debug flag.
🤖 Prompt for AI Agents
In @src/pages/sponsors/sponsor-cart-tab/index.js around lines 55 - 78, Remove
the temporary console.log placeholders from the stub handlers handleManageItems,
handleLock, handlePayCreditCard, and handlePayInvoice; either implement the
actual flows (call the appropriate actions/handlers such as opening a manage
modal, toggling lock state, invoking payment flow, or routing to invoice flow)
or replace the logs with no-op/placeholder logic behind a debug flag or a TODO
comment so no sensitive data is logged in production. Ensure setFormEdit and
deleteSponsorCartForm remain unchanged for edit/delete handlers.
| columnKey: "allowed_add_ons", | ||
| header: T.translate("edit_sponsor.cart_tab.add_ons"), | ||
| render: (row) => | ||
| row.allowed_add_ons?.length > 0 | ||
| ? row.allowed_add_ons.map((a) => `${a.type} ${a.name}`).join(", ") | ||
| : "None" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize the “None” fallback for add-ons.
Hardcoding "None" bypasses i18n; prefer an existing generic key (e.g., general.no) or a dedicated cart string.
🤖 Prompt for AI Agents
In @src/pages/sponsors/sponsor-cart-tab/index.js around lines 89 - 95, The
render fallback currently returns the hardcoded string "None"; change it to use
the i18n helper instead (e.g., replace "None" with T.translate("general.no") or
a dedicated key like T.translate("edit_sponsor.cart_tab.no_add_ons")) inside the
column definition where columnKey is "allowed_add_ons" and the render function
inspects row.allowed_add_ons so the UI uses localized text.
| {cart && ( | ||
| <Box component="span">{cart?.forms.length} forms in Cart</Box> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid potential crash: cart?.forms.length should be cart?.forms?.length.
If cart exists but forms is missing/null, cart?.forms.length will throw.
Proposed diff
{cart && (
- <Box component="span">{cart?.forms.length} forms in Cart</Box>
+ <Box component="span">
+ {(cart?.forms?.length ?? 0)} {T.translate("edit_sponsor.cart_tab.forms")} in Cart
+ </Box>
)}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/pages/sponsors/sponsor-cart-tab/index.js around lines 147 - 149, The
render uses cart?.forms.length which will throw if cart.forms is null/undefined;
change usages (in the sponsor cart tab where cart and forms are referenced —
e.g., the JSX block rendering Box and the expression cart?.forms.length) to
safely check forms as well (use cart?.forms?.length or guard with cart &&
cart.forms && cart.forms.length) so the component won't crash when forms is
missing.
.env.example
Outdated
| OS_BASE_URL= | ||
| SCOPES_BASE_REALM=${API_BASE_URL} | ||
| PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write | ||
| PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write my-cart/read my-cart/write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque my-cart/read my-cart/write are not necessary on this context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.env.example:
- Line 15: The PURCHASES_API_SCOPES env variable value is not quoted and
contains space-separated scopes; update the .env.example to wrap the
PURCHASES_API_SCOPES value in double quotes (e.g., PURCHASES_API_SCOPES="...")
so it matches other scope variables and parses correctly by dotenv-linter and
dotenv parsers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.example
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 15-15: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
| OS_BASE_URL= | ||
| SCOPES_BASE_REALM=${API_BASE_URL} | ||
| PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write | ||
| PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add quotes around the PURCHASES_API_SCOPES value for consistency and correct parsing.
The value contains space-separated scopes but lacks surrounding quotes, inconsistent with other scope variables in the file (lines 5, 16, 17, 18) and flagged by dotenv-linter. Space-separated values should be quoted to ensure proper dotenv parsing.
🔧 Proposed fix
-PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write
+PURCHASES_API_SCOPES="purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write"📝 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.
| PURCHASES_API_SCOPES=purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write | |
| PURCHASES_API_SCOPES="purchases-show-medata/read purchases-show-medata/write show-form/read show-form/write customized-form/write customized-form/read carts/read carts/write" |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 15-15: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
In @.env.example at line 15, The PURCHASES_API_SCOPES env variable value is not
quoted and contains space-separated scopes; update the .env.example to wrap the
PURCHASES_API_SCOPES value in double quotes (e.g., PURCHASES_API_SCOPES="...")
so it matches other scope variables and parses correctly by dotenv-linter and
dotenv parsers.
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ref: https://app.clickup.com/t/86b66n5ke
Summary by CodeRabbit
New Features
UI Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.