Skip to content

Commit

Permalink
fix(app): avoid reodering in robot dashboard (#16583)
Browse files Browse the repository at this point in the history
Here are some interesting facts:
- Array.sort() mutates
- Array.reverse() mutates
- react query caches data

These facts combined to mean that in the ODD robot dashboard, protocols
view, and run history, we would almost always rerender multiple times
with rapidly-reordering data and move stuff out from under someone
trying to interact with us. I'm actually pretty surprised that these
would usually end up in the right order instead of exactly reversed.

The fix for this is to quit using those methods, and the way to do that
is to mark the data read only. We should do this for all data returned
by a react-query hook IMO.

## testing 
- [x] on the odd, you can navigate away from and back to the dashboard
(particularly via the protocols tab) and when you come back to the
dashboard all the cards will render in order once and not jump around

Closes RQA-3422
  • Loading branch information
sfoster1 authored Oct 24, 2024
1 parent 5c04eab commit 2b18f3f
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export interface GetRunsParams {
}

export interface Runs {
data: RunData[]
data: readonly RunData[]
links: RunsLinks
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function useHistoricRunDetails(

return allHistoricRuns == null
? []
: allHistoricRuns.data.sort(
: allHistoricRuns.data.toSorted(
(a, b) =>
new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,19 @@ describe('RecentProtocolRuns', () => {
})
it('renders table headers if there are runs', () => {
vi.mocked(useIsRobotViewable).mockReturnValue(true)
vi.mocked(useNotifyAllRunsQuery).mockReturnValue({
vi.mocked(useNotifyAllRunsQuery).mockReturnValue(({
data: {
data: [
data: ([
{
createdAt: '2022-05-04T18:24:40.833862+00:00',
current: false,
id: 'test_id',
protocolId: 'test_protocol_id',
status: 'succeeded',
},
],
] as any) as Runs,
},
} as UseQueryResult<Runs, AxiosError>)
} as any) as UseQueryResult<Runs, AxiosError>)
render()
screen.getByText('Recent Protocol Runs')
screen.getByText('Run')
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/ODD/ProtocolDashboard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export function ProtocolDashboard(): JSX.Element {
}

const runData = runs.data?.data != null ? runs.data?.data : []
const allRunsNewestFirst = runData.sort(
const allRunsNewestFirst = runData.toSorted(
(a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()
)
const sortedProtocols = sortProtocols(
Expand Down
2 changes: 1 addition & 1 deletion app/src/pages/ODD/RobotDashboard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function RobotDashboard(): JSX.Element {
)

const recentRunsOfUniqueProtocols = (allRunsQueryData?.data ?? [])
.reverse() // newest runs first
.toReversed() // newest runs first
.reduce<RunData[]>((acc, run) => {
if (
acc.some(collectedRun => collectedRun.protocolId === run.protocolId)
Expand Down

0 comments on commit 2b18f3f

Please sign in to comment.