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

New getter category-next/getCategoryFrom and category-next/getCurrentCategory always returns the root category #3286

Closed
2 of 5 tasks
cewald opened this issue Jul 26, 2019 · 4 comments
Assignees
Labels
bug Bug reports
Milestone

Comments

@cewald
Copy link
Contributor

cewald commented Jul 26, 2019

Current behavior

The new getter category-next/getCategoryFrom (and category-next/getCurrentCategory) on the new category page always returns the root category because of a wrong filter in its getter.

This happens if there is a category with a path like / or a empty one in the category-next/categoriesMap state.

E.g. if you are in /boys.html and the it crawls to the states it tests like:

return getters.getCategories.find(category => path.includes(category.url_path)) || {}

If there is a category with path / (or empty) in the state, it will always return this one. This filter is quiet flacky. I would prefer something more concrete like this:

return getters.getCategories.find(category => path.replace(/^(\/)/gm, '') === category.url_path) || {}

Also what happens if you use a different url path, then it would never find the right current category, e.g. I added an extra mapper fallback to let the /new.html category availaible ever /new this won't over work with the new getters based on route path. You will need at least another getter for this at least.

I think this new functionality leads a bit in the wrong direction and needs way more flexibility.

Expected behavior

Get the right category from state.

Steps to reproduce the issue

See above.

Repository

develop

Can you handle fixing this bug by yourself?

  • YES
  • NO

Which Release Cycle state this refers to? Info for developer.

Pick one option.

  • This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

  • Browser: All
  • OS: All
  • Node: All
  • Code Version: develop:latest
@cewald cewald added the bug Bug reports label Jul 26, 2019
@pkarw pkarw assigned filrak and patzick and unassigned filrak Jul 26, 2019
@cewald
Copy link
Contributor Author

cewald commented Jul 26, 2019

Okay, I think I found a solution and will open an PR later today for a more flexible integration :)

@cewald
Copy link
Contributor Author

cewald commented Jul 26, 2019

I added a PR with some comments: #3295

cewald referenced this issue in icmaa/vue-storefront Jul 26, 2019
… module extension and implement it into our modules

* @todo: remove module-extension when #3295 is approved and merged
cewald referenced this issue in icmaa/vue-storefront Jul 26, 2019
* Change `category-next/getCurrentCategory` as module extension and implement it into our modules
* @todo: remove module-extension when #3295 is approved and merged
@cewald
Copy link
Contributor Author

cewald commented Aug 2, 2019

@patzick I made some improvements in the related PR #3295

@cewald cewald changed the title New getter catalog-next/getCategoryFrom and catalog-next/getCurrentCategory always returns the root category New getter category-next/getCategoryFrom and category-next/getCurrentCategory always returns the root category Aug 5, 2019
patzick added a commit that referenced this issue Aug 20, 2019
@patzick
Copy link
Collaborator

patzick commented Aug 20, 2019

Great job @cewald, thanks for your input! :)

@patzick patzick closed this as completed Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports
Projects
None yet
Development

No branches or pull requests

3 participants