Merge pull request 'Better number for UserCards pagination' (#2584) from 0ko/forgejo:its39 into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2584
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org>

modules/setting/repository.go

@@ -22,8 +22,12 @@ const (
 
 var RecognisedRepositoryDownloadOrCloneMethods = []string{"download-zip", "download-targz", "download-bundle", "vscode-clone", "vscodium-clone", "cite"}
 
-// ItemsPerPage maximum items per page in forks, watchers and stars of a repo
+// MaxUserCardsPerPage sets maximum amount of watchers and stargazers shown per page
-const ItemsPerPage = 40
+// those pages use 2 or 3 column layout, so the value should be divisible by 2 and 3
+var MaxUserCardsPerPage = 36
+
+// MaxForksPerPage sets maximum amount of forks shown per page
+var MaxForksPerPage = 40
 
 // Repository settings
 var (

routers/web/repo/view.go

@@ -1127,12 +1127,12 @@ func RenderUserCards(ctx *context.Context, total int, getter func(opts db.ListOp
 	if page <= 0 {
 		page = 1
 	}
-	pager := context.NewPagination(total, setting.ItemsPerPage, page, 5)
+	pager := context.NewPagination(total, setting.MaxUserCardsPerPage, page, 5)
 	ctx.Data["Page"] = pager
 
 	items, err := getter(db.ListOptions{
 		Page:     pager.Paginater.Current(),
-		PageSize: setting.ItemsPerPage,
+		PageSize: setting.MaxUserCardsPerPage,
 	})
 	if err != nil {
 		ctx.ServerError("getter", err)
@@ -1173,12 +1173,12 @@ func Forks(ctx *context.Context) {
 		page = 1
 	}
 
-	pager := context.NewPagination(ctx.Repo.Repository.NumForks, setting.ItemsPerPage, page, 5)
+	pager := context.NewPagination(ctx.Repo.Repository.NumForks, setting.MaxForksPerPage, page, 5)
 	ctx.Data["Page"] = pager
 
 	forks, err := repo_model.GetForks(ctx, ctx.Repo.Repository, db.ListOptions{
 		Page:     pager.Paginater.Current(),
-		PageSize: setting.ItemsPerPage,
+		PageSize: setting.MaxForksPerPage,
 	})
 	if err != nil {
 		ctx.ServerError("GetForks", err)

tests/integration/repo_pagination_test.go

@@ -0,0 +1,83 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"net/http"
+	"path"
+	"testing"
+
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/tests"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRepoPaginations(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	t.Run("Fork", func(t *testing.T) {
+		// Make forks of user2/repo1
+		session := loginUser(t, "user2")
+		testRepoFork(t, session, "user2", "repo1", "org3", "repo1")
+		session = loginUser(t, "user5")
+		testRepoFork(t, session, "user2", "repo1", "org6", "repo1")
+
+		unittest.AssertCount(t, &repo_model.Repository{ForkID: 1}, 2)
+
+		testRepoPagination(t, session, "user2/repo1", "forks", &setting.MaxForksPerPage)
+	})
+	t.Run("Stars", func(t *testing.T) {
+		// Add stars to user2/repo1.
+		session := loginUser(t, "user2")
+		req := NewRequestWithValues(t, "POST", "/user2/repo1/action/star", map[string]string{
+			"_csrf": GetCSRF(t, session, "/user2/repo1"),
+		})
+		session.MakeRequest(t, req, http.StatusOK)
+
+		session = loginUser(t, "user1")
+		req = NewRequestWithValues(t, "POST", "/user2/repo1/action/star", map[string]string{
+			"_csrf": GetCSRF(t, session, "/user2/repo1"),
+		})
+		session.MakeRequest(t, req, http.StatusOK)
+
+		testRepoPagination(t, session, "user2/repo1", "stars", &setting.MaxUserCardsPerPage)
+	})
+	t.Run("Watcher", func(t *testing.T) {
+		// user2/repo2 is watched by its creator user2. Watch it by user1 to make it watched by 2 users.
+		session := loginUser(t, "user1")
+		req := NewRequestWithValues(t, "POST", "/user2/repo2/action/watch", map[string]string{
+			"_csrf": GetCSRF(t, session, "/user2/repo2"),
+		})
+		session.MakeRequest(t, req, http.StatusOK)
+
+		testRepoPagination(t, session, "user2/repo2", "watchers", &setting.MaxUserCardsPerPage)
+	})
+}
+
+func testRepoPagination(t *testing.T, session *TestSession, repo, kind string, mockableVar *int) {
+	t.Run("Should paginate", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+		defer test.MockVariableValue(mockableVar, 1)()
+		req := NewRequest(t, "GET", "/"+path.Join(repo, kind))
+		resp := session.MakeRequest(t, req, http.StatusOK)
+		htmlDoc := NewHTMLParser(t, resp.Body)
+
+		paginationButton := htmlDoc.Find(".item.navigation[href='/" + path.Join(repo, kind) + "?page=2']")
+		// Next and Last button.
+		assert.Equal(t, 2, paginationButton.Length())
+	})
+
+	t.Run("Shouldn't paginate", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+		defer test.MockVariableValue(mockableVar, 2)()
+		req := NewRequest(t, "GET", "/"+path.Join(repo, kind))
+		resp := session.MakeRequest(t, req, http.StatusOK)
+		htmlDoc := NewHTMLParser(t, resp.Body)
+
+		htmlDoc.AssertElement(t, ".item.navigation[href='/"+path.Join(repo, kind)+"?page=2']", false)
+	})
+}