Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't run app config initializer for generator #1144

Merged
merged 7 commits into from
Jan 18, 2021
Merged

Don't run app config initializer for generator #1144

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d7a1f0c
Don't run app config for generator
andyw8 Jan 17, 2021
10c8941
Improve error message
andyw8 Jan 17, 2021
8f95799
Fix tests
andyw8 Jan 17, 2021
bb2cb0d
Update README
andyw8 Jan 17, 2021
b5fe5f7
Update CHANGELOG
andyw8 Jan 17, 2021
c7a8daf
Update README
andyw8 Jan 17, 2021
a563c79
Fix URL
andyw8 Jan 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased
----------

* Don't attempt to read Shopify environment variables when the generators are running, since they may not be present yet [#1144](https://github.com/Shopify/shopify_app/pull/1144)

17.0.0 (January 13, 2021)
------
* Rails 6.1 is not yet supported [#1134](https://github.com/Shopify/shopify_app/pull/1134)
Expand Down
10 changes: 6 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,19 @@ Generators
----------

### API Keys
<!-- This anchor name `#api-keys` is linked to from user output in `templates/shopify_app.rb.tt` so beware of changing -->
Before starting the app, you'll need to ensure it can read the Shopify environment variables `SHOPIFY_API_KEY` and `SHOPIFY_API_SECRET`.

Before running the generators, you'll need to ensure your app can read the Shopify environment variables `SHOPIFY_API_KEY` and `SHOPIFY_API_SECRET`.

A common approach is to use the [dotenv-rails](https://github.com/bkeepers/dotenv) gem, along with an `.env` file in the following format:
In a development environment, a common approach is to use the [dotenv-rails](https://github.com/bkeepers/dotenv) gem, along with an `.env` file in the following format:

```
SHOPIFY_API_KEY=your api key
SHOPIFY_API_SECRET=your api secret
```

These values can be found on the "App Setup" page in the [Shopify Partners Dashboard][dashboard]. If you are checking your code into a code repository, ensure your `.gitignore` prevents your `.env` file from being checked into any publicly accessible code.
These values can be found on the "App Setup" page in the [Shopify Partners Dashboard][dashboard].
(If you are using [shopify-app-cli](https://github.com/Shopify/) this `.env` file will be created automatically).
If you are checking your code into a code repository, ensure your `.gitignore` prevents your `.env` file from being checked into any publicly accessible code.

### Default Generator

Expand Down
28 changes: 15 additions & 13 deletions lib/generators/shopify_app/install/templates/shopify_app.rb.tt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
ShopifyApp.configure do |config|
config.application_name = "<%= @application_name %>"
config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence || raise('Missing SHOPIFY_API_KEY')
config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence || raise('Missing SHOPIFY_API_SECRET')
config.old_secret = "<%= @old_secret %>"
config.scope = "<%= @scope %>" # Consult this page for more scope options:
# https://help.shopify.com/en/api/getting-started/authentication/oauth/scopes
config.embedded_app = <%= embedded_app? %>
config.after_authenticate_job = false
config.api_version = "<%= @api_version %>"
config.shop_session_repository = 'Shop'
config.allow_jwt_authentication = <%= !with_cookie_authentication? %>
config.allow_cookie_authentication = <%= with_cookie_authentication? %>
unless defined? Rails::Generators
Copy link

@xiaoboa xiaoboa Jan 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyw8 with this change, the reference of ShopifyApp.configuration in generator code(not template code) should also be removed, there is one at home_controller_generator.rb

      def embedded_app?
        ShopifyApp.configuration.embedded_app?
      end

the method embedded_app? is no longer needed any more.
Otherwise the generated code of HomeController by default will be an authenticated home_controller instead of the unauthenticated home_controller, which is conflict with the README.md

By default, this generator creates an unauthenticated home_controller and a sample protected products_controller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point @xiaoboa, I'm actually investigating that myself. We still need need the method if we're creating non-embedded apps but I'll make sure that home_controller is not authenticated.

Thanks!

ShopifyApp.configure do |config|
config.application_name = "<%= @application_name %>"
config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence || raise('Missing SHOPIFY_API_KEY. See https://github.com/Shopify/shopify_app#api-keys')
config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence || raise('Missing SHOPIFY_API_SECRET. See https://github.com/Shopify/shopify_app#api-keys')
config.old_secret = "<%= @old_secret %>"
config.scope = "<%= @scope %>" # Consult this page for more scope options:
# https://help.shopify.com/en/api/getting-started/authentication/oauth/scopes
config.embedded_app = <%= embedded_app? %>
config.after_authenticate_job = false
config.api_version = "<%= @api_version %>"
config.shop_session_repository = 'Shop'
config.allow_jwt_authentication = <%= !with_cookie_authentication? %>
config.allow_cookie_authentication = <%= with_cookie_authentication? %>
end
end

# ShopifyApp::Utils.fetch_known_api_versions # Uncomment to fetch known api versions from shopify servers on boot
Expand Down
18 changes: 6 additions & 12 deletions test/generators/install_generator_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ class InstallGeneratorTest < Rails::Generators::TestCase
run_generator
assert_file "config/initializers/shopify_app.rb" do |shopify_app|
assert_match 'config.application_name = "My Shopify App"', shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence " \
"|| raise('Missing SHOPIFY_API_KEY')", shopify_app
assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence " \
"|| raise('Missing SHOPIFY_API_SECRET')", shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '')", shopify_app
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than copy the exact updated template code into the test, I've simplified this to check for just the 'essential' part. Verifying the template code doesn't prove that the generator works – ideally we should generate an app as part of the test suite.

assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '')", shopify_app
assert_match 'config.scope = "read_products"', shopify_app
assert_match "config.embedded_app = true", shopify_app
assert_match 'config.api_version = "2019-10"', shopify_app
Expand All @@ -41,10 +39,8 @@ class InstallGeneratorTest < Rails::Generators::TestCase
--api_version unstable --scope read_orders write_products)
assert_file "config/initializers/shopify_app.rb" do |shopify_app|
assert_match 'config.application_name = "Test Name"', shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence " \
"|| raise('Missing SHOPIFY_API_KEY')", shopify_app
assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence " \
"|| raise('Missing SHOPIFY_API_SECRET')", shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '')", shopify_app
assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '')", shopify_app
assert_match 'config.scope = "read_orders write_products"', shopify_app
assert_match 'config.embedded_app = true', shopify_app
assert_match 'config.api_version = "unstable"', shopify_app
Expand All @@ -56,10 +52,8 @@ class InstallGeneratorTest < Rails::Generators::TestCase
run_generator %w(--application_name Test Name --scope read_orders write_products)
assert_file "config/initializers/shopify_app.rb" do |shopify_app|
assert_match 'config.application_name = "Test Name"', shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '').presence " \
"|| raise('Missing SHOPIFY_API_KEY')", shopify_app
assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '').presence " \
"|| raise('Missing SHOPIFY_API_SECRET')", shopify_app
assert_match "config.api_key = ENV.fetch('SHOPIFY_API_KEY', '')", shopify_app
assert_match "config.secret = ENV.fetch('SHOPIFY_API_SECRET', '')", shopify_app
assert_match 'config.scope = "read_orders write_products"', shopify_app
assert_match 'config.embedded_app = true', shopify_app
assert_match "config.shop_session_repository = 'Shop'", shopify_app
Expand Down