Update milestone counters on new issue. (#16183)
Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: zeripath <art27@cantab.net>
This commit is contained in:
		
							parent
							
								
									6a083a7234
								
							
						
					
					
						commit
						36c158bc93
					
				
					 4 changed files with 39 additions and 56 deletions
				
			
		|  | @ -141,6 +141,12 @@ func (milestone *Milestone) checkForConsistency(t *testing.T) { | |||
| 	actual := getCount(t, x.Where("is_closed=?", true), &Issue{MilestoneID: milestone.ID}) | ||||
| 	assert.EqualValues(t, milestone.NumClosedIssues, actual, | ||||
| 		"Unexpected number of closed issues for milestone %+v", milestone) | ||||
| 
 | ||||
| 	completeness := 0 | ||||
| 	if milestone.NumIssues > 0 { | ||||
| 		completeness = milestone.NumClosedIssues * 100 / milestone.NumIssues | ||||
| 	} | ||||
| 	assert.Equal(t, completeness, milestone.Completeness) | ||||
| } | ||||
| 
 | ||||
| func (label *Label) checkForConsistency(t *testing.T) { | ||||
|  |  | |||
|  | @ -647,9 +647,11 @@ func (issue *Issue) doChangeStatus(e *xorm.Session, doer *User, isMergePull bool | |||
| 	} | ||||
| 
 | ||||
| 	// Update issue count of milestone
 | ||||
| 	if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil { | ||||
| 	if issue.MilestoneID > 0 { | ||||
| 		if err := updateMilestoneCounters(e, issue.MilestoneID); err != nil { | ||||
| 			return nil, err | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if err := issue.updateClosedNum(e); err != nil { | ||||
| 		return nil, err | ||||
|  | @ -907,7 +909,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { | |||
| 	} | ||||
| 
 | ||||
| 	if opts.Issue.MilestoneID > 0 { | ||||
| 		if _, err = e.Exec("UPDATE `milestone` SET num_issues=num_issues+1 WHERE id=?", opts.Issue.MilestoneID); err != nil { | ||||
| 		if err := updateMilestoneCounters(e, opts.Issue.MilestoneID); err != nil { | ||||
| 			return err | ||||
| 		} | ||||
| 
 | ||||
|  |  | |||
|  | @ -129,8 +129,12 @@ func GetMilestoneByRepoIDANDName(repoID int64, name string) (*Milestone, error) | |||
| 
 | ||||
| // GetMilestoneByID returns the milestone via id .
 | ||||
| func GetMilestoneByID(id int64) (*Milestone, error) { | ||||
| 	return getMilestoneByID(x, id) | ||||
| } | ||||
| 
 | ||||
| func getMilestoneByID(e Engine, id int64) (*Milestone, error) { | ||||
| 	var m Milestone | ||||
| 	has, err := x.ID(id).Get(&m) | ||||
| 	has, err := e.ID(id).Get(&m) | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
| 	} else if !has { | ||||
|  | @ -155,10 +159,6 @@ func UpdateMilestone(m *Milestone, oldIsClosed bool) error { | |||
| 		return err | ||||
| 	} | ||||
| 
 | ||||
| 	if err := updateMilestoneCompleteness(sess, m.ID); err != nil { | ||||
| 		return err | ||||
| 	} | ||||
| 
 | ||||
| 	// if IsClosed changed, update milestone numbers of repository
 | ||||
| 	if oldIsClosed != m.IsClosed { | ||||
| 		if err := updateRepoMilestoneNum(sess, m.RepoID); err != nil { | ||||
|  | @ -171,23 +171,31 @@ func UpdateMilestone(m *Milestone, oldIsClosed bool) error { | |||
| 
 | ||||
| func updateMilestone(e Engine, m *Milestone) error { | ||||
| 	m.Name = strings.TrimSpace(m.Name) | ||||
| 	_, err := e.ID(m.ID).AllCols(). | ||||
| 	_, err := e.ID(m.ID).AllCols().Update(m) | ||||
| 	if err != nil { | ||||
| 		return err | ||||
| 	} | ||||
| 	return updateMilestoneCounters(e, m.ID) | ||||
| } | ||||
| 
 | ||||
| // updateMilestoneCounters calculates NumIssues, NumClosesIssues and Completeness
 | ||||
| func updateMilestoneCounters(e Engine, id int64) error { | ||||
| 	_, err := e.ID(id). | ||||
| 		SetExpr("num_issues", builder.Select("count(*)").From("issue").Where( | ||||
| 			builder.Eq{"milestone_id": m.ID}, | ||||
| 			builder.Eq{"milestone_id": id}, | ||||
| 		)). | ||||
| 		SetExpr("num_closed_issues", builder.Select("count(*)").From("issue").Where( | ||||
| 			builder.Eq{ | ||||
| 				"milestone_id": m.ID, | ||||
| 				"milestone_id": id, | ||||
| 				"is_closed":    true, | ||||
| 			}, | ||||
| 		)). | ||||
| 		Update(m) | ||||
| 		Update(&Milestone{}) | ||||
| 	if err != nil { | ||||
| 		return err | ||||
| } | ||||
| 
 | ||||
| func updateMilestoneCompleteness(e Engine, milestoneID int64) error { | ||||
| 	_, 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, | ||||
| 	} | ||||
| 	_, err = e.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?", | ||||
| 		id, | ||||
| 	) | ||||
| 	return err | ||||
| } | ||||
|  | @ -256,25 +264,15 @@ func changeMilestoneAssign(e *xorm.Session, doer *User, issue *Issue, oldMilesto | |||
| 	} | ||||
| 
 | ||||
| 	if oldMilestoneID > 0 { | ||||
| 		if err := updateMilestoneTotalNum(e, oldMilestoneID); err != nil { | ||||
| 		if err := updateMilestoneCounters(e, oldMilestoneID); err != nil { | ||||
| 			return err | ||||
| 		} | ||||
| 		if issue.IsClosed { | ||||
| 			if err := updateMilestoneClosedNum(e, oldMilestoneID); err != nil { | ||||
| 				return err | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if issue.MilestoneID > 0 { | ||||
| 		if err := updateMilestoneTotalNum(e, issue.MilestoneID); err != nil { | ||||
| 		if err := updateMilestoneCounters(e, issue.MilestoneID); err != nil { | ||||
| 			return err | ||||
| 		} | ||||
| 		if issue.IsClosed { | ||||
| 			if err := updateMilestoneClosedNum(e, issue.MilestoneID); err != nil { | ||||
| 				return err | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if oldMilestoneID > 0 || issue.MilestoneID > 0 { | ||||
|  | @ -622,29 +620,6 @@ func updateRepoMilestoneNum(e Engine, repoID int64) error { | |||
| 	return err | ||||
| } | ||||
| 
 | ||||
| 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 | ||||
| 	} | ||||
| 
 | ||||
| 	return updateMilestoneCompleteness(e, milestoneID) | ||||
| } | ||||
| 
 | ||||
| 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 updateMilestoneCompleteness(e, milestoneID) | ||||
| } | ||||
| 
 | ||||
| //  _____               _            _ _____ _
 | ||||
| // |_   _| __ __ _  ___| | _____  __| |_   _(_)_ __ ___   ___  ___
 | ||||
| //   | || '__/ _` |/ __| |/ / _ \/ _` | | | | | '_ ` _ \ / _ \/ __|
 | ||||
|  |  | |||
|  | @ -215,7 +215,7 @@ func TestChangeMilestoneStatus(t *testing.T) { | |||
| 	CheckConsistencyFor(t, &Repository{ID: milestone.RepoID}, &Milestone{}) | ||||
| } | ||||
| 
 | ||||
| func TestUpdateMilestoneClosedNum(t *testing.T) { | ||||
| func TestUpdateMilestoneCounters(t *testing.T) { | ||||
| 	assert.NoError(t, PrepareTestDatabase()) | ||||
| 	issue := AssertExistsAndLoadBean(t, &Issue{MilestoneID: 1}, | ||||
| 		"is_closed=0").(*Issue) | ||||
|  | @ -224,14 +224,14 @@ func TestUpdateMilestoneClosedNum(t *testing.T) { | |||
| 	issue.ClosedUnix = timeutil.TimeStampNow() | ||||
| 	_, err := x.ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) | ||||
| 	assert.NoError(t, err) | ||||
| 	assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID)) | ||||
| 	assert.NoError(t, updateMilestoneCounters(x, issue.MilestoneID)) | ||||
| 	CheckConsistencyFor(t, &Milestone{}) | ||||
| 
 | ||||
| 	issue.IsClosed = false | ||||
| 	issue.ClosedUnix = 0 | ||||
| 	_, err = x.ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) | ||||
| 	assert.NoError(t, err) | ||||
| 	assert.NoError(t, updateMilestoneClosedNum(x, issue.MilestoneID)) | ||||
| 	assert.NoError(t, updateMilestoneCounters(x, issue.MilestoneID)) | ||||
| 	CheckConsistencyFor(t, &Milestone{}) | ||||
| } | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue