Skip to content

Commit 47651e6

Browse files
salazarmsalazarm
authored
[1/n]Remove conditional selection syntax flag gating in asset graph. (#29258)
## Summary & Motivation The feature flag is now always enabled and the ability to turn it off to be disabled. What's left is removing all of the conditional logic. There is a lot of it so I'm going to do it over multiple PRs. This is part 1 and removes the selection syntax from the asset graph (and another spot or two). ## How I Tested These Changes Just ran local host cloud for this one
1 parent c589442 commit 47651e6

10 files changed

+95
-332
lines changed

Diff for: ‎js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetGraphExplorer.tsx

+18-122
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@ import pickBy from 'lodash/pickBy';
1515
import uniq from 'lodash/uniq';
1616
import without from 'lodash/without';
1717
import * as React from 'react';
18-
import {useCallback, useLayoutEffect, useMemo, useRef, useState} from 'react';
19-
import {FeatureFlag} from 'shared/app/FeatureFlags.oss';
20-
import {AssetGraphAssetSelectionInput} from 'shared/asset-graph/AssetGraphAssetSelectionInput.oss';
21-
import {useAssetGraphExplorerFilters} from 'shared/asset-graph/useAssetGraphExplorerFilters.oss';
18+
import {useCallback, useMemo, useRef, useState} from 'react';
2219
import {AssetSelectionInput} from 'shared/asset-selection/input/AssetSelectionInput.oss';
2320
import {CreateCatalogViewButton} from 'shared/assets/CreateCatalogViewButton.oss';
2421
import styled from 'styled-components';
@@ -53,18 +50,15 @@ import {
5350
useFullAssetGraphData,
5451
} from './useAssetGraphData';
5552
import {AssetLocation, useFindAssetLocation} from './useFindAssetLocation';
56-
import {featureEnabled} from '../app/Flags';
5753
import {AssetLiveDataRefreshButton} from '../asset-data/AssetLiveDataProvider';
5854
import {LaunchAssetExecutionButton} from '../assets/LaunchAssetExecutionButton';
5955
import {AssetKey} from '../assets/types';
6056
import {DEFAULT_MAX_ZOOM} from '../graph/SVGConsts';
6157
import {SVGViewport, SVGViewportRef} from '../graph/SVGViewport';
6258
import {useAssetLayout} from '../graph/asyncGraphLayout';
6359
import {closestNodeInDirection, isNodeOffscreen} from '../graph/common';
64-
import {AssetGroupSelector} from '../graphql/types';
6560
import {usePreviousDistinctValue} from '../hooks/usePrevious';
6661
import {useQueryAndLocalStoragePersistedState} from '../hooks/useQueryAndLocalStoragePersistedState';
67-
import {useUpdatingRef} from '../hooks/useUpdatingRef';
6862
import {
6963
GraphExplorerOptions,
7064
OptionsOverlay,
@@ -80,7 +74,6 @@ import {
8074
} from '../pipelines/GraphNotices';
8175
import {ExplorerPath} from '../pipelines/PipelinePathUtils';
8276
import {SyntaxError} from '../selection/CustomErrorListener';
83-
import {StaticSetFilter} from '../ui/BaseFilters/useStaticSetFilter';
8477
import {IndeterminateLoadingBar} from '../ui/IndeterminateLoadingBar';
8578
import {LoadingSpinner} from '../ui/Loading';
8679
import {isIframe} from '../util/isIframe';
@@ -105,71 +98,19 @@ type Props = {
10598
export const MINIMAL_SCALE = 0.6;
10699
export const GROUPS_ONLY_SCALE = 0.15;
107100

108-
const DEFAULT_SET_HIDE_NODES_MATCH = (_node: AssetNodeForGraphQueryFragment) => true;
109-
110101
export const AssetGraphExplorer = React.memo((props: Props) => {
111102
const {fullAssetGraphData: currentFullAssetGraphData} = useFullAssetGraphData(props.fetchOptions);
112103
const previousFullAssetGraphData = usePreviousDistinctValue(currentFullAssetGraphData);
113104

114105
const fullAssetGraphData = currentFullAssetGraphData ?? previousFullAssetGraphData;
115-
const [hideNodesMatching, setHideNodesMatching] = useState(() => DEFAULT_SET_HIDE_NODES_MATCH);
116106

117107
const {
118108
loading: graphDataLoading,
119109
fetchResult,
120110
assetGraphData: currentAssetGraphData,
121111
graphQueryItems: currentGraphQueryItems,
122112
allAssetKeys: currentAllAssetKeys,
123-
} = useAssetGraphData(
124-
props.explorerPath.opsQuery,
125-
useMemo(
126-
() => ({
127-
...props.fetchOptions,
128-
hideNodesMatching: (node) => {
129-
let hide = false;
130-
if (props.fetchOptions.hideNodesMatching?.(node)) {
131-
hide = true;
132-
}
133-
if (hideNodesMatching(node)) {
134-
hide = true;
135-
}
136-
return hide;
137-
},
138-
}),
139-
[props.fetchOptions, hideNodesMatching],
140-
),
141-
);
142-
143-
const {explorerPath, onChangeExplorerPath} = props;
144-
145-
const explorerPathRef = useUpdatingRef(explorerPath);
146-
147-
const {button, filterBar, groupsFilter, kindFilter, filterFn, filteredAssetsLoading} =
148-
useAssetGraphExplorerFilters({
149-
nodes: React.useMemo(
150-
() => (fullAssetGraphData ? Object.values(fullAssetGraphData.nodes) : []),
151-
[fullAssetGraphData],
152-
),
153-
loading: graphDataLoading,
154-
viewType: props.viewType,
155-
assetSelection: explorerPath.opsQuery,
156-
setAssetSelection: React.useCallback(
157-
(assetSelection: string) => {
158-
onChangeExplorerPath(
159-
{
160-
...explorerPathRef.current,
161-
opsQuery: assetSelection,
162-
},
163-
'push',
164-
);
165-
},
166-
[explorerPathRef, onChangeExplorerPath],
167-
),
168-
});
169-
170-
useLayoutEffect(() => {
171-
setHideNodesMatching(() => (node: AssetNodeForGraphQueryFragment) => !filterFn(node));
172-
}, [filterFn]);
113+
} = useAssetGraphData(props.explorerPath.opsQuery, props.fetchOptions);
173114

174115
const previousAssetGraphData = usePreviousDistinctValue(currentAssetGraphData);
175116
const previousGraphQueryItems = usePreviousDistinctValue(currentGraphQueryItems);
@@ -179,10 +120,7 @@ export const AssetGraphExplorer = React.memo((props: Props) => {
179120
const graphQueryItems = currentGraphQueryItems ?? previousGraphQueryItems;
180121
const allAssetKeys = currentAllAssetKeys ?? previousAllAssetKeys;
181122

182-
if (
183-
(fetchResult.loading || graphDataLoading || filteredAssetsLoading) &&
184-
(!assetGraphData || !allAssetKeys)
185-
) {
123+
if ((fetchResult.loading || graphDataLoading) && (!assetGraphData || !allAssetKeys)) {
186124
return <LoadingSpinner purpose="page" />;
187125
}
188126

@@ -203,11 +141,7 @@ export const AssetGraphExplorer = React.memo((props: Props) => {
203141
fullAssetGraphData={fullAssetGraphData ?? assetGraphData}
204142
allAssetKeys={allAssetKeys}
205143
graphQueryItems={graphQueryItems}
206-
filterBar={filterBar}
207-
filterButton={button}
208-
kindFilter={kindFilter}
209-
groupsFilter={groupsFilter}
210-
loading={filteredAssetsLoading || graphDataLoading || fetchResult.loading}
144+
loading={graphDataLoading || fetchResult.loading}
211145
{...props}
212146
/>
213147
);
@@ -220,12 +154,7 @@ type WithDataProps = Props & {
220154
graphQueryItems: AssetGraphQueryItem[];
221155
loading: boolean;
222156

223-
filterButton: React.ReactNode;
224-
filterBar: React.ReactNode;
225157
viewType: AssetGraphViewType;
226-
227-
kindFilter: StaticSetFilter<string>;
228-
groupsFilter: StaticSetFilter<AssetGroupSelector>;
229158
};
230159

231160
const AssetGraphExplorerWithData = ({
@@ -239,11 +168,7 @@ const AssetGraphExplorerWithData = ({
239168
graphQueryItems,
240169
fetchOptions,
241170
allAssetKeys,
242-
filterButton,
243-
filterBar,
244171
viewType,
245-
kindFilter,
246-
groupsFilter,
247172
loading: dataLoading,
248173
setHideEdgesToNodesOutsideQuery,
249174
}: WithDataProps) => {
@@ -501,21 +426,9 @@ const AssetGraphExplorerWithData = ({
501426
const [showSidebar, setShowSidebar] = React.useState(viewType === 'global');
502427

503428
const onFilterToGroup = (group: AssetGroup | GroupLayout) => {
504-
if (featureEnabled(FeatureFlag.flagSelectionSyntax)) {
505-
onChangeAssetSelection(
506-
`group:"${group.groupName}" and code_location:"${group.repositoryLocationName}"`,
507-
);
508-
} else {
509-
groupsFilter?.setState(
510-
new Set([
511-
{
512-
groupName: group.groupName,
513-
repositoryName: group.repositoryName,
514-
repositoryLocationName: group.repositoryLocationName,
515-
},
516-
]),
517-
);
518-
}
429+
onChangeAssetSelection(
430+
`group:"${group.groupName}" and code_location:"${group.repositoryLocationName}"`,
431+
);
519432
};
520433

521434
const svgViewport = layout ? (
@@ -691,7 +604,6 @@ const AssetGraphExplorerWithData = ({
691604
<AssetNode
692605
definition={graphNode.definition}
693606
selected={selectedGraphNodes.includes(graphNode)}
694-
kindFilter={kindFilter}
695607
onChangeAssetSelection={onChangeAssetSelection}
696608
/>
697609
</AssetNodeContextMenuWrapper>
@@ -788,34 +700,19 @@ const AssetGraphExplorerWithData = ({
788700
/>
789701
</Tooltip>
790702
)}
791-
{featureEnabled(FeatureFlag.flagSelectionSyntax) ? null : (
792-
<div>{filterButton}</div>
793-
)}
794703
<GraphQueryInputFlexWrap>
795-
{featureEnabled(FeatureFlag.flagSelectionSyntax) ? (
796-
<AssetSelectionInput
797-
assets={graphQueryItems}
798-
value={explorerPath.opsQuery}
799-
onChange={onChangeAssetSelection}
800-
onErrorStateChange={(errors) => {
801-
if (errors !== errorState) {
802-
setErrorState(errors);
803-
}
804-
}}
805-
/>
806-
) : (
807-
<AssetGraphAssetSelectionInput
808-
items={graphQueryItems}
809-
value={explorerPath.opsQuery}
810-
placeholder="Type an asset subset…"
811-
onChange={onChangeAssetSelection}
812-
popoverPosition="bottom-left"
813-
/>
814-
)}
704+
<AssetSelectionInput
705+
assets={graphQueryItems}
706+
value={explorerPath.opsQuery}
707+
onChange={onChangeAssetSelection}
708+
onErrorStateChange={(errors: SyntaxError[]) => {
709+
if (errors !== errorState) {
710+
setErrorState(errors);
711+
}
712+
}}
713+
/>
815714
</GraphQueryInputFlexWrap>
816-
{featureEnabled(FeatureFlag.flagSelectionSyntax) ? (
817-
<CreateCatalogViewButton />
818-
) : null}
715+
<CreateCatalogViewButton />
819716
<AssetLiveDataRefreshButton />
820717
{isIframe() ? null : (
821718
<LaunchAssetExecutionButton
@@ -828,7 +725,6 @@ const AssetGraphExplorerWithData = ({
828725
/>
829726
)}
830727
</Box>
831-
{featureEnabled(FeatureFlag.flagSelectionSyntax) ? null : filterBar}
832728
<IndeterminateLoadingBar
833729
$loading={nextLayoutLoading}
834730
style={{

Diff for: ‎js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx

+51-57
Original file line numberDiff line numberDiff line change
@@ -23,74 +23,68 @@ import {MinimalNodeStaleDot, StaleReasonsTag, isAssetStale} from '../assets/Stal
2323
import {AssetChecksStatusSummary} from '../assets/asset-checks/AssetChecksStatusSummary';
2424
import {assetDetailsPathForKey} from '../assets/assetDetailsPathForKey';
2525
import {AssetKind} from '../graph/KindTags';
26-
import {StaticSetFilter} from '../ui/BaseFilters/useStaticSetFilter';
2726
import {markdownToPlaintext} from '../ui/markdownToPlaintext';
2827

2928
interface Props {
3029
definition: AssetNodeFragment;
3130
selected: boolean;
32-
kindFilter?: StaticSetFilter<string>;
3331
onChangeAssetSelection?: (selection: string) => void;
3432
}
3533

36-
export const AssetNode = React.memo(
37-
({definition, selected, kindFilter, onChangeAssetSelection}: Props) => {
38-
const {liveData} = useAssetLiveData(definition.assetKey);
39-
const hasChecks = (liveData?.assetChecks || []).length > 0;
34+
export const AssetNode = React.memo(({definition, selected, onChangeAssetSelection}: Props) => {
35+
const {liveData} = useAssetLiveData(definition.assetKey);
36+
const hasChecks = (liveData?.assetChecks || []).length > 0;
4037

41-
const marginTopForCenteringNode = !hasChecks ? ASSET_NODE_STATUS_ROW_HEIGHT / 2 : 0;
38+
const marginTopForCenteringNode = !hasChecks ? ASSET_NODE_STATUS_ROW_HEIGHT / 2 : 0;
4239

43-
return (
44-
<AssetInsetForHoverEffect>
45-
<AssetNodeContainer $selected={selected}>
46-
<Box
47-
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
48-
style={{minHeight: ASSET_NODE_TAGS_HEIGHT, marginTop: marginTopForCenteringNode}}
49-
>
50-
<StaleReasonsTag liveData={liveData} assetKey={definition.assetKey} />
51-
<ChangedReasonsTag
52-
changedReasons={definition.changedReasons}
53-
assetKey={definition.assetKey}
54-
/>
55-
</Box>
56-
<AssetNodeBox $selected={selected} $isMaterializable={definition.isMaterializable}>
57-
<AssetNameRow definition={definition} />
58-
<Box style={{padding: '6px 8px'}} flex={{direction: 'column', gap: 4}} border="top">
59-
{definition.description ? (
60-
<AssetDescription $color={Colors.textDefault()}>
61-
{markdownToPlaintext(definition.description).split('\n')[0]}
62-
</AssetDescription>
63-
) : (
64-
<AssetDescription $color={Colors.textLight()}>No description</AssetDescription>
65-
)}
66-
{definition.isPartitioned && definition.isMaterializable && (
67-
<PartitionCountTags definition={definition} liveData={liveData} />
68-
)}
69-
</Box>
70-
71-
<AssetNodeStatusRow definition={definition} liveData={liveData} />
72-
{hasChecks && <AssetNodeChecksRow definition={definition} liveData={liveData} />}
73-
</AssetNodeBox>
74-
<Box
75-
style={{minHeight: ASSET_NODE_TAGS_HEIGHT}}
76-
flex={{alignItems: 'center', direction: 'row-reverse', gap: 8}}
77-
>
78-
{definition.kinds.map((kind) => (
79-
<AssetKind
80-
key={kind}
81-
kind={kind}
82-
style={{position: 'relative', margin: 0}}
83-
currentPageFilter={kindFilter}
84-
onChangeAssetSelection={onChangeAssetSelection}
85-
/>
86-
))}
40+
return (
41+
<AssetInsetForHoverEffect>
42+
<AssetNodeContainer $selected={selected}>
43+
<Box
44+
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
45+
style={{minHeight: ASSET_NODE_TAGS_HEIGHT, marginTop: marginTopForCenteringNode}}
46+
>
47+
<StaleReasonsTag liveData={liveData} assetKey={definition.assetKey} />
48+
<ChangedReasonsTag
49+
changedReasons={definition.changedReasons}
50+
assetKey={definition.assetKey}
51+
/>
52+
</Box>
53+
<AssetNodeBox $selected={selected} $isMaterializable={definition.isMaterializable}>
54+
<AssetNameRow definition={definition} />
55+
<Box style={{padding: '6px 8px'}} flex={{direction: 'column', gap: 4}} border="top">
56+
{definition.description ? (
57+
<AssetDescription $color={Colors.textDefault()}>
58+
{markdownToPlaintext(definition.description).split('\n')[0]}
59+
</AssetDescription>
60+
) : (
61+
<AssetDescription $color={Colors.textLight()}>No description</AssetDescription>
62+
)}
63+
{definition.isPartitioned && definition.isMaterializable && (
64+
<PartitionCountTags definition={definition} liveData={liveData} />
65+
)}
8766
</Box>
88-
</AssetNodeContainer>
89-
</AssetInsetForHoverEffect>
90-
);
91-
},
92-
isEqual,
93-
);
67+
68+
<AssetNodeStatusRow definition={definition} liveData={liveData} />
69+
{hasChecks && <AssetNodeChecksRow definition={definition} liveData={liveData} />}
70+
</AssetNodeBox>
71+
<Box
72+
style={{minHeight: ASSET_NODE_TAGS_HEIGHT}}
73+
flex={{alignItems: 'center', direction: 'row-reverse', gap: 8}}
74+
>
75+
{definition.kinds.map((kind) => (
76+
<AssetKind
77+
key={kind}
78+
kind={kind}
79+
style={{position: 'relative', margin: 0}}
80+
onChangeAssetSelection={onChangeAssetSelection}
81+
/>
82+
))}
83+
</Box>
84+
</AssetNodeContainer>
85+
</AssetInsetForHoverEffect>
86+
);
87+
}, isEqual);
9488

9589
export const AssetNameRow = ({definition}: {definition: AssetNodeFragment}) => {
9690
const displayName = definition.assetKey.path[definition.assetKey.path.length - 1]!;

0 commit comments

Comments
 (0)