From 27c7bb9f79ee67939ead03a9f4a59a9bddea1877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Tue, 2 Apr 2024 11:57:37 -0400 Subject: [PATCH] interview filters: Allow an array of values It is possible to receive an array of strings for filter values. In that case, the result is and AND of those values in the resulting SQL query. --- .../src/api/survey.validation.routes.ts | 8 +- .../models/__tests__/interviews.db.test.ts | 40 +++++- .../src/models/interviews.db.queries.ts | 115 +++++++++++------- .../interviews/__tests__/interviews.test.ts | 36 ++++++ .../src/services/interviews/interviews.ts | 62 ++++++---- 5 files changed, 186 insertions(+), 75 deletions(-) diff --git a/packages/evolution-backend/src/api/survey.validation.routes.ts b/packages/evolution-backend/src/api/survey.validation.routes.ts index 5b6e8373..7437bf4b 100644 --- a/packages/evolution-backend/src/api/survey.validation.routes.ts +++ b/packages/evolution-backend/src/api/survey.validation.routes.ts @@ -9,7 +9,7 @@ * */ import express, { Request, Response } from 'express'; import moment from 'moment'; -import Interviews from '../services/interviews/interviews'; +import Interviews, { FilterType } from '../services/interviews/interviews'; import { updateInterview, copyResponsesToValidatedData } from '../services/interviews/interview'; import interviewUserIsAuthorized, { isUserAllowed } from '../services/auth/userAuthorization'; import projectConfig from '../config/projectConfig'; @@ -141,10 +141,10 @@ router.post('/validationList', async (req, res) => { typeof pageIndex === 'number' ? pageSize : typeof pageSize === 'string' ? parseInt(pageSize) : -1; const updatedAtNb = typeof updatedAt === 'string' ? parseInt(updatedAt) : 0; - const actualFilters: { [key: string]: string } = {}; + const actualFilters: { [key: string]: FilterType } = {}; Object.keys(filters).forEach((key) => { - if (typeof filters[key] === 'string') { - actualFilters[key] = filters[key] as string; + if (typeof filters[key] === 'string' || Array.isArray(filters[key])) { + actualFilters[key] = filters[key]; } else if (typeof filters[key] === 'object' && filters[key].value !== undefined) { actualFilters[key] = filters[key]; } diff --git a/packages/evolution-backend/src/models/__tests__/interviews.db.test.ts b/packages/evolution-backend/src/models/__tests__/interviews.db.test.ts index d4dc000b..030dc5d8 100644 --- a/packages/evolution-backend/src/models/__tests__/interviews.db.test.ts +++ b/packages/evolution-backend/src/models/__tests__/interviews.db.test.ts @@ -579,6 +579,19 @@ describe('list interviews', () => { expect(filterResponsesBoolean.length).toEqual(1); expect((filterResponsesBoolean[0].responses as any).booleanField).toBeTruthy(); + // Query with array of values without results + const { interviews: filterAccessCodeArray, totalCount: countAccessCodeArray } = + await dbQueries.getList({ filters: { 'responses.accessCode': { value: [localUserInterviewAttributes.responses.accessCode.substring(0, 3), '222'], op: 'like' } }, pageIndex: 0, pageSize: -1 }); + expect(countAccessCodeArray).toEqual(0); + expect(filterAccessCodeArray.length).toEqual(0); + + // Query with array of values with results + const { interviews: filterAccessCodeArrayRes, totalCount: countAccessCodeArrayRes } = + await dbQueries.getList({ filters: { 'responses.accessCode': { value: [localUserInterviewAttributes.responses.accessCode.substring(0, 3), localUserInterviewAttributes.responses.accessCode.substring(0, 3)], op: 'like' } }, pageIndex: 0, pageSize: -1 }); + expect(countAccessCodeArrayRes).toEqual(1); + expect(filterAccessCodeArrayRes.length).toEqual(1); + expect((filterAccessCodeArrayRes[0].responses as any).accessCode).toEqual(localUserInterviewAttributes.responses.accessCode); + }); test('Combine filter and paging', async () => { @@ -707,7 +720,7 @@ describe('Queries with audits', () => { const errorThreeCode = 'errorThree'; beforeAll(async () => { - // Add 3 errors per of type one, and one of type three for one of the interviews + // Add 3 errors per person of type one, and one of type three for one of the interviews const firstInterview = await dbQueries.getInterviewByUuid(localUserInterviewAttributes.uuid); if (firstInterview === undefined) { throw 'error getting interview 1 for audits'; @@ -736,11 +749,9 @@ describe('Queries with audits', () => { test('Get the complete list of validation errors', async () => { const { errors } = await dbQueries.getValidationErrors({ filters: {} }); expect(errors.length).toEqual(3); - expect(errors).toEqual([ - { key: 'errorOne', cnt: '6' }, - { key: 'errorTwo', cnt: '1' }, - { key: 'errorThree', cnt: '1' } - ]); + expect(errors.find(error => error.key === 'errorOne' && error.cnt === '6')).toBeDefined(); + expect(errors.find(error => error.key === 'errorTwo' && error.cnt === '1')).toBeDefined(); + expect(errors.find(error => error.key === 'errorThree' && error.cnt === '1')).toBeDefined(); }); test('Get validation errors with a validity filter', async () => { @@ -773,6 +784,23 @@ describe('Queries with audits', () => { }); + test('List interviews with multiple audit filters', async () => { + + // Query for errorThree and errorOne + const { interviews: filterAudit, totalCount: countAudit } = + await dbQueries.getList({ filters: { 'audits': { value: ['errorThree', 'errorOne'] } }, pageIndex: 0, pageSize: -1 }); + expect(countAudit).toEqual(1); + expect(filterAudit.length).toEqual(1); + expect(filterAudit[0].uuid).toEqual(localUserInterviewAttributes.uuid); + + // Query for errorThree and errorTwo, no result expected + const { interviews: filterAudit2, totalCount: countAudit2 } = + await dbQueries.getList({ filters: { 'audits': { value: ['errorThree', 'errorTwo'] } }, pageIndex: 0, pageSize: -1 }); + expect(countAudit2).toEqual(0); + expect(filterAudit2.length).toEqual(0); + + }); + test('List interviews, validate audits', async () => { // Query by audit diff --git a/packages/evolution-backend/src/models/interviews.db.queries.ts b/packages/evolution-backend/src/models/interviews.db.queries.ts index eeb6d127..cdae6377 100644 --- a/packages/evolution-backend/src/models/interviews.db.queries.ts +++ b/packages/evolution-backend/src/models/interviews.db.queries.ts @@ -33,6 +33,8 @@ export interface InterviewSearchAttributes { surveyId?: number; } +export type ValueFilterType = { value: string | string[] | boolean | number | null; op?: keyof OperatorSigns }; + /** this will return the survey id or create a new survey in * table sv_surveys if no existing survey shortname matches * the project shortname @@ -371,9 +373,9 @@ const addLikeBinding = (operator: keyof OperatorSigns | undefined, binding: stri */ const getRawWhereClause = ( field: string, - filter: { value: string | boolean | number | null; op?: keyof OperatorSigns }, + filter: ValueFilterType, tblAlias: string -): string | [string, string | boolean | number] | undefined => { +): (string | [string, string | boolean | number] | undefined)[] => { // Make sure the field is a legitimate field to avoid sql injection. Field // is either the name of a field, or a dot-separated path in a json object // of the 'responses' field. We should not accept anything else. @@ -389,16 +391,13 @@ const getRawWhereClause = ( 'DatabaseInvalidWhereClauseUserEntry' ); } - const getBooleanFilter = ( - fieldName: string, - filter: { value: string | boolean | number | null; op?: keyof OperatorSigns } - ) => { + const getBooleanFilter = (fieldName: string, filter: ValueFilterType) => { const validityValue = _booleish(filter.value); const notStr = filter.op === 'not' ? ' NOT ' : ''; if (validityValue !== null) { - return `${fieldName} IS ${notStr} ${validityValue ? 'TRUE' : 'FALSE'} `; + return [`${fieldName} IS ${notStr} ${validityValue ? 'TRUE' : 'FALSE'} `]; } - return `${fieldName} IS ${notStr} null`; + return [`${fieldName} IS ${notStr} null`]; }; switch (field) { // For created_at and updated_at, we also accept a date range as an array two unix timestamps. @@ -406,32 +405,43 @@ const getRawWhereClause = ( // In such a case, the filter operation parmeter is ignored (>= and <= are used). case 'created_at': if (Array.isArray(filter.value) && filter.value.length === 2) { - return `${tblAlias}.created_at >= to_timestamp(${filter.value[0]}) AND ${tblAlias}.created_at <= to_timestamp(${filter.value[1]}) `; + return [ + `${tblAlias}.created_at >= to_timestamp(${filter.value[0]}) AND ${tblAlias}.created_at <= to_timestamp(${filter.value[1]}) ` + ]; } else { - return `${tblAlias}.created_at ${ - filter.op ? operatorSigns[filter.op] : operatorSigns.eq - } to_timestamp(${filter.value}) `; + return [ + `${tblAlias}.created_at ${filter.op ? operatorSigns[filter.op] : operatorSigns.eq} to_timestamp(${ + filter.value + }) ` + ]; } case 'updated_at': if (Array.isArray(filter.value) && filter.value.length === 2) { - return `${tblAlias}.updated_at >= to_timestamp(${filter.value[0]}) AND ${tblAlias}.updated_at <= to_timestamp(${filter.value[1]}) `; + return [ + `${tblAlias}.updated_at >= to_timestamp(${filter.value[0]}) AND ${tblAlias}.updated_at <= to_timestamp(${filter.value[1]}) ` + ]; } else { - return `${tblAlias}.updated_at ${ - filter.op ? operatorSigns[filter.op] : operatorSigns.eq - } to_timestamp(${filter.value}) `; + return [ + `${tblAlias}.updated_at ${filter.op ? operatorSigns[filter.op] : operatorSigns.eq} to_timestamp(${ + filter.value + }) ` + ]; } case 'is_valid': return getBooleanFilter(`${tblAlias}.is_valid`, filter); case 'is_questionable': return getBooleanFilter(`${tblAlias}.is_questionable`, filter); case 'uuid': - return `${tblAlias}.${field} ${filter.op ? operatorSigns[filter.op] : operatorSigns.eq} '${filter.value}'`; + return [ + `${tblAlias}.${field} ${filter.op ? operatorSigns[filter.op] : operatorSigns.eq} '${filter.value}'` + ]; case 'audits': { - if (typeof filter.value !== 'string') { - return undefined; + if (typeof filter.value !== 'string' && !Array.isArray(filter.value)) { + return [undefined]; } - const match = filter.value.match(dotSeparatedStringRegex); - if (match === null) { + const values = typeof filter.value === 'string' ? [filter.value] : filter.value; + const matches = values.map((value) => value.match(dotSeparatedStringRegex)); + if (matches.find((m) => m === null) !== undefined) { throw new TrError( `Invalid value for where clause in ${tableName} database`, 'DBQCR0006', @@ -439,8 +449,12 @@ const getRawWhereClause = ( ); } // Add subquery to audits table - const auditSubQuery = knex('sv_audits').select('interview_id').distinct().where('error_code', filter.value); - return [`${tblAlias}.id in (${auditSubQuery.toSQL().sql})`, filter.value]; + return values.map((value) => [ + `${tblAlias}.id in (${ + knex('sv_audits').select('interview_id').distinct().where('error_code', value).toSQL().sql + })`, + value + ]); } } const jsonObject = field.split('.'); @@ -449,17 +463,28 @@ const getRawWhereClause = ( if (prefix !== undefined) { const field = jsonObject.slice(1).join('.'); return filter.value === null - ? `${prefix}->>'${field}' ${filter.op === 'not' ? ' IS NOT NULL' : ' IS NULL'}` - : [ - `${prefix}->>'${field}' ${ - filter.op !== undefined - ? `${operatorSigns[filter.op] || operatorSigns.eq} ?` - : `${operatorSigns.eq} ?` - }`, - addLikeBinding(filter.op, filter.value) - ]; + ? [`${prefix}->>'${field}' ${filter.op === 'not' ? ' IS NOT NULL' : ' IS NULL'}`] + : Array.isArray(filter.value) + ? filter.value.map((value) => [ + `${prefix}->>'${field}' ${ + filter.op !== undefined + ? `${operatorSigns[filter.op] || operatorSigns.eq} ?` + : `${operatorSigns.eq} ?` + }`, + addLikeBinding(filter.op, value) + ]) + : [ + [ + `${prefix}->>'${field}' ${ + filter.op !== undefined + ? `${operatorSigns[filter.op] || operatorSigns.eq} ?` + : `${operatorSigns.eq} ?` + }`, + addLikeBinding(filter.op, filter.value) + ] + ]; } - return undefined; + return [undefined]; }; /** @@ -475,19 +500,21 @@ const getRawWhereClause = ( * and the second element is the array of bindings */ const updateRawWhereClause = ( - filters: { [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns } }, + filters: { [key: string]: ValueFilterType }, baseFilter: string ): [string, (string | boolean | number)[]] => { let rawFilter = baseFilter; const bindings: (string | number | boolean)[] = []; Object.keys(filters).forEach((key) => { - const whereClause = getRawWhereClause(key, filters[key], 'i'); - if (typeof whereClause === 'string') { - rawFilter += ` AND ${whereClause}`; - } else if (Array.isArray(whereClause)) { - rawFilter += ` AND ${whereClause[0]}`; - bindings.push(whereClause[1]); - } + const whereClauses = getRawWhereClause(key, filters[key], 'i'); + whereClauses.forEach((whereClause) => { + if (typeof whereClause === 'string') { + rawFilter += ` AND ${whereClause}`; + } else if (Array.isArray(whereClause)) { + rawFilter += ` AND ${whereClause[0]}`; + bindings.push(whereClause[1]); + } + }); }); return [rawFilter, bindings]; }; @@ -506,7 +533,7 @@ const updateRawWhereClause = ( * corresponding to the query */ const getList = async (params: { - filters: { [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns } }; + filters: { [key: string]: ValueFilterType }; pageIndex: number; pageSize: number; sort?: (string | { field: string; order: 'asc' | 'desc' })[]; @@ -621,7 +648,7 @@ const getList = async ( * interviews */ const getValidationErrors = async (params: { - filters: { [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns } }; + filters: { [key: string]: ValueFilterType }; }): Promise<{ errors: { key: string; cnt: string }[] }> => { try { const baseRawFilter = @@ -669,7 +696,7 @@ const getValidationErrors = async (params: { * @returns An interview stream */ const getInterviewsStream = function (params: { - filters: { [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns } }; + filters: { [key: string]: ValueFilterType }; select?: { includeAudits?: boolean; responses?: 'none' | 'participant' | 'validated' | 'both' | 'validatedIfAvailable'; diff --git a/packages/evolution-backend/src/services/interviews/__tests__/interviews.test.ts b/packages/evolution-backend/src/services/interviews/__tests__/interviews.test.ts index e2298103..78b47c92 100644 --- a/packages/evolution-backend/src/services/interviews/__tests__/interviews.test.ts +++ b/packages/evolution-backend/src/services/interviews/__tests__/interviews.test.ts @@ -416,6 +416,42 @@ describe('Get all matching', () => { }); }); + test('Filters: various filters', async() => { + // string audit + await Interviews.getAllMatching({ + filter: { audits: 'myAudit' } + }); + expect(interviewsQueries.getList).toHaveBeenCalledTimes(1); + expect(interviewsQueries.getList).toHaveBeenCalledWith({ + filters: { audits: { value: 'myAudit' } }, + pageIndex: 0, + pageSize: -1 + }); + + // array of string audit + await Interviews.getAllMatching({ + filter: { audits: ['myAudit1', 'myAudit2'] } + }); + expect(interviewsQueries.getList).toHaveBeenCalledTimes(2); + expect(interviewsQueries.getList).toHaveBeenCalledWith({ + filters: { audits: { value: ['myAudit1', 'myAudit2'] } }, + pageIndex: 0, + pageSize: -1 + }); + + // object filter + await Interviews.getAllMatching({ + filter: { audits: { value: 'myAudit', op: 'like' } } + }); + expect(interviewsQueries.getList).toHaveBeenCalledTimes(3); + expect(interviewsQueries.getList).toHaveBeenCalledWith({ + filters: { audits: { value: 'myAudit', op: 'like' } }, + pageIndex: 0, + pageSize: -1 + }); + + }); + }); describe('Get Validation errors', () => { diff --git a/packages/evolution-backend/src/services/interviews/interviews.ts b/packages/evolution-backend/src/services/interviews/interviews.ts index a07ba3ab..f126b70e 100644 --- a/packages/evolution-backend/src/services/interviews/interviews.ts +++ b/packages/evolution-backend/src/services/interviews/interviews.ts @@ -10,7 +10,11 @@ import { mapResponsesToValidatedData } from './interviewUtils'; import { validateAccessCode } from '../accessCode'; import { validate as validateUuid } from 'uuid'; import { _isBlank } from 'chaire-lib-common/lib/utils/LodashExtensions'; -import interviewsDbQueries, { InterviewSearchAttributes, OperatorSigns } from '../../models/interviews.db.queries'; +import interviewsDbQueries, { + InterviewSearchAttributes, + OperatorSigns, + ValueFilterType +} from '../../models/interviews.db.queries'; import interviewsAccessesDbQueries from '../../models/interviewsAccesses.db.queries'; import { InterviewAttributes, @@ -20,17 +24,19 @@ import { import { UserInterviewAccesses } from '../logging/loggingTypes'; import { Audits } from '../../services/audits/Audits'; +export type FilterType = string | string[] | ValueFilterType; + const getFiltersForDb = ( filter: { is_valid?: 'valid' | 'invalid' | 'notInvalid' | 'notValidated' | 'questionable' | 'all' } & { - [key: string]: string | { value: string | boolean | number | null; op?: keyof OperatorSigns }; + [key: string]: FilterType; } ): { - [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns }; + [key: string]: ValueFilterType; } => { const { is_valid, ...filters } = filter; const actualFilters: { - [key: string]: { value: string | boolean | number | null; op?: keyof OperatorSigns }; + [key: string]: ValueFilterType; } = {}; // Add the desired validity query @@ -57,7 +63,7 @@ const getFiltersForDb = ( Object.keys(filters).forEach((key) => { const filter = filters[key]; - if (typeof filter === 'string') { + if (typeof filter === 'string' || Array.isArray(filter)) { actualFilters[key] = { value: filter }; } else { actualFilters[key] = filter; @@ -126,7 +132,7 @@ export default class Interviews { static getAllMatching = async ( params: { filter?: { is_valid?: 'valid' | 'invalid' | 'notInvalid' | 'notValidated' | 'all' } & { - [key: string]: string | { value: string | boolean | number | null; op?: keyof OperatorSigns }; + [key: string]: FilterType; }; pageIndex?: number; pageSize?: number; @@ -165,7 +171,9 @@ export default class Interviews { static async auditInterviewsV2(disableConsoleLog = false): Promise { const oldConsoleLog = console.log; if (disableConsoleLog) { - console.log = () => { return; }; + console.log = () => { + return; + }; console.info = oldConsoleLog; } let i = 1; @@ -187,22 +195,34 @@ export default class Interviews { } i++; if (_isBlank(interview.validated_data)) { - copyResponsesToValidatedData(interview).then(() => new Promise((res1, rej1) => { - Audits.runAndSaveInterviewAudits(interview).then(() => { - res1(true); - }).catch((error) => { - console.error('Error running and saving interview audits', error); - res1(false); + copyResponsesToValidatedData(interview) + .then( + () => + new Promise((res1, rej1) => { + Audits.runAndSaveInterviewAudits(interview) + .then(() => { + res1(true); + }) + .catch((error) => { + console.error('Error running and saving interview audits', error); + res1(false); + }); + }) + ) + .catch((error) => { + console.error('Error copying responses to validated data', error); + }) + .finally(() => { + queryStream.resume(); }); - })).catch((error) => { console.error('Error copying responses to validated data', error); }).finally(() => { - queryStream.resume(); - }); } else { - Audits.runAndSaveInterviewAudits(interview).catch((error) => { - console.error('Error running and saving interview audits', error); - }).finally(() => { - queryStream.resume(); - }); + Audits.runAndSaveInterviewAudits(interview) + .catch((error) => { + console.error('Error running and saving interview audits', error); + }) + .finally(() => { + queryStream.resume(); + }); } }) .on('end', () => {