Fixes #4653 - Mobile view fails when creating tickets in groups with just 'create' permission.

Co-authored-by: Vladimir Sheremet <vs@zammad.com>
Co-authored-by: Mantas Masalskis <mm@zammad.com>
Co-authored-by: Dominik Klein <dk@zammad.com>

app/frontend/apps/mobile/pages/ticket/views/TicketCreate.vue

@@ -28,7 +28,7 @@ import {
   NotificationTypes,
   useNotifications,
 } from '#shared/components/CommonNotifications/index.ts'
-import { ErrorStatusCodes } from '#shared/types/error.ts'
+import { ErrorStatusCodes, GraphQLErrorTypes } from '#shared/types/error.ts'
 import type UserError from '#shared/errors/UserError.ts'
 import { defineFormSchema } from '#mobile/form/defineFormSchema.ts'
 import { populateEditorNewLines } from '#shared/components/Form/fields/FieldEditor/utils.ts'
@@ -46,6 +46,7 @@ import type { TicketFormData } from '#shared/entities/ticket/types.ts'
 import { convertFilesToAttachmentInput } from '#shared/utils/files.ts'
 import { useDialog } from '#shared/composables/useDialog.ts'
 import { useStickyHeader } from '#shared/composables/useStickyHeader.ts'
+import type { ApolloError } from '@apollo/client'
 import { useTicketCreateMutation } from '../graphql/mutations/create.api.ts'
 
 const router = useRouter()
@@ -291,7 +292,7 @@ const formSchema = defineFormSchema(
 )
 
 const ticketCreateMutation = new MutationHandler(useTicketCreateMutation({}), {
-  errorNotificationMessage: __('Ticket could not be created.'),
+  errorShowNotification: false,
 })
 
 const redirectAfterCreate = (internalId?: number) => {
@@ -306,9 +307,38 @@ const smimeIntegration = computed(
   () => (application.config.smime_integration as boolean) || {},
 )
 
-const createTicket = async (formData: FormSubmitData<TicketFormData>) => {
-  const { notify } = useNotifications()
+const { notify } = useNotifications()
+
+const notifySuccess = () => {
+  notify({
+    type: NotificationTypes.Success,
+    message: __('Ticket has been created successfully.'),
+  })
+}
+
+const handleTicketCreateError = (error: UserError | ApolloError) => {
+  if ('graphQLErrors' in error) {
+    const graphQLErrors = error.graphQLErrors?.[0]
+    // treat this as successful
+    if (graphQLErrors?.extensions?.type === GraphQLErrorTypes.Forbidden) {
+      notifySuccess()
+
+      return () => redirectAfterCreate()
+    }
+
+    notify({
+      message: __('Ticket could not be created.'),
+      type: NotificationTypes.Error,
+    })
+  } else {
+    notify({
+      message: error.generalErrors[0],
+      type: NotificationTypes.Error,
+    })
+  }
+}
 
+const createTicket = async (formData: FormSubmitData<TicketFormData>) => {
   const { attributesLookup: ticketObjectAttributesLookup } =
     useObjectAttributes(EnumObjectManagerObjects.Ticket)
 
@@ -343,10 +373,7 @@ const createTicket = async (formData: FormSubmitData<TicketFormData>) => {
     .send({ input })
     .then((result) => {
       if (result?.ticketCreate?.ticket) {
-        notify({
-          type: NotificationTypes.Success,
-          message: __('Ticket has been created successfully.'),
-        })
+        notifySuccess()
 
         return () => {
           const ticket = result.ticketCreate?.ticket
@@ -358,12 +385,7 @@ const createTicket = async (formData: FormSubmitData<TicketFormData>) => {
       }
       return null
     })
-    .catch((errors: UserError) => {
-      notify({
-        message: errors.generalErrors[0],
-        type: NotificationTypes.Error,
-      })
-    })
+    .catch(handleTicketCreateError)
 }
 
 const additionalCreateNotes = computed(

app/models/ticket/article/adds_metadata_general.rb

@@ -61,7 +61,7 @@ module Ticket::Article::AddsMetadataGeneral
 
   def metadata_general_process_from
     type        = Ticket::Article::Type.lookup(id: type_id)
-    is_customer = !TicketPolicy.new(author, ticket).agent_read_access?
+    is_customer = !author.permissions?('ticket.agent')
 
     self.from = if %w[web phone].include?(type.name) && is_customer
                   Channel::EmailBuild.recipient_line "#{author.firstname} #{author.lastname}", author.email

app/services/service/ticket/article/create.rb

@@ -149,10 +149,8 @@ class Service::Ticket::Article::Create < Service::BaseWithCurrentUser
       .destroy
   end
 
-  def agent_on_ticket?(ticket)
-    TicketPolicy
-      .new(current_user, ticket)
-      .agent_read_access?
+  def agent_on_ticket?(_ticket)
+    current_user.permissions?('ticket.agent')
   end
 
   def display_name(user)

spec/services/service/ticket/article/create_spec.rb

@@ -55,17 +55,29 @@ RSpec.describe Service::Ticket::Article::Create, current_user_id: -> { user.id }
       end
 
       context 'when user is customer' do
-        let(:user) { ticket.customer }
+        let(:user)   { ticket.customer }
+        let(:ticket) { create(:ticket, customer: create(:customer)) }
 
         it 'ensures sender is set to customer' do
           expect(article.sender.name).to eq 'Customer'
         end
       end
+
+      # Agent-Customer is incorrectly detected as Agent in a group he has no access to
+      # https://github.com/zammad/zammad/issues/4649
+      context 'when user is agent-customer' do
+        let(:user) { ticket.customer }
+
+        it 'ensures sender is set to customer' do
+          expect(article.sender.name).to eq 'Agent'
+        end
+      end
     end
 
     describe 'processing for customer' do
       context 'when user is customer' do
-        let(:user) { ticket.customer }
+        let(:user)   { ticket.customer }
+        let(:ticket) { create(:ticket, customer: create(:customer)) }
 
         it 'ensures internal is false' do
           payload[:internal] = true
@@ -80,6 +92,24 @@ RSpec.describe Service::Ticket::Article::Create, current_user_id: -> { user.id }
         end
       end
 
+      # Agent-Customer is incorrectly detected as Agent in a group he has no access to
+      # https://github.com/zammad/zammad/issues/4649
+      context 'when user is agent-customer' do
+        let(:user) { ticket.customer }
+
+        it 'ensures internal is false' do
+          payload[:internal] = false
+
+          expect(article.internal).to be_falsey
+        end
+
+        it 'changes type from web to note' do
+          payload[:type] = 'phone'
+
+          expect(article.type.name).to eq('phone')
+        end
+      end
+
       context 'when user is agent' do
         it 'allows internal to be true' do
           payload[:internal] = true

spec/system/apps/mobile/tickets/ticket_create_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe 'Mobile > Ticket > Create', app: :mobile, authenticated_as: :user
     wait_for_form_to_settle('ticket-create')
   end
 
-  shared_examples 'creating a ticket' do |article_type:, direction: nil|
+  shared_examples 'creating a ticket' do |article_type:, direction: nil, redirect: nil|
     it 'can complete all steps' do
       expect(find_button('Create', disabled: true).disabled?).to be(true)
 
@@ -76,7 +76,7 @@ RSpec.describe 'Mobile > Ticket > Create', app: :mobile, authenticated_as: :user
 
       find('[role=alert]', text: 'Ticket has been created successfully.')
 
-      expect(page).to have_current_path("/mobile/tickets/#{Ticket.last.id}")
+      expect(page).to have_current_path(redirect || "/mobile/tickets/#{Ticket.last.id}")
       expect(Ticket.last.create_article_type_id).to eq(Ticket::Article::Type.find_by(name: article_type).id)
     end
   end
@@ -86,6 +86,16 @@ RSpec.describe 'Mobile > Ticket > Create', app: :mobile, authenticated_as: :user
     it_behaves_like 'creating a ticket', article_type: 'phone', direction: 'out'
     it_behaves_like 'creating a ticket', article_type: 'email'
 
+    context 'when dont have a "read" permission, but have "create" permission' do
+      it_behaves_like 'creating a ticket', article_type: 'email', redirect: '/mobile/' do
+        before do
+          user.group_names_access_map = {
+            group.name => ['create']
+          }
+        end
+      end
+    end
+
     context 'when a customer', authenticated_as: :customer do
       it_behaves_like 'creating a ticket', article_type: 'web'
     end