Skip to content

Commit 8d9ea68

Browse files
Fix push message behavior (#33215) (#33317)
Backport #33215 Manually resolved "reqctx" conflict --------- Co-authored-by: Chai-Shi <[email protected]>
1 parent bf664c2 commit 8d9ea68

File tree

6 files changed

+139
-135
lines changed

6 files changed

+139
-135
lines changed

Diff for: models/git/branch.go

+4
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,8 @@ type FindRecentlyPushedNewBranchesOptions struct {
440440
}
441441

442442
type RecentlyPushedNewBranch struct {
443+
BranchRepo *repo_model.Repository
444+
BranchName string
443445
BranchDisplayName string
444446
BranchLink string
445447
BranchCompareURL string
@@ -540,7 +542,9 @@ func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, o
540542
branchDisplayName = fmt.Sprintf("%s:%s", branch.Repo.FullName(), branchDisplayName)
541543
}
542544
newBranches = append(newBranches, &RecentlyPushedNewBranch{
545+
BranchRepo: branch.Repo,
543546
BranchDisplayName: branchDisplayName,
547+
BranchName: branch.Name,
544548
BranchLink: fmt.Sprintf("%s/src/branch/%s", branch.Repo.Link(), util.PathEscapeSegments(branch.Name)),
545549
BranchCompareURL: branch.Repo.ComposeBranchCompareURL(opts.BaseRepo, branch.Name),
546550
CommitTime: branch.CommitTime,

Diff for: routers/web/repo/view_home.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,28 @@ func prepareRecentlyPushedNewBranches(ctx *context.Context) {
215215
if !opts.Repo.IsMirror && !opts.BaseRepo.IsMirror &&
216216
opts.BaseRepo.UnitEnabled(ctx, unit_model.TypePullRequests) &&
217217
baseRepoPerm.CanRead(unit_model.TypePullRequests) {
218-
ctx.Data["RecentlyPushedNewBranches"], err = git_model.FindRecentlyPushedNewBranches(ctx, ctx.Doer, opts)
218+
var finalBranches []*git_model.RecentlyPushedNewBranch
219+
branches, err := git_model.FindRecentlyPushedNewBranches(ctx, ctx.Doer, opts)
219220
if err != nil {
220221
log.Error("FindRecentlyPushedNewBranches failed: %v", err)
221222
}
223+
224+
for _, branch := range branches {
225+
divergingInfo, err := repo_service.GetBranchDivergingInfo(ctx,
226+
branch.BranchRepo, branch.BranchName, // "base" repo for diverging info
227+
opts.BaseRepo, opts.BaseRepo.DefaultBranch, // "head" repo for diverging info
228+
)
229+
if err != nil {
230+
log.Error("GetBranchDivergingInfo failed: %v", err)
231+
continue
232+
}
233+
branchRepoHasNewCommits := divergingInfo.BaseHasNewCommits
234+
baseRepoCommitsBehind := divergingInfo.HeadCommitsBehind
235+
if branchRepoHasNewCommits || baseRepoCommitsBehind > 0 {
236+
finalBranches = append(finalBranches, branch)
237+
}
238+
}
239+
ctx.Data["RecentlyPushedNewBranches"] = finalBranches
222240
}
223241
}
224242
}

Diff for: services/repository/branch.go

+58
Original file line numberDiff line numberDiff line change
@@ -636,3 +636,61 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR
636636

637637
return nil
638638
}
639+
640+
// BranchDivergingInfo contains the information about the divergence of a head branch to the base branch.
641+
// This struct is also used in templates, so it needs to search for all references before changing it.
642+
type BranchDivergingInfo struct {
643+
BaseHasNewCommits bool
644+
HeadCommitsBehind int
645+
HeadCommitsAhead int
646+
}
647+
648+
// GetBranchDivergingInfo returns the information about the divergence of a patch branch to the base branch.
649+
func GetBranchDivergingInfo(ctx context.Context, baseRepo *repo_model.Repository, baseBranch string, headRepo *repo_model.Repository, headBranch string) (*BranchDivergingInfo, error) {
650+
headGitBranch, err := git_model.GetBranch(ctx, headRepo.ID, headBranch)
651+
if err != nil {
652+
return nil, err
653+
}
654+
655+
baseGitBranch, err := git_model.GetBranch(ctx, baseRepo.ID, baseBranch)
656+
if err != nil {
657+
return nil, err
658+
}
659+
660+
info := &BranchDivergingInfo{}
661+
if headGitBranch.CommitID == baseGitBranch.CommitID {
662+
return info, nil
663+
}
664+
665+
// if the fork repo has new commits, this call will fail because they are not in the base repo
666+
// exit status 128 - fatal: Invalid symmetric difference expression aaaaaaaaaaaa...bbbbbbbbbbbb
667+
// so at the moment, we first check the update time, then check whether the fork branch has base's head
668+
diff, err := git.GetDivergingCommits(ctx, baseRepo.RepoPath(), baseGitBranch.CommitID, headGitBranch.CommitID)
669+
if err != nil {
670+
info.BaseHasNewCommits = baseGitBranch.UpdatedUnix > headGitBranch.UpdatedUnix
671+
if headRepo.IsFork && info.BaseHasNewCommits {
672+
return info, nil
673+
}
674+
// if the base's update time is before the fork, check whether the base's head is in the fork
675+
headGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, headRepo)
676+
if err != nil {
677+
return nil, err
678+
}
679+
defer closer.Close()
680+
681+
headCommit, err := headGitRepo.GetCommit(headGitBranch.CommitID)
682+
if err != nil {
683+
return nil, err
684+
}
685+
baseCommitID, err := git.NewIDFromString(baseGitBranch.CommitID)
686+
if err != nil {
687+
return nil, err
688+
}
689+
hasPreviousCommit, _ := headCommit.HasPreviousCommit(baseCommitID)
690+
info.BaseHasNewCommits = !hasPreviousCommit
691+
return info, nil
692+
}
693+
694+
info.HeadCommitsBehind, info.HeadCommitsAhead = diff.Behind, diff.Ahead
695+
return info, nil
696+
}

Diff for: services/repository/merge_upstream.go

+5-65
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,15 @@ import (
77
"context"
88
"fmt"
99

10-
git_model "code.gitea.io/gitea/models/git"
1110
issue_model "code.gitea.io/gitea/models/issues"
1211
repo_model "code.gitea.io/gitea/models/repo"
1312
user_model "code.gitea.io/gitea/models/user"
1413
"code.gitea.io/gitea/modules/git"
15-
"code.gitea.io/gitea/modules/gitrepo"
1614
repo_module "code.gitea.io/gitea/modules/repository"
1715
"code.gitea.io/gitea/modules/util"
1816
"code.gitea.io/gitea/services/pull"
1917
)
2018

21-
type UpstreamDivergingInfo struct {
22-
BaseHasNewCommits bool
23-
CommitsBehind int
24-
CommitsAhead int
25-
}
26-
2719
// MergeUpstream merges the base repository's default branch into the fork repository's current branch.
2820
func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branch string) (mergeStyle string, err error) {
2921
if err = repo.MustNotBeArchived(); err != nil {
@@ -77,70 +69,18 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.
7769
}
7870

7971
// GetUpstreamDivergingInfo returns the information about the divergence between the fork repository's branch and the base repository's default branch.
80-
func GetUpstreamDivergingInfo(ctx context.Context, repo *repo_model.Repository, branch string) (*UpstreamDivergingInfo, error) {
81-
if !repo.IsFork {
72+
func GetUpstreamDivergingInfo(ctx context.Context, forkRepo *repo_model.Repository, forkBranch string) (*BranchDivergingInfo, error) {
73+
if !forkRepo.IsFork {
8274
return nil, util.NewInvalidArgumentErrorf("repo is not a fork")
8375
}
8476

85-
if repo.IsArchived {
77+
if forkRepo.IsArchived {
8678
return nil, util.NewInvalidArgumentErrorf("repo is archived")
8779
}
8880

89-
if err := repo.GetBaseRepo(ctx); err != nil {
90-
return nil, err
91-
}
92-
93-
forkBranch, err := git_model.GetBranch(ctx, repo.ID, branch)
94-
if err != nil {
95-
return nil, err
96-
}
97-
98-
baseBranch, err := git_model.GetBranch(ctx, repo.BaseRepo.ID, repo.BaseRepo.DefaultBranch)
99-
if err != nil {
81+
if err := forkRepo.GetBaseRepo(ctx); err != nil {
10082
return nil, err
10183
}
10284

103-
info := &UpstreamDivergingInfo{}
104-
if forkBranch.CommitID == baseBranch.CommitID {
105-
return info, nil
106-
}
107-
108-
// if the fork repo has new commits, this call will fail because they are not in the base repo
109-
// exit status 128 - fatal: Invalid symmetric difference expression aaaaaaaaaaaa...bbbbbbbbbbbb
110-
// so at the moment, we first check the update time, then check whether the fork branch has base's head
111-
diff, err := git.GetDivergingCommits(ctx, repo.BaseRepo.RepoPath(), baseBranch.CommitID, forkBranch.CommitID)
112-
if err != nil {
113-
info.BaseHasNewCommits = baseBranch.UpdatedUnix > forkBranch.UpdatedUnix
114-
if info.BaseHasNewCommits {
115-
return info, nil
116-
}
117-
118-
// if the base's update time is before the fork, check whether the base's head is in the fork
119-
baseGitRepo, baseGitRepoCloser, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo.BaseRepo)
120-
if err != nil {
121-
return nil, err
122-
}
123-
defer baseGitRepoCloser.Close()
124-
125-
headGitRepo, headGitRepoCloser, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
126-
if err != nil {
127-
return nil, err
128-
}
129-
defer headGitRepoCloser.Close()
130-
131-
baseCommitID, err := baseGitRepo.ConvertToGitID(baseBranch.CommitID)
132-
if err != nil {
133-
return nil, err
134-
}
135-
headCommit, err := headGitRepo.GetCommit(forkBranch.CommitID)
136-
if err != nil {
137-
return nil, err
138-
}
139-
hasPreviousCommit, _ := headCommit.HasPreviousCommit(baseCommitID)
140-
info.BaseHasNewCommits = !hasPreviousCommit
141-
return info, nil
142-
}
143-
144-
info.CommitsBehind, info.CommitsAhead = diff.Behind, diff.Ahead
145-
return info, nil
85+
return GetBranchDivergingInfo(ctx, forkRepo.BaseRepo, forkRepo.BaseRepo.DefaultBranch, forkRepo, forkBranch)
14686
}

Diff for: templates/repo/code/upstream_diverging_info.tmpl

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseHasNewCommits .UpstreamDivergingInfo.CommitsBehind)}}
1+
{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseHasNewCommits .UpstreamDivergingInfo.HeadCommitsBehind)}}
22
<div class="ui message flex-text-block">
33
<div class="tw-flex-1">
44
{{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.Repository.BaseRepo.DefaultBranch|PathEscapeSegments)}}
55
{{$upstreamHtml := HTMLFormat `<a href="%s">%s:%s</a>` $upstreamLink .Repository.BaseRepo.FullName .Repository.BaseRepo.DefaultBranch}}
6-
{{if .UpstreamDivergingInfo.CommitsBehind}}
7-
{{ctx.Locale.TrN .UpstreamDivergingInfo.CommitsBehind "repo.pulls.upstream_diverging_prompt_behind_1" "repo.pulls.upstream_diverging_prompt_behind_n" .UpstreamDivergingInfo.CommitsBehind $upstreamHtml}}
6+
{{if .UpstreamDivergingInfo.HeadCommitsBehind}}
7+
{{ctx.Locale.TrN .UpstreamDivergingInfo.HeadCommitsBehind "repo.pulls.upstream_diverging_prompt_behind_1" "repo.pulls.upstream_diverging_prompt_behind_n" .UpstreamDivergingInfo.HeadCommitsBehind $upstreamHtml}}
88
{{else}}
99
{{ctx.Locale.Tr "repo.pulls.upstream_diverging_prompt_base_newer" $upstreamHtml}}
1010
{{end}}

Diff for: tests/integration/repo_branch_test.go

+50-66
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,8 @@ import (
1111
"strings"
1212
"testing"
1313

14-
auth_model "code.gitea.io/gitea/models/auth"
15-
org_model "code.gitea.io/gitea/models/organization"
16-
"code.gitea.io/gitea/models/perm"
1714
repo_model "code.gitea.io/gitea/models/repo"
18-
"code.gitea.io/gitea/models/unit"
1915
"code.gitea.io/gitea/models/unittest"
20-
api "code.gitea.io/gitea/modules/structs"
2116
"code.gitea.io/gitea/modules/test"
2217
"code.gitea.io/gitea/modules/translation"
2318
"code.gitea.io/gitea/tests"
@@ -142,19 +137,51 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
142137
assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
143138
}
144139

145-
func prepareBranch(t *testing.T, session *TestSession, repo *repo_model.Repository) {
146-
baseRefSubURL := fmt.Sprintf("branch/%s", repo.DefaultBranch)
147-
140+
func prepareRecentlyPushedBranchTest(t *testing.T, headSession *TestSession, baseRepo, headRepo *repo_model.Repository) {
141+
refSubURL := fmt.Sprintf("branch/%s", headRepo.DefaultBranch)
142+
baseRepoPath := baseRepo.OwnerName + "/" + baseRepo.Name
143+
headRepoPath := headRepo.OwnerName + "/" + headRepo.Name
144+
// Case 1: Normal branch changeset to display pushed message
148145
// create branch with no new commit
149-
testCreateBranch(t, session, repo.OwnerName, repo.Name, baseRefSubURL, "no-commit", http.StatusSeeOther)
146+
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, refSubURL, "no-commit", http.StatusSeeOther)
150147

151148
// create branch with commit
152-
testCreateBranch(t, session, repo.OwnerName, repo.Name, baseRefSubURL, "new-commit", http.StatusSeeOther)
153-
testAPINewFile(t, session, repo.OwnerName, repo.Name, "new-commit", "new-commit.txt", "new-commit")
149+
testAPINewFile(t, headSession, headRepo.OwnerName, headRepo.Name, "new-commit", fmt.Sprintf("new-file-%s.txt", headRepo.Name), "new-commit")
150+
151+
// create a branch then delete it
152+
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "deleted-branch", http.StatusSeeOther)
153+
testUIDeleteBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "deleted-branch")
154+
155+
// only `new-commit` branch has commits ahead the base branch
156+
checkRecentlyPushedNewBranches(t, headSession, headRepoPath, []string{"new-commit"})
157+
if baseRepo.RepoPath() != headRepo.RepoPath() {
158+
checkRecentlyPushedNewBranches(t, headSession, baseRepoPath, []string{fmt.Sprintf("%v:new-commit", headRepo.FullName())})
159+
}
160+
161+
// Case 2: Create PR so that `new-commit` branch will not show
162+
testCreatePullToDefaultBranch(t, headSession, baseRepo, headRepo, "new-commit", "merge new-commit to default branch")
163+
// No push message show because of active PR
164+
checkRecentlyPushedNewBranches(t, headSession, headRepoPath, []string{})
165+
if baseRepo.RepoPath() != headRepo.RepoPath() {
166+
checkRecentlyPushedNewBranches(t, headSession, baseRepoPath, []string{})
167+
}
168+
}
169+
170+
func prepareRecentlyPushedBranchSpecialTest(t *testing.T, session *TestSession, baseRepo, headRepo *repo_model.Repository) {
171+
refSubURL := fmt.Sprintf("branch/%s", headRepo.DefaultBranch)
172+
baseRepoPath := baseRepo.OwnerName + "/" + baseRepo.Name
173+
headRepoPath := headRepo.OwnerName + "/" + headRepo.Name
174+
// create branch with no new commit
175+
testCreateBranch(t, session, headRepo.OwnerName, headRepo.Name, refSubURL, "no-commit-special", http.StatusSeeOther)
176+
177+
// update base (default) branch before head branch is updated
178+
testAPINewFile(t, session, baseRepo.OwnerName, baseRepo.Name, baseRepo.DefaultBranch, fmt.Sprintf("new-file-special-%s.txt", headRepo.Name), "new-commit")
154179

155-
// create deleted branch
156-
testCreateBranch(t, session, repo.OwnerName, repo.Name, "branch/new-commit", "deleted-branch", http.StatusSeeOther)
157-
testUIDeleteBranch(t, session, repo.OwnerName, repo.Name, "deleted-branch")
180+
// Though we have new `no-commit` branch, but the headBranch is not newer or commits ahead baseBranch. No message show.
181+
checkRecentlyPushedNewBranches(t, session, headRepoPath, []string{})
182+
if baseRepo.RepoPath() != headRepo.RepoPath() {
183+
checkRecentlyPushedNewBranches(t, session, baseRepoPath, []string{})
184+
}
158185
}
159186

160187
func testCreatePullToDefaultBranch(t *testing.T, session *TestSession, baseRepo, headRepo *repo_model.Repository, headBranch, title string) string {
@@ -169,6 +196,9 @@ func testCreatePullToDefaultBranch(t *testing.T, session *TestSession, baseRepo,
169196
}
170197

171198
func prepareRepoPR(t *testing.T, baseSession, headSession *TestSession, baseRepo, headRepo *repo_model.Repository) {
199+
refSubURL := fmt.Sprintf("branch/%s", headRepo.DefaultBranch)
200+
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, refSubURL, "new-commit", http.StatusSeeOther)
201+
172202
// create opening PR
173203
testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "opening-pr", http.StatusSeeOther)
174204
testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "opening-pr", "opening pr")
@@ -210,65 +240,19 @@ func checkRecentlyPushedNewBranches(t *testing.T, session *TestSession, repoPath
210240

211241
func TestRecentlyPushedNewBranches(t *testing.T) {
212242
onGiteaRun(t, func(t *testing.T, u *url.URL) {
213-
user1Session := loginUser(t, "user1")
214-
user2Session := loginUser(t, "user2")
215243
user12Session := loginUser(t, "user12")
216-
user13Session := loginUser(t, "user13")
217244

218-
// prepare branch and PRs in original repo
245+
// Same reposioty check
219246
repo10 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
220-
prepareBranch(t, user12Session, repo10)
221247
prepareRepoPR(t, user12Session, user12Session, repo10, repo10)
222-
223-
// outdated new branch should not be displayed
224-
checkRecentlyPushedNewBranches(t, user12Session, "user12/repo10", []string{"new-commit"})
248+
prepareRecentlyPushedBranchTest(t, user12Session, repo10, repo10)
249+
prepareRecentlyPushedBranchSpecialTest(t, user12Session, repo10, repo10)
225250

226251
// create a fork repo in public org
227-
testRepoFork(t, user12Session, repo10.OwnerName, repo10.Name, "org25", "org25_fork_repo10", "new-commit")
252+
testRepoFork(t, user12Session, repo10.OwnerName, repo10.Name, "org25", "org25_fork_repo10", repo10.DefaultBranch)
228253
orgPublicForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 25, Name: "org25_fork_repo10"})
229254
prepareRepoPR(t, user12Session, user12Session, repo10, orgPublicForkRepo)
230-
231-
// user12 is the owner of the repo10 and the organization org25
232-
// in repo10, user12 has opening/closed/merged pr and closed/merged pr with deleted branch
233-
checkRecentlyPushedNewBranches(t, user12Session, "user12/repo10", []string{"org25/org25_fork_repo10:new-commit", "new-commit"})
234-
235-
userForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11})
236-
testCtx := NewAPITestContext(t, repo10.OwnerName, repo10.Name, auth_model.AccessTokenScopeWriteRepository)
237-
t.Run("AddUser13AsCollaborator", doAPIAddCollaborator(testCtx, "user13", perm.AccessModeWrite))
238-
prepareBranch(t, user13Session, userForkRepo)
239-
prepareRepoPR(t, user13Session, user13Session, repo10, userForkRepo)
240-
241-
// create branch with same name in different repo by user13
242-
testCreateBranch(t, user13Session, repo10.OwnerName, repo10.Name, "branch/new-commit", "same-name-branch", http.StatusSeeOther)
243-
testCreateBranch(t, user13Session, userForkRepo.OwnerName, userForkRepo.Name, "branch/new-commit", "same-name-branch", http.StatusSeeOther)
244-
testCreatePullToDefaultBranch(t, user13Session, repo10, userForkRepo, "same-name-branch", "same name branch pr")
245-
246-
// user13 pushed 2 branches with the same name in repo10 and repo11
247-
// and repo11's branch has a pr, but repo10's branch doesn't
248-
// in this case, we should get repo10's branch but not repo11's branch
249-
checkRecentlyPushedNewBranches(t, user13Session, "user12/repo10", []string{"same-name-branch", "user13/repo11:new-commit"})
250-
251-
// create a fork repo in private org
252-
testRepoFork(t, user1Session, repo10.OwnerName, repo10.Name, "private_org35", "org35_fork_repo10", "new-commit")
253-
orgPrivateForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 35, Name: "org35_fork_repo10"})
254-
prepareRepoPR(t, user1Session, user1Session, repo10, orgPrivateForkRepo)
255-
256-
// user1 is the owner of private_org35 and no write permission to repo10
257-
// so user1 can only see the branch in org35_fork_repo10
258-
checkRecentlyPushedNewBranches(t, user1Session, "user12/repo10", []string{"private_org35/org35_fork_repo10:new-commit"})
259-
260-
// user2 push a branch in private_org35
261-
testCreateBranch(t, user2Session, orgPrivateForkRepo.OwnerName, orgPrivateForkRepo.Name, "branch/new-commit", "user-read-permission", http.StatusSeeOther)
262-
// convert write permission to read permission for code unit
263-
token := getTokenForLoggedInUser(t, user1Session, auth_model.AccessTokenScopeWriteOrganization)
264-
req := NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", 24), &api.EditTeamOption{
265-
Name: "team24",
266-
UnitsMap: map[string]string{"repo.code": "read"},
267-
}).AddTokenAuth(token)
268-
MakeRequest(t, req, http.StatusOK)
269-
teamUnit := unittest.AssertExistsAndLoadBean(t, &org_model.TeamUnit{TeamID: 24, Type: unit.TypeCode})
270-
assert.Equal(t, perm.AccessModeRead, teamUnit.AccessMode)
271-
// user2 can see the branch as it is created by user2
272-
checkRecentlyPushedNewBranches(t, user2Session, "user12/repo10", []string{"private_org35/org35_fork_repo10:user-read-permission"})
255+
prepareRecentlyPushedBranchTest(t, user12Session, repo10, orgPublicForkRepo)
256+
prepareRecentlyPushedBranchSpecialTest(t, user12Session, repo10, orgPublicForkRepo)
273257
})
274258
}

0 commit comments

Comments
 (0)