Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jul 17, 2013

eeb123d contains a test that fails against master; the other commits contain a proposed fix and doc updates.

It seems that the changes for schema output overloaded the behaviour for setting an alternate attribute key that was already there, as changing the code so that it matched the behaviour described in the README broke other tests.

Here's the local output from the failing test, note the :string => nil.

  1) Failure:
SerializerTest#test_attributes_method_with_computed_attribute_with_type [/Users/ali/Work/github/ali-graham/active_model_serializers/test/serializer_test.rb:90]:
--- expected
+++ actual
@@ -1 +1 @@
-{:user_attributes_with_computed=>{:first_name=>"Jose", :last_name=>"Valim", :full_name=>"Jose Valim"}}
+{:user_attributes_with_computed=>{:first_name=>"Jose", :last_name=>"Valim", :string=>nil, :full_name=>"Jose Valim"}}

I couldn't think of any way around this that didn't involve changing the attributes calling conventions, so I chose to preserve what was already in the code and add type as a separate property. The fix I've adopted may not be quite what's needed, but hopefully it gets part of the way there...

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 9ba634a on ali-graham:computed_attribute_type_fix into 8ac4bf9 on rails-api:master.

@spastorino
Copy link
Contributor

schema method was removed from master and would need to be reimplemented at some point.
Thanks for doing this work anyway.

@spastorino spastorino closed this Oct 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants