]> source.dussan.org Git - gitea.git/commitdiff
Hide unactive on explore users and some refactors (#2741)
authorLunny Xiao <xiaolunwen@gmail.com>
Tue, 24 Oct 2017 17:36:19 +0000 (01:36 +0800)
committerLauris BH <lauris@nix.lv>
Tue, 24 Oct 2017 17:36:19 +0000 (20:36 +0300)
* hide unactive on explore users and some refactors

* fix test for removed Organizations

* fix test for removed Organizations

* fix imports

* fix logic bug

* refactor the toConds

* Rename TestOrganizations to TestSearchUsers and add tests for users

* fix other tests

* fix other tests

* fix watchers tests

* fix comments and remove unused code

15 files changed:
models/fixtures/issue_watch.yml
models/fixtures/user.yml
models/fixtures/watch.yml
models/issue_watch_test.go
models/notification_test.go
models/org.go
models/org_test.go
models/repo_watch_test.go
models/user.go
models/user_test.go
modules/util/util.go
routers/admin/orgs.go
routers/admin/users.go
routers/api/v1/user/user.go
routers/home.go

index 596662d20c14ad6a8e3e1b56fa480edb29c4b7d0..75351eb17f883d3cb12371dc08dbad4638e0baf0 100644 (file)
@@ -1,6 +1,6 @@
 -
   id: 1
-  user_id: 1
+  user_id: 9
   issue_id: 1
   is_watching: true
   created_unix: 946684800
index 73ec1c85935c7b91ce68f7cf80584ec617a544f0..1e0625598892c65819e6c30f7e42a152aa0da54f 100644 (file)
@@ -13,6 +13,7 @@
   avatar: avatar1
   avatar_email: user1@example.com
   num_repos: 0
+  is_active: true
 
 -
   id: 2
@@ -62,6 +63,7 @@
   avatar_email: user4@example.com
   num_repos: 0
   num_following: 1
+  is_active: true
 
 -
   id: 5
index 2ff888b7c78ffa92c41af63c08208abe37d326b1..5cd3b55fc43e1bbf84c4c58c8cc0ac7a68dc03c9 100644 (file)
@@ -10,5 +10,5 @@
 
 -
   id: 3
-  user_id: 10
+  user_id: 9
   repo_id: 1
index a4819b22240316008ca0b981a6733708814fd39a..ce0d2045ca6883275f7ad546b8ebb798fdbbd52b 100644 (file)
@@ -25,7 +25,7 @@ func TestCreateOrUpdateIssueWatch(t *testing.T) {
 func TestGetIssueWatch(t *testing.T) {
        assert.NoError(t, PrepareTestDatabase())
 
-       _, exists, err := GetIssueWatch(1, 1)
+       _, exists, err := GetIssueWatch(9, 1)
        assert.Equal(t, true, exists)
        assert.NoError(t, err)
 
index c1b7d50529833fda35c5abe56fefc6a88979b7b8..07d9ee764c372e1a3871310b463afe67bc4093fc 100644 (file)
@@ -16,10 +16,13 @@ func TestCreateOrUpdateIssueNotifications(t *testing.T) {
 
        assert.NoError(t, CreateOrUpdateIssueNotifications(issue, 2))
 
-       // Two watchers are inactive, thus only notification for user 10 is created
-       notf := AssertExistsAndLoadBean(t, &Notification{UserID: 10, IssueID: issue.ID}).(*Notification)
+       // User 9 is inactive, thus notifications for user 1 and 4 are created
+       notf := AssertExistsAndLoadBean(t, &Notification{UserID: 1, IssueID: issue.ID}).(*Notification)
        assert.Equal(t, NotificationStatusUnread, notf.Status)
        CheckConsistencyFor(t, &Issue{ID: issue.ID})
+
+       notf = AssertExistsAndLoadBean(t, &Notification{UserID: 4, IssueID: issue.ID}).(*Notification)
+       assert.Equal(t, NotificationStatusUnread, notf.Status)
 }
 
 func TestNotificationsForUser(t *testing.T) {
index 3c16572603fb6f7d3c6052c45ef94602c951483a..4a4fcb4add78dcae336ccb86eabb61d5328dc573 100644 (file)
@@ -201,23 +201,6 @@ func CountOrganizations() int64 {
        return count
 }
 
-// Organizations returns number of organizations in given page.
-func Organizations(opts *SearchUserOptions) ([]*User, error) {
-       orgs := make([]*User, 0, opts.PageSize)
-
-       if len(opts.OrderBy) == 0 {
-               opts.OrderBy = "name ASC"
-       }
-
-       sess := x.
-               Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
-               Where("type=1")
-
-       return orgs, sess.
-               OrderBy(opts.OrderBy).
-               Find(&orgs)
-}
-
 // DeleteOrganization completely and permanently deletes everything of organization.
 func DeleteOrganization(org *User) (err error) {
        sess := x.NewSession()
index c7bdb8b5d3dbbd2eed266abe631375306471e79e..8f59af0744a813459ca3f127a94e75a42a8561c4 100644 (file)
@@ -237,27 +237,6 @@ func TestCountOrganizations(t *testing.T) {
        assert.Equal(t, expected, CountOrganizations())
 }
 
-func TestOrganizations(t *testing.T) {
-       assert.NoError(t, PrepareTestDatabase())
-       testSuccess := func(opts *SearchUserOptions, expectedOrgIDs []int64) {
-               orgs, err := Organizations(opts)
-               assert.NoError(t, err)
-               if assert.Len(t, orgs, len(expectedOrgIDs)) {
-                       for i, expectedOrgID := range expectedOrgIDs {
-                               assert.EqualValues(t, expectedOrgID, orgs[i].ID)
-                       }
-               }
-       }
-       testSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1, PageSize: 2},
-               []int64{3, 6})
-
-       testSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 2, PageSize: 2},
-               []int64{7, 17})
-
-       testSuccess(&SearchUserOptions{Page: 3, PageSize: 2},
-               []int64{})
-}
-
 func TestDeleteOrganization(t *testing.T) {
        assert.NoError(t, PrepareTestDatabase())
        org := AssertExistsAndLoadBean(t, &User{ID: 6}).(*User)
index 1277b156c5c2a429dbd40060f9dc2aa0635429ab..852f09f1c7e0cc0e2e24f67cef3f00080276a3bb 100644 (file)
@@ -40,8 +40,8 @@ func TestGetWatchers(t *testing.T) {
        repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
        watches, err := GetWatchers(repo.ID)
        assert.NoError(t, err)
-       // Two watchers are inactive, thus minus 2
-       assert.Len(t, watches, repo.NumWatches-2)
+       // One watchers are inactive, thus minus 1
+       assert.Len(t, watches, repo.NumWatches-1)
        for _, watch := range watches {
                assert.EqualValues(t, repo.ID, watch.RepoID)
        }
@@ -62,7 +62,7 @@ func TestRepository_GetWatchers(t *testing.T) {
                AssertExistsAndLoadBean(t, &Watch{UserID: watcher.ID, RepoID: repo.ID})
        }
 
-       repo = AssertExistsAndLoadBean(t, &Repository{ID: 10}).(*Repository)
+       repo = AssertExistsAndLoadBean(t, &Repository{ID: 9}).(*Repository)
        watchers, err = repo.GetWatchers(1)
        assert.NoError(t, err)
        assert.Len(t, watchers, 0)
@@ -78,7 +78,7 @@ func TestNotifyWatchers(t *testing.T) {
        }
        assert.NoError(t, NotifyWatchers(action))
 
-       // Two watchers are inactive, thus action is only created for user 8, 10
+       // One watchers are inactive, thus action is only created for user 8, 1, 4
        AssertExistsAndLoadBean(t, &Action{
                ActUserID: action.ActUserID,
                UserID:    8,
@@ -87,7 +87,13 @@ func TestNotifyWatchers(t *testing.T) {
        })
        AssertExistsAndLoadBean(t, &Action{
                ActUserID: action.ActUserID,
-               UserID:    10,
+               UserID:    1,
+               RepoID:    action.RepoID,
+               OpType:    action.OpType,
+       })
+       AssertExistsAndLoadBean(t, &Action{
+               ActUserID: action.ActUserID,
+               UserID:    4,
                RepoID:    action.RepoID,
                OpType:    action.OpType,
        })
index 835a775ebcba930cc1f695887181b44621946424..337c39efe49e0d92d1204fd549703d4e75ffdc48 100644 (file)
@@ -36,6 +36,7 @@ import (
        "code.gitea.io/gitea/modules/base"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/util"
 )
 
 // UserType defines the user type
@@ -729,22 +730,6 @@ func CountUsers() int64 {
        return countUsers(x)
 }
 
-// Users returns number of users in given page.
-func Users(opts *SearchUserOptions) ([]*User, error) {
-       if len(opts.OrderBy) == 0 {
-               opts.OrderBy = "name ASC"
-       }
-
-       users := make([]*User, 0, opts.PageSize)
-       sess := x.
-               Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
-               Where("type=0")
-
-       return users, sess.
-               OrderBy(opts.OrderBy).
-               Find(&users)
-}
-
 // get user by verify code
 func getVerifyUser(code string) (user *User) {
        if len(code) <= base.TimeLimitCodeLength {
@@ -1284,45 +1269,50 @@ type SearchUserOptions struct {
        OrderBy  string
        Page     int
        PageSize int // Can be smaller than or equal to setting.UI.ExplorePagingNum
+       IsActive util.OptionalBool
 }
 
-// SearchUserByName takes keyword and part of user name to search,
-// it returns results in given range and number of total results.
-func SearchUserByName(opts *SearchUserOptions) (users []*User, _ int64, _ error) {
-       if len(opts.Keyword) == 0 {
-               return users, 0, nil
+func (opts *SearchUserOptions) toConds() builder.Cond {
+       var cond builder.Cond = builder.Eq{"type": opts.Type}
+       if len(opts.Keyword) > 0 {
+               lowerKeyword := strings.ToLower(opts.Keyword)
+               cond = cond.And(builder.Or(
+                       builder.Like{"lower_name", lowerKeyword},
+                       builder.Like{"LOWER(full_name)", lowerKeyword},
+               ))
        }
-       opts.Keyword = strings.ToLower(opts.Keyword)
 
-       if opts.PageSize <= 0 || opts.PageSize > setting.UI.ExplorePagingNum {
-               opts.PageSize = setting.UI.ExplorePagingNum
-       }
-       if opts.Page <= 0 {
-               opts.Page = 1
+       if !opts.IsActive.IsNone() {
+               cond = cond.And(builder.Eq{"is_active": opts.IsActive.IsTrue()})
        }
 
-       users = make([]*User, 0, opts.PageSize)
-
-       // Append conditions
-       cond := builder.And(
-               builder.Eq{"type": opts.Type},
-               builder.Or(
-                       builder.Like{"lower_name", opts.Keyword},
-                       builder.Like{"LOWER(full_name)", opts.Keyword},
-               ),
-       )
+       return cond
+}
 
+// SearchUsers takes options i.e. keyword and part of user name to search,
+// it returns results in given range and number of total results.
+func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) {
+       cond := opts.toConds()
        count, err := x.Where(cond).Count(new(User))
        if err != nil {
                return nil, 0, fmt.Errorf("Count: %v", err)
        }
 
-       sess := x.Where(cond).
-               Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
-       if len(opts.OrderBy) > 0 {
-               sess.OrderBy(opts.OrderBy)
+       if opts.PageSize <= 0 || opts.PageSize > setting.UI.ExplorePagingNum {
+               opts.PageSize = setting.UI.ExplorePagingNum
+       }
+       if opts.Page <= 0 {
+               opts.Page = 1
        }
-       return users, count, sess.Find(&users)
+       if len(opts.OrderBy) == 0 {
+               opts.OrderBy = "name ASC"
+       }
+
+       users = make([]*User, 0, opts.PageSize)
+       return users, count, x.Where(cond).
+               Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
+               OrderBy(opts.OrderBy).
+               Find(&users)
 }
 
 // GetStarredRepos returns the repos starred by a particular user
index a16f979530e9436323bd3a84c7770794b91aa22e..7ac9ebb0f5fb4601c3f8b172a09ad548fe40f578 100644 (file)
@@ -8,6 +8,7 @@ import (
        "testing"
 
        "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/util"
 
        "github.com/stretchr/testify/assert"
 )
@@ -38,6 +39,56 @@ func TestCanCreateOrganization(t *testing.T) {
        assert.False(t, user.CanCreateOrganization())
 }
 
+func TestSearchUsers(t *testing.T) {
+       assert.NoError(t, PrepareTestDatabase())
+       testSuccess := func(opts *SearchUserOptions, expectedUserOrOrgIDs []int64) {
+               users, _, err := SearchUsers(opts)
+               assert.NoError(t, err)
+               if assert.Len(t, users, len(expectedUserOrOrgIDs)) {
+                       for i, expectedID := range expectedUserOrOrgIDs {
+                               assert.EqualValues(t, expectedID, users[i].ID)
+                       }
+               }
+       }
+
+       // test orgs
+       testOrgSuccess := func(opts *SearchUserOptions, expectedOrgIDs []int64) {
+               opts.Type = UserTypeOrganization
+               testSuccess(opts, expectedOrgIDs)
+       }
+
+       testOrgSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1, PageSize: 2},
+               []int64{3, 6})
+
+       testOrgSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 2, PageSize: 2},
+               []int64{7, 17})
+
+       testOrgSuccess(&SearchUserOptions{Page: 3, PageSize: 2},
+               []int64{})
+
+       // test users
+       testUserSuccess := func(opts *SearchUserOptions, expectedUserIDs []int64) {
+               opts.Type = UserTypeIndividual
+               testSuccess(opts, expectedUserIDs)
+       }
+
+       testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1},
+               []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18})
+
+       testUserSuccess(&SearchUserOptions{Page: 1, IsActive: util.OptionalBoolFalse},
+               []int64{9})
+
+       testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 1, IsActive: util.OptionalBoolTrue},
+               []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18})
+
+       testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", Page: 1, IsActive: util.OptionalBoolTrue},
+               []int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
+
+       // order by name asc default
+       testUserSuccess(&SearchUserOptions{Keyword: "user1", Page: 1, IsActive: util.OptionalBoolTrue},
+               []int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
+}
+
 func TestDeleteUser(t *testing.T) {
        test := func(userID int64) {
                assert.NoError(t, PrepareTestDatabase())
index 4859965388b5b8dffcfb3e68e65fecba54cebf37..104c80f524fdc6a263979daf296dbd36caa594db 100644 (file)
@@ -16,6 +16,21 @@ const (
        OptionalBoolFalse
 )
 
+// IsTrue return true if equal to OptionalBoolTrue
+func (o OptionalBool) IsTrue() bool {
+       return o == OptionalBoolTrue
+}
+
+// IsFalse return true if equal to OptionalBoolFalse
+func (o OptionalBool) IsFalse() bool {
+       return o == OptionalBoolFalse
+}
+
+// IsNone return true if equal to OptionalBoolNone
+func (o OptionalBool) IsNone() bool {
+       return o == OptionalBoolNone
+}
+
 // OptionalBoolOf get the corresponding OptionalBool of a bool
 func OptionalBoolOf(b bool) OptionalBool {
        if b {
index eeb9002bc2e14ec37dec5d9ec23ea70752fe2581..c556d3fcf558243a6af353fb8e421aabce707fb3 100644 (file)
@@ -22,11 +22,8 @@ func Organizations(ctx *context.Context) {
        ctx.Data["PageIsAdmin"] = true
        ctx.Data["PageIsAdminOrganizations"] = true
 
-       routers.RenderUserSearch(ctx, &routers.UserSearchOptions{
+       routers.RenderUserSearch(ctx, &models.SearchUserOptions{
                Type:     models.UserTypeOrganization,
-               Counter:  models.CountOrganizations,
-               Ranger:   models.Organizations,
                PageSize: setting.UI.Admin.OrgPagingNum,
-               TplName:  tplOrgs,
-       })
+       }, tplOrgs)
 }
index d480029143fa6d5e668b1902fb32d525166deae6..c22accc2029d0a4ca7482ae0b8532bacc448fcdd 100644 (file)
@@ -30,13 +30,10 @@ func Users(ctx *context.Context) {
        ctx.Data["PageIsAdmin"] = true
        ctx.Data["PageIsAdminUsers"] = true
 
-       routers.RenderUserSearch(ctx, &routers.UserSearchOptions{
+       routers.RenderUserSearch(ctx, &models.SearchUserOptions{
                Type:     models.UserTypeIndividual,
-               Counter:  models.CountUsers,
-               Ranger:   models.Users,
                PageSize: setting.UI.Admin.UserPagingNum,
-               TplName:  tplUsers,
-       })
+       }, tplUsers)
 }
 
 // NewUser render adding a new user page
index 0ab6eaf44cf6fbeba3d121a375d082133faf35fb..5a59fd7ca92a34db50d77d030b7bde728940d94e 100644 (file)
@@ -35,7 +35,7 @@ func Search(ctx *context.APIContext) {
                opts.PageSize = 10
        }
 
-       users, _, err := models.SearchUserByName(opts)
+       users, _, err := models.SearchUsers(opts)
        if err != nil {
                ctx.JSON(500, map[string]interface{}{
                        "ok":    false,
index 768eb220776e20ca7415c3897c60db8296c527a4..d653d1e8432e273a682200da9e103e7c7542c7c7 100644 (file)
@@ -12,6 +12,7 @@ import (
        "code.gitea.io/gitea/modules/base"
        "code.gitea.io/gitea/modules/context"
        "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/util"
        "code.gitea.io/gitea/routers/user"
 
        "github.com/Unknwon/paginater"
@@ -147,20 +148,11 @@ func ExploreRepos(ctx *context.Context) {
        })
 }
 
-// UserSearchOptions options when render search user page
-type UserSearchOptions struct {
-       Type     models.UserType
-       Counter  func() int64
-       Ranger   func(*models.SearchUserOptions) ([]*models.User, error)
-       PageSize int
-       TplName  base.TplName
-}
-
 // RenderUserSearch render user search page
-func RenderUserSearch(ctx *context.Context, opts *UserSearchOptions) {
-       page := ctx.QueryInt("page")
-       if page <= 1 {
-               page = 1
+func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplName base.TplName) {
+       opts.Page = ctx.QueryInt("page")
+       if opts.Page <= 1 {
+               opts.Page = 1
        }
 
        var (
@@ -189,40 +181,22 @@ func RenderUserSearch(ctx *context.Context, opts *UserSearchOptions) {
                orderBy = "name ASC"
        }
 
-       keyword := strings.Trim(ctx.Query("q"), " ")
-       if len(keyword) == 0 {
-               users, err = opts.Ranger(&models.SearchUserOptions{
-                       OrderBy:  orderBy,
-                       Page:     page,
-                       PageSize: opts.PageSize,
-               })
+       opts.Keyword = strings.Trim(ctx.Query("q"), " ")
+       opts.OrderBy = orderBy
+       if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) {
+               users, count, err = models.SearchUsers(opts)
                if err != nil {
-                       ctx.Handle(500, "opts.Ranger", err)
+                       ctx.Handle(500, "SearchUsers", err)
                        return
                }
-               count = opts.Counter()
-       } else {
-               if isKeywordValid(keyword) {
-                       users, count, err = models.SearchUserByName(&models.SearchUserOptions{
-                               Keyword:  keyword,
-                               Type:     opts.Type,
-                               OrderBy:  orderBy,
-                               Page:     page,
-                               PageSize: opts.PageSize,
-                       })
-                       if err != nil {
-                               ctx.Handle(500, "SearchUserByName", err)
-                               return
-                       }
-               }
        }
-       ctx.Data["Keyword"] = keyword
+       ctx.Data["Keyword"] = opts.Keyword
        ctx.Data["Total"] = count
-       ctx.Data["Page"] = paginater.New(int(count), opts.PageSize, page, 5)
+       ctx.Data["Page"] = paginater.New(int(count), opts.PageSize, opts.Page, 5)
        ctx.Data["Users"] = users
        ctx.Data["ShowUserEmail"] = setting.UI.ShowUserEmail
 
-       ctx.HTML(200, opts.TplName)
+       ctx.HTML(200, tplName)
 }
 
 // ExploreUsers render explore users page
@@ -231,13 +205,11 @@ func ExploreUsers(ctx *context.Context) {
        ctx.Data["PageIsExplore"] = true
        ctx.Data["PageIsExploreUsers"] = true
 
-       RenderUserSearch(ctx, &UserSearchOptions{
+       RenderUserSearch(ctx, &models.SearchUserOptions{
                Type:     models.UserTypeIndividual,
-               Counter:  models.CountUsers,
-               Ranger:   models.Users,
                PageSize: setting.UI.ExplorePagingNum,
-               TplName:  tplExploreUsers,
-       })
+               IsActive: util.OptionalBoolTrue,
+       }, tplExploreUsers)
 }
 
 // ExploreOrganizations render explore organizations page
@@ -246,13 +218,10 @@ func ExploreOrganizations(ctx *context.Context) {
        ctx.Data["PageIsExplore"] = true
        ctx.Data["PageIsExploreOrganizations"] = true
 
-       RenderUserSearch(ctx, &UserSearchOptions{
+       RenderUserSearch(ctx, &models.SearchUserOptions{
                Type:     models.UserTypeOrganization,
-               Counter:  models.CountOrganizations,
-               Ranger:   models.Organizations,
                PageSize: setting.UI.ExplorePagingNum,
-               TplName:  tplExploreOrganizations,
-       })
+       }, tplExploreOrganizations)
 }
 
 // NotFound render 404 page