Fix milestone num_issues (#8221) (#8400)

* fix milestone num_issues

* update missing completeness

* only update milestone closed number when closed issue is assigned a new milestone or clear milestone

* fix tests

* fix update milestone num

* fix completeness calculate

* make completeness calucation more clear

diff --git a/models/issue.go b/models/issue.go
index 27f6ba0bcabed8fc5aa1a4ca8c146069ef63cf88..33e13bb180c55c6734379d9f1e7bfc3bc926c81d 100644 (file)
--- a/models/issue.go
+++ b/models/issue.go
@@ -755,7 +755,7 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er
        }
 
        // Update issue count of milestone
-       if err = changeMilestoneIssueStats(e, issue); err != nil {
+       if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil {
                return err
        }
 
@@ -1099,7 +1099,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
        }
 
        if opts.Issue.MilestoneID > 0 {
-               if err = changeMilestoneAssign(e, doer, opts.Issue, -1); err != nil {
+               if _, err = e.Exec("UPDATE `milestone` SET num_issues=num_issues+1 WHERE id=?", opts.Issue.MilestoneID); err != nil {
                        return err
                }
        }

diff --git a/models/issue_milestone.go b/models/issue_milestone.go
index f279dda195e7d82e887b9b426dacb5b5aaf7c3d4..6db488473a645cc14e87d10acca79e60d57cef3d 100644 (file)
--- a/models/issue_milestone.go
+++ b/models/issue_milestone.go
@@ -311,71 +311,74 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) {
        return sess.Commit()
 }
 
-func changeMilestoneIssueStats(e *xorm.Session, issue *Issue) error {
-       if issue.MilestoneID == 0 {
-               return nil
+func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) {
+       if _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?) WHERE id=?",
+               milestoneID,
+               milestoneID,
+       ); err != nil {
+               return
        }
 
-       m, err := getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID)
-       if err != nil {
-               return err
-       }
+       _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?",
+               milestoneID,
+       )
 
-       if issue.IsClosed {
-               m.NumOpenIssues--
-               m.NumClosedIssues++
-       } else {
-               m.NumOpenIssues++
-               m.NumClosedIssues--
+       return
+}
+
+func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) {
+       if _, err = e.Exec("UPDATE `milestone` SET num_closed_issues=(SELECT count(*) FROM issue WHERE milestone_id=? AND is_closed=?) WHERE id=?",
+               milestoneID,
+               true,
+               milestoneID,
+       ); err != nil {
+               return
        }
 
-       return updateMilestone(e, m)
+       _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?",
+               milestoneID,
+       )
+       return
 }
 
 func changeMilestoneAssign(e *xorm.Session, doer *User, issue *Issue, oldMilestoneID int64) error {
+       if err := updateIssueCols(e, issue, "milestone_id"); err != nil {
+               return err
+       }
+
        if oldMilestoneID > 0 {
-               m, err := getMilestoneByRepoID(e, issue.RepoID, oldMilestoneID)
-               if err != nil {
+               if err := updateMilestoneTotalNum(e, oldMilestoneID); err != nil {
                        return err
                }
-
-               m.NumIssues--
                if issue.IsClosed {
-                       m.NumClosedIssues--
-               }
-
-               if err = updateMilestone(e, m); err != nil {
-                       return err
+                       if err := updateMilestoneClosedNum(e, oldMilestoneID); err != nil {
+                               return err
+                       }
                }
        }
 
        if issue.MilestoneID > 0 {
-               m, err := getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID)
-               if err != nil {
+               if err := updateMilestoneTotalNum(e, issue.MilestoneID); err != nil {
                        return err
                }
-
-               m.NumIssues++
                if issue.IsClosed {
-                       m.NumClosedIssues++
+                       if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil {
+                               return err
+                       }
                }
+       }
 
-               if err = updateMilestone(e, m); err != nil {
+       if oldMilestoneID > 0 || issue.MilestoneID > 0 {
+               if err := issue.loadRepo(e); err != nil {
                        return err
                }
-       }
-
-       if err := issue.loadRepo(e); err != nil {
-               return err
-       }
 
-       if oldMilestoneID > 0 || issue.MilestoneID > 0 {
                if _, err := createMilestoneComment(e, doer, issue.Repo, issue, oldMilestoneID, issue.MilestoneID); err != nil {
                        return err
                }
        }
 
-       return updateIssueCols(e, issue, "milestone_id")
+       return nil
 }
 
 // ChangeMilestoneAssign changes assignment of milestone for issue.

diff --git a/models/issue_milestone_test.go b/models/issue_milestone_test.go
index f9e51aff318eede30691f9886fde3800743ab88d..a38b964b65bc62d0bbb013259a434509ad845573 100644 (file)
--- a/models/issue_milestone_test.go
+++ b/models/issue_milestone_test.go
@@ -231,7 +231,7 @@ func TestChangeMilestoneStatus(t *testing.T) {
        CheckConsistencyFor(t, &Repository{ID: milestone.RepoID}, &Milestone{})
 }
 
-func TestChangeMilestoneIssueStats(t *testing.T) {
+func TestUpdateMilestoneClosedNum(t *testing.T) {
        assert.NoError(t, PrepareTestDatabase())
        issue := AssertExistsAndLoadBean(t, &Issue{MilestoneID: 1},
                "is_closed=0").(*Issue)
@@ -240,14 +240,14 @@ func TestChangeMilestoneIssueStats(t *testing.T) {
        issue.ClosedUnix = util.TimeStampNow()
        _, err := x.Cols("is_closed", "closed_unix").Update(issue)
        assert.NoError(t, err)
-       assert.NoError(t, changeMilestoneIssueStats(x.NewSession(), issue))
+       assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID))
        CheckConsistencyFor(t, &Milestone{})
 
        issue.IsClosed = false
        issue.ClosedUnix = 0
        _, err = x.Cols("is_closed", "closed_unix").Update(issue)
        assert.NoError(t, err)
-       assert.NoError(t, changeMilestoneIssueStats(x.NewSession(), issue))
+       assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID))
        CheckConsistencyFor(t, &Milestone{})
 }