Sylius / Sylius/RefundPlugin

[Bug] Provider\RemainingTotalProvider meant to work by ShipmentRefund, but in practice works by shipment Adjustment

Open
#286 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

DX Enhancement
Dominant language
PHP
Stars
73
Forks
76
PR merge metrics
No merged PRs in 30d

Description

Provider\RemainingTotalProvider is used to get available amount for refund for both OrderItemUnitRefund and ShipmentRefund.

That's the theory, in practice Validator\RefundAmountValidator expects to be working with Model\ShipmentRefund, which is correct, but order refund page _shipping.html.twig works with shipping Adjustments which is incorrect and source codes of Provider\RemainingTotalProvider were hacked for UI benefit.

    $shippingAdjustment = $this->adjustmentRepository->findOneBy([
            'id' => $id,
            'type' => AdjustmentInterface::SHIPPING_ADJUSTMENT,
        ]);
        Assert::notNull($shippingAdjustment);

So problem is public function getTotalLeftToRefund(int $id, RefundType $type): int expects one kind of $id and gets totally another and underling code was hacked to support adjustments usage.

Maybe this was conscious decision due to shipment and adjustments not being related in prior 1.9, but it should have been handled explicitly in such case.

Solution is to refactor UI to work with shipments directly.

Personally I've encountered this at non-standard use-case, while loading historical data in DB, so there might be no active bugs with standard usage.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Provider/RemainingTotalProvider.php and Validator/RefundAmountValidator.php to trace which refund ID each path expects, then inspect Resources/views/_shipping.html.twig and its use of shipping adjustments. Done means the refund UI works with shipments directly and the provider no longer needs adjustment IDs as a workaround.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend, web-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.