Page MenuHomePhorge
Files F117757879

D4794.1775208538.diff
No OneTemporary
Actions

Authored By
Unknown
Size
20 KB
Referenced Files
None
Subscribers
None

D4794.1775208538.diff
View Options

diff --git a/src/app/Console/Commands/Job/WalletCharge.php b/src/app/Console/Commands/Job/WalletCharge.php
--- a/src/app/Console/Commands/Job/WalletCharge.php
+++ b/src/app/Console/Commands/Job/WalletCharge.php
@@ -34,7 +34,7 @@
return 1;
}
- $job = new \App\Jobs\WalletCharge($wallet);
+ $job = new \App\Jobs\WalletCharge($wallet->id);
$job->handle();
}
}
diff --git a/src/app/Console/Commands/Job/WalletCheck.php b/src/app/Console/Commands/Job/WalletCheck.php
--- a/src/app/Console/Commands/Job/WalletCheck.php
+++ b/src/app/Console/Commands/Job/WalletCheck.php
@@ -34,7 +34,7 @@
return 1;
}
- $job = new \App\Jobs\WalletCheck($wallet);
+ $job = new \App\Jobs\WalletCheck($wallet->id);
$job->handle();
}
}
diff --git a/src/app/Console/Commands/Wallet/ChargeCommand.php b/src/app/Console/Commands/Wallet/ChargeCommand.php
--- a/src/app/Console/Commands/Wallet/ChargeCommand.php
+++ b/src/app/Console/Commands/Wallet/ChargeCommand.php
@@ -51,30 +51,7 @@
}
foreach ($wallets as $wallet) {
- // This is a long-running process. Because another process might have modified
- // the wallet balance in meantime we have to refresh it.
- // Note: This is needed despite the use of cursor() above.
- $wallet->refresh();
-
- // Sanity check after refresh (owner deleted in meantime)
- if (!$wallet->owner) {
- continue;
- }
-
- $charge = $wallet->chargeEntitlements();
-
- if ($charge > 0) {
- $this->info("Charged wallet {$wallet->id} for user {$wallet->owner->email} with {$charge}");
-
- // Top-up the wallet if auto-payment enabled for the wallet
- \App\Jobs\WalletCharge::dispatch($wallet);
- }
-
- if ($wallet->balance < 0) {
- // Check the account balance, send notifications, (suspend, delete,) degrade
- // Also sends reminders to the degraded account owners
- \App\Jobs\WalletCheck::dispatch($wallet);
- }
+ \App\Jobs\WalletCheck::dispatch($wallet->id);
}
}
}
diff --git a/src/app/Http/Controllers/API/V4/PaymentsController.php b/src/app/Http/Controllers/API/V4/PaymentsController.php
--- a/src/app/Http/Controllers/API/V4/PaymentsController.php
+++ b/src/app/Http/Controllers/API/V4/PaymentsController.php
@@ -132,7 +132,7 @@
// Trigger auto-payment if the balance is below the threshold
if ($wallet->balance < round($request->balance * 100)) {
- \App\Jobs\WalletCharge::dispatch($wallet);
+ \App\Jobs\WalletCharge::dispatch($wallet->id);
}
$result = self::walletMandate($wallet);
diff --git a/src/app/Jobs/WalletCharge.php b/src/app/Jobs/WalletCharge.php
--- a/src/app/Jobs/WalletCharge.php
+++ b/src/app/Jobs/WalletCharge.php
@@ -8,17 +8,12 @@
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
-use Illuminate\Queue\SerializesModels;
class WalletCharge implements ShouldQueue
{
use Dispatchable;
use InteractsWithQueue;
use Queueable;
- use SerializesModels;
-
- /** @var \App\Wallet A wallet object */
- protected $wallet;
/** @var int The number of seconds to wait before retrying the job. */
public $backoff = 10;
@@ -29,17 +24,19 @@
/** @var bool Delete the job if the wallet no longer exist. */
public $deleteWhenMissingModels = true;
+ /** @var string A wallet identifier */
+ protected $walletId;
/**
* Create a new job instance.
*
- * @param \App\Wallet $wallet The wallet that has been charged.
+ * @param string $walletId The wallet that has been charged.
*
* @return void
*/
- public function __construct(Wallet $wallet)
+ public function __construct(string $walletId)
{
- $this->wallet = $wallet;
+ $this->walletId = $walletId;
}
/**
@@ -49,6 +46,8 @@
*/
public function handle()
{
- PaymentsController::topUpWallet($this->wallet);
+ if ($wallet = Wallet::find($this->walletId)) {
+ PaymentsController::topUpWallet($wallet);
+ }
}
}
diff --git a/src/app/Jobs/WalletCheck.php b/src/app/Jobs/WalletCheck.php
--- a/src/app/Jobs/WalletCheck.php
+++ b/src/app/Jobs/WalletCheck.php
@@ -9,14 +9,12 @@
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
-use Illuminate\Queue\SerializesModels;
class WalletCheck implements ShouldQueue
{
use Dispatchable;
use InteractsWithQueue;
use Queueable;
- use SerializesModels;
public const THRESHOLD_DEGRADE = 'degrade';
public const THRESHOLD_DEGRADE_REMINDER = 'degrade-reminder';
@@ -34,20 +32,23 @@
/** @var bool Delete the job if the wallet no longer exist. */
public $deleteWhenMissingModels = true;
- /** @var \App\Wallet A wallet object */
+ /** @var ?Wallet A wallet object */
protected $wallet;
+ /** @var string A wallet identifier */
+ protected $walletId;
+
/**
* Create a new job instance.
*
- * @param \App\Wallet $wallet The wallet that has been charged.
+ * @param string $walletId The wallet that has been charged.
*
* @return void
*/
- public function __construct(Wallet $wallet)
+ public function __construct(string $walletId)
{
- $this->wallet = $wallet;
+ $this->walletId = $walletId;
}
/**
@@ -57,6 +58,25 @@
*/
public function handle()
{
+ $this->wallet = Wallet::find($this->walletId);
+
+ // Sanity check (owner deleted in meantime)
+ if (!$this->wallet || !$this->wallet->owner) {
+ return null;
+ }
+
+ if ($this->wallet->chargeEntitlements() > 0) {
+ // We make a payment when there's a charge. If for some reason the
+ // payment failed we can't just throw here, as another execution of this job
+ // will not re-try the payment. So, we attempt a payment in a separate job.
+ try {
+ $this->topUpWallet();
+ } catch (\Exception $e) {
+ \Log::error("Failed to top-up wallet {$this->walletId}: " . $e->getMessage());
+ WalletCharge::dispatch($this->wallet->id);
+ }
+ }
+
if ($this->wallet->balance >= 0) {
return null;
}
diff --git a/src/app/Providers/Payment/Stripe.php b/src/app/Providers/Payment/Stripe.php
--- a/src/app/Providers/Payment/Stripe.php
+++ b/src/app/Providers/Payment/Stripe.php
@@ -375,7 +375,7 @@
// Top-up the wallet if balance is below the threshold
if ($payment->wallet->balance < $threshold && $payment->status != Payment::STATUS_PAID) {
- \App\Jobs\WalletCharge::dispatch($payment->wallet);
+ \App\Jobs\WalletCharge::dispatch($payment->wallet->id);
}
}
diff --git a/src/tests/Feature/Console/Wallet/ChargeTest.php b/src/tests/Feature/Console/Wallet/ChargeTest.php
--- a/src/tests/Feature/Console/Wallet/ChargeTest.php
+++ b/src/tests/Feature/Console/Wallet/ChargeTest.php
@@ -50,57 +50,10 @@
$this->artisan('wallet:charge ' . $wallet->id)
->assertExitCode(0);
- Queue::assertNothingPushed();
-
- // The wallet has no entitlements, but has negative balance
- $wallet->balance = -100;
- $wallet->save();
-
- $this->artisan('wallet:charge ' . $wallet->id)
- ->assertExitCode(0);
-
- Queue::assertPushed(\App\Jobs\WalletCharge::class, 0);
- Queue::assertPushed(\App\Jobs\WalletCheck::class, 1);
- Queue::assertPushed(\App\Jobs\WalletCheck::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
- });
-
- Queue::fake();
-
- // The wallet has entitlements to charge, and negative balance
- $sku = \App\Sku::withObjectTenantContext($user)->where('title', 'mailbox')->first();
- $entitlement = \App\Entitlement::create([
- 'wallet_id' => $wallet->id,
- 'sku_id' => $sku->id,
- 'cost' => 100,
- 'entitleable_id' => $user->id,
- 'entitleable_type' => \App\User::class,
- ]);
- \App\Entitlement::where('id', $entitlement->id)->update([
- 'created_at' => \Carbon\Carbon::now()->subMonthsNoOverflow(1),
- 'updated_at' => \Carbon\Carbon::now()->subMonthsNoOverflow(1),
- ]);
- \App\User::where('id', $user->id)->update([
- 'created_at' => \Carbon\Carbon::now()->subMonthsNoOverflow(1),
- 'updated_at' => \Carbon\Carbon::now()->subMonthsNoOverflow(1),
- ]);
-
- $this->assertSame(100, $wallet->fresh()->chargeEntitlements(false));
-
- $this->artisan('wallet:charge ' . $wallet->id)
- ->assertExitCode(0);
-
- Queue::assertPushed(\App\Jobs\WalletCharge::class, 1);
- Queue::assertPushed(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
- });
-
Queue::assertPushed(\App\Jobs\WalletCheck::class, 1);
Queue::assertPushed(\App\Jobs\WalletCheck::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = TestCase::getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet->id;
});
}
@@ -109,39 +62,29 @@
*/
public function testHandleAll(): void
{
- $user = $this->getTestUser('john@kolab.org');
- $wallet = $user->wallets()->first();
- $wallet->balance = 0;
- $wallet->save();
-
- // backdate john's entitlements and set balance=0 for all wallets
- $this->backdateEntitlements($user->entitlements, \Carbon\Carbon::now()->subWeeks(5));
- \App\Wallet::where('balance', '<', '0')->update(['balance' => 0]);
+ $user1 = $this->getTestUser('john@kolab.org');
+ $wallet1 = $user1->wallets()->first();
$user2 = $this->getTestUser('wallet-charge@kolabnow.com');
$wallet2 = $user2->wallets()->first();
- $wallet2->balance = -100;
- $wallet2->save();
+
+ $count = \App\Wallet::join('users', 'users.id', '=', 'wallets.user_id')
+ ->withEnvTenantContext('users')
+ ->whereNull('users.deleted_at')
+ ->count();
Queue::fake();
- // Non-existing wallet ID
$this->artisan('wallet:charge')->assertExitCode(0);
- Queue::assertPushed(\App\Jobs\WalletCheck::class, 2);
- Queue::assertPushed(\App\Jobs\WalletCheck::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ Queue::assertPushed(\App\Jobs\WalletCheck::class, $count);
+ Queue::assertPushed(\App\Jobs\WalletCheck::class, function ($job) use ($wallet1) {
+ $job_wallet_id = TestCase::getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet1->id;
});
Queue::assertPushed(\App\Jobs\WalletCheck::class, function ($job) use ($wallet2) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet2->id;
- });
-
- Queue::assertPushed(\App\Jobs\WalletCharge::class, 1);
- Queue::assertPushed(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = TestCase::getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet2->id;
});
}
}
diff --git a/src/tests/Feature/Controller/PaymentsMollieEuroTest.php b/src/tests/Feature/Controller/PaymentsMollieEuroTest.php
--- a/src/tests/Feature/Controller/PaymentsMollieEuroTest.php
+++ b/src/tests/Feature/Controller/PaymentsMollieEuroTest.php
@@ -256,8 +256,8 @@
Bus::assertDispatchedTimes(\App\Jobs\WalletCharge::class, 1);
Bus::assertDispatched(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = $this->getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = $this->getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet->id;
});
$this->unmockMollie();
diff --git a/src/tests/Feature/Controller/PaymentsMollieTest.php b/src/tests/Feature/Controller/PaymentsMollieTest.php
--- a/src/tests/Feature/Controller/PaymentsMollieTest.php
+++ b/src/tests/Feature/Controller/PaymentsMollieTest.php
@@ -290,8 +290,8 @@
Bus::assertDispatchedTimes(\App\Jobs\WalletCharge::class, 1);
Bus::assertDispatched(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = $this->getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = $this->getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet->id;
});
$this->unmockMollie();
diff --git a/src/tests/Feature/Controller/PaymentsStripeTest.php b/src/tests/Feature/Controller/PaymentsStripeTest.php
--- a/src/tests/Feature/Controller/PaymentsStripeTest.php
+++ b/src/tests/Feature/Controller/PaymentsStripeTest.php
@@ -280,8 +280,8 @@
Bus::assertDispatchedTimes(\App\Jobs\WalletCharge::class, 1);
Bus::assertDispatched(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = $this->getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = $this->getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet->id;
});
$this->unmockStripe();
@@ -513,8 +513,8 @@
// Expect a WalletCharge job if the balance is negative
Bus::assertDispatchedTimes(\App\Jobs\WalletCharge::class, 1);
Bus::assertDispatched(\App\Jobs\WalletCharge::class, function ($job) use ($wallet) {
- $job_wallet = TestCase::getObjectProperty($job, 'wallet');
- return $job_wallet->id === $wallet->id;
+ $job_wallet_id = TestCase::getObjectProperty($job, 'walletId');
+ return $job_wallet_id === $wallet->id;
});
// TODO: test other setup_intent.* events
diff --git a/src/tests/Feature/Jobs/WalletCheckTest.php b/src/tests/Feature/Jobs/WalletCheckTest.php
--- a/src/tests/Feature/Jobs/WalletCheckTest.php
+++ b/src/tests/Feature/Jobs/WalletCheckTest.php
@@ -7,6 +7,7 @@
use App\Wallet;
use Carbon\Carbon;
use Illuminate\Support\Facades\Mail;
+use Illuminate\Support\Facades\Queue;
use Tests\TestCase;
class WalletCheckTest extends TestCase
@@ -45,7 +46,7 @@
$wallet->balance = 0;
$wallet->save();
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -54,7 +55,7 @@
$wallet->balance = -100;
$wallet->save();
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -63,7 +64,7 @@
$wallet->setSetting('balance_negative_since', $now->subHours(2)->toDateTimeString());
$wallet->setSetting('balance_warning_initial', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
// Assert the mail was sent to the user's email, but not to his external email
@@ -74,7 +75,7 @@
// Run the job again to make sure the notification is not sent again
Mail::fake();
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -84,7 +85,7 @@
$wallet->setSetting('balance_negative_since', null);
$wallet->setSetting('balance_warning_initial', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
// Assert the mail was sent to the user's email, but not to his external email
@@ -103,12 +104,37 @@
$wallet->owner->suspend();
$wallet->setSetting('balance_warning_initial', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
}
+ /**
+ * Test job handle, wallet charge and top-up
+ */
+ public function testHandleInitialCharge(): void
+ {
+ Mail::fake();
+ Queue::fake();
+
+ $user = $this->prepareTestUser($wallet);
+ $wallet->balance = 0;
+ $wallet->save();
+ $this->backdateEntitlements($wallet->entitlements, Carbon::now()->subWeeks(5));
+
+ $job = new WalletCheck($wallet->id);
+ $job->handle();
+
+ $wallet->refresh();
+
+ $this->assertTrue($wallet->balance < 0); // @phpstan-ignore-line
+
+ // TODO: How to mock PaymentsProvider::topUpWallet() to make sure it was called?
+ // We should probably move this method to the App\Wallet class to make it possible.
+ $this->markTestIncomplete();
+ }
+
/**
* Test job handle, top-up before reminder notification
*
@@ -124,7 +150,7 @@
// Balance turned negative 7-1 days ago
$wallet->setSetting('balance_negative_since', $now->subDays(7 - 1)->toDateTimeString());
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$res = $job->handle();
Mail::assertNothingSent();
@@ -148,7 +174,7 @@
// Balance turned negative 7+1 days ago, expect mail sent
$wallet->setSetting('balance_negative_since', $now->subDays(7 + 1)->toDateTimeString());
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
// Assert the mail was sent to the user's email and to his external email
@@ -159,7 +185,7 @@
// Run the job again to make sure the notification is not sent again
Mail::fake();
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -169,7 +195,7 @@
$wallet->owner->suspend();
$wallet->setSetting('balance_warning_reminder', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -193,7 +219,7 @@
$days = 7 + 7 + 1;
$wallet->setSetting('balance_negative_since', $now->subDays($days)->toDateTimeString());
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
// Assert the mail was sent to the user's email, and his external email
@@ -210,7 +236,7 @@
$wallet->owner->suspend();
$wallet->owner->undegrade();
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();
@@ -234,7 +260,7 @@
// Test degraded_last_reminder not set
$wallet->setSetting('degraded_last_reminder', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$res = $job->handle();
Mail::assertNothingSent();
@@ -247,7 +273,7 @@
$last = $now->copy()->subDays(10);
$wallet->setSetting('degraded_last_reminder', $last->toDateTimeString());
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$res = $job->handle();
Mail::assertNothingSent();
@@ -259,7 +285,7 @@
// Test degraded_last_reminder set, and 14 days passed
$wallet->setSetting('degraded_last_reminder', $now->copy()->subDays(14)->setSeconds(0));
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$res = $job->handle();
// Assert the mail was sent to the user's email, and his external email
@@ -278,7 +304,7 @@
$wallet->owner->undegrade();
$wallet->setSetting('degraded_last_reminder', null);
- $job = new WalletCheck($wallet);
+ $job = new WalletCheck($wallet->id);
$job->handle();
Mail::assertNothingSent();

File Metadata

Mime Type
text/plain
Expires
Fri, Apr 3, 9:28 AM (2 d, 10 h ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18823463
Default Alt Text
D4794.1775208538.diff (20 KB)

Event Timeline