From d3de658882031a3859b747de16977c6d316501b1 Mon Sep 17 00:00:00 2001 From: konrad Date: Fri, 9 Nov 2018 17:33:06 +0000 Subject: [PATCH] Implemented search for everything (#17) --- Featurecreep.md | 3 +-- REST-Tests/auth.http | 2 +- docs/concepts.md | 12 ++++++++++-- pkg/models/crudable.go | 2 +- pkg/models/list.go | 15 ++++++++------- pkg/models/list_read_test.go | 4 ++-- pkg/models/list_users_readall.go | 3 ++- pkg/models/namespace.go | 3 ++- pkg/models/namespace_test.go | 2 +- pkg/models/namespace_users_readall.go | 3 ++- pkg/models/team_list_readall.go | 3 ++- pkg/models/team_list_test.go | 8 ++++---- pkg/models/team_namespace_readall.go | 3 ++- pkg/models/team_namespace_test.go | 6 +++--- pkg/models/teams.go | 3 ++- pkg/models/teams_test.go | 2 +- pkg/routes/api/v1/caldav.go | 2 +- pkg/routes/crud/read_all.go | 5 ++++- 18 files changed, 49 insertions(+), 32 deletions(-) diff --git a/Featurecreep.md b/Featurecreep.md index 3a2ed6ba..bb24b48d 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -217,8 +217,7 @@ Teams sind global, d.h. Ein Team kann mehrere Namespaces verwalten. * [x] Ne extra funktion für list exists machen, damit die nicht immer über GetListByID gehen, um sql-abfragen zu sparen * [x] Rausfinden warum xorm teilweise beim einfügen IDs mit einfügen will -> Das schlägt dann wegen duplicate fehl * [x] Bei den Structs "AfterLoad" raus, das verbraucht bei Gruppenabfragen zu viele SQL-Abfragen -> Die sollen einfach die entsprechenden Read()-Methoden verwenden (Krassestes bsp. ist GET /namespaces mit so ca 50 Abfragen) -* [ ] Search endpoints /users?s=name und /teams?s=name, erstmal nur mit Namen suchen, später einstellbar auch mit email. - -> Search methode in den Handler einbauen und dann die Endpoints entsprechend anpassen integrieren kann (so nach dem Motto einfach nen Searchstring anhängen) +* [x] General search endpoints * [ ] Wir brauchen noch ne gute idee, wie man die listen kriegt, auf die man nur so Zugriff hat (ohne namespace) * Dazu am Besten nen pseudonamespace anlegen (id -1 oder so), der hat das dann alles * [ ] Validation der ankommenden structs, am besten mit https://github.com/go-validator/validator oder mit dem Ding von echo diff --git a/REST-Tests/auth.http b/REST-Tests/auth.http index 7da9d12d..acba0145 100644 --- a/REST-Tests/auth.http +++ b/REST-Tests/auth.http @@ -4,7 +4,7 @@ Content-Type: application/json { "username": "user", - "password": "1234" + "password": "12345" } > {% client.global.set("auth_token", response.body.token); %} diff --git a/docs/concepts.md b/docs/concepts.md index e2844605..9795faa0 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -14,7 +14,7 @@ The interface is defined as followed: type CRUDable interface { Create(*User) error ReadOne() error - ReadAll(*User, int) (interface{}, error) + ReadAll(string, *User, int) (interface{}, error) Update() error Delete() error } @@ -38,7 +38,7 @@ make an array of a set type (If you know a way to do this, don't hesitate to dro ### Pagination -When using the `ReadAll`-method, the second parameter contains the requested page. Your function should return only the number of results +When using the `ReadAll`-method, the third parameter contains the requested page. Your function should return only the number of results corresponding to that page. The number of items per page is definied in the config as `service.pagecount` (Get it with `viper.GetInt("service.pagecount")`). These can be calculated in combination with a helper function, `getLimitFromPageIndex(pageIndex)` which returns @@ -49,6 +49,14 @@ lists := []List{} err := x.Limit(getLimitFromPageIndex(pageIndex)).Find(&lists) ``` +### Search + +When using the `ReadAll`-method, the first parameter is a search term which should be used to search items of your struct. You define the critera. + +Users can then pass the `?s=something` parameter to the url to search. + +As the logic for "give me everything" and "give me everything where the name contains 'something'" is mostly the same, we made the decision to design the function like this, in order to keep the places with mostly the same logic as few as possible. Also just adding `?s=query` to the url one already knows and uses is a lot more convenient. + ## Rights This interface defines methods to check for rights on structs. They accept a `User` as parameter and usually return a `bool`. diff --git a/pkg/models/crudable.go b/pkg/models/crudable.go index 3f9e9bed..03030baf 100644 --- a/pkg/models/crudable.go +++ b/pkg/models/crudable.go @@ -4,7 +4,7 @@ package models type CRUDable interface { Create(*User) error ReadOne() error - ReadAll(*User, int) (interface{}, error) + ReadAll(string, *User, int) (interface{}, error) Update() error Delete() error } diff --git a/pkg/models/list.go b/pkg/models/list.go index fe9c4bf7..74437449 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -27,8 +27,8 @@ func GetListsByNamespaceID(nID int64) (lists []*List, err error) { } // ReadAll gets all lists a user has access to -func (l *List) ReadAll(u *User, page int) (interface{}, error) { - lists, err := getRawListsForUser(u, page) +func (l *List) ReadAll(search string, u *User, page int) (interface{}, error) { + lists, err := getRawListsForUser(search, u, page) if err != nil { return nil, err } @@ -80,7 +80,7 @@ func (l *List) GetSimpleByID() (err error) { } // Gets the lists only, without any tasks or so -func getRawListsForUser(u *User, page int) (lists []*List, err error) { +func getRawListsForUser(search string, u *User, page int) (lists []*List, err error) { fullUser, err := GetUserByID(u.ID) if err != nil { return lists, err @@ -105,6 +105,7 @@ func getRawListsForUser(u *User, page int) (lists []*List, err error) { Or("un.user_id = ?", fullUser.ID). GroupBy("l.id"). Limit(getLimitFromPageIndex(page)). + Where("l.title LIKE ?", "%"+search+"%"). Find(&lists) return lists, err @@ -161,14 +162,14 @@ type ListTasksDummy struct { } // ReadAll gets all tasks for a user -func (lt *ListTasksDummy) ReadAll(u *User, page int) (interface{}, error) { - return GetTasksByUser(u, page) +func (lt *ListTasksDummy) ReadAll(search string, u *User, page int) (interface{}, error) { + return GetTasksByUser(search, u, page) } //GetTasksByUser returns all tasks for a user -func GetTasksByUser(u *User, page int) (tasks []*ListTask, err error) { +func GetTasksByUser(search string, u *User, page int) (tasks []*ListTask, err error) { // Get all lists - lists, err := getRawListsForUser(u, page) + lists, err := getRawListsForUser(search, u, page) if err != nil { return nil, err } diff --git a/pkg/models/list_read_test.go b/pkg/models/list_read_test.go index adf24e98..67851294 100644 --- a/pkg/models/list_read_test.go +++ b/pkg/models/list_read_test.go @@ -20,14 +20,14 @@ func TestList_ReadAll(t *testing.T) { assert.NoError(t, err) lists2 := List{} - lists3, err := lists2.ReadAll(&u, 1) + lists3, err := lists2.ReadAll("", &u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(lists3).Kind(), reflect.Slice) s := reflect.ValueOf(lists3) assert.Equal(t, s.Len(), 1) // Try getting lists for a nonexistant user - _, err = lists2.ReadAll(&User{ID: 984234}, 1) + _, err = lists2.ReadAll("", &User{ID: 984234}, 1) assert.Error(t, err) assert.True(t, IsErrUserDoesNotExist(err)) } diff --git a/pkg/models/list_users_readall.go b/pkg/models/list_users_readall.go index 73b85840..94be903a 100644 --- a/pkg/models/list_users_readall.go +++ b/pkg/models/list_users_readall.go @@ -1,7 +1,7 @@ package models // ReadAll gets all users who have access to a list -func (ul *ListUser) ReadAll(u *User, page int) (interface{}, error) { +func (ul *ListUser) ReadAll(search string, u *User, page int) (interface{}, error) { // Check if the user has access to the list l := &List{ID: ul.ListID} if err := l.GetSimpleByID(); err != nil { @@ -17,6 +17,7 @@ func (ul *ListUser) ReadAll(u *User, page int) (interface{}, error) { Join("INNER", "users_list", "user_id = users.id"). Where("users_list.list_id = ?", ul.ListID). Limit(getLimitFromPageIndex(page)). + Where("users.username LIKE ?", "%"+search+"%"). Find(&all) return all, err diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index 94003f74..ce3b794b 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -53,7 +53,7 @@ func (n *Namespace) ReadOne() (err error) { } // ReadAll gets all namespaces a user has access to -func (n *Namespace) ReadAll(doer *User, page int) (interface{}, error) { +func (n *Namespace) ReadAll(search string, doer *User, page int) (interface{}, error) { type namespaceWithLists struct { Namespace `xorm:"extends"` @@ -72,6 +72,7 @@ func (n *Namespace) ReadAll(doer *User, page int) (interface{}, error) { Or("users_namespace.user_id = ?", doer.ID). GroupBy("namespaces.id"). Limit(getLimitFromPageIndex(page)). + Where("namespaces.name LIKE ?", "%"+search+"%"). Find(&all) if err != nil { diff --git a/pkg/models/namespace_test.go b/pkg/models/namespace_test.go index 05ddd4b7..33e11a95 100644 --- a/pkg/models/namespace_test.go +++ b/pkg/models/namespace_test.go @@ -85,7 +85,7 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Get all namespaces of a user - nsps, err := n.ReadAll(&doer, 1) + nsps, err := n.ReadAll("", &doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(nsps).Kind(), reflect.Slice) s := reflect.ValueOf(nsps) diff --git a/pkg/models/namespace_users_readall.go b/pkg/models/namespace_users_readall.go index b2939f04..3dde7784 100644 --- a/pkg/models/namespace_users_readall.go +++ b/pkg/models/namespace_users_readall.go @@ -1,7 +1,7 @@ package models // ReadAll gets all users who have access to a namespace -func (un *NamespaceUser) ReadAll(u *User, page int) (interface{}, error) { +func (un *NamespaceUser) ReadAll(search string, u *User, page int) (interface{}, error) { // Check if the user has access to the namespace l, err := GetNamespaceByID(un.NamespaceID) if err != nil { @@ -17,6 +17,7 @@ func (un *NamespaceUser) ReadAll(u *User, page int) (interface{}, error) { Join("INNER", "users_namespace", "user_id = users.id"). Where("users_namespace.namespace_id = ?", un.NamespaceID). Limit(getLimitFromPageIndex(page)). + Where("users.username LIKE ?", "%"+search+"%"). Find(&all) return all, err diff --git a/pkg/models/team_list_readall.go b/pkg/models/team_list_readall.go index 4e3a3a05..05a29b65 100644 --- a/pkg/models/team_list_readall.go +++ b/pkg/models/team_list_readall.go @@ -1,7 +1,7 @@ package models // ReadAll implements the method to read all teams of a list -func (tl *TeamList) ReadAll(u *User, page int) (interface{}, error) { +func (tl *TeamList) ReadAll(search string, u *User, page int) (interface{}, error) { // Check if the user can read the namespace l := &List{ID: tl.ListID} if err := l.GetSimpleByID(); err != nil { @@ -18,6 +18,7 @@ func (tl *TeamList) ReadAll(u *User, page int) (interface{}, error) { Join("INNER", "team_list", "team_id = teams.id"). Where("team_list.list_id = ?", tl.ListID). Limit(getLimitFromPageIndex(page)). + Where("teams.name LIKE ?", "%"+search+"%"). Find(&all) return all, err diff --git a/pkg/models/team_list_test.go b/pkg/models/team_list_test.go index 76b7b824..9e647e8e 100644 --- a/pkg/models/team_list_test.go +++ b/pkg/models/team_list_test.go @@ -50,27 +50,27 @@ func TestTeamList(t *testing.T) { assert.True(t, IsErrListDoesNotExist(err)) // Test Read all - teams, err := tl.ReadAll(&u, 1) + teams, err := tl.ReadAll("", &u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Test Read all for nonexistant list - _, err = tl4.ReadAll(&u, 1) + _, err = tl4.ReadAll("", &u, 1) assert.Error(t, err) assert.True(t, IsErrListDoesNotExist(err)) // Test Read all for a list where the user is owner of the namespace this list belongs to tl5 := tl tl5.ListID = 2 - _, err = tl5.ReadAll(&u, 1) + _, err = tl5.ReadAll("", &u, 1) assert.NoError(t, err) // Test read all for a list where the user not has access tl6 := tl tl6.ListID = 3 - _, err = tl6.ReadAll(&u, 1) + _, err = tl6.ReadAll("", &u, 1) assert.Error(t, err) assert.True(t, IsErrNeedToHaveListReadAccess(err)) diff --git a/pkg/models/team_namespace_readall.go b/pkg/models/team_namespace_readall.go index cef5d5bb..0dbe8270 100644 --- a/pkg/models/team_namespace_readall.go +++ b/pkg/models/team_namespace_readall.go @@ -1,7 +1,7 @@ package models // ReadAll implements the method to read all teams of a namespace -func (tn *TeamNamespace) ReadAll(user *User, page int) (interface{}, error) { +func (tn *TeamNamespace) ReadAll(search string, user *User, page int) (interface{}, error) { // Check if the user can read the namespace n, err := GetNamespaceByID(tn.NamespaceID) if err != nil { @@ -18,6 +18,7 @@ func (tn *TeamNamespace) ReadAll(user *User, page int) (interface{}, error) { Join("INNER", "team_namespaces", "team_id = teams.id"). Where("team_namespaces.namespace_id = ?", tn.NamespaceID). Limit(getLimitFromPageIndex(page)). + Where("teams.name LIKE ?", "%"+search+"%"). Find(&all) return all, err diff --git a/pkg/models/team_namespace_test.go b/pkg/models/team_namespace_test.go index 6773eba5..307b1e72 100644 --- a/pkg/models/team_namespace_test.go +++ b/pkg/models/team_namespace_test.go @@ -49,20 +49,20 @@ func TestTeamNamespace(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check readall - teams, err := tn.ReadAll(&dummyuser, 1) + teams, err := tn.ReadAll("", &dummyuser, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Check readall for a nonexistant namespace - _, err = tn4.ReadAll(&dummyuser, 1) + _, err = tn4.ReadAll("", &dummyuser, 1) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check with no right to read the namespace nouser := &User{ID: 393} - _, err = tn.ReadAll(nouser, 1) + _, err = tn.ReadAll("", nouser, 1) assert.Error(t, err) assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index c6228f84..71d01534 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -84,13 +84,14 @@ func (t *Team) ReadOne() (err error) { } // ReadAll gets all teams the user is part of -func (t *Team) ReadAll(user *User, page int) (teams interface{}, err error) { +func (t *Team) ReadAll(search string, user *User, page int) (teams interface{}, err error) { all := []*Team{} err = x.Select("teams.*"). Table("teams"). Join("INNER", "team_members", "team_members.team_id = teams.id"). Where("team_members.user_id = ?", user.ID). Limit(getLimitFromPageIndex(page)). + Where("teams.name LIKE ?", "%"+search+"%"). Find(&all) return all, err diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index 4f5cc76d..61dceb3b 100644 --- a/pkg/models/teams_test.go +++ b/pkg/models/teams_test.go @@ -32,7 +32,7 @@ func TestTeam_Create(t *testing.T) { assert.True(t, dummyteam.CanRead(&doer)) // Get all teams the user is part of - ts, err := tm.ReadAll(&doer, 1) + ts, err := tm.ReadAll("", &doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(ts).Kind(), reflect.Slice) s := reflect.ValueOf(ts) diff --git a/pkg/routes/api/v1/caldav.go b/pkg/routes/api/v1/caldav.go index cff68827..cdb691f0 100644 --- a/pkg/routes/api/v1/caldav.go +++ b/pkg/routes/api/v1/caldav.go @@ -39,7 +39,7 @@ func Caldav(c echo.Context) error { } // Get all tasks for that user - tasks, err := models.GetTasksByUser(&u, -1) + tasks, err := models.GetTasksByUser("", &u, -1) if err != nil { return crud.HandleHTTPError(err) } diff --git a/pkg/routes/crud/read_all.go b/pkg/routes/crud/read_all.go index b47808ae..b2ef6b57 100644 --- a/pkg/routes/crud/read_all.go +++ b/pkg/routes/crud/read_all.go @@ -37,7 +37,10 @@ func (c *WebHandler) ReadAllWeb(ctx echo.Context) error { return echo.NewHTTPError(http.StatusBadRequest, "Bad page requested.") } - lists, err := currentStruct.ReadAll(¤tUser, pageNumber) + // Search + search := ctx.QueryParam("s") + + lists, err := currentStruct.ReadAll(search, ¤tUser, pageNumber) if err != nil { return HandleHTTPError(err) }