thoughtbot / administrate

A Rails engine that helps you put together a super-flexible admin dashboard.
http://administrate-demo.herokuapp.com
MIT License
5.89k stars 1.12k forks source link

Fallback 'display name' for optional belongs_to associations #2286

Open ammancilla opened 1 year ago

ammancilla commented 1 year ago

What would you like to be able to do? Can you provide some examples?

Recently, I ran into the following use case:

In the index and show pages of a resource that has an optional belongs to association, display a customizable fallback string (i.e 'none'), when the associated record is not present.

For example, in the index page of a Book that optionally belongs to an Author, display Anonymous if there isn't an associated author.

Is there already a way to achieve this with Field::BelongsTo without having to generate and modify the default views of the field/resource?

How could we go about implementing that?

If possible, having an option (fallback_display_name or similar) that can be passed to Field::BelongsTo to be displayed in the views when the association is not present.

  # app/dashboards/book_dashboard.rb

  ATTRIBUTE_TYPES = {
    author: Field::BelongsTo.with_options(fallback_display_name: 'Anonymous'),
  }

Having a custom field OptionalBelongsTo < Field::BelongsTo that supports the mentioned behavior.

require 'administrate/field/belongs_to'

class OptionalBelongsTo < Administrate::Field::BelongsTo
  def display_associated_resource
    data.present? ? super : options.fetch(:fallback_display_name, "")
  end
end
pablobm commented 1 year ago

That's a valid point. I had a look and my current impression is that we may want to change the belongs_to templates to render the value even if it's not present. Currently this is the code for the show template:

https://github.com/thoughtbot/administrate/blob/9ed2534be2a4129ab077fc76ffb993f6c7b1c980/app/views/fields/belongs_to/_show.html.erb#L18-L27

Perhaps it should change to the following:

<% if field.data && accessible_action?(field.data, :show) %>
  <%= link_to(
    field.display_associated_resource,
    [namespace, field.data],
  ) %>
<% else %>
  <%= field.display_associated_resource %>
<% end %>

The problem though is that this would introduce an incompatibility with existing code. Any display_resource that is not looking out for nils could raise an exception.

I wonder if there's a way to handle this in a way that offers a good transition path. Or perhaps we should do it differently, in one of the ways that you propose.

ammancilla commented 1 year ago

Hi @pablobm,

A combination of:

  1. Changing the views (show & index) as you described.
  2. A new option fallback_display_name (or similar) for Field::BelongsTo.
  3. Overriding display_associated_resource in Field::BelongsTo to use the fallback.

sounds like a good way to tackle the described use case. Only changing the views not only introduces the incompatibility that you mentioned in your comment

The problem though is that this would introduce an incompatibility with existing code. Any display_resource that is not looking out for nils could raise an exception.

but also has the (minor) secondary effect of having to care for handling nils also for the optional belongs to associations for which the current behavior is totally fine.

In general, passing the fallback as an option is not only compatible with existing but it is also more flexible and requires less changes around dashboards.

Let me know what you think.

ammancilla commented 1 year ago

Disclaimer: the option described above is the one I ended up following for now.

pablobm commented 1 year ago

Do you want to prepare a PR to prototype your proposal? No need for specs or docs just yet. Only to see what it would look like and discuss.