Merge pull request 'Rework when recently pushed branches are displayed' (#2195) from algernon/forgejo:b/recently-pushed/banner-logic-change into forgejo-dependency

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2195
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

models/git/branch.go

@@ -128,6 +128,10 @@ func (b *Branch) LoadDeletedBy(ctx context.Context) (err error) {
 	return err
 }
 
+func (b *Branch) GetRepo(ctx context.Context) (*repo_model.Repository, error) {
+	return repo_model.GetRepositoryByID(ctx, b.RepoID)
+}
+
 func (b *Branch) LoadPusher(ctx context.Context) (err error) {
 	if b.Pusher == nil && b.PusherID > 0 {
 		b.Pusher, err = user_model.GetUserByID(ctx, b.PusherID)

routers/web/repo/view.go

@@ -1002,20 +1002,43 @@ func renderCode(ctx *context.Context) {
 			return
 		}
 
-		showRecentlyPushedNewBranches := true
+		// If the repo is a mirror, don't display recently pushed branches.
-		if ctx.Repo.Repository.IsMirror ||
+		if ctx.Repo.Repository.IsMirror {
-			!ctx.Repo.Repository.UnitEnabled(ctx, unit_model.TypePullRequests) {
+			goto PostRecentBranchCheck
-			showRecentlyPushedNewBranches = false
 		}
-		if showRecentlyPushedNewBranches {
+
-			ctx.Data["RecentlyPushedNewBranches"], err = git_model.FindRecentlyPushedNewBranches(ctx, ctx.Repo.Repository.ID, ctx.Doer.ID, ctx.Repo.Repository.DefaultBranch)
+		// If pull requests aren't enabled for either the current repo, or its
-			if err != nil {
+		// base, don't display recently pushed branches.
-				ctx.ServerError("GetRecentlyPushedBranches", err)
+		if !(ctx.Repo.Repository.AllowsPulls(ctx) ||
-				return
+			(ctx.Repo.Repository.BaseRepo != nil && ctx.Repo.Repository.BaseRepo.AllowsPulls(ctx))) {
+			goto PostRecentBranchCheck
+		}
+
+		// Find recently pushed new branches to *this* repo.
+		branches, err := git_model.FindRecentlyPushedNewBranches(ctx, ctx.Repo.Repository.ID, ctx.Doer.ID, ctx.Repo.Repository.DefaultBranch)
+		if err != nil {
+			ctx.ServerError("FindRecentlyPushedBranches", err)
+			return
+		}
+
+		// If this is not a fork, check if the signed in user has a fork, and
+		// check branches there.
+		if !ctx.Repo.Repository.IsFork {
+			repo := repo_model.GetForkedRepo(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID)
+			if repo != nil {
+				baseBranches, err := git_model.FindRecentlyPushedNewBranches(ctx, repo.ID, ctx.Doer.ID, repo.DefaultBranch)
+				if err != nil {
+					ctx.ServerError("FindRecentlyPushedBranches", err)
+					return
+				}
+				branches = append(branches, baseBranches...)
 			}
 		}
+
+		ctx.Data["RecentlyPushedNewBranches"] = branches
 	}
 
+PostRecentBranchCheck:
 	var treeNames []string
 	paths := make([]string, 0, 5)
 	if len(ctx.Repo.TreePath) > 0 {

templates/repo/code/recently_pushed_new_branches.tmpl

@@ -2,10 +2,15 @@
 	<div class="ui positive message gt-df gt-ac">
 		<div class="gt-f1">
 			{{$timeSince := TimeSince .CommitTime.AsTime ctx.Locale}}
-			{{$branchLink := (print $.RepoLink "/src/branch/" (PathEscapeSegments .Name))}}
+			{{$repo := .GetRepo $.Context}}
-			{{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" (Escape .Name) $timeSince $branchLink | Safe}}
+			{{$name := .Name}}
+			{{if ne $repo.ID $.Repository.ID}}
+				{{$name = (print $repo.FullName ":" .Name)}}
+			{{end}}
+			{{$branchLink := (print ($repo.Link) "/src/branch/" (PathEscapeSegments .Name))}}
+			{{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" (Escape $name) $timeSince $branchLink | Safe}}
 		</div>
-		<a role="button" class="ui compact positive button gt-m-0" href="{{$.Repository.ComposeBranchCompareURL $.Repository.BaseRepo .Name}}">
+		<a role="button" class="ui compact positive button gt-m-0" href="{{$.Repository.ComposeBranchCompareURL $.Repository.BaseRepo $name}}">
 			{{ctx.Locale.Tr "repo.pulls.compare_changes"}}
 		</a>
 	</div>

tests/integration/pull_create_test.go

@@ -1,4 +1,5 @@
 // Copyright 2017 The Gitea Authors. All rights reserved.
+// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
@@ -12,7 +13,11 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	repo_model "code.gitea.io/gitea/models/repo"
+	unit_model "code.gitea.io/gitea/models/unit"
 	"code.gitea.io/gitea/modules/test"
+	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -162,3 +167,169 @@ func TestPullBranchDelete(t *testing.T) {
 		session.MakeRequest(t, req, http.StatusOK)
 	})
 }
+
+func TestRecentlyPushed(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, u *url.URL) {
+		session := loginUser(t, "user1")
+		testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+
+		testCreateBranch(t, session, "user1", "repo1", "branch/master", "recent-push", http.StatusSeeOther)
+		testEditFile(t, session, "user1", "repo1", "recent-push", "README.md", "Hello recently!\n")
+
+		testCreateBranch(t, session, "user2", "repo1", "branch/master", "recent-push-base", http.StatusSeeOther)
+		testEditFile(t, session, "user2", "repo1", "recent-push-base", "README.md", "Hello, recently, from base!\n")
+
+		baseRepo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user2", "repo1")
+		assert.NoError(t, err)
+		repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user1", "repo1")
+		assert.NoError(t, err)
+
+		enablePRs := func(t *testing.T, repo *repo_model.Repository) {
+			t.Helper()
+
+			err := repo_service.UpdateRepositoryUnits(db.DefaultContext, repo,
+				[]repo_model.RepoUnit{{
+					RepoID: repo.ID,
+					Type:   unit_model.TypePullRequests,
+				}},
+				nil)
+			assert.NoError(t, err)
+		}
+
+		disablePRs := func(t *testing.T, repo *repo_model.Repository) {
+			t.Helper()
+
+			err := repo_service.UpdateRepositoryUnits(db.DefaultContext, repo, nil,
+				[]unit_model.Type{unit_model.TypePullRequests})
+			assert.NoError(t, err)
+		}
+
+		testBanner := func(t *testing.T) {
+			t.Helper()
+
+			req := NewRequest(t, "GET", "/user1/repo1")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+
+			message := strings.TrimSpace(htmlDoc.Find(".ui.message").Text())
+			link, _ := htmlDoc.Find(".ui.message a").Attr("href")
+			expectedMessage := "You pushed on branch recent-push"
+
+			assert.Contains(t, message, expectedMessage)
+			assert.Equal(t, "/user1/repo1/src/branch/recent-push", link)
+		}
+
+		// Test that there's a recently pushed branches banner, and it contains
+		// a link to the branch.
+		t.Run("recently-pushed-banner", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			testBanner(t)
+		})
+
+		// Test that it is still there if the fork has PRs disabled, but the
+		// base repo still has them enabled.
+		t.Run("with-fork-prs-disabled", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			defer func() {
+				enablePRs(t, repo)
+			}()
+
+			disablePRs(t, repo)
+			testBanner(t)
+		})
+
+		// Test that it is still there if the fork has PRs enabled, but the base
+		// repo does not.
+		t.Run("with-base-prs-disabled", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			defer func() {
+				enablePRs(t, baseRepo)
+			}()
+
+			disablePRs(t, baseRepo)
+			testBanner(t)
+		})
+
+		// Test that the banner is not present if both the base and current
+		// repo have PRs disabled.
+		t.Run("with-prs-disabled", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			defer func() {
+				enablePRs(t, baseRepo)
+				enablePRs(t, repo)
+			}()
+
+			disablePRs(t, repo)
+			disablePRs(t, baseRepo)
+
+			req := NewRequest(t, "GET", "/user1/repo1")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			htmlDoc.AssertElement(t, ".ui.message", false)
+		})
+
+		// Test that visiting the base repo has the banner too, and includes
+		// recent push notifications from both the fork, and the base repo.
+		t.Run("on the base repo", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// Count recently pushed branches on the fork
+			req := NewRequest(t, "GET", "/user1/repo1")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			htmlDoc.AssertElement(t, ".ui.message", true)
+
+			// Count recently pushed branches on the base repo
+			req = NewRequest(t, "GET", "/user2/repo1")
+			resp = session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc = NewHTMLParser(t, resp.Body)
+			messageCountOnBase := htmlDoc.Find(".ui.message").Length()
+
+			// We have two messages on the base: one from the fork, one on the
+			// base itself.
+			assert.Equal(t, 2, messageCountOnBase)
+		})
+
+		// Test that the banner's links point to the right repos
+		t.Run("link validity", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// We're testing against the origin repo, because that has both
+			// local branches, and another from a fork, so we can test both in
+			// one test!
+
+			req := NewRequest(t, "GET", "/user2/repo1")
+			resp := session.MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+			messages := htmlDoc.Find(".ui.message")
+
+			prButtons := messages.Find("a[role='button']")
+			branchLinks := messages.Find("a[href*='/src/branch/']")
+
+			// ** base repo tests **
+			basePRLink, _ := prButtons.First().Attr("href")
+			baseBranchLink, _ := branchLinks.First().Attr("href")
+			baseBranchName := branchLinks.First().Text()
+
+			// branch in the same repo does not have a `user/repo:` qualifier.
+			assert.Equal(t, "recent-push-base", baseBranchName)
+			// branch link points to the same repo
+			assert.Equal(t, "/user2/repo1/src/branch/recent-push-base", baseBranchLink)
+			// PR link compares against the correct rep, and unqualified branch name
+			assert.Equal(t, "/user2/repo1/compare/master...recent-push-base", basePRLink)
+
+			// ** forked repo tests **
+			forkPRLink, _ := prButtons.Last().Attr("href")
+			forkBranchLink, _ := branchLinks.Last().Attr("href")
+			forkBranchName := branchLinks.Last().Text()
+
+			// branch in the forked repo has a `user/repo:` qualifier.
+			assert.Equal(t, "user1/repo1:recent-push", forkBranchName)
+			// branch link points to the forked repo
+			assert.Equal(t, "/user1/repo1/src/branch/recent-push", forkBranchLink)
+			// PR link compares against the correct rep, and qualified branch name
+			assert.Equal(t, "/user2/repo1/compare/master...user1/repo1:recent-push", forkPRLink)
+		})
+	})
+}