Merge remote-tracking branch 'upstream/main'

* upstream/main: Fix issues count bug (go-gitea#21557) Improve code comment review on mobile (go-gitea#21461) Consolidate remaining colors into variables (go-gitea#21582)
zjjhot · Oct 25, 2022 · bde6a30 · bde6a30
2 parents 0c1b151 + 5e8e3ec
commit bde6a30
Show file tree

Hide file tree

Showing 14 changed files with 441 additions and 183 deletions.
diff --git a/models/issues/comment.go b/models/issues/comment.go
@@ -886,7 +886,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment
 			}
 		}
 	case CommentTypeReopen, CommentTypeClose:
-		if err = updateIssueClosedNum(ctx, opts.Issue); err != nil {
+		if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil {
 			return err
 		}
 	}

diff --git a/models/issues/issue.go b/models/issues/issue.go
@@ -725,7 +725,8 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
 		}
 	}
 
-	if err := updateIssueClosedNum(ctx, issue); err != nil {
+	// update repository's issue closed number
+	if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
 		return nil, err
 	}
 
@@ -2137,15 +2138,6 @@ func (issue *Issue) BlockingDependencies(ctx context.Context) (issueDeps []*Depe
 	return issueDeps, err
 }
 
-func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) {
-	if issue.IsPull {
-		err = repo_model.StatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls")
-	} else {
-		err = repo_model.StatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues")
-	}
-	return err
-}
-
 // FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database.
 func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_model.User, content string) (mentions []*user_model.User, err error) {
 	rawMentions := references.FindAllMentionsMarkdown(content)

diff --git a/models/repo.go b/models/repo.go
@@ -404,24 +404,19 @@ func repoStatsCorrectIssueNumComments(ctx context.Context, id int64) error {
 }
 
 func repoStatsCorrectNumIssues(ctx context.Context, id int64) error {
-	return repoStatsCorrectNum(ctx, id, false, "num_issues")
+	return repo_model.UpdateRepoIssueNumbers(ctx, id, false, false)
 }
 
 func repoStatsCorrectNumPulls(ctx context.Context, id int64) error {
-	return repoStatsCorrectNum(ctx, id, true, "num_pulls")
-}
-
-func repoStatsCorrectNum(ctx context.Context, id int64, isPull bool, field string) error {
-	_, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_pull=?) WHERE id=?", id, isPull, id)
-	return err
+	return repo_model.UpdateRepoIssueNumbers(ctx, id, true, false)
 }
 
 func repoStatsCorrectNumClosedIssues(ctx context.Context, id int64) error {
-	return repo_model.StatsCorrectNumClosed(ctx, id, false, "num_closed_issues")
+	return repo_model.UpdateRepoIssueNumbers(ctx, id, false, true)
 }
 
 func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error {
-	return repo_model.StatsCorrectNumClosed(ctx, id, true, "num_closed_pulls")
+	return repo_model.UpdateRepoIssueNumbers(ctx, id, true, true)
 }
 
 func statsQuery(args ...interface{}) func(context.Context) ([]map[string][]byte, error) {

diff --git a/models/repo/repo.go b/models/repo/repo.go
@@ -765,35 +765,30 @@ func CountRepositories(ctx context.Context, opts CountRepositoryOptions) (int64,
 	return count, nil
 }
 
-// StatsCorrectNumClosed update repository's issue related numbers
-func StatsCorrectNumClosed(ctx context.Context, id int64, isPull bool, field string) error {
-	_, err := db.Exec(ctx, "UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=? AND is_pull=?) WHERE id=?", id, true, isPull, id)
-	return err
-}
-
-// UpdateRepoIssueNumbers update repository issue numbers
+// UpdateRepoIssueNumbers updates one of a repositories amount of (open|closed) (issues|PRs) with the current count
 func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed bool) error {
-	e := db.GetEngine(ctx)
+	field := "num_"
+	if isClosed {
+		field += "closed_"
+	}
 	if isPull {
-		if _, err := e.ID(repoID).Decr("num_pulls").Update(new(Repository)); err != nil {
-			return err
-		}
-		if isClosed {
-			if _, err := e.ID(repoID).Decr("num_closed_pulls").Update(new(Repository)); err != nil {
-				return err
-			}
-		}
+		field += "pulls"
 	} else {
-		if _, err := e.ID(repoID).Decr("num_issues").Update(new(Repository)); err != nil {
-			return err
-		}
-		if isClosed {
-			if _, err := e.ID(repoID).Decr("num_closed_issues").Update(new(Repository)); err != nil {
-				return err
-			}
-		}
+		field += "issues"
 	}
-	return nil
+
+	subQuery := builder.Select("count(*)").
+		From("issue").Where(builder.Eq{
+		"repo_id": repoID,
+		"is_pull": isPull,
+	}.And(builder.If(isClosed, builder.Eq{"is_closed": isClosed})))
+
+	// builder.Update(cond) will generate SQL like UPDATE ... SET cond
+	query := builder.Update(builder.Eq{field: subQuery}).
+		From("repository").
+		Where(builder.Eq{"id": repoID})
+	_, err := db.Exec(ctx, query)
+	return err
 }
 
 // CountNullArchivedRepository counts the number of repositories with is_archived is null

diff --git a/templates/repo/diff/conversation.tmpl b/templates/repo/diff/conversation.tmpl
@@ -26,7 +26,7 @@
 				{{template "repo/diff/comments" dict "root" $ "comments" .comments}}
 			</ui>
 		</div>
-		<div class="df je ac fw mt-3">
+		<div class="df je js-small ac fw mt-3">
 			<div class="ui buttons mr-2">
 				<button class="ui icon tiny basic button previous-conversation">
 					{{svg "octicon-arrow-up" 12 "icon"}} {{$.locale.Tr "repo.issues.previous"}}

diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl
@@ -101,7 +101,7 @@
 						<td class="lines-num"></td>
 						<td class="lines-escape"></td>
 						<td class="lines-type-marker"></td>
-						<td class="add-comment-left">
+						<td class="add-comment-left" colspan="4">
 							{{if gt (len $line.Comments) 0}}
 								{{if eq $line.GetCommentSide "previous"}}
 									{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
@@ -116,7 +116,7 @@
 						<td class="lines-num"></td>
 						<td class="lines-escape"></td>
 						<td class="lines-type-marker"></td>
-						<td class="add-comment-right">
+						<td class="add-comment-right" colspan="4">
 							{{if eq $line.GetCommentSide "proposed"}}
 								{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
 							{{end}}
@@ -133,7 +133,7 @@
 					<td class="lines-num"></td>
 					<td class="lines-escape"></td>
 					<td class="lines-type-marker"></td>
-					<td class="add-comment-left">
+					<td class="add-comment-left" colspan="4">
 						{{if gt (len $line.Comments) 0}}
 							{{if eq $line.GetCommentSide "previous"}}
 								{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
@@ -143,7 +143,7 @@
 					<td class="lines-num"></td>
 					<td class="lines-escape"></td>
 					<td class="lines-type-marker"></td>
-					<td class="add-comment-right">
+					<td class="add-comment-right" colspan="4">
 						{{if eq $line.GetCommentSide "proposed"}}
 							{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
 						{{end}}

diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl
@@ -46,7 +46,7 @@
 			{{if gt (len $line.Comments) 0}}
 				<tr class="add-comment" data-line-type="{{DiffLineTypeToStr .GetType}}">
 					<td colspan="3" class="lines-num"></td>
-					<td class="add-comment-left add-comment-right" colspan="2">
+					<td class="add-comment-left add-comment-right" colspan="5">
 						{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
 					</td>
 				</tr>

diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js
@@ -513,16 +513,16 @@ export function initRepoPullRequestReview() {
             <td class="lines-num"></td>
             <td class="lines-escape"></td>
             <td class="lines-type-marker"></td>
-            <td class="add-comment-left"></td>
+            <td class="add-comment-left" colspan="4"></td>
             <td class="lines-num"></td>
             <td class="lines-escape"></td>
             <td class="lines-type-marker"></td>
-            <td class="add-comment-right"></td>
+            <td class="add-comment-right" colspan="4"></td>
           ` : `
             <td class="lines-num"></td>
             <td class="lines-num"></td>
             <td class="lines-escape"></td>
-            <td class="add-comment-left add-comment-right" colspan="2"></td>
+            <td class="add-comment-left add-comment-right" colspan="5"></td>
           `}
         </tr>`);
       tr.after(ntr);