From 87fabebb68984c4e6d5ac9447a244191975d83c3 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 6 Feb 2023 15:31:36 -0800 Subject: [PATCH 01/25] start on gh rs --- pkg/cmd/ruleset/check/check.go | 65 ++++++++++++++++++++++++++++++++++ pkg/cmd/ruleset/ruleset.go | 26 ++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 pkg/cmd/ruleset/check/check.go create mode 100644 pkg/cmd/ruleset/ruleset.go diff --git a/pkg/cmd/ruleset/check/check.go b/pkg/cmd/ruleset/check/check.go new file mode 100644 index 000000000..a143981b1 --- /dev/null +++ b/pkg/cmd/ruleset/check/check.go @@ -0,0 +1,65 @@ +package check + +import ( + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type CheckOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + Config func() (config.Config, error) + BaseRepo func() (ghrepo.Interface, error) + + Branch string +} + +func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Command { + opts := &CheckOptions{ + IO: f.IOStreams, + Config: f.Config, + HttpClient: f.HttpClient, + } + cmd := &cobra.Command{ + Use: "check []", + Short: "Print rules that would apply to a given branch", + Long: heredoc.Doc(` + TODO + `), + Example: "TODO", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + // TODO flag to do a push + + if len(args) > 0 { + opts.Branch = args[0] + } + + if runF != nil { + return runF(opts) + } + + return checkRun(opts) + }, + } + + return cmd +} + +func checkRun(opts *CheckOptions) error { + // TODO sniff local branch if opts.Branch is empty + // TODO ask about pushing (if interactive) + // TODO error if not interactive and --push not specified + + // is the --push redundant? like, it needs to be specified every time for scripted use. can i tell if a branch is up to date with remote without a push? i could figure that out i think and then would know a push wasn't needed (but it will require a fetch per invocation. that seems fine?) + return nil +} diff --git a/pkg/cmd/ruleset/ruleset.go b/pkg/cmd/ruleset/ruleset.go new file mode 100644 index 000000000..2fa86684c --- /dev/null +++ b/pkg/cmd/ruleset/ruleset.go @@ -0,0 +1,26 @@ +package ruleset + +import ( + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdRuleset(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "ruleset ", + Short: "Manage repository and organization rulesets", + Long: heredoc.Doc(` + TODO + `), + Aliases: []string{"rs"}, + Example: "TODO", + } + + cmdutil.EnableRepoOverride(cmd, f) + // cmd.AddCommand(cmdList.NewCmdList(f, nil) + // cmd.AddCommand(cmdList.NewCmdView(f, nil) + // cmd.AddCommand(cmdCheck.NewCmdCheck(f, nil) + + return cmd +} From bcb4194692020200cf89d8892918594a3905f7e5 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 8 Feb 2023 11:41:09 -0800 Subject: [PATCH 02/25] WIP compute branch, call API --- pkg/cmd/root/root.go | 2 ++ pkg/cmd/ruleset/check/check.go | 61 ++++++++++++++++++++++++++++++++-- pkg/cmd/ruleset/ruleset.go | 3 +- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 2929ca720..6957ff7b6 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -31,6 +31,7 @@ import ( releaseCmd "github.com/cli/cli/v2/pkg/cmd/release" repoCmd "github.com/cli/cli/v2/pkg/cmd/repo" creditsCmd "github.com/cli/cli/v2/pkg/cmd/repo/credits" + rulesetCmd "github.com/cli/cli/v2/pkg/cmd/ruleset" runCmd "github.com/cli/cli/v2/pkg/cmd/run" searchCmd "github.com/cli/cli/v2/pkg/cmd/search" secretCmd "github.com/cli/cli/v2/pkg/cmd/secret" @@ -152,6 +153,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command, cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) cmd.AddCommand(releaseCmd.NewCmdRelease(&repoResolvingCmdFactory)) cmd.AddCommand(repoCmd.NewCmdRepo(&repoResolvingCmdFactory)) + cmd.AddCommand(rulesetCmd.NewCmdRuleset(&repoResolvingCmdFactory)) cmd.AddCommand(runCmd.NewCmdRun(&repoResolvingCmdFactory)) cmd.AddCommand(workflowCmd.NewCmdWorkflow(&repoResolvingCmdFactory)) cmd.AddCommand(labelCmd.NewCmdLabel(&repoResolvingCmdFactory)) diff --git a/pkg/cmd/ruleset/check/check.go b/pkg/cmd/ruleset/check/check.go index a143981b1..34c708c69 100644 --- a/pkg/cmd/ruleset/check/check.go +++ b/pkg/cmd/ruleset/check/check.go @@ -1,9 +1,14 @@ package check import ( + "context" + "fmt" "net/http" + "net/url" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" @@ -16,8 +21,10 @@ type CheckOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) + Git *git.Client - Branch string + Branch string + Default bool } func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Command { @@ -25,6 +32,7 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Git: f.GitClient, } cmd := &cobra.Command{ Use: "check []", @@ -48,18 +56,65 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm return runF(opts) } + if opts.Branch != "" && opts.Default { + return cmdutil.FlagErrorf( + "branch argument '%s' and --default mutually exclusive", opts.Branch) + } + return checkRun(opts) }, } + cmd.Flags().BoolVar(&opts.Default, "default", false, "Check rules on default branch") + return cmd } func checkRun(opts *CheckOptions) error { - // TODO sniff local branch if opts.Branch is empty // TODO ask about pushing (if interactive) // TODO error if not interactive and --push not specified + // TODO parsing for errors on push + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + client := api.NewClientFromHTTP(httpClient) + + repoI, err := opts.BaseRepo() + if err != nil { + return err + } + + git := opts.Git + + if opts.Default { + repo, err := api.GitHubRepo(client, repoI) + if err != nil { + return fmt.Errorf("could not get repository information: %w", err) + } + opts.Branch = repo.DefaultBranchRef.Name + } + + if opts.Branch == "" { + opts.Branch, err = git.CurrentBranch(context.Background()) + if err != nil { + return fmt.Errorf("could not determine current branch: %w", err) + } + } + + var lol interface{} + + endpoint := fmt.Sprintf("repos/%s/%s/rules/branches/%s", repoI.RepoOwner(), repoI.RepoName(), url.PathEscape(opts.Branch)) + + if err = client.REST(repoI.RepoHost(), "GET", endpoint, nil, &lol); err != nil { + return fmt.Errorf("GET %s failed: %w", endpoint, err) + } + + // TODO handle 404s gracefully + // TODO actually parse JSON + + fmt.Printf("DBG %#v\n", lol) - // is the --push redundant? like, it needs to be specified every time for scripted use. can i tell if a branch is up to date with remote without a push? i could figure that out i think and then would know a push wasn't needed (but it will require a fetch per invocation. that seems fine?) return nil } diff --git a/pkg/cmd/ruleset/ruleset.go b/pkg/cmd/ruleset/ruleset.go index 2fa86684c..266fbc93e 100644 --- a/pkg/cmd/ruleset/ruleset.go +++ b/pkg/cmd/ruleset/ruleset.go @@ -2,6 +2,7 @@ package ruleset import ( "github.com/MakeNowJust/heredoc" + cmdCheck "github.com/cli/cli/v2/pkg/cmd/ruleset/check" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -20,7 +21,7 @@ func NewCmdRuleset(f *cmdutil.Factory) *cobra.Command { cmdutil.EnableRepoOverride(cmd, f) // cmd.AddCommand(cmdList.NewCmdList(f, nil) // cmd.AddCommand(cmdList.NewCmdView(f, nil) - // cmd.AddCommand(cmdCheck.NewCmdCheck(f, nil) + cmd.AddCommand(cmdCheck.NewCmdCheck(f, nil)) return cmd } From 79113bd304eb9c8e14a4771011f4d3709eedc1be Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 8 Feb 2023 18:30:04 -0800 Subject: [PATCH 03/25] start writing up structs for rules API payloads --- pkg/cmd/ruleset/rules.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 pkg/cmd/ruleset/rules.go diff --git a/pkg/cmd/ruleset/rules.go b/pkg/cmd/ruleset/rules.go new file mode 100644 index 000000000..b3405d839 --- /dev/null +++ b/pkg/cmd/ruleset/rules.go @@ -0,0 +1,39 @@ +package ruleset + +type RuleType string +type Enforcement string +type MatchingOperator string + +const ( + RuleTypeCommitAuthorEmailPattern RuleType = "commit_author_email_pattern" + RuleTypePullRequest RuleType = "pull_request" + // TODO others + + EnforcementEnabled Enforcement = "enabled" + EnforcementDisabled Enforcement = "disabled" + + MatchingOperatorStartsWith MatchingOperator = "starts_with" + MatchingOperatorEndsWith MatchingOperator = "ends_with" + MatchingOperatorContains MatchingOperator = "contains" + MatchingOperatorRegex MatchingOperator = "regex" +) + +type ConfigurationPullRequest struct { + DissmissStaleReviewsOnPush bool + RequireCodeOwnerReview bool + RequestLastPushApproval bool + RequiredApprovingReviewCount int +} + +type BranchNamePattern struct { + Name string + Negate bool + Operator MatchingOperator +} + +type Rule struct { + ID string + Type RuleType + Enforcement Enforcement + Configuration interface{} +} From 24911ffa240796332424b743dd6495be5fbc08fa Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 14 Feb 2023 15:20:10 -0800 Subject: [PATCH 04/25] start on rs list --- pkg/cmd/ruleset/list/list.go | 56 ++++++++++++++++++++++++++++++++++++ pkg/cmd/ruleset/ruleset.go | 5 ++-- 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/ruleset/list/list.go diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go new file mode 100644 index 000000000..ca0505f0b --- /dev/null +++ b/pkg/cmd/ruleset/list/list.go @@ -0,0 +1,56 @@ +package list + +import ( + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type ListOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + Organization string +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + } + cmd := &cobra.Command{ + Use: "list", + Short: "List rulesets for a repository or organization", + Long: heredoc.Doc(` + TODO + `), + Example: "TODO", + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if runF != nil { + return runF(opts) + } + + return listRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rules") + + return cmd +} + +func listRun(opts *ListOptions) error { + fmt.Println(opts.Organization) + fmt.Println("LOL TODO") + return nil +} diff --git a/pkg/cmd/ruleset/ruleset.go b/pkg/cmd/ruleset/ruleset.go index 266fbc93e..f7d7799b6 100644 --- a/pkg/cmd/ruleset/ruleset.go +++ b/pkg/cmd/ruleset/ruleset.go @@ -3,6 +3,7 @@ package ruleset import ( "github.com/MakeNowJust/heredoc" cmdCheck "github.com/cli/cli/v2/pkg/cmd/ruleset/check" + cmdList "github.com/cli/cli/v2/pkg/cmd/ruleset/list" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -19,8 +20,8 @@ func NewCmdRuleset(f *cmdutil.Factory) *cobra.Command { } cmdutil.EnableRepoOverride(cmd, f) - // cmd.AddCommand(cmdList.NewCmdList(f, nil) - // cmd.AddCommand(cmdList.NewCmdView(f, nil) + cmd.AddCommand(cmdList.NewCmdList(f, nil)) + // cmd.AddCommand(cmdList.NewCmdView(f, nil)) cmd.AddCommand(cmdCheck.NewCmdCheck(f, nil)) return cmd From a742e9f8dfc05a1bec0faa55b42f4c0cf222019d Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 15 Feb 2023 12:51:06 -0800 Subject: [PATCH 05/25] wip --- pkg/cmd/ruleset/list/list.go | 45 ++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index ca0505f0b..251b887b6 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -5,6 +5,8 @@ import ( "net/http" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -14,6 +16,7 @@ import ( type ListOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams + Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) Organization string @@ -23,6 +26,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts := &ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Config: f.Config, } cmd := &cobra.Command{ Use: "list", @@ -49,8 +53,45 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return cmd } +type Ruleset struct { + // TODO +} + func listRun(opts *ListOptions) error { - fmt.Println(opts.Organization) - fmt.Println("LOL TODO") + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + client := api.NewClientFromHTTP(httpClient) + + repoI, err := opts.BaseRepo() + if err != nil { + return err + } + + cfg, err := opts.Config() + if err != nil { + return err + } + + endpoint := fmt.Sprintf("repos/%s/%s/rulesets", + repoI.RepoOwner(), repoI.RepoName()) + hostname := repoI.RepoHost() + + if opts.Organization != "" { + endpoint = fmt.Sprintf("orgs/%s/rulesets", opts.Organization) + hostname, _ = cfg.DefaultHost() + } + + //var response []Ruleset + var response interface{} + + err = client.REST(hostname, "GET", endpoint, nil, &response) + if err != nil { + return fmt.Errorf("failed to call '%s': %w", endpoint, err) + } + + fmt.Printf("DBG %#v\n", response) + return nil } From bb9a9fa3c2c598145c64714cf03160486e21b360 Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 7 Apr 2023 17:14:03 -0400 Subject: [PATCH 06/25] GQL query works but it's messy --- api/client.go | 2 +- pkg/cmd/ruleset/list/http.go | 94 ++++++++++++++++++++++++++++++++++++ pkg/cmd/ruleset/list/list.go | 64 ++++++++++++++++-------- 3 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 pkg/cmd/ruleset/list/http.go diff --git a/api/client.go b/api/client.go index e32856554..0d2690378 100644 --- a/api/client.go +++ b/api/client.go @@ -19,7 +19,7 @@ const ( authorization = "Authorization" cacheTTL = "X-GH-CACHE-TTL" graphqlFeatures = "GraphQL-Features" - features = "merge_queue" + features = "merge_queue,push_policies" userAgent = "User-Agent" ) diff --git a/pkg/cmd/ruleset/list/http.go b/pkg/cmd/ruleset/list/http.go new file mode 100644 index 000000000..aa58a8a70 --- /dev/null +++ b/pkg/cmd/ruleset/list/http.go @@ -0,0 +1,94 @@ +package list + +import ( + "net/http" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" +) + +type RepositoryRuleset struct { + Id string + Name string + Target string + // Enforcement string + Conditions struct { + RefName struct { + Include []string + Exclude []string + } + RepositoryName struct { + Include []string + Exclude []string + } + } +} + +type RepositoryRulesetList struct { + TotalCount int + Rulesets []RepositoryRuleset +} + +func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) (*RepositoryRulesetList, error) { + type response struct { + Repository struct { + Rulesets struct { + TotalCount int + Nodes []RepositoryRuleset + } + } + } + + query := ` + query RepositoryRulesetList( + $owner: String!, + $repo: String!, + $limit: Int! + ) { + repository(owner: $owner, name: $repo) { + rulesets(first: $limit) { + totalCount + nodes { + id + name + target + conditions { + refName { + include + exclude + } + repositoryName { + include + exclude + protected + } + }, + rules { + totalCount + } + } + } + } + } + ` + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), + "limit": limit, + } + + client := api.NewClientFromHTTP(httpClient) + var res response + err := client.GraphQL(repo.RepoHost(), query, variables, &res) + if err != nil { + return nil, err + } + + list := RepositoryRulesetList{ + TotalCount: res.Repository.Rulesets.TotalCount, + Rulesets: res.Repository.Rulesets.Nodes, + } + + return &list, nil +} diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 251b887b6..c7bffd94b 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -3,11 +3,14 @@ package list import ( "fmt" "net/http" + "strings" "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/tableprinter" + "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -18,7 +21,10 @@ type ListOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + Limit int + WebMode bool Organization string } @@ -40,6 +46,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + // if opts.Limit < 1 { + // return cmdutil.FlagErrorf("invalid value for --limit: %v", opts.Limit) + // } + if runF != nil { return runF(opts) } @@ -48,13 +58,21 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman }, } + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of rules to list") cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rules") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List rules in the web browser") return cmd } type Ruleset struct { - // TODO + Id string `json:"id"` + Name string `json:"name"` + Target string `json:"target"` + SourceType string `json:"source_type"` + Source string `json:"source"` + Enforcement string `json:"enforcement"` + BypassMode string `json:"bypass_mode"` } func listRun(opts *ListOptions) error { @@ -62,36 +80,40 @@ func listRun(opts *ListOptions) error { if err != nil { return err } - client := api.NewClientFromHTTP(httpClient) repoI, err := opts.BaseRepo() if err != nil { return err } - cfg, err := opts.Config() + if opts.WebMode { + rulesetURL := ghrepo.GenerateRepoURL(repoI, "settings/rules") + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) + } + + return opts.Browser.Browse(rulesetURL) + } + + result, err := listRepoRulesets(httpClient, repoI, opts.Limit) if err != nil { return err } - endpoint := fmt.Sprintf("repos/%s/%s/rulesets", - repoI.RepoOwner(), repoI.RepoName()) - hostname := repoI.RepoHost() + cs := opts.IO.ColorScheme() - if opts.Organization != "" { - endpoint = fmt.Sprintf("orgs/%s/rulesets", opts.Organization) - hostname, _ = cfg.DefaultHost() + tp := tableprinter.New(opts.IO) + tp.HeaderRow("ID", "NAME" /* "STATUS",*/, "TARGET") + + for _, rs := range result.Rulesets { + tp.AddField(rs.Id) + tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) + // tp.AddField(strings.ToLower(rs.Enforcement)) + tp.AddField(strings.ToLower(rs.Target)) + tp.EndRow() } - //var response []Ruleset - var response interface{} - - err = client.REST(hostname, "GET", endpoint, nil, &response) - if err != nil { - return fmt.Errorf("failed to call '%s': %w", endpoint, err) - } - - fmt.Printf("DBG %#v\n", response) - - return nil + return tp.Render() } + +// func getRulesets() From 7b2c8aba8c5ec9add1f9e01c48ada235fae628d7 Mon Sep 17 00:00:00 2001 From: vaindil Date: Mon, 10 Apr 2023 17:13:50 -0400 Subject: [PATCH 07/25] refactor and support org rulesets --- pkg/cmd/ruleset/list/http.go | 174 +++++++++++++++++++++++------------ pkg/cmd/ruleset/list/list.go | 25 ++--- 2 files changed, 128 insertions(+), 71 deletions(-) diff --git a/pkg/cmd/ruleset/list/http.go b/pkg/cmd/ruleset/list/http.go index aa58a8a70..22e218974 100644 --- a/pkg/cmd/ruleset/list/http.go +++ b/pkg/cmd/ruleset/list/http.go @@ -7,7 +7,20 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" ) -type RepositoryRuleset struct { +type RulesetResponse struct { + Level struct { + Rulesets struct { + TotalCount int + Nodes []Ruleset + PageInfo struct { + HasNextPage bool + EndCursor string + } + } + } +} + +type Ruleset struct { Id string Name string Target string @@ -24,71 +37,112 @@ type RepositoryRuleset struct { } } -type RepositoryRulesetList struct { +type RulesetList struct { TotalCount int - Rulesets []RepositoryRuleset + Rulesets []Ruleset } -func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) (*RepositoryRulesetList, error) { - type response struct { - Repository struct { - Rulesets struct { - TotalCount int - Nodes []RepositoryRuleset - } - } - } - - query := ` - query RepositoryRulesetList( - $owner: String!, - $repo: String!, - $limit: Int! - ) { - repository(owner: $owner, name: $repo) { - rulesets(first: $limit) { - totalCount - nodes { - id - name - target - conditions { - refName { - include - exclude - } - repositoryName { - include - exclude - protected - } - }, - rules { - totalCount - } - } - } - } - } - ` - +func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) (*RulesetList, error) { variables := map[string]interface{}{ "owner": repo.RepoOwner(), "repo": repo.RepoName(), - "limit": limit, } - client := api.NewClientFromHTTP(httpClient) - var res response - err := client.GraphQL(repo.RepoHost(), query, variables, &res) - if err != nil { - return nil, err - } - - list := RepositoryRulesetList{ - TotalCount: res.Repository.Rulesets.TotalCount, - Rulesets: res.Repository.Rulesets.Nodes, - } - - return &list, nil + return listRulesets(httpClient, rulesetQuery(false), variables, limit, repo.RepoHost()) +} + +func listOrgRulesets(httpClient *http.Client, orgLogin string, limit int, host string) (*RulesetList, error) { + variables := map[string]interface{}{ + "login": orgLogin, + } + + return listRulesets(httpClient, rulesetQuery(true), variables, limit, host) +} + +func listRulesets(httpClient *http.Client, query string, variables map[string]interface{}, limit int, host string) (*RulesetList, error) { + pageLimit := min(limit, 100) + + res := RulesetList{ + Rulesets: []Ruleset{}, + } + client := api.NewClientFromHTTP(httpClient) + + for { + variables["limit"] = pageLimit + var data RulesetResponse + err := client.GraphQL(host, query, variables, &data) + if err != nil { + return nil, err + } + + res.TotalCount = data.Level.Rulesets.TotalCount + res.Rulesets = append(res.Rulesets, data.Level.Rulesets.Nodes...) + + if len(res.Rulesets) >= limit { + break + } + + if data.Level.Rulesets.PageInfo.HasNextPage { + variables["endCursor"] = data.Level.Rulesets.PageInfo.EndCursor + pageLimit = min(pageLimit, limit-len(res.Rulesets)) + } else { + break + } + } + + return &res, nil +} + +func rulesetQuery(org bool) string { + var args string + var level string + + if org { + args = "$login: String!" + level = "organization(login: $login)" + } else { + args = "$owner: String!, $repo: String!" + level = "repository(owner: $owner, name: $repo)" + } + + str := "query RulesetList($limit: Int!, $endCursor: String, " + args + ") { level: " + level + " {" + + str += ` + rulesets(first: $limit, after: $endCursor) { + totalCount + nodes { + id + #database_id + name + target + #enforcement + conditions { + refName { + include + exclude + } + repositoryName { + include + exclude + protected + } + } + rules { + totalCount + } + } + pageInfo { + hasNextPage + endCursor + } + }` + + return str + "}}" +} + +func min(a, b int) int { + if a < b { + return a + } + return b } diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index c7bffd94b..5d5f87ace 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -65,16 +65,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman return cmd } -type Ruleset struct { - Id string `json:"id"` - Name string `json:"name"` - Target string `json:"target"` - SourceType string `json:"source_type"` - Source string `json:"source"` - Enforcement string `json:"enforcement"` - BypassMode string `json:"bypass_mode"` -} - func listRun(opts *ListOptions) error { httpClient, err := opts.HttpClient() if err != nil { @@ -95,7 +85,20 @@ func listRun(opts *ListOptions) error { return opts.Browser.Browse(rulesetURL) } - result, err := listRepoRulesets(httpClient, repoI, opts.Limit) + var result *RulesetList + + if opts.Organization != "" { + var cfg config.Config + cfg, err = opts.Config() + if err != nil { + return err + } + hostname, _ := cfg.DefaultHost() + result, err = listOrgRulesets(httpClient, opts.Organization, opts.Limit, hostname) + } else { + result, err = listRepoRulesets(httpClient, repoI, opts.Limit) + } + if err != nil { return err } From 5155844d7f302d88359afa52961d1f46c5fb3303 Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 14 Apr 2023 17:07:02 -0400 Subject: [PATCH 08/25] updates, initial list test --- .../ruleset/list/fixtures/rulesetList.json | 69 ++++++ pkg/cmd/ruleset/list/http.go | 15 +- pkg/cmd/ruleset/list/list.go | 58 +++-- pkg/cmd/ruleset/list/list_test.go | 199 ++++++++++++++++++ 4 files changed, 320 insertions(+), 21 deletions(-) create mode 100644 pkg/cmd/ruleset/list/fixtures/rulesetList.json create mode 100644 pkg/cmd/ruleset/list/list_test.go diff --git a/pkg/cmd/ruleset/list/fixtures/rulesetList.json b/pkg/cmd/ruleset/list/fixtures/rulesetList.json new file mode 100644 index 000000000..560c9fb4c --- /dev/null +++ b/pkg/cmd/ruleset/list/fixtures/rulesetList.json @@ -0,0 +1,69 @@ +{ + "data": { + "level": { + "rulesets": { + "totalCount": 3, + "nodes": [ + { + "databaseId": 4, + "name": "test", + "target": "BRANCH", + "enforcement": "EVALUATE", + "conditions": { + "refName": { + "include": [ + "~DEFAULT_BRANCH" + ], + "exclude": [] + }, + "repositoryName": null + }, + "rules": { + "totalCount": 1 + } + }, + { + "databaseId": 42, + "name": "asdf", + "target": "BRANCH", + "enforcement": "ACTIVE", + "conditions": { + "refName": { + "include": [ + "~DEFAULT_BRANCH" + ], + "exclude": [] + }, + "repositoryName": null + }, + "rules": { + "totalCount": 2 + } + }, + { + "databaseId": 77, + "name": "foobar", + "target": "BRANCH", + "enforcement": "DISABLED", + "conditions": { + "refName": { + "include": [ + "~DEFAULT_BRANCH" + ], + "exclude": [] + }, + "repositoryName": null + }, + "rules": { + "totalCount": 4 + } + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "Y3Vyc29yOnYyOpHNA8E=" + } + } + } + } +} diff --git a/pkg/cmd/ruleset/list/http.go b/pkg/cmd/ruleset/list/http.go index 22e218974..10be1f57d 100644 --- a/pkg/cmd/ruleset/list/http.go +++ b/pkg/cmd/ruleset/list/http.go @@ -21,11 +21,11 @@ type RulesetResponse struct { } type Ruleset struct { - Id string - Name string - Target string - // Enforcement string - Conditions struct { + DatabaseId int + Name string + Target string + Enforcement string + Conditions struct { RefName struct { Include []string Exclude []string @@ -111,11 +111,10 @@ func rulesetQuery(org bool) string { rulesets(first: $limit, after: $endCursor) { totalCount nodes { - id - #database_id + databaseId name target - #enforcement + enforcement conditions { refName { include diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 5d5f87ace..4b30fabe2 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -3,11 +3,13 @@ package list import ( "fmt" "net/http" + "strconv" "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" @@ -32,6 +34,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts := &ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Browser: f.Browser, Config: f.Config, } cmd := &cobra.Command{ @@ -46,9 +49,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - // if opts.Limit < 1 { - // return cmdutil.FlagErrorf("invalid value for --limit: %v", opts.Limit) - // } + if opts.Limit < 1 { + return cmdutil.FlagErrorf("invalid value for --limit: %v", opts.Limit) + } if runF != nil { return runF(opts) @@ -76,8 +79,21 @@ func listRun(opts *ListOptions) error { return err } + cfg, err := opts.Config() + if err != nil { + return err + } + + hostname, _ := cfg.DefaultHost() + if opts.WebMode { - rulesetURL := ghrepo.GenerateRepoURL(repoI, "settings/rules") + var rulesetURL string + if opts.Organization != "" { + rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules", ghinstance.HostPrefix(hostname), opts.Organization) + } else { + rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules") + } + if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) } @@ -88,12 +104,6 @@ func listRun(opts *ListOptions) error { var result *RulesetList if opts.Organization != "" { - var cfg config.Config - cfg, err = opts.Config() - if err != nil { - return err - } - hostname, _ := cfg.DefaultHost() result, err = listOrgRulesets(httpClient, opts.Organization, opts.Limit, hostname) } else { result, err = listRepoRulesets(httpClient, repoI, opts.Limit) @@ -103,15 +113,37 @@ func listRun(opts *ListOptions) error { return err } + var entityName string + if opts.Organization != "" { + entityName = opts.Organization + } else { + entityName = ghrepo.FullName(repoI) + } + + if result.TotalCount == 0 { + msg := fmt.Sprintf("no rulesets found in %s", entityName) + return cmdutil.NewNoResultsError(msg) + } + + if err := opts.IO.StartPager(); err == nil { + defer opts.IO.StopPager() + } else { + fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err) + } + cs := opts.IO.ColorScheme() + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "\nShowing %d of %d rulesets in %s\n\n", len(result.Rulesets), result.TotalCount, entityName) + } + tp := tableprinter.New(opts.IO) - tp.HeaderRow("ID", "NAME" /* "STATUS",*/, "TARGET") + tp.HeaderRow("ID", "NAME", "STATUS", "TARGET") for _, rs := range result.Rulesets { - tp.AddField(rs.Id) + tp.AddField(strconv.Itoa(rs.DatabaseId)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) - // tp.AddField(strings.ToLower(rs.Enforcement)) + tp.AddField(strings.ToLower(rs.Enforcement)) tp.AddField(strings.ToLower(rs.Target)) tp.EndRow() } diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go new file mode 100644 index 000000000..0a44b5438 --- /dev/null +++ b/pkg/cmd/ruleset/list/list_test.go @@ -0,0 +1,199 @@ +package list + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_NewCmdList(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want ListOptions + wantErr string + }{ + { + name: "no arguments", + args: "", + isTTY: true, + want: ListOptions{ + Limit: 30, + WebMode: false, + Organization: "", + }, + }, + { + name: "limit", + args: "--limit 1", + isTTY: true, + want: ListOptions{ + Limit: 1, + WebMode: false, + Organization: "", + }, + }, + { + name: "org", + args: "--org \"my-org\"", + isTTY: true, + want: ListOptions{ + Limit: 30, + WebMode: false, + Organization: "my-org", + }, + }, + { + name: "web mode", + args: "--web", + isTTY: true, + want: ListOptions{ + Limit: 30, + WebMode: true, + Organization: "", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: ios, + } + + var opts *ListOptions + cmd := NewCmdList(f, func(o *ListOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.Limit, opts.Limit) + assert.Equal(t, tt.want.WebMode, opts.WebMode) + assert.Equal(t, tt.want.Organization, opts.Organization) + }) + } +} + +func Test_listRun(t *testing.T) { + tests := []struct { + name string + isTTY bool + opts ListOptions + wantErr string + wantStdout string + wantStderr string + }{ + { + name: "list repo rulesets", + isTTY: true, + wantStdout: heredoc.Doc(` + + Showing 3 of 3 rulesets in OWNER/REPO + + ID NAME STATUS TARGET + 4 test evaluate branch + 42 asdf active branch + 77 foobar disabled branch + `), + wantStderr: "", + }, + { + name: "list org rulesets", + isTTY: true, + opts: ListOptions{ + Organization: "my-org", + }, + wantStdout: heredoc.Doc(` + + Showing 3 of 3 rulesets in my-org + + ID NAME STATUS TARGET + 4 test evaluate branch + 42 asdf active branch + 77 foobar disabled branch + `), + wantStderr: "", + }, + { + name: "machine-readable", + isTTY: false, + wantStdout: heredoc.Doc(` + 4 test evaluate branch + 42 asdf active branch + 77 foobar disabled branch + `), + wantStderr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + fakeHTTP := &httpmock.Registry{} + fakeHTTP.Register(httpmock.GraphQL(`query RulesetList\b`), httpmock.FileResponse("./fixtures/rulesetList.json")) + + tt.opts.IO = ios + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: fakeHTTP}, nil + } + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + } + tt.opts.Browser = &browser.Stub{} + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + err := listRun(&tt.opts) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} From 7f81645c785d7eaedd298fe5916b8a1ecb665542 Mon Sep 17 00:00:00 2001 From: vaindil Date: Tue, 25 Apr 2023 16:42:36 -0400 Subject: [PATCH 09/25] basic ruleset view works --- pkg/cmd/ruleset/list/http.go | 66 ++++----- pkg/cmd/ruleset/list/list.go | 9 +- pkg/cmd/ruleset/ruleset.go | 3 +- pkg/cmd/ruleset/shared/shared.go | 28 ++++ pkg/cmd/ruleset/view/http.go | 32 +++++ pkg/cmd/ruleset/view/view.go | 227 +++++++++++++++++++++++++++++++ 6 files changed, 319 insertions(+), 46 deletions(-) create mode 100644 pkg/cmd/ruleset/shared/shared.go create mode 100644 pkg/cmd/ruleset/view/http.go create mode 100644 pkg/cmd/ruleset/view/view.go diff --git a/pkg/cmd/ruleset/list/http.go b/pkg/cmd/ruleset/list/http.go index 10be1f57d..592bdeda5 100644 --- a/pkg/cmd/ruleset/list/http.go +++ b/pkg/cmd/ruleset/list/http.go @@ -1,17 +1,19 @@ package list import ( + "fmt" "net/http" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" ) type RulesetResponse struct { Level struct { Rulesets struct { TotalCount int - Nodes []Ruleset + Nodes []shared.Ruleset PageInfo struct { HasNextPage bool EndCursor string @@ -20,26 +22,9 @@ type RulesetResponse struct { } } -type Ruleset struct { - DatabaseId int - Name string - Target string - Enforcement string - Conditions struct { - RefName struct { - Include []string - Exclude []string - } - RepositoryName struct { - Include []string - Exclude []string - } - } -} - type RulesetList struct { TotalCount int - Rulesets []Ruleset + Rulesets []shared.Ruleset } func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) (*RulesetList, error) { @@ -48,7 +33,7 @@ func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) "repo": repo.RepoName(), } - return listRulesets(httpClient, rulesetQuery(false), variables, limit, repo.RepoHost()) + return listRulesets(httpClient, rulesetsQuery(false), variables, limit, repo.RepoHost()) } func listOrgRulesets(httpClient *http.Client, orgLogin string, limit int, host string) (*RulesetList, error) { @@ -56,14 +41,14 @@ func listOrgRulesets(httpClient *http.Client, orgLogin string, limit int, host s "login": orgLogin, } - return listRulesets(httpClient, rulesetQuery(true), variables, limit, host) + return listRulesets(httpClient, rulesetsQuery(true), variables, limit, host) } func listRulesets(httpClient *http.Client, query string, variables map[string]interface{}, limit int, host string) (*RulesetList, error) { pageLimit := min(limit, 100) res := RulesetList{ - Rulesets: []Ruleset{}, + Rulesets: []shared.Ruleset{}, } client := api.NewClientFromHTTP(httpClient) @@ -93,7 +78,7 @@ func listRulesets(httpClient *http.Client, query string, variables map[string]in return &res, nil } -func rulesetQuery(org bool) string { +func rulesetsQuery(org bool) string { var args string var level string @@ -105,28 +90,28 @@ func rulesetQuery(org bool) string { level = "repository(owner: $owner, name: $repo)" } - str := "query RulesetList($limit: Int!, $endCursor: String, " + args + ") { level: " + level + " {" + str := fmt.Sprintf("query RulesetList($limit: Int!, $endCursor: String, %s) { level: %s {", args, level) - str += ` + return str + ` rulesets(first: $limit, after: $endCursor) { totalCount nodes { - databaseId + id: databaseId name target enforcement - conditions { - refName { - include - exclude - } - repositoryName { - include - exclude - protected - } - } - rules { + # conditions { + # refName { + # include + # exclude + # } + # repositoryName { + # include + # exclude + # protected + # } + # } + rulesGql: rules { totalCount } } @@ -134,9 +119,8 @@ func rulesetQuery(org bool) string { hasNextPage endCursor } - }` - - return str + "}}" + } + }}` } func min(a, b int) int { diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 4b30fabe2..2c9835124 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -61,9 +61,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman }, } - cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of rules to list") - cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rules") - cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List rules in the web browser") + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of rulesets to list") + cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rulesets") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List rulesets in the web browser") return cmd } @@ -125,6 +125,7 @@ func listRun(opts *ListOptions) error { return cmdutil.NewNoResultsError(msg) } + opts.IO.DetectTerminalTheme() if err := opts.IO.StartPager(); err == nil { defer opts.IO.StopPager() } else { @@ -141,7 +142,7 @@ func listRun(opts *ListOptions) error { tp.HeaderRow("ID", "NAME", "STATUS", "TARGET") for _, rs := range result.Rulesets { - tp.AddField(strconv.Itoa(rs.DatabaseId)) + tp.AddField(strconv.Itoa(rs.Id)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) tp.AddField(strings.ToLower(rs.Enforcement)) tp.AddField(strings.ToLower(rs.Target)) diff --git a/pkg/cmd/ruleset/ruleset.go b/pkg/cmd/ruleset/ruleset.go index f7d7799b6..64536e868 100644 --- a/pkg/cmd/ruleset/ruleset.go +++ b/pkg/cmd/ruleset/ruleset.go @@ -4,6 +4,7 @@ import ( "github.com/MakeNowJust/heredoc" cmdCheck "github.com/cli/cli/v2/pkg/cmd/ruleset/check" cmdList "github.com/cli/cli/v2/pkg/cmd/ruleset/list" + cmdView "github.com/cli/cli/v2/pkg/cmd/ruleset/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -21,7 +22,7 @@ func NewCmdRuleset(f *cmdutil.Factory) *cobra.Command { cmdutil.EnableRepoOverride(cmd, f) cmd.AddCommand(cmdList.NewCmdList(f, nil)) - // cmd.AddCommand(cmdList.NewCmdView(f, nil)) + cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdCheck.NewCmdCheck(f, nil)) return cmd diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go new file mode 100644 index 000000000..d226ad18c --- /dev/null +++ b/pkg/cmd/ruleset/shared/shared.go @@ -0,0 +1,28 @@ +package shared + +type Ruleset struct { + Id int + Name string + Target string + Enforcement string + BypassMode string `json:"bypass_mode"` + BypassActors []struct { + ActorId int `json:"actor_id"` + ActorType string `json:"actor_type"` + } `json:"bypass_actors"` + Conditions map[string]map[string]interface { + // RefName struct { + // Include []string + // Exclude []string + // } `json:"ref_name"` + // RepositoryName struct { + // Include []string + // Exclude []string + // Protected bool + // } `json:"repository_name"` + } + RulesGql struct { + TotalCount int + } + Rules []interface{} +} diff --git a/pkg/cmd/ruleset/view/http.go b/pkg/cmd/ruleset/view/http.go new file mode 100644 index 000000000..bd56c3cf6 --- /dev/null +++ b/pkg/cmd/ruleset/view/http.go @@ -0,0 +1,32 @@ +package view + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" +) + +func viewRepoRuleset(httpClient *http.Client, repo ghrepo.Interface, databaseId string) (*shared.Ruleset, error) { + path := fmt.Sprintf("repos/%s/%s/rulesets/%s", repo.RepoOwner(), repo.RepoName(), databaseId) + return viewRuleset(httpClient, repo.RepoHost(), path) +} + +func viewOrgRuleset(httpClient *http.Client, orgLogin string, databaseId string, host string) (*shared.Ruleset, error) { + path := fmt.Sprintf("orgs/%s/rulesets/%s", orgLogin, databaseId) + return viewRuleset(httpClient, host, path) +} + +func viewRuleset(httpClient *http.Client, hostname string, path string) (*shared.Ruleset, error) { + apiClient := api.NewClientFromHTTP(httpClient) + result := shared.Ruleset{} + + err := apiClient.REST(hostname, "GET", path, nil, &result) + if err != nil { + return nil, err + } + + return &result, nil +} diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go new file mode 100644 index 000000000..49c74581c --- /dev/null +++ b/pkg/cmd/ruleset/view/view.go @@ -0,0 +1,227 @@ +package view + +import ( + "fmt" + "net/http" + "reflect" + "sort" + "strconv" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type ViewOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + Config func() (config.Config, error) + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + + ID string + WebMode bool + Organization string +} + +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Browser: f.Browser, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "view []", + Short: "View information about a ruleset", + Long: heredoc.Doc(` + View information about a GitHub ruleset. + + If no ID is provided, an interactive prompt will be used to choose + the ruleset to view. + `), + Example: heredoc.Doc(` + # View a ruleset in the current repository + $ gh ruleset view 43 + + # View a ruleset in a different repository + $ gh ruleset view 23 --repo owner/repo + + # View an organization-level ruleset + $ gh ruleset view 23 --org my-org + `), + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if len(args) > 0 { + // a string is actually needed later on, so verify that it's numeric + // but use the string anyway + _, err := strconv.Atoi(args[0]) + if err != nil { + return cmdutil.FlagErrorf("invalid value for ruleset ID: %v is not an integer", args[0]) + } + opts.ID = args[0] + } + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the ruleset in the browser") + cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "Organization name if the ID ") + + return cmd +} + +func viewRun(opts *ViewOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + repoI, err := opts.BaseRepo() + if err != nil { + return err + } + + cfg, err := opts.Config() + if err != nil { + return err + } + + hostname, _ := cfg.DefaultHost() + + if opts.WebMode { + // TODO need to validate ruleset's existence before opening + var rulesetURL string + if opts.Organization != "" { + rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules/%s", ghinstance.HostPrefix(hostname), opts.Organization, opts.ID) + } else { + rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules/%s", opts.ID) + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) + } + + return opts.Browser.Browse(rulesetURL) + } + + var rs *shared.Ruleset + if opts.Organization != "" { + rs, err = viewOrgRuleset(httpClient, opts.Organization, opts.ID, hostname) + } else { + rs, err = viewRepoRuleset(httpClient, repoI, opts.ID) + } + + if err != nil { + return err + } + + cs := opts.IO.ColorScheme() + w := opts.IO.Out + + fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) + fmt.Fprintf(w, "ID: %d\n", rs.Id) + + switch rs.Enforcement { + case "disabled": + fmt.Fprintf(w, "%s\n", cs.Red("Disabled")) + case "evaluate": + fmt.Fprintf(w, "%s\n", cs.Yellow("Evaluate Mode (not being enforced)")) + case "active": + fmt.Fprintf(w, "%s\n", cs.Green("Active")) + default: + fmt.Fprintf(w, "Enforcement: %s\n", rs.Enforcement) + } + + fmt.Fprintf(w, "\n%s\n", cs.Bold("Bypassing")) + fmt.Fprintf(w, "Mode: %s\n", rs.BypassMode) + if len(rs.BypassActors) == 0 { + fmt.Fprintf(w, "No actors configured for bypass\n") + } else { + types := make(map[string]int) + for _, t := range rs.BypassActors { + val, exists := types[t.ActorType] + if exists { + types[t.ActorType] = val + 1 + } else { + types[t.ActorType] = 1 + } + } + + fmt.Fprintf(w, "Actor types allowed to bypass:\n") + for name, count := range types { + fmt.Fprintf(w, "- %s: %d actors\n", name, count) + } + } + + fmt.Fprintf(w, "\n%s\n", cs.Bold("Conditions")) + if len(rs.Conditions) == 0 { + fmt.Fprintf(w, "No conditions configured\n") + } else { + // sort keys for consistent responses, mismatched types don't allow this to be broken + // into a separate function + keys := make([]string, 0, len(rs.Conditions)) + for key := range rs.Conditions { + keys = append(keys, key) + } + sort.Strings(keys) + + for _, name := range keys { + condition := rs.Conditions[name] + fmt.Fprintf(w, "- %s: ", name) + + // sort these keys too for consistency + subkeys := make([]string, 0, len(condition)) + for subkey := range condition { + subkeys = append(subkeys, subkey) + } + sort.Strings(subkeys) + + for _, n := range subkeys { + rawVal := condition[n] + + k := reflect.TypeOf(rawVal).Kind() + if rawVal == nil || + ((k == reflect.Slice || k == reflect.Map) && len(rawVal.([]interface{})) == 0) { + continue + } + + printVal := fmt.Sprint(rawVal) + + // fmt.Fprintf(w, "n: %s, type: %s\n", n, reflect.TypeOf(rawVal).String()) + + // switch val := rawVal.(type) { + // case []interface{}: + // // currently only string arrays are returned by the API at this level + // printVal = fmt.Sprint(val) + // default: + // printVal = fmt.Sprint(val) + // } + + fmt.Fprintf(w, "[%s: %s] ", n, printVal) + } + + fmt.Fprint(w, "\n") + } + } + + fmt.Fprintf(w, "\n%s\n", cs.Bold("Rules")) + fmt.Fprintf(w, "%d configured\n", reflect.ValueOf(rs.Rules).Len()) + + return nil +} From 3e6419237d67a6d70e508bd51b18fee5a3c60b2d Mon Sep 17 00:00:00 2001 From: vaindil Date: Mon, 1 May 2023 14:13:15 -0400 Subject: [PATCH 10/25] allow getting parents, move list to shared package --- pkg/cmd/ruleset/list/list.go | 39 +++++++++++++------- pkg/cmd/ruleset/{list => shared}/http.go | 29 ++++++++------- pkg/cmd/ruleset/shared/shared.go | 4 +++ pkg/cmd/ruleset/view/view.go | 45 +++++++++++++----------- 4 files changed, 72 insertions(+), 45 deletions(-) rename pkg/cmd/ruleset/{list => shared}/http.go (73%) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 2c9835124..e49710ba8 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -13,6 +13,7 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -25,9 +26,10 @@ type ListOptions struct { BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser - Limit int - WebMode bool - Organization string + Limit int + IncludeParents bool + WebMode bool + Organization string } func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { @@ -62,8 +64,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of rulesets to list") - cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rulesets") - cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List rulesets in the web browser") + cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rulesets for the provided organization") + cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", false, "Include rulesets configured at higher levels that also apply") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the list of rulesets in the web browser") return cmd } @@ -101,12 +104,12 @@ func listRun(opts *ListOptions) error { return opts.Browser.Browse(rulesetURL) } - var result *RulesetList + var result *shared.RulesetList if opts.Organization != "" { - result, err = listOrgRulesets(httpClient, opts.Organization, opts.Limit, hostname) + result, err = shared.ListOrgRulesets(httpClient, opts.Organization, opts.Limit, hostname, opts.IncludeParents) } else { - result, err = listRepoRulesets(httpClient, repoI, opts.Limit) + result, err = shared.ListRepoRulesets(httpClient, repoI, opts.Limit, opts.IncludeParents) } if err != nil { @@ -121,7 +124,11 @@ func listRun(opts *ListOptions) error { } if result.TotalCount == 0 { - msg := fmt.Sprintf("no rulesets found in %s", entityName) + parentsMsg := "" + if opts.IncludeParents { + parentsMsg = " or its parents" + } + msg := fmt.Sprintf("no rulesets found in %s%s", entityName, parentsMsg) return cmdutil.NewNoResultsError(msg) } @@ -139,17 +146,25 @@ func listRun(opts *ListOptions) error { } tp := tableprinter.New(opts.IO) - tp.HeaderRow("ID", "NAME", "STATUS", "TARGET") + tp.HeaderRow("ID", "NAME", "SOURCE", "STATUS", "TARGET", "RULES") for _, rs := range result.Rulesets { tp.AddField(strconv.Itoa(rs.Id)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) + var ownerString string + if rs.Source.RepoOwner != "" { + ownerString = fmt.Sprintf("%s (repo)", rs.Source.RepoOwner) + } else if rs.Source.OrgOwner != "" { + ownerString = fmt.Sprintf("%s (org)", rs.Source.OrgOwner) + } else { + ownerString = "(unknown)" + } + tp.AddField(ownerString) tp.AddField(strings.ToLower(rs.Enforcement)) tp.AddField(strings.ToLower(rs.Target)) + tp.AddField(strconv.Itoa(rs.RulesGql.TotalCount)) tp.EndRow() } return tp.Render() } - -// func getRulesets() diff --git a/pkg/cmd/ruleset/list/http.go b/pkg/cmd/ruleset/shared/http.go similarity index 73% rename from pkg/cmd/ruleset/list/http.go rename to pkg/cmd/ruleset/shared/http.go index 592bdeda5..9dae0d7e6 100644 --- a/pkg/cmd/ruleset/list/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -1,4 +1,4 @@ -package list +package shared import ( "fmt" @@ -6,14 +6,13 @@ import ( "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" ) type RulesetResponse struct { Level struct { Rulesets struct { TotalCount int - Nodes []shared.Ruleset + Nodes []Ruleset PageInfo struct { HasNextPage bool EndCursor string @@ -24,21 +23,23 @@ type RulesetResponse struct { type RulesetList struct { TotalCount int - Rulesets []shared.Ruleset + Rulesets []Ruleset } -func listRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int) (*RulesetList, error) { +func ListRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int, includeParents bool) (*RulesetList, error) { variables := map[string]interface{}{ - "owner": repo.RepoOwner(), - "repo": repo.RepoName(), + "owner": repo.RepoOwner(), + "repo": repo.RepoName(), + "includeParents": includeParents, } return listRulesets(httpClient, rulesetsQuery(false), variables, limit, repo.RepoHost()) } -func listOrgRulesets(httpClient *http.Client, orgLogin string, limit int, host string) (*RulesetList, error) { +func ListOrgRulesets(httpClient *http.Client, orgLogin string, limit int, host string, includeParents bool) (*RulesetList, error) { variables := map[string]interface{}{ - "login": orgLogin, + "login": orgLogin, + "includeParents": includeParents, } return listRulesets(httpClient, rulesetsQuery(true), variables, limit, host) @@ -48,7 +49,7 @@ func listRulesets(httpClient *http.Client, query string, variables map[string]in pageLimit := min(limit, 100) res := RulesetList{ - Rulesets: []shared.Ruleset{}, + Rulesets: []Ruleset{}, } client := api.NewClientFromHTTP(httpClient) @@ -90,16 +91,20 @@ func rulesetsQuery(org bool) string { level = "repository(owner: $owner, name: $repo)" } - str := fmt.Sprintf("query RulesetList($limit: Int!, $endCursor: String, %s) { level: %s {", args, level) + str := fmt.Sprintf("query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, %s) { level: %s {", args, level) return str + ` - rulesets(first: $limit, after: $endCursor) { + rulesets(first: $limit, after: $endCursor, includeParents: $includeParents) { totalCount nodes { id: databaseId name target enforcement + source { + ... on Repository { repoOwner: nameWithOwner } + ... on Organization { orgOwner: login } + } # conditions { # refName { # include diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index d226ad18c..fa5f2e30f 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -21,6 +21,10 @@ type Ruleset struct { // Protected bool // } `json:"repository_name"` } + Source struct { + RepoOwner string + OrgOwner string + } RulesGql struct { TotalCount int } diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 49c74581c..6d0005909 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -81,7 +81,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the ruleset in the browser") - cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "Organization name if the ID ") + cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "Organization name if the provided ID is an organization-level ruleset") return cmd } @@ -104,22 +104,6 @@ func viewRun(opts *ViewOptions) error { hostname, _ := cfg.DefaultHost() - if opts.WebMode { - // TODO need to validate ruleset's existence before opening - var rulesetURL string - if opts.Organization != "" { - rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules/%s", ghinstance.HostPrefix(hostname), opts.Organization, opts.ID) - } else { - rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules/%s", opts.ID) - } - - if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) - } - - return opts.Browser.Browse(rulesetURL) - } - var rs *shared.Ruleset if opts.Organization != "" { rs, err = viewOrgRuleset(httpClient, opts.Organization, opts.ID, hostname) @@ -134,6 +118,25 @@ func viewRun(opts *ViewOptions) error { cs := opts.IO.ColorScheme() w := opts.IO.Out + if opts.WebMode { + if rs != nil { + var rulesetURL string + if opts.Organization != "" { + rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules/%s", ghinstance.HostPrefix(hostname), opts.Organization, opts.ID) + } else { + rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules/%s", opts.ID) + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) + } + + return opts.Browser.Browse(rulesetURL) + } else { + fmt.Fprintf(w, "ruleset not found\n") + } + } + fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) fmt.Fprintf(w, "ID: %d\n", rs.Id) @@ -141,7 +144,7 @@ func viewRun(opts *ViewOptions) error { case "disabled": fmt.Fprintf(w, "%s\n", cs.Red("Disabled")) case "evaluate": - fmt.Fprintf(w, "%s\n", cs.Yellow("Evaluate Mode (not being enforced)")) + fmt.Fprintf(w, "%s\n", cs.Yellow("Evaluate Mode (not enforced)")) case "active": fmt.Fprintf(w, "%s\n", cs.Green("Active")) default: @@ -165,7 +168,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(w, "Actor types allowed to bypass:\n") for name, count := range types { - fmt.Fprintf(w, "- %s: %d actors\n", name, count) + fmt.Fprintf(w, "- %s: %d configured\n", name, count) } } @@ -173,8 +176,8 @@ func viewRun(opts *ViewOptions) error { if len(rs.Conditions) == 0 { fmt.Fprintf(w, "No conditions configured\n") } else { - // sort keys for consistent responses, mismatched types don't allow this to be broken - // into a separate function + // sort keys for consistent responses, can't make a separate function due to + // mismatched types keys := make([]string, 0, len(rs.Conditions)) for key := range rs.Conditions { keys = append(keys, key) From e91670edcce73aa31396b4ac1071df291641270e Mon Sep 17 00:00:00 2001 From: vaindil Date: Tue, 2 May 2023 11:55:37 -0400 Subject: [PATCH 11/25] split ruleset types by API type --- api/client.go | 2 +- pkg/cmd/ruleset/list/list.go | 4 ++-- pkg/cmd/ruleset/shared/http.go | 21 ++++------------- pkg/cmd/ruleset/shared/shared.go | 40 ++++++++++++++++---------------- pkg/cmd/ruleset/view/http.go | 8 +++---- pkg/cmd/ruleset/view/view.go | 2 +- 6 files changed, 33 insertions(+), 44 deletions(-) diff --git a/api/client.go b/api/client.go index 0d2690378..e32856554 100644 --- a/api/client.go +++ b/api/client.go @@ -19,7 +19,7 @@ const ( authorization = "Authorization" cacheTTL = "X-GH-CACHE-TTL" graphqlFeatures = "GraphQL-Features" - features = "merge_queue,push_policies" + features = "merge_queue" userAgent = "User-Agent" ) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index e49710ba8..ee13afeef 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -149,7 +149,7 @@ func listRun(opts *ListOptions) error { tp.HeaderRow("ID", "NAME", "SOURCE", "STATUS", "TARGET", "RULES") for _, rs := range result.Rulesets { - tp.AddField(strconv.Itoa(rs.Id)) + tp.AddField(strconv.Itoa(rs.DatabaseId)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) var ownerString string if rs.Source.RepoOwner != "" { @@ -162,7 +162,7 @@ func listRun(opts *ListOptions) error { tp.AddField(ownerString) tp.AddField(strings.ToLower(rs.Enforcement)) tp.AddField(strings.ToLower(rs.Target)) - tp.AddField(strconv.Itoa(rs.RulesGql.TotalCount)) + tp.AddField(strconv.Itoa(rs.Rules.TotalCount)) tp.EndRow() } diff --git a/pkg/cmd/ruleset/shared/http.go b/pkg/cmd/ruleset/shared/http.go index 9dae0d7e6..2276f8989 100644 --- a/pkg/cmd/ruleset/shared/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -12,7 +12,7 @@ type RulesetResponse struct { Level struct { Rulesets struct { TotalCount int - Nodes []Ruleset + Nodes []RulesetGraphQL PageInfo struct { HasNextPage bool EndCursor string @@ -23,7 +23,7 @@ type RulesetResponse struct { type RulesetList struct { TotalCount int - Rulesets []Ruleset + Rulesets []RulesetGraphQL } func ListRepoRulesets(httpClient *http.Client, repo ghrepo.Interface, limit int, includeParents bool) (*RulesetList, error) { @@ -49,7 +49,7 @@ func listRulesets(httpClient *http.Client, query string, variables map[string]in pageLimit := min(limit, 100) res := RulesetList{ - Rulesets: []Ruleset{}, + Rulesets: []RulesetGraphQL{}, } client := api.NewClientFromHTTP(httpClient) @@ -97,7 +97,7 @@ func rulesetsQuery(org bool) string { rulesets(first: $limit, after: $endCursor, includeParents: $includeParents) { totalCount nodes { - id: databaseId + databaseId name target enforcement @@ -105,18 +105,7 @@ func rulesetsQuery(org bool) string { ... on Repository { repoOwner: nameWithOwner } ... on Organization { orgOwner: login } } - # conditions { - # refName { - # include - # exclude - # } - # repositoryName { - # include - # exclude - # protected - # } - # } - rulesGql: rules { + rules { totalCount } } diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index fa5f2e30f..090d3d989 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -1,6 +1,20 @@ package shared -type Ruleset struct { +type RulesetGraphQL struct { + DatabaseId int + Name string + Target string + Enforcement string + Source struct { + RepoOwner string + OrgOwner string + } + Rules struct { + TotalCount int + } +} + +type RulesetREST struct { Id int Name string Target string @@ -10,23 +24,9 @@ type Ruleset struct { ActorId int `json:"actor_id"` ActorType string `json:"actor_type"` } `json:"bypass_actors"` - Conditions map[string]map[string]interface { - // RefName struct { - // Include []string - // Exclude []string - // } `json:"ref_name"` - // RepositoryName struct { - // Include []string - // Exclude []string - // Protected bool - // } `json:"repository_name"` - } - Source struct { - RepoOwner string - OrgOwner string - } - RulesGql struct { - TotalCount int - } - Rules []interface{} + Conditions map[string]map[string]interface{} + // TODO is this source field used? + SourceType string `json:"source_type"` + Source string + Rules []struct{} } diff --git a/pkg/cmd/ruleset/view/http.go b/pkg/cmd/ruleset/view/http.go index bd56c3cf6..d0b26f530 100644 --- a/pkg/cmd/ruleset/view/http.go +++ b/pkg/cmd/ruleset/view/http.go @@ -9,19 +9,19 @@ import ( "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" ) -func viewRepoRuleset(httpClient *http.Client, repo ghrepo.Interface, databaseId string) (*shared.Ruleset, error) { +func viewRepoRuleset(httpClient *http.Client, repo ghrepo.Interface, databaseId string) (*shared.RulesetREST, error) { path := fmt.Sprintf("repos/%s/%s/rulesets/%s", repo.RepoOwner(), repo.RepoName(), databaseId) return viewRuleset(httpClient, repo.RepoHost(), path) } -func viewOrgRuleset(httpClient *http.Client, orgLogin string, databaseId string, host string) (*shared.Ruleset, error) { +func viewOrgRuleset(httpClient *http.Client, orgLogin string, databaseId string, host string) (*shared.RulesetREST, error) { path := fmt.Sprintf("orgs/%s/rulesets/%s", orgLogin, databaseId) return viewRuleset(httpClient, host, path) } -func viewRuleset(httpClient *http.Client, hostname string, path string) (*shared.Ruleset, error) { +func viewRuleset(httpClient *http.Client, hostname string, path string) (*shared.RulesetREST, error) { apiClient := api.NewClientFromHTTP(httpClient) - result := shared.Ruleset{} + result := shared.RulesetREST{} err := apiClient.REST(hostname, "GET", path, nil, &result) if err != nil { diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 6d0005909..7569d3e62 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -104,7 +104,7 @@ func viewRun(opts *ViewOptions) error { hostname, _ := cfg.DefaultHost() - var rs *shared.Ruleset + var rs *shared.RulesetREST if opts.Organization != "" { rs, err = viewOrgRuleset(httpClient, opts.Organization, opts.ID, hostname) } else { From 8fa69c491507707c64faa9223bc7678e85dc37ac Mon Sep 17 00:00:00 2001 From: vaindil Date: Tue, 2 May 2023 12:21:57 -0400 Subject: [PATCH 12/25] don't allow both --repo and --org, add docs --- pkg/cmd/ruleset/list/list.go | 27 ++++++++++++++++++++++++--- pkg/cmd/ruleset/view/view.go | 4 ++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index ee13afeef..6183adb6a 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -43,11 +43,32 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Use: "list", Short: "List rulesets for a repository or organization", Long: heredoc.Doc(` - TODO + List GitHub rulesets for a repository or organization. + + If no options are provided, the current repository's rulesets are listed. You can query a different + repository's rulesets by using the --repo flag. You can also use the --org flag to list rulesets + configured for the provided organization. + + Use the --parents flag to include rulesets configured at higher levels that also apply to the current repository. + + Your access token must have the admin:org scope, which can be granted by running "gh auth refresh -s admin:org". `), - Example: "TODO", - Args: cobra.ExactArgs(0), + Example: heredoc.Doc(` + # List rulesets in the current repository + $ gh ruleset list + + # List rulesets in a different repository, including those configured at higher levels + $ gh ruleset list --repo owner/repo --parents + + # List rulesets in an organization + $ gh ruleset list --org org-name + `), + Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && opts.Organization != "" { + return cmdutil.FlagErrorf("only one of --repo and --org may be specified") + } + // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 7569d3e62..fca7b17bf 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -60,6 +60,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" && opts.Organization != "" { + return cmdutil.FlagErrorf("only one of --repo and --org may be specified") + } + // support `-R, --repo` override opts.BaseRepo = f.BaseRepo From ef30d875b503a295cd7d3e11678f858d49eb80d0 Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 3 May 2023 11:28:04 -0400 Subject: [PATCH 13/25] gracefully handle missing admin:org scope --- pkg/cmd/ruleset/shared/http.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cmd/ruleset/shared/http.go b/pkg/cmd/ruleset/shared/http.go index 2276f8989..4899369bf 100644 --- a/pkg/cmd/ruleset/shared/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -1,8 +1,10 @@ package shared import ( + "errors" "fmt" "net/http" + "strings" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghrepo" @@ -58,6 +60,10 @@ func listRulesets(httpClient *http.Client, query string, variables map[string]in var data RulesetResponse err := client.GraphQL(host, query, variables, &data) if err != nil { + if strings.Contains(err.Error(), "requires one of the following scopes: ['admin:org']") { + return nil, errors.New("the 'admin:org' scope is required to view organization rulesets, try running 'gh auth refresh -s admin:org'") + } + return nil, err } From f25b2af05389fdee0300ac09364a6acf9691fd38 Mon Sep 17 00:00:00 2001 From: vaindil Date: Thu, 4 May 2023 14:16:34 -0400 Subject: [PATCH 14/25] interactive ruleset selection, move shared logic --- pkg/cmd/ruleset/list/list.go | 32 ++++---------- pkg/cmd/ruleset/shared/http.go | 5 ++- pkg/cmd/ruleset/shared/shared.go | 41 +++++++++++++++++- pkg/cmd/ruleset/view/view.go | 74 ++++++++++++++++++++++++++++++-- 4 files changed, 122 insertions(+), 30 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 6183adb6a..1b8a9cc00 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -137,20 +137,8 @@ func listRun(opts *ListOptions) error { return err } - var entityName string - if opts.Organization != "" { - entityName = opts.Organization - } else { - entityName = ghrepo.FullName(repoI) - } - if result.TotalCount == 0 { - parentsMsg := "" - if opts.IncludeParents { - parentsMsg = " or its parents" - } - msg := fmt.Sprintf("no rulesets found in %s%s", entityName, parentsMsg) - return cmdutil.NewNoResultsError(msg) + return shared.NoRulesetsFoundError(opts.Organization, repoI, opts.IncludeParents) } opts.IO.DetectTerminalTheme() @@ -163,7 +151,13 @@ func listRun(opts *ListOptions) error { cs := opts.IO.ColorScheme() if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "\nShowing %d of %d rulesets in %s\n\n", len(result.Rulesets), result.TotalCount, entityName) + parentsMsg := "" + if opts.IncludeParents { + parentsMsg = " and its parents" + } + + inMsg := fmt.Sprintf("%s%s", shared.EntityName(opts.Organization, repoI), parentsMsg) + fmt.Fprintf(opts.IO.Out, "\nShowing %d of %d rulesets in %s\n\n", len(result.Rulesets), result.TotalCount, inMsg) } tp := tableprinter.New(opts.IO) @@ -172,15 +166,7 @@ func listRun(opts *ListOptions) error { for _, rs := range result.Rulesets { tp.AddField(strconv.Itoa(rs.DatabaseId)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) - var ownerString string - if rs.Source.RepoOwner != "" { - ownerString = fmt.Sprintf("%s (repo)", rs.Source.RepoOwner) - } else if rs.Source.OrgOwner != "" { - ownerString = fmt.Sprintf("%s (org)", rs.Source.OrgOwner) - } else { - ownerString = "(unknown)" - } - tp.AddField(ownerString) + tp.AddField(shared.RulesetSource(rs)) tp.AddField(strings.ToLower(rs.Enforcement)) tp.AddField(strings.ToLower(rs.Target)) tp.AddField(strconv.Itoa(rs.Rules.TotalCount)) diff --git a/pkg/cmd/ruleset/shared/http.go b/pkg/cmd/ruleset/shared/http.go index 4899369bf..05a8e3cf7 100644 --- a/pkg/cmd/ruleset/shared/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -108,8 +108,9 @@ func rulesetsQuery(org bool) string { target enforcement source { - ... on Repository { repoOwner: nameWithOwner } - ... on Organization { orgOwner: login } + __typename + ... on Repository { owner: nameWithOwner } + ... on Organization { owner: login } } rules { totalCount diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 090d3d989..682eafb38 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -1,13 +1,20 @@ package shared +import ( + "fmt" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" +) + type RulesetGraphQL struct { DatabaseId int Name string Target string Enforcement string Source struct { - RepoOwner string - OrgOwner string + TypeName string `json:"__typename"` + Owner string } Rules struct { TotalCount int @@ -30,3 +37,33 @@ type RulesetREST struct { Source string Rules []struct{} } + +// Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" +func RulesetSource(rs RulesetGraphQL) string { + var level string + if rs.Source.TypeName == "Repository" { + level = "repo" + } else if rs.Source.TypeName == "Organization" { + level = "org" + } else { + level = "unknown" + } + + return fmt.Sprintf("%s (%s)", rs.Source.Owner, level) +} + +func NoRulesetsFoundError(orgOption string, repoI ghrepo.Interface, includeParents bool) error { + entityName := EntityName(orgOption, repoI) + parentsMsg := "" + if includeParents { + parentsMsg = " or its parents" + } + return cmdutil.NewNoResultsError(fmt.Sprintf("no rulesets found in %s%s", entityName, parentsMsg)) +} + +func EntityName(orgOption string, repoI ghrepo.Interface) string { + if orgOption != "" { + return orgOption + } + return ghrepo.FullName(repoI) +} diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index fca7b17bf..0c642e838 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -6,12 +6,14 @@ import ( "reflect" "sort" "strconv" + "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -25,10 +27,13 @@ type ViewOptions struct { Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser + Prompter prompter.Prompter - ID string - WebMode bool - Organization string + ID string + WebMode bool + IncludeParents bool + InteractiveMode bool + Organization string } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -37,6 +42,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman HttpClient: f.HttpClient, Browser: f.Browser, Config: f.Config, + Prompter: f.Prompter, } cmd := &cobra.Command{ @@ -49,6 +55,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman the ruleset to view. `), Example: heredoc.Doc(` + # Interactively choose a ruleset to view + $ gh ruleset view + # View a ruleset in the current repository $ gh ruleset view 43 @@ -75,6 +84,10 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman return cmdutil.FlagErrorf("invalid value for ruleset ID: %v is not an integer", args[0]) } opts.ID = args[0] + } else if !opts.IO.CanPrompt() { + return cmdutil.FlagErrorf("a ruleset ID must be provided when not running interactively") + } else { + opts.InteractiveMode = true } if runF != nil { @@ -86,6 +99,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the ruleset in the browser") cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "Organization name if the provided ID is an organization-level ruleset") + cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", false, "When choosing interactively, include rulesets configured at higher levels that also apply") return cmd } @@ -108,6 +122,38 @@ func viewRun(opts *ViewOptions) error { hostname, _ := cfg.DefaultHost() + if opts.InteractiveMode { + var rsList *shared.RulesetList + limit := 30 + if opts.Organization != "" { + rsList, err = shared.ListOrgRulesets(httpClient, opts.Organization, limit, hostname, opts.IncludeParents) + } else { + rsList, err = shared.ListRepoRulesets(httpClient, repoI, limit, opts.IncludeParents) + } + + if err != nil { + return err + } + + if rsList.TotalCount == 0 { + return shared.NoRulesetsFoundError(opts.Organization, repoI, opts.IncludeParents) + } + + rs, err := selectRulesetID(rsList, opts.Prompter) + if err != nil { + return err + } + + if rs != nil { + opts.ID = strconv.Itoa(rs.DatabaseId) + + // can't get a ruleset lower in the chain than what was queried, so no need to handle repos here + if rs.Source.TypeName == "Organization" { + opts.Organization = rs.Source.Owner + } + } + } + var rs *shared.RulesetREST if opts.Organization != "" { rs, err = viewOrgRuleset(httpClient, opts.Organization, opts.ID, hostname) @@ -232,3 +278,25 @@ func viewRun(opts *ViewOptions) error { return nil } + +func selectRulesetID(rsList *shared.RulesetList, p prompter.Prompter) (*shared.RulesetGraphQL, error) { + rulesets := make([]string, len(rsList.Rulesets)) + for i, rs := range rsList.Rulesets { + s := fmt.Sprintf( + "%d: %s | %s | contains %s | configured in %s", + rs.DatabaseId, + rs.Name, + strings.ToLower(rs.Enforcement), + text.Pluralize(rs.Rules.TotalCount, "rule"), + shared.RulesetSource(rs), + ) + rulesets[i] = s + } + + r, err := p.Select("Which ruleset would you like to view?", rulesets[0], rulesets) + if err != nil { + return nil, err + } + + return &rsList.Rulesets[r], nil +} From 8328a0abdcb380bc6669743206fc18bac10d8753 Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 21 Jun 2023 14:09:44 -0400 Subject: [PATCH 15/25] update tests, add rules to view command --- .../ruleset/list/fixtures/rulesetList.json | 12 ++++ pkg/cmd/ruleset/list/list.go | 3 +- pkg/cmd/ruleset/list/list_test.go | 22 +++---- pkg/cmd/ruleset/shared/shared.go | 5 +- pkg/cmd/ruleset/view/view.go | 57 +++++++++++-------- 5 files changed, 61 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/ruleset/list/fixtures/rulesetList.json b/pkg/cmd/ruleset/list/fixtures/rulesetList.json index 560c9fb4c..d7c631623 100644 --- a/pkg/cmd/ruleset/list/fixtures/rulesetList.json +++ b/pkg/cmd/ruleset/list/fixtures/rulesetList.json @@ -9,6 +9,10 @@ "name": "test", "target": "BRANCH", "enforcement": "EVALUATE", + "source": { + "__typename": "Repository", + "owner": "OWNER/REPO" + }, "conditions": { "refName": { "include": [ @@ -27,6 +31,10 @@ "name": "asdf", "target": "BRANCH", "enforcement": "ACTIVE", + "source": { + "__typename": "Repository", + "owner": "OWNER/REPO" + }, "conditions": { "refName": { "include": [ @@ -45,6 +53,10 @@ "name": "foobar", "target": "BRANCH", "enforcement": "DISABLED", + "source": { + "__typename": "Organization", + "owner": "Org-Name" + }, "conditions": { "refName": { "include": [ diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 1b8a9cc00..e49e573f0 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -161,14 +161,13 @@ func listRun(opts *ListOptions) error { } tp := tableprinter.New(opts.IO) - tp.HeaderRow("ID", "NAME", "SOURCE", "STATUS", "TARGET", "RULES") + tp.HeaderRow("ID", "NAME", "SOURCE", "STATUS", "RULES") for _, rs := range result.Rulesets { tp.AddField(strconv.Itoa(rs.DatabaseId)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) tp.AddField(shared.RulesetSource(rs)) tp.AddField(strings.ToLower(rs.Enforcement)) - tp.AddField(strings.ToLower(rs.Target)) tp.AddField(strconv.Itoa(rs.Rules.TotalCount)) tp.EndRow() } diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index 0a44b5438..8e25a87fc 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -125,10 +125,10 @@ func Test_listRun(t *testing.T) { Showing 3 of 3 rulesets in OWNER/REPO - ID NAME STATUS TARGET - 4 test evaluate branch - 42 asdf active branch - 77 foobar disabled branch + ID NAME SOURCE STATUS RULES + 4 test OWNER/REPO (repo) evaluate 1 + 42 asdf OWNER/REPO (repo) active 2 + 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", }, @@ -142,10 +142,10 @@ func Test_listRun(t *testing.T) { Showing 3 of 3 rulesets in my-org - ID NAME STATUS TARGET - 4 test evaluate branch - 42 asdf active branch - 77 foobar disabled branch + ID NAME SOURCE STATUS RULES + 4 test OWNER/REPO (repo) evaluate 1 + 42 asdf OWNER/REPO (repo) active 2 + 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", }, @@ -153,9 +153,9 @@ func Test_listRun(t *testing.T) { name: "machine-readable", isTTY: false, wantStdout: heredoc.Doc(` - 4 test evaluate branch - 42 asdf active branch - 77 foobar disabled branch + 4 test OWNER/REPO (repo) evaluate 1 + 42 asdf OWNER/REPO (repo) active 2 + 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", }, diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 682eafb38..0299f056a 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -35,7 +35,10 @@ type RulesetREST struct { // TODO is this source field used? SourceType string `json:"source_type"` Source string - Rules []struct{} + Rules []struct { + Type string + Parameters map[string]interface{} + } } // Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 0c642e838..df4930de7 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -3,7 +3,6 @@ package view import ( "fmt" "net/http" - "reflect" "sort" "strconv" "strings" @@ -189,7 +188,9 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) fmt.Fprintf(w, "ID: %d\n", rs.Id) + fmt.Fprintf(w, "Source: %s (%s)\n", rs.Source, rs.SourceType) + fmt.Fprint(w, "Enforceument: ") switch rs.Enforcement { case "disabled": fmt.Fprintf(w, "%s\n", cs.Red("Disabled")) @@ -198,7 +199,7 @@ func viewRun(opts *ViewOptions) error { case "active": fmt.Fprintf(w, "%s\n", cs.Green("Active")) default: - fmt.Fprintf(w, "Enforcement: %s\n", rs.Enforcement) + fmt.Fprintf(w, "%s\n", rs.Enforcement) } fmt.Fprintf(w, "\n%s\n", cs.Bold("Bypassing")) @@ -246,27 +247,7 @@ func viewRun(opts *ViewOptions) error { sort.Strings(subkeys) for _, n := range subkeys { - rawVal := condition[n] - - k := reflect.TypeOf(rawVal).Kind() - if rawVal == nil || - ((k == reflect.Slice || k == reflect.Map) && len(rawVal.([]interface{})) == 0) { - continue - } - - printVal := fmt.Sprint(rawVal) - - // fmt.Fprintf(w, "n: %s, type: %s\n", n, reflect.TypeOf(rawVal).String()) - - // switch val := rawVal.(type) { - // case []interface{}: - // // currently only string arrays are returned by the API at this level - // printVal = fmt.Sprint(val) - // default: - // printVal = fmt.Sprint(val) - // } - - fmt.Fprintf(w, "[%s: %s] ", n, printVal) + fmt.Fprintf(w, "[%s: %v] ", n, condition[n]) } fmt.Fprint(w, "\n") @@ -274,7 +255,35 @@ func viewRun(opts *ViewOptions) error { } fmt.Fprintf(w, "\n%s\n", cs.Bold("Rules")) - fmt.Fprintf(w, "%d configured\n", reflect.ValueOf(rs.Rules).Len()) + if len(rs.Rules) == 0 { + fmt.Fprintf(w, "No rules configured\n") + } else { + // sort keys for consistent responses + sort.SliceStable(rs.Rules, func(i, j int) bool { + return rs.Rules[i].Type < rs.Rules[j].Type + }) + + for _, rule := range rs.Rules { + fmt.Fprintf(w, "- %s", rule.Type) + + if rule.Parameters != nil && len(rule.Parameters) > 0 { + fmt.Fprintf(w, ": ") + + // sort these keys too for consistency + params := make([]string, 0, len(rule.Parameters)) + for p := range rule.Parameters { + params = append(params, p) + } + sort.Strings(params) + + for _, n := range params { + fmt.Fprintf(w, "[%s: %v] ", n, rule.Parameters[n]) + } + } + + fmt.Fprint(w, "\n") + } + } return nil } From 2f7caf8502415d89d365a261db2e5b5262c999a8 Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 30 Jun 2023 14:08:23 -0400 Subject: [PATCH 16/25] update bypass response and parents logic, tests --- pkg/cmd/ruleset/list/list.go | 9 +++--- pkg/cmd/ruleset/list/list_test.go | 12 +++++++ pkg/cmd/ruleset/shared/shared.go | 11 +++++-- pkg/cmd/ruleset/view/view.go | 52 +++++++++++++------------------ 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index e49e573f0..0091df00a 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -49,9 +49,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman repository's rulesets by using the --repo flag. You can also use the --org flag to list rulesets configured for the provided organization. - Use the --parents flag to include rulesets configured at higher levels that also apply to the current repository. + Use the --parents flag to control whether rulesets configured at higher levels that also apply to the provided + repository or organization should be returned. The default is true. - Your access token must have the admin:org scope, which can be granted by running "gh auth refresh -s admin:org". + Your access token must have the admin:org scope to use the --org flag, which can be granted by running "gh auth refresh -s admin:org". `), Example: heredoc.Doc(` # List rulesets in the current repository @@ -73,7 +74,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts.BaseRepo = f.BaseRepo if opts.Limit < 1 { - return cmdutil.FlagErrorf("invalid value for --limit: %v", opts.Limit) + return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit) } if runF != nil { @@ -86,7 +87,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum number of rulesets to list") cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "List organization-wide rulesets for the provided organization") - cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", false, "Include rulesets configured at higher levels that also apply") + cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", true, "Whether to include rulesets configured at higher levels that also apply") cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the list of rulesets in the web browser") return cmd diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index 8e25a87fc..fd220a3be 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -66,6 +66,18 @@ func Test_NewCmdList(t *testing.T) { Organization: "", }, }, + { + name: "invalid limit", + args: "--limit 0", + isTTY: true, + wantErr: "invalid limit: 0", + }, + { + name: "repo and org specified", + args: "--org \"my-org\" -R \"owner/repo\"", + isTTY: true, + wantErr: "only one of --repo and --org may be specified", + }, } for _, tt := range tests { diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 0299f056a..5631a24e7 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -26,10 +26,10 @@ type RulesetREST struct { Name string Target string Enforcement string - BypassMode string `json:"bypass_mode"` BypassActors []struct { - ActorId int `json:"actor_id"` - ActorType string `json:"actor_type"` + ActorId int `json:"actor_id"` + ActorType string `json:"actor_type"` + BypassMode string `json:"bypass_mode"` } `json:"bypass_actors"` Conditions map[string]map[string]interface{} // TODO is this source field used? @@ -39,6 +39,11 @@ type RulesetREST struct { Type string Parameters map[string]interface{} } + Links struct { + Html struct { + Href string + } + } `json:"_links"` } // Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index df4930de7..6b3604fa4 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -10,7 +10,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" @@ -52,15 +51,22 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman If no ID is provided, an interactive prompt will be used to choose the ruleset to view. + + Use the --parents flag to control whether rulesets configured at higher + levels that also apply to the provided repository or organization should + be returned. The default is true. `), Example: heredoc.Doc(` - # Interactively choose a ruleset to view + # Interactively choose a ruleset to view from all rulesets that apply to the current repository $ gh ruleset view - # View a ruleset in the current repository + # Interactively choose a ruleset to view from only rulesets configured in the current repository + $ gh ruleset view --no-parents + + # View a ruleset configured in the current repository or any of its parents $ gh ruleset view 43 - # View a ruleset in a different repository + # View a ruleset configured in a different repository or any of its parents $ gh ruleset view 23 --repo owner/repo # View an organization-level ruleset @@ -98,7 +104,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the ruleset in the browser") cmd.Flags().StringVarP(&opts.Organization, "org", "o", "", "Organization name if the provided ID is an organization-level ruleset") - cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", false, "When choosing interactively, include rulesets configured at higher levels that also apply") + cmd.Flags().BoolVarP(&opts.IncludeParents, "parents", "p", true, "Whether to include rulesets configured at higher levels that also apply") return cmd } @@ -169,18 +175,11 @@ func viewRun(opts *ViewOptions) error { if opts.WebMode { if rs != nil { - var rulesetURL string - if opts.Organization != "" { - rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules/%s", ghinstance.HostPrefix(hostname), opts.Organization, opts.ID) - } else { - rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules/%s", opts.ID) - } - if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesetURL)) + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rs.Links.Html.Href)) } - return opts.Browser.Browse(rulesetURL) + return opts.Browser.Browse(rs.Links.Html.Href) } else { fmt.Fprintf(w, "ruleset not found\n") } @@ -202,24 +201,16 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(w, "%s\n", rs.Enforcement) } - fmt.Fprintf(w, "\n%s\n", cs.Bold("Bypassing")) - fmt.Fprintf(w, "Mode: %s\n", rs.BypassMode) + fmt.Fprintf(w, "\n%s\n", cs.Bold("Bypass List")) if len(rs.BypassActors) == 0 { - fmt.Fprintf(w, "No actors configured for bypass\n") + fmt.Fprintf(w, "This ruleset cannot be bypassed\n") } else { - types := make(map[string]int) - for _, t := range rs.BypassActors { - val, exists := types[t.ActorType] - if exists { - types[t.ActorType] = val + 1 - } else { - types[t.ActorType] = 1 - } - } + sort.Slice(rs.BypassActors, func(i, j int) bool { + return rs.BypassActors[i].ActorId < rs.BypassActors[j].ActorId + }) - fmt.Fprintf(w, "Actor types allowed to bypass:\n") - for name, count := range types { - fmt.Fprintf(w, "- %s: %d configured\n", name, count) + for _, t := range rs.BypassActors { + fmt.Fprintf(w, "- %s (ID: %d), mode: %s\n", t.ActorType, t.ActorId, t.BypassMode) } } @@ -227,8 +218,7 @@ func viewRun(opts *ViewOptions) error { if len(rs.Conditions) == 0 { fmt.Fprintf(w, "No conditions configured\n") } else { - // sort keys for consistent responses, can't make a separate function due to - // mismatched types + // sort keys for consistent responses keys := make([]string, 0, len(rs.Conditions)) for key := range rs.Conditions { keys = append(keys, key) From 3add6721324b8e2f67ad3dfea998b8791537f4ea Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 30 Jun 2023 18:11:22 -0400 Subject: [PATCH 17/25] list test updates --- pkg/cmd/ruleset/list/list.go | 2 +- pkg/cmd/ruleset/list/list_test.go | 163 +++++++++++++++++++++++++++--- 2 files changed, 151 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 0091df00a..6d2bc3cb1 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -116,7 +116,7 @@ func listRun(opts *ListOptions) error { if opts.Organization != "" { rulesetURL = fmt.Sprintf("%sorganizations/%s/settings/rules", ghinstance.HostPrefix(hostname), opts.Organization) } else { - rulesetURL = ghrepo.GenerateRepoURL(repoI, "settings/rules") + rulesetURL = ghrepo.GenerateRepoURL(repoI, "rules") } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index fd220a3be..2bb0e3cf7 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -31,9 +32,10 @@ func Test_NewCmdList(t *testing.T) { args: "", isTTY: true, want: ListOptions{ - Limit: 30, - WebMode: false, - Organization: "", + Limit: 30, + IncludeParents: true, + WebMode: false, + Organization: "", }, }, { @@ -41,9 +43,21 @@ func Test_NewCmdList(t *testing.T) { args: "--limit 1", isTTY: true, want: ListOptions{ - Limit: 1, - WebMode: false, - Organization: "", + Limit: 1, + IncludeParents: true, + WebMode: false, + Organization: "", + }, + }, + { + name: "include parents", + args: "--parents=false", + isTTY: true, + want: ListOptions{ + Limit: 30, + IncludeParents: false, + WebMode: false, + Organization: "", }, }, { @@ -51,9 +65,10 @@ func Test_NewCmdList(t *testing.T) { args: "--org \"my-org\"", isTTY: true, want: ListOptions{ - Limit: 30, - WebMode: false, - Organization: "my-org", + Limit: 30, + IncludeParents: true, + WebMode: false, + Organization: "my-org", }, }, { @@ -61,9 +76,10 @@ func Test_NewCmdList(t *testing.T) { args: "--web", isTTY: true, want: ListOptions{ - Limit: 30, - WebMode: true, - Organization: "", + Limit: 30, + IncludeParents: true, + WebMode: true, + Organization: "", }, }, { @@ -121,6 +137,76 @@ func Test_NewCmdList(t *testing.T) { } } +func Test_RulesetList_Web(t *testing.T) { + tests := []struct { + name string + stdoutTTY bool + wantStdout string + wantBrowse string + }{ + { + name: "repo tty", + stdoutTTY: true, + wantStdout: "Opening github.com/OWNER/REPO in your browser.\n", + wantBrowse: "https://github.com/OWNER/REPO", + }, + { + name: "org tty", + stdoutTTY: true, + wantStdout: "Opening github.com/OWNER/REPO in your browser.\n", + wantBrowse: "https://github.com/OWNER/REPO", + }, + { + name: "repo non-tty", + stdoutTTY: false, + wantStdout: "", + wantBrowse: "https://github.com/OWNER/REPO", + }, + { + name: "org non-tty", + stdoutTTY: false, + wantStdout: "", + wantBrowse: "https://github.com/OWNER/REPO", + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + reg.StubRepoInfoResponse("OWNER", "REPO", "main") + + browser := &browser.Stub{} + opts := &ListOptions{ + WebMode: true, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Browser: browser, + } + + io, _, stdout, _ := iostreams.Test() + + opts.IO = io + + t.Run(tt.name, func(t *testing.T) { + io.SetStdoutTTY(tt.stdoutTTY) + + _, teardown := run.Stub() + defer teardown(t) + + if err := listRun(opts); err != nil { + t.Errorf("listRun() error = %v", err) + } + assert.Equal(t, "", stdout.String()) + assert.Equal(t, tt.wantStdout, stdout.String()) + reg.Verify(t) + browser.Verify(t, tt.wantBrowse) + }) + } +} + func Test_listRun(t *testing.T) { tests := []struct { name string @@ -129,6 +215,7 @@ func Test_listRun(t *testing.T) { wantErr string wantStdout string wantStderr string + wantBrowse string }{ { name: "list repo rulesets", @@ -143,6 +230,7 @@ func Test_listRun(t *testing.T) { 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", + wantBrowse: "", }, { name: "list org rulesets", @@ -160,6 +248,7 @@ func Test_listRun(t *testing.T) { 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", + wantBrowse: "", }, { name: "machine-readable", @@ -170,6 +259,49 @@ func Test_listRun(t *testing.T) { 77 foobar Org-Name (org) disabled 4 `), wantStderr: "", + wantBrowse: "", + }, + { + name: "repo web mode, TTY", + isTTY: true, + opts: ListOptions{ + WebMode: true, + }, + wantStdout: "Opening github.com/OWNER/REPO/rules in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/OWNER/REPO/rules", + }, + { + name: "org web mode, TTY", + isTTY: true, + opts: ListOptions{ + WebMode: true, + Organization: "my-org", + }, + wantStdout: "Opening github.com/organizations/my-org/settings/rules in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/organizations/my-org/settings/rules", + }, + { + name: "repo web mode, non-TTY", + isTTY: false, + opts: ListOptions{ + WebMode: true, + }, + wantStdout: "", + wantStderr: "", + wantBrowse: "https://github.com/OWNER/REPO/rules", + }, + { + name: "org web mode, non-TTY", + isTTY: false, + opts: ListOptions{ + WebMode: true, + Organization: "my-org", + }, + wantStdout: "", + wantStderr: "", + wantBrowse: "https://github.com/organizations/my-org/settings/rules", }, } @@ -190,7 +322,8 @@ func Test_listRun(t *testing.T) { tt.opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.FromFullName("OWNER/REPO") } - tt.opts.Browser = &browser.Stub{} + browser := &browser.Stub{} + tt.opts.Browser = browser tt.opts.Config = func() (config.Config, error) { return config.NewBlankConfig(), nil } @@ -204,6 +337,10 @@ func Test_listRun(t *testing.T) { require.NoError(t, err) } + if tt.wantBrowse != "" { + browser.Verify(t, tt.wantBrowse) + } + assert.Equal(t, tt.wantStdout, stdout.String()) assert.Equal(t, tt.wantStderr, stderr.String()) }) From 5c5dcd2a802ce2dacf238b422ddbe8b0d04e191e Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 30 Jun 2023 18:34:41 -0400 Subject: [PATCH 18/25] initial view tests --- .../ruleset/view/fixtures/rulesetView.json | 62 +++++ pkg/cmd/ruleset/view/view_test.go | 257 ++++++++++++++++++ 2 files changed, 319 insertions(+) create mode 100644 pkg/cmd/ruleset/view/fixtures/rulesetView.json create mode 100644 pkg/cmd/ruleset/view/view_test.go diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetView.json b/pkg/cmd/ruleset/view/fixtures/rulesetView.json new file mode 100644 index 000000000..5824b341d --- /dev/null +++ b/pkg/cmd/ruleset/view/fixtures/rulesetView.json @@ -0,0 +1,62 @@ +{ + "id": 42, + "name": "Test Ruleset", + "target": "branch", + "source_type": "Repository", + "source": "my-owner/repo-name", + "enforcement": "active", + "conditions": { + "ref_name": { + "exclude": [], + "include": [ + "~ALL" + ] + } + }, + "rules": [ + { + "type": "commit_message_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "asdf", + "operator": "contains" + } + }, + { + "type": "commit_author_email_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "@example.com", + "operator": "ends_with" + } + }, + { + "type": "creation" + } + ], + "node_id": "RRS_lACqUmVwb3NpdG9yec4dwx_uzSNG", + "_links": { + "self": { + "href": "https://api.github.com/repos/my-owner/repo-name/rulesets/42" + }, + "html": { + "href": "https://github.com/my-owner/repo-name/rules/42" + } + }, + "created_at": "2023-05-01T13:53:37.185-04:00", + "updated_at": "2023-06-29T17:38:03.722-04:00", + "bypass_actors": [ + { + "actor_id": 5, + "actor_type": "RepositoryRole", + "bypass_mode": "always" + }, + { + "actor_id": 1, + "actor_type": "OrganizationAdmin", + "bypass_mode": "always" + } + ] +} diff --git a/pkg/cmd/ruleset/view/view_test.go b/pkg/cmd/ruleset/view/view_test.go new file mode 100644 index 000000000..0548ff4fe --- /dev/null +++ b/pkg/cmd/ruleset/view/view_test.go @@ -0,0 +1,257 @@ +package view + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_NewCmdView(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want ViewOptions + wantErr string + }{ + { + name: "no arguments", + args: "", + isTTY: true, + want: ViewOptions{ + ID: "", + WebMode: false, + IncludeParents: true, + InteractiveMode: true, + Organization: "", + }, + }, + { + name: "only ID", + args: "3", + isTTY: true, + want: ViewOptions{ + ID: "3", + WebMode: false, + IncludeParents: true, + InteractiveMode: false, + Organization: "", + }, + }, + { + name: "org", + args: "--org \"my-org\"", + isTTY: true, + want: ViewOptions{ + ID: "", + WebMode: false, + IncludeParents: true, + InteractiveMode: true, + Organization: "my-org", + }, + }, + { + name: "web mode", + args: "--web", + isTTY: true, + want: ViewOptions{ + ID: "", + WebMode: true, + IncludeParents: true, + InteractiveMode: true, + Organization: "", + }, + }, + { + name: "parents", + args: "--parents=false", + isTTY: true, + want: ViewOptions{ + ID: "", + WebMode: false, + IncludeParents: false, + InteractiveMode: true, + Organization: "", + }, + }, + { + name: "repo and org specified", + args: "--org \"my-org\" -R \"owner/repo\"", + isTTY: true, + wantErr: "only one of --repo and --org may be specified", + }, + { + name: "invalid ID", + args: "1.5", + isTTY: true, + wantErr: "invalid value for ruleset ID: 1.5 is not an integer", + }, + { + name: "ID not provided and not TTY", + args: "", + isTTY: false, + wantErr: "a ruleset ID must be provided when not running interactively", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: ios, + } + + var opts *ViewOptions + cmd := NewCmdView(f, func(o *ViewOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.ID, opts.ID) + assert.Equal(t, tt.want.WebMode, opts.WebMode) + assert.Equal(t, tt.want.IncludeParents, opts.IncludeParents) + assert.Equal(t, tt.want.InteractiveMode, opts.InteractiveMode) + assert.Equal(t, tt.want.Organization, opts.Organization) + }) + } +} + +func Test_viewRun(t *testing.T) { + tests := []struct { + name string + isTTY bool + opts ViewOptions + wantErr string + wantStdout string + wantStderr string + wantBrowse string + }{ + { + name: "view repo ruleset", + isTTY: true, + opts: ViewOptions{ + ID: "42", + }, + wantStdout: heredoc.Doc(` + + Test Ruleset + ID: 42 + Source: my-owner/repo-name (Repository) + Enforceument: Active + + Bypass List + - OrganizationAdmin (ID: 1), mode: always + - RepositoryRole (ID: 5), mode: always + + Conditions + - ref_name: [exclude: []] [include: [~ALL]] + + Rules + - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] + - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] + - creation + `), + wantStderr: "", + wantBrowse: "", + }, + { + name: "web mode, TTY", + isTTY: true, + opts: ViewOptions{ + ID: "42", + WebMode: true, + }, + wantStdout: "Opening github.com/my-owner/repo-name/rules/42 in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/my-owner/repo-name/rules/42", + }, + { + name: "web mode, non-TTY", + isTTY: false, + opts: ViewOptions{ + ID: "42", + WebMode: true, + }, + wantStdout: "", + wantStderr: "", + wantBrowse: "https://github.com/my-owner/repo-name/rules/42", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + fakeHTTP := &httpmock.Registry{} + fakeHTTP.Register( + httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), + httpmock.FileResponse("./fixtures/rulesetView.json"), + ) + + tt.opts.IO = ios + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: fakeHTTP}, nil + } + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("my-owner/repo-name") + } + browser := &browser.Stub{} + tt.opts.Browser = browser + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + err := viewRun(&tt.opts) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + if tt.wantBrowse != "" { + browser.Verify(t, tt.wantBrowse) + } + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} From e4aa5ba84c06b7e48288e361d5aadfef63380446 Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 30 Jun 2023 19:25:28 -0400 Subject: [PATCH 19/25] add ruleset check command --- pkg/cmd/ruleset/check/check.go | 81 ++++++++++++++++++++++++-------- pkg/cmd/ruleset/shared/shared.go | 61 ++++++++++++++++++++++-- pkg/cmd/ruleset/view/view.go | 26 +--------- 3 files changed, 118 insertions(+), 50 deletions(-) diff --git a/pkg/cmd/ruleset/check/check.go b/pkg/cmd/ruleset/check/check.go index 34c708c69..53a73efb1 100644 --- a/pkg/cmd/ruleset/check/check.go +++ b/pkg/cmd/ruleset/check/check.go @@ -9,8 +9,11 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -22,9 +25,11 @@ type CheckOptions struct { Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) Git *git.Client + Browser browser.Browser Branch string Default bool + WebMode bool } func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Command { @@ -32,33 +37,58 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm IO: f.IOStreams, Config: f.Config, HttpClient: f.HttpClient, + Browser: f.Browser, Git: f.GitClient, } cmd := &cobra.Command{ Use: "check []", - Short: "Print rules that would apply to a given branch", + Short: "View rules that would apply to a given branch", Long: heredoc.Doc(` - TODO + View information about GitHub rules that apply to a given branch. + + The provided branch name does not need to exist, rules will be displayed that would apply + to a branch with that name. All rules are returned regardless of where they are configured. + + If no branch name is provided, then the current branch will be used. + + The --default flag can be used to view rules that apply to the default branch of the current + repository. `), - Example: "TODO", - Args: cobra.MaximumNArgs(1), + Example: heredoc.Doc(` + # View all rules that apply to the current branch + $ gh ruleset check + + # View all rules that apply to a branch named "my-branch" in a different repository + $ gh ruleset check my-branch --repo owner/repo + + # View all rules that apply to the default branch in a different repository + $ gh ruleset check --default --repo owner/repo + + # View a ruleset configured in a different repository or any of its parents + $ gh ruleset view 23 --repo owner/repo + + # View an organization-level ruleset + $ gh ruleset view 23 --org my-org + `), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - // TODO flag to do a push - if len(args) > 0 { opts.Branch = args[0] } - if runF != nil { - return runF(opts) + if err := cmdutil.MutuallyExclusive( + "specify only one of `--default` or a branch name", + opts.Branch != "", + opts.Default, + ); err != nil { + return err } - if opts.Branch != "" && opts.Default { - return cmdutil.FlagErrorf( - "branch argument '%s' and --default mutually exclusive", opts.Branch) + if runF != nil { + return runF(opts) } return checkRun(opts) @@ -71,10 +101,6 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm } func checkRun(opts *CheckOptions) error { - // TODO ask about pushing (if interactive) - // TODO error if not interactive and --push not specified - // TODO parsing for errors on push - httpClient, err := opts.HttpClient() if err != nil { return err @@ -103,18 +129,33 @@ func checkRun(opts *CheckOptions) error { } } - var lol interface{} + rawPath := fmt.Sprintf("rules?ref=%s%s", url.QueryEscape("refs/heads/"), url.QueryEscape(opts.Branch)) + rulesURL := ghrepo.GenerateRepoURL(repoI, rawPath) + + if opts.WebMode { + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesURL)) + } + + return opts.Browser.Browse(rulesURL) + } + + var rules []shared.RulesetRule endpoint := fmt.Sprintf("repos/%s/%s/rules/branches/%s", repoI.RepoOwner(), repoI.RepoName(), url.PathEscape(opts.Branch)) - if err = client.REST(repoI.RepoHost(), "GET", endpoint, nil, &lol); err != nil { + if err = client.REST(repoI.RepoHost(), "GET", endpoint, nil, &rules); err != nil { return fmt.Errorf("GET %s failed: %w", endpoint, err) } - // TODO handle 404s gracefully - // TODO actually parse JSON + w := opts.IO.Out - fmt.Printf("DBG %#v\n", lol) + fmt.Fprintf(w, "%d rules apply to branch %s in repo %s/%s\n", len(rules), opts.Branch, repoI.RepoOwner(), repoI.RepoName()) + + if len(rules) > 0 { + fmt.Fprint(w, "\n") + fmt.Fprint(w, shared.ParseRulesForDisplay(rules)) + } return nil } diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 5631a24e7..b365a006d 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -2,6 +2,8 @@ package shared import ( "fmt" + "sort" + "strings" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" @@ -35,17 +37,22 @@ type RulesetREST struct { // TODO is this source field used? SourceType string `json:"source_type"` Source string - Rules []struct { - Type string - Parameters map[string]interface{} - } - Links struct { + Rules []RulesetRule + Links struct { Html struct { Href string } } `json:"_links"` } +type RulesetRule struct { + Type string + Parameters map[string]interface{} + RulesetSourceType string `json:"ruleset_source_type"` + RulesetSource string `json:"ruleset_source"` + RulesetId int `json:"ruleset_id"` +} + // Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" func RulesetSource(rs RulesetGraphQL) string { var level string @@ -60,6 +67,50 @@ func RulesetSource(rs RulesetGraphQL) string { return fmt.Sprintf("%s (%s)", rs.Source.Owner, level) } +func ParseRulesForDisplay(rules []RulesetRule) string { + var display strings.Builder + + // sort keys for consistent responses + sort.SliceStable(rules, func(i, j int) bool { + return rules[i].Type < rules[j].Type + }) + + for _, rule := range rules { + display.WriteString(fmt.Sprintf("- %s", rule.Type)) + + if rule.Parameters != nil && len(rule.Parameters) > 0 { + display.WriteString(": ") + + // sort these keys too for consistency + params := make([]string, 0, len(rule.Parameters)) + for p := range rule.Parameters { + params = append(params, p) + } + sort.Strings(params) + + for _, n := range params { + display.WriteString(fmt.Sprintf("[%s: %v] ", n, rule.Parameters[n])) + } + } + + // ruleset source info is only returned from the "get rules for a branch" endpoint + if rule.RulesetSource != "" { + display.WriteString( + fmt.Sprintf( + "\n (configured in ruleset %d from %s %s)\n", + rule.RulesetId, + strings.ToLower(rule.RulesetSourceType), + rule.RulesetSource, + ), + ) + } + + display.WriteString("\n") + } + + return display.String() +} + func NoRulesetsFoundError(orgOption string, repoI ghrepo.Interface, includeParents bool) error { entityName := EntityName(orgOption, repoI) parentsMsg := "" diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 6b3604fa4..b842e2e69 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -248,31 +248,7 @@ func viewRun(opts *ViewOptions) error { if len(rs.Rules) == 0 { fmt.Fprintf(w, "No rules configured\n") } else { - // sort keys for consistent responses - sort.SliceStable(rs.Rules, func(i, j int) bool { - return rs.Rules[i].Type < rs.Rules[j].Type - }) - - for _, rule := range rs.Rules { - fmt.Fprintf(w, "- %s", rule.Type) - - if rule.Parameters != nil && len(rule.Parameters) > 0 { - fmt.Fprintf(w, ": ") - - // sort these keys too for consistency - params := make([]string, 0, len(rule.Parameters)) - for p := range rule.Parameters { - params = append(params, p) - } - sort.Strings(params) - - for _, n := range params { - fmt.Fprintf(w, "[%s: %v] ", n, rule.Parameters[n]) - } - } - - fmt.Fprint(w, "\n") - } + fmt.Fprint(w, shared.ParseRulesForDisplay(rs.Rules)) } return nil From 77df68e11ac862e807ff720bd899b513175ed6e5 Mon Sep 17 00:00:00 2001 From: vaindil Date: Fri, 30 Jun 2023 20:01:17 -0400 Subject: [PATCH 20/25] misc cleanup --- pkg/cmd/ruleset/list/list_test.go | 71 ------------------------------- pkg/cmd/ruleset/rules.go | 39 ----------------- pkg/cmd/ruleset/ruleset.go | 11 +++-- pkg/cmd/ruleset/shared/shared.go | 1 - 4 files changed, 8 insertions(+), 114 deletions(-) delete mode 100644 pkg/cmd/ruleset/rules.go diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index 2bb0e3cf7..4a01b803a 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -137,76 +136,6 @@ func Test_NewCmdList(t *testing.T) { } } -func Test_RulesetList_Web(t *testing.T) { - tests := []struct { - name string - stdoutTTY bool - wantStdout string - wantBrowse string - }{ - { - name: "repo tty", - stdoutTTY: true, - wantStdout: "Opening github.com/OWNER/REPO in your browser.\n", - wantBrowse: "https://github.com/OWNER/REPO", - }, - { - name: "org tty", - stdoutTTY: true, - wantStdout: "Opening github.com/OWNER/REPO in your browser.\n", - wantBrowse: "https://github.com/OWNER/REPO", - }, - { - name: "repo non-tty", - stdoutTTY: false, - wantStdout: "", - wantBrowse: "https://github.com/OWNER/REPO", - }, - { - name: "org non-tty", - stdoutTTY: false, - wantStdout: "", - wantBrowse: "https://github.com/OWNER/REPO", - }, - } - - for _, tt := range tests { - reg := &httpmock.Registry{} - reg.StubRepoInfoResponse("OWNER", "REPO", "main") - - browser := &browser.Stub{} - opts := &ListOptions{ - WebMode: true, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Browser: browser, - } - - io, _, stdout, _ := iostreams.Test() - - opts.IO = io - - t.Run(tt.name, func(t *testing.T) { - io.SetStdoutTTY(tt.stdoutTTY) - - _, teardown := run.Stub() - defer teardown(t) - - if err := listRun(opts); err != nil { - t.Errorf("listRun() error = %v", err) - } - assert.Equal(t, "", stdout.String()) - assert.Equal(t, tt.wantStdout, stdout.String()) - reg.Verify(t) - browser.Verify(t, tt.wantBrowse) - }) - } -} - func Test_listRun(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/ruleset/rules.go b/pkg/cmd/ruleset/rules.go deleted file mode 100644 index b3405d839..000000000 --- a/pkg/cmd/ruleset/rules.go +++ /dev/null @@ -1,39 +0,0 @@ -package ruleset - -type RuleType string -type Enforcement string -type MatchingOperator string - -const ( - RuleTypeCommitAuthorEmailPattern RuleType = "commit_author_email_pattern" - RuleTypePullRequest RuleType = "pull_request" - // TODO others - - EnforcementEnabled Enforcement = "enabled" - EnforcementDisabled Enforcement = "disabled" - - MatchingOperatorStartsWith MatchingOperator = "starts_with" - MatchingOperatorEndsWith MatchingOperator = "ends_with" - MatchingOperatorContains MatchingOperator = "contains" - MatchingOperatorRegex MatchingOperator = "regex" -) - -type ConfigurationPullRequest struct { - DissmissStaleReviewsOnPush bool - RequireCodeOwnerReview bool - RequestLastPushApproval bool - RequiredApprovingReviewCount int -} - -type BranchNamePattern struct { - Name string - Negate bool - Operator MatchingOperator -} - -type Rule struct { - ID string - Type RuleType - Enforcement Enforcement - Configuration interface{} -} diff --git a/pkg/cmd/ruleset/ruleset.go b/pkg/cmd/ruleset/ruleset.go index 64536e868..86411d717 100644 --- a/pkg/cmd/ruleset/ruleset.go +++ b/pkg/cmd/ruleset/ruleset.go @@ -12,12 +12,17 @@ import ( func NewCmdRuleset(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "ruleset ", - Short: "Manage repository and organization rulesets", + Short: "View info about repo rulesets", Long: heredoc.Doc(` - TODO + Repository rulesets are a way to define a set of rules that apply to a repository. + These commands allow you to view information about them. `), Aliases: []string{"rs"}, - Example: "TODO", + Example: heredoc.Doc(` + $ gh ruleset list + $ gh ruleset view --repo OWNER/REPO --web + $ gh ruleset check branch-name + `), } cmdutil.EnableRepoOverride(cmd, f) diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index b365a006d..a71b20522 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -34,7 +34,6 @@ type RulesetREST struct { BypassMode string `json:"bypass_mode"` } `json:"bypass_actors"` Conditions map[string]map[string]interface{} - // TODO is this source field used? SourceType string `json:"source_type"` Source string Rules []RulesetRule From 159ac8ba0ef1e8493803c1a6c44fb12132fd8aa5 Mon Sep 17 00:00:00 2001 From: vaindil Date: Mon, 3 Jul 2023 11:59:55 -0400 Subject: [PATCH 21/25] fix merge errors --- pkg/cmd/ruleset/list/list.go | 11 ++--------- pkg/cmd/ruleset/list/list_test.go | 4 ---- pkg/cmd/ruleset/view/view.go | 11 ++--------- pkg/cmd/ruleset/view/view_test.go | 4 ---- 4 files changed, 4 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 6d2bc3cb1..2b8a210ed 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -8,7 +8,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/tableprinter" @@ -16,13 +15,13 @@ import ( "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + ghAuth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) type ListOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams - Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser @@ -37,7 +36,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman IO: f.IOStreams, HttpClient: f.HttpClient, Browser: f.Browser, - Config: f.Config, } cmd := &cobra.Command{ Use: "list", @@ -104,12 +102,7 @@ func listRun(opts *ListOptions) error { return err } - cfg, err := opts.Config() - if err != nil { - return err - } - - hostname, _ := cfg.DefaultHost() + hostname, _ := ghAuth.DefaultHost() if opts.WebMode { var rulesetURL string diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index 4a01b803a..8d1f0bb99 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -8,7 +8,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -253,9 +252,6 @@ func Test_listRun(t *testing.T) { } browser := &browser.Stub{} tt.opts.Browser = browser - tt.opts.Config = func() (config.Config, error) { - return config.NewBlankConfig(), nil - } err := listRun(&tt.opts) diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index b842e2e69..2b67060df 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -9,20 +9,19 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/ruleset/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + ghAuth "github.com/cli/go-gh/v2/pkg/auth" "github.com/spf13/cobra" ) type ViewOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams - Config func() (config.Config, error) BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser Prompter prompter.Prompter @@ -39,7 +38,6 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman IO: f.IOStreams, HttpClient: f.HttpClient, Browser: f.Browser, - Config: f.Config, Prompter: f.Prompter, } @@ -120,12 +118,7 @@ func viewRun(opts *ViewOptions) error { return err } - cfg, err := opts.Config() - if err != nil { - return err - } - - hostname, _ := cfg.DefaultHost() + hostname, _ := ghAuth.DefaultHost() if opts.InteractiveMode { var rsList *shared.RulesetList diff --git a/pkg/cmd/ruleset/view/view_test.go b/pkg/cmd/ruleset/view/view_test.go index 0548ff4fe..7e184ddc4 100644 --- a/pkg/cmd/ruleset/view/view_test.go +++ b/pkg/cmd/ruleset/view/view_test.go @@ -8,7 +8,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" - "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -233,9 +232,6 @@ func Test_viewRun(t *testing.T) { } browser := &browser.Stub{} tt.opts.Browser = browser - tt.opts.Config = func() (config.Config, error) { - return config.NewBlankConfig(), nil - } err := viewRun(&tt.opts) From dcefe340ada6fa887b90f581f5632853f616bd4b Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 5 Jul 2023 13:17:50 -0400 Subject: [PATCH 22/25] add rs check tests --- pkg/cmd/ruleset/check/check.go | 18 +- pkg/cmd/ruleset/check/check_test.go | 233 ++++++++++++++++++ .../ruleset/check/fixtures/rulesetCheck.json | 62 +++++ 3 files changed, 305 insertions(+), 8 deletions(-) create mode 100644 pkg/cmd/ruleset/check/check_test.go create mode 100644 pkg/cmd/ruleset/check/fixtures/rulesetCheck.json diff --git a/pkg/cmd/ruleset/check/check.go b/pkg/cmd/ruleset/check/check.go index 53a73efb1..3007fe36a 100644 --- a/pkg/cmd/ruleset/check/check.go +++ b/pkg/cmd/ruleset/check/check.go @@ -46,12 +46,12 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm Long: heredoc.Doc(` View information about GitHub rules that apply to a given branch. - The provided branch name does not need to exist, rules will be displayed that would apply + The provided branch name does not need to exist; rules will be displayed that would apply to a branch with that name. All rules are returned regardless of where they are configured. If no branch name is provided, then the current branch will be used. - The --default flag can be used to view rules that apply to the default branch of the current + The --default flag can be used to view rules that apply to the default branch of the repository. `), Example: heredoc.Doc(` @@ -96,6 +96,7 @@ func NewCmdCheck(f *cmdutil.Factory, runF func(*CheckOptions) error) *cobra.Comm } cmd.Flags().BoolVar(&opts.Default, "default", false, "Check rules on default branch") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the branch rules page in a web browser") return cmd } @@ -109,7 +110,7 @@ func checkRun(opts *CheckOptions) error { repoI, err := opts.BaseRepo() if err != nil { - return err + return fmt.Errorf("could not determine repo to use: %w", err) } git := opts.Git @@ -129,15 +130,16 @@ func checkRun(opts *CheckOptions) error { } } - rawPath := fmt.Sprintf("rules?ref=%s%s", url.QueryEscape("refs/heads/"), url.QueryEscape(opts.Branch)) - rulesURL := ghrepo.GenerateRepoURL(repoI, rawPath) - if opts.WebMode { + // the query string parameter may have % signs in it, so it must be carefully used with Printf functions + queryString := fmt.Sprintf("?ref=%s", url.QueryEscape("refs/heads/"+opts.Branch)) + rawUrl := ghrepo.GenerateRepoURL(repoI, "rules") + if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rulesURL)) + fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", text.DisplayURL(rawUrl)) } - return opts.Browser.Browse(rulesURL) + return opts.Browser.Browse(rawUrl + queryString) } var rules []shared.RulesetRule diff --git a/pkg/cmd/ruleset/check/check_test.go b/pkg/cmd/ruleset/check/check_test.go new file mode 100644 index 000000000..2d9dffae4 --- /dev/null +++ b/pkg/cmd/ruleset/check/check_test.go @@ -0,0 +1,233 @@ +package check + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_NewCmdCheck(t *testing.T) { + tests := []struct { + name string + args string + isTTY bool + want CheckOptions + wantErr string + }{ + { + name: "no arguments", + args: "", + isTTY: true, + want: CheckOptions{ + Branch: "", + Default: false, + WebMode: false, + }, + }, + { + name: "branch name", + args: "my-branch", + isTTY: true, + want: CheckOptions{ + Branch: "my-branch", + Default: false, + WebMode: false, + }, + }, + { + name: "default", + args: "--default=true", + isTTY: true, + want: CheckOptions{ + Branch: "", + Default: true, + WebMode: false, + }, + }, + { + name: "web mode", + args: "--web", + isTTY: true, + want: CheckOptions{ + Branch: "", + Default: false, + WebMode: true, + }, + }, + { + name: "both --default and branch name specified", + args: "--default asdf", + isTTY: true, + wantErr: "specify only one of `--default` or a branch name", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + f := &cmdutil.Factory{ + IOStreams: ios, + } + + var opts *CheckOptions + cmd := NewCmdCheck(f, func(o *CheckOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + assert.Equal(t, tt.want.Branch, opts.Branch) + assert.Equal(t, tt.want.Default, opts.Default) + assert.Equal(t, tt.want.WebMode, opts.WebMode) + }) + } +} + +func Test_checkRun(t *testing.T) { + tests := []struct { + name string + isTTY bool + opts CheckOptions + wantErr string + wantStdout string + wantStderr string + wantBrowse string + }{ + { + name: "view rules for branch", + isTTY: true, + opts: CheckOptions{ + Branch: "my-branch", + }, + wantStdout: heredoc.Doc(` + 6 rules apply to branch my-branch in repo my-org/repo-name + + - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] + (configured in ruleset 1234 from organization my-org) + + - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] + (configured in ruleset 5678 from repository my-org/repo-name) + + - commit_message_pattern: [name: ] [negate: false] [operator: starts_with] [pattern: fff] + (configured in ruleset 1234 from organization my-org) + + - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] + (configured in ruleset 5678 from repository my-org/repo-name) + + - creation + (configured in ruleset 5678 from repository my-org/repo-name) + + - required_signatures + (configured in ruleset 1234 from organization my-org) + + `), + wantStderr: "", + wantBrowse: "", + }, + { + name: "web mode, TTY", + isTTY: true, + opts: CheckOptions{ + Branch: "my-branch", + WebMode: true, + }, + wantStdout: "Opening github.com/my-org/repo-name/rules in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/my-org/repo-name/rules?ref=refs%2Fheads%2Fmy-branch", + }, + { + name: "web mode, TTY, special character in branch name", + isTTY: true, + opts: CheckOptions{ + Branch: "my-feature/my-branch", + WebMode: true, + }, + wantStdout: "Opening github.com/my-org/repo-name/rules in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/my-org/repo-name/rules?ref=refs%2Fheads%2Fmy-feature%2Fmy-branch", + }, + { + name: "web mode, non-TTY", + isTTY: false, + opts: CheckOptions{ + Branch: "my-branch", + WebMode: true, + }, + wantStdout: "", + wantStderr: "", + wantBrowse: "https://github.com/my-org/repo-name/rules?ref=refs%2Fheads%2Fmy-branch", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + fakeHTTP := &httpmock.Registry{} + fakeHTTP.Register( + httpmock.REST("GET", "repos/my-org/repo-name/rules/branches/my-branch"), + httpmock.FileResponse("./fixtures/rulesetCheck.json"), + ) + + tt.opts.IO = ios + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: fakeHTTP}, nil + } + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("my-org/repo-name") + } + browser := &browser.Stub{} + tt.opts.Browser = browser + + err := checkRun(&tt.opts) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } else { + require.NoError(t, err) + } + + if tt.wantBrowse != "" { + browser.Verify(t, tt.wantBrowse) + } + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} diff --git a/pkg/cmd/ruleset/check/fixtures/rulesetCheck.json b/pkg/cmd/ruleset/check/fixtures/rulesetCheck.json new file mode 100644 index 000000000..c02fddef4 --- /dev/null +++ b/pkg/cmd/ruleset/check/fixtures/rulesetCheck.json @@ -0,0 +1,62 @@ +[ + { + "type": "commit_author_email_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "@example.com", + "operator": "ends_with" + }, + "ruleset_source_type": "Organization", + "ruleset_source": "my-org", + "ruleset_id": 1234 + }, + { + "type": "commit_message_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "fff", + "operator": "starts_with" + }, + "ruleset_source_type": "Organization", + "ruleset_source": "my-org", + "ruleset_id": 1234 + }, + { + "type": "required_signatures", + "ruleset_source_type": "Organization", + "ruleset_source": "my-org", + "ruleset_id": 1234 + }, + { + "type": "commit_message_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "asdf", + "operator": "contains" + }, + "ruleset_source_type": "Repository", + "ruleset_source": "my-org/repo-name", + "ruleset_id": 5678 + }, + { + "type": "commit_author_email_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "@example.com", + "operator": "ends_with" + }, + "ruleset_source_type": "Repository", + "ruleset_source": "my-org/repo-name", + "ruleset_id": 5678 + }, + { + "type": "creation", + "ruleset_source_type": "Repository", + "ruleset_source": "my-org/repo-name", + "ruleset_id": 5678 + } +] From 069faef86c4c63b7358050a2f832e1f64993b04f Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 5 Jul 2023 14:29:52 -0400 Subject: [PATCH 23/25] refactor graphql query a bit --- pkg/cmd/ruleset/shared/http.go | 81 +++++++++++++++++----------------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/ruleset/shared/http.go b/pkg/cmd/ruleset/shared/http.go index 05a8e3cf7..b9114aa85 100644 --- a/pkg/cmd/ruleset/shared/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -2,7 +2,6 @@ package shared import ( "errors" - "fmt" "net/http" "strings" @@ -85,48 +84,50 @@ func listRulesets(httpClient *http.Client, query string, variables map[string]in return &res, nil } -func rulesetsQuery(org bool) string { - var args string - var level string - - if org { - args = "$login: String!" - level = "organization(login: $login)" - } else { - args = "$owner: String!, $repo: String!" - level = "repository(owner: $owner, name: $repo)" - } - - str := fmt.Sprintf("query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, %s) { level: %s {", args, level) - - return str + ` - rulesets(first: $limit, after: $endCursor, includeParents: $includeParents) { - totalCount - nodes { - databaseId - name - target - enforcement - source { - __typename - ... on Repository { owner: nameWithOwner } - ... on Organization { owner: login } - } - rules { - totalCount - } - } - pageInfo { - hasNextPage - endCursor - } - } - }}` -} - func min(a, b int) int { if a < b { return a } return b } + +func rulesetsQuery(org bool) string { + if org { + return orgGraphQLHeader + sharedGraphQLBody + } else { + return repoGraphQLHeader + sharedGraphQLBody + } +} + +const repoGraphQLHeader = ` +query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $owner: String!, $repo: String!) { + level: repository(owner: $owner, name: $repo) { +` + +const orgGraphQLHeader = ` +query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $login: String!) { + level: organization(login: $login) { +` + +const sharedGraphQLBody = ` +rulesets(first: $limit, after: $endCursor, includeParents: $includeParents) { + totalCount + nodes { + databaseId + name + target + enforcement + source { + __typename + ... on Repository { owner: nameWithOwner } + ... on Organization { owner: login } + } + rules { + totalCount + } + } + pageInfo { + hasNextPage + endCursor + } +}}}` From ab921f96e64aaa3d6b83ed279db93b3490801fbd Mon Sep 17 00:00:00 2001 From: vaindil Date: Wed, 5 Jul 2023 15:13:09 -0400 Subject: [PATCH 24/25] split org/repo graphql queries, better tests --- pkg/cmd/ruleset/list/list_test.go | 28 +++++- pkg/cmd/ruleset/shared/http.go | 4 +- .../ruleset/view/fixtures/rulesetViewOrg.json | 58 ++++++++++++ ...{rulesetView.json => rulesetViewRepo.json} | 0 pkg/cmd/ruleset/view/view.go | 2 +- pkg/cmd/ruleset/view/view_test.go | 90 +++++++++++++++++-- 6 files changed, 167 insertions(+), 15 deletions(-) create mode 100644 pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json rename pkg/cmd/ruleset/view/fixtures/{rulesetView.json => rulesetViewRepo.json} (100%) diff --git a/pkg/cmd/ruleset/list/list_test.go b/pkg/cmd/ruleset/list/list_test.go index 8d1f0bb99..d075e46d6 100644 --- a/pkg/cmd/ruleset/list/list_test.go +++ b/pkg/cmd/ruleset/list/list_test.go @@ -140,6 +140,7 @@ func Test_listRun(t *testing.T) { name string isTTY bool opts ListOptions + httpStubs func(*httpmock.Registry) wantErr string wantStdout string wantStderr string @@ -157,6 +158,12 @@ func Test_listRun(t *testing.T) { 42 asdf OWNER/REPO (repo) active 2 77 foobar Org-Name (org) disabled 4 `), + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepoRulesetList\b`), + httpmock.FileResponse("./fixtures/rulesetList.json"), + ) + }, wantStderr: "", wantBrowse: "", }, @@ -175,6 +182,12 @@ func Test_listRun(t *testing.T) { 42 asdf OWNER/REPO (repo) active 2 77 foobar Org-Name (org) disabled 4 `), + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query OrgRulesetList\b`), + httpmock.FileResponse("./fixtures/rulesetList.json"), + ) + }, wantStderr: "", wantBrowse: "", }, @@ -186,6 +199,12 @@ func Test_listRun(t *testing.T) { 42 asdf OWNER/REPO (repo) active 2 77 foobar Org-Name (org) disabled 4 `), + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepoRulesetList\b`), + httpmock.FileResponse("./fixtures/rulesetList.json"), + ) + }, wantStderr: "", wantBrowse: "", }, @@ -240,12 +259,15 @@ func Test_listRun(t *testing.T) { ios.SetStdinTTY(tt.isTTY) ios.SetStderrTTY(tt.isTTY) - fakeHTTP := &httpmock.Registry{} - fakeHTTP.Register(httpmock.GraphQL(`query RulesetList\b`), httpmock.FileResponse("./fixtures/rulesetList.json")) + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(reg) + } tt.opts.IO = ios tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: fakeHTTP}, nil + return &http.Client{Transport: reg}, nil } tt.opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.FromFullName("OWNER/REPO") diff --git a/pkg/cmd/ruleset/shared/http.go b/pkg/cmd/ruleset/shared/http.go index b9114aa85..59480dadf 100644 --- a/pkg/cmd/ruleset/shared/http.go +++ b/pkg/cmd/ruleset/shared/http.go @@ -100,12 +100,12 @@ func rulesetsQuery(org bool) string { } const repoGraphQLHeader = ` -query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $owner: String!, $repo: String!) { +query RepoRulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $owner: String!, $repo: String!) { level: repository(owner: $owner, name: $repo) { ` const orgGraphQLHeader = ` -query RulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $login: String!) { +query OrgRulesetList($limit: Int!, $endCursor: String, $includeParents: Boolean, $login: String!) { level: organization(login: $login) { ` diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json b/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json new file mode 100644 index 000000000..d18509bbf --- /dev/null +++ b/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json @@ -0,0 +1,58 @@ +{ + "id": 74, + "name": "My Org Ruleset", + "target": "branch", + "source_type": "Organization", + "source": "my-owner", + "enforcement": "disabled", + "conditions": { + "ref_name": { + "exclude": [], + "include": [ + "~ALL" + ] + }, + "repository_name": { + "exclude": [], + "include": [ + "~ALL" + ], + "protected": true + } + }, + "rules": [ + { + "type": "commit_message_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "asdf", + "operator": "contains" + } + }, + { + "type": "commit_author_email_pattern", + "parameters": { + "name": "", + "negate": false, + "pattern": "@example.com", + "operator": "ends_with" + } + }, + { + "type": "creation" + } + ], + "node_id": "RRS_lACqUmVwb3NpdG9yec4dwx_uzSNG", + "_links": { + "self": { + "href": "https://api.github.com/repos/my-owner/repo-name/rulesets/74" + }, + "html": { + "href": "https://github.com/organizations/my-owner/settings/rules/74" + } + }, + "created_at": "2023-05-01T13:53:37.185-04:00", + "updated_at": "2023-06-29T17:38:03.722-04:00", + "bypass_actors": [] +} diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetView.json b/pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json similarity index 100% rename from pkg/cmd/ruleset/view/fixtures/rulesetView.json rename to pkg/cmd/ruleset/view/fixtures/rulesetViewRepo.json diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 2b67060df..615abdac5 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -182,7 +182,7 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(w, "ID: %d\n", rs.Id) fmt.Fprintf(w, "Source: %s (%s)\n", rs.Source, rs.SourceType) - fmt.Fprint(w, "Enforceument: ") + fmt.Fprint(w, "Enforcement: ") switch rs.Enforcement { case "disabled": fmt.Fprintf(w, "%s\n", cs.Red("Disabled")) diff --git a/pkg/cmd/ruleset/view/view_test.go b/pkg/cmd/ruleset/view/view_test.go index 7e184ddc4..78c32d076 100644 --- a/pkg/cmd/ruleset/view/view_test.go +++ b/pkg/cmd/ruleset/view/view_test.go @@ -153,6 +153,7 @@ func Test_viewRun(t *testing.T) { name string isTTY bool opts ViewOptions + httpStubs func(*httpmock.Registry) wantErr string wantStdout string wantStderr string @@ -169,7 +170,7 @@ func Test_viewRun(t *testing.T) { Test Ruleset ID: 42 Source: my-owner/repo-name (Repository) - Enforceument: Active + Enforcement: Active Bypass List - OrganizationAdmin (ID: 1), mode: always @@ -183,31 +184,102 @@ func Test_viewRun(t *testing.T) { - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] - creation `), + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), + httpmock.FileResponse("./fixtures/rulesetViewRepo.json"), + ) + }, wantStderr: "", wantBrowse: "", }, { - name: "web mode, TTY", + name: "view org ruleset", + isTTY: true, + opts: ViewOptions{ + ID: "74", + Organization: "my-owner", + }, + wantStdout: heredoc.Doc(` + + My Org Ruleset + ID: 74 + Source: my-owner (Organization) + Enforcement: Disabled + + Bypass List + This ruleset cannot be bypassed + + Conditions + - ref_name: [exclude: []] [include: [~ALL]] + - repository_name: [exclude: []] [include: [~ALL]] [protected: true] + + Rules + - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] + - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] + - creation + `), + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "orgs/my-owner/rulesets/74"), + httpmock.FileResponse("./fixtures/rulesetViewOrg.json"), + ) + }, + wantStderr: "", + wantBrowse: "", + }, + { + name: "web mode, TTY, repo", isTTY: true, opts: ViewOptions{ ID: "42", WebMode: true, }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), + httpmock.FileResponse("./fixtures/rulesetViewRepo.json"), + ) + }, wantStdout: "Opening github.com/my-owner/repo-name/rules/42 in your browser.\n", wantStderr: "", wantBrowse: "https://github.com/my-owner/repo-name/rules/42", }, { - name: "web mode, non-TTY", + name: "web mode, non-TTY, repo", isTTY: false, opts: ViewOptions{ ID: "42", WebMode: true, }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), + httpmock.FileResponse("./fixtures/rulesetViewRepo.json"), + ) + }, wantStdout: "", wantStderr: "", wantBrowse: "https://github.com/my-owner/repo-name/rules/42", }, + { + name: "web mode, TTY, org", + isTTY: true, + opts: ViewOptions{ + ID: "74", + Organization: "my-owner", + WebMode: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "orgs/my-owner/rulesets/74"), + httpmock.FileResponse("./fixtures/rulesetViewOrg.json"), + ) + }, + wantStdout: "Opening github.com/organizations/my-owner/settings/rules/74 in your browser.\n", + wantStderr: "", + wantBrowse: "https://github.com/organizations/my-owner/settings/rules/74", + }, } for _, tt := range tests { @@ -217,15 +289,15 @@ func Test_viewRun(t *testing.T) { ios.SetStdinTTY(tt.isTTY) ios.SetStderrTTY(tt.isTTY) - fakeHTTP := &httpmock.Registry{} - fakeHTTP.Register( - httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), - httpmock.FileResponse("./fixtures/rulesetView.json"), - ) + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(reg) + } tt.opts.IO = ios tt.opts.HttpClient = func() (*http.Client, error) { - return &http.Client{Transport: fakeHTTP}, nil + return &http.Client{Transport: reg}, nil } tt.opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.FromFullName("my-owner/repo-name") From d82c1193b3ea3eb2ab537d3e52073a0b2137c779 Mon Sep 17 00:00:00 2001 From: vaindil Date: Thu, 6 Jul 2023 14:56:55 -0400 Subject: [PATCH 25/25] make IDs cyan, add prompter test --- pkg/cmd/ruleset/list/list.go | 2 +- .../view/fixtures/rulesetViewMultiple.json | 41 ++++++++ .../ruleset/view/fixtures/rulesetViewOrg.json | 2 +- pkg/cmd/ruleset/view/view.go | 12 +-- pkg/cmd/ruleset/view/view_test.go | 95 +++++++++++++------ 5 files changed, 116 insertions(+), 36 deletions(-) create mode 100644 pkg/cmd/ruleset/view/fixtures/rulesetViewMultiple.json diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 2b8a210ed..60afc69ab 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -158,7 +158,7 @@ func listRun(opts *ListOptions) error { tp.HeaderRow("ID", "NAME", "SOURCE", "STATUS", "RULES") for _, rs := range result.Rulesets { - tp.AddField(strconv.Itoa(rs.DatabaseId)) + tp.AddField(strconv.Itoa(rs.DatabaseId), tableprinter.WithColor(cs.Cyan)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) tp.AddField(shared.RulesetSource(rs)) tp.AddField(strings.ToLower(rs.Enforcement)) diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetViewMultiple.json b/pkg/cmd/ruleset/view/fixtures/rulesetViewMultiple.json new file mode 100644 index 000000000..1b0d9e2cf --- /dev/null +++ b/pkg/cmd/ruleset/view/fixtures/rulesetViewMultiple.json @@ -0,0 +1,41 @@ +{ + "data": { + "level": { + "rulesets": { + "totalCount": 2, + "nodes": [ + { + "databaseId": 74, + "name": "My Org Ruleset", + "target": "BRANCH", + "enforcement": "EVALUATE", + "source": { + "__typename": "Organization", + "owner": "my-owner" + }, + "rules": { + "totalCount": 3 + } + }, + { + "databaseId": 42, + "name": "Test Ruleset", + "target": "BRANCH", + "enforcement": "ACTIVE", + "source": { + "__typename": "Repository", + "owner": "my-owner/repo-name" + }, + "rules": { + "totalCount": 3 + } + } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "Mg" + } + } + } + } +} diff --git a/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json b/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json index d18509bbf..88a2bd7ee 100644 --- a/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json +++ b/pkg/cmd/ruleset/view/fixtures/rulesetViewOrg.json @@ -4,7 +4,7 @@ "target": "branch", "source_type": "Organization", "source": "my-owner", - "enforcement": "disabled", + "enforcement": "evaluate", "conditions": { "ref_name": { "exclude": [], diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index 615abdac5..813f72c51 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -119,6 +119,7 @@ func viewRun(opts *ViewOptions) error { } hostname, _ := ghAuth.DefaultHost() + cs := opts.IO.ColorScheme() if opts.InteractiveMode { var rsList *shared.RulesetList @@ -137,7 +138,7 @@ func viewRun(opts *ViewOptions) error { return shared.NoRulesetsFoundError(opts.Organization, repoI, opts.IncludeParents) } - rs, err := selectRulesetID(rsList, opts.Prompter) + rs, err := selectRulesetID(rsList, opts.Prompter, cs) if err != nil { return err } @@ -163,7 +164,6 @@ func viewRun(opts *ViewOptions) error { return err } - cs := opts.IO.ColorScheme() w := opts.IO.Out if opts.WebMode { @@ -179,7 +179,7 @@ func viewRun(opts *ViewOptions) error { } fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) - fmt.Fprintf(w, "ID: %d\n", rs.Id) + fmt.Fprintf(w, "ID: %s\n", cs.Cyan(strconv.Itoa(rs.Id))) fmt.Fprintf(w, "Source: %s (%s)\n", rs.Source, rs.SourceType) fmt.Fprint(w, "Enforcement: ") @@ -247,12 +247,12 @@ func viewRun(opts *ViewOptions) error { return nil } -func selectRulesetID(rsList *shared.RulesetList, p prompter.Prompter) (*shared.RulesetGraphQL, error) { +func selectRulesetID(rsList *shared.RulesetList, p prompter.Prompter, cs *iostreams.ColorScheme) (*shared.RulesetGraphQL, error) { rulesets := make([]string, len(rsList.Rulesets)) for i, rs := range rsList.Rulesets { s := fmt.Sprintf( - "%d: %s | %s | contains %s | configured in %s", - rs.DatabaseId, + "%s: %s | %s | contains %s | configured in %s", + cs.Cyan(strconv.Itoa(rs.DatabaseId)), rs.Name, strings.ToLower(rs.Enforcement), text.Pluralize(rs.Rules.TotalCount, "rule"), diff --git a/pkg/cmd/ruleset/view/view_test.go b/pkg/cmd/ruleset/view/view_test.go index 78c32d076..ce9c21db4 100644 --- a/pkg/cmd/ruleset/view/view_test.go +++ b/pkg/cmd/ruleset/view/view_test.go @@ -9,6 +9,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -149,15 +150,36 @@ func Test_NewCmdView(t *testing.T) { } func Test_viewRun(t *testing.T) { + repoRulesetStdout := heredoc.Doc(` + + Test Ruleset + ID: 42 + Source: my-owner/repo-name (Repository) + Enforcement: Active + + Bypass List + - OrganizationAdmin (ID: 1), mode: always + - RepositoryRole (ID: 5), mode: always + + Conditions + - ref_name: [exclude: []] [include: [~ALL]] + + Rules + - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] + - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] + - creation + `) + tests := []struct { - name string - isTTY bool - opts ViewOptions - httpStubs func(*httpmock.Registry) - wantErr string - wantStdout string - wantStderr string - wantBrowse string + name string + isTTY bool + opts ViewOptions + httpStubs func(*httpmock.Registry) + prompterStubs func(*prompter.MockPrompter) + wantErr string + wantStdout string + wantStderr string + wantBrowse string }{ { name: "view repo ruleset", @@ -165,25 +187,7 @@ func Test_viewRun(t *testing.T) { opts: ViewOptions{ ID: "42", }, - wantStdout: heredoc.Doc(` - - Test Ruleset - ID: 42 - Source: my-owner/repo-name (Repository) - Enforcement: Active - - Bypass List - - OrganizationAdmin (ID: 1), mode: always - - RepositoryRole (ID: 5), mode: always - - Conditions - - ref_name: [exclude: []] [include: [~ALL]] - - Rules - - commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] - - commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] - - creation - `), + wantStdout: repoRulesetStdout, httpStubs: func(reg *httpmock.Registry) { reg.Register( httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), @@ -205,7 +209,7 @@ func Test_viewRun(t *testing.T) { My Org Ruleset ID: 74 Source: my-owner (Organization) - Enforcement: Disabled + Enforcement: Evaluate Mode (not enforced) Bypass List This ruleset cannot be bypassed @@ -228,6 +232,35 @@ func Test_viewRun(t *testing.T) { wantStderr: "", wantBrowse: "", }, + { + name: "prompter", + isTTY: true, + opts: ViewOptions{ + InteractiveMode: true, + }, + wantStdout: repoRulesetStdout, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepoRulesetList\b`), + httpmock.FileResponse("./fixtures/rulesetViewMultiple.json"), + ) + reg.Register( + httpmock.REST("GET", "repos/my-owner/repo-name/rulesets/42"), + httpmock.FileResponse("./fixtures/rulesetViewRepo.json"), + ) + }, + prompterStubs: func(pm *prompter.MockPrompter) { + const repoRuleset = "42: Test Ruleset | active | contains 3 rules | configured in my-owner/repo-name (repo)" + pm.RegisterSelect("Which ruleset would you like to view?", + []string{ + "74: My Org Ruleset | evaluate | contains 3 rules | configured in my-owner (org)", + repoRuleset, + }, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, repoRuleset) + }) + }, + }, { name: "web mode, TTY, repo", isTTY: true, @@ -289,6 +322,12 @@ func Test_viewRun(t *testing.T) { ios.SetStdinTTY(tt.isTTY) ios.SetStderrTTY(tt.isTTY) + pm := prompter.NewMockPrompter(t) + if tt.prompterStubs != nil { + tt.prompterStubs(pm) + } + tt.opts.Prompter = pm + reg := &httpmock.Registry{} defer reg.Verify(t) if tt.httpStubs != nil {