Skip to content

Commit

Permalink
[ui] Sensor history: Fix tick ID variable (#23802)
Browse files Browse the repository at this point in the history
## Summary & Motivation

There is a bug on the sensor tick table where clicking to view a run
requested for a tick can show information about an unrelated tick.

This is because sensor tick IDs can be larger than JavaScript's
`Number.MAX_SAFE_INTEGER`, but they're being cast to `Number` for the
GraphQL variable. JS may cast the value to a number that doesn't match
the intended value, resulting in the wrong tick ID being sent in the
query.

We shouldn't be trying to represent them as `number` in JS. The GraphQL
input type expects it to be a BigInt, and we can just pass a string for
that.

## How I Tested These Changes

View sensor tick history with ticks that have tick IDs being cast to
incorrect values. Verify that the values are now correct, and that when
I click to view runs requested for the tick, the query variable and
rendered output are correct.
  • Loading branch information
hellendag authored Aug 22, 2024
1 parent e8f3f1d commit 9e2898c
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const TickDetailsDialog = ({tickId, isOpen, instigationSelector, onClose}
};

interface InnerProps {
tickId: number | undefined;
tickId: string | undefined;
instigationSelector: InstigationSelector;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,9 @@ export const TickHistoryTimeline = ({
afterTimestamp?: number;
statuses?: InstigationTickStatus[];
}) => {
const [selectedTickId, setSelectedTickId] = useQueryPersistedState<number | undefined>({
const [selectedTickId, setSelectedTickId] = useQueryPersistedState<string | undefined>({
encode: (tickId) => ({tickId}),
decode: (qs) => (qs['tickId'] ? Number(qs['tickId']) : undefined),
decode: (qs) => qs['tickId'] ?? undefined,
});

const [pollingPaused, pausePolling] = React.useState<boolean>(false);
Expand Down Expand Up @@ -364,7 +364,7 @@ export const TickHistoryTimeline = ({
const {ticks = []} = data.instigationStateOrError;

const onTickClick = (tick?: InstigationTick) => {
setSelectedTickId(tick ? Number(tick.tickId) : undefined);
setSelectedTickId(tick ? tick.tickId : undefined);
};

const onTickHover = (tick?: InstigationTick) => {
Expand All @@ -376,6 +376,7 @@ export const TickHistoryTimeline = ({
pausePolling(true);
}
};

return (
<>
<TickDetailsDialog
Expand Down Expand Up @@ -512,7 +513,7 @@ function TickRow({
) : null}
<TickDetailsDialog
isOpen={showResults}
tickId={Number(tick.tickId)}
tickId={tick.tickId}
instigationSelector={instigationSelector}
onClose={() => {
setShowResults(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const TickTagForRun = ({instigationSelector, instigationType, tickId}: Pr
isOpen={isOpen}
onClose={() => setIsOpen(false)}
instigationSelector={instigationSelector}
tickId={Number(tickId)}
tickId={tickId}
/>
</>
);
Expand Down

1 comment on commit 9e2898c

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-6ja9bo9uh-elementl.vercel.app

Built with commit 9e2898c.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.