Skip to content

Commit 2036b4a

Browse files
refac: defer queries until the AssetActionMenu is actually opened
1 parent 1491beb commit 2036b4a

5 files changed

Lines changed: 87 additions & 59 deletions

File tree

js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
/* eslint-disable @typescript-eslint/no-var-requires */
21
// eslint-disable-next-line no-restricted-imports
3-
import {Popover2, Popover2Props} from '@blueprintjs/popover2';
2+
import {
3+
Popover as BlueprintPopover,
4+
PopoverProps as BlueprintPopoverProps,
5+
} from '@blueprintjs/core';
46
import deepmerge from 'deepmerge';
5-
import * as React from 'react';
67
import {createGlobalStyle, css} from 'styled-components';
78

89
import {Colors} from './Color';
@@ -105,13 +106,9 @@ export const GlobalPopoverStyle = createGlobalStyle`
105106
// Overwrite arrays instead of concatting them.
106107
const overwriteMerge = (destination: any[], source: any[]) => source;
107108

108-
interface Props extends Popover2Props {
109-
children: React.ReactNode;
110-
}
111-
112-
export const Popover = (props: Props) => {
109+
export const Popover = (props: BlueprintPopoverProps) => {
113110
return (
114-
<Popover2
111+
<BlueprintPopover
115112
minimal
116113
autoFocus={false}
117114
enforceFocus={false}

js_modules/dagster-ui/packages/ui-core/src/assets/AssetActionMenu.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
HoverButton,
44
Icon,
55
Menu,
6+
MenuDivider,
67
MenuItem,
78
Popover,
89
Spinner,
@@ -24,6 +25,7 @@ import {showSharedToaster} from '../app/DomUtils';
2425
import {AssetKeyInput} from '../graphql/types';
2526
import {MenuLink} from '../ui/MenuLink';
2627
import {RepoAddress} from '../workspace/types';
28+
2729
interface Props {
2830
path: string[];
2931
definition: AssetTableDefinitionFragment | null;
@@ -89,7 +91,6 @@ export const AssetActionMenu = memo((props: Props) => {
8991
<Menu>
9092
{executeItem}
9193
<AddToFavoritesMenuItem assetKey={{path}} />
92-
9394
<MenuLink
9495
text="Show in group"
9596
to={
@@ -134,6 +135,7 @@ export const AssetActionMenu = memo((props: Props) => {
134135
/>
135136
))
136137
: undefined}
138+
<MenuDivider />
137139
{wipe.dropdownOptions}
138140
{deletePartitions.dropdownOptions}
139141
</Menu>

js_modules/dagster-ui/packages/ui-core/src/assets/useAssetPermissions.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,12 @@ export const useAssetPermissions = (assetKeys: AssetKeyInput[], locationName: st
5151
}
5252

5353
// Permission is allowed only if ALL assets allow it
54-
const hasMaterializePermission = assetNodes.every((node) => node.hasMaterializePermission);
55-
const hasWipePermission = assetNodes.every((node) => node.hasWipePermission);
56-
const hasReportRunlessAssetEventPermission = assetNodes.every(
57-
(node) => node.hasReportRunlessAssetEventPermission,
58-
);
59-
6054
return {
61-
hasMaterializePermission,
62-
hasWipePermission,
63-
hasReportRunlessAssetEventPermission,
55+
hasMaterializePermission: assetNodes.every((node) => node.hasMaterializePermission),
56+
hasWipePermission: assetNodes.every((node) => node.hasWipePermission),
57+
hasReportRunlessAssetEventPermission: assetNodes.every(
58+
(node) => node.hasReportRunlessAssetEventPermission,
59+
),
6460
loading,
6561
};
6662
}, [data, loading, locationLoading, fallbackPermissions, assetKeys]);
Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import {Colors, Icon, MenuItem} from '@dagster-io/ui-components';
2-
import {useContext, useState} from 'react';
2+
import {Dispatch, SetStateAction, useContext, useState} from 'react';
33

44
import {DeleteDynamicPartitionsDialog} from './DeleteDynamicPartitionsDialog';
55
import {useAssetPermissions} from './useAssetPermissions';
66
import {CloudOSSContext} from '../app/CloudOSSContext';
77
import {AssetKeyInput, PartitionDefinitionType} from '../graphql/types';
88
import {RepoAddress} from '../workspace/types';
99

10-
export function useDeleteDynamicPartitionsDialog(
10+
export const useDeleteDynamicPartitionsDialog = (
1111
opts: {
1212
repoAddress: RepoAddress;
1313
assetKey: AssetKeyInput;
@@ -20,8 +20,8 @@ export function useDeleteDynamicPartitionsDialog(
2020
};
2121
} | null,
2222
refresh?: () => void,
23-
) {
24-
const [showing, setShowing] = useState(false);
23+
) => {
24+
const [isShowing, setIsShowing] = useState(false);
2525

2626
const {
2727
featureContext: {canSeeWipeMaterializationAction},
@@ -31,19 +31,14 @@ export function useDeleteDynamicPartitionsDialog(
3131
(d) => d.type === PartitionDefinitionType.DYNAMIC,
3232
);
3333

34-
const {hasWipePermission} = useAssetPermissions(
35-
opts?.assetKey ? [opts.assetKey] : [],
36-
opts?.repoAddress.location || '',
37-
);
38-
3934
if (
4035
!opts ||
4136
!dynamicDimension?.dynamicPartitionsDefinitionName ||
4237
!canSeeWipeMaterializationAction
4338
) {
4439
return {
4540
element: undefined,
46-
dropdownOptions: [] as JSX.Element[],
41+
dropdownOptions: [],
4742
};
4843
}
4944

@@ -53,20 +48,44 @@ export function useDeleteDynamicPartitionsDialog(
5348
repoAddress={opts.repoAddress}
5449
assetKey={opts.assetKey}
5550
partitionsDefName={dynamicDimension.dynamicPartitionsDefinitionName}
56-
isOpen={showing}
57-
onClose={() => setShowing(false)}
51+
isOpen={isShowing}
52+
onClose={() => setIsShowing(false)}
5853
onComplete={refresh}
5954
/>
6055
),
6156
dropdownOptions: [
62-
<MenuItem
57+
<DeleteDynamicPartitionsMenuItem
6358
key="delete"
64-
text="Delete partitions"
65-
icon={<Icon name="delete" color={Colors.accentRed()} />}
66-
disabled={!hasWipePermission}
67-
intent="danger"
68-
onClick={() => setShowing(true)}
59+
setIsShowing={setIsShowing}
60+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
61+
locationName={opts?.repoAddress.location || ''}
6962
/>,
7063
],
7164
};
72-
}
65+
};
66+
67+
type DeleteDynamicPartitionsMenuItemProps = {
68+
assetKeys: AssetKeyInput[];
69+
locationName: string;
70+
setIsShowing: Dispatch<SetStateAction<boolean>>;
71+
};
72+
73+
const DeleteDynamicPartitionsMenuItem = ({
74+
assetKeys,
75+
locationName,
76+
setIsShowing,
77+
}: DeleteDynamicPartitionsMenuItemProps) => {
78+
// this separate comp exists to defer this expensive query until
79+
// the menuItem is rendered instead of when the hook is called
80+
const {hasWipePermission} = useAssetPermissions(assetKeys, locationName);
81+
82+
return (
83+
<MenuItem
84+
text="Delete partitions"
85+
icon={<Icon name="delete" color={Colors.accentRed()} />}
86+
disabled={!hasWipePermission}
87+
intent="danger"
88+
onClick={() => setIsShowing(true)}
89+
/>
90+
);
91+
};
Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,62 @@
1-
import {Colors, Icon, MenuDivider, MenuItem} from '@dagster-io/ui-components';
2-
import {useContext, useState} from 'react';
1+
import {Colors, Icon, MenuItem} from '@dagster-io/ui-components';
2+
import {Dispatch, SetStateAction, useContext, useState} from 'react';
33
import {AssetWipeDialog} from 'shared/assets/AssetWipeDialog.oss';
44

55
import {useAssetPermissions} from './useAssetPermissions';
66
import {CloudOSSContext} from '../app/CloudOSSContext';
77
import {AssetKeyInput} from '../graphql/types';
88

9-
export function useWipeDialog(
9+
export const useWipeDialog = (
1010
opts: {assetKey: AssetKeyInput; repository: {location: {name: string}} | null} | null,
11-
refresh?: () => void,
12-
) {
13-
const [showing, setShowing] = useState(false);
11+
refresh?: VoidFunction,
12+
) => {
13+
const [isShowing, setIsShowing] = useState(false);
1414

1515
const {
1616
featureContext: {canSeeWipeMaterializationAction},
1717
} = useContext(CloudOSSContext);
1818

19-
const {hasWipePermission} = useAssetPermissions(
20-
opts?.assetKey ? [opts.assetKey] : [],
21-
opts?.repository?.location.name || '',
22-
);
23-
2419
return {
2520
element: (
2621
<AssetWipeDialog
2722
assetKeys={opts ? [opts.assetKey] : []}
28-
isOpen={showing}
29-
onClose={() => setShowing(false)}
23+
isOpen={isShowing}
24+
onClose={() => setIsShowing(false)}
3025
onComplete={refresh}
3126
/>
3227
),
3328
dropdownOptions:
3429
opts && canSeeWipeMaterializationAction
3530
? [
36-
<MenuDivider key="wipe-divider" />,
37-
<MenuItem
31+
<WipeDialogMenuItem
3832
key="wipe"
39-
text="Wipe materializations"
40-
icon={<Icon name="delete" color={Colors.accentRed()} />}
41-
disabled={!hasWipePermission}
42-
intent="danger"
43-
onClick={() => setShowing(true)}
33+
setIsShowing={setIsShowing}
34+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
35+
locationName={opts?.repository?.location.name || ''}
4436
/>,
4537
]
4638
: [],
4739
};
48-
}
40+
};
41+
42+
type WipeDialogMenuItemProps = {
43+
assetKeys: AssetKeyInput[];
44+
locationName: string;
45+
setIsShowing: Dispatch<SetStateAction<boolean>>;
46+
};
47+
48+
const WipeDialogMenuItem = ({assetKeys, locationName, setIsShowing}: WipeDialogMenuItemProps) => {
49+
// this separate comp exists to defer this expensive query until
50+
// the menuItem is rendered instead of when the hook is called
51+
const {hasWipePermission} = useAssetPermissions(assetKeys, locationName);
52+
53+
return (
54+
<MenuItem
55+
text="Wipe materializationz"
56+
icon={<Icon name="delete" color={Colors.accentRed()} />}
57+
disabled={!hasWipePermission}
58+
intent="danger"
59+
onClick={() => setIsShowing(true)}
60+
/>
61+
);
62+
};

0 commit comments

Comments
 (0)