search

jsonapi-suite / jsonapi_compliable

MIT License
20 stars 35 forks source link

Allow combining sparse fieldsets and extra fields #77

Closed aribouius closed 6 years ago

aribouius commented 6 years ago

Currently, when both fields and extra_fields are specified, only the fields param is respected, while the extra_fields param gets ignored. As a result it is impossible to combine a subset of default fields with extra fields.

Per your suggestion @richmolj, this PR modifies the RenderOptions class to merge extra_fields into fields when both are provided.

aribouius commented 6 years ago

@richmolj it was not immediately clear to me if/how this change might affect guarded attributes. I wrote a couple of tests that attempted to verify that guards weren't accidentally getting bypassed, but ultimately wound up ditching them as it felt like they were just testing the jsonapi-rb interface. Please let me know if you'd like me to revisiting them.

richmolj commented 6 years ago

tests that attempted to verify that guards weren't accidentally getting bypassed, but ultimately wound up ditching them as it felt like they were just testing the jsonapi-rb interface

Great thought! I think this is a perfect example of the test informing us of the design. The guard logic actually gets applied after this code fires - in other words, within jsonapi-rb (actually a mixin we apply to jsonapi-rb but still). So we can avoid the test as this is independent functionality.

I love the tests you did write! Great work and really appreciate the help 👏 ❤️

richmolj commented 6 years ago

Released in 0.11.6 🎉