From bf7fb89178f41c712b8a8863667a442ec1e1c5d4 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Fri, 23 Feb 2024 01:26:17 +0100 Subject: [PATCH] [UI] Agit: Add AGit label to AGit-created PRs Adds a label to Pull Requests that were created using AGit-Flow, in order to prevent situations where a contributor uses AGit-Flow to push new changes - only to realize that they did not use AGit-Flow in the first place, and that they just opened a new PR accidentally (that was me). Also intended to raise general awareness about the feature. Some additional work, such as adding a tooltip, still needs to be done. A small typo fix for a comment and (exclusively) formatting fixes in the copyright header are also included. Refs: https://codeberg.org/forgejo/forgejo/issues/2433 --- options/locale/locale_en-US.ini | 1 + routers/web/repo/pull.go | 10 ++++++++-- templates/repo/issue/view_title.tmpl | 10 +++++++++- tests/integration/git_test.go | 26 +++++++++++++++++++++++++- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index bdae9a29ac..8253c6ced5 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1505,6 +1505,7 @@ issues.action_check_all = Check/Uncheck all items issues.opened_by = opened %[1]s by %[3]s pulls.merged_by = by %[3]s was merged %[1]s pulls.merged_by_fake = by %[2]s was merged %[1]s +pulls.made_using_agit = AGit issues.closed_by = by %[3]s was closed %[1]s issues.opened_by_fake = opened %[1]s by %[2]s issues.closed_by_fake = by %[2]s was closed %[1]s diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ab821f8884..ac244b1551 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1,5 +1,6 @@ -// Copyright 2018 The Gitea Authors. -// Copyright 2014 The Gogs Authors. +// Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2018 The Gitea Authors. All rights reserved. +// Copyright 2024 The Forgejo Authors. All rights reserved. // All rights reserved. // SPDX-License-Identifier: MIT @@ -381,6 +382,11 @@ func setMergeTarget(ctx *context.Context, pull *issues_model.PullRequest) { } else { ctx.Data["HeadTarget"] = pull.MustHeadUserName(ctx) + "/" + pull.HeadRepo.Name + ":" + pull.HeadBranch } + + if pull.Flow == issues_model.PullRequestFlowAGit { + ctx.Data["MadeUsingAGit"] = true + } + ctx.Data["BaseTarget"] = pull.BaseBranch ctx.Data["HeadBranchLink"] = pull.GetHeadBranchLink(ctx) ctx.Data["BaseBranchLink"] = pull.GetBaseBranchLink(ctx) diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index 582e9864fb..8a5954681b 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -47,7 +47,9 @@ {{if .HeadBranchLink}} {{$headHref = printf `%s` (.HeadBranchLink | Escape) $headHref}} {{end}} - {{$headHref = printf `%s ` $headHref (ctx.Locale.Tr "copy_branch") (.HeadTarget | Escape) (svg "octicon-copy" 14)}} + {{if not .MadeUsingAGit}} + {{$headHref = printf `%s ` $headHref (ctx.Locale.Tr "copy_branch") (.HeadTarget | Escape) (svg "octicon-copy" 14)}} + {{end}} {{$baseHref := .BaseTarget|Escape}} {{if .BaseBranchLink}} {{$baseHref = printf `%s` (.BaseBranchLink | Escape) $baseHref}} @@ -70,6 +72,12 @@ {{ctx.Locale.Tr "repo.pulls.title_desc" .NumCommits ($headHref|Safe) ($baseHref|Safe)}} {{end}} + {{if .MadeUsingAGit}} + {{/* TODO: Add tooltip and a link to the documentation */}} + + {{ctx.Locale.Tr "repo.pulls.made_using_agit"}} + + {{end}}
diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index d02427ffcf..ead649490e 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -450,7 +450,7 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun var pr api.PullRequest var err error - // Create a test pullrequest + // Create a test pull request t.Run("CreatePullRequest", func(t *testing.T) { pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) assert.NoError(t, err) @@ -470,6 +470,19 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun } t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, true)) + // Confirm that there is no AGit Label + // TODO: Refactor and move this check to a function + t.Run("AGitLabelIsMissing", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, ctx.Username) + + req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", baseCtx.Username, baseCtx.Reponame, pr.Index)) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + htmlDoc.AssertElement(t, "#agit-label", false) + }) + // Then get the diff string var diffHash string var diffLength int @@ -813,6 +826,17 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB return } + t.Run("AGitLabelIsPresent", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, ctx.Username) + + req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr2.Index)) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + htmlDoc.AssertElement(t, "#agit-label", true) + }) + t.Run("AddCommit2", func(t *testing.T) { err := os.WriteFile(path.Join(dstPath, "test_file"), []byte("## test content \n ## test content 2"), 0o666) if !assert.NoError(t, err) {