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

Improve jbuilder serialization for Oj gem #3210

Merged
merged 4 commits into from
May 28, 2019

Conversation

kennyadsl
Copy link
Member

@kennyadsl kennyadsl commented May 14, 2019

Description

This PR makes sure we are always nullifying total_on_hand before passing trying to serialize it, if it is a Float::Infinity, which was causing issues with oj.

Checklist:

@kennyadsl kennyadsl added type:bug Error, flaw or fault Need Backport labels May 14, 2019
@kennyadsl kennyadsl self-assigned this May 14, 2019
Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

👍 Thanks Alberto!

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

LGTM!

@pelargir
Copy link
Contributor

v2.9.1 fixes this problem. No need to backport anything.

rails/jbuilder@821f514

@kennyadsl
Copy link
Member Author

@pelargir thanks! I think we can still keep these changes, even it's no more a bug.

@kennyadsl kennyadsl removed type:bug Error, flaw or fault Need Backport labels May 15, 2019
@kennyadsl kennyadsl changed the title Fixes for jbuilder 2.9.0 Improve jbuilder serialization for oj gem May 16, 2019
@kennyadsl
Copy link
Member Author

@ericsaupe @jacobherrington This is no more strictly required but it's a nice to have if we want to support serializing with Oj

@kennyadsl kennyadsl changed the title Improve jbuilder serialization for oj gem Improve jbuilder serialization for Oj gem May 16, 2019
@kennyadsl kennyadsl force-pushed the kennyadsl/jbuilder-2.9.0 branch from 53d1ca2 to 85141c4 Compare May 16, 2019 12:51
kennyadsl added 4 commits May 27, 2019 16:53
We were not testing that Float::Infinity serialization
is working correctly here.
Be sure we are checking that Float::Infinity is being serialized
correctly.
This will be used also in the _product partial.

We can't represent Float::INFINITY in JSON:
  Under JSON this would be NULL
  Under oj this would error
Float::Infinity serialization is broken with Oj gem, see
solidusio#2495 (comment)

This commit makes sure we are always nullifying total_on_hand before
passing trying to serialize it, if it is a Float::Infinite.
@kennyadsl kennyadsl force-pushed the kennyadsl/jbuilder-2.9.0 branch from 85141c4 to 71a1ef7 Compare May 27, 2019 14:54
@kennyadsl kennyadsl added the changelog:solidus_api Changes to the solidus_api gem label May 27, 2019
@kennyadsl kennyadsl merged commit 5fc79f6 into solidusio:master May 28, 2019
@kennyadsl kennyadsl deleted the kennyadsl/jbuilder-2.9.0 branch May 28, 2019 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_api Changes to the solidus_api gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants