Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace has_one with attribute in template #822

Merged
merged 1 commit into from
Mar 3, 2015

Conversation

bf4
Copy link
Member

@bf4 bf4 commented Feb 25, 2015

has_one is no longer supported

this way we generate valid templates

@@ -3,6 +3,6 @@ class <%= class_name %>Serializer < <%= parent_class_name %>
attributes <%= attributes_names.map(&:inspect).join(", ") %>
end
<% association_names.each do |attribute| -%>
has_one :<%= attribute %>
attribute :<%= attribute %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be belongs_to, not attribute.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say so. There's basically no usage tests around this code. Would you like me to change it?

The generator defines association_names as

def association_names
  attributes.select { |attr| attr.reference? }.map { |a| a.name.to_sym }
end

belongs_to

    # Defines an association in the object that should be rendered.
    #
    # The serializer object should implement the association name
    # as a method which should return an object when invoked. If a method
    # with the association name does not exist, the association name is
    # dispatched to the serialized object.
    def self.belongs_to(*attrs)
      associate(:belongs_to, attrs)
    end

Whereas attribute

    def self.attribute(attr, options = {})
      key = options.fetch(:key, attr)
      @_attributes.concat [key]
      define_method key do
        object.read_attribute_for_serialization(attr)
      end unless method_defined?(key)
    end

I'm not really sure where reference? comes from. Seems to be a schema definition for a has_many, no?

@kurko kurko added the V: 0.10.x label Mar 1, 2015
@kurko
Copy link
Member

kurko commented Mar 3, 2015

Nevermind. I misread this PR before.

kurko added a commit that referenced this pull request Mar 3, 2015
Replace has_one with attribute in template
@kurko kurko merged commit 32343d4 into rails-api:master Mar 3, 2015
@bf4
Copy link
Member Author

bf4 commented Mar 3, 2015 via email

@bf4
Copy link
Member Author

bf4 commented Jun 21, 2015

@kurko Any desire to revert this since was re-added in #725 ? I'm happy to submit a PR.

@bf4 bf4 deleted the fix_has_one branch June 21, 2015 08:06
@kurko
Copy link
Member

kurko commented Jun 25, 2015

Yes, definitely.

bf4 added a commit to bf4/active_model_serializers that referenced this pull request Jun 25, 2015
@bf4 bf4 mentioned this pull request Jun 25, 2015
@bf4
Copy link
Member Author

bf4 commented Jun 25, 2015

Done

aaronlerch pushed a commit to aaronlerch/active_model_serializers that referenced this pull request Jun 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants