From 0e96ad992fab602e27629a86f424f48c65baf950 Mon Sep 17 00:00:00 2001 From: Constantin Graf Date: Thu, 11 Apr 2024 12:37:14 +0200 Subject: [PATCH] Fixed timezones in time entry endpoints --- .../Api/V1/TimeEntryController.php | 22 ++++-- .../Endpoint/Api/V1/TimeEntryEndpointTest.php | 74 +++++++++++++++++++ tests/Unit/Model/TimeEntryModelTest.php | 20 +++++ 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php index 10c5d338..32e1c723 100644 --- a/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -13,6 +13,7 @@ use App\Http\Resources\V1\TimeEntry\TimeEntryCollection; use App\Http\Resources\V1\TimeEntry\TimeEntryResource; use App\Models\Organization; use App\Models\TimeEntry; +use App\Service\TimezoneService; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Resources\Json\JsonResource; @@ -56,8 +57,13 @@ class TimeEntryController extends Controller $timeEntriesQuery->whereDate('start', '>', $request->input('after')); } - if ($request->has('active') && (bool) $request->get('active') === true) { - $timeEntriesQuery->whereNull('end'); + if ($request->has('active')) { + if ($request->get('active') === 'true') { + $timeEntriesQuery->whereNull('end'); + } + if ($request->get('active') === 'false') { + $timeEntriesQuery->whereNotNull('end'); + } } if ($request->has('user_id')) { @@ -72,19 +78,19 @@ class TimeEntryController extends Controller $timeEntries = $timeEntriesQuery->get(); if ($timeEntries->count() === $limit && $request->has('only_full_dates') && (bool) $request->get('only_full_dates') === true) { - // TODO: handle user timezone! + $user = Auth::user(); + $timezone = app(TimezoneService::class)->getTimezoneFromUser($user); $lastDate = null; /** @var TimeEntry $timeEntry */ foreach ($timeEntries as $timeEntry) { - if ($lastDate === null || abs($lastDate->diffInDays($timeEntry->start->startOfDay())) > 0) { - $lastDate = $timeEntry->start->startOfDay(); + if ($lastDate === null || abs($lastDate->diffInDays($timeEntry->start->toImmutable()->timezone($timezone)->startOfDay())) > 0) { + $lastDate = $timeEntry->start->toImmutable()->timezone($timezone)->startOfDay(); } } - $timeEntries = $timeEntries->filter(function (TimeEntry $timeEntry) use ($lastDate): bool { - return $timeEntry->end === null || $timeEntry->start->toDateString() !== $lastDate->toDateString(); + $timeEntries = $timeEntries->filter(function (TimeEntry $timeEntry) use ($lastDate, $timezone): bool { + return $timeEntry->start->toImmutable()->timezone($timezone)->toDateString() !== $lastDate->toDateString(); }); - // TODO: fix edge case with current time entry that is more than one day running if ($timeEntries->count() === 0) { Log::warning('User has has more than '.$limit.' time entries on one date', [ diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index 43e4cf20..59717357 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -153,6 +153,29 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract $response->assertJsonPath('data.0.id', $activeTimeEntry->getKey()); } + public function test_index_endpoint_returns_only_non_active_time_entries(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:view:own', + ]); + $activeTimeEntry = TimeEntry::factory()->forOrganization($data->organization)->forUser($data->user)->active()->createMany(3); + $nonActiveTimeEntries = TimeEntry::factory()->forOrganization($data->organization)->forUser($data->user)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->getJson(route('api.v1.time-entries.index', [ + $data->organization->getKey(), + 'active' => 'false', + 'user_id' => $data->user->getKey(), + ])); + + // Assert + $response->assertStatus(200); + $response->assertJsonCount(1, 'data'); + $response->assertJsonPath('data.0.id', $nonActiveTimeEntries->getKey()); + } + public function test_index_endpoint_filter_only_full_dates_returns_time_entries_for_the_whole_day_case_less_time_entries_than_limit(): void { // Arrange @@ -203,6 +226,57 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract $response->assertJsonCount(3, 'data'); } + public function test_index_endpoint_filter_only_full_dates_returns_time_entries_for_the_whole_day_case_more_time_entries_than_limit_with_a_timezone_edge_case(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:view:own', + ]); + $data->user->timezone = 'America/New_York'; + $data->user->save(); + /** + * We create in the eyes of the users timezone 2 time entries yesterday, 5 time entries two days ago, and 3 time entries three days ago + * The time entries are created in a way that they jump to the next day if the endpoint ignores the users timezone and just uses UTC + */ + + // Note: This entry is yesterday in user timezone and yesterday in UTC + $timeEntriesDay1InUserTimeZone = TimeEntry::factory()->forOrganization($data->organization)->forUser($data->user) + ->state([ + 'start' => Carbon::now($data->user->timezone)->subDay()->startOfDay()->utc(), + ]) + ->createMany(2); + //dump($timeEntriesDay1InUserTimeZone->first()->refresh()->start->toImmutable()->timezone('UTC')->toDateString()); + //dump($timeEntriesDay1InUserTimeZone->first()->refresh()->start->toImmutable()->timezone($data->user->timezone)->toDateString()); + // Note: This entry is yesterday in UTC timezone, but two days ago in user timezone + $timeEntriesDay1InUTC = TimeEntry::factory()->forOrganization($data->organization)->forUser($data->user) + ->state([ + 'start' => Carbon::now('UTC')->subDay()->startOfDay()->utc(), + ]) + ->createMany(2); + //dump($timeEntriesDay1InUTC->first()->refresh()->start->toImmutable()->timezone('UTC')->toDateString()); + //dump($timeEntriesDay1InUTC->first()->refresh()->start->toImmutable()->timezone($data->user->timezone)->toDateString()); + // Note: This entry is two days ago in user timezone + $timeEntriesDay2InUserTimeZone = TimeEntry::factory()->forOrganization($data->organization)->forUser($data->user) + ->state([ + 'start' => Carbon::now($data->user->timezone)->subDays(2)->startOfDay()->utc(), + ]) + ->createMany(3); + + Passport::actingAs($data->user); + + // Act + $response = $this->getJson(route('api.v1.time-entries.index', [ + $data->organization->getKey(), + 'only_full_dates' => 'true', + 'limit' => 5, + 'user_id' => $data->user->getKey(), + ])); + + // Assert + $response->assertStatus(200); + $response->assertJsonCount(2, 'data'); + } + public function test_index_endpoint_filter_only_full_dates_returns_time_entries_for_the_whole_day_case_more_time_entries_in_latest_day_than_limit(): void { // Arrange diff --git a/tests/Unit/Model/TimeEntryModelTest.php b/tests/Unit/Model/TimeEntryModelTest.php index 190bf9b6..14f7af7c 100644 --- a/tests/Unit/Model/TimeEntryModelTest.php +++ b/tests/Unit/Model/TimeEntryModelTest.php @@ -9,6 +9,7 @@ use App\Models\Project; use App\Models\Task; use App\Models\TimeEntry; use App\Models\User; +use Carbon\Carbon; class TimeEntryModelTest extends ModelTestAbstract { @@ -97,4 +98,23 @@ class TimeEntryModelTest extends ModelTestAbstract // Assert $this->assertNull($taskRel); } + + public function test_eloquent_datetime_columns_remove_timezone_information_during_save(): void + { + // Arrange + $timeEntry = TimeEntry::factory()->forTask(null)->create(); + + // Act + $timeEntry->start = Carbon::create(2021, 1, 1, 12, 0, 0, 'UTC')->timezone('+1'); + $timeEntry->save(); + + // Assert + $timeEntry->refresh(); + $this->assertSame('UTC', $timeEntry->start->getTimezone()->toRegionName()); + $this->assertSame('2021-01-01 13:00:00', $timeEntry->start->toDateTimeString()); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->id, + 'start' => '2021-01-01 13:00:00', + ]); + } }