Merge pull request 'KAN-12: Simplify links to action and ticket' (#175) from feature/KAN-12-simplify-link-actions into main

Reviewed-on: #175

resources/js/components/tickets/detail/TicketDetailRelationships.vue

@@ -12,8 +12,12 @@ import { Button } from '@/components/ui/button';
 import { Card, CardContent, CardHeader } from '@/components/ui/card';
 import { useFormProtection } from '@/composables/useFormProtection';
 import { useLocalization } from '@/composables/useLocalization';
+import { useTicketLinkActions } from '@/composables/useTicketLinkActions';
 import { show as showTicket } from '@/routes/projects/tickets';
-import type { TicketDetailRelationshipsProps } from '@/types/ticket-detail';
+import type {
+    TicketDetailRelationshipsProps,
+    TicketLinkFormData,
+} from '@/types/ticket-detail';
 
 const props = defineProps<TicketDetailRelationshipsProps>();
 defineEmits<{ editParent: [] }>();
@@ -33,7 +37,7 @@ const canAddLink = computed(
         props.ticketLinkTypes.length > 0 &&
         props.ticketLinkOptions.length > 0,
 );
-const form = useForm({
+const form = useForm<TicketLinkFormData>({
     ticket_link_type_id: props.ticketLinkTypes[0]?.id ?? '',
     link_direction: 'source',
     linked_ticket_id: '',
@@ -41,10 +45,9 @@ const form = useForm({
 const protection = useFormProtection(form, {
     isDirty: () => props.canEditTicket && form.isDirty,
 });
-const selectedType = computed(() =>
+const { actions, selectedAction, selectedActionLabel } = useTicketLinkActions(
-    props.ticketLinkTypes.find(
+    () => props.ticketLinkTypes,
-        (type) => String(type.id) === String(form.ticket_link_type_id),
+    form,
-    ),
 );
 const selectedTicket = computed(() =>
     props.ticketLinkOptions.find(
@@ -221,40 +224,18 @@ function submit(): void {
                     />
                     <fieldset
                         :disabled="form.processing"
-                        class="grid min-w-0 gap-4 sm:grid-cols-3"
+                        class="grid min-w-0 gap-4 sm:grid-cols-[minmax(0,1fr)_minmax(0,2fr)]"
                     >
                         <TicketFormField
                             id="ticket_link_type_id"
-                            v-model="form.ticket_link_type_id"
+                            v-model="selectedAction"
                             kind="select"
-                            :label="t('ticketDetail.linkType')"
+                            :label="t('ticketDetail.linkAction')"
-                            :options="
+                            :options="actions"
-                                ticketLinkTypes.map((type) => ({
+                            :error="
-                                    value: type.id,
+                                form.errors.ticket_link_type_id ||
-                                    label: `${type.label} / ${type.counterpart_label}`,
+                                form.errors.link_direction
-                                }))
                             "
-                            :error="form.errors.ticket_link_type_id"
-                            :disabled="form.processing"
-                            required
-                        />
-                        <TicketFormField
-                            id="link_direction"
-                            v-model="form.link_direction"
-                            kind="select"
-                            :label="t('ticketRelationships.direction')"
-                            :options="[
-                                {
-                                    value: 'source',
-                                    label: selectedType?.label ?? '',
-                                },
-                                {
-                                    value: 'counterpart',
-                                    label:
-                                        selectedType?.counterpart_label ?? '',
-                                },
-                            ]"
-                            :error="form.errors.link_direction"
                             :disabled="form.processing"
                             required
                         />
@@ -281,26 +262,19 @@ function submit(): void {
                         />
                     </fieldset>
                     <p
-                        v-if="selectedType && selectedTicket"
+                        v-if="selectedActionLabel && selectedTicket"
                         aria-live="polite"
                         class="text-muted-foreground flex flex-wrap items-center gap-2 text-sm"
                     >
                         <span class="font-mono text-xs">{{
                             ticket.ticket_key
                         }}</span
-                        ><span>{{
+                        ><span>{{ selectedActionLabel }}</span
-                            form.link_direction === 'source'
-                                ? selectedType.label
-                                : selectedType.counterpart_label
-                        }}</span
                         ><ArrowRight class="size-4" aria-hidden="true" /><span
                             class="font-mono text-xs"
                             >{{ selectedTicket.ticket_key }}</span
                         >
                     </p>
-                    <p class="text-muted-foreground text-xs">
-                        {{ t('ticketRelationships.directionHelp') }}
-                    </p>
                     <Button
                         type="submit"
                         :disabled="form.processing || !selectedTicket"

resources/js/components/tickets/detail/TicketRelationshipLink.vue

@@ -1,10 +1,8 @@
 <script setup lang="ts">
 import { Link, useForm } from '@inertiajs/vue3';
-import { ArrowDownLeft, ArrowUpRight, Unlink } from 'lucide-vue-next';
+import { Unlink } from 'lucide-vue-next';
-import { computed } from 'vue';
 import { destroy } from '@/actions/App/Http/Controllers/ProjectTicketLinkController';
 import FormFeedback from '@/components/FormFeedback.vue';
-import { Badge } from '@/components/ui/badge';
 import { Button } from '@/components/ui/button';
 import { useFormProtection } from '@/composables/useFormProtection';
 import { useLocalization } from '@/composables/useLocalization';
@@ -20,7 +18,6 @@ const props = defineProps<{
 }>();
 const emit = defineEmits<{ removed: [] }>();
 const { t } = useLocalization();
-const outgoing = computed(() => props.link.source_ticket_id === props.ticketId);
 const form = useForm({});
 const protection = useFormProtection(form);
 function remove(): void {
@@ -47,27 +44,6 @@ function remove(): void {
         :data-ticket-link="link.id"
     >
         <div class="flex min-w-0 flex-wrap items-center gap-2">
-            <Badge
-                variant="outline"
-                class="text-muted-foreground max-w-full whitespace-normal"
-            >
-                <ArrowUpRight
-                    v-if="outgoing"
-                    class="size-3.5 shrink-0"
-                    aria-hidden="true"
-                /><ArrowDownLeft
-                    v-else
-                    class="size-3.5 shrink-0"
-                    aria-hidden="true"
-                />
-                {{
-                    t(
-                        outgoing
-                            ? 'ticketRelationships.outgoing'
-                            : 'ticketRelationships.incoming',
-                    )
-                }}
-            </Badge>
             <span class="text-muted-foreground font-mono text-xs">{{
                 ticketKey
             }}</span>

resources/js/composables/useTicketLinkActions.ts

@@ -0,0 +1,57 @@
+import { computed, toValue } from 'vue';
+import type { MaybeRefOrGetter } from 'vue';
+import type { TicketLinkTypeModel } from '@/types/models/ticket-link-type-model';
+import type {
+    TicketLinkActionOption,
+    TicketLinkFormData,
+} from '@/types/ticket-detail';
+
+export function useTicketLinkActions(
+    ticketLinkTypes: MaybeRefOrGetter<TicketLinkTypeModel[]>,
+    form: TicketLinkFormData,
+) {
+    const actions = computed<TicketLinkActionOption[]>(() =>
+        toValue(ticketLinkTypes).flatMap((type) => {
+            const choices: TicketLinkActionOption[] = [
+                {
+                    value: `${type.id}:source`,
+                    label: type.label,
+                    ticketLinkTypeId: type.id,
+                    direction: 'source',
+                },
+            ];
+            if (
+                type.counterpart_label &&
+                type.counterpart_label !== type.label
+            ) {
+                choices.push({
+                    value: `${type.id}:counterpart`,
+                    label: type.counterpart_label,
+                    ticketLinkTypeId: type.id,
+                    direction: 'counterpart',
+                });
+            }
+            return choices;
+        }),
+    );
+    const selectedAction = computed({
+        get: () => `${form.ticket_link_type_id}:${form.link_direction}`,
+        set: (value: string | number | null) => {
+            const action = actions.value.find(
+                (option) => option.value === value,
+            );
+            if (action) {
+                form.ticket_link_type_id = action.ticketLinkTypeId;
+                form.link_direction = action.direction;
+            }
+        },
+    });
+    const selectedActionLabel = computed(
+        () =>
+            actions.value.find(
+                (action) => action.value === selectedAction.value,
+            )?.label,
+    );
+
+    return { actions, selectedAction, selectedActionLabel };
+}

resources/js/locales/en.ts

@@ -1213,7 +1213,7 @@ export const en = {
     'ticketDetail.linkedTickets': 'Linked tickets',
     'ticketDetail.linkedTicketsDescription':
         'Track cross-ticket relationships with project-defined link labels.',
-    'ticketDetail.linkType': 'Link type',
+    'ticketDetail.linkAction': 'Action',
     'ticketDetail.linkedTicket': 'Linked ticket',
     'ticketDetail.saveLink': 'Save link',
     'ticketDetail.noLinks':
@@ -1227,11 +1227,6 @@ export const en = {
         'Open a child ticket to change its parent. Expand a group to see its descendants.',
     'ticketRelationships.addLink': 'Add or update a link',
     'ticketRelationships.selectTicket': 'Select a ticket',
-    'ticketRelationships.direction': 'This ticket',
-    'ticketRelationships.directionHelp':
-        'Choose how this ticket relates to the linked ticket. The reverse label appears on the other ticket. Saving an existing pair updates its type and direction.',
-    'ticketRelationships.outgoing': 'Outgoing',
-    'ticketRelationships.incoming': 'Incoming',
     'ticketRelationships.removeLink': 'Remove link to {key}',
     'ticketRelationships.noTypes':
         'No link types are configured for this project.',

resources/js/types/ticket-detail.ts

@@ -67,3 +67,14 @@ export type TicketBranchFormData = {
     branch_name: string;
     repository_connection_id: string;
 };
+export type TicketLinkFormData = {
+    ticket_link_type_id: number | string;
+    link_direction: 'source' | 'counterpart';
+    linked_ticket_id: number | string;
+};
+export type TicketLinkActionOption = {
+    value: string;
+    label: string;
+    ticketLinkTypeId: number;
+    direction: TicketLinkFormData['link_direction'];
+};

tests/Fixtures/ticket-detail.mjs

@@ -5,7 +5,7 @@ import tailwind from '@tailwindcss/vite';
 import vue from '@vitejs/plugin-vue';
 import { createInertiaApp } from '@inertiajs/vue3';
 import { createServer } from 'vite';
-import { createSSRApp, h } from 'vue';
+import { createSSRApp, h, reactive, ref } from 'vue';
 import { renderToString } from 'vue/server-renderer';
 
 const serve = process.argv.includes('--serve');
@@ -463,6 +463,40 @@ if (serve) {
     );
 } else {
     try {
+        const { useTicketLinkActions } = await server.ssrLoadModule(
+            '/resources/js/composables/useTicketLinkActions.ts',
+        );
+        const linkTypes = ref([
+            { id: 1, label: 'Blocks', counterpart_label: 'Blocked by' },
+            { id: 2, label: 'Relates to', counterpart_label: 'Relates to' },
+        ]);
+        const linkForm = reactive({
+            ticket_link_type_id: 1,
+            link_direction: 'source',
+            linked_ticket_id: 41,
+        });
+        const { actions, selectedAction, selectedActionLabel } =
+            useTicketLinkActions(linkTypes, linkForm);
+        assert.deepEqual(
+            actions.value.map((action) => action.label),
+            ['Blocks', 'Blocked by', 'Relates to'],
+        );
+        assert.equal(selectedActionLabel.value, 'Blocks');
+        selectedAction.value = '1:counterpart';
+        assert.equal(selectedActionLabel.value, 'Blocked by');
+        assert.deepEqual(
+            { ...linkForm },
+            {
+                ticket_link_type_id: 1,
+                link_direction: 'counterpart',
+                linked_ticket_id: 41,
+            },
+        );
+        selectedAction.value = '2:source';
+        assert.equal(linkForm.ticket_link_type_id, 2);
+        assert.equal(linkForm.link_direction, 'source');
+        selectedAction.value = 'unknown';
+        assert.equal(selectedActionLabel.value, 'Relates to');
         const { createLocalization } = await server.ssrLoadModule(
             '/resources/js/lib/localization.ts',
         );
@@ -491,8 +525,24 @@ if (serve) {
             return result.body.split('id="main-content"')[1];
         };
         const html = await render();
-        assert.ok(html.includes('id="link_direction"'));
+        assert.ok(!html.includes('id="link_direction"'));
         assert.ok(html.includes('Blocked by'));
+        const linkFields = [
+            ...html.matchAll(/<fieldset\b[^>]*>[\s\S]*?<\/fieldset>/g),
+        ].find(([fieldset]) =>
+            fieldset.includes('id="ticket_link_type_id"'),
+        )?.[0];
+        assert.ok(linkFields);
+        assert.equal((linkFields.match(/<select\b/g) ?? []).length, 2);
+        assert.match(
+            linkFields,
+            /<option value="1:source"[^>]*>Blocks<\/option>/,
+        );
+        assert.match(
+            linkFields,
+            /<option value="1:counterpart"[^>]*>Blocked by<\/option>/,
+        );
+        assert.doesNotMatch(linkFields, /Blocks \/ Blocked by/);
         for (const text of [
             ticket.title,
             'Keep every existing ticket action available.',
@@ -570,9 +620,9 @@ if (serve) {
             /<a(?=[^>]*href="https:\/\/git\.example\.test\/Engineering\/platform\/pulls\/12")(?=[^>]*target="_blank")(?=[^>]*rel="noopener noreferrer")[^>]*>/,
         );
         assert.match(html, /Save link/);
-        assert.match(html, /Outgoing/);
+        assert.doesNotMatch(html, /Outgoing/);
         const incoming = await render('?incoming');
-        assert.match(incoming, /Incoming/);
+        assert.doesNotMatch(incoming, /Incoming/);
         assert.match(incoming, /Blocked by/);
         assert.match(await render('?noTypes'), /No link types are configured/);
         assert.match(

tests/Fixtures/ticket-relationships-browser.mjs

@@ -41,8 +41,6 @@ try {
                 'Platform delivery',
                 'Child tickets',
                 'Nested child with a long title that must wrap on mobile',
-                'Incoming',
-                'Blocked by',
             ]) {
                 assert.equal(
                     await section()
@@ -52,6 +50,13 @@ try {
                     text,
                 );
             }
+            assert.equal(
+                await section()
+                    .locator('[data-ticket-link]')
+                    .getByText('Blocked by', { exact: true })
+                    .isVisible(),
+                true,
+            );
             const collapse = section().getByRole('button', {
                 name: 'Collapse children of ENG-41',
             });
@@ -68,16 +73,7 @@ try {
             );
             await page.keyboard.press('Space');
             assert.equal(await collapse.getAttribute('aria-expanded'), 'true');
-            await control('Link type').focus();
+            await control('Action').focus();
-            await page.keyboard.press('Tab');
-            assert.equal(
-                await control('This ticket').evaluate(
-                    (el) =>
-                        el === document.activeElement &&
-                        el.matches(':focus-visible'),
-                ),
-                true,
-            );
             await page.keyboard.press('Tab');
             assert.equal(
                 await control('Linked ticket').evaluate(
@@ -130,7 +126,10 @@ try {
         link_direction: 'source',
         linked_ticket_id: 41,
     });
-    await section().getByText('Outgoing', { exact: true }).waitFor();
+    await section()
+        .locator('[data-ticket-link]')
+        .getByText('Blocks', { exact: true })
+        .waitFor();
     await page.waitForFunction(
         () => document.querySelector('#linked_ticket_id')?.value === '',
     );
@@ -152,11 +151,10 @@ try {
     );
 
     await visit('?noLinks');
-    await control('This ticket').selectOption('counterpart');
+    await control('Action').selectOption('1:counterpart');
     await control('Linked ticket').selectOption('41');
     await section()
-        .locator('form')
+        .locator('p[aria-live="polite"]')
-        .last()
         .getByText('Blocked by', { exact: true })
         .waitFor();
     request = page.waitForRequest(
@@ -168,7 +166,10 @@ try {
         link_direction: 'counterpart',
         linked_ticket_id: 41,
     });
-    await section().getByText('Incoming', { exact: true }).waitFor();
+    await section()
+        .locator('[data-ticket-link]')
+        .getByText('Blocked by', { exact: true })
+        .waitFor();
 
     await visit('?rejectLink');
     await control('Linked ticket').selectOption('41');
@@ -177,7 +178,7 @@ try {
         .getByText('Choose another ticket.', { exact: true })
         .waitFor();
     assert.equal(await control('Linked ticket').inputValue(), '41');
-    for (const label of ['Link type', 'Linked ticket']) {
+    for (const label of ['Action', 'Linked ticket']) {
         assert.equal(await control(label).getAttribute('aria-invalid'), 'true');
     }
     await page.route('**/tickets/42/links', (route) => route.abort('failed'));