Skip to content

Conversation

@grimasod
Copy link
Contributor

@grimasod grimasod commented Oct 7, 2019

Related issues

Short description and why it's useful

  1. Fixes breadcrumbs for products that are in multiple categories, in different branches of the category tree

  2. Uses breadcrumbs module store, so breadcrumb categories are only calculated once

  3. Adds an optional filter of breadcrumb categories to local.json

  4. Also adds pre-defined category search query filters in local.json that are applied to every query. This removes the need to define more and more parameters for category/list calls, but still allows parameters to override the pre-defined filters. It also means the currently hardcoded platform-specific parameters, like "is_active" and "product_count", could be deprecated.

Screenshots of visual changes before/after (if there are any)

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

@andrzejewsky andrzejewsky self-requested a review October 9, 2019 20:20
@andrzejewsky andrzejewsky added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Oct 10, 2019
@ArturDivante
Copy link
Collaborator

@andrzejewsky @grimasod Test instructions / suggestion needed here :)

@andrzejewsky andrzejewsky added this to the 1.11.0-rc2 milestone Oct 14, 2019
@andrzejewsky
Copy link
Contributor

@ArturDivante so, first of all:

  1. Check breadcrumbs across entire app, especially on product page that has assigned to more that one category
  2. Check if all of categories are loaded correctly
  3. Test new options in the config file entities.category.filterFields entities.category.breadcrumbFilterFields that are filtering these elements by given field

You can also, text me on slack if you want more details

@grimasod
Copy link
Contributor Author

Here's one way to test the new config options.

To prepare the catalog:

  • Create a deep-level category, for example Default category / Sales / October / Week 2 / Friday / Women / Tops / Tees. (This is because the current breadcrumbs gives preference to deeper paths and we want to compare to that)
  • In this category, have Include in Menu turned off
  • Add a Product to the new Category, say "Radiant Tee" SKU: WS12-XS-BLUE, which is already in an existing Category Women / Tops / Tees where Include in Menu is turned on
  • Update / Reindex Elastic Search

To test entities.category.filterFields

  • Navigate to the new category, to confirm it's appearing and contains the product added above
  • set "filterFields": { "include_in_menu": true } in local.json and rebuild
  • Reload and check that the category will not be accessible now. (Please note, if the side menu has still not been updated to use category-next, the category will still appear in menu. This is a separate issue)
  • You can also check in Vuex that the category is not included
  • remove "filterFields": { "include_in_menu": true } in local.json, to be ready for the next test

To test entities.category.breadcrumbFilterFields

  • navigate to the Radiant Tee product page, using the side menu: Women -> Tops -> Tees -> Radiant Tee
  • Note the breadcrumbs - they should show the deeper path created above, or maybe none
  • Reload the product page and observe the breadcrumbs again
  • Now set "breadcrumbFilterFields": { "include_in_menu": true } in local.json and rebuild
  • Reload the product page and check that the breadcrumbs show the standard path Women / Tops / Tees
  • Navigate away and back to this product using the menu, the breadcrumbs should be consistent

I've tested using our real catalog data, but not with the test data

@andrzejewsky
Copy link
Contributor

@grimasod 😮 nice explanation and test scenario! @alinadivante / @ArturDivante Can you have a look?

@alinadivante
Copy link
Collaborator

@ArturDivante Please take a look at this and test it in your free time:)

@pkarw pkarw modified the milestones: 1.11.0-rc2, 1.11.0 Oct 29, 2019
@grimasod
Copy link
Contributor Author

grimasod commented Nov 5, 2019

@andrzejewsky I see this PR didn't make it into 1.11.0-rc2. Will there be an rc3? Thanks

@andrzejewsky
Copy link
Contributor

@grimasod We want to merge it in 1.11, sorry for that long time.

@andrzejewsky
Copy link
Contributor

Hey @grimasod seems like everything works fine, but one thing that must be changed here... Could you add a new options that you've introduced to the default.json and set them default value? probably filterFields in both cases will be just empty ({}) (it also must be covered in the code)

@ArturDivante ArturDivante removed the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Nov 19, 2019
@grimasod
Copy link
Contributor Author

Hi @andrzejewsky no problem, added to default.json :) No code changes.

Copy link
Contributor

@andrzejewsky andrzejewsky left a comment

Choose a reason for hiding this comment

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

2 simple comments @grimasod they are related to your changes i default.json now you don't have to check parameters because they always exist

@andrzejewsky andrzejewsky self-requested a review November 22, 2019 11:38
@andrzejewsky andrzejewsky merged commit 869dae6 into vuestorefront:release/v1.11 Nov 22, 2019
@grimasod grimasod deleted the feature/catalog-filters branch January 6, 2020 02:44
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.

5 participants