Merge pull request #10815 from cli/wm-kw/projectsv1-deprecation-issue-create

Feature detect v1 projects on non web-mode `issue create`
This commit is contained in:
William Martin 2025-04-23 13:31:24 +02:00 committed by GitHub
commit 2f06fafd1b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 258 additions and 42 deletions

View file

@ -863,7 +863,8 @@ type RepoMetadataInput struct {
Assignees bool
Reviewers bool
Labels bool
Projects bool
ProjectsV1 bool
ProjectsV2 bool
Milestones bool
}
@ -882,6 +883,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
return err
})
}
if input.Reviewers {
g.Go(func() error {
teams, err := OrganizationTeams(client, repo)
@ -894,6 +896,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
return nil
})
}
if input.Reviewers {
g.Go(func() error {
login, err := CurrentLoginName(client, repo.RepoHost())
@ -904,6 +907,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
return err
})
}
if input.Labels {
g.Go(func() error {
labels, err := RepoLabels(client, repo)
@ -914,13 +918,23 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
return err
})
}
if input.Projects {
if input.ProjectsV1 {
g.Go(func() error {
var err error
result.Projects, result.ProjectsV2, err = relevantProjects(client, repo)
result.Projects, err = v1Projects(client, repo)
return err
})
}
if input.ProjectsV2 {
g.Go(func() error {
var err error
result.ProjectsV2, err = v2Projects(client, repo)
return err
})
}
if input.Milestones {
g.Go(func() error {
milestones, err := RepoMilestones(client, repo, "open")
@ -943,7 +957,8 @@ type RepoResolveInput struct {
Assignees []string
Reviewers []string
Labels []string
Projects []string
ProjectsV1 bool
ProjectsV2 bool
Milestones []string
}
@ -970,7 +985,8 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes
// there is no way to look up projects nor milestones by name, so preload them all
mi := RepoMetadataInput{
Projects: len(input.Projects) > 0,
ProjectsV1: input.ProjectsV1,
ProjectsV2: input.ProjectsV2,
Milestones: len(input.Milestones) > 0,
}
result, err := RepoMetadata(client, repo, mi)
@ -1245,18 +1261,12 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
return ProjectsToPaths(projects, projectsV2, projectNames)
}
// RelevantProjects retrieves set of Projects and ProjectsV2 relevant to given repository:
// v1Projects retrieves set of RepoProjects relevant to given repository:
// - Projects for repository
// - Projects for repository organization, if it belongs to one
// - ProjectsV2 owned by current user
// - ProjectsV2 linked to repository
// - ProjectsV2 owned by repository organization, if it belongs to one
func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []ProjectV2, error) {
func v1Projects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
var repoProjects []RepoProject
var orgProjects []RepoProject
var userProjectsV2 []ProjectV2
var repoProjectsV2 []ProjectV2
var orgProjectsV2 []ProjectV2
g, _ := errgroup.WithContext(context.Background())
@ -1268,6 +1278,7 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
}
return err
})
g.Go(func() error {
var err error
orgProjects, err = OrganizationProjects(client, repo)
@ -1277,6 +1288,29 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
}
return nil
})
if err := g.Wait(); err != nil {
return nil, err
}
projects := make([]RepoProject, 0, len(repoProjects)+len(orgProjects))
projects = append(projects, repoProjects...)
projects = append(projects, orgProjects...)
return projects, nil
}
// v2Projects retrieves set of ProjectV2 relevant to given repository:
// - ProjectsV2 owned by current user
// - ProjectsV2 linked to repository
// - ProjectsV2 owned by repository organization, if it belongs to one
func v2Projects(client *Client, repo ghrepo.Interface) ([]ProjectV2, error) {
var userProjectsV2 []ProjectV2
var repoProjectsV2 []ProjectV2
var orgProjectsV2 []ProjectV2
g, _ := errgroup.WithContext(context.Background())
g.Go(func() error {
var err error
userProjectsV2, err = CurrentUserProjectsV2(client, repo.RepoHost())
@ -1286,6 +1320,7 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
}
return nil
})
g.Go(func() error {
var err error
repoProjectsV2, err = RepoProjectsV2(client, repo)
@ -1295,6 +1330,7 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
}
return nil
})
g.Go(func() error {
var err error
orgProjectsV2, err = OrganizationProjectsV2(client, repo)
@ -1308,13 +1344,9 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
})
if err := g.Wait(); err != nil {
return nil, nil, err
return nil, err
}
projects := make([]RepoProject, 0, len(repoProjects)+len(orgProjects))
projects = append(projects, repoProjects...)
projects = append(projects, orgProjects...)
// ProjectV2 might appear across multiple queries so use a map to keep them deduplicated.
m := make(map[string]ProjectV2, len(userProjectsV2)+len(repoProjectsV2)+len(orgProjectsV2))
for _, p := range userProjectsV2 {
@ -1331,7 +1363,27 @@ func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []P
projectsV2 = append(projectsV2, p)
}
return projects, projectsV2, nil
return projectsV2, nil
}
// relevantProjects retrieves set of Project or ProjectV2 relevant to given repository:
// - Projects for repository
// - Projects for repository organization, if it belongs to one
// - ProjectsV2 owned by current user
// - ProjectsV2 linked to repository
// - ProjectsV2 owned by repository organization, if it belongs to one
func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []ProjectV2, error) {
v1Projects, err := v1Projects(client, repo)
if err != nil {
return nil, nil, err
}
v2Projects, err := v2Projects(client, repo)
if err != nil {
return nil, nil, err
}
return v1Projects, v2Projects, nil
}
func CreateRepoTransformToV4(apiClient *Client, hostname string, method string, path string, body io.Reader) (*Repository, error) {

View file

@ -44,7 +44,8 @@ func Test_RepoMetadata(t *testing.T) {
Assignees: true,
Reviewers: true,
Labels: true,
Projects: true,
ProjectsV1: true,
ProjectsV2: true,
Milestones: true,
}

View file

@ -4,10 +4,12 @@ import (
"errors"
"fmt"
"net/http"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
@ -24,6 +26,7 @@ type CreateOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
Prompter prShared.Prompt
Detector fd.Detector
TitledEditSurvey func(string, string) (string, string, error)
RootDirOverride string
@ -46,11 +49,12 @@ type CreateOptions struct {
func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command {
opts := &CreateOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
Config: f.Config,
Browser: f.Browser,
Prompter: f.Prompter,
IO: f.IOStreams,
HttpClient: f.HttpClient,
Config: f.Config,
Browser: f.Browser,
Prompter: f.Prompter,
TitledEditSurvey: prShared.TitledEditSurvey(&prShared.UserEditor{Config: f.Config, IO: f.IOStreams}),
}
@ -146,6 +150,15 @@ func createRun(opts *CreateOptions) (err error) {
return
}
// TODO projectsV1Deprecation
// Remove this section as we should no longer need to detect
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
}
projectsV1Support := opts.Detector.ProjectsV1()
isTerminal := opts.IO.IsStdoutTTY()
var milestones []string
@ -279,7 +292,7 @@ func createRun(opts *CreateOptions) (err error) {
Repo: baseRepo,
State: &tb,
}
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb)
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support)
if err != nil {
return
}
@ -335,7 +348,7 @@ func createRun(opts *CreateOptions) (err error) {
params["issueTemplate"] = templateNameForSubmit
}
err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb)
err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb, projectsV1Support)
if err != nil {
return
}

View file

@ -14,6 +14,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/config"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
@ -521,6 +522,7 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, isTTY bool, cli strin
cmd := NewCmdCreate(factory, func(opts *CreateOptions) error {
opts.RootDirOverride = rootDir
opts.Detector = &fd.EnabledDetectorMock{}
return createRun(opts)
})
@ -1026,3 +1028,79 @@ func TestIssueCreate_projectsV2(t *testing.T) {
assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String())
}
// TODO projectsV1Deprecation
// Remove this test.
func TestProjectsV1Deprecation(t *testing.T) {
t.Run("non-interactive submission", func(t *testing.T) {
t.Run("when projects v1 is supported, queries for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
reg.Register(
// ( is required to avoid matching projectsV2
httpmock.GraphQL(`projects\(`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = createRun(&CreateOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Detector: &fd.EnabledDetectorMock{},
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request contained projects
reg.Verify(t)
})
t.Run("when projects v1 is not supported, does not query for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
// ( is required to avoid matching projectsV2
reg.Exclude(t, httpmock.GraphQL(`projects\(`))
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we're not really interested in it.
_ = createRun(&CreateOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Detector: &fd.DisabledDetectorMock{},
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request contained projectCards
reg.Verify(t)
})
})
}

View file

@ -440,7 +440,8 @@ func createRun(opts *CreateOptions) error {
if err != nil {
return err
}
return submitPR(*opts, *ctx, *state)
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
}
if opts.RecoverFile != "" {
@ -536,7 +537,8 @@ func createRun(opts *CreateOptions) error {
Repo: ctx.PRRefs.BaseRepo(),
State: state,
}
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state)
// TODO wm: revisit project support
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, gh.ProjectsV1Supported)
if err != nil {
return err
}
@ -565,11 +567,13 @@ func createRun(opts *CreateOptions) error {
if action == shared.SubmitDraftAction {
state.Draft = true
return submitPR(*opts, *ctx, *state)
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
}
if action == shared.SubmitAction {
return submitPR(*opts, *ctx, *state)
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
}
err = errors.New("expected to cancel, preview, or submit")
@ -966,7 +970,7 @@ func getRemotes(opts *CreateOptions) (ghContext.Remotes, error) {
return remotes, nil
}
func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataState) error {
func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataState, projectV1Support gh.ProjectsV1Support) error {
client := ctx.Client
params := map[string]interface{}{
@ -982,7 +986,7 @@ func submitPR(opts CreateOptions, ctx CreateContext, state shared.IssueMetadataS
return errors.New("pull request title must not be blank")
}
err := shared.AddMetadataToIssueParams(client, ctx.PRRefs.BaseRepo(), params, &state)
err := shared.AddMetadataToIssueParams(client, ctx.PRRefs.BaseRepo(), params, &state, projectV1Support)
if err != nil {
return err
}

View file

@ -381,7 +381,8 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable)
Reviewers: editable.Reviewers.Edited,
Assignees: editable.Assignees.Edited,
Labels: editable.Labels.Edited,
Projects: editable.Projects.Edited,
ProjectsV1: editable.Projects.Edited,
ProjectsV2: editable.Projects.Edited,
Milestones: editable.Milestone.Edited,
}
metadata, err := api.RepoMetadata(client, repo, input)

View file

@ -6,6 +6,7 @@ import (
"strings"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/search"
"github.com/google/shlex"
@ -56,7 +57,7 @@ func ValidURL(urlStr string) bool {
// 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) error {
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) {
@ -72,7 +73,11 @@ func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetada
}
if len(tb.Projects) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Projects) == 0) {
resolveInput.Projects = tb.Projects
if projectV1Support == gh.ProjectsV1Supported {
resolveInput.ProjectsV1 = true
}
resolveInput.ProjectsV2 = true
}
if len(tb.Milestones) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Milestones) == 0) {
@ -93,12 +98,12 @@ func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetada
return nil
}
func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState) error {
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); err != nil {
if err := fillMetadata(client, baseRepo, tb, projectV1Support); err != nil {
return err
}

View file

@ -151,7 +151,7 @@ type RepoMetadataFetcher interface {
RepoMetadataFetch(api.RepoMetadataInput) (*api.RepoMetadataResult, error)
}
func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState) error {
func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support) error {
isChosen := func(m string) bool {
for _, c := range state.Metadata {
if m == c {
@ -181,7 +181,8 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface
Reviewers: isChosen("Reviewers"),
Assignees: isChosen("Assignees"),
Labels: isChosen("Labels"),
Projects: isChosen("Projects"),
ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported,
ProjectsV2: isChosen("Projects"),
Milestones: isChosen("Milestone"),
}
metadataResult, err := fetcher.RepoMetadataFetch(metadataInput)

View file

@ -1,13 +1,16 @@
package shared
import (
"errors"
"testing"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type metadataFetcher struct {
@ -68,7 +71,7 @@ func TestMetadataSurvey_selectAll(t *testing.T) {
Assignees: []string{"hubot"},
Type: PRMetadata,
}
err := MetadataSurvey(pm, ios, repo, fetcher, state)
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported)
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
@ -113,7 +116,8 @@ func TestMetadataSurvey_keepExisting(t *testing.T) {
state := &IssueMetadataState{
Assignees: []string{"hubot"},
}
err := MetadataSurvey(pm, ios, repo, fetcher, state)
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported)
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
@ -124,6 +128,63 @@ func TestMetadataSurvey_keepExisting(t *testing.T) {
assert.Equal(t, []string{"The road to 1.0"}, state.Projects)
}
// TODO projectsV1Deprecation
// Remove this test and projectsV1MetadataFetcherSpy
func TestMetadataSurveyProjectV1Deprecation(t *testing.T) {
t.Run("when projectsV1 is supported, requests projectsV1", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
repo := ghrepo.New("OWNER", "REPO")
fetcher := &projectsV1MetadataFetcherSpy{}
pm := prompter.NewMockPrompter(t)
pm.RegisterMultiSelect("What would you like to add?", []string{}, []string{"Assignees", "Labels", "Projects", "Milestone"}, func(_ string, _, options []string) ([]int, error) {
i, err := prompter.IndexFor(options, "Projects")
require.NoError(t, err)
return []int{i}, nil
})
pm.RegisterMultiSelect("Projects", []string{}, []string{"Huge Refactoring"}, func(_ string, _, _ []string) ([]int, error) {
return []int{0}, nil
})
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Supported)
require.ErrorContains(t, err, "expected test error")
require.True(t, fetcher.projectsV1Requested, "expected projectsV1 to be requested")
})
t.Run("when projectsV1 is supported, does not request projectsV1", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
repo := ghrepo.New("OWNER", "REPO")
fetcher := &projectsV1MetadataFetcherSpy{}
pm := prompter.NewMockPrompter(t)
pm.RegisterMultiSelect("What would you like to add?", []string{}, []string{"Assignees", "Labels", "Projects", "Milestone"}, func(_ string, _, options []string) ([]int, error) {
i, err := prompter.IndexFor(options, "Projects")
require.NoError(t, err)
return []int{i}, nil
})
pm.RegisterMultiSelect("Projects", []string{}, []string{"Huge Refactoring"}, func(_ string, _, _ []string) ([]int, error) {
return []int{0}, nil
})
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Unsupported)
require.ErrorContains(t, err, "expected test error")
require.False(t, fetcher.projectsV1Requested, "expected projectsV1 not to be requested")
})
}
type projectsV1MetadataFetcherSpy struct {
projectsV1Requested bool
}
func (mf *projectsV1MetadataFetcherSpy) RepoMetadataFetch(input api.RepoMetadataInput) (*api.RepoMetadataResult, error) {
if input.ProjectsV1 {
mf.projectsV1Requested = true
}
return nil, errors.New("expected test error")
}
func TestTitledEditSurvey_cleanupHint(t *testing.T) {
var editorInitialText string
editor := &testEditor{