From 4618673ce7c415840418b7ad433d04a0fc08f8db Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Thu, 4 Apr 2024 11:30:48 +0200 Subject: [PATCH] Fix issues with date handling By using toDateString() in the tests, some edge cases were not caught. Now toDateString() is only called when it makes sense. Also, whereBetween() seems to handle the $to variable exclusively so this must be the start of the next month instead of the end of the current month. --- src/Console/Commands/CountUniqueUser.php | 19 +++++++++---------- src/Console/Commands/CountUser.php | 5 +++-- .../Commands/DetermineStorageUsage.php | 2 +- src/Requests.php | 12 +++++++----- src/Storage.php | 3 ++- src/User.php | 13 +++++++++---- .../Console/Commands/CountUniqueUserTest.php | 10 +++++----- tests/Console/Commands/CountUserTest.php | 10 +++++----- tests/RequestsTest.php | 5 +++-- tests/StorageTest.php | 2 +- tests/UserTest.php | 6 +++--- 11 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/Console/Commands/CountUniqueUser.php b/src/Console/Commands/CountUniqueUser.php index 834e7d3..6953723 100644 --- a/src/Console/Commands/CountUniqueUser.php +++ b/src/Console/Commands/CountUniqueUser.php @@ -30,15 +30,14 @@ class CountUniqueUser extends Command */ public function handle() { - $fromDate = Carbon::now()->subMonth()->startOfMonth()->toDateString(); - $tillDate = Carbon::now()->subMonth()->endOfMonth()->toDateString(); - - $nbrUser = User::whereBetween('login_at', [$fromDate, $tillDate])->count(); - - DB::table('kpis_unique_users')->insert(['date' => $tillDate, 'value' => $nbrUser]); - } - - private function countUniqueUser() - { + $nbrUser = User::whereBetween('login_at', [ + Carbon::now()->subMonth()->startOfMonth(), + Carbon::now()->startOfMonth(), + ])->count(); + + DB::table('kpis_unique_users')->insert([ + 'date' => Carbon::now()->subMonth()->endOfMonth(), + 'value' => $nbrUser, + ]); } } diff --git a/src/Console/Commands/CountUser.php b/src/Console/Commands/CountUser.php index 6efff5b..ebb377d 100644 --- a/src/Console/Commands/CountUser.php +++ b/src/Console/Commands/CountUser.php @@ -30,8 +30,9 @@ class CountUser extends Command */ public function handle() { - $yesterday = Carbon::yesterday()->toDateString(); - $nbrUser = User::where('login_at', '=', $yesterday)->count(); + $yesterday = Carbon::yesterday(); + $today = Carbon::today(); + $nbrUser = User::whereBetween('login_at', [$yesterday, $today])->count(); DB::table('kpis_users')->insert(['date' => $yesterday, 'value' => $nbrUser]); } diff --git a/src/Console/Commands/DetermineStorageUsage.php b/src/Console/Commands/DetermineStorageUsage.php index 3e0f8b0..a2dca78 100644 --- a/src/Console/Commands/DetermineStorageUsage.php +++ b/src/Console/Commands/DetermineStorageUsage.php @@ -38,7 +38,7 @@ public function handle() { $size = $this->getSizeInGB(); - $date = Carbon::now()->subMonth()->endOfMonth()->toDateString(); + $date = Carbon::now()->subMonth()->endOfMonth(); DB::table('kpis_storage_usage')->insert(['date' => $date, 'value' => $size]); } diff --git a/src/Requests.php b/src/Requests.php index b77e01a..64ce099 100644 --- a/src/Requests.php +++ b/src/Requests.php @@ -10,7 +10,7 @@ class Requests public static function save($visits, $actions) { DB::transaction(function () use ($visits, $actions) { - $yesterday = Carbon::yesterday()->toDateString(); + $yesterday = Carbon::yesterday(); DB::table('kpis_actions')->insert(['date' => $yesterday, 'value' => $actions]); DB::table('kpis_visits')->insert(['date' => $yesterday, 'value' => $visits]); }); @@ -18,16 +18,18 @@ public static function save($visits, $actions) public static function getActions($year, $month) { - $start = Carbon::createFromDate($year, $month, 1)->toDateString(); - $end = Carbon::createFromDate($year, $month, 1)->endOfMonth()->toDateString(); + $start = Carbon::createFromDate($year, $month)->startOfMonth(); + $end = $start->copy()->addMonth(); $res = DB::table('kpis_actions')->whereBetween('date', [$start, $end])->sum('value'); + return $res; } public static function getVisits($year, $month) { - $start = Carbon::createFromDate($year, $month, 1)->toDateString(); - $end = Carbon::createFromDate($year, $month, 1)->endOfMonth()->toDateString(); + $start = Carbon::createFromDate($year, $month)->startOfMonth(); + $end = $start->copy()->addMonth(); $res = DB::table('kpis_visits')->whereBetween('date', [$start, $end])->sum('value'); + return $res; } } diff --git a/src/Storage.php b/src/Storage.php index 268eac7..febe939 100644 --- a/src/Storage.php +++ b/src/Storage.php @@ -9,8 +9,9 @@ class Storage { public static function getStorageUsage($year, $month) { - $date = Carbon::createFromDate($year, $month, 1)->endOfMonth()->toDateString(); + $date = Carbon::createFromDate($year, $month, 1)->endOfMonth(); $res = DB::table('kpis_storage_usage')->where('date', '=', $date)->sum('value'); + return $res; } } diff --git a/src/User.php b/src/User.php index 38644a2..cb21336 100644 --- a/src/User.php +++ b/src/User.php @@ -9,14 +9,19 @@ class User { public static function getUser($year, $month) { - $first = Carbon::createFromDate($year, $month, 1); - $last = $first->copy()->endOfMonth(); - return DB::table('kpis_users')->whereBetween('date', [$first->toDateString(), $last->toDateString()])->sum('value'); + $first = Carbon::createFromDate($year, $month)->startOfMonth(); + $last = $first->copy()->addMonth(); + + return DB::table('kpis_users') + ->whereBetween('date', [$first, $last]) + ->sum('value'); } + public static function getUniqueUser($year, $month) { - $date = Carbon::createFromDate($year, $month, 1)->endOfMonth()->toDateString(); + $date = Carbon::createFromDate($year, $month, 1)->endOfMonth(); $res = DB::table('kpis_unique_users')->where('date', '=', $date)->sum('value'); + return $res; } } diff --git a/tests/Console/Commands/CountUniqueUserTest.php b/tests/Console/Commands/CountUniqueUserTest.php index 76c3a60..223c7c4 100644 --- a/tests/Console/Commands/CountUniqueUserTest.php +++ b/tests/Console/Commands/CountUniqueUserTest.php @@ -11,9 +11,9 @@ class CountUniqueUserTest extends TestCase { public function testHandle() { - UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()->toDateString()]); - UserTest::create(['login_at' => Carbon::now()->subMonth()->toDateString()]); - UserTest::create(['login_at' => Carbon::now()->subMonth()->endOfMonth()->toDateString()]); + UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()]); + UserTest::create(['login_at' => Carbon::now()->subMonth()]); + UserTest::create(['login_at' => Carbon::now()->subMonth()->endOfMonth()]); $this->artisan('kpis:count-unique-user')->assertExitCode(0); @@ -28,8 +28,8 @@ public function testHandle() public function testLoginWasNotLastMonth() { - UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()->toDateString()]); - UserTest::create(['login_at' => Carbon::now()->subMonths(2)->endOfMonth()->toDateString()]); + UserTest::create(['login_at' => Carbon::now()->subMonth()->firstOfMonth()]); + UserTest::create(['login_at' => Carbon::now()->subMonths(2)->endOfMonth()]); $this->artisan('kpis:count-unique-user')->assertExitCode(0); diff --git a/tests/Console/Commands/CountUserTest.php b/tests/Console/Commands/CountUserTest.php index c73f4af..8d6fcd5 100644 --- a/tests/Console/Commands/CountUserTest.php +++ b/tests/Console/Commands/CountUserTest.php @@ -11,14 +11,14 @@ class CountUserTest extends TestCase { public function testHandle() { - $yesterday = Carbon::yesterday()->toDateString(); + $yesterday = Carbon::now()->subDay(); UserTest::create(['login_at' => $yesterday]); UserTest::create(['login_at' => $yesterday]); $this->artisan('kpis:count-user')->assertExitCode(0); - $users = DB::table('kpis_users')->where('date', '=', $yesterday)->pluck('value'); + $users = DB::table('kpis_users')->where('date', '=', $yesterday->toDateString())->pluck('value'); $this->assertCount(1, $users); $this->assertEquals(2, $users[0]); @@ -28,14 +28,14 @@ public function testHandle() public function testDifferentLogInDates() { - $yesterday = Carbon::yesterday()->toDateString(); + $yesterday = Carbon::now()->subDay(); UserTest::create(['login_at' => $yesterday]); - UserTest::create(['login_at' => Carbon::now()->subDays(2)->toDateString()]); + UserTest::create(['login_at' => Carbon::now()->subDays(2)]); $this->artisan('kpis:count-user')->assertExitCode(0); - $users = DB::table('kpis_users')->where('date', '=', $yesterday)->pluck('value'); + $users = DB::table('kpis_users')->where('date', '=', $yesterday->toDateString())->pluck('value'); $this->assertCount(1, $users); $this->assertEquals(1, $users[0]); diff --git a/tests/RequestsTest.php b/tests/RequestsTest.php index 5dbba18..8fee12f 100644 --- a/tests/RequestsTest.php +++ b/tests/RequestsTest.php @@ -46,17 +46,18 @@ public function testGetActions() { $date = Carbon::now()->subMonth()->lastOfMonth(); - DB::table('kpis_actions')->insert(['date' => $date->toDateString(), 'value' => 10]); + DB::table('kpis_actions')->insert(['date' => $date, 'value' => 10]); $count = Requests::getActions($date->year, $date->month); $this->assertEquals(10, $count); } + public function testGetVisits() { $date = Carbon::now()->subMonth()->lastOfMonth(); - DB::table('kpis_visits')->insert(['date' => $date->toDateString(), 'value' => 10]); + DB::table('kpis_visits')->insert(['date' => $date, 'value' => 10]); $count = Requests::getVisits($date->year, $date->month); diff --git a/tests/StorageTest.php b/tests/StorageTest.php index 9a4df31..8478145 100644 --- a/tests/StorageTest.php +++ b/tests/StorageTest.php @@ -16,7 +16,7 @@ public function testGetStorage() $noFiles = Storage::getStorageUsage($date->year, $date->month); - DB::table('kpis_storage_usage')->insert(['date' => $date->toDateString(), 'value' => 100]); + DB::table('kpis_storage_usage')->insert(['date' => $date, 'value' => 100]); $size = Storage::getStorageUsage($date->year, $date->month); diff --git a/tests/UserTest.php b/tests/UserTest.php index e457f19..e9e0986 100644 --- a/tests/UserTest.php +++ b/tests/UserTest.php @@ -17,8 +17,8 @@ public function testGetUser(){ $noUserCounted = User::getUser($first->year, $first->month); - DB::table('kpis_users')->insert(['date' => $first->toDateString(), 'value' => 10]); - DB::table('kpis_users')->insert(['date' => $last->toDateString(), 'value' => 10]); + DB::table('kpis_users')->insert(['date' => $first, 'value' => 10]); + DB::table('kpis_users')->insert(['date' => $last, 'value' => 10]); $count = User::getUser($first->year, $first->month); @@ -31,7 +31,7 @@ public function testGetUniqueUser(){ $noUserCounted = User::getUniqueUser($date->year, $date->month); - DB::table('kpis_unique_users')->insert(['date' => $date->toDateString(), 'value' => 10]); + DB::table('kpis_unique_users')->insert(['date' => $date, 'value' => 10]); $count = User::getUniqueUser($date->year, $date->month);