feat(core): B2B contract switcher in the account drawer#3390
Open
rodrigo-tavares wants to merge 20 commits into
Open
feat(core): B2B contract switcher in the account drawer#3390rodrigo-tavares wants to merge 20 commits into
rodrigo-tavares wants to merge 20 commits into
Conversation
## What's the purpose of this pull request? Reverts an accidental `4.2.1` release This PR restores `main` to the last legitimate release state (`4.2.0`). > No npm publish happened: the local script had the `pnpm -r publish` step removed, so `4.2.1` only exists as git history (now being reverted). The `v4.2.1` tag was already deleted. ## How it works? Reverts the three commits that landed on `main`, newest → oldest: - `516e784` — `[no ci] Release: 4.2.1` (version bump + CHANGELOGs) - `e9b2105` — `fix: fix vtex message` (test-only log string change in `generate-i18n.ts`) - `6a13c8c` — `chore: lern force-publish release` After the revert, `package.json`/`lerna.json`/`CHANGELOG.md` are back at `4.2.0`, the `release` script has its `pnpm publish` step restored, and `generate-i18n.ts` keeps the original message.
Lets B2B buyers change the active contract within their Organization Unit from the account drawer (Phase 2, REQ-03..06). API (@faststore/api): - Add StoreContract type and the governed `availableContracts(orgUnitId)` query, resolved from the Org Unit scopes + MasterData corporate names, flagging the currently active contract. Per-contract lookups are isolated. Core (@faststore/core): - useAvailableContracts: on-demand fetch of the Org Unit's contracts. - useSwitchContract: full commercial-context change (ChangeToken seam -> session revalidation -> cart reset); keeps the previous contract on failure. - ContractSwitcher sub-view + a "Change" CTA in the drawer header, with loading / empty / error / active-indicator states per Figma (node 103-5434). The concrete VTEX ChangeToken endpoint remains an open integration point (see specs/contract-switcher.md, Decision 2) and is isolated in changeContractToken.ts. Tests: resolver governance/name-resolution/error handling; switch flow happy path + failure; switcher UI states. Spec: specs/contract-switcher.md Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the temporary UI-validation mocks with the real backend wiring: - ContractSwitcher now consumes the real useAvailableContracts / useSwitchContract SDK hooks instead of the local mock module. - OrganizationDrawer reads the active contract from b2b.contractName, gates the Change CTA on b2b.unitId, and drops the FORCE_* flag and hardcoded contract name/default. - Delete contractSwitcher.mock.ts and the throwaway prototype component. - Repoint ContractSwitcher tests at the real SDK modules. The switch flow (list, governance, session revalidation, cart reset) is real and testable in a B2B store. The VTEX ChangeToken endpoint remains a documented stub (spec Decision 2) pending confirmation from other teams. Note: touches B2B session consumption only; no auth/CSRF mechanism changes. Co-authored-by: Cursor <cursoragent@cursor.com>
rodrigo-tavares
requested review from
hellofanny and
lucasfp13
and removed request for
a team
Contributor
WalkthroughThe PR adds VTEX contract availability and switching support across schema, resolver, hooks, and OrganizationDrawer UI. It also updates refresh-token request handling and local discovery config support, plus tests for the new account flow. ChangesB2B Contract Switching
Sequence Diagram(s)sequenceDiagram
participant QueryAvailableContracts as Query.availableContracts
participant VtexCommerceSession as VtexCommerce.session
participant ParseSessionAvailableContracts as parseSessionAvailableContracts
participant MapSessionContractsToStoreContracts as mapSessionContractsToStoreContracts
QueryAvailableContracts->>VtexCommerceSession: load session namespaces and JWT data
VtexCommerceSession-->>QueryAvailableContracts: sessionData
QueryAvailableContracts->>ParseSessionAvailableContracts: parse shopper.availableContracts.value
ParseSessionAvailableContracts-->>QueryAvailableContracts: SessionAvailableContract[]
QueryAvailableContracts->>MapSessionContractsToStoreContracts: map to StoreContract[]
MapSessionContractsToStoreContracts-->>QueryAvailableContracts: StoreContract[]
sequenceDiagram
participant UseSwitchContract as useSwitchContract
participant ChangeContractToken as changeContractToken
participant ApplyCookies as applyVtexAuthCookieFromSwitchResponse
participant ClearSession as clearPersistedSessionState
participant WindowLocation as window.location
UseSwitchContract->>ChangeContractToken: switchContract(contractId)
ChangeContractToken->>ApplyCookies: apply returned auth cookies
ChangeContractToken-->>UseSwitchContract: true
UseSwitchContract->>ClearSession: clear persisted session state
UseSwitchContract->>WindowLocation: reload()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
![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: 6
🧹 Nitpick comments (3)
packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx (1)
1-1: 📐 Maintainability & Code Quality | ⚡ Quick winUse
UI-prefixed aliases for@faststore/uiimports in core components.Please align this file with the component import convention to avoid local name collisions over time.
As per coding guidelines,packages/core/src/components/**/*.{ts,tsx}must “Import from@faststore/uiand alias withUIprefix to avoid local name collisions when composing library components”.Suggested diff
-import { Button, Icon, IconButton, Loader } from '`@faststore/ui`' +import { + Button as UIButton, + Icon as UIIcon, + IconButton as UIIconButton, + Loader as UILoader, +} from '`@faststore/ui`'- <IconButton + <UIIconButton ... - icon={<Icon name="ArrowLeft" width={20} height={20} />} + icon={<UIIcon name="ArrowLeft" width={20} height={20} />} ... - <IconButton + <UIIconButton ... - icon={<Icon name="X" width={20} height={20} />} + icon={<UIIcon name="X" width={20} height={20} />} ... - <Loader /> + <UILoader /> ... - <Icon + <UIIcon ... - <Icon name="MagnifyingGlass" width={20} height={20} /> + <UIIcon name="MagnifyingGlass" width={20} height={20} /> ... - <Icon name="X" width={18} height={18} /> + <UIIcon name="X" width={18} height={18} /> ... - <Icon + <UIIcon ... - <Button variant="tertiary" onClick={onBack}> + <UIButton variant="tertiary" onClick={onBack}> ... - </Button> + </UIButton> - <Button + <UIButton ... - </Button> + </UIButton>🤖 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 `@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx` at line 1, The imports from `@faststore/ui` in ContractSwitcher.tsx use local names that can collide; update the import to alias each UI component with a UI prefix (e.g., Button -> UIButton, Icon -> UIIcon, IconButton -> UIIconButton, Loader -> UILoader) and update any usages inside the ContractSwitcher component to use these new symbols so the file follows the core components convention.Source: Coding guidelines
packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerFooter.tsx (1)
1-1: 📐 Maintainability & Code Quality | ⚡ Quick winConsolidated: standardize
@faststore/uialiasing acrossOrganizationDrawerFooter.tsxandOrganizationDrawerHeader.tsx.Both files import UI primitives without the required
UIprefix. Apply the same aliasing convention inpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerFooter.tsxandpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerHeader.tsxto keep component code consistent and collision-safe.As per coding guidelines,
packages/core/src/components/**/*.{ts,tsx}: "Import from@faststore/uiand alias withUIprefix to avoid local name collisions."🤖 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 `@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerFooter.tsx` at line 1, The imports from `@faststore/ui` in OrganizationDrawerFooter and OrganizationDrawerHeader must use the UI alias to avoid collisions; update the import statements that currently read like "import { Icon, IconButton, Link } from '`@faststore/ui`'" to use aliased names (e.g., import { Icon as UIIcon, IconButton as UIIconButton, Link as UILink } from '`@faststore/ui`') and then update usages inside OrganizationDrawerFooter and OrganizationDrawerHeader to reference UIIcon, UIIconButton, and UILink respectively so both files follow the project's UI prefix convention.Source: Coding guidelines
packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/index.ts (1)
3-5: 📐 Maintainability & Code Quality | ⚡ Quick winRe-export component
Propstypes from the barrel.This barrel should also export the corresponding
*Propstypes for the components introduced/maintained in this folder.Proposed fix
export { OrganizationDrawerFooter } from './OrganizationDrawerFooter' +export type { OrganizationDrawerFooterProps } from './OrganizationDrawerFooter' export { OrganizationDrawerHeader } from './OrganizationDrawerHeader' +export type { OrganizationDrawerHeaderProps } from './OrganizationDrawerHeader' export { ContractSwitcher } from './ContractSwitcher'As per coding guidelines,
packages/{components,core}/src/components/**/*.{ts,tsx}: "AllPropsand variant types MUST be re-exported from the barrelindex.ts."🤖 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 `@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/index.ts` around lines 3 - 5, The barrel currently re-exports OrganizationDrawerFooter, OrganizationDrawerHeader, and ContractSwitcher components but omits their Props types; update this file to also re-export the corresponding type aliases (e.g., OrganizationDrawerFooterProps, OrganizationDrawerHeaderProps, ContractSwitcherProps) so callers can import both components and their prop types from the same barrel; locate the index.ts and add named exports for each component's exported Props type to satisfy the codebase rule that all Props/variant types are re-exported from the component barrel.Source: Coding guidelines
🤖 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 `@packages/api/src/platforms/vtex/resolvers/query.ts`:
- Around line 685-687: The current contractIds computation pulls ids from all
scope types; restrict it to contract scopes by filtering scopesByUnit.scopes to
only include entries where scope === 'contract' before flatMapping ids. Update
the expression that builds contractIds (referencing variables contractIds and
scopesByUnit and the flatMap over scope?.ids) to apply scope === 'contract' so
only contract scope ids are returned and then deduplicated and filtered.
In
`@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawer.tsx`:
- Line 146: The header can end up empty because contractName falls back to an
empty string; update the fallback in OrganizationDrawer so contractName =
b2b?.contractName ?? b2b?.unitName ?? t('organization.unknown') (or another
localized placeholder) so the header/link label always has a non-empty value;
locate the assignment to contractName in the OrganizationDrawer component and
replace the empty-string fallback with unitName or a localization key.
- Around line 171-176: The drawer's subview state (view) isn't reset when
closing so reopening can still show 'switch'; update the close/open flow to
reset view to 'menu' by calling setView('menu') whenever the drawer is closed or
when it opens: e.g., add setView('menu') inside the fadeOut/close handler used
by ContractSwitcher (and/or in the drawer open handler or a useEffect watching
the drawer's isOpen prop) so ContractSwitcher and OrganizationDrawer always
start at the menu on reopen.
In `@packages/core/src/sdk/account/changeContractToken.ts`:
- Around line 22-28: The function changeContractToken currently returns success
without performing the server-side contract change, causing callers to proceed
incorrectly; modify changeContractToken to "fail closed" until the VTEX
ChangeToken endpoint is implemented by removing the no-op and instead
immediately throwing a ContractSwitchError (or a clearly named NotImplemented
error) indicating the server-side change is not wired, so callers receive a
rejection rather than a false success; reference the changeContractToken
function and existing ContractSwitchError type when making this change.
In `@packages/core/src/sdk/account/useSwitchContract.ts`:
- Around line 34-42: The code currently calls validateSession(previousSession)
and only sets sessionStore when truthy, but proceeds to empty the cart and
return success even if revalidated is falsy; change the control flow so a falsy
revalidated is treated as a failed switch: if validateSession(previousSession)
returns a truthy revalidated, call sessionStore.set(revalidated) and continue;
otherwise call sessionStore.set(previousSession) (to rollback any partial
changes) and return false without calling cartStore.emptyCart; update the logic
around validateSession / previousSession / sessionStore.set and
cartStore.emptyCart accordingly.
In `@packages/core/test/sdk/account/useSwitchContract.test.ts`:
- Around line 1-3: The test is a browser/jsdom test but uses the .test.ts
suffix; rename the file packages/core/test/sdk/account/useSwitchContract.test.ts
to useSwitchContract.browser.test.ts (or .browser.test.tsx if JSX) so it uses
the required .browser.test.{ts,tsx} convention, and update any imports or
references to this file in test infra or exports if present; ensure the existing
jsdom header (/** `@vitest-environment` jsdom */) remains and the test runner
picks up the renamed file.
---
Nitpick comments:
In
`@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx`:
- Line 1: The imports from `@faststore/ui` in ContractSwitcher.tsx use local names
that can collide; update the import to alias each UI component with a UI prefix
(e.g., Button -> UIButton, Icon -> UIIcon, IconButton -> UIIconButton, Loader ->
UILoader) and update any usages inside the ContractSwitcher component to use
these new symbols so the file follows the core components convention.
In
`@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/index.ts`:
- Around line 3-5: The barrel currently re-exports OrganizationDrawerFooter,
OrganizationDrawerHeader, and ContractSwitcher components but omits their Props
types; update this file to also re-export the corresponding type aliases (e.g.,
OrganizationDrawerFooterProps, OrganizationDrawerHeaderProps,
ContractSwitcherProps) so callers can import both components and their prop
types from the same barrel; locate the index.ts and add named exports for each
component's exported Props type to satisfy the codebase rule that all
Props/variant types are re-exported from the component barrel.
In
`@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerFooter.tsx`:
- Line 1: The imports from `@faststore/ui` in OrganizationDrawerFooter and
OrganizationDrawerHeader must use the UI alias to avoid collisions; update the
import statements that currently read like "import { Icon, IconButton, Link }
from '`@faststore/ui`'" to use aliased names (e.g., import { Icon as UIIcon,
IconButton as UIIconButton, Link as UILink } from '`@faststore/ui`') and then
update usages inside OrganizationDrawerFooter and OrganizationDrawerHeader to
reference UIIcon, UIIconButton, and UILink respectively so both files follow the
project's UI prefix convention.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 776fc6fe-3ac7-4314-b5ad-e4e631b58a1f
⛔ Files ignored due to path filters (4)
packages/api/src/__generated__/schema.tsis excluded by!**/__generated__/**and included bypackages/**packages/core/@generated/gql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/@generated/graphql.tsis excluded by!**/@generated/**,!**/@generated/**and included bypackages/**packages/core/public/icons.svgis excluded by!**/*.svgand included bypackages/**
📒 Files selected for processing (17)
packages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/typeDefs/organization.graphqlpackages/api/src/platforms/vtex/typeDefs/query.graphqlpackages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.tspackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsxpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/index.tspackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawer.tsxpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerFooter.tsxpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawerHeader.tsxpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/index.tspackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/section.module.scsspackages/core/src/sdk/account/changeContractToken.tspackages/core/src/sdk/account/useAvailableContracts.tspackages/core/src/sdk/account/useSwitchContract.tspackages/core/test/components/account/ContractSwitcher.test.tsxpackages/core/test/sdk/account/useSwitchContract.test.tspackages/core/test/server/index.test.ts
renatomaurovtex
left a comment
renatomaurovtex
left a comment
Contributor
There was a problem hiding this comment.
Review — feat(core): B2B contract switcher in the account drawer
Nice, well-scoped PR: clean package boundaries (@faststore/api stays framework-agnostic, new GraphQL is governed + @auth + @cacheControl(scope: "private"), React lives only in core), real tests for the resolver, the switch flow and the UI, and __generated__/@generated look consistent with the typedefs. p-limit is already a runtime dep of @faststore/api (^3.1.0) and commerce.units.getScopesByOrgUnit / commerce.masterData.getContractById already exist in the commerce client, so no new dependency or missing-symbol risk. Reviewed against the diff + the existing client/resolver code.
🟠 bug useSwitchContract — Confirm empties the cart even though the switch is a documented no-op
changeContractToken is currently a stub that resolves without flipping anything server-side, but switchContract still runs cartStore.emptyCart() and returns true. So in a real B2B store today, a buyer who opens the switcher, selects a contract and confirms will lose their cart while staying on the same contract. Emptying a cart is destructive and hard to reverse — shipping that as a no-op is a real UX trap even on dev.
Suggest gating the destructive step (or the whole CTA) until ChangeToken is wired — e.g. don't reset the cart unless the token flip actually changed the context:
// changeContractToken should signal whether the context really changed,
// and the cart reset should be conditional on that — not unconditional.
const switched = await changeContractToken(contractId) // returns false while stubbed
if (!switched) return false
const revalidated = await validateSession(previousSession)
...
cartStore.emptyCart()🟠 question availableContracts resolver — every scope id is treated as a contract id
contractIds flat-maps scopes[].ids across all scope kinds (the dedup test even mixes scope: 'price'), then resolves each via getContractById. Non-contract scope ids (price tables, payment, etc.) will either fail the lookup (wasted MasterData calls) or, worse, resolve to something and surface as a fake "contract". Should this filter to the contract scope only?
- (scopesByUnit?.scopes ?? []).flatMap((scope) => scope?.ids ?? [])
+ (scopesByUnit?.scopes ?? [])
+ .filter((scope) => scope?.scope === 'contract')
+ .flatMap((scope) => scope?.ids ?? [])💬 context isActive source is fine — it matches the canonical pattern
Re: your open point about sessionData.namespaces.profile?.id?.value being the active contract id — this is consistent with validateSession.ts:139-143, which already resolves the active contract's corporate name from profile.id.value (guarded by isRepresentative). So you're using the same mapping the rest of the codebase trusts. One small note: the resolver doesn't gate on isRepresentative, but for non-representatives profile.id.value simply won't match any contract id, so isActive stays false — harmless.
💬 i18n Hardcoded English strings in the new UI
"Change contract", "Current session", "Manage", "Log out", "Search", the count/empty/error copy and the aria-labels are hardcoded. Consistent with the existing orders/account UI, so not blocking — but worth tracking against the My Account localization work (#3354) so this view doesn't get left behind.
💬 check Confirm @generated/__generated__ came from pnpm codegen
packages/api/src/__generated__/schema.ts, packages/core/@generated/{gql,graphql}.ts look byte-consistent with the new StoreContract type + AvailableContractsQuery (operation hash present), but please confirm they were produced by pnpm codegen rather than hand-edited.
Verdict: Approved with comments
Blocking (🟠):
- Cart is emptied on a no-op switch while ChangeToken is stubbed — gate the cart reset (and ideally the CTA) on a real context change.
- Confirm
availableContractsshould filter to thecontractscope rather than flat-mapping every scope id.
Non-blocking (💬):
isActivesource is consistent withvalidateSession.ts— no change needed.- Hardcoded English strings — track against #3354 localization.
- Confirm generated files came from
pnpm codegen.
Checks to confirm before merge: pnpm lint · build · pnpm turbo run test --filter=@faststore/api --filter=@faststore/core · pnpm size.
nicholas-maestrello
left a comment
nicholas-maestrello
left a comment
There was a problem hiding this comment.
Alignment with the Suma BFF (buyer-portal-graphql)
Note: Supersedes the previous review in Portuguese.
I reviewed the availableContracts resolver against the BFF APIs that expose Organization Unit scopes (api/organization-units/v1). The intent is correct, but there are important gaps relative to the BFF contract.
What is aligned
- Governance source: reading scopes from the Org Unit (
GET .../scopes) is the right approach. - Name resolution:
getContractByIdagainst Master Data (CL) forcorporateName. - Storefront auth: buyer cookie forwarded; no app keys exposed to the client.
- Resilience: per-contract lookup failures do not break the whole list.
The canonical BFF storefront endpoint for listing is:
GET /_v/store-front/units/:unitId/contracts/attached?details=trueCritical gaps
1. Scope filter — highest priority
The resolver aggregates IDs from all scopes:
const contractIds = Array.from(
new Set((scopesByUnit?.scopes ?? []).flatMap((scope) => scope?.ids ?? []))
).filter(Boolean)In the BFF, only the contractIds scope is used (SCOPE_KEYS.CONTRACTS in unit-scope-contracts.service.ts). Other scopes (collectionIds, paymentSystemIds, addresses, etc.) are not contracts.
The test deduplicates contract ids across scopes validates behavior the BFF does not have — mixing contract and price scopes would surface invalid IDs in the switcher list.
The correct platform scope name is contractIds, not contract (as used in the test mocks).
2. attached vs legacy semantics
contracts/attached→ the unit's owncontractIdsscope (governed list for switching).contracts/available→ root pool (not the buyer's switchable list).- Legacy
GET /units/{unitId}/contractsresolves from the root — different semantics.
This PR reads the correct unit, but aggregates scopes incorrectly.
3. Master Data validation
The BFF requires both corporateName and email before including a contract in summaries. The resolver only checks corporateName.
4. Open points (already documented in the PR)
isActiveviasession.namespaces.profile.id— the BFF does not define this on contract endpoints.changeContractToken— stub; the BFF also does not expose an equivalent endpoint today.
Suggested fix
const scopesByUnit = await commerce.units.getScopesByOrgUnit({ orgUnitId })
const contractScope = scopesByUnit?.scopes?.find(
(s) => s.scope === 'contractIds'
)
const contractIds = [...new Set(contractScope?.ids ?? [])].filter(Boolean)Alternative: call the BFF storefront endpoint contracts/attached?details=true directly and map the response, avoiding reimplementing governance logic.
Summary
| Item | Status |
|---|---|
| Scopes platform (Suma) | OK |
contractIds scope only |
Not aligned |
contracts/attached endpoint |
Partial semantics; implementation diverges |
| Master Data / corporateName | Partial (missing email check) |
Contract switch (ChangeToken) |
Not implemented on either side |
Recommend fixing the scope filter and updating tests to use contractIds as the scope name, consistent with the BFF.
Contributor
Author
Review feedback addressedBlocking fixes
Non-blocking notes
Checks
|
Gate cart reset and Confirm until ChangeToken confirms a context change, read only the contractIds Org Unit scope in availableContracts, and skip contracts missing corporateName or email to match the Suma BFF. Co-authored-by: Cursor <cursoragent@cursor.com>
🛡️ SDD Check — action requiredI couldn't detect an SDD in this PR. Please check one option below (requires write access to the repo):
|
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
Merge origin/dev into the contract switcher branch and commit specs/contract-switcher.md so the SDD check can detect the approved spec. Update the spec with contractIds scope filtering, email validation, and ChangeToken gating aligned with the implementation. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract nested ternaries into sub-components, remove the ChangeToken TODO comment, and add tests for search clear, empty results, switch errors, and failed confirm flows to improve new-code coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/core/test/components/account/ContractSwitcher.test.tsx`:
- Around line 159-216: This suite contains browser/DOM tests and must follow the
core browser test naming convention. Rename the ContractSwitcher test file from
a generic test filename to a browser test filename, and keep the same test cases
intact so the suite is picked up correctly by the browser test runner.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 890ae824-f393-4433-925d-dcf13bc2d1ba
📒 Files selected for processing (3)
packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsxpackages/core/src/sdk/account/changeContractToken.tspackages/core/test/components/account/ContractSwitcher.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/sdk/account/changeContractToken.ts
- packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx
Replace org-units scopes + Master Data with a single proxy call to
/_v/store-front/units/{orgUnitId}/contracts/attached?details=true,
matching faststore-plugin-buyer-portal and avoiding FS_DISCOVERY app key.
Co-authored-by: Cursor <cursoragent@cursor.com>
Load availableContracts from VTEX session instead of the store-front BFF, switch via switch-properties with token refresh and a full page reload, and resolve the active contract from activeContractId to keep the switcher in sync after context changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Reconcile MyAccountDrawer → Drawer rename from dev, apply CodeRabbit feedback (view reset on close, contractName fallback, browser test naming, Props barrel exports), and keep ContractSwitcher under the new path. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@packages/api/src/platforms/vtex/resolvers/query.ts`:
- Around line 715-738: The contract-listing flow in query.ts is swallowing
session fetch failures by turning commerce.session('') errors into null, which
then makes parseSessionAvailableContracts return an empty array. Update the
session lookup around sessionData and parseSessionAvailableContracts to preserve
and surface the failure state instead of defaulting to null, so a VTEX/session
outage can be handled as an error rather than an empty result.
In `@packages/api/src/platforms/vtex/utils/contract.ts`:
- Around line 87-90: Treat blank shopper.activeContractId values as missing in
the contract selection logic. In the contract resolver that returns the active
ID from sessionData namespaces, replace the direct trim-and-nullish-coalescing
flow so an empty string from shopper.activeContractId does not short-circuit
fallback; instead, only return a trimmed value when it is non-blank, and
otherwise continue to authentication.customerId and profile.id.
- Around line 133-143: The fallback in resolveActiveContractDisplayName
currently drops the profile last name, which truncates names when VTEX splits
them across firstName and lastName. Update the fallback logic in
resolveActiveContractDisplayName to build the display name from both
profile.firstName and profile.lastName, preserving existing trim behavior and
only falling back after resolveContractDisplayNameFromMd returns nothing.
In `@packages/core/discovery.config.js`:
- Around line 11-15: The try/catch around loading discovery.config.local in the
discovery config merge is swallowing all failures, not just the missing-file
case. Update the require('./discovery.config.local') handling so it only ignores
the module-not-found scenario for that local override file, and rethrow any
other error from discovery.config.local.js; keep the existing
deepmerge/finalConfig flow intact and use the same local-config load block to
apply the fix.
In
`@packages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx`:
- Around line 274-278: The Confirm action in ContractSwitcher should respect the
hook’s enabled state, since canConfirm currently only checks the local
conditions and ignores enabled from useSwitchContract(). Update the canConfirm
logic and any related confirm button gating so it includes enabled alongside the
existing switch conditions, using the switchContract, switching, switchError,
and enabled values from useSwitchContract() to locate the affected flow.
In `@packages/core/src/sdk/account/clearPersistedSessionState.ts`:
- Around line 12-25: The cleanup in clearPersistedSessionState is currently
swallowing both IndexedDB and sessionStorage failures, which lets stale session
data survive a successful switch. Update this helper so failures from the
idb-keyval del call and the sessionStorage.removeItem calls are surfaced to the
caller instead of being ignored, and then handle that in useSwitchContract so
the reload only happens after cleanup succeeds or the error is explicitly dealt
with.
In `@packages/core/test/components/account/ContractSwitcher.test.tsx`:
- Around line 192-201: The test name in ContractSwitcher.test.tsx overstates
what it verifies because useSwitchContract is mocked, so it cannot prove any
reload behavior. Rename the test to reflect the actual assertion that
ContractSwitcher still calls switchContract with "b" when the mocked hook
resolves false, or move the no-reload expectation into useSwitchContract.test.ts
where the real side effect can be asserted.
In `@packages/core/test/sdk/account/changeContractToken.test.ts`:
- Around line 1-3: The changeContractToken test suite is forcing jsdom via the
top-of-file Vitest environment directive, but this spec only uses mocks and
should run in the default Node environment. Remove the jsdom directive from
changeContractToken.test.ts and keep the test as a normal .test.ts Node unit
test; only use the browser-specific .browser.test.ts naming and environment if
browser APIs are actually needed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45912920-ceb6-46de-ac1e-d861589a8010
⛔ Files ignored due to path filters (1)
specs/contract-switcher.mdis excluded by none and included by none
📒 Files selected for processing (20)
packages/api/src/platforms/vtex/clients/commerce/index.tspackages/api/src/platforms/vtex/clients/commerce/types/Session.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/resolvers/validateSession.tspackages/api/src/platforms/vtex/typeDefs/query.graphqlpackages/api/src/platforms/vtex/utils/contract.tspackages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.tspackages/api/test/unit/platforms/vtex/utils/contract.test.tspackages/core/discovery.config.jspackages/core/discovery.config.local.jspackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsxpackages/core/src/components/account/MyAccountDrawer/OrganizationDrawer/OrganizationDrawer.tsxpackages/core/src/sdk/account/changeContractToken.tspackages/core/src/sdk/account/clearPersistedSessionState.tspackages/core/src/sdk/account/refreshToken.tspackages/core/src/sdk/account/useAvailableContracts.tspackages/core/src/sdk/account/useSwitchContract.tspackages/core/test/components/account/ContractSwitcher.test.tsxpackages/core/test/sdk/account/changeContractToken.test.tspackages/core/test/sdk/account/useSwitchContract.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/sdk/account/useAvailableContracts.ts
Contributor
|
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:
WalkthroughThis PR adds B2B contract discovery and switching, plus drawer UI, SDK, and configuration updates to support the flow. ChangesB2B Contract Switching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Remove invalid maxAge from @CacheControl (fixes generate/dev failure) and move B2B dev rewrites plus account overrides to a gitignored local file with a committed example template. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Fix Biome formatting check failing in CI for query.graphql. Co-authored-by: Cursor <cursoragent@cursor.com>
Fixes Next.js type check failure in starter build CI. Co-authored-by: Cursor <cursoragent@cursor.com>
Use globalThis.window for environment checks and add unit tests for refreshToken and clearPersistedSessionState to cover server/browser paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Address Sonar readability rules in refreshToken and useSwitchContract. Co-authored-by: Cursor <cursoragent@cursor.com>
Add drawer, hook, logout, and API unit tests covering previously untested organization drawer flows and session contract resolution paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Persist VtexIdclientAutCookie from switch-properties instead of relying on refreshtoken, which does not update JWT claims on localhost. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx (1)
298-313: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear or invalidate hidden selections after filtering.
Line 311 trusts any previous
selectedId, even when Lines 299-303 filter that contract out of the visible list. After selecting one option, a buyer can type a search that hides it and still confirm an invisible choice.🤖 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 `@packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx` around lines 298 - 313, The ContractSwitcher selection state can remain valid even after the selected contract is filtered out of the visible results. Update the logic around the search filtering and canConfirm calculation in ContractSwitcher so that a hidden selection is cleared or treated as invalid when it is no longer present in ordered/filtered, and only allow confirmation when selectedId matches an item still visible in the current filtered list.packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsx (1)
3-10: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse an interface for component props.
Line 3 exports
OrganizationDrawerFooterPropsas a type alias, but this component path requires a props interface. Switching this toexport interface OrganizationDrawerFooterPropskeeps the name while matching the repo contract.Based on learnings from coding guidelines,
packages/{components,core}/src/components/**/*.{ts,tsx}:TypeScript prop interfaces MUST be named {ComponentName}Props.🤖 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 `@packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsx` around lines 3 - 10, OrganizationDrawerFooterProps is currently declared as a type alias, but this component path expects a props interface. Update the OrganizationDrawerFooterProps declaration in OrganizationDrawerFooter to use an exported interface instead of a type alias, keeping the same name so it matches the repo’s component props contract.Source: Coding guidelines
♻️ Duplicate comments (2)
packages/core/discovery.config.js (1)
11-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winOnly ignore the missing-file case here.
This bare
catchstill hides syntax/runtime errors insidepackages/core/discovery.config.local.js, so broken local auth/refresh-token rewrites silently fall back to the base config.Proposed fix
try { finalConfig = deepmerge(finalConfig, require('./discovery.config.local')) -} catch { +} catch (error) { + if ( + error?.code !== 'MODULE_NOT_FOUND' || + !String(error?.message ?? '').includes('./discovery.config.local') + ) { + throw error + } // Optional local overrides (packages/core/discovery.config.local.js). }🤖 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 `@packages/core/discovery.config.js` around lines 11 - 15, The bare catch around require('./discovery.config.local') in discovery.config.js is swallowing real syntax/runtime failures as well as the missing-file case. Update the error handling so discovery.config.local is merged only when the module is absent, and rethrow any other exception from the local config load path; keep the existing deepmerge/finalConfig flow intact while narrowing the fallback behavior to missing-file-only.packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx (1)
274-279: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate Confirm on
useSwitchContract().enabled.Line 311 still computes
canConfirmwithout the hook’senabledflag, even thoughuseSwitchContract()exposes it. That leaves Confirm clickable when switching is unavailable and pushes users into the hook’s error path instead of disabling the action up front.Suggested fix
const { switchContract, loading: switching, error: switchError, + enabled: switchEnabled, } = useSwitchContract() @@ const canConfirm = - selectedId !== null && selectedId !== currentContract?.id && !switching + switchEnabled && + selectedId !== null && + selectedId !== currentContract?.id && + !switchingAlso applies to: 311-313
🤖 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 `@packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx` around lines 274 - 279, The confirm-state logic in ContractSwitcher still allows action even when useSwitchContract().enabled is false. Update the canConfirm computation and the Confirm button gating in ContractSwitcher to include the enabled flag alongside the existing session and validation checks, so the action is disabled up front when switching is unavailable. Use the enabled value returned by useSwitchContract() together with canConfirm/switchContract handling to keep the confirm flow consistent.
🧹 Nitpick comments (3)
packages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.ts (1)
7-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test to match the source filename.
This suite exercises
Query.availableContractsfrompackages/api/src/platforms/vtex/resolvers/query.ts, soavailableContracts.test.tsbreaks the repo rule that test files mirror the source filename. Rename it to matchquery.tsto stay consistent with the unit/integration naming scheme. As per coding guidelines,packages/**/*.test.{ts,tsx}:Test files MUST match the source file name with .test. infix.🤖 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 `@packages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.ts` around lines 7 - 226, The test suite name does not match the source file it covers, which breaks the repository naming rule. Rename the test file for Query.availableContracts so it mirrors the source filename query.ts with the .test. infix, and keep the existing test contents intact while updating any references needed to the new test filename.Source: Coding guidelines
packages/core/test/sdk/account/useAvailableContracts.browser.test.ts (1)
27-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the disabled and missing-unit cases into separate tests.
Lines 27-45 set
enabled = falseandunitId = ''together, so the suite never proves that theenabledflag alone setsdoNotRun: truewhen a valid org unit exists. A regression in that branch would still pass.🤖 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 `@packages/core/test/sdk/account/useAvailableContracts.browser.test.ts` around lines 27 - 45, Split the combined `useAvailableContracts` coverage into two separate tests so each branch is verified independently: one test should exercise `useAvailableContracts(false)` with a valid `mockUseSession` org unit to confirm `doNotRun: true` comes from the disabled flag, and another should keep the missing `unitId` case to confirm the query is skipped when `b2b.unitId` is empty. Use the existing `useAvailableContracts`, `mockUseSession`, and `mockUseQuery` setup to isolate each scenario and keep the assertions focused on the specific condition being tested.packages/core/test/components/account/ContractSwitcher.browser.test.tsx (1)
121-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a disabled-switch regression case here.
These confirm-state tests never mock
useSwitchContract()withenabled: false, so the current UI regression slips through. Add one case that selects an alternative contract and still expects Confirm to stay disabled when switching is unavailable.🤖 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 `@packages/core/test/components/account/ContractSwitcher.browser.test.tsx` around lines 121 - 142, Add a regression test in ContractSwitcher.browser.test.tsx that covers the unavailable-switching path by mocking useSwitchContract() with enabled set to false. Use the existing ContractSwitcher test setup to select a different contract (for example, the same alternative selection flow used in the current confirm-state tests), then assert that the Confirm button remains disabled even after selection. Keep the test grouped with the existing confirm-state cases so the behavior is verified in the ContractSwitcher component.
🤖 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 `@packages/core/discovery.config.local.example.js`:
- Around line 4-6: Clarify the local setup copy command by making its working
directory explicit: the current guidance in the example config only works from
packages/core, so update the copy instruction in
discovery.config.local.example.js to either state that it must be run from that
directory or switch it to a repo-root-relative command; use the existing
discovery.config.local.example.js copy guidance as the place to fix this.
In `@packages/core/src/sdk/account/changeContractToken.ts`:
- Around line 68-82: The switch-properties POST inside changeContractToken’s
request path can hang indefinitely and keep useSwitchContract stuck in loading.
Update the fetch call to use an AbortController with a timeout around the
/api/authenticator/storefront/credential/switch-properties request, and make
sure aborted requests are caught and rethrown as a ContractSwitchError so
useSwitchContract’s error handling can treat them consistently.
In `@packages/core/test/sdk/account/changeContractToken.test.ts`:
- Around line 1-3: The test suite in changeContractToken.test.ts is a
browser-oriented jsdom test and must use the browser test naming convention.
Rename the suite file to changeContractToken.browser.test.ts so it is correctly
picked up under the core browser test split, keeping the existing jsdom setup
and assertions intact.
In `@packages/core/test/sdk/account/clearPersistedSessionState.node.test.ts`:
- Around line 1-20: The test suite is using a .node.test.ts filename, which does
not match the required Node test naming convention for `@faststore/core`. Rename
this suite to clearPersistedSessionState.test.ts and keep the existing
clearPersistedSessionState test coverage intact; the relevant symbols to
preserve are clearPersistedSessionState, describe('clearPersistedSessionState
(node)'), and the idb-keyval mock setup.
In `@packages/core/test/sdk/account/clearPersistedSessionState.test.ts`:
- Around line 1-3: This test suite is currently named like a Node test even
though it forces jsdom and uses sessionStorage, so it needs to be moved into the
browser-test bucket. Rename the file to
clearPersistedSessionState.browser.test.ts and keep the existing
`@vitest-environment` jsdom setup in place so it follows the browser test naming
convention used by clearPersistedSessionState.
---
Outside diff comments:
In
`@packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx`:
- Around line 298-313: The ContractSwitcher selection state can remain valid
even after the selected contract is filtered out of the visible results. Update
the logic around the search filtering and canConfirm calculation in
ContractSwitcher so that a hidden selection is cleared or treated as invalid
when it is no longer present in ordered/filtered, and only allow confirmation
when selectedId matches an item still visible in the current filtered list.
In
`@packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsx`:
- Around line 3-10: OrganizationDrawerFooterProps is currently declared as a
type alias, but this component path expects a props interface. Update the
OrganizationDrawerFooterProps declaration in OrganizationDrawerFooter to use an
exported interface instead of a type alias, keeping the same name so it matches
the repo’s component props contract.
---
Duplicate comments:
In `@packages/core/discovery.config.js`:
- Around line 11-15: The bare catch around require('./discovery.config.local')
in discovery.config.js is swallowing real syntax/runtime failures as well as the
missing-file case. Update the error handling so discovery.config.local is merged
only when the module is absent, and rethrow any other exception from the local
config load path; keep the existing deepmerge/finalConfig flow intact while
narrowing the fallback behavior to missing-file-only.
In
`@packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx`:
- Around line 274-279: The confirm-state logic in ContractSwitcher still allows
action even when useSwitchContract().enabled is false. Update the canConfirm
computation and the Confirm button gating in ContractSwitcher to include the
enabled flag alongside the existing session and validation checks, so the action
is disabled up front when switching is unavailable. Use the enabled value
returned by useSwitchContract() together with canConfirm/switchContract handling
to keep the confirm flow consistent.
---
Nitpick comments:
In `@packages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.ts`:
- Around line 7-226: The test suite name does not match the source file it
covers, which breaks the repository naming rule. Rename the test file for
Query.availableContracts so it mirrors the source filename query.ts with the
.test. infix, and keep the existing test contents intact while updating any
references needed to the new test filename.
In `@packages/core/test/components/account/ContractSwitcher.browser.test.tsx`:
- Around line 121-142: Add a regression test in
ContractSwitcher.browser.test.tsx that covers the unavailable-switching path by
mocking useSwitchContract() with enabled set to false. Use the existing
ContractSwitcher test setup to select a different contract (for example, the
same alternative selection flow used in the current confirm-state tests), then
assert that the Confirm button remains disabled even after selection. Keep the
test grouped with the existing confirm-state cases so the behavior is verified
in the ContractSwitcher component.
In `@packages/core/test/sdk/account/useAvailableContracts.browser.test.ts`:
- Around line 27-45: Split the combined `useAvailableContracts` coverage into
two separate tests so each branch is verified independently: one test should
exercise `useAvailableContracts(false)` with a valid `mockUseSession` org unit
to confirm `doNotRun: true` comes from the disabled flag, and another should
keep the missing `unitId` case to confirm the query is skipped when `b2b.unitId`
is empty. Use the existing `useAvailableContracts`, `mockUseSession`, and
`mockUseQuery` setup to isolate each scenario and keep the assertions focused on
the specific condition being tested.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 385edbfa-e69c-4ca9-85ae-d027ddcf30d5
⛔ Files ignored due to path filters (2)
.gitignoreis excluded by none and included by nonespecs/contract-switcher.mdis excluded by none and included by none
📒 Files selected for processing (35)
packages/api/src/platforms/vtex/clients/commerce/index.tspackages/api/src/platforms/vtex/clients/commerce/types/Session.tspackages/api/src/platforms/vtex/resolvers/query.tspackages/api/src/platforms/vtex/resolvers/validateSession.tspackages/api/src/platforms/vtex/typeDefs/query.graphqlpackages/api/src/platforms/vtex/utils/contract.tspackages/api/test/unit/platforms/vtex/clients/commerce.test.tspackages/api/test/unit/platforms/vtex/resolvers/availableContracts.test.tspackages/api/test/unit/platforms/vtex/utils/contract.test.tspackages/core/discovery.config.jspackages/core/discovery.config.local.example.jspackages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsxpackages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/index.tspackages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawer.tsxpackages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsxpackages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerHeader.tsxpackages/core/src/components/account/Drawer/OrganizationDrawer/index.tspackages/core/src/components/account/Drawer/OrganizationDrawer/section.module.scsspackages/core/src/sdk/account/changeContractToken.tspackages/core/src/sdk/account/clearPersistedSessionState.tspackages/core/src/sdk/account/refreshToken.tspackages/core/src/sdk/account/useAvailableContracts.tspackages/core/src/sdk/account/useSwitchContract.tspackages/core/test/components/account/ContractSwitcher.browser.test.tsxpackages/core/test/components/account/OrganizationDrawer.browser.test.tsxpackages/core/test/components/account/OrganizationDrawerFooter.browser.test.tsxpackages/core/test/components/account/OrganizationDrawerHeader.browser.test.tsxpackages/core/test/components/account/doLogout.browser.test.tspackages/core/test/sdk/account/changeContractToken.test.tspackages/core/test/sdk/account/clearPersistedSessionState.node.test.tspackages/core/test/sdk/account/clearPersistedSessionState.test.tspackages/core/test/sdk/account/refreshToken.browser.test.tspackages/core/test/sdk/account/refreshToken.test.tspackages/core/test/sdk/account/useAvailableContracts.browser.test.tspackages/core/test/sdk/account/useSwitchContract.browser.test.ts
💤 Files with no reviewable changes (3)
- packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/index.ts
- packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerHeader.tsx
- packages/core/src/components/account/Drawer/OrganizationDrawer/section.module.scss
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/core/src/sdk/account/clearPersistedSessionState.ts
- packages/api/src/platforms/vtex/resolvers/validateSession.ts
- packages/api/src/platforms/vtex/typeDefs/query.graphql
- packages/core/src/sdk/account/useAvailableContracts.ts
- packages/api/src/platforms/vtex/resolvers/query.ts
- packages/api/src/platforms/vtex/clients/commerce/index.ts
- packages/api/src/platforms/vtex/clients/commerce/types/Session.ts
- packages/api/src/platforms/vtex/utils/contract.ts
- packages/core/src/sdk/account/refreshToken.ts
Apply auth cookie timeout, tighten confirm gating, fix local config error handling, and align test file naming with core browser/node conventions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/test/sdk/account/clearPersistedSessionState.test.ts (1)
1-17: 🎯 Functional Correctness | 🔵 TrivialPin this suite to the Node environment
The
packages/core/vitest.config.tsdefines anodetest project without explicitly settingenvironment: 'node', relying on Vitest's implicit default. Since other tests in this directory (e.g.,useSetPassword.test.ts) use explicit@vitest-environment jsdompragmas and thebrowserproject explicitly overrides tojsdom, omitting the pragma here creates ambiguity. If the global default shifts or workspace settings prioritizejsdom, this test will run in a browser context wheresessionStorageexists, failing to cover the intended Node-specific logic.Add
@vitest-environment nodeto the top of the file to guarantee deterministic execution:/** `@vitest-environment` node */🤖 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 `@packages/core/test/sdk/account/clearPersistedSessionState.test.ts` around lines 1 - 17, This test suite is implicitly relying on the default Vitest environment, which can make it run in the wrong context; pin clearPersistedSessionState.test.ts to the Node environment explicitly. Add the Node test pragma at the top of the file so the clearPersistedSessionState suite always exercises the sessionStorage-unavailable path instead of inheriting jsdom from workspace defaults or other projects.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/core/test/sdk/account/clearPersistedSessionState.test.ts`:
- Around line 1-17: This test suite is implicitly relying on the default Vitest
environment, which can make it run in the wrong context; pin
clearPersistedSessionState.test.ts to the Node environment explicitly. Add the
Node test pragma at the top of the file so the clearPersistedSessionState suite
always exercises the sessionStorage-unavailable path instead of inheriting jsdom
from workspace defaults or other projects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f88a6ba5-53bf-4657-9a38-403c2f848f16
📒 Files selected for processing (11)
packages/api/test/unit/platforms/vtex/resolvers/query.test.tspackages/core/discovery.config.jspackages/core/discovery.config.local.example.jspackages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsxpackages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsxpackages/core/src/sdk/account/changeContractToken.tspackages/core/test/components/account/ContractSwitcher.browser.test.tsxpackages/core/test/sdk/account/changeContractToken.browser.test.tspackages/core/test/sdk/account/clearPersistedSessionState.browser.test.tspackages/core/test/sdk/account/clearPersistedSessionState.test.tspackages/core/test/sdk/account/useAvailableContracts.browser.test.ts
💤 Files with no reviewable changes (1)
- packages/api/test/unit/platforms/vtex/resolvers/query.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/core/src/components/account/Drawer/OrganizationDrawer/OrganizationDrawerFooter.tsx
- packages/core/discovery.config.js
- packages/core/test/sdk/account/useAvailableContracts.browser.test.ts
- packages/core/discovery.config.local.example.js
- packages/core/src/components/account/Drawer/OrganizationDrawer/ContractSwitcher/ContractSwitcher.tsx
- packages/core/test/components/account/ContractSwitcher.browser.test.tsx
- packages/core/src/sdk/account/changeContractToken.ts
ArthurTriis1
approved these changes
Jun 26, 2026
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
6 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
Implements Phase 2 — Contract Switching and Selection (REQ-03 to REQ-06): B2B buyers can view the active contract and switch between contracts associated with their Organization Unit from the FastStore account drawer.
Spec:
specs/contract-switcher.md(Approved). Figma node103-5434.What's included
@faststore/api) —availableContracts(orgUnitId)readsshopper.availableContractsfrom the VTEX session (no buyer-portal BFF). Active contract is resolved fromshopper.activeContractId→authentication.customerId→profile.id(not staleisCurrentflags alone). Private cache disabled (maxAge: 0).@faststore/core/src/sdk/account) —useAvailableContracts(on-demand list) anduseSwitchContract(atomic switch:switch-properties→ refresh token → clear persisted session → full page reload).ContractSwitchersub-view insideOrganizationDrawer(underaccount/Drawer/after dev merge), searchable governed list, loading/empty/error states, Change CTA in drawer header. Confirm shows spinner only (no loading label).discovery.config.local.jsforb2bfaststoredevoverrides;discovery.config.jsmerges it and adds dev-only rewrites for/api/authenticator/*and/api/vtexid/*(WebOps handles this in production)..browser.test.*convention).Preview:
Screen.Recording.2026-06-26.at.10.53.35.mov
Contract switch flow
POST /api/authenticator/storefront/credential/switch-properties?an={account}with{ properties: { customerId } }POST /api/vtexid/refreshtoken/webstore— re-issues JWT with updatedcustomerIdwindow.location.reload()— full commercial context resetReview feedback addressed
shopper.availableContracts, aligned with Identity session shape.contractNamefallback; Props exported from barrel; browser test naming.MyAccountDrawer→Drawerrename and My Account CMS refactor.Security & data handling
Notes for reviewers
discovery.config.default.jsstays on starter defaults; local account overrides live indiscovery.config.local.js(committed for team dev/testing).next-env.d.ts,tsconfig.json) and local build artifacts are intentionally excluded from commits.Test plan
pnpm turbo run test --filter=@faststore/api --filter=@faststore/corepassespnpm lintandpnpm buildpasscustomerIdupdated after refresh){storeId}.myvtex.comSummary by CodeRabbit
New Features
Bug Fixes