mirror of
				https://codeberg.org/forgejo/forgejo.git
				synced 2025-10-31 14:31:02 +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 commit79c75164ca) (cherry picked from commit58c76aad8f)
This commit is contained in:
		
					parent
					
						
							
								0e1e09e77d
							
						
					
				
			
			
				commit
				
					
						5bdb3c6c53
					
				
			
		
					 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