[WIP] Refactor catalog module #2890

patzick · 2019-05-13T07:56:26Z

Short description and why it's useful

This is WIP and should be for now treated as POC. PR created at early stage to show concept.

things to complete this PR

resolve problem with URL dispatcher. After page refresh there is 14 beforeEach invocations, and finally it removes query params from path
add support for multiple filter
move created logic in filters to GenericSelector
[OPTIONAL] add support for filters from path params, not only query params
refactor names - this will be done at the end with team, for now i'm naming things as i believe they should be named
add upgrade notes
add ending comparison to ensure that we didn't missed any functionalities

Additional TODO to complete this PR

display filters when navigate directly to product page
display proper images variants on color filter
improve filters on product page to work old way
refactor module name

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

core/modules/catalog-magento/store/category/actions.ts

core/modules/catalog-magento/store/category/getters.ts

core/modules/catalog/components/CategorySort.ts

src/themes/default/pages/Category.vue

pkarw · 2019-05-13T11:01:55Z

I think it's good step, right direction. The considerations:

we need more strict coding standards for vuex + convention to apply over all the catalog modules and probably the other modules as well,
it's not clear to the developer - using the Category.vue from where to get filters, pagination, sorting options and products - the Category.vue interface should be more self-explanatory based on Vuex getters + computed attributes

patzick · 2019-06-26T14:40:38Z

I'll need add docs and info what's changed, but this can be reviewed.
Version is deployed under: https://theme20.storefrontcloud.io/

pkarw · 2019-06-26T20:09:21Z

@patzick there is a check missing if the category is available in the offline mode - I can enter it and there is no even basic data like category title: https://www.dropbox.com/s/te3xe9fzl2bv2i9/Screenshot%202019-06-26%2022.07.23.png?dl=0

on demo - when you enter category that hasn't been visited before you get notification that this product is not availble in the offline mode + you can't just enter it. We should have some better sollution than this "broken" category page like it's right now

pkarw · 2019-06-26T20:12:15Z

Moreover, I see that currently we're not setting the category data into local cache (categoriesCollection). Not sure about the consequences of this behavior for the offline mode (I can imagine some issues regarding the breadcrumbs on the product page or some other problems when we won't cache the category (?)). I don't say we must have it but just please test it out and do some code-review what will happen in the offline mode in this situation please. The cool thing about caching is we're pre-caching categories for later (the same we can potentially do with the categoriesMap and hierarchy - storing them in local cache could save us with some addional calls).

UPDATE: What's pretty interesting even with the setItem on demo - the cache is not being populated (categories collection), However, please do check the offlne mode scenarios

pkarw

It's really cool one! I had some questions regarding categoriesMap (to make sure we won't duplicate data), categoryHierarchy (if it's really needed) and mostly about the offline mode (caching/exception handling).

BUT I must admit it works perfectly well for most scenarios I've tested :D That's great, the code is very coherent - I like it.

pkarw · 2019-05-13T12:48:08Z

core/modules/catalog-magento/store/category/getters.ts

+  getCategories: (state) => state.categories,
+  getCategoryProducts: (state) => state.products,
+  getAvailableFilters: state => state.availableFilters,
+  calculateCurrentCategory: (state, getters, rootState) => (route) => {


I'd say get is easier to get :D

core/modules/catalog-magento/store/category/getters.ts

core/modules/catalog/components/CategorySort.ts

src/themes/default/pages/Category.vue

core/modules/catalog-magento/store/category/actions.ts

pkarw · 2019-06-19T20:19:44Z

core/modules/catalog-next/store/category/actions.ts

+    await dispatch('loadCategoryFilters', searchCategory)
+    const searchQuery = getters.getCurrentFiltersFrom(route[products.routerFiltersSource])
+    let filterQr = buildFilterProductsQuery(searchCategory, searchQuery.filters)
+    const searchResult = await quickSearchByQuery({ query: filterQr, sort: searchQuery.sort })


I'm not sure if'we re using the config.entites.products.includeFields ... as it was done with the progressive caching approach: https://github.com/DivanteLtd/vue-storefront/blob/70543b3cfa379971220f6514348fe0386a108878/core/modules/catalog/store/category/actions.ts#L240

It seems like not. Moreover, we're not running these 2 calls with the doublecheck (if it gains the performance) as it was originally done. It should work exact the same way

Moreover, the stock prefetching mechanism is also not ported to this refactored version: https://github.com/DivanteLtd/vue-storefront/blob/70543b3cfa379971220f6514348fe0386a108878/core/modules/catalog/store/category/actions.ts#L267

Without prefetching the stocks - when the config.products. filterUnavailableVariants is set to true - it will take forever to fetch the stocks on demand on the product page

pkarw · 2019-06-26T19:56:18Z

core/modules/catalog-next/helpers/categoryHelpers.ts

+  return 0
+}
+
+export const _prepareCategoryHierarchyMap = (category: Category|ChildrenData) => {


I'd suggest you to reconsider if this is really needed. Potentially it will be huuuge object (take wonect.com for an example) - building it, passing thru INITIAL_STATE - not sure if it makes sense. shouldn't we create this kind of mapping just for the current category (we do have category.path which consist the ids of category accestors as far as I remember) (?)

pkarw · 2019-06-26T19:57:01Z

core/modules/catalog-next/hooks/afterRegistration.ts

@@ -0,0 +1,3 @@
+export async function afterRegistration ({ Vue, config, store, isServer }) {
+  if (isServer) await store.dispatch('category-next/_prepareCategoriesHierarchyMap')


I don't liek to have such a pre-caching mechanism for the whole category tree in the module init :(

core/modules/catalog-next/store/category/actions.ts

pkarw · 2019-06-26T20:15:34Z

core/modules/catalog-next/store/category/getters.ts

+    }
+    return filters
+  },
+  getAvailableFilters: state => state.availableFilters,


I like this interface :)

pkarw · 2019-06-26T20:30:49Z

OK one more thing - we're not caching the products when browsing in the CSR (elasticCache collection is not being populated); we're also not having the progressive caching (taking the limites set of product fields for the listing + second request for caching). This should be re-considered

filrak · 2019-07-01T09:30:28Z

core/data-resolver/index.ts

@@ -0,0 +1,5 @@
+import { CategoryService } from './CategoryService';


set sideEffects: false for this in package.json

filrak · 2019-07-01T09:31:48Z

core/modules/catalog-next/helpers/categoryHelpers.ts

+  return 0
+}
+
+export const _prepareCategoryPathIds = (category: Category): string[] => {


_ indicates private fn yet you export it, why?

filrak · 2019-07-01T09:40:30Z

core/modules/catalog-next/store/category/actions.ts

+import chunk from 'lodash-es/chunk'
+
+const actions: ActionTree<CategoryState, RootState> = {
+  async loadCategoryProducts ({ commit, getters, dispatch, rootState }, { route, category } = {}) {


imho products should be cached by default. No additional work should be required to do this

patzick and others added 19 commits May 8, 2019 13:10

prepare module draft

5d2827c

add default values

3889092

init module actions

9d2462a

move helper from old module

16ca33e

getting available filters

0207857

invoke search with current filters

087d45e

current category from getter

e834b21

default value as string

d372696

add todo for multiple filters

59f2130

switching search filters

8a9d28d

switching filters query

d2b4664

multiple dilters draft and reseting filters

23e70d7

migrate filter components

c60508d

migrate category page

54a231b

filters and currentCategory as getters

f45fe5b

active filters

d027a4c

recomputing getters

8e71fdf

ui active filters

8f9e795

add sorty by property

f5cdd82

pkarw requested changes May 13, 2019

View reviewed changes

patzick and others added 9 commits May 13, 2019 13:51

system filters not visible on search filters

0a9f7d5

get turned off by default

f979051

add category breadcrumbs

fece9d5

stop using of sidebar mixin

fb380e7

add multiple filters support

bd32c8c

merge develop

9084808

moved filter logic to mixin

487a847

refactor calculate getters

a819f6f

move logic to getters

25eabd7

patzick added 9 commits June 25, 2019 13:15

improve filters reactivity on product page

7329923

fix problem with available options

097eebb

merge develop

cfafe84

sorting filters

b868176

lnter

dee2ead

Merge branch 'develop' into magento-catalog

e345a04

add infinite scroll pagination

a0e993d

categoryHierarchyMap

9659e6f

Merge branch 'develop' into magento-catalog

0f61390

patzick requested review from pkarw and filrak June 26, 2019 14:20

pkarw approved these changes Jun 26, 2019

View reviewed changes

patzick and others added 8 commits June 27, 2019 15:40

remove hierarchyMap - build from specific category

aa41c09

Fix problem with products caching on SSR

8545d57

refactor category page getters names

52e8854

Merge branch 'develop' into magento-catalog

61ae322

remove unused hook

6153d69

fix for product option values incomplete

779ce67

prefetch stock for cached products

f31ac67

Merge branch 'develop' into magento-catalog

ddc3cde

filrak reviewed Jul 1, 2019

View reviewed changes

patzick merged commit dd1f755 into vuestorefront:develop Jul 1, 2019

patzick deleted the magento-catalog branch July 1, 2019 09:45

alinadivante mentioned this pull request Jul 1, 2019

Sorting at a price in different ranges doesn't work properly #3163

Closed

5 tasks

patzick mentioned this pull request Aug 27, 2019

Feature/category improvements #3439

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Refactor catalog module #2890

[WIP] Refactor catalog module #2890

patzick commented May 13, 2019 •

edited

Loading

pkarw commented May 13, 2019

patzick commented Jun 26, 2019

pkarw commented Jun 26, 2019

pkarw commented Jun 26, 2019 •

edited

Loading

pkarw left a comment

pkarw May 13, 2019

pkarw Jun 19, 2019

pkarw Jun 19, 2019

pkarw Jun 26, 2019

pkarw Jun 26, 2019

pkarw Jun 26, 2019

pkarw commented Jun 26, 2019

filrak Jul 1, 2019

filrak Jul 1, 2019

filrak Jul 1, 2019

		@@ -0,0 +1,3 @@
		export async function afterRegistration ({ Vue, config, store, isServer }) {
		if (isServer) await store.dispatch('category-next/_prepareCategoriesHierarchyMap')

		@@ -0,0 +1,5 @@
		import { CategoryService } from './CategoryService';

[WIP] Refactor catalog module #2890

[WIP] Refactor catalog module #2890

Conversation

patzick commented May 13, 2019 • edited Loading

Short description and why it's useful

Additional TODO to complete this PR

Which environment this relates to

Upgrade Notes and Changelog

pkarw commented May 13, 2019

patzick commented Jun 26, 2019

pkarw commented Jun 26, 2019

pkarw commented Jun 26, 2019 • edited Loading

pkarw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkarw commented Jun 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patzick commented May 13, 2019 •

edited

Loading

pkarw commented Jun 26, 2019 •

edited

Loading