diff options
author | 6543 | 2019-10-07 05:03:43 +0200 |
---|---|---|
committer | Lunny Xiao | 2019-10-07 11:03:43 +0800 |
commit | b0dcf417ea0f68703c5e14b4c55bd9255a146bd2 (patch) | |
tree | 3ee46ccc48dbbb8d5aa175fdf4ea33409be19fa0 | |
parent | 797194d2d00eff7421b27d4238d96d2683895755 (diff) |
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
-rw-r--r-- | models/issue.go | 4 | ||||
-rw-r--r-- | models/issue_milestone.go | 75 | ||||
-rw-r--r-- | models/issue_milestone_test.go | 6 |
3 files changed, 44 insertions, 41 deletions
diff --git a/models/issue.go b/models/issue.go index 27f6ba0bc..33e13bb18 100644 --- 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 f279dda19..6db488473 100644 --- 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 f9e51aff3..a38b964b6 100644 --- 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{}) } |