Skip to content

Commit

Permalink
Asset checks polish (#20263)
Browse files Browse the repository at this point in the history
## Summary & Motivation
<img width="1727" alt="Screenshot 2024-03-05 at 1 42 03 PM"
src="https://github.com/dagster-io/dagster/assets/2286579/e1f51883-2b00-4d49-be73-f16c6c2f821b">

1. Lowercase "result" in "Evaluation result".
2. Move the collapse arrows to the right side
3. Don't show the latest execution in the table since we already show it
above

## How I Tested These Changes
  • Loading branch information
salazarm authored Mar 6, 2024
1 parent e526683 commit 5487ee2
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,48 @@ export const CollapsibleSection = ({
headerWrapperProps,
children,
isInitiallyCollapsed = false,
arrowSide = 'left',
}: {
header: React.ReactNode;
headerWrapperProps?: React.ComponentProps<typeof Box>;
children: React.ReactNode;
isInitiallyCollapsed?: boolean;
arrowSide?: 'left' | 'right';
}) => {
const [isCollapsed, setIsCollapsed] = React.useState(isInitiallyCollapsed);
return (
<Box flex={{direction: 'column'}}>
<Box
{...headerWrapperProps}
flex={{direction: 'row', alignItems: 'center', gap: 6, ...(headerWrapperProps?.flex || {})}}
flex={{
direction: 'row',
alignItems: 'center',
gap: 6,
grow: 1,
...(headerWrapperProps?.flex || {}),
}}
onClick={() => {
setIsCollapsed(!isCollapsed);
headerWrapperProps?.onClick?.();
}}
>
<Icon
name="arrow_drop_down"
style={{transform: isCollapsed ? 'rotate(-90deg)' : 'rotate(0deg)'}}
/>
<div>{header}</div>
{arrowSide === 'left' ? (
<>
<Icon
name="arrow_drop_down"
style={{transform: isCollapsed ? 'rotate(-90deg)' : 'rotate(0deg)'}}
/>
<div>{header}</div>
</>
) : (
<Box flex={{justifyContent: 'space-between', alignItems: 'center'}}>
<div>{header}</div>
<Icon
name="arrow_drop_down"
style={{transform: isCollapsed ? 'rotate(-90deg)' : 'rotate(0deg)'}}
/>
</Box>
)}
</Box>
{isCollapsed ? null : children}
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export const AutomaterializeMiddlePanelWithData = ({
<Box border="bottom" padding={{vertical: 12}} margin={{bottom: 12}}>
<div style={{display: 'grid', gridTemplateColumns: '1fr 1fr 1fr', gap: 24}}>
<Box flex={{direction: 'column', gap: 5}}>
<Subtitle2>Evaluation Result</Subtitle2>
<Subtitle2>Evaluation result</Subtitle2>
<div>{statusTag}</div>
</Box>
{selectedEvaluation?.timestamp ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {Timestamp} from '../../app/time/Timestamp';
import {AssetKeyInput} from '../../graphql/types';
import {useQueryPersistedState} from '../../hooks/useQueryPersistedState';
import {useStateWithStorage} from '../../hooks/useStateWithStorage';
import {MetadataEntries} from '../../metadata/MetadataEntry';
import {linkToRunEvent} from '../../runs/RunUtils';
import {useCursorPaginatedQuery} from '../../runs/useCursorPaginatedQuery';
import {TimestampDisplay} from '../../schedules/TimestampDisplay';
Expand Down Expand Up @@ -142,6 +143,8 @@ export const AssetChecks = ({
const lastExecution = selectedCheck.executionForLatestMaterialization;
const targetMaterialization = lastExecution?.evaluation?.targetMaterialization;

console.log({lastExecution});

return (
<Box flex={{grow: 1, direction: 'column'}}>
{didDismissAssetChecksBanner ? null : (
Expand Down Expand Up @@ -241,6 +244,7 @@ export const AssetChecks = ({
<CollapsibleSection
header={<Subtitle2>About</Subtitle2>}
headerWrapperProps={headerWrapperProps}
arrowSide="right"
>
<Box padding={{top: 12}} flex={{gap: 12, direction: 'column'}}>
<Body2>
Expand All @@ -267,11 +271,12 @@ export const AssetChecks = ({
<CollapsibleSection
header={<Subtitle2>Latest execution</Subtitle2>}
headerWrapperProps={headerWrapperProps}
arrowSide="right"
>
<Box padding={{top: 12}}>
<div style={{display: 'grid', gridTemplateColumns: '1fr 1fr 1fr', gap: 24}}>
<Box padding={{top: 12}} flex={{direction: 'column', gap: 12}}>
<div style={{display: 'grid', gridTemplateColumns: '1fr 1fr 1fr 1fr', gap: 24}}>
<Box flex={{direction: 'column', gap: 6}}>
<Subtitle2>Evaluation Result</Subtitle2>
<Subtitle2>Evaluation result</Subtitle2>
<div>
<AssetCheckStatusTag
execution={selectedCheck.executionForLatestMaterialization}
Expand Down Expand Up @@ -300,11 +305,18 @@ export const AssetChecks = ({
</Box>
) : null}
</div>
{lastExecution?.evaluation?.metadataEntries.length ? (
<Box flex={{direction: 'column', gap: 6}}>
<Subtitle2>Metadata</Subtitle2>
<MetadataEntries entries={lastExecution.evaluation.metadataEntries} />
</Box>
) : null}
</Box>
</CollapsibleSection>
<CollapsibleSection
header={<Subtitle2>Execution history</Subtitle2>}
headerWrapperProps={headerWrapperProps}
arrowSide="right"
>
<Box padding={{top: 12}}>
{lastExecution ? (
Expand Down Expand Up @@ -352,7 +364,11 @@ const CheckExecutions = ({assetKey, checkName}: {assetKey: AssetKeyInput; checkN
// TODO - in a follow up PR we should have some kind of queryRefresh context that can merge all of the uses of queryRefresh.
useQueryRefreshAtInterval(queryResult, FIFTEEN_SECONDS);

const executions = queryResult.data?.assetCheckExecutions;
const executions = React.useMemo(
// Remove first element since the latest execution info is already shown above
() => queryResult.data?.assetCheckExecutions.slice(1),
[queryResult],
);

const runHistory = () => {
if (!executions) {
Expand All @@ -363,7 +379,7 @@ const CheckExecutions = ({assetKey, checkName}: {assetKey: AssetKeyInput; checkN
<Table>
<thead>
<tr>
<th style={{width: '160px'}}>Evaluation Result</th>
<th style={{width: '160px'}}>Evaluation result</th>
<th style={{width: '200px'}}>Timestamp</th>
<th style={{width: '200px'}}>Target materialization</th>
<th>Metadata</th>
Expand Down Expand Up @@ -471,8 +487,8 @@ export const ASSET_CHECKS_QUERY = gql`
}
... on AssetChecks {
checks {
...AssetCheckTableFragment
...ExecuteChecksButtonCheckFragment
...AssetCheckTableFragment
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const TestAssetCheck = buildAssetCheck({
timestamp: testLatestMaterializationTimeStamp,
runId: testLatestMaterializationRunId,
}),
metadataEntries: [buildIntMetadataEntry({})],
metadataEntries: [],
severity: AssetCheckSeverity.ERROR,
}),
runId: testLatestMaterializationRunId,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 comments on commit 5487ee2

@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-storybook ready!

✅ Preview
https://dagit-storybook-lx204nua0-elementl.vercel.app

Built with commit 5487ee2.
This pull request is being automatically deployed with vercel-action

@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-arma16wjk-elementl.vercel.app

Built with commit 5487ee2.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.