add: Passar o segmento via props#1516
Conversation
Tagging OptionsShould a new tag be published when this PR is merged?
|
|
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:
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Action as Cart Action
participant Context as AppContext (ctx)
participant Segment as Segment
participant API as VTEX API
Client->>Action: invoke action(props including optional sc)
Action->>Context: read ctx.allowMixedSegments, ctx.salesChannel
Action->>Segment: read segment?.payload.channel
alt sc provided
Action->>API: call API with sc (props.sc)
else sc not provided and ctx.allowMixedSegments true
Action->>API: call API with sc = segment?.payload.channel
else
Action->>API: call API with sc = ctx.salesChannel
end
API-->>Action: response
Action-->>Client: return response.json()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
Co-authored-by: Gabriel Matos Boubee <108771428+gabrielMatosBoubee@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="vtex/actions/cart/simulation.ts">
<violation number="1" location="vtex/actions/cart/simulation.ts:39">
P0: Operator precedence bug: the nullish coalescing operator (`??`) has higher precedence than the ternary operator (`?:`), so this expression is evaluated as `(sc ?? ctx.allowMixedSegments) ? ... : ...` instead of `sc ?? (... ? ... : ...)`. When `sc` is provided, it will be used as the ternary condition rather than as the actual `sc` value, which defeats the entire purpose of this PR. Add parentheses to fix the precedence.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@vtex/actions/cart/addItems.ts`:
- Around line 33-46: The call to vcsDeprecated POST uses sc fallback that misses
ctx.salesChannel, so update the sc argument in addItems (where props.sc is read)
to use sc ?? segment?.payload.channel ?? ctx.salesChannel (matching
simulation.ts) to ensure a sales channel is always sent; also update the
Props.sc typing or coerce it to a number before passing to vcsDeprecated to
align with the VTEX OpenAPI expectation.
- Around line 16-20: The Props interface's sc field is currently typed
inconsistently (sc?: string in the Props interface in addItems.ts vs sc?: number
in simulation.ts); change sc's type in the Props interface to sc?: string |
number to match across cart actions, and ensure any place that sends sc (e.g.,
where the request payload is built / send call) coerces it with String(sc)
before transmission so callers can pass numeric or string values without type
errors; update any related usages of Props/sc to accept the union type.
| export interface Props { | ||
| orderItems: Item[]; | ||
| allowedOutdatedData?: Array<"paymentData">; | ||
| sc?: string; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n vtex/actions/cart/addItems.tsRepository: deco-cx/apps
Length of output: 2284
🏁 Script executed:
fd . vtex/actions/cart -type f -name "*.ts" | head -20Repository: deco-cx/apps
Length of output: 226
🏁 Script executed:
fd . vtex/actions/cart -type f -name "*.ts"Repository: deco-cx/apps
Length of output: 226
🏁 Script executed:
git ls-files vtex/actions/cart/*.tsRepository: deco-cx/apps
Length of output: 627
🏁 Script executed:
cat -n vtex/actions/cart/simulation.tsRepository: deco-cx/apps
Length of output: 1796
🏁 Script executed:
rg -n "sc\?" vtex/actions/cart -A1 -B1Repository: deco-cx/apps
Length of output: 365
🏁 Script executed:
rg -n "sc:" vtex/actions/cart -B2 -A2Repository: deco-cx/apps
Length of output: 4285
🏁 Script executed:
cat -n vtex/utils/segment.ts | head -50Repository: deco-cx/apps
Length of output: 1586
🏁 Script executed:
rg -n "interface Segment" vtex/utils/types.ts -A20Repository: deco-cx/apps
Length of output: 778
🏁 Script executed:
rg -n "salesChannel" vtex/mod.ts -B5 -A5Repository: deco-cx/apps
Length of output: 1017
🏁 Script executed:
rg -n "export interface Props" vtex/actions/cart -A10 | grep -E "(interface Props|sc\?)"Repository: deco-cx/apps
Length of output: 994
Align sc type across cart actions to avoid type inconsistencies.
sc is typed as string in addItems.ts but simulation.ts accepts sc?: number. Both fallback to segment?.payload.channel (which is string). Standardizing to string | number with explicit String(...) coercion on send would prevent callers from hitting type errors when using similar actions with numeric values.
♻️ Suggested normalization
export interface Props {
orderItems: Item[];
allowedOutdatedData?: Array<"paymentData">;
- sc?: string;
+ sc?: string | number;
}
@@
- sc: sc ?? segment?.payload.channel,
+ sc: sc != null ? String(sc) : segment?.payload.channel,🤖 Prompt for AI Agents
In `@vtex/actions/cart/addItems.ts` around lines 16 - 20, The Props interface's sc
field is currently typed inconsistently (sc?: string in the Props interface in
addItems.ts vs sc?: number in simulation.ts); change sc's type in the Props
interface to sc?: string | number to match across cart actions, and ensure any
place that sends sc (e.g., where the request payload is built / send call)
coerces it with String(sc) before transmission so callers can pass numeric or
string values without type errors; update any related usages of Props/sc to
accept the union type.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@vtex/actions/cart/addItems.ts`:
- Around line 45-48: The ternary currently uses (sc ?? ctx.allowMixedSegments)
as the condition which ignores an explicit sc value; change the expression so sc
is used when defined by applying the nullish coalescing first and only falling
back to the conditional on ctx.allowMixedSegments, e.g. replace the existing
ternary with sc ?? (ctx.allowMixedSegments ? segment?.payload.channel :
ctx.salesChannel) so that explicit sc values are returned and only when sc is
null/undefined the allowMixedSegments branch selects segment?.payload.channel or
ctx.salesChannel.
In `@vtex/actions/cart/getInstallment.ts`:
- Around line 34-37: The current assignment for sc incorrectly uses the ternary
with the nullish coalescing, causing an explicitly provided sc to be ignored;
change the expression so sc is used when defined and only fall back to choosing
between segment?.payload.channel and ctx.salesChannel based on
ctx.allowMixedSegments (i.e. use sc ?? (ctx.allowMixedSegments ?
segment?.payload.channel : ctx.salesChannel)), updating the sc assignment in
getInstallment.ts where sc, ctx.allowMixedSegments, segment?.payload.channel and
ctx.salesChannel are referenced.
In `@vtex/actions/cart/removeItemAttachment.ts`:
- Around line 65-68: The ternary uses (sc ?? ctx.allowMixedSegments) as the
condition so a provided sc never becomes the returned value; change the
expression to prefer sc first and only if sc is null/undefined evaluate the
existing branch — e.g. replace the current expression with one that returns sc
when defined, otherwise returns ctx.allowMixedSegments ?
segment?.payload.channel : ctx.salesChannel; update the assignment for sc in the
same scope where sc, ctx.allowMixedSegments, segment?.payload.channel and
ctx.salesChannel are referenced.
In `@vtex/actions/cart/updateAttachment.ts`:
- Around line 47-50: The current expression uses the ternary before the nullish
coalescing so a provided sc is effectively ignored; change the logic so sc is
used when defined, otherwise evaluate the ternary based on
ctx.allowMixedSegments to pick between segment?.payload.channel and
ctx.salesChannel (i.e., make the ternary the RHS fallback of the sc nullish
coalescing). Update the expression in updateAttachment.ts where sc,
ctx.allowMixedSegments, segment?.payload.channel and ctx.salesChannel are
referenced to ensure sc wins when present and the ternary only applies as the
fallback.
In `@vtex/actions/cart/updateCoupons.ts`:
- Around line 33-36: The sc assignment uses (sc ?? ctx.allowMixedSegments) ? ...
: ... which causes an operator-precedence bug that ignores an explicitly
provided sc; update the expression in updateCoupons.ts so the nullish-coalescing
applies to the entire ternary result, e.g. replace the current line with: sc ??
(ctx.allowMixedSegments ? segment?.payload.channel : ctx.salesChannel) to ensure
explicit sc is honored (referencing the sc variable, ctx.allowMixedSegments,
segment?.payload.channel and ctx.salesChannel).
In `@vtex/actions/cart/updateItemAttachment.ts`:
- Around line 58-69: The request currently ignores an explicit sc value because
the ternary tests sc for truthiness but always returns segment?.payload.channel;
update the sc parameter in the POST call to use the explicit sc first (if
provided), otherwise fall back to the existing behavior: if
ctx.allowMixedSegments is true use segment?.payload.channel, else use
ctx.salesChannel; adjust the sc expression in the vcsDeprecated["POST
/api/checkout/..."] call accordingly so passing sc="7" will be honored.
In `@vtex/actions/cart/updateItemPrice.ts`:
- Around line 35-38: The ternary is being parsed with the ?? operator
incorrectly so the explicit property sc is ignored; fix the expression in
updateItemPrice.ts by wrapping the nullish-coalescing check in an extra set of
parentheses before the ? operator (i.e. ensure the right-hand side of the sc:
property is ((sc ?? ctx.allowMixedSegments) ? segment?.payload.channel :
ctx.salesChannel)), so the decision uses (sc ?? ctx.allowMixedSegments) as
intended; update the expression that references sc, ctx.allowMixedSegments,
segment?.payload.channel and ctx.salesChannel accordingly.
In `@vtex/actions/cart/updateProfile.ts`:
- Around line 33-36: The ternary is affected by operator-precedence with ?? so
an explicit sc value gets ignored; replace the current left-hand expression with
an explicit presence check—use (typeof sc !== 'undefined' ? sc :
ctx.allowMixedSegments) as the condition in the ternary that sets sc, i.e.
update the expression that currently uses (sc ?? ctx.allowMixedSegments) ?
segment?.payload.channel : ctx.salesChannel to use the typeof check so sc is
honored correctly.
🧹 Nitpick comments (2)
vtex/actions/cart/simulation.ts (1)
11-17: Alignsctype with other cart actions.Other actions (and segment payloads) treat
scas a string; usingnumberhere forces casts and drifts the API. Consider usingstring(orstring | number) for consistency.♻️ Suggested type alignment
- sc?: number; + sc?: string;vtex/actions/cart/updateUser.ts (1)
16-23: Guard against undefined props for backward compatibility.This action previously required no props. If it’s invoked without props, destructuring will throw. Consider defaulting to
{}.🛡️ Suggested guard
- const { sc } = props; + const { sc } = props ?? {};
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@vtex/actions/cart/removeItems.ts`:
- Around line 32-35: The current ternary in removeItems treats a provided sc as
a boolean and returns segment?.payload.channel instead of the sc value; change
the expression so when sc is supplied it is used directly, otherwise fall back
to ctx.allowMixedSegments ? segment?.payload.channel : ctx.salesChannel (e.g.
use sc ?? (ctx.allowMixedSegments ? segment?.payload.channel : ctx.salesChannel)
or an equivalent conditional) to ensure the provided sc overrides the other
branches.
In `@vtex/actions/cart/updateUser.ts`:
- Around line 32-35: The ternary currently only checks sc for truthiness but
never returns it; change the expression to return sc when present (e.g., use the
nullish-coalescing pattern) so sc is passed through: replace the current block
that computes sc with something like sc ?? (ctx.allowMixedSegments ?
segment?.payload.channel : ctx.salesChannel), ensuring sc is used directly when
defined and falling back to segment?.payload.channel or ctx.salesChannel
otherwise.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@vtex/actions/cart/removeItemAttachment.ts`:
- Line 15: The sc query param is typed incorrectly as number; change its
declaration from sc?: number to sc?: string in the cart remove-item attachment
action (look for the sc field in the request/params interface in
removeItemAttachment.ts) so it matches other cart actions (updateProfile.ts,
updateUser.ts, updateItemPrice.ts) and the VTEX API; after changing the type,
run typechecks to ensure callers passing string values like "7" compile without
errors and update any tests or mock objects that declared sc as a number.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="vtex/actions/cart/getInstallment.ts">
<violation number="1" location="vtex/actions/cart/getInstallment.ts:35">
P2: Using a truthy check for `sc` changes behavior vs the previous nullish fallback. If callers pass an explicit but falsy value (e.g., empty string), it will now be ignored and replaced by the fallback channel. Preserve nullish-coalescing semantics to avoid unintended channel overrides.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| orderFormId, | ||
| paymentSystem, | ||
| sc: | ||
| sc ? sc : |
There was a problem hiding this comment.
P2: Using a truthy check for sc changes behavior vs the previous nullish fallback. If callers pass an explicit but falsy value (e.g., empty string), it will now be ignored and replaced by the fallback channel. Preserve nullish-coalescing semantics to avoid unintended channel overrides.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At vtex/actions/cart/getInstallment.ts, line 35:
<comment>Using a truthy check for `sc` changes behavior vs the previous nullish fallback. If callers pass an explicit but falsy value (e.g., empty string), it will now be ignored and replaced by the fallback channel. Preserve nullish-coalescing semantics to avoid unintended channel overrides.</comment>
<file context>
@@ -32,8 +32,10 @@ const action = async (
sc:
- sc ??
- (ctx.allowMixedSegments ? segment?.payload.channel : ctx.salesChannel),
+ sc ? sc :
+ (ctx.allowMixedSegments
+ ? segment?.payload.channel
</file context>
| sc ? sc : | |
| sc ?? | |
| (ctx.allowMixedSegments | |
| ? segment?.payload.channel | |
| : ctx.salesChannel) |
3 participants
Ajuste em passar o segmento via props, por conta que quando loga puxa do seguemento 1, sendo que está configurado o 7 no admin. Tendo a opção de passar via props, resolver esse problema.
Summary by CodeRabbit
New Features
Bug Fixes