Skip to content

Commit

Permalink
Fixes attribute diffing logic when updating REST resources with `.sav…
Browse files Browse the repository at this point in the history
…e` (#1282)

* Adds attributes comparator and test case
* Moves hash_diff to comparator
* Adds changelog entry
* Updates docs/usage/rest.md to explain how updating resource works

Co-authored-by: Paulo Margarido <[email protected]>

* Updates rest.md to omit implementation details

---------

Co-authored-by: Paulo Margarido <[email protected]>
  • Loading branch information
sle-c and paulomarg authored Feb 23, 2024
1 parent 2bb2cff commit 11cbfd3
Show file tree
Hide file tree
Showing 6 changed files with 306 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Note: For changes to the API, see https://shopify.dev/changelog?filter=api

## Unreleased
- [#1282](https://github.com/Shopify/shopify-api-ruby/pull/1282) Fixes a bug where diffing attributes to update not take into account of Array changes and required ids.
- [#1254](https://github.com/Shopify/shopify-api-ruby/pull/1254) Introduce token exchange API for fetching access tokens. This feature is currently unstable and cannot be used yet.
- [#1268](https://github.com/Shopify/shopify-api-ruby/pull/1268) Add [new webhook handler interface](https://github.com/Shopify/shopify-api-ruby/blob/main/docs/usage/webhooks.md#create-a-webhook-handler) to provide `webhook_id ` and `api_version` information to webhook handlers.
- [#1274](https://github.com/Shopify/shopify-api-ruby/pull/1274) Update sorbet and rbi dependencies. Remove support for ruby 2.7.
Expand Down
63 changes: 53 additions & 10 deletions docs/usage/rest.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,29 +69,72 @@ Typical methods provided for each resources are:
Full list of methods can be found on each of the resource class.
- Path:
- https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/rest/resources/#{version}/#{resource}.rb
- Example for `Order` resource on `2023-04` version:
- https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/rest/resources/2023_04/order.rb
- Example for `Order` resource on `2024-01` version:
- https://github.com/Shopify/shopify-api-ruby/blob/main/lib/shopify_api/rest/resources/2024_01/order.rb

### Usage Examples
⚠️ Reference documentation on [shopify.dev](https://shopify.dev/docs/api/admin-rest) contains more examples on how to use each REST Resources.
### The `save` method

```Ruby
# Find and update a customer email
customer = ShopifyAPI::Customer.find(id: customer_id)
customer.email = "[email protected]"
customer.save!
The `save` or `save!` method on a resource allows you to `create` or `update` that resource.

#### Create a new resource

To create a new resource using the `save` or `save!` method, you can initialize the resource with a hash of values or simply assigning them manually. For example:

```Ruby
# Create a new product from hash
product_properties = {
title: "My awesome product"
}
product = ShopifyAPI::Product.new(from_hash: product_properties)
product.save!

# Create a product manually
# Create a new product manually
product = ShopifyAPI::Product.new
product.title = "Another one"
product.save!
```

#### Update an existing resource

To update an existing resource using the `save` or `save!` method, you'll need to fetch the resource from Shopify first. Then, you can manually assign new values to the resource before calling `save` or `save!`. For example:

```Ruby
# Update a product's title
product = ShopifyAPI::Product.find(id: product_id)
product.title = "My new title"
product.save!

# Remove a line item from a draft order
draft_order = ShopifyAPI::DraftOrder.find(id: draft_order_id)

new_line_items = draft_order.line_items.reject { |line_item| line_item["id"] == 12345 }
draft_order.line_items = new_line_items

draft_order.save!
```

> [!IMPORTANT]
> If you need to unset an existing value,
> please explicitly set that attribute to `nil` or empty values such as `[]` or `{}`. For example:
>
> ```Ruby
> # Removes shipping address from draft_order
> draft_order.shipping_address = {}
> draft_order.save!
> ```
>
> This is because only modified values are sent to the API, so if `shipping_address` is not "modified" to `{}`. It won't be part of the PUT request payload
When updating a resource, only the modified attributes, the resource's primary key, and required parameters are sent to the API. The primary key is usually the `id` attribute of the resource, but it can vary if the `primary_key` method is overwritten in the resource's class. The required parameters are identified using the path parameters of the `PUT` endpoint of the resource.
### Usage Examples
⚠️ The [API reference documentation](https://shopify.dev/docs/api/admin-rest) contains more examples on how to use each REST Resources.
```Ruby
# Find and update a customer email
customer = ShopifyAPI::Customer.find(id: customer_id)
customer.email = "[email protected]"
customer.save!
# Get all orders
orders = ShopifyAPI::Orders.all
Expand Down
67 changes: 55 additions & 12 deletions lib/shopify_api/rest/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# frozen_string_literal: true

require "active_support/inflector"
require "hash_diff"

module ShopifyAPI
module Rest
Expand Down Expand Up @@ -187,6 +186,21 @@ def get_path(http_method:, operation:, entity: nil, ids: {})
custom_prefix ? "#{T.must(custom_prefix).sub(%r{\A/}, "")}/#{match}" : match
end

sig do
params(
http_method: Symbol,
operation: Symbol,
).returns(T.nilable(T::Array[Symbol]))
end
def get_path_ids(http_method:, operation:)
found_path = @paths.find do |path|
http_method == path[:http_method] && operation == path[:operation]
end
return nil if found_path.nil?

T.cast(found_path[:ids], T::Array[Symbol])
end

sig do
params(
http_method: Symbol,
Expand Down Expand Up @@ -355,9 +369,14 @@ def save!
sig { params(update_object: T::Boolean).void }
def save(update_object: false)
method = deduce_write_verb

body = {
self.class.json_body_name => attributes_to_update.merge(build_required_attributes(http_method: method)),
}

response = @client.public_send(
method,
body: { self.class.json_body_name => attributes_to_update },
body: body,
path: deduce_write_path(method),
)

Expand All @@ -374,22 +393,34 @@ def save(update_object: false)

sig { returns(T::Hash[String, String]) }
def attributes_to_update
original_state_for_update = original_state.reject do |attribute, _|
updatable_attributes = original_state.reject do |attribute, _|
self.class.read_only_attributes&.include?("@#{attribute}".to_sym)
end

diff = HashDiff::Comparison.new(
deep_stringify_keys(original_state_for_update),
deep_stringify_keys(to_hash(true)),
).left_diff
stringified_updatable_attributes = deep_stringify_keys(updatable_attributes)
stringified_new_attributes = deep_stringify_keys(to_hash(true))
ShopifyAPI::Utils::AttributesComparator.compare(
stringified_updatable_attributes,
stringified_new_attributes,
)
end

diff.each do |attribute, value|
if value.is_a?(Hash) && value[0] == HashDiff::NO_VALUE
diff[attribute] = send(attribute)
end
sig { params(http_method: Symbol).returns(T::Hash[String, T.untyped]) }
def build_required_attributes(http_method:)
required_attributes = {}

primary_key_value = send(self.class.primary_key)
unless primary_key_value.nil?
required_attributes[self.class.primary_key] = primary_key_value
end

diff
path_ids = deduce_path_ids(http_method)
path_ids&.each do |path_id|
path_id_value = send(path_id)
required_attributes[path_id.to_s] = path_id_value unless path_id_value.nil?
end

required_attributes
end

sig { returns(Symbol) }
Expand All @@ -409,6 +440,18 @@ def deduce_write_path(method)
path
end

sig { params(method: Symbol).returns(T.nilable(T::Array[Symbol])) }
def deduce_path_ids(method)
path_ids = self.class.get_path_ids(http_method: method, operation: method)

if path_ids.nil?
method = method == :post ? :put : :post
path_ids = self.class.get_path_ids(http_method: method, operation: method)
end

path_ids
end

sig { params(hash: T::Hash[T.any(String, Symbol), T.untyped]).returns(T::Hash[String, String]) }
def deep_stringify_keys(hash)
hash.each_with_object({}) do |(key, value), result|
Expand Down
85 changes: 85 additions & 0 deletions lib/shopify_api/utils/attributes_comparator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# typed: strict
# frozen_string_literal: true

require "hash_diff"

module ShopifyAPI
module Utils
module AttributesComparator
class << self
extend T::Sig

sig do
params(
original_attributes: T::Hash[String, T.untyped],
updated_attributes: T::Hash[String, T.untyped],
).returns(T::Hash[String, T.untyped])
end
def compare(original_attributes, updated_attributes)
attributes_diff = HashDiff::Comparison.new(
original_attributes,
updated_attributes,
).left_diff

update_value = build_update_value(
attributes_diff,
reference_values: updated_attributes,
)

update_value
end

sig do
params(
diff: T::Hash[String, T.untyped],
path: T::Array[String],
reference_values: T::Hash[String, T.untyped],
).returns(T::Hash[String, T.untyped])
end
def build_update_value(diff, path: [], reference_values: {})
new_hash = {}

diff.each do |key, value|
current_path = path + [key.to_s]

if value.is_a?(Hash)
has_numbered_key = value.keys.any? { |k| k.is_a?(Integer) }
ref_value = T.unsafe(reference_values).dig(*current_path)

if has_numbered_key && ref_value.is_a?(Array)
new_hash[key] = ref_value
else
new_value = build_update_value(value, path: current_path, reference_values: reference_values)

# Only add to new_hash if the user intentionally updates
# to empty value like `{}` or `[]`. For example:
#
# original = { "a" => { "foo" => 1 } }
# updated = { "a" => {} }
# diff = { "a" => { "foo" => HashDiff::NO_VALUE } }
# key = "a", new_value = {}, ref_value = {}
# new_hash = { "a" => {} }
#
# In addition, we omit cases where after removing `HashDiff::NO_VALUE`
# we only have `{}` left. For example:
#
# original = { "a" => { "foo" => 1, "bar" => 2} }
# updated = { "a" => { "foo" => 1 } }
# diff = { "a" => { "bar" => HashDiff::NO_VALUE } }
# key = "a", new_value = {}, ref_value = { "foo" => 1 }
# new_hash = {}
#
# new_hash is empty because nothing changes
new_hash[key] = new_value if !new_value.empty? || ref_value.empty?
end
elsif value != HashDiff::NO_VALUE
new_hash[key] = value
end
end

new_hash
end
end
end
end
end
Loading

0 comments on commit 11cbfd3

Please sign in to comment.