Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • A administrate
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 96
    • Issues 96
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 32
    • Merge requests 32
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • thoughtbot, inc.
  • administrate
  • Merge requests
  • !1633

Automatic associations

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Pablo Brasero requested to merge github/fork/pablobm/automatic-associations into master May 01, 2020
  • Overview 29
  • Commits 1
  • Pipelines 0
  • Changes 25

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 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.
  • 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.

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: github/fork/pablobm/automatic-associations