find_if_allowed() raises NilClass exception if record not found

[ghost]

undefined method `authorized_for?' for nil:NilClass

activesupport (3.0.5) lib/active_support/whiny_nil.rb:48:in `method_missing'
active_scaffold_vho (3.0.17) lib/active_scaffold/finder.rb:242:in `find_if_allowed'

May be, some nil checking?... And return nil without attempting to raise ActiveScaffold::RecordNotAllowed ...

[clyfe]

def find_if_allowed(id, crud_type, klass = beginning_of_chain)
  record = klass.find(id)
  raise ActiveScaffold::RecordNotAllowed, "#{klass} with id = #{id}" unless record.authorized_for?(:crud_type => crud_type.to_sym)
  return record
end

That error *cannot be raised in the find_if_allowed because klass.find(id) would raise ActiveRecord::RecordNotFound if no record would be found. Maybe you can pastie or gist a stacktrace ?

[ghost]

I'm rather newbie in rails, but as I know, it's find!(), which raises ActiveRecord::RecordNotFound; ordinary find() without ! sign just returns nil. In case of record = klass.find(id) returns nil, the next line still tries to check permissions on it: ... unless record.authorized_for?(... where record actually is nil! That is what I meant. find_if_allowed doesn't have ! sign, so I can expect it to return nil and not to raise exception if record not found. So I just propose some nil checking before permissions checking. It may look like this: raise ActiveScaffold::RecordNotAllowed, "#{klass} with id = #{id}" if record and not record.authorized_for?(:crud_type => crud_type.to_sym)

@clyfe: here's the pastie, if you wish: http://pastebin.com/2AaH2XXC

[clyfe]

User.find(10)
# ActiveRecord::RecordNotFound: Couldn't find User with ID=10

http://api.rubyonrails.org/classes/ActiveResource/Base.html#method-c-find

See "Failure or missing data" section.

I don't know why it does not raise exception in your case... Maybe you overrided find ?

[ghost]

Thanks for explanation. You are right. I didn't know about different find behavior in case of searching by id. So I'm not "rather newbie", but "completely newbie"... :) Yes. I use https://github.com/Sutto/slugged to find records by slugs. Slugged by itself declares special method find_using_slug(), but I've overrided find to call that method. So while searching by slugs find behaves as usually and don't raise exception. So, it's not a bug of ActiveScaffold. But what about my case? May be I'm doing these things wrong way? Is find overriding good and proper? May it be used? What is the right way to handle this case to have RecordNotFound instead of RecordNotAllowed exception?

[ghost]

But, may be there is some reason to adapt ActiveScaffold to this use case? What do you think?

[clyfe]

The general advice in the community is that overriding core methods like find, save, etc is a bad practice. My personal opinion, especially in the context of AS is that sometimes quick and dirty is useful. Most times not.

1. Your app should never need to "find" something that does not exist. If it does not exist there is no link_to it. How can a user reach there other than forging a URL ?
2. At worst, override in your controller the find_if_allowed method.