Make the Judge.me and Klaviyo surfaces integration-ready - #65
Merged
Merged
Conversation
- move the reviews loader and action to routes/api/reviews.ts - register /api/product/:productHandle/reviews as its own route - return empty data instead of throwing when the token is unset
- expose only integrations.klaviyo / integrations.judgeme booleans from root - hide the newsletter footer card, the popup and the review form when the matching token is unset - return generic errors from the Klaviyo route instead of its payload - document the private token setup in README and .env.example
Member
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Member
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
hta218
requested changes
Sep 14, 2026
The payload spread the whole form after shop_domain and platform, so a
crafted request could set either of them — the request still went out
signed with our private Judge.me token — and could smuggle any extra
field Judge.me accepts.
The six fields the form actually sends are now named, and the server's
own values come last so nothing can displace them. Replaying a request
carrying shop_domain=attacker.myshopify.com, platform=evil and
verified_buyer=true now sends our domain, "shopify", and eight keys
rather than nine.
formDataToObject returned `{}`, which does not allow destructuring, so
it now declares Record<string, FormDataEntryValue>. The review route is
its only caller.
`page` and `per_page` were parsed and handed to Judge.me unchecked, so `page=abc` arrived as NaN and `per_page=100000` asked for the whole review table in one request. Both now clamp at the point they are read: anything that is not a whole number above zero falls back, and per_page tops out at 50. The guard further down that divided by `perPage > 0 ? perPage : 5` goes with it — perPage cannot be zero any more, and leaving it would suggest otherwise.
The route forwarded Klaviyo's status to the browser, so a wrong or expired private token answered the visitor with 401 — a status about our call to Klaviyo, not about their request to us. It now answers 503, matching the branch above that handles a missing token: from the visitor's side, a token that is absent and a token that is refused are the same failure. The 400 branch stays, since that one really is about the email they typed.
Without a token the reviews API answers empty, so a store that has not configured Judge.me still showed a review section inviting a write-up through a form that cannot submit, and a stars rating stuck at zero. The newsletter signup already hides itself in the same situation. Both now follow that gate exactly — `(configured || isDesignMode)`, reading the integrations.judgeme flag this PR already sends to the client — so the surfaces disappear for shoppers while staying editable in Studio. Verified by running the preview build with JUDGEME_PRIVATE_API_TOKEN removed: the review section stops rendering while the Klaviyo signup, whose own token is untouched, still does.
Reviewer asked for a note so nobody reorders these two by accident. The order turns out not to matter: swapping them and rebuilding leaves /api/product/:handle, /api/product/:handle/reviews and the locale- prefixed form all answering correctly, because a dynamic segment never matches across a slash. The constraint worth writing down is the one above it — these are two distinct paths rather than one route with an optional `/reviews?` segment, which is what makes them unambiguous in the first place.
hta218
approved these changes
Sep 15, 2026
hta218
left a comment
hta218
left a comment
Member
There was a problem hiding this comment.
All five points are addressed, thanks. Gating the whole reviews section, not just the stars, was the right call.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the integration surfaces from this ticket that have approved designs — reviews and newsletter signup — and puts both behind server-side tokens with fallback states for when those tokens are absent. Four scope items are deferred; see Deferred scope below.
Closes #53.
Scope covered
Reviews summary / stars / list. Judge.me was living inside the product route. It moves to app/routes/api/reviews.ts, with its own entry in app/routes.ts, so the vendor's request shapes, error handling and empty states sit in one file rather than being threaded through a route that also loads the product. review-summary.tsx and judgeme-stars-rating.tsx consume it.
Klaviyo / newsletter signup. The footer card and the popup now gate on whether Klaviyo is configured, so an unconfigured store does not offer a form that cannot work.
Subscription selector and analytics extension points needed no work: the selector already reads Shopify selling plan groups, and analytics already runs through Analytics.Provider plus GTM behind the public PUBLIC_GOOGLE_GTM_ID.
Technical requirements
No private token reaches the browser. JUDGEME_PRIVATE_API_TOKEN and KLAVIYO_PRIVATE_API_TOKEN are read from context.env inside routes only. What the client receives is a pair of booleans from app/utils/root.server.ts:
integrations: {
klaviyo: Boolean(env.KLAVIYO_PRIVATE_API_TOKEN),
judgeme: Boolean(env.JUDGEME_PRIVATE_API_TOKEN && env.PUBLIC_STORE_DOMAIN),
}
Surfaces gate on those, so no token value is needed client-side to decide what to render.
Public env vars stay client-safe. The only PUBLIC_ key involved is PUBLIC_GOOGLE_GTM_ID, a GTM container ID, which is public by design.
Vendor logic is isolated. Judge.me lives in app/routes/api/reviews.ts, Klaviyo in app/routes/($locale).api.klaviyo.ts. Neither vendor's shapes leak into components.
Unconfigured states fail closed rather than erroring. Reviews: GET returns 200 with an empty rating and an empty list, so the product page renders normally and the widget shows its empty state; POST returns 503 with a generic message. Newsletter: both surfaces are hidden. Provider error text is logged server-side only — the browser gets a generic message either way.
Studio still shows the gated surfaces. The gate is configured || isDesignMode, so a merchant can lay a section out before the token exists. Only the storefront hides it.
Also fixes
Splitting the early-return path out of the Klaviyo route left the provider's response body unread, and in workerd an unread stream keeps the request's I/O context alive, so the worker never completed — the request failed with "The script will never generate a response" after the profile had already been created. The previous code always parsed the response, which consumed the stream as a side effect. Both routes now cancel the body before returning early.
Deferred scope
Four items are not implemented, because the design work this ticket lists under Blocked by has not landed for them:
Happy to pick these up once the designs are approved; the token boundary and the fail-closed pattern established here should carry over unchanged.
Testing
Both states were exercised for Klaviyo and Judge.me. Tokens absent: the newsletter surfaces are hidden and the review widget renders its empty state without offering the form. Tokens present: the newsletter creates a profile in Klaviyo, and a review submitted through the widget reached the Judge.me admin.
Confirmed neither token reaches the browser — absent from the production build output, from the rendered product page, and from the loader payload, which carries only the two booleans.
npm run build, npx tsc --noEmit and npm run biome all pass; the 17 biome warnings are pre-existing on main.
Setup notes
README.md gains a "Third-party integrations" section covering local versus Oxygen configuration and the public/private boundary, pointing at #56 for the full guide. .env.example gains comments marking which tokens are private.
Deploying this needs both tokens set on Oxygen for Preview and Production. Without them the surfaces stay in their unconfigured state — intended, but it will read as a regression, and one cosmetic artefact shows there: the review widget's empty layout reserves two columns while only one renders, so the stars sit left of centre. Left as is to keep this PR to the ticket's scope.