From 783c11caa1dcca6db183503fe1701d50dbb9b1aa Mon Sep 17 00:00:00 2001 From: Bjarne Koll Date: Fri, 28 Feb 2020 22:43:06 +0100 Subject: [PATCH] Fix incorrect open issue count after commit close This commit fixes the bug that a repositories open issue value is not updated when an issue is closed through a commit that references such issue. More specifically, creating an issue on a repository with currently 0 open bugs lead to the repository showing one open issue. Closing such issue through a commit onto the branch this issue was targetting would close the issue correctly but failed to update the issue counter in the repositories header. Hence the repository would show one open issue while the open issue list remains empty. As this commit is a response to #10536, more information about the bug can be aquired there. Fixes #10536. --- models/repo.go | 21 +++++++++++++++++++++ modules/repofiles/action.go | 16 ++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/models/repo.go b/models/repo.go index ac9efaf7a..a081345dd 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1872,6 +1872,27 @@ func repoStatsCheck(ctx context.Context, checker *repoChecker) { } } +// FetchClosedIssueNum fetches the currently stored repositories NumClosedIssues from the database. +// To further clarify, this method does NOT: +// - Recalculate the number of closed issues in the database +// - Return the repositories value of the locally stored NumClosedIssues +// The method will only return an error if an actual SQL error occurred. +// If no value was found, a -1 and a nil error will be returned. +func (repo *Repository) FetchClosedIssueNum() (int, error) { + var value int + found, err := x.Table("repository"). + Select("num_closed_issues"). + Where("id = ?", repo.ID). + Get(&value) + if err != nil { + return 0, err + } + if !found { + return -1, nil + } + return value, nil +} + // CheckRepoStats checks the repository stats func CheckRepoStats(ctx context.Context) { log.Trace("Doing: CheckRepoStats") diff --git a/modules/repofiles/action.go b/modules/repofiles/action.go index 44ca285ef..c203dd848 100644 --- a/modules/repofiles/action.go +++ b/modules/repofiles/action.go @@ -60,6 +60,9 @@ func changeIssueStatus(repo *models.Repository, issue *models.Issue, doer *model // UpdateIssuesCommit checks if issues are manipulated by commit message. func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*repository.PushCommit, branchName string) error { + // Track if a single commit changed an issue, as we need to update the repo instance in that case. + commitChangedIssue := false + // Commits are appended in the reverse order. for i := len(commits) - 1; i >= 0; i-- { c := commits[i] @@ -142,9 +145,22 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r if err := changeIssueStatus(refRepo, refIssue, doer, close); err != nil { return err } + // Set the issue changed flag to true, to later on update the local repo instance + commitChangedIssue = true } } } + + if commitChangedIssue { + if fetchedValue, err := repo.FetchClosedIssueNum(); err == nil { + if fetchedValue > -1 { + repo.NumClosedIssues = fetchedValue + } + } else { + return err + } + } + return nil }