Skip to content

Commit

Permalink
Require visibility confirmation in gh repo edit
Browse files Browse the repository at this point in the history 
This commit modifies interactive and non-interactive behaviors around `gh repo edit` as well as providing greater information about the impact.

1. `--help` usage is expanded to highlight the most significant consequences of changing visibility
1. `--help` usage and interactive experience call out GitHub Docs content that act as source of truth about full consequences of various changes
1. `gh repo edit` interactive experience will require confirmation for any visibility change
1. `gh repo edit` interactive experience will output potential stars and watchers lose regardless of visibility transition
1. `gh repo edit` will require `--visibility` flag to include new `--accept-visibility-change-consequences` flag regardless of interactivity
  • Loading branch information
@andyfeller
andyfeller committed Oct 29, 2024
1 parent ff9b0a1 commit 15b2db9
Show file tree
Hide file tree
Showing 2 changed files with 195 additions and 29 deletions.
59 changes: 39 additions & 20 deletions pkg/cmd/repo/edit/edit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
fd "github.com/cli/cli/v2/internal/featuredetection"
"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/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/pkg/set"
Expand Down Expand Up @@ -49,15 +50,16 @@ const (
)

type EditOptions struct {
HTTPClient *http.Client
Repository ghrepo.Interface
IO *iostreams.IOStreams
Edits EditRepositoryInput
AddTopics []string
RemoveTopics []string
InteractiveMode bool
Detector fd.Detector
Prompter iprompter
HTTPClient *http.Client
Repository ghrepo.Interface
IO *iostreams.IOStreams
Edits EditRepositoryInput
AddTopics []string
RemoveTopics []string
AcceptVisibilityChangeConsequences bool
InteractiveMode bool
Detector fd.Detector
Prompter iprompter
// Cache of current repo topics to avoid retrieving them
// in multiple flows.
topicsCache []string
Expand Down Expand Up @@ -103,7 +105,16 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
To toggle a setting off, use the %[1]s--<flag>=false%[1]s syntax.
Note that changing repository visibility to private will cause loss of stars and watchers.
Changing repository visibility can have unexpected consequences including but not limited to:
- Losing stars and watchers, affecting repository ranking
- Detaching public forks from the network
- Disabling push rulesets
- Allowing access to GitHub Actions history and logs
When the %[1]s--visibility%[1]s flag is used, %[1]s--accept-visibility-change-consequences%[1]s flag is required.
For information on all the potential consequences, see <https://gh.io/setting-repository-visibility>
`, "`"),
Args: cobra.MaximumNArgs(1),
Example: heredoc.Doc(`
Expand Down Expand Up @@ -142,6 +153,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
return cmdutil.FlagErrorf("specify properties to edit when not running interactively")
}

if opts.Edits.Visibility != nil && !opts.AcceptVisibilityChangeConsequences {
return cmdutil.FlagErrorf("use of --visibility flag requires --accept-visibility-change-consequences flag")
}

if runF != nil {
return runF(opts)
}
Expand All @@ -167,6 +182,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowUpdateBranch, "allow-update-branch", "", "Allow a pull request head branch that is behind its base branch to be updated")
cmd.Flags().StringSliceVar(&opts.AddTopics, "add-topic", nil, "Add repository topic")
cmd.Flags().StringSliceVar(&opts.RemoveTopics, "remove-topic", nil, "Remove repository topic")
cmd.Flags().BoolVar(&opts.AcceptVisibilityChangeConsequences, "accept-visibility-change-consequences", false, "Accept the consequences of changing the repository visibility")

return cmd
}
Expand Down Expand Up @@ -379,23 +395,26 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error {
}
opts.Edits.EnableProjects = &a
case optionVisibility:
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Danger zone: changing repository visibility can have unexpected consequences; consult https://gh.io/setting-repository-visibility before continuing.\n", cs.WarningIcon())

visibilityOptions := []string{"public", "private", "internal"}
selected, err := p.Select("Visibility", strings.ToLower(r.Visibility), visibilityOptions)
if err != nil {
return err
}
confirmed := true
if visibilityOptions[selected] == "private" &&
(r.StargazerCount > 0 || r.Watchers.TotalCount > 0) {
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Changing the repository visibility to private will cause permanent loss of stars and watchers.\n", cs.WarningIcon())
confirmed, err = p.Confirm("Do you want to change visibility to private?", false)
if err != nil {
return err
}
selectedVisibility := visibilityOptions[selected]

if selectedVisibility != r.Visibility && (r.StargazerCount > 0 || r.Watchers.TotalCount > 0) {
fmt.Fprintf(opts.IO.ErrOut, "%s Changing the repository visibility to %s will cause permanent loss of %s and %s.\n", cs.WarningIcon(), selectedVisibility, text.Pluralize(r.StargazerCount, "star"), text.Pluralize(r.Watchers.TotalCount, "watcher"))
}

confirmed, err := p.Confirm(fmt.Sprintf("Do you want to change visibility to %s?", selectedVisibility), false)
if err != nil {
return err
}
if confirmed {
opts.Edits.Visibility = &visibilityOptions[selected]
opts.Edits.Visibility = &selectedVisibility
}
case optionMergeOptions:
var defaultMergeOptions []string
Expand Down
165 changes: 156 additions & 9 deletions pkg/cmd/repo/edit/edit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,63 @@ func TestNewCmdEdit(t *testing.T) {
},
},
},
{
name: "deny public visibility change without accepting consequences",
args: "--visibility public",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{},
},
wantErr: "use of --visibility flag requires --accept-visibility-change-consequences flag",
},
{
name: "allow public visibility change with accepting consequences",
args: "--visibility public --accept-visibility-change-consequences",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{
Visibility: sp("public"),
},
},
},
{
name: "deny private visibility change without accepting consequences",
args: "--visibility private",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{},
},
wantErr: "use of --visibility flag requires --accept-visibility-change-consequences flag",
},
{
name: "allow private visibility change with accepting consequences",
args: "--visibility private --accept-visibility-change-consequences",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{
Visibility: sp("private"),
},
},
},
{
name: "deny internal visibility change without accepting consequences",
args: "--visibility internal",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{},
},
wantErr: "use of --visibility flag requires --accept-visibility-change-consequences flag",
},
{
name: "allow internal visibility change with accepting consequences",
args: "--visibility internal --accept-visibility-change-consequences",
wantOpts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
Edits: EditRepositoryInput{
Visibility: sp("internal"),
},
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -241,6 +298,104 @@ func Test_editRun_interactive(t *testing.T) {
}))
},
},
{
name: "skipping visibility without confirmation",
opts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
InteractiveMode: true,
},
promptStubs: func(pm *prompter.MockPrompter) {
pm.RegisterMultiSelect("What do you want to edit?", nil, editList,
func(_ string, _, opts []string) ([]int, error) {
return []int{8}, nil
})
pm.RegisterSelect("Visibility", []string{"public", "private", "internal"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "private")
})
pm.RegisterConfirm("Do you want to change visibility to private?", func(_ string, _ bool) (bool, error) {
return false, nil
})
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`
{
"data": {
"repository": {
"visibility": "public",
"description": "description",
"homePageUrl": "https://url.com",
"defaultBranchRef": {
"name": "main"
},
"stargazerCount": 10,
"isInOrganization": false,
"repositoryTopics": {
"nodes": [{
"topic": {
"name": "x"
}
}]
}
}
}
}`))
reg.Exclude(t, httpmock.REST("PATCH", "repos/OWNER/REPO"))
},
},
{
name: "changing visibility with confirmation",
opts: EditOptions{
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
InteractiveMode: true,
},
promptStubs: func(pm *prompter.MockPrompter) {
pm.RegisterMultiSelect("What do you want to edit?", nil, editList,
func(_ string, _, opts []string) ([]int, error) {
return []int{8}, nil
})
pm.RegisterSelect("Visibility", []string{"public", "private", "internal"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "private")
})
pm.RegisterConfirm("Do you want to change visibility to private?", func(_ string, _ bool) (bool, error) {
return true, nil
})
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`
{
"data": {
"repository": {
"visibility": "public",
"description": "description",
"homePageUrl": "https://url.com",
"defaultBranchRef": {
"name": "main"
},
"stargazerCount": 10,
"isInOrganization": false,
"repositoryTopics": {
"nodes": [{
"topic": {
"name": "x"
}
}]
}
}
}
}`))
reg.Register(
httpmock.REST("PATCH", "repos/OWNER/REPO"),
httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
assert.Equal(t, "private", payload["visibility"])
}))
},
},
{
name: "the rest",
opts: EditOptions{
Expand All @@ -250,7 +405,7 @@ func Test_editRun_interactive(t *testing.T) {
promptStubs: func(pm *prompter.MockPrompter) {
pm.RegisterMultiSelect("What do you want to edit?", nil, editList,
func(_ string, _, opts []string) ([]int, error) {
return []int{0, 2, 3, 5, 6, 8, 9}, nil
return []int{0, 2, 3, 5, 6, 9}, nil
})
pm.RegisterInput("Default branch name", func(_, _ string) (string, error) {
return "trunk", nil
Expand All @@ -267,13 +422,6 @@ func Test_editRun_interactive(t *testing.T) {
pm.RegisterConfirm("Convert into a template repository?", func(_ string, _ bool) (bool, error) {
return true, nil
})
pm.RegisterSelect("Visibility", []string{"public", "private", "internal"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "private")
})
pm.RegisterConfirm("Do you want to change visibility to private?", func(_ string, _ bool) (bool, error) {
return true, nil
})
pm.RegisterConfirm("Enable Wikis?", func(_ string, _ bool) (bool, error) {
return true, nil
})
Expand Down Expand Up @@ -310,7 +458,6 @@ func Test_editRun_interactive(t *testing.T) {
assert.Equal(t, "https://zombo.com", payload["homepage"])
assert.Equal(t, true, payload["has_issues"])
assert.Equal(t, true, payload["has_projects"])
assert.Equal(t, "private", payload["visibility"])
assert.Equal(t, true, payload["is_template"])
assert.Equal(t, true, payload["has_wiki"])
}))
Expand Down

0 comments on commit 15b2db9

Please sign in to comment.