fix: add popup to manual scan badge on sponsor badge scans tab#835
Open
fix: add popup to manual scan badge on sponsor badge scans tab#835
Conversation
📝 WalkthroughWalkthroughAdds sponsor badge-scan functionality: new MUI QR scan dialog, Redux action/thunk to create scans, page wiring to open/save scans, i18n strings, utility tweak for checkbox parsing, and an added badge-scans write scope in Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Page as SponsorBadgeScans<br/>Page
participant Popup as MuiQrBadgePopup<br/>Component
participant Redux as Redux Store<br/>(sponsor-actions)
participant API as Badge Scans API<br/>/badge-scans
User->>Page: Click "New Manual Scan"
Page->>Page: set showManualBadgeScanPopup = true
Page->>Popup: render with extraQuestions & isAdmin
User->>Popup: Scan QR code
Popup->>Popup: set scannedCode, show success state
User->>Popup: Fill form, click Save
Popup->>Redux: dispatch addBadgeScan(entity)
Redux->>Redux: startLoading()
Redux->>API: POST /badge-scans (entity)
API-->>Redux: 200 OK
Redux->>Redux: snackbarSuccessHandler, stopLoading()
Redux-->>Page: resolve addBadgeScan
Page->>Page: getBadgeScans(sponsor.id), close popup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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📝 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: 3
🧹 Nitpick comments (1)
src/components/mui/mui-qr-badge-popup.js (1)
151-154: Avoid mutatingextraQuestionsduring render.
sort(...)mutates the same array instance received from the parent, which is risky when that data comes from Redux-backed props. Clone it before sorting.💡 Suggested fix
<ExtraQuestionsMUI - extraQuestions={extraQuestions.sort( + extraQuestions={[...extraQuestions].sort( (a, b) => a.order - b.order )} formik={formik} allowEdit />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/mui-qr-badge-popup.js` around lines 151 - 154, The render currently calls extraQuestions.sort(...) which mutates the prop array; clone extraQuestions before sorting to avoid mutating parent/Redux state (e.g., use extraQuestions.slice().sort(...) or [...extraQuestions].sort(...)) and pass that cloned, sorted array into ExtraQuestionsMUI; also guard for undefined/null extraQuestions before cloning if needed.
🤖 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/actions/sponsor-actions.js`:
- Around line 1476-1502: In addBadgeScan, read the active sponsor from the Redux
state (e.g., from currentSponsorState.activeSponsor or selectedSponsor returned
by getState()) and inject it into the payload before calling postRequest (for
example add sponsor_id: activeSponsor.id or sponsor: activeSponsor as required
by the API), then pass the new payload (not the original entity) into
postRequest; also handle the case where no active sponsor is present (no-op or
early return/error) to avoid sending an un-scoped summit-level badge scan.
In `@src/components/mui/mui-qr-badge-popup.js`:
- Around line 50-70: The onSubmit handler in useFormik currently calls
onScan(...) without preventing duplicate submits; wrap the create call in a
submitting guard by setting formik.setSubmitting(true) at the start of onSubmit,
await the onScan(...) call (assume/ensure onScan returns a Promise), and then
call formik.setSubmitting(false) in a finally block; also ensure the Save button
is disabled using formik.isSubmitting so the UI cannot be clicked while the
create is in flight. Target the useFormik onSubmit function and the Save button
binding to formik.isSubmitting (and adjust onScan to return a Promise if it
doesn’t already).
In `@src/pages/sponsors/sponsor-badge-scans/index.js`:
- Around line 111-114: handleManualScanSubmit currently calls
addBadgeScan(...).then(...).finally(...) which always closes the MuiQrBadgePopup
even on failure; change it to return the promise chain and only close the popup
in the success path: call addBadgeScan(entity).then(() =>
getBadgeScans(sponsor.id)).then(() => setShowManualBadgeScanPopup(false)) and
return that promise so callers can await/rely on errors (do not use .finally to
close). Ensure any errors are allowed to propagate so the popup can keep form
state for retry.
---
Nitpick comments:
In `@src/components/mui/mui-qr-badge-popup.js`:
- Around line 151-154: The render currently calls extraQuestions.sort(...) which
mutates the prop array; clone extraQuestions before sorting to avoid mutating
parent/Redux state (e.g., use extraQuestions.slice().sort(...) or
[...extraQuestions].sort(...)) and pass that cloned, sorted array into
ExtraQuestionsMUI; also guard for undefined/null extraQuestions before cloning
if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65b591a0-e4eb-49f5-b47e-0f3f27748335
📒 Files selected for processing (5)
.env.examplesrc/actions/sponsor-actions.jssrc/components/mui/mui-qr-badge-popup.jssrc/i18n/en.jsonsrc/pages/sponsors/sponsor-badge-scans/index.js
Comment on lines
+1476
to
+1502
| export const addBadgeScan = (entity) => async (dispatch, getState) => { | ||
| const { currentSummitState } = getState(); | ||
| const accessToken = await getAccessTokenSafely(); | ||
| const { currentSummit } = currentSummitState; | ||
|
|
||
| dispatch(startLoading()); | ||
|
|
||
| const params = { | ||
| access_token: accessToken | ||
| }; | ||
|
|
||
| return postRequest( | ||
| null, | ||
| createAction(BADGE_SCAN_ADDED), | ||
| `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`, | ||
| entity, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); | ||
| dispatch( | ||
| snackbarSuccessHandler({ | ||
| title: T.translate("general.success"), | ||
| html: T.translate("edit_badge_scan.badge_scan_saved") | ||
| }) | ||
| ); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Inject the active sponsor before posting a new badge scan.
src/components/mui/mui-qr-badge-popup.js (Lines 64-68) submits only qr_code, scan_date, notes, and extra_questions, and this thunk forwards that payload unchanged to the summit-level /badge-scans endpoint. Because the sponsor is not part of the URL, admin-created scans are not explicitly tied to the sponsor whose badge-scans tab is open.
💡 Suggested fix
export const addBadgeScan = (entity) => async (dispatch, getState) => {
- const { currentSummitState } = getState();
+ const { currentSummitState, currentSponsorState } = getState();
const accessToken = await getAccessTokenSafely();
const { currentSummit } = currentSummitState;
+ const sponsorId = currentSponsorState?.entity?.id;
dispatch(startLoading());
const params = {
access_token: accessToken
};
return postRequest(
null,
createAction(BADGE_SCAN_ADDED),
`${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`,
- entity,
+ { ...entity, sponsor_id: sponsorId },
snackbarErrorHandler
)(params)(dispatch).then(() => {
dispatch(stopLoading());
dispatch(
snackbarSuccessHandler({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/sponsor-actions.js` around lines 1476 - 1502, In addBadgeScan,
read the active sponsor from the Redux state (e.g., from
currentSponsorState.activeSponsor or selectedSponsor returned by getState()) and
inject it into the payload before calling postRequest (for example add
sponsor_id: activeSponsor.id or sponsor: activeSponsor as required by the API),
then pass the new payload (not the original entity) into postRequest; also
handle the case where no active sponsor is present (no-op or early return/error)
to avoid sending an un-scoped summit-level badge scan.
Comment on lines
+111
to
+114
| const handleManualScanSubmit = (entity) => { | ||
| addBadgeScan(entity) | ||
| .then(() => getBadgeScans(sponsor.id)) | ||
| .finally(() => setShowManualBadgeScanPopup(false)); |
There was a problem hiding this comment.
Only close the popup after a successful create, and return the request promise.
If the POST fails, this .finally(...) still unmounts MuiQrBadgePopup and drops the scanned QR plus any notes/extra-question answers. Returning the promise and closing only on success preserves the form for retry and lets the popup manage its submitting state.
💡 Suggested fix
const handleManualScanSubmit = (entity) => {
- addBadgeScan(entity)
- .then(() => getBadgeScans(sponsor.id))
- .finally(() => setShowManualBadgeScanPopup(false));
+ return addBadgeScan(entity).then(() => {
+ setShowManualBadgeScanPopup(false);
+ return getBadgeScans(
+ sponsor.id,
+ term,
+ currentPage,
+ perPage,
+ order,
+ orderDir
+ );
+ });
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-badge-scans/index.js` around lines 111 - 114,
handleManualScanSubmit currently calls addBadgeScan(...).then(...).finally(...)
which always closes the MuiQrBadgePopup even on failure; change it to return the
promise chain and only close the popup in the success path: call
addBadgeScan(entity).then(() => getBadgeScans(sponsor.id)).then(() =>
setShowManualBadgeScanPopup(false)) and return that promise so callers can
await/rely on errors (do not use .finally to close). Ensure any errors are
allowed to propagate so the popup can keep form state for retry.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
smarcet
reviewed
Mar 26, 2026
| })) | ||
| .filter((q) => q.answer); | ||
|
|
||
| onScan({ |
There was a problem hiding this comment.
@tomrndom please review this could lead to a doble click ( button is not disabled ) we need to prevent double submit
smarcet
reviewed
Mar 26, 2026
src/actions/sponsor-actions.js
Outdated
| entity, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); |
There was a problem hiding this comment.
@tomrndom move dispatch(stopLoading()); to finally
smarcet
requested changes
Mar 26, 2026
tomrndom
force-pushed
the
fix/sponsor-badge-scan-manual
branch
from
5c39eb7 to
ca1efc3
Compare
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
…checkboxlist answers Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
fix/sponsor-badge-scan-manual
branch
from
ca1efc3 to
0d06380
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/actions/sponsor-actions.js (1)
1483-1509:⚠️ Potential issue | 🟠 MajorInject the active sponsor before posting the new badge scan.
This still sends the raw
entityto the summit-level/badge-scansendpoint, so the created scan is not explicitly tied to the sponsor whose tab is open. Insrc/pages/sponsors/sponsor-badge-scans/index.js, Lines 111-115 immediately refresh the list withgetBadgeScans(sponsor.id), so an unscoped create can disappear from the current sponsor view right after submit.💡 Suggested fix
export const addBadgeScan = (entity) => async (dispatch, getState) => { - const { currentSummitState } = getState(); + const { currentSummitState, currentSponsorState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; + const sponsorId = currentSponsorState?.entity?.id; + + if (!sponsorId) { + return Promise.reject(new Error("Missing sponsor context")); + } dispatch(startLoading()); const params = { access_token: accessToken }; + + const normalizedEntity = normalizeBadgeScan({ + ...entity, + sponsor_id: sponsorId + }); return postRequest( null, createAction(BADGE_SCAN_ADDED), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`, - entity, + normalizedEntity, snackbarErrorHandler )(params)(dispatch)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-actions.js` around lines 1483 - 1509, In addBadgeScan, ensure the posted payload includes the active sponsor so the created scan is scoped to the open sponsor tab: read the active sponsor from state (e.g., currentSponsorState.activeSponsor or selectedSponsor), clone the incoming entity and inject the sponsor identifier (e.g., set sponsor_id = sponsor.id or sponsor = { id: sponsor.id } per API) before calling postRequest to `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`; do this prior to invoking postRequest/createAction(BADGE_SCAN_ADDED) and avoid mutating the original entity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/actions/sponsor-actions.js`:
- Around line 1483-1509: In addBadgeScan, ensure the posted payload includes the
active sponsor so the created scan is scoped to the open sponsor tab: read the
active sponsor from state (e.g., currentSponsorState.activeSponsor or
selectedSponsor), clone the incoming entity and inject the sponsor identifier
(e.g., set sponsor_id = sponsor.id or sponsor = { id: sponsor.id } per API)
before calling postRequest to
`${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`; do this
prior to invoking postRequest/createAction(BADGE_SCAN_ADDED) and avoid mutating
the original entity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9d3fa66-0d47-4666-b7d8-787cd50b0bb7
📒 Files selected for processing (4)
src/actions/sponsor-actions.jssrc/components/mui/mui-qr-badge-popup.jssrc/pages/sponsors/sponsor-badge-scans/index.jssrc/utils/extra-questions.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/sponsors/sponsor-badge-scans/index.js
- src/components/mui/mui-qr-badge-popup.js
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/actions/sponsor-actions.js (1)
1483-1510:⚠️ Potential issue | 🟠 MajorInject sponsor ID into the badge scan payload.
The
addBadgeScanthunk posts to the summit-level/badge-scansendpoint without including asponsor_id. This means scans created via the manual QR popup won't be associated with the sponsor whose badge-scans tab is open.🔧 Suggested fix
export const addBadgeScan = (entity) => async (dispatch, getState) => { - const { currentSummitState } = getState(); + const { currentSummitState, currentSponsorState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; + const sponsorId = currentSponsorState?.entity?.id; + + if (!sponsorId) { + dispatch(stopLoading()); + return; + } dispatch(startLoading()); const params = { access_token: accessToken }; return postRequest( null, createAction(BADGE_SCAN_ADDED), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/badge-scans`, - entity, + { ...entity, sponsor_id: sponsorId }, snackbarErrorHandler )(params)(dispatch)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-actions.js` around lines 1483 - 1510, The addBadgeScan thunk posts the provided entity to the summit-level badge-scans endpoint but never attaches the sponsor identifier, so scans won't be tied to the sponsor currently open; modify addBadgeScan to read the current sponsor from getState (e.g. const { currentSponsor } = getState().sponsorState) and inject sponsor_id into the payload (either by mutating or building a new payload like {...entity, sponsor_id: currentSponsor.id}) before calling postRequest (the call that creates BADGE_SCAN_ADDED), then continue to dispatch success/finally as before.
🤖 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/components/mui/mui-qr-badge-popup.js`:
- Around line 152-154: The code is mutating the extraQuestions prop by calling
extraQuestions.sort(...); instead, create a shallow copy before sorting (e.g.
[...extraQuestions].sort(...) or extraQuestions.slice().sort(...)) where the
prop is passed (the JSX prop assignment for extraQuestions) so the original prop
is not mutated and parent state is preserved.
---
Duplicate comments:
In `@src/actions/sponsor-actions.js`:
- Around line 1483-1510: The addBadgeScan thunk posts the provided entity to the
summit-level badge-scans endpoint but never attaches the sponsor identifier, so
scans won't be tied to the sponsor currently open; modify addBadgeScan to read
the current sponsor from getState (e.g. const { currentSponsor } =
getState().sponsorState) and inject sponsor_id into the payload (either by
mutating or building a new payload like {...entity, sponsor_id:
currentSponsor.id}) before calling postRequest (the call that creates
BADGE_SCAN_ADDED), then continue to dispatch success/finally as before.
🪄 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: 0911b59e-ec9b-4ef5-980d-cc83a03c7ca9
📒 Files selected for processing (6)
.env.examplesrc/actions/sponsor-actions.jssrc/components/mui/mui-qr-badge-popup.jssrc/i18n/en.jsonsrc/pages/sponsors/sponsor-badge-scans/index.jssrc/utils/extra-questions.js
✅ Files skipped from review due to trivial changes (2)
- src/i18n/en.json
- src/utils/extra-questions.js
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.example
- src/pages/sponsors/sponsor-badge-scans/index.js
Comment on lines
+152
to
+154
| extraQuestions={extraQuestions.sort( | ||
| (a, b) => a.order - b.order | ||
| )} |
There was a problem hiding this comment.
Avoid mutating the extraQuestions prop with .sort().
Array.prototype.sort() mutates the array in place. Since extraQuestions is a prop, mutating it can cause unexpected behavior in parent components or trigger unnecessary re-renders.
🔧 Suggested fix
<ExtraQuestionsMUI
- extraQuestions={extraQuestions.sort(
+ extraQuestions={[...extraQuestions].sort(
(a, b) => a.order - b.order
)}📝 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
| extraQuestions={extraQuestions.sort( | |
| (a, b) => a.order - b.order | |
| )} | |
| extraQuestions={[...extraQuestions].sort( | |
| (a, b) => a.order - b.order | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/mui-qr-badge-popup.js` around lines 152 - 154, The code is
mutating the extraQuestions prop by calling extraQuestions.sort(...); instead,
create a shallow copy before sorting (e.g. [...extraQuestions].sort(...) or
extraQuestions.slice().sort(...)) where the prop is passed (the JSX prop
assignment for extraQuestions) so the original prop is not mutated and parent
state is preserved.
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.
ref: https://app.clickup.com/t/86b7tdv03
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit