Skip to content

Commit

Permalink
fix(app): fix pinned protocol deletion
Browse files Browse the repository at this point in the history
The issue was that the right protocolId wasn't set correctly when tapping the delete button
  • Loading branch information
koji committed Sep 14, 2023
1 parent 552cd65 commit 5dc392c
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,19 @@ import {
Box,
ALIGN_CENTER,
} from '@opentrons/components'
import { useHost } from '@opentrons/react-api-client'
import { useHost, useProtocolQuery } from '@opentrons/react-api-client'

import { SmallButton } from '../../atoms/buttons'
import { Modal } from '../../molecules/Modal'

import type { ModalHeaderBaseProps } from '../../molecules/Modal/types'

interface DeleteProtocolConfirmationModalProps {
protocolName?: string
protocolId?: string
protocolId: string
setShowDeleteConfirmationModal: (showDeleteConfirmationModal: boolean) => void
}

export function DeleteProtocolConfirmationModal({
protocolName,
protocolId,
setShowDeleteConfirmationModal,
}: DeleteProtocolConfirmationModalProps): JSX.Element {
Expand All @@ -41,6 +39,10 @@ export function DeleteProtocolConfirmationModal({
}
const host = useHost()
const queryClient = useQueryClient()
const { data: protocolRecord } = useProtocolQuery(protocolId)
const protocolName =
protocolRecord?.data.metadata.protocolName ??
protocolRecord?.data.files[0].name

const handleCloseModal = (): void => {
setShowDeleteConfirmationModal(false)
Expand Down
3 changes: 3 additions & 0 deletions app/src/pages/ProtocolDashboard/LongPressModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ interface LongPressModalProps {
longpress: UseLongPressResult
protocolId: string
setShowDeleteConfirmationModal: (showDeleteConfirmationModal: boolean) => void
setTargetProtocolId: (targetProtocolId: string) => void
}

export function LongPressModal({
longpress,
protocolId,
setShowDeleteConfirmationModal,
setTargetProtocolId,
}: LongPressModalProps): JSX.Element {
const history = useHistory()
let pinnedProtocolIds = useSelector(getPinnedProtocolIds) ?? []
Expand Down Expand Up @@ -59,6 +61,7 @@ export function LongPressModal({
}

const handleDeleteClick = (): void => {
setTargetProtocolId(protocolId)
setShowDeleteConfirmationModal(true)
longpress.setIsLongPressed(false)
}
Expand Down
10 changes: 9 additions & 1 deletion app/src/pages/ProtocolDashboard/PinnedProtocol.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,15 @@ export function PinnedProtocol(props: {
cardSize?: CardSizeType
lastRun?: string
setShowDeleteConfirmationModal: (showDeleteConfirmationModal: boolean) => void
setTargetProtocolId: (targetProtocolId: string) => void
}): JSX.Element {
const { lastRun, protocol, longPress, setShowDeleteConfirmationModal } = props
const {
lastRun,
protocol,
longPress,
setShowDeleteConfirmationModal,
setTargetProtocolId,
} = props
const cardSize = props.cardSize ?? 'full'
const history = useHistory()
const longpress = useLongPress()
Expand Down Expand Up @@ -139,6 +146,7 @@ export function PinnedProtocol(props: {
<LongPressModal
longpress={longpress}
protocolId={protocol.id}
setTargetProtocolId={setTargetProtocolId}
setShowDeleteConfirmationModal={setShowDeleteConfirmationModal}
/>
)}
Expand Down
9 changes: 8 additions & 1 deletion app/src/pages/ProtocolDashboard/PinnedProtocolCarousel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ export function PinnedProtocolCarousel(props: {
pinnedProtocols: ProtocolResource[]
longPress: React.Dispatch<React.SetStateAction<boolean>>
setShowDeleteConfirmationModal: (showDeleteConfirmationModal: boolean) => void
setTargetProtocolId: (targetProtocolId: string) => void
}): JSX.Element {
const { pinnedProtocols, longPress, setShowDeleteConfirmationModal } = props
const {
pinnedProtocols,
longPress,
setShowDeleteConfirmationModal,
setTargetProtocolId,
} = props
const runs = useAllRunsQuery()
const cardSize = (): CardSizeType => {
let size: CardSizeType = 'regular'
Expand Down Expand Up @@ -46,6 +52,7 @@ export function PinnedProtocolCarousel(props: {
protocol={protocol}
longPress={longPress}
setShowDeleteConfirmationModal={setShowDeleteConfirmationModal}
setTargetProtocolId={setTargetProtocolId}
/>
)
})}
Expand Down
9 changes: 5 additions & 4 deletions app/src/pages/ProtocolDashboard/ProtocolCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ export function ProtocolCard(props: {
protocol: ProtocolResource
longPress: React.Dispatch<React.SetStateAction<boolean>>
setShowDeleteConfirmationModal: (showDeleteConfirmationModal: boolean) => void
setTargetProtocol: (targetProtocol: ProtocolResource) => void
setTargetProtocolId: (targetProtocolId: string) => void
lastRun?: string
}): JSX.Element {
const {
protocol,
lastRun,
longPress,
setShowDeleteConfirmationModal,
setTargetProtocol,
setTargetProtocolId,
} = props
const history = useHistory()
const [showIcon, setShowIcon] = React.useState<boolean>(false)
Expand Down Expand Up @@ -90,9 +90,9 @@ export function ProtocolCard(props: {
React.useEffect(() => {
if (longpress.isLongPressed) {
longPress(true)
setTargetProtocol(protocol)
setTargetProtocolId(protocol.id)
}
}, [longpress.isLongPressed, longPress])
}, [longpress.isLongPressed, longPress, protocol.id, setTargetProtocolId])

const failedAnalysisHeader: ModalHeaderBaseProps = {
title: i18n.format(t('protocol_analysis_failed'), 'capitalize'),
Expand Down Expand Up @@ -199,6 +199,7 @@ export function ProtocolCard(props: {
<LongPressModal
longpress={longpress}
protocolId={protocol.id}
setTargetProtocolId={setTargetProtocolId}
setShowDeleteConfirmationModal={setShowDeleteConfirmationModal}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
HostConfig,
} from '@opentrons/api-client'
import { renderWithProviders } from '@opentrons/components'
import { useHost } from '@opentrons/react-api-client'
import { useHost, useProtocolQuery } from '@opentrons/react-api-client'

import { i18n } from '../../../i18n'
import { DeleteProtocolConfirmationModal } from '../DeleteProtocolConfirmationModal'
Expand All @@ -18,14 +18,17 @@ jest.mock('@opentrons/api-client')
jest.mock('@opentrons/react-api-client')

const mockFunc = jest.fn()

const PROTOCOL_ID = 'mockProtocolId'
const MOCK_HOST_CONFIG = {} as HostConfig
const mockuseHost = useHost as jest.MockedFunction<typeof useHost>
const mockUseHost = useHost as jest.MockedFunction<typeof useHost>
const mockGetProtocol = getProtocol as jest.MockedFunction<typeof getProtocol>
const mockDeleteProtocol = deleteProtocol as jest.MockedFunction<
typeof deleteProtocol
>
const mockDeleteRun = deleteRun as jest.MockedFunction<typeof deleteRun>
const mockUseProtocolQuery = useProtocolQuery as jest.MockedFunction<
typeof useProtocolQuery
>

jest.mock('react-router-dom', () => {
const reactRouterDom = jest.requireActual('react-router-dom')
Expand All @@ -46,12 +49,20 @@ describe('DeleteProtocolConfirmationModal', () => {
let props: React.ComponentProps<typeof DeleteProtocolConfirmationModal>

beforeEach(() => {
when(mockuseHost).calledWith().mockReturnValue(MOCK_HOST_CONFIG)
props = {
protocolId: 'mockProtocol1',
protocolName: 'mockProtocol1',
protocolId: PROTOCOL_ID,
setShowDeleteConfirmationModal: mockFunc,
}
when(mockUseHost).calledWith().mockReturnValue(MOCK_HOST_CONFIG)
when(mockUseProtocolQuery)
.calledWith(PROTOCOL_ID)
.mockReturnValue({
data: {
data: {
metadata: { protocolName: 'mockProtocol1' },
},
},
} as any)
})

afterEach(() => {
Expand All @@ -75,7 +86,7 @@ describe('DeleteProtocolConfirmationModal', () => {

it('should call a mock function when tapping delete button', async () => {
when(mockGetProtocol)
.calledWith(MOCK_HOST_CONFIG, 'mockProtocol1')
.calledWith(MOCK_HOST_CONFIG, PROTOCOL_ID)
.mockResolvedValue({
data: { links: { referencingRuns: [{ id: '1' }, { id: '2' }] } },
} as any)
Expand All @@ -91,7 +102,7 @@ describe('DeleteProtocolConfirmationModal', () => {
expect(mockDeleteRun).toHaveBeenCalledWith(MOCK_HOST_CONFIG, '2')
expect(mockDeleteProtocol).toHaveBeenCalledWith(
MOCK_HOST_CONFIG,
'mockProtocol1'
PROTOCOL_ID
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const mockUseCreateRunMutation = useCreateRunMutation as jest.MockedFunction<
>
const mockuseHost = useHost as jest.MockedFunction<typeof useHost>
const mockFunc = jest.fn()
const mockSetTargetProtocolId = jest.fn()
jest.mock('react-router-dom', () => {
const reactRouterDom = jest.requireActual('react-router-dom')
return {
Expand All @@ -36,6 +37,7 @@ const render = (longPress: UseLongPressResult) => {
longpress={longPress}
protocolId={'mockProtocol1'}
setShowDeleteConfirmationModal={mockFunc}
setTargetProtocolId={mockSetTargetProtocolId}
/>
</MemoryRouter>,
{
Expand Down Expand Up @@ -66,6 +68,7 @@ describe('Long Press Modal', () => {
const [{ getByText }] = render(result.current)
const button = getByText('Delete protocol')
button.click()
expect(mockSetTargetProtocolId).toHaveBeenCalledWith('mockProtocol1')
expect(mockFunc).toHaveBeenCalled()
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const props = {
protocol: mockProtocol,
longPress: jest.fn(),
setShowDeleteConfirmationModal: jest.fn(),
setTargetProtocolId: jest.fn(),
}

const render = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const props = {
longPress: jest.fn(),
setTargetProtocol: jest.fn(),
setShowDeleteConfirmationModal: jest.fn(),
setTargetProtocolId: jest.fn(),
}

const render = () => {
Expand Down
15 changes: 4 additions & 11 deletions app/src/pages/ProtocolDashboard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ export function ProtocolDashboard(): JSX.Element {
showDeleteConfirmationModal,
setShowDeleteConfirmationModal,
] = React.useState<boolean>(false)
const [
targetProtocol,
setTargetProtocol,
] = React.useState<ProtocolResource | null>(null)
const [targetProtocolId, setTargetProtocolId] = React.useState<string>('')
const sortBy = useSelector(getProtocolsOnDeviceSortKey) ?? 'alphabetical'
const protocolsData = protocols.data?.data ?? []
let unpinnedProtocols: ProtocolResource[] = protocolsData
Expand Down Expand Up @@ -134,12 +131,7 @@ export function ProtocolDashboard(): JSX.Element {
<>
{showDeleteConfirmationModal ? (
<DeleteProtocolConfirmationModal
protocolId={targetProtocol?.id}
protocolName={
targetProtocol?.metadata.protocolName != null
? targetProtocol.metadata.protocolName
: targetProtocol?.files[0].name
}
protocolId={targetProtocolId}
setShowDeleteConfirmationModal={setShowDeleteConfirmationModal}
/>
) : null}
Expand Down Expand Up @@ -170,6 +162,7 @@ export function ProtocolDashboard(): JSX.Element {
pinnedProtocols={pinnedProtocols}
longPress={setLongPressModalOpened}
setShowDeleteConfirmationModal={setShowDeleteConfirmationModal}
setTargetProtocolId={setTargetProtocolId}
/>
</Flex>
)}
Expand Down Expand Up @@ -263,7 +256,7 @@ export function ProtocolDashboard(): JSX.Element {
setShowDeleteConfirmationModal={
setShowDeleteConfirmationModal
}
setTargetProtocol={setTargetProtocol}
setTargetProtocolId={setTargetProtocolId}
/>
)
})}
Expand Down

0 comments on commit 5dc392c

Please sign in to comment.