Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix searching deleted products by SKU #4164

Merged

Conversation

adammathys
Copy link
Member

Description

We've added two new scopes to Spree::Product and deprecated on existing one.

  • with_variant_sku_cont: Has been deprecated in favour of with_kept_variant_sku_cont
  • with_all_variant_sku_cont: Will search for products that have a variant containing the provided SKU, even if it has been discarded.
  • with_kept_variant_sku_cont: Maintains the existing behaviour of with_variant_sku_cont, but with a more descriptive name.

This allows us, along with a new Backbone view to help control the two inputs we now have on the page (one for each scope), to correctly return deleted products when searching by SKU. Our view will swap out the inputs based on the value of the "with_deleted" checkbox so that the user experience is unchanged.

While attempting to fix this issue, there are a few other approaches we tried before settling on this one:

  • Updating the scope to always include discarded variants. This came close but had an issue when the product had some discarded variants and would return the product regardless of the value of the "with_deleted" checkbox.
  • Adding a parameter to the scope to conditionally include discarded variants. Ransack had some trouble with parameterizing the values from this. It would add extra stuff to the input field.

Fixes #3912

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

This scope is currently only in use on the product search page.
Unfortunately, it did not return the correct results when attempting to
search with "deleted" products by SKU. This is due to the scope
excluding any discarded variants by default.

To remedy the issue, we've addded a second scope that will include the
discarded variants in the results. We've also renamed the old scope to
better reflect what it is doing and have added a deprecation for the old
method name.
We're going to want to be able to filter products using our new variant
SKU scopes that correctly consider discarded variants.
Now that we've got a new scope we can use that will correctly return
products with discarded variants, we've updated the admin interface to
use it.

This meant adding a new Backbone view to help control the two inputs we
now have on the page, one for each scope. It will hide and disable the
appropriate input based on the value of the "with deleted" checkbox.
Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

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

This all looks reasonable to me! Thanks, @adammathys!

@kennyadsl kennyadsl added changelog:solidus_backend Changes to the solidus_backend gem type:bug Error, flaw or fault labels Sep 17, 2021
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.

Thanks for the contribution, @adammathys!

@kennyadsl kennyadsl merged commit edca67a into solidusio:master Sep 17, 2021
@adammathys adammathys deleted the adammathys/search-deleted-by-sku branch September 20, 2021 22:57
kennyadsl added a commit to nebulab/solidus that referenced this pull request Mar 28, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 12, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 14, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 18, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 19, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
kennyadsl added a commit to nebulab/solidus that referenced this pull request Apr 24, 2023
Please use with_kept_variant_sku_cont now.

Ref solidusio#4164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem type:bug Error, flaw or fault
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Searching deleted products by SKU does not work
3 participants