Compare commits

...

8 Commits

Author SHA1 Message Date
Gregor Vostrak
7c3f7e2b67 make sure dropdown/combobox lists stay visible during close animation to
avoid layout shifts
2026-07-09 12:46:11 +02:00
Gregor Vostrak
65fbb43aa6 add stable secondary sorting based on id to the tests to avoid flakyness 2026-07-08 18:24:04 +02:00
Gregor Vostrak
4a5ba9ff28 use pinned selection instead of fix items on dropdown open so async
loaded collections update the dropdown properly
2026-07-08 18:06:58 +02:00
Gregor Vostrak
56c45adc1a add validationa for project names in toggl importer 2026-07-08 17:19:57 +02:00
Gregor Vostrak
fa8d350c4a add secondary name sorting as tie breaker in client and project tables 2026-07-08 15:20:00 +02:00
Gregor Vostrak
27f5d4a200 add unique id tiebreaker to paginated index endpoints to make pagination
stable (#1138)
2026-07-08 14:48:54 +02:00
Gregor Vostrak
c0f5baace1 Add virtualizer to ProjectDropdown, ClientDropdown and Reporting
Comboboxes; Remove redundant focus loop on Project/ClientDropdown
2026-06-30 19:04:30 +02:00
Gregor Vostrak
fddc9abf05 remove the measure row logic and rely on static values only for the
virtualizer and remove duplicated focus trap to avoid infinite loop in
project task dropdown
2026-06-30 14:24:28 +02:00
28 changed files with 541 additions and 152 deletions

View File

@@ -43,7 +43,8 @@ class ClientController extends Controller
$clientsQuery = Client::query()
->whereBelongsTo($organization, 'organization')
->orderBy('created_at', 'desc');
->orderBy('created_at', 'desc')
->orderBy('id');
if (! $canViewAllClients) {
$clientsQuery->visibleByEmployee($user);

View File

@@ -42,6 +42,7 @@ class InvitationController extends Controller
$invitations = $organization->organizationInvitations()
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
return InvitationCollection::make($invitations);

View File

@@ -61,6 +61,7 @@ class MemberController extends Controller
->whereBelongsTo($organization, 'organization')
->with(['user'])
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
return MemberCollection::make($members);

View File

@@ -62,6 +62,7 @@ class ProjectController extends Controller
$projects = $projectsQuery
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
$showBillableRate = $this->member($organization)->role !== Role::Employee->value || $organization->employees_can_see_billable_rates;

View File

@@ -49,6 +49,7 @@ class ProjectMemberController extends Controller
$projectMembers = ProjectMember::query()
->whereBelongsTo($project, 'project')
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
return new ProjectMemberCollection($projectMembers);

View File

@@ -47,6 +47,7 @@ class ReportController extends Controller
$reports = Report::query()
->orderBy('created_at', 'desc')
->orderBy('id')
->whereBelongsTo($organization, 'organization')
->paginate(config('app.pagination_per_page_default'));

View File

@@ -42,6 +42,7 @@ class TagController extends Controller
$tags = Tag::query()
->whereBelongsTo($organization, 'organization')
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
return new TagCollection($tags);

View File

@@ -84,6 +84,7 @@ class TaskController extends Controller
$tasks = $query
->orderBy('created_at', 'desc')
->orderBy('id')
->paginate(config('app.pagination_per_page_default'));
return new TaskCollection($tasks);

View File

@@ -194,7 +194,8 @@ class TimeEntryController extends Controller
$timeEntriesQuery = TimeEntry::query()
->whereBelongsTo($organization, 'organization')
->select($select)
->orderBy('start', 'desc');
->orderBy('time_entries.start', 'desc')
->orderBy('time_entries.id');
$filter = new TimeEntryFilter($timeEntriesQuery);
$filter->addStartFilter($request->input('start'));

View File

@@ -123,6 +123,7 @@ class TogglDataImporter extends DefaultImporter
}
foreach ($projects as $project) {
$projectExternalId = $this->guardExternalIdentifier($project->id);
$clientId = null;
if ($project->client_id !== null) {
$clientId = $this->clientImportHelper->getKeyByExternalIdentifier((string) $project->client_id);
@@ -146,16 +147,16 @@ class TogglDataImporter extends DefaultImporter
'billable_rate' => $project->rate !== null ? (int) ($project->rate * 100) : null,
], (string) $project->id);
if (! file_exists($temporaryDirectory->path('projects_users/'.$project->id.'.json'))) {
throw new ImportException('File "projects_users/'.$project->id.'.json" missing in ZIP');
if (! file_exists($temporaryDirectory->path('projects_users/'.$projectExternalId.'.json'))) {
throw new ImportException('File "projects_users/'.$projectExternalId.'.json" missing in ZIP');
}
$projectMembersFileContent = file_get_contents($temporaryDirectory->path('projects_users/'.$project->id.'.json'));
$projectMembersFileContent = file_get_contents($temporaryDirectory->path('projects_users/'.$projectExternalId.'.json'));
if ($projectMembersFileContent === false) {
throw new ImportException('File "projects_users/'.$project->id.'.json" can not be opened');
throw new ImportException('File "projects_users/'.$projectExternalId.'.json" can not be opened');
}
$projectMembers = json_decode($projectMembersFileContent);
if ($projectMembers === null) {
throw new ImportException('File "projects_users/'.$project->id.'.json" is empty');
throw new ImportException('File "projects_users/'.$projectExternalId.'.json" is empty');
}
foreach ($projectMembers as $projectMember) {
$userId = $this->userImportHelper->getKeyByExternalIdentifier((string) $projectMember->user_id);
@@ -170,6 +171,7 @@ class TogglDataImporter extends DefaultImporter
}
$projectIds = $this->projectImportHelper->getExternalIds();
foreach ($projectIds as $projectIdExternal) {
$projectIdExternal = $this->guardExternalIdentifier($projectIdExternal);
if (! file_exists($temporaryDirectory->path('tasks/'.$projectIdExternal.'.json'))) {
continue;
}
@@ -209,6 +211,30 @@ class TogglDataImporter extends DefaultImporter
}
}
/**
* Ensure an externally-sourced identifier can be safely used inside a
* filesystem path. The identifiers originate from the untrusted uploaded
* ZIP, and Spatie's TemporaryDirectory::path() auto-creates any missing
* parent directory of the resolved path, so an unfiltered "../" sequence
* would escape the import sandbox and create/probe arbitrary paths on the
* host (CWE-22). Toggl identifiers are numeric, so restricting them to a
* conservative allow-list rejects traversal without affecting real data.
*
* @throws ImportException
*/
private function guardExternalIdentifier(mixed $id): string
{
if (! is_string($id) && ! is_int($id)) {
throw new ImportException('Invalid identifier in import data');
}
$id = (string) $id;
if (preg_match('/^[A-Za-z0-9_-]+$/', $id) !== 1) {
throw new ImportException('Invalid identifier in import data');
}
return $id;
}
#[Override]
public function getName(): string
{

View File

@@ -117,6 +117,43 @@ test('test that archiving and unarchiving projects works', async ({ page, ctx })
await expect(page.getByText(newProjectName)).toBeVisible();
});
test('test that the client can be changed in the edit project modal', async ({ page, ctx }) => {
const projectName = 'Edit Client Project ' + Math.floor(1 + Math.random() * 100000);
const clientName = 'Assigned Client ' + Math.floor(1 + Math.random() * 100000);
await createProjectViaApi(ctx, { name: projectName });
const client = await createClientViaApi(ctx, { name: clientName });
await page.goto(PLAYWRIGHT_BASE_URL + '/projects');
await expect(page.getByText(projectName)).toBeVisible({ timeout: 10000 });
// Open the project's Edit modal.
await page.getByRole('row').first().getByRole('button').click();
await page.getByRole('menuitem').getByText('Edit').first().click();
await expect(page.getByRole('dialog')).toBeVisible();
// Open the client dropdown (currently "No Client"), confirm it focuses, and pick the client.
await page.getByRole('dialog').getByRole('button', { name: 'No Client' }).click();
const clientSearch = page.getByPlaceholder('Search for a client...');
await expect(clientSearch).toBeFocused();
await clientSearch.fill(clientName);
await page.getByRole('option', { name: clientName }).click();
// The trigger updates to the chosen client.
await expect(page.getByRole('dialog').getByRole('button', { name: clientName })).toBeVisible();
// Saving persists the client assignment.
await Promise.all([
page.getByRole('button', { name: 'Update Project' }).click(),
page.waitForResponse(
async (response) =>
response.url().includes('/projects/') &&
response.request().method() === 'PUT' &&
response.status() === 200 &&
(await response.json()).data.client_id === client.id
),
]);
});
test('test that updating billable rate works with existing time entries', async ({ page, ctx }) => {
const newProjectName = 'New Project ' + Math.floor(1 + Math.random() * 10000);
const newBillableRate = Math.round(Math.random() * 10000);

View File

@@ -96,6 +96,37 @@ test('test that project multiselect search filters the option list', async ({ pa
await page.keyboard.press('Escape');
});
test('test that the project filter virtualizes a long list (renders only a window)', async ({
page,
ctx,
}) => {
// Create many projects so the dropdown must virtualize rather than render all of them.
const projectNames = Array.from(
{ length: 80 },
(_, i) => `VirtProj ${String(i).padStart(2, '0')}`
);
await Promise.all(projectNames.map((name) => createProjectViaApi(ctx, { name })));
await goToReporting(page);
await expect(page.getByRole('button', { name: 'Export' })).toBeVisible();
await page.getByRole('button', { name: 'Projects' }).first().click();
// Only a small window of options is mounted, far fewer than the 80+ projects that exist.
await expect(page.getByRole('option').first()).toBeVisible();
const renderedCount = await page.getByRole('option').count();
expect(renderedCount).toBeGreaterThan(0);
expect(renderedCount).toBeLessThan(60);
// Virtualization must not drop options: searching narrows the list to the one deep match.
// Wait for the filtered count to settle to 1 before asserting — checking the option while
// the virtualizer is still re-rendering can transiently match a stale row (Firefox CI flake).
await page.getByPlaceholder('Search for a Project...').fill('VirtProj 79');
await expect(page.getByRole('option')).toHaveCount(1);
await expect(page.getByRole('option')).toContainText('VirtProj 79');
await page.keyboard.press('Escape');
});
test('test that selecting multiple projects shows correct badge count', async ({ page, ctx }) => {
const project1Name = 'MultiProj1 ' + Math.floor(Math.random() * 10000);
const project2Name = 'MultiProj2 ' + Math.floor(Math.random() * 10000);

View File

@@ -152,6 +152,49 @@ test('test that editing a task name works', async ({ page, ctx }) => {
await expect(page.getByTestId('task_table')).not.toContainText(originalTaskName);
});
test('test that the project can be searched and changed in the create task modal', async ({
page,
ctx,
}) => {
const sourceProject = 'Source Project ' + Math.floor(1 + Math.random() * 100000);
const targetProject = 'Target Project ' + Math.floor(1 + Math.random() * 100000);
await createProjectViaApi(ctx, { name: sourceProject });
const target = await createProjectViaApi(ctx, { name: targetProject });
await goToProjectsOverview(page);
await page.getByText(sourceProject).first().click();
await page.getByRole('button', { name: 'Create Task' }).click();
await expect(page.getByRole('dialog')).toBeVisible();
// The project dropdown is pre-filled with the source project; open it.
await page.getByRole('dialog').getByRole('button', { name: sourceProject }).click();
// Opening the dropdown focuses the search input; searching narrows it to the target project.
const projectSearch = page.getByPlaceholder('Search for a project...');
await expect(projectSearch).toBeFocused();
await projectSearch.fill('Target Project');
await page.getByRole('option', { name: targetProject }).click();
// Selecting closes the dropdown and updates the trigger to the chosen project.
await expect(
page.getByRole('dialog').getByRole('button', { name: targetProject })
).toBeVisible();
// The new selection is what gets used when the task is created.
const taskName = 'Switched Task ' + Math.floor(1 + Math.random() * 100000);
await page.getByPlaceholder('Task Name').fill(taskName);
await Promise.all([
page.getByRole('dialog').getByRole('button', { name: 'Create Task' }).click(),
page.waitForResponse(
async (response) =>
response.url().includes('/tasks') &&
response.request().method() === 'POST' &&
response.status() === 201 &&
(await response.json()).data.project_id === target.id
),
]);
});
test('test that creating a project with an existing client works', async ({ page, ctx }) => {
const clientName = 'Existing Client ' + Math.floor(1 + Math.random() * 10000);
const projectName = 'Project With Client ' + Math.floor(1 + Math.random() * 10000);

View File

@@ -502,8 +502,10 @@ test.describe('Project Task Dropdown', () => {
await projectOption.getByText(/Tasks/).click();
await page.getByText(taskName, { exact: true }).click();
// The trigger reflects the selected task.
await expect(page.getByText(taskName)).toBeVisible();
// Scoped to the trigger button: the closing dropdown also contains the name while animating out.
await expect(
page.getByRole('button', { name: `${projectName} ${taskName}` })
).toBeVisible();
});
test('test that keyboard navigation selects a project', async ({ page, ctx }) => {
@@ -654,7 +656,10 @@ test.describe('Project Task Dropdown', () => {
await search.press('ArrowDown');
await search.press('Enter');
await expect(page.getByText(taskName)).toBeVisible();
// Scoped to the trigger button: the closing dropdown also contains the name while animating out.
await expect(
page.getByRole('button', { name: `${projectName} ${taskName}` })
).toBeVisible();
});
test('test that pressing space selects the highlighted project', async ({ page, ctx }) => {

View File

@@ -44,11 +44,14 @@ const projectCountMap = computed(() => {
return map;
});
// Name is always the secondary sort so rows with equal values render
// alphabetically instead of in API (created_at) order.
const sorting = computed<SortingState>(() => [
{
id: props.sortColumn,
desc: props.sortDirection === 'desc',
},
...(props.sortColumn !== 'name' ? [{ id: 'name', desc: false }] : []),
]);
const columns = computed(() => [

View File

@@ -89,11 +89,17 @@ function selectMember(member: Member) {
</Button>
</template>
<template #content>
<!-- kept open so the list stays visible during the popover close animation -->
<ComboboxRoot
v-model:search-term="searchValue"
v-model:open="open"
:open="true"
class="relative"
:filter-function="(val: string[]) => val">
:filter-function="(val: string[]) => val"
@update:open="
(value: boolean) => {
if (!value) open = false;
}
">
<ComboboxAnchor>
<ComboboxInput
ref="searchInput"

View File

@@ -9,10 +9,10 @@ import {
ComboboxItem,
ComboboxRoot,
ComboboxViewport,
} from 'radix-vue';
ComboboxVirtualizer,
} from 'reka-ui';
import { Check, Plus } from '@lucide/vue';
import type { CreateClientBody, CreateProjectBody, Project } from '@/packages/api/src';
import { UseFocusTrap } from '@vueuse/integrations/useFocusTrap/component';
import ProjectCreateModal from '@/packages/ui/src/Project/ProjectCreateModal.vue';
import { useProjectsStore } from '@/utils/useProjects';
import { useClientsStore } from '@/utils/useClients';
@@ -37,7 +37,16 @@ const emit = defineEmits(['update:modelValue', 'changed']);
const activeClients = computed(() => clients.value.filter((c) => !c.is_archived));
const sortedProjects = ref<Project[]>([]);
// Pinned on open so rows don't re-sort while interacting; the project list itself stays reactive.
const pinnedProjectId = ref<string | null>(null);
const sortedProjects = computed(() => {
return [...projects.value].sort((a, b) => {
const aPinned = pinnedProjectId.value === a.id ? 0 : 1;
const bPinned = pinnedProjectId.value === b.id ? 0 : 1;
return aPinned - bPinned;
});
});
const shownProjects = computed(() => {
return sortedProjects.value.filter((project) => {
@@ -65,9 +74,7 @@ watch(open, (isOpen) => {
searchInput.value?.$el?.focus();
});
sortedProjects.value = [...projects.value].sort((iteratingProject) => {
return model.value === iteratingProject.id ? -1 : 1;
});
pinnedProjectId.value = model.value;
}
});
@@ -103,40 +110,51 @@ function updateValue(project: Project) {
</template>
<template #content>
<UseFocusTrap v-if="open" :options="{ immediate: true, allowOutsideClick: true }">
<!-- kept open so the list stays visible during the popover close animation -->
<div>
<ComboboxRoot
v-model:search-term="searchValue"
v-model:open="open"
:open="true"
:model-value="currentProject"
class="relative"
@update:model-value="updateValue">
:ignore-filter="true"
@update:model-value="updateValue"
@update:open="
(value: boolean) => {
if (!value) open = false;
}
">
<ComboboxAnchor>
<ComboboxInput
ref="searchInput"
v-model="searchValue"
class="bg-transparent border-0 placeholder-muted-foreground text-sm text-popover-foreground py-2 px-3 focus:ring-0 border-b border-popover-border focus:border-popover-border w-full"
placeholder="Search for a project..." />
</ComboboxAnchor>
<ComboboxContent>
<ComboboxViewport
class="w-[--reka-popper-anchor-width] max-h-60 overflow-y-scroll p-1">
<ComboboxItem
v-for="project in shownProjects"
:key="project.id"
:value="project"
class="relative flex w-full cursor-default select-none items-center rounded-sm py-1.5 pl-2 pr-8 text-sm outline-none data-[highlighted]:bg-accent data-[highlighted]:text-accent-foreground"
:data-project-id="project.id">
<span class="flex items-center gap-2">
<ComboboxVirtualizer
v-slot="{ option: project }"
:options="shownProjects"
:estimate-size="32"
:text-content="(p: Project) => p.name">
<ComboboxItem
:value="project"
class="relative flex w-full cursor-default select-none items-center rounded-sm py-1.5 pl-2 pr-8 text-sm outline-none data-[highlighted]:bg-accent data-[highlighted]:text-accent-foreground"
:data-project-id="project.id">
<span class="flex min-w-0 flex-1 items-center gap-2">
<span
:style="{ backgroundColor: project.color }"
class="w-3 h-3 rounded-full shrink-0"></span>
<span class="truncate">{{ project.name }}</span>
</span>
<span
:style="{ backgroundColor: project.color }"
class="w-3 h-3 rounded-full shrink-0"></span>
<span>{{ project.name }}</span>
</span>
<span
v-if="isProjectSelected(project)"
class="absolute right-2 flex h-3.5 w-3.5 items-center justify-center">
<Check class="h-4 w-4" />
</span>
</ComboboxItem>
v-if="isProjectSelected(project)"
class="absolute right-2 flex h-3.5 w-3.5 items-center justify-center">
<Check class="h-4 w-4" />
</span>
</ComboboxItem>
</ComboboxVirtualizer>
</ComboboxViewport>
<div
v-if="canCreateProjects()"
@@ -150,7 +168,7 @@ function updateValue(project: Project) {
</div>
</ComboboxContent>
</ComboboxRoot>
</UseFocusTrap>
</div>
</template>
</Dropdown>
<ProjectCreateModal

View File

@@ -57,12 +57,15 @@ const clientNameMap = computed(() => {
return map;
});
// Convert sort props to TanStack Table format
// Convert sort props to TanStack Table format.
// Name is always the secondary sort so rows with equal values render
// alphabetically instead of in API (created_at) order.
const sorting = computed<SortingState>(() => [
{
id: props.sortColumn,
desc: props.sortDirection === 'desc',
},
...(props.sortColumn !== 'name' ? [{ id: 'name', desc: false }] : []),
]);
// Define column accessors for sorting.

View File

@@ -8,8 +8,8 @@ import {
ComboboxItem,
ComboboxRoot,
ComboboxViewport,
} from 'radix-vue';
import { UseFocusTrap } from '@vueuse/integrations/useFocusTrap/component';
ComboboxVirtualizer,
} from 'reka-ui';
import Dropdown from '@/packages/ui/src/Input/Dropdown.vue';
import { Check, Plus } from '@lucide/vue';
@@ -26,10 +26,6 @@ const searchInput = ref<HTMLElement | null>(null);
const open = ref(false);
const searchValue = ref('');
function isClientSelected(id: string) {
return model.value === id;
}
watch(open, (isOpen) => {
if (isOpen) {
nextTick(() => {
@@ -58,15 +54,23 @@ async function addClientIfNoneExists() {
}
}
const NO_CLIENT: { id: string | null; name: string } = { id: null, name: 'No Client' };
const currentClient = computed(() => {
return (
props.clients.find((client) => client.id === model.value) ?? {
id: null,
name: 'No Client',
}
);
return props.clients.find((client) => client.id === model.value) ?? NO_CLIENT;
});
type ClientRow = Client | typeof NO_CLIENT;
// Fold the "No Client" entry in as the first row so the whole list virtualizes through one
// ComboboxVirtualizer. NO_CLIENT is a shared constant so currentClient and the row reference
// the same object and single-select highlighting still matches.
const clientRows = computed<ClientRow[]>(() => [NO_CLIENT, ...filteredClients.value]);
function clientRowName(row: ClientRow) {
return row.name;
}
const emit = defineEmits(['update:modelValue', 'changed']);
function updateValue(client: { id: string | null; name: string }) {
@@ -81,56 +85,56 @@ function updateValue(client: { id: string | null; name: string }) {
<slot name="trigger"></slot>
</template>
<template #content>
<UseFocusTrap v-if="open" :options="{ immediate: true, allowOutsideClick: true }">
<div>
<ComboboxRoot
v-model:search-term="searchValue"
v-model:open="open"
:open="true"
:model-value="currentClient"
class="relative"
@update:model-value="updateValue">
:ignore-filter="true"
@update:model-value="updateValue"
@update:open="
(value: boolean) => {
if (!value) open = false;
}
">
<ComboboxAnchor>
<ComboboxInput
ref="searchInput"
v-model="searchValue"
class="bg-transparent border-0 placeholder-muted-foreground text-sm text-popover-foreground py-2 px-3 focus:ring-0 border-b border-popover-border focus:border-popover-border w-full"
placeholder="Search for a client..." />
</ComboboxAnchor>
<ComboboxContent>
<ComboboxViewport
class="w-[--reka-popper-anchor-width] max-h-60 overflow-y-scroll p-1">
<ComboboxItem
:value="{ id: null, name: 'No Client' }"
class="relative flex w-full cursor-default select-none items-center rounded-sm py-1.5 pl-2 pr-8 text-sm outline-none data-[highlighted]:bg-accent data-[highlighted]:text-accent-foreground">
<span>No Client</span>
<span
v-if="model === null"
class="absolute right-2 flex h-3.5 w-3.5 items-center justify-center">
<Check class="h-4 w-4" />
</span>
</ComboboxItem>
<ComboboxItem
v-for="client in filteredClients"
:key="client.id"
:value="client"
class="relative flex w-full cursor-default select-none items-center rounded-sm py-1.5 pl-2 pr-8 text-sm outline-none data-[highlighted]:bg-accent data-[highlighted]:text-accent-foreground"
:data-client-id="client.id">
<span>{{ client.name }}</span>
<span
v-if="isClientSelected(client.id)"
class="absolute right-2 flex h-3.5 w-3.5 items-center justify-center">
<Check class="h-4 w-4" />
</span>
</ComboboxItem>
<div
v-if="searchValue.length > 0 && filteredClients.length === 0"
class="flex items-center gap-2 rounded-sm px-2 py-1.5 text-sm cursor-pointer hover:bg-accent hover:text-accent-foreground"
@click="addClientIfNoneExists">
<Plus class="h-4 w-4 shrink-0" />
<span>Add "{{ searchValue }}" as a new Client</span>
</div>
<ComboboxVirtualizer
v-slot="{ option: row }"
:options="clientRows"
:estimate-size="32"
:text-content="clientRowName">
<ComboboxItem
:value="row"
class="relative flex w-full cursor-default select-none items-center rounded-sm py-1.5 pl-2 pr-8 text-sm outline-none data-[highlighted]:bg-accent data-[highlighted]:text-accent-foreground"
:data-client-id="row.id">
<span class="min-w-0 flex-1 truncate">{{ row.name }}</span>
<span
v-if="model === row.id"
class="absolute right-2 flex h-3.5 w-3.5 items-center justify-center">
<Check class="h-4 w-4" />
</span>
</ComboboxItem>
</ComboboxVirtualizer>
</ComboboxViewport>
<div
v-if="searchValue.length > 0 && filteredClients.length === 0"
class="flex items-center gap-2 rounded-sm mx-1 px-2 py-1.5 text-sm cursor-pointer hover:bg-accent hover:text-accent-foreground"
@click="addClientIfNoneExists">
<Plus class="h-4 w-4 shrink-0" />
<span>Add "{{ searchValue }}" as a new Client</span>
</div>
</ComboboxContent>
</ComboboxRoot>
</UseFocusTrap>
</div>
</template>
</Dropdown>
</template>

View File

@@ -1,6 +1,6 @@
<script setup lang="ts" generic="T">
import Dropdown from '@/packages/ui/src/Input/Dropdown.vue';
import { computed, type Ref, ref, watch } from 'vue';
import { computed, ref, watch } from 'vue';
import Checkbox from '@/packages/ui/src/Input/Checkbox.vue';
import {
ComboboxAnchor,
@@ -9,10 +9,16 @@ import {
ComboboxItem,
ComboboxRoot,
ComboboxViewport,
} from 'radix-vue';
ComboboxVirtualizer,
} from 'reka-ui';
const NONE_ID = 'none';
// height of one row (px-2 py-1.5 text-sm → 12px padding + 20px line box).
// Rows are uniform single-line, so a fixed size is exact enough for the virtualizer and avoids
// any per-row DOM measurement.
const ROW_HEIGHT = 32;
const model = defineModel<string[]>({
default: [],
});
@@ -27,20 +33,25 @@ const props = defineProps<{
const open = ref(false);
const searchValue = ref('');
const sortedItems = ref<T[]>([]) as Ref<T[]>;
// Pinned on open so rows don't re-sort while toggling; the item list itself stays reactive.
const pinnedSelection = ref<Set<string>>(new Set());
watch(open, (isOpen) => {
if (isOpen) {
searchValue.value = '';
sortedItems.value = [...props.items].sort((a, b) => {
const aSelected = model.value.includes(props.getKeyFromItem(a)) ? 0 : 1;
const bSelected = model.value.includes(props.getKeyFromItem(b)) ? 0 : 1;
if (aSelected !== bSelected) return aSelected - bSelected;
return props.getNameForItem(a).localeCompare(props.getNameForItem(b));
});
pinnedSelection.value = new Set(model.value);
}
});
const sortedItems = computed(() => {
return [...props.items].sort((a, b) => {
const aSelected = pinnedSelection.value.has(props.getKeyFromItem(a)) ? 0 : 1;
const bSelected = pinnedSelection.value.has(props.getKeyFromItem(b)) ? 0 : 1;
if (aSelected !== bSelected) return aSelected - bSelected;
return props.getNameForItem(a).localeCompare(props.getNameForItem(b));
});
});
const filteredItems = computed(() => {
const search = searchValue.value.toLowerCase().trim();
if (!search) return sortedItems.value;
@@ -56,6 +67,23 @@ const showNoItem = computed(() => {
return props.noItemLabel.toLowerCase().includes(search);
});
// A single flat list for the virtualizer. The optional "no item" entry is folded in as the
// first row so the whole list (including it) is virtualized through one ComboboxVirtualizer.
type Row = { kind: 'none' } | { kind: 'item'; item: T };
const rows = computed<Row[]>(() => {
const itemRows = filteredItems.value.map((item): Row => ({ kind: 'item', item }));
return showNoItem.value ? [{ kind: 'none' }, ...itemRows] : itemRows;
});
function keyForRow(row: Row): string {
return row.kind === 'none' ? NONE_ID : props.getKeyFromItem(row.item);
}
function nameForRow(row: Row): string {
return row.kind === 'none' ? (props.noItemLabel ?? '') : props.getNameForItem(row.item);
}
function toggleItem(id: string) {
if (model.value.includes(id)) {
model.value = model.value.filter((itemId) => itemId !== id);
@@ -74,46 +102,44 @@ const emit = defineEmits(['update:modelValue', 'changed', 'submit']);
<slot name="trigger"></slot>
</template>
<template #content>
<!-- kept open so the list stays visible during the popover close animation -->
<ComboboxRoot
v-model:search-term="searchValue"
v-model:open="open"
:open="true"
class="p-2"
:filter-function="(val: string[]) => val">
:ignore-filter="true"
@update:open="
(value: boolean) => {
if (!value) open = false;
}
">
<ComboboxAnchor>
<ComboboxInput
v-model="searchValue"
class="w-full h-8 rounded-md border border-input-border bg-input-background px-3 text-sm text-text-primary placeholder:text-text-tertiary focus:outline-none"
:placeholder="searchPlaceholder" />
</ComboboxAnchor>
<ComboboxContent
:dismiss-able="false"
position="inline"
class="mt-2 min-w-60 max-w-80 max-h-60 overflow-y-auto">
<ComboboxViewport>
<ComboboxItem
v-if="showNoItem"
:value="NONE_ID"
class="flex items-center gap-2 rounded-md px-2 py-1.5 text-sm text-text-primary data-[highlighted]:bg-card-background-active cursor-default"
@select.prevent="toggleItem(NONE_ID)">
<Checkbox
:checked="model.includes(NONE_ID)"
aria-hidden="true"
:tabindex="-1"
class="pointer-events-none" />
<span class="truncate">{{ noItemLabel }}</span>
</ComboboxItem>
<ComboboxItem
v-for="item in filteredItems"
:key="getKeyFromItem(item)"
:value="getKeyFromItem(item)"
class="flex items-center gap-2 rounded-md px-2 py-1.5 text-sm text-text-primary data-[highlighted]:bg-card-background-active cursor-default"
@select.prevent="toggleItem(getKeyFromItem(item))">
<Checkbox
:checked="model.includes(getKeyFromItem(item))"
aria-hidden="true"
:tabindex="-1"
class="pointer-events-none" />
<span class="truncate">{{ getNameForItem(item) }}</span>
</ComboboxItem>
class="mt-2 min-w-60 max-w-80">
<ComboboxViewport class="max-h-60 overflow-y-auto">
<ComboboxVirtualizer
v-slot="{ option }"
:options="rows"
:estimate-size="ROW_HEIGHT"
:text-content="nameForRow">
<ComboboxItem
:value="keyForRow(option)"
class="flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-sm text-text-primary data-[highlighted]:bg-card-background-active cursor-default"
@select.prevent="toggleItem(keyForRow(option))">
<Checkbox
:checked="model.includes(keyForRow(option))"
aria-hidden="true"
:tabindex="-1"
class="pointer-events-none" />
<span class="truncate">{{ nameForRow(option) }}</span>
</ComboboxItem>
</ComboboxVirtualizer>
</ComboboxViewport>
</ComboboxContent>
</ComboboxRoot>

View File

@@ -37,19 +37,24 @@ const model = defineModel<string[]>({
const open = ref(false);
const searchValue = ref('');
const sortedTags = ref<Tag[]>([]);
// Pinned on open so rows don't re-sort while toggling; the tag list itself stays reactive.
const pinnedSelection = ref<Set<string>>(new Set());
watch(open, (isOpen) => {
if (isOpen) {
searchValue.value = '';
sortedTags.value = [...props.tags].sort((a, b) => {
const aSelected = model.value.includes(a.id) ? 0 : 1;
const bSelected = model.value.includes(b.id) ? 0 : 1;
return aSelected - bSelected;
});
pinnedSelection.value = new Set(model.value);
}
});
const sortedTags = computed(() => {
return [...props.tags].sort((a, b) => {
const aSelected = pinnedSelection.value.has(a.id) ? 0 : 1;
const bSelected = pinnedSelection.value.has(b.id) ? 0 : 1;
return aSelected - bSelected;
});
});
const filteredTags = computed(() => {
const search = searchValue.value.toLowerCase().trim();
if (!search) return sortedTags.value;

View File

@@ -30,7 +30,6 @@ const project = defineModel<string | null>('project', {
const searchInput = ref<HTMLInputElement | null>(null);
const open = ref(false);
const dropdownViewport = ref<HTMLElement | null>(null);
import { UseFocusTrap } from '@vueuse/integrations/useFocusTrap/component';
const searchValue = ref('');
@@ -117,15 +116,17 @@ const flatRows = computed<FlatRow[]>(() => {
return rows;
});
const ROW_HEIGHT = { client: 28, project: 36, task: 32 } as const;
const rowVirtualizer = useVirtualizer(
computed(() => ({
count: flatRows.value.length,
getScrollElement: () => dropdownViewport.value,
estimateSize: (index: number) => {
const row = flatRows.value[index];
if (row?.kind === 'client') return 28;
if (row?.kind === 'task') return 32;
return 38;
if (row?.kind === 'client') return ROW_HEIGHT.client;
if (row?.kind === 'task') return ROW_HEIGHT.task;
return ROW_HEIGHT.project;
},
getItemKey: (index: number) => flatRows.value[index]?.key ?? index,
overscan: 12,
@@ -141,12 +142,6 @@ const visibleRows = computed(() =>
}))
);
function measureRow(el: unknown): void {
if (el instanceof HTMLElement) {
rowVirtualizer.value.measureElement(el);
}
}
// Lookup maps so filtering is O(projects + tasks + clients) instead of
// O(projects × (tasks + clients)). They are rebuilt only when the underlying task/client
// props change, not on every keystroke.
@@ -599,7 +594,7 @@ const showCreateProject = ref(false);
</slot>
</template>
<template #content>
<UseFocusTrap v-if="open" :options="{ immediate: true, allowOutsideClick: true }">
<div>
<input
ref="searchInput"
:value="searchValue"
@@ -621,8 +616,6 @@ const showCreateProject = ref(false);
<div
v-for="{ virtualRow, row } in visibleRows"
:key="row.key"
:ref="measureRow"
:data-index="virtualRow.index"
class="absolute left-0 top-0 w-full"
:style="{ transform: `translateY(${virtualRow.start}px)` }">
<div
@@ -711,7 +704,7 @@ const showCreateProject = ref(false);
<span>Create new Project</span>
</button>
</div>
</UseFocusTrap>
</div>
</template>
</Dropdown>
<ProjectCreateModal

View File

@@ -45,7 +45,7 @@ class ClientEndpointTest extends ApiEndpointTestAbstract
// Assert
$response->assertStatus(200);
$response->assertJsonCount(4, 'data');
$clients = Client::query()->orderBy('created_at', 'desc')->get();
$clients = Client::query()->orderBy('created_at', 'desc')->orderBy('id')->get();
$response->assertJson(fn (AssertableJson $json) => $json
->has('data')
->has('links')
@@ -84,9 +84,12 @@ class ClientEndpointTest extends ApiEndpointTestAbstract
->has('links')
->has('meta')
->count('data', 2)
->where('data.0.id', $clients->get(0)->getKey())
->where('data.1.id', $clients->get(1)->getKey())
);
// Both clients share the same created_at, so their relative order is not defined.
$this->assertEqualsCanonicalizing([
$clients->get(0)->getKey(),
$clients->get(1)->getKey(),
], $response->json('data.*.id'));
}
public function test_index_endpoint_without_filter_archived_returns_only_non_archived_clients(): void

View File

@@ -13,6 +13,8 @@ use App\Models\ProjectMember;
use App\Models\Task;
use App\Models\TimeEntry;
use App\Service\BillableRateService;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Str;
use Illuminate\Testing\Fluent\AssertableJson;
use Laravel\Passport\Passport;
use Mockery\MockInterface;
@@ -81,6 +83,49 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
$this->assertSame([$projectNewest->getKey(), $projectMiddle->getKey(), $projectOldest->getKey()], $ids);
}
public function test_index_endpoint_pagination_returns_every_project_exactly_once_when_they_share_created_at(): void
{
// Arrange
$data = $this->createUserWithPermission([
'projects:view',
'projects:view:all',
]);
config(['app.pagination_per_page_default' => 15]);
// Bulk import: 300 projects that all share the exact same created_at.
$sharedCreatedAt = now()->subDay()->startOfSecond();
$rows = [];
for ($i = 0; $i < 300; $i++) {
$rows[] = [
'id' => (string) Str::uuid(),
'name' => 'Project '.$i,
'color' => '#000000',
'is_billable' => false,
'is_public' => false,
'organization_id' => $data->organization->getKey(),
'created_at' => $sharedCreatedAt,
'updated_at' => $sharedCreatedAt,
];
}
DB::table('projects')->insert($rows);
Passport::actingAs($data->user);
// Act - walk every page like resources/js/utils/fetchAllPages.ts does.
$orgId = $data->organization->getKey();
$first = $this->getJson(route('api.v1.projects.index', [$orgId]).'?page=1');
$this->assertResponseCode($first, 200);
$lastPage = $first->json('meta.last_page');
$collected = collect($first->json('data.*.id'));
for ($page = 2; $page <= $lastPage; $page++) {
$response = $this->getJson(route('api.v1.projects.index', [$orgId]).'?page='.$page);
$this->assertResponseCode($response, 200);
$collected = $collected->concat($response->json('data.*.id'));
}
// Assert - every project appears exactly once, none duplicated or missing.
$this->assertEqualsCanonicalizing(array_column($rows, 'id'), $collected->all(), 'Some projects were duplicated or missing across pages');
}
public function test_index_endpoint_without_filter_archived_returns_only_non_archived_projects(): void
{
// Arrange

View File

@@ -51,7 +51,7 @@ class ReportEndpointTest extends ApiEndpointTestAbstract
// Assert
$response->assertStatus(200);
$response->assertJsonCount(4, 'data');
$reports = Report::query()->orderBy('created_at', 'desc')->get();
$reports = Report::query()->orderBy('created_at', 'desc')->orderBy('id')->get();
$response->assertJson(fn (AssertableJson $json) => $json
->has('data')
->has('links')

View File

@@ -44,7 +44,7 @@ class TagEndpointTest extends ApiEndpointTestAbstract
// Assert
$response->assertStatus(200);
$response->assertJsonCount(4, 'data');
$tags = Tag::query()->orderBy('created_at', 'desc')->get();
$tags = Tag::query()->orderBy('created_at', 'desc')->orderBy('id')->get();
$response->assertJson(fn (AssertableJson $json) => $json
->has('data')
->has('links')

View File

@@ -23,6 +23,7 @@ use App\Models\User;
use App\Service\TimeEntryFilter;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Queue;
use Illuminate\Support\Facades\Storage;
@@ -391,6 +392,59 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
);
}
public function test_index_endpoint_pagination_returns_every_time_entry_exactly_once_with_rounding(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:view:own',
]);
// Bulk import: 300 time entries that all share the exact same start.
$sharedStart = Carbon::createFromFormat('Y-m-d H:i:s', '2020-01-01 00:00:07');
$rows = [];
for ($i = 0; $i < 300; $i++) {
$rows[] = [
'id' => (string) Str::uuid(),
'description' => 'Entry '.$i,
'start' => $sharedStart,
'end' => $sharedStart,
'billable' => false,
'is_imported' => true,
'user_id' => $data->member->user_id,
'member_id' => $data->member->getKey(),
'organization_id' => $data->organization->getKey(),
'created_at' => $sharedStart,
'updated_at' => $sharedStart,
];
}
DB::table('time_entries')->insert($rows);
$this->actAsOrganizationWithSubscription();
Passport::actingAs($data->user);
// Act - walk every page like the client does (limit/offset), with rounding enabled.
$orgId = $data->organization->getKey();
$limit = 15;
$collected = collect();
$offset = 0;
do {
$response = $this->getJson(route('api.v1.time-entries.index', [
$orgId,
'member_id' => $data->member->getKey(),
'rounding_type' => TimeEntryRoundingType::Nearest,
'rounding_minutes' => 6,
'limit' => $limit,
'offset' => $offset,
]));
$this->assertResponseCode($response, 200);
$ids = $response->json('data.*.id');
$collected = $collected->concat($ids);
$offset += $limit;
} while (count($ids) === $limit);
// Assert - every time entry appears exactly once, none duplicated or missing.
$this->assertEqualsCanonicalizing(array_column($rows, 'id'), $collected->all(), 'Some time entries were duplicated or missing across pages');
}
public function test_index_endpoint_can_round_up(): void
{
// Arrange

View File

@@ -11,6 +11,8 @@ use App\Service\Import\Importers\ImportException;
use App\Service\Import\Importers\TogglDataImporter;
use Exception;
use PHPUnit\Framework\Attributes\CoversClass;
use Spatie\TemporaryDirectory\TemporaryDirectory;
use ZipArchive;
#[CoversClass(TogglDataImporter::class)]
#[CoversClass(ImportException::class)]
@@ -88,6 +90,82 @@ class TogglDataImporterTest extends ImporterTestAbstract
$this->assertSame(0, $report->clientsCreated);
}
public function test_import_with_path_traversal_in_project_id_is_rejected_without_touching_the_filesystem(): void
{
// Arrange
$organization = Organization::factory()->create();
$importer = new TogglDataImporter;
$importer->init($organization);
$markerDir = sys_get_temp_dir().'/solidtime_path_traversal_'.uniqid();
$this->assertDirectoryDoesNotExist($markerDir);
// Enough "../" to reach the filesystem root from any temp location, then
// back down into the attacker-chosen marker directory. The importer
// appends ".json", so the parent directory Spatie's TemporaryDirectory
// would auto-create for the resolved path is exactly $markerDir.
$traversalId = str_repeat('../', 40).ltrim($markerDir, '/').'/probe';
$data = file_get_contents($this->buildTogglZipWithProjectId($traversalId));
// Act
try {
$importer->importData($data, 'Europe/Vienna');
$this->fail('Expected ImportException was not thrown');
} catch (ImportException $e) {
// Rejected by the identifier guard, not by a downstream
// "missing in ZIP" error (which would mean the sink was reached
// and the directory had already been created).
$this->assertSame('Invalid identifier in import data', $e->getMessage());
}
// Assert: no directory was created outside the import sandbox.
$this->assertDirectoryDoesNotExist($markerDir);
}
public function test_import_with_valid_numeric_project_id_is_accepted(): void
{
// Arrange
$organization = Organization::factory()->create();
$importer = new TogglDataImporter;
$importer->init($organization);
// A legitimate Toggl numeric id must still pass the guard. The
// projects_users file is intentionally absent, so the importer fails
// with the ordinary "missing in ZIP" error rather than the guard error.
$data = file_get_contents($this->buildTogglZipWithProjectId(402));
// Act
try {
$importer->importData($data, 'Europe/Vienna');
$this->fail('Expected ImportException was not thrown');
} catch (ImportException $e) {
// Assert: the numeric id passed the guard and reached the ZIP
// content check (proving valid data is not rejected).
$this->assertSame('File "projects_users/402.json" missing in ZIP', $e->getMessage());
}
}
private function buildTogglZipWithProjectId(mixed $projectId): string
{
$tempDir = TemporaryDirectory::make();
$zipPath = $tempDir->path('traversal.zip');
$zip = new ZipArchive;
$zip->open($zipPath, ZipArchive::CREATE);
$zip->addFromString('clients.json', '[]');
$zip->addFromString('tags.json', '[]');
$zip->addFromString('workspace_users.json', '[]');
$zip->addFromString('projects.json', (string) json_encode([[
'id' => $projectId,
'client_id' => null,
'color' => '#ff0000',
'billable' => false,
'is_private' => false,
'rate' => null,
'name' => 'Traversal',
]]));
$zip->close();
return $zipPath;
}
public function test_import_of_user_with_unknown_timezone_will_be_mapped_to_utc(): void
{
// Arrange