Skip to content

Commit

Permalink
[FW-1848] SEO categories with ignored filters (#2891)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebaholesz committed Nov 20, 2023
2 parents 0f698d1 + 603fb6c commit ea316f8
Show file tree
Hide file tree
Showing 29 changed files with 666 additions and 212 deletions.
9 changes: 9 additions & 0 deletions UPGRADE-14.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ Follow the instructions in relevant sections, e.g. `shopsys/coding-standards` or
- see #project-base-diff to update your project
- replace custom application bootstrapping with symfony/runtime ([#2914](https://github.com/shopsys/shopsys/pull/2914))
- see #project-base-diff to update your project
- prevent duplicate color parameters in data fixtures ([#2911](https://github.com/shopsys/shopsys/pull/2911))
- see #project-base-diff to update your project

### Storefront

Expand Down Expand Up @@ -222,3 +224,10 @@ Follow the instructions in relevant sections, e.g. `shopsys/coding-standards` or
- if you have any logic for syncing your events, you can move it to this provider
- docs were added, so you can base your changes on those
- see #project-base-diff to update your project
- improve SEO categories logic in regards to non-SEO-sensitive filters ([#2891](https://github.com/shopsys/shopsys/pull/2891))
- we moved some config from various files to `config/constants` and warmly suggest you do the same, as it improves the app and testing
- we also moved some hooks from `helpers` to `hooks` and suggest you do the same with all hooks, as it again improves the app and testing
- the implemented functionality for dynamic switching between SEO-sensitivity for various filters is only implemented on SF, so applying these changes won't make it work on BE
- if you do not need SEO categories, these changes might be irrelevant altogether
- flag and brand values are now merged after change of default parameters (leaving SEO category) instead of overwritting. This is a bug fix and you should apply it to your changes as well.
- see #project-base-diff to update your project
4 changes: 4 additions & 0 deletions docs/storefront/unit-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ export const useModuleValue = () => {
### How to mock different scenarios
Before showing different mocking scenarios, there are some common known pitfals with mocking which might be a problem for you as well. Below is a list with a simple solution:
- **If I test function foo and want to mock a function or constant bar from the same module, it will not work as needed** - If this is the case, you should find another way of testing, either by moving one of these files, or by avoiding the mock altogether (for example using IoC and parameters)
#### 1. Default mock of a function
This approach is helpful if you want to mock an exported function in a specific way which stays consistent across the file. If you want this mock function to return the same value for all your test suites inside this file, this is how you do it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class ReadyCategorySeoDataFixture extends AbstractReferenceFixture implements De
public const READY_CATEGORY_SEO_TV_IN_SALE = 'ready_category_seo_tv_in_sale';
public const READY_CATEGORY_SEO_TV_PLASMA_WITH_HDMI = 'ready_category_seo_tv_plasma_with_hdmi';
public const READY_CATEGORY_SEO_PC_NEW_WITH_USB = 'ready_category_seo_pc_new_with_usb';
public const READY_CATEGORY_SEO_BLACK_ELECTRONICS = 'ready_category_seo_black_electronics';

/**
* @param \App\Model\CategorySeo\ReadyCategorySeoMixDataFactory $readyCategorySeoMixDataFactory
Expand Down Expand Up @@ -164,7 +165,7 @@ public function load(ObjectManager $manager)
t('Electronics in black', [], Translator::DATA_FIXTURES_TRANSLATION_DOMAIN, $firstDomainLocale),
['elektro-barva-cerna'],
$firstDomainId,
null,
self::READY_CATEGORY_SEO_BLACK_ELECTRONICS,
t('description of Electronics in black seo category', [], Translator::DATA_FIXTURES_TRANSLATION_DOMAIN, $firstDomainLocale),
t('short description of Electronics in black seo category', [], Translator::DATA_FIXTURES_TRANSLATION_DOMAIN, $firstDomainLocale),
t('title of Electronics in black seo category', [], Translator::DATA_FIXTURES_TRANSLATION_DOMAIN, $firstDomainLocale),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ public function findOrCreateParameterValueByParameterValueData(
$parameterValue = $this->getParameterValueRepository()->findOneBy([
'text' => $parameterValueData->text,
'locale' => $parameterValueData->locale,
'rgbHex' => $parameterValueData->rgbHex,
]);

if ($parameterValue === null) {
Expand All @@ -255,6 +254,11 @@ public function findOrCreateParameterValueByParameterValueData(
$this->em->flush();
}

if ($parameterValue->getRgbHex() !== $parameterValueData->rgbHex) {
$parameterValue->edit($parameterValueData);
$this->em->flush();
}

return $parameterValue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,4 +405,34 @@ private function getDataForCategoryWithFiltersMatchingSeoMix(): array

return $this->getResponseDataForGraphQlType($responseForCategory, 'slug');
}

public function testCategoryFilterIsCorrectlySetAsSelected(): void
{
/** @var \App\Model\CategorySeo\ReadyCategorySeoMix $blackElectronicsSeoCategory */
$blackElectronicsSeoCategory = $this->getReferenceForDomain(ReadyCategorySeoDataFixture::READY_CATEGORY_SEO_BLACK_ELECTRONICS, 1);
$seoMixUrlSlug = $this->urlGenerator->generate('front_category_seo', ['id' => $blackElectronicsSeoCategory->getId()]);
$variables = array_merge(['slug' => $seoMixUrlSlug]);
$responseForSeoMix = $this->getResponseContentForGql(__DIR__ . '/../../_graphql/query/ReadyCategorySeoMixQuery.graphql', $variables);

$data = $this->getResponseDataForGraphQlType($responseForSeoMix, 'slug');

$colorFilter = array_filter(
$data['products']['productFilterOptions']['parameters'],
static function ($item) {
return $item['__typename'] === 'ParameterColorFilterOption';
},
);

$colorFilterValues = reset($colorFilter)['values'];
$black = t('black', [], Translator::DATA_FIXTURES_TRANSLATION_DOMAIN, $this->getFirstDomainLocale());

$blackColorFilter = array_filter(
$colorFilterValues,
static function ($item) use ($black) {
return $item['text'] === $black;
},
);

$this->assertTrue(reset($blackColorFilter)['isSelected']);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ query ReadyCategorySeoMix($slug: String!, $orderingMode: ProductOrderingModeEnum
...on ParameterSliderFilterOption {
selectedValue
}
... on ParameterColorFilterOption {
name
values {
text
isSelected
}
}
}
flags {
flag {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SortingBarItem } from './SortingBarItem';
import { SortIcon } from 'components/Basic/Icon/IconsSvg';
import { DEFAULT_SORT } from 'config/constants';
import { ProductOrderingModeEnumApi } from 'graphql/generated';
import { DEFAULT_SORT } from 'helpers/filterOptions/seoCategories';
import { getUrlQueriesWithoutDynamicPageQueries } from 'helpers/parsing/urlParsing';
import { twMergeCustom } from 'helpers/twMerge';
import { useQueryParams } from 'hooks/useQueryParams';
Expand Down
34 changes: 24 additions & 10 deletions project-base/storefront/components/Pages/CategoryDetail/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,16 @@ import {
CategoryDetailQueryDocumentApi,
CategoryDetailQueryVariablesApi,
} from 'graphql/generated';
import { buildNewQueryAfterFilterChange } from 'helpers/filterOptions/buildNewQueryAfterFilterChange';
import { getFilterWithoutEmpty } from 'helpers/filterOptions/getFilterWithoutEmpty';
import { mapParametersFilter } from 'helpers/filterOptions/mapParametersFilter';
import { getFilterWithoutSeoSensitiveFilters } from 'helpers/filterOptions/seoCategories';
import { getStringWithoutLeadingSlash } from 'helpers/parsing/stringWIthoutSlash';
import { getSlugFromUrl } from 'helpers/parsing/urlParsing';
import {
getSlugFromUrl,
getUrlQueriesWithoutFalsyValues,
getUrlQueriesWithoutDynamicPageQueries,
} from 'helpers/parsing/urlParsing';
import { useQueryParams } from 'hooks/useQueryParams';
import { NextRouter, useRouter } from 'next/router';
import { useState, useEffect } from 'react';
Expand All @@ -25,11 +32,11 @@ export const useCategoryDetailData = (
const urlSlug = getSlugFromUrl(router.asPath);
const [lastFetchedUrlSlug, setLastFetchedUrlSlug] = useState(urlSlug);
const client = useClient();
const mappedFilter = mapParametersFilter(filter);
const mappedProductFilter = mapParametersFilter(filter);
const { sort } = useQueryParams();
const wasRedirectedToSeoCategory = useSessionStore((s) => s.wasRedirectedToSeoCategory);
const [categoryDetailData, setCategoryDetailData] = useState(
readCategoryDetailFromCache(client, urlSlug, sort, mappedFilter),
readCategoryDetailFromCache(client, urlSlug, sort, mappedProductFilter),
);
const setOriginalCategorySlug = useSessionStore((s) => s.setOriginalCategorySlug);
const setWasRedirectedToSeoCategory = useSessionStore((s) => s.setWasRedirectedToSeoCategory);
Expand All @@ -44,8 +51,8 @@ export const useCategoryDetailData = (
client
.query<CategoryDetailQueryApi, CategoryDetailQueryVariablesApi>(CategoryDetailQueryDocumentApi, {
urlSlug,
orderingMode: sort ?? null,
filter: mappedFilter,
orderingMode: sort,
filter: mappedProductFilter,
})
.toPromise()
.then((response) => {
Expand All @@ -55,6 +62,8 @@ export const useCategoryDetailData = (
urlSlug,
response.data?.category?.originalCategorySlug,
response.data?.category?.slug,
filter,
sort,
setWasRedirectedToSeoCategory,
setOriginalCategorySlug,
);
Expand All @@ -73,19 +82,24 @@ const handleSeoCategorySlugUpdate = (
urlSlug: string,
originalCategorySlug: string | undefined | null,
categorySlug: string | undefined,
currentFilter: FilterOptionsUrlQueryType | null,
currentSort: ProductOrderingModeEnumApi | null,
setWasRedirectedToSeoCategory: (value: boolean) => void,
setOriginalCategorySlug: (value: string | undefined) => void,
) => {
const isCurrentAndRedirectSlugDifferent = getStringWithoutLeadingSlash(categorySlug ?? '') !== urlSlug;

if (originalCategorySlug && isCurrentAndRedirectSlugDifferent && categorySlug) {
const { filteredFilter, filteredSort } = getFilterWithoutSeoSensitiveFilters(currentFilter, currentSort);
const filterWithoutEmpty = getFilterWithoutEmpty(filteredFilter);
const newQuery = buildNewQueryAfterFilterChange({}, filterWithoutEmpty, filteredSort);
const filteredQueries = getUrlQueriesWithoutDynamicPageQueries(getUrlQueriesWithoutFalsyValues(newQuery));

setWasRedirectedToSeoCategory(true);
router.replace(
{ pathname: '/categories/[categorySlug]', query: { categorySlug } },
{ pathname: categorySlug },
{
shallow: true,
},
{ pathname: '/categories/[categorySlug]', query: { categorySlug, ...filteredQueries } },
{ pathname: categorySlug, query: filteredQueries },
{ shallow: true },
);
}

Expand Down
27 changes: 27 additions & 0 deletions project-base/storefront/config/constants.ts
Original file line number Diff line number Diff line change
@@ -1 +1,28 @@
import { ProductOrderingModeEnumApi } from 'graphql/generated';

export const DEFAULT_PAGE_SIZE = 9;
export const DEFAULT_SORT = ProductOrderingModeEnumApi.PriorityApi as const;
/**
* For those that are set to "true", we optimistically navigate out from a SEO category when a value of that type is changed
* This setting needs to mirror the API functionality in the following way
* - if a filter type "blocks" SEO category on API, it needs to be set as SEO sensitive
* - if a filter type "allows" SEO category on API, it needs to be set as SEO insensitive
*
* @example
* if the current URL is a SEO category "/my-seo-category" and sorting (which is SEO sensitive)
* is changed, we navigate right away to "/my-normal-category?sort=NEW_SORTING"
*
* if the current URL is a SEO category "/my-seo-category" and availability (which is SEO insensitive)
* is changed, we stay in the SEO category and navigate to "/my-seo-category?onlyInStock=true"
*/
export const SEO_SENSITIVE_FILTERS = {
SORT: true,
AVAILABILITY: false,
PRICE: false,
FLAGS: true,
BRANDS: false,
PARAMETERS: {
CHECKBOX: true,
SLIDER: false,
},
};
7 changes: 7 additions & 0 deletions project-base/storefront/helpers/arrayUtils.ts
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
export const createEmptyArray = (length: number): null[] => Array(length).fill(null);

export const mergeNullableArrays = <T>(array1: T[] | undefined | null, array2: T[] | undefined | null) => {
const nonNullableArray1 = array1 ? array1 : [];
const nonNullableArray2 = array2 ? array2 : [];

return [...nonNullableArray1, ...nonNullableArray2];
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { DEFAULT_SORT } from 'config/constants';
import { ProductOrderingModeEnumApi } from 'graphql/generated';
import {
PAGE_QUERY_PARAMETER_NAME,
LOAD_MORE_QUERY_PARAMETER_NAME,
FILTER_QUERY_PARAMETER_NAME,
SORT_QUERY_PARAMETER_NAME,
} from 'helpers/queryParamNames';
import { UrlQueries, FilterQueries } from 'hooks/useQueryParams';

export const buildNewQueryAfterFilterChange = (
currentQuery: UrlQueries,
newFilter: FilterQueries,
newSort: ProductOrderingModeEnumApi | undefined,
) => {
const isWithFilterParams =
!!newFilter &&
(!!newFilter.onlyInStock ||
!!(newFilter.minimalPrice ?? undefined) ||
!!(newFilter.maximalPrice ?? null) ||
!!newFilter.brands?.length ||
!!newFilter.flags?.length ||
!!newFilter.parameters?.length);

const newQuery: UrlQueries = {
...currentQuery,
[PAGE_QUERY_PARAMETER_NAME]: undefined,
[LOAD_MORE_QUERY_PARAMETER_NAME]: undefined,
[FILTER_QUERY_PARAMETER_NAME]: isWithFilterParams ? JSON.stringify(newFilter) : undefined,
} as const;

if (newSort && newSort !== DEFAULT_SORT) {
newQuery[SORT_QUERY_PARAMETER_NAME] = newSort;
}

return newQuery;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { FilterOptionsUrlQueryType } from 'types/productFilter';

export const getFilterWithoutEmpty = (filter: FilterOptionsUrlQueryType | undefined | null) => {
if (!filter) {
return undefined;
}

const updatedFilter = { ...filter };

(Object.keys(updatedFilter) as Array<keyof typeof updatedFilter>).forEach((key) => {
const newFilterValue = updatedFilter[key];
if (Array.isArray(newFilterValue) && newFilterValue.length === 0) {
delete updatedFilter[key];
}
});

return updatedFilter;
};

0 comments on commit ea316f8

Please sign in to comment.