mirror of
https://github.com/solidtime-io/solidtime.git
synced 2026-10-09 14:23:19 +01:00
Compare commits
1 Commits
feature/up
...
fix/time-e
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8d34513bde |
@@ -59,6 +59,14 @@ class SelfHostDatabaseConsistency extends Command
|
||||
->get();
|
||||
$this->logProblems($problems, 'Time entries have a client but no project', $hadAProblem);
|
||||
|
||||
// End of time entries can not be before the start
|
||||
$problems = DB::table('time_entries')
|
||||
->select(['id'])
|
||||
->whereNotNull('end')
|
||||
->whereColumn('end', '<', 'start')
|
||||
->get();
|
||||
$this->logProblems($problems, 'Time entries have an end that is before the start', $hadAProblem);
|
||||
|
||||
// Every user needs to be a member of at least one organization
|
||||
$problems = DB::table('users')
|
||||
->select(['users.id as id'])
|
||||
|
||||
@@ -13,9 +13,11 @@ use App\Models\Tag;
|
||||
use App\Models\Task;
|
||||
use App\Models\TimeEntry;
|
||||
use App\Service\PermissionStore;
|
||||
use Carbon\Exceptions\InvalidFormatException;
|
||||
use Closure;
|
||||
use Illuminate\Contracts\Validation\ValidationRule;
|
||||
use Illuminate\Database\Eloquent\Builder;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Illuminate\Validation\ConditionalRules;
|
||||
use Illuminate\Validation\Rule;
|
||||
@@ -90,12 +92,32 @@ class TimeEntryUpdateRequest extends BaseFormRequest
|
||||
// Start of time entry (Format: "Y-m-d\TH:i:s\Z", UTC timezone, Example: "2000-02-22T14:58:59Z")
|
||||
'start' => [
|
||||
'date_format:Y-m-d\TH:i:s\Z',
|
||||
function (string $attribute, mixed $value, Closure $fail) use ($timeEntry): void {
|
||||
// If the payload does not contain an end, the start needs to be validated against the persisted end
|
||||
if ($this->has('end') || $timeEntry?->end === null) {
|
||||
return;
|
||||
}
|
||||
$start = $this->parseDate($value);
|
||||
if ($start !== null && $start->gt($timeEntry->end)) {
|
||||
$fail('The start field must be a date before or equal to end.');
|
||||
}
|
||||
},
|
||||
],
|
||||
// End of time entry (Format: "Y-m-d\TH:i:s\Z", UTC timezone, Example: "2000-02-22T14:58:59Z")
|
||||
'end' => [
|
||||
'nullable',
|
||||
'date_format:Y-m-d\TH:i:s\Z',
|
||||
'after_or_equal:start',
|
||||
function (string $attribute, mixed $value, Closure $fail) use ($timeEntry): void {
|
||||
// If the payload does not contain a start, the end needs to be validated against the persisted start
|
||||
if ($this->has('start') || $timeEntry === null) {
|
||||
return;
|
||||
}
|
||||
$end = $this->parseDate($value);
|
||||
if ($end !== null && $end->lt($timeEntry->start)) {
|
||||
$fail('The end field must be a date after or equal to start.');
|
||||
}
|
||||
},
|
||||
],
|
||||
// Whether time entry is billable
|
||||
'billable' => [
|
||||
@@ -137,4 +159,16 @@ class TimeEntryUpdateRequest extends BaseFormRequest
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
private function parseDate(mixed $value): ?Carbon
|
||||
{
|
||||
if (! is_string($value)) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $value, 'UTC');
|
||||
} catch (InvalidFormatException) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -190,6 +190,9 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
|
||||
if ($end === null) {
|
||||
throw new ImportException('End date ("'.$endDateStr.'") or time ("'.$endTimeStr.'") are invalid');
|
||||
}
|
||||
if ($end->lt($start)) {
|
||||
throw new ImportException('End ("'.$endStr.'") is before start ("'.$startStr.'")');
|
||||
}
|
||||
$timeEntry->end = $end->utc();
|
||||
|
||||
$timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
|
||||
|
||||
@@ -155,6 +155,9 @@ class GenericTimeEntriesImporter extends DefaultImporter
|
||||
if ($end === null) {
|
||||
throw new ImportException('Value of end ("'.$record['end'].'") is invalid');
|
||||
}
|
||||
if ($end->lt($start)) {
|
||||
throw new ImportException('Value of end ("'.$record['end'].'") is before start ("'.$record['start'].'")');
|
||||
}
|
||||
$timeEntry->end = $end->utc();
|
||||
$timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
|
||||
$timeEntry,
|
||||
|
||||
@@ -136,6 +136,9 @@ class HarvestTimeEntriesImporter extends DefaultImporter
|
||||
throw new ImportException('Hours ("'.$record['Hours'].'") is invalid');
|
||||
}
|
||||
$hours = (float) $hoursField;
|
||||
if ($hours < 0) {
|
||||
throw new ImportException('Hours ("'.$record['Hours'].'") is negative');
|
||||
}
|
||||
$timeEntry->start = $date->copy()->startOfDay()->utc();
|
||||
$timeEntry->end = $date->copy()->startOfDay()->addHours($hours)->utc();
|
||||
$timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
|
||||
|
||||
@@ -280,6 +280,9 @@ class SolidtimeImporter extends DefaultImporter
|
||||
if ($end === null) {
|
||||
throw new ImportException('End date ("'.$timeEntryRow['end'].'") is invalid');
|
||||
}
|
||||
if ($end->lt($start)) {
|
||||
throw new ImportException('End date ("'.$timeEntryRow['end'].'") is before start date ("'.$timeEntryRow['start'].'")');
|
||||
}
|
||||
$timeEntry->end = $end->utc();
|
||||
} else {
|
||||
$timeEntry->end = null;
|
||||
|
||||
@@ -139,6 +139,9 @@ class TogglTimeEntriesImporter extends DefaultImporter
|
||||
if ($end === null) {
|
||||
throw new ImportException('End date ("'.$record['End date'].'") or time ("'.$record['End time'].'") are invalid');
|
||||
}
|
||||
if ($end->lt($start)) {
|
||||
throw new ImportException('End ("'.$record['End date'].' '.$record['End time'].'") is before start ("'.$record['Start date'].' '.$record['Start time'].'")');
|
||||
}
|
||||
$timeEntry->end = $end->utc();
|
||||
$timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
|
||||
$timeEntry,
|
||||
|
||||
@@ -13,6 +13,7 @@ use App\Models\Task;
|
||||
use App\Models\TimeEntry;
|
||||
use App\Models\User;
|
||||
use Illuminate\Console\Command;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Facades\Artisan;
|
||||
use PHPUnit\Framework\Attributes\CoversClass;
|
||||
use Tests\TestCaseWithDatabase;
|
||||
@@ -158,4 +159,26 @@ class SelfHostDatabaseConsistencyCommandTest extends TestCaseWithDatabase
|
||||
$output = Artisan::output();
|
||||
$this->assertSame("Consistency problem: Users have a current organization that they are not a member of\n - ".$user1->user->getKey()."\n", $output);
|
||||
}
|
||||
|
||||
public function test_checks_that_end_of_time_entries_is_not_before_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$user = $this->createUserWithRole(Role::Owner);
|
||||
$timeEntry = TimeEntry::factory()->forMember($user->member)->create([
|
||||
'start' => Carbon::parse('2026-08-01T03:00:00Z'),
|
||||
'end' => Carbon::parse('2026-08-01T02:59:55Z'),
|
||||
]);
|
||||
TimeEntry::factory()->forMember($user->member)->create([
|
||||
'start' => Carbon::parse('2026-08-01T04:00:00Z'),
|
||||
'end' => Carbon::parse('2026-08-01T04:00:00Z'),
|
||||
]);
|
||||
|
||||
// Act
|
||||
$exitCode = $this->withoutMockingConsoleOutput()->artisan('self-host:database-consistency');
|
||||
|
||||
// Assert
|
||||
$this->assertSame(Command::FAILURE, $exitCode);
|
||||
$output = Artisan::output();
|
||||
$this->assertSame("Consistency problem: Time entries have an end that is before the start\n - ".$timeEntry->getKey()."\n", $output);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2961,6 +2961,101 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
|
||||
]);
|
||||
}
|
||||
|
||||
public function test_update_endpoint_validation_fails_if_only_end_is_sent_and_it_is_before_the_persisted_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:update:own',
|
||||
]);
|
||||
$timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create([
|
||||
'start' => Carbon::parse('2026-08-01T03:00:00Z'),
|
||||
'end' => Carbon::parse('2026-08-01T03:01:00Z'),
|
||||
]);
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [
|
||||
'end' => '2026-08-01T02:59:55Z',
|
||||
]);
|
||||
|
||||
// Assert
|
||||
$response->assertStatus(422);
|
||||
$response->assertJsonValidationErrors([
|
||||
'end' => 'The end field must be a date after or equal to start.',
|
||||
]);
|
||||
$timeEntry->refresh();
|
||||
$this->assertSame('2026-08-01T03:01:00Z', $timeEntry->end->toIso8601ZuluString());
|
||||
}
|
||||
|
||||
public function test_update_endpoint_validation_fails_if_only_start_is_sent_and_it_is_after_the_persisted_end(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:update:own',
|
||||
]);
|
||||
$timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create([
|
||||
'start' => Carbon::parse('2026-08-01T03:00:00Z'),
|
||||
'end' => Carbon::parse('2026-08-01T03:01:00Z'),
|
||||
]);
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [
|
||||
'start' => '2026-08-01T03:01:05Z',
|
||||
]);
|
||||
|
||||
// Assert
|
||||
$response->assertStatus(422);
|
||||
$response->assertJsonValidationErrors([
|
||||
'start' => 'The start field must be a date before or equal to end.',
|
||||
]);
|
||||
$timeEntry->refresh();
|
||||
$this->assertSame('2026-08-01T03:00:00Z', $timeEntry->start->toIso8601ZuluString());
|
||||
}
|
||||
|
||||
public function test_update_endpoint_allows_updating_only_start_of_running_time_entry(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:update:own',
|
||||
]);
|
||||
$timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create();
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [
|
||||
'start' => '2026-08-01T03:00:00Z',
|
||||
]);
|
||||
|
||||
// Assert
|
||||
$response->assertStatus(200);
|
||||
$timeEntry->refresh();
|
||||
$this->assertSame('2026-08-01T03:00:00Z', $timeEntry->start->toIso8601ZuluString());
|
||||
}
|
||||
|
||||
public function test_update_endpoint_allows_updating_only_end_if_it_is_after_the_persisted_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:update:own',
|
||||
]);
|
||||
$timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create([
|
||||
'start' => Carbon::parse('2026-08-01T03:00:00Z'),
|
||||
'end' => Carbon::parse('2026-08-01T03:01:00Z'),
|
||||
]);
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [
|
||||
'end' => '2026-08-01T03:00:00Z',
|
||||
]);
|
||||
|
||||
// Assert
|
||||
$response->assertStatus(200);
|
||||
$timeEntry->refresh();
|
||||
$this->assertSame('2026-08-01T03:00:00Z', $timeEntry->end->toIso8601ZuluString());
|
||||
}
|
||||
|
||||
public function test_update_endpoint_validation_fails_if_project_id_is_missing_but_request_has_task_id(): void
|
||||
{
|
||||
// Arrange
|
||||
|
||||
@@ -231,4 +231,29 @@ class ClockifyTimeEntriesImporterTest extends ImporterTestAbstract
|
||||
$this->assertSame(0, Tag::query()->count());
|
||||
$this->assertSame(0, Client::query()->count());
|
||||
}
|
||||
|
||||
public function test_import_fails_if_end_is_before_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$organization = Organization::factory()->create();
|
||||
$timezone = 'Europe/Vienna';
|
||||
$importer = new ClockifyTimeEntriesImporter;
|
||||
$importer->init($organization);
|
||||
$data = <<<'CSV'
|
||||
"Project","Client","Description","Task","User","Group","Email","Tags","Billable","Start Date","Start Time","End Date","End Time","Duration (h)","Duration (decimal)","Billable Rate (USD)","Billable Amount (USD)"
|
||||
"Project","Client","Working hard","","Peter Tester","","peter.test@email.test","","Yes","03/04/2024","10:30:00 AM","03/04/2024","10:00:00 AM","00:30:00","0.50","0.00","0.00"
|
||||
CSV;
|
||||
|
||||
// Act
|
||||
try {
|
||||
$importer->importData($data, $timezone);
|
||||
} catch (ImportException $e) {
|
||||
// Assert
|
||||
$this->assertSame('End ("03/04/2024 10:00:00 AM") is before start ("03/04/2024 10:30:00 AM")', $e->getMessage());
|
||||
$this->assertSame(0, TimeEntry::query()->count());
|
||||
|
||||
return;
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ declare(strict_types=1);
|
||||
namespace Tests\Unit\Service\Import\Importers;
|
||||
|
||||
use App\Models\Organization;
|
||||
use App\Models\TimeEntry;
|
||||
use App\Service\Import\Importers\DefaultImporter;
|
||||
use App\Service\Import\Importers\GenericTimeEntriesImporter;
|
||||
use App\Service\Import\Importers\ImportException;
|
||||
@@ -89,4 +90,27 @@ class GenericTimeEntriesImporterTest extends ImporterTestAbstract
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
|
||||
public function test_import_fails_if_end_is_before_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$organization = Organization::factory()->create();
|
||||
$timezone = 'Europe/Vienna';
|
||||
$importer = new GenericTimeEntriesImporter;
|
||||
$importer->init($organization);
|
||||
$data = "description,billable,client,project,tags,start,end,task,user_name,user_email\n".
|
||||
'"Working hard","true","Big Company","Project for Big Company","","2024-03-04T10:23:00Z","2024-03-04T09:23:00Z","","Peter Tester","peter.test@email.test"';
|
||||
|
||||
// Act
|
||||
try {
|
||||
$importer->importData($data, $timezone);
|
||||
} catch (ImportException $e) {
|
||||
// Assert
|
||||
$this->assertSame('Value of end ("2024-03-04T09:23:00Z") is before start ("2024-03-04T10:23:00Z")', $e->getMessage());
|
||||
$this->assertSame(0, TimeEntry::query()->count());
|
||||
|
||||
return;
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -105,4 +105,27 @@ class HarvestTimeEntriesImporterTest extends ImporterTestAbstract
|
||||
$this->assertSame(2, $report->projectsCreated);
|
||||
$this->assertSame(1, $report->clientsCreated);
|
||||
}
|
||||
|
||||
public function test_import_fails_if_hours_are_negative(): void
|
||||
{
|
||||
// Arrange
|
||||
$organization = Organization::factory()->create();
|
||||
$timezone = 'Europe/Vienna';
|
||||
$importer = new HarvestTimeEntriesImporter;
|
||||
$importer->init($organization);
|
||||
$data = "Date,Client,Project,Project Code,Task,Notes,Hours,Billable?,Invoiced?,Approved?,First Name,Last Name,Roles,Employee?,Billable Rate,Billable Amount,Cost Rate,Cost Amount,Currency,External Reference URL\n".
|
||||
'2024-03-04,,Project without Client,,,"","-2,0",No,No,No,Peter,Tester,,Yes,"100,0","2.000,0","0,0","0,0",Euro - EUR,';
|
||||
|
||||
// Act
|
||||
try {
|
||||
$importer->importData($data, $timezone);
|
||||
} catch (ImportException $e) {
|
||||
// Assert
|
||||
$this->assertSame('Hours ("-2,0") is negative', $e->getMessage());
|
||||
$this->assertSame(0, TimeEntry::query()->count());
|
||||
|
||||
return;
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,9 @@ use App\Service\Import\Importers\SolidtimeImporter;
|
||||
use Exception;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Support\Str;
|
||||
use PHPUnit\Framework\Attributes\CoversClass;
|
||||
use ZipArchive;
|
||||
|
||||
#[CoversClass(SolidtimeImporter::class)]
|
||||
#[CoversClass(ImportException::class)]
|
||||
@@ -177,4 +179,32 @@ class SolidtimeImporterTest extends ImporterTestAbstract
|
||||
Queue::assertPushed(RecalculateSpentTimeForProject::class, 1);
|
||||
Queue::assertPushed(RecalculateSpentTimeForTask::class, 1);
|
||||
}
|
||||
|
||||
public function test_import_fails_if_end_of_time_entry_is_before_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$organization = Organization::factory()->create();
|
||||
$timezone = 'Europe/Vienna';
|
||||
$zipPath = $this->createTestZip('solidtime_import_test_1');
|
||||
$zip = new ZipArchive;
|
||||
$zip->open($zipPath);
|
||||
$timeEntries = $zip->getFromName('time_entries.csv');
|
||||
$timeEntries = Str::replaceFirst(',2024-03-04T09:23:52Z,2024-03-04T09:23:52Z,', ',2024-03-04T09:23:52Z,2024-03-04T08:23:52Z,', $timeEntries);
|
||||
$zip->addFromString('time_entries.csv', $timeEntries);
|
||||
$zip->close();
|
||||
$importer = new SolidtimeImporter;
|
||||
$importer->init($organization);
|
||||
$data = file_get_contents($zipPath);
|
||||
|
||||
// Act
|
||||
try {
|
||||
$importer->importData($data, $timezone);
|
||||
} catch (ImportException $e) {
|
||||
// Assert
|
||||
$this->assertSame('End date ("2024-03-04T08:23:52Z") is before start date ("2024-03-04T09:23:52Z")', $e->getMessage());
|
||||
|
||||
return;
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,4 +120,27 @@ class TogglTimeEntriesImporterTest extends ImporterTestAbstract
|
||||
Queue::assertPushed(RecalculateSpentTimeForProject::class, 2);
|
||||
Queue::assertPushed(RecalculateSpentTimeForTask::class, 1);
|
||||
}
|
||||
|
||||
public function test_import_fails_if_end_is_before_start(): void
|
||||
{
|
||||
// Arrange
|
||||
$organization = Organization::factory()->create();
|
||||
$timezone = 'Europe/Vienna';
|
||||
$importer = new TogglTimeEntriesImporter;
|
||||
$importer->init($organization);
|
||||
$data = "User,Email,Client,Project,Task,Description,Billable,Start date,Start time,End date,End time,Duration,Tags,Amount (EUR)\n".
|
||||
'Peter Tester,peter.test@email.test,,Project without Client,,"",No,2024-03-04,10:23:52,2024-03-04,09:23:52,-01:00:00,"",';
|
||||
|
||||
// Act
|
||||
try {
|
||||
$importer->importData($data, $timezone);
|
||||
} catch (ImportException $e) {
|
||||
// Assert
|
||||
$this->assertSame('End ("2024-03-04 09:23:52") is before start ("2024-03-04 10:23:52")', $e->getMessage());
|
||||
$this->assertSame(0, TimeEntry::query()->count());
|
||||
|
||||
return;
|
||||
}
|
||||
$this->fail();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user