From da66eb73141c0aa56d4fcbb4c0eaf5c08bc07e4a Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Wed, 19 Jun 2024 17:28:16 +0200
Subject: [PATCH] fix(filters): do not match partial labels

This change fixes a bug where an input query like "labels in test || labels in l" would be replaced with something like "undefinedabels in test || labels in l" or "3abels in test || labels in l" when there was a label starting with "l" - when it should not have touched that.
The matching was changed so that only exact label matches are taken into account when searching for labels.

Now, the above string would be replaced by "labels in 1 || labels in l" (when the label "test" has the id 1).

Maybe resolves https://community.vikunja.io/t/filtering-by-label-ux-issues/2393/8
---
 .../project/partials/FilterInput.vue          | 72 ++++++++++---------
 .../components/project/partials/Filters.vue   |  8 +--
 frontend/src/helpers/filters.test.ts          | 10 +++
 3 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/frontend/src/components/project/partials/FilterInput.vue b/frontend/src/components/project/partials/FilterInput.vue
index e32cbdd2b..dc55776dd 100644
--- a/frontend/src/components/project/partials/FilterInput.vue
+++ b/frontend/src/components/project/partials/FilterInput.vue
@@ -84,6 +84,10 @@ function unEscapeHtml(unsafe: string): string {
 }
 
 const highlightedFilterQuery = computed(() => {
+	if (filterQuery.value === '') {
+		return ''
+	}
+
 	let highlighted = escapeHtml(filterQuery.value)
 	DATE_FIELDS
 		.forEach(o => {
@@ -94,7 +98,7 @@ const highlightedFilterQuery = computed(() => {
 				}
 
 				let endPadding = ''
-				if(value.endsWith(' ')) {
+				if (value.endsWith(' ')) {
 					const fullLength = value.length
 					value = value.trimEnd()
 					const numberOfRemovedSpaces = fullLength - value.length
@@ -208,36 +212,38 @@ function handleFieldInput() {
 		const pattern = new RegExp('(' + field + '\\s*' + FILTER_OPERATORS_REGEX + '\\s*)([\'"]?)([^\'"&|()]+\\1?)?$', 'ig')
 		const match = pattern.exec(textUpToCursor)
 
-		if (match !== null) {
-			// eslint-disable-next-line @typescript-eslint/no-unused-vars
-			const [matched, prefix, operator, space, keyword] = match
-			if (keyword) {
-				let search = keyword
-				if (operator === 'in' || operator === '?=') {
-					const keywords = keyword.split(',')
-					search = keywords[keywords.length - 1].trim()
-				}
-				if (matched.startsWith('label')) {
-					autocompleteResultType.value = 'labels'
-					autocompleteResults.value = labelStore.filterLabelsByQuery([], search)
-				}
-				if (matched.startsWith('assignee')) {
-					autocompleteResultType.value = 'assignees'
-					if (projectId) {
-						projectUserService.getAll({projectId}, {s: search})
-							.then(users => autocompleteResults.value = users.length > 1 ? users : [])
-					} else {
-						userService.getAll({}, {s: search})
-							.then(users => autocompleteResults.value = users.length > 1 ? users : [])
-					}
-				}
-				if (!projectId && matched.startsWith('project')) {
-					autocompleteResultType.value = 'projects'
-					autocompleteResults.value = projectStore.searchProject(search)
-				}
-				autocompleteMatchText.value = keyword
-				autocompleteMatchPosition.value = match.index + prefix.length - 1 + keyword.replace(search, '').length
+		if (match === null) {
+			return
+		}
+
+		// eslint-disable-next-line @typescript-eslint/no-unused-vars
+		const [matched, prefix, operator, space, keyword] = match
+		if (keyword) {
+			let search = keyword
+			if (operator === 'in' || operator === '?=') {
+				const keywords = keyword.split(',')
+				search = keywords[keywords.length - 1].trim()
 			}
+			if (matched.startsWith('label')) {
+				autocompleteResultType.value = 'labels'
+				autocompleteResults.value = labelStore.filterLabelsByQuery([], search)
+			}
+			if (matched.startsWith('assignee')) {
+				autocompleteResultType.value = 'assignees'
+				if (projectId) {
+					projectUserService.getAll({projectId}, {s: search})
+						.then(users => autocompleteResults.value = users.length > 1 ? users : [])
+				} else {
+					userService.getAll({}, {s: search})
+						.then(users => autocompleteResults.value = users.length > 1 ? users : [])
+				}
+			}
+			if (!projectId && matched.startsWith('project')) {
+				autocompleteResultType.value = 'projects'
+				autocompleteResults.value = projectStore.searchProject(search)
+			}
+			autocompleteMatchText.value = keyword
+			autocompleteMatchPosition.value = match.index + prefix.length - 1 + keyword.replace(search, '').length
 		}
 	})
 }
@@ -259,7 +265,7 @@ const blurDebounced = useDebounceFn(() => emit('blur'), 500)
 
 <template>
 	<div class="field">
-		<label 
+		<label
 			class="label"
 			:for="id"
 		>
@@ -326,11 +332,11 @@ const blurDebounced = useDebounceFn(() => emit('blur'), 500)
 
 <style lang="scss">
 .filter-input-highlight {
-	
+
 	&, button.filter-query__date_value {
 		color: var(--card-color);
 	}
-	
+
 	span {
 		&.filter-query__field {
 			color: var(--code-literal);
diff --git a/frontend/src/components/project/partials/Filters.vue b/frontend/src/components/project/partials/Filters.vue
index f78cbd13b..2b896b89b 100644
--- a/frontend/src/components/project/partials/Filters.vue
+++ b/frontend/src/components/project/partials/Filters.vue
@@ -97,6 +97,9 @@ watch(
 	},
 )
 
+const labelStore = useLabelStore()
+const projectStore = useProjectStore()
+
 // Using watchDebounced to prevent the filter re-triggering itself.
 watch(
 	() => modelValue,
@@ -112,13 +115,10 @@ watch(
 	{immediate: true},
 )
 
-const labelStore = useLabelStore()
-const projectStore = useProjectStore()
-
 function change() {
 	const filter = transformFilterStringForApi(
 		filterQuery.value,
-		labelTitle => labelStore.filterLabelsByQuery([], labelTitle)[0]?.id || null,
+		labelTitle => labelStore.getLabelByExactTitle(labelTitle)?.id || null,
 		projectTitle => {
 			const found = projectStore.findProjectByExactname(projectTitle)
 			return found?.id || null
diff --git a/frontend/src/helpers/filters.test.ts b/frontend/src/helpers/filters.test.ts
index c6935d9a4..8c3267755 100644
--- a/frontend/src/helpers/filters.test.ts
+++ b/frontend/src/helpers/filters.test.ts
@@ -127,6 +127,16 @@ describe('Filter Transformation', () => {
 			
 			expect(transformed).toBe('due_date = now/d || due_date > now/w+1w')
 		})
+		
+		it('should only transform one label occurrence at a time', () => {
+			const transformed = transformFilterStringForApi(
+				'labels in ipsum || labels in l',
+				multipleDummyResolver,
+				nullTitleToIdResolver,
+			)
+
+			expect(transformed).toBe('labels in 2 || labels in l')
+		})
 	})
 
 	describe('To API', () => {