Compare commits

...

4 Commits

Author SHA1 Message Date
Gregor Vostrak
326b76a07f fix daterange presets, fix e2e test 2025-06-30 12:43:20 +02:00
Gregor Vostrak
af5e894c83 add information about placeholders in delete modal 2025-06-30 12:20:05 +02:00
Gregor Vostrak
ba866751ff add delete modal for member delete with relations
allow admins to delete members
fix Dialog cloes on click outside of content
2025-06-26 16:05:46 +02:00
Constantin Graf
c8d6ad734e Add option to delete members with relations 2025-06-24 17:37:10 +02:00
12 changed files with 348 additions and 52 deletions

View File

@@ -16,6 +16,7 @@ use App\Exceptions\Api\OrganizationNeedsAtLeastOneOwner;
use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException;
use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException;
use App\Exceptions\Api\UserNotPlaceholderApiException;
use App\Http\Requests\V1\Member\MemberDestroyRequest;
use App\Http\Requests\V1\Member\MemberIndexRequest;
use App\Http\Requests\V1\Member\MemberMergeIntoRequest;
use App\Http\Requests\V1\Member\MemberUpdateRequest;
@@ -100,11 +101,13 @@ class MemberController extends Controller
*
* @operationId removeMember
*/
public function destroy(Organization $organization, Member $member, MemberService $memberService): JsonResponse
public function destroy(MemberDestroyRequest $request, Organization $organization, Member $member, MemberService $memberService): JsonResponse
{
$this->checkPermission($organization, 'members:delete', $member);
$memberService->removeMember($member, $organization);
$deleteRelated = $request->getDeleteRelated();
$memberService->removeMember($member, $organization, $deleteRelated);
return response()
->json(null, 204);

View File

@@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
namespace App\Http\Requests\V1\Member;
use App\Http\Requests\V1\BaseFormRequest;
use App\Models\Organization;
use Illuminate\Contracts\Validation\ValidationRule;
/**
* @property Organization $organization
*/
class MemberDestroyRequest extends BaseFormRequest
{
/**
* Get the validation rules that apply to the request.
*
* @return array<string, array<string|ValidationRule>>
*/
public function rules(): array
{
return [
'delete_related' => [
'string',
'in:true,false',
],
];
}
public function getDeleteRelated(): bool
{
return $this->input('delete_related', 'false') === 'true';
}
}

View File

@@ -187,6 +187,7 @@ class JetstreamServiceProvider extends ServiceProvider
'members:invite-placeholder',
'members:make-placeholder',
'members:merge-into',
'members:delete',
'members:update',
'reports:view',
'reports:create',

View File

@@ -61,18 +61,24 @@ class MemberService
* @throws CanNotRemoveOwnerFromOrganization
* @throws EntityStillInUseApiException
*/
public function removeMember(Member $member, Organization $organization): void
public function removeMember(Member $member, Organization $organization, bool $withRelations = false): void
{
if (TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->exists()) {
throw new EntityStillInUseApiException('member', 'time_entry');
}
if (ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->exists()) {
throw new EntityStillInUseApiException('member', 'project_member');
}
if ($member->role === Role::Owner->value) {
throw new CanNotRemoveOwnerFromOrganization;
}
if ($withRelations) {
TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->delete();
ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->delete();
} else {
if (TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->exists()) {
throw new EntityStillInUseApiException('member', 'time_entry');
}
if (ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->exists()) {
throw new EntityStillInUseApiException('member', 'project_member');
}
}
$member->delete();
MemberRemoved::dispatch($member, $organization);
}

View File

@@ -102,7 +102,7 @@ test('test that updating billable rate works with existing time entries', async
await page.getByRole('row').first().getByRole('button').click();
await page.getByRole('menuitem').getByText('Edit').first().click();
await page.getByText('Non-Billable').click();
await page.getByText('Non-Billable').click();
await page.getByText('Custom Rate').click();
await page
.getByPlaceholder('Billable Rate')
@@ -111,8 +111,8 @@ test('test that updating billable rate works with existing time entries', async
await Promise.all([
page
.getByRole('button', { name: 'Yes, update existing time entries' })
.click(),
.locator('button').filter({ hasText: 'Yes, update existing time' })
.click(),
page.waitForRequest(
async (request) =>
request.url().includes('/projects/') &&

View File

@@ -0,0 +1,151 @@
<script setup lang="ts">
import type { Member } from '@/packages/api/src';
import { api } from '@/packages/api/src';
import { useForm } from '@tanstack/vue-form';
import { useMutation } from '@tanstack/vue-query';
import Modal from '@/packages/ui/src/Modal.vue';
import DangerButton from '@/packages/ui/src/Buttons/DangerButton.vue';
import SecondaryButton from '@/packages/ui/src/Buttons/SecondaryButton.vue';
import Checkbox from '@/packages/ui/src/Input/Checkbox.vue';
import { useNotificationsStore } from '@/utils/notification';
import { getCurrentOrganizationId } from '@/utils/useUser';
import InputLabel from '@/packages/ui/src/Input/InputLabel.vue';
import InputError from '@/packages/ui/src/Input/InputError.vue';
import { useMembersStore } from '@/utils/useMembers';
const props = defineProps<{
show: boolean;
member: Member;
}>();
const emit = defineEmits<{
'update:show': [value: boolean];
}>();
const { handleApiRequestNotifications } = useNotificationsStore();
const deleteMutation = useMutation({
mutationFn: async () => {
const organizationId = getCurrentOrganizationId();
if (!organizationId) {
throw new Error('No organization ID found');
}
return api.removeMember(undefined, {
params: {
member: props.member.id,
organization: organizationId,
},
queries: {
delete_related: 'true',
},
});
},
onSuccess: () => {
close();
useMembersStore().fetchMembers();
}
});
const form = useForm({
canSubmitWhenInvalid: true,
defaultValues: {
confirmDelete: false,
},
onSubmit: async () => {
await handleApiRequestNotifications(
() => deleteMutation.mutateAsync(),
'Member deleted successfully',
'Error deleting member'
);
},
});
const close = () => {
emit('update:show', false);
form.reset();
};
</script>
<template>
<Modal :show="show" max-width="md" @close="close">
<div class="p-6">
<h2 class="text-lg font-medium text-text-primary">
Delete Member
</h2>
<div class="mt-4 text-sm text-text-secondary">
<p class="mb-4">
Are you sure you want to delete {{ member.name }}? This action cannot be undone.
</p>
<p class="mb-4">
This will permanently delete:
</p>
<ul class="list-disc ml-6 mt-2">
<li>All time entries created by this member</li>
<li>Their project assignments</li>
<li>Their organization membership</li>
</ul>
<p class="pt-4">
<strong>Note:</strong> Deleting time entries will affect all reports and statistics.
If you want to keep the time entries but remove the member from your organization, you can convert them to a placeholder user instead. Placeholder users are not charged and their time entries remain intact for reporting purposes.
</p>
</div>
<form
class="mt-6" @submit="
(e) => {
e.preventDefault();
e.stopPropagation();
form.handleSubmit();
}
">
<div class="flex items-start">
<form.Field
name="confirmDelete"
:validators="{
onSubmit: ({value}) => {
if (!value) {
return 'You must confirm that you understand the consequences of this action';
}
return '';
}
}"
>
<template #default="{ field }">
<div class="flex flex-col">
<div class="flex items-center space-x-3 text-sm">
<Checkbox
:id="field.name"
:name="field.name"
:checked="field.state.value"
@update:checked="field.handleChange"
@blur="field.handleBlur"
/>
<InputLabel :for="field.name" class="font-medium text-text-primary">
I understand that this will permanently delete all data related to this member
</InputLabel>
</div>
<InputError class="pl-7 pt-2" :message="field.state.meta.errors[0]" />
</div>
</template>
</form.Field>
</div>
<div class="mt-6 flex justify-end space-x-3">
<SecondaryButton @click="close">Cancel</SecondaryButton>
<form.Subscribe>
<template #default="{ canSubmit, isSubmitting }">
<DangerButton
type="submit"
:disabled="!canSubmit"
>
{{ isSubmitting ? 'Deleting...' : 'Delete Member' }}
</DangerButton>
</template>
</form.Subscribe>
</div>
</form>
</div>
</Modal>
</template>

View File

@@ -49,15 +49,6 @@ const props = defineProps<{
<PencilSquareIcon class="w-5 text-icon-active" />
<span>Edit</span>
</DropdownMenuItem>
<DropdownMenuItem
v-if="canDeleteMembers()"
:aria-label="'Delete Member ' + props.member.name"
data-testid="member_delete"
class="flex items-center space-x-3 cursor-pointer text-destructive focus:text-destructive"
@click="emit('delete')">
<TrashIcon class="w-5" />
<span>Delete</span>
</DropdownMenuItem>
<DropdownMenuItem
v-if="props.member.role === 'placeholder' && canMergeMembers()"
:aria-label="'Merge Member ' + props.member.name"
@@ -75,6 +66,15 @@ const props = defineProps<{
<UserCircleIcon class="w-5 text-icon-active" />
<span>Deactivate</span>
</DropdownMenuItem>
<DropdownMenuItem
v-if="canDeleteMembers()"
:aria-label="'Delete Member ' + props.member.name"
data-testid="member_delete"
class="flex items-center space-x-3 cursor-pointer text-destructive focus:text-destructive"
@click="emit('delete')">
<TrashIcon class="w-5" />
<span>Delete</span>
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
</template>

View File

@@ -8,26 +8,30 @@ import SecondaryButton from '@/packages/ui/src/Buttons/SecondaryButton.vue';
import { getCurrentOrganizationId } from '@/utils/useUser';
import { useNotificationsStore } from '@/utils/notification';
import { canInvitePlaceholderMembers } from '@/utils/permissions';
import { useMembersStore } from '@/utils/useMembers';
import { computed, type ComputedRef, inject, ref } from 'vue';
import MemberEditModal from '@/Components/Common/Member/MemberEditModal.vue';
import MemberMergeModal from '@/Components/Common/Member/MemberMergeModal.vue';
import MemberMakePlaceholderModal from '@/Components/Common/Member/MemberMakePlaceholderModal.vue';
import MemberDeleteModal from '@/Components/Common/Member/MemberDeleteModal.vue';
import { capitalizeFirstLetter } from '../../../utils/format';
import { formatCents } from '../../../packages/ui/src/utils/money';
import { useMembersStore } from '@/utils/useMembers';
const props = defineProps<{
member: Member;
}>();
const organization = inject<ComputedRef<Organization>>('organization');
const memberStore = useMembersStore();
const showEditMemberModal = ref(false);
const showMergeMemberModal = ref(false);
const showMakeMemberPlaceholderModal = ref(false);
const showDeleteMemberModal = ref(false);
function removeMember() {
useMembersStore().removeMember(props.member.id);
showDeleteMemberModal.value = true;
memberStore.fetchMembers();
}
async function invitePlaceholder(id: string) {
@@ -121,6 +125,9 @@ const userHasValidMailAddress = computed(() => {
<MemberMakePlaceholderModal
v-model:show="showMakeMemberPlaceholderModal"
:member="member"></MemberMakePlaceholderModal>
<MemberDeleteModal
v-model:show="showDeleteMemberModal"
:member="member"></MemberDeleteModal>
</TableRow>
</template>

View File

@@ -30,22 +30,21 @@ const forwarded = useForwardPropsEmits(delegatedProps, emits)
<div
class="absolute inset-0 bg-default-background opacity-30" />
</DialogOverlay>
<DialogContent
v-bind="forwarded"
<div
:class="
cn(
'fixed top-0 left-0 z-50 w-screen h-screen flex items-start pt-6 md:pt-20 xl:pt-32 justify-center overflow-auto data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95',
'fixed top-0 left-0 z-50 pointer-events-none w-screen h-screen flex items-start pt-6 md:pt-20 xl:pt-32 justify-center overflow-auto',
)"
>
<div
<DialogContent
v-bind="forwarded"
:class="cn(
'bg-default-background grid w-full max-w-lg border shadow-lg duration-200 sm:rounded-lg',
'bg-default-background grid w-full max-w-lg border border-border-tertiary shadow-lg duration-200 sm:rounded-lg data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95',
props.class,
)"
>
<slot />
</DialogContent>
</div>
</DialogContent>
</DialogPortal>
</template>

View File

@@ -2407,6 +2407,11 @@ const endpoints = makeApi([
type: 'Path',
schema: z.string(),
},
{
name: 'delete_related',
type: 'Query',
schema: z.enum(['true', 'false']).optional(),
},
],
response: z.void(),
errors: [
@@ -2436,6 +2441,16 @@ const endpoints = makeApi([
description: `Not found`,
schema: z.object({ message: z.string() }).passthrough(),
},
{
status: 422,
description: `Validation error`,
schema: z
.object({
message: z.string(),
errors: z.record(z.array(z.string())),
})
.passthrough(),
},
],
},
{

View File

@@ -73,18 +73,18 @@ const open = ref(false);
function setToday() {
emit(
'update:start',
getLocalizedDayJs().startOf('day').format('YYYY-MM-DD')
getLocalizedDayJs().startOf('day').format()
);
emit('update:end', getLocalizedDayJs().endOf('day').format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().endOf('day').format());
open.value = false;
}
function setThisWeek() {
emit(
'update:start',
getLocalizedDayJs().startOf('week').format('YYYY-MM-DD')
getLocalizedDayJs().startOf('week').format()
);
emit('update:end', getLocalizedDayJs().endOf('week').format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().endOf('week').format());
open.value = false;
}
@@ -94,14 +94,14 @@ function setLastWeek() {
getLocalizedDayJs()
.subtract(1, 'week')
.startOf('week')
.format('YYYY-MM-DD')
.format()
);
emit(
'update:end',
getLocalizedDayJs()
.subtract(1, 'week')
.endOf('week')
.format('YYYY-MM-DD')
.format()
);
open.value = false;
}
@@ -109,18 +109,18 @@ function setLastWeek() {
function setLast14Days() {
emit(
'update:start',
getLocalizedDayJs().subtract(14, 'days').format('YYYY-MM-DD')
getLocalizedDayJs().subtract(14, 'days').format()
);
emit('update:end', getLocalizedDayJs().format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().format());
open.value = false;
}
function setThisMonth() {
emit(
'update:start',
getLocalizedDayJs().startOf('month').format('YYYY-MM-DD')
getLocalizedDayJs().startOf('month').format()
);
emit('update:end', getLocalizedDayJs().endOf('month').format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().endOf('month').format());
open.value = false;
}
@@ -130,14 +130,14 @@ function setLastMonth() {
getLocalizedDayJs()
.subtract(1, 'month')
.startOf('month')
.format('YYYY-MM-DD')
.format()
);
emit(
'update:end',
getLocalizedDayJs()
.subtract(1, 'month')
.endOf('month')
.format('YYYY-MM-DD')
.format()
);
open.value = false;
}
@@ -145,36 +145,36 @@ function setLastMonth() {
function setLast30Days() {
emit(
'update:start',
getLocalizedDayJs().subtract(30, 'days').format('YYYY-MM-DD')
getLocalizedDayJs().subtract(30, 'days').format()
);
emit('update:end', getLocalizedDayJs().format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().format());
open.value = false;
}
function setLast90Days() {
emit(
'update:start',
getDayJsInstance()().subtract(90, 'days').format('YYYY-MM-DD')
getDayJsInstance()().subtract(90, 'days').format()
);
emit('update:end', getDayJsInstance()().format('YYYY-MM-DD'));
emit('update:end', getDayJsInstance()().format());
open.value = false;
}
function setLast12Months() {
emit(
'update:start',
getLocalizedDayJs().subtract(12, 'months').format('YYYY-MM-DD')
getLocalizedDayJs().subtract(12, 'months').format()
);
emit('update:end', getLocalizedDayJs().format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().format());
open.value = false;
}
function setThisYear() {
emit(
'update:start',
getLocalizedDayJs().startOf('year').format('YYYY-MM-DD')
getLocalizedDayJs().startOf('year').format()
);
emit('update:end', getLocalizedDayJs().endOf('year').format('YYYY-MM-DD'));
emit('update:end', getLocalizedDayJs().endOf('year').format());
open.value = false;
}
@@ -184,14 +184,14 @@ function setLastYear() {
getLocalizedDayJs()
.subtract(1, 'year')
.startOf('year')
.format('YYYY-MM-DD')
.format()
);
emit(
'update:end',
getLocalizedDayJs()
.subtract(1, 'year')
.endOf('year')
.format('YYYY-MM-DD')
.format()
);
open.value = false;
}

View File

@@ -653,6 +653,85 @@ class MemberEndpointTest extends ApiEndpointTestAbstract
Event::assertNotDispatched(MemberRemoved::class);
}
public function test_destroy_endpoint_succeeds_if_member_is_still_in_use_by_a_project_member_and_delete_related_is_active(): void
{
// Arrange
$data = $this->createUserWithPermission([
'members:delete',
]);
$otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create();
$project = Project::factory()->forOrganization($data->organization)->create();
$projectMember = ProjectMember::factory()->forProject($project)->forMember($data->member)->create();
$otherProjectMember = ProjectMember::factory()->forProject($project)->forMember($otherMember)->create();
Passport::actingAs($data->user);
Event::fake([
MemberRemoved::class,
]);
// Act
$response = $this->deleteJson(route('api.v1.members.destroy', [
'organization' => $data->organization->getKey(),
'member' => $data->member->getKey(),
'delete_related' => 'true',
]));
// Assert
$response->assertStatus(204);
$this->assertDatabaseMissing(Member::class, [
'id' => $data->member->getKey(),
]);
$this->assertDatabaseHas(ProjectMember::class, [
'id' => $otherProjectMember->getKey(),
'member_id' => $otherMember->getKey(),
'user_id' => $otherMember->user_id,
]);
$this->assertDatabaseMissing(ProjectMember::class, [
'id' => $projectMember->getKey(),
]);
Event::assertDispatched(function (MemberRemoved $event) use ($data): bool {
return $event->organization->is($data->organization) &&
$event->member->is($data->member);
}, 1);
}
public function test_destroy_endpoint_succeeds_if_member_is_still_in_use_by_a_time_entry_and_delete_related_is_active(): void
{
// Arrange
$data = $this->createUserWithPermission([
'members:delete',
]);
$otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create();
$timeEntry = TimeEntry::factory()->forMember($data->member)->forOrganization($data->organization)->create();
$otherTimeEntry = TimeEntry::factory()->forMember($otherMember)->forOrganization($data->organization)->create();
Passport::actingAs($data->user);
Event::fake([
MemberRemoved::class,
]);
// Act
$response = $this->deleteJson(route('api.v1.members.destroy', [
'organization' => $data->organization->getKey(),
'member' => $data->member->getKey(),
'delete_related' => 'true',
]));
// Assert
$response->assertStatus(204);
$this->assertDatabaseMissing(Member::class, [
'id' => $data->member->getKey(),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherTimeEntry->getKey(),
]);
$this->assertDatabaseMissing(TimeEntry::class, [
'id' => $timeEntry->getKey(),
]);
Event::assertDispatched(function (MemberRemoved $event) use ($data): bool {
return $event->organization->is($data->organization) &&
$event->member->is($data->member);
}, 1);
}
public function test_destroy_member_succeeds_if_data_is_valid(): void
{
// Arrange