diff --git a/api/queries_repo.go b/api/queries_repo.go index 5d3475b4f..8480e3868 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1033,117 +1033,6 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput return &result, nil } -type RepoResolveInput struct { - Assignees []string - Reviewers []string - Labels []string - ProjectsV1 bool - ProjectsV2 bool - Milestones []string -} - -// RepoResolveMetadataIDs looks up GraphQL node IDs in bulk -func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput) (*RepoMetadataResult, error) { - users := input.Assignees - hasUser := func(target string) bool { - for _, u := range users { - if strings.EqualFold(u, target) { - return true - } - } - return false - } - - var teams []string - for _, r := range input.Reviewers { - if i := strings.IndexRune(r, '/'); i > -1 { - teams = append(teams, r[i+1:]) - } else if !hasUser(r) { - users = append(users, r) - } - } - - // TODO: Should this retrieve actors separate from `user`? - // there is no way to look up projects nor milestones by name, so preload them all - mi := RepoMetadataInput{ - ProjectsV1: input.ProjectsV1, - ProjectsV2: input.ProjectsV2, - Milestones: len(input.Milestones) > 0, - } - result, err := RepoMetadata(client, repo, mi) - if err != nil { - return result, err - } - if len(users) == 0 && len(teams) == 0 && len(input.Labels) == 0 { - return result, nil - } - - query := &bytes.Buffer{} - fmt.Fprint(query, "query RepositoryResolveMetadataIDs {\n") - // TODO: Should we really be using the `user` query to lookup bots even though it technically works? - // This logic is used to lookup information about assignees that were explicitly defined upfront - for i, u := range users { - fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) - } - if len(input.Labels) > 0 { - fmt.Fprintf(query, "repository(owner:%q,name:%q){\n", repo.RepoOwner(), repo.RepoName()) - for i, l := range input.Labels { - fmt.Fprintf(query, "l%03d: label(name:%q){id,name}\n", i, l) - } - fmt.Fprint(query, "}\n") - } - if len(teams) > 0 { - fmt.Fprintf(query, "organization(login:%q){\n", repo.RepoOwner()) - for i, t := range teams { - fmt.Fprintf(query, "t%03d: team(slug:%q){id,slug}\n", i, t) - } - fmt.Fprint(query, "}\n") - } - fmt.Fprint(query, "}\n") - - response := make(map[string]json.RawMessage) - err = client.GraphQL(repo.RepoHost(), query.String(), nil, &response) - if err != nil { - return result, err - } - - for key, v := range response { - switch key { - case "repository": - repoResponse := make(map[string]RepoLabel) - err := json.Unmarshal(v, &repoResponse) - if err != nil { - return result, err - } - for _, l := range repoResponse { - result.Labels = append(result.Labels, l) - } - case "organization": - orgResponse := make(map[string]OrgTeam) - err := json.Unmarshal(v, &orgResponse) - if err != nil { - return result, err - } - for _, t := range orgResponse { - result.Teams = append(result.Teams, t) - } - default: - user := struct { - Id string - Login string - Name string - }{} - err := json.Unmarshal(v, &user) - if err != nil { - return result, err - } - result.AssignableUsers = append(result.AssignableUsers, NewAssignableUser(user.Id, user.Login, user.Name)) - } - } - - return result, nil -} - type RepoProject struct { ID string `json:"id"` Name string `json:"name"` diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index c291fc468..c885b9968 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -379,88 +379,6 @@ func Test_ProjectNamesToPaths(t *testing.T) { }) } -func Test_RepoResolveMetadataIDs(t *testing.T) { - http := &httpmock.Registry{} - client := newTestClient(http) - - repo, _ := ghrepo.FromFullName("OWNER/REPO") - input := RepoResolveInput{ - Assignees: []string{"monalisa", "hubot"}, - Reviewers: []string{"monalisa", "octocat", "OWNER/core", "/robots"}, - Labels: []string{"bug", "help wanted"}, - } - - expectedQuery := `query RepositoryResolveMetadataIDs { -u000: user(login:"monalisa"){id,login} -u001: user(login:"hubot"){id,login} -u002: user(login:"octocat"){id,login} -repository(owner:"OWNER",name:"REPO"){ -l000: label(name:"bug"){id,name} -l001: label(name:"help wanted"){id,name} -} -organization(login:"OWNER"){ -t000: team(slug:"core"){id,slug} -t001: team(slug:"robots"){id,slug} -} -} -` - responseJSON := ` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "u002": { "login": "octocat", "id": "OCTOID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "Help Wanted", "id": "HELPID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "Robots", "id": "ROBOTID" } - } - } } - ` - - http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), - httpmock.GraphQLQuery(responseJSON, func(q string, _ map[string]interface{}) { - if q != expectedQuery { - t.Errorf("expected query %q, got %q", expectedQuery, q) - } - })) - - result, err := RepoResolveMetadataIDs(client, repo, input) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - expectedMemberIDs := []string{"MONAID", "HUBOTID", "OCTOID"} - memberIDs, err := result.MembersToIDs([]string{"monalisa", "hubot", "octocat"}) - if err != nil { - t.Errorf("error resolving members: %v", err) - } - if !sliceEqual(memberIDs, expectedMemberIDs) { - t.Errorf("expected members %v, got %v", expectedMemberIDs, memberIDs) - } - - expectedTeamIDs := []string{"COREID", "ROBOTID"} - teamIDs, err := result.TeamsToIDs([]string{"/core", "/robots"}) - if err != nil { - t.Errorf("error resolving teams: %v", err) - } - if !sliceEqual(teamIDs, expectedTeamIDs) { - t.Errorf("expected members %v, got %v", expectedTeamIDs, teamIDs) - } - - expectedLabelIDs := []string{"BUGID", "HELPID"} - labelIDs, err := result.LabelsToIDs([]string{"bug", "help wanted"}) - if err != nil { - t.Errorf("error resolving labels: %v", err) - } - if !sliceEqual(labelIDs, expectedLabelIDs) { - t.Errorf("expected members %v, got %v", expectedLabelIDs, labelIDs) - } -} - func TestMembersToIDs(t *testing.T) { t.Parallel() diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 8483280b0..61d39611b 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -55,59 +55,30 @@ func ValidURL(urlStr string) bool { return len(urlStr) < 8192 } -// Ensure that tb.MetadataResult object exists and contains enough pre-fetched API data to be able -// to resolve all object listed in tb to GraphQL IDs. -func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { - resolveInput := api.RepoResolveInput{} - - if len(tb.Assignees) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Assignees = tb.Assignees - } - - if len(tb.Reviewers) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Reviewers = tb.Reviewers - } - - if len(tb.Labels) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Labels) == 0) { - resolveInput.Labels = tb.Labels - } - - if len(tb.ProjectTitles) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Projects) == 0) { - if projectV1Support == gh.ProjectsV1Supported { - resolveInput.ProjectsV1 = true - } - - resolveInput.ProjectsV2 = true - } - - if len(tb.Milestones) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Milestones) == 0) { - resolveInput.Milestones = tb.Milestones - } - - metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) - if err != nil { - return err - } - - if tb.MetadataResult == nil { - tb.MetadataResult = metadataResult - } else { - tb.MetadataResult.Merge(metadataResult) - } - - return nil -} - func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { if !tb.HasMetadata() { return nil } - if err := fillMetadata(client, baseRepo, tb, projectV1Support); err != nil { - return err + // Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey. + if tb.MetadataResult == nil { + input := api.RepoMetadataInput{ + Reviewers: len(tb.Reviewers) > 0, + Assignees: len(tb.Assignees) > 0, + ActorAssignees: tb.ActorAssignees, + Labels: len(tb.Labels) > 0, + ProjectsV1: len(tb.ProjectTitles) > 0 && projectV1Support == gh.ProjectsV1Supported, + ProjectsV2: len(tb.ProjectTitles) > 0, + Milestones: len(tb.Milestones) > 0, + } + + metadataResult, err := api.RepoMetadata(client, baseRepo, input) + if err != nil { + return err + } + tb.MetadataResult = metadataResult } - // TODO: Think we need to adapt the logic in `Editable.AssigneeIds()` which is called by `UpdateIssue()` here to replace @me and @copilot assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) if err != nil { return fmt.Errorf("could not assign user: %w", err)