Skip to content

Commit

Permalink
fix(SAI/search): solve issue with duplicate search results (#306)
Browse files Browse the repository at this point in the history
  • Loading branch information
FrancescoBorzi committed Dec 27, 2019
1 parent 2042077 commit b1005b2
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}]`, () => {
Expand Down
20 changes: 20 additions & 0 deletions src/app/services/query.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
13 changes: 12 additions & 1 deletion src/app/services/query.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
}
Expand Down
5 changes: 4 additions & 1 deletion src/app/services/search/sai-search.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SmartScripts> {
selectFields = SAI_ID_FIELDS;
groupFields = SAI_ID_FIELDS;

/* istanbul ignore next */ // because of: https://github.com/gotwarlost/istanbul/issues/690
constructor(
protected queryService: QueryService,
Expand Down
2 changes: 1 addition & 1 deletion src/app/services/search/search.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
16 changes: 14 additions & 2 deletions src/app/services/search/search.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export abstract class SearchService<T extends TableRow> extends SubscriptionHand
'limit': new FormControl(100),
'fields': this.fields,
});
selectFields: string[] = null;
groupFields: string[] = null;

constructor(
protected queryService: QueryService,
Expand All @@ -24,12 +26,22 @@ export abstract class SearchService<T extends TableRow> 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,
);
}
})
);
Expand Down

0 comments on commit b1005b2

Please sign in to comment.