Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

use has_attribute? as per https://github.com/rails/rails/issues/4208 #432

Closed
wants to merge 1 commit into from

2 participants

@kidpollo

The raise_on_missing_attribute seems not to be working. I only tested on rails 3.2.11

It seems the current behavior is respond_to? will always return true so has_attribute? is recommended.

I put this together very quickly. Let me know if further investigation or code is required.

@nesquena
Owner

I'm not sure this works considering that rabl is agnostic to ORM. I want rabl to work with every ORM and not just activerecord. Why would everything return true as if it responds, that doesn't make sense?

@kidpollo

I corrected my commit it also needs the respond_to

@kidpollo

@nesquena you are totally right it does not make sense. You can look at the last comment on rails/rails#4208. In AR the method does exist even if you did not fetch the attribute. @josevalim actually seems to agree the expected behavior should be that it does not respond to the method :@

Without this the raise_on_missing_attribute has no effect on Rails. Could there be an alternative?

@kidpollo

Never mind my corrected commit does not work either. I fear this requires more complex logic. Back to the drawing board :(

@kidpollo

The raise_on_missing_attribute option is very useful Rails just does not like it.

@kidpollo

well AR actually

@kidpollo kidpollo closed this
@kidpollo kidpollo reopened this
@kidpollo

Re opened @nesquena I came up with a Fugly hack that seems to works for now. I dont expect you will like this but are there other cases where you handle special cases for a ORM? :S

@nesquena
Owner

It feels too hacky for me to incorporate into rabl but feel free to use it as a fork. I don't know how to fix this but it seems more likely Rails will fix that weird behavior because it shouldn't be returning true for methods it doesn't respond to, thats crazy.

@kidpollo

I agree

@nesquena
Owner

Thanks for taking the time to submit a patch though, I appreciate it. Sorry this came up, I'm hoping this is treated as a bug on their end and fixed.

@nesquena nesquena closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 1 deletion.
  1. +9 −1 lib/rabl/builder.rb
View
10 lib/rabl/builder.rb
@@ -130,7 +130,15 @@ def resolve_condition(options)
# Checks if an attribute is present. If not, check if the configuration specifies that this is an error
# attribute_present?(created_at) => true
def attribute_present?(name)
- if @_object.respond_to?(name)
+ if defined?(Rails)
+ begin
+ @_object.send(name) and return true
+ rescue ActiveModel::MissingAttributeError
+ return false
+ end
+ end
+
+ if @_object.respond_to?(name)
return true
elsif Rabl.configuration.raise_on_missing_attribute
raise "Failed to render missing attribute #{name}"
Something went wrong with that request. Please try again.