Support linking to STI models

[matteeyah]

Summary

Administrate doesn't support creating links to STI resources. This change changes ApplicationHelper#accessible_action? so that it:

1. Checks if an action exists for the target class
2. If the action for the target class doesn't exist, check if the base class exists, and if it exists, check if an action exists for the base class.

Backwards compatibility

The behavior for STI superclasses is already broken. It displays flat resources, instead of linking to their respective pages. However, if people are relying on this broken behavior, merging this change will break the experience for them and make it result in an error. If they want to continue treating subclasses as the base class, they can add the following to their STI base class.

def self.model_name
  return super if self == BaseClass

  BaseClass.model_name
end

Related https://github.com/thoughtbot/administrate/issues/2813

[gh-axel-czarniak]

When I proposed this solution (target.class.base_class), I only checked for one level of inheritance. But does it work with multiple level ?

For example:

Request < Message

Authentication < Request

[matteeyah]

@gh-axel-czarniak It probably doesn't work with multiple levels of inheritance. However, this feature is currently broken for codebases with both one level and multiple levels of inheritance. This PR fixes it for relationships with one level of inheritance, so it's a step in the right direction. We can always follow up by adding additional logic to support multiple levels of inheritance.

[mitchwolfe1]

def self.model_name
  return super if self == BaseClass

  BaseClass.model_name
end

Is your proposed workaround supposed to make accessible_action?(resource, :show) return true, or is this intended to be used in combination with the changes in the PR? I've tried adding this to my STI model (replacing BaseClass with the actual base class) and the issue persists.

[matteeyah]

@mitchwolfe1 It needs to be used in combination with the code in the PR.

Overwriting .model_name gives you the same path for all subclasses. Not overwriting it keeps the default behavior of every subclass having its own path.

[pablobm]

Thank you for looking into this @matteeyah! I've been playing with it a bit today.

Regarding @gh-axel-czarniak's question on multilevel inheritance, how about the following implementation? (Inspired by the one in this PR).

    def accessible_action?(target, action_name)
      target = target.to_sym if target.is_a?(String)
      target_class_or_class_name =
        target.is_a?(ActiveRecord::Base) ? target.class : target

      existing_action?(target_class_or_class_name, action_name) &&
        authorized_action?(target, action_name) || superclass_accessible_action?(target, action_name)
    end

    def superclass_accessible_action?(target, action_name)
      return false unless target.is_a?(ActiveRecord::Base)
      return false unless target.class.superclass <= target.class.base_class

      accessible_action?(target.becomes(target.class.superclass), action_name)
    end

Re: having the old behaviour of flat hierarchy, I'd rather we didn't need have to implement self.model_name, as that can have effects elsewhere... This is quite tricky though as the link is determined in the template, so I'm not sure right now what would be a "proper" way to do it. I need to think about it a bit more.

[matteeyah]

@pablobm Nice idea, I implemented your suggestion for adding support for multi-level inheritance.

[pablobm]

I've gone through this again, experimenting and comparing the behaviour currently in main with the behaviour proposed here. Now I'm newly confused and I'm not sure about what we are talking about :sweat_smile: Let's see if we can clarify.

The original issue https://github.com/thoughtbot/administrate/issues/2813 is described in the context of a HasMany field, so I'll go for that. Happy to discuss other variants.

When I compare v0.20.1, main, and the code proposed here, I see no difference in behaviour. Can someone please point me at what I'm missing?

The behaviour I see is the following:

• Assuming that the base class doesn't have a resources entry (this is: there's no dashboard for the base class).
• Edit/destroy links don't appear.
• Rows are still clickable and take the user to the correct show dashboard page for the subclass.

This is not something that can be changed via accessible_action? as there are two issues in the collection template instead:

1. The header of the "actions" column is rendered if existing_action?(collection_presenter.resource_name, action) (see code). This refers to the whole collection, so initially can ask about whether specific subclasses have available actions or not.
   • Alternatives: a) always render the header, b) navigate the inheritance tree to see if any subclass has available actions.
2. The actions for the individual items are rendered under the same condition (see code) However at this point we do have access to the individual records, so it's possible to instead ask the question accessible_action?(resource, action)

Would you be able to share your experiences to help me understand a bit better?

[gh-axel-czarniak]

Maybe we could use this: resource.class.polymorphic_name to correctly handle any class polymorphic or not

[pablobm]

@gh-axel-czarniak - The problem is that when rendering the headers we don't know what the class is yet.

So if we have Car < Vehicle and Boat < Vehicle and visit the /vehicles dashboard, we don't know if we should render the headers. What if Car can be edited but Boat cannot? In that case we should render the "edit" header. To know this, we need to traverse the inheritance chain of Vehicle and see what's available. Or render the headers always.