fast_jsonapi icon indicating copy to clipboard operation
fast_jsonapi copied to clipboard

Usage of block inside of an `attribute` block nullifies `attributes` hash

Open kushniryb opened this issue 7 years ago • 8 comments

Hey guys šŸ––

Seems like there's a pretty big issue with custom attributes definition that makes this gem a little bit unusable in some scenarios.

I've a following serializer in my application

class ProductSerializer
  include FastJsonapi::ObjectSerializer

  set_type :product

  attributes :name, :description, :price, :currency, :display_price

  attribute :backorderable do |product|
    product.variants_including_master.any?(&:backorderable?)
  end
end

Here's the response I get

{
    "data": {
        "id": "3",
        "type": "product",
        "attributes": null
    }
}

It seems there's no way to use a block inside of an attribute do / end block without nullifying the attributes hash.

I've tried changing any?(&:backorderable) to any? { |variant| variant.backorderable? } but it produces the same result.

Changing block contents to product.variants_including_master.first.backorderable? for the sake of testing works though and returns the appropriate response.

kushniryb avatar Jun 25 '18 14:06 kushniryb

Update - it does work in some cases, investigating for more information.

kushniryb avatar Jun 26 '18 08:06 kushniryb

2.5.1 :001 > Spree::V2::Storefront::ProductSerializer.new(Spree::Product.first).serialized_json
 => "{\"data\":{\"id\":\"1\",\"type\":\"product\",\"attributes\":null,\"relationships\":{\"variants\":{\"data\":[]},\"option_types\":{\"data\":[]}}}}"
2.5.1 :002 > Spree::V2::Storefront::ProductSerializer.new(Spree::Product.first).serialized_json
 => "{\"data\":{\"id\":\"1\",\"type\":\"product\",\"attributes\":{\"id\":1,\"name\":\"Ruby on Rails Tote\",\"description\":\"Accusamus nisi quia eaque quod magnam reiciendis. Deserunt quod velit excepturi perspiciatis. Aperiam fugit molestias eligendi odit quod numquam. Totam doloribus quia labore dignissimos quae dolores aspernatur.\",\"price\":\"15.99\",\"display_price\":\"$15.99\",\"available_on\":\"2018-06-19 14:28:47 UTC\",\"slug\":\"ruby-on-rails-tote\",\"meta_description\":null,\"meta_keywords\":null,\"shipping_category_id\":1,\"taxon_ids\":[3,11],\"total_on_hand\":10,\"purchasable\":false},\"relationships\":{\"variants\":{\"data\":[]},\"option_types\":{\"data\":[]}}}}"

Somehow it fails during first serialization but works for each next one

bbonislawski avatar Jun 26 '18 08:06 bbonislawski

I've done more investigation and it's definitely connected with this https://gist.github.com/rsutphin/5102133 and https://github.com/Netflix/fast_jsonapi/blob/dev/lib/fast_jsonapi/serialization_core.rb#L76

any? called on activerecord collection somehow calls break underneath and whole hash becomes nil.

Simple fix that works for this case is changing attributes_hash to something like this:

      def attributes_hash(record, params = {})
        attr_hash = {}

        attributes_to_serialize.each do |key, attribute|
          attribute.serialize(record, params, attr_hash)
        end

        attr_hash
      end

I've tried to rewrite it to inject/reduce but same bug appears there. I can make PR with that fix but I cannot provide specs to reproduce the issue ATM.

bbonislawski avatar Jun 26 '18 10:06 bbonislawski

Somehow it fails during first serialization but works for each next one

For me it fails the first 2 times as opposed to one šŸ¤” @bbonislawski is right, it has something to do with any? implementation.

kushniryb avatar Jun 26 '18 12:06 kushniryb

Can you please provide more details what you expect here? For me behavior of block attributes is very predictable

I'm just asking for clear explanation what you want: get true/false in backorderable or what?

ababich avatar Jun 30 '18 15:06 ababich

I’m expecting to receive true or false in backorderable as opposed to the nullified attributes hash

kushniryb avatar Jun 30 '18 15:06 kushniryb

Interesting issue, but without failing spec it's hard to "fix"

Or you can already do some specs?

ababich avatar Jun 30 '18 16:06 ababich

Agree with @ababich please share a failing spec

shishirmk avatar Jul 04 '18 03:07 shishirmk