✨ Add CNPL (Care Now Pay Later) flow support by introducing new API i…#1
Open
borquezmartin wants to merge 2 commits intomainfrom
Open
✨ Add CNPL (Care Now Pay Later) flow support by introducing new API i…#1borquezmartin wants to merge 2 commits intomainfrom
borquezmartin wants to merge 2 commits intomainfrom
Conversation
…ntegration, updating environment variables, and enhancing checkout and appointment confirmation components for improved user experience.
📝 WalkthroughWalkthroughAdds CNPL (Care Now Pay Later) support: new env vars, atoms, API route to create SkipPay orders, extended public-key modal and hooks, and CNPL-aware UI and pricing logic in checkout and appointment confirmation components. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as App UI
participant State as State (atoms/hooks)
participant API as /api/create-order
participant SkipPay as SkipPay API
User->>UI: Selects CNPL and proceeds
UI->>State: set workflowType, set clientSecret, set consultaCosto
State-->>UI: persisted settings
User->>UI: Confirm appointment / pay
UI->>API: POST patient + clientSecret + amount
API->>API: validate & build payload
API->>SkipPay: POST create order (Authorization: clientSecret)
SkipPay-->>API: returns order hash/details
API-->>UI: CreateOrderResponse (hash, reference, status)
UI->>UI: build widget URL with order_token
UI->>User: open payment widget (CNPL)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/features/checkout.tsx (1)
36-45:⚠️ Potential issue | 🟠 MajorCNPL “Total” is only the upfront payment.
In the CNPL branch, Lines 37-39 return just the amount due today, but Lines 135-141 still label it as
Total. That understates the customer's full obligation; this should distinguish “pay now” from the remaining 70%.Also applies to: 135-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/features/checkout.tsx` around lines 36 - 45, calculateTotal currently returns only the upfront CNPL amount when isCnpl is true (using cnplPayNow + platformFee + cnplCommission), but the UI still labels that value as "Total"; update the logic to clearly separate "pay now" from "total obligation": either (A) change calculateTotal to return the full customer obligation (cnplPayNow + remainingCnplBalance + platformFee + cnplCommission) and add a new calculatePayNow() that returns cnplPayNow + platformFee + cnplCommission, or (B) keep calculateTotal as full obligation and introduce calculatePayNow() for the upfront amount; then update the UI rendering (where the component displays the “Total” label) to show both values when isCnpl is true (use calculatePayNow() for the "Pay now" label and calculateTotal() for "Total obligation"), ensuring variables referenced are cnplPayNow, platformFee, cnplCommission, basePrice, reimbursementFee, isCnpl, isChecked, and isSubscribed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/atoms/simulation-settings.ts`:
- Line 23: clientSecretAtom is currently persisted via atomWithStorage which
writes the SkipPay client secret to browser storage; replace this with a
non-persistent in-memory solution (e.g., use atom instead of atomWithStorage for
clientSecretAtom) and remove any client-side flows that require storing the
secret. Update consumers to obtain auth from the server-side flow already
supported in create-order.ts (env.SKIPAY_CLIENT_SECRET) or request a token from
a server endpoint rather than reading clientSecretAtom from storage. Ensure no
other atoms or helpers persist the secret.
In `@src/components/features/appointment-confirmation.tsx`:
- Around line 39-53: The effect that defines initializeGokeiWidget currently
closes over clientSecret and calls createOrder(clientSecret, patient) but does
not list clientSecret in its dependency array; update the useEffect that
contains initializeGokeiWidget to include clientSecret (alongside existing
dependencies like patient and workflowType) so the effect re-runs when the
atom-backed clientSecret changes and CNPL order creation uses the correct
secret.
In `@src/components/features/public-key-modal.tsx`:
- Around line 92-95: The canSave guard only checks keys/secrets and misses
validating the paid reimbursement amount, so update the save-enabling logic
(where isCnpl, isKeyMissing, isSecretMissing, canSave are computed) to also
validate the reimbursement number when draftWorkflowType === "paid": ensure the
reimbursement input (the state holding the fee) is not empty/NaN, is parseable
as a number, and is >= 0 (or meets your minimum), and include that boolean in
canSave; also enforce the same validation in the save/submit handler that
persists the modal so an invalid fee cannot be saved even if UI state was
tampered with.
- Around line 64-67: The modal is incorrectly storing a server-only CNPL secret
in client state (draftClientSecret, setDraftClientSecret and related state usage
in public-key-modal.tsx), which exposes it to browser memory; remove any
client-side state for the CNPL secret (including draftClientSecret,
setDraftClientSecret and any UI inputs that persist it) and instead have the UI
query a server-side API or config endpoint that returns only capability/flag
information (e.g., "cnplEnabled" or masked presence) and use that to drive
draftWorkflowType and save behavior; update save/submit logic to call a server
route that validates/stores the CNPL secret on the server (no raw secret sent to
or stored in client state) and remove any persistence of the raw secret when
switching flows.
- Around line 29-35: The icon-only tooltip trigger button (uses Info and toggles
open via setOpen/open) and the secret-visibility toggle (the icon-only button
around show/hide secret state, lines ~221-227) lack accessible names and state;
add aria-label attributes (e.g., aria-label="Show info" or "Hide info" for the
tooltip trigger, and aria-label="Show secret" / "Hide secret" for the visibility
toggle) and expose the toggle state using aria-pressed on both buttons bound to
their respective state values (open for the Info button, and the visibility
state—e.g., showSecret or similar—for the secret toggle) so assistive tech
announces meaningful labels and current pressed state.
In `@src/pages/api/create-order.ts`:
- Around line 50-52: The logs in create-order.ts are exposing sensitive
data—redact the bearer secret and any PII before logging: replace the current
console.log calls that reference clientSecret and requestBody with safe variants
that mask clientSecret (e.g., show only fixed-length masked suffix/prefix like
****) and remove or redact PII fields (RUT, email, phone) from requestBody
before stringifying; alternatively log only non-sensitive metadata (HTTP method,
endpoint, request size or presence of fields) to preserve debugging context
without leaking credentials or personal data.
- Around line 23-27: The code destructures clientSecret/bodySecret, patient and
totalAmount from req.body without runtime checks; add explicit validation in the
create-order handler to verify req.body.patient is an object and patient.name
(and other required patient fields like rut/email/phone_number) are non-empty
strings and that totalAmount is a finite number (or coerce/parse it and check >
0) before using them; if validation fails, return a 400 response with an error
message instead of proceeding to access patient.name or stringifying invalid
totals. Ensure the checks reference the same identifiers (patient, patient.name,
totalAmount, bodySecret/req.body) so you validate the exact values used later
(lines noted 34–48) and short-circuit early on invalid input.
- Around line 29-32: The code uses nullish coalescing for clientSecret (const
clientSecret = bodySecret ?? env.SKIPAY_CLIENT_SECRET;) which won't fall back
when bodySecret is an empty string; change this to use logical OR so empty
strings also fall back (const clientSecret = bodySecret ||
env.SKIPAY_CLIENT_SECRET;), keeping the existing validation (if (!clientSecret)
...) intact; reference symbols: clientSecret, bodySecret,
env.SKIPAY_CLIENT_SECRET in create-order.ts.
- Around line 54-70: Wrap the fetch call that assigns const response in the
create-order route handler in a try/catch to catch network/transport errors
(DNS, timeout, socket) and ensure the endpoint always returns JSON; on catch,
log the error (including error.message) and return a JSON error response (e.g.
res.status(502).json({ error: "Failed to create order", detail: error.message
})) so callers expecting JSON (and code using CreateOrderResponse) don’t throw.
Ensure the existing response.ok branch remains unchanged and only network
failures are handled by the new catch.
---
Outside diff comments:
In `@src/components/features/checkout.tsx`:
- Around line 36-45: calculateTotal currently returns only the upfront CNPL
amount when isCnpl is true (using cnplPayNow + platformFee + cnplCommission),
but the UI still labels that value as "Total"; update the logic to clearly
separate "pay now" from "total obligation": either (A) change calculateTotal to
return the full customer obligation (cnplPayNow + remainingCnplBalance +
platformFee + cnplCommission) and add a new calculatePayNow() that returns
cnplPayNow + platformFee + cnplCommission, or (B) keep calculateTotal as full
obligation and introduce calculatePayNow() for the upfront amount; then update
the UI rendering (where the component displays the “Total” label) to show both
values when isCnpl is true (use calculatePayNow() for the "Pay now" label and
calculateTotal() for "Total obligation"), ensuring variables referenced are
cnplPayNow, platformFee, cnplCommission, basePrice, reimbursementFee, isCnpl,
isChecked, and isSubscribed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 806795ac-197f-40df-874a-22e4fa2c3013
📒 Files selected for processing (10)
.env.example.gitignorenext.config.jssrc/atoms/simulation-settings.tssrc/components/features/appointment-confirmation.tsxsrc/components/features/checkout.tsxsrc/components/features/public-key-modal.tsxsrc/env.jssrc/hooks/use-public-key.tssrc/pages/api/create-order.ts
💤 Files with no reviewable changes (1)
- next.config.js
| "workflow-type", | ||
| "standard", | ||
| ); | ||
| export const clientSecretAtom = atomWithStorage<string>("skipay-client-secret", ""); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the simulation-settings.ts file
fd simulation-settings.ts -x cat -n {}Repository: gokeigo/spot-integration-example
Length of output: 1020
🏁 Script executed:
# Check what atomWithStorage does and how clientSecretAtom is used
rg -A 2 -B 2 "clientSecretAtom" --type ts --type tsxRepository: gokeigo/spot-integration-example
Length of output: 103
🏁 Script executed:
# Examine the create-order.ts file, especially around line 29
fd create-order.ts -x cat -n {}Repository: gokeigo/spot-integration-example
Length of output: 2703
🏁 Script executed:
# Check the use-public-key.ts hook to understand clientSecretAtom usage
fd use-public-key.ts -x cat -n {}Repository: gokeigo/spot-integration-example
Length of output: 1234
🏁 Script executed:
# Verify atomWithStorage behavior from jotai/utils documentation or package.json
cat -n package.json | head -50Repository: gokeigo/spot-integration-example
Length of output: 1927
🏁 Script executed:
# Search for any sensitive data handling patterns in the codebase
rg "atomWithStorage|clientSecret" --type ts --type tsx -B 1 -A 1Repository: gokeigo/spot-integration-example
Length of output: 103
🏁 Script executed:
rg "usePublicKey|clientSecret" --type ts -B 2 -A 2 | head -100Repository: gokeigo/spot-integration-example
Length of output: 6906
🏁 Script executed:
# Search for where clientSecret might be sent to the API
rg "clientSecret" -B 3 -A 3Repository: gokeigo/spot-integration-example
Length of output: 8309
Do not persist the SkipPay client secret in browser storage.
atomWithStorage makes this Bearer credential readable from localStorage on every load. Since create-order.ts already supports server-side authentication via env.SKIPAY_CLIENT_SECRET, client-side credentials should remain in-memory only or be sourced from the server.
🔒 Proposed fix
-export const clientSecretAtom = atomWithStorage<string>("skipay-client-secret", "");
+export const clientSecretAtom = atom<string>("");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const clientSecretAtom = atomWithStorage<string>("skipay-client-secret", ""); | |
| export const clientSecretAtom = atom<string>(""); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/atoms/simulation-settings.ts` at line 23, clientSecretAtom is currently
persisted via atomWithStorage which writes the SkipPay client secret to browser
storage; replace this with a non-persistent in-memory solution (e.g., use atom
instead of atomWithStorage for clientSecretAtom) and remove any client-side
flows that require storing the secret. Update consumers to obtain auth from the
server-side flow already supported in create-order.ts (env.SKIPAY_CLIENT_SECRET)
or request a token from a server endpoint rather than reading clientSecretAtom
from storage. Ensure no other atoms or helpers persist the secret.
Comment on lines
+39
to
+53
| const { integrationType, publicKey, workflowType, clientSecret } = usePublicKey(); | ||
|
|
||
| useEffect(() => { | ||
| const initializeGokeiWidget = async () => { | ||
| if (patient.wantsReimbursement) { | ||
| try { | ||
| const nameParts = patient.name.split(" "); | ||
| const surname = nameParts.slice(1).join(" "); | ||
| const firstName = nameParts[0]; | ||
| const isCnpl = workflowType === "cnpl"; | ||
| if (!patient.wantsReimbursement && !isCnpl) return; | ||
|
|
||
| const response = await fetch( | ||
| `${env.NEXT_PUBLIC_GOKEI_API_URL}/widget?public_key=${publicKey}`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| rut: patient.rut, | ||
| user_data: { | ||
| name: firstName, | ||
| surname, | ||
| rut: patient.rut, | ||
| email: patient.email, | ||
| phone_number: patient.phone_number, | ||
| }, | ||
| }), | ||
| try { | ||
| const nameParts = patient.name.split(" "); | ||
| const surname = nameParts.slice(1).join(" "); | ||
| const firstName = nameParts[0] ?? ""; | ||
|
|
||
| let orderToken: string | undefined; | ||
| if (isCnpl) { | ||
| orderToken = await createOrder(clientSecret, patient); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "appointment-confirmation.tsx" --type fRepository: gokeigo/spot-integration-example
Length of output: 128
🏁 Script executed:
cat -n src/components/features/appointment-confirmation.tsx | head -100Repository: gokeigo/spot-integration-example
Length of output: 4369
🏁 Script executed:
fd "use-public-key" --type fRepository: gokeigo/spot-integration-example
Length of output: 103
🏁 Script executed:
cat -n src/hooks/use-public-key.tsRepository: gokeigo/spot-integration-example
Length of output: 1234
🏁 Script executed:
fd "simulation-settings" --type fRepository: gokeigo/spot-integration-example
Length of output: 108
🏁 Script executed:
cat -n src/atoms/simulation-settings.tsRepository: gokeigo/spot-integration-example
Length of output: 1020
Include clientSecret in the widget-init effect dependencies.
createOrder(clientSecret, patient) on line 53 closes over the current secret, but line 93 omits clientSecret from the dependency array. Since clientSecretAtom is backed by atomWithStorage and initializes to "", the effect can capture an empty secret on first render. If the real value arrives later from localStorage, the effect won't re-run, leaving CNPL order creation permanently stuck with an empty secret.
🔁 Proposed fix
- }, [patient, publicKey, workflowType]);
+ }, [patient, publicKey, workflowType, clientSecret]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/features/appointment-confirmation.tsx` around lines 39 - 53,
The effect that defines initializeGokeiWidget currently closes over clientSecret
and calls createOrder(clientSecret, patient) but does not list clientSecret in
its dependency array; update the useEffect that contains initializeGokeiWidget
to include clientSecret (alongside existing dependencies like patient and
workflowType) so the effect re-runs when the atom-backed clientSecret changes
and CNPL order creation uses the correct secret.
Comment on lines
+29
to
+35
| <button | ||
| type="button" | ||
| onClick={() => setOpen(!open)} | ||
| className="text-gray-400 hover:text-gray-600" | ||
| > | ||
| <Info className="h-3.5 w-3.5" /> | ||
| </button> |
There was a problem hiding this comment.
Add accessible names to the icon-only buttons.
The tooltip trigger and secret-visibility toggle render only icons, so assistive tech will announce just “button”. Add aria-label, and expose the toggle state with aria-pressed.
♿ Suggested accessibility fix
<button
type="button"
onClick={() => setOpen(!open)}
+ aria-label="Mostrar ayuda"
className="text-gray-400 hover:text-gray-600"
>
...
<button
type="button"
onClick={() => setShowSecret((v) => !v)}
+ aria-label={showSecret ? "Ocultar client secret" : "Mostrar client secret"}
+ aria-pressed={showSecret}
className="absolute right-2.5 top-1/2 -translate-y-1/2 text-gray-400 hover:text-gray-600"
>Also applies to: 221-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/features/public-key-modal.tsx` around lines 29 - 35, The
icon-only tooltip trigger button (uses Info and toggles open via setOpen/open)
and the secret-visibility toggle (the icon-only button around show/hide secret
state, lines ~221-227) lack accessible names and state; add aria-label
attributes (e.g., aria-label="Show info" or "Hide info" for the tooltip trigger,
and aria-label="Show secret" / "Hide secret" for the visibility toggle) and
expose the toggle state using aria-pressed on both buttons bound to their
respective state values (open for the Info button, and the visibility
state—e.g., showSecret or similar—for the secret toggle) so assistive tech
announces meaningful labels and current pressed state.
Comment on lines
+64
to
+67
| const [draftWorkflowType, setDraftWorkflowType] = useState<"standard" | "cnpl">(workflowType); | ||
| const [showSecret, setShowSecret] = useState(false); | ||
| const [draftKey, setDraftKey] = useState(publicKey ?? ""); | ||
| const [draftClientSecret, setDraftClientSecret] = useState(clientSecret); |
There was a problem hiding this comment.
Don't collect a server-side CNPL secret in the browser.
This modal asks for a credential that the copy on Line 206 says is “never exposed to the browser”, but these changes do exactly that by storing it in client state and persisting it on save. That makes the secret available to browser memory/devtools and leaves it lingering even after switching back to the standard flow. Please keep this value in server-only config/API routes and let the UI reference a server-side environment/capability instead of persisting the raw secret here.
Also applies to: 97-100, 201-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/features/public-key-modal.tsx` around lines 64 - 67, The modal
is incorrectly storing a server-only CNPL secret in client state
(draftClientSecret, setDraftClientSecret and related state usage in
public-key-modal.tsx), which exposes it to browser memory; remove any
client-side state for the CNPL secret (including draftClientSecret,
setDraftClientSecret and any UI inputs that persist it) and instead have the UI
query a server-side API or config endpoint that returns only capability/flag
information (e.g., "cnplEnabled" or masked presence) and use that to drive
draftWorkflowType and save behavior; update save/submit logic to call a server
route that validates/stores the CNPL secret on the server (no raw secret sent to
or stored in client state) and remove any persistence of the raw secret when
switching flows.
Comment on lines
+92
to
+95
| const isCnpl = draftWorkflowType === "cnpl"; | ||
| const isKeyMissing = draftKey.trim() === ""; | ||
| const isSecretMissing = isCnpl && draftClientSecret.trim() === ""; | ||
| const canSave = !isKeyMissing && !isSecretMissing; |
There was a problem hiding this comment.
Validate the paid reimbursement amount before saving.
In paid mode, clearing the number input coerces the fee to 0, and other invalid states can still reach Line 107 because canSave only checks keys/secrets. That lets the modal persist an invalid reimbursement fee and silently changes the simulation behavior.
💡 Suggested guard
const isCnpl = draftWorkflowType === "cnpl";
const isKeyMissing = draftKey.trim() === "";
const isSecretMissing = isCnpl && draftClientSecret.trim() === "";
- const canSave = !isKeyMissing && !isSecretMissing;
+ const isReimbursementFeeInvalid =
+ !isCnpl &&
+ draftReimbursementMode === "paid" &&
+ (!Number.isFinite(draftReimbursementFee) || draftReimbursementFee < 1);
+ const canSave = !isKeyMissing && !isSecretMissing && !isReimbursementFeeInvalid;
...
- onChange={(e) => setDraftReimbursementFee(Number(e.target.value))}
+ onChange={(e) => {
+ const value = e.target.value;
+ setDraftReimbursementFee(value === "" ? 0 : Number(value));
+ }}
min={1}
className="w-full rounded-lg border border-gray-200 px-3 py-2 text-sm text-gray-900 outline-none transition focus:border-blue-500 focus:ring-2 focus:ring-blue-500/20"
/>
+ {isReimbursementFeeInvalid && (
+ <p className="text-xs text-red-500">Ingresa un monto mayor a 0.</p>
+ )}Also applies to: 107-107, 325-334
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/features/public-key-modal.tsx` around lines 92 - 95, The
canSave guard only checks keys/secrets and misses validating the paid
reimbursement amount, so update the save-enabling logic (where isCnpl,
isKeyMissing, isSecretMissing, canSave are computed) to also validate the
reimbursement number when draftWorkflowType === "paid": ensure the reimbursement
input (the state holding the fee) is not empty/NaN, is parseable as a number,
and is >= 0 (or meets your minimum), and include that boolean in canSave; also
enforce the same validation in the save/submit handler that persists the modal
so an invalid fee cannot be saved even if UI state was tampered with.
Comment on lines
+23
to
+27
| const { clientSecret: bodySecret, patient, totalAmount } = req.body as { | ||
| clientSecret?: string; | ||
| patient: { name: string; rut: string; email: string; phone_number: string }; | ||
| totalAmount: number; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/pages/api/create-order.tsRepository: gokeigo/spot-integration-example
Length of output: 2703
Validate the request body before accessing patient.name on line 34.
The type assertion on lines 23-27 performs no runtime validation. If req.body.patient is undefined, null, or lacks a name property, line 34 will throw an unhandled error, returning a 500 instead of a 400. Additionally, totalAmount is never validated before line 40, and will stringify to unexpected values if not a number.
Also applies to: 34-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/create-order.ts` around lines 23 - 27, The code destructures
clientSecret/bodySecret, patient and totalAmount from req.body without runtime
checks; add explicit validation in the create-order handler to verify
req.body.patient is an object and patient.name (and other required patient
fields like rut/email/phone_number) are non-empty strings and that totalAmount
is a finite number (or coerce/parse it and check > 0) before using them; if
validation fails, return a 400 response with an error message instead of
proceeding to access patient.name or stringifying invalid totals. Ensure the
checks reference the same identifiers (patient, patient.name, totalAmount,
bodySecret/req.body) so you validate the exact values used later (lines noted
34–48) and short-circuit early on invalid input.
Comment on lines
+29
to
+32
| const clientSecret = bodySecret ?? env.SKIPAY_CLIENT_SECRET; | ||
| if (!clientSecret) { | ||
| return res.status(500).json({ error: "No client secret provided" }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the structure and find the files mentioned
git ls-files | grep -E "(create-order|appointment-confirmation|simulation-settings)" | head -20Repository: gokeigo/spot-integration-example
Length of output: 191
🏁 Script executed:
# Read the create-order.ts file around lines 29-32
cat -n src/pages/api/create-order.ts | sed -n '25,40p'Repository: gokeigo/spot-integration-example
Length of output: 699
🏁 Script executed:
# Read appointment-confirmation.tsx around line 25
cat -n src/components/features/appointment-confirmation.tsx | sed -n '20,30p'Repository: gokeigo/spot-integration-example
Length of output: 765
🏁 Script executed:
# Read simulation-settings.ts around line 23
cat -n src/atoms/simulation-settings.ts | sed -n '18,28p'Repository: gokeigo/spot-integration-example
Length of output: 311
🏁 Script executed:
# Get more context from create-order.ts to understand the function signature
cat -n src/pages/api/create-order.ts | sed -n '1,50p'Repository: gokeigo/spot-integration-example
Length of output: 1809
Empty clientSecret currently disables the env fallback.
Line 29 uses ?? (nullish coalescing), which only treats null and undefined as falsy. An empty string "" from src/atoms/simulation-settings.ts line 23 will not trigger the fallback to env.SKIPAY_CLIENT_SECRET. This causes the CNPL path to fail with "No client secret provided" even when the server secret is configured, since the if (!clientSecret) check on line 30 will catch the empty string.
- const clientSecret = bodySecret ?? env.SKIPAY_CLIENT_SECRET;
+ const clientSecret = bodySecret?.trim() || env.SKIPAY_CLIENT_SECRET;This uses || (logical OR) instead, which treats empty strings and whitespace-only strings as falsy and correctly falls back to the environment variable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const clientSecret = bodySecret ?? env.SKIPAY_CLIENT_SECRET; | |
| if (!clientSecret) { | |
| return res.status(500).json({ error: "No client secret provided" }); | |
| } | |
| const clientSecret = bodySecret?.trim() || env.SKIPAY_CLIENT_SECRET; | |
| if (!clientSecret) { | |
| return res.status(500).json({ error: "No client secret provided" }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/create-order.ts` around lines 29 - 32, The code uses nullish
coalescing for clientSecret (const clientSecret = bodySecret ??
env.SKIPAY_CLIENT_SECRET;) which won't fall back when bodySecret is an empty
string; change this to use logical OR so empty strings also fall back (const
clientSecret = bodySecret || env.SKIPAY_CLIENT_SECRET;), keeping the existing
validation (if (!clientSecret) ...) intact; reference symbols: clientSecret,
bodySecret, env.SKIPAY_CLIENT_SECRET in create-order.ts.
Comment on lines
+50
to
+52
| console.log("[create-order] POST", `${env.SKIPPAY_API_URL}/orders`); | ||
| console.log("[create-order] key prefix:", clientSecret.slice(0, 6) + "..."); | ||
| console.log("[create-order] body:", JSON.stringify(requestBody)); |
There was a problem hiding this comment.
Remove PII and credential material from these logs.
Line 52 writes RUT, email, and phone number to server logs, and Line 51 still exposes part of the bearer secret. Please redact these before shipping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/create-order.ts` around lines 50 - 52, The logs in
create-order.ts are exposing sensitive data—redact the bearer secret and any PII
before logging: replace the current console.log calls that reference
clientSecret and requestBody with safe variants that mask clientSecret (e.g.,
show only fixed-length masked suffix/prefix like ****) and remove or redact PII
fields (RUT, email, phone) from requestBody before stringifying; alternatively
log only non-sensitive metadata (HTTP method, endpoint, request size or presence
of fields) to preserve debugging context without leaking credentials or personal
data.
Comment on lines
+54
to
+70
| const response = await fetch(`${env.SKIPPAY_API_URL}/orders`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${clientSecret}`, | ||
| }, | ||
| body: JSON.stringify(requestBody), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const errorBody = await response.text(); | ||
| console.error("SkipPay order creation failed:", response.status, errorBody); | ||
| return res.status(response.status).json({ error: "Failed to create order", detail: errorBody }); | ||
| } | ||
|
|
||
| const data = (await response.json()) as CreateOrderResponse; | ||
| return res.status(200).json(data); |
There was a problem hiding this comment.
Catch transport failures from SkipPay and return JSON.
The !response.ok branch only handles HTTP responses. DNS, timeout, or socket failures will throw out of the route, and src/components/features/appointment-confirmation.tsx, Line 27 assumes this endpoint always returns JSON.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/create-order.ts` around lines 54 - 70, Wrap the fetch call that
assigns const response in the create-order route handler in a try/catch to catch
network/transport errors (DNS, timeout, socket) and ensure the endpoint always
returns JSON; on catch, log the error (including error.message) and return a
JSON error response (e.g. res.status(502).json({ error: "Failed to create
order", detail: error.message })) so callers expecting JSON (and code using
CreateOrderResponse) don’t throw. Ensure the existing response.ok branch remains
unchanged and only network failures are handled by the new catch.
…in simulation settings, enhancing the checkout and appointment confirmation components for improved financial calculations and user experience.
Deploying spot-integration-example with
|
| Latest commit: |
19d1f9f
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/components/features/public-key-modal.tsx (3)
29-35:⚠️ Potential issue | 🟡 MinorIcon-only buttons still need accessible names and toggle state.
Both icon-only controls should expose
aria-label, and toggles should exposearia-pressedso assistive tech can announce purpose/state.Also applies to: 231-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/features/public-key-modal.tsx` around lines 29 - 35, The icon-only toggle button that calls setOpen(!open) and renders the Info component lacks accessible name and state; add an appropriate aria-label (e.g., "Toggle public key info") and expose the current state via aria-pressed={open} on that button. Apply the same fix to the other icon-only control referenced later (the control around lines 231-237): ensure it has an aria-label describing its purpose and aria-pressed bound to its toggle state variable so assistive tech can announce both purpose and state.
68-72:⚠️ Potential issue | 🔴 CriticalServer-only CNPL secret is still handled in browser state.
draftClientSecret/setClientSecretkeep the raw secret in client memory/UI despite the copy saying it should not be exposed. Move secret capture/storage/validation to server-side endpoints and keep only non-sensitive status in client state.Also applies to: 85-86, 107-107, 211-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/features/public-key-modal.tsx` around lines 68 - 72, The component currently stores the raw CNPL secret in browser state via draftClientSecret / setDraftClientSecret (and uses related handlers elsewhere in this file), which violates the server-only requirement; remove any client-side storage of the raw secret and instead POST the secret directly from the input submit handler to a secure server endpoint that validates and persists it, keeping only non-sensitive status flags (e.g., "secret_set" boolean or validation result) in component state; update all places referencing draftClientSecret / setDraftClientSecret (and any showSecret toggles or local validation logic in this file) to call the new API endpoint (e.g., submitSecret API call) and store only the returned status/metadata in state, and ensure the UI never holds or displays the raw secret.
100-103:⚠️ Potential issue | 🟠 MajorSave guard still allows invalid numeric config values.
canSaveonly checks key/secret presence. Numeric inputs can still persist invalid values (e.g., empty→0/NaN, out-of-range commission, non-positive costs/fees), which later breaks pricing behavior.Suggested validation guard
const isCnpl = draftWorkflowType === "cnpl"; const isKeyMissing = draftKey.trim() === ""; const isSecretMissing = isCnpl && draftClientSecret.trim() === ""; - const canSave = !isKeyMissing && !isSecretMissing; + const isReimbursementFeeInvalid = + !isCnpl && + draftReimbursementMode === "paid" && + (!Number.isFinite(draftReimbursementFee) || draftReimbursementFee < 1); + const isCnplCommissionInvalid = + isCnpl && + (!Number.isFinite(draftCnplSkipCommissionPercent) || + draftCnplSkipCommissionPercent < 0 || + draftCnplSkipCommissionPercent > 100); + const isConsultaCostoInvalid = + isCnpl && (!Number.isFinite(draftConsultaCosto) || draftConsultaCosto < 1); + const canSave = + !isKeyMissing && + !isSecretMissing && + !isReimbursementFeeInvalid && + !isCnplCommissionInvalid && + !isConsultaCostoInvalid;Also applies to: 115-118, 256-259, 273-277, 380-383, 405-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/features/public-key-modal.tsx` around lines 100 - 103, The current save guard (variables isCnpl, isKeyMissing, isSecretMissing, canSave) only checks presence of key/secret and misses numeric validation, so update the guard to also validate all numeric config fields (commission, cost, fee, etc.) by parsing them to numbers and ensuring they are finite, not NaN, within allowed ranges (e.g., commission 0-100 if percent, costs/fees > 0), and enforce integer/decimal constraints as required; implement a small helper like isNumericValid(value, {min, max, positiveOnly}) and use it in the canSave calculation (and replace the equivalent guards used elsewhere in the file where similar presence-only checks appear) so the Save button is disabled when any numeric field is invalid.src/components/features/appointment-confirmation.tsx (1)
52-53:⚠️ Potential issue | 🟠 MajorEffect dependencies are incomplete for CNPL order creation.
The effect uses
clientSecretandconsultaCosto(Line 52), but they’re missing from the dependency array (Line 92). This can send stale values when those settings are updated.Fix
- }, [patient, publicKey, workflowType]); + }, [patient, publicKey, workflowType, clientSecret, consultaCosto]);Also applies to: 92-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/features/appointment-confirmation.tsx` around lines 52 - 53, The effect that calls createOrder(clientSecret, patient, consultaCosto) (inside the AppointmentConfirmation component/useEffect) has an incomplete dependency array and can read stale clientSecret and consultaCosto; update the effect’s dependency array to include clientSecret and consultaCosto (alongside existing deps like patient) so the effect re-runs when those values change and ensure createOrder and any callbacks used inside are stable or wrapped in useCallback if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/features/checkout.tsx`:
- Around line 29-35: Sanitize numeric inputs before doing price math: coerce
consultaCosto and cnplSkipCommissionPercent to numbers with safe defaults (e.g.,
basePrice = Number(consultaCosto) || 0) and clamp percent to 0–100 before using
it; then use these sanitized values when computing cnplPayNow and cnplCommission
(and any other derived values). Update the calculations around basePrice,
cnplPayNow and cnplCommission to reference the sanitized variables and ensure
Math.round receives valid numbers. Apply the same sanitization pattern to the
other places mentioned (the blocks around lines 71–73, 83–96, and 136) where
consultaCosto or cnplSkipCommissionPercent are used.
In `@src/hooks/use-public-key.ts`:
- Around line 10-13: The hook is exposing the sensitive CNPL secret via the
client-side atom clientSecret / setter setClientSecret in use-public-key.ts;
remove any reads/writes of clientSecret and its atom from this client hook, stop
persisting it to client-side storage, and instead fetch any required
non-sensitive capability or status from a server endpoint that holds the raw
secret server-side; update the hook to use a non-sensitive flag atom (or an API
call) for UI-only behavior and ensure any server interactions that need the
secret call a secure server API (note the same change must be applied for the
other client-side uses referenced around the lines 24–31).
---
Duplicate comments:
In `@src/components/features/appointment-confirmation.tsx`:
- Around line 52-53: The effect that calls createOrder(clientSecret, patient,
consultaCosto) (inside the AppointmentConfirmation component/useEffect) has an
incomplete dependency array and can read stale clientSecret and consultaCosto;
update the effect’s dependency array to include clientSecret and consultaCosto
(alongside existing deps like patient) so the effect re-runs when those values
change and ensure createOrder and any callbacks used inside are stable or
wrapped in useCallback if needed.
In `@src/components/features/public-key-modal.tsx`:
- Around line 29-35: The icon-only toggle button that calls setOpen(!open) and
renders the Info component lacks accessible name and state; add an appropriate
aria-label (e.g., "Toggle public key info") and expose the current state via
aria-pressed={open} on that button. Apply the same fix to the other icon-only
control referenced later (the control around lines 231-237): ensure it has an
aria-label describing its purpose and aria-pressed bound to its toggle state
variable so assistive tech can announce both purpose and state.
- Around line 68-72: The component currently stores the raw CNPL secret in
browser state via draftClientSecret / setDraftClientSecret (and uses related
handlers elsewhere in this file), which violates the server-only requirement;
remove any client-side storage of the raw secret and instead POST the secret
directly from the input submit handler to a secure server endpoint that
validates and persists it, keeping only non-sensitive status flags (e.g.,
"secret_set" boolean or validation result) in component state; update all places
referencing draftClientSecret / setDraftClientSecret (and any showSecret toggles
or local validation logic in this file) to call the new API endpoint (e.g.,
submitSecret API call) and store only the returned status/metadata in state, and
ensure the UI never holds or displays the raw secret.
- Around line 100-103: The current save guard (variables isCnpl, isKeyMissing,
isSecretMissing, canSave) only checks presence of key/secret and misses numeric
validation, so update the guard to also validate all numeric config fields
(commission, cost, fee, etc.) by parsing them to numbers and ensuring they are
finite, not NaN, within allowed ranges (e.g., commission 0-100 if percent,
costs/fees > 0), and enforce integer/decimal constraints as required; implement
a small helper like isNumericValid(value, {min, max, positiveOnly}) and use it
in the canSave calculation (and replace the equivalent guards used elsewhere in
the file where similar presence-only checks appear) so the Save button is
disabled when any numeric field is invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c4c7a9a-a33c-41e1-91c6-2964e96fabcb
📒 Files selected for processing (5)
src/atoms/simulation-settings.tssrc/components/features/appointment-confirmation.tsxsrc/components/features/checkout.tsxsrc/components/features/public-key-modal.tsxsrc/hooks/use-public-key.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/atoms/simulation-settings.ts
Comment on lines
+29
to
35
| const basePrice = consultaCosto; | ||
| const platformFee = 5000; | ||
| const isCnpl = workflowType === "cnpl"; | ||
|
|
||
| const cnplPayNow = Math.round(basePrice * 0.3); | ||
| const cnplCommission = Math.round(basePrice * cnplSkipCommissionPercent / 100); | ||
|
|
There was a problem hiding this comment.
Sanitize CNPL numeric inputs before price math/rendering.
consultaCosto and cnplSkipCommissionPercent are used directly. Invalid values (empty/NaN/out-of-range) can surface as broken totals and $NaN in UI. Add bounded defaults before calculating and formatting.
Suggested hardening
- const basePrice = consultaCosto;
+ const basePrice =
+ Number.isFinite(consultaCosto) && consultaCosto > 0 ? consultaCosto : 0;
+ const commissionPercent = Number.isFinite(cnplSkipCommissionPercent)
+ ? Math.min(100, Math.max(0, cnplSkipCommissionPercent))
+ : 0;
...
- const cnplCommission = Math.round(basePrice * cnplSkipCommissionPercent / 100);
+ const cnplCommission = Math.round((basePrice * commissionPercent) / 100);
...
- <span className="text-xs text-gray-400">Comisión Skip ({cnplSkipCommissionPercent}%)</span>
+ <span className="text-xs text-gray-400">Comisión Skip ({commissionPercent}%)</span>Also applies to: 71-73, 83-96, 136-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/features/checkout.tsx` around lines 29 - 35, Sanitize numeric
inputs before doing price math: coerce consultaCosto and
cnplSkipCommissionPercent to numbers with safe defaults (e.g., basePrice =
Number(consultaCosto) || 0) and clamp percent to 0–100 before using it; then use
these sanitized values when computing cnplPayNow and cnplCommission (and any
other derived values). Update the calculations around basePrice, cnplPayNow and
cnplCommission to reference the sanitized variables and ensure Math.round
receives valid numbers. Apply the same sanitization pattern to the other places
mentioned (the blocks around lines 71–73, 83–96, and 136) where consultaCosto or
cnplSkipCommissionPercent are used.
Comment on lines
+10
to
+13
| const [workflowType, setWorkflowType] = useAtom(workflowTypeAtom); | ||
| const [clientSecret, setClientSecret] = useAtom(clientSecretAtom); | ||
| const [cnplSkipCommissionPercent, setCnplSkipCommissionPercent] = useAtom(cnplSkipCommissionPercentAtom); | ||
| const [consultaCosto, setConsultaCosto] = useAtom(consultaCostoAtom); |
There was a problem hiding this comment.
Do not expose CNPL client secrets through client-side state.
clientSecret is being read/written via a client hook, which makes the secret accessible in browser memory/devtools (and persistent storage if the atom is storage-backed). Keep raw CNPL secrets server-only and expose only non-sensitive capability/config flags to the UI.
Also applies to: 24-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/use-public-key.ts` around lines 10 - 13, The hook is exposing the
sensitive CNPL secret via the client-side atom clientSecret / setter
setClientSecret in use-public-key.ts; remove any reads/writes of clientSecret
and its atom from this client hook, stop persisting it to client-side storage,
and instead fetch any required non-sensitive capability or status from a server
endpoint that holds the raw secret server-side; update the hook to use a
non-sensitive flag atom (or an API call) for UI-only behavior and ensure any
server interactions that need the secret call a secure server API (note the same
change must be applied for the other client-side uses referenced around the
lines 24–31).
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
1 participant
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.
…ntegration, updating environment variables, and enhancing checkout and appointment confirmation components for improved user experience.
Summary by CodeRabbit
New Features
Chores