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

Add validation and tests for deprecation of product and variant inventory-related fields #655

Merged
merged 1 commit into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions lib/shopify_api/resources/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ def price_range
end
end

def total_inventory=(new_value)
raise_deprecated_inventory_call('total_inventory') unless allow_inventory_params?
super
end

def serializable_hash(options = {})
allow_inventory_params? ? super(options) : super(options).except('total_inventory')
end

def collections
CustomCollection.find(:all, :params => {:product_id => self.id})
end
Expand All @@ -31,5 +40,17 @@ def add_to_collection(collection)
def remove_from_collection(collection)
collection.remove_product(self)
end

private

def raise_deprecated_inventory_call(parameter)
raise(ShopifyAPI::ValidationException,
"'#{parameter}' is deprecated - see https://help.shopify.com/en/api/guides/inventory-migration-guide",
)
end

def allow_inventory_params?
Base.api_version < ApiVersion.find_version('2019-10')
end
end
end
31 changes: 31 additions & 0 deletions lib/shopify_api/resources/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,36 @@ class Variant < Base
include DisablePrefixCheck

conditional_prefix :product

def inventory_quantity_adjustment=(new_value)
raise_deprecated_inventory_call('inventory_quantity_adjustment') unless allow_inventory_params?
super
end

def inventory_quantity=(new_value)
raise_deprecated_inventory_call('inventory_quantity') unless allow_inventory_params?
super
end

def old_inventory_quantity=(new_value)
raise_deprecated_inventory_call('old_inventory_quantity') unless allow_inventory_params?
super
end

def serializable_hash(options = {})
allow_inventory_params? ? super(options) : super(options).except('inventory_quantity', 'old_inventory_quantity')
Copy link
Contributor

@marschattha marschattha Apr 2, 2020

Choose a reason for hiding this comment

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

Changing to

  def serializable_hash(options = {})
    allow_inventory_params? ? super(options) : super(options).tap do |s|
      (s['variant'] || s).except!('inventory_quantity', 'old_inventory_quantity')
    end
  end

Fixes this PR.

Copy link

Choose a reason for hiding this comment

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

+1

Choose a reason for hiding this comment

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

Is this accurate? Those fields were deprecated for writes, but reading variant.total_inventory should still be supported

end

private

def raise_deprecated_inventory_call(parameter)
raise(ShopifyAPI::ValidationException,
"'#{parameter}' is deprecated - see https://help.shopify.com/en/api/guides/inventory-migration-guide",
)
end

def allow_inventory_params?
Base.api_version < ApiVersion.find_version('2019-10')
end
end
end
39 changes: 39 additions & 0 deletions test/product_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,43 @@ def test_price_range_when_has_different_price

assert_equal('100.00 - 199.00', @product.price_range)
end

def test_deprecated_variant_inventory_fields_are_included_in_2019_07
ShopifyAPI::Base.api_version = '2019-07'
variant = @product.variants.first
assert variant.as_json.include?('inventory_quantity')
end

def test_deprecated_variant_inventory_fields_are_removed_in_2020_01
ShopifyAPI::Base.api_version = '2020-01'
variant = @product.variants.first
refute variant.as_json.include?('inventory_quantity')
end

def test_deprecated_inventory_fields_are_removed_in_2020_01
ShopifyAPI::Base.api_version = '2020-01'
refute @product.as_json.include?('total_inventory')
end

def test_setting_product_total_inventory_passes_in_api_before_2019_10
ShopifyAPI::Base.api_version = '2019-07'
fake("products/632910392", {:body => load_fixture('product')})
@product.total_inventory = 8
end

def test_setting_product_total_inventory_fails_in_2019_10_api
ShopifyAPI::Base.api_version = '2019-10'
fake("products/632910392", {:body => load_fixture('product')})
assert_raises(ShopifyAPI::ValidationException) do
@product.total_inventory = 8
end
end

def test_setting_product_total_inventory_fails_in_the_unstable_api
ShopifyAPI::Base.api_version = :unstable
fake("products/632910392", {:body => load_fixture('product')})
assert_raises(ShopifyAPI::ValidationException) do
@product.total_inventory = 8
end
end
end
2 changes: 1 addition & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def fake(endpoint, options={})
body = options.has_key?(:body) ? options.delete(:body) : load_fixture(endpoint)
format = options.delete(:format) || :json
method = options.delete(:method) || :get
api_version = options.delete(:api_version) || ShopifyAPI::ApiVersion.find_version('2019-01')
api_version = options.delete(:api_version) || ShopifyAPI::Base.api_version
extension = ".#{options.delete(:extension)||'json'}" unless options[:extension]==false
status = options.delete(:status) || 200
url = if options.has_key?(:url)
Expand Down
89 changes: 89 additions & 0 deletions test/variant_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,93 @@ def test_delete_variant
v = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert v.destroy
end

def test_deprecated_inventory_fields_are_included_in_2019_07
ShopifyAPI::Base.api_version = '2019-07'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert variant.as_json.include?('inventory_quantity')
end

def test_deprecated_inventory_fields_are_removed_in_2020_01
ShopifyAPI::Base.api_version = '2020-01'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
refute variant.as_json.include?('inventory_quantity')
end

def test_setting_variant_inventory_quantity_adjustment_passes_in_api_before_2019_10
ShopifyAPI::Base.api_version = '2019-07'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
variant.inventory_quantity_adjustment = 8
end

def test_setting_variant_inventory_quantity_adjustment_fails_in_2019_10_api
ShopifyAPI::Base.api_version = '2019-10'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.inventory_quantity_adjustment = 8
end
end

def test_setting_variant_inventory_quantity_adjustment_fails_in_the_unstable_api
ShopifyAPI::Base.api_version = :unstable
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.inventory_quantity_adjustment = 8
end
end

def test_setting_variant_inventory_quantity_passes_in_api_before_2019_10
ShopifyAPI::Base.api_version = '2019-07'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
variant.inventory_quantity = 8
end

def test_setting_variant_inventory_quantity_fails_in_2019_10_api
ShopifyAPI::Base.api_version = '2019-10'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.inventory_quantity = 8
end
end

def test_setting_variant_inventory_quantity_fails_in_the_unstable_api
ShopifyAPI::Base.api_version = :unstable
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.inventory_quantity = 8
end
end

def test_setting_variant_old_inventory_quantity_passes_in_api_before_2019_10
ShopifyAPI::Base.api_version = '2019-07'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
variant.old_inventory_quantity = 8
end

def test_setting_variant_old_inventory_quantity_fails_in_2019_10_api
ShopifyAPI::Base.api_version = '2019-10'
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.old_inventory_quantity = 8
end
end

def test_setting_variant_old_inventory_quantity_fails_in_the_unstable_api
ShopifyAPI::Base.api_version = :unstable
fake "products/632910392/variants/808950810", :method => :get, :body => load_fixture('variant')
variant = ShopifyAPI::Variant.find(808950810, :params => {:product_id => 632910392})
assert_raises(ShopifyAPI::ValidationException) do
variant.old_inventory_quantity = 8
end
end
end