Taxes displayed properly on credit memo #264
Conversation
AdamKasp
force-pushed
the
taxes-displayed-properly-on-tax-memo
branch
3 times, most recently
from
fdd9b77 to
36f832d
Compare
GSadee
force-pushed
the
taxes-displayed-properly-on-tax-memo
branch
from
36f832d to
af6d25e
Compare
GSadee
changed the title
GSadee
force-pushed
the
taxes-displayed-properly-on-tax-memo
branch
2 times, most recently
from
48ea44d to
130932a
Compare
GSadee
force-pushed
the
taxes-displayed-properly-on-tax-memo
branch
from
130932a to
a63ae1e
Compare
| } | ||
|
|
||
| if ($taxAdjustments->count() > 1) { | ||
| throw MoreThanOneTaxAdjustment::occur(); |
There was a problem hiding this comment.
Should we consider the Exception keyword at the end of the exception name as a standard in Sylius or not?
There was a problem hiding this comment.
In this particular plugin, half of the exceptions have Exception suffix and half do not. As I remember, we wanted to avoid using this keyword but I don't insist and I don't have a good argument for either of these approaches.
|
|
| @@ -45,19 +52,45 @@ private function convertUnitRefundToLineItem(ShipmentRefund $shipmentRefund): Li | |||
| ; | |||
| Assert::notNull($shippingAdjustment); | |||
|
|
|||
| /** @var ShipmentInterface $shipment */ | |||
There was a problem hiding this comment.
Not exactly related to PR, but maybe it should be like so
--- private function convertUnitRefundToLineItem(ShipmentRefund $shipmentRefund): LineItemInterface
+++ private function convertShipmentRefundToLineItem(ShipmentRefund $shipmentRefund): LineItemInterface
| <tr class="total"> | ||
| <td colspan="6"></td> | ||
| <td>{{ 'sylius_refund.ui.net_total'|trans([], 'messages', creditMemo.localeCode) }}:</td> | ||
| <td>{{ '%0.2f'|format(creditMemo.getNetValueTotal()/100) }}</td> |
There was a problem hiding this comment.
I see this logic in several places, but I'm wondering why don't we use some kind of many formatter here, just to unify extract this division to separate template
|
Thank you, Adam! 🥇 |
| /** @var TaxRateProviderInterface */ | ||
| private $taxRateProvider; | ||
|
|
||
| public function __construct(RepositoryInterface $adjustmentRepository, TaxRateProviderInterface $taxRateProvider) |
There was a problem hiding this comment.
This is also a bc break. Can we add a note to upgrade?
…ineItemsConverter (GSadee) This PR was merged into the 1.0-dev branch. Discussion ---------- Fix for comment: #264 (comment) Commits ------- f8a0d9d Add missing note about changes in constructor of ShipmentLineItemsConverter
Show Page:
PDF:
TODO: