nozzlegear / ShopifySharp

ShopifySharp is a .NET library that helps developers easily authenticate with and manage Shopify stores.
https://nozzlegear.com/shopify-development-handbook
MIT License
764 stars 313 forks source link

Problem with InventoryLevelService Delete #711

Open jeanux opened 2 years ago

jeanux commented 2 years ago

Hi, I think I detected a little issue with the InventoryLevelService.DeleteAsync function. It currently doesn't actually delete the InventoryLevel. I've pintpointed it to the way you pass the uri string to the ShopifyService.PrepareRequest function. DeleteAsync passes the path including the query string to the PrepareRequest function, but as I've been able to confim (at least using .net framework 4.7.2), the UriBuilder takes a path like for example inventory_levels.json?inventory_item_id=12622724694118&location_id=60937896038 and the outputs the AbsoluteUri as https://mystore.myshopify.com/admin/api/2021-10/inventory_levels.json**%3F**inventory_item_id=12622724694118&location_id=60937896038, so it encodes the question mark that precedes the query into %3F. If you instead pass the query string separately to the UriBuilder constructor (e.g., Path ="inventory_levels.json" Query = "inventory_item_id=12622724694118&location_id=60937896038") the AbsoluteUri is properly generated as https://mystore.myshopify.com/admin/api/2021-10/inventory_levels.json?inventory_item_id=12622724694118&location_id=60937896038, so that could be a possible solution.

I don't know if this affecs other users and possibly other function that, as DeleteAsync, pass the query string as part of the path to the PrepareRequest function. Anyway, let me know if you can confirm this is a bug.

jeanux commented 2 years ago

On second inspection (and after digging further into your Infrastructure code) I now realize you actually use a RequestUri class with a ToUri method to construct the query string part of the request. Applying this to the InventoryLevelService.DeleteAsync, I could get it to work with this change:

public virtual async Task DeleteAsync(long inventoryItemId, long locationId, CancellationToken cancellationToken = default) { //await ExecuteRequestAsync(PrepareRequest($"inventory_levels.json?inventory_item_id={inventoryItemId}&location_id={locationId}"), HttpMethod.Delete, cancellationToken);

         var req = PrepareRequest($"inventory_levels.json");

         req.QueryParams.Add("inventory_item_id", inventoryItemId);
         req.QueryParams.Add("location_id", locationId);

         await ExecuteRequestAsync(req, HttpMethod.Delete, cancellationToken);

}

This is similar to what you do in other places, like for example the PageService.GetAsync function.

BTW, I should clarify that I'm using version 5.10 on my project, but I'm suspecting the issue is also present in your latest version as the code for InventoryLevelService.DeleteAsync hasn't changed.

Hopefully these are my 5 cents of contribution to your awesome project :)