Skip to content

Commit 5397228

Browse files
refac: defer queries until the AssetActionMenu is actually opened (#33078)
## Summary & Motivation Hammer the backend less with `ASSETS_PERMISSIONS_QUERY`s ## How I Tested These Changes Checked visually and logged the rendering behavior of the relevant components with console.log
1 parent 376ce74 commit 5397228

5 files changed

Lines changed: 92 additions & 57 deletions

File tree

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

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

79
import {Colors} from './Color';
@@ -104,13 +106,9 @@ export const GlobalPopoverStyle = createGlobalStyle`
104106
// Overwrite arrays instead of concatting them.
105107
const overwriteMerge = (destination: any[], source: any[]) => source;
106108

107-
interface Props extends Popover2Props {
108-
children: React.ReactNode;
109-
}
110-
111-
export const Popover = (props: Props) => {
109+
export const Popover = (props: BlueprintPopoverProps) => {
112110
return (
113-
<Popover2
111+
<BlueprintPopover
114112
minimal
115113
autoFocus={false}
116114
enforceFocus={false}

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

Lines changed: 11 additions & 3 deletions
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;
@@ -70,6 +72,9 @@ export const AssetActionMenu = memo((props: Props) => {
7072
}
7173
: null,
7274
);
75+
76+
const additionalMenuItems = [...wipe.dropdownOptions, ...deletePartitions.dropdownOptions];
77+
7378
return (
7479
<>
7580
{launchpadElement}
@@ -89,7 +94,6 @@ export const AssetActionMenu = memo((props: Props) => {
8994
<Menu>
9095
{executeItem}
9196
<AddToFavoritesMenuItem assetKey={{path}} />
92-
9397
<MenuLink
9498
text="Show in group"
9599
to={
@@ -134,8 +138,12 @@ export const AssetActionMenu = memo((props: Props) => {
134138
/>
135139
))
136140
: undefined}
137-
{wipe.dropdownOptions}
138-
{deletePartitions.dropdownOptions}
141+
{additionalMenuItems.length && (
142+
<>
143+
<MenuDivider />
144+
{additionalMenuItems}
145+
</>
146+
)}
139147
</Menu>
140148
}
141149
>

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]);

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

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ 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+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
60+
locationName={opts?.repoAddress.location || ''}
61+
onClick={() => setIsShowing(true)}
6962
/>,
7063
],
7164
};
72-
}
65+
};
66+
67+
type DeleteDynamicPartitionsMenuItemProps = {
68+
assetKeys: AssetKeyInput[];
69+
locationName: string;
70+
onClick: () => void;
71+
};
72+
73+
const DeleteDynamicPartitionsMenuItem = ({
74+
assetKeys,
75+
locationName,
76+
onClick,
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={onClick}
89+
/>
90+
);
91+
};
Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,62 @@
1-
import {Colors, Icon, MenuDivider, MenuItem} from '@dagster-io/ui-components';
1+
import {Colors, Icon, MenuItem} from '@dagster-io/ui-components';
22
import {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,
1111
refresh?: () => void,
12-
) {
13-
const [showing, setShowing] = useState(false);
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+
assetKeys={opts?.assetKey ? [opts.assetKey] : []}
34+
locationName={opts?.repository?.location.name || ''}
35+
onClick={() => setIsShowing(true)}
4436
/>,
4537
]
4638
: [],
4739
};
48-
}
40+
};
41+
42+
type WipeDialogMenuItemProps = {
43+
assetKeys: AssetKeyInput[];
44+
locationName: string;
45+
onClick: () => void;
46+
};
47+
48+
const WipeDialogMenuItem = ({assetKeys, locationName, onClick}: 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 materializations"
56+
icon={<Icon name="delete" color={Colors.accentRed()} />}
57+
disabled={!hasWipePermission}
58+
intent="danger"
59+
onClick={onClick}
60+
/>
61+
);
62+
};

0 commit comments

Comments
 (0)