refactor github pkg

pPrecel · Oct 16, 2023 · 45fa8d4 · 45fa8d4
1 parent 353e0f8
commit 45fa8d4
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 77 deletions.
diff --git a/cmd/gen.go b/cmd/gen.go
@@ -65,15 +65,32 @@ func genCommandAction(ctx *cli.Context, opts *genActionOpts) error {
 				defer close(errChan)
 				defer close(valChan)
 
-				prs, err := artifacts.GenUserArtifactsToFile(client, &artifacts.GenerateOpts{
+				config := artifacts.GenerateOpts{
 					Org:          org,
 					Repo:         repo,
 					Username:     opts.username,
 					Dir:          opts.dir,
 					WithClosed:   opts.withClosed,
 					MergedAfter:  mergedAfter,
 					MergedBefore: mergedBefore,
-				})
+				}
+
+				log.Debug("starting process for repo with config", log.Args(
+					"org", config.Org,
+					"repo", config.Repo,
+					"username", config.Username,
+					"dir", config.Dir,
+					"withClosed", config.WithClosed,
+					"mergedAfter", config.MergedAfter.String(),
+					"mergedBefore", config.MergedBefore.String(),
+				))
+
+				prs, err := artifacts.GenUserArtifactsToFile(client, &config)
+
+				log.Debug("ending process for repo", log.Args(
+					"prs", prs,
+					"error", err,
+				))
 				if err != nil {
 					errChan <- err
 					return

diff --git a/pkg/artifacts/artifacts.go b/pkg/artifacts/artifacts.go
@@ -20,14 +20,17 @@ type GenerateOpts struct {
 }
 
 func GenUserArtifactsToFile(client github.Client, opts *GenerateOpts) ([]string, error) {
+	filters := []github.FilterFunc{github.FilterPRsByMergedAt}
+	if opts.WithClosed {
+		filters = append(filters, github.FilterPRsByClosedAt)
+	}
 	prs, err := client.ListUserPRsForRepo(github.Options{
 		Org:          opts.Org,
 		Repo:         opts.Repo,
 		Username:     opts.Username,
-		WithClosed:   opts.WithClosed,
 		MergedAfter:  opts.MergedAfter,
 		MergedBefore: opts.MergedBefore,
-	})
+	}, filters)
 	if err != nil {
 		return nil, fmt.Errorf("list users PRs in repo '%s/%s' error: %s", opts.Org, opts.Repo, err.Error())
 	}

diff --git a/pkg/artifacts/artifacts_test.go b/pkg/artifacts/artifacts_test.go
@@ -38,10 +38,9 @@ func TestGenUserArtifactsToFile(t *testing.T) {
 			Org:          "test-org",
 			Repo:         "test-repo",
 			Username:     "test-username",
-			WithClosed:   true,
 			MergedBefore: time.Time{},
 			MergedAfter:  time.Time{},
-		}).Return(testPRs, nil).Once()
+		}, mock.Anything).Return(testPRs, nil).Once()
 
 		prs, err := GenUserArtifactsToFile(clientMock, &GenerateOpts{
 			Org:          "test-org",
@@ -73,10 +72,9 @@ func TestGenUserArtifactsToFile(t *testing.T) {
 			Org:          "test-org",
 			Repo:         "test-repo",
 			Username:     "test-username",
-			WithClosed:   true,
 			MergedBefore: time.Time{},
 			MergedAfter:  time.Time{},
-		}).Return(nil, errors.New("test error")).Once()
+		}, mock.Anything).Return(nil, errors.New("test error")).Once()
 
 		prs, err := GenUserArtifactsToFile(clientMock, &GenerateOpts{
 			Org:          "test-org",
@@ -98,10 +96,9 @@ func TestGenUserArtifactsToFile(t *testing.T) {
 			Org:          "test-org",
 			Repo:         "test-repo",
 			Username:     "test-username",
-			WithClosed:   true,
 			MergedBefore: time.Time{},
 			MergedAfter:  time.Time{},
-		}).Return([]*gh.PullRequest{}, nil).Once()
+		}, mock.Anything).Return([]*gh.PullRequest{}, nil).Once()
 
 		prs, err := GenUserArtifactsToFile(clientMock, &GenerateOpts{
 			Org:          "test-org",

diff --git a/pkg/github/automock/client.go b/pkg/github/automock/client.go
diff --git a/pkg/github/client.go b/pkg/github/client.go
@@ -10,7 +10,7 @@ import (
 //go:generate mockery --name=Client --output=automock --outpkg=automock --case=underscore
 type Client interface {
 	GetFileDiffForPRs([]*github.PullRequest, string, string) (string, error)
-	ListUserPRsForRepo(Options) ([]*github.PullRequest, error)
+	ListUserPRsForRepo(Options, []FilterFunc) ([]*github.PullRequest, error)
 }
 
 type gh_client struct {

diff --git a/pkg/github/filters.go b/pkg/github/filters.go
@@ -7,7 +7,9 @@ import (
 	"github.com/pterm/pterm"
 )
 
-func filterPRsByClosedAt(log *pterm.Logger, prs []*github.PullRequest, opts Options) []*github.PullRequest {
+type FilterFunc func(*pterm.Logger, []*github.PullRequest, Options) []*github.PullRequest
+
+func FilterPRsByClosedAt(log *pterm.Logger, prs []*github.PullRequest, opts Options) []*github.PullRequest {
 	filtered := []*github.PullRequest{}
 	for i := range prs {
 		pr := *prs[i]
@@ -25,7 +27,7 @@ func filterPRsByClosedAt(log *pterm.Logger, prs []*github.PullRequest, opts Opti
 	return filtered
 }
 
-func filterPRsByMergedAt(log *pterm.Logger, prs []*github.PullRequest, opts Options) []*github.PullRequest {
+func FilterPRsByMergedAt(log *pterm.Logger, prs []*github.PullRequest, opts Options) []*github.PullRequest {
 	filtered := []*github.PullRequest{}
 	for i := range prs {
 		pr := *prs[i]
@@ -42,3 +44,12 @@ func filterPRsByMergedAt(log *pterm.Logger, prs []*github.PullRequest, opts Opti
 	))
 	return filtered
 }
+
+func (gh *gh_client) fireFilters(prs []*github.PullRequest, opts Options, filters []FilterFunc) []*github.PullRequest {
+	filtered := []*github.PullRequest{}
+	for i := range filters {
+		filtered = append(filtered, filters[i](gh.log, prs, opts)...)
+	}
+
+	return filtered
+}
diff --git a/pkg/github/pullrequest.go b/pkg/github/pullrequest.go
@@ -1,7 +1,6 @@
 package github
 
 import (
-	"fmt"
 	"sort"
 	"time"
 
@@ -21,36 +20,60 @@ type Options struct {
 	Org          string
 	Repo         string
 	Username     string
-	WithClosed   bool
 	MergedBefore time.Time
 	MergedAfter  time.Time
 }
 
-func (gh *gh_client) ListUserPRsForRepo(opts Options) ([]*github.PullRequest, error) {
-	userPullRequests := []*github.PullRequest{}
-	page := 1
+func (gh *gh_client) ListUserPRsForRepo(opts Options, filters []FilterFunc) ([]*github.PullRequest, error) {
+	allUserPRs, err := gh.listLastPRsForRepo(opts, filters)
+	if err != nil {
+		return nil, err
+	}
+
+	sorted := sortPRsByMergedAt(allUserPRs)
+	filtered := gh.fireFilters(sorted, opts, filters)
 
-	filters := []filterFunc{filterPRsByMergedAt}
-	if opts.WithClosed {
-		filters = append(filters, filterPRsByClosedAt)
+	pullRequests, err := gh.listUserPRs(filtered, opts)
+	if err != nil {
+		return nil, err
 	}
 
+	gh.log.Trace("PRs related with user", gh.log.Args(
+		"org", opts.Org,
+		"repo", opts.Repo,
+		"username", opts.Username,
+		"count", len(pullRequests),
+	))
+
+	return pullRequests, nil
+}
+
+func (gh *gh_client) listLastPRsForRepo(opts Options, filters []FilterFunc) ([]*github.PullRequest, error) {
+	userPullRequests := []*github.PullRequest{}
+	page := 1
+
 	for page <= maxPage {
-		prs, wasLast, err := gh.listUserPRsForRepo(opts, page, filters)
+		pagePRs, _, err := gh.client.PullRequests.List(gh.ctx, opts.Org, opts.Repo, &github.PullRequestListOptions{
+			State: "closed",
+			ListOptions: github.ListOptions{
+				PerPage: perPage, // max
+				Page:    page,
+			},
+		})
 		if err != nil {
 			return nil, err
 		}
 
-		gh.log.Debug("prs on the page", gh.log.Args(
+		gh.log.Trace("prs on the page", gh.log.Args(
 			"org", opts.Org,
 			"repo", opts.Repo,
 			"page", page,
-			"prs", len(prs),
+			"prs", len(pagePRs),
 		))
 
-		userPullRequests = append(userPullRequests, prs...)
+		userPullRequests = append(userPullRequests, pagePRs...)
 
-		if wasLast {
+		if len(pagePRs) < perPage {
 			break
 		}
 		page++
@@ -59,47 +82,6 @@ func (gh *gh_client) ListUserPRsForRepo(opts Options) ([]*github.PullRequest, er
 	return userPullRequests, nil
 }
 
-type filterFunc func(*pterm.Logger, []*github.PullRequest, Options) []*github.PullRequest
-
-func (gh *gh_client) listUserPRsForRepo(opts Options, page int, filters []filterFunc) ([]*github.PullRequest, bool, error) {
-	pagePRs, _, err := gh.client.PullRequests.List(gh.ctx, opts.Org, opts.Repo, &github.PullRequestListOptions{
-		State: "closed",
-		ListOptions: github.ListOptions{
-			PerPage: perPage, // max
-			Page:    page,
-		},
-	})
-	if err != nil {
-		return nil, true, err
-	}
-
-	gh.log.Trace(fmt.Sprintf("listed %d PRs on the page", len(pagePRs)), gh.log.Args(
-		"org", opts.Org,
-		"repo", opts.Repo,
-	))
-
-	sort.Slice(pagePRs, func(i, j int) bool {
-		return pagePRs[i].GetMergedAt().After(
-			pagePRs[j].GetMergedAt().Time,
-		)
-	})
-
-	filtered := []*github.PullRequest{}
-	for i := range filters {
-		filtered = append(filtered, filters[i](gh.log, pagePRs, opts)...)
-	}
-
-	pullRequests, err := gh.listUserPRs(filtered, opts)
-
-	gh.log.Trace(fmt.Sprintf("%d PRs are related with user %s", len(pullRequests), opts.Username), gh.log.Args(
-		"org", opts.Org,
-		"repo", opts.Repo,
-	))
-	return pullRequests,
-		len(pagePRs) < perPage,
-		err
-}
-
 func (gh *gh_client) listUserPRs(prs []*github.PullRequest, opts Options) ([]*github.PullRequest, error) {
 	userPRs := []*github.PullRequest{}
 	for i := range prs {
@@ -155,3 +137,12 @@ func isAuthorOrCommitter(log *pterm.Logger, commits []*github.RepositoryCommit,
 
 	return false
 }
+
+func sortPRsByMergedAt(prs []*github.PullRequest) []*github.PullRequest {
+	sort.Slice(prs, func(i, j int) bool {
+		return prs[i].GetMergedAt().After(
+			prs[j].GetMergedAt().Time,
+		)
+	})
+	return prs
+}
diff --git a/pkg/github/pullrequest_test.go b/pkg/github/pullrequest_test.go
@@ -81,7 +81,7 @@ func Test_gh_client_ListUserPRsForRepo(t *testing.T) {
 			Username:     "pPrecel",
 			MergedBefore: testMergedBefore,
 			MergedAfter:  testMergedAfter,
-		})
+		}, []FilterFunc{FilterPRsByMergedAt})
 		require.NoError(t, err)
 		require.Len(t, PRs, 2)
 	})