Skip to content

Commit

Permalink
Fixed bugs causing incorrect computed attributes in imported data
Browse files Browse the repository at this point in the history
  • Loading branch information
@korridor
korridor committed Feb 6, 2025
1 parent f14bd64 commit 84c9cfe
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 5 deletions.
3 changes: 2 additions & 1 deletion app/Jobs/RecalculateSpentTimeForProject.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
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;
Expand Down
3 changes: 2 additions & 1 deletion app/Jobs/RecalculateSpentTimeForTask.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
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;
Expand Down
11 changes: 11 additions & 0 deletions app/Service/Import/ImportDatabaseHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,17 @@ public function getModelById(string $id): ?Model
return $model;
}

/**
* @return array<TModel>
*/
public function getCachedModels(): array
{
if ($this->mapKeyToModel === null) {
return [];
}
return array_values($this->mapKeyToModel);
}

/**
* @param array<string, mixed> $identifierData
* @return TModel|null
Expand Down
9 changes: 9 additions & 0 deletions app/Service/Import/Importers/ClockifyTimeEntriesImporter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
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;
Expand Down Expand Up @@ -99,6 +101,7 @@ public function importData(string $data, string $timezone): void
'project_id' => $projectId,
'organization_id' => $this->organization->id,
]);
$this->taskImportHelper->getModelById($taskId);
}
$timeEntry = new TimeEntry;
$timeEntry->disableAuditing();
Expand Down Expand Up @@ -158,6 +161,12 @@ public function importData(string $data, string $timezone): void
$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) {
Expand Down
9 changes: 9 additions & 0 deletions app/Service/Import/Importers/SolidtimeImporter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
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;
Expand Down Expand Up @@ -235,6 +237,7 @@ public function importData(string $data, string $timezone): void
$taskId = null;
if ($timeEntryRow['task_id'] !== '') {
$taskId = $this->taskImportHelper->getKeyByExternalIdentifier($timeEntryRow['task_id']);
$this->taskImportHelper->getModelById($taskId);
}
$timeEntry = new TimeEntry;
$timeEntry->disableAuditing();
Expand Down Expand Up @@ -303,6 +306,12 @@ public function importData(string $data, string $timezone): void
$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) {
Expand Down
9 changes: 9 additions & 0 deletions app/Service/Import/Importers/TogglTimeEntriesImporter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
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;
Expand Down Expand Up @@ -99,6 +101,7 @@ public function importData(string $data, string $timezone): void
'project_id' => $projectId,
'organization_id' => $this->organization->id,
]);
$this->taskImportHelper->getModelById($taskId);
}
$timeEntry = new TimeEntry;
$timeEntry->disableAuditing();
Expand Down Expand Up @@ -144,6 +147,12 @@ public function importData(string $data, string $timezone): void
$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) {
Expand Down
2 changes: 1 addition & 1 deletion resources/testfiles/solidtime_import_test_1/time_entries.csv
View file
Original file line number Diff line number Diff line change
@@ -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
24 changes: 24 additions & 0 deletions tests/Unit/Service/Import/Importers/SolidtimeImporterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@

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;

Expand Down Expand Up @@ -47,12 +51,20 @@ public function test_import_of_test_file_succeeds(): void
$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);
Expand All @@ -61,6 +73,8 @@ public function test_import_of_test_file_succeeds(): void
$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
Expand All @@ -75,12 +89,20 @@ public function test_import_of_test_file_twice_succeeds(): void
$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);
Expand All @@ -89,5 +111,7 @@ public function test_import_of_test_file_twice_succeeds(): void
$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);
}
}
23 changes: 21 additions & 2 deletions tests/Unit/Service/Import/Importers/TogglTimeEntriesImporterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@

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;
Expand All @@ -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;
Expand All @@ -37,7 +44,7 @@ public function test_import_of_test_file_succeeds(): void
$queryLog = DB::getQueryLog();

// Assert
$this->assertCount(21, $queryLog);
$this->assertCount(22, $queryLog);
$testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries();
$this->checkTimeEntries($testScenario);
$this->assertSame(2, $report->timeEntriesCreated);
Expand All @@ -46,11 +53,17 @@ public function test_import_of_test_file_succeeds(): void
$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;
Expand Down Expand Up @@ -84,6 +97,10 @@ public function test_import_of_test_file_twice_succeeds(): void
$importer->importData($data, $timezone);
$importer = new TogglTimeEntriesImporter;
$importer->init($organization);
Queue::fake([
RecalculateSpentTimeForProject::class,
RecalculateSpentTimeForTask::class,
]);

// Act
DB::enableQueryLog();
Expand All @@ -93,7 +110,7 @@ public function test_import_of_test_file_twice_succeeds(): void
$queryLog = DB::getQueryLog();

// Assert
$this->assertCount(13, $queryLog);
$this->assertCount(14, $queryLog);
$testScenario = $this->checkTestScenarioAfterImportExcludingTimeEntries();
$this->checkTimeEntries($testScenario, true);
$this->assertSame(2, $report->timeEntriesCreated);
Expand All @@ -102,5 +119,7 @@ public function test_import_of_test_file_twice_succeeds(): void
$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);
}
}

0 comments on commit 84c9cfe

Please sign in to comment.