From 5651c0b818c4df3d92c9053ef26475239d707640 Mon Sep 17 00:00:00 2001 From: kolaente Date: Sun, 10 Aug 2025 18:17:46 +0200 Subject: [PATCH] fix(filter): correctly replace quoted values https://github.com/go-vikunja/vikunja/issues/743 --- .../components/project/partials/Filters.vue | 4 +- frontend/src/helpers/filters.test.ts | 176 ++++++++++++++++++ frontend/src/helpers/filters.ts | 48 +++-- frontend/src/views/tasks/TaskDetailView.vue | 2 +- 4 files changed, 213 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/project/partials/Filters.vue b/frontend/src/components/project/partials/Filters.vue index 609bb1fd7..5ed6a7a27 100644 --- a/frontend/src/components/project/partials/Filters.vue +++ b/frontend/src/components/project/partials/Filters.vue @@ -8,12 +8,12 @@ ref="filterInputRef" v-model="filterQuery" :project-id="projectId" + class="mbe-2" @update:modelValue="() => change('modelValue')" /> -
{{ filterQuery }}
{{ $t('filters.fromView') }} {{ filterFromView }}
diff --git a/frontend/src/helpers/filters.test.ts b/frontend/src/helpers/filters.test.ts index 32b803337..2fc095521 100644 --- a/frontend/src/helpers/filters.test.ts +++ b/frontend/src/helpers/filters.test.ts @@ -189,6 +189,182 @@ describe('Filter Transformation', () => { }) }) + describe('Special Characters', () => { + const apostropheResolver = (title: string) => { + switch (title.toLowerCase()) { + case "john's task": + return 1 + case "mary's project": + return 2 + case "user's label": + return 3 + case "it's working": + return 4 + default: + return null + } + } + + const apostropheIdResolver = (id: number) => { + switch (id) { + case 1: + return "John's Task" + case 2: + return "Mary's Project" + case 3: + return "User's Label" + case 4: + return "It's Working" + default: + return null + } + } + + describe('Apostrophes in quoted values', () => { + it('should handle double-quoted labels with apostrophes', () => { + const transformed = transformFilterStringForApi( + 'labels = "John\'s Task"', + apostropheResolver, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels = 1') + }) + + it('should handle single-quoted labels with apostrophes', () => { + const transformed = transformFilterStringForApi( + "labels = 'Mary\\'s Project'", + apostropheResolver, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels = 2') + }) + + it('should handle projects with apostrophes in double quotes', () => { + const transformed = transformFilterStringForApi( + 'project = "User\'s Label"', + nullTitleToIdResolver, + apostropheResolver, + ) + + expect(transformed).toBe('project = 3') + }) + }) + + describe('Apostrophes in unquoted values', () => { + it('should handle unquoted labels with apostrophes', () => { + const transformed = transformFilterStringForApi( + 'labels = John\'s', + (title: string) => title === 'John\'s' ? 1 : null, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels = 1') + }) + + it('should handle unquoted projects with apostrophes', () => { + const transformed = transformFilterStringForApi( + 'project = Mary\'s', + nullTitleToIdResolver, + (title: string) => title === 'Mary\'s' ? 2 : null, + ) + + expect(transformed).toBe('project = 2') + }) + }) + + describe('Multiple values with apostrophes', () => { + it('should handle multiple labels with apostrophes using in operator', () => { + const transformed = transformFilterStringForApi( + 'labels in "John\'s Task", "Mary\'s Project"', + apostropheResolver, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels in 1, 2') + }) + + it('should handle multiple labels with apostrophes using not in operator', () => { + const transformed = transformFilterStringForApi( + 'labels not in "User\'s Label", "It\'s Working"', + apostropheResolver, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels not in 3, 4') + }) + + it('should handle mixed quoted and unquoted values with apostrophes', () => { + const mixedResolver = (title: string) => { + if (title === "John's Task") return 1 + if (title === "Mary's") return 2 + return null + } + + const transformed = transformFilterStringForApi( + 'labels in "John\'s Task", Mary\'s', + mixedResolver, + nullTitleToIdResolver, + ) + + expect(transformed).toBe('labels in 1, 2') + }) + }) + + it('should handle apostrophes in complex filter queries', () => { + const transformed = transformFilterStringForApi( + 'labels = "John\'s Task" && project = "Mary\'s Project" || priority = 1', + apostropheResolver, + apostropheResolver, + ) + + expect(transformed).toBe('labels = 1 && project = 2 || priority = 1') + }) + + describe('Reverse transformation with apostrophes', () => { + it('should transform labels with apostrophes from API to frontend', () => { + const transformed = transformFilterStringFromApi( + 'labels = 1', + apostropheIdResolver, + nullIdToTitleResolver, + ) + + expect(transformed).toBe('labels = John\'s Task') + }) + + it('should transform projects with apostrophes from API to frontend', () => { + const transformed = transformFilterStringFromApi( + 'project = 2', + nullIdToTitleResolver, + apostropheIdResolver, + ) + + expect(transformed).toBe('project = Mary\'s Project') + }) + + it('should handle multiple values with apostrophes in reverse transformation', () => { + const transformed = transformFilterStringFromApi( + 'labels in 1, 2', + apostropheIdResolver, + nullIdToTitleResolver, + ) + + expect(transformed).toBe('labels in John\'s Task, Mary\'s Project') + }) + + it('should handle complex queries with apostrophes in reverse transformation', () => { + const transformed = transformFilterStringFromApi( + 'labels = 1 && project = 2 || priority = 1', + apostropheIdResolver, + apostropheIdResolver, + ) + + expect(transformed).toBe('labels = John\'s Task && project = Mary\'s Project || priority = 1') + }) + }) + }) + describe('To API', () => { for (const c in fieldCases) { it('should transform all filter params for ' + c + ' to snake_case', () => { diff --git a/frontend/src/helpers/filters.ts b/frontend/src/helpers/filters.ts index b193ea7e4..4925488a4 100644 --- a/frontend/src/helpers/filters.ts +++ b/frontend/src/helpers/filters.ts @@ -1,5 +1,16 @@ import {snakeCase} from 'change-case' +function trimQuotes(str: string): string { + + str = str.trim() + + if ((str.startsWith('"') && str.endsWith('"')) || + (str.startsWith('\'') && str.endsWith('\''))) { + return str.slice(1, -1) + } + return str +} + export const DATE_FIELDS = [ 'dueDate', 'startDate', @@ -70,7 +81,7 @@ export function hasFilterQuery(filter: string): boolean { } export function getFilterFieldRegexPattern(field: string): RegExp { - return new RegExp('\\b(' + field + ')\\s*' + FILTER_OPERATORS_REGEX + '\\s*([\'"]?)([^\'"&|()<]+?)(?=\\s*(?:&&|\\|\\||$))', 'ig') + return new RegExp('\\b(' + field + ')\\s*' + FILTER_OPERATORS_REGEX + '\\s*(?:(["\'])((?:\\\\.|(?!\\3)[^\\\\])*?)\\3|([^&|()<]+?))(?=\\s*(?:&&|\\||$))', 'g') } export function transformFilterStringForApi( @@ -86,7 +97,8 @@ export function transformFilterStringForApi( } AVAILABLE_FILTER_FIELDS.forEach(f => { - filter = filter.replace(new RegExp(f, 'ig'), f) + const fieldPattern = new RegExp('\\b(' + f + ')\\b(?=\\s*' + FILTER_OPERATORS_REGEX + ')', 'gi') + filter = filter.replace(fieldPattern, f) }) // Transform labels and projects to ids @@ -102,8 +114,8 @@ export function transformFilterStringForApi( const replacements: { start: number, length: number, replacement: string }[] = [] while ((match = pattern.exec(filter)) !== null) { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [matched, fieldName, operator, quotes, keyword] = match + const [matched, fieldName, operator, quotes, quotedContent, unquotedContent] = match + const keyword = quotedContent || unquotedContent if (!keyword) { continue } @@ -115,11 +127,18 @@ export function transformFilterStringForApi( let replaced = keyword + const transformedKeywords: string[] = [] keywords.forEach(k => { - const id = resolver(k) - if (id !== null) { - replaced = replaced.replace(k, String(id)) + let id = resolver(k) + if (id === null && k.includes('\\')) { + id = resolver(k.replaceAll('\\', '')) } + if (id === null) { + transformedKeywords.push(k) + return + } + + transformedKeywords.push(String(id)) }) // Join the transformed keywords back together @@ -143,11 +162,10 @@ export function transformFilterStringForApi( continue } - const actualKeywordStart = (match?.index || 0) + matched.length - keyword.length replacements.push({ - start: actualKeywordStart, - length: keyword.length, - replacement: replaced, + start: match.index!, + length: matched.length, + replacement: reconstructedMatch, }) } @@ -170,9 +188,10 @@ export function transformFilterStringForApi( // Transform projects to ids filter = transformFieldToIds(PROJECT_FIELDS, projectResolver, filter) - // Transform all attributes to snake case + // Transform all field names (not values) to snake case AVAILABLE_FILTER_FIELDS.forEach(f => { - filter = filter.replaceAll(f, snakeCase(f)) + const fieldPattern = new RegExp('\\b' + f + '\\b(?=\\s*' + FILTER_OPERATORS_REGEX + ')', 'gi') + filter = filter.replace(fieldPattern, snakeCase(f)) }) return filter @@ -204,7 +223,8 @@ export function transformFilterStringFromApi( let match: RegExpExecArray | null while ((match = pattern.exec(filter)) !== null) { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [matched, fieldName, operator, quotes, keyword] = match + const [matched, fieldName, operator, quotes, quotedContent, unquotedContent] = match + const keyword = quotedContent || unquotedContent if (keyword) { let keywords = [keyword.trim()] if (isMultiValueOperator(operator)) { diff --git a/frontend/src/views/tasks/TaskDetailView.vue b/frontend/src/views/tasks/TaskDetailView.vue index e5c80dff3..9c8b6399c 100644 --- a/frontend/src/views/tasks/TaskDetailView.vue +++ b/frontend/src/views/tasks/TaskDetailView.vue @@ -576,7 +576,7 @@