From e559d97738b6658c1f0a3da23adc8f934cf63990 Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Tue, 1 Dec 2020 11:18:24 -0500 Subject: [PATCH] Send Shopify header for employees (#906) * Add support for Employee Authentication * Remove unnecessary line * Stub so we don't need an existing config * Simplify test * Add test for handling of general errors * Move partners URL building to PartnerAPI --- Gemfile | 1 + Gemfile.lock | 3 ++ lib/project_types/node/commands/create.rb | 13 +++---- lib/project_types/node/messages/messages.rb | 6 --- lib/project_types/rails/commands/create.rb | 13 +++---- lib/project_types/rails/messages/messages.rb | 4 -- lib/shopify-cli/api.rb | 4 +- lib/shopify-cli/messages/messages.rb | 12 ++++++ lib/shopify-cli/partners_api.rb | 18 ++++++++- lib/shopify-cli/task.rb | 8 ++++ lib/shopify-cli/tasks/create_api_client.rb | 9 +++++ lib/shopify-cli/tasks/ensure_env.rb | 3 ++ lib/shopify-cli/tasks/select_org_and_shop.rb | 3 ++ test/minitest_ext.rb | 1 + .../extension/extension_project_test.rb | 1 + test/project_types/node/forms/create_test.rb | 2 + test/project_types/rails/forms/create_test.rb | 6 +++ test/shopify-cli/api_test.rb | 13 +++++++ test/shopify-cli/core/monorail_test.rb | 9 ++++- test/shopify-cli/project_test.rb | 4 +- .../tasks/create_api_client_test.rb | 28 ++++++++++++++ test/shopify-cli/tasks/ensure_env_test.rb | 1 + .../tasks/select_org_and_shop_test.rb | 37 +++++++++++++++++++ test/test_helpers/partners.rb | 7 ++++ 24 files changed, 178 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index 9f3c61b32f..48815b93fa 100644 --- a/Gemfile +++ b/Gemfile @@ -16,6 +16,7 @@ group :test do gem 'mocha', require: false gem 'minitest', '>= 5.0.0', require: false gem 'minitest-reporters', require: false + gem 'minitest-fail-fast', require: false gem 'fakefs', '>= 1.0', require: false gem 'webmock', require: false gem 'timecop', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 87d2affbd6..c9097ce318 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -13,6 +13,8 @@ GEM hashdiff (1.0.1) method_source (1.0.0) minitest (5.14.2) + minitest-fail-fast (0.1.0) + minitest (~> 5) minitest-reporters (1.4.2) ansi builder @@ -63,6 +65,7 @@ DEPENDENCIES byebug fakefs (>= 1.0) minitest (>= 5.0.0) + minitest-fail-fast minitest-reporters mocha pry-byebug diff --git a/lib/project_types/node/commands/create.rb b/lib/project_types/node/commands/create.rb index d581b28d54..4f4ee0e5f8 100644 --- a/lib/project_types/node/commands/create.rb +++ b/lib/project_types/node/commands/create.rb @@ -40,11 +40,11 @@ def call(args, _name) scopes: 'write_products,write_customers,write_draft_orders', ).write(@ctx) - partners_url = "#{partners_endpoint}/#{form.organization_id}/apps/#{api_client['id']}" + partners_url = ShopifyCli::PartnersAPI.partners_url_for(form.organization_id, api_client['id'], local_debug?) - @ctx.puts(@ctx.message('node.create.info.created', form.title, partners_url)) - @ctx.puts(@ctx.message('node.create.info.serve', form.name, ShopifyCli::TOOL_NAME)) - @ctx.puts(@ctx.message('node.create.info.install', partners_url, form.title)) + @ctx.puts(@ctx.message('apps.create.info.created', form.title, partners_url)) + @ctx.puts(@ctx.message('apps.create.info.serve', form.name, ShopifyCli::TOOL_NAME)) + @ctx.puts(@ctx.message('apps.create.info.install', partners_url, form.title)) end def self.help @@ -113,9 +113,8 @@ def build(name) end end - def partners_endpoint - return 'https://partners.myshopify.io' if @ctx.getenv(ShopifyCli::PartnersAPI::LOCAL_DEBUG) - 'https://partners.shopify.com' + def local_debug? + @ctx.getenv(ShopifyCli::PartnersAPI::LOCAL_DEBUG) end end end diff --git a/lib/project_types/node/messages/messages.rb b/lib/project_types/node/messages/messages.rb index d2808f9a76..f2ded270f0 100644 --- a/lib/project_types/node/messages/messages.rb +++ b/lib/project_types/node/messages/messages.rb @@ -33,12 +33,6 @@ module Messages npm_version_failure: "Failed to get the current npm version. Please make sure it is installed as per " \ "the instructions at https://www.npmjs.com/get-npm.", }, - info: { - created: "{{v}} {{green:%s}} was created in your Partner Dashboard {{underline:%s}}", - serve: "{{*}} Change directories to your new project folder {{green:%s}} and run {{command:%s serve}} " \ - "to start a local server", - install: "{{*}} Then, visit {{underline:%s/test}} to install {{green:%s}} on your Dev Store", - }, node_version: "node %s", npm_version: "npm %s", }, diff --git a/lib/project_types/rails/commands/create.rb b/lib/project_types/rails/commands/create.rb index 3593cd18a0..afa097be60 100644 --- a/lib/project_types/rails/commands/create.rb +++ b/lib/project_types/rails/commands/create.rb @@ -55,11 +55,11 @@ def call(args, _name) scopes: 'write_products,write_customers,write_draft_orders', ).write(@ctx) - partners_url = "#{partners_endpoint}/#{form.organization_id}/apps/#{api_client['id']}" + partners_url = ShopifyCli::PartnersAPI.partners_url_for(form.organization_id, api_client['id'], local_debug?) - @ctx.puts(@ctx.message('rails.create.info.created', form.title, partners_url)) - @ctx.puts(@ctx.message('rails.create.info.serve', form.name, ShopifyCli::TOOL_NAME)) - @ctx.puts(@ctx.message('rails.create.info.install', partners_url, form.title)) + @ctx.puts(@ctx.message('apps.create.info.created', form.title, partners_url)) + @ctx.puts(@ctx.message('apps.create.info.serve', form.name, ShopifyCli::TOOL_NAME)) + @ctx.puts(@ctx.message('apps.create.info.install', partners_url, form.title)) end def self.help @@ -173,9 +173,8 @@ def install_gem(name, version = nil) Gem.install(@ctx, name, version) end - def partners_endpoint - return 'https://partners.myshopify.io' if @ctx.getenv(ShopifyCli::PartnersAPI::LOCAL_DEBUG) - 'https://partners.shopify.com' + def local_debug? + @ctx.getenv(ShopifyCli::PartnersAPI::LOCAL_DEBUG) end end end diff --git a/lib/project_types/rails/messages/messages.rb b/lib/project_types/rails/messages/messages.rb index 022ee0c593..b2d2054ebb 100644 --- a/lib/project_types/rails/messages/messages.rb +++ b/lib/project_types/rails/messages/messages.rb @@ -55,10 +55,6 @@ module Messages }, info: { - created: "{{v}} {{green:%s}} was created in your Partner Dashboard {{underline:%s}}", - serve: "{{*}} Change directories to your new project folder {{green:%s}} and run {{command:%s serve}} " \ - "to start a local server", - install: "{{*}} Then, visit {{underline:%s/test}} to install {{green:%s}} on your Dev Store", open_new_shell: "{{*}} {{yellow:After installing %s, please open a new Command Prompt or PowerShell " \ "window to continue.}}", }, diff --git a/lib/shopify-cli/api.rb b/lib/shopify-cli/api.rb index 925a6fd408..1212eaae86 100644 --- a/lib/shopify-cli/api.rb +++ b/lib/shopify-cli/api.rb @@ -91,7 +91,9 @@ def current_sha def default_headers { 'User-Agent' => "Shopify App CLI #{ShopifyCli::VERSION} #{current_sha} | #{ctx.uname}", - }.merge(auth_headers(token)) + }.tap do |headers| + headers['X-Shopify-Cli-Employee'] = '1' if Shopifolk.acting_as_shopify_organization? + end.merge(auth_headers(token)) end def auth_headers(token) diff --git a/lib/shopify-cli/messages/messages.rb b/lib/shopify-cli/messages/messages.rb index 41f81c0b91..d4a19c5591 100644 --- a/lib/shopify-cli/messages/messages.rb +++ b/lib/shopify-cli/messages/messages.rb @@ -3,6 +3,16 @@ module ShopifyCli module Messages MESSAGES = { + apps: { + create: { + info: { + created: "{{v}} {{green:%s}} was created in the organization's Partner Dashboard {{underline:%s}}", + serve: "{{*}} Change directories to your new project folder {{green:%s}} and run {{command:%s serve}} " \ + "to start a local server", + install: "{{*}} Then, visit {{underline:%s/test}} to install {{green:%s}} on your Dev Store", + }, + }, + }, core: { connect: { help: <<~HELP, @@ -296,6 +306,8 @@ module Messages organization_not_found: "Cannot find a partner organization with that ID", partners_notice: "Please visit https://partners.shopify.com/ to create a partners account", }, + first_party: "Are you working on a 1P (1st Party) app?", + identified_as_shopify: "We've identified you as a {{green:Shopify}} employee.", organization: "Partner organization {{green:%s (%s)}}", organization_select: "Select partner organization", }, diff --git a/lib/shopify-cli/partners_api.rb b/lib/shopify-cli/partners_api.rb index 01d6b2da35..c2f20e58b2 100644 --- a/lib/shopify-cli/partners_api.rb +++ b/lib/shopify-cli/partners_api.rb @@ -27,7 +27,7 @@ class << self # #### Parameters # - `ctx`: running context from your command # - `query_name`: name of the query you want to use, loaded from the `lib/graphql` directory. - # - `**variable`: a hash of variables to be supplied to the query ro mutation + # - `**variables`: a hash of variables to be supplied to the query or mutation # # #### Raises # @@ -50,6 +50,13 @@ def query(ctx, query_name, **variables) end end + def partners_url_for(organization_id, api_client_id, local_debug) + if ShopifyCli::Shopifolk.acting_as_shopify_organization? + organization_id = 'internal' + end + "#{partners_endpoint(local_debug)}/#{organization_id}/apps/#{api_client_id}" + end + private def authenticated_req(ctx) @@ -105,6 +112,15 @@ def endpoint return 'https://partners.shopify.com' if ENV[LOCAL_DEBUG].nil? 'https://partners.myshopify.io/' end + + def partners_endpoint(local_debug) + domain = if local_debug + 'partners.myshopify.io' + else + 'partners.shopify.com' + end + "https://#{domain}" + end end def auth_headers(token) diff --git a/lib/shopify-cli/task.rb b/lib/shopify-cli/task.rb index 599d262b3f..bb78463bb1 100644 --- a/lib/shopify-cli/task.rb +++ b/lib/shopify-cli/task.rb @@ -6,5 +6,13 @@ def self.call(*args, **kwargs) task = new task.call(*args, **kwargs) end + + private + + def wants_to_run_against_shopify_org? + @ctx.puts(@ctx.message('core.tasks.select_org_and_shop.identified_as_shopify')) + message = @ctx.message('core.tasks.select_org_and_shop.first_party') + CLI::UI::Prompt.confirm(message, default: false) + end end end diff --git a/lib/shopify-cli/tasks/create_api_client.rb b/lib/shopify-cli/tasks/create_api_client.rb index a589ed18b0..72db07dc68 100644 --- a/lib/shopify-cli/tasks/create_api_client.rb +++ b/lib/shopify-cli/tasks/create_api_client.rb @@ -17,6 +17,15 @@ def call(ctx, org_id:, title:, type:) redir: [OAuth::REDIRECT_HOST] ) + unless resp + ctx.abort("Error - empty response") + end + + errors = resp.dig("errors") + if !errors.nil? && errors.any? + ctx.abort(errors.map { |err| "#{err['field']} #{err['message']}" }.join(", ")) + end + user_errors = resp.dig("data", "appCreate", "userErrors") if !user_errors.nil? && user_errors.any? ctx.abort(user_errors.map { |err| "#{err['field']} #{err['message']}" }.join(", ")) diff --git a/lib/shopify-cli/tasks/ensure_env.rb b/lib/shopify-cli/tasks/ensure_env.rb index 46fd49fdfd..3604de917e 100644 --- a/lib/shopify-cli/tasks/ensure_env.rb +++ b/lib/shopify-cli/tasks/ensure_env.rb @@ -22,6 +22,9 @@ def call(ctx, regenerate: false, required: [:api_key, :secret]) private def fetch_org + if Shopifolk.check && wants_to_run_against_shopify_org? + Shopifolk.act_as_shopify_organization + end orgs = PartnersAPI::Organizations.fetch_with_app(@ctx) org_id = if orgs.count == 1 orgs.first["id"] diff --git a/lib/shopify-cli/tasks/select_org_and_shop.rb b/lib/shopify-cli/tasks/select_org_and_shop.rb index ef98c3d4b5..52ca4a1ec4 100644 --- a/lib/shopify-cli/tasks/select_org_and_shop.rb +++ b/lib/shopify-cli/tasks/select_org_and_shop.rb @@ -8,6 +8,9 @@ class SelectOrgAndShop < ShopifyCli::Task def call(ctx, organization_id: nil, shop_domain: nil) @ctx = ctx return response(organization_id.to_i, shop_domain) unless organization_id.nil? || shop_domain.nil? + if Shopifolk.check && wants_to_run_against_shopify_org? + Shopifolk.act_as_shopify_organization + end org = get_organization(organization_id) shop_domain ||= get_shop_domain(org) ShopifyCli::Core::Monorail.metadata[:organization_id] = org["id"].to_i diff --git a/test/minitest_ext.rb b/test/minitest_ext.rb index 7e9c9c351f..8591239cc5 100644 --- a/test/minitest_ext.rb +++ b/test/minitest_ext.rb @@ -22,6 +22,7 @@ def setup end def teardown + ShopifyCli::Shopifolk.reset # Some tests stub the File class, but we need to call the real methods when checking if the config file has # changed. # diff --git a/test/project_types/extension/extension_project_test.rb b/test/project_types/extension/extension_project_test.rb index f2381d61e8..dc8d82ed79 100644 --- a/test/project_types/extension/extension_project_test.rb +++ b/test/project_types/extension/extension_project_test.rb @@ -12,6 +12,7 @@ def test_write_cli_file_create_shopify_cli_yml_file FileUtils.cd(new_context.root) ExtensionProject.write_cli_file(context: new_context, type: @test_extension_type.identifier) + ::ShopifyCli::Project.clear assert File.exist?('.shopify-cli.yml') assert_equal :extension, ShopifyCli::Project.current_project_type diff --git a/test/project_types/node/forms/create_test.rb b/test/project_types/node/forms/create_test.rb index 2f2be4c35c..11ae6aedce 100644 --- a/test/project_types/node/forms/create_test.rb +++ b/test/project_types/node/forms/create_test.rb @@ -8,6 +8,8 @@ class CreateTest < MiniTest::Test def setup super + stub_shopify_org_confirmation + ShopifyCli::Shopifolk.stubs(:check) ShopifyCli::ProjectType.load_type(:node) end diff --git a/test/project_types/rails/forms/create_test.rb b/test/project_types/rails/forms/create_test.rb index 7a021edef1..7a527d72da 100644 --- a/test/project_types/rails/forms/create_test.rb +++ b/test/project_types/rails/forms/create_test.rb @@ -6,6 +6,12 @@ module Forms class CreateTest < MiniTest::Test include TestHelpers::Partners + def setup + super + ShopifyCli::Shopifolk.stubs(:check) + stub_shopify_org_confirmation + end + def test_returns_all_defined_attributes_if_valid form = ask assert_equal('test_app', form.name) diff --git a/test/shopify-cli/api_test.rb b/test/shopify-cli/api_test.rb index 8c24cec6b2..0c4fb8df62 100644 --- a/test/shopify-cli/api_test.rb +++ b/test/shopify-cli/api_test.rb @@ -139,5 +139,18 @@ def test_load_query_will_not_read_project_type_queries_if_not_in_project File.expects(:read).with(expected_path).returns('content') assert_equal('content', new_api.call_load_query('my_query')) end + + def test_include_shopify_cli_header_if_acting_as_shopify_organization + Shopifolk.act_as_shopify_organization + File.stubs(:read) + .with(File.join(ShopifyCli::ROOT, "lib/graphql/api/mutation.graphql")) + .returns(@mutation) + response = stub('response', code: '200', body: '{}') + HttpRequest + .expects(:call) + .with(anything, @mutation, {}, has_entry({ 'X-Shopify-Cli-Employee' => '1' })) + .returns(response) + @api.query('api/mutation') + end end end diff --git a/test/shopify-cli/core/monorail_test.rb b/test/shopify-cli/core/monorail_test.rb index b4bee26704..9e545eb8ff 100644 --- a/test/shopify-cli/core/monorail_test.rb +++ b/test/shopify-cli/core/monorail_test.rb @@ -6,7 +6,7 @@ module Core class MonorailTest < MiniTest::Test def setup super - CLI::UI::Prompt.stubs(:confirm).returns(true) + stub_shopify_org_confirmation ShopifyCli::Core::Monorail.metadata = {} end @@ -190,6 +190,13 @@ def enabled_and_consented(enabled, consented) ShopifyCli::Context.any_instance.stubs(:system?).returns(enabled) ShopifyCli::Config.stubs(:get_bool).with('analytics', 'enabled').returns(consented) end + + def stub_shopify_org_confirmation(response: true) + CLI::UI::Prompt + .stubs(:confirm) + .with(includes("Are you working a 1P (1st Party) app?"), anything) + .returns(response) + end end end end diff --git a/test/shopify-cli/project_test.rb b/test/shopify-cli/project_test.rb index 55bd47fb39..c41ab84bf5 100644 --- a/test/shopify-cli/project_test.rb +++ b/test/shopify-cli/project_test.rb @@ -29,11 +29,12 @@ def test_current_fails_if_no_config end def test_write_writes_yaml - create_empty_config Shopifolk.stubs(:acting_as_shopify_organization?).returns(false) + Dir.stubs(:pwd).returns(@context.root) ShopifyCli::Project.write(@context, project_type: :node, organization_id: 42) assert_equal :node, Project.current.config['project_type'] assert_equal 42, Project.current.config['organization_id'] + refute Project.current.config['shopify_organization'] end def test_write_writes_yaml_with_shopify_organization_field @@ -59,6 +60,7 @@ def test_write_includes_identifiers organization_id: 42, other_option: true, ) + Project.clear assert Project.current.config['other_option'] end diff --git a/test/shopify-cli/tasks/create_api_client_test.rb b/test/shopify-cli/tasks/create_api_client_test.rb index 83288d9f06..4f08d9ecb5 100644 --- a/test/shopify-cli/tasks/create_api_client_test.rb +++ b/test/shopify-cli/tasks/create_api_client_test.rb @@ -44,6 +44,34 @@ def test_call_will_query_partners_dashboard assert_equal("newapikey", ShopifyCli::Core::Monorail.metadata[:api_key]) end + def test_call_will_return_any_general_errors + stub_partner_req( + 'create_app', + variables: { + org: 42, + title: 'Test app', + type: 'public', + app_url: ShopifyCli::Tasks::CreateApiClient::DEFAULT_APP_URL, + redir: ["http://127.0.0.1:3456"], + }, + resp: { + 'errors': [ + { 'field': 'title', 'message': 'is not a valid title' }, + ], + } + ) + + err = assert_raises ShopifyCli::Abort do + Tasks::CreateApiClient.call( + @context, + org_id: 42, + title: 'Test app', + type: 'public', + ) + end + assert_equal("{{x}} title is not a valid title", err.message) + end + def test_call_will_return_any_user_errors stub_partner_req( 'create_app', diff --git a/test/shopify-cli/tasks/ensure_env_test.rb b/test/shopify-cli/tasks/ensure_env_test.rb index 6916d3da1d..f7aaa66fe4 100644 --- a/test/shopify-cli/tasks/ensure_env_test.rb +++ b/test/shopify-cli/tasks/ensure_env_test.rb @@ -12,6 +12,7 @@ def setup Project.write(@context, project_type: :fake, organization_id: 42) FileUtils.cd(@context.root) ShopifyCli::Tunnel.stubs(:start) + Shopifolk.stubs(:check) end def test_create_new_app_if_none_available diff --git a/test/shopify-cli/tasks/select_org_and_shop_test.rb b/test/shopify-cli/tasks/select_org_and_shop_test.rb index b2a49d4b56..3b448dee37 100644 --- a/test/shopify-cli/tasks/select_org_and_shop_test.rb +++ b/test/shopify-cli/tasks/select_org_and_shop_test.rb @@ -3,6 +3,14 @@ module ShopifyCli module Tasks class SelectOrgAndShopTest < MiniTest::Test + include TestHelpers::Partners + + def setup + super + stub_shopify_org_confirmation + Shopifolk.stubs(:check).returns(false) # note that we re-stub this in some tests below + end + def teardown ShopifyCli::Core::Monorail.metadata = {} super @@ -139,6 +147,35 @@ def test_prompts_user_to_pick_from_shops assert_equal('selected', form[:shop_domain]) end + def test_persists_organization_preference_if_chosen + ShopifyCli::PartnersAPI::Organizations.expects(:fetch).with(@context, id: 123).returns({ + "id" => 123, + "stores" => [ + { "shopDomain" => "shopdomain.myshopify.com" }, + ], + }) + + stub_shopify_org_confirmation(response: true) + Shopifolk.stubs(:check).returns(true) + call(org_id: 123, shop: nil) + + assert(Shopifolk.acting_as_shopify_organization?) + end + + def test_does_not_persist_organization_preference_if_not_chosen + ShopifyCli::PartnersAPI::Organizations.expects(:fetch).with(@context, id: 123).returns({ + "id" => 123, + "stores" => [ + { "shopDomain" => "shopdomain.myshopify.com" }, + ], + }) + stub_shopify_org_confirmation(response: false) + Shopifolk.stubs(:check).returns(true) + call(org_id: 123, shop: nil) + + refute(Shopifolk.acting_as_shopify_organization?) + end + private def call(org_id: 421, shop: 'store.myshopify.com') diff --git a/test/test_helpers/partners.rb b/test/test_helpers/partners.rb index 7fec353c71..513612a96d 100644 --- a/test/test_helpers/partners.rb +++ b/test/test_helpers/partners.rb @@ -21,5 +21,12 @@ def stub_partner_req(query, variables: {}, status: 200, resp: {}) variables: variables, }.to_json).to_return(status: status, body: resp.to_json) end + + def stub_shopify_org_confirmation(response: false) + CLI::UI::Prompt + .stubs(:confirm) + .with(includes("Are you working on a 1P (1st Party) app?"), anything) + .returns(response) + end end end