fix(filters): do not replace filter or project values when the id value resolves to undefined

This change fixes a bug where the label title in the query string would be replaced to undefined, resulting in an invalid filter. The underlying problem was the resolved filter query string got re-parsed and the id value of the labels were resolved to undefined (and rendered as that string) in the process.

Resolves https://community.vikunja.io/t/filtering-by-label-ux-issues/2393/14

(cherry picked from commit f425d98b4d7593445ab2fda74f827ad8b5abe759)

frontend/src/components/project/partials/FilterInput.vue

@@ -218,33 +218,35 @@ function handleFieldInput() {
 
 		// eslint-disable-next-line @typescript-eslint/no-unused-vars
 		const [matched, prefix, operator, space, keyword] = match
-		if (keyword) {
+		if(!keyword) {
-			let search = keyword
+			return
-			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 (props.projectId) {
-					projectUserService.getAll({projectId: props.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 (!props.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
 		}
+
+		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 (props.projectId) {
+				projectUserService.getAll({projectId: props.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 (!props.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
 	})
 }
 

frontend/src/components/project/partials/Filters.vue

@@ -112,7 +112,7 @@ watch(
 		const val = {...value}
 		val.filter = transformFilterStringFromApi(
 			val?.filter || '',
-			labelId => labelStore.getLabelById(labelId)?.title,
+			labelId => labelStore.getLabelById(labelId)?.title || null,
 			projectId => projectStore.projects[projectId]?.title || null,
 		)
 		params.value = val

frontend/src/helpers/filters.test.ts

@@ -199,6 +199,26 @@ describe('Filter Transformation', () => {
 			expect(transformed).toBe('labels in lorem, ipsum')
 		})
 
+		it('should not touch the label value when it is undefined', () => {
+			const transformed = transformFilterStringFromApi(
+				'labels = one',
+				(id: number) => undefined,
+				nullIdToTitleResolver,
+			)
+
+			expect(transformed).toBe('labels = one')
+		})
+
+		it('should not touch the label value when it is null', () => {
+			const transformed = transformFilterStringFromApi(
+				'labels = one',
+				(id: number) => null,
+				nullIdToTitleResolver,
+			)
+
+			expect(transformed).toBe('labels = one')
+		})
+
 		it('should correctly resolve projects', () => {
 			const transformed = transformFilterStringFromApi(
 				'project = 1',
@@ -228,6 +248,26 @@ describe('Filter Transformation', () => {
 
 			expect(transformed).toBe('project in lorem, ipsum')
 		})
+
+		it('should not touch the project value when it is undefined', () => {
+			const transformed = transformFilterStringFromApi(
+				'project = one',
+				nullIdToTitleResolver,
+				(id: number) => undefined,
+			)
+
+			expect(transformed).toBe('project = one')
+		})
+
+		it('should not touch the project value when it is null', () => {
+			const transformed = transformFilterStringFromApi(
+				'project = one',
+				nullIdToTitleResolver,
+				(id: number) => null,
+			)
+
+			expect(transformed).toBe('project = one')
+		})
 		
 		it('should transform the same attribute multiple times', () => {
 			const transformed = transformFilterStringFromApi(

frontend/src/helpers/filters.ts

@@ -141,8 +141,8 @@ export function transformFilterStringForApi(
 
 export function transformFilterStringFromApi(
 	filter: string,
-	labelResolver: (id: number) => string | null,
+	labelResolver: (id: number) => string | null | undefined,
-	projectResolver: (id: number) => string | null,
+	projectResolver: (id: number) => string | null | undefined,
 ): string {
 
 	if (filter.trim() === '') {
@@ -170,7 +170,7 @@ export function transformFilterStringFromApi(
 
 				keywords.forEach(k => {
 					const labelTitle = labelResolver(parseInt(k))
-					if (labelTitle !== null) {
+					if (labelTitle) {
 						filter = filter.replace(k, labelTitle)
 					}
 				})
@@ -194,7 +194,7 @@ export function transformFilterStringFromApi(
 
 				keywords.forEach(k => {
 					const project = projectResolver(parseInt(k))
-					if (project !== null) {
+					if (project) {
 						filter = filter.replace(k, project)
 					}
 				})