From dfd600713f001e3c1385ee9ea21597d78eaca535 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 14 May 2025 08:53:47 -0600 Subject: [PATCH] refactor(api): rename assignable user types and methods --- api/queries_repo.go | 148 +++++++++++++++++++++++-------- api/queries_repo_test.go | 6 +- pkg/cmd/pr/shared/completion.go | 8 +- pkg/cmd/pr/shared/editable.go | 4 +- pkg/cmd/pr/shared/survey.go | 2 +- pkg/cmd/pr/shared/survey_test.go | 6 +- 6 files changed, 123 insertions(+), 51 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index ea60319a3..9e822767d 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -682,8 +682,8 @@ func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Reposit type RepoMetadataResult struct { CurrentLogin string - AssignableUsers []RepoAssignee - AssignableActors []RepoAssignee + AssignableUsers []AssignableUser + AssignableActors []AssignableActor Labels []RepoLabel Projects []RepoProject ProjectsV2 []ProjectV2 @@ -696,8 +696,8 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { for _, assigneeLogin := range names { found := false for _, u := range m.AssignableUsers { - if strings.EqualFold(assigneeLogin, u.Login) { - ids = append(ids, u.ID) + if strings.EqualFold(assigneeLogin, (u.Login())) { + ids = append(ids, u.ID()) found = true break } @@ -705,8 +705,8 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { // Look for ID in assignable actors if not found in assignable users for _, a := range m.AssignableActors { - if strings.EqualFold(assigneeLogin, a.Login) { - ids = append(ids, a.ID) + if strings.EqualFold(assigneeLogin, a.Login()) { + ids = append(ids, a.ID()) found = true break } @@ -1126,12 +1126,16 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes result.Teams = append(result.Teams, t) } default: - user := RepoAssignee{} + user := struct { + Id string + Login string + Name string + }{} err := json.Unmarshal(v, &user) if err != nil { return result, err } - result.AssignableUsers = append(result.AssignableUsers, user) + result.AssignableUsers = append(result.AssignableUsers, NewAssignableUser(user.Id, user.Login, user.Name)) } } @@ -1183,26 +1187,86 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) return projects, nil } -type RepoAssignee struct { - ID string - Login string - Name string +type AssignableActor interface { + DisplayName() string + ID() string + Login() string + + sealedAssignableActor() +} + +// Always a user +type AssignableUser struct { + id string + login string + name string +} + +// NewAssignableUser is a test helper to create a new AssignableUser +// since the ID and Login are private fields +func NewAssignableUser(id, login, name string) AssignableUser { + return AssignableUser{ + id: id, + login: login, + name: name, + } } // DisplayName returns a formatted string that uses Login and Name to be displayed e.g. 'Login (Name)' or 'Login' -func (ra RepoAssignee) DisplayName() string { - if ra.Name != "" { - return fmt.Sprintf("%s (%s)", ra.Login, ra.Name) +func (u AssignableUser) DisplayName() string { + if u.name != "" { + return fmt.Sprintf("%s (%s)", u.login, u.name) } - return ra.Login + return u.login } +func (u AssignableUser) ID() string { + return u.id +} + +func (u AssignableUser) Login() string { + return u.login +} + +func (u AssignableUser) Name() string { + return u.name +} + +func (u AssignableUser) sealedAssignableActor() {} + +type AssignableBot struct { + id string + login string +} + +func (b AssignableBot) DisplayName() string { + return b.Login() +} + +func (b AssignableBot) ID() string { + return b.id +} + +func (b AssignableBot) Login() string { + return b.login +} + +func (b AssignableBot) Name() string { + return "" +} + +func (b AssignableBot) sealedAssignableActor() {} + // RepoAssignableUsers fetches all the assignable users for a repository -func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, error) { +func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]AssignableUser, error) { type responseData struct { Repository struct { AssignableUsers struct { - Nodes []RepoAssignee + Nodes []struct { + ID string + Login string + Name string + } PageInfo struct { HasNextPage bool EndCursor string @@ -1217,7 +1281,7 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, "endCursor": (*githubv4.String)(nil), } - var users []RepoAssignee + var users []AssignableUser for { var query responseData err := client.Query(repo.RepoHost(), "RepositoryAssignableUsers", &query, variables) @@ -1225,7 +1289,15 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, return nil, err } - users = append(users, query.Repository.AssignableUsers.Nodes...) + for _, node := range query.Repository.AssignableUsers.Nodes { + user := AssignableUser{ + id: node.ID, + login: node.Login, + name: node.Name, + } + + users = append(users, user) + } if !query.Repository.AssignableUsers.PageInfo.HasNextPage { break } @@ -1237,26 +1309,26 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, // RepoAssignableActors fetches all the assignable actors for a repository on // GitHub hosts that support Actor assignees. -func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee, error) { - type repoBotAssignee struct { - ID string - Login string - TypeName string `graphql:"__typename"` - } - - type repoUserAssignee struct { +func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]AssignableActor, error) { + type assignableUser struct { ID string Login string Name string TypeName string `graphql:"__typename"` } + type assignableBot struct { + ID string + Login string + TypeName string `graphql:"__typename"` + } + type responseData struct { Repository struct { SuggestedActors struct { Nodes []struct { - User repoUserAssignee `graphql:"... on User"` - Bot repoBotAssignee `graphql:"... on Bot"` + User assignableUser `graphql:"... on User"` + Bot assignableBot `graphql:"... on Bot"` } PageInfo struct { HasNextPage bool @@ -1272,7 +1344,7 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee "endCursor": (*githubv4.String)(nil), } - var actors []RepoAssignee + var actors []AssignableActor for { var query responseData err := client.Query(repo.RepoHost(), "RepositoryAssignableActors", &query, variables) @@ -1282,16 +1354,16 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee for _, node := range query.Repository.SuggestedActors.Nodes { if node.User.TypeName == "User" { - actor := RepoAssignee{ - ID: node.User.ID, - Login: node.User.Login, - Name: node.User.Name, + actor := AssignableUser{ + id: node.User.ID, + login: node.User.Login, + name: node.User.Name, } actors = append(actors, actor) } else if node.Bot.TypeName == "Bot" { - actor := RepoAssignee{ - ID: node.Bot.ID, - Login: node.Bot.Login, + actor := AssignableBot{ + id: node.Bot.ID, + login: node.Bot.Login, } actors = append(actors, actor) } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 01fc7a4c7..9040a0018 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -526,17 +526,17 @@ func Test_RepoMilestones(t *testing.T) { func TestDisplayName(t *testing.T) { tests := []struct { name string - assignee RepoAssignee + assignee AssignableUser want string }{ { name: "assignee with name", - assignee: RepoAssignee{"123", "octocat123", "Octavious Cath"}, + assignee: AssignableUser{"123", "octocat123", "Octavious Cath"}, want: "octocat123 (Octavious Cath)", }, { name: "assignee without name", - assignee: RepoAssignee{"123", "octocat123", ""}, + assignee: AssignableUser{"123", "octocat123", ""}, want: "octocat123", }, } diff --git a/pkg/cmd/pr/shared/completion.go b/pkg/cmd/pr/shared/completion.go index e07abc5a7..c1296be71 100644 --- a/pkg/cmd/pr/shared/completion.go +++ b/pkg/cmd/pr/shared/completion.go @@ -21,13 +21,13 @@ func RequestableReviewersForCompletion(httpClient *http.Client, repo ghrepo.Inte results := []string{} for _, user := range metadata.AssignableUsers { - if strings.EqualFold(user.Login, metadata.CurrentLogin) { + if strings.EqualFold(user.Login(), metadata.CurrentLogin) { continue } - if user.Name != "" { - results = append(results, fmt.Sprintf("%s\t%s", user.Login, user.Name)) + if user.Name() != "" { + results = append(results, fmt.Sprintf("%s\t%s", user.Login(), user.Name())) } else { - results = append(results, user.Login) + results = append(results, user.Login()) } } for _, team := range metadata.Teams { diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 238939efe..d51405acd 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -407,11 +407,11 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) var users []string for _, u := range metadata.AssignableUsers { - users = append(users, u.Login) + users = append(users, u.Login()) } var actors []string for _, a := range metadata.AssignableActors { - actors = append(actors, a.Login) + actors = append(actors, a.Login()) } var teams []string for _, t := range metadata.Teams { diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index bf4476ca1..b6c927a2d 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -192,7 +192,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface var reviewers []string for _, u := range metadataResult.AssignableUsers { - if u.Login != metadataResult.CurrentLogin { + if u.Login() != metadataResult.CurrentLogin { reviewers = append(reviewers, u.DisplayName()) } } diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index 6895b52ac..7097d0761 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -28,9 +28,9 @@ func TestMetadataSurvey_selectAll(t *testing.T) { fetcher := &metadataFetcher{ metadataResult: &api.RepoMetadataResult{ - AssignableUsers: []api.RepoAssignee{ - {Login: "hubot"}, - {Login: "monalisa"}, + AssignableUsers: []api.AssignableUser{ + api.NewAssignableUser("", "hubot", ""), + api.NewAssignableUser("", "monalisa", ""), }, Labels: []api.RepoLabel{ {Name: "help wanted"},