Feature: Mobile - Fix display of inline attachments.

app/frontend/apps/mobile/pages/ticket/__tests__/mocks/detail-view.ts

@@ -128,7 +128,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               authorizations: [],
             },
             internal: false,
-            body: '<p>Body <b>of a test ticket</b></p>',
+            bodyWithUrls: '<p>Body <b>of a test ticket</b></p>',
             sender: {
               __typename: 'TicketArticleSender',
               name: 'Customer',
@@ -138,7 +138,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               name: 'article',
             },
             contentType: 'text/html',
-            attachments: [
+            attachmentsWithoutInline: [
               // should not be visible
               {
                 __typename: 'StoredFile',
@@ -180,7 +180,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               authorizations: [],
             },
             internal: false,
-            body: '<p>energy equals power times time</p>',
+            bodyWithUrls: '<p>energy equals power times time</p>',
             sender: {
               __typename: 'TicketArticleSender',
               name: 'Agent',
@@ -189,7 +189,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               __typename: 'TicketArticleType',
               name: 'article',
             },
-            attachments: [],
+            attachmentsWithoutInline: [],
             preferences: {},
             contentType: 'text/html',
           },
@@ -215,7 +215,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               authorizations: [],
             },
             internal: true,
-            body: '<p>only agents can see this haha</p>',
+            bodyWithUrls: '<p>only agents can see this haha</p>',
             sender: {
               __typename: 'TicketArticleSender',
               name: 'Agent',
@@ -225,7 +225,7 @@ export const defaultArticles = (): TicketArticlesQuery =>
               name: 'article',
             },
             contentType: 'text/html',
-            attachments: [],
+            attachmentsWithoutInline: [],
             preferences: {},
           },
           cursor: 'MI',

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleMetadataDialog.vue

@@ -36,7 +36,7 @@ const links = computed(() => {
       target: '_blank',
     })
   }
-  article.attachments.forEach((file) => {
+  article.attachmentsWithoutInline.forEach((file) => {
     if (file.preferences?.['original-format'] !== true) {
       return
     }

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticlesList.vue

@@ -46,7 +46,7 @@ const { rows } = useTicketArticleRows(
 )
 
 const filterAttachments = (article: TicketArticle) => {
-  return article.attachments.filter(
+  return article.attachmentsWithoutInline.filter(
     (file) => !file.preferences || !file.preferences['original-format'],
   )
 }
@@ -62,7 +62,7 @@ const filterAttachments = (article: TicketArticle) => {
     <template v-for="row in rows" :key="row.key">
       <ArticleBubble
         v-if="row.type === 'article-bubble'"
-        :content="row.article.body"
+        :content="row.article.bodyWithUrls"
         :user="row.article.createdBy"
         :internal="row.article.internal"
         :content-type="row.article.contentType"

app/frontend/apps/mobile/pages/ticket/graphql/fragments/ticketArticleAttributes.api.ts

@@ -40,7 +40,7 @@ export const TicketArticleAttributesFragmentDoc = gql`
   inReplyTo
   contentType
   references
-  attachments {
+  attachmentsWithoutInline {
     internalId
     name
     size
@@ -48,7 +48,7 @@ export const TicketArticleAttributesFragmentDoc = gql`
     preferences
   }
   preferences
-  body
+  bodyWithUrls
   internal
   createdAt
   createdBy {

app/frontend/apps/mobile/pages/ticket/graphql/fragments/ticketArticleAttributes.graphql

@@ -36,7 +36,7 @@ fragment ticketArticleAttributes on TicketArticle {
   inReplyTo
   contentType
   references
-  attachments {
+  attachmentsWithoutInline {
     internalId
     name
     size
@@ -44,7 +44,7 @@ fragment ticketArticleAttributes on TicketArticle {
     preferences
   }
   preferences
-  body
+  bodyWithUrls
   internal
   createdAt
   createdBy {

app/frontend/shared/entities/ticket/types.ts

@@ -39,7 +39,8 @@ export type TicketArticle = ConfidentTake<
   'articles.edges.node'
 >
 
-export type TicketArticleAttachment = TicketArticle['attachments'][number]
+export type TicketArticleAttachment =
+  TicketArticle['attachmentsWithoutInline'][number]
 
 export interface TicketCustomerUpdateFormData {
   customer_id: number

app/graphql/gql/types/ticket/article_type.rb

@@ -21,19 +21,42 @@ module Gql::Types::Ticket
     field :in_reply_to, String
     field :content_type, String, null: false
     field :references, String
-    field :body, String, null: false
+    field :body, String, null: false, description: 'Raw body as saved in the database.'
+    field :body_with_urls, String, null: false, description: 'Body with cid: URLs replaced for inline images in HTML articles.'
     field :internal, Boolean, null: false
     field :origin_by, Gql::Types::UserType
 
     field :preferences, ::GraphQL::Types::JSON
     field :security_state, Gql::Types::Ticket::Article::SecurityStateType
 
-    field :attachments, [Gql::Types::StoredFileType, { null: false }], null: false
+    field :attachments, [Gql::Types::StoredFileType, { null: false }], null: false, description: 'All attached files as stored in the database.'
+    field :attachments_without_inline, [Gql::Types::StoredFileType, { null: false }], null: false, description: 'Attachments for display, with inline images filtered out.'
 
     belongs_to :ticket, Gql::Types::TicketType, null: false
 
+    def body_with_urls
+      display_article['body']
+    end
+
+    def attachments_without_inline
+      # TODO: This uses asset handling related code which does more than what we need here.
+      #   On the long run it might be better to store the display flag directly with the attachments,
+      #   rather than always calculating it on-the-fly.
+      select_ids = display_article['attachments'].pluck('id')
+      @object.attachments.select do |attachment|
+        select_ids.include?(attachment.id)
+      end
+    end
+
     def security_state
       @object.preferences['security']
     end
+
+    private
+
+    def display_article
+      # TODO: This uses asset handling related code which does more than what we need here.
+      @display_article ||= @object.class.insert_urls(@object.attributes_with_association_ids)
+    end
   end
 end

spec/graphql/gql/queries/ticket/articles_spec.rb

@@ -61,11 +61,10 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
                 contentType
                 references
                 attachments {
-                  internalId
                   name
-                  size
-                  type
-                  preferences
+                }
+                attachmentsWithoutInline {
+                  name
                 }
                 preferences
                 securityState {
@@ -76,6 +75,7 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
                   encryptionMessage
                 }
                 body
+                bodyWithUrls
                 internal
                 createdAt
                 createdBy @include(if: $isAgent) {
@@ -105,7 +105,37 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
     let(:ticket)               { create(:ticket, customer: customer) }
     let(:cc)                   { 'Zammad CI <ci@zammad.org>' }
     let(:to)                   { '@unparseable_address' }
-    let!(:articles)            { create_list(:ticket_article, 5, :outbound_email, ticket: ticket, to: to, cc: cc) }
+    let(:cid)                  { "#{SecureRandom.uuid}@zammad.example.com" }
+    let!(:articles) do
+      create_list(:ticket_article, 2, :outbound_email, ticket: ticket, to: to, cc: cc, content_type: 'text/html', body: "<img src=\"cid:#{cid}\"> some text") do |article, _i|
+        create(
+          :store,
+          object:      'Ticket::Article',
+          o_id:        article.id,
+          data:        'fake',
+          filename:    'inline_image.jpg',
+          preferences: {
+            'Content-Type'        => 'image/jpeg',
+            'Mime-Type'           => 'image/jpeg',
+            'Content-ID'          => "<#{cid}>",
+            'Content-Disposition' => 'inline',
+          }
+        )
+        create(
+          :store,
+          object:      'Ticket::Article',
+          o_id:        article.id,
+          data:        'fake',
+          filename:    'attached_image.jpg',
+          preferences: {
+            'Content-Type'        => 'image/jpeg',
+            'Mime-Type'           => 'image/jpeg',
+            'Content-ID'          => "<#{cid}.not.referenced>",
+            'Content-Disposition' => 'inline',
+          }
+        )
+      end
+    end
     let!(:internal_article)    { create(:ticket_article, :outbound_email, ticket: ticket, internal: true) }
     let(:response_articles)    { gql.result.nodes }
     let(:response_total_count) { gql.result.data['totalCount'] }
@@ -118,11 +148,12 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
 
       context 'with permission' do
         let(:agent) { create(:agent, groups: [ticket.group]) }
-        let(:article1) { articles.first }
+        let(:article1)   { articles.first }
+        let(:inline_url) { "/api/v1/ticket_attachment/#{article1['ticket_id']}/#{article1['id']}/#{article1.attachments.first[:id]}?view=inline" }
         let(:expected_article1) do
           {
-            'subject'       => article1.subject,
-            'cc'            => {
+            'subject'                  => article1.subject,
+            'cc'                       => {
               'parsed' => [
                 {
                   'emailAddress' => 'ci@zammad.org',
@@ -131,18 +162,22 @@ RSpec.describe Gql::Queries::Ticket::Articles, type: :graphql do
               ],
               'raw'    => cc,
             },
-            'to'            => {
+            'to'                       => {
               'parsed' => nil,
               'raw'    => to,
             },
-            'references'    => article1.references,
-            'type'          => {
+            'references'               => article1.references,
+            'type'                     => {
               'name' => article1.type.name,
             },
-            'sender'        => {
+            'sender'                   => {
               'name' => article1.sender.name,
             },
-            'securityState' => nil,
+            'securityState'            => nil,
+            'body'                     => "<img src=\"cid:#{cid}\"> some text",
+            'bodyWithUrls'             => "<img src=\"#{inline_url}\" style=\"max-width:100%;\"> some text",
+            'attachments'              => [{ 'name'=>'inline_image.jpg' }, { 'name'=>'attached_image.jpg' }],
+            'attachmentsWithoutInline' => [{ 'name'=>'attached_image.jpg' }],
           }
         end
 

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

@@ -0,0 +1,56 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Mobile > Ticket > Articles', app: :mobile, authenticated_as: :agent, type: :system do
+  let(:group)  { create(:group) }
+  let(:agent)  { create(:agent, groups: [group]) }
+  let(:cid)    { "#{SecureRandom.uuid}@zammad.example.com" }
+  let(:ticket) { create(:ticket, title: 'Ticket Title', group: group) }
+  let(:article) do
+    create(:ticket_article, :outbound_email, ticket: ticket, to: 'Zammad CI <ci@zammad.org>', content_type: 'text/html', body: "<img src=\"cid:#{cid}\"> some text").tap do |article|
+      create(
+        :store,
+        object:      'Ticket::Article',
+        o_id:        article.id,
+        data:        'fake',
+        filename:    'inline_image.jpg',
+        preferences: {
+          'Content-Type'        => 'image/jpeg',
+          'Mime-Type'           => 'image/jpeg',
+          'Content-ID'          => "<#{cid}>",
+          'Content-Disposition' => 'inline',
+        }
+      )
+      create(
+        :store,
+        object:      'Ticket::Article',
+        o_id:        article.id,
+        data:        'fake',
+        filename:    'attached_image.jpg',
+        preferences: {
+          'Content-Type'        => 'image/jpeg',
+          'Mime-Type'           => 'image/jpeg',
+          'Content-ID'          => "<#{cid}.not.referenced>",
+          'Content-Disposition' => 'inline',
+        }
+      )
+    end
+  end
+
+  context 'when looking at a ticket' do
+    it 'shows inline images and attachments correctly' do
+      visit "/tickets/#{article.ticket_id}"
+
+      wait_for_gql 'apps/mobile/pages/ticket/graphql/queries/ticket.graphql'
+      expect(page).to have_text('Ticket Title')
+
+      # Inline image is present with cid: replaced by REST API URL.
+      expect(page).to have_css('img[src^="/api/v1/ticket_attachment"]')
+      # Inline image does not show in attachments list.
+      expect(page).to have_no_text('inline_image')
+      # Attached image does show in attachments list.
+      expect(page).to have_text('attached_image')
+    end
+  end
+end