From b0b40680b57e0ab9c3ad6c6b44eb23654c169b81 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 24 Jan 2025 14:38:17 -0800 Subject: [PATCH] Fix tests --- models/issues/pull.go | 1 + routers/api/v1/repo/pull_review.go | 5 ++++- services/pull/review_request.go | 5 +++++ tests/integration/actions_trigger_test.go | 5 +++-- tests/integration/api_pull_review_test.go | 18 ++++++++++++------ 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index e3af00224d..67e1ff1217 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -353,6 +353,7 @@ func (pr *PullRequest) LoadIssue(ctx context.Context) (err error) { pr.Issue, err = GetIssueByID(ctx, pr.IssueID) if err == nil { pr.Issue.PullRequest = pr + pr.Issue.Repo = pr.BaseRepo } return err } diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 7377913ea2..a30db396e1 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -611,7 +611,8 @@ func CreateReviewRequests(ctx *context.APIContext) { opts := web.GetForm(ctx).(*api.PullReviewRequestOptions) - pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64(":index")) + // this will load issue + pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("index")) if err != nil { if issues_model.IsErrPullRequestNotExist(err) { ctx.NotFound("GetPullRequestByIndex", err) @@ -621,6 +622,8 @@ func CreateReviewRequests(ctx *context.APIContext) { return } + pr.Issue.Repo = ctx.Repo.Repository + allowedUsers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, pr.Issue.PosterID) if err != nil { ctx.Error(http.StatusInternalServerError, "GetReviewers", err) diff --git a/services/pull/review_request.go b/services/pull/review_request.go index 49d262f847..216354bfc3 100644 --- a/services/pull/review_request.go +++ b/services/pull/review_request.go @@ -212,6 +212,11 @@ func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, } } + // reviewers can remove themself + if !isAdd && doer.ID == reviewer.ID { + return nil + } + if err := issue.LoadRepo(ctx); err != nil { return err } diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 2c76aa826f..0bcbe21b8c 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -622,13 +622,14 @@ func TestPullRequestCommitStatusEvent(t *testing.T) { assert.NoError(t, err) checkCommitStatusAndInsertFakeStatus(t, repo, sha) + assert.NoError(t, pullIssue.LoadPullRequest(db.DefaultContext)) // review_requested - _, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user2, nil, user4, true) + _, err = pull_service.ReviewRequest(db.DefaultContext, pullIssue.PullRequest, user2, nil, user4, true) assert.NoError(t, err) checkCommitStatusAndInsertFakeStatus(t, repo, sha) // review_request_removed - _, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user2, nil, user4, false) + _, err = pull_service.ReviewRequest(db.DefaultContext, pullIssue.PullRequest, user2, nil, user4, false) assert.NoError(t, err) checkCommitStatusAndInsertFakeStatus(t, repo, sha) }) diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go index eea35e4486..c1a5e21389 100644 --- a/tests/integration/api_pull_review_test.go +++ b/tests/integration/api_pull_review_test.go @@ -296,12 +296,12 @@ func TestAPIPullReviewRequest(t *testing.T) { user38Session := loginUser(t, "user38") user38Token := getTokenForLoggedInUser(t, user38Session, auth_model.AccessTokenScopeWriteRepository) req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{ - Reviewers: []string{"user4@example.com"}, + Reviewers: []string{"user40@example.com"}, }).AddTokenAuth(user38Token) MakeRequest(t, req, http.StatusCreated) req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{ - Reviewers: []string{"user4@example.com"}, + Reviewers: []string{"user40@example.com"}, }).AddTokenAuth(user38Token) MakeRequest(t, req, http.StatusNoContent) @@ -309,12 +309,12 @@ func TestAPIPullReviewRequest(t *testing.T) { user39Session := loginUser(t, "user39") user39Token := getTokenForLoggedInUser(t, user39Session, auth_model.AccessTokenScopeWriteRepository) req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{ - Reviewers: []string{"user8"}, + Reviewers: []string{"user38"}, }).AddTokenAuth(user39Token) MakeRequest(t, req, http.StatusCreated) req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{ - Reviewers: []string{"user8"}, + Reviewers: []string{"user38"}, }).AddTokenAuth(user39Token) MakeRequest(t, req, http.StatusNoContent) @@ -332,6 +332,12 @@ func TestAPIPullReviewRequest(t *testing.T) { }).AddTokenAuth(user39Token) // user39 is from a team with read permission on pull requests unit MakeRequest(t, req, http.StatusNoContent) + // user8 is not a reviewer, so this will return 422 + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull22Repo.OwnerName, pull22Repo.Name, pullIssue22.Index), &api.PullReviewRequestOptions{ + Reviewers: []string{"user8"}, + }).AddTokenAuth(user39Token) // user39 is from a team with read permission on pull requests unit + MakeRequest(t, req, http.StatusUnprocessableEntity) + // Test team review request pullIssue12 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 12}) assert.NoError(t, pullIssue12.LoadAttributes(db.DefaultContext)) @@ -339,7 +345,7 @@ func TestAPIPullReviewRequest(t *testing.T) { // Test add Team Review Request req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", repo3.OwnerName, repo3.Name, pullIssue12.Index), &api.PullReviewRequestOptions{ - TeamReviewers: []string{"team1", "owners"}, + TeamReviewers: []string{"team1", "Owners"}, }).AddTokenAuth(token) MakeRequest(t, req, http.StatusCreated) @@ -353,7 +359,7 @@ func TestAPIPullReviewRequest(t *testing.T) { req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", repo3.OwnerName, repo3.Name, pullIssue12.Index), &api.PullReviewRequestOptions{ TeamReviewers: []string{"not_exist_team"}, }).AddTokenAuth(token) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusUnprocessableEntity) // Test Remove team Review Request req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", repo3.OwnerName, repo3.Name, pullIssue12.Index), &api.PullReviewRequestOptions{