feat: prevent ticket reassignment based on promo code#139
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/utils/utils.js (1)
146-146:isSentryInitializedis now dead code.This function is no longer called anywhere since
handleSentryExceptionwas changed to only log to console. Either remove it along with the Sentry changes, or keep it if you plan to restore Sentry functionality soon.🧹 Proposed cleanup if Sentry won't be restored soon
-const isSentryInitialized = () => !!window.SENTRY_DSN; - // TODO: Fix Sentry dependency issue export const handleSentryException = (err) => console.log("Error on registration: ", err);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/utils.js` at line 146, The isSentryInitialized helper is dead code since handleSentryException no longer uses Sentry; remove the unused function isSentryInitialized from utils.js and any related imports/usages, or if you intend to restore Sentry telemetry, rewire handleSentryException to check isSentryInitialized (and call Sentry.captureException) instead of only logging to console; refer to the isSentryInitialized function name and handleSentryException to locate the change points and make the corresponding cleanup or restoration.src/components/registration-lite.js (1)
287-310: Review the error handling condition.The error handler only calls
onErrorwhene?.res?.body?.errorsexists. If the API returns a different error structure (e.g., network error, timeout, or error withoutbody.errors), the UI won't display feedback to the user—only Sentry logging occurs.Consider whether all error scenarios should surface to the user:
♻️ Proposed fix to handle broader error cases
.catch((e) => { - if (e?.res?.body?.errors) { - onError(e, e.res); - } + if (e?.res?.body?.errors) { + onError(e, e.res); + } else if (e?.res) { + // Handle errors with different structure + const fallbackErrors = [e?.res?.body?.message || 'An error occurred validating the promo code.']; + onError(e, { body: { errors: fallbackErrors } }); + } handleSentryException(e); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/registration-lite.js` around lines 287 - 310, handleValidatePromocode currently only calls onError when e?.res?.body?.errors exists, so other failures (network, timeout, different error shapes) never surface to the UI; update the catch in handleValidatePromocode to always call onError with a user-friendly error payload for any rejected promise from validatePromoCode (pass the original error as first arg and a normalized response object as second), preserving the existing branch that uses e.res when available (i.e., prefer e.res for detailed errors but fall back to a default { body: { errors: [<friendly message or e.message>] } }); keep calling handleSentryException(e) and ensure stopWidgetLoading() still runs in finally.src/components/ticket-type/index.js (1)
102-105: Consider adding minimal error logging in the catch block.The empty
.catch(() => {})silently swallows errors. WhilevalidatePromoCodehandles the Redux state update viaVALIDATE_PROMO_CODE_ERROR, having no logging here can make debugging difficult if issues arise.♻️ Optional: Add console logging for debugging
// Validate promo code if already applied (ticket selected after promo applied) if (promoCode) { - validatePromoCode({ id: t.id, ticketQuantity: minQuantity, sub_type: t.sub_type }).catch(() => {}); + validatePromoCode({ id: t.id, ticketQuantity: minQuantity, sub_type: t.sub_type }).catch((e) => { + console.log('Promo code validation failed for ticket change:', e); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ticket-type/index.js` around lines 102 - 105, The empty catch on the validatePromoCode call swallows errors; update the promise error handler so it logs a minimal, contextual message and the error (for example include promoCode, ticket id t.id, minQuantity and t.sub_type) instead of doing nothing — locate the validatePromoCode invocation in the ticket-type component and replace .catch(() => {}) with a catch that logs a short message plus the caught error to console or the existing logger.src/components/personal-information/index.js (1)
54-65: Consider using constants oruseMemoinstead of functions for computed values.
isSingleTicketOrderandshouldDisplayTicketAssignmentare defined as functions but don't take any parameters - they read from component scope. This is inconsistent withticketTypeAllowsReassign,canReassign, andisNonTransferablewhich are computed as constants.♻️ Proposed refactor for consistency
- // if there's only one ticket on the order and there is no invitation available, handle ticket assignment - const isSingleTicketOrder = () => formValues.ticketQuantity === 1 && !invitation && !isPrePaidTicketType(formValues.ticketType); - - // check if reassignment is allowed by both promo code AND ticket type - const ticketTypeAllowsReassign = formValues.ticketType?.allows_to_reassign !== false; - const canReassign = promoCodeAllowsReassign && ticketTypeAllowsReassign; - - // show radio options only if reassignment is allowed by both sources - const shouldDisplayTicketAssignment = () => isSingleTicketOrder() && canReassign; - - // show notice when ticket is non-transferable (either promo code or ticket type disallows) - const isNonTransferable = isSingleTicketOrder() && !canReassign; + // if there's only one ticket on the order and there is no invitation available, handle ticket assignment + const isSingleTicketOrder = formValues.ticketQuantity === 1 && !invitation && !isPrePaidTicketType(formValues.ticketType); + + // check if reassignment is allowed by both promo code AND ticket type + const ticketTypeAllowsReassign = formValues.ticketType?.allows_to_reassign !== false; + const canReassign = promoCodeAllowsReassign && ticketTypeAllowsReassign; + + // show radio options only if reassignment is allowed by both sources + const shouldDisplayTicketAssignment = isSingleTicketOrder && canReassign; + + // show notice when ticket is non-transferable (either promo code or ticket type disallows) + const isNonTransferable = isSingleTicketOrder && !canReassign;Then update usages from
isSingleTicketOrder()andshouldDisplayTicketAssignment()toisSingleTicketOrderandshouldDisplayTicketAssignment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/personal-information/index.js` around lines 54 - 65, Replace the zero-arg functions isSingleTicketOrder and shouldDisplayTicketAssignment with computed constants (or wrap them in useMemo if you want memoization) that read from formValues, invitation, promoCodeAllowsReassign and isPrePaidTicketType (i.e., compute const isSingleTicketOrder = formValues.ticketQuantity === 1 && !invitation && !isPrePaidTicketType(formValues.ticketType) and const shouldDisplayTicketAssignment = isSingleTicketOrder && canReassign), keep ticketTypeAllowsReassign, canReassign and isNonTransferable as-is, and update all usages to refer to these identifiers without calling them as functions (remove the trailing () where used).src/reducer.js (1)
169-172: Consider resettingpromoCodeAllowsReassignwhen setting a new promo code.When
SET_CURRENT_PROMO_CODEis dispatched (e.g., user applies a different promo code), thepromoCodeAllowsReassignvalue from the previous promo code persists until validation completes. This could cause a brief UI inconsistency where the reassignment state reflects the old promo code.♻️ Proposed fix to reset the flag when applying a new code
case SET_CURRENT_PROMO_CODE:{ const { currentPromoCode } = payload; - return { ...state, promoCode: currentPromoCode } + return { ...state, promoCode: currentPromoCode, promoCodeAllowsReassign: true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/reducer.js` around lines 169 - 172, In the reducer case handling SET_CURRENT_PROMO_CODE (in src/reducer.js) you currently set promoCode from payload.currentPromoCode but leave promoCodeAllowsReassign from the previous state; update that case to also reset promoCodeAllowsReassign (e.g., to false or null/undefined) when applying a new promo code so the old flag doesn’t briefly persist in the UI; ensure you modify the object returned in the SET_CURRENT_PROMO_CODE branch (the case block that references currentPromoCode) to include promoCodeAllowsReassign: <desired-reset-value>.
🤖 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/components/personal-information/index.js`:
- Around line 54-65: Replace the zero-arg functions isSingleTicketOrder and
shouldDisplayTicketAssignment with computed constants (or wrap them in useMemo
if you want memoization) that read from formValues, invitation,
promoCodeAllowsReassign and isPrePaidTicketType (i.e., compute const
isSingleTicketOrder = formValues.ticketQuantity === 1 && !invitation &&
!isPrePaidTicketType(formValues.ticketType) and const
shouldDisplayTicketAssignment = isSingleTicketOrder && canReassign), keep
ticketTypeAllowsReassign, canReassign and isNonTransferable as-is, and update
all usages to refer to these identifiers without calling them as functions
(remove the trailing () where used).
In `@src/components/registration-lite.js`:
- Around line 287-310: handleValidatePromocode currently only calls onError when
e?.res?.body?.errors exists, so other failures (network, timeout, different
error shapes) never surface to the UI; update the catch in
handleValidatePromocode to always call onError with a user-friendly error
payload for any rejected promise from validatePromoCode (pass the original error
as first arg and a normalized response object as second), preserving the
existing branch that uses e.res when available (i.e., prefer e.res for detailed
errors but fall back to a default { body: { errors: [<friendly message or
e.message>] } }); keep calling handleSentryException(e) and ensure
stopWidgetLoading() still runs in finally.
In `@src/components/ticket-type/index.js`:
- Around line 102-105: The empty catch on the validatePromoCode call swallows
errors; update the promise error handler so it logs a minimal, contextual
message and the error (for example include promoCode, ticket id t.id,
minQuantity and t.sub_type) instead of doing nothing — locate the
validatePromoCode invocation in the ticket-type component and replace .catch(()
=> {}) with a catch that logs a short message plus the caught error to console
or the existing logger.
In `@src/reducer.js`:
- Around line 169-172: In the reducer case handling SET_CURRENT_PROMO_CODE (in
src/reducer.js) you currently set promoCode from payload.currentPromoCode but
leave promoCodeAllowsReassign from the previous state; update that case to also
reset promoCodeAllowsReassign (e.g., to false or null/undefined) when applying a
new promo code so the old flag doesn’t briefly persist in the UI; ensure you
modify the object returned in the SET_CURRENT_PROMO_CODE branch (the case block
that references currentPromoCode) to include promoCodeAllowsReassign:
<desired-reset-value>.
In `@src/utils/utils.js`:
- Line 146: The isSentryInitialized helper is dead code since
handleSentryException no longer uses Sentry; remove the unused function
isSentryInitialized from utils.js and any related imports/usages, or if you
intend to restore Sentry telemetry, rewire handleSentryException to check
isSentryInitialized (and call Sentry.captureException) instead of only logging
to console; refer to the isSentryInitialized function name and
handleSentryException to locate the change points and make the corresponding
cleanup or restoration.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/actions.jssrc/components/personal-information/index.jssrc/components/promocode-input/index.module.scsssrc/components/registration-lite.jssrc/components/ticket-type/index.jssrc/components/ticket-type/index.module.scsssrc/reducer.jssrc/utils/utils.js
gcutrini
force-pushed
the
feature/prevent-ticket-reassignment
branch
from
27da930 to
d26a8f2
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/personal-information/index.js (1)
282-282:⚠️ Potential issue | 🔴 CriticalCritical:
shouldDisplayTicketAssignmentis invoked as a function but is a boolean.
shouldDisplayTicketAssignmentis defined as a boolean constant on line 62, but here it's called with(). This will throw aTypeError: shouldDisplayTicketAssignment is not a functionat runtime.🐛 Proposed fix
- {shouldDisplayTicketAssignment() && + {shouldDisplayTicketAssignment &&🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/personal-information/index.js` at line 282, The code calls shouldDisplayTicketAssignment as a function but it is defined as a boolean (causing a TypeError); fix by removing the invocation at the render site and use the boolean directly (change {shouldDisplayTicketAssignment() && ...} to {shouldDisplayTicketAssignment && ...}), or if the intent was a computed check, convert the boolean constant into a function (e.g., rename/implement shouldDisplayTicketAssignment() accordingly) and update the definition referenced in the component so the call site and definition types match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/personal-information/index.js`:
- Line 282: The code calls shouldDisplayTicketAssignment as a function but it is
defined as a boolean (causing a TypeError); fix by removing the invocation at
the render site and use the boolean directly (change
{shouldDisplayTicketAssignment() && ...} to {shouldDisplayTicketAssignment &&
...}), or if the intent was a computed check, convert the boolean constant into
a function (e.g., rename/implement shouldDisplayTicketAssignment() accordingly)
and update the definition referenced in the component so the call site and
definition types match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0036bddd-0209-4947-8324-054cc799212d
📒 Files selected for processing (7)
src/actions.jssrc/components/personal-information/index.jssrc/components/promocode-input/index.module.scsssrc/components/registration-form/index.jssrc/components/ticket-type/index.jssrc/components/ticket-type/index.module.scsssrc/reducer.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ticket-type/index.module.scss
- src/components/promocode-input/index.module.scss
- src/components/ticket-type/index.js
Rebased on v7.0.0 (registration-form/index.js replaces registration-lite.js). Use the /apply endpoint's allows_to_reassign response to control whether tickets can be reassigned. When a promo code disallows reassignment, the widget hides the assignee radio options and shows a non-transferable notice. Reset the flag on validation errors so switching to an unsupported ticket type clears the notice. Changes: - Add promoCodeAllowsReassign state in reducer from /apply response - Add VALIDATE_PROMO_CODE_ERROR action to reset on errors - Rework personal-info reassignment gate: dual-source check (promo code + ticket type allows_to_reassign) - Auto-assign to purchaser when non-transferable - Validate promo code on ticket change and on apply - Reset promoCodeAllowsReassign on SET_CURRENT_PROMO_CODE (CodeRabbit fix) - Move promo code input below quantity stepper - Add non-transferable notice in ticket-type component - Refactor validatePromoCode: remove UI concerns from action, caller handles step change and error display
gcutrini
force-pushed
the
feature/prevent-ticket-reassignment
branch
from
d26a8f2 to
67b40db
Compare
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/actions.js`:
- Around line 199-202: The early return in validatePromoCode (when summitId or
currentPromoCode is falsy) returns null and breaks promise chains; change that
branch to return a resolved Promise (e.g., return Promise.resolve(null)) so
validatePromoCode always returns a Promise and callers like
handleValidatePromocode can safely call .then(...).
🪄 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: 045ab2ec-d7c0-47fc-bcce-61d193019232
📒 Files selected for processing (7)
src/actions.jssrc/components/personal-information/index.jssrc/components/promocode-input/index.module.scsssrc/components/registration-form/index.jssrc/components/ticket-type/index.jssrc/components/ticket-type/index.module.scsssrc/reducer.js
✅ Files skipped from review due to trivial changes (2)
- src/components/ticket-type/index.module.scss
- src/components/promocode-input/index.module.scss
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/personal-information/index.js
- src/components/ticket-type/index.js
- src/components/registration-form/index.js
| dispatch(stopWidgetLoading()); | ||
| return res; | ||
| }).catch((error) => { | ||
| dispatch(stopWidgetLoading()); |
There was a problem hiding this comment.
@gcutrini
The old validatePromoCode had a custom error handler that routed 404/412/429 to the onError callback (inline form errors). The new code passes authErrorHandler to getRequest at src/actions.js:218, which shows SweetAlert modal dialogs for these status codes.
There was a problem hiding this comment.
Fixed. Restored custom error handler that lets 404/412/429 bubble up to the caller for inline display. Also fixed the catch block to normalize all response shapes (412 has body.errors[], 404 has body.message, 429 varies) into a consistent errors array for onError.
| </ReactTooltip> | ||
|
|
||
| {allowPromoCodes && | ||
| <> |
There was a problem hiding this comment.
@gcutrini
The "cannot be reassigned" notice in TicketTypeComponent is nested inside the {allowPromoCodes && ...} JSX block. If a ticket type has allows_to_reassign: false but promo codes are disabled for the event, users won't see the notice on the ticket selection step. The personal-info component correctly auto-assigns, but the user gets no advance warning.
There was a problem hiding this comment.
Fixed. Moved the non-transferable notice outside the allowPromoCodes conditional block so it renders based purely on canReassign, regardless of whether promo codes are enabled for the event.
Route 404 (code not found), 412 (invalid for ticket type/qty), and 429 (rate limited) to the caller for inline form error display instead of letting authErrorHandler intercept them with SweetAlert modals. These are user-actionable validation errors that belong inline, not in modal dialogs.
The notice was nested inside the allowPromoCodes conditional block, so it would not render when promo codes are disabled for the event. A ticket type with allows_to_reassign: false should always show the notice regardless of promo code settings.
Test the dual-source reassignment check: promo code allows_to_reassign and ticket type allows_to_reassign. Covers radio visibility for each combination and default prop behavior.
|
Addressed all review comments:
Ready for re-review. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/actions.js (1)
199-202:⚠️ Potential issue | 🔴 CriticalAlways return a Promise from
validatePromoCode.When this branch returns
null, callers likehandleValidatePromocodeimmediately dovalidatePromoCode(data).then(...), which throws synchronously on the no-promo path and can leave the widget stuck in loading state.🐛 Proposed fix
// Nothing to validate if no promo code applied if (!summitId || !currentPromoCode) { - return null; + return Promise.resolve(null); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions.js` around lines 199 - 202, The branch in validatePromoCode synchronously returns null causing callers like handleValidatePromocode to call .then on a non-promise and crash; change that early return to always return a Promise (e.g., return Promise.resolve(null) or Promise.resolve({/* same shape as success case */})) so validatePromoCode consistently returns a Promise and downstream .then/.catch flows remain safe; update the validatePromoCode function (and any related return paths) to ensure a Promise is returned when !summitId || !currentPromoCode.
🧹 Nitpick comments (1)
src/components/personal-information/__tests__/reassignment-gate.test.js (1)
72-74: Also assertLeave Unassignedis hidden in both disallowed scenarios.Both “hidden” tests currently check only two options. If
Leave Unassignedis mistakenly rendered alone, these tests would still pass.✅ Suggested test hardening
it('hides assignment radio when promo code disallows reassignment', () => { @@ expect(screen.queryByText('Myself')).toBeNull(); expect(screen.queryByText('Someone Else')).toBeNull(); + expect(screen.queryByText('Leave Unassigned')).toBeNull(); }); @@ it('hides assignment radio when ticket type disallows reassignment', () => { @@ expect(screen.queryByText('Myself')).toBeNull(); expect(screen.queryByText('Someone Else')).toBeNull(); + expect(screen.queryByText('Leave Unassigned')).toBeNull(); });Also applies to: 91-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/personal-information/__tests__/reassignment-gate.test.js` around lines 72 - 74, The two "hidden" tests in reassignment-gate.test.js only assert that 'Myself' and 'Someone Else' are absent, which would miss the case where 'Leave Unassigned' is rendered alone; update both disallowed-scenario tests (the blocks containing expect(screen.queryByText('Myself')).toBeNull() and expect(screen.queryByText('Someone Else')).toBeNull()) to also assert that screen.queryByText('Leave Unassigned') is null so all three options are verified hidden.
🤖 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.js`:
- Around line 229-231: The current catch unconditionally dispatches
createAction(VALIDATE_PROMO_CODE_ERROR)({}) which resets promoCodeAllowsReassign
on any rejection (500/network/auth), incorrectly re-enabling reassignment;
update the catch in the promise returned by the validation call (the block
starting with ")({})(dispatch).catch((e) => {") to inspect the error (e.status
or e.response?.status) and only dispatch VALIDATE_PROMO_CODE_ERROR for
client/validation failures (e.g., 4xx statuses or explicit validation error
codes), otherwise rethrow or handle server/auth/network errors without
dispatching that action so promoCodeAllowsReassign remains unchanged for
non-validation errors.
---
Duplicate comments:
In `@src/actions.js`:
- Around line 199-202: The branch in validatePromoCode synchronously returns
null causing callers like handleValidatePromocode to call .then on a non-promise
and crash; change that early return to always return a Promise (e.g., return
Promise.resolve(null) or Promise.resolve({/* same shape as success case */})) so
validatePromoCode consistently returns a Promise and downstream .then/.catch
flows remain safe; update the validatePromoCode function (and any related return
paths) to ensure a Promise is returned when !summitId || !currentPromoCode.
---
Nitpick comments:
In `@src/components/personal-information/__tests__/reassignment-gate.test.js`:
- Around line 72-74: The two "hidden" tests in reassignment-gate.test.js only
assert that 'Myself' and 'Someone Else' are absent, which would miss the case
where 'Leave Unassigned' is rendered alone; update both disallowed-scenario
tests (the blocks containing expect(screen.queryByText('Myself')).toBeNull() and
expect(screen.queryByText('Someone Else')).toBeNull()) to also assert that
screen.queryByText('Leave Unassigned') is null so all three options are verified
hidden.
🪄 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: bf74a539-545c-421a-8321-fdb45557c400
📒 Files selected for processing (4)
src/actions.jssrc/components/personal-information/__tests__/reassignment-gate.test.jssrc/components/registration-form/index.jssrc/components/ticket-type/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ticket-type/index.js
Move VALIDATE_PROMO_CODE_ERROR dispatch into the error handler where the status code is already known, instead of re-checking in the catch block. Removes code duplication and prevents network/auth errors from incorrectly resetting the reassignment state.
| return { ...state, promoCodeAllowsReassign: allows_to_reassign ?? true } | ||
| } | ||
| case VALIDATE_PROMO_CODE_ERROR: { | ||
| return { ...state, promoCodeAllowsReassign: true } |
There was a problem hiding this comment.
@gcutrini
There is no validationPending flag. Between "user applied promo" and "response arrived", the reducer holds promoCodeAllowsReassign: true from SET_CURRENT_PROMO_CODE, a slow false response can arrive after a fast error → true response, leaving the UI permanently permissive until the user forces another revalidation.
here at error should be promoCodeAllowsReassign: false
There was a problem hiding this comment.
Fixed. SET_CURRENT_PROMO_CODE and VALIDATE_PROMO_CODE_ERROR now default to false.
| // 429: rate limited | ||
| // User-actionable errors: reset reassignment state and bubble up | ||
| // to the caller for inline display | ||
| if (res && [404, 412, 429].includes(res.statusCode)) { |
There was a problem hiding this comment.
@gcutrini i do think here
clearing the promo entirely on 404/412 (the code is genuinely invalid) so the user isn't stuck in a "promo applied but everything blocked" state:
if (res && [404, 412].includes(res.statusCode)) {
// invalid promo — clear it outright
dispatch(createAction(CLEAR_CURRENT_PROMO_CODE)({}));
return;
}
if (res && res.statusCode === 429) {
// transient — keep promo, fail closed on the gate
dispatch(createAction(VALIDATE_PROMO_CODE_ERROR)({}));
return;
}There was a problem hiding this comment.
Fixed. 404/412 now dispatch removePromoCode() to clear the code and re-fetch ticket types. 429 keeps the code and dispatches VALIDATE_PROMO_CODE_ERROR.
| changeStep(STEP_PERSONAL_INFO); | ||
| }) | ||
| .catch((e) => { | ||
| if (e?.res?.body) { |
There was a problem hiding this comment.
@gcutrini
In handleValidatePromocode, onError only runs when e?.res?.body is truthy. On timeouts, CORS errors, or offline, the promise rejects without res, so the spinner stops, step doesn't advance, and the user sees nothing ( a dead-click UX )
proposed fix
Always call onError with a generic fallback message when e?.res?.body is missing (reuse the existing "An error occurred" constant) and still call handleSentryException(e).
There was a problem hiding this comment.
Network errors (timeout, CORS, offline) fall through the error handler to authErrorHandler, which shows a modal via the default switch case (undefined status code). The user does see feedback. The catch block skipping onError for these is correct since the modal already handled it.
| } | ||
|
|
||
| const handleApplyPromoCode = async (code) => { | ||
| await applyPromoCode(code); |
There was a problem hiding this comment.
@gcutrini
applyPromoCode leaves promo state applied after ticket-types fetch fails
- User types SPEAKER50, clicks Apply.
- handleApplyPromoCode('SPEAKER50') runs.
- dispatch(SET_CURRENT_PROMO_CODE) writes promoCode: 'SPEAKER50', promoCodeAllowsReassign: true to Redux immediately.
- await dispatch(getTicketTypes(summitId)) starts. The input becomes readOnly because promoCode is now truthy (see readOnly={!isEmptyString(promoCode)} at promocode-input/index.js:48), and the
Apply button flips to Remove. - getTicketTypes rejects — for example, a 500 from /ticket-types/allowed, a lost connection, or an expired token (customErrorHandler in actions.js:73-84 passes anything non-404/500 to
authErrorHandler). - await throws. Control leaves handleApplyPromoCode with an uncaught promise rejection — the second block (if (ticket) { validatePromoCode(...) }) never runs, so no validation gets issued.
- Redux still holds promoCode: 'SPEAKER50' and the old ticketTypes array. The UI shows:
- Promo input in "applied" visual state (green checkmark, Remove button).
- The pre-promo ticket list (no discount, possibly including tickets the promo would have filtered out).
- promoCodeAllowsReassign: true with no validation ever run.
suggested fixes
1.Make the action atomic (only commit the promo code on success)
export const applyPromoCode = (currentPromoCode) => async (dispatch, getState) => {
const { registrationLiteState: { settings: { summitId } } } = getState();
try {
dispatch(createAction(SET_CURRENT_PROMO_CODE)({ currentPromoCode }));
await dispatch(getTicketTypes(summitId));
} catch (e) {
dispatch(createAction(CLEAR_CURRENT_PROMO_CODE)({}));
throw e;
}
}** This guarantees: either the promo code and refreshed ticket types are in state, or neither is
Handle the rejection at the caller so the user sees feedback (src/components/ticket-type/index.js:125)
const handleApplyPromoCode = async (code) => {
try {
await applyPromoCode(code);
if (ticket) {
await validatePromoCode({ id: ticket.id, ticketQuantity: quantity, sub_type: ticket.sub_type });
}
} catch (e) {
handleSentryException(e);
// surface to user — reuse the existing formErrors/promocodeError channel
// e.g. via a new `onApplyError` prop from RegistrationForm
}
}There was a problem hiding this comment.
Fixed. applyPromoCode now wraps in try/catch and dispatches CLEAR_CURRENT_PROMO_CODE on failure.
Consolidate both validation flows into a shared method with consistent inline error display. Add PromoCodeNotice component for unified notice rendering. Track validation state with promoCodeValid to show correct icon (checkmark on success, error on failure). Move error handler to module level. Reset promo state on widget init. Hide ticket summary when expanded.
|
Your review raised valid concerns about stuck states and error handling. Here's what we found and changed: The fail-closed default (item 1) solved the stuck state but caused a new issue: the non-transferable notice showed for invalid codes, since promoCodeAllowsReassign was false on validation error. An invalid code shouldn't impose any reassignment restriction. We reverted to defaulting true on error and instead prevent advancement by disabling the Next button when promoCodeVerified is not true. The removePromoCode on 404/412 (item 2) fixed the stuck state but created silent clearing with no explanation. Users saw the code disappear without knowing why. We reverted to keeping the code applied on validation errors and showing inline errors via a unified PromoCodeNotice component. The user sees what went wrong and decides to remove or retry. To support this, we unified both validation flows (Apply button and Next button) into a shared handlePromoCodeValidation method. Previously the Apply flow caught errors with console.log while the Next flow showed inline errors, leading to inconsistent behavior. We also added promoCodeVerified (null/true/false) and promoCodeValidating to Redux so the promo code input shows a spinner during validation, checkmark on success, and X on failure, replacing the old SVG icon that showed a checkmark even for invalid codes. The atomic applyPromoCode (item 4) is kept: if getTicketTypes fails during code application, the code is cleared since ticket data would be stale. Separately, removing a promo code previously cleared the ticket selection upfront (setTicket(null)), forcing users to re-select even when the ticket was still available. Now the existing useEffect that syncs with the refreshed ticket types list handles this: if the ticket still exists after re-fetch, it updates with fresh pricing; if it no longer exists (e.g. a promo-only ticket type), it clears the selection automatically. |
| // auto-assign to purchaser when ticket is non-transferable | ||
| data = { | ||
| ...data, | ||
| attendee: { firstName: personalInfo.firstName, lastName: personalInfo.lastName, email: personalInfo.email } |
There was a problem hiding this comment.
@gcutrini
Auto-assign uses stale personalInfo state instead of current form data
In PersonalInfoComponent.onSubmit, when isNonTransferable, the attendee is populated from personalInfo React state (initialized from userProfile) rather than data (the react-hook-form submission values). If the user's profile has no given_name (the name field becomes editable), and they type a name, the ticket is assigned with the original empty value ( not what they entered.)
2 participants
/applyendpoint'sallows_to_reassignresponse to control ticket reassignment UIVALIDATE_PROMO_CODE_ERRORaction to reset reassignment flag when validation fails (e.g. switching to an unsupported ticket type)ref: https://app.clickup.com/t/86b7fapmr
Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes