Fixes invalid typing of discount code errors

garethson commented 1 year ago

Description

It appears as though when https://github.com/Shopify/shopify-api-ruby/pull/1153 was shipped, it set the errors type the ShopifyAPI::DiscountCode class to be T.nilable(T::Hash[T.untyped, T.untyped])) when it should have just been left as the type defined in ShopifyAPI::Rest::Base.

As reported by @martinsp, this results in an error when ShopifyAPI::Rest::Base attempts to @errors.errors << e.

Note that, specifically in https://github.com/Shopify/shopify-api-ruby/commit/125b6932bc1ab45e3d4d82d809d3aa11b17a71aa, the nuance of the DiscountCode resource returning a list of serialized errors on a normal fetch was dealt with, but I think that PR then should have removed those explicit definitions from the DiscountCode class as I have here. The other solution may have been to actually hook in and create a ShopifyAPI::Rest::Base for the returned errors when listing DiscountCodes after a batch create, but that's a deeper change.

How has this been tested?

Tested manually and also added a test here. It's unclear to me how these tests are now auto-generated so I suspect my manually added test needs to go ... elsewhere?

Checklist:

[x] My commit message follow the pattern described in here
[x] I have performed a self-review of my own code.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] I have updated the project documentation.
[x] I have added a changelog line.

nelsonwittwer commented 1 year ago

Thanks for your contribution! Unfortunately, these REST resource files are auto generated upstream so we won't be able to merge this in as the next time we generate these resources your fixes will be reverted. Having said that, we can apply your fix upstream! We are working on a few other fixes in this area and will add these to our list!

garethson commented 1 year ago

@nelsonwittwer Thanks! I'll watch for updates on this issue, appreciate you pulling this in. 👍

garethson commented 11 months ago

@nelsonwittwer This has been rebased and everything is passing, is it possible to get this looked at?

Thanks!

lizkenyon commented 11 months ago

Hi there 👋

Sorry for the sluggish responses on this. Tldr; we do want to get this merged. We have a ton of upstream friction with the REST resources, and we would ideally make these changes upstream, like we outline in our contribution guidelines.

Though we are planning on merging this when the January API release goes out, and apply these changes.

sle-c commented 9 months ago

hi @garethson , this is great! Would you mind updating the 2024-01 and 2024-04 versions? It'll be nice to apply changes to the latest versions too.

Shopify / shopify-api-ruby