vhochstein / active_scaffold

Rails 4 Version of activescaffold supporting jquery
MIT License
156 stars 34 forks source link

embedded scaffolds not working under certain conditions #64

Closed crosebrugh closed 13 years ago

crosebrugh commented 13 years ago

AS 2.* called the model method but this fork doesn't (only calls it on the controller). In 2.* if the method returns false then the link is inactive (but shown) on the record.

I'm working around it by calling authorized_for? on the record (when != nil) in .../lib/active_scaffold/actions/delete.rb#delete_authorized?. Also, doing it this way removes the link instead of making it inactive.

Not sure of intent relative to this functionality in 3.* so advice/pointers are welcome.

Thanks,

-cpr

vhochstein commented 13 years ago

Please make sure that you are using a version of activescaffold which is including this commit https://github.com/vhochstein/active_scaffold/commit/881e8d183feb4243f1268b6b6ec062e38f81454f

crosebrugh commented 13 years ago

Thanks.

I updated and it blew up in .../lib/active_scaffold/helpers/view_helpers.rb:193

  def action_link_html(link, url, html_options)
    # issue 260, use url_options[:link] if it exists. This prevents DB data from being localized.
    label = url.delete(:link) if url.is_a?(Hash) 
    label ||= link.label
    if link.image.nil?
   html = link_to(label, url || '', html_options)

due to url being nil: ActionView::Template::Error (No route matches {})

I added the "|| ''" to fix it but I'm not sure that that's correct. Seems something's missing from as_routes in routes.rb?

vhochstein commented 13 years ago

Can you please post your action link definition

crosebrugh commented 13 years ago

It's nothing explicit - just the built-in Delete link. The failure occurs when the model returns false from authorized_for_delete?

vhochstein commented 13 years ago

Just verified that it works on my side. Question is what s the reason for your issue. Can you please setup a clean environment as explained in this post: http://vhochstein.wordpress.com/2010/10/08/one-step-rails-app-with-activescaffold-installation/

Then try to add your authorized_for_delete? method to players model

crosebrugh commented 13 years ago

I confirmed that it works fine with that one step setup. I even moved your controllers to an 'admin' namespace (as in my app) and it works fine.

Comparing that app to mine via the debugger shows the smoking gun to be .../actionpack-3.0.3/lib/action_controller/metal/url_for.rb:11

:_path_segments => request.symbolized_path_parameters

in my app's request the path_parameters are an empty hash, yours are:

{:action=>"index", :controller=>"admin/players"}

the router does not like an empty hash so it bails.

My bet is that my params are {} because I use embedded scaffolds. They are all rendered via a single controller method (Admin::EmbedsController#show) which passes @table as a url param:

render :active_scaffold => "admin/#{@table}", :params => { :inline => true, :layout => false }

In my app each embedded table is under a JS tab and this stuff worked fine in AS 2.

I'm fine with the little workaround. Nice work on this stuff.

-cpr

vhochstein commented 13 years ago

Thanks for your explanation. Seems something is broken... However, I did not quite understand what your workaround is to get that up and running with rails 3 ?

crosebrugh commented 13 years ago

The workaround is referenced in an earlier comment in this thread. It's changing .../lib/active_scaffold/helpers/view_helpers.rb:193 from

html = link_to(label, url, html_options)

to

html = link_to(label, url || '', html_options)

-cpr

vhochstein commented 13 years ago

Do you think you can provide a test app which shows your issue as a git repository?

crosebrugh commented 13 years ago

Here 'tis:

https://github.com/tlatim/howto_embedded

To see the failure just undo the patch (described in the README)

vhochstein commented 13 years ago

Thanks a lot for providing. Unfortunelty, it s working for me even if I undo your patch. I ve tried both ruby 18.7 and 1.9.2 I ve removed debug gem and switched to sqlite3 in gem file, but I do not think that this is the reason....

crosebrugh commented 13 years ago

That's a bummer. I assume that you restarted the server after undoing?

I just updated the repo with vendored gems (no debug) and with the patch not in place by default. Still fails for me.

vhochstein commented 13 years ago

Please change your code in player.rb model and try again:

def authorized_for_delete? return false if self.salary? && self.salary < 50000 true end

crosebrugh commented 13 years ago

That doesn't affect anything. If you ran:

% rake db:seed

you'd have salaries for all players anyway.

vhochstein commented 13 years ago

Maybe, but you will still get a null pointer exception if salary is nil. That s the only issue I had so far with your code. :-(

Hopefully I will be able to try your updated version next days.

crosebrugh commented 13 years ago

OK. I thought that you were saying that that was a problem with debugging this. I'm sure there are a zillion bugs in the code but none that prevent the core issue from being exposed (since I can expose it with the code as is).

vhochstein commented 13 years ago

Great, I am able to reproduce your issue. That s a good start. Unfortuently, it s a really ugly one.. which I think is origined in render_component plugin.

I should have been able to reproduce that issue all the time... So first time I tried I must have your patch still installed...

Anyway, I think you ve got a workaround and I will see how I might be able to fix it.

vhochstein commented 13 years ago

I think I ve fixed the issue in render_component. Can you please update render_component and recheck?

crosebrugh commented 13 years ago

Looks good.

I updated https://github.com/tlatim/howto_embedded with your latest AS and render_component.

Thanks for digging into this.

vhochstein commented 13 years ago

Great. I love to close a bug. :-)