Skip to content

Commit

Permalink
Bug fixed: When changing the status of an operation, the payment was …
Browse files Browse the repository at this point in the history
…not credited to the balance.
  • Loading branch information
arhitov committed Apr 23, 2024
1 parent 3b0ad0a commit 77c666e
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 22 deletions.
2 changes: 0 additions & 2 deletions src/Decrease.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

class Decrease extends Transfer
{
const RECIPIENT_BALANCE_CHANGE = false;

/**
* @param Balance $sender
* @param float $amount
Expand Down
5 changes: 5 additions & 0 deletions src/Enums/OperationStateEnum.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public function isPaid(): bool
OperationStateEnum::Succeeded->value,
]);
}

public function isSucceeded(): bool
{
return $this->value === OperationStateEnum::Succeeded->value;
}
}
2 changes: 0 additions & 2 deletions src/Increase.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

class Increase extends Transfer
{
const SENDER_BALANCE_CHANGE = false;

/**
* @param Balance $recipient
* @param float $amount
Expand Down
22 changes: 21 additions & 1 deletion src/Models/Operation.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use Arhitov\Helpers\Validating\EloquentModelExtendTrait;
use Arhitov\LaravelBilling\Enums\CurrencyEnum;
use Arhitov\LaravelBilling\Enums\OperationStateEnum;
use Arhitov\LaravelBilling\Exceptions\OperationException;
use Arhitov\LaravelBilling\Transfer;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
Expand Down Expand Up @@ -203,6 +205,8 @@ public function recipient_balance(): BelongsTo
/**
* @param AbstractResponse $response
* @return $this
* @throws \Arhitov\LaravelBilling\Exceptions\LaravelBillingException
* @throws \Throwable
*/
public function setStateByOmnipayGateway(AbstractResponse $response): self
{
Expand All @@ -213,14 +217,30 @@ public function setStateByOmnipayGateway(AbstractResponse $response): self
$response->isCancelled() => 'cancel',
};

$this->state = match (true) {
$newState = match (true) {
'waiting_for_capture' === $state => OperationStateEnum::WaitingForCapture,
$response->isSuccessful() => OperationStateEnum::Succeeded,
$response->isCancelled() => OperationStateEnum::Canceled,
default => $this->state,
};

$this->gateway_payment_state = $state;


// If it was active and not paid, but is now paid, then we carry out payment.
if (
$this->state->isActive() &&
! $this->state->isSucceeded() &&
$newState->isSucceeded()
) {
Transfer::make($this)->executeOrFail();
if (! $this->state->isSucceeded()) {
throw new OperationException($this, 'Error transfer');
}
} else {
$this->state = $newState;
}

return $this;
}
}
62 changes: 46 additions & 16 deletions src/Transfer.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Arhitov\LaravelBilling;

use Arhitov\LaravelBilling\Contracts\BillableRootInterface;
use Arhitov\LaravelBilling\Enums\OperationStateEnum;
use Arhitov\LaravelBilling\Events\BalanceDecreaseEvent;
use Arhitov\LaravelBilling\Events\BalanceIncreaseEvent;
Expand All @@ -16,12 +17,28 @@

class Transfer
{
const SENDER_BALANCE_CHANGE = true;
const RECIPIENT_BALANCE_CHANGE = true;

protected ?Operation $operation = null;
protected ?Throwable $exception = null;

/**
* @param Operation $operation
* @return self
* @throws \Arhitov\LaravelBilling\Exceptions\Common\AmountException
*/
public static function make(Operation $operation): self
{
return new self(
$operation->sender_balance,
$operation->recipient_balance,
$operation->amount,
$operation->gateway,
$operation->description,
$operation->operation_identifier,
$operation->operation_uuid,
operation: $operation,
);
}

/**
* @param Balance $sender
* @param Balance $recipient
Expand All @@ -30,6 +47,7 @@ class Transfer
* @param string|null $description
* @param string|null $operation_identifier
* @param string|null $operation_uuid
* @param Operation|null $operation
* @throws \Arhitov\LaravelBilling\Exceptions\Common\AmountException
*/
public function __construct(
Expand All @@ -40,14 +58,15 @@ public function __construct(
protected ?string $description = null,
protected ?string $operation_identifier = null,
protected ?string $operation_uuid = null,
Operation $operation = null,
)
{
if (0 > $this->amount || $this->amount > INF) {
throw new Exceptions\Common\AmountException($this->amount);
}

// Create operation
$this->operation = Operation::make([
$this->operation = $operation ?? Operation::make([
'operation_identifier' => $this->operation_identifier ?? null,
'operation_uuid' => $this->operation_uuid ?? Str::orderedUuid()->toString(),
'gateway' => $this->gateway,
Expand Down Expand Up @@ -97,6 +116,16 @@ public function isAllow(): bool
}
}

public function isChangeSenderBalance(): bool
{
return ! $this->sender->owner instanceof BillableRootInterface;
}

public function isChangeRecipientBalance(): bool
{
return ! $this->recipient->owner instanceof BillableRootInterface;
}

/**
* @return void
* @throws Exceptions\BalanceException
Expand All @@ -107,15 +136,16 @@ public function isAllowOrFail(): void
if ($this->sender->currency !== $this->recipient->currency) {
throw new Exceptions\OperationCurrencyNotMatchException($this->operation);
}
if (static::SENDER_BALANCE_CHANGE) {

if ($this->isChangeSenderBalance()) {
if (! $this->sender->state->isAllowDecrease()) {
throw new Exceptions\BalanceNotAllowDecreaseException($this->sender);
}
if (! is_null($this->sender->limit) && 0 > (($this->sender->amount - $this->amount) + $this->sender->limit)) {
throw new Exceptions\BalanceEmptyException($this->sender);
}
}
if (static::RECIPIENT_BALANCE_CHANGE && ! $this->recipient->state->isAllowIncrease()) {
if ($this->isChangeRecipientBalance() && ! $this->recipient->state->isAllowIncrease()) {
throw new Exceptions\BalanceNotAllowIncreaseException($this->recipient);
}
}
Expand Down Expand Up @@ -213,22 +243,22 @@ public function executeOrFail(): void
/** @var Balance $balanceSender */
if (config('billing.database.use_lock_line') && config('billing.database.use_transaction')) {

if (static::SENDER_BALANCE_CHANGE) {
if ($this->isChangeSenderBalance()) {
$this->sender->lockForUpdate();
}
if (static::RECIPIENT_BALANCE_CHANGE) {
if ($this->isChangeRecipientBalance()) {
$this->recipient->lockForUpdate();
}
$this->operation->lockForUpdate();

if (static::SENDER_BALANCE_CHANGE) {
if ($this->isChangeSenderBalance()) {
// Load actual data
$balanceSender = Balance::findOrFail($this->sender->id);
$balanceSender->amount = $balanceSender->amount - $this->amount;
$balanceSender->saveOrFail();
$this->operation->sender_amount_after = $balanceSender->amount;
}
if (static::RECIPIENT_BALANCE_CHANGE) {
if ($this->isChangeRecipientBalance()) {
// Load actual data
$balanceRecipient = Balance::findOrFail($this->recipient->id);
$balanceRecipient->amount = $balanceRecipient->amount + $this->amount;
Expand All @@ -238,7 +268,7 @@ public function executeOrFail(): void

} else { // If a lock is used, then such a complex mechanism is not needed

if (static::SENDER_BALANCE_CHANGE) {
if ($this->isChangeSenderBalance()) {
$this->sender->newQuery()
->where('id', '=', $this->sender->id)
->limit(1)
Expand All @@ -254,7 +284,7 @@ public function executeOrFail(): void
$this->operation->sender_amount_after = $balanceSender->amount;
}

if (static::RECIPIENT_BALANCE_CHANGE) {
if ($this->isChangeRecipientBalance()) {
$this->recipient->newQuery()
->where('id', '=', $this->recipient->id)
->limit(1)
Expand All @@ -274,18 +304,18 @@ public function executeOrFail(): void
$this->operation->state = OperationStateEnum::Succeeded;
$this->operation->saveOrFail();

if (static::SENDER_BALANCE_CHANGE) {
if ($this->isChangeSenderBalance()) {
$this->sender->amount = $this->operation->sender_amount_after;
}
if (static::RECIPIENT_BALANCE_CHANGE) {
if ($this->isChangeRecipientBalance()) {
$this->recipient->amount = $this->operation->recipient_amount_after;
}
});

if (static::SENDER_BALANCE_CHANGE) {
if ($this->isChangeSenderBalance()) {
event(new BalanceDecreaseEvent($this->sender));
}
if (static::RECIPIENT_BALANCE_CHANGE) {
if ($this->isChangeRecipientBalance()) {
event(new BalanceIncreaseEvent($this->recipient));
}

Expand Down
12 changes: 11 additions & 1 deletion tests/Manual/Console/Commands/YooKassa/WebhookControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public function testRequestByYookassa()
$balance = $owner->getBalanceOrCreate();
$amount = '123.45';

$this->assertEquals(0, $balance->amount);

$omnipayGateway = new OmnipayGateway(self::GATEWAY);

$this
Expand All @@ -43,10 +45,18 @@ public function testRequestByYookassa()
$httpRequest = new Request(
content: json_encode($notification, JSON_UNESCAPED_UNICODE)
);

$response = (new WebhookController())->webhookNotification($httpRequest, 'yookassa');

$this->assertTrue($response->isSuccessful());
$this->assertEquals($omnipayGateway->getConfig('webhook.response.status', 201), $response->getStatusCode());

$operation->refresh();

$this->assertEquals('succeeded', $operation->state->value);
$this->assertEquals('success', $operation->gateway_payment_state);

$balance->refresh();

$this->assertEquals($amount, $balance->amount);
}
}

0 comments on commit 77c666e

Please sign in to comment.