From 2d7e6e9482da19aae4660823f38c2153cf942bb8 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Tue, 17 Dec 2024 09:15:18 +0800
Subject: [PATCH] Fix various trivial problems (#32861)

1. add/improve comments to help future readers could understand the
problem more easily.
2. add an error log to LDAP with username fallback
3. use `or` instead of `Iif` for "repo/branch_dropdown" (`Iif` was a
mistake, but it doesn't really affect the UI)
4. add `tw-font-mono` style to container digest to match dockerhub
5. fix a bug in RepoBranchTagSelector: the form is not updated when
there is no click to an item

---------

Co-authored-by: delvh <dev.lh@web.de>
---
 models/actions/run_job_status_test.go            |  2 +-
 modules/markup/markdown/markdown.go              |  3 ++-
 modules/repository/fork.go                       |  3 +++
 routers/web/repo/actions/view.go                 |  3 +++
 services/auth/source/ldap/source_authenticate.go |  6 ++++++
 templates/package/content/container.tmpl         |  3 ++-
 templates/repo/commit_page.tmpl                  |  4 +++-
 templates/repo/issue/filter_item_label.tmpl      |  3 +++
 templates/user/auth/signin_inner.tmpl            |  1 +
 templates/user/auth/signup_inner.tmpl            |  2 ++
 templates/user/dashboard/issues.tmpl             |  2 ++
 web_src/js/components/RepoBranchTagSelector.vue  | 10 ++++++++++
 12 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go
index 04fd9ceba7..523d38327e 100644
--- a/models/actions/run_job_status_test.go
+++ b/models/actions/run_job_status_test.go
@@ -69,7 +69,7 @@ func TestAggregateJobStatus(t *testing.T) {
 		{[]Status{StatusFailure, StatusBlocked}, StatusFailure},
 
 		// skipped with other status
-		// TODO: need to clarify whether a PR with "skipped" job status is considered as "mergeable" or not.
+		// "all skipped" is also considered as "mergeable" by "services/actions.toCommitStatus", the same as GitHub
 		{[]Status{StatusSkipped}, StatusSkipped},
 		{[]Status{StatusSkipped, StatusSuccess}, StatusSuccess},
 		{[]Status{StatusSkipped, StatusFailure}, StatusFailure},
diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go
index a14c0cad59..b5fffccdb9 100644
--- a/modules/markup/markdown/markdown.go
+++ b/modules/markup/markdown/markdown.go
@@ -129,7 +129,8 @@ func SpecializedMarkdown(ctx *markup.RenderContext) *GlodmarkRender {
 				Enabled:           setting.Markdown.EnableMath,
 				ParseDollarInline: true,
 				ParseDollarBlock:  true,
-				ParseSquareBlock:  true, // TODO: this is a bad syntax, it should be deprecated in the future (by some config options)
+				ParseSquareBlock:  true, // TODO: this is a bad syntax "\[ ... \]", it conflicts with normal markdown escaping, it should be deprecated in the future (by some config options)
+				// ParseBracketInline: true, // TODO: this is also a bad syntax "\( ... \)", it also conflicts, it should be deprecated in the future
 			}),
 			meta.Meta,
 		),
diff --git a/modules/repository/fork.go b/modules/repository/fork.go
index d530634071..84ed4b23d6 100644
--- a/modules/repository/fork.go
+++ b/modules/repository/fork.go
@@ -12,6 +12,9 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 )
 
+// CanUserForkBetweenOwners returns true if user can fork between owners.
+// By default, a user can fork a repository from another owner, but not from themselves.
+// Many users really like to fork their own repositories, so add an experimental setting to allow this.
 func CanUserForkBetweenOwners(id1, id2 int64) bool {
 	if id1 != id2 {
 		return true
diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go
index b711038da0..ba17fa427d 100644
--- a/routers/web/repo/actions/view.go
+++ b/routers/web/repo/actions/view.go
@@ -205,8 +205,11 @@ func ViewPost(ctx *context_module.Context) {
 		}
 	}
 
+	// TODO: "ComposeMetas" (usually for comment) is not quite right, but it is still the same as what template "RenderCommitMessage" does.
+	// need to be refactored together in the future
 	metas := ctx.Repo.Repository.ComposeMetas(ctx)
 
+	// the title for the "run" is from the commit message
 	resp.State.Run.Title = run.Title
 	resp.State.Run.TitleHTML = templates.NewRenderUtils(ctx).RenderCommitMessage(run.Title, metas)
 	resp.State.Run.Link = run.Link()
diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go
index 020e5784dc..6a6c60cd40 100644
--- a/services/auth/source/ldap/source_authenticate.go
+++ b/services/auth/source/ldap/source_authenticate.go
@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/auth"
 	user_model "code.gitea.io/gitea/models/user"
 	auth_module "code.gitea.io/gitea/modules/auth"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 	source_service "code.gitea.io/gitea/services/auth/source"
@@ -31,7 +32,12 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
 		return nil, user_model.ErrUserNotExist{Name: loginName}
 	}
 	// Fallback.
+	// FIXME: this fallback would cause problems when the "Username" attribute is not set and a user inputs their email.
+	// In this case, the email would be used as the username, and will cause the "CreateUser" failure for the first login.
 	if sr.Username == "" {
+		if strings.Contains(userName, "@") {
+			log.Error("No username in search result (Username Attribute is not set properly?), using email as username might cause problems")
+		}
 		sr.Username = userName
 	}
 	if sr.Mail == "" {
diff --git a/templates/package/content/container.tmpl b/templates/package/content/container.tmpl
index 207774bfef..04732d276a 100644
--- a/templates/package/content/container.tmpl
+++ b/templates/package/content/container.tmpl
@@ -36,9 +36,10 @@
 				</thead>
 				<tbody>
 					{{range .PackageDescriptor.Metadata.Manifests}}
+						{{/* "unknown/unknown" is attestation-manifest, so we should skip it */}}
 						{{if ne .Platform "unknown/unknown"}}
 						<tr>
-							<td><a href="{{$.PackageDescriptor.PackageWebLink}}/{{PathEscape .Digest}}">{{StringUtils.TrimPrefix .Digest "sha256:" | ShortSha}}</a></td>
+							<td><a class="tw-font-mono" href="{{$.PackageDescriptor.PackageWebLink}}/{{PathEscape .Digest}}">{{StringUtils.TrimPrefix .Digest "sha256:" | ShortSha}}</a></td>
 							<td>{{.Platform}}</td>
 							<td>{{FileSize .Size}}</td>
 						</tr>
diff --git a/templates/repo/commit_page.tmpl b/templates/repo/commit_page.tmpl
index 8792c4e1b2..71f77154fb 100644
--- a/templates/repo/commit_page.tmpl
+++ b/templates/repo/commit_page.tmpl
@@ -68,11 +68,13 @@
 											<p id="cherry-pick-content" class="branch-dropdown"></p>
 
 											<form method="get">
+												{{/*FIXME: CurrentRefShortName seems not making sense here (old code),
+												because the "commit page" has no "$.BranchName" info, so only using DefaultBranch should be enough */}}
 												{{template "repo/branch_dropdown" dict
 													"Repository" .Repository
 													"ShowTabBranches" true
 													"CurrentRefType" "branch"
-													"CurrentRefShortName" (Iif $.BranchName $.Repository.DefaultBranch)
+													"CurrentRefShortName" (or $.BranchName $.Repository.DefaultBranch)
 													"RefFormActionTemplate" (print "{RepoLink}/_cherrypick/" .CommitID "/{RefShortName}")
 												}}
 												<input type="hidden" id="cherry-pick-type" name="cherry-pick-type"><br>
diff --git a/templates/repo/issue/filter_item_label.tmpl b/templates/repo/issue/filter_item_label.tmpl
index 927328ba14..88e2e43120 100644
--- a/templates/repo/issue/filter_item_label.tmpl
+++ b/templates/repo/issue/filter_item_label.tmpl
@@ -23,6 +23,9 @@
 		<div class="divider"></div>
 		<a class="item label-filter-query-default" href="{{QueryBuild $queryLink "labels" NIL}}">{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}}</a>
 		<a class="item label-filter-query-not-set" href="{{QueryBuild $queryLink "labels" 0}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
+		{{/* The logic here is not the same as the label selector in the issue sidebar.
+		The one in the issue sidebar renders "repo labels | divider | org labels".
+		Maybe the logic should be updated to be consistent.*/}}
 		{{$previousExclusiveScope := "_no_scope"}}
 		{{range .Labels}}
 			{{$exclusiveScope := .ExclusiveScope}}
diff --git a/templates/user/auth/signin_inner.tmpl b/templates/user/auth/signin_inner.tmpl
index 9daa051e06..dd608e5aa1 100644
--- a/templates/user/auth/signin_inner.tmpl
+++ b/templates/user/auth/signin_inner.tmpl
@@ -48,6 +48,7 @@
 			</div>
 		</form>
 		{{end}}{{/*if .EnablePasswordSignInForm*/}}
+		{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
 		{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
 		{{if and $showOAuth2Methods .EnablePasswordSignInForm}}
 			<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>
diff --git a/templates/user/auth/signup_inner.tmpl b/templates/user/auth/signup_inner.tmpl
index ea8d0bafe4..b3b2a4205e 100644
--- a/templates/user/auth/signup_inner.tmpl
+++ b/templates/user/auth/signup_inner.tmpl
@@ -47,6 +47,8 @@
 					</button>
 				</div>
 			{{end}}
+			{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
+			{{/* TODO: it seems that "EnableSSPI" is only set in "sign-in" handlers, but it should use the same logic to control its display */}}
 			{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
 			{{if $showOAuth2Methods}}
 				<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>
diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl
index 7f960a4709..7924dd2305 100644
--- a/templates/user/dashboard/issues.tmpl
+++ b/templates/user/dashboard/issues.tmpl
@@ -61,6 +61,7 @@
 							{{template "repo/issue/filter_item_label" dict "Labels" .Labels "QueryLink" $queryLinkWithFilter "SupportArchivedLabel" true}}
 						{{end}}
 
+						{{/* at the moment there is no easy way to get poster candidates on this page, so only show a username input, search for what the end user enters */}}
 						{{if ne $.ViewType "created_by"}}
 							{{template "repo/issue/filter_item_user_fetch" dict
 								"QueryParamKey" "poster"
@@ -70,6 +71,7 @@
 							}}
 						{{end}}
 
+						{{/* at the moment there is no easy way to get assignee candidates on this page, so only show a username input, search for what the end user enters */}}
 						{{if ne $.ViewType "assigned"}}
 							{{template "repo/issue/filter_item_user_fetch" dict
 								"QueryParamKey" "assignee"
diff --git a/web_src/js/components/RepoBranchTagSelector.vue b/web_src/js/components/RepoBranchTagSelector.vue
index 2f66336a66..4b7ca1429d 100644
--- a/web_src/js/components/RepoBranchTagSelector.vue
+++ b/web_src/js/components/RepoBranchTagSelector.vue
@@ -113,6 +113,16 @@ const sfc = {
       if (this.menuVisible) this.menuVisible = false;
     });
   },
+
+  mounted() {
+    if (this.refFormActionTemplate) {
+      // if the selector is used in a form and needs to change the form action,
+      // make a mock item and select it to update the form action
+      const item: ListItem = {selected: true, refType: this.currentRefType, refShortName: this.currentRefShortName, rssFeedLink: ''};
+      this.selectItem(item);
+    }
+  },
+
   methods: {
     selectItem(item: ListItem) {
       this.menuVisible = false;