From 5ff462d528c7c11c8dd2641d8a262fc1e8c05b16 Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Tue, 30 Apr 2024 12:43:38 +0200 Subject: [PATCH] Fix month addition/subtraction The calculations can be made more deterministic with immutable objects. Also, the monthOverflow setting is required to give the expected results. --- src/Console/Commands/CountUniqueUser.php | 8 +++++--- src/Console/Commands/DetermineStorageUsage.php | 5 ++++- src/Http/Controllers/Views/KpiController.php | 5 +++-- tests/Console/Commands/CountUniqueUserTest.php | 16 +++++++++------- .../Commands/DetermineStorageUsageTest.php | 16 ++++++++++++++-- tests/RequestsTest.php | 10 ++++++++-- tests/StorageTest.php | 5 ++++- tests/UserTest.php | 14 +++++++++----- 8 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/Console/Commands/CountUniqueUser.php b/src/Console/Commands/CountUniqueUser.php index 6953723..520ba8a 100644 --- a/src/Console/Commands/CountUniqueUser.php +++ b/src/Console/Commands/CountUniqueUser.php @@ -30,13 +30,15 @@ class CountUniqueUser extends Command */ public function handle() { + $now = Carbon::now()->toImmutable()->settings(['monthOverflow' => false]); + $nbrUser = User::whereBetween('login_at', [ - Carbon::now()->subMonth()->startOfMonth(), - Carbon::now()->startOfMonth(), + $now->subMonth()->startOfMonth(), + $now->startOfMonth(), ])->count(); DB::table('kpis_unique_users')->insert([ - 'date' => Carbon::now()->subMonth()->endOfMonth(), + 'date' => $now->subMonth()->endOfMonth(), 'value' => $nbrUser, ]); } diff --git a/src/Console/Commands/DetermineStorageUsage.php b/src/Console/Commands/DetermineStorageUsage.php index a2dca78..8b035d3 100644 --- a/src/Console/Commands/DetermineStorageUsage.php +++ b/src/Console/Commands/DetermineStorageUsage.php @@ -38,7 +38,10 @@ public function handle() { $size = $this->getSizeInGB(); - $date = Carbon::now()->subMonth()->endOfMonth(); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->endOfMonth(); DB::table('kpis_storage_usage')->insert(['date' => $date, 'value' => $size]); } diff --git a/src/Http/Controllers/Views/KpiController.php b/src/Http/Controllers/Views/KpiController.php index 035a80c..9b377bd 100644 --- a/src/Http/Controllers/Views/KpiController.php +++ b/src/Http/Controllers/Views/KpiController.php @@ -22,7 +22,8 @@ public function show($idx = 6) abort(404); } - $date = Carbon::now()->subMonths(7 - $idx); + $now = Carbon::now()->toImmutable()->settings(['monthOverflow' => false]); + $date = $now->subMonths(7 - $idx); $year = $date->year; $month = $date->month; @@ -34,7 +35,7 @@ public function show($idx = 6) $monthOverview = []; for($i = 6;$i >= 1;$i--) { - $monthOverview[] = Carbon::now()->subMonths($i)->format('M'); + $monthOverview[] = $now->subMonths($i)->format('M'); } return view('kpis::show', [ diff --git a/tests/Console/Commands/CountUniqueUserTest.php b/tests/Console/Commands/CountUniqueUserTest.php index 223c7c4..67d4aaa 100644 --- a/tests/Console/Commands/CountUniqueUserTest.php +++ b/tests/Console/Commands/CountUniqueUserTest.php @@ -11,14 +11,15 @@ class CountUniqueUserTest extends TestCase { public function testHandle() { - UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()]); - UserTest::create(['login_at' => Carbon::now()->subMonth()]); - UserTest::create(['login_at' => Carbon::now()->subMonth()->endOfMonth()]); + $now = Carbon::now()->toImmutable()->settings(['monthOverflow' => false]); + UserTest::create(['login_at' => $now->subMonth()->firstOfMonth()]); + UserTest::create(['login_at' => $now->subMonth()]); + UserTest::create(['login_at' => $now->subMonth()->endOfMonth()]); $this->artisan('kpis:count-unique-user')->assertExitCode(0); $uniqueUsers = DB::table('kpis_unique_users') - ->where('date', '=', Carbon::now()->subMonth()->endOfMonth())->pluck('value'); + ->where('date', '=', $now->subMonth()->endOfMonth())->pluck('value'); $this->assertCount(1, $uniqueUsers); $this->assertEquals(3, $uniqueUsers[0]); @@ -28,13 +29,14 @@ public function testHandle() public function testLoginWasNotLastMonth() { - UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()]); - UserTest::create(['login_at' => Carbon::now()->subMonths(2)->endOfMonth()]); + $now = Carbon::now()->toImmutable()->settings(['monthOverflow' => false]); + $u1 = UserTest::create(['login_at' => $now->subMonth()->firstOfMonth()]); + $u2 = UserTest::create(['login_at' => $now->subMonths(2)->endOfMonth()]); $this->artisan('kpis:count-unique-user')->assertExitCode(0); $uniqueUsers = DB::table('kpis_unique_users') - ->where('date', '=', Carbon::now()->subMonth()->endOfMonth())->pluck('value'); + ->where('date', '=', $now->subMonth()->endOfMonth())->pluck('value'); $this->assertCount(1, $uniqueUsers); $this->assertEquals(1, $uniqueUsers[0]); diff --git a/tests/Console/Commands/DetermineStorageUsageTest.php b/tests/Console/Commands/DetermineStorageUsageTest.php index df92faf..53dee16 100644 --- a/tests/Console/Commands/DetermineStorageUsageTest.php +++ b/tests/Console/Commands/DetermineStorageUsageTest.php @@ -27,7 +27,13 @@ public function testHandle() $this->artisan('kpis:determine-storage-usage')->assertExitCode(0); - $users = DB::table('kpis_storage_usage')->where('date', '=', Carbon::now()->subMonth()->endOfMonth())->pluck('value'); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->endOfMonth(); + $users = DB::table('kpis_storage_usage') + ->where('date', '=', $date) + ->pluck('value'); $this->assertCount(1, $users); $this->assertEquals(2, $users[0]); @@ -44,7 +50,13 @@ public function testEmptyAttributeArrays() $this->artisan('kpis:determine-storage-usage')->assertExitCode(0); - $users = DB::table('kpis_storage_usage')->where('date', '=', Carbon::now()->subMonth()->endOfMonth())->pluck('value'); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->endOfMonth(); + $users = DB::table('kpis_storage_usage') + ->where('date', '=', $date) + ->pluck('value'); $this->assertCount(1, $users); $this->assertEquals(0, $users[0]); diff --git a/tests/RequestsTest.php b/tests/RequestsTest.php index 8fee12f..d269441 100644 --- a/tests/RequestsTest.php +++ b/tests/RequestsTest.php @@ -44,7 +44,10 @@ public function testSaveBigInt(){ public function testGetActions() { - $date = Carbon::now()->subMonth()->lastOfMonth(); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->lastOfMonth(); DB::table('kpis_actions')->insert(['date' => $date, 'value' => 10]); @@ -55,7 +58,10 @@ public function testGetActions() public function testGetVisits() { - $date = Carbon::now()->subMonth()->lastOfMonth(); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->lastOfMonth(); DB::table('kpis_visits')->insert(['date' => $date, 'value' => 10]); diff --git a/tests/StorageTest.php b/tests/StorageTest.php index 8478145..01ca142 100644 --- a/tests/StorageTest.php +++ b/tests/StorageTest.php @@ -12,7 +12,10 @@ class StorageTest extends TestCase public function testGetStorage() { - $date = Carbon::now()->subMonth()->lastOfMonth(); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->lastOfMonth(); $noFiles = Storage::getStorageUsage($date->year, $date->month); diff --git a/tests/UserTest.php b/tests/UserTest.php index e9e0986..ccb8b43 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -10,10 +10,11 @@ class UserTest extends TestCase { - public function testGetUser(){ - - $first = Carbon::now()->subMonth()->firstOfMonth(); - $last = Carbon::now()->subMonth()->endOfMonth(); + public function testGetUser() + { + $now = Carbon::now()->toImmutable()->settings(['monthOverflow' => false]); + $first = $now->subMonth()->firstOfMonth(); + $last = $now->subMonth()->endOfMonth(); $noUserCounted = User::getUser($first->year, $first->month); @@ -27,7 +28,10 @@ public function testGetUser(){ } public function testGetUniqueUser(){ - $date = Carbon::now()->subMonth()->endOfMonth(); + $date = Carbon::now() + ->settings(['monthOverflow' => false]) + ->subMonth() + ->endOfMonth(); $noUserCounted = User::getUniqueUser($date->year, $date->month);