From d9f1bbfa3476626c60fe5cbb66f0b2914315642b Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Fri, 6 Dec 2019 17:40:12 -0800 Subject: [PATCH 01/18] Show reviews labels - Review required, Approved and Changes requested - in the pull request lists. Related to issue #8257 --- models/branches.go | 14 ++++++++++ models/issue.go | 3 +++ models/pull.go | 31 ++++++++++++++++++++++ options/locale/locale_cs-CZ.ini | 3 +++ options/locale/locale_de-DE.ini | 3 +++ options/locale/locale_en-US.ini | 3 +++ options/locale/locale_es-ES.ini | 3 +++ options/locale/locale_fa-IR.ini | 3 +++ options/locale/locale_fr-FR.ini | 3 +++ options/locale/locale_ja-JP.ini | 3 +++ options/locale/locale_lv-LV.ini | 3 +++ options/locale/locale_pl-PL.ini | 3 +++ options/locale/locale_pt-BR.ini | 3 +++ options/locale/locale_tr-TR.ini | 3 +++ options/locale/locale_zh-CN.ini | 3 +++ routers/user/home.go | 4 +++ templates/repo/issue/list.tmpl | 4 ++- templates/repo/issue/milestone_issues.tmpl | 3 +++ templates/user/dashboard/issues.tmpl | 3 +++ 19 files changed, 97 insertions(+), 1 deletion(-) diff --git a/models/branches.go b/models/branches.go index cf4b4fe393205..9678a895eed69 100644 --- a/models/branches.go +++ b/models/branches.go @@ -165,6 +165,20 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) return approvals } +// GetRejectedReviewsCount returns the number of rejected reviews for pr. +func (protectBranch *ProtectedBranch) GetRejectedReviewsCount(pr *PullRequest) int64 { + rejects, err := x.Where("issue_id = ?", pr.Issue.ID). + And("type = ?", ReviewTypeReject). + And("official = ?", true). + Count(new(Review)) + if err != nil { + log.Error("GetRejectedReviewsCount: %v", err) + return 0 + } + + return rejects +} + // GetProtectedBranchByRepoID getting protected branch by repo ID func GetProtectedBranchByRepoID(repoID int64) ([]*ProtectedBranch, error) { protectedBranches := make([]*ProtectedBranch, 0) diff --git a/models/issue.go b/models/issue.go index 0a08a97fdd35b..0e121ade0d70a 100644 --- a/models/issue.go +++ b/models/issue.go @@ -180,6 +180,9 @@ func (issue *Issue) loadPullRequest(e Engine) (err error) { } issue.PullRequest.Issue = issue } + if issue.PullRequest != nil && issue.PullRequest.Issue == nil { + issue.PullRequest.Issue = issue + } return nil } diff --git a/models/pull.go b/models/pull.go index 388ee16b469e3..c6dd170f0f7a1 100644 --- a/models/pull.go +++ b/models/pull.go @@ -176,6 +176,37 @@ func (pr *PullRequest) loadProtectedBranch(e Engine) (err error) { return } +func (pr *PullRequest) requiredApprovals() int64 { + if pr.ProtectedBranch == nil { + if err := pr.loadProtectedBranch(x); err != nil { + log.Error("Error loading ProtectedBranch", err) + } + } + if pr.ProtectedBranch != nil { + return pr.ProtectedBranch.RequiredApprovals + } + return 0 +} + +// RequiresApproval returns true if this pull request requires approval, otherwise returns false +func (pr *PullRequest) RequiresApproval() bool { + return pr.requiredApprovals() > 0 +} + +// GetReviewLabel returns the localization label for the review of this pull request +func (pr *PullRequest) GetReviewLabel() string { + if pr.RequiresApproval() { + if pr.ProtectedBranch.HasEnoughApprovals(pr) { + return "repo.pulls.review_approved" + } + if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { + return "repo.pulls.review_rejected" + } + return "repo.pulls.review_required" + } + return "repo.pulls.review_approved" // by default +} + // GetDefaultMergeMessage returns default message used when merging pull request func (pr *PullRequest) GetDefaultMergeMessage() string { if pr.HeadRepo == nil { diff --git a/options/locale/locale_cs-CZ.ini b/options/locale/locale_cs-CZ.ini index 06c7307c06762..1e277ab5658f7 100644 --- a/options/locale/locale_cs-CZ.ini +++ b/options/locale/locale_cs-CZ.ini @@ -1047,6 +1047,9 @@ pulls.open_unmerged_pull_exists=`Nemůžete provést operaci znovuotevření pro pulls.status_checking=Některé kontroly jsou nedořešeny pulls.status_checks_success=Všechny kontroly byly úspěšné pulls.status_checks_error=Některé kontroly selhaly +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Nový milník milestones.open_tab=%d otevřených diff --git a/options/locale/locale_de-DE.ini b/options/locale/locale_de-DE.ini index 51504f8d7bf2e..b0155943eca16 100644 --- a/options/locale/locale_de-DE.ini +++ b/options/locale/locale_de-DE.ini @@ -1055,6 +1055,9 @@ pulls.open_unmerged_pull_exists=`Du kannst diesen Pull-Request nicht erneut öff pulls.status_checking=Einige Prüfungen sind noch ausstehend pulls.status_checks_success=Alle Prüfungen waren erfolgreich pulls.status_checks_error=Einige Prüfungen sind fehlgeschlagen +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Neuer Meilenstein milestones.open_tab=%d offen diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 98133cdab3bfb..4d24d4f615ea9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1070,6 +1070,9 @@ pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because pulls.status_checking = Some checks are pending pulls.status_checks_success = All checks were successful pulls.status_checks_error = Some checks failed +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new = New Milestone milestones.open_tab = %d Open diff --git a/options/locale/locale_es-ES.ini b/options/locale/locale_es-ES.ini index 02cb1b6b6d104..aface9360dbe7 100644 --- a/options/locale/locale_es-ES.ini +++ b/options/locale/locale_es-ES.ini @@ -1041,6 +1041,9 @@ pulls.open_unmerged_pull_exists=`No puede realizar la reapertura porque hay un p pulls.status_checking=Algunas comprobaciones están pendientes pulls.status_checks_success=Todas las comprobaciones han sido exitosas pulls.status_checks_error=Algunas comprobaciones han fallado +pulls.review_required = Revisión requerida +pulls.review_approved = Aprobada +pulls.review_rejected = Cambios solicitados milestones.new=Nuevo hito milestones.open_tab=%d abiertas diff --git a/options/locale/locale_fa-IR.ini b/options/locale/locale_fa-IR.ini index e25e0b1b12459..a6556f750b0e4 100644 --- a/options/locale/locale_fa-IR.ini +++ b/options/locale/locale_fa-IR.ini @@ -1022,6 +1022,9 @@ pulls.open_unmerged_pull_exists=`شما نمی‌توانید یک عملیات pulls.status_checking=برخی از بررسی‎ها در حال تعلیق هستند pulls.status_checks_success=تمامی بررسی‎ها موفق بودند pulls.status_checks_error=برخی بررسی‎ها با مشکل مواجه شد +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=نقطه عطف جدید milestones.open_tab=%d باز شد diff --git a/options/locale/locale_fr-FR.ini b/options/locale/locale_fr-FR.ini index a2c7155331b46..bebeb7aa5b478 100644 --- a/options/locale/locale_fr-FR.ini +++ b/options/locale/locale_fr-FR.ini @@ -1058,6 +1058,9 @@ pulls.open_unmerged_pull_exists=`Vous ne pouvez pas ré-ouvrir cette demande de pulls.status_checking=Certains contrôles sont en attente pulls.status_checks_success=Tous les contrôles ont réussi pulls.status_checks_error=Certains contrôles ont échoué +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Nouveau jalon milestones.open_tab=%d Ouvert diff --git a/options/locale/locale_ja-JP.ini b/options/locale/locale_ja-JP.ini index b54bb97f1dc88..a51d18ef38072 100644 --- a/options/locale/locale_ja-JP.ini +++ b/options/locale/locale_ja-JP.ini @@ -1069,6 +1069,9 @@ pulls.open_unmerged_pull_exists=`同じ条件のプルリクエスト (#%d) が pulls.status_checking=いくつかの検証が待機中です pulls.status_checks_success=検証はすべて成功しました pulls.status_checks_error=検証がいくつか失敗しました +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=新しいマイルストーン milestones.open_tab=%d件 オープン中 diff --git a/options/locale/locale_lv-LV.ini b/options/locale/locale_lv-LV.ini index a98c8d2a707e6..367315f08ef59 100644 --- a/options/locale/locale_lv-LV.ini +++ b/options/locale/locale_lv-LV.ini @@ -1006,6 +1006,9 @@ pulls.open_unmerged_pull_exists=`Jūs nevarat veikt atkārtotas atvēršanas dar pulls.status_checking=Dažas pārbaudes vēl tiek veiktas pulls.status_checks_success=Visas pārbaudes ir veiksmīgas pulls.status_checks_error=Dažas pārbaudes bija neveiksmīgas +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Jauns atskaites punkts milestones.open_tab=%d atvērti diff --git a/options/locale/locale_pl-PL.ini b/options/locale/locale_pl-PL.ini index ef1c52de9afad..2a39c5ce52ad3 100644 --- a/options/locale/locale_pl-PL.ini +++ b/options/locale/locale_pl-PL.ini @@ -1003,6 +1003,9 @@ pulls.open_unmerged_pull_exists=`Nie możesz wykonać operacji ponownego otwarci pulls.status_checking=Niektóre etapy są w toku pulls.status_checks_success=Wszystkie etapy powiodły się pulls.status_checks_error=Niektóre etapy nie powiodły się +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Nowy kamień milowy milestones.open_tab=Otwarte %d diff --git a/options/locale/locale_pt-BR.ini b/options/locale/locale_pt-BR.ini index 6812fd6408fd2..f22e1078ab6b2 100644 --- a/options/locale/locale_pt-BR.ini +++ b/options/locale/locale_pt-BR.ini @@ -1069,6 +1069,9 @@ pulls.open_unmerged_pull_exists=`Não é possível executar uma operação de re pulls.status_checking=Algumas verificações estão pendentes pulls.status_checks_success=Todas as verificações foram bem sucedidas pulls.status_checks_error=Algumas verificações falharam +pulls.review_required = Revisão requerida +pulls.review_approved = Aprovado +pulls.review_rejected = Alterações solicitadas milestones.new=Novo marco milestones.open_tab=%d Aberto diff --git a/options/locale/locale_tr-TR.ini b/options/locale/locale_tr-TR.ini index 81ed475d54ef2..b40565521e6b5 100644 --- a/options/locale/locale_tr-TR.ini +++ b/options/locale/locale_tr-TR.ini @@ -1012,6 +1012,9 @@ pulls.open_unmerged_pull_exists=`Aynı özelliklere sahip bekleyen bir çekme is pulls.status_checking=Bazı denetlemeler beklemede pulls.status_checks_success=Tüm denetlemeler başarılı oldu pulls.status_checks_error=Bazı denetlemeler başarısız oldu +pulls.review_required = Review required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=Yeni Kilometre Taşı milestones.open_tab=%d Açık diff --git a/options/locale/locale_zh-CN.ini b/options/locale/locale_zh-CN.ini index fd5f4f7a892de..931a3b62994ff 100644 --- a/options/locale/locale_zh-CN.ini +++ b/options/locale/locale_zh-CN.ini @@ -1069,6 +1069,9 @@ pulls.open_unmerged_pull_exists=`您不能执行重新打开操作, 因为已经 pulls.status_checking=一些检测仍在等待运行 pulls.status_checks_success=所有检测均成功 pulls.status_checks_error=一些检测失败 +pulls.review_required = Review Required +pulls.review_approved = Approved +pulls.review_rejected = Changes requested milestones.new=新的里程碑 milestones.open_tab=%d 开启中 diff --git a/routers/user/home.go b/routers/user/home.go index 2eff889105ab2..9124b9795d5b8 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -335,6 +335,10 @@ func Issues(ctx *context.Context) { issue.Repo = showReposMap[issue.RepoID] if isPullList { + if err := issue.LoadPullRequest(); err != nil { + ctx.ServerError("LoadPullRequest", err) + return + } commitStatus[issue.PullRequest.ID], _ = issue.PullRequest.GetLastCommitStatus() } } diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 41f90b1c134cf..6354c1bd3fefa 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -239,7 +239,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} - + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{end}}{{end}} {{if .Milestone}} {{.Milestone.Name}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index ad3f0b5c8625b..731be2309cf95 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -206,6 +206,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{end}}{{end}} {{if .Ref}} {{.Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 0310f5fafe39d..e26d7d11217aa 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -124,6 +124,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{end}}{{end}} {{if .Milestone}} {{.Milestone.Name}} From 4ff54062e166fee76954245811115ab0f8559c07 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Fri, 6 Dec 2019 22:59:32 -0800 Subject: [PATCH 02/18] Removing non translated labels --- options/locale/locale_cs-CZ.ini | 3 --- options/locale/locale_de-DE.ini | 3 --- options/locale/locale_fa-IR.ini | 3 --- options/locale/locale_fr-FR.ini | 3 --- options/locale/locale_ja-JP.ini | 3 --- options/locale/locale_lv-LV.ini | 3 --- options/locale/locale_pl-PL.ini | 3 --- options/locale/locale_tr-TR.ini | 3 --- options/locale/locale_zh-CN.ini | 3 --- 9 files changed, 27 deletions(-) diff --git a/options/locale/locale_cs-CZ.ini b/options/locale/locale_cs-CZ.ini index 1e277ab5658f7..06c7307c06762 100644 --- a/options/locale/locale_cs-CZ.ini +++ b/options/locale/locale_cs-CZ.ini @@ -1047,9 +1047,6 @@ pulls.open_unmerged_pull_exists=`Nemůžete provést operaci znovuotevření pro pulls.status_checking=Některé kontroly jsou nedořešeny pulls.status_checks_success=Všechny kontroly byly úspěšné pulls.status_checks_error=Některé kontroly selhaly -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Nový milník milestones.open_tab=%d otevřených diff --git a/options/locale/locale_de-DE.ini b/options/locale/locale_de-DE.ini index b0155943eca16..51504f8d7bf2e 100644 --- a/options/locale/locale_de-DE.ini +++ b/options/locale/locale_de-DE.ini @@ -1055,9 +1055,6 @@ pulls.open_unmerged_pull_exists=`Du kannst diesen Pull-Request nicht erneut öff pulls.status_checking=Einige Prüfungen sind noch ausstehend pulls.status_checks_success=Alle Prüfungen waren erfolgreich pulls.status_checks_error=Einige Prüfungen sind fehlgeschlagen -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Neuer Meilenstein milestones.open_tab=%d offen diff --git a/options/locale/locale_fa-IR.ini b/options/locale/locale_fa-IR.ini index a6556f750b0e4..e25e0b1b12459 100644 --- a/options/locale/locale_fa-IR.ini +++ b/options/locale/locale_fa-IR.ini @@ -1022,9 +1022,6 @@ pulls.open_unmerged_pull_exists=`شما نمی‌توانید یک عملیات pulls.status_checking=برخی از بررسی‎ها در حال تعلیق هستند pulls.status_checks_success=تمامی بررسی‎ها موفق بودند pulls.status_checks_error=برخی بررسی‎ها با مشکل مواجه شد -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=نقطه عطف جدید milestones.open_tab=%d باز شد diff --git a/options/locale/locale_fr-FR.ini b/options/locale/locale_fr-FR.ini index bebeb7aa5b478..a2c7155331b46 100644 --- a/options/locale/locale_fr-FR.ini +++ b/options/locale/locale_fr-FR.ini @@ -1058,9 +1058,6 @@ pulls.open_unmerged_pull_exists=`Vous ne pouvez pas ré-ouvrir cette demande de pulls.status_checking=Certains contrôles sont en attente pulls.status_checks_success=Tous les contrôles ont réussi pulls.status_checks_error=Certains contrôles ont échoué -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Nouveau jalon milestones.open_tab=%d Ouvert diff --git a/options/locale/locale_ja-JP.ini b/options/locale/locale_ja-JP.ini index a51d18ef38072..b54bb97f1dc88 100644 --- a/options/locale/locale_ja-JP.ini +++ b/options/locale/locale_ja-JP.ini @@ -1069,9 +1069,6 @@ pulls.open_unmerged_pull_exists=`同じ条件のプルリクエスト (#%d) が pulls.status_checking=いくつかの検証が待機中です pulls.status_checks_success=検証はすべて成功しました pulls.status_checks_error=検証がいくつか失敗しました -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=新しいマイルストーン milestones.open_tab=%d件 オープン中 diff --git a/options/locale/locale_lv-LV.ini b/options/locale/locale_lv-LV.ini index 367315f08ef59..a98c8d2a707e6 100644 --- a/options/locale/locale_lv-LV.ini +++ b/options/locale/locale_lv-LV.ini @@ -1006,9 +1006,6 @@ pulls.open_unmerged_pull_exists=`Jūs nevarat veikt atkārtotas atvēršanas dar pulls.status_checking=Dažas pārbaudes vēl tiek veiktas pulls.status_checks_success=Visas pārbaudes ir veiksmīgas pulls.status_checks_error=Dažas pārbaudes bija neveiksmīgas -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Jauns atskaites punkts milestones.open_tab=%d atvērti diff --git a/options/locale/locale_pl-PL.ini b/options/locale/locale_pl-PL.ini index 2a39c5ce52ad3..ef1c52de9afad 100644 --- a/options/locale/locale_pl-PL.ini +++ b/options/locale/locale_pl-PL.ini @@ -1003,9 +1003,6 @@ pulls.open_unmerged_pull_exists=`Nie możesz wykonać operacji ponownego otwarci pulls.status_checking=Niektóre etapy są w toku pulls.status_checks_success=Wszystkie etapy powiodły się pulls.status_checks_error=Niektóre etapy nie powiodły się -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Nowy kamień milowy milestones.open_tab=Otwarte %d diff --git a/options/locale/locale_tr-TR.ini b/options/locale/locale_tr-TR.ini index b40565521e6b5..81ed475d54ef2 100644 --- a/options/locale/locale_tr-TR.ini +++ b/options/locale/locale_tr-TR.ini @@ -1012,9 +1012,6 @@ pulls.open_unmerged_pull_exists=`Aynı özelliklere sahip bekleyen bir çekme is pulls.status_checking=Bazı denetlemeler beklemede pulls.status_checks_success=Tüm denetlemeler başarılı oldu pulls.status_checks_error=Bazı denetlemeler başarısız oldu -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=Yeni Kilometre Taşı milestones.open_tab=%d Açık diff --git a/options/locale/locale_zh-CN.ini b/options/locale/locale_zh-CN.ini index 931a3b62994ff..fd5f4f7a892de 100644 --- a/options/locale/locale_zh-CN.ini +++ b/options/locale/locale_zh-CN.ini @@ -1069,9 +1069,6 @@ pulls.open_unmerged_pull_exists=`您不能执行重新打开操作, 因为已经 pulls.status_checking=一些检测仍在等待运行 pulls.status_checks_success=所有检测均成功 pulls.status_checks_error=一些检测失败 -pulls.review_required = Review Required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested milestones.new=新的里程碑 milestones.open_tab=%d 开启中 From 46b9e29e8377d086dccd403baaecec21d3a3ca00 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Fri, 6 Dec 2019 23:06:59 -0800 Subject: [PATCH 03/18] Checking rejected reviews before enough approvals. --- models/pull.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/pull.go b/models/pull.go index c6dd170f0f7a1..b4e77810033ed 100644 --- a/models/pull.go +++ b/models/pull.go @@ -196,12 +196,12 @@ func (pr *PullRequest) RequiresApproval() bool { // GetReviewLabel returns the localization label for the review of this pull request func (pr *PullRequest) GetReviewLabel() string { if pr.RequiresApproval() { - if pr.ProtectedBranch.HasEnoughApprovals(pr) { - return "repo.pulls.review_approved" - } if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { return "repo.pulls.review_rejected" } + if pr.ProtectedBranch.HasEnoughApprovals(pr) { + return "repo.pulls.review_approved" + } return "repo.pulls.review_required" } return "repo.pulls.review_approved" // by default From 8236a04d8394b850b30b90731ae77af88ddedf29 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sat, 7 Dec 2019 00:13:01 -0800 Subject: [PATCH 04/18] Showing number of reviews required, approvals and changes requests --- models/pull.go | 17 ++++++++++------- options/locale/locale_en-US.ini | 6 +++--- options/locale/locale_es-ES.ini | 6 +++--- options/locale/locale_pt-BR.ini | 6 +++--- templates/repo/issue/list.tmpl | 2 +- templates/repo/issue/milestone_issues.tmpl | 2 +- templates/user/dashboard/issues.tmpl | 2 +- 7 files changed, 22 insertions(+), 19 deletions(-) diff --git a/models/pull.go b/models/pull.go index b4e77810033ed..6fe5d05c63048 100644 --- a/models/pull.go +++ b/models/pull.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "github.com/unknwon/com" + "github.com/unknwon/i18n" ) var pullRequestQueue = sync.NewUniqueQueue(setting.Repository.PullRequestQueueLength) @@ -193,18 +194,20 @@ func (pr *PullRequest) RequiresApproval() bool { return pr.requiredApprovals() > 0 } -// GetReviewLabel returns the localization label for the review of this pull request -func (pr *PullRequest) GetReviewLabel() string { +// GetReviewLabel returns the formatted review label with counter for the review of this pull request +func (pr *PullRequest) GetReviewLabel(lang string) string { if pr.RequiresApproval() { + rejections := pr.ProtectedBranch.GetRejectedReviewsCount(pr) if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { - return "repo.pulls.review_rejected" + return i18n.Tr(lang, "repo.pulls.review_rejected", rejections) } - if pr.ProtectedBranch.HasEnoughApprovals(pr) { - return "repo.pulls.review_approved" + approvals := pr.ProtectedBranch.GetGrantedApprovalsCount(pr) + if approvals >= pr.ProtectedBranch.RequiredApprovals { + return i18n.Tr(lang, "repo.pulls.review_approved", approvals) } - return "repo.pulls.review_required" + return i18n.Tr(lang, "repo.pulls.review_required", pr.ProtectedBranch.RequiredApprovals-approvals) } - return "repo.pulls.review_approved" // by default + return i18n.Tr(lang, "repo.pulls.review_approved", 0) // by default } // GetDefaultMergeMessage returns default message used when merging pull request diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4d24d4f615ea9..2feff619e2af5 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1070,9 +1070,9 @@ pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because pulls.status_checking = Some checks are pending pulls.status_checks_success = All checks were successful pulls.status_checks_error = Some checks failed -pulls.review_required = Review required -pulls.review_approved = Approved -pulls.review_rejected = Changes requested +pulls.review_required = Review required %d +pulls.review_approved = Approved %d +pulls.review_rejected = Changes requested %d milestones.new = New Milestone milestones.open_tab = %d Open diff --git a/options/locale/locale_es-ES.ini b/options/locale/locale_es-ES.ini index aface9360dbe7..e8ad965542e01 100644 --- a/options/locale/locale_es-ES.ini +++ b/options/locale/locale_es-ES.ini @@ -1041,9 +1041,9 @@ pulls.open_unmerged_pull_exists=`No puede realizar la reapertura porque hay un p pulls.status_checking=Algunas comprobaciones están pendientes pulls.status_checks_success=Todas las comprobaciones han sido exitosas pulls.status_checks_error=Algunas comprobaciones han fallado -pulls.review_required = Revisión requerida -pulls.review_approved = Aprobada -pulls.review_rejected = Cambios solicitados +pulls.review_required = Revisión requerida %d +pulls.review_approved = Aprobada %d +pulls.review_rejected = Cambios solicitados %d milestones.new=Nuevo hito milestones.open_tab=%d abiertas diff --git a/options/locale/locale_pt-BR.ini b/options/locale/locale_pt-BR.ini index f22e1078ab6b2..c51db8f3ac19f 100644 --- a/options/locale/locale_pt-BR.ini +++ b/options/locale/locale_pt-BR.ini @@ -1069,9 +1069,9 @@ pulls.open_unmerged_pull_exists=`Não é possível executar uma operação de re pulls.status_checking=Algumas verificações estão pendentes pulls.status_checks_success=Todas as verificações foram bem sucedidas pulls.status_checks_error=Algumas verificações falharam -pulls.review_required = Revisão requerida -pulls.review_approved = Aprovado -pulls.review_rejected = Alterações solicitadas +pulls.review_required = Revisão requerida %d +pulls.review_approved = Aprovado %d +pulls.review_rejected = Alterações solicitadas %d milestones.new=Novo marco milestones.open_tab=%d Aberto diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 6354c1bd3fefa..43c170ea2c7cd 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -240,7 +240,7 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} {{end}}{{end}} {{if .Milestone}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 731be2309cf95..5584327858b09 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -207,7 +207,7 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} {{end}}{{end}} {{if .Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index e26d7d11217aa..5e0ceac2e0dca 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -125,7 +125,7 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - • {{$.i18n.Tr .PullRequest.GetReviewLabel }} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} {{end}}{{end}} {{if .Milestone}} From 6686bd7f6fe81e1fd98162c4fe473a95d5f6882a Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sat, 7 Dec 2019 12:28:50 -0800 Subject: [PATCH 05/18] Removing translations as they are addressed through Crowdin. --- options/locale/locale_es-ES.ini | 3 --- options/locale/locale_pt-BR.ini | 3 --- 2 files changed, 6 deletions(-) diff --git a/options/locale/locale_es-ES.ini b/options/locale/locale_es-ES.ini index e8ad965542e01..02cb1b6b6d104 100644 --- a/options/locale/locale_es-ES.ini +++ b/options/locale/locale_es-ES.ini @@ -1041,9 +1041,6 @@ pulls.open_unmerged_pull_exists=`No puede realizar la reapertura porque hay un p pulls.status_checking=Algunas comprobaciones están pendientes pulls.status_checks_success=Todas las comprobaciones han sido exitosas pulls.status_checks_error=Algunas comprobaciones han fallado -pulls.review_required = Revisión requerida %d -pulls.review_approved = Aprobada %d -pulls.review_rejected = Cambios solicitados %d milestones.new=Nuevo hito milestones.open_tab=%d abiertas diff --git a/options/locale/locale_pt-BR.ini b/options/locale/locale_pt-BR.ini index c51db8f3ac19f..6812fd6408fd2 100644 --- a/options/locale/locale_pt-BR.ini +++ b/options/locale/locale_pt-BR.ini @@ -1069,9 +1069,6 @@ pulls.open_unmerged_pull_exists=`Não é possível executar uma operação de re pulls.status_checking=Algumas verificações estão pendentes pulls.status_checks_success=Todas as verificações foram bem sucedidas pulls.status_checks_error=Algumas verificações falharam -pulls.review_required = Revisão requerida %d -pulls.review_approved = Aprovado %d -pulls.review_rejected = Alterações solicitadas %d milestones.new=Novo marco milestones.open_tab=%d Aberto From 3761e06f58cd4d7ad1ea08f673547287edc56706 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sat, 7 Dec 2019 12:46:33 -0800 Subject: [PATCH 06/18] Setting Issue's reference in PullRequest when loading PullRequests. --- models/issue_list.go | 1 + routers/user/home.go | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/models/issue_list.go b/models/issue_list.go index e3516b55b948e..8ec1ffa3cafcc 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -308,6 +308,7 @@ func (issues IssueList) loadPullRequests(e Engine) error { for _, issue := range issues { issue.PullRequest = pullRequestMaps[issue.ID] + issue.PullRequest.Issue = issue } return nil } diff --git a/routers/user/home.go b/routers/user/home.go index 9124b9795d5b8..2eff889105ab2 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -335,10 +335,6 @@ func Issues(ctx *context.Context) { issue.Repo = showReposMap[issue.RepoID] if isPullList { - if err := issue.LoadPullRequest(); err != nil { - ctx.ServerError("LoadPullRequest", err) - return - } commitStatus[issue.PullRequest.ID], _ = issue.PullRequest.GetLastCommitStatus() } } From 582d7ef9e89c3292ede3ecd6ae258bd2574bb14a Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sat, 7 Dec 2019 14:01:01 -0800 Subject: [PATCH 07/18] Fix runtime error: invalid memory address or nil pointer dereference. --- models/issue_list.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/issue_list.go b/models/issue_list.go index 8ec1ffa3cafcc..542edb3593405 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -308,7 +308,9 @@ func (issues IssueList) loadPullRequests(e Engine) error { for _, issue := range issues { issue.PullRequest = pullRequestMaps[issue.ID] - issue.PullRequest.Issue = issue + if issue.PullRequest != nil { + issue.PullRequest.Issue = issue + } } return nil } From d723929d84ca0a84aeeeb586fc01c62203dd2030 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 8 Dec 2019 11:17:55 -0800 Subject: [PATCH 08/18] Simplifying conditional expressions. --- templates/repo/issue/list.tmpl | 4 ++-- templates/repo/issue/milestone_issues.tmpl | 6 +++--- templates/user/dashboard/issues.tmpl | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 43c170ea2c7cd..99b79f0e292e7 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -239,9 +239,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} - {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{if and .IsPull .PullRequest.RequiresApproval}} {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}}{{end}} + {{end}} {{if .Milestone}} {{.Milestone.Name}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 5584327858b09..3d88bea6fb0f8 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -206,9 +206,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} - {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}}{{end}} + {{if and .IsPull .PullRequest.RequiresApproval}} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{end}} {{if .Ref}} {{.Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 5e0ceac2e0dca..0ec3d4c682aef 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -124,9 +124,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} - {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}}{{end}} + {{if and .IsPull .PullRequest.RequiresApproval}} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{end}} {{if .Milestone}} {{.Milestone.Name}} From 9a6f4a8a767aad5ba7e702aca840ea6962d28f5e Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 8 Dec 2019 11:49:23 -0800 Subject: [PATCH 09/18] Revert "Simplifying conditional expressions." This reverts commit d723929d --- templates/repo/issue/list.tmpl | 4 ++-- templates/repo/issue/milestone_issues.tmpl | 6 +++--- templates/user/dashboard/issues.tmpl | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 99b79f0e292e7..43c170ea2c7cd 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -239,9 +239,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} - {{if and .IsPull .PullRequest.RequiresApproval}} + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}} + {{end}}{{end}} {{if .Milestone}} {{.Milestone.Name}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 3d88bea6fb0f8..5584327858b09 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -206,9 +206,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} - {{if and .IsPull .PullRequest.RequiresApproval}} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}} + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{end}}{{end}} {{if .Ref}} {{.Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 0ec3d4c682aef..5e0ceac2e0dca 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -124,9 +124,9 @@ {{else}} {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} - {{if and .IsPull .PullRequest.RequiresApproval}} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} - {{end}} + {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{end}}{{end}} {{if .Milestone}} {{.Milestone.Name}} From 3f93cfe9b62365d6a380ec6502506a28eb905a76 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 2 Feb 2020 19:06:47 -0800 Subject: [PATCH 10/18] Calling pr.LoadIssue() to load issue info from database. --- models/branches.go | 2 +- models/issue.go | 10 ++++++++-- models/issue_list.go | 4 +++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/models/branches.go b/models/branches.go index 0519742fa70f7..081388b54dcce 100644 --- a/models/branches.go +++ b/models/branches.go @@ -175,7 +175,7 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) // GetRejectedReviewsCount returns the number of rejected reviews for pr. func (protectBranch *ProtectedBranch) GetRejectedReviewsCount(pr *PullRequest) int64 { - rejects, err := x.Where("issue_id = ?", pr.Issue.ID). + rejects, err := x.Where("issue_id = ?", pr.IssueID). And("type = ?", ReviewTypeReject). And("official = ?", true). Count(new(Review)) diff --git a/models/issue.go b/models/issue.go index e5b4d305feb69..78698b03cc609 100644 --- a/models/issue.go +++ b/models/issue.go @@ -178,10 +178,16 @@ func (issue *Issue) loadPullRequest(e Engine) (err error) { } return fmt.Errorf("getPullRequestByIssueID [%d]: %v", issue.ID, err) } - issue.PullRequest.Issue = issue + err = issue.PullRequest.LoadIssue() + if err != nil { + return fmt.Errorf("LoadIssue: %v", err) + } } if issue.PullRequest != nil && issue.PullRequest.Issue == nil { - issue.PullRequest.Issue = issue + issue.PullRequest.LoadIssue() + if err != nil { + return fmt.Errorf("LoadIssue: %v", err) + } } return nil } diff --git a/models/issue_list.go b/models/issue_list.go index a9af18e7cdc19..c86abef7747ee 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -309,7 +309,9 @@ func (issues IssueList) loadPullRequests(e Engine) error { for _, issue := range issues { issue.PullRequest = pullRequestMaps[issue.ID] if issue.PullRequest != nil { - issue.PullRequest.Issue = issue + if err := issue.PullRequest.LoadIssue(); err != nil { + return err + } } } return nil From d3657fddabcb3abbb2bb63cb5af69c4a36382f76 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 3 Feb 2020 04:38:16 +0100 Subject: [PATCH 11/18] suggestions --- models/issue.go | 11 +---------- models/issue_list.go | 5 ----- templates/repo/issue/milestone_issues.tmpl | 2 +- templates/user/dashboard/issues.tmpl | 2 +- 4 files changed, 3 insertions(+), 17 deletions(-) diff --git a/models/issue.go b/models/issue.go index 78698b03cc609..c0be987ac9fbd 100644 --- a/models/issue.go +++ b/models/issue.go @@ -178,16 +178,7 @@ func (issue *Issue) loadPullRequest(e Engine) (err error) { } return fmt.Errorf("getPullRequestByIssueID [%d]: %v", issue.ID, err) } - err = issue.PullRequest.LoadIssue() - if err != nil { - return fmt.Errorf("LoadIssue: %v", err) - } - } - if issue.PullRequest != nil && issue.PullRequest.Issue == nil { - issue.PullRequest.LoadIssue() - if err != nil { - return fmt.Errorf("LoadIssue: %v", err) - } + issue.PullRequest.Issue = issue } return nil } diff --git a/models/issue_list.go b/models/issue_list.go index c86abef7747ee..4554f906c48c3 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -308,11 +308,6 @@ func (issues IssueList) loadPullRequests(e Engine) error { for _, issue := range issues { issue.PullRequest = pullRequestMaps[issue.ID] - if issue.PullRequest != nil { - if err := issue.PullRequest.LoadIssue(); err != nil { - return err - } - } } return nil } diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 88e3b67aae553..d11abaf7ef587 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -207,7 +207,7 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} {{end}}{{end}} {{if .Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 28b31c3920531..80a6495a6ed6f 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -125,7 +125,7 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} {{end}}{{end}} {{if .Milestone}} From 4f792ec1a1642142ea92a7a4041931eddcd210d2 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 2 Feb 2020 20:29:13 -0800 Subject: [PATCH 12/18] Getting labels and counters of reviews from two methods, thus removing the i18n from the model. --- models/pull.go | 28 +++++++++++++++------- templates/repo/issue/list.tmpl | 4 +++- templates/repo/issue/milestone_issues.tmpl | 4 +++- templates/user/dashboard/issues.tmpl | 4 +++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/models/pull.go b/models/pull.go index 4ff29fb984bc9..6c36207bae66f 100644 --- a/models/pull.go +++ b/models/pull.go @@ -14,8 +14,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - - "github.com/unknwon/i18n" ) // PullRequestType defines pull request type @@ -190,20 +188,34 @@ func (pr *PullRequest) RequiresApproval() bool { return pr.requiredApprovals() > 0 } -// GetReviewLabel returns the formatted review label with counter for the review of this pull request -func (pr *PullRequest) GetReviewLabel(lang string) string { +// GetReviewLabel returns the localization label for the review of this pull request +func (pr *PullRequest) GetReviewLabel() string { + if pr.RequiresApproval() { + if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { + return "repo.pulls.review_rejected" + } + if pr.ProtectedBranch.GetGrantedApprovalsCount(pr) >= pr.ProtectedBranch.RequiredApprovals { + return "repo.pulls.review_approved" + } + return "repo.pulls.review_required" + } + return "repo.pulls.review_approved" // by default +} + +// GetReviewLabel returns the counter for the review of this pull request +func (pr *PullRequest) GetReviewCount() int64 { if pr.RequiresApproval() { rejections := pr.ProtectedBranch.GetRejectedReviewsCount(pr) if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { - return i18n.Tr(lang, "repo.pulls.review_rejected", rejections) + return rejections } approvals := pr.ProtectedBranch.GetGrantedApprovalsCount(pr) if approvals >= pr.ProtectedBranch.RequiredApprovals { - return i18n.Tr(lang, "repo.pulls.review_approved", approvals) + return approvals } - return i18n.Tr(lang, "repo.pulls.review_required", pr.ProtectedBranch.RequiredApprovals-approvals) + return pr.ProtectedBranch.RequiredApprovals - approvals } - return i18n.Tr(lang, "repo.pulls.review_approved", 0) // by default + return 0 // by default } // GetDefaultMergeMessage returns default message used when merging pull request diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 2ca98117f0a6b..91f9a564ce358 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -240,7 +240,9 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + + {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{end}}{{end}} {{if .Milestone}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index d11abaf7ef587..9fd67d8aa28f0 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -207,7 +207,9 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + + {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{end}}{{end}} {{if .Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 80a6495a6ed6f..7aa290958e55e 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -125,7 +125,9 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} - {{.PullRequest.GetReviewLabel $.i18n.Lang | Safe}} + + {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{end}}{{end}} {{if .Milestone}} From fbd4c090542e6c0425a4afeb7a20c15e401e372f Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 2 Feb 2020 20:44:51 -0800 Subject: [PATCH 13/18] Moved methods GetRejectedReviewsCount, HasEnoughApprovals and GetGrantedApprovalsCount from ProtectedBranch to PullRequest. --- models/branches.go | 39 --------------------------------- models/pull.go | 49 +++++++++++++++++++++++++++++++++++++----- models/pull_sign.go | 2 +- routers/repo/issue.go | 4 ++-- services/pull/merge.go | 2 +- 5 files changed, 48 insertions(+), 48 deletions(-) diff --git a/models/branches.go b/models/branches.go index 081388b54dcce..540b6614ffc66 100644 --- a/models/branches.go +++ b/models/branches.go @@ -148,45 +148,6 @@ func (protectBranch *ProtectedBranch) isUserOfficialReviewer(e Engine, user *Use return inTeam, nil } -// HasEnoughApprovals returns true if pr has enough granted approvals. -func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool { - if protectBranch.RequiredApprovals == 0 { - return true - } - return protectBranch.GetGrantedApprovalsCount(pr) >= protectBranch.RequiredApprovals -} - -// GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist. -func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 { - sess := x.Where("issue_id = ?", pr.IssueID). - And("type = ?", ReviewTypeApprove). - And("official = ?", true) - if protectBranch.DismissStaleApprovals { - sess = sess.And("stale = ?", false) - } - approvals, err := sess.Count(new(Review)) - if err != nil { - log.Error("GetGrantedApprovalsCount: %v", err) - return 0 - } - - return approvals -} - -// GetRejectedReviewsCount returns the number of rejected reviews for pr. -func (protectBranch *ProtectedBranch) GetRejectedReviewsCount(pr *PullRequest) int64 { - rejects, err := x.Where("issue_id = ?", pr.IssueID). - And("type = ?", ReviewTypeReject). - And("official = ?", true). - Count(new(Review)) - if err != nil { - log.Error("GetRejectedReviewsCount: %v", err) - return 0 - } - - return rejects -} - // MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool { if !protectBranch.BlockOnRejectedReviews { diff --git a/models/pull.go b/models/pull.go index 6c36207bae66f..6de5f88e8d1e8 100644 --- a/models/pull.go +++ b/models/pull.go @@ -191,10 +191,10 @@ func (pr *PullRequest) RequiresApproval() bool { // GetReviewLabel returns the localization label for the review of this pull request func (pr *PullRequest) GetReviewLabel() string { if pr.RequiresApproval() { - if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { + if pr.GetRejectedReviewsCount() > 0 { return "repo.pulls.review_rejected" } - if pr.ProtectedBranch.GetGrantedApprovalsCount(pr) >= pr.ProtectedBranch.RequiredApprovals { + if pr.GetGrantedApprovalsCount() >= pr.ProtectedBranch.RequiredApprovals { return "repo.pulls.review_approved" } return "repo.pulls.review_required" @@ -205,11 +205,11 @@ func (pr *PullRequest) GetReviewLabel() string { // GetReviewLabel returns the counter for the review of this pull request func (pr *PullRequest) GetReviewCount() int64 { if pr.RequiresApproval() { - rejections := pr.ProtectedBranch.GetRejectedReviewsCount(pr) - if pr.ProtectedBranch.GetRejectedReviewsCount(pr) > 0 { + rejections := pr.GetRejectedReviewsCount() + if rejections > 0 { return rejections } - approvals := pr.ProtectedBranch.GetGrantedApprovalsCount(pr) + approvals := pr.GetGrantedApprovalsCount() if approvals >= pr.ProtectedBranch.RequiredApprovals { return approvals } @@ -218,6 +218,45 @@ func (pr *PullRequest) GetReviewCount() int64 { return 0 // by default } +// GetRejectedReviewsCount returns the number of rejected reviews for pr. +func (pr *PullRequest) GetRejectedReviewsCount() int64 { + rejects, err := x.Where("issue_id = ?", pr.IssueID). + And("type = ?", ReviewTypeReject). + And("official = ?", true). + Count(new(Review)) + if err != nil { + log.Error("GetRejectedReviewsCount: %v", err) + return 0 + } + + return rejects +} + +// HasEnoughApprovals returns true if pr has enough granted approvals. +func (pr *PullRequest) HasEnoughApprovals() bool { + if pr.ProtectedBranch.RequiredApprovals == 0 { + return true + } + return pr.GetGrantedApprovalsCount() >= pr.ProtectedBranch.RequiredApprovals +} + +// GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist. +func (pr *PullRequest) GetGrantedApprovalsCount() int64 { + sess := x.Where("issue_id = ?", pr.IssueID). + And("type = ?", ReviewTypeApprove). + And("official = ?", true) + if pr.ProtectedBranch.DismissStaleApprovals { + sess = sess.And("stale = ?", false) + } + approvals, err := sess.Count(new(Review)) + if err != nil { + log.Error("GetGrantedApprovalsCount: %v", err) + return 0 + } + + return approvals +} + // GetDefaultMergeMessage returns default message used when merging pull request func (pr *PullRequest) GetDefaultMergeMessage() string { if pr.HeadRepo == nil { diff --git a/models/pull_sign.go b/models/pull_sign.go index 5b26b4bdc9e80..3c36b37f6cb32 100644 --- a/models/pull_sign.go +++ b/models/pull_sign.go @@ -57,7 +57,7 @@ func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit st if protectedBranch == nil { return false, "", &ErrWontSign{approved} } - if protectedBranch.GetGrantedApprovalsCount(pr) < 1 { + if pr.GetGrantedApprovalsCount() < 1 { return false, "", &ErrWontSign{approved} } case baseSigned: diff --git a/routers/repo/issue.go b/routers/repo/issue.go index fdade2795d164..e117e981f1930 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -971,8 +971,8 @@ func ViewIssue(ctx *context.Context) { return } if pull.ProtectedBranch != nil { - cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull) - ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) + cnt := pull.GetGrantedApprovalsCount() + ctx.Data["IsBlockedByApprovals"] = !pull.HasEnoughApprovals() ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) ctx.Data["GrantedApprovals"] = cnt ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits diff --git a/services/pull/merge.go b/services/pull/merge.go index 26c9ab3d1cfae..18c4657e8e117 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -539,7 +539,7 @@ func CheckPRReadyToMerge(pr *models.PullRequest) (err error) { } } - if enoughApprovals := pr.ProtectedBranch.HasEnoughApprovals(pr); !enoughApprovals { + if enoughApprovals := pr.HasEnoughApprovals(); !enoughApprovals { return models.ErrNotAllowedToMerge{ Reason: "Does not have enough approvals", } From 64b47d50330a0747f521490f4579bd98330f0409 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 2 Feb 2020 22:03:10 -0800 Subject: [PATCH 14/18] Fix comment --- models/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index 6de5f88e8d1e8..ab7c72d9528ba 100644 --- a/models/pull.go +++ b/models/pull.go @@ -202,7 +202,7 @@ func (pr *PullRequest) GetReviewLabel() string { return "repo.pulls.review_approved" // by default } -// GetReviewLabel returns the counter for the review of this pull request +// GetReviewCount returns the counter for the review of this pull request func (pr *PullRequest) GetReviewCount() int64 { if pr.RequiresApproval() { rejections := pr.GetRejectedReviewsCount() From 01bd93c226eacdfa9d51859bcd7d35e367e6d884 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Wed, 12 Feb 2020 00:58:08 -0800 Subject: [PATCH 15/18] Returning status for reviews --- models/pull.go | 28 ++++++++-------------- templates/repo/issue/list.tmpl | 9 ++++++- templates/repo/issue/milestone_issues.tmpl | 9 ++++++- templates/user/dashboard/issues.tmpl | 9 ++++++- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/models/pull.go b/models/pull.go index 88e47014d096a..3da5867aab0e2 100644 --- a/models/pull.go +++ b/models/pull.go @@ -188,34 +188,26 @@ func (pr *PullRequest) RequiresApproval() bool { return pr.requiredApprovals() > 0 } -// GetReviewLabel returns the localization label for the review of this pull request -func (pr *PullRequest) GetReviewLabel() string { - if pr.RequiresApproval() { - if pr.GetRejectedReviewsCount() > 0 { - return "repo.pulls.review_rejected" - } - if pr.GetGrantedApprovalsCount() >= pr.ProtectedBranch.RequiredApprovals { - return "repo.pulls.review_approved" - } - return "repo.pulls.review_required" - } - return "repo.pulls.review_approved" // by default +// ReviewStatusCount holds the Status and Count of a Review +type ReviewStatusCount struct { + Status string + Count int64 } -// GetReviewCount returns the counter for the review of this pull request -func (pr *PullRequest) GetReviewCount() int64 { +// GetReviewStatus returns a ReviewStatusCount of this pull request +func (pr *PullRequest) GetReviewStatus() ReviewStatusCount { if pr.RequiresApproval() { rejections := pr.GetRejectedReviewsCount() if rejections > 0 { - return rejections + return ReviewStatusCount{Status: "rejected", Count: rejections} } approvals := pr.GetGrantedApprovalsCount() if approvals >= pr.ProtectedBranch.RequiredApprovals { - return approvals + return ReviewStatusCount{Status: "approved", Count: approvals} } - return pr.ProtectedBranch.RequiredApprovals - approvals + return ReviewStatusCount{Status: "required", Count: pr.ProtectedBranch.RequiredApprovals - approvals} } - return 0 // by default + return ReviewStatusCount{Status: "approved", Count: 0} // by default } // GetRejectedReviewsCount returns the number of rejected reviews for pr. diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 46d3399481c99..dd562f3852504 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -240,8 +240,15 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{ $reviewStatus := .PullRequest.GetReviewStatus }} - {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{if eq $reviewStatus.Status "rejected"}} + {{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}} + {{else if eq $reviewStatus.Status "required"}} + {{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}} + {{else}} + {{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}} + {{end}} {{end}}{{end}} {{if .Milestone}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 9a65e182a32c9..6be8b20de7fea 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -207,8 +207,15 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{ $reviewStatus := .PullRequest.GetReviewStatus }} - {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{if eq $reviewStatus.Status "rejected"}} + {{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}} + {{else if eq $reviewStatus.Status "required"}} + {{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}} + {{else}} + {{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}} + {{end}} {{end}}{{end}} {{if .Ref}} diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 34309b5e9521a..fe4f42d3e048f 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -125,8 +125,15 @@ {{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}} {{end}} {{if .IsPull }}{{if .PullRequest.RequiresApproval }} + {{ $reviewStatus := .PullRequest.GetReviewStatus }} - {{$.i18n.Tr .PullRequest.GetReviewLabel .PullRequest.GetReviewCount | Safe}} + {{if eq $reviewStatus.Status "rejected"}} + {{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}} + {{else if eq $reviewStatus.Status "required"}} + {{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}} + {{else}} + {{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}} + {{end}} {{end}}{{end}} {{if .Milestone}} From 6577c95e9236b2d95125c49acbae479d33d94873 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 23 Feb 2020 09:02:21 -0800 Subject: [PATCH 16/18] Refactor: extracting code that load issues in PulRequestList to a method --- models/pull_list.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/models/pull_list.go b/models/pull_list.go index 989de46891e22..214248b02bb07 100644 --- a/models/pull_list.go +++ b/models/pull_list.go @@ -113,7 +113,25 @@ func (prs PullRequestList) loadAttributes(e Engine) error { return nil } - // Load issues. + if err := prs.loadIssues(e); err != nil { + return fmt.Errorf("prs.loadAttributes: loadIssues: %v", err) + } + return nil +} + +func (prs PullRequestList) getIssueIDs() []int64 { + issueIDs := make([]int64, 0, len(prs)) + for i := range prs { + issueIDs = append(issueIDs, prs[i].IssueID) + } + return issueIDs +} + +func (prs PullRequestList) loadIssues(e Engine) (err error) { + if len(prs) == 0 { + return nil + } + issueIDs := prs.getIssueIDs() issues := make([]*Issue, 0, len(issueIDs)) if err := e. @@ -133,14 +151,6 @@ func (prs PullRequestList) loadAttributes(e Engine) error { return nil } -func (prs PullRequestList) getIssueIDs() []int64 { - issueIDs := make([]int64, 0, len(prs)) - for i := range prs { - issueIDs = append(issueIDs, prs[i].IssueID) - } - return issueIDs -} - // LoadAttributes load all the prs attributes func (prs PullRequestList) LoadAttributes() error { return prs.loadAttributes(x) From d97dc256d4733473b7112d9fea1a6a3bb8aa1d31 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 23 Feb 2020 10:20:18 -0800 Subject: [PATCH 17/18] Batch loading protected branch objects in pull_list.go --- models/pull_list.go | 55 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/models/pull_list.go b/models/pull_list.go index 214248b02bb07..44e2f0d3988e4 100644 --- a/models/pull_list.go +++ b/models/pull_list.go @@ -116,6 +116,11 @@ func (prs PullRequestList) loadAttributes(e Engine) error { if err := prs.loadIssues(e); err != nil { return fmt.Errorf("prs.loadAttributes: loadIssues: %v", err) } + + if err := prs.loadProtectedBranches(e); err != nil { + return fmt.Errorf("prs.loadAttributes: loadProtectedBranches: %v", err) + } + return nil } @@ -151,6 +156,56 @@ func (prs PullRequestList) loadIssues(e Engine) (err error) { return nil } +type protectedBranchKey struct { + RepoID int64 + BranchName string +} + +func (prs PullRequestList) getProtectedBranchKeys() []protectedBranchKey { + prKeys := make(map[protectedBranchKey]struct{}, len(prs)) + for _, pr := range prs { + if pr.BaseRepoID <= 0 { + continue + } + key := protectedBranchKey{pr.BaseRepoID, pr.BaseBranch} + if _, ok := prKeys[key]; !ok { + prKeys[key] = struct{}{} + } + } + return valuesProtectedBranchKeys(prKeys) +} + +func valuesProtectedBranchKeys(m map[protectedBranchKey]struct{}) []protectedBranchKey { + values := make([]protectedBranchKey, 0, len(m)) + for k := range m { + values = append(values, k) + } + return values +} + +func (prs PullRequestList) loadProtectedBranches(e Engine) (err error) { + if len(prs) == 0 { + return nil + } + + prKeys := prs.getProtectedBranchKeys() + prMaps := make(map[protectedBranchKey]*ProtectedBranch, len(prKeys)) + sess := e.Where("repo_id = ? and branch_name = ?", prKeys[0].RepoID, prKeys[0].BranchName) + for _, prk := range prKeys[1:] { + sess.Or("repo_id = ? and branch_name = ?", prk.RepoID, prk.BranchName) + } + err = sess.Find(&prMaps) + if err != nil { + return err + } + + for _, pr := range prs { + pr.ProtectedBranch = prMaps[protectedBranchKey{pr.BaseRepoID, pr.BaseBranch}] + } + + return nil +} + // LoadAttributes load all the prs attributes func (prs PullRequestList) LoadAttributes() error { return prs.loadAttributes(x) From c63d0a9dffe4f2e2d894ce3723a12b9f581ae447 Mon Sep 17 00:00:00 2001 From: Oscar Costa Date: Sun, 23 Feb 2020 20:54:29 -0800 Subject: [PATCH 18/18] Loading pull request attributes --- models/pull.go | 9 ++++----- routers/repo/issue.go | 4 ++-- routers/user/home.go | 5 +++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/models/pull.go b/models/pull.go index 3da5867aab0e2..657306b7edf58 100644 --- a/models/pull.go +++ b/models/pull.go @@ -89,6 +89,10 @@ func (pr *PullRequest) loadAttributes(e Engine) (err error) { } } + if err = pr.loadProtectedBranch(e); err != nil { + return fmt.Errorf("pr.loadAttributes: loadProtectedBranch: %v", err) + } + return nil } @@ -172,11 +176,6 @@ func (pr *PullRequest) loadProtectedBranch(e Engine) (err error) { } func (pr *PullRequest) requiredApprovals() int64 { - if pr.ProtectedBranch == nil { - if err := pr.loadProtectedBranch(x); err != nil { - log.Error("Error loading ProtectedBranch", err) - } - } if pr.ProtectedBranch != nil { return pr.ProtectedBranch.RequiredApprovals } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index e117e981f1930..1d1313ca23c8a 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -229,8 +229,8 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB } if issues[i].IsPull { - if err := issues[i].LoadPullRequest(); err != nil { - ctx.ServerError("LoadPullRequest", err) + if err := issues[i].PullRequest.LoadAttributes(); err != nil { + ctx.ServerError("LoadAttributes", err) return } diff --git a/routers/user/home.go b/routers/user/home.go index 6b71c51de32a5..3cf5a35e921da 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -535,6 +535,11 @@ func Issues(ctx *context.Context) { if isPullList { commitStatus[issue.PullRequest.ID], _ = issue.PullRequest.GetLastCommitStatus() + + if err := issue.PullRequest.LoadAttributes(); err != nil { + ctx.ServerError("LoadAttributes", err) + return + } } }