Useless HashDiff comparison when saving resource

[kaarelss]

Issue summary

When updating/saving any resource, there is useless HashDiff comparison. It looks like the intended logic is to not send attributes which are not changed, but all attributes are sent always, because comparison result contains duplicate attributes as symbols and as strings.

Expected behavior

original_state should return hash with same attribute types as to_hash methods result, so the HashDiff comparison works correctly.

Actual behavior

original_state return hash with keys as symbols, but to_hash return hash with keys as strings. Therefore the HashDiff comparison have both sets of attributes.

Steps to reproduce the problem

1. Execute the code below

blog = ShopifyAPI::Blog.new(session: ShopifyAPI::Context.active_session)
blog.title = "My Blog"
blog.handle = "my-blog"
blog.save!

blog.handle = "my-blog-changed"
blog.save!

Look at the image, you can see that there are duplicate set of attributes after HashDiff comparison.

Logs

D, [2022-10-12T10:05:40.261857 #41270] DEBUG -- : Pool shutdown finished
D, [2022-10-12T10:05:42.024169 #41270] DEBUG -- : [httplog] Connecting: excelify-karlis-2.myshopify.com:443
D, [2022-10-12T10:05:42.570946 #41270] DEBUG -- : [httplog] Sending: POST http://excelify-karlis-2.myshopify.com:443/admin/api/2022-04/blogs.json
D, [2022-10-12T10:05:42.571025 #41270] DEBUG -- : [httplog] Data: {"blog":{"handle":"my-blog","title":"My Blog"}}
D, [2022-10-12T10:05:42.571044 #41270] DEBUG -- : [httplog] Status: 201
D, [2022-10-12T10:05:42.571062 #41270] DEBUG -- : [httplog] Benchmark: 0.508879 seconds
D, [2022-10-12T10:05:42.571085 #41270] DEBUG -- : [httplog] Response:
{"blog":{"id":79965618227,"handle":"my-blog","title":"My Blog","updated_at":"2022-10-12T10:05:42+03:00","commentable":"no","feedburner":null,"feedburner_location":null,"created_at":"2022-10-12T10:05:42+03:00","template_suffix":null,"tags":"","admin_graphql_api_id":"gid:\/\/shopify\/OnlineStoreBlog\/79965618227"}}
D, [2022-10-12T10:05:43.091768 #41270] DEBUG -- : [httplog] Connecting: excelify-karlis-2.myshopify.com:443
D, [2022-10-12T10:05:43.524617 #41270] DEBUG -- : [httplog] Sending: PUT http://excelify-karlis-2.myshopify.com:443/admin/api/2022-04/blogs/79965618227.json
D, [2022-10-12T10:05:43.524692 #41270] DEBUG -- : [httplog] Data: {"blog":{"id":79965618227,"handle":"my-blog-changed","title":"My Blog","updated_at":"2022-10-12T10:05:42+03:00","commentable":"no","feedburner":null,"feedburner_location":null,"created_at":"2022-10-12T10:05:42+03:00","template_suffix":null,"tags":"","admin_graphql_api_id":"gid://shopify/OnlineStoreBlog/79965618227"}}
D, [2022-10-12T10:05:43.524711 #41270] DEBUG -- : [httplog] Status: 200
D, [2022-10-12T10:05:43.524729 #41270] DEBUG -- : [httplog] Benchmark: 0.413646 seconds
D, [2022-10-12T10:05:43.524760 #41270] DEBUG -- : [httplog] Response:
{"blog":{"title":"My Blog","handle":"my-blog-changed","commentable":"no","feedburner":null,"feedburner_location":null,"template_suffix":null,"id":79965618227,"updated_at":"2022-10-12T10:05:43+03:00","created_at":"2022-10-12T10:05:42+03:00","tags":"","admin_graphql_api_id":"gid:\/\/shopify\/OnlineStoreBlog\/79965618227"}}

Specifications

• shopify_api version: 12.0.0
• Shopify API version used (example: 2022-04):

@itissible/matrixify

[github-actions]

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

[kaarelss]

Here is another case where HashDiff is causing bug.

customer = ShopifyAPI::Customer.new(session: ShopifyAPI::Context.active_session)
customer.first_name = "John"
customer.last_name = "Doe"
customer.save!

customer.last_name = "Doe Doe"
customer.save!

Currently when you update ShopifyAPI::Customer without email (with the example above), you receive error:

{"errors":{"email_marketing_consent":["An email address is required when setting the email marketing consent."]}}

This is probably another bug, because you can not send email_marketing_consent as nil when there is no email . So the workaround for this bug is to remove email_marketing_consent attribute (or now instance variable) for the Customer object.

customer = ShopifyAPI::Customer.new(session: ShopifyAPI::Context.active_session)
customer.first_name = "John"
customer.last_name = "Doe"
customer.save!

customer.last_name = "Doe Doe"
customer.remove_instance_variable(:@email_marketing_consent)
customer.save!

But this example above is causing the same error:

{"errors":{"email_marketing_consent":["An email address is required when setting the email marketing consent."]}}

when removing the email_marketing_consent with customer.remove_instance_variable(:@email_marketing_consent)
It is still sent in the body as empty hash and causing the same error. After HashDiff we have email_marketing_consent as this in the hash :email_marketing_consent => {Class} HashDiff::NO_VALUE and when to_json is called it results as {} empty hash.

{
  "customer": {
    "id": 5994465493043,
    "email": null,
    "accepts_marketing": false,
    "created_at": "2022-12-14T09:06:29+02:00",
    "updated_at": "2022-12-14T09:06:29+02:00",
    "first_name": "John",
    "last_name": "Doe Doe",
    "orders_count": 0,
    "state": "disabled",
    "total_spent": "0.00",
    "last_order_id": null,
    "note": null,
    "verified_email": true,
    "multipass_identifier": null,
    "tax_exempt": false,
    "tags": "",
    "last_order_name": null,
    "currency": "EUR",
    "phone": null,
    "addresses": [],
    "accepts_marketing_updated_at": "2022-12-14T09:06:29+02:00",
    "marketing_opt_in_level": null,
    "tax_exemptions": [],
    "email_marketing_consent": {},
    "sms_marketing_consent": null,
    "admin_graphql_api_id": "gid://shopify/Customer/5994465493043"
  }
}

[github-actions]

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

[github-actions]

We are closing this issue because it has been inactive for a few months.
This probably means that it is not reproducible or it has been fixed in a newer version.
If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!

[matthewlein]

@kaarelss excellent job finding and describing this bug. So disappointing that no one is doing anything about it.

[nelsonwittwer]

Fixed with #1149