remove implied org functionality from secret remove

vilmibm · vilmibm · commit 1675bd9249c7 · 2020-12-10T12:45:38.000-08:00
also fill in missing test cases &gt;_&gt;
diff --git a/pkg/cmd/secret/remove/remove.go b/pkg/cmd/secret/remove/remove.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/MakeNowJust/heredoc"
 	"github.com/cli/cli/api"
 	"github.com/cli/cli/internal/ghinstance"
 	"github.com/cli/cli/internal/ghrepo"
@@ -31,12 +30,7 @@ func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co
 	cmd := &cobra.Command{
 		Use:   "remove <secret name>",
 		Short: "Remove an organization or repository secret",
-		Example: heredoc.Doc(`
-			$ gh secret remove REPO_SECRET
-			$ gh secret remove --org ORG_SECRET
-			$ gh secret remove --org="anotherOrg" ORG_SECRET
-		`),
-		Args: cobra.ExactArgs(1),
+		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// support `-R, --repo` override
 			opts.BaseRepo = f.BaseRepo
@@ -50,8 +44,7 @@ func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co
 			return removeRun(opts)
 		},
 	}
-	cmd.Flags().StringVar(&opts.OrgName, "org", "", "List secrets for an organization")
-	cmd.Flags().Lookup("org").NoOptDefVal = "@owner"
+	cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "List secrets for an organization")
 
 	return cmd
 }
@@ -63,27 +56,24 @@ func removeRun(opts *RemoveOptions) error {
 	}
 	client := api.NewClientFromHTTP(c)
 
+	orgName := opts.OrgName
+
 	var baseRepo ghrepo.Interface
-	if opts.OrgName == "" || opts.OrgName == "@owner" {
+	if orgName == "" {
 		baseRepo, err = opts.BaseRepo()
 		if err != nil {
 			return fmt.Errorf("could not determine base repo: %w", err)
 		}
 	}
 
-	host := ghinstance.OverridableDefault()
-	if opts.OrgName == "@owner" {
-		opts.OrgName = baseRepo.RepoOwner()
-		host = baseRepo.RepoHost()
-	}
-
 	var path string
 	if opts.OrgName == "" {
 		path = fmt.Sprintf("repos/%s/actions/secrets/%s", ghrepo.FullName(baseRepo), opts.SecretName)
 	} else {
 		path = fmt.Sprintf("orgs/%s/actions/secrets/%s", opts.OrgName, opts.SecretName)
 	}
 
+	host := ghinstance.OverridableDefault()
 	err = client.REST(host, "DELETE", path, nil, nil)
 	if err != nil {
 		return fmt.Errorf("failed to delete secret %s: %w", opts.SecretName, err)
diff --git a/pkg/cmd/secret/remove/remove_test.go b/pkg/cmd/secret/remove/remove_test.go
@@ -26,16 +26,15 @@ func TestNewCmdRemove(t *testing.T) {
 			wantsErr: true,
 		},
 		{
-			name: "implicit org",
-			cli:  "cool --org",
+			name: "repo",
+			cli:  "cool",
 			wants: RemoveOptions{
 				SecretName: "cool",
-				OrgName:    "@owner",
 			},
 		},
 		{
-			name: "explicit org",
-			cli:  "cool --org=anOrg",
+			name: "org",
+			cli:  "cool --org anOrg",
 			wants: RemoveOptions{
 				SecretName: "cool",
 				OrgName:    "anOrg",
@@ -109,13 +108,11 @@ func Test_removeRun_org(t *testing.T) {
 		opts *RemoveOptions
 	}{
 		{
-			name: "implicit org",
-			opts: &RemoveOptions{
-				OrgName: "@owner",
-			},
+			name: "repo",
+			opts: &RemoveOptions{},
 		},
 		{
-			name: "explicit org",
+			name: "org",
 			opts: &RemoveOptions{
 				OrgName: "UmbrellaCorporation",
 			},
@@ -126,21 +123,22 @@ func Test_removeRun_org(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			reg := &httpmock.Registry{}
 
-			impliedOrgName := "NeoUmbrella"
-
 			orgName := tt.opts.OrgName
-			if orgName == "@owner" {
-				orgName = impliedOrgName
-			}
 
-			reg.Register(
-				httpmock.REST("DELETE", fmt.Sprintf("orgs/%s/actions/secrets/tVirus", orgName)),
-				httpmock.StatusStringResponse(204, "No Content"))
+			if orgName == "" {
+				reg.Register(
+					httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/tVirus"),
+					httpmock.StatusStringResponse(204, "No Content"))
+			} else {
+				reg.Register(
+					httpmock.REST("DELETE", fmt.Sprintf("orgs/%s/actions/secrets/tVirus", orgName)),
+					httpmock.StatusStringResponse(204, "No Content"))
+			}
 
 			io, _, _, _ := iostreams.Test()
 
 			tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
-				return ghrepo.FromFullName(fmt.Sprintf("%s/repo", impliedOrgName))
+				return ghrepo.FromFullName("owner/repo")
 			}
 			tt.opts.HttpClient = func() (*http.Client, error) {
 				return &http.Client{Transport: reg}, nil
