mirror of
https://github.com/go-vikunja/vikunja.git
synced 2026-04-24 22:25:15 +00:00
fix(filter): correct filter autocomplete for project names with spaces (#2012)
Filter expressions with multi-word values (such as project names with spaces) are now automatically quoted to preserve them correctly as single tokens in the filter syntax. Fixes #2010 🐰 A filter's journey, refined with care, Multi-word values now wear quotes fair, Offsets aligned by the rabbit's precision, Autocomplete flows with mathematical vision, From comma to space, each boundary divine! ✨
This commit is contained in:
221
frontend/src/components/input/filter/FilterAutocomplete.test.ts
Normal file
221
frontend/src/components/input/filter/FilterAutocomplete.test.ts
Normal file
@@ -0,0 +1,221 @@
|
||||
import {describe, it, expect} from 'vitest'
|
||||
import {calculateReplacementRange} from './FilterAutocomplete'
|
||||
|
||||
describe('FilterAutocomplete', () => {
|
||||
describe('calculateReplacementRange', () => {
|
||||
// Note: calculateReplacementRange adds +1 to convert string indices to ProseMirror positions
|
||||
// In ProseMirror, position 0 is before the document, text starts at position 1
|
||||
|
||||
describe('single value replacement', () => {
|
||||
it('should use startPos and endPos for replacement boundaries with +1 offset for ProseMirror', () => {
|
||||
const context = {
|
||||
keyword: 'Work To Do',
|
||||
startPos: 11, // position after "project in "
|
||||
endPos: 21, // position after "Work To Do"
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
expect(result.replaceFrom).toBe(12) // 11 + 1 for ProseMirror offset
|
||||
expect(result.replaceTo).toBe(22) // 21 + 1 for ProseMirror offset
|
||||
expect(result.replaceTo - result.replaceFrom).toBe(context.keyword.length)
|
||||
})
|
||||
|
||||
it('should handle single-word values correctly', () => {
|
||||
const context = {
|
||||
keyword: 'Inbox',
|
||||
startPos: 11,
|
||||
endPos: 16,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
expect(result.replaceTo - result.replaceFrom).toBe(5) // "Inbox".length
|
||||
})
|
||||
|
||||
it('should handle equals operator', () => {
|
||||
const context = {
|
||||
keyword: 'MyProject',
|
||||
startPos: 10,
|
||||
endPos: 19,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '=')
|
||||
|
||||
expect(result.replaceFrom).toBe(11) // 10 + 1
|
||||
expect(result.replaceTo).toBe(20) // 19 + 1
|
||||
})
|
||||
})
|
||||
|
||||
describe('multi-value operator replacement', () => {
|
||||
it('should only replace text after last comma for multi-value operators', () => {
|
||||
const context = {
|
||||
keyword: 'Inbox, Work To Do',
|
||||
startPos: 11,
|
||||
endPos: 28, // 11 + 17
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
// lastCommaIndex = 5 (position of comma in "Inbox, Work To Do")
|
||||
// textAfterComma = " Work To Do" (11 chars)
|
||||
// leadingSpaces = 1
|
||||
// replaceFrom = 11 + 5 + 1 + 1 + 1 = 19 (extra +1 for ProseMirror offset)
|
||||
expect(result.replaceFrom).toBe(19)
|
||||
expect(result.replaceTo).toBe(29) // 28 + 1
|
||||
})
|
||||
|
||||
it('should handle multiple commas correctly', () => {
|
||||
const context = {
|
||||
keyword: 'One, Two, Three',
|
||||
startPos: 11,
|
||||
endPos: 26,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
// lastCommaIndex = 8 (position of second comma in "One, Two, Three")
|
||||
// textAfterComma = " Three" (6 chars)
|
||||
// leadingSpaces = 1
|
||||
// replaceFrom = 11 + 8 + 1 + 1 + 1 = 22 (extra +1 for ProseMirror offset)
|
||||
expect(result.replaceFrom).toBe(22)
|
||||
expect(result.replaceTo).toBe(27) // 26 + 1
|
||||
})
|
||||
|
||||
it('should handle ?= operator as multi-value', () => {
|
||||
const context = {
|
||||
keyword: 'Label1, Label2',
|
||||
startPos: 10,
|
||||
endPos: 24,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '?=')
|
||||
|
||||
// lastCommaIndex = 6
|
||||
// textAfterComma = " Label2" (7 chars)
|
||||
// leadingSpaces = 1
|
||||
// replaceFrom = 10 + 6 + 1 + 1 + 1 = 19 (extra +1 for ProseMirror offset)
|
||||
expect(result.replaceFrom).toBe(19)
|
||||
expect(result.replaceTo).toBe(25) // 24 + 1
|
||||
})
|
||||
|
||||
it('should handle no spaces after comma', () => {
|
||||
const context = {
|
||||
keyword: 'A,B,C',
|
||||
startPos: 5,
|
||||
endPos: 10,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
// lastCommaIndex = 3 (position of second comma)
|
||||
// textAfterComma = "C" (1 char)
|
||||
// leadingSpaces = 0
|
||||
// replaceFrom = 5 + 3 + 1 + 0 + 1 = 10 (extra +1 for ProseMirror offset)
|
||||
expect(result.replaceFrom).toBe(10)
|
||||
expect(result.replaceTo).toBe(11) // 10 + 1
|
||||
})
|
||||
|
||||
it('should not modify range for single values even with in operator', () => {
|
||||
const context = {
|
||||
keyword: 'SingleProject',
|
||||
startPos: 11,
|
||||
endPos: 24,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in')
|
||||
|
||||
// No comma in keyword, so full range should be used (with +1 offset)
|
||||
expect(result.replaceFrom).toBe(12) // 11 + 1
|
||||
expect(result.replaceTo).toBe(25) // 24 + 1
|
||||
})
|
||||
})
|
||||
|
||||
describe('non-multi-value operators', () => {
|
||||
it('should not modify range for = operator even with commas in value', () => {
|
||||
const context = {
|
||||
keyword: 'Value, With, Commas',
|
||||
startPos: 10,
|
||||
endPos: 29,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '=')
|
||||
|
||||
// = is not a multi-value operator, so full range should be used (with +1 offset)
|
||||
expect(result.replaceFrom).toBe(11) // 10 + 1
|
||||
expect(result.replaceTo).toBe(30) // 29 + 1
|
||||
})
|
||||
|
||||
it('should not modify range for != operator', () => {
|
||||
const context = {
|
||||
keyword: 'A, B',
|
||||
startPos: 10,
|
||||
endPos: 14,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '!=')
|
||||
|
||||
expect(result.replaceFrom).toBe(11) // 10 + 1
|
||||
expect(result.replaceTo).toBe(15) // 14 + 1
|
||||
})
|
||||
})
|
||||
|
||||
describe('closing quote handling', () => {
|
||||
it('should extend replaceTo by 1 when hasClosingQuote is true', () => {
|
||||
const context = {
|
||||
keyword: 'Work To Do',
|
||||
startPos: 12, // position after opening quote in 'project = "'
|
||||
endPos: 22,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '=', true)
|
||||
|
||||
expect(result.replaceFrom).toBe(13) // 12 + 1
|
||||
expect(result.replaceTo).toBe(24) // 22 + 1 + 1 (extra 1 for closing quote)
|
||||
})
|
||||
|
||||
it('should not extend replaceTo when hasClosingQuote is false', () => {
|
||||
const context = {
|
||||
keyword: 'Work To Do',
|
||||
startPos: 12,
|
||||
endPos: 22,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '=', false)
|
||||
|
||||
expect(result.replaceFrom).toBe(13) // 12 + 1
|
||||
expect(result.replaceTo).toBe(23) // 22 + 1 (no extra for closing quote)
|
||||
})
|
||||
|
||||
it('should default hasClosingQuote to false when not provided', () => {
|
||||
const context = {
|
||||
keyword: 'Work To Do',
|
||||
startPos: 12,
|
||||
endPos: 22,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, '=')
|
||||
|
||||
expect(result.replaceTo).toBe(23) // 22 + 1 (no extra for closing quote)
|
||||
})
|
||||
|
||||
it('should handle closing quote with multi-value operators', () => {
|
||||
const context = {
|
||||
keyword: 'Inbox, Work To Do',
|
||||
startPos: 12,
|
||||
endPos: 29,
|
||||
}
|
||||
|
||||
const result = calculateReplacementRange(context, 'in', true)
|
||||
|
||||
// lastCommaIndex = 5
|
||||
// textAfterComma = " Work To Do" (11 chars)
|
||||
// leadingSpaces = 1
|
||||
// replaceFrom = 12 + 5 + 1 + 1 + 1 = 20
|
||||
expect(result.replaceFrom).toBe(20)
|
||||
// replaceTo = 29 + 1 + 1 = 31 (extra 1 for closing quote)
|
||||
expect(result.replaceTo).toBe(31)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -35,6 +35,7 @@ interface AutocompleteContext {
|
||||
startPos: number
|
||||
endPos: number
|
||||
isComplete: boolean
|
||||
quoteChar: string // The quote character surrounding the keyword ('"', "'", or '' if unquoted)
|
||||
}
|
||||
|
||||
interface SuggestionItem {
|
||||
@@ -46,11 +47,49 @@ interface SuggestionItem {
|
||||
|
||||
export type AutocompleteField = 'labels' | 'assignees' | 'projects'
|
||||
|
||||
/**
|
||||
* Calculates the replacement range for autocomplete selection.
|
||||
* For single-value operators: replaces the entire keyword
|
||||
* For multi-value operators with commas: only replaces the text after the last comma
|
||||
* When inside quotes, extends the range to include the closing quote
|
||||
*
|
||||
* @param context - The autocomplete context containing position and keyword info
|
||||
* @param operator - The filter operator (e.g., 'in', '=', '?=')
|
||||
* @param hasClosingQuote - Whether there's a closing quote to include in replacement
|
||||
* @returns Object with replaceFrom and replaceTo positions
|
||||
*/
|
||||
export function calculateReplacementRange(
|
||||
context: { startPos: number; endPos: number; keyword: string },
|
||||
operator: string,
|
||||
hasClosingQuote: boolean = false,
|
||||
): { replaceFrom: number; replaceTo: number } {
|
||||
// Add 1 to convert from string indices to ProseMirror positions
|
||||
// In ProseMirror, position 0 is before the document, text starts at position 1
|
||||
let replaceFrom = context.startPos + 1
|
||||
let replaceTo = context.endPos + 1
|
||||
|
||||
// Handle multi-value operators - only replace the last value after comma
|
||||
if (isMultiValueOperator(operator) && context.keyword.includes(',')) {
|
||||
const lastCommaIndex = context.keyword.lastIndexOf(',')
|
||||
const textAfterComma = context.keyword.substring(lastCommaIndex + 1)
|
||||
const leadingSpaces = textAfterComma.length - textAfterComma.trimStart().length
|
||||
replaceFrom = context.startPos + lastCommaIndex + 1 + leadingSpaces + 1
|
||||
}
|
||||
|
||||
// Extend range to include closing quote if present
|
||||
if (hasClosingQuote) {
|
||||
replaceTo += 1
|
||||
}
|
||||
|
||||
return { replaceFrom, replaceTo }
|
||||
}
|
||||
|
||||
export interface AutocompleteItem {
|
||||
id: number | string
|
||||
title: string
|
||||
item: ILabel | IUser | IProject
|
||||
fieldType: AutocompleteField
|
||||
context: AutocompleteContext
|
||||
}
|
||||
|
||||
export default Extension.create<FilterAutocompleteOptions>({
|
||||
@@ -97,6 +136,14 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
return false
|
||||
}
|
||||
|
||||
// Check if cursor is in the middle of a word/value
|
||||
// If the character immediately after the cursor is not whitespace, operator, or delimiter,
|
||||
// then we're in the middle of a value and shouldn't show autocomplete
|
||||
const firstCharAfter = textAfterExpression[0]
|
||||
if (firstCharAfter && !/[\s&|(),"']/.test(firstCharAfter)) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Check if we're immediately after a recent selection
|
||||
const timeSinceLastSelection = Date.now() - lastSelectionTime
|
||||
if (timeSinceLastSelection < 1000) { // 1 second grace period
|
||||
@@ -112,10 +159,15 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
|
||||
// Check what comes after the expression
|
||||
const trimmedAfter = textAfterExpression.trim()
|
||||
|
||||
// If there's a logical operator or end of string immediately after, it's likely complete
|
||||
if (trimmedAfter === '' || trimmedAfter.startsWith('&&') || trimmedAfter.startsWith('||') || trimmedAfter.startsWith(')')) {
|
||||
return keyword.trim().length > 1
|
||||
|
||||
// If at end of expression (nothing after), keep autocomplete open to allow selection
|
||||
if (trimmedAfter === '') {
|
||||
return false
|
||||
}
|
||||
|
||||
// If there's a logical operator after, expression is complete (user has moved on)
|
||||
if (trimmedAfter.startsWith('&&') || trimmedAfter.startsWith('||') || trimmedAfter.startsWith(')')) {
|
||||
return true
|
||||
}
|
||||
|
||||
// If there's a space followed by non-operator text, it's likely complete
|
||||
@@ -259,7 +311,7 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
const match = pattern.exec(textUpToCursor)
|
||||
|
||||
if (match && match.index !== undefined) {
|
||||
const [, prefix = '', , , keyword = ''] = match
|
||||
const [, prefix = '', , quoteChar = '', keyword = ''] = match
|
||||
|
||||
let search = keyword.trim()
|
||||
const operator = match[0].match(new RegExp(FILTER_OPERATORS_REGEX))?.[0] || ''
|
||||
@@ -281,6 +333,7 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
startPos: match.index + prefix.length,
|
||||
endPos: match.index + prefix.length + keyword.length,
|
||||
isComplete,
|
||||
quoteChar,
|
||||
}
|
||||
|
||||
if (LABEL_FIELDS.includes(field)) {
|
||||
@@ -322,6 +375,13 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
return
|
||||
}
|
||||
|
||||
// If there's only one suggestion and it exactly matches the search term,
|
||||
// don't show autocomplete - the user has already typed/selected the complete value
|
||||
if (items.length === 1 && items[0].title?.toLowerCase() === autocompleteContext.search.toLowerCase()) {
|
||||
hidePopup()
|
||||
return
|
||||
}
|
||||
|
||||
if (!component) {
|
||||
component = new VueRenderer(FilterCommandsList, {
|
||||
props: {
|
||||
@@ -331,32 +391,22 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
const newValue = item.fieldType === 'assignees'
|
||||
? (item.item as IUser).username
|
||||
: (item.item as IProject | ILabel).title
|
||||
const {from} = view.state.selection
|
||||
const context = autocompleteContext
|
||||
// Use currentAutocompleteContext (outer variable) for up-to-date positions
|
||||
// The local autocompleteContext would be stale since this callback
|
||||
// was created on first component render
|
||||
const context = currentAutocompleteContext
|
||||
if (!context) {
|
||||
return
|
||||
}
|
||||
const operator = context.operator
|
||||
|
||||
let insertValue: string = newValue ?? ''
|
||||
const replaceFrom = Math.max(0, from - context.search.length)
|
||||
const replaceTo = from
|
||||
// Check if there's a closing quote immediately after the keyword
|
||||
const docText = view.state.doc.textContent
|
||||
const charAfterKeyword = docText[context.endPos] || ''
|
||||
const hasClosingQuote = context.quoteChar !== '' && charAfterKeyword === context.quoteChar
|
||||
|
||||
// Handle multi-value operators
|
||||
if (isMultiValueOperator(operator) && context.keyword.includes(',')) {
|
||||
// For multi-value fields, we need to replace only the current search term
|
||||
const keywords = context.keyword.split(',')
|
||||
const currentKeywordIndex = keywords.length - 1
|
||||
|
||||
// If we're not adding the first item, add comma prefix
|
||||
if (currentKeywordIndex > 0 && keywords[currentKeywordIndex]?.trim() === context.search.trim()) {
|
||||
// We're replacing the last incomplete keyword
|
||||
insertValue = newValue ?? ''
|
||||
} else {
|
||||
// We're adding to existing keywords
|
||||
insertValue = ',' + newValue
|
||||
}
|
||||
}
|
||||
const insertValue: string = newValue ?? ''
|
||||
const { replaceFrom, replaceTo } = calculateReplacementRange(context, operator, hasClosingQuote)
|
||||
|
||||
const tr = view.state.tr.replaceWith(
|
||||
replaceFrom,
|
||||
@@ -378,24 +428,10 @@ export default Extension.create<FilterAutocompleteOptions>({
|
||||
view.focus()
|
||||
}, 0)
|
||||
|
||||
// For multi-value operators, don't suppress autocomplete to keep dropdown open
|
||||
if (isMultiValueOperator(operator)) {
|
||||
// Add comma and space for next entry if not already present
|
||||
setTimeout(() => {
|
||||
const currentText = view.state.doc.textContent
|
||||
const currentPos = view.state.selection.from
|
||||
if (currentText.charAt(currentPos) !== ',') {
|
||||
const tr = view.state.tr.insertText(',', currentPos)
|
||||
view.dispatch(tr)
|
||||
// Update position after comma insertion
|
||||
lastSelectionPosition = currentPos + 1
|
||||
lastSelectionTime = Date.now()
|
||||
}
|
||||
}, 10)
|
||||
} else {
|
||||
suppressNextAutocomplete = true
|
||||
hidePopup()
|
||||
}
|
||||
// Always suppress and hide after selection
|
||||
// User can type comma manually if they want to add more values
|
||||
suppressNextAutocomplete = true
|
||||
hidePopup()
|
||||
},
|
||||
},
|
||||
editor: this.editor,
|
||||
|
||||
@@ -527,12 +527,34 @@ describe('Filter Transformation', () => {
|
||||
|
||||
it('should transform the same attribute multiple times', () => {
|
||||
const transformed = transformFilterStringFromApi(
|
||||
'due_date = now/d || due_date > now/w+1w',
|
||||
'due_date = now/d || due_date > now/w+1w',
|
||||
nullIdToTitleResolver,
|
||||
nullIdToTitleResolver,
|
||||
)
|
||||
|
||||
expect(transformed).toBe('dueDate = now/d || dueDate > now/w+1w')
|
||||
})
|
||||
|
||||
it('should not replace label id that appears in other parts of the filter', () => {
|
||||
// This tests that position-based replacement is used, not global string replace
|
||||
// The label id "1" should only be replaced in the labels clause, not in "priority = 1"
|
||||
const transformed = transformFilterStringFromApi(
|
||||
'priority = 1 && labels = 1',
|
||||
(id: number) => id === 1 ? 'My Label' : null,
|
||||
nullIdToTitleResolver,
|
||||
)
|
||||
|
||||
expect(transformed).toBe('priority = 1 && labels = My Label')
|
||||
})
|
||||
|
||||
it('should not replace project id that appears in other parts of the filter', () => {
|
||||
const transformed = transformFilterStringFromApi(
|
||||
'priority = 2 && project = 2',
|
||||
nullIdToTitleResolver,
|
||||
(id: number) => id === 2 ? 'My Project' : null,
|
||||
)
|
||||
|
||||
expect(transformed).toBe('priority = 2 && project = My Project')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -114,7 +114,7 @@ export function transformFilterStringForApi(
|
||||
const replacements: { start: number, length: number, replacement: string }[] = []
|
||||
|
||||
while ((match = pattern.exec(filter)) !== null) {
|
||||
const [matched, fieldName, operator, quotes, quotedContent, unquotedContent] = match
|
||||
const [matched, fieldName, operator, _quotes, quotedContent, unquotedContent] = match
|
||||
const keyword = quotedContent || unquotedContent
|
||||
if (!keyword) {
|
||||
continue
|
||||
@@ -151,16 +151,7 @@ export function transformFilterStringForApi(
|
||||
replaced = replaced.replaceAll('"', '').replaceAll('\'', '')
|
||||
|
||||
// Reconstruct the entire match with the replaced value
|
||||
let reconstructedMatch
|
||||
if (quotes && quotedContent) {
|
||||
// For quoted values, remove quotes since we converted to IDs
|
||||
reconstructedMatch = `${fieldName} ${operator} ${replaced}`
|
||||
} else if (unquotedContent) {
|
||||
// For unquoted values
|
||||
reconstructedMatch = `${fieldName} ${operator} ${replaced}`
|
||||
} else {
|
||||
continue
|
||||
}
|
||||
const reconstructedMatch = `${fieldName} ${operator} ${replaced}`
|
||||
|
||||
replacements.push({
|
||||
start: match.index!,
|
||||
@@ -221,29 +212,59 @@ export function transformFilterStringFromApi(
|
||||
const pattern = getFilterFieldRegexPattern(field)
|
||||
|
||||
let match: RegExpExecArray | null
|
||||
while ((match = pattern.exec(filter)) !== null) {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const [matched, fieldName, operator, quotes, quotedContent, unquotedContent] = match
|
||||
const keyword = quotedContent || unquotedContent
|
||||
if (keyword) {
|
||||
let keywords = [keyword.trim()]
|
||||
if (isMultiValueOperator(operator)) {
|
||||
keywords = keyword.trim().split(',').map(k => {
|
||||
let trimmed = k.trim()
|
||||
// Strip quotes from individual values in multi-value scenarios
|
||||
trimmed = trimQuotes(trimmed)
|
||||
return trimmed
|
||||
})
|
||||
}
|
||||
const replacements: { start: number, length: number, replacement: string }[] = []
|
||||
|
||||
keywords.forEach(k => {
|
||||
const title = resolver(parseInt(k))
|
||||
if (title) {
|
||||
filter = filter.replace(k, title)
|
||||
}
|
||||
while ((match = pattern.exec(filter)) !== null) {
|
||||
const [matched, fieldName, operator, _quotes, quotedContent, unquotedContent] = match
|
||||
const keyword = quotedContent || unquotedContent
|
||||
if (!keyword) {
|
||||
continue
|
||||
}
|
||||
|
||||
let keywords = [keyword.trim()]
|
||||
if (isMultiValueOperator(operator)) {
|
||||
keywords = keyword.trim().split(',').map(k => {
|
||||
let trimmed = k.trim()
|
||||
// Strip quotes from individual values in multi-value scenarios
|
||||
trimmed = trimQuotes(trimmed)
|
||||
return trimmed
|
||||
})
|
||||
}
|
||||
|
||||
const transformedKeywords: string[] = []
|
||||
keywords.forEach(k => {
|
||||
const title = resolver(parseInt(k))
|
||||
if (title) {
|
||||
transformedKeywords.push(title)
|
||||
} else {
|
||||
// Keep original value if resolver returns null
|
||||
transformedKeywords.push(k)
|
||||
}
|
||||
})
|
||||
|
||||
// Reconstruct the entire match with the replaced values
|
||||
const replaced = isMultiValueOperator(operator)
|
||||
? transformedKeywords.join(', ')
|
||||
: transformedKeywords[0] || keyword
|
||||
|
||||
const reconstructedMatch = `${fieldName} ${operator} ${replaced}`
|
||||
|
||||
replacements.push({
|
||||
start: match.index!,
|
||||
length: matched.length,
|
||||
replacement: reconstructedMatch,
|
||||
})
|
||||
}
|
||||
|
||||
// Apply replacements using position-based replacement to avoid
|
||||
// incorrectly replacing values that appear elsewhere in the string
|
||||
let offset = 0
|
||||
replacements.forEach(({start, length, replacement}) => {
|
||||
filter = filter.substring(0, start + offset) +
|
||||
replacement +
|
||||
filter.substring(start + offset + length)
|
||||
offset += replacement.length - length
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
266
frontend/tests/e2e/filters/filter-autocomplete.spec.ts
Normal file
266
frontend/tests/e2e/filters/filter-autocomplete.spec.ts
Normal file
@@ -0,0 +1,266 @@
|
||||
import {test, expect} from '../../support/fixtures'
|
||||
import {ProjectFactory} from '../../factories/project'
|
||||
import {TaskFactory} from '../../factories/task'
|
||||
import {ProjectViewFactory} from '../../factories/project_view'
|
||||
import {SavedFilterFactory} from '../../factories/saved_filter'
|
||||
|
||||
/**
|
||||
* Tests for filter autocomplete functionality, specifically for:
|
||||
* - Project names with spaces (Issue #2010)
|
||||
* - Verifying filters save correctly without corruption
|
||||
*/
|
||||
|
||||
async function createProjectWithViews(id: number, title: string, ownerId: number, truncate = false) {
|
||||
await ProjectFactory.create(1, {
|
||||
id,
|
||||
title,
|
||||
owner_id: ownerId,
|
||||
}, truncate)
|
||||
await ProjectViewFactory.create(1, {
|
||||
id: id * 4,
|
||||
project_id: id,
|
||||
view_kind: 0, // List
|
||||
}, false)
|
||||
}
|
||||
|
||||
// Helper to get the filter input ProseMirror editor
|
||||
function getFilterInput(page) {
|
||||
return page.locator('.filter-input .ProseMirror')
|
||||
}
|
||||
|
||||
test.describe('Filter Autocomplete', () => {
|
||||
test.beforeEach(async ({authenticatedPage, currentUser}) => {
|
||||
// authenticatedPage fixture triggers apiContext which sets up Factory.request
|
||||
await ProjectFactory.truncate()
|
||||
await TaskFactory.truncate()
|
||||
await ProjectViewFactory.truncate()
|
||||
await SavedFilterFactory.truncate()
|
||||
|
||||
const userId = currentUser.id
|
||||
|
||||
// Create projects - one with spaces in name (the bug case)
|
||||
await createProjectWithViews(1, 'Inbox', userId)
|
||||
await createProjectWithViews(2, 'Work To Do', userId)
|
||||
await createProjectWithViews(3, 'Personal Tasks', userId)
|
||||
|
||||
// Create tasks in each project
|
||||
await TaskFactory.create(1, {
|
||||
id: 1,
|
||||
project_id: 1,
|
||||
title: 'Inbox Task',
|
||||
})
|
||||
await TaskFactory.create(1, {
|
||||
id: 2,
|
||||
project_id: 2,
|
||||
title: 'Work Task 1',
|
||||
})
|
||||
await TaskFactory.create(1, {
|
||||
id: 3,
|
||||
project_id: 2,
|
||||
title: 'Work Task 2',
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Saved Filter Creation with Autocomplete', () => {
|
||||
test('should replace single-word project name via autocomplete', async ({authenticatedPage: page}) => {
|
||||
await page.goto('/filters/new')
|
||||
|
||||
// Wait for projects to be loaded
|
||||
await expect(page.getByRole('link', {name: 'Inbox', exact: true})).toBeVisible({timeout: 10000})
|
||||
|
||||
// Fill in filter name
|
||||
await page.locator('input#Title').fill('Inbox Filter')
|
||||
|
||||
// Type filter with project name to trigger autocomplete
|
||||
const filterInput = getFilterInput(page)
|
||||
await filterInput.click()
|
||||
await page.keyboard.press('ControlOrMeta+a')
|
||||
await page.keyboard.press('Backspace')
|
||||
await filterInput.pressSequentially('project = Inb', {delay: 50})
|
||||
|
||||
// Wait for autocomplete popup and select "Inbox"
|
||||
const autocompletePopup = page.locator('#filter-autocomplete-popup')
|
||||
await expect(autocompletePopup).toBeVisible({timeout: 5000})
|
||||
await autocompletePopup.getByRole('button', {name: 'Inbox'}).click()
|
||||
|
||||
// Verify the filter text is correct after autocomplete replacement
|
||||
await expect(filterInput).toContainText('project = Inbox')
|
||||
|
||||
// Save the filter and verify no error
|
||||
await page.locator('button.is-primary.is-fullwidth').click()
|
||||
await expect(page.locator('.notification.is-danger')).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('should replace multi-word project name with spaces via autocomplete', async ({authenticatedPage: page}) => {
|
||||
await page.goto('/filters/new')
|
||||
|
||||
// Wait for projects to be loaded
|
||||
await expect(page.locator('.menu-list a').filter({hasText: 'Work To Do'})).toBeVisible({timeout: 10000})
|
||||
|
||||
// Fill in filter name
|
||||
await page.locator('input#Title').fill('Work Filter')
|
||||
|
||||
// Type filter with partial project name to trigger autocomplete
|
||||
const filterInput = getFilterInput(page)
|
||||
await filterInput.click()
|
||||
await page.keyboard.press('ControlOrMeta+a')
|
||||
await page.keyboard.press('Backspace')
|
||||
await filterInput.pressSequentially('project = Work', {delay: 50})
|
||||
|
||||
// Wait for autocomplete popup with the specific option we want to click
|
||||
const autocompletePopup = page.locator('#filter-autocomplete-popup')
|
||||
await expect(autocompletePopup).toBeVisible({timeout: 5000})
|
||||
// Wait for the specific autocomplete option to be visible (ensures context is updated)
|
||||
const workToDoButton = autocompletePopup.getByRole('button', {name: 'Work To Do'})
|
||||
await expect(workToDoButton).toBeVisible({timeout: 2000})
|
||||
await workToDoButton.click()
|
||||
|
||||
// Verify the filter text is correct after autocomplete replacement
|
||||
await expect(filterInput).toContainText('project = Work To Do')
|
||||
|
||||
// Save the filter and verify no error
|
||||
await page.locator('button.is-primary.is-fullwidth').click()
|
||||
await expect(page.locator('.notification.is-danger')).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('should handle autocomplete after logical operator', async ({authenticatedPage: page}) => {
|
||||
await page.goto('/filters/new')
|
||||
|
||||
// Wait for projects to be loaded
|
||||
await expect(page.getByRole('link', {name: 'Inbox', exact: true})).toBeVisible({timeout: 10000})
|
||||
|
||||
await page.locator('input#Title').fill('Complex Filter')
|
||||
|
||||
const filterInput = getFilterInput(page)
|
||||
await filterInput.click()
|
||||
await page.keyboard.press('ControlOrMeta+a')
|
||||
await page.keyboard.press('Backspace')
|
||||
|
||||
// Type a complex filter with autocomplete for project name
|
||||
await filterInput.pressSequentially('done = false && project = Pers', {delay: 50})
|
||||
|
||||
// Wait for autocomplete popup and select "Personal Tasks"
|
||||
const autocompletePopup = page.locator('#filter-autocomplete-popup')
|
||||
await expect(autocompletePopup).toBeVisible({timeout: 5000})
|
||||
await autocompletePopup.getByRole('button', {name: 'Personal Tasks'}).click()
|
||||
|
||||
// Verify correct filter text
|
||||
await expect(filterInput).toContainText('done = false && project = Personal Tasks')
|
||||
|
||||
// Save and verify no error
|
||||
await page.locator('button.is-primary.is-fullwidth').click()
|
||||
await expect(page.locator('.notification.is-danger')).not.toBeVisible()
|
||||
})
|
||||
})
|
||||
|
||||
test.describe('Edit Saved Filter with Multi-Value Autocomplete (Issue #2010 Regression)', () => {
|
||||
test('should preserve filter text after editing and adding trailing space', async ({authenticatedPage: page}) => {
|
||||
// This test covers the specific bug from Issue #2010:
|
||||
// Creating a filter with 'project in Work To Do, Inbox', then editing
|
||||
// and adding a trailing space should not corrupt the filter or cause errors
|
||||
|
||||
await page.goto('/filters/new')
|
||||
|
||||
// Wait for projects to be loaded
|
||||
await expect(page.locator('.menu-list a').filter({hasText: 'Work To Do'})).toBeVisible({timeout: 10000})
|
||||
|
||||
// Step 1: Create a filter with multi-value project using 'in' operator
|
||||
await page.locator('input#Title').fill('Work Filter')
|
||||
|
||||
const filterInput = getFilterInput(page)
|
||||
await filterInput.click()
|
||||
await page.keyboard.press('ControlOrMeta+a')
|
||||
await page.keyboard.press('Backspace')
|
||||
|
||||
// Type 'project in Work' to trigger autocomplete
|
||||
await filterInput.pressSequentially('project in Work', {delay: 50})
|
||||
|
||||
// Wait for autocomplete popup with the specific option we want to click
|
||||
const autocompletePopup = page.locator('#filter-autocomplete-popup')
|
||||
await expect(autocompletePopup).toBeVisible({timeout: 5000})
|
||||
// Wait for the specific autocomplete option to be visible (ensures context is updated)
|
||||
const workToDoButton = autocompletePopup.getByRole('button', {name: 'Work To Do'})
|
||||
await expect(workToDoButton).toBeVisible({timeout: 2000})
|
||||
await workToDoButton.click()
|
||||
|
||||
// Wait for autocomplete to close and text to stabilize
|
||||
await expect(autocompletePopup).not.toBeVisible({timeout: 2000})
|
||||
await expect(filterInput).toContainText('project in Work To Do')
|
||||
|
||||
// Continue typing the second value: ', Inbox'
|
||||
await filterInput.click()
|
||||
await page.keyboard.press('End')
|
||||
await filterInput.pressSequentially(', Inbox', {delay: 50})
|
||||
|
||||
// Verify the filter text shows the multi-value 'in' clause
|
||||
await expect(filterInput).toContainText('project in Work To Do, Inbox')
|
||||
|
||||
// Step 2: Save the filter and verify no error
|
||||
await page.locator('button.is-primary.is-fullwidth').click()
|
||||
await expect(page.locator('.notification.is-danger')).not.toBeVisible()
|
||||
|
||||
// Wait for navigation to the saved filter view
|
||||
await expect(page).toHaveURL(/\/projects\/-\d+/, {timeout: 5000})
|
||||
|
||||
// Step 3: Open the filter settings menu from sidebar and click Edit
|
||||
// Find the Work Filter link in the sidebar and click its settings menu
|
||||
const filterLink = page.locator('.menu-list').getByRole('link', {name: 'Work Filter', exact: true})
|
||||
await expect(filterLink).toBeVisible()
|
||||
|
||||
// Hover over the filter to show the settings menu button
|
||||
const filterItem = filterLink.locator('..')
|
||||
await filterItem.hover()
|
||||
|
||||
// Click the settings menu button
|
||||
const settingsButton = filterItem.getByRole('button', {name: 'Open project settings menu'})
|
||||
await settingsButton.click()
|
||||
|
||||
// Click "Edit" link in the dropdown menu
|
||||
await page.getByRole('link', {name: 'Edit', exact: true}).click()
|
||||
|
||||
// Wait for the edit modal/form to be loaded
|
||||
await expect(page.locator('input#Title')).toHaveValue('Work Filter', {timeout: 5000})
|
||||
|
||||
// Find the filter input inside the Filters component
|
||||
const editFilterInput = page.locator('.filters .filter-input .ProseMirror')
|
||||
await expect(editFilterInput).toBeVisible()
|
||||
|
||||
// Verify the filter text is correctly loaded
|
||||
await expect(editFilterInput).toContainText('project in Work To Do, Inbox')
|
||||
|
||||
// Step 4: Add a trailing space (the bug trigger from #2010)
|
||||
await editFilterInput.click()
|
||||
// Move to end of text
|
||||
await page.keyboard.press('End')
|
||||
// Type a space
|
||||
await page.keyboard.type(' ')
|
||||
|
||||
// Step 5: Save again and wait for the modal to close (indicates save complete)
|
||||
const saveButton = page.locator('.card-footer .button.is-primary')
|
||||
await saveButton.click()
|
||||
// Wait for the edit card/modal to close after save
|
||||
await expect(saveButton).not.toBeVisible({timeout: 5000})
|
||||
|
||||
// Step 6: Assert no error occurred
|
||||
await expect(page.locator('.notification.is-danger')).not.toBeVisible()
|
||||
|
||||
// Step 7: Reload and re-open edit to verify the filter text is still intact
|
||||
await page.reload()
|
||||
await expect(page.locator('.menu-list').getByRole('link', {name: 'Work Filter', exact: true})).toBeVisible({timeout: 5000})
|
||||
|
||||
// Re-open the edit modal
|
||||
const filterLinkAfterReload = page.locator('.menu-list').getByRole('link', {name: 'Work Filter', exact: true})
|
||||
const filterItemAfterReload = filterLinkAfterReload.locator('..')
|
||||
await filterItemAfterReload.hover()
|
||||
await filterItemAfterReload.getByRole('button', {name: 'Open project settings menu'}).click()
|
||||
await page.getByRole('link', {name: 'Edit', exact: true}).click()
|
||||
|
||||
// Verify the filter title and content are intact
|
||||
await expect(page.locator('input#Title')).toHaveValue('Work Filter', {timeout: 5000})
|
||||
|
||||
const reloadedFilterInput = page.locator('.filters .filter-input .ProseMirror')
|
||||
// The trailing space may be trimmed, but the core filter should be preserved
|
||||
await expect(reloadedFilterInput).toContainText('project in Work To Do, Inbox')
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user