Support automatic pluralization of types

[amysutedja]

Resolves #301

Enables support for pluralize_type in serializer definitions and relationship definitions, which pluralizes the type as expected.

Additional Note: In theory it's possible that we could try to infer whether relationship types should be pluralized by asking the relationship record's serializer about its pluralization state. But currently the code only interacts with the relationship record very lightly, and I was concerned about potentially missing performance targets for complicated records, so I didn't research this path very extensively.

[glyoko]

Should remove this line, or else pluralize_type true isn't doing anything. This line manually sets the type to :films.

[glyoko]

Every example in this group should be using let(:type_name) { :film }, so this should be describe-level.

[amysutedja]

Can you explain more about why this should be the case?

[glyoko]

Look at line 541. That's directly setting the type to be plural in the before block, before the MovieSerializer.pluralize_type true line is even hit. In fact, you could comment out line 524 and that test would still pass.

Given that, both examples should be given a singular type name for these tests to check anything useful. In fact, you could even get rid of the let altogether and just say MovieSerializer.set_type :film on line 523.

[glyoko]

As it stands there's an issue where type is not correctly pluralized in relationships. The following test shows this:

    subject(:serializable_hash) { MovieSerializer.new(movie, include: [:actors]).serializable_hash }
    # ...
    context 'when pluralizing a relationship type after #set_type' do
      before do
        ActorSerializer.pluralize_type true
      end

      after do
        ActorSerializer.pluralize_type nil
      end

      it 'returns correct hash which relationship type equals transformed set_type value' do
        expect(serializable_hash[:data][:relationships][:actors][:data][0][:type]).to eq(:actors)
        expect(serializable_hash[:included][0][:type]).to eq(:actors)
      end
    end

This also means there's a mismatch between relationship and include types. Currently:

> serializable_hash[:data][:relationships][:actors][:data][0][:type]
# => :actor
> serializable_hash[:included][:type]
# => :actors

[amysutedja]

Unfortunately, as far as I understand, fast_jsonapi does not actually "know" internally what serializer is associated with any given relationship. The relationship data is built via simply reflecting on the object and constructing the abbreviated relationship structure. In theory it's possible to make it aware of the serializer, but see my original "Additional Note" -- I ended up being worried about missing performance targets, so I didn't implement a change here.

In order to make this scenario work, I implemented the ability to mark a specific relationship as plural with pluralize_type: true. (Check out object_serializer_pluralization_spec.rb.) This isn't ideal, but it solves this scenario in lieu of a higher-level decision about perf targets.

[glyoko]

I sort of glossed over that note when I first reviewed this PR, and after banging my head against this for a bit, I'm coming to the same conclusion... I'm going to investigate this a bit more because it is important for my use case, but I understand that the complexity involved in such a refactor may not be worth the effort.

At any rate it may not be worth including in this PR, as it already achieves the majority of what it sets out to do.

[sdhull]

This would help us move our serializers over to fast_jsonapi. Any update on this PR?

[amysutedja]

@sdhull I haven't heard back on any suggested changes from the maintainers. And the Travis builds break, for some reason.

You might want to note your interest in the original ticket, #301.

[zdavis]

I wonder if you'd be willing to open this PR against https://github.com/fast-jsonapi/fast_jsonapi, which seems to me to be the way forward with this library, since the maintainers aren't responsive here. The travis failures seem completely unrelated to your changes, and I'd love to see this PR merged into that fork of this repo.