-
Notifications
You must be signed in to change notification settings - Fork 44
fix(orgusers): support case-insensitive name filtering #1723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -325,16 +325,29 @@ func (r OrgUsersRepository) buildHasAnyRoleSubquery(orgID string) *goqu.SelectDa | |
| } | ||
|
|
||
| func (r OrgUsersRepository) buildNonRoleFilterCondition(filter rql.Filter) (goqu.Expression, error) { | ||
| //this list will always be a subset of all RQL supported operators | ||
| supportedStringOperators := []string{"eq", "neq", "like", "in", "notin", "notlike", "empty", "notempty"} | ||
| supportedDateTimeOperators := []string{"eq", "neq", "gt", "lt", "gte", "lte"} | ||
|
|
||
| if !slices.Contains(supportedStringOperators, filter.Operator) && !slices.Contains(supportedDateTimeOperators, filter.Operator) { | ||
| return nil, wrapBadOperatorError(filter.Operator, filter.Name) | ||
| // Route by the column's declared datatype so that string-only operators | ||
| // (like/ilike) are rejected on the datetime column org_joined_at. | ||
| datatype, err := rql.GetDataTypeOfField(filter.Name, svc.AggregatedUser{}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| columnName := r.getNonRoleColumnName(filter.Name) | ||
|
|
||
| if datatype == "datetime" { | ||
| supportedDateTimeOperators := []string{"eq", "neq", "gt", "lt", "gte", "lte"} | ||
| if !slices.Contains(supportedDateTimeOperators, filter.Operator) { | ||
| return nil, wrapBadOperatorError(filter.Operator, filter.Name) | ||
| } | ||
| return goqu.Ex{columnName: goqu.Op{filter.Operator: filter.Value.(string)}}, nil | ||
| } | ||
|
|
||
| // string columns | ||
| supportedStringOperators := []string{"eq", "neq", "like", "ilike", "in", "notin", "notlike", "notilike", "empty", "notempty"} | ||
| if !slices.Contains(supportedStringOperators, filter.Operator) { | ||
| return nil, wrapBadOperatorError(filter.Operator, filter.Name) | ||
| } | ||
|
|
||
| switch filter.Operator { | ||
| case "empty": | ||
| return goqu.Or(goqu.I(columnName).IsNull(), goqu.I(columnName).Eq("")), nil | ||
|
|
@@ -343,11 +356,17 @@ func (r OrgUsersRepository) buildNonRoleFilterCondition(filter rql.Filter) (goqu | |
| case "in", "notin": | ||
| return goqu.Ex{columnName: goqu.Op{filter.Operator: strings.Split(filter.Value.(string), ",")}}, nil | ||
| case "like": | ||
| // case-insensitive match; wildcards are added here | ||
| searchPattern := "%" + filter.Value.(string) + "%" | ||
| return goqu.Cast(goqu.I(columnName), "TEXT").ILike(searchPattern), nil | ||
| case "notlike": | ||
| searchPattern := "%" + filter.Value.(string) + "%" | ||
| return goqu.Cast(goqu.I(columnName), "TEXT").NotILike(searchPattern), nil | ||
| case "ilike": | ||
| // case-insensitive match; the frontend already includes the wildcards (e.g. "%foo%") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not follow the same pattern? In one case, the frontend adds
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both the In
So for the org-users name filter the client always sends You're right that it's inconsistent, though:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done — wildcards are now caller-owned for all four operators ( Updated the |
||
| return goqu.Cast(goqu.I(columnName), "TEXT").ILike(filter.Value.(string)), nil | ||
| case "notilike": | ||
| return goqu.Cast(goqu.I(columnName), "TEXT").NotILike(filter.Value.(string)), nil | ||
| default: // eq, neq | ||
| return goqu.Ex{columnName: goqu.Op{filter.Operator: filter.Value.(string)}}, nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like also uses iLIKE only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct — goqu's
.ILike()emits SQLILIKE, solikehere is already case-insensitive. The only real difference between thelikeandilikebranches is who owns the%..%wildcards (server-side forlike, caller-side forilike). See my note on theilikecase below — Apsara only ever sendsilikewith wildcards baked in, so I'll collapse these to one convention.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — fixed. I aligned this file to the same convention
org_pats_repositoryandorg_billing_repositoryalready use (the two other repos that own wildcards caller-side + cast to TEXT):So
likenow maps to real SQLLIKE(case-sensitive) instead ofILIKE, andilikestaysILIKE. No more silent case-insensitivity onlike.