mirror of
				https://codeberg.org/forgejo/forgejo.git
				synced 2025-11-04 00:11:04 +00:00 
			
		
		
		
	[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)
	
	
This commit is contained in:
		
					parent
					
						
							
								ca65deba1c
							
						
					
				
			
			
				commit
				
					
						58c76aad8f
					
				
			
		
					 4 changed files with 43 additions and 8 deletions
				
			
		| 
						 | 
					@ -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)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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}}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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))
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue