Feature: Mobile - Fix sender/type handling when creating an article.

app/frontend/shared/entities/ticket-article/action/plugins/__tests__/article-action-types.spec.ts

@@ -1,15 +1,17 @@
 // Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
 import { defaultTicket } from '@mobile/pages/ticket/__tests__/mocks/detail-view'
+import { mockPermissions } from '@tests/support/mock-permissions'
 import { createTestArticleTypes } from './utils'
 
 describe('article action plugins - types', () => {
   it('successfully returns available types', () => {
+    mockPermissions(['ticket.customer'])
     const { ticket } = defaultTicket()
     const types = createTestArticleTypes(ticket)
     expect(types).toHaveLength(1)
     expect(types[0]).toMatchObject({
-      value: 'note',
+      value: 'web',
       attributes: ['attachments'],
     })
   })

app/frontend/shared/entities/ticket-article/action/plugins/__tests__/type-note.spec.ts

@@ -7,24 +7,31 @@ import { createTestArticleTypes } from './utils'
 
 describe('note type', () => {
   it.each([
-    ['ticket.customer', false, false],
-    ['ticket.customer', true, false],
     ['ticket.agent', false, false],
     ['ticket.agent', true, true],
   ])(
     'check article internal for "%s" when config is %s',
     (permission, config, internal) => {
-      const { ticket } = defaultTicket()
-      ticket.policy.agentReadAccess = permission === 'ticket.agent'
       mockPermissions([permission])
+      const { ticket } = defaultTicket()
       mockApplicationConfig({
         ui_ticket_zoom_article_note_new_internal: config,
       })
+
       const types = createTestArticleTypes(ticket)
-      expect(types[0]).toMatchObject({
-        value: 'note',
-        internal,
-      })
+
+      expect(types).toContainEqual(
+        expect.objectContaining({ value: 'note', internal }),
+      )
     },
   )
+
+  it('customer does not get note type', () => {
+    mockPermissions(['ticket.customer'])
+    const { ticket } = defaultTicket()
+
+    const types = createTestArticleTypes(ticket)
+
+    expect(types).not.toContainEqual(expect.objectContaining({ value: 'note' }))
+  })
 })

app/frontend/shared/entities/ticket-article/action/plugins/__tests__/type-web.spec.ts

@@ -0,0 +1,25 @@
+// Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+import { defaultTicket } from '@mobile/pages/ticket/__tests__/mocks/detail-view'
+import { mockPermissions } from '@tests/support/mock-permissions'
+import { createTestArticleTypes } from './utils'
+
+describe('web type', () => {
+  it('customer does get web type', () => {
+    mockPermissions(['ticket.customer'])
+    const { ticket } = defaultTicket()
+
+    const types = createTestArticleTypes(ticket)
+
+    expect(types).toContainEqual(expect.objectContaining({ value: 'web' }))
+  })
+
+  it('agent does not get web type', () => {
+    mockPermissions(['ticket.agent'])
+    const { ticket } = defaultTicket()
+
+    const types = createTestArticleTypes(ticket)
+
+    expect(types).not.toContainEqual(expect.objectContaining({ value: 'web' }))
+  })
+})

app/frontend/shared/entities/ticket-article/action/plugins/note.ts

@@ -5,10 +5,7 @@ import type { TicketArticleActionPlugin, TicketArticleType } from './types'
 const actionPlugin: TicketArticleActionPlugin = {
   order: 100,
 
-  addTypes(ticket, { view, config }) {
-    let internal = false
-    if (view.isTicketAgent)
-      internal = !!config.ui_ticket_zoom_article_note_new_internal
+  addTypes(ticket, { config }) {
     const type: TicketArticleType = {
       apps: ['mobile'],
       value: 'note',
@@ -17,11 +14,10 @@ const actionPlugin: TicketArticleActionPlugin = {
         mobile: 'mobile-note',
       },
       view: {
-        agent: ['read'],
-        customer: ['read'],
+        agent: ['change'],
       },
       attributes: ['attachments'],
-      internal,
+      internal: !!config.ui_ticket_zoom_article_note_new_internal,
     }
     return [type]
   },

app/frontend/shared/entities/ticket-article/action/plugins/web.ts

@@ -0,0 +1,26 @@
+// Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+import type { TicketArticleActionPlugin, TicketArticleType } from './types'
+
+const actionPlugin: TicketArticleActionPlugin = {
+  order: 100,
+
+  addTypes() {
+    const type: TicketArticleType = {
+      apps: ['mobile'],
+      value: 'web',
+      label: __('Web'),
+      icon: {
+        mobile: 'mobile-web',
+      },
+      view: {
+        customer: ['change'],
+      },
+      attributes: ['attachments'],
+      internal: false,
+    }
+    return [type]
+  },
+}
+
+export default actionPlugin

app/graphql/gql/mutations/ticket/create.rb

@@ -13,7 +13,11 @@ module Gql::Mutations
     end
 
     def resolve(input:)
-      { ticket: Service::Ticket::Create.new(current_user: context.current_user).execute(ticket_data: input.to_h) }
+      {
+        ticket: Service::Ticket::Create
+          .new(current_user: context.current_user)
+          .execute(ticket_data: input)
+      }
     end
   end
 end

app/graphql/gql/mutations/ticket/update.rb

@@ -14,7 +14,11 @@ module Gql::Mutations
     end
 
     def resolve(ticket:, input:)
-      { ticket: Service::Ticket::Update.new(current_user: context.current_user).execute(ticket: ticket, ticket_data: input.to_h) }
+      {
+        ticket: Service::Ticket::Update
+          .new(current_user: context.current_user)
+          .execute(ticket: ticket, ticket_data: input)
+      }
     end
   end
 end

app/graphql/gql/types/input/ticket/article_input_type.rb

@@ -5,7 +5,7 @@ module Gql::Types::Input::Ticket
     description 'Represents the article attributes to be used in ticket create/update.'
 
     argument :body, String, required: false, description: 'The article body.'
-    argument :subject, String, required: false, description: 'The article subject.'
+    argument :subject, String, required: false, description: 'The article subject.', default_value: ''
     argument :internal, Boolean, required: false, description: 'Whether the article is internal.'
     argument :type, String, required: false, description: 'The article type.'
     argument :sender, String, required: false, description: 'The article sender.'
@@ -14,49 +14,17 @@ module Gql::Types::Input::Ticket
     argument :cc, [String], required: false, description: 'The article CC address.'
     argument :content_type, String, required: false, description: 'The article content type.'
     argument :subtype, String, required: false, description: 'The article subtype.'
-    argument :in_reply_to, String, required: false, description: 'Message id of the article this article replies to.'
+    argument :in_reply_to, String, required: false, description: 'Message id of the article this article replies to.', default_value: ''
     argument :time_unit, Float, required: false, description: 'The article accounted time.'
     argument :preferences, GraphQL::Types::JSON, required: false, description: 'The article preferences.'
     argument :attachments, Gql::Types::Input::AttachmentInputType, required: false, description: 'The article attachments.'
     argument :security, [Gql::Types::Enum::SecurityOptionType], required: false, description: 'The article security options.'
 
-    transform :transform_type
-    transform :transform_subtype
-    transform :transform_sender
-    transform :transform_customer_article
     transform :transform_security
-
-    def transform_type(payload)
-      payload.to_h.tap do |result|
-        result[:type] = Ticket::Article::Type.lookup(name: result[:type].presence || 'note')
-      end
-    end
-
-    def transform_sender(payload)
-      # TODO: not correct, should use "agent_read_access?" check from ticket_policy
-      sender_name = context.current_user.permissions?('ticket.agent') ? 'Agent' : 'Customer'
-      article_sender = payload[:sender].presence || sender_name
-
-      payload[:sender] = Ticket::Article::Sender.lookup(name: article_sender)
-
-      payload
-    end
-
-    def transform_customer_article(payload)
-      return payload if context.current_user.permissions?('ticket.agent')
-
-      payload[:sender] = Ticket::Article::Sender.lookup(name: 'Customer')
-
-      if payload[:type].name.match?(%r{^(note|web)$})
-        payload[:type] = Ticket::Article::Type.lookup(name: 'note')
-      end
-
-      payload[:internal] = false
-
-      payload
-    end
+    transform :transform_subtype
 
     def transform_subtype(payload)
+      payload = payload.to_h
       subtype = payload.delete(:subtype) if payload[:subtype]
 
       if subtype.present?
@@ -68,6 +36,7 @@ module Gql::Types::Input::Ticket
     end
 
     def transform_security(payload)
+      payload = payload.to_h
       security = payload.delete(:security) if payload[:security]
 
       return payload if !Setting.get('smime_integration')

app/graphql/gql/types/input/ticket/create_input_type.rb

@@ -14,18 +14,5 @@ module Gql::Types::Input::Ticket
 
     # Arguments specific to create.
     argument :tags, [String], required: false, description: 'The tags that should be assigned to the new ticket.', prepare: only_for_ticket_agents
-
-    transform :lazy_default_values
-
-    def lazy_default_values(payload)
-      payload.to_h.tap do |result|
-
-        result[:state] ||= Ticket::State.find_by(default_create: true)
-
-        if context.current_user.permissions?('ticket.customer')
-          result[:customer_id] ||= context.current_user.id
-        end
-      end
-    end
   end
 end

app/models/ticket/article/adds_metadata_email.rb

@@ -11,37 +11,39 @@ module Ticket::Article::AddsMetadataEmail
   private
 
   def ticket_article_add_metadata_email
+    return if !neither_importing_nor_postmaster?
+    return if !sender_needs_metadata?
+    return if !type_needs_metadata?
+
+    metadata_email_process_subject
+    metadata_email_process_message_id
+    metadata_email_process_email_address
+    metadata_email_process_from
+  end
 
-    # return if we run import mode
-    return true if Setting.get('import_mode')
-
-    # only do fill of email from if article got created via application_server (e. g. not
-    # if article and sender type is set via *.postmaster)
-    return if ApplicationHandleInfo.postmaster?
-
-    # if sender is customer, do not change anything
-    return true if !sender_id
+  def sender_needs_metadata?
+    return if !sender_id
 
     sender = Ticket::Article::Sender.lookup(id: sender_id)
-    return true if sender.nil?
-    return true if sender.name == 'Customer'
+    return if sender&.name == 'Customer'
+
+    true
+  end
 
-    # set email attributes
-    return true if !type_id
+  def type_needs_metadata?
+    return if !type_id
 
     type = Ticket::Article::Type.lookup(id: type_id)
-    return true if type.nil?
-    return true if type.name != 'email'
+    return if type&.name != 'email'
 
-    # set subject if empty
-    ticket = self.ticket
-    if !subject || subject == ''
-      self.subject = ticket.title
-    end
+    true
+  end
 
-    # clean subject
-    self.subject = ticket.subject_clean(subject)
+  def metadata_email_process_subject
+    self.subject = ticket.subject_clean subject.presence || ticket.title
+  end
 
+  def metadata_email_process_message_id
     # generate message id, force it in production, in test allow to set it for testing reasons
     if !message_id || Rails.env.production?
       fqdn = Setting.get('fqdn')
@@ -50,33 +52,37 @@ module Ticket::Article::AddsMetadataEmail
 
     # generate message_id_md5
     check_message_id_md5
+  end
 
+  def metadata_email_process_email_address
     # set sender
     email_address = ticket.group.email_address
+
     if !email_address
       raise "No email address found for group '#{ticket.group.name}' (#{ticket.group_id})"
     end
 
     # remember email address for background job
     preferences['email_address_id'] = email_address.id
+  end
 
-    # fill from
-    if created_by_id != 1 && Setting.get('ticket_define_email_from') == 'AgentNameSystemAddressName'
-      separator   = Setting.get('ticket_define_email_from_separator')
-      sender      = User.find(created_by_id)
-      realname    = "#{sender.firstname} #{sender.lastname} #{separator} #{email_address.realname}"
-      self.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
-    elsif Setting.get('ticket_define_email_from') == 'AgentName'
-      sender      = User.find(created_by_id)
-      realname    = "#{sender.firstname} #{sender.lastname}"
-
-      # avoid "-" as realname, see https://github.com/zammad/zammad/issues/3890
-      realname = email_address.realname if sender.id == 1
-
-      self.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
-    else
-      self.from = Channel::EmailBuild.recipient_line(email_address.realname, email_address.email)
+  def recipient_name(email_address)
+    if created_by_id != 1
+      case Setting.get('ticket_define_email_from')
+      when 'AgentNameSystemAddressName'
+        separator = Setting.get('ticket_define_email_from_separator')
+        return "#{created_by.firstname} #{created_by.lastname} #{separator} #{email_address.realname}"
+      when 'AgentName'
+        return "#{created_by.firstname} #{created_by.lastname}"
+      end
     end
-    true
+
+    email_address.realname
+  end
+
+  def metadata_email_process_from
+    email_address = ticket.group.email_address
+
+    self.from = Channel::EmailBuild.recipient_line(recipient_name(email_address), email_address.email)
   end
 end