Skip to content

Commit

Permalink
fix(ChatDisplay/index.tsx): users are only able to download a protocol (
Browse files Browse the repository at this point in the history
#17015)

<!--
Thanks for taking the time to open a Pull Request (PR)! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

GitHub provides robust markdown to format your PR. Links, diagrams,
pictures, and videos along with text formatting make it possible to
create a rich and informative PR. For more information on GitHub
markdown, see:


https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview
Previously, when users clicked the download button, it created a Python
file even for an empty string.
Now, the system downloads only valid protocols that are not single-line
and must include `def run(`.

Closes AUTH-1064
<!--
Describe your PR at a high level. State acceptance criteria and how this
PR fits into other work. Link issues, PRs, and other relevant resources.
-->

## Test Plan and Hands on Testing
- Go to OpentronsAI.
- Generate a protocol and download it. You should see a Python file
containing the protocol.
- Simulate the protocol and get feedback.
- Click Download again, but this time, you should not be able to
download the file if the protocol is invalid.
<!--
Describe your testing of the PR. Emphasize testing not reflected in the
code. Attach protocols, logs, screenshots and any other assets that
support your testing.
-->

## Changelog
- Modified the functionality of the Download button to ensure only valid
protocols can be downloaded.

<!--
List changes introduced by this PR considering future developers and the
end user. Give careful thought and clear documentation to breaking
changes.
-->

## Review requests
Look at the section `Test Plan and Hands on Testing`
<!--
- What do you need from reviewers to feel confident this PR is ready to
merge?
- Ask questions.
-->

## Risk assessment
Low
<!--
- Indicate the level of attention this PR needs.
- Provide context to guide reviewers.
- Discuss trade-offs, coupling, and side effects.
- Look for the possibility, even if you think it's small, that your
change may affect some other part of the system.
- For instance, changing return tip behavior may also change the
behavior of labware calibration.
- How do your unit tests and on hands on testing mitigate this PR's
risks and the risk of future regressions?
- Especially in high risk PRs, explain how you know your testing is
enough.
-->
  • Loading branch information
Elyorcv authored Dec 5, 2024
1 parent 35120d3 commit c2816ce
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ describe('ChatDisplay', () => {
})

it('should call trackEvent when download button is clicked', () => {
props.chat = {
...props.chat,
role: 'assistant',
reply:
'```python\ndef run(protocol):\n print("hello")\n print("protocol")\n return True\n```',
}
URL.createObjectURL = vi.fn()
window.URL.revokeObjectURL = vi.fn()
HTMLAnchorElement.prototype.click = vi.fn()
Expand All @@ -110,6 +116,24 @@ describe('ChatDisplay', () => {
})
})

it('should not call trackEvent when download button is clicked', () => {
URL.createObjectURL = vi.fn()
window.URL.revokeObjectURL = vi.fn()
HTMLAnchorElement.prototype.click = vi.fn()

render(props)
// eslint-disable-next-line testing-library/no-node-access, @typescript-eslint/non-nullable-type-assertion-style
const downloadPath = document.querySelector(
'[aria-roledescription="download"]'
) as Element
fireEvent.click(downloadPath)

expect(mockUseTrackEvent).not.toHaveBeenCalledWith({
name: 'download-protocol',
properties: {},
})
})

it('should call trackEvent when copy button is clicked', async () => {
Object.defineProperty(navigator, 'clipboard', {
value: {
Expand Down
13 changes: 12 additions & 1 deletion opentrons-ai-client/src/molecules/ChatDisplay/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,18 @@ export function ChatDisplay({ chat, chatId }: ChatDisplayProps): JSX.Element {

const handleFileDownload = (): void => {
const lastCodeBlock = document.querySelector(`#${chatId}`)
const code = lastCodeBlock?.textContent ?? ''
const code = lastCodeBlock?.textContent?.trim() ?? ''
// Don't proceed if code is empty, no need to download as a python file
if (!code) {
return
}
// Make sure python protocol is valid
const hasRunFunction = code.includes('def run(')
const numberOfLines = code.split('\n').length
if (!hasRunFunction || numberOfLines <= 3) {
return
}

const blobParts: BlobPart[] = [code]

const file = new File(blobParts, 'OpentronsAI.py', { type: 'text/python' })
Expand Down

0 comments on commit c2816ce

Please sign in to comment.