slince / shopify-api-php

:rocket: Shopify API Client for PHP
MIT License
128 stars 48 forks source link

Fix client delete method for no content response #12

Closed maximzasorin closed 6 years ago

maximzasorin commented 6 years ago

Upon deletion a PriceRule or DiscountCode API answers 204 code without content, and the doRequest method tries do decode incorrect JSON and throws an exceptionInvalidArgumentException.

PR adds a body size check.

I did not immediately notice the error, because the tests were successful. Maybe we need to add tests for the delete methods?

https://help.shopify.com/en/api/reference/discounts/discountcode#destroy https://help.shopify.com/en/api/reference/discounts/pricerule#destroy

codecov[bot] commented 6 years ago

Codecov Report

Merging #12 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #12      +/-   ##
============================================
+ Coverage     90.11%   90.12%   +0.01%     
- Complexity     1144     1145       +1     
============================================
  Files            85       85              
  Lines          2832     2835       +3     
============================================
+ Hits           2552     2555       +3     
  Misses          280      280
Impacted Files Coverage Δ Complexity Δ
src/Client.php 95.52% <100%> (+0.2%) 23 <0> (+1) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c8a3077...707f685. Read the comment docs.

slince commented 6 years ago

Maybe we need to add tests for the delete methods?

yeah, i have added some tests; Slince the remove method has no return value, no assertions is added. This will cause phpunit risk notice.

maximzasorin commented 5 years ago

@slince I added @doesNotPerformAssertions annotation in PR #15 to avoid risky notices