administrate icon indicating copy to clipboard operation
administrate copied to clipboard

Errors for STI models (undefined method for paths)

Open dmitryzuev opened this issue 9 years ago • 24 comments

I have models:

  • Topic < ActiveRecord::Base
  • Topics::News < Topic
  • Topics::Article < Topic

The $ rails generate administrate:install created following routes:

# config/routes.rb
# ...
  namespace :admin
    resources :topics
    root to: 'topics#index'
  end

And /admin/ had given me following error:

undefined method `admin_topics_article_path' for #<#<Class:0x007fb0bbf85e30>:0x007fb0c7d159f0>
Did you mean?  admin_topics_path

I think it's problem with how Administrate or Rails handles single table inheritance and model_name of STI models.

My temporary workaround:

class Topic < ActiveRecord::Base
  def self.model_name
    return super if self == Topic
    Topic.model_name
  end
end

dmitryzuev avatar Dec 02 '16 22:12 dmitryzuev

@dmitryzuev Which version of Administrate and Rails are you working with? I wasn't able to reproduce the issue using Administrate 0.3.0 and Rails 5.

I also noticed the following message when I ran rails g administrate:install to create the dashboards.

WARNING: Unable to generate a dashboard for Topics::Article.
Administrate does not yet support namespaced models.

There's been a couple of issues and pull requests regarding namespaced models. This one seems to have a lot of activity.

carlosramireziii avatar Feb 23 '17 20:02 carlosramireziii

Can confirm this issue. Occuring at https://github.com/thoughtbot/administrate/blob/v0.3.0/app/views/administrate/application/_collection.html.erb#L59

Using versions Rails 5.0.2 and administrate 0.3.0 (strangely, I did not see the warning you mentioned, @carlosramireziii)

Thanks, the workaround @dmitryzuev mentioned works (don't know if it will introduce other issues later though)

stevenmilton58 avatar Mar 09 '17 04:03 stevenmilton58

I am encountering the same issue. Is there something new on this topic? Using version 0.8.1

henvo avatar Sep 04 '17 08:09 henvo

@dmitryzuev @henvo, i also had the same issue and i think where is the problem, @dmitryzuev in db/schema.rb in topics table, did you set default to "Article" ?

update: @carlosramireziii the probleme seems to occur when we set a default value to type field in the STI and i noticed 2 cases: first case: when we set the default in the migration (and db/schema.rb) Administrate will force us to use the model which inherit: for my case i used someting like that

User < ActiveRecord::Base
Administrator < User
Public < User

second case: because i set the type default to Public in the migration, Administrate will force me to use Public class in the dashboard or it will give us the undefined method 'admin_public_path' so, i resolved the problem by removing the default from the migration and put in the model class like this:

class User < ApplicationRecord
  attribute :type, :string, default: 'Public'
end

with this approach i can remove Public from routes ressources without having this error in the main page, but the error comes again when i try to create a new User.

i hope this will help you to understand to source of this error.

update: @carlosramireziii i fixed this issue with this solution in routes.rb:

Rails.application.routes.draw do
  namespace :admin do
    resources :users
    resources :users, as: :administrators
    resources :users, as: :publics

i hope this solution will not create security issues

@dmitryzuev in your case try to do this

namespace :admin
    resources :topics
    resources :topics, as: :articles
    resources :topics, as: :news

i hope it will also works for you

Update: the last solution i suggested generate another erros when i try to update a user with this error message param is missing or the value is empty: user

taab84 avatar Sep 16 '17 16:09 taab84

Since this issue was originally opened, quite a few bits have changed (e.g.: we now support namespaced models, so you won't see warnings there now). What do you think can could improve to make using STI models easier to work with?

nickcharlton avatar Sep 20 '17 12:09 nickcharlton

there is another thing i also noticed with STI, we have the following exemple: -User: Base model -Public: inherit form User -Employee: inherit from User

when i try to edit a public user with an id 5 and change type to Employee, it show an error page inform me that the is no route that match Pulic with an id 5.

taab84 avatar Sep 20 '17 13:09 taab84

not the same issue, because in that case when i update, the form will redirect me to the url: /admin/public/5 and that url don't exist anymore, because after the update it becomes /admin/employee/5. one soultion is to use the base model name in the urls, in our exmple it's /admin/user/5 even if we use child models

taab84 avatar Sep 20 '17 15:09 taab84

Reading through this issue, it sounds like we can solve a significant proportion of this with some better documentation of this use case.

@taab84 Would you be able to try and start this?

It seems like the rest should be bugs in determining paths (which we can fix!) or which should otherwise emerge out.

nickcharlton avatar Sep 26 '17 13:09 nickcharlton

@nickcharlton sorry for late answer, how can i help you?

taab84 avatar Oct 05 '17 21:10 taab84

Hi @taab84!

From reading what you'd said, it seems like we might generally have a bit of a lack of documentation. Do you think you'd be able to help contribute some?

nickcharlton avatar Oct 18 '17 21:10 nickcharlton

I have User and AminUser < User models with STI, and I was able to fix the issue by adding resources :admin_users to the routes:

namespace :admin do
  resources :users
  resources :admin_users
  root to: 'users#index'
end

then running:

rails generate administrate:dashboard admin_user

mibradev avatar Aug 15 '19 05:08 mibradev

@dmitryzuev Following @mibradev suggestion I was able to find solution for my case. In your situation you should be able to do following:

# routes.rb
resources :topics
resources :topics_news, controller: :topics, path: :topics
resources :topics_articles, controller: :topics, path: :topics

This will give you the necessary routes.

The next problem is with edition form because it will send params with keys :topics_news or :topics_article but you need :topic. In this case you could add before_action to Administrate controller.

# topics_controller.rb
module Admin
  class TopicsController < Admin::ApplicationController
    before_action :set_aliases, only: [:edit, :update]

    def set_aliases
      params[:topic] = params[:topics_news] if params[:topics_news]
      params[:topic] = params[:topics_article] if params[:topics_article]
    end
end

olhor avatar Feb 13 '20 01:02 olhor

I've been looking into this today. My current understanding is that Administrate works well with STI models, as long as they are not namespaced. So for example, I think these will work well:

Topic
News < Topic
Article < Topic

But these may be problematic depending on the case:

Topic
Topic::News < Topic
Topic::Article < Topic

A first issue is with Administrate's routes generator, which still doesn't support namespaced models, and will show these warnings:

WARNING: Unable to generate a dashboard for Topic::News.
       - Administrate does not yet support namespaced models.
WARNING: Unable to generate a dashboard for Topic::Article.
       - Administrate does not yet support namespaced models.

This might be ok if you want to handle all STI models from the parent's dashboard. For example: Topic dashboard to manage Topic::News and Topic::Article. However, this creates the problems described in previous comments: the controller sees params for Topic::News and tries to redirect the user to /admin/topics/news/:id, which is not the appropriate route.

A potential solution would be to inspect the model's inheritance chain, trying to figure out the correct controller from there. I have experimented with this at: https://github.com/thoughtbot/administrate/compare/main...pablobm:flexible-sti I'm not creating a PR because this is just an experiment, and I'm sure there are edge cases I'm missing.

Now, say that instead you do want Topic::News and Topic::Article to have each their own dashboard. You could try to invoke the dashboard generator manually ($ ./bin/rails g administrate:dashboard Topic::News). However this doesn't quite generate the expected dashboard and controller, as the paths are missing the namespace. Tweaking manually works.

So a potential solution here would be to work on the generators so that they can figure these things out more effectively.

And that's my research for today. Before trying to tackle these issues, I think we need to have a look at namespacing in general, taking into account all the issues currently opened on the topic. From there, we can try figure out a solution that works in all sensible cases. Unfortunately that's going to take a while.

Until then, understanding what Rails is doing, what the expected paths, controllers, etc are (as per Rails's conventions) and overriding things as described in previous comments, is the best approach.

Remember that Administrate tries to stick to Rails's conventions and avoid magic. Next time you have an issue like this, think "what is Rails trying to do?", and try follow from there.

pablobm avatar Feb 20 '20 12:02 pablobm

There are situations with namespaced models which cannot be automatically resolved by Administrate::ResourceResolver. Fiddling around in Administrate::ApplicationController I found that overrding two methods: resource_class and dashboard_class in my own controller resolves many cases which Administrate::ResourceResolver cannot handle.

olhor avatar Feb 24 '20 10:02 olhor

That observation about the resolver ties in with a comment I made at a different issue: https://github.com/thoughtbot/administrate/issues/405#issuecomment-562161572

To reiterate what I said there: I think the resolver is a bit too clever, trying to figure things out from the path, when it could be doing it from the controller object instance. I want to see more use cases etc first though, before making a change that could be far-reaching.

pablobm avatar Mar 05 '20 14:03 pablobm

@pablobm, what do you think of opening that PR? That's two cases where it's being too clever and as much as I'd like to see a third, reducing cleverness is valuable in itself.

nickcharlton avatar Mar 26 '20 19:03 nickcharlton

@nickcharlton Do you mean flexible-sti (linked from my long comment from 20 Feb), or the stuff about the resolver (mentioned on 5 Mar)?

pablobm avatar Apr 10 '20 06:04 pablobm

I'm following this issue. I created a standalone demo-app to help me with solving the problem. Maybe it will be useful to others as a sandbox.

administrate_sti_demo

softwaregravy avatar Apr 26 '20 15:04 softwaregravy

@pablobm, flexible-sti.

nickcharlton avatar Apr 30 '20 14:04 nickcharlton

Added to the backlog.

pablobm avatar Jun 11 '20 08:06 pablobm

Another approach for STI class with namespace and using pundit

#config/routes.conf
namespace :admin do
   namespace :message do
     resources :messages
     resources :metatraders
   end
end

image

#models/admin/message/message.rb
class Message::Message < ApplicationRecord
  self.table_name = "messages"
end

#models/admin/message/metatrader.rb
class Message::Metatrader < Message::Message
  self.table_name = "messages"
end

#controllers/admin/message/messages_controller.rb
module Admin
  class Message::MessagesController < Admin::BaseController
    def resource_name
      ::Message::Message
    end
    
    def resource_class 
      ::Message::Message
    end

    def dashboard_class
      ::Message::MessageDashboard
     end
  end
end

#controllers/admin/message/metatraders_controller.rb
module Admin
  class Message::MetatradersController < Admin::BaseController
    def resource_name
     ::Message::Metatrader
    end

    def resource_class 
      ::Message::Metatrader
     end

    def dashboard_class
      ::Message::MetatraderDashboard
    end
  end
end

#app/dashboards/admin/message/message_dashboard.rb
require "administrate/base_dashboard"
module Message
  class MessageDashboard < Administrate::BaseDashboard
   ...
  end
end

#app/dashboards/admin/message/metatrader_dashboard.rb
require "administrate/base_dashboard"
module Message
  class MetatraderDashboard < Administrate::BaseDashboard
   ...
  end
end


#app/policies/message/message_policy.rb
class Message::MessagePolicy < ApplicationPolicy
  ...
end

brenoperucchi avatar Dec 27 '22 03:12 brenoperucchi