Skip to content

Conversation

@RenanHCosta
Copy link
Contributor

@RenanHCosta RenanHCosta commented Nov 18, 2025

Adds a Sort property to VNDA productListingPage Loader, allowing commerces to set default order option on their pages.

Summary by CodeRabbit

  • New Features

    • Users can set an optional default sort preference for product listings.
  • Refactor

    • Sort selection now resolves more predictably (explicit preference → URL → default) and caching updated to respect the chosen preference.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 18, 2025

Walkthrough

Product listing loader updated to accept an optional sort prop and to resolve sort via a fallback chain; cache key generation logic was adjusted to change which sort value is serialized.

Changes

Cohort / File(s) Summary
Product listing loader (sort handling)
vnda/loaders/productListingPage.ts
Added optional sort?: Sort to Props; searchLoader now resolves sort via a fallback chain (URL param → props.sort → default VNDA_SORT_OPTIONS[0]); cache key generation altered to incorporate props.sort in its fallback logic when serializing the sort value.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant Loader as productListingPage.searchLoader
    participant Cache

    Client->>Loader: Request (includes URL sort?)
    note right of Loader `#dfefff`: Sort resolution
    Loader->>Loader: read URL sort param
    alt URL sort present
        Loader->>Loader: use URL sort
    else URL sort missing
        Loader->>Loader: use props.sort if provided
    else props.sort missing
        Loader->>Loader: use default VNDA_SORT_OPTIONS[0]
    end
    note right of Loader `#fff5df`: Cache key serialization uses props.sort as an additional fallback before URL sort (per change)
    Loader->>Cache: compute cacheKey (serializes sort per new fallback order)
    Loader->>Client: Return results
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review vnda/loaders/productListingPage.ts for correct fallback ordering when resolving sort.
  • Verify cache key serialization logic now uses props.sort appropriately and that it won't break cache hits/misses.
  • Confirm type correctness for the new sort?: Sort property and any related imports or exports.

Poem

🐰 I found a new sort in my burrow today,

URL first, then props — a tidy relay.
Keys now remember which order we choose,
Hop, fetch, and cache — no sorting to lose! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is minimal but covers the core change. However, it lacks required sections from the template including Issue Link, Loom Video, and Demonstration Link. Add missing template sections: link the relevant issue, include a Loom video, and provide a demonstration link for reviewers to test the feature.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a default sorting parameter feature to the VNDA productListingPage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 122c45c and 1f803b8.

📒 Files selected for processing (1)
  • vnda/loaders/productListingPage.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • vnda/loaders/productListingPage.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 0.133.5 update
  • 🎉 for Minor 0.134.0 update
  • 🚀 for Major 1.0.0 update

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b1a4a5 and 122c45c.

📒 Files selected for processing (1)
  • vnda/loaders/productListingPage.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
vnda/loaders/productListingPage.ts (2)
wake/loaders/productListingPage.ts (1)
  • Sort (41-48)
vnda/utils/client/types.ts (1)
  • Sort (3-3)
🔇 Additional comments (2)
vnda/loaders/productListingPage.ts (2)

50-53: LGTM!

The optional sort property is properly typed and documented, allowing loaders to set a default sort option.


104-105: LGTM!

The fallback chain correctly prioritizes URL parameters over props, allowing user-initiated sorts to override component defaults.

@JonasJesus42 JonasJesus42 merged commit 52100b1 into deco-cx:main Nov 25, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants