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

feat: allow queries preview with same query but different filters or params #1392

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f37961f
feat(query-preview): convert queriesPreview keys into hash keys
lauramargar Dec 19, 2023
17f6daf
chore: remove rollup from renovate restrictions
diegopf Dec 19, 2023
9b56106
feat(query-preview): implementation with 2 lists
lauramargar Dec 20, 2023
a4a0802
Merge remote-tracking branch 'origin/main' into feature/EMP-3079-allo…
lauramargar Dec 21, 2023
9ecec58
feat(query-preview): new implementation with an internal cache
lauramargar Dec 27, 2023
7da122b
draft `QueryPreviewMounted` and `QueryPreviewUnmounted` events
diegopf Dec 29, 2023
143211f
fix(query-preview): component, actions and getters tests
lauramargar Jan 11, 2024
52f2206
fix(query-preview): fix tests and add documentation
lauramargar Jan 12, 2024
b7a39a2
fix(query-preview): fix e2e tests
lauramargar Jan 15, 2024
a2da941
Merge branch 'main' into feature/EMP-3079-allow-queries-preview-with-…
annacv Jan 15, 2024
0359041
update lockfile
annacv Jan 15, 2024
218bf6e
doc: fix doc test typo
annacv Jan 15, 2024
1d86255
doc: add query preview events types documentation
annacv Jan 15, 2024
8c0b15a
chore: rm unused constant
annacv Jan 15, 2024
2f1f995
doc: add get-hash-from-query-preview.ts doc
annacv Jan 15, 2024
11e0e76
fix(query-preview): pr comments
lauramargar Jan 17, 2024
4ecb9a6
Merge branch 'main' into feature/EMP-3079-allow-queries-preview-with-…
lauramargar Jan 17, 2024
7c4f689
fix(query-preview): add a missing coma
lauramargar Jan 17, 2024
2782b4b
fix: pr comments
lauramargar Jan 24, 2024
46fd292
fix: QueryPreviewUnmounted payload
lauramargar Jan 24, 2024
c28dd0c
Merge branch 'main' into feature/EMP-3079-allow-queries-preview-with-…
lauramargar Jan 25, 2024
1f32b13
fix: leak of doc and types
lauramargar Jan 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,11 @@
*/
protected get renderedQueryPreviews(): QueryPreviewInfo[] {
return this.queriesPreviewInfo.filter(item => {
const query = getHashFromQueryPreviewInfo(item);
return this.queriesStatus[query] === 'success' || this.queriesStatus[query] === 'loading';
const queryPreviewHash = getHashFromQueryPreviewInfo(item);
return (
this.queriesStatus[queryPreviewHash] === 'success' ||
this.queriesStatus[queryPreviewHash] === 'loading'
);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,12 @@
/**
* Query Preview key converted into a unique id.
*
* @returns The query hash.
* @internal
*/
public queryOfQueryPreviewHash = getHashFromQueryPreviewInfo(this.queryPreviewInfo);
public get queryPreviewHash(): string {
return getHashFromQueryPreviewInfo(this.queryPreviewInfo);
}

/**
* The computed request object to be used to retrieve the query preview results.
Expand Down Expand Up @@ -199,7 +202,7 @@
* @returns The results preview of the actual query preview.
*/
public get queryPreviewResults(): Partial<QueryPreviewItem> | undefined {
const previewResults = this.previewResults[this.queryOfQueryPreviewHash];
const previewResults = this.previewResults[this.queryPreviewHash];
return previewResults?.results
? {
...previewResults,
Expand Down Expand Up @@ -236,12 +239,12 @@
}
);

const cachedQueryPreview = this.previewResults[this.queryOfQueryPreviewHash];
const cachedQueryPreview = this.previewResults[this.queryPreviewHash];

// If the query has been saved it will emit load instead of the emitting the updated request.
if (cachedQueryPreview?.status === 'success') {
this.$emit('load', this.queryOfQueryPreviewHash);
this.$x.emit('QueryPreviewMounted', this.queryOfQueryPreviewHash, {
this.$emit('load', this.queryPreviewHash);
this.$x.emit('QueryPreviewMounted', this.queryPreviewHash, {
priority: 0,
replaceable: false
});
Expand All @@ -262,7 +265,7 @@
this.emitQueryPreviewRequestUpdated.cancel();
this.$x.emit(
'QueryPreviewUnmounted',
{ query: this.queryOfQueryPreviewHash, cache: this.persistInCache },
{ query: this.queryPreviewHash, cache: this.persistInCache },
{
priority: 0,
replaceable: false
Expand Down Expand Up @@ -295,9 +298,9 @@
@Watch('queryPreviewResults.status')
emitLoad(status: RequestStatus | undefined): void {
if (status === 'success') {
this.$emit(this.results?.length ? 'load' : 'error', this.queryOfQueryPreviewHash);
this.$emit(this.results?.length ? 'load' : 'error', this.queryPreviewHash);
} else if (status === 'error') {
this.$emit('error', this.queryOfQueryPreviewHash);
this.$emit('error', this.queryPreviewHash);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ export interface QueriesPreviewXEvents {
/**
* The query preview has been mounted.
* Payload: The query preview query as a key converted into a unique id (query hash).
* When QueryPreviewMounted is emitted, the instances count is increased.
*/
QueryPreviewMounted: string;
/**
* The query preview has been unmounted.
* Payload: The query preview's unique id (query hash) and its cache value.
* When QueryPreviewUnmounted is emitted, the instance count for that query hash
* will be decreased. If the cache is set to false, the query will be removed
* safely from the state since there will be no component showing that data.
*/
QueryPreviewUnmounted: { query: string; cache: boolean };
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const fetchAndSaveQueryPreview: QueriesPreviewXStoreModule['actions']['fe
.catch(error => {
// eslint-disable-next-line no-console
console.error(error);
const query = getHashFromQueryPreviewItem(queryPreviewItem);
commit('setStatus', { query, status: 'error' });
const queryPreviewHash = getHashFromQueryPreviewItem(queryPreviewItem);
commit('setStatus', { queryPreviewHash, status: 'error' });
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Most (if not all) of the methods here no longer receive the query of the QP, they receive a queryPreviewHash. I think we should replace the namings of the parameters to make it clear

Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,37 @@ export const queriesPreviewXStoreModule: QueriesPreviewXStoreModule = {
}),
getters: { loadedQueriesPreview },
mutations: {
clearQueryPreview(state, query) {
Vue.delete(state.queriesPreview, query);
clearQueryPreview(state, queryPreviewHash) {
Vue.delete(state.queriesPreview, queryPreviewHash);
},
setParams(state, params) {
state.params = params;
},
setQueryPreviewCached(state, queryPreview) {
Vue.set(state.queriesPreview, getHashFromQueryPreviewItem(queryPreview), queryPreview);
},
setStatus(state, { query, status }) {
state.queriesPreview[query].status = status;
setStatus(state, { queryPreviewHash, status }) {
state.queriesPreview[queryPreviewHash].status = status;
},
setSelectedQueryPreview(state, selectedQueryPreview) {
state.selectedQueryPreview = selectedQueryPreview;
},
setConfig,
mergeConfig,
addQueryPreviewInstance(state, query: string) {
if (state.queriesPreview[query]) {
state.queriesPreview[query].instances += 1;
addQueryPreviewInstance(state, queryPreviewHash: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to specify the type in queryPreviewHash: string, it's already being done in the types file, remove the string.

if (state.queriesPreview[queryPreviewHash]) {
state.queriesPreview[queryPreviewHash].instances += 1;
}
},
removeQueryPreviewInstance(state, { query, cache }: { query: string; cache: boolean }) {
if (state.queriesPreview[query]) {
state.queriesPreview[query].instances -= 1;
removeQueryPreviewInstance(
state,
{ queryPreviewHash, cache }: { queryPreviewHash: string; cache: boolean }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, you don't need the type in { queryPreviewHash, cache }: { queryPreviewHash: string; cache: boolean }, leave it as { queryPreviewHash, cache }

) {
if (state.queriesPreview[queryPreviewHash]) {
state.queriesPreview[queryPreviewHash].instances -= 1;

if (!cache && state.queriesPreview[query].instances === 0) {
Vue.delete(state.queriesPreview, query);
if (!cache && state.queriesPreview[queryPreviewHash].instances === 0) {
Vue.delete(state.queriesPreview, queryPreviewHash);
}
}
}
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, we have to update the docs of the methods and parameters to reflect that we no longer use query as the Dictionary key.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface QueryPreviewItem extends StatusState {
request: SearchRequest;
/** Results of the query preview request. */
results: Result[];
/** Display tagging info */
/** Display tagging info. */
displayTagging?: TaggingRequest;
/** The total number of results for the search query. */
totalResults: number;
Expand Down Expand Up @@ -84,9 +84,9 @@ export interface QueriesPreviewMutations extends ConfigMutations<QueriesPreviewS
/**
* Removes a query preview entry from the queries preview's dictionary.
*
* @param query - Query whose entry will be removed.
* @param queryPreviewHash - Query hash whose entry will be removed.
*/
clearQueryPreview(query: string): void;
clearQueryPreview(queryPreviewHash: string): void;
/**
* Sets the extra params of the module.
*
Expand All @@ -111,8 +111,14 @@ export interface QueriesPreviewMutations extends ConfigMutations<QueriesPreviewS
* @param selectedQueryPreview - The selected query preview to save to the state.
*/
setSelectedQueryPreview(selectedQueryPreview: QueryPreviewInfo | null): void;
addQueryPreviewInstance(query: string): void;
removeQueryPreviewInstance({ query, cache }: { query: string; cache: boolean }): void;
addQueryPreviewInstance(queryPreviewHash: string): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docs

removeQueryPreviewInstance({
Copy link
Contributor

Choose a reason for hiding this comment

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

Add docs

queryPreviewHash,
cache
}: {
queryPreviewHash: string;
cache: boolean;
}): void;
}

/**
Expand Down Expand Up @@ -169,9 +175,9 @@ export type QueriesPreviewActionContext = XActionContext<
*/
export interface QueryPreviewStatusPayload {
/**
* The query whose request status to modify.
* The query hash whose request status to modify.
*/
query: string;
queryPreviewHash: string;
/**
* The new request status.
*/
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Make the QP in the tests have filters. Half of the implementation is not being tested right now

Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ describe('testing queries preview module utils', () => {
status: 'success',
request: {
query: 'tshirt',
filters: {
fit: [
{
id: 'fit:regular',
modelName: 'RawFilter',
selected: true
}
]
},
rows: 3
}
};
Expand All @@ -56,11 +65,11 @@ describe('testing queries preview module utils', () => {
});

it('should check if a query hash from a QueryPreviewInfo is created correctly', () => {
const queryPreviewInfo: QueryPreviewInfo = { query: 'tshirt' };
const queryPreviewInfo: QueryPreviewInfo = { query: 'tshirt', filters: ['fit:regular'] };

const queryPreviewHash = getHashFromQueryPreviewInfo(queryPreviewInfo);

expect(queryPreviewHash).toBe('8f58dcbfc41b2074e9311014903a1a5f');
expect(queryPreviewHash).toBe('ba83786514cc76ebfd00da880b8068b2');
});

// eslint-disable-next-line max-len
Expand All @@ -72,12 +81,21 @@ describe('testing queries preview module utils', () => {
status: 'success',
request: {
query: 'tshirt',
filters: {
fit: [
{
id: 'fit:regular',
modelName: 'RawFilter',
selected: true
}
]
},
rows: 3
}
};
const queryPreviewItemHash = getHashFromQueryPreviewItem(queryPreviewItem);

const queryPreviewInfo: QueryPreviewInfo = { query: 'tshirt' };
const queryPreviewInfo: QueryPreviewInfo = { query: 'tshirt', filters: ['fit:regular'] };
const queryPreviewInfoHash = getHashFromQueryPreviewInfo(queryPreviewInfo);

expect(queryPreviewItemHash).toBe(queryPreviewInfoHash);
Expand Down
lauramargar marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,12 @@ import { QueryPreviewInfo, QueryPreviewItem } from '../store/index';
* @returns A unique id that will be used as a key to store the QueryPreviewItem in the state.
*/
export const getHashFromQueryPreviewItem = (queryPreview: QueryPreviewItem): string => {
let queryPreviewFilters = '';
if (queryPreview.request.filters) {
const queryPreviewFiltersKeys = Object.keys(queryPreview.request.filters);
queryPreviewFiltersKeys.forEach(key => {
queryPreview.request.filters![key].forEach(filter => {
queryPreviewFilters = queryPreviewFilters.concat('-' + filter.id.toString());
});
});
}
const queryPreviewFilters = queryPreview.request.filters
? Object.values(queryPreview.request.filters)
.flat()
.map(filter => filter.id.toString())
.join('-')
: '';

return md5(queryPreview.request.query.concat(queryPreviewFilters));
};
Expand All @@ -29,12 +26,7 @@ export const getHashFromQueryPreviewItem = (queryPreview: QueryPreviewItem): str
* @returns A unique id that will be used as a key to check the QueryPreview in the state.
*/
export const getHashFromQueryPreviewInfo = (queryPreviewInfo: QueryPreviewInfo): string => {
let queryPreviewFilters = '';
if (queryPreviewInfo.filters) {
queryPreviewInfo.filters.forEach(filter => {
queryPreviewFilters = queryPreviewFilters.concat('-' + filter);
});
}
const queryPreviewFilters = queryPreviewInfo.filters ? queryPreviewInfo.filters.join('-') : '';

return md5(queryPreviewInfo.query.concat(queryPreviewFilters));
};
Loading
Loading