adalal/solidtime
1
0
Fork 0
mirror of https://github.com/solidtime-io/solidtime.git synced 2026-10-14 16:53:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

Reject changing the member of running time entries

Browse Source 
Moving a running time entry to another member could give that member's
user a second running time entry. Stopping the time entry and changing
the member in the same request is still allowed.
This commit is contained in:
Constantin Graf
2026-10-13 17:08:25 +02:00
parent f77fe85ea0
commit cc44f5891b
5 changed files with 151 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
app/Exceptions/Api/TimeEntryMemberCanNotBeChangedWhileRunningApiException.php Normal file
View File

@@ -0,0 +1,15 @@
<?php
declare(strict_types=1);
namespace App\Exceptions\Api;
class TimeEntryMemberCanNotBeChangedWhileRunningApiException extends ApiException
{
public const string KEY = 'time_entry_member_can_not_be_changed_while_running';
public function report(): bool
{
return true;
}
}

14
app/Http/Controllers/Api/V1/TimeEntryController.php
View File

@@ -11,6 +11,7 @@ use App\Exceptions\Api\FeatureIsNotAvailableInFreePlanApiException;
use App\Exceptions\Api\OverlappingTimeEntryApiException;
use App\Exceptions\Api\PdfRendererIsNotConfiguredException;
use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException;
use App\Exceptions\Api\TimeEntryMemberCanNotBeChangedWhileRunningApiException;
use App\Exceptions\Api\TimeEntryStillRunningApiException;
use App\Http\Requests\V1\TimeEntry\TimeEntryAggregateExportRequest;
use App\Http\Requests\V1\TimeEntry\TimeEntryAggregateRequest;
@@ -668,7 +669,7 @@ class TimeEntryController extends Controller
*
* To stop a running timer, set `end` to the stop time (UTC). Times the user gives in their own timezone must be converted to UTC first.
*
* @throws AuthorizationException|TimeEntryCanNotBeRestartedApiException
* @throws AuthorizationException|TimeEntryCanNotBeRestartedApiException|TimeEntryMemberCanNotBeChangedWhileRunningApiException
*
* @operationId updateTimeEntry
*/
@@ -694,6 +695,11 @@ class TimeEntryController extends Controller
$effectiveEnd = $request->has('end') ? ($request->input('end') !== null ? Carbon::parse($request->input('end')) : null) : $timeEntry->end;
$this->assertNoOverlap($organization, $effectiveMember, $effectiveStart, $effectiveEnd, $timeEntry);
// The new member could already have a running time entry
if ($effectiveEnd === null && $newMember !== null && $newMember->getKey() !== $timeEntry->member_id) {
throw new TimeEntryMemberCanNotBeChangedWhileRunningApiException;
}
$oldProject = $timeEntry->project;
$oldTask = $timeEntry->task;
@@ -819,6 +825,12 @@ class TimeEntryController extends Controller
continue;
}
// The new member could already have a running time entry
if ($newMember !== null && $timeEntry->end === null && $newMember->getKey() !== $timeEntry->member_id) {
$error->push($id);
continue;
}
$oldProject = $timeEntry->project;
$oldTask = $timeEntry->task;

2
lang/en/exceptions.php
View File

@@ -19,6 +19,7 @@ use App\Exceptions\Api\PdfRendererIsNotConfiguredException;
use App\Exceptions\Api\PersonalAccessClientIsNotConfiguredException;
use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException;
use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException;
use App\Exceptions\Api\TimeEntryMemberCanNotBeChangedWhileRunningApiException;
use App\Exceptions\Api\TimeEntryStillRunningApiException;
use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException;
use App\Exceptions\Api\UserIsAlreadyMemberOfProjectApiException;
@@ -31,6 +32,7 @@ return [
TimeEntryStillRunningApiException::KEY => 'Time entry is still running',
UserNotPlaceholderApiException::KEY => 'The given user is not a placeholder',
TimeEntryCanNotBeRestartedApiException::KEY => 'Time entry is already stopped and can not be restarted',
TimeEntryMemberCanNotBeChangedWhileRunningApiException::KEY => 'The member of a running time entry can not be changed',
InactiveUserCanNotBeUsedApiException::KEY => 'Inactive user can not be used',
UserIsAlreadyMemberOfOrganizationApiException::KEY => 'User is already a member of the organization',
UserIsAlreadyMemberOfProjectApiException::KEY => 'User is already a member of the project',

119
tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php
View File

@@ -3332,6 +3332,80 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_update_endpoint_fails_if_member_of_running_time_entry_is_changed(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update:all',
]);
$otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create();
$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()]), [
'member_id' => $otherMember->getKey(),
]);
// Assert
$response->assertStatus(400);
$response->assertJsonPath('key', 'time_entry_member_can_not_be_changed_while_running');
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $timeEntry->getKey(),
'member_id' => $data->member->getKey(),
'user_id' => $data->user->getKey(),
]);
}
public function test_update_endpoint_can_update_running_time_entry_with_unchanged_member_id(): 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()]), [
'member_id' => $data->member->getKey(),
'description' => 'New description',
]);
// Assert
$response->assertStatus(200);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $timeEntry->getKey(),
'description' => 'New description',
]);
}
public function test_update_endpoint_can_stop_running_time_entry_and_change_member_in_the_same_request(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update:all',
]);
$otherUser = User::factory()->create();
$otherMember = Member::factory()->forOrganization($data->organization)->forUser($otherUser)->role(Role::Employee)->create();
$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()]), [
'member_id' => $otherMember->getKey(),
'end' => Carbon::now()->toIso8601ZuluString(),
]);
// Assert
$response->assertStatus(200);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $timeEntry->getKey(),
'member_id' => $otherMember->getKey(),
'user_id' => $otherUser->getKey(),
]);
}
public function test_update_endpoint_can_update_project_and_automatically_set_client(): void
{
// Arrange
@@ -4199,6 +4273,51 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_update_multiple_does_not_change_member_of_running_time_entries(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update:all',
]);
$otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create();
$runningTimeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create();
$runningTimeEntryOfOtherMember = TimeEntry::factory()->forOrganization($data->organization)->forMember($otherMember)->active()->create();
$finishedTimeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$runningTimeEntry->getKey(),
$runningTimeEntryOfOtherMember->getKey(),
$finishedTimeEntry->getKey(),
],
'changes' => [
'member_id' => $otherMember->getKey(),
],
]);
// Assert
$response->assertStatus(200);
$response->assertExactJson([
'success' => [
$runningTimeEntryOfOtherMember->getKey(),
$finishedTimeEntry->getKey(),
],
'error' => [
$runningTimeEntry->getKey(),
],
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $runningTimeEntry->getKey(),
'member_id' => $data->member->getKey(),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $finishedTimeEntry->getKey(),
'member_id' => $otherMember->getKey(),
]);
}
public function test_update_multiple_updates_user_id_when_member_id_changes(): void
{
// Arrange

2
tests/Unit/Exceptions/Api/ApiExceptionTest.php
View File

@@ -22,6 +22,7 @@ use App\Exceptions\Api\PdfRendererIsNotConfiguredException;
use App\Exceptions\Api\PersonalAccessClientIsNotConfiguredException;
use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException;
use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException;
use App\Exceptions\Api\TimeEntryMemberCanNotBeChangedWhileRunningApiException;
use App\Exceptions\Api\TimeEntryStillRunningApiException;
use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException;
use App\Exceptions\Api\UserIsAlreadyMemberOfProjectApiException;
@@ -53,6 +54,7 @@ class ApiExceptionTest extends TestCase
yield OverlappingTimeEntryApiException::class => [new OverlappingTimeEntryApiException];
yield ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException::class => [new ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException];
yield TimeEntryCanNotBeRestartedApiException::class => [new TimeEntryCanNotBeRestartedApiException];
yield TimeEntryMemberCanNotBeChangedWhileRunningApiException::class => [new TimeEntryMemberCanNotBeChangedWhileRunningApiException];
yield TimeEntryStillRunningApiException::class => [new TimeEntryStillRunningApiException];
yield UserIsAlreadyMemberOfOrganizationApiException::class => [new UserIsAlreadyMemberOfOrganizationApiException];
yield UserIsAlreadyMemberOfProjectApiException::class => [new UserIsAlreadyMemberOfProjectApiException];