mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
fix: add support for spaces in search & enable searching by display name in templates (#19552)
## Summary In this pull request we're updating search to support queries with spaces in addition to the `field:value` pattern that is currently supported. Additionally templates search now defaults to `display_name` (since `display_name` is optional the search will fallback to `name`) when searching without the `field:value` pattern Closes: https://github.com/coder/coder/issues/14384 ### Downsides with searching on `name` and `display_name` Because the `name` field cannot include spaces, we end up in a situation where including a space in the query will result in no results since the query searches on both `name` AND `display_name`. In the following example, we can see the results of searching by both `name` and `display_name` on these templates: | Name | Display Name | | ------ | ------------- | | docker | Docker Template | | faketemplate | A Fake Template | | azure | Fake Azure Template | | anotherfake | Another Fake Template | | azurefake | Another Fake Fake Azure Template | https://github.com/user-attachments/assets/b0e0793e-e77d-46bc-9a42-d7cf4f8bd910 ### Proposal: Search on `display_name` by default and allow for `name` using the `field:value` pattern If we remove `name` from the default template search, we're now able to search with spaces on template `display_names`. Since `display_names` are what users see in the templates list they might expect the search to work this way. Below is an example of `name` being removed from the default template search. https://github.com/user-attachments/assets/9aba5911-4960-4384-befb-08ea1acaa3ab With this approach users would still be able to search on template names by specifying `exact_name:foo`. ### Testing Added additional test cases to ensure spaces were handled as expected in combination with `field:value` patterns.
This commit is contained in:
@@ -78,7 +78,9 @@ func (q *sqlQuerier) GetAuthorizedTemplates(ctx context.Context, arg GetTemplate
|
||||
arg.Deleted,
|
||||
arg.OrganizationID,
|
||||
arg.ExactName,
|
||||
arg.ExactDisplayName,
|
||||
arg.FuzzyName,
|
||||
arg.FuzzyDisplayName,
|
||||
pq.Array(arg.IDs),
|
||||
arg.Deprecated,
|
||||
arg.HasAITask,
|
||||
|
||||
@@ -12230,23 +12230,41 @@ WHERE
|
||||
LOWER(t.name) = LOWER($3)
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by name, matching on substring
|
||||
-- Filter by exact display name
|
||||
AND CASE
|
||||
WHEN $4 :: text != '' THEN
|
||||
lower(t.name) ILIKE '%' || lower($4) || '%'
|
||||
LOWER(t.display_name) = LOWER($4)
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by name, matching on substring
|
||||
AND CASE
|
||||
WHEN $5 :: text != '' THEN
|
||||
lower(t.name) ILIKE '%' || lower($5) || '%'
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by display_name, matching on substring (fallback to name if display_name is empty)
|
||||
AND CASE
|
||||
WHEN $6 :: text != '' THEN
|
||||
CASE
|
||||
WHEN t.display_name IS NOT NULL AND t.display_name != '' THEN
|
||||
lower(t.display_name) ILIKE '%' || lower($6) || '%'
|
||||
ELSE
|
||||
-- Remove spaces if present since 't.name' cannot have any spaces
|
||||
lower(t.name) ILIKE '%' || REPLACE(lower($6), ' ', '') || '%'
|
||||
END
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by ids
|
||||
AND CASE
|
||||
WHEN array_length($5 :: uuid[], 1) > 0 THEN
|
||||
t.id = ANY($5)
|
||||
WHEN array_length($7 :: uuid[], 1) > 0 THEN
|
||||
t.id = ANY($7)
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by deprecated
|
||||
AND CASE
|
||||
WHEN $6 :: boolean IS NOT NULL THEN
|
||||
WHEN $8 :: boolean IS NOT NULL THEN
|
||||
CASE
|
||||
WHEN $6 :: boolean THEN
|
||||
WHEN $8 :: boolean THEN
|
||||
t.deprecated != ''
|
||||
ELSE
|
||||
t.deprecated = ''
|
||||
@@ -12255,27 +12273,27 @@ WHERE
|
||||
END
|
||||
-- Filter by has_ai_task in latest version
|
||||
AND CASE
|
||||
WHEN $7 :: boolean IS NOT NULL THEN
|
||||
tv.has_ai_task = $7 :: boolean
|
||||
WHEN $9 :: boolean IS NOT NULL THEN
|
||||
tv.has_ai_task = $9 :: boolean
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by author_id
|
||||
AND CASE
|
||||
WHEN $8 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
|
||||
t.created_by = $8
|
||||
WHEN $10 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
|
||||
t.created_by = $10
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by author_username
|
||||
AND CASE
|
||||
WHEN $9 :: text != '' THEN
|
||||
t.created_by = (SELECT id FROM users WHERE lower(users.username) = lower($9) AND deleted = false)
|
||||
WHEN $11 :: text != '' THEN
|
||||
t.created_by = (SELECT id FROM users WHERE lower(users.username) = lower($11) AND deleted = false)
|
||||
ELSE true
|
||||
END
|
||||
|
||||
-- Filter by has_external_agent in latest version
|
||||
AND CASE
|
||||
WHEN $10 :: boolean IS NOT NULL THEN
|
||||
tv.has_external_agent = $10 :: boolean
|
||||
WHEN $12 :: boolean IS NOT NULL THEN
|
||||
tv.has_external_agent = $12 :: boolean
|
||||
ELSE true
|
||||
END
|
||||
-- Authorize Filter clause will be injected below in GetAuthorizedTemplates
|
||||
@@ -12287,7 +12305,9 @@ type GetTemplatesWithFilterParams struct {
|
||||
Deleted bool `db:"deleted" json:"deleted"`
|
||||
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
|
||||
ExactName string `db:"exact_name" json:"exact_name"`
|
||||
ExactDisplayName string `db:"exact_display_name" json:"exact_display_name"`
|
||||
FuzzyName string `db:"fuzzy_name" json:"fuzzy_name"`
|
||||
FuzzyDisplayName string `db:"fuzzy_display_name" json:"fuzzy_display_name"`
|
||||
IDs []uuid.UUID `db:"ids" json:"ids"`
|
||||
Deprecated sql.NullBool `db:"deprecated" json:"deprecated"`
|
||||
HasAITask sql.NullBool `db:"has_ai_task" json:"has_ai_task"`
|
||||
@@ -12301,7 +12321,9 @@ func (q *sqlQuerier) GetTemplatesWithFilter(ctx context.Context, arg GetTemplate
|
||||
arg.Deleted,
|
||||
arg.OrganizationID,
|
||||
arg.ExactName,
|
||||
arg.ExactDisplayName,
|
||||
arg.FuzzyName,
|
||||
arg.FuzzyDisplayName,
|
||||
pq.Array(arg.IDs),
|
||||
arg.Deprecated,
|
||||
arg.HasAITask,
|
||||
|
||||
@@ -30,12 +30,30 @@ WHERE
|
||||
LOWER(t.name) = LOWER(@exact_name)
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by exact display name
|
||||
AND CASE
|
||||
WHEN @exact_display_name :: text != '' THEN
|
||||
LOWER(t.display_name) = LOWER(@exact_display_name)
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by name, matching on substring
|
||||
AND CASE
|
||||
WHEN @fuzzy_name :: text != '' THEN
|
||||
lower(t.name) ILIKE '%' || lower(@fuzzy_name) || '%'
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by display_name, matching on substring (fallback to name if display_name is empty)
|
||||
AND CASE
|
||||
WHEN @fuzzy_display_name :: text != '' THEN
|
||||
CASE
|
||||
WHEN t.display_name IS NOT NULL AND t.display_name != '' THEN
|
||||
lower(t.display_name) ILIKE '%' || lower(@fuzzy_display_name) || '%'
|
||||
ELSE
|
||||
-- Remove spaces if present since 't.name' cannot have any spaces
|
||||
lower(t.name) ILIKE '%' || REPLACE(lower(@fuzzy_display_name), ' ', '') || '%'
|
||||
END
|
||||
ELSE true
|
||||
END
|
||||
-- Filter by ids
|
||||
AND CASE
|
||||
WHEN array_length(@ids :: uuid[], 1) > 0 THEN
|
||||
|
||||
@@ -268,8 +268,8 @@ func Templates(ctx context.Context, db database.Store, actorID uuid.UUID, query
|
||||
// Always lowercase for all searches.
|
||||
query = strings.ToLower(query)
|
||||
values, errors := searchTerms(query, func(term string, values url.Values) error {
|
||||
// Default to the template name
|
||||
values.Add("name", term)
|
||||
// Default to the display name
|
||||
values.Add("display_name", term)
|
||||
return nil
|
||||
})
|
||||
if len(errors) > 0 {
|
||||
@@ -281,7 +281,9 @@ func Templates(ctx context.Context, db database.Store, actorID uuid.UUID, query
|
||||
Deleted: parser.Boolean(values, false, "deleted"),
|
||||
OrganizationID: parseOrganization(ctx, db, parser, values, "organization"),
|
||||
ExactName: parser.String(values, "", "exact_name"),
|
||||
ExactDisplayName: parser.String(values, "", "exact_display_name"),
|
||||
FuzzyName: parser.String(values, "", "name"),
|
||||
FuzzyDisplayName: parser.String(values, "", "display_name"),
|
||||
IDs: parser.UUIDs(values, []uuid.UUID{}, "ids"),
|
||||
Deprecated: parser.NullableBoolean(values, sql.NullBool{}, "deprecated"),
|
||||
HasAITask: parser.NullableBoolean(values, sql.NullBool{}, "has-ai-task"),
|
||||
@@ -305,7 +307,8 @@ func searchTerms(query string, defaultKey func(term string, values url.Values) e
|
||||
// Because we do this in 2 passes, we want to maintain quotes on the first
|
||||
// pass. Further splitting occurs on the second pass and quotes will be
|
||||
// dropped.
|
||||
elements := splitQueryParameterByDelimiter(query, ' ', true)
|
||||
tokens := splitQueryParameterByDelimiter(query, ' ', true)
|
||||
elements := processTokens(tokens)
|
||||
for _, element := range elements {
|
||||
if strings.HasPrefix(element, ":") || strings.HasSuffix(element, ":") {
|
||||
return nil, []codersdk.ValidationError{
|
||||
@@ -385,3 +388,24 @@ func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes
|
||||
|
||||
return parts
|
||||
}
|
||||
|
||||
// processTokens takes the split tokens and groups them based on a delimiter (':').
|
||||
// Tokens without a delimiter present are joined to support searching with spaces.
|
||||
//
|
||||
// Example Input: ['deprecated:false', 'test', 'template']
|
||||
// Example Output: ['deprecated:false', 'test template']
|
||||
func processTokens(tokens []string) []string {
|
||||
var results []string
|
||||
var nonFieldTerms []string
|
||||
for _, token := range tokens {
|
||||
if strings.Contains(token, string(':')) {
|
||||
results = append(results, token)
|
||||
} else {
|
||||
nonFieldTerms = append(nonFieldTerms, token)
|
||||
}
|
||||
}
|
||||
if len(nonFieldTerms) > 0 {
|
||||
results = append(results, strings.Join(nonFieldTerms, " "))
|
||||
}
|
||||
return results
|
||||
}
|
||||
|
||||
@@ -686,7 +686,7 @@ func TestSearchTemplates(t *testing.T) {
|
||||
Name: "OnlyName",
|
||||
Query: "foobar",
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
FuzzyName: "foobar",
|
||||
FuzzyDisplayName: "foobar",
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -757,6 +757,43 @@ func TestSearchTemplates(t *testing.T) {
|
||||
AuthorID: userID,
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "SearchOnDisplayName",
|
||||
Query: "test name",
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
FuzzyDisplayName: "test name",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "NameField",
|
||||
Query: "name:testname",
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
FuzzyName: "testname",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "QuotedValue",
|
||||
Query: `name:"test name"`,
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
FuzzyName: "test name",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "MultipleTerms",
|
||||
Query: `foo bar exact_name:"test display name"`,
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
ExactName: "test display name",
|
||||
FuzzyDisplayName: "foo bar",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "FieldAndSpaces",
|
||||
Query: "deprecated:false test template",
|
||||
Expected: database.GetTemplatesWithFilterParams{
|
||||
Deprecated: sql.NullBool{Bool: false, Valid: true},
|
||||
FuzzyDisplayName: "test template",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range testCases {
|
||||
|
||||
Reference in New Issue
Block a user