From b1005b28d383e6f7f5c00bfeffc872248bfb2c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francesco=20Borz=C3=AC?= Date: Fri, 27 Dec 2019 13:33:21 +0100 Subject: [PATCH] fix(SAI/search): solve issue with duplicate search results (#306) --- .../sai-search-existing.integration.spec.ts | 8 ++++---- src/app/services/query.service.spec.ts | 20 +++++++++++++++++++ src/app/services/query.service.ts | 13 +++++++++++- src/app/services/search/sai-search.service.ts | 5 ++++- .../services/search/search.service.spec.ts | 2 +- src/app/services/search/search.service.ts | 16 +++++++++++++-- 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/app/components/editors/smart-scripts/sai-search-existing/sai-search-existing.integration.spec.ts b/src/app/components/editors/smart-scripts/sai-search-existing/sai-search-existing.integration.spec.ts index 90e7fda735..364ffe85bd 100644 --- a/src/app/components/editors/smart-scripts/sai-search-existing/sai-search-existing.integration.spec.ts +++ b/src/app/components/editors/smart-scripts/sai-search-existing/sai-search-existing.integration.spec.ts @@ -66,19 +66,19 @@ describe('SaiSearchExisting integration tests', () => { for (const { testId, entryorguid, source_type, limit, expectedQuery } of [ { testId: 1, entryorguid: 1, source_type: 2, limit: '100', expectedQuery: - 'SELECT * FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%1%\') AND (`source_type` LIKE \'%2%\') LIMIT 100' + 'SELECT `entryorguid`, `source_type` FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%1%\') AND (`source_type` LIKE \'%2%\') GROUP BY entryorguid, source_type LIMIT 100' }, { testId: 2, entryorguid: '', source_type: 2, limit: '100', expectedQuery: - 'SELECT * FROM `smart_scripts` WHERE (`source_type` LIKE \'%2%\') LIMIT 100' + 'SELECT `entryorguid`, `source_type` FROM `smart_scripts` WHERE (`source_type` LIKE \'%2%\') GROUP BY entryorguid, source_type LIMIT 100' }, { testId: 3, entryorguid: 123, source_type: '', limit: '100', expectedQuery: - 'SELECT * FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%123%\') LIMIT 100' + 'SELECT `entryorguid`, `source_type` FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%123%\') GROUP BY entryorguid, source_type LIMIT 100' }, { testId: 4, entryorguid: 123, source_type: '', limit: '', expectedQuery: - 'SELECT * FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%123%\')' + 'SELECT `entryorguid`, `source_type` FROM `smart_scripts` WHERE (`entryorguid` LIKE \'%123%\') GROUP BY entryorguid, source_type' }, ]) { it(`searching an existing entity should correctly work [${testId}]`, () => { diff --git a/src/app/services/query.service.spec.ts b/src/app/services/query.service.spec.ts index b69fd126c7..e27d0a1440 100644 --- a/src/app/services/query.service.spec.ts +++ b/src/app/services/query.service.spec.ts @@ -136,6 +136,26 @@ describe('QueryService', () => { 'FROM `my_keira3` LIMIT 20' ); }); + + it('should properly work when using fields, limit, selectFields and groupField', () => { + const queryForm: QueryForm = { + fields: { + myField1: 'myValue1', + myField2: 'myValue2', + }, + limit: '20', + }; + + const selectFields = ['sel1', 'sel2']; + const groupFields = ['sel1', 'sel2', 'sel3']; + + expect(service.getSearchQuery(table, queryForm, selectFields, groupFields)).toEqual( + 'SELECT `sel1`, `sel2` ' + + 'FROM `my_keira3` WHERE (`myField1` LIKE \'%myValue1%\') AND (`myField2` LIKE \'%myValue2%\') ' + + 'GROUP BY sel1, sel2, sel3 LIMIT 20' + ); + }); + }); it('selectAll() should correctly work', async(() => { diff --git a/src/app/services/query.service.ts b/src/app/services/query.service.ts index 54d206f3b6..b11abe47bc 100644 --- a/src/app/services/query.service.ts +++ b/src/app/services/query.service.ts @@ -34,8 +34,13 @@ export class QueryService { ); } - getSearchQuery(table: string, queryForm: QueryForm) { + getSearchQuery(table: string, queryForm: QueryForm, selectFields: string[] = null, groupFields: string[] = null): string { const query = squel.select(squelConfig).from(table); + + if (selectFields) { + query.fields(selectFields); + } + const filters = queryForm.fields; for (const filter in filters) { @@ -46,6 +51,12 @@ export class QueryService { } } + if (groupFields) { + for (const groupField of groupFields) { + query.group(groupField); + } + } + if (queryForm.limit) { query.limit(Number.parseInt(queryForm.limit, 10)); } diff --git a/src/app/services/search/sai-search.service.ts b/src/app/services/search/sai-search.service.ts index 1a4d458b58..5140c10b3a 100644 --- a/src/app/services/search/sai-search.service.ts +++ b/src/app/services/search/sai-search.service.ts @@ -2,12 +2,15 @@ import { Injectable } from '@angular/core'; import { SearchService } from './search.service'; import { QueryService } from '../query.service'; -import { SAI_SEARCH_FIELDS, SAI_TABLE, SmartScripts } from '../../types/smart-scripts.type'; +import { SAI_ID_FIELDS, SAI_SEARCH_FIELDS, SAI_TABLE, SmartScripts } from '../../types/smart-scripts.type'; @Injectable({ providedIn: 'root' }) export class SaiSearchService extends SearchService { + selectFields = SAI_ID_FIELDS; + groupFields = SAI_ID_FIELDS; + /* istanbul ignore next */ // because of: https://github.com/gotwarlost/istanbul/issues/690 constructor( protected queryService: QueryService, diff --git a/src/app/services/search/search.service.spec.ts b/src/app/services/search/search.service.spec.ts index ee56f20d0d..8aa7ac5878 100644 --- a/src/app/services/search/search.service.spec.ts +++ b/src/app/services/search/search.service.spec.ts @@ -35,7 +35,7 @@ describe('SearchService', () => { it('should update the query if the form is valid', () => { service.queryForm.get('limit').setValue(123); - expect(spy).toHaveBeenCalledWith(service['entityTable'], service.queryForm.getRawValue()); + expect(spy).toHaveBeenCalledWith(service['entityTable'], service.queryForm.getRawValue(), null, null); expect(service.query).toEqual(newQuery); }); diff --git a/src/app/services/search/search.service.ts b/src/app/services/search/search.service.ts index 9380e9b04c..54ae334ecc 100644 --- a/src/app/services/search/search.service.ts +++ b/src/app/services/search/search.service.ts @@ -12,6 +12,8 @@ export abstract class SearchService extends SubscriptionHand 'limit': new FormControl(100), 'fields': this.fields, }); + selectFields: string[] = null; + groupFields: string[] = null; constructor( protected queryService: QueryService, @@ -24,12 +26,22 @@ export abstract class SearchService extends SubscriptionHand this.fields.addControl(field, new FormControl()); } - this.query = this.queryService.getSearchQuery(this.entityTable, this.queryForm.getRawValue()); + this.query = this.queryService.getSearchQuery( + this.entityTable, + this.queryForm.getRawValue(), + this.selectFields, + this.groupFields, + ); this.subscriptions.push( this.queryForm.valueChanges.subscribe(() => { if (this.queryForm.valid) { - this.query = this.queryService.getSearchQuery(this.entityTable, this.queryForm.getRawValue()); + this.query = this.queryService.getSearchQuery( + this.entityTable, + this.queryForm.getRawValue(), + this.selectFields, + this.groupFields, + ); } }) );