1
0

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
This commit is contained in:
kolaente 2024-06-19 17:28:16 +02:00
parent 2690c99438
commit da66eb7314
No known key found for this signature in database
GPG Key ID: F40E70337AB24C9B
3 changed files with 53 additions and 37 deletions

View File

@ -84,6 +84,10 @@ function unEscapeHtml(unsafe: string): string {
} }
const highlightedFilterQuery = computed(() => { const highlightedFilterQuery = computed(() => {
if (filterQuery.value === '') {
return ''
}
let highlighted = escapeHtml(filterQuery.value) let highlighted = escapeHtml(filterQuery.value)
DATE_FIELDS DATE_FIELDS
.forEach(o => { .forEach(o => {
@ -94,7 +98,7 @@ const highlightedFilterQuery = computed(() => {
} }
let endPadding = '' let endPadding = ''
if(value.endsWith(' ')) { if (value.endsWith(' ')) {
const fullLength = value.length const fullLength = value.length
value = value.trimEnd() value = value.trimEnd()
const numberOfRemovedSpaces = fullLength - value.length const numberOfRemovedSpaces = fullLength - value.length
@ -208,36 +212,38 @@ function handleFieldInput() {
const pattern = new RegExp('(' + field + '\\s*' + FILTER_OPERATORS_REGEX + '\\s*)([\'"]?)([^\'"&|()]+\\1?)?$', 'ig') const pattern = new RegExp('(' + field + '\\s*' + FILTER_OPERATORS_REGEX + '\\s*)([\'"]?)([^\'"&|()]+\\1?)?$', 'ig')
const match = pattern.exec(textUpToCursor) const match = pattern.exec(textUpToCursor)
if (match !== null) { if (match === null) {
// eslint-disable-next-line @typescript-eslint/no-unused-vars return
const [matched, prefix, operator, space, keyword] = match }
if (keyword) {
let search = keyword // eslint-disable-next-line @typescript-eslint/no-unused-vars
if (operator === 'in' || operator === '?=') { const [matched, prefix, operator, space, keyword] = match
const keywords = keyword.split(',') if (keyword) {
search = keywords[keywords.length - 1].trim() let search = keyword
} if (operator === 'in' || operator === '?=') {
if (matched.startsWith('label')) { const keywords = keyword.split(',')
autocompleteResultType.value = 'labels' search = keywords[keywords.length - 1].trim()
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 (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> <template>
<div class="field"> <div class="field">
<label <label
class="label" class="label"
:for="id" :for="id"
> >
@ -326,11 +332,11 @@ const blurDebounced = useDebounceFn(() => emit('blur'), 500)
<style lang="scss"> <style lang="scss">
.filter-input-highlight { .filter-input-highlight {
&, button.filter-query__date_value { &, button.filter-query__date_value {
color: var(--card-color); color: var(--card-color);
} }
span { span {
&.filter-query__field { &.filter-query__field {
color: var(--code-literal); color: var(--code-literal);

View File

@ -97,6 +97,9 @@ watch(
}, },
) )
const labelStore = useLabelStore()
const projectStore = useProjectStore()
// Using watchDebounced to prevent the filter re-triggering itself. // Using watchDebounced to prevent the filter re-triggering itself.
watch( watch(
() => modelValue, () => modelValue,
@ -112,13 +115,10 @@ watch(
{immediate: true}, {immediate: true},
) )
const labelStore = useLabelStore()
const projectStore = useProjectStore()
function change() { function change() {
const filter = transformFilterStringForApi( const filter = transformFilterStringForApi(
filterQuery.value, filterQuery.value,
labelTitle => labelStore.filterLabelsByQuery([], labelTitle)[0]?.id || null, labelTitle => labelStore.getLabelByExactTitle(labelTitle)?.id || null,
projectTitle => { projectTitle => {
const found = projectStore.findProjectByExactname(projectTitle) const found = projectStore.findProjectByExactname(projectTitle)
return found?.id || null return found?.id || null

View File

@ -127,6 +127,16 @@ describe('Filter Transformation', () => {
expect(transformed).toBe('due_date = now/d || due_date > now/w+1w') 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', () => { describe('To API', () => {