diff --git a/models/action.go b/models/action.go index a09e42066..fa610ff5c 100644 --- a/models/action.go +++ b/models/action.go @@ -672,33 +672,39 @@ func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error return mergePullRequestAction(x, actUser, repo, pull) } -// GetFeeds returns action list of given user in given context. -// actorID is the user who's requesting, ctxUserID is the user/org that is requested. -// actorID can be -1 when isProfile is true or to skip the permission check. -func GetFeeds(ctxUser *User, actorID, offset int64, isProfile bool) ([]*Action, error) { - actions := make([]*Action, 0, 20) - sess := x. - Limit(20, int(offset)). - Desc("id"). - Where("user_id = ?", ctxUser.ID) - if isProfile { - sess. - And("is_private = ?", false). - And("act_user_id = ?", ctxUser.ID) - } else if actorID != -1 && ctxUser.IsOrganization() { - env, err := ctxUser.AccessibleReposEnv(actorID) +// GetFeedsOptions options for retrieving feeds +type GetFeedsOptions struct { + RequestedUser *User + RequestingUserID int64 + IncludePrivate bool // include private actions + OnlyPerformedBy bool // only actions performed by requested user +} + +// GetFeeds returns actions according to the provided options +func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { + var repoIDs []int64 + if opts.RequestedUser.IsOrganization() { + env, err := opts.RequestedUser.AccessibleReposEnv(opts.RequestingUserID) if err != nil { return nil, fmt.Errorf("AccessibleReposEnv: %v", err) } - repoIDs, err := env.RepoIDs(1, ctxUser.NumRepos) - if err != nil { + if repoIDs, err = env.RepoIDs(1, opts.RequestedUser.NumRepos); err != nil { return nil, fmt.Errorf("GetUserRepositories: %v", err) } - if len(repoIDs) > 0 { - sess.In("repo_id", repoIDs) - } } - err := sess.Find(&actions) - return actions, err + actions := make([]*Action, 0, 20) + sess := x.Limit(20). + Desc("id"). + Where("user_id = ?", opts.RequestedUser.ID) + if opts.OnlyPerformedBy { + sess.And("act_user_id = ?", opts.RequestedUser.ID) + } + if !opts.IncludePrivate { + sess.And("is_private = ?", false) + } + if opts.RequestedUser.IsOrganization() { + sess.In("repo_id", repoIDs) + } + return actions, sess.Find(&actions) } diff --git a/models/action_test.go b/models/action_test.go index c6d3911a6..debc884b3 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -305,13 +305,23 @@ func TestGetFeeds(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) - actions, err := GetFeeds(user, user.ID, 0, false) + actions, err := GetFeeds(GetFeedsOptions{ + RequestedUser: user, + RequestingUserID: user.ID, + IncludePrivate: true, + OnlyPerformedBy: false, + }) assert.NoError(t, err) assert.Len(t, actions, 1) - assert.Equal(t, int64(1), actions[0].ID) - assert.Equal(t, user.ID, actions[0].UserID) + assert.EqualValues(t, 1, actions[0].ID) + assert.EqualValues(t, user.ID, actions[0].UserID) - actions, err = GetFeeds(user, user.ID, 0, true) + actions, err = GetFeeds(GetFeedsOptions{ + RequestedUser: user, + RequestingUserID: user.ID, + IncludePrivate: false, + OnlyPerformedBy: false, + }) assert.NoError(t, err) assert.Len(t, actions, 0) } @@ -319,15 +329,26 @@ func TestGetFeeds(t *testing.T) { func TestGetFeeds2(t *testing.T) { // test with an organization user assert.NoError(t, PrepareTestDatabase()) - user := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) + org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) + userID := AssertExistsAndLoadBean(t, &OrgUser{OrgID: org.ID, IsOwner: true}).(*OrgUser).UID - actions, err := GetFeeds(user, user.ID, 0, false) + actions, err := GetFeeds(GetFeedsOptions{ + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: true, + OnlyPerformedBy: false, + }) assert.NoError(t, err) assert.Len(t, actions, 1) - assert.Equal(t, int64(2), actions[0].ID) - assert.Equal(t, user.ID, actions[0].UserID) + assert.EqualValues(t, 2, actions[0].ID) + assert.EqualValues(t, org.ID, actions[0].UserID) - actions, err = GetFeeds(user, user.ID, 0, true) + actions, err = GetFeeds(GetFeedsOptions{ + RequestedUser: org, + RequestingUserID: userID, + IncludePrivate: false, + OnlyPerformedBy: false, + }) assert.NoError(t, err) assert.Len(t, actions, 0) } diff --git a/models/fixtures/action.yml b/models/fixtures/action.yml index 10c0dc129..ad46e6b68 100644 --- a/models/fixtures/action.yml +++ b/models/fixtures/action.yml @@ -10,7 +10,7 @@ id: 2 user_id: 3 op_type: 2 # rename repo - act_user_id: 3 + act_user_id: 2 repo_id: 3 is_private: true content: oldRepoName diff --git a/routers/user/home.go b/routers/user/home.go index bafe62754..0db170bfd 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -53,19 +53,20 @@ func getDashboardContextUser(ctx *context.Context) *models.User { return ctxUser } -// retrieveFeeds loads feeds from database by given context user. -// The user could be organization so it is not always the logged in user, -// which is why we have to explicitly pass the context user ID. -func retrieveFeeds(ctx *context.Context, ctxUser *models.User, userID, offset int64, isProfile bool) { - actions, err := models.GetFeeds(ctxUser, userID, offset, isProfile) +// retrieveFeeds loads feeds for the specified user +func retrieveFeeds(ctx *context.Context, user *models.User, includePrivate, isProfile bool) { + actions, err := models.GetFeeds(models.GetFeedsOptions{ + RequestedUser: user, + RequestingUserID: ctx.User.ID, + IncludePrivate: includePrivate, + OnlyPerformedBy: isProfile, + }) if err != nil { ctx.Handle(500, "GetFeeds", err) return } - // Check access of private repositories. - feeds := make([]*models.Action, 0, len(actions)) - userCache := map[int64]*models.User{ctxUser.ID: ctxUser} + userCache := map[int64]*models.User{user.ID: user} repoCache := map[int64]*models.Repository{} for _, act := range actions { // Cache results to reduce queries. @@ -108,10 +109,8 @@ func retrieveFeeds(ctx *context.Context, ctxUser *models.User, userID, offset in } } repo.Owner = repoOwner - - feeds = append(feeds, act) } - ctx.Data["Feeds"] = feeds + ctx.Data["Feeds"] = actions } // Dashboard render the dashborad page @@ -180,7 +179,7 @@ func Dashboard(ctx *context.Context) { ctx.Data["MirrorCount"] = len(mirrors) ctx.Data["Mirrors"] = mirrors - retrieveFeeds(ctx, ctxUser, ctx.User.ID, 0, false) + retrieveFeeds(ctx, ctxUser, true, false) if ctx.Written() { return } diff --git a/routers/user/profile.go b/routers/user/profile.go index eb862d654..1768cec7b 100644 --- a/routers/user/profile.go +++ b/routers/user/profile.go @@ -138,7 +138,7 @@ func Profile(ctx *context.Context) { ctx.Data["Keyword"] = keyword switch tab { case "activity": - retrieveFeeds(ctx, ctxUser, -1, 0, !showPrivate) + retrieveFeeds(ctx, ctxUser, showPrivate, true) if ctx.Written() { return }