Updating assets is broken in v13.0.0

[ClaytonPassmore]

Issue summary

In v12.5.0, you could update the value of an asset by fetching it, setting a new value, and then saving it:

asset = ShopifyAPI::Asset.all(asset: { key: "layout/theme.liquid" }, theme_id: theme.id).first
asset.value = asset.value + "<p>Something new</p>\n"
asset.save!

In v13.0.0, updating an asset results in a 406 Not Acceptable error.

I'm fairly confident this is broken because of the changes made in #1149. We're no longer sending up the required key parameter (because it hasn't changed).

With access to the asset API being removed in API version 2023-04, I'm not super hopeful that this is actually going to get fixed 🙈

• shopify_api version: v13.0.0
• Ruby version: 3.2.2
• Operating system: Ubuntu 22.04 LTS

Expected behavior

Updating an asset should behave similar to the way it did in v12.5.0 (in that it should work 😄). Including the key when we save the model would fix the problem.

v12.5.0 was obviously sending up more attributes than it should have, but it did work. See the screenshot below. Excuse the large code block - I'm replicating how the gem (incorrectly) determined which attributes changed in v12.5.0.

Actual behavior

In v13.0.0, updating an asset results in a 406 Not Acceptable error. Note that I'm using the new private methods introduced in #1149 for determining the changed attributes.

Steps to reproduce the problem

1. Fetch an asset
2. Change the asset's value
3. Try to save the asset

[garethson]

https://github.com/Shopify/shopify-api-ruby/pull/1149 also appeared to have broken saving metafields. Example:

metafield = ShopifyAPI::Metafield.all(limit: 1, namespace: "foo", key: "bar").first
metafield.value = "new value"
metafield.save

This returns {"errors":{"metafield":"Required parameter missing or invalid"} an error because neither key nor namespace are sent, as those fields did not change.

[nickmealey]

I'm also seeing this issue. If the key/value aren't changed they aren't included in the request.

[nickmealey]

Looking at this closer it appears this commit is causing the issue: https://github.com/Shopify/shopify-api-ruby/commit/9a7ef8a6a22548b6ad7049ecb02b86f598b5f3a5

[flavio-b]

Yup, we're seeing the same issue. I've opened a ticket with Shopify via Partner Support and reference this thread.

[flavio-b]

While updating the value directly used to work, I noticed the example in the official docs, about updating an Asset's value, actually involves creating a new instance, setting the key and value manually, and saving. That seems to do the trick.

I'm not sure if the error we're seeing now is the expected behaviour.

[nelsonwittwer]

Thanks for bring this to our attention and thanks to @flavio-b for linking the docs for the endpoint in question.

It's unlcear if this is a documentation error or an API error but we will take a look! If we need to provide the key / value for this update endpoint to work we can update our logic accordingly. These REST resources are auto generated from an OpenAPI schema, so tracking down that question can be a little tricky. If there is a path forward of creating a new instance as @flavio-b pointed out, I'd use that fix in the meantime.

[mvanholstyn]

I've recently run into an issue that I believe may be related to this.

In my use case, I am trying to update a DraftOrder via the api.

This code:

@shopify_draft_order = ShopifyAPI::DraftOrder.find(id: @shopify_draft_id)
@shopify_draft_order.line_items = []
@shopify_draft_order.save!

Results in this error:

ShopifyAPI::Errors::HttpResponseError ({"errors":{"line_items":"expected Hash to be a Array"},"error_reference":"If you report this error, please include this id: a055933c-9dbf-4972-9060-012ffc9dd971."})

Including actual actual line items results in the same.

While digging into this, I found this to be the request generated by the gem:

[HTTParty] [2023-08-04 21:07:30 +0000] > {"draft_order":{"line_items":{"0":{"id":"HashDiff::NO_VALUE","variant_id":"HashDiff::NO_VALUE","product_id":"HashDiff::NO_VALUE","title":"16AH-CC ","variant_title":"HashDiff::NO_VALUE","sku":"16AH-CC","vendor":"HashDiff::NO_VALUE","quantity":1,"requires_shipping":true,"taxable":true,"gift_card":"HashDiff::NO_VALUE","fulfillment_service":"HashDiff::NO_VALUE","grams":"HashDiff::NO_VALUE","tax_lines":"HashDiff::NO_VALUE","applied_discount":{},"name":"HashDiff::NO_VALUE","properties":[{"name":"Name","value":"Chad Durrance"},{"name":"Address","value":"300 S Hyde Park Ave Ste 201, Tampa, FL 33606-2293"},{"name":"ShipDate","value":"2023-08-29"},{"name":"Phone","value":"(813) 477-3390"},{"name":"Shipping","value":"Ground ($8.95)"},{"name":"_key","value":"86590f55-1f2a-479c-8409-04d487e758a7"},{"name":"_data","value":"eJwVj81qwzAMx1_F6LRBPRR_xHZuo2X0sMMg7QOIxEmhaRqcpdCNvfuk2-__ISH9wvUJDcTaJxy819VgSLuQOh0dJo2udzHk4CMF2MEwc3d_oZ55Ej5spdDcZdbdHZp5m6YdLBdOXmJlX5ULQVubkHOq2LWIqlXHZ5_VF5Wren_IKBmO2u-sDFYiV5Yfn0Id04luC4n4kQW2xlobkyw7pYdmoGnNHMoX55bN9cY0lvs2y5UrV8CgsRqjNkmcRf59S555lC78_QP8fkHK"}],"custom":"HashDiff::NO_VALUE","price":"$10.00","admin_graphql_api_id":"HashDiff::NO_VALUE"}},"billing_address":{"province":"FL","country":"US","latitude":"HashDiff::NO_VALUE","longitude":"HashDiff::NO_VALUE","name":"HashDiff::NO_VALUE","default":false},"shipping_line":{"handle":"HashDiff::NO_VALUE","price":"$8.95"},"note_attributes":{"0":{"name":"TAPPID","value":13},"1":{"name":"OrderType","value":"corporate"},"2":{"name":"ShipDate","value":"2023-08-29"},"3":{"name":"UseMultiship","value":"Yes"},"4":{"name":"Summarized","value":false}},"customer":{"accepts_marketing":"HashDiff::NO_VALUE","accepts_marketing_updated_at":"HashDiff::NO_VALUE","created_at":"HashDiff::NO_VALUE","currency":"HashDiff::NO_VALUE","default_address":"HashDiff::NO_VALUE","email":"HashDiff::NO_VALUE","email_marketing_consent":"HashDiff::NO_VALUE","first_name":"HashDiff::NO_VALUE","id":null,"last_name":"HashDiff::NO_VALUE","last_order_id":"HashDiff::NO_VALUE","last_order_name":"HashDiff::NO_VALUE","marketing_opt_in_level":"HashDiff::NO_VALUE","multipass_identifier":"HashDiff::NO_VALUE","note":"HashDiff::NO_VALUE","orders_count":"HashDiff::NO_VALUE","phone":"HashDiff::NO_VALUE","sms_marketing_consent":"HashDiff::NO_VALUE","state":"HashDiff::NO_VALUE","tags":"HashDiff::NO_VALUE","tax_exempt":"HashDiff::NO_VALUE","tax_exemptions":"HashDiff::NO_VALUE","total_spent":"HashDiff::NO_VALUE","updated_at":"HashDiff::NO_VALUE","verified_email":"HashDiff::NO_VALUE","admin_graphql_api_id":"HashDiff::NO_VALUE"}}}

Notice specifically that the line_items attribute is not being sent as an array, but rather as a hash with the key being the index of the line item from the array.

I've tracked it back to this commit, back in January of 2022 (https://github.com/Shopify/shopify-api-ruby/commit/0abd7235c0aab8e94b414430bc10fb746ed354ee).

This commit introduced the usage of HashDiff to do the comparison, which is what is causing the erroneous data to be generated and sent to the API. Here is a simplified view of the issue:

array1 = [{a:1},{b:2}]
array2 = [{a:2},{b:2}]
HashDiff::Comparison.new({list_items: array1}, {list_items: array2}).left_diff

=> {:list_items=>{0=>{:a=>2}}}

This NEEDS to return an array under the list_items key in order for this to with the Shopify API, but instead it is returning a hash with the keys being the index of items in the array.

@nelsonwittwer hopefully this helps pinpoint the fix for this.

[nelsonwittwer]

From what I can tell there are few problems going on with tracking attribute changes outstanding:

1. Required fields for some resources are not being sent in update requests
2. Attributes that were added after creation don't work

Required Fields

This worked in previous versions because we were sending the entire state of the current object which led to other big unexpected issues as reported in https://github.com/Shopify/shopify-api-ruby/issues/1133 and https://github.com/Shopify/shopify-api-ruby/issues/1037. Since we are now only sending modified fields, this has the unfortunate side affect of not always sending all the required fields.

The solution to this issue would be to send the attributes_to_update as well as the required fields needed for each request. Unfortunately, we don't have the concept of required fields in our REST resources right now :(

The OpenAPI schemas that generate these resources do know the required fields for each resource, however; so getting at these required fields in the resources should be doable. I will open an issue to look into adding that functionality within our next release.

Work around

Reverting to sending the whole modified object isn't an option as it had it's own bad consequences as earlier noted.

I'd recommend using using @flavio-b 's suggested work around for the Asset resource or using the GraphQL interface for modifying assets or any other similarly affected REST resource until we can add those required fields in our next release.

Attributes added after creation

This will be fixed with #1197

[luklapp]

@nelsonwittwer I have a similar issue when updating arrays. Example:

@order = ShopifyAPI::Order.find(id: params[:id])
@order.note_attributes = @order.note_attributes + [NEW ATTRIBUTES]
@order.save!

It produces the broken diff mentioned by @mvanholstyn. My current workaround looks like this:

@order.note_attributes = @order.note_attributes + [{'name' => 'foo', 'value' => 'bar'}]
@order.original_state['note_attributes'] = nil

@order.save!

Resetting the note_attributes of the original_state to nil produces the correct diff and saving works.

[mvanholstyn]

@luklapp what I found works for me is instead of finding the order, doing this..

@order = ShopifyAPI::Order.new
@order.id = params[:id]
@order.note_attributes = @order.note_attributes + [NEW ATTRIBUTES]
@order.save!

Doing it this way causes it to only update the attributes you set, and the attributes you don't set get left as-is.

[ristooi]

While there are dirty workarounds for this issue, is Shopify planning to fix this issue in a clean way before the API_VERSION 2023-04 expires (the last that gem version 12.5.0 supports)?

Failure to get a proper fix will require me to refactor a lot of code. If such refactors would rely on dirty workarounds, sooner or later I can expect to refactor again.