Skip to content

Conversation

@forkata
Copy link
Contributor

@forkata forkata commented Jan 15, 2026

Summary

This change removes the explicit require for product filters to simplify overriding it in client applications. We have confirmed that only the test files that require the filters depend on them. The product filters are also explicitly required in the solidus_starter_frontend which depends on them.

Fixes #6406

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@github-actions github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Jan 15, 2026
@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.49%. Comparing base (714b3ec) to head (d7cb914).

Files with missing lines Patch % Lines
core/lib/spree/core/product_filters.rb 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6408      +/-   ##
==========================================
- Coverage   89.50%   89.49%   -0.01%     
==========================================
  Files         981      981              
  Lines       20479    20476       -3     
==========================================
- Hits        18329    18326       -3     
  Misses       2150     2150              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

forkata and others added 2 commits January 15, 2026 11:41
This is already required in solidus_starter_frontend. Removing the
explicit require from core simplifies overriding it in client
applications.

Co-authored-by: Jared Norman <jared@super.gd>
Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
This change cleans up some of the formatting and language around how
product filters should be constructed.

Co-authored-by: Jared Norman <jared@super.gd>
Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
@forkata forkata force-pushed the remove-product-filters-require branch from e971bae to 7461408 Compare January 15, 2026 19:41
This cleans up some of the sample product filters provided to be a bit
more performant and easier to read.

Co-authored-by: Jared Norman <jared@super.gd>
Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
@forkata forkata force-pushed the remove-product-filters-require branch from 7461408 to d7cb914 Compare January 15, 2026 19:44
@forkata forkata marked this pull request as ready for review January 15, 2026 19:44
@forkata forkata requested a review from a team as a code owner January 15, 2026 19:44
@forkata
Copy link
Contributor Author

forkata commented Jan 15, 2026

I don't believe the code coverage changed here, I think the check here is just indicating that the product filters specs did not cover all the code paths. We can look at adding some code coverage if that's needed to merge this change.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍 I don't think it's needed to add more coverage for the purpose of this PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_core Changes to the solidus_core gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Product Filter Overriding Less Clunky

2 participants