[fix-6165]: Hide PDFs from forward to external (#3017)

* Hide PDFs when forwarding melding to external

* Add ExplanationSection tests

* Add forwardToExternal test

* Simplify changes to fixture

* Revert change to fixture, fix tests instead

* Fix typo

Author: alimpens

app.base.json

@@ -20,7 +20,7 @@
     "showThorButton": false,
     "showVulaanControls": false,
     "useProjectenSignalType": false,
-    "enableForwardIncidentToExternal": false,
+    "enableForwardIncidentToExternal": true,
     "showMainCategories": false,
     "showStandardTextAdminV1": true,
     "showStandardTextAdminV2": false,
@@ -72,21 +72,12 @@
     "municipality": "amsterdam",
     "options": {
       "crs": "EPSG:28992",
-      "center": [
-        52.3731081,
-        4.8932945
-      ],
+      "center": [52.3731081, 4.8932945],
       "maxBounds": [
-        [
-          52.25168,
-          4.64034
-        ],
-        [
-          52.50536,
-          5.10737
-        ]
+        [52.25168, 4.64034],
+        [52.50536, 5.10737]
       ],
-      "maxNumberOfAssets" : {
+      "maxNumberOfAssets": {
         "straatverlichting": 3,
         "klokken": 1,
         "afvalContainer": 5,
@@ -181,7 +172,7 @@
     }
   },
   "frontPageAlert": {
-    "text":""
+    "text": ""
   },
   "additionalCode": {
     "css": ""

internals/mocks/fixtures/attachments.json

@@ -53,6 +53,20 @@
       "created_by": "me",
       "public": true,
       "caption": ""
+    },
+    {
+      "_display": "Attachment object (983)",
+      "_links": {
+        "self": {
+          "href": "http://localhost:8000/signals/v1/private/signals/63/attachments/88"
+        }
+      },
+      "location": "https://not-an-image.pdf",
+      "is_image": false,
+      "created_at": "2022-06-10T11:51:24.281272+02:00",
+      "created_by": "me",
+      "public": true,
+      "caption": ""
     }
   ]
 }

src/signals/incident-management/containers/IncidentDetail/components/Attachments/Attachments.test.tsx

@@ -503,7 +503,7 @@ describe('Attachments', () => {
         )
       )
 
-      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(3)
+      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(4)
     })
 
     it('shows the delete button when allowed from others and for normal incidents', () => {
@@ -529,7 +529,7 @@ describe('Attachments', () => {
         )
       )
 
-      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(3)
+      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(4)
     })
 
     it('shows the delete button when allowed from others and for child incidents', () => {
@@ -555,7 +555,7 @@ describe('Attachments', () => {
         )
       )
 
-      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(3)
+      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(4)
     })
 
     it('shows the delete button when allowed from others and for parent incidents', () => {
@@ -581,7 +581,7 @@ describe('Attachments', () => {
         )
       )
 
-      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(3)
+      expect(screen.getAllByTitle(/bijlage verwijderen/i)).toHaveLength(4)
     })
 
     it('shows the delete button when its your own attachment and allowed for normal incidents', () => {

src/signals/incident-management/containers/IncidentDetail/components/ForwardToExternal/ForwardToExternal.test.tsx

@@ -215,6 +215,25 @@ describe('ForwardToExternal', () => {
         })
         expect(closeSpy).toHaveBeenCalled()
       })
+
+      it('only shows image attachments', async () => {
+        render(
+          withAppContext(
+            <IncidentDetailContext.Provider
+              value={{
+                update: jest.fn(),
+                attachments: attachmentsFixture,
+              }}
+            >
+              <ForwardToExternal onClose={jest.fn()} />
+            </IncidentDetailContext.Provider>
+          )
+        )
+
+        const images = screen.getAllByRole('img')
+
+        expect(images).toHaveLength(3)
+      })
     })
   })
 })

src/signals/incident-management/containers/IncidentDetail/components/ForwardToExternal/ForwardToExternal.tsx

@@ -135,6 +135,11 @@ const ForwardToExternal = ({ onClose }: ForwardToExternalProps) => {
     }
   }, [emailTemplateError, storeDispatch])
 
+  // Only show image attachments, pdf attachments are not supported
+  const imageAttachments = attachments?.results.filter(
+    (attachment) => attachment.is_image
+  )
+
   return (
     <Form
       ref={formRef}
@@ -182,11 +187,11 @@ const ForwardToExternal = ({ onClose }: ForwardToExternalProps) => {
         </ErrorWrapper>
       </StyledSection>
 
-      {attachments?.count ? (
+      {imageAttachments && imageAttachments.length > 0 ? (
         <StyledSection>
           <StyledParagraph strong>Foto&apos;s</StyledParagraph>
           <ImageWrapper>
-            {attachments.results.map((attachment) => (
+            {imageAttachments.map((attachment) => (
               <Image
                 key={attachment.location}
                 src={attachment.location}

src/signals/incident/containers/ExternalReplyContainer/components/ExplanationSection/ExplanationSection.test.tsx

@@ -69,4 +69,51 @@ describe('ExplanationSection', () => {
 
     expect(selectFileSpy).toHaveBeenCalledWith(file)
   })
+
+  it('shows a section with only text', () => {
+    render(<ExplanationSection title="Foo" text={'Bar\nBaz'} />)
+
+    const heading = screen.getByRole('heading', { name: 'Foo' })
+
+    expect(heading).toBeInTheDocument()
+  })
+
+  it('shows a section with only files', () => {
+    render(<ExplanationSection title="Foo" text={null} files={files} />)
+
+    const heading = screen.getByRole('heading', { name: 'Foo' })
+
+    expect(heading).toBeInTheDocument()
+  })
+
+  it('does not show when there is no text and files', () => {
+    render(<ExplanationSection title="Foo" text={null} />)
+
+    const heading = screen.queryByRole('heading', { name: 'Foo' })
+
+    expect(heading).not.toBeInTheDocument()
+  })
+
+  it('does not show when there is no text and only PDF files', () => {
+    const pdfFiles = [{ description: 'pdf', file: 'file.pdf' }]
+
+    render(<ExplanationSection title="Foo" text={null} files={pdfFiles} />)
+
+    const heading = screen.queryByRole('heading', { name: 'Foo' })
+
+    expect(heading).not.toBeInTheDocument()
+  })
+
+  it('filters out PDF files', () => {
+    const filesWithPdf = [
+      { description: 'image', file: 'file.jpg' },
+      { description: 'pdf', file: 'file.pdf' },
+    ]
+
+    render(<ExplanationSection title="Foo" text={null} files={filesWithPdf} />)
+
+    const images = screen.getAllByRole('img')
+
+    expect(images).toHaveLength(1)
+  })
 })

src/signals/incident/containers/ExternalReplyContainer/components/ExplanationSection/ExplanationSection.tsx

@@ -4,6 +4,7 @@ import { Heading, themeSpacing } from '@amsterdam/asc-ui'
 import styled from 'styled-components'
 
 import Paragraph from 'components/Paragraph'
+import { isPdf } from 'signals/incident-management/containers/IncidentDetail/utils/isPdf'
 
 type File = {
   description: string
@@ -56,7 +57,13 @@ const ExplanationSection = ({
       onSelectFile(file)
     }
   }
-  return (
+
+  // Only show image files, pdf files are not supported
+  const imageFiles = files.filter(({ file }) => !isPdf(file))
+
+  const showSection = text || imageFiles.length > 0
+
+  return showSection ? (
     <Section className={className}>
       <StyledHeading forwardedAs="h4">{title}</StyledHeading>
 
@@ -67,9 +74,9 @@ const ExplanationSection = ({
           </Paragraph>
         ))}
 
-      {files.length > 0 ? (
+      {imageFiles.length > 0 ? (
         <ImageWrapper>
-          {files.map((file) => (
+          {imageFiles.map((file) => (
             <Image
               tabIndex={0}
               onKeyDown={handleImageKeyPress(file)}
@@ -86,7 +93,7 @@ const ExplanationSection = ({
         </ImageWrapper>
       ) : null}
     </Section>
-  )
+  ) : null
 }
 
 export default ExplanationSection