From e0ea3811c4178ffa30452b7ca4bd211e59326f91 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Sat, 16 Mar 2024 17:20:13 +0800
Subject: [PATCH] Refactor AddParam to AddParamIfExist (#29834)

When read the code: `pager.AddParam(ctx, "search", "search")`, the
question always comes: What is it doing? Where is the value from? Why
"search" / "search" ?

Now it is clear: `pager.AddParamIfExist("search", ctx.Data["search"])`
---
 routers/web/admin/repos.go       |  4 ++--
 routers/web/explore/code.go      |  2 +-
 routers/web/explore/repo.go      |  4 ++--
 routers/web/org/home.go          |  2 +-
 routers/web/org/projects.go      |  2 +-
 routers/web/repo/commit.go       |  4 ++--
 routers/web/repo/issue.go        | 20 ++++++++++----------
 routers/web/repo/milestone.go    |  4 ++--
 routers/web/repo/packages.go     |  4 ++--
 routers/web/repo/projects.go     |  2 +-
 routers/web/repo/search.go       |  2 +-
 routers/web/user/code.go         |  2 +-
 routers/web/user/home.go         | 24 ++++++++++++------------
 routers/web/user/notification.go |  4 ++--
 routers/web/user/package.go      |  4 ++--
 routers/web/user/profile.go      |  6 +++---
 services/context/pagination.go   | 17 ++++++++---------
 17 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/routers/web/admin/repos.go b/routers/web/admin/repos.go
index ddf4440167..5504037df0 100644
--- a/routers/web/admin/repos.go
+++ b/routers/web/admin/repos.go
@@ -84,7 +84,7 @@ func UnadoptedRepos(ctx *context.Context) {
 	if !doSearch {
 		pager := context.NewPagination(0, opts.PageSize, opts.Page, 5)
 		pager.SetDefaultParams(ctx)
-		pager.AddParam(ctx, "search", "search")
+		pager.AddParamIfExist("search", ctx.Data["search"])
 		ctx.Data["Page"] = pager
 		ctx.HTML(http.StatusOK, tplUnadoptedRepos)
 		return
@@ -98,7 +98,7 @@ func UnadoptedRepos(ctx *context.Context) {
 	ctx.Data["Dirs"] = repoNames
 	pager := context.NewPagination(count, opts.PageSize, opts.Page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "search", "search")
+	pager.AddParamIfExist("search", ctx.Data["search"])
 	ctx.Data["Page"] = pager
 	ctx.HTML(http.StatusOK, tplUnadoptedRepos)
 }
diff --git a/routers/web/explore/code.go b/routers/web/explore/code.go
index 7ba5984002..bfc798a97d 100644
--- a/routers/web/explore/code.go
+++ b/routers/web/explore/code.go
@@ -127,7 +127,7 @@ func Code(ctx *context.Context) {
 
 	pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "l", "Language")
+	pager.AddParamIfExist("l", ctx.Data["Language"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplExploreCode)
diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go
index cf7381512b..2cc22c50cf 100644
--- a/routers/web/explore/repo.go
+++ b/routers/web/explore/repo.go
@@ -169,8 +169,8 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
 
 	pager := context.NewPagination(int(count), opts.PageSize, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "topic", "TopicOnly")
-	pager.AddParam(ctx, "language", "Language")
+	pager.AddParamIfExist("topic", ctx.Data["TopicOnly"])
+	pager.AddParamIfExist("language", ctx.Data["Language"])
 	pager.AddParamString(relevantReposOnlyParam, fmt.Sprint(opts.OnlyShowRelevant))
 	ctx.Data["Page"] = pager
 
diff --git a/routers/web/org/home.go b/routers/web/org/home.go
index 71d10f3a43..947721dc41 100644
--- a/routers/web/org/home.go
+++ b/routers/web/org/home.go
@@ -154,7 +154,7 @@ func Home(ctx *context.Context) {
 
 	pager := context.NewPagination(int(count), setting.UI.User.RepoPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "language", "Language")
+	pager.AddParamIfExist("language", ctx.Data["Language"])
 	ctx.Data["Page"] = pager
 
 	ctx.Data["ShowMemberAndTeamTab"] = ctx.Org.IsMember || len(members) > 0
diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go
index ad8bb90d9e..094d14d194 100644
--- a/routers/web/org/projects.go
+++ b/routers/web/org/projects.go
@@ -120,7 +120,7 @@ func Projects(ctx *context.Context) {
 	}
 
 	pager := context.NewPagination(int(total), setting.UI.IssuePagingNum, page, numPages)
-	pager.AddParam(ctx, "state", "State")
+	pager.AddParamIfExist("state", ctx.Data["State"])
 	ctx.Data["Page"] = pager
 
 	ctx.Data["CanWriteProjects"] = canWriteProjects(ctx)
diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go
index 1b99f4183c..21fafd4901 100644
--- a/routers/web/repo/commit.go
+++ b/routers/web/repo/commit.go
@@ -163,8 +163,8 @@ func Graph(ctx *context.Context) {
 	ctx.Data["CommitCount"] = commitsCount
 
 	paginator := context.NewPagination(int(graphCommitsCount), setting.UI.GraphMaxCommitNum, page, 5)
-	paginator.AddParam(ctx, "mode", "Mode")
-	paginator.AddParam(ctx, "hide-pr-refs", "HidePRRefs")
+	paginator.AddParamIfExist("mode", ctx.Data["Mode"])
+	paginator.AddParamIfExist("hide-pr-refs", ctx.Data["HidePRRefs"])
 	for _, branch := range branches {
 		paginator.AddParamString("branch", branch)
 	}
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index d5fd8439f3..69e09371b3 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -472,16 +472,16 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
 	}
 	ctx.Data["ShowArchivedLabels"] = archived
 
-	pager.AddParam(ctx, "q", "Keyword")
-	pager.AddParam(ctx, "type", "ViewType")
-	pager.AddParam(ctx, "sort", "SortType")
-	pager.AddParam(ctx, "state", "State")
-	pager.AddParam(ctx, "labels", "SelectLabels")
-	pager.AddParam(ctx, "milestone", "MilestoneID")
-	pager.AddParam(ctx, "project", "ProjectID")
-	pager.AddParam(ctx, "assignee", "AssigneeID")
-	pager.AddParam(ctx, "poster", "PosterID")
-	pager.AddParam(ctx, "archived", "ShowArchivedLabels")
+	pager.AddParamIfExist("q", ctx.Data["Keyword"])
+	pager.AddParamIfExist("type", ctx.Data["ViewType"])
+	pager.AddParamIfExist("sort", ctx.Data["SortType"])
+	pager.AddParamIfExist("state", ctx.Data["State"])
+	pager.AddParamIfExist("labels", ctx.Data["SelectLabels"])
+	pager.AddParamIfExist("milestone", ctx.Data["MilestoneID"])
+	pager.AddParamIfExist("project", ctx.Data["ProjectID"])
+	pager.AddParamIfExist("assignee", ctx.Data["AssigneeID"])
+	pager.AddParamIfExist("poster", ctx.Data["PosterID"])
+	pager.AddParamIfExist("archived", ctx.Data["ShowArchivedLabels"])
 
 	ctx.Data["Page"] = pager
 }
diff --git a/routers/web/repo/milestone.go b/routers/web/repo/milestone.go
index c41b844ce4..2f7c2cb187 100644
--- a/routers/web/repo/milestone.go
+++ b/routers/web/repo/milestone.go
@@ -106,8 +106,8 @@ func Milestones(ctx *context.Context) {
 	ctx.Data["IsShowClosed"] = isShowClosed
 
 	pager := context.NewPagination(int(total), setting.UI.IssuePagingNum, page, 5)
-	pager.AddParam(ctx, "state", "State")
-	pager.AddParam(ctx, "q", "Keyword")
+	pager.AddParamIfExist("state", ctx.Data["State"])
+	pager.AddParamIfExist("q", ctx.Data["Keyword"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplMilestone)
diff --git a/routers/web/repo/packages.go b/routers/web/repo/packages.go
index 11874ab0d0..41fc38dedb 100644
--- a/routers/web/repo/packages.go
+++ b/routers/web/repo/packages.go
@@ -70,8 +70,8 @@ func Packages(ctx *context.Context) {
 	ctx.Data["RepositoryAccessMap"] = map[int64]bool{ctx.Repo.Repository.ID: true} // There is only the current repository
 
 	pager := context.NewPagination(int(total), setting.UI.PackagesPagingNum, page, 5)
-	pager.AddParam(ctx, "q", "Query")
-	pager.AddParam(ctx, "type", "PackageType")
+	pager.AddParamIfExist("q", ctx.Data["Query"])
+	pager.AddParamIfExist("type", ctx.Data["PackageType"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplPackagesList)
diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go
index 86909b5fd0..39f1b62ff7 100644
--- a/routers/web/repo/projects.go
+++ b/routers/web/repo/projects.go
@@ -118,7 +118,7 @@ func Projects(ctx *context.Context) {
 	}
 
 	pager := context.NewPagination(total, setting.UI.IssuePagingNum, page, numPages)
-	pager.AddParam(ctx, "state", "State")
+	pager.AddParamIfExist("state", ctx.Data["State"])
 	ctx.Data["Page"] = pager
 
 	ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects)
diff --git a/routers/web/repo/search.go b/routers/web/repo/search.go
index 16e620f57d..73537e64bc 100644
--- a/routers/web/repo/search.go
+++ b/routers/web/repo/search.go
@@ -59,7 +59,7 @@ func Search(ctx *context.Context) {
 
 	pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "l", "Language")
+	pager.AddParamIfExist("l", ctx.Data["Language"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplSearch)
diff --git a/routers/web/user/code.go b/routers/web/user/code.go
index 92911edfe9..a8ac50639d 100644
--- a/routers/web/user/code.go
+++ b/routers/web/user/code.go
@@ -112,7 +112,7 @@ func CodeSearch(ctx *context.Context) {
 
 	pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "l", "Language")
+	pager.AddParamIfExist("l", ctx.Data["Language"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplUserCode)
diff --git a/routers/web/user/home.go b/routers/web/user/home.go
index 4ec6f6be3f..dddd03e21f 100644
--- a/routers/web/user/home.go
+++ b/routers/web/user/home.go
@@ -133,7 +133,7 @@ func Dashboard(ctx *context.Context) {
 	ctx.Data["Feeds"] = feeds
 
 	pager := context.NewPagination(int(count), setting.UI.FeedPagingNum, page, 5)
-	pager.AddParam(ctx, "date", "Date")
+	pager.AddParamIfExist("date", ctx.Data["Date"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplDashboard)
@@ -329,10 +329,10 @@ func Milestones(ctx *context.Context) {
 	ctx.Data["IsShowClosed"] = isShowClosed
 
 	pager := context.NewPagination(pagerCount, setting.UI.IssuePagingNum, page, 5)
-	pager.AddParam(ctx, "q", "Keyword")
-	pager.AddParam(ctx, "repos", "RepoIDs")
-	pager.AddParam(ctx, "sort", "SortType")
-	pager.AddParam(ctx, "state", "State")
+	pager.AddParamIfExist("q", ctx.Data["Keyword"])
+	pager.AddParamIfExist("repos", ctx.Data["RepoIDs"])
+	pager.AddParamIfExist("sort", ctx.Data["SortType"])
+	pager.AddParamIfExist("state", ctx.Data["State"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplMilestones)
@@ -632,13 +632,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
 	}
 
 	pager := context.NewPagination(shownIssues, setting.UI.IssuePagingNum, page, 5)
-	pager.AddParam(ctx, "q", "Keyword")
-	pager.AddParam(ctx, "type", "ViewType")
-	pager.AddParam(ctx, "sort", "SortType")
-	pager.AddParam(ctx, "state", "State")
-	pager.AddParam(ctx, "labels", "SelectLabels")
-	pager.AddParam(ctx, "milestone", "MilestoneID")
-	pager.AddParam(ctx, "assignee", "AssigneeID")
+	pager.AddParamIfExist("q", ctx.Data["Keyword"])
+	pager.AddParamIfExist("type", ctx.Data["ViewType"])
+	pager.AddParamIfExist("sort", ctx.Data["SortType"])
+	pager.AddParamIfExist("state", ctx.Data["State"])
+	pager.AddParamIfExist("labels", ctx.Data["SelectLabels"])
+	pager.AddParamIfExist("milestone", ctx.Data["MilestoneID"])
+	pager.AddParamIfExist("assignee", ctx.Data["AssigneeID"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplIssues)
diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go
index 324205ed91..81afeae043 100644
--- a/routers/web/user/notification.go
+++ b/routers/web/user/notification.go
@@ -344,8 +344,8 @@ func NotificationSubscriptions(ctx *context.Context) {
 		ctx.Redirect(fmt.Sprintf("/notifications/subscriptions?page=%d", pager.Paginater.Current()))
 		return
 	}
-	pager.AddParam(ctx, "sort", "SortType")
-	pager.AddParam(ctx, "state", "State")
+	pager.AddParamIfExist("sort", ctx.Data["SortType"])
+	pager.AddParamIfExist("state", ctx.Data["State"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplNotificationSubscriptions)
diff --git a/routers/web/user/package.go b/routers/web/user/package.go
index 3ecc59a2ab..911ca12bf0 100644
--- a/routers/web/user/package.go
+++ b/routers/web/user/package.go
@@ -125,8 +125,8 @@ func ListPackages(ctx *context.Context) {
 	}
 
 	pager := context.NewPagination(int(total), setting.UI.PackagesPagingNum, page, 5)
-	pager.AddParam(ctx, "q", "Query")
-	pager.AddParam(ctx, "type", "PackageType")
+	pager.AddParamIfExist("q", ctx.Data["Query"])
+	pager.AddParamIfExist("type", ctx.Data["PackageType"])
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, tplPackagesList)
diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go
index 9851ea90a6..f9df511f60 100644
--- a/routers/web/user/profile.go
+++ b/routers/web/user/profile.go
@@ -324,12 +324,12 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
 
 	pager := context.NewPagination(total, pagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "tab", "TabName")
+	pager.AddParamIfExist("tab", ctx.Data["TabName"])
 	if tab != "followers" && tab != "following" && tab != "activity" && tab != "projects" {
-		pager.AddParam(ctx, "language", "Language")
+		pager.AddParamIfExist("language", ctx.Data["Language"])
 	}
 	if tab == "activity" {
-		pager.AddParam(ctx, "date", "Date")
+		pager.AddParamIfExist("date", ctx.Data["Date"])
 	}
 	ctx.Data["Page"] = pager
 }
diff --git a/services/context/pagination.go b/services/context/pagination.go
index 655a278f9f..11d37283c9 100644
--- a/services/context/pagination.go
+++ b/services/context/pagination.go
@@ -26,14 +26,13 @@ func NewPagination(total, pagingNum, current, numPages int) *Pagination {
 	return p
 }
 
-// AddParam adds a value from context identified by ctxKey as link param under a given paramKey
-func (p *Pagination) AddParam(ctx *Context, paramKey, ctxKey string) {
-	_, exists := ctx.Data[ctxKey]
-	if !exists {
+// AddParamIfExist adds a value to the query parameters if the value is not nil
+func (p *Pagination) AddParamIfExist(key string, val any) {
+	if val == nil {
 		return
 	}
-	paramData := fmt.Sprintf("%v", ctx.Data[ctxKey]) // cast any to string
-	urlParam := fmt.Sprintf("%s=%v", url.QueryEscape(paramKey), url.QueryEscape(paramData))
+	paramData := fmt.Sprint(val)
+	urlParam := fmt.Sprintf("%s=%v", url.QueryEscape(key), url.QueryEscape(paramData))
 	p.urlParams = append(p.urlParams, urlParam)
 }
 
@@ -50,8 +49,8 @@ func (p *Pagination) GetParams() template.URL {
 
 // SetDefaultParams sets common pagination params that are often used
 func (p *Pagination) SetDefaultParams(ctx *Context) {
-	p.AddParam(ctx, "sort", "SortType")
-	p.AddParam(ctx, "q", "Keyword")
+	p.AddParamIfExist("sort", ctx.Data["SortType"])
+	p.AddParamIfExist("q", ctx.Data["Keyword"])
 	// do not add any more uncommon params here!
-	p.AddParam(ctx, "fuzzy", "IsFuzzy")
+	p.AddParamIfExist("fuzzy", ctx.Data["IsFuzzy"])
 }