Solidtime/solidtime
1
0
Fork 0
mirror of https://github.com/solidtime-io/solidtime.git synced 2026-05-07 20:32:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixed problem with merge into when project members already exist in destination member

Browse Source
This commit is contained in:
Constantin Graf
2025-03-10 13:46:57 +01:00
committed by Constantin Graf
parent 02a716897d
commit 73ce5f793d
7 changed files with 190 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Http/Controllers/Api/V1/MemberController.php
+3 -4
View File
@@ -26,7 +26,6 @@ use App\Models\Organization;
use App\Service\BillableRateService;
use App\Service\InvitationService;
use App\Service\MemberService;
use App\Service\UserService;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Resources\Json\JsonResource;
@@ -141,7 +140,7 @@ class MemberController extends Controller
*
* @operationId mergeMember
*/
public function mergeInto(Organization $organization, Member $member, MemberMergeIntoRequest $request): JsonResponse
public function mergeInto(Organization $organization, Member $member, MemberMergeIntoRequest $request, MemberService $memberService): JsonResponse
{
$this->checkPermission($organization, 'members:merge-into', $member);
@@ -151,8 +150,8 @@ class MemberController extends Controller
}
$memberTo = Member::findOrFail($request->getMemberId());
DB::transaction(function () use ($organization, $member, $user, $memberTo): void {
app(UserService::class)->assignOrganizationEntitiesToDifferentMember($organization, $user, $memberTo->user, $memberTo);
DB::transaction(function () use ($organization, $member, $user, $memberTo, $memberService): void {
$memberService->assignOrganizationEntitiesToDifferentMember($organization, $member, $memberTo);
$member->delete();
$user->delete();
});
app/Listeners/RemovePlaceholder.php
+7 -4
View File
@@ -6,7 +6,7 @@ namespace App\Listeners;
use App\Models\Member;
use App\Models\User;
use App\Service\UserService;
use App\Service\MemberService;
use Illuminate\Database\Eloquent\Builder;
use Laravel\Jetstream\Events\TeamMemberAdded;
@@ -17,8 +17,11 @@ class RemovePlaceholder
*/
public function handle(TeamMemberAdded $event): void
{
/** @var UserService $userService */
$userService = app(UserService::class);
$memberService = app(MemberService::class);
$member = Member::query()
->whereBelongsTo($event->team, 'organization')
->whereBelongsTo($event->user, 'user')
->firstOrFail();
$placeholders = Member::query()
->whereHas('user', function (Builder $query) use ($event): void {
/** @var Builder<User> $query */
@@ -32,7 +35,7 @@ class RemovePlaceholder
foreach ($placeholders as $placeholder) {
/** @var Member $placeholder */
$placeholderUser = $placeholder->user;
$userService->assignOrganizationEntitiesToDifferentUser($event->team, $placeholderUser, $event->user);
$memberService->assignOrganizationEntitiesToDifferentMember($event->team, $placeholder, $member);
$placeholder->delete();
$placeholderUser->delete();
}
app/Service/MemberService.php
+36 -1
View File
@@ -14,9 +14,11 @@ use App\Exceptions\Api\OnlyOwnerCanChangeOwnership;
use App\Exceptions\Api\OrganizationNeedsAtLeastOneOwner;
use App\Models\Member;
use App\Models\Organization;
use App\Models\Project;
use App\Models\ProjectMember;
use App\Models\TimeEntry;
use App\Models\User;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Facades\DB;
use InvalidArgumentException;
use Laravel\Jetstream\Events\AddingTeamMember;
@@ -101,6 +103,39 @@ class MemberService
}
}
public function assignOrganizationEntitiesToDifferentMember(Organization $organization, Member $fromMember, Member $toMember): void
{
// Time entries
TimeEntry::query()
->whereBelongsTo($organization, 'organization')
->whereBelongsTo($fromMember, 'member')
->update([
'user_id' => $toMember->user_id,
'member_id' => $toMember->getKey(),
]);
// Project members
ProjectMember::query()
->whereBelongsToOrganization($organization)
->whereBelongsTo($fromMember, 'member')
->whereDoesntHave('project', function (Builder $builder) use ($toMember): void {
/** @var Builder<Project> $builder */
$builder->whereHas('members', function (Builder $builder) use ($toMember): void {
/** @var Builder<ProjectMember> $builder */
$builder->where('member_id', $toMember->getKey());
});
})
->update([
'user_id' => $toMember->user_id,
'member_id' => $toMember->getKey(),
]);
ProjectMember::query()
->whereBelongsToOrganization($organization)
->whereBelongsTo($fromMember, 'member')
->delete();
}
/**
* Change the ownership of an organization to a new user.
* The previous owner will be demoted to an admin.
@@ -137,7 +172,7 @@ class MemberService
$member->role = Role::Placeholder->value;
$member->save();
$this->userService->assignOrganizationEntitiesToDifferentMember($member->organization, $user, $placeholderUser, $member);
$this->userService->assignOrganizationEntitiesToDifferentUser($member->organization, $user, $placeholderUser);
if ($makeSureUserHasAtLeastOneOrganization) {
$this->userService->makeSureUserHasAtLeastOneOrganization($user);
}
app/Service/UserService.php
+2 -18
View File
@@ -49,24 +49,10 @@ class UserService
}
/**
* Assign all organization entities (time entries, project members) from one user to another.
* This is useful when a placeholder user is replaced with a real user.
* This does NOT change the member id.
* This should only be used in if you want to change a member to a placeholder!
*/
public function assignOrganizationEntitiesToDifferentUser(Organization $organization, User $fromUser, User $toUser): void
{
/** @var Member|null $toMember */
$toMember = Member::query()
->whereBelongsTo($organization, 'organization')
->whereBelongsTo($toUser, 'user')
->first();
if ($toMember === null) {
throw new \InvalidArgumentException('User is not a member of the organization');
}
$this->assignOrganizationEntitiesToDifferentMember($organization, $fromUser, $toUser, $toMember);
}
public function assignOrganizationEntitiesToDifferentMember(Organization $organization, User $fromUser, User $toUser, Member $toMember): void
{
// Time entries
TimeEntry::query()
@@ -74,7 +60,6 @@ class UserService
->whereBelongsTo($fromUser, 'user')
->update([
'user_id' => $toUser->getKey(),
'member_id' => $toMember->getKey(),
]);
// Project members
@@ -83,7 +68,6 @@ class UserService
->whereBelongsTo($fromUser, 'user')
->update([
'user_id' => $toUser->getKey(),
'member_id' => $toMember->getKey(),
]);
}
tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php
+52
View File
@@ -350,6 +350,58 @@ class MemberEndpointTest extends ApiEndpointTestAbstract
$memberDestination->refresh();
$this->assertCount(3, $memberDestination->timeEntries);
$this->assertCount(1, $memberDestination->projectMembers);
$this->assertDatabaseHas(ProjectMember::class, [
'project_id' => $project->getKey(),
'member_id' => $memberDestination->getKey(),
'user_id' => $userDestination->getKey(),
]);
}
public function test_merge_into_assigns_resources_of_source_member_to_destination_member_and_deletes_member_with_existing_destination_resources(): void
{
// Arrange
$data = $this->createUserWithPermission([
'members:merge-into',
]);
$userSource = User::factory()->placeholder()->create();
$memberSource = Member::factory()->forUser($userSource)->forOrganization($data->organization)->role(Role::Placeholder)->create();
TimeEntry::factory()->forMember($memberSource)->createMany(3);
$project = Project::factory()->forOrganization($data->organization)->create();
ProjectMember::factory()->forMember($memberSource)->forProject($project)->create([
'billable_rate' => 32100,
]);
$userDestination = User::factory()->create();
$memberDestination = Member::factory()->forUser($userDestination)->forOrganization($data->organization)->role(Role::Admin)->create();
ProjectMember::factory()->forMember($memberDestination)->forProject($project)->create([
'billable_rate' => 12300,
]);
TimeEntry::factory()->forMember($memberDestination)->createMany(3);
Passport::actingAs($data->user);
// Act
$response = $this->withoutExceptionHandling()->postJson(route('api.v1.members.merge-into', [$data->organization->getKey(), $memberSource->getKey()]), [
'member_id' => $memberDestination->getKey(),
]);
// Assert
$response->assertStatus(204);
$this->assertSame('', $response->getContent());
$this->assertDatabaseMissing(Member::class, [
'id' => $memberSource->getKey(),
]);
$this->assertDatabaseMissing(User::class, [
'id' => $userSource->getKey(),
]);
$memberDestination->refresh();
$this->assertCount(6, $memberDestination->timeEntries);
$this->assertCount(1, $memberDestination->projectMembers);
$this->assertDatabaseHas(ProjectMember::class, [
'project_id' => $project->getKey(),
'billable_rate' => 12300,
'member_id' => $memberDestination->getKey(),
'user_id' => $userDestination->getKey(),
]);
}
public function test_update_member_fails_if_user_tries_to_change_role_of_the_current_owner(): void
tests/Unit/Service/MemberServiceTest.php
+90
View File
@@ -113,4 +113,94 @@ class MemberServiceTest extends TestCaseWithDatabase
$this->assertSame($otherMember->getKey(), $otherTimeEntry->member_id);
$this->assertSame(1, $otherUser->organizations()->count());
}
public function test_assign_organization_entities_to_different_member_without_any_entries(): void
{
// Arrange
$organization = Organization::factory()->create();
$project = Project::factory()->forOrganization($organization)->create();
$otherUser = User::factory()->create();
$fromUser = User::factory()->create();
$toUser = User::factory()->create();
$otherUserMember = Member::factory()->forOrganization($organization)->forUser($otherUser)->create();
$fromUserMember = Member::factory()->forOrganization($organization)->forUser($fromUser)->create();
$toUserMember = Member::factory()->forOrganization($organization)->forUser($toUser)->create();
TimeEntry::factory()->forOrganization($organization)->forMember($otherUserMember)->createMany(3);
TimeEntry::factory()->forOrganization($organization)->forMember($fromUserMember)->createMany(3);
ProjectMember::factory()->forProject($project)->forMember($otherUserMember)->create();
ProjectMember::factory()->forProject($project)->forMember($fromUserMember)->create();
// Act
$this->memberService->assignOrganizationEntitiesToDifferentMember($organization, $fromUserMember, $toUserMember);
// Assert
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($toUser, 'user')->count());
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($otherUser, 'user')->count());
$this->assertSame(0, TimeEntry::query()->whereBelongsTo($fromUser, 'user')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($toUser, 'user')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($otherUser, 'user')->count());
$this->assertSame(0, ProjectMember::query()->whereBelongsTo($fromUser, 'user')->count());
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($toUserMember, 'member')->count());
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($otherUserMember, 'member')->count());
$this->assertSame(0, TimeEntry::query()->whereBelongsTo($fromUserMember, 'member')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($toUserMember, 'member')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($otherUserMember, 'member')->count());
$this->assertSame(0, ProjectMember::query()->whereBelongsTo($fromUserMember, 'member')->count());
}
public function test_assign_organization_entities_to_different_member_with_entries(): void
{
// Arrange
$organization = Organization::factory()->create();
$project = Project::factory()->forOrganization($organization)->create();
$otherUser = User::factory()->create();
$fromUser = User::factory()->create();
$toUser = User::factory()->create();
$otherUserMember = Member::factory()->forOrganization($organization)->forUser($otherUser)->create();
$fromUserMember = Member::factory()->forOrganization($organization)->forUser($fromUser)->create();
$toUserMember = Member::factory()->forOrganization($organization)->forUser($toUser)->create();
TimeEntry::factory()->forOrganization($organization)->forMember($otherUserMember)->createMany(3);
TimeEntry::factory()->forOrganization($organization)->forMember($fromUserMember)->createMany(3);
TimeEntry::factory()->forOrganization($organization)->forMember($toUserMember)->createMany(3);
ProjectMember::factory()->forProject($project)->forMember($otherUserMember)->create([
'billable_rate' => 1,
]);
ProjectMember::factory()->forProject($project)->forMember($fromUserMember)->create([
'billable_rate' => 2,
]);
ProjectMember::factory()->forProject($project)->forMember($toUserMember)->create([
'billable_rate' => 3,
]);
// Act
$this->memberService->assignOrganizationEntitiesToDifferentMember($organization, $fromUserMember, $toUserMember);
// Assert
$this->assertSame(6, TimeEntry::query()->whereBelongsTo($toUser, 'user')->count());
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($otherUser, 'user')->count());
$this->assertSame(0, TimeEntry::query()->whereBelongsTo($fromUser, 'user')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($toUser, 'user')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($otherUser, 'user')->count());
$this->assertSame(0, ProjectMember::query()->whereBelongsTo($fromUser, 'user')->count());
$this->assertSame(6, TimeEntry::query()->whereBelongsTo($toUserMember, 'member')->count());
$this->assertSame(3, TimeEntry::query()->whereBelongsTo($otherUserMember, 'member')->count());
$this->assertSame(0, TimeEntry::query()->whereBelongsTo($fromUserMember, 'member')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($toUserMember, 'member')->count());
$this->assertSame(1, ProjectMember::query()->whereBelongsTo($otherUserMember, 'member')->count());
$this->assertSame(0, ProjectMember::query()->whereBelongsTo($fromUserMember, 'member')->count());
$this->assertDatabaseCount(ProjectMember::class, 2);
$this->assertDatabaseHas(ProjectMember::class, [
'project_id' => $project->id,
'member_id' => $toUserMember->id,
'billable_rate' => 3,
]);
$this->assertDatabaseHas(ProjectMember::class, [
'project_id' => $project->id,
'member_id' => $otherUserMember->id,
'billable_rate' => 1,
]);
}
}
tests/Unit/Service/UserServiceTest.php
-16
View File
@@ -59,22 +59,6 @@ class UserServiceTest extends TestCase
$this->assertSame(0, ProjectMember::query()->whereBelongsTo($fromUser, 'user')->count());
}
public function test_assign_organization_entities_to_different_user_fails_if_new_user_is_not_member_of_organization(): void
{
// Arrange
$organization = Organization::factory()->create();
$fromUser = User::factory()->create();
$toUser = User::factory()->create();
$fromUserMember = Member::factory()->forOrganization($organization)->forUser($fromUser)->create();
// Act
try {
$this->userService->assignOrganizationEntitiesToDifferentUser($organization, $fromUser, $toUser);
} catch (\InvalidArgumentException $e) {
$this->assertSame('User is not a member of the organization', $e->getMessage());
}
}
public function test_change_ownership_changes_ownership_of_organization_to_new_user(): void
{
// Arrange