feat(next-queries): add next query preview logic #611
Conversation
…romise when a new one is created Add tests to the fetch and save action. EX-6119
packages/x-components/src/x-modules/next-queries/store/types.ts
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/next-queries/store/module.ts
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/next-queries/store/module.ts
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/next-queries/store/types.ts
Outdated
Show resolved
Hide resolved
| if (!query) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The query is a required parameter for this action, so no checks should be needed. We can trust TypeScript. With this I believe we can simplify the return type and just set a SearchResponse
There was a problem hiding this comment.
Even if the query is a required parameter, nothing avoids calling the function with an empty string, although that would not be a legit use of this function 🤔
Should I remove that comprobation anyway?
There was a problem hiding this comment.
I think Abraham is right, and I would leave the empty string check. Specially thinking of reusing the wire with custom wiring config in setups. Better prevent an error request to API
...ges/x-components/src/x-modules/next-queries/store/actions/fetch-next-query-preview.action.ts
Outdated
Show resolved
Hide resolved
...ponents/src/x-modules/next-queries/store/actions/fetch-and-save-next-query-preview.action.ts
Outdated
Show resolved
Hide resolved
...ponents/src/x-modules/next-queries/store/actions/fetch-and-save-next-query-preview.action.ts
Outdated
Show resolved
Hide resolved
...ponents/src/x-modules/next-queries/store/actions/fetch-and-save-next-query-preview.action.ts
Outdated
Show resolved
Hide resolved
...ges/x-components/src/x-modules/next-queries/store/actions/fetch-next-query-preview.action.ts
Outdated
Show resolved
Hide resolved
| const request: SearchRequest = { | ||
| query, | ||
| rows: state.config.resultsPreviewCount, | ||
| extraParams: state.params |
There was a problem hiding this comment.
Should we add an origin to these requests? Even if it is hardcoded I remember talking about it but I can't find the info in the JIRA task 😅
There was a problem hiding this comment.
Ook, which should be the origin?
Something like next_query:results?
There was a problem hiding this comment.
Im not sure because the query feature is the next-query, but the feature location is the predictive_layer? Because in this case, they are not still the query inside the grid, but im not sure about this or if we should create a new feature location?
packages/x-components/src/x-modules/next-queries/store/types.ts
Outdated
Show resolved
Hide resolved
… accept a dictionary of results EX-6119
packages/x-components/src/x-modules/next-queries/store/module.ts
Outdated
Show resolved
Hide resolved
- Rename NextQueryPreviewResults to PreviewResults - Add query to PreviewResults - Change event used to reset the preview results EX-6119
packages/x-components/src/x-modules/next-queries/config.types.ts
Outdated
Show resolved
Hide resolved
packages/search-types/src/query-signals/preview-results.model.ts
Outdated
Show resolved
Hide resolved
| if (!query) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
I think Abraham is right, and I would leave the empty string check. Specially thinking of reusing the wire with custom wiring config in setups. Better prevent an error request to API
tajespasarela
dismissed
javieri-empathy’s stale review
to do in a new task: https://searchbroker.atlassian.net/browse/EX-6756
EX-6119