From 3d33b7c8d1bb273e2604ae7046fb0fe49f760e17 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Tue, 21 Jan 2025 18:22:56 +0100
Subject: [PATCH] fix(filters): correctly replace the same filter input part
 when it occurs multiple times

This fixes a bug where a query like "labels in lorem || labels in ipsum" would only replace the first occurrence, leading to errors when sending the query string to the api.

Resolves https://github.com/go-vikunja/vikunja/issues/346
---
 frontend/src/helpers/filters.test.ts | 20 ++++++++++++++++++++
 frontend/src/helpers/filters.ts      | 20 +++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/frontend/src/helpers/filters.test.ts b/frontend/src/helpers/filters.test.ts
index a63e611b5..32b803337 100644
--- a/frontend/src/helpers/filters.test.ts
+++ b/frontend/src/helpers/filters.test.ts
@@ -78,6 +78,16 @@ describe('Filter Transformation', () => {
 			expect(transformed).toBe('labels not in 1, 2 && due_date = now')
 		})
 
+		it('should correctly resolve labels with multiple in clauses', () => {
+			const transformed = transformFilterStringForApi(
+				'labels in lorem || labels in ipsum',
+				multipleDummyResolver,
+				nullTitleToIdResolver,
+			)
+
+			expect(transformed).toBe('labels in 1 || labels in 2')
+		})
+
 		it('should correctly resolve projects', () => {
 			const transformed = transformFilterStringForApi(
 				'project = lorem',
@@ -228,6 +238,16 @@ describe('Filter Transformation', () => {
 
 			expect(transformed).toBe('labels in lorem, ipsum')
 		})
+
+		it('should correctly resolve multiple labels in clauses', () => {
+			const transformed = transformFilterStringFromApi(
+				'labels in 1 || labels in 2',
+				multipleIdToTitleResolver,
+				nullIdToTitleResolver,
+			)
+
+			expect(transformed).toBe('labels in lorem || labels in ipsum')
+		})
 		
 		it('should correctly resolve multiple labels not in', () => {
 			const transformed = transformFilterStringFromApi(
diff --git a/frontend/src/helpers/filters.ts b/frontend/src/helpers/filters.ts
index 44ae42b60..739362f2f 100644
--- a/frontend/src/helpers/filters.ts
+++ b/frontend/src/helpers/filters.ts
@@ -94,6 +94,8 @@ export function transformFilterStringForApi(
 			const pattern = getFilterFieldRegexPattern(field)
 
 			let match: RegExpExecArray | null
+			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, prefix, operator, space, keyword] = match
@@ -116,10 +118,22 @@ export function transformFilterStringForApi(
 				})
 
 				const actualKeywordStart = (match?.index || 0) + prefix.length
-				filter = filter.substring(0, actualKeywordStart) +
-					replaced +
-					filter.substring(actualKeywordStart + keyword.length)
+				replacements.push({
+					start: actualKeywordStart,
+					length: keyword.length,
+					replacement: replaced,
+				})
 			}
+
+			// We're collecting the results first and then replacing the filter string in the end
+			// to avoid modifying the input string as we iterate over it.
+			let offset = 0
+			replacements.forEach(({start, length, replacement}) => {
+				filter = filter.substring(0, start + offset) +
+					replacement +
+					filter.substring(start + offset + length)
+				offset += replacement.length - length
+			})
 		})
 		return filter
 	}