fix(filters): do not replace filter or project values when the id value resolves to undefined
All checks were successful
continuous-integration/drone/push Build is passing

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
This commit is contained in:
kolaente 2024-09-13 18:42:42 +02:00
parent 4c55016c1a
commit f425d98b4d
Signed by: konrad
GPG Key ID: F40E70337AB24C9B
4 changed files with 73 additions and 31 deletions

@ -218,33 +218,35 @@ function handleFieldInput() {
// 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 (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
if(!keyword) {
return
}
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
})
}

@ -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

@ -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(

@ -141,8 +141,8 @@ export function transformFilterStringForApi(
export function transformFilterStringFromApi(
filter: string,
labelResolver: (id: number) => string | null,
projectResolver: (id: number) => string | null,
labelResolver: (id: number) => string | null | undefined,
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)
}
})