Skip to content

Commit

Permalink
[GITEA] pulls: "Edit File" button in "Files Changed" tab
Browse files Browse the repository at this point in the history
Closes #1894.
Gitea issue: go-gitea/gitea#23848

(cherry picked from commit 79c7516)
(cherry picked from commit 58c76aa)
(cherry picked from commit 5bdb3c6)
(cherry picked from commit 94e954c)
  • Loading branch information
wetneb authored and earl-warren committed Jan 22, 2024
1 parent 23c887f commit 1388e7c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 8 deletions.
12 changes: 12 additions & 0 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,18 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return
}

// determine if the user viewing the pull request can edit the head branch
if ctx.Doer != nil && pull.HeadRepo != nil && !pull.HasMerged {
headRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return
}
ctx.Data["HeadBranchIsEditable"] = pull.HeadRepo.CanEnableEditor() && issues_model.CanMaintainerWriteToBranch(ctx, headRepoPerm, pull.HeadBranch, ctx.Doer)
ctx.Data["SourceRepoLink"] = pull.HeadRepo.Link()
ctx.Data["HeadBranch"] = pull.HeadBranch
}

if ctx.IsSigned && ctx.Doer != nil {
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err)
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@
<a class="ui basic tiny button" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{else}}
<a class="ui basic tiny button" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{if and $.HeadBranchIsEditable (not $file.IsLFSFile)}}
<a class="ui basic tiny button" rel="nofollow" href="{{$.SourceRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranch}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
{{end}}
{{end}}
{{end}}
{{if $isReviewFile}}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/forgejo_git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,6 @@ func doActionsUserPR(ctx, doerCtx APITestContext, baseBranch, headBranch string)
doerCtx.ExpectedCode = http.StatusCreated
t.Run("AutoMergePR", doAPIAutoMergePullRequest(doerCtx, ctx.Username, ctx.Reponame, pr.Index))
// Ensure the PR page works
t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr, true))
}
}
34 changes: 27 additions & 7 deletions tests/integration/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,19 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
assert.NoError(t, err)
})

// Ensure the PR page works
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
// Ensure the PR page works.
// For the base repository owner, the PR is not editable (maintainer edits are not enabled):
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
// For the head repository owner, the PR is editable:
headSession := loginUser(t, "user2")
headToken := getTokenForLoggedInUser(t, headSession, auth_model.AccessTokenScopeReadRepository, auth_model.AccessTokenScopeReadUser)
headCtx := APITestContext{
Session: headSession,
Token: headToken,
Username: baseCtx.Username,
Reponame: baseCtx.Reponame,
}
t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, true))

// Then get the diff string
var diffHash string
Expand All @@ -470,7 +481,9 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun

// Now: Merge the PR & make sure that doesn't break the PR page or change its diff
t.Run("MergePR", doAPIMergePullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
// for both users the PR is still visible but not editable anymore after it was merged
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, false))
t.Run("CheckPR", func(t *testing.T) {
oldMergeBase := pr.MergeBase
pr2, err := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
Expand All @@ -481,12 +494,12 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun

// Then: Delete the head branch & make sure that doesn't break the PR page or change its diff
t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))

// Delete the head repository & make sure that doesn't break the PR page or change its diff
t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))
}
}
Expand Down Expand Up @@ -520,12 +533,19 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr
}
}

func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) {
func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest, editable bool) func(t *testing.T) {
return func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK)
resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
editButtonCount := doc.doc.Find("div.diff-file-header-actions a[href*='/_edit/']").Length()
if editable {
assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none")
} else {
assert.Equal(t, 0, editButtonCount, "Expected not to find any buttons to edit files in PR diff view but there were some")
}
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK)
}
Expand Down

0 comments on commit 1388e7c

Please sign in to comment.