Feature: Desktop view - Refactored ticket sidebar to use teleport ins…

…tead of two sibling component. Co-authored-by: Dominik Klein <[email protected]> Co-authored-by: Florian Liebe <[email protected]>
The-Autists · Aug 27, 2024 · ba056fc · ba056fc
1 parent 127aa9d
commit ba056fc
Show file tree

Hide file tree

Showing 68 changed files with 1,456 additions and 1,260 deletions.
diff --git a/app/frontend/apps/desktop/components/CommonSimpleEntityList/CommonSimpleEntityList.vue b/app/frontend/apps/desktop/components/CommonSimpleEntityList/CommonSimpleEntityList.vue
@@ -3,6 +3,8 @@
 <script setup lang="ts" generic="T">
 import { computed } from 'vue'
 
+import type { ObjectLike } from '#shared/types/utils.ts'
+
 import CommonShowMoreButton from '#desktop/components/CommonShowMoreButton/CommonShowMoreButton.vue'
 import entityModules from '#desktop/components/CommonSimpleEntityList/plugins/index.ts'
 import {
@@ -11,11 +13,7 @@ import {
 } from '#desktop/components/CommonSimpleEntityList/types.ts'
 
 interface Props {
-  /**
-   * Populate entity through `normalizesEdges` function
-   * @type {T[]} -> ReturnType of `normalizesEdges` function
-   * */
-  entity: Entity<T>
+  entity: Entity<ObjectLike>
   type: EntityType
   label?: string
 }
@@ -31,7 +29,7 @@ const entitySetup = computed(() => {
   return {
     component,
     context,
-    data: props.entity.array,
+    array: props.entity.array,
   }
 })
 </script>
@@ -45,23 +43,15 @@ const entitySetup = computed(() => {
       {{ label }}
     </CommonLabel>
 
-    <TransitionGroup
-      v-if="entity.array?.length"
-      tag="ul"
-      name="fade"
-      class="flex flex-col gap-1.5"
-    >
-      <li
-        v-for="(entityValue, index) in entitySetup.data"
-        :key="`entity-${index}`"
-      >
+    <ul v-if="entity.array?.length" class="flex flex-col gap-1.5">
+      <li v-for="item in entitySetup.array" :key="`entity-${item.id}`">
         <component
           :is="entitySetup.component"
-          :entity="entityValue"
+          :entity="item"
           :context="entitySetup.context"
         />
       </li>
-    </TransitionGroup>
+    </ul>
 
     <CommonLabel v-if="!entity.array?.length" class="block"
       >{{ entitySetup.context.emptyMessage }}

diff --git a/app/frontend/apps/desktop/components/CommonSimpleEntityList/types.ts b/app/frontend/apps/desktop/components/CommonSimpleEntityList/types.ts
@@ -1,14 +1,16 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
+import type { ObjectLike } from '#shared/types/utils.ts'
+
 import type { Component } from 'vue'
 
 export enum EntityType {
   User = 'User',
   Organization = 'Organization',
 }
 
-export interface Entity<T> {
-  array: (unknown | T)[]
+export interface Entity<T = ObjectLike> {
+  array: T[]
   totalCount: number
 }
 

diff --git a/app/frontend/apps/desktop/entities/ticket/types.ts b/app/frontend/apps/desktop/entities/ticket/types.ts
@@ -0,0 +1,11 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import type { TicketById } from '#shared/entities/ticket/types.ts'
+
+import type { ComputedRef, Ref } from 'vue'
+
+export interface TicketInformation {
+  ticket: ComputedRef<TicketById | undefined>
+  ticketId: ComputedRef<ID>
+  ticketInternalId: Ref<number>
+}
diff --git a/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view.spec.ts b/app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view.spec.ts
@@ -240,6 +240,7 @@ describe('ticket detail view', () => {
     })
 
     const view = await visitView('/tickets/1')
+    await view.events.click(view.getByLabelText('Checklist'))
 
     expect(
       view.getByRole('status', { name: 'Incomplete checklist items' }),

diff --git a/...esktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleActionList.vue b/...esktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleActionList.vue
@@ -20,7 +20,7 @@ const props = defineProps<{
 }>()
 
 const { canUpdateTicket } = useTicketArticleReply()
-const { ticket } = useTicketInformation()
+const { ticket, ticketInternalId } = useTicketInformation()
 
 const { isTouchDevice } = useTouchDevice()
 
@@ -42,12 +42,18 @@ const popoverActions: MenuItem[] = [
     label: __('Download original email'),
     onClick(arg) {
       const { article } = arg as { article: TicketArticle }
-      const { originalFormattingUrl } = useEmailFileUrls(article)
+      const { originalFormattingUrl } = useEmailFileUrls(
+        article,
+        ticketInternalId,
+      )
       openExternalLink(originalFormattingUrl.value as string)
     },
     show: (arg) => {
       const { article } = arg as { article: TicketArticle }
-      const { originalFormattingUrl } = useEmailFileUrls(article)
+      const { originalFormattingUrl } = useEmailFileUrls(
+        article,
+        ticketInternalId,
+      )
       return !!(article.type?.name === 'email' && originalFormattingUrl.value)
     },
     icon: 'download',
@@ -57,12 +63,12 @@ const popoverActions: MenuItem[] = [
     label: __('Download raw email'),
     onClick(arg) {
       const { article } = arg as { article: TicketArticle }
-      const { rawMessageUrl } = useEmailFileUrls(article)
+      const { rawMessageUrl } = useEmailFileUrls(article, ticketInternalId)
       openExternalLink(rawMessageUrl.value)
     },
     show: (arg) => {
       const { article } = arg as { article: TicketArticle }
-      const { rawMessageUrl } = useEmailFileUrls(article)
+      const { rawMessageUrl } = useEmailFileUrls(article, ticketInternalId)
       return !!(article.type?.name === 'email' && rawMessageUrl.value)
     },
     icon: 'download',

diff --git a/...s/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleBlockedContentWarning.vue b/...s/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleBlockedContentWarning.vue
@@ -5,14 +5,20 @@ import CommonIcon from '#shared/components/CommonIcon/CommonIcon.vue'
 import type { TicketArticle } from '#shared/entities/ticket/types'
 
 import { useEmailFileUrls } from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/useEmailFileUrls.ts'
+import { useTicketInformation } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
 
 interface Props {
   article: TicketArticle
 }
 
 const props = defineProps<Props>()
 
-const { originalFormattingUrl } = useEmailFileUrls(props.article)
+const { ticketInternalId } = useTicketInformation()
+
+const { originalFormattingUrl } = useEmailFileUrls(
+  props.article,
+  ticketInternalId,
+)
 </script>
 
 <template>

diff --git a/...icket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleActionList.spec.ts b/...icket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleActionList.spec.ts
@@ -1,6 +1,7 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-import { computed, provide } from 'vue'
+import { provideLocal } from '@vueuse/shared'
+import { computed, ref } from 'vue'
 
 import { renderComponent } from '#tests/support/components/index.ts'
 
@@ -10,7 +11,7 @@ import { EnumTicketArticleSenderName } from '#shared/graphql/types.ts'
 import { convertToGraphQLId } from '#shared/graphql/utils.ts'
 
 import ArticleBubbleActionList from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleActionList.vue'
-import { TICKET_INFORMATION_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
+import { TICKET_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
 
 const renderArticleBubbleActionList = () =>
   renderComponent(
@@ -37,9 +38,10 @@ const renderArticleBubbleActionList = () =>
 
         const ticket = createDummyTicket()
 
-        provide(TICKET_INFORMATION_KEY, {
+        provideLocal(TICKET_KEY, {
           ticket: computed(() => ticket),
           ticketId: computed(() => ticket.id),
+          ticketInternalId: ref(ticket.internalId),
         })
 
         return { position, article }

diff --git a/...nents/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleBlockedContentWarning.spec.ts b/...nents/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleBlockedContentWarning.spec.ts
@@ -1,13 +1,14 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-import { computed, provide } from 'vue'
+import { provideLocal } from '@vueuse/shared'
+import { computed, ref } from 'vue'
 
 import { renderComponent } from '#tests/support/components/index.ts'
 
 import { createDummyTicket } from '#shared/entities/ticket-article/__tests__/mocks/ticket.ts'
 
 import ArticleBubbleBlockedContentWarning from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleBlockedContentWarning.vue'
-import { TICKET_INFORMATION_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
+import { TICKET_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
 
 describe('ArticleBubbleBlockedContentWarning', () => {
   it('does not show if there is no blocked content', () => {
@@ -18,9 +19,10 @@ describe('ArticleBubbleBlockedContentWarning', () => {
       },
       setup: () => {
         const ticket = createDummyTicket()
-        provide(TICKET_INFORMATION_KEY, {
+        provideLocal(TICKET_KEY, {
           ticket: computed(() => ticket),
           ticketId: computed(() => ticket.id),
+          ticketInternalId: ref(ticket.internalId),
         })
       },
     })
@@ -30,6 +32,7 @@ describe('ArticleBubbleBlockedContentWarning', () => {
     expect(wrapper.queryByText('Original Formatting')).not.toBeInTheDocument()
   })
 
+  // TODO: still skipped?!
   it.skip('shows if there is blocked content', () => {
     const wrapper = renderComponent(ArticleBubbleBlockedContentWarning, {
       router: true,
@@ -42,9 +45,10 @@ describe('ArticleBubbleBlockedContentWarning', () => {
       },
       setup: () => {
         const ticket = createDummyTicket()
-        provide(TICKET_INFORMATION_KEY, {
+        provideLocal(TICKET_KEY, {
           ticket: computed(() => ticket),
           ticketId: computed(() => ticket.id),
+          ticketInternalId: ref(ticket.internalId),
         })
       },
     })

diff --git a/...ages/ticket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleBody.spec.ts b/...ages/ticket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleBody.spec.ts
@@ -1,14 +1,15 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-import { computed, provide } from 'vue'
+import { provideLocal } from '@vueuse/shared'
+import { computed, ref } from 'vue'
 
 import { renderComponent } from '#tests/support/components/index.ts'
 
 import { createDummyArticle } from '#shared/entities/ticket-article/__tests__/mocks/ticket-articles.ts'
 import { createDummyTicket } from '#shared/entities/ticket-article/__tests__/mocks/ticket.ts'
 
 import ArticleBubbleBody from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleBody.vue'
-import { TICKET_INFORMATION_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
+import { TICKET_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
 
 const renderBody = (
   article: ReturnType<typeof createDummyArticle>,
@@ -21,9 +22,10 @@ const renderBody = (
       setup: () => {
         const dummyTicket = createDummyTicket()
 
-        provide(TICKET_INFORMATION_KEY, {
+        provideLocal(TICKET_KEY, {
           ticketId: computed(() => dummyTicket.id),
           ticket: computed(() => dummyTicket),
+          ticketInternalId: ref(dummyTicket.internalId),
         })
         return {
           article,

diff --git a/...es/ticket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleFooter.spec.ts b/...es/ticket/components/TicketDetailView/ArticleBubble/__tests__/ArticleBubbleFooter.spec.ts
@@ -1,6 +1,7 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-import { computed, provide } from 'vue'
+import { provideLocal } from '@vueuse/shared'
+import { computed, ref } from 'vue'
 
 import { renderComponent } from '#tests/support/components/index.ts'
 
@@ -9,7 +10,7 @@ import { createDummyTicket } from '#shared/entities/ticket-article/__tests__/moc
 import { convertToGraphQLId } from '#shared/graphql/utils.ts'
 
 import ArticleBubbleFooter from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/ArticleBubbleFooter.vue'
-import { TICKET_INFORMATION_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
+import { TICKET_KEY } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
 
 describe('ArticleBubbleFooter', () => {
   it('does not display for articles without attachments', () => {
@@ -18,9 +19,10 @@ describe('ArticleBubbleFooter', () => {
       router: true,
       setup: () => {
         const ticket = createDummyTicket()
-        provide(TICKET_INFORMATION_KEY, {
+        provideLocal(TICKET_KEY, {
           ticket: computed(() => ticket),
           ticketId: computed(() => ticket.id),
+          ticketInternalId: ref(ticket.internalId),
         })
         return {
           article: createDummyArticle(),
@@ -39,9 +41,10 @@ describe('ArticleBubbleFooter', () => {
         components: { ArticleBubbleFooter },
         setup: () => {
           const ticket = createDummyTicket()
-          provide(TICKET_INFORMATION_KEY, {
+          provideLocal(TICKET_KEY, {
             ticket: computed(() => ticket),
             ticketId: computed(() => ticket.id),
+            ticketInternalId: ref(ticket.internalId),
           })
           return {
             article: createDummyArticle({

diff --git a/...pages/ticket/components/TicketDetailView/ArticleBubble/__tests__/useEmailFileUrls.spec.ts b/...pages/ticket/components/TicketDetailView/ArticleBubble/__tests__/useEmailFileUrls.spec.ts
@@ -3,17 +3,10 @@
 import { ref } from 'vue'
 
 import { createDummyArticle } from '#shared/entities/ticket-article/__tests__/mocks/ticket-articles.ts'
-import { createDummyTicket } from '#shared/entities/ticket-article/__tests__/mocks/ticket.ts'
 import { convertToGraphQLId } from '#shared/graphql/utils.ts'
 
 import { useEmailFileUrls } from '#desktop/pages/ticket/components/TicketDetailView/ArticleBubble/useEmailFileUrls.ts'
 
-vi.mock('#desktop/pages/ticket/composables/useTicketInformation.ts', () => ({
-  useTicketInformation: () => ({
-    ticket: ref(createDummyTicket()),
-  }),
-}))
-
 describe('useEmailFileUrls', () => {
   it('should return originalFormattingUrl and rawMessageUrl', () => {
     const { originalFormattingUrl, rawMessageUrl } = useEmailFileUrls(
@@ -31,10 +24,11 @@ describe('useEmailFileUrls', () => {
           ],
         }),
       ),
+      ref(222),
     )
 
     expect(originalFormattingUrl.value).toBe(
-      '/ticket_attachment/1/1/123?disposition=attachment',
+      '/ticket_attachment/222/1/123?disposition=attachment',
     )
     expect(rawMessageUrl.value).toBe('/api/v1/ticket_article_plain/1')
   })

diff --git a/...d/apps/desktop/pages/ticket/components/TicketDetailView/ArticleBubble/useEmailFileUrls.ts b/...d/apps/desktop/pages/ticket/components/TicketDetailView/ArticleBubble/useEmailFileUrls.ts
@@ -1,15 +1,16 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
 import { find } from 'lodash-es'
-import { computed, type MaybeRef, toValue } from 'vue'
+import { computed, type Ref, type MaybeRef, toValue } from 'vue'
 
 import type { TicketArticle } from '#shared/entities/ticket/types'
 
-import { useTicketInformation } from '#desktop/pages/ticket/composables/useTicketInformation.ts'
-
-export const useEmailFileUrls = (ticketArticle: MaybeRef<TicketArticle>) => {
+// TODO MaybeRef needed? Check...
+export const useEmailFileUrls = (
+  ticketArticle: MaybeRef<TicketArticle>,
+  ticketInternalId: Ref<number>,
+) => {
   const article = computed(() => toValue(ticketArticle))
-  const ticketInformation = useTicketInformation()
 
   const originalFormattingUrl = computed(() => {
     const originalFormattingFile = find(
@@ -21,7 +22,7 @@ export const useEmailFileUrls = (ticketArticle: MaybeRef<TicketArticle>) => {
 
     if (!originalFormattingFile) return
 
-    return `/ticket_attachment/${ticketInformation?.ticket?.value?.internalId}/${article.value.internalId}/${originalFormattingFile.internalId}?disposition=attachment`
+    return `/ticket_attachment/${ticketInternalId.value}/${article.value.internalId}/${originalFormattingFile.internalId}?disposition=attachment`
   })
 
   const rawMessageUrl = computed(

diff --git a/...esktop/pages/ticket/components/TicketDetailView/TicketDetailTopBar/TicketDetailTopBar.vue b/...esktop/pages/ticket/components/TicketDetailView/TicketDetailTopBar/TicketDetailTopBar.vue
@@ -90,9 +90,9 @@ const { hasChannelAlert, channelAlert } = useTicketChannel(ticket)
         />
       </template>
     </CommonBreadcrumb>
-
+    <!-- TODO: we should have some computed for this policy thing or maybe we have already something? -->
     <HighlightMenu
-      v-if="ticket?.policy.update"
+      v-if="ticket?.policy?.update"
       class="justify-self-end"
       :style="{ gridTemplate: 'actions' }"
     />
-Original file line number
+Diff line change
@@ Expand Up / @@ -240,6 +240,7 @@ describe('ticket detail view', () => { @@
         })
         const view = await visitView('/tickets/1')
+        await view.events.click(view.getByLabelText('Checklist'))
         expect(
           view.getByRole('status', { name: 'Incomplete checklist items' }),
@@ Expand Down @@