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

[FW-1848] SEO categories with ignored filters #2891

Merged
merged 7 commits into from Nov 20, 2023

Conversation

sebaholesz
Copy link
Contributor

@sebaholesz sebaholesz commented Oct 19, 2023

Q A
Description, reason for the PR After adding the option for SEO categories to ignore certain filters, we had to implement this on SF as well.
New feature Yes
BC breaks No
Fixes issues ...
Have you read and signed our License Agreement for contributions? Yes

🌐 Live Preview:

@sebaholesz sebaholesz added the Enhancement New feature or request for change from user point of view label Oct 19, 2023
@sebaholesz sebaholesz self-assigned this Oct 19, 2023
@sebaholesz sebaholesz changed the base branch from 14.0 to tv-FW-1711-add-instant-skeletons October 21, 2023 09:26
@sebaholesz sebaholesz changed the base branch from tv-FW-1711-add-instant-skeletons to 14.0 October 21, 2023 09:27
@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch 2 times, most recently from df6ef3f to 42612b8 Compare October 23, 2023 11:31
Copy link
Member

@pk16011990 pk16011990 left a comment

Choose a reason for hiding this comment

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

@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch 2 times, most recently from 0fd80f8 to f0fbc6e Compare October 27, 2023 09:37
@TomasLudvik TomasLudvik force-pushed the sh-seo-categories-with-ignored-filters branch from f0fbc6e to 061480d Compare November 7, 2023 08:01
@TomasLudvik TomasLudvik changed the base branch from 14.0 to tl-fix-colour-parameters November 7, 2023 08:02
@romankuna
Copy link

There are this problems now:

First scenario

  1. i am on page https://sh-seo-categories-with-ignored-filters.odin.shopsys.cloud/electronics
  2. i select black color parameter
  3. URL changed correctly https://sh-seo-categories-with-ignored-filters.odin.shopsys.cloud/elektro-barva-cerna
  4. i choose brand "LG" from filter
  5. i thing, URL is correctly https://sh-seo-categories-with-ignored-filters.odin.shopsys.cloud/elektro-barva-cerna?filter=%7B%22brands%22%3A%5B%228a8dfdac-223b-41b6-a35d-a1577d376255%22%5D%7D
    image
  6. when i remove color "black" parameter from filter, brand "LG" is remove too
Zaznam.obrazovky.2023-11-07.v.14.10.23.mov

Second scenario

  1. i am on page https://sh-seo-categories-with-ignored-filters.odin.shopsys.cloud/tv-audio
  2. i choose seo category "TV,audio plasma with HDMI" and i see correcty https://sh-seo-categories-with-ignored-filters.odin.shopsys.cloud/televize-audio-plasma-s-hdmi and selected parameters "Technology", "HDMI"
  3. i select brand "Hyundai" => i see still correctly seo category, with brand parameter
  4. i select "screen size" parameter - value 27"
  5. now, after selecting Screen size paremeter, "Hyundai" parameter was removed automaticly - this is not correctly
Zaznam.obrazovky.2023-11-07.v.14.25.13.mov

@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch from fc59f61 to 2751a1e Compare November 15, 2023 11:41
@sebaholesz sebaholesz changed the base branch from tl-fix-colour-parameters to 14.0 November 15, 2023 11:41
@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch from 9d5364a to 2751a1e Compare November 15, 2023 11:59
@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch from 97dc7df to e87d92e Compare November 15, 2023 16:44
@romankuna
Copy link

Now it's ok for me

TomasLudvik and others added 7 commits November 20, 2023 14:43
…s in the URL query

- the need for this functionality is based on the requirement that SEO
categories should ignore (be insensitive) to some filters (such as
brands or availability)
- to support this functionality, some common logic was extracted into
helper functions (getFilterWithoutEmpty, and
buildNewQueryAfterFilterChange)
-  getUrlQueriesWithoutDynamicPageQueries which previously also
removed falsy values from query (such as undefined or null) was split
into two functions, namely getUrlQueriesWithoutDynamicPageQueries
which now only takes care of removing the friendly page dynamic segment
and getUrlQueriesWithoutFalsyValues which removes the falsy values
…nd seoCategories

- this required some additional changes, such as moving hooks from
helpers/seoCategories to hooks/seoCategories, which is necessary,
because it is both a bad design and also complicates the matters
when writing tests as it requires additional mocking
- constants were moved to config/constants to improve both
how we work with constants and how we can mock them in tests
- this had to be fixed in all existing tests
- unit-tests.md was formatted to satisfy standards
@sebaholesz sebaholesz force-pushed the sh-seo-categories-with-ignored-filters branch from e87d92e to 603fb6c Compare November 20, 2023 13:44
@sebaholesz sebaholesz merged commit ea316f8 into 14.0 Nov 20, 2023
18 checks passed
@sebaholesz sebaholesz deleted the sh-seo-categories-with-ignored-filters branch November 20, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request for change from user point of view
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants