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

Rework needed of \Smile\ElasticsuiteCatalogOptimizer\Model\Optimizer\Preview::canApply #1528

Closed
rbayet opened this issue Sep 2, 2019 · 0 comments
Assignees
Labels

Comments

@rbayet
Copy link
Collaborator

rbayet commented Sep 2, 2019

\Smile\ElasticsuiteCatalogOptimizer\Model\Optimizer\Preview::canApply has been heavily reworked to address the bug #1418.

Unfortunately, the current code has two new bugs

  • the preview for Catalog Product Search is broken when apply_to is set to 0 (All search terms)
  • the preview for Category Product View is broken when apply_to is set to 0 but there are some categories selected (the user selected one or several categories, then changed is mind by switching back the select from Selected categories to All categories

Preconditions

Magento Version : EE 2.3.2

ElasticSuite Version : 2.8-x

Environment : Developer

Third party modules : N/A

Steps to reproduce for Catalog Product Search

  1. On a Luma, disable ALL existing optimizers
  2. Create a new optimizer of constant model, active, available on all 3 request types (Catalog Product Search, Catalog Product Autocomplete, Category Product View)
  3. Input a sensible boost value (for instance 20 %)
  4. In 'Apply to products', add a rule
If ALL of these conditions are TRUE 
    Attribute Set is Bag
  1. DO NOT SAVE the optimizer yet
  2. In the preview, select Catalog Product Search and type bag inside the Query Text input field

Expected result for Catalog Product Search

  1. Bags products should be boosted while non-bags products (eg Affirm Water Bottle, Rocco Gym Tank, etc) should be "un-boosted"

Actual result for Catalog Product Search

  1. The optimizer is not applied, no product has its score changed

Steps to reproduce for Category Product View

  1. On a Luma, disable ALL existing optimizers
  2. Create a new optimizer of constant model, active, available on all 3 request types (Catalog Product Search, Catalog Product Autocomplete, Category Product View)
  3. Input a sensible boost value (for instance 20 %)
  4. In 'Apply to products', add a rule
If ALL of these conditions are TRUE 
    Attribute Set is Bag
  1. In the Category Product View fieldset, change "Apply to" to "Selected categories"
  2. In Apply to categories select the "Gear" category, then change back "Apply to" to "All Categories"
  3. DO NOT SAVE the optimizer yet
  4. In the preview, select Category Product View and select the Gear category

Expected result for Category Product View

  1. Bags products should be boosted while non-bags products (eg Set of Sprite Yoga Straps, sport watches, etc) should be "un-boosted"

Actual result for Category Product View

  1. The optimizer is not applied, no product has its score changed

Explanation

The behavior of \Smile\ElasticsuiteCatalogOptimizer\Model\Optimizer\Preview::canApply with regards to evaluating "canApply" has been reversed :

  • in 41c11b7648455deed8034b09cf3c10b3b2529eb8, canApply
    • is initially valued to true or false depending on the fact that the current container is selected in applicable containers
    • and then if canApply is true potential additional restrictions are pulled from the containers
      • if on quick_search_container/Catalog Product Search and apply_to is set to 1 (Selected search terms), the preview search term is compared to the selected search terms
      • if on catalog_view_container/Category Product View and apply_to is set to 1 (Selected categories), the preview category is compared to the selected categories
  • in HEAD/6be4ab01ed2253f291b28b2b552ae6e9be50b347 version
    • is initially valued to false
    • and then if the current container is selected in applicable containers
      • if on quick_search_container/Catalog Product Search, canApply can only become true if apply_to is set to 1, it thus can never become true if it is set to 0 (All search terms)
      • if on catalog_view_container/Category Product View there is a useless checkempty($config['category_ids'] when $config['apply_to'] is set to 0 : if the user switched from All categories to Selected categories, selected one or several categories, then switched back to All categories without saving the Optimizer, the `$config['category_ids'] is not empty but it should not matter

Conclusion

The current code has bugs and is too complex, the refactoring should be undone :

  • \Smile\ElasticsuiteCatalogOptimizer\Model\Optimizer\Preview::canApply should be reverted back to its 41c11b7648455deed8034b09cf3c10b3b2529eb8 version
  • the issue error in optimizer #1418 should be re-fixed while keeping as much as possible the initial boolean evaluation logic (using strict comparison operators is ok if there are no side-effects)
  • all implicit functional tests cases should work as expected (with regard to before / after saving the Optimizer, having / not having selected search terms/categories)
@rbayet rbayet added the bug label Sep 2, 2019
@rbayet rbayet closed this as completed in d177ad0 Oct 15, 2019
rbayet added a commit that referenced this issue Oct 15, 2019
Fixes #1528 rework method canApply and fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants