Skip to content

Conversation

@paulomarg
Copy link
Contributor

@paulomarg paulomarg commented Oct 5, 2020

Fixes #775

Why am I making these changes?

We had reports that the changes made in #731 were causing the JSON serialization of API objects to break when Base.api_version wasn't set, for instance outside the scope of a temp session.

The issue here was that we were filtering the fields out only when serializing the Product / Variant objects, rather than removing them altogether (even though they were no longer allowed in the API after version 2019-10).

What is this PR doing?

This PR changes those objects so that we remove the fields when the object is first created, so that we don't need to check later on.

early_july_pagination_release!

def initialize(*args, **kwargs)
super(*args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fine as just super.

@paulomarg paulomarg marked this pull request as ready for review October 5, 2020 17:12
@paulomarg paulomarg requested a review from a team as a code owner October 5, 2020 17:12
@andyw8
Copy link
Contributor

andyw8 commented Oct 5, 2020

Since 2019-10 is no longer supported, would we be able to remove that check?

@paulomarg paulomarg force-pushed the fix_variant_serialization branch from 9678533 to f6362d4 Compare October 5, 2020 17:23
@paulomarg paulomarg changed the title [WIP] Filter unwanted product / variant attributes on creation Filter unwanted product / variant attributes on creation Oct 5, 2020
@paulomarg
Copy link
Contributor Author

Since 2019-10 is no longer supported, would we be able to remove that check?

Seems we may not be able to fully drop this code yet, as the Admin API still returns these fields and explicitly forbids saving them. So if we remove this code to filter out these fields, we'll end up trying to save them and that may lead to API requests failing.

@paulomarg paulomarg force-pushed the fix_variant_serialization branch from f6362d4 to f35d6c1 Compare October 6, 2020 14:21
@paulomarg paulomarg merged commit 6c83470 into master Oct 6, 2020
@rezaansyed rezaansyed temporarily deployed to rubygems January 27, 2021 17:18 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Variant#serializable_hash in 9.2.0

4 participants