neerajsingh0101 / admin_data

A non instrusive gem which helps you browse, search and manage your data using browser
http://admin-data-demo.heroku.com/admin_data
MIT License
376 stars 56 forks source link

Works on some models but not others #18

Closed davecan closed 14 years ago

davecan commented 15 years ago

Hi, I installed admin_data and it works well on most of my models, but on a few it simply throws errors when I select them. For example, when I select one of the problem models, it returns this error message:

 ActionController::RoutingError in Admin_data/search#search

Showing vendor/plugins/admin_data/app/views/admin_data/search/search/_listing.html.erb where line #25 raised:

admin_data_on_k_url failed to generate from {:action=>"show", :controller=>"admin_data/main", :id=>#, :klass=>"category"}, expected: {:controller=>"admin_data/main", :action=>"show"}, diff: {:id=>#, :klass=>"category"}

Extracted source (around line #25):

22:         
23:           <% if (column.name == klass.primary_key) %>
24:             <%= link_to(record.send(column.name), 
25:                       admin_data_on_k_path(:klass => klass.name.underscore, :id => record)) %>
26:           <% else %>
27:             <%=h  admin_data_get_value_for_column(column,record) %>
28:           <% end %>  

I'm not really sure why it works on some but not others, but any help or advice is appreciated. I can provide more details if needed.

neerajsingh0101 commented 15 years ago

This line is mentioned in the exception trace. {:action=>"show", :controller=>"admin_data/main", :id=>#, :klass=>"category"}, expected: {:controller=>"admin_data/main", :action=>"show"}

Notice that id has a value of #. Not sure why the value of id is hash. Is this model non-standard in any way. For example the primary key of the model might not be id. Or to_param is overriden. Can you go through the model and send me some more information that might seen non-standard. Also if you could extract the business logic out and if you could print gist of the model then that would help.

davecan commented 15 years ago

I figured out the problem a few minutes ago, and while it apparently has nothing to do with admin_data I think it exposes a bug. At least that's the way it appears to me.

The model is standard. It only has a has_many, validates_presence_of, and one custom to_param method. Yes, the primary key column is named id, and it is populated. The problem is that there was another field on the model (named "slug") that was NULL on the exact same record referenced by the error admin_data threw above. This made me stop and think. I dropped into the database and manually updated the row to include the slug, and now admin_data works again.

So why would admin_data error out on a NULL field, when it didn't even report that field as part of the model in the error message? I'm not sure, and I'm assuming the plugin is robust enough to handle NULL values normally, so I don't really know why this would have happened.

Anyway, it works for what I need now. Thanks for the very quick response BTW!

davecan commented 15 years ago

Here's what the model looks like, verbatim. I'm trying to track crime trends and this contains the crime categories.

class Category < ActiveRecord::Base
  has_many :crimes

  validates_presence_of :name, :human_name

  def to_param
    slug
  end
end
davecan commented 15 years ago

Ok, I just ran into something else. Because I overloaded to_param now I can't browse individual records in this model within admin_data. I'll have to remove to_param and go back and change my app to accomodate. Other models return the id properly for URL generation, but this one returns the slug I manually put into each record.

Based on this I'm thinking overloading to_param should be considered a bad idea for interop, even though its touted as a "neato" Rails feature.

neerajsingh0101 commented 15 years ago

This is will help in handling to_param case.

http://wiki.github.com/neerajdotname/admin_data/how-to-handle-to_param-case

neerajsingh0101 commented 15 years ago

that's how the demo is setup. http://demo.neeraj.name/admin_data/user/501-stefanie-blick . notice the SEO friendly url.

neerajsingh0101 commented 15 years ago

I am still trying to understand why you were getting error the first time.

davecan commented 15 years ago

I just duplicated the problem. It is definitely caused by the NULL value in the field I was using in to_param. Here's how it happened. I don't know if you will consider this a bug or not, but thanks for looking.

I have a model called Category. The schema for the table Categories is:

  create_table "categories", :force => true do |t|
    t.string   "name",       :null => false
    t.string   "human_name", :null => false
    t.datetime "created_at"
    t.datetime "updated_at"
    t.string   "slug"
  end

I create the following record:

Category.create(:name=>"TEST", :human_name=>"Test", :slug=>nil)

I now have a record with id=11 with the above info, with slug=NULL in the database. Now reloading /admin_data/category/search causes the following error to appear:

 ActionController::RoutingError in Admin_data/search#search

Showing vendor/plugins/admin_data/app/views/admin_data/search/search/_listing.html.erb where line #25 raised:

admin_data_on_k_url failed to generate from {:controller=>"admin_data/main", :action=>"show", :id=>#, :klass=>"category"}, expected: {:controller=>"admin_data/main", :action=>"show"}, diff: {:id=>#, :klass=>"category"}

Extracted source (around line #25):

22:         
23:           <% if (column.name == klass.primary_key) %>
24:             <%= link_to(record.send(column.name), 
25:                       admin_data_on_k_path(:klass => klass.name.underscore, :id => record)) %>
26:           <% else %>
27:             <%=h  admin_data_get_value_for_column(column,record) %>
28:           <% end %>  

Now I execute this command:

Category.update(11, :slug=>'test')

Now record with id=11 in the database has field slug="test". Now I reload /admin_data/category/search and the screen appears properly.

So as I said, by having the field "slug" set to NULL the page will not load, even though that field is not referenced as belonging to that model at all in the error message that is displayed. The error is definitely caused by the NULL value in the field I have set to use to_param.

If I comment out to_param in the model but leave the NULL value, it works fine, because it then uses the id field.

neerajsingh0101 commented 15 years ago

As I understand to_param, it is not a bug. Since you are using to_param and in to_param you are using slug then that column should never be null. This is why when the id is generated then # is the value and you are getting error.

Fixing either slug value or taking the to_param off is fixing the problem which is the desired behavior.

davecan commented 15 years ago

Yes, exactly. I have modified the call I make elsewhere in the app to use Category#slug instead of relying on a to_param override. Like I said before, I'm thinking overriding to_param can be bad for interop with other code / modules exactly as happened here, but I'm not a Rails expert by any stretch so I may be wrong on this.

Thanks for such a quick response on this though. It's encouraging to have such rapid support on an open source project. Great job!

neerajsingh0101 commented 15 years ago

sure no problem. Rails is full of magic and one of them is to_param.

Thank you for posting your question and explaining in detail what was causing the problem and what made problem go away.

neerajsingh0101 commented 14 years ago

Closing this issue.