From 0ed7ec90d85f1fd66841cb6c5dc8485fec515610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Sun, 15 Oct 2017 03:22:02 +0300 Subject: [PATCH 1/3] Integration test for activity page --- integrations/editor_test.go | 13 ++----- integrations/pull_create_test.go | 2 +- integrations/pull_merge_test.go | 4 +- integrations/repo_activity_test.go | 61 ++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 integrations/repo_activity_test.go diff --git a/integrations/editor_test.go b/integrations/editor_test.go index cc94edfd3f5f5..453b38491d740 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -89,10 +89,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { } -func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePath string) *TestResponse { - - newContent := "Hello, World (Edited)\n" - +func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePath, newContent string) *TestResponse { // Get to the 'edit this file' page req := NewRequest(t, "GET", path.Join(user, repo, "_edit", branch, filePath)) resp := session.MakeRequest(t, req, http.StatusOK) @@ -121,9 +118,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa return resp } -func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath string) *TestResponse { - - newContent := "Hello, World (Edited)\n" +func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *TestResponse { // Get to the 'edit this file' page req := NewRequest(t, "GET", path.Join(user, repo, "_edit", branch, filePath)) @@ -157,11 +152,11 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra func TestEditFile(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") - testEditFile(t, session, "user2", "repo1", "master", "README.md") + testEditFile(t, session, "user2", "repo1", "master", "README.md", "Hello, World (Edited)\n") } func TestEditFileToNewBranch(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") - testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md") + testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n") } diff --git a/integrations/pull_create_test.go b/integrations/pull_create_test.go index a62144c613fba..8f1658b11863d 100644 --- a/integrations/pull_create_test.go +++ b/integrations/pull_create_test.go @@ -47,6 +47,6 @@ func TestPullCreate(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") - testEditFile(t, session, "user1", "repo1", "master", "README.md") + testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") testPullCreate(t, session, "user1", "repo1", "master") } diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index 100298b083293..f3be6f2885471 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -49,7 +49,7 @@ func TestPullMerge(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") - testEditFile(t, session, "user1", "repo1", "master", "README.md") + testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", "master") @@ -62,7 +62,7 @@ func TestPullCleanUpAfterMerge(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") - testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md") + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", "feature/test") diff --git a/integrations/repo_activity_test.go b/integrations/repo_activity_test.go new file mode 100644 index 0000000000000..5a374ff6a9e12 --- /dev/null +++ b/integrations/repo_activity_test.go @@ -0,0 +1,61 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRepoActivity(t *testing.T) { + prepareTestEnv(t) + session := loginUser(t, "user1") + + // Create PRs (1 merged & 2 proposed) + testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + resp := testPullCreate(t, session, "user1", "repo1", "master") + elem := strings.Split(RedirectURL(t, resp), "/") + assert.EqualValues(t, "pulls", elem[3]) + testPullMerge(t, session, elem[1], elem[2], elem[4]) + + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/better_readme", "README.md", "Hello, World (Edited Again)\n") + testPullCreate(t, session, "user1", "repo1", "feat/better_readme") + + testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feat/much_better_readme", "README.md", "Hello, World (Edited More)\n") + testPullCreate(t, session, "user1", "repo1", "feat/much_better_readme") + + // Create issues (3 new issues) + testNewIssue(t, session, "user2", "repo1", "Issue 1") + testNewIssue(t, session, "user2", "repo1", "Issue 2") + testNewIssue(t, session, "user2", "repo1", "Issue 3") + + // Create releases (1 new release) + createNewRelease(t, session, "/user2/repo1", "v1.0.0", "v1.0.0", false, false) + + // Open Activity page and check stats + req := NewRequest(t, "GET", "/user2/repo1/activity") + resp = session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + // Should be 1 published release + list := htmlDoc.doc.Find("#published-releases").Next().Find("p.desc") + assert.Len(t, list.Nodes, 1) + + // Should be 1 merged pull request + list = htmlDoc.doc.Find("#merged-pull-requests").Next().Find("p.desc") + assert.Len(t, list.Nodes, 1) + + // Should be 2 merged proposed pull requests + list = htmlDoc.doc.Find("#proposed-pull-requests").Next().Find("p.desc") + assert.Len(t, list.Nodes, 2) + + // Should be 3 new issues + list = htmlDoc.doc.Find("#new-issues").Next().Find("p.desc") + assert.Len(t, list.Nodes, 3) +} From 75f9fa494a47dfb332558d50ce967b6bd1973424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Sun, 15 Oct 2017 04:09:11 +0300 Subject: [PATCH 2/3] Small code refactoring for acitvity page --- models/repo_activity.go | 16 ++++++++-------- routers/repo/activity.go | 26 ++++++++++++++------------ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/models/repo_activity.go b/models/repo_activity.go index 6f01bf20e45b2..35bac40e870e3 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -85,8 +85,8 @@ func (stats *ActivityStats) PublishedReleaseCount() int { return len(stats.PublishedReleases) } -// FillPullRequestsForActivity returns pull request information for activity page -func FillPullRequestsForActivity(stats *ActivityStats, baseRepoID int64, fromTime time.Time) error { +// FillPullRequests returns pull request information for activity page +func (stats *ActivityStats) FillPullRequests(baseRepoID int64, fromTime time.Time) error { var err error var count int64 @@ -144,8 +144,8 @@ func pullRequestsForActivityStatement(baseRepoID int64, fromTime time.Time, merg return sess } -// FillIssuesForActivity returns issue information for activity page -func FillIssuesForActivity(stats *ActivityStats, baseRepoID int64, fromTime time.Time) error { +// FillIssues returns issue information for activity page +func (stats *ActivityStats) FillIssues(baseRepoID int64, fromTime time.Time) error { var err error var count int64 @@ -182,8 +182,8 @@ func FillIssuesForActivity(stats *ActivityStats, baseRepoID int64, fromTime time return nil } -// FillUnresolvedIssuesForActivity returns unresolved issue and pull request information for activity page -func FillUnresolvedIssuesForActivity(stats *ActivityStats, baseRepoID int64, fromTime time.Time, issues, prs bool) error { +// FillUnresolvedIssues returns unresolved issue and pull request information for activity page +func (stats *ActivityStats) FillUnresolvedIssues(baseRepoID int64, fromTime time.Time, issues, prs bool) error { // Check if we need to select anything if !issues && !prs { return nil @@ -212,8 +212,8 @@ func issuesForActivityStatement(baseRepoID int64, fromTime time.Time, closed, un return sess } -// FillReleasesForActivity returns release information for activity page -func FillReleasesForActivity(stats *ActivityStats, baseRepoID int64, fromTime time.Time) error { +// FillReleases returns release information for activity page +func (stats *ActivityStats) FillReleases(baseRepoID int64, fromTime time.Time) error { var err error var count int64 diff --git a/routers/repo/activity.go b/routers/repo/activity.go index 564537c1fd4ba..5d5d9120d9066 100644 --- a/routers/repo/activity.go +++ b/routers/repo/activity.go @@ -45,28 +45,30 @@ func Activity(ctx *context.Context) { stats := &models.ActivityStats{} + pullRequestsEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) + issuesEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) + if ctx.Repo.Repository.UnitEnabled(models.UnitTypeReleases) { - if err := models.FillReleasesForActivity(stats, ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillReleasesForActivity", err) + if err := stats.FillReleases(ctx.Repo.Repository.ID, timeFrom); err != nil { + ctx.Handle(500, "FillReleases", err) return } } - if ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) { - if err := models.FillPullRequestsForActivity(stats, ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillPullRequestsForActivity", err) + if pullRequestsEnabled { + if err := stats.FillPullRequests(ctx.Repo.Repository.ID, timeFrom); err != nil { + ctx.Handle(500, "FillPullRequests", err) return } } - if ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) { - if err := models.FillIssuesForActivity(stats, ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillIssuesForActivity", err) + if issuesEnabled { + if err := stats.FillIssues(ctx.Repo.Repository.ID, timeFrom); err != nil { + ctx.Handle(500, "FillIssues", err) return } } - if err := models.FillUnresolvedIssuesForActivity(stats, ctx.Repo.Repository.ID, timeFrom, - ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues), - ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests)); err != nil { - ctx.Handle(500, "FillUnresolvedIssuesForActivity", err) + if err := stats.FillUnresolvedIssues(ctx.Repo.Repository.ID, timeFrom, + issuesEnabled, pullRequestsEnabled); err != nil { + ctx.Handle(500, "FillUnresolvedIssues", err) return } From b672d3c63e950eac3670305cb0819ad1a0ecf0db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauris=20Buk=C5=A1is-Haberkorns?= Date: Sun, 15 Oct 2017 17:21:15 +0300 Subject: [PATCH 3/3] Move activity stats calculation logic to model --- models/repo_activity.go | 67 +++++++++++++++++++++++++++------------- routers/repo/activity.go | 34 ++++---------------- 2 files changed, 52 insertions(+), 49 deletions(-) diff --git a/models/repo_activity.go b/models/repo_activity.go index 35bac40e870e3..839a74a7e6254 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -5,6 +5,7 @@ package models import ( + "fmt" "time" "github.com/go-xorm/xorm" @@ -25,6 +26,30 @@ type ActivityStats struct { PublishedReleaseAuthorCount int64 } +// GetActivityStats return stats for repository at given time range +func GetActivityStats(repoID int64, timeFrom time.Time, releases, issues, prs bool) (*ActivityStats, error) { + stats := &ActivityStats{} + if releases { + if err := stats.FillReleases(repoID, timeFrom); err != nil { + return nil, fmt.Errorf("FillReleases: %v", err) + } + } + if prs { + if err := stats.FillPullRequests(repoID, timeFrom); err != nil { + return nil, fmt.Errorf("FillPullRequests: %v", err) + } + } + if issues { + if err := stats.FillIssues(repoID, timeFrom); err != nil { + return nil, fmt.Errorf("FillIssues: %v", err) + } + } + if err := stats.FillUnresolvedIssues(repoID, timeFrom, issues, prs); err != nil { + return nil, fmt.Errorf("FillUnresolvedIssues: %v", err) + } + return stats, nil +} + // ActivePRCount returns total active pull request count func (stats *ActivityStats) ActivePRCount() int { return stats.OpenedPRCount() + stats.MergedPRCount() @@ -86,12 +111,12 @@ func (stats *ActivityStats) PublishedReleaseCount() int { } // FillPullRequests returns pull request information for activity page -func (stats *ActivityStats) FillPullRequests(baseRepoID int64, fromTime time.Time) error { +func (stats *ActivityStats) FillPullRequests(repoID int64, fromTime time.Time) error { var err error var count int64 // Merged pull requests - sess := pullRequestsForActivityStatement(baseRepoID, fromTime, true) + sess := pullRequestsForActivityStatement(repoID, fromTime, true) sess.OrderBy("pull_request.merged_unix DESC") stats.MergedPRs = make(PullRequestList, 0) if err = sess.Find(&stats.MergedPRs); err != nil { @@ -102,14 +127,14 @@ func (stats *ActivityStats) FillPullRequests(baseRepoID int64, fromTime time.Tim } // Merged pull request authors - sess = pullRequestsForActivityStatement(baseRepoID, fromTime, true) + sess = pullRequestsForActivityStatement(repoID, fromTime, true) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("pull_request").Get(&count); err != nil { return err } stats.MergedPRAuthorCount = count // Opened pull requests - sess = pullRequestsForActivityStatement(baseRepoID, fromTime, false) + sess = pullRequestsForActivityStatement(repoID, fromTime, false) sess.OrderBy("issue.created_unix ASC") stats.OpenedPRs = make(PullRequestList, 0) if err = sess.Find(&stats.OpenedPRs); err != nil { @@ -120,7 +145,7 @@ func (stats *ActivityStats) FillPullRequests(baseRepoID int64, fromTime time.Tim } // Opened pull request authors - sess = pullRequestsForActivityStatement(baseRepoID, fromTime, false) + sess = pullRequestsForActivityStatement(repoID, fromTime, false) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("pull_request").Get(&count); err != nil { return err } @@ -129,8 +154,8 @@ func (stats *ActivityStats) FillPullRequests(baseRepoID int64, fromTime time.Tim return nil } -func pullRequestsForActivityStatement(baseRepoID int64, fromTime time.Time, merged bool) *xorm.Session { - sess := x.Where("pull_request.base_repo_id=?", baseRepoID). +func pullRequestsForActivityStatement(repoID int64, fromTime time.Time, merged bool) *xorm.Session { + sess := x.Where("pull_request.base_repo_id=?", repoID). Join("INNER", "issue", "pull_request.issue_id = issue.id") if merged { @@ -145,12 +170,12 @@ func pullRequestsForActivityStatement(baseRepoID int64, fromTime time.Time, merg } // FillIssues returns issue information for activity page -func (stats *ActivityStats) FillIssues(baseRepoID int64, fromTime time.Time) error { +func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time) error { var err error var count int64 // Closed issues - sess := issuesForActivityStatement(baseRepoID, fromTime, true, false) + sess := issuesForActivityStatement(repoID, fromTime, true, false) sess.OrderBy("issue.updated_unix DESC") stats.ClosedIssues = make(IssueList, 0) if err = sess.Find(&stats.ClosedIssues); err != nil { @@ -158,14 +183,14 @@ func (stats *ActivityStats) FillIssues(baseRepoID int64, fromTime time.Time) err } // Closed issue authors - sess = issuesForActivityStatement(baseRepoID, fromTime, true, false) + sess = issuesForActivityStatement(repoID, fromTime, true, false) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("issue").Get(&count); err != nil { return err } stats.ClosedIssueAuthorCount = count // New issues - sess = issuesForActivityStatement(baseRepoID, fromTime, false, false) + sess = issuesForActivityStatement(repoID, fromTime, false, false) sess.OrderBy("issue.created_unix ASC") stats.OpenedIssues = make(IssueList, 0) if err = sess.Find(&stats.OpenedIssues); err != nil { @@ -173,7 +198,7 @@ func (stats *ActivityStats) FillIssues(baseRepoID int64, fromTime time.Time) err } // Opened issue authors - sess = issuesForActivityStatement(baseRepoID, fromTime, false, false) + sess = issuesForActivityStatement(repoID, fromTime, false, false) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("issue").Get(&count); err != nil { return err } @@ -183,12 +208,12 @@ func (stats *ActivityStats) FillIssues(baseRepoID int64, fromTime time.Time) err } // FillUnresolvedIssues returns unresolved issue and pull request information for activity page -func (stats *ActivityStats) FillUnresolvedIssues(baseRepoID int64, fromTime time.Time, issues, prs bool) error { +func (stats *ActivityStats) FillUnresolvedIssues(repoID int64, fromTime time.Time, issues, prs bool) error { // Check if we need to select anything if !issues && !prs { return nil } - sess := issuesForActivityStatement(baseRepoID, fromTime, false, true) + sess := issuesForActivityStatement(repoID, fromTime, false, true) if !issues || !prs { sess.And("issue.is_pull = ?", prs) } @@ -197,8 +222,8 @@ func (stats *ActivityStats) FillUnresolvedIssues(baseRepoID int64, fromTime time return sess.Find(&stats.UnresolvedIssues) } -func issuesForActivityStatement(baseRepoID int64, fromTime time.Time, closed, unresolved bool) *xorm.Session { - sess := x.Where("issue.repo_id = ?", baseRepoID). +func issuesForActivityStatement(repoID int64, fromTime time.Time, closed, unresolved bool) *xorm.Session { + sess := x.Where("issue.repo_id = ?", repoID). And("issue.is_closed = ?", closed) if !unresolved { @@ -213,12 +238,12 @@ func issuesForActivityStatement(baseRepoID int64, fromTime time.Time, closed, un } // FillReleases returns release information for activity page -func (stats *ActivityStats) FillReleases(baseRepoID int64, fromTime time.Time) error { +func (stats *ActivityStats) FillReleases(repoID int64, fromTime time.Time) error { var err error var count int64 // Published releases list - sess := releasesForActivityStatement(baseRepoID, fromTime) + sess := releasesForActivityStatement(repoID, fromTime) sess.OrderBy("release.created_unix DESC") stats.PublishedReleases = make([]*Release, 0) if err = sess.Find(&stats.PublishedReleases); err != nil { @@ -226,7 +251,7 @@ func (stats *ActivityStats) FillReleases(baseRepoID int64, fromTime time.Time) e } // Published releases authors - sess = releasesForActivityStatement(baseRepoID, fromTime) + sess = releasesForActivityStatement(repoID, fromTime) if _, err = sess.Select("count(distinct release.publisher_id) as `count`").Table("release").Get(&count); err != nil { return err } @@ -235,8 +260,8 @@ func (stats *ActivityStats) FillReleases(baseRepoID int64, fromTime time.Time) e return nil } -func releasesForActivityStatement(baseRepoID int64, fromTime time.Time) *xorm.Session { - return x.Where("release.repo_id = ?", baseRepoID). +func releasesForActivityStatement(repoID int64, fromTime time.Time) *xorm.Session { + return x.Where("release.repo_id = ?", repoID). And("release.is_draft = ?", false). And("release.created_unix >= ?", fromTime.Unix()) } diff --git a/routers/repo/activity.go b/routers/repo/activity.go index 5d5d9120d9066..fbe51e152e10c 100644 --- a/routers/repo/activity.go +++ b/routers/repo/activity.go @@ -43,36 +43,14 @@ func Activity(ctx *context.Context) { ctx.Data["DateUntil"] = timeUntil.Format("January 2, 2006") ctx.Data["PeriodText"] = ctx.Tr("repo.activity.period." + ctx.Data["Period"].(string)) - stats := &models.ActivityStats{} - - pullRequestsEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) - issuesEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) - - if ctx.Repo.Repository.UnitEnabled(models.UnitTypeReleases) { - if err := stats.FillReleases(ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillReleases", err) - return - } - } - if pullRequestsEnabled { - if err := stats.FillPullRequests(ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillPullRequests", err) - return - } - } - if issuesEnabled { - if err := stats.FillIssues(ctx.Repo.Repository.ID, timeFrom); err != nil { - ctx.Handle(500, "FillIssues", err) - return - } - } - if err := stats.FillUnresolvedIssues(ctx.Repo.Repository.ID, timeFrom, - issuesEnabled, pullRequestsEnabled); err != nil { - ctx.Handle(500, "FillUnresolvedIssues", err) + var err error + if ctx.Data["Activity"], err = models.GetActivityStats(ctx.Repo.Repository.ID, timeFrom, + ctx.Repo.Repository.UnitEnabled(models.UnitTypeReleases), + ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues), + ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests)); err != nil { + ctx.Handle(500, "GetActivityStats", err) return } - ctx.Data["Activity"] = stats - ctx.HTML(200, tplActivity) }