From cc98737ca81d9552f20c277e6ad0031927f9b757 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 10 Mar 2022 15:54:51 +0100 Subject: [PATCH] RSS/Atom support for Orgs (#17714) part of #569 --- models/action.go | 94 +++++++++++++++++++++---------------- models/action_test.go | 40 ++++++++++++++++ models/user_heatmap_test.go | 47 ++++++++++++------- routers/web/feed/profile.go | 45 +++++++++--------- routers/web/user/profile.go | 13 ++--- 5 files changed, 154 insertions(+), 85 deletions(-) diff --git a/models/action.go b/models/action.go index 26d05730c5..f2723a2014 100644 --- a/models/action.go +++ b/models/action.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -315,19 +316,21 @@ func (a *Action) GetIssueContent() string { // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { - RequestedUser *user_model.User // the user we want activity for - RequestedTeam *Team // the team we want activity for - Actor *user_model.User // the user viewing the activity - IncludePrivate bool // include private actions - OnlyPerformedBy bool // only actions performed by requested user - IncludeDeleted bool // include deleted actions - Date string // the day we want activity for: YYYY-MM-DD + db.ListOptions + RequestedUser *user_model.User // the user we want activity for + RequestedTeam *Team // the team we want activity for + RequestedRepo *repo_model.Repository // the repo we want activity for + Actor *user_model.User // the user viewing the activity + IncludePrivate bool // include private actions + OnlyPerformedBy bool // only actions performed by requested user + IncludeDeleted bool // include deleted actions + Date string // the day we want activity for: YYYY-MM-DD } // GetFeeds returns actions according to the provided options func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { - if !activityReadable(opts.RequestedUser, opts.Actor) { - return make([]*Action, 0), nil + if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil { + return nil, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo") } cond, err := activityQueryCondition(opts) @@ -335,9 +338,14 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { return nil, err } - actions := make([]*Action, 0, setting.UI.FeedPagingNum) + sess := db.GetEngine(db.DefaultContext).Where(cond) - if err := db.GetEngine(db.DefaultContext).Limit(setting.UI.FeedPagingNum).Desc("created_unix").Where(cond).Find(&actions); err != nil { + opts.SetDefaultValues() + sess = db.SetSessionPagination(sess, &opts) + + actions := make([]*Action, 0, opts.PageSize) + + if err := sess.Desc("created_unix").Find(&actions); err != nil { return nil, fmt.Errorf("Find: %v", err) } @@ -349,41 +357,44 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { } func activityReadable(user, doer *user_model.User) bool { - var doerID int64 - if doer != nil { - doerID = doer.ID - } - if doer == nil || !doer.IsAdmin { - if user.KeepActivityPrivate && doerID != user.ID { - return false - } - } - return true + return !user.KeepActivityPrivate || + doer != nil && (doer.IsAdmin || user.ID == doer.ID) } func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) { cond := builder.NewCond() - var repoIDs []int64 - var actorID int64 - if opts.Actor != nil { - actorID = opts.Actor.ID + if opts.RequestedTeam != nil && opts.RequestedUser == nil { + org, err := user_model.GetUserByID(opts.RequestedTeam.OrgID) + if err != nil { + return nil, err + } + opts.RequestedUser = org + } + + // check activity visibility for actor ( similar to activityReadable() ) + if opts.Actor == nil { + cond = cond.And(builder.In("act_user_id", + builder.Select("`user`.id").Where( + builder.Eq{"keep_activity_private": false, "visibility": structs.VisibleTypePublic}, + ).From("`user`"), + )) + } else if !opts.Actor.IsAdmin { + cond = cond.And(builder.In("act_user_id", + builder.Select("`user`.id").Where( + builder.Eq{"keep_activity_private": false}. + And(builder.In("visibility", structs.VisibleTypePublic, structs.VisibleTypeLimited))). + Or(builder.Eq{"id": opts.Actor.ID}).From("`user`"), + )) } // check readable repositories by doer/actor if opts.Actor == nil || !opts.Actor.IsAdmin { - if opts.RequestedUser.IsOrganization() { - env, err := OrgFromUser(opts.RequestedUser).AccessibleReposEnv(actorID) - if err != nil { - return nil, fmt.Errorf("AccessibleReposEnv: %v", err) - } - if repoIDs, err = env.RepoIDs(1, opts.RequestedUser.NumRepos); err != nil { - return nil, fmt.Errorf("GetUserRepositories: %v", err) - } - cond = cond.And(builder.In("repo_id", repoIDs)) - } else { - cond = cond.And(builder.In("repo_id", AccessibleRepoIDsQuery(opts.Actor))) - } + cond = cond.And(builder.In("repo_id", AccessibleRepoIDsQuery(opts.Actor))) + } + + if opts.RequestedRepo != nil { + cond = cond.And(builder.Eq{"repo_id": opts.RequestedRepo.ID}) } if opts.RequestedTeam != nil { @@ -395,11 +406,14 @@ func activityQueryCondition(opts GetFeedsOptions) (builder.Cond, error) { cond = cond.And(builder.In("repo_id", teamRepoIDs)) } - cond = cond.And(builder.Eq{"user_id": opts.RequestedUser.ID}) + if opts.RequestedUser != nil { + cond = cond.And(builder.Eq{"user_id": opts.RequestedUser.ID}) - if opts.OnlyPerformedBy { - cond = cond.And(builder.Eq{"act_user_id": opts.RequestedUser.ID}) + if opts.OnlyPerformedBy { + cond = cond.And(builder.Eq{"act_user_id": opts.RequestedUser.ID}) + } } + if !opts.IncludePrivate { cond = cond.And(builder.Eq{"is_private": false}) } diff --git a/models/action_test.go b/models/action_test.go index 306d382364..0ce9183b96 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -93,6 +93,46 @@ func TestGetFeeds2(t *testing.T) { assert.Len(t, actions, 0) } +func TestActivityReadable(t *testing.T) { + tt := []struct { + desc string + user *user_model.User + doer *user_model.User + result bool + }{{ + desc: "user should see own activity", + user: &user_model.User{ID: 1}, + doer: &user_model.User{ID: 1}, + result: true, + }, { + desc: "anon should see activity if public", + user: &user_model.User{ID: 1}, + result: true, + }, { + desc: "anon should NOT see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + result: false, + }, { + desc: "user should see own activity if private too", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 1}, + result: true, + }, { + desc: "other user should NOT see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 2}, + result: false, + }, { + desc: "admin should see activity", + user: &user_model.User{ID: 1, KeepActivityPrivate: true}, + doer: &user_model.User{ID: 2, IsAdmin: true}, + result: true, + }} + for _, test := range tt { + assert.Equal(t, test.result, activityReadable(test.user, test.doer), test.desc) + } +} + func TestNotifyWatchers(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/user_heatmap_test.go b/models/user_heatmap_test.go index 7d2997648d..7915363d95 100644 --- a/models/user_heatmap_test.go +++ b/models/user_heatmap_test.go @@ -19,25 +19,40 @@ import ( func TestGetUserHeatmapDataByUser(t *testing.T) { testCases := []struct { + desc string userID int64 doerID int64 CountResult int JSONResult string }{ - // self looks at action in private repo - {2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`}, - // admin looks at action in private repo - {2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`}, - // other user looks at action in private repo - {2, 3, 0, `[]`}, - // nobody looks at action in private repo - {2, 0, 0, `[]`}, - // collaborator looks at action in private repo - {16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`}, - // no action action not performed by target user - {3, 3, 0, `[]`}, - // multiple actions performed with two grouped together - {10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`}, + { + "self looks at action in private repo", + 2, 2, 1, `[{"timestamp":1603227600,"contributions":1}]`, + }, + { + "admin looks at action in private repo", + 2, 1, 1, `[{"timestamp":1603227600,"contributions":1}]`, + }, + { + "other user looks at action in private repo", + 2, 3, 0, `[]`, + }, + { + "nobody looks at action in private repo", + 2, 0, 0, `[]`, + }, + { + "collaborator looks at action in private repo", + 16, 15, 1, `[{"timestamp":1603267200,"contributions":1}]`, + }, + { + "no action action not performed by target user", + 3, 3, 0, `[]`, + }, + { + "multiple actions performed with two grouped together", + 10, 10, 3, `[{"timestamp":1603009800,"contributions":1},{"timestamp":1603010700,"contributions":2}]`, + }, } // Prepare assert.NoError(t, unittest.PrepareTestDatabase()) @@ -46,7 +61,7 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { timeutil.Set(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)) defer timeutil.Unset() - for i, tc := range testCases { + for _, tc := range testCases { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.userID}).(*user_model.User) doer := &user_model.User{ID: tc.doerID} @@ -74,7 +89,7 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { } assert.NoError(t, err) assert.Len(t, actions, contributions, "invalid action count: did the test data became too old?") - assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase %d", i)) + assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase '%s'", tc.desc)) // Test JSON rendering jsonData, err := json.Marshal(heatmap) diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 1a7f4ad24b..4c1eff04a9 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -23,31 +23,34 @@ func RetrieveFeeds(ctx *context.Context, options models.GetFeedsOptions) []*mode return nil } - userCache := map[int64]*user_model.User{options.RequestedUser.ID: options.RequestedUser} - if ctx.User != nil { - userCache[ctx.User.ID] = ctx.User - } - for _, act := range actions { - if act.ActUser != nil { - userCache[act.ActUserID] = act.ActUser + // TODO: move load repoOwner of act.Repo into models.GetFeeds->loadAttributes() + { + userCache := map[int64]*user_model.User{options.RequestedUser.ID: options.RequestedUser} + if ctx.User != nil { + userCache[ctx.User.ID] = ctx.User } - } - - for _, act := range actions { - repoOwner, ok := userCache[act.Repo.OwnerID] - if !ok { - repoOwner, err = user_model.GetUserByID(act.Repo.OwnerID) - if err != nil { - if user_model.IsErrUserNotExist(err) { - continue + for _, act := range actions { + if act.ActUser != nil { + userCache[act.ActUserID] = act.ActUser + } + } + for _, act := range actions { + repoOwner, ok := userCache[act.Repo.OwnerID] + if !ok { + repoOwner, err = user_model.GetUserByID(act.Repo.OwnerID) + if err != nil { + if user_model.IsErrUserNotExist(err) { + continue + } + ctx.ServerError("GetUserByID", err) + return nil } - ctx.ServerError("GetUserByID", err) - return nil + userCache[repoOwner.ID] = repoOwner } - userCache[repoOwner.ID] = repoOwner + act.Repo.Owner = repoOwner } - act.Repo.Owner = repoOwner } + return actions } @@ -57,7 +60,7 @@ func ShowUserFeed(ctx *context.Context, ctxUser *user_model.User, formatType str RequestedUser: ctxUser, Actor: ctx.User, IncludePrivate: false, - OnlyPerformedBy: true, + OnlyPerformedBy: !ctxUser.IsOrganization(), IncludeDeleted: false, Date: ctx.FormString("date"), }) diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 9c0ce10dae..b4198ef8fd 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -94,14 +94,11 @@ func Profile(ctx *context.Context) { } if ctxUser.IsOrganization() { - /* - // TODO: enable after rss.RetrieveFeeds() do handle org correctly - // Show Org RSS feed - if len(showFeedType) != 0 { - rss.ShowUserFeed(ctx, ctxUser, showFeedType) - return - } - */ + // Show Org RSS feed + if len(showFeedType) != 0 { + feed.ShowUserFeed(ctx, ctxUser, showFeedType) + return + } org.Home(ctx) return