From e9c64f41a6895806e578fc6c9746044bdd31c563 Mon Sep 17 00:00:00 2001
From: william-allspice <william@allspice.io>
Date: Fri, 6 Sep 2024 01:40:02 -0500
Subject: [PATCH] =?UTF-8?q?Distinguish=20official=20vs=20non-official=20re?=
=?UTF-8?q?views,=20add=20tool=20tips,=20and=20upgr=E2=80=A6=20(#31924)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This Pull Request is a follow up to
https://github.com/go-gitea/gitea/pull/31886:
1. Adds a UI indicator between official (green) and unofficial (grey)
approved pull requests on the Pull Request page (as suggested by
@kdumontnu )
2. Adds tooltips adding clarity to the type and status of a review on
the Pull Request page (as suggested by @kdumontnu)
3. Updates text adding more clarity to required approvals (as suggested
by @kdumontnu)
4. Updates text on the branch settings page explaining what branch
approval limitations (as suggested by @yp05327)
Official approval:
<img width="376" alt="Screenshot 2024-08-26 at 1 03 52 PM"
src="https://github.com/user-attachments/assets/500f083d-bfc0-45c5-82b7-b98e20495696">
Unofficial approval:
<img width="442" alt="Screenshot 2024-08-26 at 12 53 15 PM"
src="https://github.com/user-attachments/assets/e8c565ff-5886-4ce1-8b79-a0fa26c282f7">
Rejected approval:
<img width="452" alt="Screenshot 2024-08-26 at 12 53 06 PM"
src="https://github.com/user-attachments/assets/aebc0e2f-7052-4dea-8098-7caa0db86617">
Stale approval:
<img width="546" alt="Screenshot 2024-08-26 at 1 07 59 PM"
src="https://github.com/user-attachments/assets/da599ff3-e35c-4fa3-8141-ed80b738dd77">
Requested review tooltip:
<img width="434" alt="Screenshot 2024-08-26 at 12 53 22 PM"
src="https://github.com/user-attachments/assets/460d163e-8724-43b6-8760-34b285da8fe2">
Updated text for approvals:
<img width="991" alt="Screenshot 2024-08-26 at 12 54 00 PM"
src="https://github.com/user-attachments/assets/ab3ff012-9742-4c1b-933d-21addcb89f2c">
Updated text for allowlisted/whitelisted approvals:
<img width="990" alt="Screenshot 2024-08-26 at 1 01 40 PM"
src="https://github.com/user-attachments/assets/1a5bae61-d9e0-4d96-b86f-92610b0940d1">
Protected branch settings text:
<img width="1022" alt="Screenshot 2024-08-26 at 1 01 14 PM"
src="https://github.com/user-attachments/assets/892ce208-e1c2-41f7-8fec-46d5a0e7e776">
Comments list:
<img width="1048" alt="Screenshot 2024-08-28 at 9 25 31 AM"
src="https://github.com/user-attachments/assets/9c5c00c5-06cf-43b3-b413-4f7f673609b2">
---------
Co-authored-by: Kyle D. <kdumontnu@gmail.com>
---
models/issues/review.go | 25 +++++++++++++++++++
options/locale/locale_en-US.ini | 11 ++++++--
routers/web/repo/issue.go | 1 +
.../repo/issue/view_content/comments.tmpl | 2 +-
templates/repo/issue/view_content/pull.tmpl | 6 ++++-
.../repo/issue/view_content/sidebar.tmpl | 8 ++++--
6 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/models/issues/review.go b/models/issues/review.go
index ca6fd6035b..9a08e265ff 100644
--- a/models/issues/review.go
+++ b/models/issues/review.go
@@ -214,9 +214,13 @@ func (r *Review) LoadAttributes(ctx context.Context) (err error) {
return err
}
+// HTMLTypeColorName returns the color used in the ui indicating the review
func (r *Review) HTMLTypeColorName() string {
switch r.Type {
case ReviewTypeApprove:
+ if !r.Official {
+ return "grey"
+ }
if r.Stale {
return "yellow"
}
@@ -231,6 +235,27 @@ func (r *Review) HTMLTypeColorName() string {
return "grey"
}
+// TooltipContent returns the locale string describing the review type
+func (r *Review) TooltipContent() string {
+ switch r.Type {
+ case ReviewTypeApprove:
+ if r.Stale {
+ return "repo.issues.review.stale"
+ }
+ if !r.Official {
+ return "repo.issues.review.unofficial"
+ }
+ return "repo.issues.review.official"
+ case ReviewTypeComment:
+ return "repo.issues.review.comment"
+ case ReviewTypeReject:
+ return "repo.issues.review.rejected"
+ case ReviewTypeRequest:
+ return "repo.issues.review.requested"
+ }
+ return ""
+}
+
// GetReviewByID returns the review by the given ID
func GetReviewByID(ctx context.Context, id int64) (*Review, error) {
review := new(Review)
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 8d51864d3d..7d8eb1a475 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1752,6 +1752,12 @@ issues.review.hide_resolved = Hide resolved
issues.review.resolve_conversation = Resolve conversation
issues.review.un_resolve_conversation = Unresolve conversation
issues.review.resolved_by = marked this conversation as resolved
+issues.review.comment = Comment
+issues.review.official = Approved
+issues.review.requested = Review pending
+issues.review.rejected = Changes requested
+issues.review.stale = Updated since approval
+issues.review.unofficial = Uncounted approval
issues.assignee.error = Not all assignees was added due to an unexpected error.
issues.reference_issue.body = Body
issues.content_history.deleted = deleted
@@ -1825,7 +1831,8 @@ pulls.is_empty = "The changes on this branch are already on the target branch. T
pulls.required_status_check_failed = Some required checks were not successful.
pulls.required_status_check_missing = Some required checks are missing.
pulls.required_status_check_administrator = As an administrator, you may still merge this pull request.
-pulls.blocked_by_approvals = "This pull request doesn't have enough approvals yet. %d of %d approvals granted."
+pulls.blocked_by_approvals = "This pull request doesn't have enough required approvals yet. %d of %d official approvals granted."
+pulls.blocked_by_approvals_whitelisted = "This pull request doesn't have enough required approvals yet. %d of %d approvals granted from users or teams on the allowlist."
pulls.blocked_by_rejection = "This pull request has changes requested by an official reviewer."
pulls.blocked_by_official_review_requests = "This pull request has official review requests."
pulls.blocked_by_outdated_branch = "This pull request is blocked because it's outdated."
@@ -2413,7 +2420,7 @@ settings.protect_status_check_matched = Matched
settings.protect_invalid_status_check_pattern = Invalid status check pattern: "%s".
settings.protect_no_valid_status_check_patterns = No valid status check patterns.
settings.protect_required_approvals = Required approvals:
-settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews.
+settings.protect_required_approvals_desc = Allow only to merge pull request with enough required approvals. Required approvals are either from users or teams who are on the allowlist or anyone with write access.
settings.protect_approvals_whitelist_enabled = Restrict approvals to allowlisted users or teams
settings.protect_approvals_whitelist_enabled_desc = Only reviews from allowlisted users or teams will count to the required approvals. Without approval allowlist, reviews from anyone with write access count to the required approvals.
settings.protect_approvals_whitelist_users = Allowlisted reviewers:
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 856e2f7392..fd6abe04fe 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -1940,6 +1940,7 @@ func ViewIssue(ctx *context.Context) {
ctx.Data["ChangedProtectedFiles"] = pull.ChangedProtectedFiles
ctx.Data["IsBlockedByChangedProtectedFiles"] = len(pull.ChangedProtectedFiles) != 0
ctx.Data["ChangedProtectedFilesNum"] = len(pull.ChangedProtectedFiles)
+ ctx.Data["RequireApprovalsWhitelist"] = pb.EnableApprovalsWhitelist
}
ctx.Data["WillSign"] = false
if ctx.Doer != nil {
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl
index 1cf9287111..57abbeb8f7 100644
--- a/templates/repo/issue/view_content/comments.tmpl
+++ b/templates/repo/issue/view_content/comments.tmpl
@@ -380,7 +380,7 @@
{{ctx.AvatarUtils.Avatar .Poster 40}}
</a>
{{end}}
- <span class="badge{{if eq $reviewType 1}} tw-bg-green tw-text-white{{else if eq $reviewType 3}} tw-bg-red tw-text-white{{end}}">
+ <span class="badge tw-text-white{{if eq $reviewType 1}}{{if .Review.Official}} tw-bg-green {{else}} tw-bg-grey{{end}}{{else if eq $reviewType 3}} tw-bg-red{{end}}">
{{if .Review}}{{svg (printf "octicon-%s" .Review.Type.Icon)}}{{end}}
</span>
<span class="text grey muted-links">
diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl
index 69e74da3a0..1ce658ed00 100644
--- a/templates/repo/issue/view_content/pull.tmpl
+++ b/templates/repo/issue/view_content/pull.tmpl
@@ -109,7 +109,11 @@
{{if .IsBlockedByApprovals}}
<div class="item">
{{svg "octicon-x"}}
- {{ctx.Locale.Tr "repo.pulls.blocked_by_approvals" .GrantedApprovals .ProtectedBranch.RequiredApprovals}}
+ {{if .RequireApprovalsWhitelist}}
+ {{ctx.Locale.Tr "repo.pulls.blocked_by_approvals_whitelisted" .GrantedApprovals .ProtectedBranch.RequiredApprovals}}
+ {{else}}
+ {{ctx.Locale.Tr "repo.pulls.blocked_by_approvals" .GrantedApprovals .ProtectedBranch.RequiredApprovals}}
+ {{end}}
</div>
{{else if .IsBlockedByRejection}}
<div class="item">
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl
index ce34c5e939..49c40be5a9 100644
--- a/templates/repo/issue/view_content/sidebar.tmpl
+++ b/templates/repo/issue/view_content/sidebar.tmpl
@@ -94,7 +94,9 @@
{{if and .CanChange (or .Checked (and (not $.Issue.IsClosed) (not $.Issue.PullRequest.HasMerged)))}}
<a href="#" class="ui muted icon re-request-review{{if .Checked}} checked{{end}}" data-tooltip-content="{{if .Checked}}{{ctx.Locale.Tr "repo.issues.remove_request_review"}}{{else}}{{ctx.Locale.Tr "repo.issues.re_request_review"}}{{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">{{svg (Iif .Checked "octicon-trash" "octicon-sync")}}</a>
{{end}}
- {{svg (printf "octicon-%s" .Review.Type.Icon) 16 (printf "text %s" (.Review.HTMLTypeColorName))}}
+ <span {{if .Review.TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .Review.TooltipContent}}"{{end}}>
+ {{svg (printf "octicon-%s" .Review.Type.Icon) 16 (printf "text %s" (.Review.HTMLTypeColorName))}}
+ </span>
</div>
</div>
{{end}}
@@ -107,7 +109,9 @@
</a>
</div>
<div class="tw-flex tw-items-center tw-gap-2">
- {{svg (printf "octicon-%s" .Type.Icon) 16 (printf "text %s" (.HTMLTypeColorName))}}
+ <span {{if .Review.TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .Review.TooltipContent}}"{{end}}>
+ {{svg (printf "octicon-%s" .Type.Icon) 16 (printf "text %s" (.HTMLTypeColorName))}}
+ </span>
</div>
</div>
{{end}}