From 519cf0aa43e6ac3085974506ff4eb1f0a70156c4 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Fri, 14 Feb 2020 15:50:42 -0300 Subject: [PATCH] 15781: Fixes property removal on the advanced search editor. Also, adds some tests ensuring that properties IDs are used when available. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- .../search-bar/search-bar-actions.test.ts | 96 ++++++++++++++++++- src/store/search-bar/search-bar-actions.ts | 39 ++++---- 2 files changed, 107 insertions(+), 28 deletions(-) diff --git a/src/store/search-bar/search-bar-actions.test.ts b/src/store/search-bar/search-bar-actions.test.ts index 51a73cc3..68804dfb 100644 --- a/src/store/search-bar/search-bar-actions.test.ts +++ b/src/store/search-bar/search-bar-actions.test.ts @@ -59,14 +59,100 @@ describe('search-bar-actions', () => { inTrash: true, dateFrom: '2017-08-01', dateTo: '', - properties: [{ - key: 'file size', - value: '101mb' - }], + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', value: 'Human' }, + { key: 'Species', value: 'Canine' }, + ], saveQuery: false, queryName: '' }); - expect(q).toBe('document pdf type:arvados#collection cluster:c97qx is:trashed from:2017-08-01 has:"file size":"101mb"'); + expect(q).toBe('document pdf type:arvados#collection cluster:c97qx is:trashed from:2017-08-01 has:"file size":"101mb" has:"Species":"Human" has:"Species":"Canine"'); + }); + + it('should add has:"key":"value" expression to query from same property key', () => { + const searchValue = 'document pdf has:"file size":"101mb" has:"Species":"Canine"'; + const prevData = { + searchValue, + type: undefined, + cluster: undefined, + projectUuid: undefined, + inTrash: false, + dateFrom: '', + dateTo: '', + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', value: 'Canine' }, + ], + saveQuery: false, + queryName: '' + }; + const currData = { + ...prevData, + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', value: 'Canine' }, + { key: 'Species', value: 'Human' }, + ], + }; + const q = getQueryFromAdvancedData(currData, prevData); + expect(q).toBe('document pdf has:"file size":"101mb" has:"Species":"Canine" has:"Species":"Human"'); + }); + + it('should add has:"keyID":"valueID" expression to query when necessary', () => { + const searchValue = 'document pdf has:"file size":"101mb"'; + const prevData = { + searchValue, + type: undefined, + cluster: undefined, + projectUuid: undefined, + inTrash: false, + dateFrom: '', + dateTo: '', + properties: [ + { key: 'file size', value: '101mb' }, + ], + saveQuery: false, + queryName: '' + }; + const currData = { + ...prevData, + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', keyID: 'IDTAGSPECIES', value: 'Human', valueID: 'IDVALHUMAN'}, + ], + }; + const q = getQueryFromAdvancedData(currData, prevData); + expect(q).toBe('document pdf has:"file size":"101mb" has:"IDTAGSPECIES":"IDVALHUMAN"'); + }); + + it('should remove has:"key":"value" expression from query', () => { + const searchValue = 'document pdf has:"file size":"101mb" has:"Species":"Human" has:"Species":"Canine"'; + const prevData = { + searchValue, + type: undefined, + cluster: undefined, + projectUuid: undefined, + inTrash: false, + dateFrom: '', + dateTo: '', + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', value: 'Canine' }, + { key: 'Species', value: 'Human' }, + ], + saveQuery: false, + queryName: '' + }; + const currData = { + ...prevData, + properties: [ + { key: 'file size', value: '101mb' }, + { key: 'Species', value: 'Canine' }, + ], + }; + const q = getQueryFromAdvancedData(currData, prevData); + expect(q).toBe('document pdf has:"file size":"101mb" has:"Species":"Canine"'); }); }); }); diff --git a/src/store/search-bar/search-bar-actions.ts b/src/store/search-bar/search-bar-actions.ts index 794ca8dc..54678b50 100644 --- a/src/store/search-bar/search-bar-actions.ts +++ b/src/store/search-bar/search-bar-actions.ts @@ -225,12 +225,12 @@ const searchGroups = (searchValue: string, limit: number) => } }; -const buildQueryFromKeyMap = (data: any, keyMap: string[][], mode: 'rebuild' | 'reuse') => { +const buildQueryFromKeyMap = (data: any, keyMap: string[][]) => { let value = data.searchValue; const addRem = (field: string, key: string) => { const v = data[key]; - + // Remove previous search expression. if (data.hasOwnProperty(key)) { let pattern: string; if (v === false) { @@ -238,28 +238,23 @@ const buildQueryFromKeyMap = (data: any, keyMap: string[][], mode: 'rebuild' | ' } else if (key.startsWith('prop-')) { // On properties, only remove key:value duplicates, allowing // multiple properties with the same key. - pattern = `${field.replace(':', '\\:\\s*')}\\:\\s*${v}\\s*`; + const oldValue = key.slice(5).split(':')[1]; + pattern = `${field.replace(':', '\\:\\s*')}\\:\\s*${oldValue}\\s*`; } else { pattern = `${field.replace(':', '\\:\\s*')}\\:\\s*[\\w|\\#|\\-|\\/]*\\s*`; } value = value.replace(new RegExp(pattern), ''); } - + // Re-add it with the current search value. if (v) { const nv = v === true ? `${field}` : `${field}:${v}`; - - if (mode === 'rebuild') { - value = value + ' ' + nv; - } else { - value = nv + ' ' + value; - } + // Always append to the end to keep user-entered text at the start. + value = value + ' ' + nv; } }; - keyMap.forEach(km => addRem(km[0], km[1])); - return value; }; @@ -276,7 +271,9 @@ export const getQueryFromAdvancedData = (data: SearchBarAdvancedFormData, prevDa dateFrom: data.dateFrom, dateTo: data.dateTo, }; - (data.properties || []).forEach(p => fo[`prop-"${p.keyID || p.key}"`] = `"${p.valueID || p.value}"`); + (data.properties || []).forEach(p => + fo[`prop-"${p.keyID || p.key}":"${p.valueID || p.value}"`] = `"${p.valueID || p.value}"` + ); return fo; }; @@ -289,17 +286,13 @@ export const getQueryFromAdvancedData = (data: SearchBarAdvancedFormData, prevDa ['to', 'dateTo'] ]; _.union(data.properties, prevData ? prevData.properties : []) - .forEach(p => keyMap.push([`has:"${p.keyID || p.key}"`, `prop-"${p.keyID || p.key}"`])); + .forEach(p => keyMap.push( + [`has:"${p.keyID || p.key}"`, `prop-"${p.keyID || p.key}":"${p.valueID || p.value}"`] + )); - if (prevData) { - const obj = getModifiedKeysValues(flatData(data), flatData(prevData)); - value = buildQueryFromKeyMap({ - searchValue: data.searchValue, - ...obj - } as SearchBarAdvancedFormData, keyMap, "reuse"); - } else { - value = buildQueryFromKeyMap(flatData(data), keyMap, "rebuild"); - } + const modified = getModifiedKeysValues(flatData(data), prevData ? flatData(prevData):{}); + value = buildQueryFromKeyMap( + {searchValue: data.searchValue, ...modified} as SearchBarAdvancedFormData, keyMap); value = value.trim(); return value; -- 2.30.2