Skip to content

Commit

Permalink
Rename GetUsername to GetName, use GetLogin as fallback when opening …
Browse files Browse the repository at this point in the history
…a PR (#2701)

A follow-up to an earlier commit, renames the confusing GetUsername
github interface method to GetName to convey better that it's just the
name. Since the name can be empty and we'd still like to use "something"
for the PRs as an identifier, we use the login as a fallback
  • Loading branch information
jhrozek authored Mar 18, 2024
1 parent 081890c commit 8a0bc3f
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 27 deletions.
7 changes: 6 additions & 1 deletion internal/controlplane/handlers_repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,12 @@ func (s *StubGitHub) GetUserId(context.Context) (int64, error) {
}

// GetUsername implements v1.GitHub.
func (*StubGitHub) GetUsername(context.Context) (string, error) {
func (*StubGitHub) GetName(context.Context) (string, error) {
panic("unimplemented")
}

// GetLogin implements v1.GitHub.
func (*StubGitHub) GetLogin(context.Context) (string, error) {
panic("unimplemented")
}

Expand Down
19 changes: 13 additions & 6 deletions internal/engine/actions/remediate/pull_request/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,6 @@ func (r *Remediator) runGit(
}

logger.Debug().Msg("Getting authenticated user details")
username, err := r.ghCli.GetUsername(ctx)
if err != nil {
return fmt.Errorf("cannot get username: %w", err)
}

email, err := r.ghCli.GetPrimaryEmail(ctx)
if err != nil {
return fmt.Errorf("cannot get primary email: %w", err)
Expand Down Expand Up @@ -294,7 +289,7 @@ func (r *Remediator) runGit(
logger.Debug().Msg("Committing changes")
_, err = wt.Commit(title, &git.CommitOptions{
Author: &object.Signature{
Name: username,
Name: userNameForCommit(ctx, r.ghCli),
Email: email,
When: time.Now(),
},
Expand Down Expand Up @@ -391,6 +386,18 @@ func branchBaseName(prTitle string) string {
return fmt.Sprintf("%s_%s", baseName, normalizedPrTitle)
}

func userNameForCommit(ctx context.Context, gh provifv1.GitHub) string {
var name string

// we ignore errors here, as we can still create a commit without a name
// and errors are checked when getting the primary email
name, _ = gh.GetName(ctx)
if name == "" {
name, _ = gh.GetLogin(ctx)
}
return name
}

func (_ *Remediator) prMagicComment(modifier fsModifier) (string, error) {
tmpl, err := template.New(prMagicTemplateName).Option("missingkey=error").Parse(prBodyMagicTemplate)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func happyPathMockSetup(mockGitHub *mock_ghclient.MockGitHub) {
mockGitHub.EXPECT().
ListPullRequests(gomock.Any(), repoOwner, repoName, gomock.Any()).Return([]*github.PullRequest{}, nil)
mockGitHub.EXPECT().
GetUsername(gomock.Any()).Return("stacklok-bot", nil)
GetName(gomock.Any()).Return("stacklok-bot", nil)
mockGitHub.EXPECT().
GetPrimaryEmail(gomock.Any()).Return("[email protected]", nil)
mockGitHub.EXPECT().
Expand Down Expand Up @@ -461,7 +461,7 @@ func TestPullRequestRemediate(t *testing.T) {
ListPullRequests(gomock.Any(), repoOwner, repoName, gomock.Any()).Return([]*github.PullRequest{}, nil)
// we need to get the user information and update the branch
mockGitHub.EXPECT().
GetUsername(gomock.Any()).Return("stacklok-bot", nil)
GetName(gomock.Any()).Return("stacklok-bot", nil)
// likewise we need to update the branch with a valid e-mail
mockGitHub.EXPECT().
GetPrimaryEmail(gomock.Any()).Return("[email protected]", nil)
Expand Down
4 changes: 2 additions & 2 deletions internal/providers/github/github_rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,8 @@ func (c *GitHub) GetUserId(ctx context.Context) (int64, error) {
return user.GetID(), nil
}

// GetUsername returns the username for the authenticated user
func (c *GitHub) GetUsername(ctx context.Context) (string, error) {
// GetName returns the username for the authenticated user
func (c *GitHub) GetName(ctx context.Context) (string, error) {
user, _, err := c.client.Users.Get(ctx, "")
if err != nil {
return "", err
Expand Down
45 changes: 30 additions & 15 deletions internal/providers/github/mock/github.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/providers/v1/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ type GitHub interface {
CreatePullRequest(ctx context.Context, owner, repo, title, body, head, base string) (*github.PullRequest, error)
ListPullRequests(ctx context.Context, owner, repo string, opt *github.PullRequestListOptions) ([]*github.PullRequest, error)
GetUserId(ctx context.Context) (int64, error)
GetUsername(ctx context.Context) (string, error)
GetName(ctx context.Context) (string, error)
GetLogin(ctx context.Context) (string, error)
GetPrimaryEmail(ctx context.Context) (string, error)
CreateIssueComment(ctx context.Context, owner, repo string, number int, comment string) (*github.IssueComment, error)
ListIssueComments(ctx context.Context, owner, repo string, number int,
Expand Down

0 comments on commit 8a0bc3f

Please sign in to comment.