Skip to content

Commit

Permalink
[fix-6165]: Hide PDFs from forward to external (#3017)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
alimpens authored Nov 25, 2024
1 parent a96b914 commit 185a6fd
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 25 deletions.
21 changes: 6 additions & 15 deletions app.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"showThorButton": false,
"showVulaanControls": false,
"useProjectenSignalType": false,
"enableForwardIncidentToExternal": false,
"enableForwardIncidentToExternal": true,
"showMainCategories": false,
"showStandardTextAdminV1": true,
"showStandardTextAdminV2": false,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -181,7 +172,7 @@
}
},
"frontPageAlert": {
"text":""
"text": ""
},
"additionalCode": {
"css": ""
Expand Down
14 changes: 14 additions & 0 deletions internals/mocks/fixtures/attachments.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>

Expand All @@ -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)}
Expand All @@ -86,7 +93,7 @@ const ExplanationSection = ({
</ImageWrapper>
) : null}
</Section>
)
) : null
}

export default ExplanationSection

0 comments on commit 185a6fd

Please sign in to comment.