Browse Source

Feature: Desktop view - Right sidebars for organization information and shared drafts.

Fixes #5244 - Shared drafts are not applied in a consistent manner in the ticket create screen.

Co-authored-by: Benjamin Scharf <bs@zammad.com>
Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Dusan Vuckovic <dv@zammad.com>
Co-authored-by: Florian Liebe <fl@zammad.com>
Co-authored-by: Mantas Masalskis <mm@zammad.com>
Benjamin Scharf 8 months ago
parent
commit
ca72359bcd

+ 29 - 22
app/assets/javascripts/app/controllers/agent_ticket_create.coffee

@@ -395,7 +395,7 @@ class App.TicketCreate extends App.Controller
       if !_.isEmpty(params['form_id'])
         @formId = params['form_id']
 
-    # Get template params
+    # Get template (and shared draft) params
     if template && !_.isEmpty(template.options)
       templateTags = null
 
@@ -433,10 +433,14 @@ class App.TicketCreate extends App.Controller
           )
         ),
 
-        # Pick only values for "non-dirty" fields.
-        #   Skip tags, as these will get processed later.
+        # In case of templates, pick only values for "non-dirty" fields.
+        #   In case of shared drafts, pick them all, since they are supposed to overwrite the existing form.
+        #   Skip tags in case of templates, as these support complex value format that will get processed later.
         # https://github.com/zammad/zammad/issues/2434
+        # https://github.com/zammad/zammad/issues/5244
         (value, field) =>
+          return true if template.shared_draft_id
+
           if field is 'tags'
             templateTags = value
             return
@@ -447,25 +451,28 @@ class App.TicketCreate extends App.Controller
         )
       )
 
-      # Process template tags, but only if they are "dirty".
-      if @dirty?.tags and params.tags
-        switch templateTags?['operator']
-
-          # Remove tags included in the template from the existing tags.
-          when 'remove'
-            params.tags = _.difference(params.tags.split(', '), templateTags?['value']?.split(', ')).join(', ')
-
-          # Add tags included in the template by merging them with existing tags.
-          #   Do this also if the operator is missing from the template configuration (default behavior).
-          else
-            params.tags = _.uniq(_.union(params.tags.split(', '), templateTags?['value']?.split(', '))).join(', ')
-
-      # Otherwise, simply replace the tags with the value from the template.
-      #   Do not do this if they are supposed to be removed only.
-      #   This allows switching between different templates without accumulating their tags.
-      #   Note that template might not contain tags, in which case the field will be reset.
-      else if templateTags?['operator'] != 'remove'
-        params.tags = templateTags?['value']
+      # Handle template tags only, since shared drafts support only the simple value format.
+      if not template.shared_draft_id
+
+        # Process template tags, but only if they are "dirty".
+        if @dirty?.tags and params.tags
+          switch templateTags?['operator']
+
+            # Remove tags included in the template from the existing tags.
+            when 'remove'
+              params.tags = _.difference(params.tags.split(', '), templateTags?['value']?.split(', ')).join(', ')
+
+            # Add tags included in the template by merging them with existing tags.
+            #   Do this also if the operator is missing from the template configuration (default behavior).
+            else
+              params.tags = _.uniq(_.union(params.tags.split(', '), templateTags?['value']?.split(', '))).join(', ')
+
+        # Otherwise, simply replace the tags with the value from the template.
+        #   Do not do this if they are supposed to be removed only.
+        #   This allows switching between different templates without accumulating their tags.
+        #   Note that template might not contain tags, in which case the field will be reset.
+        else if templateTags?['operator'] != 'remove'
+          params.tags = templateTags?['value']
 
     if !_.isEmpty(params)
       # only use form meta once so it will not get used on templates

+ 11 - 16
app/controllers/tickets_shared_draft_starts_controller.rb

@@ -23,8 +23,9 @@ class TicketsSharedDraftStartsController < ApplicationController
   end
 
   def create
-    object = scope.create! safe_params
-    object.attach_upload_cache params[:form_id]
+    object = Service::Ticket::SharedDraft::Start::Create
+      .new(current_user, params[:form_id], **safe_params)
+      .execute
 
     render json: {
       shared_draft_id: object.id,
@@ -35,8 +36,9 @@ class TicketsSharedDraftStartsController < ApplicationController
   def update
     object = scope.find params[:id]
 
-    object.update! safe_params
-    object.attach_upload_cache params[:form_id]
+    Service::Ticket::SharedDraft::Start::Update
+      .new(current_user, object, params[:form_id], **safe_params)
+      .execute
 
     render json: {
       shared_draft_id: object.id,
@@ -73,17 +75,10 @@ class TicketsSharedDraftStartsController < ApplicationController
   end
 
   def safe_params
-    safe_params = params.permit :name, :group_id, content: {}
-
-    safe_params[:content].delete :group_id
-
-    allowed_groups = current_user.groups_access('create').map { |x| x.id.to_s }
-    group_id       = safe_params[:group_id]&.to_s
-
-    if allowed_groups.exclude? group_id
-      raise Exceptions::UnprocessableEntity, __("User does not have access to one of given group IDs: #{group_id}")
-    end
-
-    safe_params
+    @safe_params ||= params
+      .permit(:name, :group_id, content: {})
+      .to_hash
+      .to_options
+      .tap { |elem| elem[:group] = Group.find_by(id: elem.delete(:group_id)) }
   end
 end

+ 3 - 3
app/frontend/apps/desktop/components/CommonButton/CommonButton.vue

@@ -155,7 +155,7 @@ const iconSizeClass = computed(() => {
   >
     <CommonIcon
       v-if="prefixIcon"
-      class="shrink-0"
+      class="pointer-events-none shrink-0"
       decorative
       :size="iconSizeClass"
       :name="prefixIcon"
@@ -163,7 +163,7 @@ const iconSizeClass = computed(() => {
 
     <CommonIcon
       v-if="icon"
-      class="shrink-0"
+      class="pointer-events-none shrink-0"
       decorative
       :size="iconSizeClass"
       :name="icon"
@@ -174,7 +174,7 @@ const iconSizeClass = computed(() => {
 
     <CommonIcon
       v-if="suffixIcon"
-      class="shrink-0"
+      class="pointer-events-none shrink-0"
       decorative
       :size="iconSizeClass"
       :name="suffixIcon"

+ 11 - 13
app/frontend/apps/desktop/components/CommonFlyout/CommonFlyout.vue

@@ -253,19 +253,17 @@ onMounted(() => {
       }"
     >
       <slot name="header">
-        <div
-          class="flex items-center gap-2 text-base text-gray-100 dark:text-neutral-400"
+        <CommonLabel
+          v-if="headerTitle"
+          :id="`${flyoutId}-title`"
+          tag="h2"
+          class="min-h-7 grow"
+          size="large"
+          :prefix-icon="headerIcon"
+          icon-color="text-stone-200 dark:text-neutral-500"
         >
-          <CommonIcon
-            v-if="headerIcon"
-            class="flex-shrink-0"
-            size="small"
-            :name="headerIcon"
-          />
-          <h2 v-if="headerTitle" :id="`${flyoutId}-title`">
-            {{ headerTitle }}
-          </h2>
-        </div>
+          {{ $t(headerTitle) }}
+        </CommonLabel>
       </slot>
       <CommonButton
         class="ltr:ml-auto rtl:mr-auto"
@@ -295,7 +293,7 @@ onMounted(() => {
           !arrivedState.bottom && isContentOverflowing,
       }"
     >
-      <slot name="footer">
+      <slot name="footer" v-bind="{ action, close }">
         <CommonFlyoutActionFooter
           v-bind="footerActionOptions"
           @cancel="close()"

+ 1 - 1
app/frontend/apps/desktop/components/CommonFlyout/CommonFlyoutActionFooter.vue

@@ -31,7 +31,7 @@ const execute = () => {
 </script>
 
 <template>
-  <div class="flex items-center justify-end gap-2">
+  <div class="flex items-center justify-end gap-4">
     <CommonButton
       v-if="!hideCancelButton"
       size="large"

+ 0 - 2
app/frontend/apps/desktop/components/CommonPageHelp/CommonHelpText.vue

@@ -1,8 +1,6 @@
 <!-- Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/ -->
 
 <script setup lang="ts">
-import CommonLabel from '#shared/components/CommonLabel/CommonLabel.vue'
-
 interface Props {
   helpText?: string | string[]
 }

+ 0 - 1
app/frontend/apps/desktop/components/CommonSelect/CommonSelect.vue

@@ -17,7 +17,6 @@ import {
   toRef,
 } from 'vue'
 
-import CommonLabel from '#shared/components/CommonLabel/CommonLabel.vue'
 import type {
   MatchedSelectOption,
   SelectOption,

+ 78 - 0
app/frontend/apps/desktop/components/CommonSimpleEntityList/CommonSimpleEntityList.vue

@@ -0,0 +1,78 @@
+<!-- Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/ -->
+
+<script setup lang="ts" generic="T">
+import { computed } from 'vue'
+
+import CommonShowMoreButton from '#desktop/components/CommonShowMoreButton/CommonShowMoreButton.vue'
+import entityModules from '#desktop/components/CommonSimpleEntityList/plugins/index.ts'
+import {
+  type Entity,
+  EntityType,
+} from '#desktop/components/CommonSimpleEntityList/types.ts'
+
+interface Props {
+  /**
+   * Populate entity through `normalizesEdges` function
+   * @type {T[]} -> ReturnType of `normalizesEdges` function
+   * */
+  entity: Entity<T>
+  type: EntityType
+  label?: string
+}
+
+const props = defineProps<Props>()
+
+defineEmits<{
+  'load-more': []
+}>()
+
+const entitySetup = computed(() => {
+  const { component, ...context } = entityModules[props.type]
+  return {
+    component,
+    context,
+    data: props.entity.array,
+  }
+})
+</script>
+
+<template>
+  <div class="flex flex-col gap-1.5">
+    <CommonLabel
+      v-if="label"
+      class="-:inline-flex items-center text-xs leading-snug text-stone-200 dark:text-neutral-500"
+    >
+      {{ 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}`"
+      >
+        <component
+          :is="entitySetup.component"
+          :entity="entityValue"
+          :context="entitySetup.context"
+        />
+      </li>
+    </TransitionGroup>
+
+    <CommonLabel v-if="!entity.array?.length" class="block"
+      >{{ entitySetup.context.emptyMessage }}
+    </CommonLabel>
+
+    <CommonShowMoreButton
+      v-if="entity"
+      class="self-end"
+      :entities="entity.array"
+      :total-count="entity.totalCount"
+      @click="$emit('load-more')"
+    />
+  </div>
+</template>

+ 137 - 0
app/frontend/apps/desktop/components/CommonSimpleEntityList/__tests__/CommonEntityList.spec.ts

@@ -0,0 +1,137 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import { describe } from 'vitest'
+
+import renderComponent from '#tests/support/components/renderComponent.ts'
+
+import {
+  organizationOption,
+  userOption,
+} from '#desktop/components/CommonSimpleEntityList/__tests__/support/entityOptions.ts'
+import CommonSimpleEntityList from '#desktop/components/CommonSimpleEntityList/CommonSimpleEntityList.vue'
+import { EntityType } from '#desktop/components/CommonSimpleEntityList/types.ts'
+
+describe('CommonSimpleEntityList', () => {
+  describe('Entity Types', () => {
+    it('renders a list of users', () => {
+      const wrapper = renderComponent(CommonSimpleEntityList, {
+        router: true,
+        props: {
+          type: EntityType.User,
+          entity: {
+            array: userOption,
+            totalCount: userOption.length,
+          },
+        },
+      })
+
+      expect(wrapper.getByText('Nicole Braun')).toBeInTheDocument()
+      expect(wrapper.getByText('Thomas Ernst')).toBeInTheDocument()
+      expect(
+        wrapper.getByLabelText('Avatar (Nicole Braun)'),
+      ).toBeInTheDocument()
+      expect(
+        wrapper.getByLabelText('Avatar (Thomas Ernst)'),
+      ).toBeInTheDocument()
+      expect(wrapper.getAllByTestId('common-link')).toHaveLength(3)
+      expect(wrapper.getAllByTestId('common-link').at(0)).toHaveAttribute(
+        'href',
+        '/user/profile/2',
+      )
+    })
+
+    it('renders a list of organizations', () => {
+      const wrapper = renderComponent(CommonSimpleEntityList, {
+        router: true,
+        props: {
+          type: EntityType.Organization,
+          entity: {
+            array: organizationOption,
+            totalCount: organizationOption.length,
+          },
+        },
+      })
+
+      expect(wrapper.getByText('Spar')).toBeInTheDocument()
+      expect(wrapper.getByText('Mercadona')).toBeInTheDocument()
+      expect(wrapper.getByLabelText('Avatar (Spar)')).toBeInTheDocument()
+      expect(wrapper.getByLabelText('Avatar (Mercadona)')).toBeInTheDocument()
+      expect(wrapper.getAllByTestId('common-link')).toHaveLength(3)
+      expect(wrapper.getAllByTestId('common-link').at(0)).toHaveAttribute(
+        'href',
+        '/organizations/2',
+      )
+    })
+  })
+
+  it('supports entity label', () => {
+    const wrapper = renderComponent(CommonSimpleEntityList, {
+      router: true,
+      props: {
+        label: 'Foo Label',
+        type: EntityType.User,
+        entity: {
+          array: userOption,
+          totalCount: userOption.length,
+        },
+      },
+    })
+
+    expect(wrapper.getByText('Foo Label')).toBeInTheDocument()
+  })
+
+  it('hides load more button if less than three entities are available', () => {
+    const wrapper = renderComponent(CommonSimpleEntityList, {
+      router: true,
+      props: {
+        type: EntityType.User,
+        entity: {
+          array: userOption,
+          totalCount: userOption.length,
+        },
+      },
+    })
+
+    expect(
+      wrapper.queryByRole('button', {
+        name: /show/i,
+      }),
+    ).not.toBeInTheDocument()
+  })
+
+  it('emits load-more event if more than three entities are available', async () => {
+    const wrapper = renderComponent(CommonSimpleEntityList, {
+      router: true,
+      props: {
+        type: EntityType.User,
+        entity: {
+          array: userOption,
+          totalCount: userOption.length + 1,
+        },
+      },
+    })
+
+    await wrapper.events.click(
+      wrapper.getByRole('button', {
+        name: 'Show 1 more',
+      }),
+    )
+
+    expect(wrapper.emitted()['load-more']).toHaveLength(1)
+  })
+
+  it('displays message if list is empty', async () => {
+    const wrapper = renderComponent(CommonSimpleEntityList, {
+      router: true,
+      props: {
+        type: EntityType.User,
+        entity: {
+          array: [],
+          totalCount: 0,
+        },
+      },
+    })
+
+    expect(wrapper.getByText('No members found')).toBeInTheDocument()
+  })
+})

+ 72 - 0
app/frontend/apps/desktop/components/CommonSimpleEntityList/__tests__/support/entityOptions.ts

@@ -0,0 +1,72 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import { convertToGraphQLId } from '#shared/graphql/utils.ts'
+
+export const userOption = [
+  {
+    __typename: 'User',
+    id: convertToGraphQLId('User', 2),
+    internalId: 2,
+    image: null,
+    firstname: 'Nicole',
+    lastname: 'Braun',
+    fullname: 'Nicole Braun',
+    outOfOffice: false,
+    outOfOfficeStartAt: null,
+    outOfOfficeEndAt: null,
+    active: true,
+    vip: false,
+  },
+  {
+    __typename: 'User',
+    id: convertToGraphQLId('User', 4),
+    internalId: 4,
+    image: null,
+    firstname: 'Agent 1',
+    lastname: 'Test',
+    fullname: 'Agent 1 Test',
+    outOfOffice: false,
+    outOfOfficeStartAt: null,
+    outOfOfficeEndAt: null,
+    active: true,
+    vip: false,
+  },
+  {
+    __typename: 'User',
+    id: convertToGraphQLId('User', 8),
+    internalId: 8,
+    image: null,
+    firstname: 'Thomas',
+    lastname: 'Ernst',
+    fullname: 'Thomas Ernst',
+    outOfOffice: false,
+    outOfOfficeStartAt: null,
+    outOfOfficeEndAt: null,
+    active: true,
+    vip: false,
+  },
+]
+
+export const organizationOption = [
+  {
+    __typename: 'Organization',
+    id: convertToGraphQLId('Organization', 2),
+    internalId: 2,
+    active: true,
+    name: 'Spar',
+  },
+  {
+    __typename: 'Organization',
+    id: convertToGraphQLId('Organization', 3),
+    internalId: 3,
+    active: true,
+    name: 'Billa',
+  },
+  {
+    __typename: 'Organization',
+    id: convertToGraphQLId('Organization', 4),
+    internalId: 4,
+    active: true,
+    name: 'Mercadona',
+  },
+]

Some files were not shown because too many files changed in this diff