Associative fields should figure out association options automatically #1597
Assignees
Comments
Merged
nickcharlton
pushed a commit
to pablobm/administrate
that referenced
this issue
Oct 27, 2020
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 thoughtbot#1597
nickcharlton
pushed a commit
to pablobm/administrate
that referenced
this issue
Oct 27, 2020
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 thoughtbot#1597
nickcharlton
pushed a commit
that referenced
this issue
Oct 27, 2020
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It should not be necessary to specify the options
:foreign_key,:primary_key, and:class_namewhen defining an associative field. Administrate should be able to figure these out from the association.We should add this automatic configuration, mark these options as deprecated, and ultimately remove them.
Already making some headway at master...pablobm:automatic-associations
The text was updated successfully, but these errors were encountered: