From fc87539b20954cbea6b97ac17b6e24f853347022 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Sep 2019 11:00:29 +0800 Subject: [PATCH 1/7] fix milestone num_issues --- models/issue.go | 4 +- models/issue_milestone.go | 68 ++++++++++++++-------------------- models/issue_milestone_test.go | 6 +-- 3 files changed, 32 insertions(+), 46 deletions(-) diff --git a/models/issue.go b/models/issue.go index 77712c0fec72e..adabe5be117f8 100644 --- a/models/issue.go +++ b/models/issue.go @@ -766,7 +766,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 } @@ -1130,7 +1130,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { opts.Issue.Index = inserted.Index 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 f8f414e7166e6..eb9a3dc82f8e0 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -311,71 +311,57 @@ 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 - } - - m, err := getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID) - if err != nil { - return err - } - - if issue.IsClosed { - m.NumOpenIssues-- - m.NumClosedIssues++ - } else { - m.NumOpenIssues++ - m.NumClosedIssues-- - } +func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { + _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?) WHERE id=?", + milestoneID, + milestoneID, + ) + return +} - return updateMilestone(e, m) +func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { + _, err = e.Exec("UPDATE `milestone` SET num_closed_issues=(SELECT count(*) FROM issue WHERE milestone_id=? AND is_closed=?) WHERE id=?", + milestoneID, + true, + 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 { + 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 = updateMilestone(e, m); err != nil { + if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil { return err } } - if err := issue.loadRepo(e); err != nil { - return err - } - if oldMilestoneID > 0 || issue.MilestoneID > 0 { + if err := issue.loadRepo(e); err != nil { + return err + } + 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 09c6ff7595aee..6f8548ec67155 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 = timeutil.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{}) } From 75e3b55dd9eac5b99ee52b3aa19fa121566c9f08 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Sep 2019 14:14:40 +0800 Subject: [PATCH 2/7] update missing completeness --- models/issue_milestone.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index eb9a3dc82f8e0..1662a0d2e0efc 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -312,7 +312,7 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) { } func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { - _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?), completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", milestoneID, milestoneID, ) @@ -320,7 +320,7 @@ func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { } func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { - _, err = e.Exec("UPDATE `milestone` SET num_closed_issues=(SELECT count(*) FROM issue WHERE milestone_id=? AND is_closed=?) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET num_closed_issues=(SELECT count(*) FROM issue WHERE milestone_id=? AND is_closed=?), completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", milestoneID, true, milestoneID, From 57517b6aa95171960311d504b101267984207d3f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Sep 2019 14:21:15 +0800 Subject: [PATCH 3/7] only update milestone closed number when closed issue is assigned a new milestone or clear milestone --- models/issue_milestone.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 1662a0d2e0efc..ac5508a70931e 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -337,8 +337,10 @@ func changeMilestoneAssign(e *xorm.Session, doer *User, issue *Issue, oldMilesto if err := updateMilestoneTotalNum(e, oldMilestoneID); err != nil { return err } - if err := updateMilestoneClosedNum(e, oldMilestoneID); err != nil { - return err + if issue.IsClosed { + if err := updateMilestoneClosedNum(e, oldMilestoneID); err != nil { + return err + } } } @@ -346,8 +348,10 @@ func changeMilestoneAssign(e *xorm.Session, doer *User, issue *Issue, oldMilesto if err := updateMilestoneTotalNum(e, issue.MilestoneID); err != nil { return err } - if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil { - return err + if issue.IsClosed { + if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil { + return err + } } } From db0c498c71642a204065d304b507a2ab694c285d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 19 Sep 2019 23:36:05 +0800 Subject: [PATCH 4/7] fix tests --- models/issue_milestone.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index ac5508a70931e..389558f5047c2 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -312,18 +312,31 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) { } func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { - _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?), completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + if _, err = e.Exec("UPDATE `milestone` SET num_issues=(SELECT count(*) FROM issue WHERE milestone_id=?) WHERE id=?", milestoneID, milestoneID, + ); err != nil { + return + } + + _, err = e.Exec("UPDATE `milestone` SET completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + milestoneID, ) + return } func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { - _, err = e.Exec("UPDATE `milestone` SET num_closed_issues=(SELECT count(*) FROM issue WHERE milestone_id=? AND is_closed=?), completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + 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 + } + + _, err = e.Exec("UPDATE `milestone` SET completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + milestoneID, ) return } From 067826a6649200b093ed11430ff0687b12cb80ec Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 25 Sep 2019 16:47:02 +0800 Subject: [PATCH 5/7] fix update milestone num --- models/issue_milestone.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 389558f5047c2..1cce196208af5 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -319,7 +319,7 @@ func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-sign(num_issues)) WHERE id=?", milestoneID, ) @@ -335,7 +335,7 @@ func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=if(num_issues>0,100*num_closed_issues/num_issues, 0) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-sign(num_issues)) WHERE id=?", milestoneID, ) return From 912d305cce880b5c3b2bc83cb8598ff4a31a3eee Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 29 Sep 2019 21:33:32 +0800 Subject: [PATCH 6/7] fix completeness calculate --- models/issue_milestone.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 1cce196208af5..7a3bf2bdb6819 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -319,7 +319,7 @@ func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-sign(num_issues)) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-(CASE WHEN num_issues > 0 THEN 1 ELSE 0 END)) WHERE id=?", milestoneID, ) @@ -335,7 +335,7 @@ func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-sign(num_issues)) WHERE id=?", + _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-(CASE WHEN num_issues > 0 THEN 1 ELSE 0 END)) WHERE id=?", milestoneID, ) return From 240a8ae7cc6498cf8a82f8da3595d076f98077f2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 29 Sep 2019 23:12:42 +0800 Subject: [PATCH 7/7] make completeness calucation more clear --- models/issue_milestone.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index 7a3bf2bdb6819..29e13689bf3ab 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -319,7 +319,7 @@ func updateMilestoneTotalNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-(CASE WHEN num_issues > 0 THEN 1 ELSE 0 END)) WHERE id=?", + _, 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, ) @@ -335,7 +335,7 @@ func updateMilestoneClosedNum(e Engine, milestoneID int64) (err error) { return } - _, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(num_issues+1-(CASE WHEN num_issues > 0 THEN 1 ELSE 0 END)) WHERE id=?", + _, 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