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.

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

Installer UI improvements #4675

Merged
merged 12 commits into from
Oct 17, 2022
Merged
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ jobs:
command: "sudo apt-get update"
- libvips

- install_solidus: { flags: "--sample=false" }
- install_solidus: { flags: "--sample=false --frontend=starter --authentication=devise" }
- test_page: { expected_text: "The only eCommerce platform you’ll ever need." }

- install_solidus: { flags: "--sample=false --frontend=none --authentication=none" }
Expand Down
1 change: 0 additions & 1 deletion bin/rails-application-template
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ after_bundle do
"solidus:install",
"--auto-accept",
"--user_class=Spree::User",
"--enforce_available_locales=true",
"--authentication=solidus_auth_devise",
)

Expand Down
1 change: 0 additions & 1 deletion bin/sandbox
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ unbundled bin/rails db:drop db:create
echo "~~~> Running the solidus:install generator"
unbundled bin/rails generate solidus:install \
--auto-accept \
--enforce-available-locales=true \
$@

echo "
Expand Down
243 changes: 127 additions & 116 deletions core/lib/generators/solidus/install/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,22 @@ class InstallGenerator < Rails::Generators::AppBase

CORE_MOUNT_ROUTE = "mount Spree::Core::Engine"

LEGACY_FRONTEND = 'solidus_frontend'
DEFAULT_FRONTEND = 'solidus_starter_frontend'
Comment on lines -14 to -15
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but do you think it is ok to remove the constants (same for DEFAULT_AUTHENTICATION below)? It helps to check for string values that can be repeated multiple times (e.g., avoiding a typo that would create a mysterious bug).

Copy link
Member Author

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, I'll try with a local variable first and fall back to a constant if that doesn't work

Context:
Those were removed because the role was a bit confusing, and with checks on the gemfile the default is actually dynamic (e.g. if you have the legacy one in the gemfile and auto_accept it will "default" to it, etc.).

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. That's non-blocking, though 🙂

FRONTENDS = {
'none' => "#{__dir__}/app_templates/frontend/none.rb",
'solidus_frontend' => "#{__dir__}/app_templates/frontend/solidus_frontend.rb",
'solidus_starter_frontend' => "#{__dir__}/app_templates/frontend/solidus_starter_frontend.rb",
}

DEFAULT_AUTHENTICATION = 'devise'
AUTHENTICATIONS = {
'devise' => "#{__dir__}/app_templates/authentication/devise.rb",
'existing' => "#{__dir__}/app_templates/authentication/existing.rb",
'custom' => "#{__dir__}/app_templates/authentication/custom.rb",
'none' => "#{__dir__}/app_templates/authentication/none.rb",
}
FRONTENDS = %w[
none
classic
starter
]
LEGACY_FRONTENDS = %w[
solidus_starter_frontend
solidus_frontend
]

AUTHENTICATIONS = %w[
devise
existing
custom
none
]

class_option :migrate, type: :boolean, default: true, banner: 'Run Solidus migrations'
class_option :seed, type: :boolean, default: true, banner: 'Load seed data (migrations must be run)'
Expand All @@ -37,13 +38,14 @@ class InstallGenerator < Rails::Generators::AppBase
class_option :user_class, type: :string
class_option :admin_email, type: :string
class_option :admin_password, type: :string
class_option :lib_name, type: :string, default: 'spree'
class_option :enforce_available_locales, type: :boolean, default: nil
class_option :frontend, type: :string, enum: FRONTENDS.keys, default: nil, desc: "Indicates which frontend to install."
class_option :authentication, type: :string, enum: AUTHENTICATIONS.keys, default: nil, desc: "Indicates which authentication system to install."

class_option :frontend, type: :string, enum: FRONTENDS + LEGACY_FRONTENDS, default: nil, desc: "Indicates which frontend to install."
class_option :authentication, type: :string, enum: AUTHENTICATIONS, default: nil, desc: "Indicates which authentication system to install."

# DEPRECATED
class_option :with_authentication, type: :boolean, hide: true, default: nil
class_option :enforce_available_locales, type: :boolean, hide: true, default: nil
class_option :lib_name, type: :string, hide: true, default: nil

def self.source_paths
paths = superclass.source_paths
Expand All @@ -61,23 +63,25 @@ def prepare_options
@run_migrations = options[:migrate]
@load_seed_data = options[:seed]
@load_sample_data = options[:sample]
@selected_frontend = detect_frontend_to_install
@selected_authentication = detect_authentication_to_install

# Silence verbose output (e.g. Rails migrations will rely on this environment variable)
ENV['VERBOSE'] = 'false'

# No reason to check for their presence if we're about to install them
ENV['SOLIDUS_SKIP_MIGRATIONS_CHECK'] = 'true'

if options[:with_authentication] != nil
if options[:enforce_available_locales] != nil
warn \
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're removing the warning, can't we remove the option altogether now?

Copy link
Member Author

Choose a reason for hiding this comment

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

The warning is still there, just moved it to detect_authentication_to_install.

"DEPRECATION WARNING: using `solidus:install --with-authentication` is now deprecated. " \
"Please use --authentication instead (see --help for the full list of options)."
"DEPRECATION WARNING: using `solidus:install --enforce-available-locales` is now deprecated and has no effect. " \
waiting-for-dev marked this conversation as resolved.
Show resolved Hide resolved
"Since Rails 4.1 the default is `true` so we no longer need to explicitly set a value."
end

if options[:authentication].blank? && options[:with_authentication] == 'false'
# Don't use the default authentication if the user explicitly
# requested no authentication system.
options[:authentication] = 'none'
end
if options[:lib_name] != nil
warn \
"DEPRECATION WARNING: using `solidus:install --lib-name` is now deprecated and has no effect. " \
"The option is legacy and should be removed from scripts still using it."
waiting-for-dev marked this conversation as resolved.
Show resolved Hide resolved
end

unless @run_migrations
Expand All @@ -100,24 +104,7 @@ def install_file_attachment
end
end

def additional_tweaks
return unless File.exist? 'public/robots.txt'

append_file "public/robots.txt", <<-ROBOTS.strip_heredoc
User-agent: *
Disallow: /checkout
Disallow: /cart
Disallow: /orders
Disallow: /user
Disallow: /account
Disallow: /api
Disallow: /password
ROBOTS
end

def setup_assets
@lib_name = 'spree'

empty_directory 'app/assets/images'

%w{javascripts stylesheets images}.each do |path|
Expand All @@ -134,20 +121,6 @@ def create_overrides_directory
empty_directory "app/overrides"
end

def configure_application
if !options[:enforce_available_locales].nil?
application <<~RUBY
# Prevent this deprecation message: https://github.com/svenfuchs/i18n/commit/3b6e56e
I18n.enforce_available_locales = #{options[:enforce_available_locales]}
RUBY
end
end

def plugin_install_preparation
waiting-for-dev marked this conversation as resolved.
Show resolved Hide resolved
@plugins_to_be_installed = []
@plugin_generators_to_run = []
end

def include_seed_data
append_file "db/seeds.rb", <<~RUBY

Expand All @@ -166,32 +139,6 @@ def create_database
rake 'db:create'
end

def install_authentication
authentication_key = detect_authentication_to_install

authentication_template = AUTHENTICATIONS.fetch(authentication_key.presence) do
say_status :warning, "Unknown authentication: #{authentication_key.inspect}, attempting to run it with `rails app:template`"
authentication_key
end

say_status :auth, authentication_key

apply authentication_template
end

def run_bundle_install_if_needed_by_plugins
@plugins_to_be_installed.each do |plugin_name|
gem plugin_name
end

run_bundle if @plugins_to_be_installed.any?
run "spring stop" if defined?(Spring)

@plugin_generators_to_run.each do |plugin_generator_name|
generate "#{plugin_generator_name} --skip-migrations=true"
end
end

def install_routes
if Pathname(app_path).join('config', 'routes.rb').read.include? CORE_MOUNT_ROUTE
say_status :route_exist, CORE_MOUNT_ROUTE, :blue
Expand All @@ -207,19 +154,6 @@ def install_routes
end
end

def install_frontend
frontend_key = detect_frontend_to_install

frontend_template = FRONTENDS.fetch(frontend_key.presence) do
say_status :warning, "Unknown frontend: #{frontend_key.inspect}, attempting to run it with `rails app:template`"
frontend_key
end

say_status :frontend, frontend_key

apply frontend_template
end

def run_migrations
if @run_migrations
say_status :running, "migrations"
Expand All @@ -230,6 +164,14 @@ def run_migrations
end
end

def install_authentication
apply_template_for :authentication, @selected_authentication
end

def install_frontend
apply_template_for :frontend, @selected_frontend
end

def populate_seed_data
if @load_seed_data
say_status :loading, "seed data"
Expand Down Expand Up @@ -259,42 +201,111 @@ def complete

private

def generate(what, *args)
def generate(what, *args, abort_on_failure: true)
args << '--auto-accept' if options[:auto_accept]
args << '--auto-run-migrations' if options[:migrate]
super(what, *args)
super(what, *args, abort_on_failure: abort_on_failure)
end

def bundle_command(command, env = {})
# Make `bundle install` less verbose by skipping the "Using ..." messages
super(command, env.reverse_merge('BUNDLE_SUPPRESS_INSTALL_USING_MESSAGES' => 'true'))
ensure
Bundler.reset_paths!
end

def detect_frontend_to_install
ENV['FRONTEND'] ||
options[:frontend] ||
(Bundler.locked_gems.dependencies[LEGACY_FRONTEND] && LEGACY_FRONTEND) ||
(options[:auto_accept] && DEFAULT_FRONTEND) ||
ask(<<~MSG.indent(8), default: DEFAULT_FRONTEND, limited_to: FRONTENDS.keys)
def ask_with_description(desc:, limited_to:, default:)
loop do
say_status :question, desc, :yellow
answer = ask(set_color("answer:".rjust(13), :blue, :bold)).to_s.downcase.presence

case answer
when nil
say_status :using, "#{default} (default)"
break default
when *limited_to.map(&:to_s)
say_status :using, answer
break answer
else say_status :error, "Please select a valid answer:", :red
end
end
end

def apply_template_for(topic, selected)
template_path = Dir["#{__dir__}/app_templates/#{topic}/*.rb"].find do |path|
File.basename(path, '.rb') == selected
end

Which frontend would you like to use?
Selecting #{DEFAULT_FRONTEND} is recommended.
However, some extensions are still only compatible with the now deprecated #{LEGACY_FRONTEND}.
unless template_path
say_status :warning, "Unknown #{topic}: #{selected.inspect}, attempting to run it with `rails app:template`"
template_path = selected
end

say_status :installing, "[#{topic}] #{selected}", :blue
apply template_path
end

def detect_frontend_to_install
# We need to support names that were available in v3.2
selected_frontend = 'starter' if options[:frontend] == 'solidus_starter_frontend'
selected_frontend = 'classic' if options[:frontend] == 'solidus_frontend'
selected_frontend ||= options[:frontend]

MSG
ENV['FRONTEND'] ||
selected_frontend ||
(Bundler.locked_gems.dependencies['solidus_frontend'] && 'classic') ||
(options[:auto_accept] && 'starter') ||
ask_with_description(
default: 'starter',
limited_to: FRONTENDS,
desc: <<~TEXT
Which frontend would you like to use?

- [#{set_color 'starter', :bold}] Generate all necessary controllers and views directly in your Rails app (#{set_color :default, :bold}).
- [#{set_color 'classic', :bold}] Install `solidus_frontend`, was the default in previous solidus versions (#{set_color :deprecated, :bold}).
- [#{set_color 'none', :bold}] Skip installing a frontend

Selecting `starter` is recommended, however, some extensions are still only compatible with `classic`.
TEXT
)
end

def detect_authentication_to_install
return 'devise' if @selected_frontend == 'starter'

if options[:with_authentication] != nil
say_status :warning, \
"Using `solidus:install --with-authentication` is now deprecated. " \
"Please use `--authentication` instead (see --help for the full list of options).",
:red

if options[:with_authentication] == 'false'
# Don't use the default authentication if the user explicitly
# requested no authentication system.
return 'none'
else
return 'devise'
end
end

ENV['AUTHENTICATION'] ||
options[:authentication] ||
(Bundler.locked_gems.dependencies['solidus_auth_devise'] && 'devise') ||
(options[:auto_accept] && DEFAULT_AUTHENTICATION) ||
ask(<<~MSG.indent(8), default: DEFAULT_AUTHENTICATION, limited_to: AUTHENTICATIONS.keys)

Which authentication method would you like to use?
Selecting "#{DEFAULT_AUTHENTICATION}" is recommended.

MSG
(options[:auto_accept] && 'devise') ||
ask_with_description(
default: 'devise',
limited_to: AUTHENTICATIONS,
desc: <<~TEXT
Which authentication would you like to use?

- [#{set_color 'devise', :bold}] Install and configure the standard `devise` integration. (#{set_color :default, :bold}).
- [#{set_color 'existing', :bold}] Integrate and configure an existing `devise` setup.
- [#{set_color 'custom', :bold}] A starter configuration for rolling your own authentication system.
- [#{set_color 'none', :bold}] Don't add any configuration for authentication.

Selecting `devise` is recommended.
TEXT
)
end
end
end
3 changes: 1 addition & 2 deletions core/lib/spree/migrations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ def check
#{missing_migrations.map {|m| "#{prefix} - #{m}"}.join("\n")}
#{prefix}
#{prefix} Run `bin/rails railties:install:migrations` to get them.
#{prefix} You can silence thi warning by setting the environment
#{prefix} variable SOLIDUS_SKIP_MIGRATIONS_CHECK.'
#{prefix} You can silence thi warning by setting the `SOLIDUS_SKIP_MIGRATIONS_CHECK` environment variable.
WARN
end

Expand Down
3 changes: 1 addition & 2 deletions core/lib/spree/testing_support/common_rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ def initialize
# within ruby is changed to that of the dummy app.
sh({
'SKIP_SOLIDUS_BOLT' => '1',
'FRONTEND' => ENV['FRONTEND'] || 'solidus_frontend',
'FRONTEND' => ENV['FRONTEND'] || 'classic',
}, [
'bin/rails',
'generate',
'solidus:install',
Dir.pwd, # use the current dir as Rails.root
"--lib-name=#{lib_name}",
"--auto-accept",
"--authentication=none",
"--migrate=false",
Expand Down
Loading