From 4eb716d2cc651090c74732dba613b3311248c858 Mon Sep 17 00:00:00 2001 From: Constantin Graf Date: Tue, 4 Feb 2025 19:51:54 -0500 Subject: [PATCH] Fixed bugs causing incorrect computed attributes in imported data --- app/Jobs/RecalculateSpentTimeForProject.php | 3 ++- app/Jobs/RecalculateSpentTimeForTask.php | 3 ++- app/Service/Import/ImportDatabaseHelper.php | 11 +++++++++ .../Importers/ClockifyTimeEntriesImporter.php | 9 +++++++ .../Import/Importers/SolidtimeImporter.php | 9 +++++++ .../Importers/TogglTimeEntriesImporter.php | 9 +++++++ .../solidtime_import_test_1/time_entries.csv | 2 +- .../Importers/SolidtimeImporterTest.php | 24 +++++++++++++++++++ .../TogglTimeEntriesImporterTest.php | 23 ++++++++++++++++-- 9 files changed, 88 insertions(+), 5 deletions(-) diff --git a/app/Jobs/RecalculateSpentTimeForProject.php b/app/Jobs/RecalculateSpentTimeForProject.php index 528ddcc3..024e19d6 100644 --- a/app/Jobs/RecalculateSpentTimeForProject.php +++ b/app/Jobs/RecalculateSpentTimeForProject.php @@ -7,12 +7,13 @@ namespace App\Jobs; use App\Models\Project; use Exception; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; -class RecalculateSpentTimeForProject implements ShouldQueue +class RecalculateSpentTimeForProject implements ShouldDispatchAfterCommit, ShouldQueue { use Dispatchable; use InteractsWithQueue; diff --git a/app/Jobs/RecalculateSpentTimeForTask.php b/app/Jobs/RecalculateSpentTimeForTask.php index fb5e38de..5f04dc6e 100644 --- a/app/Jobs/RecalculateSpentTimeForTask.php +++ b/app/Jobs/RecalculateSpentTimeForTask.php @@ -7,12 +7,13 @@ namespace App\Jobs; use App\Models\Task; use Exception; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Events\ShouldDispatchAfterCommit; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; -class RecalculateSpentTimeForTask implements ShouldQueue +class RecalculateSpentTimeForTask implements ShouldDispatchAfterCommit, ShouldQueue { use Dispatchable; use InteractsWithQueue; diff --git a/app/Service/Import/ImportDatabaseHelper.php b/app/Service/Import/ImportDatabaseHelper.php index e27898b0..1ca251ce 100644 --- a/app/Service/Import/ImportDatabaseHelper.php +++ b/app/Service/Import/ImportDatabaseHelper.php @@ -188,6 +188,17 @@ class ImportDatabaseHelper return $model; } + /** + * @return array + */ + public function getCachedModels(): array + { + if ($this->mapKeyToModel === null) { + return []; + } + return array_values($this->mapKeyToModel); + } + /** * @param array $identifierData * @return TModel|null diff --git a/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php b/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php index 6d040d7a..aa3caa0a 100644 --- a/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php +++ b/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php @@ -5,6 +5,8 @@ declare(strict_types=1); namespace App\Service\Import\Importers; use App\Enums\Role; +use App\Jobs\RecalculateSpentTimeForProject; +use App\Jobs\RecalculateSpentTimeForTask; use App\Models\TimeEntry; use Carbon\Exceptions\InvalidFormatException; use Exception; @@ -99,6 +101,7 @@ class ClockifyTimeEntriesImporter extends DefaultImporter 'project_id' => $projectId, 'organization_id' => $this->organization->id, ]); + $this->taskImportHelper->getModelById($taskId); } $timeEntry = new TimeEntry; $timeEntry->disableAuditing(); @@ -158,6 +161,12 @@ class ClockifyTimeEntriesImporter extends DefaultImporter $timeEntry->save(); $this->timeEntriesCreated++; } + foreach ($this->projectImportHelper->getCachedModels() as $usedProject) { + RecalculateSpentTimeForProject::dispatch($usedProject); + } + foreach ($this->taskImportHelper->getCachedModels() as $usedTask) { + RecalculateSpentTimeForTask::dispatch($usedTask); + } } catch (ImportException $exception) { throw $exception; } catch (CsvException $exception) { diff --git a/app/Service/Import/Importers/SolidtimeImporter.php b/app/Service/Import/Importers/SolidtimeImporter.php index 7aa61176..41e8ae44 100644 --- a/app/Service/Import/Importers/SolidtimeImporter.php +++ b/app/Service/Import/Importers/SolidtimeImporter.php @@ -5,6 +5,8 @@ declare(strict_types=1); namespace App\Service\Import\Importers; use App\Enums\Role; +use App\Jobs\RecalculateSpentTimeForProject; +use App\Jobs\RecalculateSpentTimeForTask; use App\Models\TimeEntry; use Carbon\Exceptions\InvalidFormatException; use Exception; @@ -235,6 +237,7 @@ class SolidtimeImporter extends DefaultImporter $taskId = null; if ($timeEntryRow['task_id'] !== '') { $taskId = $this->taskImportHelper->getKeyByExternalIdentifier($timeEntryRow['task_id']); + $this->taskImportHelper->getModelById($taskId); } $timeEntry = new TimeEntry; $timeEntry->disableAuditing(); @@ -303,6 +306,12 @@ class SolidtimeImporter extends DefaultImporter $timeEntry->save(); $this->timeEntriesCreated++; } + foreach ($this->projectImportHelper->getCachedModels() as $usedProject) { + RecalculateSpentTimeForProject::dispatch($usedProject); + } + foreach ($this->taskImportHelper->getCachedModels() as $usedTask) { + RecalculateSpentTimeForTask::dispatch($usedTask); + } } catch (ImportException $exception) { throw $exception; } catch (Exception $exception) { diff --git a/app/Service/Import/Importers/TogglTimeEntriesImporter.php b/app/Service/Import/Importers/TogglTimeEntriesImporter.php index e6ec1685..9547ff8f 100644 --- a/app/Service/Import/Importers/TogglTimeEntriesImporter.php +++ b/app/Service/Import/Importers/TogglTimeEntriesImporter.php @@ -5,6 +5,8 @@ declare(strict_types=1); namespace App\Service\Import\Importers; use App\Enums\Role; +use App\Jobs\RecalculateSpentTimeForProject; +use App\Jobs\RecalculateSpentTimeForTask; use App\Models\TimeEntry; use Carbon\Exceptions\InvalidFormatException; use Exception; @@ -99,6 +101,7 @@ class TogglTimeEntriesImporter extends DefaultImporter 'project_id' => $projectId, 'organization_id' => $this->organization->id, ]); + $this->taskImportHelper->getModelById($taskId); } $timeEntry = new TimeEntry; $timeEntry->disableAuditing(); @@ -144,6 +147,12 @@ class TogglTimeEntriesImporter extends DefaultImporter $timeEntry->save(); $this->timeEntriesCreated++; } + foreach ($this->projectImportHelper->getCachedModels() as $usedProject) { + RecalculateSpentTimeForProject::dispatch($usedProject); + } + foreach ($this->taskImportHelper->getCachedModels() as $usedTask) { + RecalculateSpentTimeForTask::dispatch($usedTask); + } } catch (ImportException $exception) { throw $exception; } catch (CsvException $exception) { diff --git a/resources/testfiles/solidtime_import_test_1/time_entries.csv b/resources/testfiles/solidtime_import_test_1/time_entries.csv index daf0a565..c7ea2396 100644 --- a/resources/testfiles/solidtime_import_test_1/time_entries.csv +++ b/resources/testfiles/solidtime_import_test_1/time_entries.csv @@ -1,3 +1,3 @@ id,description,start,end,billable_rate,billable,member_id,user_id,organization_id,client_id,project_id,task_id,tags,is_imported,still_active_email_sent_at,created_at,updated_at 00aae3be-18fc-462d-bee4-350fb605b2f3,,2024-03-04T09:23:52Z,2024-03-04T09:23:52Z,,false,06e6e605-86bd-417b-b75d-02f671e5d520,0446cdd8-3ad1-43d6-9231-9e0dc4eeb71c,ee5a8cd6-312f-4ae6-b044-e2014f09ecc2,,,,"[""2c5c2da7-9ef8-4410-bb8f-6e0a90f9d2c7"",""bf6c0ac5-2587-474b-8983-40bb3ea8002f""]",false,,2024-08-22T10:36:48Z,2024-08-22T10:36:48Z -1c7a905d-aa12-4d08-bc41-7e92577e7cdf,"Working hard",2024-03-04T09:23:00Z,2024-03-04T10:23:01Z,,true,06e6e605-86bd-417b-b75d-02f671e5d520,0446cdd8-3ad1-43d6-9231-9e0dc4eeb71c,ee5a8cd6-312f-4ae6-b044-e2014f09ecc2,,,,[],false,,2024-08-22T10:36:48Z,2024-08-22T10:36:48Z +1c7a905d-aa12-4d08-bc41-7e92577e7cdf,"Working hard",2024-03-04T09:23:00Z,2024-03-04T10:23:01Z,,true,06e6e605-86bd-417b-b75d-02f671e5d520,0446cdd8-3ad1-43d6-9231-9e0dc4eeb71c,ee5a8cd6-312f-4ae6-b044-e2014f09ecc2,b4187a44-41f4-46d7-8460-f15a25b3aad6,06e79ec4-33f8-4730-804c-d03c014991d1,b49688a0-94f3-4cb3-9ca1-5003de955fb0,[],false,,2024-08-22T10:36:48Z,2024-08-22T10:36:48Z diff --git a/tests/Unit/Service/Import/Importers/SolidtimeImporterTest.php b/tests/Unit/Service/Import/Importers/SolidtimeImporterTest.php index ee0c3066..78c50363 100644 --- a/tests/Unit/Service/Import/Importers/SolidtimeImporterTest.php +++ b/tests/Unit/Service/Import/Importers/SolidtimeImporterTest.php @@ -4,11 +4,15 @@ declare(strict_types=1); namespace Tests\Unit\Service\Import\Importers; +use App\Jobs\RecalculateSpentTimeForProject; +use App\Jobs\RecalculateSpentTimeForTask; use App\Models\Organization; use App\Service\Import\Importers\DefaultImporter; use App\Service\Import\Importers\ImportException; use App\Service\Import\Importers\SolidtimeImporter; use Exception; +use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Queue; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -47,12 +51,20 @@ class SolidtimeImporterTest extends ImporterTestAbstract $importer = new SolidtimeImporter; $importer->init($organization); $data = file_get_contents($zipPath); + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); // Act + DB::enableQueryLog(); + DB::flushQueryLog(); $importer->importData($data, $timezone); $report = $importer->getReport(); + $queryLog = DB::getQueryLog(); // Assert + $this->assertCount(25, $queryLog); $testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries(true); $this->checkTimeEntries($testScenario); $this->assertSame(2, $report->timeEntriesCreated); @@ -61,6 +73,8 @@ class SolidtimeImporterTest extends ImporterTestAbstract $this->assertSame(1, $report->usersCreated); $this->assertSame(3, $report->projectsCreated); $this->assertSame(2, $report->clientsCreated); + Queue::assertPushed(RecalculateSpentTimeForProject::class, 1); + Queue::assertPushed(RecalculateSpentTimeForTask::class, 1); } public function test_import_of_test_file_twice_succeeds(): void @@ -75,12 +89,20 @@ class SolidtimeImporterTest extends ImporterTestAbstract $importer->importData($data, $timezone); $importer = new SolidtimeImporter; $importer->init($organization); + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); // Act + DB::enableQueryLog(); + DB::flushQueryLog(); $importer->importData($data, $timezone); $report = $importer->getReport(); + $queryLog = DB::getQueryLog(); // Assert + $this->assertCount(13, $queryLog); $testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries(true); $this->checkTimeEntries($testScenario, true); $this->assertSame(2, $report->timeEntriesCreated); @@ -89,5 +111,7 @@ class SolidtimeImporterTest extends ImporterTestAbstract $this->assertSame(0, $report->usersCreated); $this->assertSame(0, $report->projectsCreated); $this->assertSame(0, $report->clientsCreated); + Queue::assertPushed(RecalculateSpentTimeForProject::class, 1); + Queue::assertPushed(RecalculateSpentTimeForTask::class, 1); } } diff --git a/tests/Unit/Service/Import/Importers/TogglTimeEntriesImporterTest.php b/tests/Unit/Service/Import/Importers/TogglTimeEntriesImporterTest.php index c3a62be9..34eb7020 100644 --- a/tests/Unit/Service/Import/Importers/TogglTimeEntriesImporterTest.php +++ b/tests/Unit/Service/Import/Importers/TogglTimeEntriesImporterTest.php @@ -4,12 +4,15 @@ declare(strict_types=1); namespace Tests\Unit\Service\Import\Importers; +use App\Jobs\RecalculateSpentTimeForProject; +use App\Jobs\RecalculateSpentTimeForTask; use App\Models\Organization; use App\Models\TimeEntry; use App\Service\Import\Importers\DefaultImporter; use App\Service\Import\Importers\ImportException; use App\Service\Import\Importers\TogglTimeEntriesImporter; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -23,6 +26,10 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract public function test_import_of_test_file_succeeds(): void { // Arrange + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); $organization = Organization::factory()->create(); $timezone = 'Europe/Vienna'; $importer = new TogglTimeEntriesImporter; @@ -37,7 +44,7 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract $queryLog = DB::getQueryLog(); // Assert - $this->assertCount(21, $queryLog); + $this->assertCount(22, $queryLog); $testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries(); $this->checkTimeEntries($testScenario); $this->assertSame(2, $report->timeEntriesCreated); @@ -46,11 +53,17 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract $this->assertSame(1, $report->usersCreated); $this->assertSame(2, $report->projectsCreated); $this->assertSame(1, $report->clientsCreated); + Queue::assertPushed(RecalculateSpentTimeForProject::class, 2); + Queue::assertPushed(RecalculateSpentTimeForTask::class, 1); } public function test_import_of_test_with_special_characters_description_succeeds(): void { // Arrange + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); $organization = Organization::factory()->create(); $timezone = 'Europe/Vienna'; $importer = new TogglTimeEntriesImporter; @@ -84,6 +97,10 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract $importer->importData($data, $timezone); $importer = new TogglTimeEntriesImporter; $importer->init($organization); + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); // Act DB::enableQueryLog(); @@ -93,7 +110,7 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract $queryLog = DB::getQueryLog(); // Assert - $this->assertCount(13, $queryLog); + $this->assertCount(14, $queryLog); $testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries(); $this->checkTimeEntries($testScenario, true); $this->assertSame(2, $report->timeEntriesCreated); @@ -102,5 +119,7 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract $this->assertSame(0, $report->usersCreated); $this->assertSame(0, $report->projectsCreated); $this->assertSame(0, $report->clientsCreated); + Queue::assertPushed(RecalculateSpentTimeForProject::class, 2); + Queue::assertPushed(RecalculateSpentTimeForTask::class, 1); } }