Fix issues 191 188 180 - #196
Conversation
|
@laaichiu is attempting to deploy a commit to the Spree Commerce Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughPolicy metadata now builds localized descriptions and canonical URLs. Product details fall back to the default variant SKU and request its data. The root layout suppresses hydration warnings, with tests covering each change. ChangesPolicy Metadata
Product SKU Fallback
Root Layout Hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsx (3)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
@/alias for the component import.This test uses a relative import even though repository guidelines require absolute imports. Replace
./ProductDetailswith the corresponding@/...path.As per coding guidelines, TypeScript files should use absolute
@/imports instead of relative imports.Proposed change
-import { ProductDetails } from "./ProductDetails"; +import { ProductDetails } from "`@/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetails";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsx at line 5, Update the ProductDetails test’s ProductDetails import to use the repository’s `@/` absolute alias instead of the relative ./ProductDetails path, preserving the same module target.Source: Coding guidelines
36-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the test fixture type-checked.
as unknown as Productbypasses the SDK contract, so required or renamed fields can drift without this test catching it. Define the object withsatisfies Productand add any required fields, or construct a fully typedProduct.As per coding guidelines, use strict TypeScript checking and
satisfiesfor object literals instead of bypassing the type contract with a double assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsx around lines 36 - 70, Update the productWithoutCustomVariants fixture to use satisfies Product instead of as unknown as Product, and add or correct all fields required by the Product contract so TypeScript validates the object while preserving the existing test data.Source: Coding guidelines
77-86: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the new SKU fallback branch.
Because
ProductDetailsinitially selectsproduct.default_variant, this fixture makesselectedVariant?.skuequal"MASTER-SKU-001". The test would still pass if the new fallback were removed. Select a variant without a SKU while keeping the default variant SKU populated, then assert the fallback value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsx around lines 77 - 86, Update the ProductDetails test to select a variant whose sku is absent while leaving product.default_variant with "MASTER-SKU-001", then assert the rendered SKU is the intended fallback value. Ensure the test exercises the selectedVariant?.sku fallback branch rather than only verifying the default variant SKU.
🤖 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
`@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsx:
- Line 5: Update the ProductDetails test’s ProductDetails import to use the
repository’s `@/` absolute alias instead of the relative ./ProductDetails path,
preserving the same module target.
- Around line 36-70: Update the productWithoutCustomVariants fixture to use
satisfies Product instead of as unknown as Product, and add or correct all
fields required by the Product contract so TypeScript validates the object while
preserving the existing test data.
- Around line 77-86: Update the ProductDetails test to select a variant whose
sku is absent while leaving product.default_variant with "MASTER-SKU-001", then
assert the rendered SKU is the intended fallback value. Ensure the test
exercises the selectedVariant?.sku fallback branch rather than only verifying
the default variant SKU.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ce19e33c-ebc7-4af5-9211-1683a8a7038a
📒 Files selected for processing (7)
src/app/[country]/[locale]/(storefront)/policies/[slug]/page.test.tsxsrc/app/[country]/[locale]/(storefront)/policies/[slug]/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.test.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetails.tsxsrc/app/layout.test.tsxsrc/app/layout.tsxsrc/lib/data/cached.ts
Summary by CodeRabbit
Bug Fixes
Tests