Fixes #1597 (closed)
Summary
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.
Implementation notes
The core change of the feature specs is at spec/example_app/app/dashboards/customer_dashboard.rb
. I started by introducing that change, then proceeded slowly to make everything work.
This implementation relies on Field::Base.new
always receiving a :resource_class
option. I think this is safe as it's already the case throughout the codebase. However third party gems may have incompatibilities here.
Third party gems
I have had a look at third-party plugins listed at https://rubygems.org/gems/administrate/reverse_dependencies. 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 fromNestedHasMany
fields. - You can work around this by continuing to use the deprecated options (just for your
NestedHasMany
fields), although you'll get deprecation warnings. - I created a branch that will bring this plugin in sync with the changes in this PR: https://github.com/nickcharlton/administrate-field-nested_has_many/compare/master...pablobm:automatic-associations
Deprecations
I could have removed the deprecated options completely, but I decided against it because:
- I want to minimise disruption when updating Administrate.
- I don't have 100% trust that just removing the options will work for everyone. There may be a corner case I'm missing.
Instead I'm allowing the old behaviour to exist for a bit longer, while showing abundant deprecation warnings. Hopefully this will get people to upgrade quickly and report any incompatibility.
General review tips
The option "Hide whitespace changes" is very useful here, as I have changed a lot of indent at the specs, in order to avoid style warnings (Hound).
Also due to style warnings, I have reformatted a lot of code, adding more diff noise in the process.
So this is to say: this MR looks huge, but it's just... large, I guess.
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, but I think that would be too much for this PR.
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). I have let this be for now. I think it 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.