Feature: sponsor cart form lock & delete #753
Conversation
📝 WalkthroughWalkthroughThis PR introduces lock/unlock functionality for sponsor cart forms by adding new action types, API endpoints (PUT for lock, DELETE for unlock), and reducer logic to manage the locked state of forms with proper loading and error handling. Changes
Sequence DiagramsequenceDiagram
participant User
participant Component as SponsorCartTab
participant Action as lockSponsorCartForm/<br/>unlockSponsorCartForm
participant API as API Server
participant Store as Redux Store
participant Reducer as sponsor-page-cart-<br/>list-reducer
User->>Component: Click lock/unlock button
Component->>Action: Dispatch action with formId
Action->>API: PUT/DELETE request to lock/unlock form
API-->>Action: Response
Action->>Store: Dispatch SPONSOR_CART_FORM_LOCKED<br/>(with locked: true/false)
Store->>Reducer: SPONSOR_CART_FORM_LOCKED action
Reducer->>Reducer: Update target form's<br/>locked flag
Reducer-->>Store: Updated state
Store-->>Component: Re-render with new state
Component-->>User: Display locked/unlocked form
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In @src/actions/sponsor-cart-actions.js:
- Around line 119-143: lockSponsorCartForm currently dispatches the putRequest
promise but does not return it, preventing callers from awaiting or chaining;
update lockSponsorCartForm to return the putRequest(...) invocation (the same
expression ending with (params)(dispatch)) so the function returns the promise
chain and retains the existing .catch/.finally behavior; locate the
lockSponsorCartForm export and add the return in front of the putRequest call.
In @src/reducers/sponsors/sponsor-page-cart-list-reducer.js:
- Around line 74-91: The reducer branch for SPONSOR_CART_FORM_LOCKED is
inconsistent with other branches and the UI: update the mapping in the
SPONSOR_CART_FORM_LOCKED case to compare against form.form_id (not form.id) and
set the lock flag on the form object as is_locked instead of locked; e.g., in
the forms = state.cart.forms.map callback for SPONSOR_CART_FORM_LOCKED return
{...form, is_locked: locked} when form.form_id === formId so the UI reads the
updated property, keeping SPONSOR_CART_FORM_DELETED and other cases using
form.form_id for ID comparisons.
🧹 Nitpick comments (2)
src/actions/sponsor-cart-actions.js (2)
138-142: Improve error handling for better debugging and consistency.Using
.catch(console.log)swallows errors silently without user feedback. SincesnackbarErrorHandleris already configured, the catch block may be redundant or could be improved.Suggested improvement
)(params)(dispatch) - .catch(console.log) // need to catch promise reject + .catch((err) => { + console.error("Lock sponsor cart form failed:", err); + }) .finally(() => { dispatch(stopLoading()); });
105-112: Consider awaitinggetSponsorCartbefore showing success message.The success snackbar displays before the cart refresh completes. Consider awaiting to ensure the UI updates before confirming success.
Suggested improvement
.then(() => { - getSponsorCart()(dispatch, getState); + return getSponsorCart()(dispatch, getState); + }) + .then(() => { dispatch( snackbarSuccessHandler({ title: T.translate("general.success"), html: T.translate("sponsor_forms.form_delete_success") }) ); })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/actions/sponsor-cart-actions.jssrc/pages/sponsors/sponsor-cart-tab/index.jssrc/reducers/sponsors/sponsor-page-cart-list-reducer.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/reducers/sponsors/sponsor-page-cart-list-reducer.js (2)
src/actions/sponsor-cart-actions.js (2)
SPONSOR_CART_FORM_LOCKED(32-32)SPONSOR_CART_FORM_LOCKED(32-32)src/reducers/sponsors/sponsor-forms-list-reducer.js (1)
form(120-120)
src/actions/sponsor-cart-actions.js (2)
src/utils/methods.js (2)
getAccessTokenSafely(128-135)getAccessTokenSafely(128-135)src/components/forms/sponsor-general-form/sponsorship.js (1)
sponsor(38-45)
⏰ 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 (4)
src/actions/sponsor-cart-actions.js (1)
135-135: Verify the API base URL difference is intentional.
deleteSponsorCartFormusesPURCHASES_API_URLwhilelockSponsorCartFormandunlockSponsorCartFormuseSPONSOR_USERS_API_URL. Ensure this is the intended API architecture.Also applies to: 157-157
src/pages/sponsors/sponsor-cart-tab/index.js (3)
71-77: Lock toggle logic is correctly implemented.The toggle logic properly checks
is_lockedand calls the appropriate action. However, this depends on the reducer fix mentioned earlier—the reducer currently setslockedinstead ofis_locked, which will prevent the UI from updating after lock/unlock operations.
128-140: LGTM!The lock column rendering correctly toggles between
LockClosedIconandLockOpenIconbased on theis_lockedstate, and the click handler properly passes the full row object tohandleLock.
236-241: LGTM!The new
lockSponsorCartFormandunlockSponsorCartFormactions are correctly wired in theconnectcall.
| export const lockSponsorCartForm = (formId) => async (dispatch, getState) => { | ||
| const { currentSummitState, currentSponsorState } = getState(); | ||
| const { currentSummit } = currentSummitState; | ||
| const { entity: sponsor } = currentSponsorState; | ||
|
|
||
| const accessToken = await getAccessTokenSafely(); | ||
|
|
||
| const params = { | ||
| access_token: accessToken | ||
| }; | ||
|
|
||
| dispatch(startLoading()); | ||
|
|
||
| putRequest( | ||
| null, | ||
| createAction(SPONSOR_CART_FORM_LOCKED)({ formId, locked: true }), | ||
| `${window.SPONSOR_USERS_API_URL}/api/v1/shows/${currentSummit.id}/sponsors/${sponsor.id}/carts/current/forms/${formId}/lock`, | ||
| {}, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch) | ||
| .catch(console.log) // need to catch promise reject | ||
| .finally(() => { | ||
| dispatch(stopLoading()); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Missing return statement for the promise.
lockSponsorCartForm does not return the promise, unlike deleteSponsorCartForm and unlockSponsorCartForm. This prevents callers from chaining or awaiting the operation.
Proposed fix
dispatch(startLoading());
- putRequest(
+ return putRequest(
null,
createAction(SPONSOR_CART_FORM_LOCKED)({ formId, locked: true }),🤖 Prompt for AI Agents
In @src/actions/sponsor-cart-actions.js around lines 119 - 143,
lockSponsorCartForm currently dispatches the putRequest promise but does not
return it, preventing callers from awaiting or chaining; update
lockSponsorCartForm to return the putRequest(...) invocation (the same
expression ending with (params)(dispatch)) so the function returns the promise
chain and retains the existing .catch/.finally behavior; locate the
lockSponsorCartForm export and add the return in front of the putRequest call.
| case SPONSOR_CART_FORM_LOCKED: { | ||
| const { formId, locked } = payload; | ||
|
|
||
| const forms = state.cart.forms.map((form) => { | ||
| if (form.form_id === formId) { | ||
| return {...form, locked}; | ||
| } | ||
| return form; | ||
| }); | ||
|
|
||
| return { | ||
| ...state, | ||
| cart: { | ||
| ...state.cart, | ||
| forms | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how form objects are structured and which ID property is used
rg -n "form\.id|form\.form_id|form_id|is_locked" --type=js -C2 src/Repository: fntechgit/summit-admin
Length of output: 5263
🏁 Script executed:
#!/bin/bash
# Get the exact code for both cart form cases to confirm the issue
sed -n '62,91p' src/reducers/sponsors/sponsor-page-cart-list-reducer.jsRepository: fntechgit/summit-admin
Length of output: 693
Fix critical property name inconsistencies causing UI state mismatch.
Two critical issues require correction:
-
ID property inconsistency:
SPONSOR_CART_FORM_DELETED(line 64) usesform.idwhileSPONSOR_CART_FORM_LOCKED(line 78) usesform.form_id. Since the UI dispatches actions withform.form_id, both cases should useform.form_idfor consistency. -
Lock state property mismatch: The reducer sets
lockedbut the UI component readsis_locked(lines 72, 133). The lock state changes will not be reflected in the UI until this is corrected tois_locked.
Proposed fix
case SPONSOR_CART_FORM_DELETED: {
const { formId } = payload;
- const forms = state.cart.forms.filter((form) => form.id !== formId);
+ const forms = state.cart.forms.filter((form) => form.form_id !== formId);
return {
...state,
cart: {
...state.cart,
forms
}
};
}
case SPONSOR_CART_FORM_LOCKED: {
const { formId, locked } = payload;
const forms = state.cart.forms.map((form) => {
if (form.form_id === formId) {
- return {...form, locked};
+ return {...form, is_locked: locked};
}
return form;
});🤖 Prompt for AI Agents
In @src/reducers/sponsors/sponsor-page-cart-list-reducer.js around lines 74 -
91, The reducer branch for SPONSOR_CART_FORM_LOCKED is inconsistent with other
branches and the UI: update the mapping in the SPONSOR_CART_FORM_LOCKED case to
compare against form.form_id (not form.id) and set the lock flag on the form
object as is_locked instead of locked; e.g., in the forms = state.cart.forms.map
callback for SPONSOR_CART_FORM_LOCKED return {...form, is_locked: locked} when
form.form_id === formId so the UI reads the updated property, keeping
SPONSOR_CART_FORM_DELETED and other cases using form.form_id for ID comparisons.
https://app.clickup.com/t/86b82f0x3
https://app.clickup.com/t/86b82f1bz
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.