Vendor & Vendors Redux #86
Conversation
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 77.95% 78.92% +0.96%
==========================================
Files 88 94 +6
Lines 1692 1798 +106
==========================================
+ Hits 1319 1419 +100
- Misses 373 379 +6
Continue to review full report at Codecov.
|
| users | ||
| users, | ||
| vendor, | ||
| vendors |
There was a problem hiding this comment.
Is there a test that needs updating for this?
There was a problem hiding this comment.
Now that you mention it... it seems like there was a test for reducers' index, but it's not in the master branch.
| }); | ||
|
|
||
| it('reduces DELETE_VENDOR action', () => { | ||
| const action = actions.deleteVendor({}); |
There was a problem hiding this comment.
This should test that the vendor is actually removed from the collection
| } else if (result.error) { | ||
| throw result.error; | ||
| } else { | ||
| throw new Error('Failed to get vendor!'); |
There was a problem hiding this comment.
The case where vendor.vendorId === vendorId probably shouldn't produce an error, but should be handled silently, since the data requested is already there... or maybe it should re-fetch it anyway, either way a little more thought is needed here.
src/sagas/vendor.js
Outdated
| } | ||
| } | ||
|
|
||
| function* updateVendorWorker({ details: { vendorId, name, slug, code } }) { |
There was a problem hiding this comment.
Should this be { vendor: { id, name, slug, code } } ? Trying to standardize the object structures a bit so we don't have to do so much transformation.
src/sagas/vendors.test.js
Outdated
| }; | ||
|
|
||
| it('handles success in requestVendorsWorker', () => { | ||
| /* const gen = workers.requestVendorsWorker({ pager }); |
Adds Redux for flavor vendors