Merge pull request #1504 from BradGroux/fix/template-pending-1503

fix: retain template dialog during application
This commit is contained in:
Brad Groux 2026-09-04 12:53:35 -05:00 • committed by GitHub
commit c5dc3de3ac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 445 additions and 96 deletions

View file

@ -36,6 +36,12 @@ Reduced-motion preferences make CSS transitions immediate, with no transition de
- `task-detail.spec.ts` expanded-workspace case passed. `task-popout-stack.spec.ts` adds template and nested-utility browser checks. Fixtures do not launch agents, start preview servers, resolve conflicts, or create managed worktrees; managed ownership exists only in intercepted browser reads.
- Independent standards and specification source reviews identified scrolling utility controls and non-quiet Cancel actions. Both were corrected and cleared on recheck. Remaining evidence gaps are explicit above.
### Apply Template submission
Apply Template owns submission and dismissal until both the task update and non-fatal activity logging settle. Template selection, variables, overwrite strategy and close controls stay disabled during that operation. Update failures preserve the draft and focus a visible inline error; logging failure after a successful update still completes the application. Variable input values remain stable across deferred rendering.
The verification contract includes immediate duplicate prevention, retained inputs, deliberate retry and delayed logging failure. `task-template-pending.spec.ts` checks pending and failed states in both themes and motion settings at 1700×900/16px, 1180×760/20px and 900×480/20px, with fixed/reachable footers, exact intercepted update payloads, disabled dismissal/inputs and opener restoration. Fixture-backed browser checks do not establish native utility-operation or final documentation-media acceptance. Delivery evidence is tracked in #1503.
### Supporting task dialogs
`task-support-popouts.spec.ts` covers task, comment, attachment, observation, and deliverable deletion plus manual time entry. Each family is exercised in light/dark, normal/reduced motion, and 1700×900 at 16px, 1180×760 at 20px, and 900×480 at 20px. Checks cover bounded bodies, fixed/reachable footers, no horizontal overflow, keyboard opening, trapped focus, exact opener restoration, and retained task title. Delayed synthetic failed requests exercise Escape, header-close, and backdrop dismissal guards, one submission, inline error recovery, and preserved manual-time drafts. This proves retry availability, not a successful retry. No supporting-record mutation reaches the backend.

View file

@ -0,0 +1,195 @@
import { expect, test } from '@playwright/test';
import { bypassAuth, cleanupRoutes, deleteTask, seedTestTask } from './helpers/auth';
for (const theme of ['light', 'dark']) {
for (const reducedMotion of ['no-preference', 'reduce'] as const) {
test(`template application retains pending state in ${theme}, motion ${reducedMotion}`, async ({
page,
}) => {
await bypassAuth(page);
await page.emulateMedia({ reducedMotion });
await page.addInitScript(
(theme) => localStorage.setItem('veritas-kanban-theme', theme),
theme
);
const title = `Template pending fixture ${theme} ${reducedMotion}`;
const task = await seedTestTask(page, { title, type: 'code', description: '' });
const writes: Array<{ method: string; path: string; body: unknown }> = [];
let release = () => {};
const gate = new Promise<void>((resolve) => {
release = resolve;
});
await page.route('**/api/templates', (route) =>
route.fulfill({
json: [
{
id: 'template-pending-fixture',
name: 'Pending fixture',
category: 'bug',
taskDefaults: { descriptionTemplate: 'Investigate {{custom:ticket}}' },
subtaskTemplates: [],
},
],
})
);
await page.route(
new RegExp(`/api/tasks/${task.id}(?:/apply-template)?(?:\\?|$)`),
async (route) => {
const request = route.request();
if (request.method() === 'GET') return route.fallback();
writes.push({
method: request.method(),
path: new URL(request.url()).pathname,
body: request.postDataJSON(),
});
await gate;
return route.fulfill({
status: 503,
json: { error: 'Template fixture update failed. Your draft is retained for retry.' },
});
}
);
try {
await page.setViewportSize({ width: 1700, height: 900 });
await page.goto('/');
await page.getByRole('article', { name: `Task: ${title}` }).press('Enter');
const workspace = page.getByTestId('task-detail-panel');
await workspace.getByRole('button', { name: 'Plan', exact: true }).click();
const opener = workspace.getByRole('button', { name: 'Template', exact: true });
await opener.press('Enter');
const dialog = page.getByRole('dialog', { name: 'Apply Template to Task' });
await expect(workspace).toHaveAttribute('inert', '');
await dialog.getByRole('combobox', { name: 'Template', exact: true }).click();
await page.getByRole('option', { name: /Pending fixture/ }).click();
const variable = dialog.getByLabel('ticket', { exact: true });
await variable.fill('BUG-42');
await dialog.getByRole('switch', { name: 'Force overwrite' }).press('Space');
await expect(dialog.getByRole('switch', { name: 'Force overwrite' })).toBeChecked();
await dialog.getByRole('button', { name: 'Help', exact: true }).click();
const submit = dialog.getByRole('button', { name: 'Apply Template', exact: true });
await submit.click();
await expect.poll(() => writes.length).toBe(1);
await submit.evaluate((el) => (el as HTMLButtonElement).click());
for (const size of [
{ width: 1700, height: 900, fontSize: '16px' },
{ width: 1180, height: 760, fontSize: '20px' },
{ width: 900, height: 480, fontSize: '20px' },
]) {
await page.setViewportSize(size);
await page.evaluate((fontSize) => {
document.documentElement.style.fontSize = fontSize;
}, size.fontSize);
await expect(dialog.getByRole('button', { name: 'Close dialog' })).toHaveCSS(
'width',
size.fontSize === '20px' ? '42.5px' : '34px'
);
await dialog.evaluate(async (el) => {
await Promise.all(
el
.getAnimations({ subtree: true })
.filter((a) => a.effect?.getTiming().iterations !== Infinity)
.map((a) => a.finished.catch(() => {}))
);
});
const footer = dialog.locator('.vk-overlay-footer');
const before = await footer.boundingBox();
await dialog.locator('.vk-overlay-scroll').evaluate((el) => {
el.scrollTop = el.scrollHeight;
});
expect(await footer.boundingBox()).toEqual(before);
for (const name of ['Apply Template', 'Cancel', 'Close dialog']) {
await expect(dialog.getByRole('button', { name, exact: true })).toBeDisabled();
await expect(dialog.getByRole('button', { name, exact: true })).toBeInViewport({
ratio: 1,
});
}
await expect(variable).toBeDisabled();
await expect(dialog.getByRole('combobox', { name: 'Template' })).toBeDisabled();
await expect(dialog.getByRole('switch', { name: 'Force overwrite' })).toBeDisabled();
await page.keyboard.press('Escape');
await dialog
.getByRole('button', { name: 'Close dialog' })
.evaluate((el) => (el as HTMLButtonElement).click());
await page.mouse.click(3, 3);
await expect(dialog).toBeVisible();
expect(await dialog.evaluate((el) => el.scrollWidth - el.clientWidth)).toBe(0);
}
await page.screenshot({ path: test.info().outputPath('template-pending.png') });
release();
const error = dialog.getByRole('alert', { name: 'Template not applied' });
await expect(error).toContainText('Template fixture update failed');
await expect(error).toBeFocused();
for (const size of [
{ width: 1700, height: 900, fontSize: '16px' },
{ width: 1180, height: 760, fontSize: '20px' },
{ width: 900, height: 480, fontSize: '20px' },
]) {
await page.setViewportSize(size);
await page.evaluate((fontSize) => {
document.documentElement.style.fontSize = fontSize;
}, size.fontSize);
await expect(dialog.getByRole('button', { name: 'Close dialog' })).toHaveCSS(
'width',
size.fontSize === '20px' ? '42.5px' : '34px'
);
await dialog.evaluate(async (el) => {
await Promise.all(
el
.getAnimations({ subtree: true })
.filter((a) => a.effect?.getTiming().iterations !== Infinity)
.map((a) => a.finished.catch(() => {}))
);
});
await error.scrollIntoViewIfNeeded();
await expect(error).toBeInViewport({ ratio: 1 });
await expect(
error.getByText('Template fixture update failed. Your draft is retained for retry.', {
exact: true,
})
).toBeInViewport({ ratio: 1 });
const footer = dialog.locator('.vk-overlay-footer');
const before = await footer.boundingBox();
await dialog.locator('.vk-overlay-scroll').evaluate((el) => {
el.scrollTop = el.scrollHeight;
});
expect(await footer.boundingBox()).toEqual(before);
for (const name of ['Apply Template', 'Cancel', 'Close dialog']) {
await expect(dialog.getByRole('button', { name, exact: true })).toBeEnabled();
await expect(dialog.getByRole('button', { name, exact: true })).toBeInViewport({
ratio: 1,
});
await dialog.getByRole('button', { name, exact: true }).click({ trial: true });
}
}
await expect(error).toBeInViewport({ ratio: 1 });
await expect(
error.getByText('Template fixture update failed. Your draft is retained for retry.', {
exact: true,
})
).toBeInViewport({ ratio: 1 });
await expect(variable).toHaveValue('BUG-42');
await expect(dialog.getByRole('switch', { name: 'Force overwrite' })).toBeChecked();
await expect(submit).toBeEnabled();
await submit.click({ trial: true });
expect(writes).toEqual([
{
method: 'PATCH',
path: `/api/tasks/${task.id}`,
body: { description: 'Investigate BUG-42' },
},
]);
// The query's global failure notification is separate from the retained inline error.
const notificationClose = page.getByRole('button', { name: 'Close notification' });
if (await notificationClose.isVisible()) await notificationClose.click();
await page.screenshot({ path: test.info().outputPath('template-failure.png') });
await page.keyboard.press('Escape');
await expect(dialog).toHaveCount(0);
await expect(opener).toBeFocused();
} finally {
release();
await cleanupRoutes(page).catch(() => {});
await deleteTask(page, String(task.id)).catch(() => {});
}
});
}
}

View file

@ -160,8 +160,8 @@ describe('task detail agent, template, and metrics Mantine migration', () => {
data: { agent: 'codex', model: 'sonnet', reason: 'Best configured agent' },
});
mocks.useTemplates.mockReturnValue({ data: [template] });
mocks.updateTaskMutateAsync.mockResolvedValue({});
mocks.applyTemplateActivity.mockResolvedValue({});
mocks.updateTaskMutateAsync.mockReset().mockResolvedValue({});
mocks.applyTemplateActivity.mockReset().mockResolvedValue({});
mocks.useTaskMetrics.mockReturnValue({
data: {
totalRuns: 2,
@ -705,6 +705,100 @@ describe('task detail agent, template, and metrics Mantine migration', () => {
expect(onApplied).toHaveBeenCalled();
});
it('retains template inputs and dismissal ownership through update failure and retry', async () => {
let rejectUpdate!: (error: Error) => void;
mocks.updateTaskMutateAsync.mockImplementationOnce(
() =>
new Promise((_resolve, reject) => {
rejectUpdate = reject;
})
);
const user = userEvent.setup();
const onOpenChange = vi.fn();
const onApplied = vi.fn();
renderWithProviders(
<ApplyTemplateDialog
task={createMockTask({ id: 'task-template-pending', description: '', project: 'veritas' })}
open
onOpenChange={onOpenChange}
onApplied={onApplied}
/>
);
await user.click(screen.getByRole('combobox', { name: 'Template' }));
await user.click(await screen.findByRole('option', { name: /Bug Fix - Resolve defect/ }));
const variable = screen.getByLabelText('bugId') as HTMLInputElement;
fireEvent.change(variable, { target: { value: 'BUG-42' } });
const submit = screen.getByRole('button', { name: 'Apply Template' });
fireEvent.click(submit);
fireEvent.click(submit);
fireEvent.keyDown(document.body, { key: 'Escape' });
fireEvent.click(screen.getByRole('button', { name: 'Close dialog' }));
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(onOpenChange).not.toHaveBeenCalled();
expect(mocks.updateTaskMutateAsync).toHaveBeenCalledOnce();
expect(variable.disabled).toBe(true);
expect((screen.getByRole('combobox', { name: 'Template' }) as HTMLInputElement).disabled).toBe(
true
);
expect(
(screen.getByRole('switch', { name: 'Force overwrite' }) as HTMLInputElement).disabled
).toBe(true);
await act(async () => rejectUpdate(new Error('Template fixture update failed')));
expect(screen.getByRole('alert').textContent).toContain('Template fixture update failed');
expect(document.activeElement).toBe(screen.getByRole('alert'));
expect(variable.value).toBe('BUG-42');
expect(variable.disabled).toBe(false);
expect(onApplied).not.toHaveBeenCalled();
expect(mocks.applyTemplateActivity).not.toHaveBeenCalled();
await act(async () => fireEvent.click(submit));
expect(mocks.updateTaskMutateAsync).toHaveBeenCalledTimes(2);
expect(mocks.updateTaskMutateAsync.mock.calls[1][0].input.description).toBe(
'Fix BUG-42 for veritas'
);
expect(onOpenChange).toHaveBeenCalledExactlyOnceWith(false);
expect(onApplied).toHaveBeenCalledOnce();
});
it('keeps template application locked through non-fatal activity logging', async () => {
let rejectActivity!: (error: Error) => void;
mocks.applyTemplateActivity.mockImplementationOnce(
() =>
new Promise((_resolve, reject) => {
rejectActivity = reject;
})
);
const log = vi.spyOn(console, 'error').mockImplementation(() => {});
const user = userEvent.setup();
const onOpenChange = vi.fn();
const onApplied = vi.fn();
try {
renderWithProviders(
<ApplyTemplateDialog
task={createMockTask({ description: '', project: 'veritas' })}
open
onOpenChange={onOpenChange}
onApplied={onApplied}
/>
);
await user.click(screen.getByRole('combobox', { name: 'Template' }));
await user.click(await screen.findByRole('option', { name: /Bug Fix - Resolve defect/ }));
const submit = screen.getByRole('button', { name: 'Apply Template' });
await act(async () => fireEvent.click(submit));
expect(mocks.applyTemplateActivity).toHaveBeenCalledOnce();
fireEvent.click(submit);
fireEvent.keyDown(document.body, { key: 'Escape' });
fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
expect(mocks.updateTaskMutateAsync).toHaveBeenCalledOnce();
expect(onOpenChange).not.toHaveBeenCalled();
await act(async () => rejectActivity(new Error('Activity fixture failed')));
expect(onOpenChange).toHaveBeenCalledExactlyOnceWith(false);
expect(onApplied).toHaveBeenCalledOnce();
expect(screen.queryByRole('alert')).toBeNull();
} finally {
log.mockRestore();
}
});
it('renders task metrics and export controls through direct Mantine primitives', async () => {
const user = userEvent.setup();
const task = createMockTask({

View file

@ -1,4 +1,4 @@
import { useState, useMemo } from 'react';
import { useState, useMemo, useRef, useEffect } from 'react';
import { UiModal as Modal, OverlayFooter } from '@/components/ui/UiOverlay';
import { UiAction } from '@/components/ui/UiVocabulary';
import {
@ -69,6 +69,24 @@ export function ApplyTemplateDialog({
const [requiredCustomVars, setRequiredCustomVars] = useState<string[]>([]);
const [forceOverwrite, setForceOverwrite] = useState(false);
const [showHelp, setShowHelp] = useState(false);
const [isApplying, setIsApplying] = useState(false);
const [applyError, setApplyError] = useState<string | null>(null);
const applyInFlight = useRef(false);
const errorRef = useRef<HTMLDivElement>(null);
useEffect(() => {
if (applyError) {
errorRef.current?.focus({ preventScroll: true });
errorRef.current?.scrollIntoView({ block: 'center', behavior: 'instant' });
}
}, [applyError]);
const handleClose = () => {
if (!applyInFlight.current) {
setApplyError(null);
onOpenChange(false);
}
};
const { data: templates } = useTemplates();
const updateTask = useUpdateTask();
@ -217,97 +235,116 @@ export function ApplyTemplateDialog({
// Apply the template
const handleApply = async () => {
if (!template) return;
if (!template || applyInFlight.current) return;
applyInFlight.current = true;
setIsApplying(true);
setApplyError(null);
// Build variable context
const context: VariableContext = {
project: task.project,
author: 'User',
customVars,
};
// Build update input based on merge strategy
const updates: Record<string, unknown> = {};
// Description
if (template.taskDefaults.descriptionTemplate) {
const interpolated = interpolateVariables(template.taskDefaults.descriptionTemplate, context);
if (forceOverwrite || !task.description) {
updates.description = interpolated;
}
}
// Type
if (template.taskDefaults.type && (forceOverwrite || !task.type)) {
updates.type = template.taskDefaults.type;
}
// Priority
if (template.taskDefaults.priority && (forceOverwrite || !task.priority)) {
updates.priority = template.taskDefaults.priority;
}
// Project
if (template.taskDefaults.project && (forceOverwrite || !task.project)) {
updates.project = template.taskDefaults.project;
}
// Subtasks - APPEND to existing
if (template.subtaskTemplates && template.subtaskTemplates.length > 0) {
const now = new Date().toISOString();
const newSubtasks: Subtask[] = template.subtaskTemplates
.sort((a, b) => a.order - b.order)
.map((st) => ({
id: nanoid(),
title: interpolateVariables(st.title, context),
completed: false,
created: now,
...(st.acceptanceCriteria?.length && {
acceptanceCriteria: st.acceptanceCriteria.map((criterion) =>
interpolateVariables(criterion, context)
),
criteriaChecked: new Array(st.acceptanceCriteria.length).fill(false),
}),
}));
// Append to existing subtasks
const existingSubtasks = task.subtasks || [];
updates.subtasks = [...existingSubtasks, ...newSubtasks];
}
// Apply the updates
await updateTask.mutateAsync({
id: task.id,
input: updates,
});
// Track which fields were changed for activity logging
const changedFields = Object.keys(updates);
// Log activity
try {
await api.tasks.applyTemplate(task.id, template.id, template.name, changedFields);
} catch (error) {
// Intentionally non-fatal: don't fail the whole operation if activity logging fails
console.error('Failed to log template application:', error);
// Build variable context
const context: VariableContext = {
project: task.project,
author: 'User',
customVars,
};
// Build update input based on merge strategy
const updates: Record<string, unknown> = {};
// Description
if (template.taskDefaults.descriptionTemplate) {
const interpolated = interpolateVariables(
template.taskDefaults.descriptionTemplate,
context
);
if (forceOverwrite || !task.description) {
updates.description = interpolated;
}
}
// Type
if (template.taskDefaults.type && (forceOverwrite || !task.type)) {
updates.type = template.taskDefaults.type;
}
// Priority
if (template.taskDefaults.priority && (forceOverwrite || !task.priority)) {
updates.priority = template.taskDefaults.priority;
}
// Project
if (template.taskDefaults.project && (forceOverwrite || !task.project)) {
updates.project = template.taskDefaults.project;
}
// Subtasks - APPEND to existing
if (template.subtaskTemplates && template.subtaskTemplates.length > 0) {
const now = new Date().toISOString();
const newSubtasks: Subtask[] = [...template.subtaskTemplates]
.sort((a, b) => a.order - b.order)
.map((st) => ({
id: nanoid(),
title: interpolateVariables(st.title, context),
completed: false,
created: now,
...(st.acceptanceCriteria?.length && {
acceptanceCriteria: st.acceptanceCriteria.map((criterion) =>
interpolateVariables(criterion, context)
),
criteriaChecked: new Array(st.acceptanceCriteria.length).fill(false),
}),
}));
// Append to existing subtasks
const existingSubtasks = task.subtasks || [];
updates.subtasks = [...existingSubtasks, ...newSubtasks];
}
// Apply the updates
try {
await updateTask.mutateAsync({
id: task.id,
input: updates,
});
} catch (error) {
setApplyError(error instanceof Error ? error.message : 'Unable to apply template.');
return;
}
// Track which fields were changed for activity logging
const changedFields = Object.keys(updates);
// Log activity
try {
await api.tasks.applyTemplate(task.id, template.id, template.name, changedFields);
} catch (error) {
// Intentionally non-fatal: don't fail the whole operation if activity logging fails
console.error('Failed to log template application:', error);
}
// Close dialog and notify parent
onOpenChange(false);
onApplied?.();
// Reset state
setSelectedTemplate(null);
setCustomVars({});
setRequiredCustomVars([]);
setForceOverwrite(false);
} finally {
applyInFlight.current = false;
setIsApplying(false);
}
// Close dialog and notify parent
onOpenChange(false);
onApplied?.();
// Reset state
setSelectedTemplate(null);
setCustomVars({});
setRequiredCustomVars([]);
setForceOverwrite(false);
};
return (
<Modal
compound
opened={open}
onClose={() => onOpenChange(false)}
onClose={handleClose}
closeOnEscape={!isApplying}
closeOnClickOutside={!isApplying}
closeButtonProps={{ disabled: isApplying }}
title={
<Group gap="xs">
<FileCode className="h-5 w-5" />
@ -315,12 +352,13 @@ export function ApplyTemplateDialog({
</Group>
}
>
<Stack gap="md" className="vk-overlay-scroll">
<Stack gap="md" className="vk-overlay-scroll [&>*]:shrink-0">
<Group justify="flex-end">
<Button
variant="subtle"
size="xs"
color="gray"
disabled={isApplying}
onClick={() => setShowHelp(!showHelp)}
leftSection={<HelpCircle className="h-4 w-4" />}
rightSection={
@ -369,19 +407,22 @@ export function ApplyTemplateDialog({
</Text>
<Tabs value={categoryFilter} onChange={(value) => setCategoryFilter(value ?? 'all')}>
<Tabs.List grow>
<Tabs.Tab value="all">All</Tabs.Tab>
<Tabs.Tab value="bug" aria-label="Bug templates">
<Tabs.Tab value="all" disabled={isApplying}>
All
</Tabs.Tab>
<Tabs.Tab value="bug" aria-label="Bug templates" disabled={isApplying}>
Bug
</Tabs.Tab>
<Tabs.Tab value="feature" aria-label="Feature templates">
<Tabs.Tab value="feature" aria-label="Feature templates" disabled={isApplying}>
Feature
</Tabs.Tab>
<Tabs.Tab value="sprint" aria-label="Sprint templates">
<Tabs.Tab value="sprint" aria-label="Sprint templates" disabled={isApplying}>
Sprint
</Tabs.Tab>
</Tabs.List>
</Tabs>
<Select
disabled={isApplying}
value={selectedTemplate || 'none'}
onChange={(value) => {
if (!value || value === 'none') {
@ -410,13 +451,15 @@ export function ApplyTemplateDialog({
</Group>
{requiredCustomVars.map((varName) => (
<TextInput
disabled={isApplying}
key={varName}
id={`var-${varName}`}
label={varName}
value={customVars[varName] || ''}
onChange={(e) =>
setCustomVars((prev) => ({ ...prev, [varName]: e.currentTarget.value }))
}
onChange={(e) => {
const value = e.currentTarget.value;
setCustomVars((prev) => ({ ...prev, [varName]: value }));
}}
placeholder={`Enter ${varName}...`}
size="xs"
/>
@ -437,6 +480,7 @@ export function ApplyTemplateDialog({
</Text>
</Stack>
<Switch
disabled={isApplying}
checked={forceOverwrite}
onChange={(event) => setForceOverwrite(event.currentTarget.checked)}
aria-label="Force overwrite"
@ -501,13 +545,23 @@ export function ApplyTemplateDialog({
</Text>
)}
</Stack>
{applyError && (
<Alert ref={errorRef} tabIndex={-1} color="red" title="Template not applied">
{applyError}
</Alert>
)}
</Stack>
<OverlayFooter>
<UiAction type="button" variant="quiet" onClick={() => onOpenChange(false)}>
<UiAction type="button" variant="quiet" onClick={handleClose} disabled={isApplying}>
Cancel
</UiAction>
<UiAction type="button" onClick={handleApply} disabled={!template || updateTask.isPending}>
{updateTask.isPending ? 'Applying...' : 'Apply Template'}
<UiAction
type="button"
onClick={() => void handleApply()}
disabled={!template || isApplying}
loading={isApplying}
>
Apply Template
</UiAction>
</OverlayFooter>
</Modal>