MAGE-1465 - MAGE-1463: add configurable sort/paging parameters #1873
Conversation
damcou
changed the title
cammonro
left a comment
cammonro
left a comment
There was a problem hiding this comment.
Excellent work! 👏
This is working well so far but I think we will need to iron out this implementation a bit more. I've provided some direct feedback here in the PR but will also follow up in Jira.
| } | ||
| uiStateProductIndex['sortBy'] = routeParameters.sortBy; | ||
| uiStateProductIndex['page'] = routeParameters.page; | ||
| uiStateProductIndex['sortBy'] = algoliaConfig.sortingParameter === 'product_list_order' ? |
There was a problem hiding this comment.
Instead of an inline decision can we abstract this into a mixable function that can be invoked by the Search Adapter module?
| utils.sortParamToReplica(productIndexName, routeParameters[algoliaConfig.sortingParameter]) : | ||
| routeParameters[algoliaConfig.sortingParameter]; | ||
|
|
||
| uiStateProductIndex['page'] = routeParameters[algoliaConfig.pagingParameter]; |
There was a problem hiding this comment.
Wondering if we can also supply a mixable function here for the paging config.
| } | ||
| }, | ||
|
|
||
| replicaToSortParam: (productIndexName, replica) => { |
There was a problem hiding this comment.
This is working well! However, it's tightly coupled with the adapter module which is an optional install. If we can expose the hooks to integrate with the parameter logic via mixins as suggested above we can relocate this JavaScript logic in the adapter module and maintain separation of concerns.
| </field> | ||
| <field id="sort_param" translate="label comment" type="select" sortOrder="50" | ||
| showInDefault="1" showInWebsite="1" showInStore="1"> | ||
| <label>Sort Parameters</label> |
There was a problem hiding this comment.
Since this is a composite config for both paging and sorting I'm wondering if we should relocate and relabel - something like "Query String Parameters".
| <source_model>Algolia\AlgoliaSearch\Model\Source\SortParam</source_model> | ||
| <comment> | ||
| <model>Algolia\AlgoliaSearch\Model\Config\SortParam</model> | ||
| </comment> |
There was a problem hiding this comment.
It seems to me this setting will never make sense unless the user enables backend rendering. I suggest we provide a default value for this in the core module in the appropriate helper and implement it as a configurable setting in the adapter module. (i.e. relocate the logic to configure along with the JavaScript that performs the replica / sort attribute conversion).
I think we can also make it a dependency on catalog/search/engine - not via XML (since I don't think the core JS can handle across sections like that) but via a custom frontend_model.
This PR contains:
common.jsso it can handle this configuration