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

#as_json option :only Not working #1851

Open
brosintoski opened this issue Jul 19, 2016 · 9 comments
Open

#as_json option :only Not working #1851

brosintoski opened this issue Jul 19, 2016 · 9 comments

Comments

@brosintoski
Copy link

brosintoski commented Jul 19, 2016

Expected behavior vs actual behavior

Expected: Only fields passed to only option are returned from as_json.

Actual: All fields/attributes of the serializer are returned.

Steps to reproduce

  1. Create an activerecord model called User with attributes: name and status.
  2. Create a serializer for that model with attributes :id, :name, :status
  3. From console execute following steps:
    UserSerializer.new(User.new(name: 'foo bar', status: 'active')).as_json(only: [:id, :name])
  4. Note that status is also returned.

Environment

ActiveModelSerializers Version (commit ref if not on tag): aa4d89a

Output of ruby -e "puts RUBY_DESCRIPTION": ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]

OS Type & Version: OS X v10.11.5

Integrated application and version (e.g., Rails, Grape, etc): rails 4.2.6

Backtrace

(e.g., provide any applicable backtraces from your application)

Additonal helpful information

(e.g., Gemfile.lock, configurations, PR containing a failing test, git bisect results)

@bf4
Copy link
Member

bf4 commented Jul 21, 2016

So, this isn't currently supported. Are there docs that suggest otherwise? I've been thinking about the differences between the AMS serializer api and the active model serialization api, but nothing's happened yet.

Related #1845

@bf4
Copy link
Member

bf4 commented Jul 27, 2016

@brosintoski Well, that's clearly a major fail on my part. Those comments weren't meant to stick around so long.

    # TODO: Include <tt>ActiveModel::Serializers::JSON</tt>.
    # So that the below is true:

was meant for people reading the code to see that this should be implemented... I think if I just made it an issue it would get lost.. hmm

@brosintoski
Copy link
Author

@bf4 no problem. For now I just override #relationship_value_for in my base serializer to allow support for something like: fields: {roles: [:id, :name], default_account: [:id, :code]}. To me that format is more inline with arel include option. The nested only/exclude just add unnecessary complexity since I never use exclude logic.

With that said I'm open to any design that achieves filtering of base and associative attributes as a serialization option, so will just watch this project for updates.

@bf4
Copy link
Member

bf4 commented Jul 29, 2016

Good eyes on relationship_value_for. That method is does a bit too much. I'm working in it right now to fix #1857

@bf4
Copy link
Member

bf4 commented Jul 29, 2016

@brosintoski Mind include a snippet for anyone else whom it might help? (Or maybe it should even be a PR?)

@brosintoski
Copy link
Author

Happy to share the code but would consider it a stop gap solution for now due to how relationship_value_for is structured. Had to completely override that method (i.e. no use of super).

Here is the code:

  def options_for_association(association,adapter_options)
    options = {}
    fields_option = if !adapter_options.key?(:fields)
      nil
    elsif adapter_options[:fields].is_a?(Hash)
      adapter_options[:fields][association.name]
    elsif adapter_options[:fields].is_a?(Array)
      detected_fields_option = adapter_options[:fields].detect {|x| x[association.name] if x.is_a?(Hash) && x.key?(association.name) }
      detected_fields_option[association.name] if detected_fields_option
    end
    options.merge!(fields: fields_option) if fields_option
    options
  end

  def relationship_value_for(association, adapter_options, adapter_instance)
    return association.options[:virtual_value] if association.options[:virtual_value]
    association_serializer = association.serializer
    association_object = association_serializer && association_serializer.object
    return unless association_object

    options = options_for_association(association, adapter_options) || {}
    relationship_value = association_serializer.serializable_hash(adapter_options, options, adapter_instance)

    if association.options[:polymorphic] && relationship_value
      polymorphic_type = association_object.class.name.underscore
      relationship_value = { type: polymorphic_type, polymorphic_type.to_sym => relationship_value }
    end

    relationship_value
  end

@bf4
Copy link
Member

bf4 commented Jul 29, 2016

@brosintoski yeah, method just part of an ongoing effort to improve the design, which is why I labeled it as api private :)

@bf4
Copy link
Member

bf4 commented Aug 5, 2016

Related #1643

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants