Skip to content

Commit

Permalink
Add automatic associations (#1633)
Browse files Browse the repository at this point in the history
Currently, associative dashboard fields (`BelongsTo`, `HasMany`, `HasOne`)
require explicit configuration options (`:class_name`, `:foreign_key`,
`:primary_key`) for Administrate to find their associated models. There are
details that Administrate can figure out automatically, but not everything,
and not always reliably.

This PR changes these field types so that they do not need this
configuration, instead detecting associations by using ActiveRecord's
reflection API.

This implementation relies on `Field::Base.new` always receiving a
`:resource_class` option. This is safe as it's already the case throughout
the codebase. However third party gems may have incompatibilities here.

From looking at the list of reverse dependencies[1]. From this list, only
`administrate-field-nested_has_many` appear to have incompatibilities, which
is not surprising as it adds a new associative field with some advanced
features. Specifically you'll find that:

* It will not work if you remove your `:class_name` options from
  `NestedHasMany` fields.
* You can work around this by continuing to use the deprecated options (just
  for your `NestedHasMany` fields), although you'll get deprecation warnings.
* There's a branch that will bring this plugin in sync [2].

The old behviour will still exist for a while, to help with backwards
compatibility. This commit introduces a set of deprecation warnings.

Finally, notes for the future:

* The current code relies a lot on globals (eg: class methods) that are
  difficult to test and generally work with. In the future, we may want to
  change the interface for field types to improve the situation.
* There's still some risky string-to-class metaprogramming going on, for
  example a `"#{associated_class_name}Dashboard".constantize.new` in
  `lib/administrate/field/associative.rb` (not shown in the diff as unchanged).
  This should be reviewed in the future as part of work in the resolved to fix
  issues such as namespacing bugs, which are out of scope here.

[1]: https://rubygems.org/gems/administrate/reverse_dependencies
[2]: nickcharlton/administrate-field-nested_has_many@master...pablobm:automatic-associations

Fixes #1597
  • Loading branch information
pablobm authored Oct 27, 2020
1 parent 9931774 commit 7cddab1
Show file tree
Hide file tree
Showing 25 changed files with 801 additions and 430 deletions.
26 changes: 11 additions & 15 deletions docs/customizing_dashboards.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,9 @@ which are specified through the `.with_options` class method:
`:order` - Specifies the order of the dropdown menu, can be ordered by more
than one column. e.g.: `"name, email DESC"`.

`:primary_key` - Specifies object's primary_key. Defaults to `:id`.

`:foreign_key` - Specifies the name of the foreign key directly.
Defaults to `:#{attribute}_id`.

`:scope` - Specifies a custom scope inside a callable. Useful for preloading.
Example: `.with_options(scope: -> { MyModel.includes(:rel).limit(5) })`

`:class_name` - Specifies the name of the associated class.
Defaults to `:#{attribute}.to_s.singularize.camelcase`.

`:include_blank` - Specifies if the select element to be rendered should include
blank option. Default is `true`.

Expand All @@ -111,6 +103,12 @@ For example:
with this, you will be able to search through the column `name` from the
association `belongs_to :country`, from your model.

`:primary_key` (deprecated) - Specifies the association's primary_key.

`:foreign_key` (deprecated) - Specifies the name of the foreign key directly.

`:class_name` (deprecated) - Specifies the name of the associated class.

**Field::HasMany**

`:limit` - Set the number of resources to display in the show view. Default is
Expand All @@ -120,18 +118,14 @@ association `belongs_to :country`, from your model.

`:direction` - What direction the sort should be in, `:asc` (default) or `:desc`.

`:primary_key` - Specifies object's primary_key. Defaults to `:id`.
`:primary_key` (deprecated) - Specifies object's primary_key.

`:foreign_key` - Specifies the name of the foreign key directly. Defaults to `:#{attribute}_id`
`:foreign_key` (deprecated) - Specifies the name of the foreign key directly.

`:class_name` - Specifies the name of the associated class.
Defaults to `:#{attribute}.to_s.singularize.camelcase`.
`:class_name` (deprecated) - Specifies the name of the associated class.

**Field::HasOne**

`:class_name` - Specifies the name of the associated class.
Defaults to `:#{attribute}.to_s.singularize.camelcase`.

`:searchable` - Specify if the attribute should be considered when searching.
Default is `false`.

Expand All @@ -150,6 +144,8 @@ For example:
with this, you will be able to search through the column `name` from the
association `has_many :cities`, from your model.

`:class_name` (deprecated) - Specifies the name of the associated class.

**Field::Number**

`:searchable` - Specify if the attribute should be considered when searching.
Expand Down
19 changes: 19 additions & 0 deletions lib/administrate.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,23 @@
require "administrate/engine"

module Administrate
def self.warn_of_missing_resource_class
ActiveSupport::Deprecation.warn(
"Calling Field::Base.permitted_attribute without the option " +
":resource_class is deprecated. If you are seeing this " +
"message, you are probably using a custom field type that" +
"does this. Please make sure to update it to a version that " +
"does not use a deprecated API",
)
end

def self.warn_of_deprecated_option(name)
ActiveSupport::Deprecation.warn(
"The option :#{name} is deprecated. " +
"Administrate should detect it automatically. " +
"Please file an issue at " +
"https://github.com/thoughtbot/administrate/issues " +
"if you think otherwise.",
)
end
end
5 changes: 4 additions & 1 deletion lib/administrate/base_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ def form_attributes

def permitted_attributes
form_attributes.map do |attr|
attribute_types[attr].permitted_attribute(attr)
attribute_types[attr].permitted_attribute(
attr,
resource_class: self.class.model,
)
end.uniq
end

Expand Down
52 changes: 48 additions & 4 deletions lib/administrate/field/associative.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,32 @@
module Administrate
module Field
class Associative < Base
def self.foreign_key_for(resource_class, attr)
reflection(resource_class, attr).foreign_key
end

def self.associated_class(resource_class, attr)
reflection(resource_class, attr).klass
end

def self.associated_class_name(resource_class, attr)
reflection(resource_class, attr).class_name
end

def self.reflection(resource_class, attr)
resource_class.reflect_on_association(attr)
end

def display_associated_resource
associated_dashboard.display_resource(data)
end

def associated_class
associated_class_name.constantize
if option_given?(:class_name)
associated_class_name.constantize
else
self.class.associated_class(resource.class, attribute)
end
end

private
Expand All @@ -18,15 +38,39 @@ def associated_dashboard
end

def associated_class_name
options.fetch(:class_name, attribute.to_s.singularize.camelcase)
if option_given?(:class_name)
deprecated_option(:class_name)
else
self.class.associated_class_name(
resource.class,
attribute,
)
end
end

def primary_key
options.fetch(:primary_key, :id)
if option_given?(:primary_key)
deprecated_option(:primary_key)
else
:id
end
end

def foreign_key
options.fetch(:foreign_key, :"#{attribute}_id")
if option_given?(:foreign_key)
deprecated_option(:foreign_key)
else
self.class.foreign_key_for(resource.class, attribute)
end
end

def option_given?(name)
options.key?(name)
end

def deprecated_option(name)
Administrate.warn_of_deprecated_option(name)
options.fetch(name)
end
end
end
Expand Down
10 changes: 8 additions & 2 deletions lib/administrate/field/belongs_to.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@
module Administrate
module Field
class BelongsTo < Associative
def self.permitted_attribute(attr, _options = nil)
:"#{attr}_id"
def self.permitted_attribute(attr, options = {})
resource_class = options[:resource_class]
if resource_class
foreign_key_for(resource_class, attr)
else
Administrate.warn_of_missing_resource_class
:"#{attr}_id"
end
end

def permitted_attribute
Expand Down
10 changes: 7 additions & 3 deletions lib/administrate/field/deferred.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,13 @@ def searchable_fields
end
end

def permitted_attribute(attr, _options = nil)
options.fetch(:foreign_key,
deferred_class.permitted_attribute(attr, options))
def permitted_attribute(attr, opts = {})
if options.key?(:foreign_key)
Administrate.warn_of_deprecated_option(:foreign_key)
options.fetch(:foreign_key)
else
deferred_class.permitted_attribute(attr, options.merge(opts))
end
end

delegate :html_class, to: :deferred_class
Expand Down
17 changes: 15 additions & 2 deletions lib/administrate/field/has_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,17 @@ module Field
class HasMany < Associative
DEFAULT_LIMIT = 5

def self.permitted_attribute(attr, _options = nil)
def self.permitted_attribute(attr, _options = {})
# This may seem arbitrary, and improvable by using reflection.
# Worry not: here we do exactly what Rails does. Regardless of the name
# of the foreign key, has_many associations use the suffix `_ids`
# for this.
#
# Eg: if the associated table and primary key are `countries.code`,
# you may expect `country_codes` as attribute here, but it will
# be `country_ids` instead.
#
# See https://github.com/rails/rails/blob/b30a23f53b52e59d31358f7b80385ee5c2ba3afe/activerecord/lib/active_record/associations/builder/collection_association.rb#L48
{ "#{attr.to_s.singularize}_ids".to_sym => [] }
end

Expand Down Expand Up @@ -36,7 +46,10 @@ def limit
end

def permitted_attribute
self.class.permitted_attribute(attribute)
self.class.permitted_attribute(
attribute,
resource_class: resource.class,
)
end

def resources(page = 1, order = self.order)
Expand Down
25 changes: 17 additions & 8 deletions lib/administrate/field/has_one.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,26 @@
module Administrate
module Field
class HasOne < Associative
def self.permitted_attribute(attr, options = nil)
associated_class_name =
if options
options.fetch(:class_name, attr.to_s.singularize.camelcase)
def self.permitted_attribute(attr, options = {})
resource_class = options[:resource_class]
final_associated_class_name =
if options.key?(:class_name)
Administrate.warn_of_deprecated_option(:class_name)
options.fetch(:class_name)
elsif resource_class
associated_class_name(resource_class, attr)
else
attr
Administrate.warn_of_missing_resource_class
if options
attr.to_s.singularize.camelcase
else
attr
end
end
related_dashboard_attributes =
Administrate::ResourceResolver.new("admin/#{associated_class_name}").
Administrate::ResourceResolver.
new("admin/#{final_associated_class_name}").
dashboard_class.new.permitted_attributes + [:id]

{ "#{attr}_attributes": related_dashboard_attributes }
end

Expand All @@ -35,7 +44,7 @@ def nested_show

def resolver
@resolver ||=
Administrate::ResourceResolver.new("admin/#{associated_class_name}")
Administrate::ResourceResolver.new("admin/#{associated_class.name}")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/administrate/field/polymorphic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Administrate
module Field
class Polymorphic < BelongsTo
def self.permitted_attribute(attr, _options = nil)
def self.permitted_attribute(attr, _options = {})
{ attr => %i{type value} }
end

Expand Down
17 changes: 10 additions & 7 deletions lib/administrate/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ def query_template
search_attributes.map do |attr|
table_name = query_table_name(attr)
searchable_fields(attr).map do |field|
attr_name = column_to_query(field)
"LOWER(CAST(#{table_name}.#{attr_name} AS CHAR(256))) LIKE ?"
column_name = column_to_query(field)
"LOWER(CAST(#{table_name}.#{column_name} AS CHAR(256))) LIKE ?"
end.join(" OR ")
end.join(" OR ")
end
Expand Down Expand Up @@ -128,11 +128,14 @@ def attribute_types
def query_table_name(attr)
if association_search?(attr)
provided_class_name = attribute_types[attr].options[:class_name]
if provided_class_name
provided_class_name.constantize.table_name
else
ActiveRecord::Base.connection.quote_table_name(attr.to_s.pluralize)
end
unquoted_table_name =
if provided_class_name
Administrate.warn_of_deprecated_option(:class_name)
provided_class_name.constantize.table_name
else
@scoped_resource.reflect_on_association(attr).klass.table_name
end
ActiveRecord::Base.connection.quote_table_name(unquoted_table_name)
else
ActiveRecord::Base.connection.
quote_table_name(@scoped_resource.table_name)
Expand Down
12 changes: 2 additions & 10 deletions lib/generators/administrate/dashboard/dashboard_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,26 +110,18 @@ def association_type(attribute)
if relationship.has_one?
"Field::HasOne"
elsif relationship.collection?
"Field::HasMany" + relationship_options_string(relationship)
"Field::HasMany"
elsif relationship.polymorphic?
"Field::Polymorphic"
else
"Field::BelongsTo" + relationship_options_string(relationship)
"Field::BelongsTo"
end
end

def klass
@klass ||= Object.const_get(class_name)
end

def relationship_options_string(relationship)
if relationship.class_name != relationship.name.to_s.classify
options_string(class_name: relationship.class_name)
else
""
end
end

def options_string(options)
if options.any?
".with_options(#{inspect_hash_as_ruby(options)})"
Expand Down
2 changes: 1 addition & 1 deletion spec/dashboards/order_dashboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
it "returns the attribute_id name for belongs_to relationships" do
dashboard = OrderDashboard.new

expect(dashboard.permitted_attributes).to include(:customer_id)
expect(dashboard.permitted_attributes).to include("customer_id")
end
end
end
3 changes: 0 additions & 3 deletions spec/example_app/app/dashboards/customer_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ class CustomerDashboard < Administrate::BaseDashboard
updated_at: Field::DateTime,
kind: Field::Select.with_options(collection: Customer::KINDS),
territory: Field::BelongsTo.with_options(
primary_key: :code,
foreign_key: :country_code,
class_name: "Country",
searchable: true,
searchable_fields: ["name"],
include_blank: true,
Expand Down
Loading

0 comments on commit 7cddab1

Please sign in to comment.