From c5f6b6c8cbf2f01db6ad62271fad3d9e47bc76b2 Mon Sep 17 00:00:00 2001 From: Tim Anema Date: Thu, 23 Jul 2020 15:07:26 -0400 Subject: [PATCH 1/3] Making theme ID required to make it harder to edit a live theme --- src/cmdutil/_testdata/config.yml | 2 ++ src/cmdutil/util_test.go | 12 ++++++------ src/env/_testdata/projectdir/valid_config.yml | 1 + src/env/conf_test.go | 6 +++--- src/env/env.go | 11 +++++------ src/env/env_test.go | 6 +++--- 6 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/cmdutil/_testdata/config.yml b/src/cmdutil/_testdata/config.yml index e22a6f75..3ccc1a61 100644 --- a/src/cmdutil/_testdata/config.yml +++ b/src/cmdutil/_testdata/config.yml @@ -1,4 +1,5 @@ development: + theme_id: 123 password: abracadabra store: store.myshopify.com ignore_files: @@ -6,6 +7,7 @@ development: - bulbasaur - squirtle production: + theme_id: 123 password: abracadabra store: store.myshopify.com ignore_files: diff --git a/src/cmdutil/util_test.go b/src/cmdutil/util_test.go index 5995542c..e5a57cf0 100644 --- a/src/cmdutil/util_test.go +++ b/src/cmdutil/util_test.go @@ -96,7 +96,7 @@ func TestCtx_DoneTask(t *testing.T) { func TestGenerateContexts(t *testing.T) { factory := func(*env.Env) (shopifyClient, error) { return nil, nil } _, err := generateContexts(factory, nil, Flags{Environments: []string{"development"}}, []string{}) - assert.EqualError(t, err, "invalid environment [development]: (missing store domain,missing password)") + assert.EqualError(t, err, "invalid environment [development]: (missing theme_id,missing store domain,missing password)") client := new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, nil } @@ -109,7 +109,7 @@ func TestGenerateContexts(t *testing.T) { client = new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, nil } _, err = generateContexts(factory, nil, Flags{ConfigPath: "_testdata/config.yml", Environments: []string{"nope"}}, []string{}) - assert.EqualError(t, err, "invalid environment [nope]: (missing store domain,missing password)") + assert.EqualError(t, err, "invalid environment [nope]: (missing theme_id,missing store domain,missing password)") client = new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, fmt.Errorf("not today") } @@ -302,7 +302,7 @@ func TestForDefaultClient(t *testing.T) { factory := func(*env.Env) (shopifyClient, error) { return nil, nil } err := forDefaultClient(factory, Flags{}, []string{}, safeHandler) - assert.EqualError(t, err, "invalid environment [development]: (missing store domain,missing password)") + assert.EqualError(t, err, "invalid environment [development]: (missing theme_id,missing store domain,missing password)") client := new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, nil } @@ -315,19 +315,19 @@ func TestForDefaultClient(t *testing.T) { factory = func(*env.Env) (shopifyClient, error) { return client, nil } client.On("GetShop").Return(shopify.Shop{}, nil) client.On("Themes").Return([]shopify.Theme{}, nil) - err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123"}, []string{}, safeHandler) + err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123", ThemeID: "123"}, []string{}, safeHandler) assert.Nil(t, err) client = new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, fmt.Errorf("server err") } - err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123"}, []string{}, safeHandler) + err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123", ThemeID: "123"}, []string{}, safeHandler) assert.EqualError(t, err, "server err") client = new(mocks.ShopifyClient) factory = func(*env.Env) (shopifyClient, error) { return client, nil } client.On("GetShop").Return(shopify.Shop{}, nil) client.On("Themes").Return([]shopify.Theme{}, nil) - err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123"}, []string{}, errHandler) + err = forDefaultClient(factory, Flags{Domain: "shop.myshopify.com", Password: "123", ThemeID: "123"}, []string{}, errHandler) assert.EqualError(t, err, gandalfErr.Error()) stdErr := bytes.NewBufferString("") diff --git a/src/env/_testdata/projectdir/valid_config.yml b/src/env/_testdata/projectdir/valid_config.yml index fcdc1c44..a9557034 100644 --- a/src/env/_testdata/projectdir/valid_config.yml +++ b/src/env/_testdata/projectdir/valid_config.yml @@ -1,4 +1,5 @@ development: + theme_id: 123 password: ${STOREPASS} store: $STOREDOMAIN ignore_files: diff --git a/src/env/conf_test.go b/src/env/conf_test.go index b2d94ddf..33e7c6a7 100644 --- a/src/env/conf_test.go +++ b/src/env/conf_test.go @@ -78,8 +78,8 @@ func TestConf_Set(t *testing.T) { }{ {name: "", initial: Env{}, err: ErrInvalidEnvironmentName.Error()}, {name: "development", initial: Env{}, err: "invalid environment"}, - {name: "development", initial: Env{Domain: "yes.myshopify.com", Password: "abc123"}, expected: Env{Name: "development", Domain: "yes.myshopify.com", Password: "abc123", Directory: Default.Directory, Timeout: Default.Timeout}}, - {name: "development", initial: Env{Domain: "yes.myshopify.com", Password: "abc123", Directory: filepath.Join("..", "file")}, expected: Env{Name: "development", Domain: "yes.myshopify.com", Password: "abc123", Directory: dir, Timeout: Default.Timeout}}, + {name: "development", initial: Env{ThemeID: "123", Domain: "yes.myshopify.com", Password: "abc123"}, expected: Env{ThemeID: "123", Name: "development", Domain: "yes.myshopify.com", Password: "abc123", Directory: Default.Directory, Timeout: Default.Timeout}}, + {name: "development", initial: Env{ThemeID: "123", Domain: "yes.myshopify.com", Password: "abc123", Directory: filepath.Join("..", "file")}, expected: Env{ThemeID: "123", Name: "development", Domain: "yes.myshopify.com", Password: "abc123", Directory: dir, Timeout: Default.Timeout}}, {name: "development", initial: Env{Domain: "yes.myshopify.com", Password: "abc123"}, overrides: []Env{{ThemeID: "12345"}}, expected: Env{Name: "development", Domain: "yes.myshopify.com", Password: "abc123", ThemeID: "12345", Directory: Default.Directory, Timeout: Default.Timeout}}, } @@ -108,7 +108,7 @@ func TestConf_Get(t *testing.T) { err error overrides []Env }{ - {path: "_testdata/projectdir/valid_config.yml", toGet: "development", err: nil}, + {path: "_testdata/projectdir/valid_config.yml", toGet: "development", err: nil, themeid: "123"}, {path: "_testdata/projectdir/valid_config.yml", toGet: "development", err: nil, themeid: "12345", overrides: []Env{{ThemeID: "12345"}}}, {path: "_testdata/projectdir/valid_config.yml", toGet: "nope", err: ErrEnvDoesNotExist}, {path: "_testdata/projectdir/bad_format.yml", toGet: "other", err: ErrEnvNotDefined}, diff --git a/src/env/env.go b/src/env/env.go index c8a2c9c9..ed133497 100644 --- a/src/env/env.go +++ b/src/env/env.go @@ -50,12 +50,11 @@ func (env *Env) validate() error { errors := []string{} env.ThemeID = strings.ToLower(strings.TrimSpace(env.ThemeID)) - if env.ThemeID != "" { - if env.ThemeID == "live" { - env.ThemeID = "" - } else if _, err := strconv.ParseInt(env.ThemeID, 10, 64); err != nil { - errors = append(errors, "invalid theme_id") - } + + if env.ThemeID == "" { + errors = append(errors, "missing theme_id") + } else if _, err := strconv.ParseInt(env.ThemeID, 10, 64); err != nil { + errors = append(errors, "invalid theme_id") } if len(env.Domain) == 0 { diff --git a/src/env/env_test.go b/src/env/env_test.go index 19259664..59052a8e 100644 --- a/src/env/env_test.go +++ b/src/env/env_test.go @@ -45,13 +45,13 @@ func TestEnv_Validate(t *testing.T) { notwindows bool }{ {env: Env{Password: "file", ThemeID: "123", Domain: "test.myshopify.com"}}, - {env: Env{Password: "file", ThemeID: "live", Domain: "test.myshopify.com"}}, + {env: Env{Password: "file", ThemeID: "live", Domain: "test.myshopify.com"}, err: "invalid theme_id"}, {env: Env{ThemeID: "123", Domain: "test.myshopify.com"}, err: "missing password"}, {env: Env{Password: "test", ThemeID: "123", Domain: "test.nope.com"}, err: "invalid store domain"}, {env: Env{Password: "test", ThemeID: "123"}, err: "missing store domain"}, - {env: Env{Password: "test", Domain: "test.myshopify.com"}}, + {env: Env{Password: "test", Domain: "test.myshopify.com"}, err: "missing theme_id"}, {env: Env{Password: "file", ThemeID: "abc", Domain: "test.myshopify.com"}, err: "invalid theme_id"}, - {notwindows: true, env: Env{Password: "abc123", Domain: "test.myshopify.com", Directory: filepath.Join("_testdata", "symlink_projectdir")}}, + {notwindows: true, env: Env{Password: "abc123", Domain: "test.myshopify.com", ThemeID: "123", Directory: filepath.Join("_testdata", "symlink_projectdir")}}, {notwindows: true, env: Env{Password: "abc123", Domain: "test.myshopify.com", Directory: filepath.Join("_testdata", "bad_symlink")}, err: "invalid project symlink"}, {notwindows: true, env: Env{Password: "abc123", Domain: "test.myshopify.com", Directory: filepath.Join("_testdata", "symlink_file")}, err: "is not a directory"}, {env: Env{Password: "abc123", Domain: "test.myshopify.com", Directory: "not_a_dir"}, err: "invalid project directory"}, From 6babce3279993a038f7a397d60970febc066c4ea Mon Sep 17 00:00:00 2001 From: Tim Anema Date: Thu, 23 Jul 2020 15:14:38 -0400 Subject: [PATCH 2/3] Update documentation --- docs/commands/index.md | 3 +-- docs/configuration/index.md | 2 +- src/cmdutil/util.go | 3 +-- src/cmdutil/util_test.go | 2 +- src/shopify/theme_client.go | 4 ++-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/commands/index.md b/docs/commands/index.md index 1a6870b6..c75823e9 100644 --- a/docs/commands/index.md +++ b/docs/commands/index.md @@ -104,8 +104,7 @@ development: |**Required Flags**|| |`-p`|`--password`| Password for access to your Shopify account. |`-s`|`--store `| Your store's domain for changes to take effect -|**Optional Flags**|| -|`-t`|`--themeid `| The ID of the theme that you want changes to take effect, if no theme id is passed, your live theme will be fetched +|`-t`|`--themeid `| The ID of the theme that you want changes to take effect. ## New diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 59c44eaf..5728ab2d 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -15,7 +15,7 @@ There are general values that you will be able to config for all of Theme Kit ac | config | A custom path to your config file. This defaults to the current path that the command is run in. | store | Your store's Shopify domain with the `.myshopify.com` postfix. Please see the [setup docs]({{ '/#get-api-access' | prepend: site.baseurl }}) on how to get this value. | password | Your API password. Please see the [setup docs]({{ '/#get-api-access' | prepend: site.baseurl }}) on how to get this value. -| theme_id | The theme that you want the command to take effect on. If you want to make changes to the current live theme you may set this value to `'live'`. Please see the [setup docs]({{ '/#get-api-access' | prepend: site.baseurl }}) on how to get this value. +| theme_id | The theme that you want the command to take effect on. Please see the [setup docs]({{ '/#get-api-access' | prepend: site.baseurl }}) on how to get this value. | directory | The project root directory. This allows you to run the command from another directory. | ignore_files | A list of patterns to ignore when executing commands. Please see the [Ignore Patterns]({{ '/ignores' | prepend: site.baseurl }}) documentation. | ignores | A list of file paths to files that contain ignore patterns. Please see the [Ignore Patterns]({{ '/ignores' | prepend: site.baseurl }}) documentation. diff --git a/src/cmdutil/util.go b/src/cmdutil/util.go index 85f82386..e1474ace 100644 --- a/src/cmdutil/util.go +++ b/src/cmdutil/util.go @@ -109,8 +109,7 @@ func createCtx(newClient clientFact, conf env.Conf, e *env.Env, flags Flags, arg if setTheme { for _, theme := range themes { if theme.Role == "main" { - if fmt.Sprintf("%v", theme.ID) == e.ThemeID || e.ThemeID == "" { - e.ThemeID = fmt.Sprintf("%v", theme.ID) // record the theme id for the live id + if fmt.Sprintf("%v", theme.ID) == e.ThemeID { colors.ColorStdOut.Printf( "[%s] Warning, this is the live theme on %s.", colors.Yellow(e.Name), diff --git a/src/cmdutil/util_test.go b/src/cmdutil/util_test.go index e5a57cf0..9393a638 100644 --- a/src/cmdutil/util_test.go +++ b/src/cmdutil/util_test.go @@ -50,7 +50,7 @@ func TestCreateCtx(t *testing.T) { assert.Contains(t, err.Error(), "[API] Invalid API key or access token (unrecognized login or wrong password)") } - e = &env.Env{Proxy: "http://localhost:3000"} + e = &env.Env{ThemeID: "1234", Proxy: "http://localhost:3000"} client = new(mocks.ShopifyClient) client.On("GetShop").Return(shopify.Shop{}, nil) client.On("Themes").Return([]shopify.Theme{{ID: 65443, Role: "unpublished"}, {ID: 1234, Role: "main"}}, nil) diff --git a/src/shopify/theme_client.go b/src/shopify/theme_client.go index ba35198e..ac4b678a 100644 --- a/src/shopify/theme_client.go +++ b/src/shopify/theme_client.go @@ -26,9 +26,9 @@ var ( ErrMalformedResponse = errors.New("received a malformed response from shopify, this usually indicates a problem with your connection") // ErrZipPathRequired is returned if a source path was not provided to create a new theme ErrZipPathRequired = errors.New("theme zip path is required") - // ErrInfoWithoutThemeID will be returned if GetInfo is called on a live theme + // ErrInfoWithoutThemeID will be returned if GetInfo is called without a theme ID ErrInfoWithoutThemeID = errors.New("cannot get info without a theme id") - // ErrPublishWithoutThemeID will be returned if PublishTheme is called on a live theme + // ErrPublishWithoutThemeID will be returned if PublishTheme is called without a theme ID ErrPublishWithoutThemeID = errors.New("cannot publish a theme without a theme id set") // ErrThemeNotFound will be returned if trying to get a theme that does not exist ErrThemeNotFound = errors.New("requested theme was not found") From bf29ac5d0b39b860dfd4dd8c793ba38b087b3a2f Mon Sep 17 00:00:00 2001 From: Tim Anema Date: Thu, 23 Jul 2020 15:33:15 -0400 Subject: [PATCH 3/3] Disallow changes to live theme without override --- cmd/get.go | 4 ++++ cmd/theme.go | 1 + src/cmdutil/util.go | 15 +++++++++++++-- src/cmdutil/util_test.go | 2 +- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/cmd/get.go b/cmd/get.go index caf82a9c..11b10303 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -28,6 +28,10 @@ var getCmd = &cobra.Command{ For more documentation please see http://shopify.github.io/themekit/commands/#get `, RunE: func(cmd *cobra.Command, args []string) error { + // This is a hack to get around theme ID validation for the list operation which doesnt need it + if flags.List { + flags.ThemeID = "1337" + } return cmdutil.ForDefaultClient(flags, args, getTheme) }, } diff --git a/cmd/theme.go b/cmd/theme.go index 2584daf7..b4d58ece 100644 --- a/cmd/theme.go +++ b/cmd/theme.go @@ -88,6 +88,7 @@ func init() { ThemeCmd.PersistentFlags().StringArrayVar(&flags.IgnoredFiles, "ignored-file", []string{}, "A single file to ignore, use the flag multiple times to add multiple.") ThemeCmd.PersistentFlags().StringArrayVar(&flags.Ignores, "ignores", []string{}, "A path to a file that contains ignore patterns.") ThemeCmd.PersistentFlags().BoolVar(&flags.DisableIgnore, "no-ignore", false, "Will disable config ignores so that all files can be changed") + ThemeCmd.PersistentFlags().BoolVar(&flags.AllowLive, "allow-live", false, "Will allow themekit to make changes to the live theme on the store.") watchCmd.Flags().StringVarP(&flags.NotifyFile, "notify", "n", "", "file to touch when workers have gone idle") watchCmd.Flags().BoolVarP(&flags.AllEnvs, "allenvs", "a", false, "run command with all environments") diff --git a/src/cmdutil/util.go b/src/cmdutil/util.go index e1474ace..8154b93a 100644 --- a/src/cmdutil/util.go +++ b/src/cmdutil/util.go @@ -21,7 +21,10 @@ import ( ) // ErrReload is an error to return from a command if you want to reload and run again -var ErrReload = errors.New("reloading config") +var ( + ErrReload = errors.New("reloading config") + ErrLiveTheme = errors.New("cannot make changes to a live theme without an override") +) // Flags encapsulates all the possible flags that can be set in the themekit // command line. Some of the values are used across different commands @@ -50,6 +53,7 @@ type Flags struct { With string List bool NoDelete bool + AllowLive bool } // Ctx is a specific context that a command will run in @@ -109,12 +113,19 @@ func createCtx(newClient clientFact, conf env.Conf, e *env.Env, flags Flags, arg if setTheme { for _, theme := range themes { if theme.Role == "main" { - if fmt.Sprintf("%v", theme.ID) == e.ThemeID { + if fmt.Sprintf("%v", theme.ID) == e.ThemeID && flags.AllowLive { colors.ColorStdOut.Printf( "[%s] Warning, this is the live theme on %s.", colors.Yellow(e.Name), colors.Yellow(shop.Name), ) + } else if fmt.Sprintf("%v", theme.ID) == e.ThemeID && !flags.AllowLive { + colors.ColorStdOut.Printf( + "[%s] This is the live theme on %s. If you wish to make changes to it, then you will have to pass the --allow-live flag", + colors.Red(e.Name), + colors.Yellow(shop.Name), + ) + return &Ctx{}, ErrLiveTheme } break } diff --git a/src/cmdutil/util_test.go b/src/cmdutil/util_test.go index 9393a638..1609878c 100644 --- a/src/cmdutil/util_test.go +++ b/src/cmdutil/util_test.go @@ -55,7 +55,7 @@ func TestCreateCtx(t *testing.T) { client.On("GetShop").Return(shopify.Shop{}, nil) client.On("Themes").Return([]shopify.Theme{{ID: 65443, Role: "unpublished"}, {ID: 1234, Role: "main"}}, nil) _, err = createCtx(factory, env.Conf{}, e, Flags{DisableIgnore: true}, []string{}, nil, true) - assert.Nil(t, err) + assert.Equal(t, ErrLiveTheme, err) assert.Equal(t, e.ThemeID, "1234") }