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

🚨 refactor: better engine mounting #3533

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
3 changes: 0 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,6 @@ gem "sprockets-rails"
# Use Active Storage variant
gem "image_processing", "~> 1.12"

# source "https://rubygems.pkg.github.com/avo-hq" do
# gem "avo-dynamic_filters"
# end
gem "prefixed_ids"

gem "mapkick-rb", "~> 0.1.4"
Expand Down
2 changes: 1 addition & 1 deletion app/components/avo/resource_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def render_control(control)
end

def render_cards_component
if Avo.plugin_manager.installed?(:avo_dashboards)
if Avo.plugin_manager.installed?("avo-dashboards")
render Avo::CardsComponent.new cards: @resource.detect_cards.visible_cards, classes: "pb-4 sm:grid-cols-3"
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/components/avo/row_selector_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Avo::RowSelectorComponent < Avo::BaseComponent
def data_action
data = "input->item-selector#toggle input->item-select-all#selectRow"

if Avo.plugin_manager.installed?(:avo_pro)
if Avo.plugin_manager.installed?("avo-pro")
data += " click->record-selector#toggleMultiple"
end

Expand Down
2 changes: 1 addition & 1 deletion app/components/avo/sidebar_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<div class="space-y-6 mb-4">
<%= render Avo::Sidebar::LinkComponent.new label: 'Get started', path: helpers.avo.root_path, active: :exclusive if Rails.env.development? && Avo.configuration.home_path.nil? %>

<% if Avo.plugin_manager.installed?(:avo_menu) && Avo.has_main_menu? %>
<% if Avo.plugin_manager.installed?("avo-menu") && Avo.has_main_menu? %>
<% Avo.main_menu.items.each do |item| %>
<%= render Avo::Sidebar::ItemSwitcherComponent.new item: item %>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/components/avo/sidebar_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Avo::SidebarComponent < Avo::BaseComponent
prop :for_mobile, default: false

def dashboards
return [] unless Avo.plugin_manager.installed?(:avo_dashboards)
return [] unless Avo.plugin_manager.installed?("avo-dashboards")

Avo::Dashboards.dashboard_manager.dashboards_for_navigation
end
Expand Down
2 changes: 1 addition & 1 deletion app/components/avo/sidebar_profile_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
data-transition-leave-start="transform opacity-100 translate-y-0"
data-transition-leave-end="transform opacity-0 translate-y-3"
>
<% if Avo.plugin_manager.installed?(:avo_menu) && Avo.has_profile_menu? %>
<% if Avo.plugin_manager.installed?("avo-menu") && Avo.has_profile_menu? %>
<div class="text-black space-y-4">
<% Avo.profile_menu.items.each do |item| %>
<% if item.is_a? Avo::Menu::Link %>
Expand Down
5 changes: 0 additions & 5 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
get "resources", to: redirect(Avo.configuration.root_path)
get "dashboards", to: redirect(Avo.configuration.root_path)

# Mount Avo engines routes by default but leave it configurable in case the user wants to nest these under a scope.
if Avo.configuration.mount_avo_engines
instance_exec(&Avo.mount_engines)
end

resources :media_library, only: [:index, :show, :update, :destroy], path: "media-library"
get "attach-media", to: "media_library#attach"

Expand Down
12 changes: 4 additions & 8 deletions lib/avo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def boot
@logger = Avo.configuration.logger
@field_manager = Avo::Fields::FieldManager.build
@cache_store = Avo.configuration.cache_store
Avo.plugin_manager.reset
ActiveSupport.run_load_hooks(:avo_boot, self)
eager_load_actions
end
Expand Down Expand Up @@ -94,7 +95,7 @@ def root_path(paths: [], query: {}, **args)
end

def main_menu
return unless Avo.plugin_manager.installed?(:avo_menu)
return unless Avo.plugin_manager.installed?("avo-menu")

# Return empty menu if the app doesn't have the profile menu configured
return Avo::Menu::Builder.new.build unless has_main_menu?
Expand All @@ -103,7 +104,7 @@ def main_menu
end

def profile_menu
return unless Avo.plugin_manager.installed?(:avo_menu)
return unless Avo.plugin_manager.installed?("avo-menu")

# Return empty menu if the app doesn't have the profile menu configured
return Avo::Menu::Builder.new.build unless has_profile_menu?
Expand Down Expand Up @@ -133,14 +134,9 @@ def has_profile_menu?
true
end

# Mount all Avo engines
def mount_engines
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same. Raise and error with docs links.

-> {
mount Avo::DynamicFilters::Engine, at: "/avo-dynamic_filters" if defined?(Avo::DynamicFilters::Engine)
mount Avo::Dashboards::Engine, at: "/dashboards" if defined?(Avo::Dashboards::Engine)
mount Avo::Pro::Engine, at: "/avo-pro" if defined?(Avo::Pro::Engine)
mount Avo::Kanban::Engine, at: "/boards" if defined?(Avo::Kanban::Engine)
mount Avo::Collaborate::Engine, at: "/collaborate" if defined?(Avo::Collaborate::Engine)
raise "`mount_engines` method is now obsolete, please check LINK_HERE for more details."
}
end

Expand Down
8 changes: 5 additions & 3 deletions lib/avo/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class Configuration
attr_accessor :resources
attr_accessor :prefix_path
attr_accessor :resource_parent_controller
attr_accessor :mount_avo_engines
attr_accessor :default_url_options
attr_accessor :click_row_to_view_record
attr_accessor :alert_dismiss_time
Expand Down Expand Up @@ -112,7 +111,6 @@ def initialize
@field_wrapper_layout = :inline
@resources = nil
@resource_parent_controller = "Avo::ResourcesController"
@mount_avo_engines = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's add a setter method to raise an error with docs link.

@cache_store = computed_cache_store
@logger = default_logger
@turbo = default_turbo
Expand Down Expand Up @@ -146,7 +144,7 @@ def resource_row_controls_config
# Authorization is enabled when:
# (avo-pro gem is installed) AND (authorization_client is NOT nil)
def authorization_enabled?
@authorization_enabled ||= Avo.plugin_manager.installed?(:avo_pro) && !authorization_client.nil?
@authorization_enabled ||= Avo.plugin_manager.installed?("avo-pro") && !authorization_client.nil?
end

def current_user_method(&block)
Expand Down Expand Up @@ -302,6 +300,10 @@ def persistence
def session_persistence_enabled?
persistence[:driver] == :session
end

def mount_avo_engines=(...)
raise "`mount_avo_engines` option is now obsolete, please check LINK_HERE for more details."
end
end

def self.configuration
Expand Down
15 changes: 15 additions & 0 deletions lib/avo/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,21 @@ class Engine < ::Rails::Engine
app.config.watchable_dirs[directory_path] = [:rb]
end
end

# Add the mount_avo method to Rails
# rubocop:disable Style/ArgumentsForwarding
ActionDispatch::Routing::Mapper.include(Module.new {
def mount_avo(at: Avo.configuration.root_path, **options)
mount Avo::Engine, at:, **options

scope at do
Avo.plugin_manager.engines.each do |engine|
mount engine[:klass], **engine[:options]
end
end
end
})
# rubocop:enable Style/ArgumentsForwarding
end

initializer "avo.reloader" do |app|
Expand Down
11 changes: 11 additions & 0 deletions lib/avo/plugin_manager.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
module Avo
class PluginManager
attr_reader :plugins
attr_accessor :engines

alias_method :all, :plugins

def initialize
@plugins = []
@engines = []
end

def reset
@plugins = []
@engines = []
end

def register(name, priority: 10)
Expand Down Expand Up @@ -50,6 +57,10 @@ def installed?(name)
plugin.name.to_s == name.to_s
end
end

def mount_engine(klass, **options)
@engines << {klass:, options:}
end
end

def self.plugin_manager
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/avo/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class InstallGenerator < BaseGenerator
class_option :path, type: :string, default: "avo"

def create_initializer_file
route "mount Avo::Engine, at: Avo.configuration.root_path"
route "mount_avo"

template "initializer/avo.tt", "config/initializers/avo.rb"

Expand Down
4 changes: 0 additions & 4 deletions lib/generators/avo/templates/initializer/avo.tt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ Avo.configure do |config|
# used only when you have custom `map` configuration in your config.ru
# config.prefix_path = "/internal"

# Sometimes you might want to mount Avo's engines yourself.
# https://docs.avohq.io/3.0/routing.html
# config.mount_avo_engines = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should still be here with a warning that it's not working anymore.
Deprecation warning.

Maybe


# Where should the user be redirected when visiting the `/<%= options[:path] %>` url
# config.home_path = nil

Expand Down
2 changes: 0 additions & 2 deletions spec/dummy/config/initializers/avo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
## == Base configs ==
config.root_path = "/admin"
config.app_name = -> { "Avocadelicious #{params[:app_name_suffix]}" }
config.home_path = -> { "/admin/resources/projects" }
config.home_path = -> { avo.resources_projects_path }
# config.mount_avo_engines = false
# config.default_url_options = [:tenant_id]
# Use this to test root_path_without_url helper
# Also enable in config.ru & application.rb
Expand Down
7 changes: 4 additions & 3 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@
get "custom_tool", to: "avo/tools#custom_tool", as: :custom_tool
end

mount Avo::Engine, at: Avo.configuration.root_path
mount_avo

# Uncomment to test constraints /123/en/admin
# scope ":course", constraints: {course: /\w+(-\w+)*/} do
# scope ":locale", constraints: {locale: /\w[-\w]*/} do
# mount Avo::Engine, at: Avo.configuration.root_path
# mount_avo
# end
# end

# TODO: support locale based routes
scope "(:locale)" do
# mount Avo::Engine, at: Avo.configuration.root_path
# mount_avo
end
end
end
Expand Down
Loading