Fix milestone num_issues (#8221)

* 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
This commit is contained in:
Lunny Xiao 2019-10-07 05:26:19 +08:00 committed by techknowlogick
parent bc5a479fef
commit 51fade4c44
3 changed files with 46 additions and 43 deletions

View File

@ -766,7 +766,7 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, isClosed bool) (er
} }
// Update issue count of milestone // Update issue count of milestone
if err = changeMilestoneIssueStats(e, issue); err != nil { if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil {
return err return err
} }
@ -1119,7 +1119,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
opts.Issue.Index = inserted.Index opts.Issue.Index = inserted.Index
if opts.Issue.MilestoneID > 0 { 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 return err
} }
} }

View File

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

View File

@ -231,7 +231,7 @@ func TestChangeMilestoneStatus(t *testing.T) {
CheckConsistencyFor(t, &Repository{ID: milestone.RepoID}, &Milestone{}) CheckConsistencyFor(t, &Repository{ID: milestone.RepoID}, &Milestone{})
} }
func TestChangeMilestoneIssueStats(t *testing.T) { func TestUpdateMilestoneClosedNum(t *testing.T) {
assert.NoError(t, PrepareTestDatabase()) assert.NoError(t, PrepareTestDatabase())
issue := AssertExistsAndLoadBean(t, &Issue{MilestoneID: 1}, issue := AssertExistsAndLoadBean(t, &Issue{MilestoneID: 1},
"is_closed=0").(*Issue) "is_closed=0").(*Issue)
@ -240,14 +240,14 @@ func TestChangeMilestoneIssueStats(t *testing.T) {
issue.ClosedUnix = timeutil.TimeStampNow() issue.ClosedUnix = timeutil.TimeStampNow()
_, err := x.Cols("is_closed", "closed_unix").Update(issue) _, err := x.Cols("is_closed", "closed_unix").Update(issue)
assert.NoError(t, err) assert.NoError(t, err)
assert.NoError(t, changeMilestoneIssueStats(x.NewSession(), issue)) assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID))
CheckConsistencyFor(t, &Milestone{}) CheckConsistencyFor(t, &Milestone{})
issue.IsClosed = false issue.IsClosed = false
issue.ClosedUnix = 0 issue.ClosedUnix = 0
_, err = x.Cols("is_closed", "closed_unix").Update(issue) _, err = x.Cols("is_closed", "closed_unix").Update(issue)
assert.NoError(t, err) assert.NoError(t, err)
assert.NoError(t, changeMilestoneIssueStats(x.NewSession(), issue)) assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID))
CheckConsistencyFor(t, &Milestone{}) CheckConsistencyFor(t, &Milestone{})
} }