From 4a7e36bd6f1029c52eff7c92bd9a3c2f0a5e599f Mon Sep 17 00:00:00 2001 From: Benjamin Scharf Date: Fri, 27 Sep 2024 16:40:32 +0200 Subject: [PATCH] Feature: Desktop View - Add an explicit discard article button to the article reply form --- .../__tests__/ticket-detail-view-edit.spec.ts | 276 ++++++++++++++++++ .../TicketDetailView/ArticleReply.vue | 12 +- .../composables/useTicketArticleReply.ts | 2 +- .../pages/ticket/views/TicketCreate.vue | 6 +- .../pages/ticket/views/TicketDetailView.vue | 20 ++ i18n/zammad.pot | 26 +- 6 files changed, 325 insertions(+), 17 deletions(-) diff --git a/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view-edit.spec.ts b/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view-edit.spec.ts index c0785f52659f..28b40028d08c 100644 --- a/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view-edit.spec.ts +++ b/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view-edit.spec.ts @@ -674,5 +674,281 @@ describe('Ticket detail view', () => { view.queryByRole('textbox', { name: 'Text' }), ).not.toBeInTheDocument() }) + + it('discards reply form and it keeps the ticket attribute fields state', async () => { + mockTicketQuery({ + ticket: createDummyTicket({ + articleType: 'phone', + defaultPolicy: { + update: true, + agentReadAccess: true, + }, + }), + }) + + mockTicketArticlesQuery({ + articles: { + totalCount: 1, + edges: [ + { + node: createDummyArticle({ + articleType: 'phone', + internal: false, + }), + }, + ], + }, + }) + + mockFormUpdaterQuery({ + formUpdater: { + fields: { + group_id: { + options: [ + { + value: 1, + label: 'Users', + }, + { + value: 2, + label: 'test group', + }, + ], + }, + owner_id: { + options: [ + { + value: 3, + label: 'Test Admin Agent', + }, + ], + }, + state_id: { + options: [ + { + value: 4, + label: 'closed', + }, + { + value: 2, + label: 'open', + }, + { + value: 6, + label: 'pending close', + }, + { + value: 3, + label: 'pending reminder', + }, + ], + }, + pending_time: { + show: false, + }, + priority_id: { + options: [ + { + value: 1, + label: '1 low', + }, + { + value: 2, + label: '2 normal', + }, + { + value: 3, + label: '3 high', + }, + ], + }, + }, + flags: { + newArticlePresent: false, + }, + }, + }) + + const view = await visitView('/tickets/1') + + // Discard changes inside the reply form + await view.events.click( + view.getByRole('button', { name: 'Add phone call' }), + ) + + expect( + await view.findByRole('heading', { level: 2, name: 'Reply' }), + ).toBeInTheDocument() + + // Sets dirty set for a ticket attribute + await view.events.click(view.getByLabelText('State')) + await view.events.click( + await view.findByRole('option', { name: 'closed' }), + ) + + await view.events.click( + view.getByRole('button', { name: 'Discard unsaved reply' }), + ) + + expect( + await view.findByRole('dialog', { name: 'Unsaved Changes' }), + ).toBeInTheDocument() + + await view.events.click( + view.getByRole('button', { name: 'Discard Changes' }), + ) + + // Verify that ticket attributes state is not lost + expect(view.getByLabelText('State')).toHaveTextContent('closed') + }) + + // TODO: Currently we have a problem in our resetForm-Function but also Formkit has an bug inside the own reset handling + // (null / false will currently ignored when setting back the initial value). + // So we will improve our own reset function and create an issue on FormKit side to fix this. + it.skip('discards complete form with an reply and afterwards only the reply directly', async () => { + mockTicketQuery({ + ticket: createDummyTicket({ + group: { + id: convertToGraphQLId('Group', 1), + emailAddress: { + name: 'Zammad Helpdesk', + emailAddress: 'zammad@localhost', + }, + }, + defaultPolicy: { + update: true, + agentReadAccess: true, + }, + }), + }) + + mockTicketArticlesQuery({ + articles: { + totalCount: 1, + edges: [ + { + node: createDummyArticle({ + articleType: 'phone', + internal: false, + }), + }, + ], + }, + }) + + mockFormUpdaterQuery({ + formUpdater: { + fields: { + group_id: { + options: [ + { + value: 1, + label: 'Users', + }, + { + value: 2, + label: 'test group', + }, + ], + }, + owner_id: { + options: [ + { + value: 3, + label: 'Test Admin Agent', + }, + ], + }, + state_id: { + options: [ + { + value: 4, + label: 'closed', + }, + { + value: 2, + label: 'open', + }, + { + value: 6, + label: 'pending close', + }, + { + value: 3, + label: 'pending reminder', + }, + ], + }, + pending_time: { + show: false, + }, + priority_id: { + options: [ + { + value: 1, + label: '1 low', + }, + { + value: 2, + label: '2 normal', + }, + { + value: 3, + label: '3 high', + }, + ], + }, + }, + flags: { + newArticlePresent: false, + }, + }, + }) + + const view = await visitView('/tickets/1') + + // Discard changes inside the reply form + await view.events.click(view.getByRole('button', { name: 'Add reply' })) + + await view.events.click( + await view.findByRole('button', { + name: 'Discard your unsaved changes', + }), + ) + + expect( + await view.findByRole('dialog', { name: 'Unsaved Changes' }), + ).toBeInTheDocument() + + await view.events.click( + view.getByRole('button', { name: 'Discard Changes' }), + ) + + expect( + view.queryByRole('button', { + name: 'Discard your unsaved changes', + }), + ).not.toBeInTheDocument() + + await view.events.click(view.getByRole('button', { name: 'Add reply' })) + + await view.events.click( + view.getByRole('button', { name: 'Discard unsaved reply' }), + ) + + expect( + await view.findByRole('dialog', { name: 'Unsaved Changes' }), + ).toBeInTheDocument() + + await view.events.click( + view.getByRole('button', { name: 'Discard Changes' }), + ) + + expect( + view.queryByRole('button', { + name: 'Discard your unsaved changes', + }), + ).not.toBeInTheDocument() + }) }) }) diff --git a/app/frontend/apps/desktop/pages/ticket/components/TicketDetailView/ArticleReply.vue b/app/frontend/apps/desktop/pages/ticket/components/TicketDetailView/ArticleReply.vue index eac765e94543..50268dbd9570 100644 --- a/app/frontend/apps/desktop/pages/ticket/components/TicketDetailView/ArticleReply.vue +++ b/app/frontend/apps/desktop/pages/ticket/components/TicketDetailView/ArticleReply.vue @@ -31,6 +31,7 @@ const emit = defineEmits<{ articleType: string, performReply: AppSpecificTicketArticleType['performReply'], ] + 'discard-form': [] }>() const currentTicketArticleType = computed(() => { @@ -218,7 +219,7 @@ defineExpose({ }" >
{{ $t('Reply') }} + , ) => { const localNewTicketArticlePresent = ref() - // TODO: swichting tabs when you added a new article is shortly showing the buttons (because taskbar tab don't has the information yet?) + // TODO: switching tabs when you added a new article is shortly showing the buttons (because taskbar tab don't has the information yet?) const newTicketArticlePresent = computed({ get: () => { if (localNewTicketArticlePresent.value !== undefined) diff --git a/app/frontend/apps/desktop/pages/ticket/views/TicketCreate.vue b/app/frontend/apps/desktop/pages/ticket/views/TicketCreate.vue index 7a942ded47bd..ba52bd2cd978 100644 --- a/app/frontend/apps/desktop/pages/ticket/views/TicketCreate.vue +++ b/app/frontend/apps/desktop/pages/ticket/views/TicketCreate.vue @@ -47,9 +47,6 @@ interface Props { tabId?: string } -// - handover context to useTaskbarTab composable -// - Default output for TicketCreate-TabEntity without a "entity/state" - defineOptions({ beforeRouteEnter(to) { const { ticketCreateEnabled, checkUniqueTicketCreateRoute } = @@ -324,6 +321,9 @@ const discardChanges = async () => { } const applyTemplate = (templateId: string) => { + // Skip subscription for the current tab, to avoid not needed form updater requests. + setSkipNextStateUpdate(true) + triggerFormUpdater({ includeDirtyFields: true, additionalParams: { diff --git a/app/frontend/apps/desktop/pages/ticket/views/TicketDetailView.vue b/app/frontend/apps/desktop/pages/ticket/views/TicketDetailView.vue index f10409dcf93b..f27bc618dbf3 100644 --- a/app/frontend/apps/desktop/pages/ticket/views/TicketDetailView.vue +++ b/app/frontend/apps/desktop/pages/ticket/views/TicketDetailView.vue @@ -257,6 +257,10 @@ const discardChanges = async () => { newTicketArticlePresent.value = false await nextTick() + + // Skip subscription for the current tab, to avoid not needed form updater requests. + setSkipNextStateUpdate(true) + formReset() } } @@ -490,6 +494,21 @@ const submitEditTicket = async ( }) } +const discardReplyForm = async () => { + const confirm = await waitForVariantConfirmation('unsaved') + + if (!confirm) return + + newTicketArticlePresent.value = false + + await nextTick() + + // Skip subscription for the current tab, to avoid not needed form updater requests. + setSkipNextStateUpdate(true) + + return triggerFormUpdater() +} + const handleShowArticleForm = ( articleType: string, performReply: AppSpecificTicketArticleType['performReply'], @@ -552,6 +571,7 @@ watch(ticketId, () => { :has-internal-article="hasInternalArticle" :parent-reached-bottom-scroll="reachedBottom" @show-article-form="handleShowArticleForm" + @discard-form="discardReplyForm" />