[GITEA] pulls: "Edit File" button in "Files Changed" tab

Closes #1894.
Gitea issue: https://github.com/go-gitea/gitea/issues/23848
This commit is contained in:
Antonin Delpeuch 2023-12-19 22:18:07 +01:00
parent 43ded8ee30
commit 79c75164ca
4 changed files with 43 additions and 8 deletions

View file

@ -966,6 +966,18 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return 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.IsSigned && ctx.Doer != nil {
if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil { if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
ctx.ServerError("CanMarkConversation", err) ctx.ServerError("CanMarkConversation", err)

View file

@ -166,6 +166,9 @@
<a class="ui basic tiny button" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a> <a class="ui basic tiny button" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{else}} {{else}}
<a class="ui basic tiny button" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a> <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}}
{{end}} {{end}}
{{if $isReviewFile}} {{if $isReviewFile}}

View file

@ -134,6 +134,6 @@ func doActionsUserPR(ctx, doerCtx APITestContext, baseBranch, headBranch string)
doerCtx.ExpectedCode = http.StatusCreated doerCtx.ExpectedCode = http.StatusCreated
t.Run("AutoMergePR", doAPIAutoMergePullRequest(doerCtx, ctx.Username, ctx.Reponame, pr.Index)) t.Run("AutoMergePR", doAPIAutoMergePullRequest(doerCtx, ctx.Username, ctx.Reponame, pr.Index))
// Ensure the PR page works // Ensure the PR page works
t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr)) t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr, true))
} }
} }

View file

@ -455,8 +455,19 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
assert.NoError(t, err) assert.NoError(t, err)
}) })
// Ensure the PR page works // Ensure the PR page works.
t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) // 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 // Then get the diff string
var diffHash string var diffHash string
@ -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 // 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("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) { t.Run("CheckPR", func(t *testing.T) {
oldMergeBase := pr.MergeBase oldMergeBase := pr.MergeBase
pr2, err := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t) pr2, err := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
@ -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 // 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("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)) 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 // Delete the head repository & make sure that doesn't break the PR page or change its diff
t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx)) 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)) t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))
} }
} }
@ -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) { 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)) 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) 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)) 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)) 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) ctx.Session.MakeRequest(t, req, http.StatusOK)
} }