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

ShopifyAPI::CustomerAddress.all always returns an empty array #1018

Closed
kaarelss opened this issue Sep 22, 2022 · 11 comments · Fixed by #1208
Closed

ShopifyAPI::CustomerAddress.all always returns an empty array #1018

kaarelss opened this issue Sep 22, 2022 · 11 comments · Fixed by #1208

Comments

@kaarelss
Copy link
Contributor

kaarelss commented Sep 22, 2022

Issue summary

ShopifyAPI::CustomerAddress.all always returns an empty array.

Expected behavior

It should return an array of CustomerAddress objects

Actual behavior

This happens because create_instance_from_response method is unable to create instances if normalized resource class name is different than parent attribute in API response. It tries to find normalized class_name inside response body, but it could not find it because response from API contains parent attribute addresses instead of customer_addresses. So the resource class should be named ShopifyAPI::Addresses or API response should contain customer_addresses as parent attribute. This is same issue just with different resource - #992

So this results as no objects created and returned an empty array.

def create_instances_from_response(response:, session:)
  objects = []

  body = T.cast(response.body, T::Hash[String, T.untyped])

  if body.key?(class_name.pluralize) || (body.key?(class_name) && body[class_name].is_a?(Array))
    (body[class_name.pluralize] || body[class_name]).each do |entry|
      objects << create_instance(data: entry, session: session)
    end
  elsif body.key?(class_name)
    objects << create_instance(data: body[class_name], session: session)
  end

  objects
end

Steps to reproduce the problem

  1. Create Customer with addresses
  2. Find addresses for that customer with - ShopifyAPI::CustomerAddress.all(customer_id: customer_id)

Specifications

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

@itissible/matrixify

@kaarelss kaarelss changed the title #create_instance_from_response is unable to create instances if normalized resource class name is different than parent attribute in API response ShopifyAPI::CustomerAddress.all always returns an empty array Sep 22, 2022
@agramichael
Copy link

Duplicate of #970

@kaarelss
Copy link
Contributor Author

kaarelss commented Oct 5, 2022

This issue also happens with ShopifyAPI::OrderRisk class. API return risks instead of order_risks.

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

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 github-actions bot added the Stale label Dec 5, 2022
@github-actions github-actions bot removed the Stale label Dec 14, 2022
@github-actions
Copy link

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 github-actions bot added the Stale label Feb 13, 2023
@jagthedrummer
Copy link

Not stale.

@github-actions github-actions bot removed the Stale label Feb 18, 2023
@Trimakas
Copy link

Trimakas commented Mar 3, 2023

And its still an issue, in previous versions of the gem! I just did some sleuthing and if you upgrade to 12.4.0 then it works fine. How nice would it be to have this out in the open instead of having to sleuth for hours.. ahh a boy can dream

@github-actions
Copy link

github-actions bot commented May 3, 2023

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 github-actions bot added the Stale label May 3, 2023
@jagthedrummer
Copy link

Not stale, bot sucks.

@github-actions github-actions bot removed the Stale label May 4, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

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 github-actions bot added the Stale label Jul 3, 2023
@jagthedrummer
Copy link

not stale, bot still sucks

@github-actions github-actions bot removed the Stale label Jul 4, 2023
cdmwebs added a commit to cdmwebs/shopify-api-ruby that referenced this issue Aug 8, 2023
Previously, when fetching `all` on CustomerAddress it would return an
empty array. Two issues (Shopify#970, Shopify#1018) were opened outlining the problem.

This PR adds `json_response_body_name` to the CustomerAddress class to
properly fetch the results from the JSON.

Two assertions have been added to the the tests as well.
cdmwebs added a commit to cdmwebs/shopify-api-ruby that referenced this issue Aug 9, 2023
Previously, when fetching `all` on CustomerAddress it would return an
empty array. Two issues (Shopify#970, Shopify#1018) were opened outlining the problem.

Initial attempt was to override `json_response_body_name` which worked
for fetching arrays of results. It doesn't work for fetching individual
results though.

I wound up overriding `create_instances_from_response` for
CustomerAddress. It isn't ideal but it does seem to behave correctly.

Assertions have been added to the the tests as well.
@nelsonwittwer
Copy link
Contributor

We've got this on our plate for this sprint! Sorry this has taken so long :(

cdmwebs added a commit to cdmwebs/shopify-api-ruby that referenced this issue Oct 25, 2023
Previously, when fetching `all` on CustomerAddress it would return an
empty array. Two issues (Shopify#970, Shopify#1018) were opened outlining the problem.

Initial attempt was to override `json_response_body_name` which worked
for fetching arrays of results. It doesn't work for fetching individual
results though.

I wound up overriding `create_instances_from_response` for
CustomerAddress. It isn't ideal but it does seem to behave correctly.

Assertions have been added to the the tests as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants