aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAshley Nelson <fant@shley.email>2022-10-22 10:08:10 -0500
committerGitHub <noreply@github.com>2022-10-22 23:08:10 +0800
commit82ecd3b19eb1382521b7999bd0e2dd977c3b994d (patch)
treee6b937584fd67460fda54642b23c74736d0cccf2
parent154efa59a5a837d8375c09fb0b18a1b63bea6a3a (diff)
downloadgitea-82ecd3b19eb1382521b7999bd0e2dd977c3b994d.tar.gz
gitea-82ecd3b19eb1382521b7999bd0e2dd977c3b994d.zip
Update milestone counters when issue is deleted (#21459)
When actions besides "delete" are performed on issues, the milestone counter is updated. However, since deleting issues goes through a different code path, the associated milestone's count wasn't being updated, resulting in inaccurate counts until another issue in the same milestone had a non-delete action performed on it. I verified this change fixes the inaccurate counts using a local docker build. Fixes #21254 Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
-rw-r--r--models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml19
-rw-r--r--models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml25
-rw-r--r--models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml19
-rw-r--r--models/migrations/migrations.go2
-rw-r--r--models/migrations/v229.go47
-rw-r--r--models/migrations/v229_test.go46
-rw-r--r--services/issue/issue.go5
7 files changed, 163 insertions, 0 deletions
diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml
new file mode 100644
index 0000000000..9326fa550b
--- /dev/null
+++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/expected_milestone.yml
@@ -0,0 +1,19 @@
+# type Milestone struct {
+# ID int64 `xorm:"pk autoincr"`
+# IsClosed bool
+# NumIssues int
+# NumClosedIssues int
+# Completeness int // Percentage(1-100).
+# }
+-
+ id: 1
+ is_closed: false
+ num_issues: 3
+ num_closed_issues: 1
+ completeness: 33
+-
+ id: 2
+ is_closed: true
+ num_issues: 5
+ num_closed_issues: 5
+ completeness: 100
diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml
new file mode 100644
index 0000000000..fdaacd9f68
--- /dev/null
+++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/issue.yml
@@ -0,0 +1,25 @@
+# type Issue struct {
+# ID int64 `xorm:"pk autoincr"`
+# RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"`
+# Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository.
+# MilestoneID int64 `xorm:"INDEX"`
+# IsClosed bool `xorm:"INDEX"`
+# }
+-
+ id: 1
+ repo_id: 1
+ index: 1
+ milestone_id: 1
+ is_closed: false
+-
+ id: 2
+ repo_id: 1
+ index: 2
+ milestone_id: 1
+ is_closed: true
+-
+ id: 4
+ repo_id: 1
+ index: 3
+ milestone_id: 1
+ is_closed: false
diff --git a/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml
new file mode 100644
index 0000000000..0bcf4cffd3
--- /dev/null
+++ b/models/migrations/fixtures/Test_updateOpenMilestoneCounts/milestone.yml
@@ -0,0 +1,19 @@
+# type Milestone struct {
+# ID int64 `xorm:"pk autoincr"`
+# IsClosed bool
+# NumIssues int
+# NumClosedIssues int
+# Completeness int // Percentage(1-100).
+# }
+-
+ id: 1
+ is_closed: false
+ num_issues: 4
+ num_closed_issues: 2
+ completeness: 50
+-
+ id: 2
+ is_closed: true
+ num_issues: 5
+ num_closed_issues: 5
+ completeness: 100
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 46ef052829..cca6c52d42 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -419,6 +419,8 @@ var migrations = []Migration{
NewMigration("Create key/value table for system settings", createSystemSettingsTable),
// v228 -> v229
NewMigration("Add TeamInvite table", addTeamInviteTable),
+ // v229 -> v230
+ NewMigration("Update counts of all open milestones", updateOpenMilestoneCounts),
}
// GetCurrentDBVersion returns the current db version
diff --git a/models/migrations/v229.go b/models/migrations/v229.go
new file mode 100644
index 0000000000..42ec2033fe
--- /dev/null
+++ b/models/migrations/v229.go
@@ -0,0 +1,47 @@
+// Copyright 2022 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 migrations
+
+import (
+ "fmt"
+
+ "code.gitea.io/gitea/models/issues"
+
+ "xorm.io/builder"
+ "xorm.io/xorm"
+)
+
+func updateOpenMilestoneCounts(x *xorm.Engine) error {
+ var openMilestoneIDs []int64
+ err := x.Table("milestone").Select("id").Where(builder.Neq{"is_closed": 1}).Find(&openMilestoneIDs)
+ if err != nil {
+ return fmt.Errorf("error selecting open milestone IDs: %w", err)
+ }
+
+ for _, id := range openMilestoneIDs {
+ _, err := x.ID(id).
+ SetExpr("num_issues", builder.Select("count(*)").From("issue").Where(
+ builder.Eq{"milestone_id": id},
+ )).
+ SetExpr("num_closed_issues", builder.Select("count(*)").From("issue").Where(
+ builder.Eq{
+ "milestone_id": id,
+ "is_closed": true,
+ },
+ )).
+ Update(&issues.Milestone{})
+ if err != nil {
+ return fmt.Errorf("error updating issue counts in milestone %d: %w", id, err)
+ }
+ _, err = x.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?",
+ id,
+ )
+ if err != nil {
+ return fmt.Errorf("error setting completeness on milestone %d: %w", id, err)
+ }
+ }
+
+ return nil
+}
diff --git a/models/migrations/v229_test.go b/models/migrations/v229_test.go
new file mode 100644
index 0000000000..f8a147c9bd
--- /dev/null
+++ b/models/migrations/v229_test.go
@@ -0,0 +1,46 @@
+// Copyright 2022 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 migrations
+
+import (
+ "testing"
+
+ "code.gitea.io/gitea/models/issues"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func Test_updateOpenMilestoneCounts(t *testing.T) {
+ type ExpectedMilestone issues.Milestone
+
+ // Prepare and load the testing database
+ x, deferable := prepareTestEnv(t, 0, new(issues.Milestone), new(ExpectedMilestone), new(issues.Issue))
+ defer deferable()
+ if x == nil || t.Failed() {
+ return
+ }
+
+ if err := updateOpenMilestoneCounts(x); err != nil {
+ assert.NoError(t, err)
+ return
+ }
+
+ expected := []ExpectedMilestone{}
+ if err := x.Table("expected_milestone").Asc("id").Find(&expected); !assert.NoError(t, err) {
+ return
+ }
+
+ got := []issues.Milestone{}
+ if err := x.Table("milestone").Asc("id").Find(&got); !assert.NoError(t, err) {
+ return
+ }
+
+ for i, e := range expected {
+ got := got[i]
+ assert.Equal(t, e.ID, got.ID)
+ assert.Equal(t, e.NumIssues, got.NumIssues)
+ assert.Equal(t, e.NumClosedIssues, got.NumClosedIssues)
+ }
+}
diff --git a/services/issue/issue.go b/services/issue/issue.go
index 69b87686c1..47782e50d3 100644
--- a/services/issue/issue.go
+++ b/services/issue/issue.go
@@ -224,6 +224,11 @@ func deleteIssue(issue *issues_model.Issue) error {
return err
}
+ if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
+ return fmt.Errorf("error updating counters for milestone id %d: %w",
+ issue.MilestoneID, err)
+ }
+
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID); err != nil {
return err
}