Set correctly the filters in History Query when selecting a Query Preview with filters #1325
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CachedaCodes
suggested changes
Oct 18, 2023
...-modules/history-queries/store/actions/update-history-queries-with-search-response.action.ts
Outdated
Show resolved
Hide resolved
...-modules/history-queries/store/actions/update-history-queries-with-search-response.action.ts
Outdated
Show resolved
Hide resolved
...-modules/history-queries/store/actions/update-history-queries-with-search-response.action.ts
Outdated
Show resolved
Hide resolved
...-modules/history-queries/store/actions/update-history-queries-with-search-response.action.ts
Outdated
Show resolved
Hide resolved
packages/x-components/src/x-modules/history-queries/store/__tests__/actions.spec.ts
Outdated
Show resolved
Hide resolved
annacv
force-pushed
the
bugfix/EMP-2427-Set-filters-from-query-preview
branch
from
3d03a19 to
e0e34ac
Compare
CachedaCodes
suggested changes
Oct 20, 2023
...-modules/history-queries/store/actions/update-history-queries-with-search-response.action.ts
Outdated
Show resolved
Hide resolved
…ons/update-history-queries-with-search-response.action.ts Co-authored-by: Guillermo Cacheda <[email protected]>
CachedaCodes
suggested changes
Oct 23, 2023
| @@ -369,9 +401,8 @@ describe('testing history queries module actions', () => { | |||
| }); | |||
|
|
|||
| // eslint-disable-next-line max-len | |||
| it('updates a history query if search response change because a filter is selected', async () => { | |||
| it('updates a history query when the search response changes because a filter is selected', async () => { | |||
There was a problem hiding this comment.
It's worth testing with facets with __unknown__ as id. Create a new test for it and where done 👍
…known__' facet id in the response EMP-2427
CachedaCodes
suggested changes
Oct 25, 2023
| gato.totalResults = 50; | ||
| const selectedFilters = Object.values(requestFilters)[0]; | ||
| resetStateWith({ historyQueries: [gato, perro] }); | ||
| await store.dispatch('updateHistoryQueriesWithSearchResponse', { |
There was a problem hiding this comment.
Testing this doesn't make much sense to me. The response shouldn't return facets with id __unknown__. The backend should know the id of the facet and __unknown__ is a key used only in the front to identify a filter which we don't know the facet id.
The test I was suggesting was the behaviour of the action when a Query preview was selected. Selecting a QP creates a HQ of that request, but the QP creates the filters with facet id __unknown__. The test should check that the filters map correctly when the requestFilters have an unknown facet id.
CachedaCodes
approved these changes
Oct 26, 2023
CachedaCodes
deleted the
bugfix/EMP-2427-Set-filters-from-query-preview
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EMP-2428
When we save the filters in the History Queries action update-history-queries-with-search-response.action.ts we were doing:
const filters = createHistoryQueriesFiltersList(searchResponse.request.filters);In the case of filters that come from aQueryPreview, we only haveidso we should compare the filters with the response to get the rest of the data.In this PR we use facets with selected filters from the response to be able to provide a label for the
selectedFiltersalthough the stored item has only theidavailable.TEST
Use VueDevTools and check the history queries state:
queryPreviewwith filters (e.g.cortina) and check the historyQuery's filters have a label.Check that saving filters on history queries is still working properly:
queryPreviewwith filters, and delete the query. If you select the query from history queries, the filters are applied.