Paperclip and has_friendly_id support #67

Closed
wants to merge 13 commits into
from

Projects

None yet

2 participants

@capotej
capotej commented Feb 9, 2011

I added support for file uploads (paperclip) and fixed some errors when used with has_friendly_id. Thanks for writing a great plugin!

@neerajdotname neerajdotname commented on the diff Feb 9, 2011
app/views/admin_data/crud/misc/_modify_record.html.erb
@@ -3,14 +3,14 @@
<h3>Modify Record</h3>
<div style='padding-left:15px'>
<p>
- <%= link_to 'Edit', admin_data_edit_path(:klass => klass.name.underscore, :id => model) %>
+ <%= link_to 'Edit', admin_data_edit_path(:klass => klass.name.underscore, :id => model.id) %>
@neerajdotname
neerajdotname Feb 9, 2011 Member

Any particular reason why model.id is needed in all these cases.

@capotej
capotej Feb 9, 2011

When using friendly_id (https://github.com/norman/friendly_id), it overrides to_param on the models, so /model/edit/123 becomes /model/edit/model-friendly-name, which admin_data can't find, however, if you use .id explicitly, it works.

@neerajdotname
Member

Thanks for the pul request. I will have to write a cucumber test for the if condition that you added. Once the test is done then I will merge it.

On a lighter note I thought I was the only guy using this gem since rails_admin is so popular.

@capotej
capotej commented Feb 9, 2011

If I knew how to test it I would, I was just pushing and testing it live with my app (hence all the commits), I'll look forward to reading the test though.

I actually found your gem to be a little better than rails_admin because it was light and simple, rails_admin hangs when loading large associations (when I don't even care about association editing to begin with!). So, good job on keeping it simple.

@neerajdotname
Member

To make admin_data work with friendly_id see the solution presented here

https://github.com/neerajdotname/admin_data/issuesearch?state=open&q=friendly#issue/65

In this way we don't need to have .id everywhere .

If this solution works for you then send another pull request only for paperclip support.

@neerajdotname
Member

We are no longer supporting admin_data. Thanks for the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment