Conversation
victorcg88
force-pushed
the
feat/RST-4852-config-to-retrieve-facet
branch
from
a61786e to
58b61ce
Compare
victorcg88
force-pushed
the
feat/RST-4852-config-to-retrieve-facet
branch
from
58b61ce to
0a9e9b7
Compare
| const x = use$x() | ||
| const stores = ['Spain', 'Portugal', 'Italy'] | ||
| const initialExtraParams = { store: 'Portugal' } | ||
| const initialExtraParams = { store: 'ES', separateFacets: false } |
There was a problem hiding this comment.
My 50 cents in naming: isolateFacetsRequest
| extraParams: 'extraParams', | ||
| extraParams: ({ extraParams: { separateFacets, ...rest } = {} }) => ({ | ||
| ...rest, | ||
| facets: !separateFacets, |
There was a problem hiding this comment.
Thinking in the retrocompatibility of this assertion. ...(!!isolateFacetsRequest && { facets: false }), I think it fits better because the below caustics:
isolateFacetsRequest === undefined-> search as always with facetsisolateFacetsRequest === false-> search as always with facetsisolateFacetsRequest === true-> search without facets
As I don't know if all search endpoint will give support to this newfacetsparameter, I prefer to only send this new one where it is "mandatory"
There was a problem hiding this comment.
before merging into main: restore this file
There was a problem hiding this comment.
before merging into main: restore this file
There was a problem hiding this comment.
before merging into main: restore this file
There was a problem hiding this comment.
I'm worried about such a deep comparison that we didn't need in the search module...
There was a problem hiding this comment.
I was thinking in an alternative approach which could fits better with the current architecture. Let me try to explain that approach:
searchmodule keeps handling the request for retrieving facets. Regardless of whether it's against thesearchor thefacetsendpoint.- All structure in
searchmodule keeps remaining but with the casuistic of the newfacetsendpoint. facetsmodules keeps remaining a "symbolic" module to handle hard logic as easy- Both modules are coupled historically, so the key concept of "you can use a module without the other" is not true in that case. So if
searchperforms the request againstfacetsendpoint, I think it wouldn't be a problem itself. (you only need to check thefacetswiringupdateFacetsGroupWithSearchFacetsWireto see that coupling)
What are the advantages and disadvantages?
Advantages
- Not create a "mirror logic" for both modules.
- Stop making a monster bigger adding new needed properties and logic but without sense with the rest of the module
- A more reliable feature since it would perform the search logic
Disadvantages
facetsmodule should handle their own state/requests- Could we need in the future X for only
facetswithoutsearch?
TL;DR If we invest time in refactor deeply the facets module to make it real module isolation (refactor and understanding), go with requests in facets module and self-management (breaking changes coming...). If not, search remains responsible for retrieving facets from search or facets endpoint.
|
Conversation from 15/01/26 @victorcg88 & @joseacabaneros
Decision Going ahead with this PR (Intermediate 2) to avoid spending more time in an "intermediate" solution due to we have no time to invest in the complete solution. |
2 participants
Pull request template
Describe the purpose of the change, the specific changes done in detail, and the issue you have fixed.
Motivation and context
Type of change
What is the destination branch of this PR?
MainHow has this been tested?
Tests performed according to testing guidelines:
Checklist: