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

Closes #1894.
Gitea issue: https://github.com/go-gitea/gitea/issues/23848

(cherry picked from commit 79c75164ca)
(cherry picked from commit 58c76aad8f)
(cherry picked from commit 5bdb3c6c53)
(cherry picked from commit 94e954ce22)
(cherry picked from commit 1388e7c7be)
This commit is contained in:
Antonin Delpeuch 2023-12-19 22:18:07 +01:00 committed by Earl Warren
parent b3321d1a84
commit 6a234abff5
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
4 changed files with 43 additions and 8 deletions

View file

@ -967,6 +967,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)
} }