Fix wholesale sign-in redirect loop and nested anchor markup - #200
Merged
damianlegawiec merged 12 commits intoJul 28, 2026
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe change adds wholesale sign-in and redirect handling, seeds a gated wholesale channel for E2E tests, improves product and cart interaction behavior, and introduces Playwright coverage for browsing, authentication, ordering, cart, and application flows. ChangesWholesale portal flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Guest
participant WholesaleCatalog
participant WholesaleSignInPage
participant WholesaleSignInWall
participant WholesaleCart
Guest->>WholesaleCatalog: Open hidden-price catalog
WholesaleCatalog->>WholesaleSignInPage: Navigate with redirect target
WholesaleSignInPage->>WholesaleSignInWall: Render sign-in wall
Guest->>WholesaleSignInWall: Submit credentials
WholesaleSignInPage-->>Guest: Redirect to original product path
Guest->>WholesaleCart: Add product to wholesale cart
WholesaleCart-->>Guest: Show product in cart drawer
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
e2e/helpers.ts (1)
16-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
RegExp#testis stateful for global-flag regexes.
url.test(page.url())on line 22 will silently misbehave if a caller ever passes a regex with thegflag —.test()advanceslastIndexon that instance, so repeated calls (as happens acrosstoPass()retries) alternate matches. No current caller inwholesale.spec.tsuses a global regex, but as a shared, exported helper this is an easy trap for future test authors, and the resulting flakiness would be hard to trace back to this line.🛡️ Proposed defensive fix
await expect(async () => { - if (!url.test(page.url())) { + url.lastIndex = 0; + if (!url.test(page.url())) { await locator.click({ timeout: ATTEMPT_TIMEOUT }); } await page.waitForURL(url, { timeout: ATTEMPT_TIMEOUT }); }).toPass({ timeout: NAVIGATION_TIMEOUT });Please confirm this matches your understanding of
RegExp.prototype.testsemantics with thegflag.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/helpers.ts` around lines 16 - 27, Update clickUntilUrl to avoid stateful RegExp.test behavior when checking page.url() across retries, resetting or otherwise neutralizing the regex lastIndex before each test so global-flag patterns produce consistent results. Preserve the existing click and waitForURL retry flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/wholesale.spec.ts`:
- Around line 168-178: Update the add-to-cart retry block to track whether
addToCart has already been clicked, rather than using drawer.isVisible() as the
click guard. Click only once across toPass retries, then continue polling for
the drawer line item until it becomes visible, preserving the existing timeouts
and assertion behavior.
In `@scripts/e2e/bootstrap-spree.sh`:
- Around line 119-125: Update the channel setup around find_or_create_by! so
preferred_guest_checkout is unconditionally set to false after lookup or
creation, matching the existing preferred_storefront_access reassertion. Keep
the creation block’s name initialization and ensure the corrected value is
persisted before the success message.
In `@src/app/`[country]/[locale]/(storefront)/cart/page.tsx:
- Around line 135-138: Guard all cart mutations during in-flight updates by
adding disabled={updating} to the remove button in
src/app/[country]/[locale]/(storefront)/cart/page.tsx lines 135-138 and
src/app/[country]/[locale]/(wholesale)/wholesale/cart/WholesaleCartView.tsx
lines 123-128; leave the existing quantity-control handling unchanged.
---
Nitpick comments:
In `@e2e/helpers.ts`:
- Around line 16-27: Update clickUntilUrl to avoid stateful RegExp.test behavior
when checking page.url() across retries, resetting or otherwise neutralizing the
regex lastIndex before each test so global-flag patterns produce consistent
results. Preserve the existing click and waitForURL retry flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc62c08b-a67d-41f9-bd06-9ed7704d75d7
📒 Files selected for processing (15)
e2e/helpers.tse2e/wholesale.spec.tsscripts/e2e/bootstrap-spree.shsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleGuestBrowse.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleHeader.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleSignInWall.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/apply/page.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/cart/WholesaleCartView.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/sign-in/page.tsxsrc/components/products/HiddenPricePrompt.tsxsrc/components/products/ProductCard.tsxsrc/components/ui/quantity-picker.tsxsrc/lib/utils/path.ts
💤 Files with no reviewable changes (1)
- src/components/products/HiddenPricePrompt.tsx
damianlegawiec
force-pushed
the
claude/wholesale-pricing-redirect-loop-xg9pzk
branch
from
July 27, 2026 10:21
64b0e81 to
554a2b9
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
e2e/checkout.spec.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an absolute alias import for the E2E helper.
This new TypeScript import uses
./helpers, contrary to the repository rule requiring@/imports. Confirm that the alias coverse2e/; otherwise document an E2E exception.As per coding guidelines, TypeScript files should use absolute
@/imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/checkout.spec.ts` at line 2, Update the import of clickUntilUrl in checkout.spec.ts to use the repository’s absolute `@/` alias instead of ./helpers, and verify that the alias resolves the e2e/ directory; if it does not, document the required E2E import exception.Source: Coding guidelines
src/lib/utils/account-redirect.ts (1)
34-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for
account-redirect.ts.The rebase logic (prefix slicing, re-validation against the new base path) is non-trivial and security-relevant (it gates the login return target), yet unlike
path.tsandwholesale.tsin this PR, this file has no companion test suite in this batch. A test file mirroringsrc/lib/utils/__tests__/path.test.tscoveringresolveAccountRedirect,buildAccountLoginHref, andrebaseAccountRedirect/rebaseAccountRedirectSearchwould be valuable given how easy it is to introduce a subtle regression in this kind of path-rebasing logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/account-redirect.ts` around lines 34 - 74, Add a companion unit test suite for account-redirect.ts covering resolveAccountRedirect, buildAccountLoginHref, and rebaseAccountRedirect/rebaseAccountRedirectSearch. Include cases for valid and invalid targets, account-root handling, prefix rebasing while preserving suffixes, queries, and hashes, and re-validation against the next base path, following the conventions of the existing path and wholesale utility tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/utils/path.ts`:
- Around line 53-63: The wholesale sign-in redirect flow must not pass arbitrary
same-origin paths through safeRedirectPath. Update WholesaleSignInPage and
WholesaleSignInWall to use a wholesale-scoped redirect resolver with an
appropriate fallback, or remove the redirect query from the authenticated path,
while preserving only destinations permitted for wholesale buyers and leaving
resolveAccountRedirect behavior unchanged.
---
Nitpick comments:
In `@e2e/checkout.spec.ts`:
- Line 2: Update the import of clickUntilUrl in checkout.spec.ts to use the
repository’s absolute `@/` alias instead of ./helpers, and verify that the alias
resolves the e2e/ directory; if it does not, document the required E2E import
exception.
In `@src/lib/utils/account-redirect.ts`:
- Around line 34-74: Add a companion unit test suite for account-redirect.ts
covering resolveAccountRedirect, buildAccountLoginHref, and
rebaseAccountRedirect/rebaseAccountRedirectSearch. Include cases for valid and
invalid targets, account-root handling, prefix rebasing while preserving
suffixes, queries, and hashes, and re-validation against the next base path,
following the conventions of the existing path and wholesale utility tests.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7dd5ea93-8712-4ecf-9e62-c25fcf444791
📒 Files selected for processing (19)
e2e/checkout.spec.tse2e/helpers.tse2e/wholesale.spec.tsscripts/e2e/bootstrap-spree.shsrc/app/[country]/[locale]/(storefront)/cart/page.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleGuestBrowse.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleHeader.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleSignInWall.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/apply/page.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/cart/WholesaleCartView.tsxsrc/app/[country]/[locale]/(wholesale)/wholesale/sign-in/page.tsxsrc/components/products/HiddenPricePrompt.tsxsrc/components/products/ProductCard.tsxsrc/components/ui/quantity-picker.tsxsrc/lib/__tests__/wholesale.test.tssrc/lib/utils/__tests__/path.test.tssrc/lib/utils/account-redirect.tssrc/lib/utils/path.tssrc/lib/wholesale.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/app/[country]/[locale]/(wholesale)/wholesale/apply/page.tsx
- e2e/helpers.ts
- src/app/[country]/[locale]/(wholesale)/wholesale/cart/WholesaleCartView.tsx
- src/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleHeader.tsx
- scripts/e2e/bootstrap-spree.sh
- src/app/[country]/[locale]/(storefront)/cart/page.tsx
- src/app/[country]/[locale]/(wholesale)/wholesale/_components/WholesaleSignInWall.tsx
- src/app/[country]/[locale]/(wholesale)/wholesale/sign-in/page.tsx
- src/components/ui/quantity-picker.tsx
- e2e/wholesale.spec.ts
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
On a prices_hidden channel the guest catalog's "Sign in for pricing" and "Sign in to order" links pointed at the catalog root, which just renders the guest view again — every click bounced back to the same page and nested the previous ?redirect= param inside a new one, so a guest could never reach a sign-in form. Add a dedicated /wholesale/sign-in page that always shows the sign-in wall (with the apply-for-access link) and honours the existing ?redirect= contract, and point every guest sign-in affordance (price prompts, header, apply-page footer) at it. The guest view also drops any stale redirect param when building its return URL so redirects can never nest again. Also restructure ProductCard as a stretched link so the hidden-price prompt's link no longer renders inside the card's link — nested anchors are invalid HTML and broke hydration on the guest catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
Covers the prices-hidden guest experience end to end: catalog browse with sign-in-for-pricing prompts (including a regression listener for the nested-anchor hydration error), the dedicated sign-in page and its ?redirect= return contract (including the redirect-nesting regression), the sign-in wall on ordering surfaces, the apply-and-under-review flow, and the approved buyer's sign-in, return-to-PDP, add-to-cart round trip. The e2e bootstrap now flips the seeded wholesale channel to prices_hidden and enables the portal via SPREE_WHOLESALE_CHANNEL — that posture exercises strictly more portal UI than the login_required default, while ordering surfaces wall guests off under either posture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
Sanitize every post-login return target through one shared helper. The previous leading-slash check admitted values like "/\evil.com", which the URL parser resolves off-site, so a crafted link could turn sign-in into an open redirect; the helper also tolerates repeated query keys, which arrive as an array and previously crashed the wholesale sign-in page for authenticated buyers. Applied to the wholesale sign-in page, the wholesale wall, and the account sign-in. The nested-anchor regression test asserted before React had hydrated, so it passed even with the bug present. It now waits for the client to take over and also asserts the rendered tree contains no nested links. Also: keep Escape inside the quantity input from dismissing the cart drawer, disable the picker on both full-page carts while a cart update is in flight so a typed quantity can't be clobbered by a stale +/- click, scope the stretched-link elevation to the card that owns the overlay, and share the e2e click-then-navigate retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
A soft navigation here leaves the previous route mounted for a while, so whole-page locators pick up content the test is not talking about: the sign-in wall's email field collided with the apply form's, and the catalog's per-card sign-in prompts were counted while asserting a product page was unlocked. Load the apply page directly (the wall's link to it is already covered) and assert only the product page's own ordering affordance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
The apply spec failed in CI with a bare "element not found", which says nothing about why registration did not complete — the reason only ever appears in the form's own alert. Surface that text instead. Registration is rate limited to 3 per IP per minute, so retrying this spec replays into a 429 and buries the original cause; it now runs once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
Disable the remove button while a cart update is in flight, matching the quantity control — concurrent mutations each write cart state, so a remove during another update could land on top of it. Click add-to-cart once rather than re-clicking whenever the drawer is not yet visible: the drawer can render behind a click that already succeeded, which would add the item twice. Reassert the wholesale channel's guest-checkout setting on every bootstrap run, not only when the channel is created. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
CI showed registration succeeding while the post-submit confirmation card never appeared and no error was reported, so the card is not a reliable signal: it is local state on a form that the registration's own refresh remounts. Assert instead that the portal gates the new applicant as pending, which only holds if the account was created, and keep reporting the form's alert text when the API refuses the application. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
Both the apply form and the sign-in wall use controlled inputs with a React submit handler, so any interaction before hydration is lost: the typed values never reach React state and the click falls through to a native form submit that reloads the page. On slower CI runners that made the application spec fail with no account created and no error to report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
The wholesale portal shipped its own open-redirect guard alongside the account one, so any future hardening had to be found and applied twice. Both now resolve through a single validator, and the portal has one owner for its sign-in destination instead of three call sites repeating it — including the rule that a stale return target must not nest. Both are covered by unit tests they previously lacked. Also folds the click-until-navigated retry into the shared e2e helper the checkout suite already had inline, and separates its click and navigation budgets so a slow first route compile is waited out rather than clicked a second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
Buyers only reach the wholesale sign-in from portal surfaces, so a return target pointing anywhere else is a crafted link rather than a real one. The portal now narrows its `?redirect=` targets the way account sign-in already narrows its own, and the permissive same-origin resolver is gone rather than left available to the next caller. Also stops an already-authenticated buyer bouncing off the sign-in page forever when the return target is the sign-in page itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
This reverts commit baed727.
The buyer sign-in spec proved unreliable: it fails on a tree byte-identical to one that passed, because the post-login navigation races between a client push, a router refresh, and the sign-in page's own server redirect. The race is worth fixing on its own terms, but not as a gate on shipping the portal fixes. Restores the checkout spec and the E2E bootstrap to their prior state, leaving this change to the feature code and its unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY
damianlegawiec
force-pushed
the
claude/wholesale-pricing-redirect-loop-xg9pzk
branch
from
July 28, 2026 12:03
758440c to
e8b9801
Compare
damianlegawiec
changed the title
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.
Summary
Fixes two defects in the wholesale portal: sign-in affordances that looped back to the catalog instead of reaching a sign-in form, and a nested-anchor markup violation that broke hydration on prices-hidden product cards.
Key Changes
Sign-in destination and redirect safety
wholesale/sign-in/page.tsx: a dedicated sign-in destination. On aprices_hiddenchannel the catalog root renders for guests, so "sign in" links pointing there landed the buyer straight back on the catalog — and with?redirect=re-appended on every click, looped. An already-authenticated buyer is bounced into the portal.wholesaleSignInHref()inlib/wholesale.tsowns the portal's sign-in path and the return-target rule in one place, including dropping a staleredirectfrom an earlier round trip so redirects can't nest inside redirects.resolveLocalPath()inlib/utils/path.tsis now the single open-redirect guard behind every post-login return target.resolveAccountRedirectis re-expressed on top of it rather than duplicating the mechanism, so hardening (//,\, encoded separators) lives in one place. A leading-slash check alone is insufficient because the URL parser treats a backslash as a slash for http(s).Markup and accessibility
after:absolute after:inset-0on the product name) instead of wrapping the card in an anchor, soHiddenPricePromptcan render its own link without nesting anchors — the invalid nesting surfaced as a React hydration error.Cart and input behaviour
Testing
Unit coverage for the redirect helpers (
path.test.ts,wholesale.test.ts) — local-path resolution, off-origin and encoded-separator rejection, return-target nesting, and sign-in href construction.An E2E suite for this flow was written and then removed from this PR: its buyer sign-in spec proved unreliable, failing on a tree byte-identical to one that passed. The cause is a race in the post-login navigation (a client push, a router refresh, and the sign-in page's server redirect all competing), which is a real user-facing issue worth fixing on its own terms — tracked separately rather than gating this change.
Follow-ups
?redirect=targets are same-origin-only but unscoped within the origin; scoping them to the portal is deferred until the navigation race is resolved, since it changes the same code path.https://claude.ai/code/session_018ud2Fsfy5yZPL8ZDnVowQY