From 367d606870948762b8acd65b8fc1979922e20f3568e3262f3782f756e1c8b6a6 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Mon, 17 Feb 2025 17:49:02 +0100 Subject: [PATCH] wip --- bots-common/gitea_utils.go | 5 ++-- workflow-pr/pr.go | 38 ++++++++++++++++++++++-- workflow-pr/pr_processor_opened.go | 4 +-- workflow-pr/pr_processor_opened_test.go | 39 ++++++++++++++++++++----- workflow-pr/reviews.go | 33 +++++++++++++++++++++ workflow-pr/reviews_test.go | 28 ++++++++++++++---- 6 files changed, 127 insertions(+), 20 deletions(-) diff --git a/bots-common/gitea_utils.go b/bots-common/gitea_utils.go index 1150cf2..54052f2 100644 --- a/bots-common/gitea_utils.go +++ b/bots-common/gitea_utils.go @@ -69,6 +69,7 @@ type GiteaMaintainershipReader interface { type GiteaPRFetcher interface { GetPullRequest(org, project string, num int64) (*models.PullRequest, error) + GetAssociatedPrjGitPR(prjGitOrg, prjGitRepo, refOrg, refRepo string, Index int64) (*models.PullRequest, error) } type GiteaReviewFetcher interface { @@ -101,7 +102,7 @@ type Gitea interface { GetOrganizationRepositories(orgName string) ([]*models.Repository, error) CreateRepositoryIfNotExist(git Git, org, repoName string) (*models.Repository, error) CreatePullRequestIfNotExist(repo *models.Repository, srcId, targetId, title, body string) (*models.PullRequest, error) - GetAssociatedPrjGitPR(prjGitOrg, prjGitRepo, refOrg, refRepo string, Index int) (*models.PullRequest, error) + GetAssociatedPrjGitPR(prjGitOrg, prjGitRepo, refOrg, refRepo string, Index int64) (*models.PullRequest, error) GetRepositoryFileContent(org, repo, hash, path string) ([]byte, error) GetPullRequestFileContent(pr *models.PullRequest, path string) ([]byte, error) GetRecentPullRequests(org, repo string) ([]*models.PullRequest, error) @@ -348,7 +349,7 @@ func (gitea *GiteaTransport) CreatePullRequestIfNotExist(repo *models.Repository return pr.GetPayload(), nil } -func (gitea *GiteaTransport) GetAssociatedPrjGitPR(prjGitOrg, prjGitRepo, refOrg, refRepo string, Index int) (*models.PullRequest, error) { +func (gitea *GiteaTransport) GetAssociatedPrjGitPR(prjGitOrg, prjGitRepo, refOrg, refRepo string, Index int64) (*models.PullRequest, error) { var page int64 state := "open" for { diff --git a/workflow-pr/pr.go b/workflow-pr/pr.go index 8c7730c..28300eb 100644 --- a/workflow-pr/pr.go +++ b/workflow-pr/pr.go @@ -50,9 +50,31 @@ func readPRData(gitea common.GiteaPRFetcher, org, repo string, num int64, curren } func FetchPRSet(gitea common.GiteaPRFetcher, org, repo string, num int64, config *common.AutogitConfig) (*PRSet, error) { - prs, err := readPRData(gitea, org, repo, num, nil) - if err != nil { - return nil, err + var prs []PRInfo + if org != config.Organization || repo != config.GitProjectName { + pr, err := gitea.GetAssociatedPrjGitPR(config.Organization, config.GitProjectName, org, repo, num) + if err != nil { + return nil, err + } + prs, err = readPRData(gitea, pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index, nil) + if err != nil { + return nil, err + } + } + + isFound := false + for _, pr := range prs { + if pr.pr.Index == num && repo == pr.pr.Base.Repo.Name && org == pr.pr.Base.Repo.Owner.UserName { + isFound = true + break + } + } + if !isFound { + pr, err := gitea.GetPullRequest(org, repo, num) + if err != nil { + return nil, err + } + prs = append(prs, PRInfo{pr: pr}) } return &PRSet{prs: prs, config: config}, nil @@ -120,6 +142,16 @@ func (rs *PRSet) AssignReviewers(gitea common.GiteaReviewRequester, maintainers reviewers = slices.Delete(reviewers, idx, idx+1) } + // remove reviewers that were already requested and are not stale + for idx:= 0; idx < len(reviewers); { + for _, r := range pr.pr.RequestedReviewers { + if r.UserName == reviewers[idx] { + + } + } + } + + // get maintainers associated with the PR too log.Printf("revieweres to be added to %s/%s#%d - [%s]\n", pr.pr.Base.Repo.Owner.UserName, pr.pr.Base.Repo.Name, pr.pr.Index, strings.Join(reviewers, ", ")) if len(reviewers) > 0 { diff --git a/workflow-pr/pr_processor_opened.go b/workflow-pr/pr_processor_opened.go index 0c5099f..a6ab790 100644 --- a/workflow-pr/pr_processor_opened.go +++ b/workflow-pr/pr_processor_opened.go @@ -78,6 +78,6 @@ referencing the following pull request: if err != nil { return err } - prset.AssignReviewers(o.gitea, maintainers) - return nil + + return prset.AssignReviewers(o.gitea, maintainers) } diff --git a/workflow-pr/pr_processor_opened_test.go b/workflow-pr/pr_processor_opened_test.go index c7c0b89..a56ba5e 100644 --- a/workflow-pr/pr_processor_opened_test.go +++ b/workflow-pr/pr_processor_opened_test.go @@ -6,6 +6,7 @@ import ( "go.uber.org/mock/gomock" "src.opensuse.org/autogits/common" + "src.opensuse.org/autogits/common/gitea-generated/client/repository" "src.opensuse.org/autogits/common/gitea-generated/models" mock_common "src.opensuse.org/autogits/common/mock" ) @@ -79,15 +80,31 @@ func TestOpenPR(t *testing.T) { git.GitPath = t.TempDir() setupGitForTests(t, git) + prjgit := &models.Repository{ SSHURL: "./prj", DefaultBranch: "testing", } - giteaPR := &models.PullRequest{} - gitea.EXPECT().CreateRepositoryIfNotExist(git, *event.Repository.Owner, "prjcopy").Return(prjgit, nil) + giteaPR := &models.PullRequest{ + Base: &models.PRBranchInfo{ + Repo: &models.Repository{ + Owner: &models.User { + UserName: "test", + }, + Name: "testRepo", + }, + }, + User: &models.User { + UserName: "test", + }, + } + gitea.EXPECT().CreateRepositoryIfNotExist(git, "test", "prjcopy").Return(prjgit, nil) gitea.EXPECT().CreatePullRequestIfNotExist(prjgit, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(giteaPR, nil) + gitea.EXPECT().GetPullRequest("test", "testRepo", int64(1)).Return(giteaPR, nil) gitea.EXPECT().RequestReviews(giteaPR, "reviewer1", "reviewer2").Return(nil, nil) -// gitea.EXPECT().RequestReviews(giteaPR, "reviewer2").Return(nil, nil) + + gitea.EXPECT().FetchMaintainershipDirFile("test", "prjcopy", "branch", "_project").Return(nil,repository.NewRepoGetRawFileNotFound()) + gitea.EXPECT().FetchMaintainershipFile("test", "prjcopy", "branch").Return(nil,repository.NewRepoGetRawFileNotFound()) err := pr.Process(event, git, config) if err != nil { @@ -107,7 +124,7 @@ func TestOpenPR(t *testing.T) { setupGitForTests(t, git) failedErr := errors.New("Returned error here") - gitea.EXPECT().CreateRepositoryIfNotExist(git, *event.Repository.Owner, "prjcopy").Return(nil, failedErr) + gitea.EXPECT().CreateRepositoryIfNotExist(git, "test", "prjcopy").Return(nil, failedErr) err := pr.Process(event, git, config) if err != failedErr { @@ -130,7 +147,7 @@ func TestOpenPR(t *testing.T) { DefaultBranch: "testing", } failedErr := errors.New("Returned error here") - gitea.EXPECT().CreateRepositoryIfNotExist(git, *event.Repository.Owner, "prjcopy").Return(prjgit, nil) + gitea.EXPECT().CreateRepositoryIfNotExist(git, "test", "prjcopy").Return(prjgit, nil) gitea.EXPECT().CreatePullRequestIfNotExist(prjgit, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, failedErr) err := pr.Process(event, git, config) @@ -162,12 +179,18 @@ func TestOpenPR(t *testing.T) { Repo: prjgit, }, Index: 13, + User: &models.User { + UserName: "test", + }, } failedErr := errors.New("Returned error here") - gitea.EXPECT().CreateRepositoryIfNotExist(git, *event.Repository.Owner, "prjcopy").Return(prjgit, nil) + gitea.EXPECT().CreateRepositoryIfNotExist(git, "test", "prjcopy").Return(prjgit, nil) + gitea.EXPECT().GetPullRequest("test", "testRepo", int64(1)).Return(giteaPR, nil) gitea.EXPECT().CreatePullRequestIfNotExist(prjgit, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(giteaPR, nil) - gitea.EXPECT().RequestReviews(giteaPR, "reviewer1").Return(nil, failedErr) - // gitea.EXPECT().RequestReviews(giteaPR, "reviewer2").Return(nil, nil) + gitea.EXPECT().RequestReviews(giteaPR, "reviewer1", "reviewer2").Return(nil, failedErr) + + gitea.EXPECT().FetchMaintainershipDirFile("test", "prjcopy", "branch", "_project").Return(nil,repository.NewRepoGetRawFileNotFound()) + gitea.EXPECT().FetchMaintainershipFile("test", "prjcopy", "branch").Return(nil,repository.NewRepoGetRawFileNotFound()) err := pr.Process(event, git, config) if errors.Unwrap(err) != failedErr { diff --git a/workflow-pr/reviews.go b/workflow-pr/reviews.go index f95ee27..b9633cd 100644 --- a/workflow-pr/reviews.go +++ b/workflow-pr/reviews.go @@ -1,6 +1,8 @@ package main import ( + "slices" + "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" ) @@ -41,3 +43,34 @@ func (r *PRReviews) IsReviewed() bool { return goodReview } + +func (r *PRReviews) HasStaleReviewsOnly(reviewer string) bool { + if !slices.Contains(r.reviewers, reviewer) { + return false + } + + hasStale := true + hasReviews := false + for _, r := range r.reviews { + if r.User.UserName == reviewer { + hasStale = hasStale && r.Stale + } + } + + return false +} + +func (r *PRReviews) IsReviewedBy(reviewer string) bool { + if !slices.Contains(r.reviewers, reviewer) { + return false + } + + for _, r := range r.reviews { + if r.User.UserName == reviewer && !r.Stale { + return r.State == common.ReviewStateApproved + } + } + + return false +} + diff --git a/workflow-pr/reviews_test.go b/workflow-pr/reviews_test.go index 7bf9fd7..5e6e4b1 100644 --- a/workflow-pr/reviews_test.go +++ b/workflow-pr/reviews_test.go @@ -12,11 +12,14 @@ import ( func TestReviews(t *testing.T) { tests := []struct { - name string - reviews []*models.PullReview - reviewers []string - fetchErr error - isReviewed bool + name string + reviews []*models.PullReview + reviewers []string + fetchErr error + isReviewed bool + isReviewedByTest1 bool + hasStaleReviews bool + isPendingByTest1 bool }{ { name: "Reviews of unreviews PR", @@ -27,6 +30,7 @@ func TestReviews(t *testing.T) { reviews: []*models.PullReview{&models.PullReview{State: common.ReviewStateApproved, User: &models.User{UserName: "user1"}}}, reviewers: []string{"user1"}, isReviewed: true, + isReviewedByTest1: true, }, { name: "Two reviewer, one not approved", @@ -93,6 +97,20 @@ func TestReviews(t *testing.T) { if r := reviews.IsReviewed(); r != test.isReviewed { t.Fatal("Unexpected IsReviewed():", r, "vs. expected", test.isReviewed) } + + if r := reviews.IsReviewPendingBy("user1"); r != test.isPendingByTest1 { + t.Fatal("Unexpected IsReviewPendingBy():", r) + } + if r := reviews.IsReviewedBy("user1"); r != test.isReviewedByTest1 { + t.Fatal("Unexpected IsReviewedBy():", r) + } + + if r := reviews.IsReviewPendingBy("random"); r { + t.Fatal("Unexpected IsReviewPendingBy(random):", r) + } + if r := reviews.IsReviewedBy("random"); r { + t.Fatal("Unexpected IsReviewedBy(random):", r) + } }) } }