Merge pull request #10909 from cli/wm-kw/projectsv1-deprecation-pr-create
Feature detect v1 projects on non-interactive pr create
This commit is contained in:
William Martin
GitHub
commit
5ee70748fa
6 changed files with 174 additions and 36 deletions
|
|
@ -738,34 +738,37 @@ func (m *RepoMetadataResult) LabelsToIDs(names []string) ([]string, error) {
|
|||
return ids, nil
|
||||
}
|
||||
|
||||
// ProjectsToIDs returns two arrays:
|
||||
// ProjectsTitlesToIDs returns two arrays:
|
||||
// - the first contains IDs of projects V1
|
||||
// - the second contains IDs of projects V2
|
||||
// - if neither project V1 or project V2 can be found with a given name, then an error is returned
|
||||
func (m *RepoMetadataResult) ProjectsToIDs(names []string) ([]string, []string, error) {
|
||||
func (m *RepoMetadataResult) ProjectsTitlesToIDs(titles []string) ([]string, []string, error) {
|
||||
var ids []string
|
||||
var idsV2 []string
|
||||
for _, projectName := range names {
|
||||
id, found := m.projectNameToID(projectName)
|
||||
for _, title := range titles {
|
||||
id, found := m.v1ProjectNameToID(title)
|
||||
if found {
|
||||
ids = append(ids, id)
|
||||
continue
|
||||
}
|
||||
|
||||
idV2, found := m.projectV2TitleToID(projectName)
|
||||
idV2, found := m.v2ProjectTitleToID(title)
|
||||
if found {
|
||||
idsV2 = append(idsV2, idV2)
|
||||
continue
|
||||
}
|
||||
|
||||
return nil, nil, fmt.Errorf("'%s' not found", projectName)
|
||||
return nil, nil, fmt.Errorf("'%s' not found", title)
|
||||
}
|
||||
return ids, idsV2, nil
|
||||
}
|
||||
|
||||
func (m *RepoMetadataResult) projectNameToID(projectName string) (string, bool) {
|
||||
// We use the word "titles" when referring to v1 and v2 projects.
|
||||
// In reality, v1 projects really have "names", so there is a bit of a
|
||||
// mismatch we just need to gloss over.
|
||||
func (m *RepoMetadataResult) v1ProjectNameToID(name string) (string, bool) {
|
||||
for _, p := range m.Projects {
|
||||
if strings.EqualFold(projectName, p.Name) {
|
||||
if strings.EqualFold(name, p.Name) {
|
||||
return p.ID, true
|
||||
}
|
||||
}
|
||||
|
|
@ -773,9 +776,9 @@ func (m *RepoMetadataResult) projectNameToID(projectName string) (string, bool)
|
|||
return "", false
|
||||
}
|
||||
|
||||
func (m *RepoMetadataResult) projectV2TitleToID(projectTitle string) (string, bool) {
|
||||
func (m *RepoMetadataResult) v2ProjectTitleToID(title string) (string, bool) {
|
||||
for _, p := range m.ProjectsV2 {
|
||||
if strings.EqualFold(projectTitle, p.Title) {
|
||||
if strings.EqualFold(title, p.Title) {
|
||||
return p.ID, true
|
||||
}
|
||||
}
|
||||
|
|
@ -783,8 +786,8 @@ func (m *RepoMetadataResult) projectV2TitleToID(projectTitle string) (string, bo
|
|||
return "", false
|
||||
}
|
||||
|
||||
func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string, projectsV1Support gh.ProjectsV1Support) ([]string, error) {
|
||||
paths := make([]string, 0, len(projectNames))
|
||||
func ProjectTitlesToPaths(client *Client, repo ghrepo.Interface, titles []string, projectsV1Support gh.ProjectsV1Support) ([]string, error) {
|
||||
paths := make([]string, 0, len(titles))
|
||||
matchedPaths := map[string]struct{}{}
|
||||
|
||||
// TODO: ProjectsV1Cleanup
|
||||
|
|
@ -796,9 +799,9 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
|
|||
return nil, err
|
||||
}
|
||||
|
||||
for _, projectName := range projectNames {
|
||||
for _, title := range titles {
|
||||
for _, p := range v1Projects {
|
||||
if strings.EqualFold(projectName, p.Name) {
|
||||
if strings.EqualFold(title, p.Name) {
|
||||
pathParts := strings.Split(p.ResourcePath, "/")
|
||||
var path string
|
||||
if pathParts[1] == "orgs" || pathParts[1] == "users" {
|
||||
|
|
@ -807,7 +810,7 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
|
|||
path = fmt.Sprintf("%s/%s/%s", pathParts[1], pathParts[2], pathParts[4])
|
||||
}
|
||||
paths = append(paths, path)
|
||||
matchedPaths[projectName] = struct{}{}
|
||||
matchedPaths[title] = struct{}{}
|
||||
break
|
||||
}
|
||||
}
|
||||
|
|
@ -820,15 +823,15 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
|
|||
return nil, err
|
||||
}
|
||||
|
||||
for _, projectName := range projectNames {
|
||||
for _, title := range titles {
|
||||
// If we already found a v1 project with this name, skip it
|
||||
if _, ok := matchedPaths[projectName]; ok {
|
||||
if _, ok := matchedPaths[title]; ok {
|
||||
continue
|
||||
}
|
||||
|
||||
found := false
|
||||
for _, p := range v2Projects {
|
||||
if strings.EqualFold(projectName, p.Title) {
|
||||
if strings.EqualFold(title, p.Title) {
|
||||
pathParts := strings.Split(p.ResourcePath, "/")
|
||||
var path string
|
||||
if pathParts[1] == "orgs" || pathParts[1] == "users" {
|
||||
|
|
@ -843,7 +846,7 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
|
|||
}
|
||||
|
||||
if !found {
|
||||
return nil, fmt.Errorf("'%s' not found", projectName)
|
||||
return nil, fmt.Errorf("'%s' not found", title)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -187,7 +187,7 @@ func Test_RepoMetadata(t *testing.T) {
|
|||
|
||||
expectedProjectIDs := []string{"TRIAGEID", "ROADMAPID"}
|
||||
expectedProjectV2IDs := []string{"TRIAGEV2ID", "ROADMAPV2ID", "MONALISAV2ID"}
|
||||
projectIDs, projectV2IDs, err := result.ProjectsToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2", "monalisav2"})
|
||||
projectIDs, projectV2IDs, err := result.ProjectsTitlesToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2", "monalisav2"})
|
||||
if err != nil {
|
||||
t.Errorf("error resolving projects: %v", err)
|
||||
}
|
||||
|
|
@ -273,7 +273,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
|
|||
} } } }
|
||||
`))
|
||||
|
||||
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Supported)
|
||||
projectPaths, err := ProjectTitlesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Supported)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -331,7 +331,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
|
|||
} } } }
|
||||
`))
|
||||
|
||||
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Unsupported)
|
||||
projectPaths, err := ProjectTitlesToPaths(client, repo, []string{"TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Unsupported)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
|
@ -374,7 +374,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
|
|||
} } } }
|
||||
`))
|
||||
|
||||
_, err := ProjectNamesToPaths(client, repo, []string{"TriageV2"}, gh.ProjectsV1Unsupported)
|
||||
_, err := ProjectTitlesToPaths(client, repo, []string{"TriageV2"}, gh.ProjectsV1Unsupported)
|
||||
require.Equal(t, err, fmt.Errorf("'TriageV2' not found"))
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import (
|
|||
ghContext "github.com/cli/cli/v2/context"
|
||||
"github.com/cli/cli/v2/git"
|
||||
"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"
|
||||
|
|
@ -31,6 +32,7 @@ import (
|
|||
|
||||
type CreateOptions struct {
|
||||
// This struct stores user input and factory functions
|
||||
Detector fd.Detector
|
||||
HttpClient func() (*http.Client, error)
|
||||
GitClient *git.Client
|
||||
Config func() (gh.Config, error)
|
||||
|
|
@ -363,6 +365,20 @@ func createRun(opts *CreateOptions) error {
|
|||
return err
|
||||
}
|
||||
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// 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, ctx.PRRefs.BaseRepo().RepoHost())
|
||||
}
|
||||
|
||||
projectsV1Support := opts.Detector.ProjectsV1()
|
||||
|
||||
client := ctx.Client
|
||||
|
||||
state, err := NewIssueState(*ctx, *opts)
|
||||
|
|
@ -384,7 +400,7 @@ func createRun(opts *CreateOptions) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
openURL, err = generateCompareURL(*ctx, *state)
|
||||
openURL, err = generateCompareURL(*ctx, *state, gh.ProjectsV1Supported)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -441,7 +457,7 @@ func createRun(opts *CreateOptions) error {
|
|||
return err
|
||||
}
|
||||
// TODO wm: revisit project support
|
||||
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
|
||||
return submitPR(*opts, *ctx, *state, projectsV1Support)
|
||||
}
|
||||
|
||||
if opts.RecoverFile != "" {
|
||||
|
|
@ -518,7 +534,7 @@ func createRun(opts *CreateOptions) error {
|
|||
}
|
||||
}
|
||||
|
||||
openURL, err = generateCompareURL(*ctx, *state)
|
||||
openURL, err = generateCompareURL(*ctx, *state, gh.ProjectsV1Supported)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -568,12 +584,12 @@ func createRun(opts *CreateOptions) error {
|
|||
if action == shared.SubmitDraftAction {
|
||||
state.Draft = true
|
||||
// TODO wm: revisit project support
|
||||
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
|
||||
return submitPR(*opts, *ctx, *state, projectsV1Support)
|
||||
}
|
||||
|
||||
if action == shared.SubmitAction {
|
||||
// TODO wm: revisit project support
|
||||
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
|
||||
return submitPR(*opts, *ctx, *state, projectsV1Support)
|
||||
}
|
||||
|
||||
err = errors.New("expected to cancel, preview, or submit")
|
||||
|
|
@ -1216,13 +1232,13 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
|
|||
return pushBranch()
|
||||
}
|
||||
|
||||
func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState) (string, error) {
|
||||
func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState, projectsV1Support gh.ProjectsV1Support) (string, error) {
|
||||
u := ghrepo.GenerateRepoURL(
|
||||
ctx.PRRefs.BaseRepo(),
|
||||
"compare/%s...%s?expand=1",
|
||||
url.PathEscape(ctx.PRRefs.BaseRef()), url.PathEscape(ctx.PRRefs.QualifiedHeadRef()))
|
||||
// TODO wm: revisit project support
|
||||
url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.PRRefs.BaseRepo(), u, state, gh.ProjectsV1Supported)
|
||||
url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.PRRefs.BaseRepo(), u, state, projectsV1Support)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import (
|
|||
"github.com/cli/cli/v2/git"
|
||||
"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"
|
||||
|
|
@ -1618,6 +1619,7 @@ func Test_createRun(t *testing.T) {
|
|||
}
|
||||
|
||||
opts := CreateOptions{}
|
||||
opts.Detector = &fd.EnabledDetectorMock{}
|
||||
opts.Prompter = pm
|
||||
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
|
|
@ -1941,7 +1943,8 @@ func Test_generateCompareURL(t *testing.T) {
|
|||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := generateCompareURL(tt.ctx, tt.state)
|
||||
// TODO wm: projects v1 support?
|
||||
got, err := generateCompareURL(tt.ctx, tt.state, gh.ProjectsV1Supported)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("generateCompareURL() error = %v, wantErr %v", err, tt.wantErr)
|
||||
return
|
||||
|
|
@ -2009,3 +2012,119 @@ func mockRetrieveProjects(_ *testing.T, reg *httpmock.Registry) {
|
|||
}
|
||||
|
||||
// TODO interactive metadata tests once: 1) we have test utils for Prompter and 2) metadata questions use Prompter
|
||||
|
||||
// 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, ""),
|
||||
)
|
||||
|
||||
cs, cmdTeardown := run.Stub()
|
||||
defer cmdTeardown(t)
|
||||
|
||||
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
|
||||
|
||||
// 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{
|
||||
Detector: &fd.EnabledDetectorMock{},
|
||||
IO: ios,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: reg}, nil
|
||||
},
|
||||
GitClient: &git.Client{
|
||||
GhPath: "some/path/gh",
|
||||
GitPath: "some/path/git",
|
||||
},
|
||||
Remotes: func() (context.Remotes, error) {
|
||||
return context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{
|
||||
Name: "upstream",
|
||||
Resolved: "base",
|
||||
},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
Finder: shared.NewMockFinder("feature", nil, nil),
|
||||
|
||||
HeadBranch: "feature",
|
||||
|
||||
TitleProvided: true,
|
||||
BodyProvided: true,
|
||||
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\(`))
|
||||
|
||||
cs, cmdTeardown := run.Stub()
|
||||
defer cmdTeardown(t)
|
||||
|
||||
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
|
||||
|
||||
// Ignore the error because we're not really interested in it.
|
||||
_ = createRun(&CreateOptions{
|
||||
Detector: &fd.DisabledDetectorMock{},
|
||||
IO: ios,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: reg}, nil
|
||||
},
|
||||
GitClient: &git.Client{
|
||||
GhPath: "some/path/gh",
|
||||
GitPath: "some/path/git",
|
||||
},
|
||||
Remotes: func() (context.Remotes, error) {
|
||||
return context.Remotes{
|
||||
{
|
||||
Remote: &git.Remote{
|
||||
Name: "upstream",
|
||||
Resolved: "base",
|
||||
},
|
||||
Repo: ghrepo.New("OWNER", "REPO"),
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
Finder: shared.NewMockFinder("feature", nil, nil),
|
||||
|
||||
HeadBranch: "feature",
|
||||
|
||||
TitleProvided: true,
|
||||
BodyProvided: true,
|
||||
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)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -137,7 +137,7 @@ func (e Editable) ProjectIds() (*[]string, error) {
|
|||
s.RemoveValues(e.Projects.Remove)
|
||||
e.Projects.Value = s.ToSlice()
|
||||
}
|
||||
p, _, err := e.Metadata.ProjectsToIDs(e.Projects.Value)
|
||||
p, _, err := e.Metadata.ProjectsTitlesToIDs(e.Projects.Value)
|
||||
return &p, err
|
||||
}
|
||||
|
||||
|
|
@ -171,14 +171,14 @@ func (e Editable) ProjectV2Ids() (*[]string, *[]string, error) {
|
|||
var err error
|
||||
|
||||
if addTitles.Len() > 0 {
|
||||
_, addIds, err = e.Metadata.ProjectsToIDs(addTitles.ToSlice())
|
||||
_, addIds, err = e.Metadata.ProjectsTitlesToIDs(addTitles.ToSlice())
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
}
|
||||
|
||||
if removeTitles.Len() > 0 {
|
||||
_, removeIds, err = e.Metadata.ProjectsToIDs(removeTitles.ToSlice())
|
||||
_, removeIds, err = e.Metadata.ProjectsTitlesToIDs(removeTitles.ToSlice())
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, ba
|
|||
q.Set("labels", strings.Join(state.Labels, ","))
|
||||
}
|
||||
if len(state.ProjectTitles) > 0 {
|
||||
projectPaths, err := api.ProjectNamesToPaths(client, baseRepo, state.ProjectTitles, projectsV1Support)
|
||||
projectPaths, err := api.ProjectTitlesToPaths(client, baseRepo, state.ProjectTitles, projectsV1Support)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("could not add to project: %w", err)
|
||||
}
|
||||
|
|
@ -119,7 +119,7 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
|
|||
}
|
||||
params["labelIds"] = labelIDs
|
||||
|
||||
projectIDs, projectV2IDs, err := tb.MetadataResult.ProjectsToIDs(tb.ProjectTitles)
|
||||
projectIDs, projectV2IDs, err := tb.MetadataResult.ProjectsTitlesToIDs(tb.ProjectTitles)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not add to project: %w", err)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue