search

findologic / findologic-api

Library for sending requests to the Findologic API
MIT License
1 stars 3 forks source link

SW-371-split-filter-items #56

Closed zaifastafa closed 4 years ago

codecov-io commented 4 years ago

Codecov Report

Merging #56 into SW-370_split_filters will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@                  Coverage Diff                  @@
##             SW-370_split_filters    #56   +/-   ##
=====================================================
  Coverage                     100%   100%           
- Complexity                    245    260   +15     
=====================================================
  Files                          34     38    +4     
  Lines                         650    682   +32     
=====================================================
+ Hits                          650    682   +32
Impacted Files Coverage Δ Complexity Δ
...nses/Xml21/Properties/Filter/Item/CategoryItem.php 100% <100%> (ø) 5 <5> (?)
...s/Xml21/Properties/Filter/Item/RangeSliderItem.php 100% <100%> (ø) 3 <3> (?)
...s/Xml21/Properties/Filter/Item/VendorImageItem.php 100% <100%> (ø) 2 <2> (?)
...sponses/Xml21/Properties/Filter/Item/ColorItem.php 100% <100%> (ø) 3 <3> (?)
...pi/Responses/Xml21/Properties/Filter/Item/Item.php 100% <100%> (ø) 13 <13> (?)
...C/Api/Responses/Xml21/Properties/Filter/Filter.php 100% <100%> (ø) 26 <14> (+3) :arrow_up:
... and 1 more

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 92ef3cc...ef5a6a2. Read the comment docs.

zaifastafa commented 4 years ago

@TheKeymaster I had refactored these properties in their own classes, but it did not work. I will re-visit and see what can be done.

EDIT: For e.g this error comes up if I declare them in their own sub-classes:

Call to undefined method FINDOLOGIC\Api\Responses\Xml21\Properties\Filter\Item\DefaultItem::getColor()
TheKeymaster commented 4 years ago

@zaifastafa please check where this method is called. I am pretty sure it's in the tests, because they'd need to be changed in a way, so they understand which properties, which filter/item has. Updating the tests accordingly will probably fix this issue.

zaifastafa commented 4 years ago

@TheKeymaster on a related issue, will the ColorFilter also have an ImageItem? Or that is an error in the test as well? because the demo XML is having an image as well in the color.

TheKeymaster commented 4 years ago

@zaifastafa that's for what tests are. Now it comes to my mind... Color filters can have image as well, please add the property for color filters as well :).