Closed madmatt closed 4 years ago
Thanks @madmatt yeah one is fine from the looks, all small. The test failure looks related to your changes though so if you could review and update test if required.
Thanks @wilr, I've fixed the test failure now by reverting one of the changes (found out that the Paid
datetime is actually set, but it's done elsewhere now, and the receipt email isn't sent if the paid date already exists 🤦♂
Great test suite to pick that up!
Merging #735 into master will decrease coverage by
0.03%
. The diff coverage is0%
.
@@ Coverage Diff @@
## master #735 +/- ##
============================================
- Coverage 46.12% 46.08% -0.04%
- Complexity 1567 1568 +1
============================================
Files 115 115
Lines 4549 4550 +1
============================================
- Hits 2098 2097 -1
- Misses 2451 2453 +2
Impacted Files | Coverage Δ | Complexity Δ | |
---|---|---|---|
src/Model/Order.php | 57.32% <0%> (-0.18%) |
60 <0> (-1) |
|
src/Model/Variation/Variation.php | 36.75% <0%> (-0.64%) |
33 <2> (+2) |
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 fa234e0...e5381c9. Read the comment docs.
Hey @wilr, any chance you'd be comfortable merging this now that the tests are fixed?
I've got a couple more things coming shortly hopefully as well.
Tests are green so can merge :) thanks for updating! Once the other fixes come in I'll tag a new release.
Awesome, thanks Will!
During a recent upgrade project, we reviewed an old fork that we were on of silverstripe-shop (the precursor to silvershop).
During this, I found the following changes that were missing from silvershop-core, there are four distinct things that we've added / resolved (each commit is a single fix/change that we've made).
The four changes are:
I'm happy to raise these as individual pull requests if you'd prefer, but thought that all four are relatively benign so it made sense to just raise them at the same time. Let me know if you'd prefer I separate them.
Thanks!