From 4c5f4573138da5b0dba5c6d3cbdb8c384bc809b9 Mon Sep 17 00:00:00 2001 From: konrad Date: Sun, 1 Aug 2021 21:40:25 +0000 Subject: [PATCH] Ensure case insensitive search on postgres (#927) Reviewed-on: https://kolaente.dev/vikunja/api/pulls/927 Co-authored-by: konrad Co-committed-by: konrad --- pkg/db/helpers.go | 35 +++++++++++++++ pkg/models/label_task.go | 5 ++- pkg/models/label_task_test.go | 52 +++++++++++++++-------- pkg/models/link_sharing.go | 14 +++++- pkg/models/link_sharing_test.go | 15 +++++++ pkg/models/list.go | 5 ++- pkg/models/list_team.go | 4 +- pkg/models/list_team_test.go | 14 ++++++ pkg/models/list_test.go | 13 ++++++ pkg/models/list_users.go | 4 +- pkg/models/list_users_test.go | 68 ++++++++++++++++++------------ pkg/models/namespace.go | 6 ++- pkg/models/namespace_team.go | 8 ++-- pkg/models/namespace_team_test.go | 15 +++++++ pkg/models/namespace_test.go | 13 ++++++ pkg/models/namespace_users.go | 8 ++-- pkg/models/namespace_users_test.go | 68 ++++++++++++++++++------------ pkg/models/task_assignees.go | 10 ++++- pkg/models/task_comments.go | 14 +++--- pkg/models/task_comments_test.go | 12 ++++++ pkg/models/tasks.go | 11 +---- pkg/models/teams.go | 10 ++--- pkg/models/teams_test.go | 12 ++++++ 23 files changed, 308 insertions(+), 108 deletions(-) create mode 100644 pkg/db/helpers.go diff --git a/pkg/db/helpers.go b/pkg/db/helpers.go new file mode 100644 index 00000000..5e30c85d --- /dev/null +++ b/pkg/db/helpers.go @@ -0,0 +1,35 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package db + +import ( + "xorm.io/builder" + "xorm.io/xorm/schemas" +) + +// ILIKE returns an ILIKE query on postgres and a LIKE query on all other platforms. +// Postgres' is case sensitive by default. +// To work around this, we're using ILIKE as opposed to normal LIKE statements. +// ILIKE is preferred over LOWER(text) LIKE for performance reasons. +// See https://stackoverflow.com/q/7005302/10924593 +func ILIKE(column, search string) builder.Cond { + if Type() == schemas.POSTGRES { + return builder.Expr(column+" ILIKE ?", "%"+search+"%") + } + + return &builder.Like{column, "%" + search + "%"} +} diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index dcaad243..132c210d 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -21,9 +21,12 @@ import ( "strings" "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" + "xorm.io/builder" "xorm.io/xorm" ) @@ -200,7 +203,7 @@ func getLabelsByTaskIDs(s *xorm.Session, opts *LabelByTaskIDsOptions) (ls []*lab if len(ids) > 0 { cond = builder.And(cond, builder.In("labels.id", ids)) } else { - cond = builder.And(cond, &builder.Like{"labels.title", "%" + opts.Search + "%"}) + cond = builder.And(cond, db.ILIKE("labels.title", opts.Search)) } limit, start := getLimitFromPageIndex(opts.Page, opts.PerPage) diff --git a/pkg/models/label_task_test.go b/pkg/models/label_task_test.go index 7bd08213..925358bc 100644 --- a/pkg/models/label_task_test.go +++ b/pkg/models/label_task_test.go @@ -30,6 +30,24 @@ import ( ) func TestLabelTask_ReadAll(t *testing.T) { + label := Label{ + ID: 4, + Title: "Label #4 - visible via other task", + Created: testCreatedTime, + Updated: testUpdatedTime, + CreatedByID: 2, + CreatedBy: &user.User{ + ID: 2, + Username: "user2", + Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", + Issuer: "local", + EmailRemindersEnabled: true, + OverdueTasksRemindersEnabled: true, + Created: testCreatedTime, + Updated: testUpdatedTime, + }, + } + type fields struct { ID int64 TaskID int64 @@ -62,23 +80,7 @@ func TestLabelTask_ReadAll(t *testing.T) { wantLabels: []*labelWithTaskID{ { TaskID: 1, - Label: Label{ - ID: 4, - Title: "Label #4 - visible via other task", - Created: testCreatedTime, - Updated: testUpdatedTime, - CreatedByID: 2, - CreatedBy: &user.User{ - ID: 2, - Username: "user2", - Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - Issuer: "local", - EmailRemindersEnabled: true, - OverdueTasksRemindersEnabled: true, - Created: testCreatedTime, - Updated: testUpdatedTime, - }, - }, + Label: label, }, }, }, @@ -104,6 +106,22 @@ func TestLabelTask_ReadAll(t *testing.T) { wantErr: true, errType: IsErrTaskDoesNotExist, }, + { + name: "search", + fields: fields{ + TaskID: 1, + }, + args: args{ + a: &user.User{ID: 1}, + search: "VISIBLE", + }, + wantLabels: []*labelWithTaskID{ + { + TaskID: 1, + Label: label, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index 5f51a2fc..ff85fbd2 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -20,12 +20,15 @@ import ( "errors" "time" - "golang.org/x/crypto/bcrypt" + "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" + "code.vikunja.io/web" "github.com/golang-jwt/jwt" + "golang.org/x/crypto/bcrypt" + "xorm.io/builder" "xorm.io/xorm" ) @@ -206,7 +209,14 @@ func (share *LinkSharing) ReadAll(s *xorm.Session, a web.Auth, search string, pa var shares []*LinkSharing query := s. - Where("list_id = ? AND hash LIKE ?", share.ListID, "%"+search+"%") + Where(builder.And( + builder.Eq{"list_id": share.ListID}, + builder.Or( + db.ILIKE("hash", search), + db.ILIKE("name", search), + ), + )) + if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go index 1e8917de..8b3e6f2e 100644 --- a/pkg/models/link_sharing_test.go +++ b/pkg/models/link_sharing_test.go @@ -103,6 +103,21 @@ func TestLinkSharing_ReadAll(t *testing.T) { assert.Empty(t, sharing.Password) } }) + t.Run("search", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ListID: 1, + } + all, _, _, err := share.ReadAll(s, doer, "wITHPASS", 1, -1) + shares := all.([]*LinkSharing) + + assert.NoError(t, err) + assert.Len(t, shares, 1) + assert.Equal(t, int64(4), shares[0].ID) + }) } func TestLinkSharing_ReadOne(t *testing.T) { diff --git a/pkg/models/list.go b/pkg/models/list.go index 3c1ac7a0..2094538d 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -21,6 +21,8 @@ import ( "strings" "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/files" @@ -403,10 +405,9 @@ func getRawListsForUser(s *xorm.Session, opts *listOptions) (lists []*List, resu } } + filterCond = db.ILIKE("l.title", opts.search) if len(ids) > 0 { filterCond = builder.In("l.id", ids) - } else { - filterCond = &builder.Like{"l.title", "%" + opts.search + "%"} } // Gets all Lists where the user is either owner or in a team which has access to the list diff --git a/pkg/models/list_team.go b/pkg/models/list_team.go index 16b3faf1..4ec979fd 100644 --- a/pkg/models/list_team.go +++ b/pkg/models/list_team.go @@ -19,6 +19,8 @@ package models import ( "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/web" @@ -198,7 +200,7 @@ func (tl *TeamList) ReadAll(s *xorm.Session, a web.Auth, search string, page int Table("teams"). Join("INNER", "team_lists", "team_id = teams.id"). Where("team_lists.list_id = ?", tl.ListID). - Where("teams.name LIKE ?", "%"+search+"%") + Where(db.ILIKE("teams.name", search)) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/list_team_test.go b/pkg/models/list_team_test.go index 83cb6480..fa5da4a9 100644 --- a/pkg/models/list_team_test.go +++ b/pkg/models/list_team_test.go @@ -81,6 +81,20 @@ func TestTeamList_ReadAll(t *testing.T) { assert.True(t, IsErrNeedToHaveListReadAccess(err)) _ = s.Close() }) + t.Run("search", func(t *testing.T) { + tl := TeamList{ + ListID: 19, + } + db.LoadAndAssertFixtures(t) + s := db.NewSession() + teams, _, _, err := tl.ReadAll(s, u, "TEAM9", 1, 50) + assert.NoError(t, err) + assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) + ts := teams.([]*TeamWithRight) + assert.Len(t, ts, 1) + assert.Equal(t, int64(9), ts[0].ID) + _ = s.Close() + }) } func TestTeamList_Create(t *testing.T) { diff --git a/pkg/models/list_test.go b/pkg/models/list_test.go index 5a52b901..0fde472a 100644 --- a/pkg/models/list_test.go +++ b/pkg/models/list_test.go @@ -217,6 +217,19 @@ func TestList_ReadAll(t *testing.T) { assert.True(t, user.IsErrUserDoesNotExist(err)) _ = s.Close() }) + t.Run("search", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + u := &user.User{ID: 1} + list := List{} + lists3, _, _, err := list.ReadAll(s, u, "TEST10", 1, 50) + + assert.NoError(t, err) + ls := lists3.([]*List) + assert.Equal(t, 1, len(ls)) + assert.Equal(t, int64(10), ls[0].ID) + _ = s.Close() + }) } func TestList_ReadOne(t *testing.T) { diff --git a/pkg/models/list_users.go b/pkg/models/list_users.go index 51a11d1e..810894ee 100644 --- a/pkg/models/list_users.go +++ b/pkg/models/list_users.go @@ -19,6 +19,8 @@ package models import ( "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/user" @@ -204,7 +206,7 @@ func (lu *ListUser) ReadAll(s *xorm.Session, a web.Auth, search string, page int query := s. Join("INNER", "users_lists", "user_id = users.id"). Where("users_lists.list_id = ?", lu.ListID). - Where("users.username LIKE ?", "%"+search+"%") + Where(db.ILIKE("users.username", search)) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/list_users_test.go b/pkg/models/list_users_test.go index f8c3968a..3ee8722f 100644 --- a/pkg/models/list_users_test.go +++ b/pkg/models/list_users_test.go @@ -143,6 +143,33 @@ func TestListUser_Create(t *testing.T) { } func TestListUser_ReadAll(t *testing.T) { + user1Read := &UserWithRight{ + User: user.User{ + ID: 1, + Username: "user1", + Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", + Issuer: "local", + EmailRemindersEnabled: true, + OverdueTasksRemindersEnabled: true, + Created: testCreatedTime, + Updated: testUpdatedTime, + }, + Right: RightRead, + } + user2Read := &UserWithRight{ + User: user.User{ + ID: 2, + Username: "user2", + Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", + Issuer: "local", + EmailRemindersEnabled: true, + OverdueTasksRemindersEnabled: true, + Created: testCreatedTime, + Updated: testUpdatedTime, + }, + Right: RightRead, + } + type fields struct { ID int64 UserID int64 @@ -175,32 +202,8 @@ func TestListUser_ReadAll(t *testing.T) { a: &user.User{ID: 3}, }, want: []*UserWithRight{ - { - User: user.User{ - ID: 1, - Username: "user1", - Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - Issuer: "local", - EmailRemindersEnabled: true, - OverdueTasksRemindersEnabled: true, - Created: testCreatedTime, - Updated: testUpdatedTime, - }, - Right: RightRead, - }, - { - User: user.User{ - ID: 2, - Username: "user2", - Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - Issuer: "local", - EmailRemindersEnabled: true, - OverdueTasksRemindersEnabled: true, - Created: testCreatedTime, - Updated: testUpdatedTime, - }, - Right: RightRead, - }, + user1Read, + user2Read, }, }, { @@ -214,6 +217,19 @@ func TestListUser_ReadAll(t *testing.T) { wantErr: true, errType: IsErrNeedToHaveListReadAccess, }, + { + name: "Search", + fields: fields{ + ListID: 3, + }, + args: args{ + a: &user.User{ID: 3}, + search: "USER2", + }, + want: []*UserWithRight{ + user2Read, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index 969bd0e2..9ae333bc 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -22,10 +22,12 @@ import ( "strings" "time" - "code.vikunja.io/api/pkg/events" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/user" + "code.vikunja.io/web" "xorm.io/builder" "xorm.io/xorm" @@ -200,7 +202,7 @@ func makeNamespaceSlice(namespaces map[int64]*NamespaceWithLists, userMap map[in } func getNamespaceFilterCond(search string) (filterCond builder.Cond) { - filterCond = &builder.Like{"namespaces.title", "%" + search + "%"} + filterCond = db.ILIKE("namespaces.title", search) if search == "" { return diff --git a/pkg/models/namespace_team.go b/pkg/models/namespace_team.go index 72d41919..39291277 100644 --- a/pkg/models/namespace_team.go +++ b/pkg/models/namespace_team.go @@ -19,9 +19,11 @@ package models import ( "time" - "code.vikunja.io/api/pkg/events" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/web" + "xorm.io/xorm" ) @@ -178,14 +180,12 @@ func (tn *TeamNamespace) ReadAll(s *xorm.Session, a web.Auth, search string, pag // Get the teams all := []*TeamWithRight{} - limit, start := getLimitFromPageIndex(page, perPage) - query := s. Table("teams"). Join("INNER", "team_namespaces", "team_id = teams.id"). Where("team_namespaces.namespace_id = ?", tn.NamespaceID). - Where("teams.name LIKE ?", "%"+search+"%") + Where(db.ILIKE("teams.name", search)) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/namespace_team_test.go b/pkg/models/namespace_team_test.go index 86b39338..df9323f7 100644 --- a/pkg/models/namespace_team_test.go +++ b/pkg/models/namespace_team_test.go @@ -66,6 +66,21 @@ func TestTeamNamespace_ReadAll(t *testing.T) { assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) _ = s.Close() }) + t.Run("search", func(t *testing.T) { + tn := TeamNamespace{ + NamespaceID: 3, + } + db.LoadAndAssertFixtures(t) + s := db.NewSession() + teams, _, _, err := tn.ReadAll(s, u, "READ_only_on_list6", 1, 50) + assert.NoError(t, err) + assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) + ts := teams.([]*TeamWithRight) + assert.Len(t, ts, 1) + assert.Equal(t, int64(2), ts[0].ID) + + _ = s.Close() + }) } func TestTeamNamespace_Create(t *testing.T) { diff --git a/pkg/models/namespace_test.go b/pkg/models/namespace_test.go index 09f6f8a5..cbd4f28a 100644 --- a/pkg/models/namespace_test.go +++ b/pkg/models/namespace_test.go @@ -356,4 +356,17 @@ func TestNamespace_ReadAll(t *testing.T) { assert.NoError(t, err) assert.Nil(t, nn) }) + t.Run("search", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + n := &Namespace{} + nn, _, _, err := n.ReadAll(s, user6, "NamespACE7", 1, -1) + assert.NoError(t, err) + namespaces := nn.([]*NamespaceWithLists) + assert.NotNil(t, namespaces) + assert.Len(t, namespaces, 2) + assert.Equal(t, int64(7), namespaces[1].ID) + }) } diff --git a/pkg/models/namespace_users.go b/pkg/models/namespace_users.go index ef326248..eda9bfc3 100644 --- a/pkg/models/namespace_users.go +++ b/pkg/models/namespace_users.go @@ -19,10 +19,12 @@ package models import ( "time" - "code.vikunja.io/api/pkg/events" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" user2 "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" + "xorm.io/xorm" ) @@ -185,13 +187,11 @@ func (nu *NamespaceUser) ReadAll(s *xorm.Session, a web.Auth, search string, pag // Get all users all := []*UserWithRight{} - limit, start := getLimitFromPageIndex(page, perPage) - query := s. Join("INNER", "users_namespaces", "user_id = users.id"). Where("users_namespaces.namespace_id = ?", nu.NamespaceID). - Where("users.username LIKE ?", "%"+search+"%") + Where(db.ILIKE("users.username", search)) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/namespace_users_test.go b/pkg/models/namespace_users_test.go index 6f75140a..0cc288bd 100644 --- a/pkg/models/namespace_users_test.go +++ b/pkg/models/namespace_users_test.go @@ -142,6 +142,33 @@ func TestNamespaceUser_Create(t *testing.T) { } func TestNamespaceUser_ReadAll(t *testing.T) { + user1 := &UserWithRight{ + User: user.User{ + ID: 1, + Username: "user1", + Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", + Issuer: "local", + EmailRemindersEnabled: true, + OverdueTasksRemindersEnabled: true, + Created: testCreatedTime, + Updated: testUpdatedTime, + }, + Right: RightRead, + } + user2 := &UserWithRight{ + User: user.User{ + ID: 2, + Username: "user2", + Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", + Issuer: "local", + EmailRemindersEnabled: true, + OverdueTasksRemindersEnabled: true, + Created: testCreatedTime, + Updated: testUpdatedTime, + }, + Right: RightRead, + } + type fields struct { ID int64 UserID int64 @@ -174,32 +201,8 @@ func TestNamespaceUser_ReadAll(t *testing.T) { a: &user.User{ID: 3}, }, want: []*UserWithRight{ - { - User: user.User{ - ID: 1, - Username: "user1", - Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - Issuer: "local", - EmailRemindersEnabled: true, - OverdueTasksRemindersEnabled: true, - Created: testCreatedTime, - Updated: testUpdatedTime, - }, - Right: RightRead, - }, - { - User: user.User{ - ID: 2, - Username: "user2", - Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", - Issuer: "local", - EmailRemindersEnabled: true, - OverdueTasksRemindersEnabled: true, - Created: testCreatedTime, - Updated: testUpdatedTime, - }, - Right: RightRead, - }, + user1, + user2, }, }, { @@ -213,6 +216,19 @@ func TestNamespaceUser_ReadAll(t *testing.T) { wantErr: true, errType: IsErrNeedToHaveNamespaceReadAccess, }, + { + name: "Search", + fields: fields{ + NamespaceID: 3, + }, + args: args{ + a: &user.User{ID: 3}, + search: "usER2", + }, + want: []*UserWithRight{ + user2, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/models/task_assignees.go b/pkg/models/task_assignees.go index 6f6546ec..e2b4a515 100644 --- a/pkg/models/task_assignees.go +++ b/pkg/models/task_assignees.go @@ -19,6 +19,10 @@ package models import ( "time" + "code.vikunja.io/api/pkg/db" + + "xorm.io/builder" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/user" @@ -267,12 +271,14 @@ func (la *TaskAssginee) ReadAll(s *xorm.Session, a web.Auth, search string, page return nil, 0, 0, ErrGenericForbidden{} } limit, start := getLimitFromPageIndex(page, perPage) - var taskAssignees []*user.User query := s.Table("task_assignees"). Select("users.*"). Join("INNER", "users", "task_assignees.user_id = users.id"). - Where("task_id = ? AND users.username LIKE ?", la.TaskID, "%"+search+"%") + Where(builder.And( + builder.Eq{"task_id": la.TaskID}, + db.ILIKE("users.username", search), + )) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/task_comments.go b/pkg/models/task_comments.go index 5e0c34e5..264b8691 100644 --- a/pkg/models/task_comments.go +++ b/pkg/models/task_comments.go @@ -19,12 +19,14 @@ package models import ( "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" - - "xorm.io/xorm" - "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" + + "xorm.io/builder" + "xorm.io/xorm" ) // TaskComment represents a task comment @@ -214,10 +216,12 @@ func (tc *TaskComment) ReadAll(s *xorm.Session, auth web.Auth, search string, pa } limit, start := getLimitFromPageIndex(page, perPage) - comments := []*TaskComment{} query := s. - Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%"). + Where(builder.And( + builder.Eq{"task_id": tc.TaskID}, + db.ILIKE("comment", search), + )). Join("LEFT", "users", "users.id = task_comments.author_id") if limit > 0 { query = query.Limit(limit, start) diff --git a/pkg/models/task_comments_test.go b/pkg/models/task_comments_test.go index 077754de..ac9590d3 100644 --- a/pkg/models/task_comments_test.go +++ b/pkg/models/task_comments_test.go @@ -233,4 +233,16 @@ func TestTaskComment_ReadAll(t *testing.T) { } assert.True(t, foundComment) }) + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + tc := &TaskComment{TaskID: 35} + u := &user.User{ID: 1} + result, _, _, err := tc.ReadAll(s, u, "COMMENT 15", 0, -1) + resultComment := result.([]*TaskComment) + assert.NoError(t, err) + assert.Equal(t, int64(15), resultComment[0].ID) + }) } diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index 89957334..8474b865 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -369,16 +369,7 @@ func getRawTasksForLists(s *xorm.Session, lists []*List, a web.Auth, opts *taskO var where builder.Cond if opts.search != "" { - // Postgres' is case sensitive by default. - // To work around this, we're using ILIKE as opposed to normal LIKE statements. - // ILIKE is preferred over LOWER(text) LIKE for performance reasons. - // See https://stackoverflow.com/q/7005302/10924593 - // Seems okay to use that now, we may need to find a better solution overall in the future. - if config.DatabaseType.GetString() == "postgres" { - where = builder.Expr("title ILIKE ?", "%"+opts.search+"%") - } else { - where = &builder.Like{"title", "%" + opts.search + "%"} - } + where = db.ILIKE("title", opts.search) searchIndex := getTaskIndexFromSearchString(opts.search) if searchIndex > 0 { diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 491a0766..3ed3cd88 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -19,13 +19,14 @@ package models import ( "time" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" - - "xorm.io/xorm" - "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" + "xorm.io/builder" + "xorm.io/xorm" ) // Team holds a team object @@ -210,13 +211,12 @@ func (t *Team) ReadAll(s *xorm.Session, a web.Auth, search string, page int, per } limit, start := getLimitFromPageIndex(page, perPage) - all := []*Team{} query := s.Select("teams.*"). Table("teams"). Join("INNER", "team_members", "team_members.team_id = teams.id"). Where("team_members.user_id = ?", a.GetID()). - Where("teams.name LIKE ?", "%"+search+"%") + Where(db.ILIKE("teams.name", search)) if limit > 0 { query = query.Limit(limit, start) } diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index de25b63e..590dc4de 100644 --- a/pkg/models/teams_test.go +++ b/pkg/models/teams_test.go @@ -112,6 +112,18 @@ func TestTeam_ReadAll(t *testing.T) { ts := reflect.ValueOf(teams) assert.Equal(t, 8, ts.Len()) }) + t.Run("search", func(t *testing.T) { + s := db.NewSession() + defer s.Close() + + team := &Team{} + teams, _, _, err := team.ReadAll(s, doer, "READ_only_on_list6", 1, 50) + assert.NoError(t, err) + assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) + ts := teams.([]*Team) + assert.Len(t, ts, 1) + assert.Equal(t, int64(2), ts[0].ID) + }) } func TestTeam_Update(t *testing.T) {