Skip to content

Commit

Permalink
Update milestone counters when issue is deleted (#21459)
Browse files Browse the repository at this point in the history
When actions besides "delete" are performed on issues, the milestone
counter is updated. However, since deleting issues goes through a
different code path, the associated milestone's count wasn't being
updated, resulting in inaccurate counts until another issue in the same
milestone had a non-delete action performed on it.

I verified this change fixes the inaccurate counts using a local docker
build.

Fixes #21254

Co-authored-by: Lunny Xiao <[email protected]>
  • Loading branch information
fantashley and lunny authored Oct 22, 2022
1 parent 154efa5 commit 82ecd3b
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# type Milestone struct {
# ID int64 `xorm:"pk autoincr"`
# IsClosed bool
# NumIssues int
# NumClosedIssues int
# Completeness int // Percentage(1-100).
# }
-
id: 1
is_closed: false
num_issues: 3
num_closed_issues: 1
completeness: 33
-
id: 2
is_closed: true
num_issues: 5
num_closed_issues: 5
completeness: 100
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# type Issue struct {
# ID int64 `xorm:"pk autoincr"`
# RepoID int64 `xorm:"INDEX UNIQUE(repo_index)"`
# Index int64 `xorm:"UNIQUE(repo_index)"` // Index in one repository.
# MilestoneID int64 `xorm:"INDEX"`
# IsClosed bool `xorm:"INDEX"`
# }
-
id: 1
repo_id: 1
index: 1
milestone_id: 1
is_closed: false
-
id: 2
repo_id: 1
index: 2
milestone_id: 1
is_closed: true
-
id: 4
repo_id: 1
index: 3
milestone_id: 1
is_closed: false
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# type Milestone struct {
# ID int64 `xorm:"pk autoincr"`
# IsClosed bool
# NumIssues int
# NumClosedIssues int
# Completeness int // Percentage(1-100).
# }
-
id: 1
is_closed: false
num_issues: 4
num_closed_issues: 2
completeness: 50
-
id: 2
is_closed: true
num_issues: 5
num_closed_issues: 5
completeness: 100
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,8 @@ var migrations = []Migration{
NewMigration("Create key/value table for system settings", createSystemSettingsTable),
// v228 -> v229
NewMigration("Add TeamInvite table", addTeamInviteTable),
// v229 -> v230
NewMigration("Update counts of all open milestones", updateOpenMilestoneCounts),
}

// GetCurrentDBVersion returns the current db version
Expand Down
47 changes: 47 additions & 0 deletions models/migrations/v229.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"fmt"

"code.gitea.io/gitea/models/issues"

"xorm.io/builder"
"xorm.io/xorm"
)

func updateOpenMilestoneCounts(x *xorm.Engine) error {
var openMilestoneIDs []int64
err := x.Table("milestone").Select("id").Where(builder.Neq{"is_closed": 1}).Find(&openMilestoneIDs)
if err != nil {
return fmt.Errorf("error selecting open milestone IDs: %w", err)
}

for _, id := range openMilestoneIDs {
_, err := x.ID(id).
SetExpr("num_issues", builder.Select("count(*)").From("issue").Where(
builder.Eq{"milestone_id": id},
)).
SetExpr("num_closed_issues", builder.Select("count(*)").From("issue").Where(
builder.Eq{
"milestone_id": id,
"is_closed": true,
},
)).
Update(&issues.Milestone{})
if err != nil {
return fmt.Errorf("error updating issue counts in milestone %d: %w", id, err)
}
_, err = x.Exec("UPDATE `milestone` SET completeness=100*num_closed_issues/(CASE WHEN num_issues > 0 THEN num_issues ELSE 1 END) WHERE id=?",
id,
)
if err != nil {
return fmt.Errorf("error setting completeness on milestone %d: %w", id, err)
}
}

return nil
}
46 changes: 46 additions & 0 deletions models/migrations/v229_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"testing"

"code.gitea.io/gitea/models/issues"

"github.com/stretchr/testify/assert"
)

func Test_updateOpenMilestoneCounts(t *testing.T) {
type ExpectedMilestone issues.Milestone

// Prepare and load the testing database
x, deferable := prepareTestEnv(t, 0, new(issues.Milestone), new(ExpectedMilestone), new(issues.Issue))
defer deferable()
if x == nil || t.Failed() {
return
}

if err := updateOpenMilestoneCounts(x); err != nil {
assert.NoError(t, err)
return
}

expected := []ExpectedMilestone{}
if err := x.Table("expected_milestone").Asc("id").Find(&expected); !assert.NoError(t, err) {
return
}

got := []issues.Milestone{}
if err := x.Table("milestone").Asc("id").Find(&got); !assert.NoError(t, err) {
return
}

for i, e := range expected {
got := got[i]
assert.Equal(t, e.ID, got.ID)
assert.Equal(t, e.NumIssues, got.NumIssues)
assert.Equal(t, e.NumClosedIssues, got.NumClosedIssues)
}
}
5 changes: 5 additions & 0 deletions services/issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ func deleteIssue(issue *issues_model.Issue) error {
return err
}

if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
return fmt.Errorf("error updating counters for milestone id %d: %w",
issue.MilestoneID, err)
}

if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID); err != nil {
return err
}
Expand Down

0 comments on commit 82ecd3b

Please sign in to comment.