[ui] Move Observe Sources into the Materialize button (#23764)

## Summary & Motivation Related: https://linear.app/dagster-labs/issue/FE-517/remove-observe-sources-button-add-to-materialize-all-dropdown Loom: https://www.loom.com/share/2e044d1117734c2dbfff4fb4bef2a570 This PR folds the Observe Sources button into the Materialize button, which brings the Observe option to the asset catalog! I also updated the asset menus to use the same logic for building their menu items so the behavior is consistent. ## How I Tested These Changes - There's pretty good test coverage of this button and it's updated + still passing - I tested this manually with observable, materializable, external, and non-SDA assets to kick the tires :-) Co-authored-by: bengotow <[email protected]>
dagster-io · Oct 7, 2024 · 6454e6a · 6454e6a · github-actions · Oct 7, 2024
1 parent 02eaf42
commit 6454e6a
Show file tree

Hide file tree

Showing 12 changed files with 234 additions and 270 deletions.
diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetGraphExplorer.tsx b/js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetGraphExplorer.tsx
@@ -51,7 +51,6 @@ import {
 import {AssetLocation, useFindAssetLocation} from './useFindAssetLocation';
 import {AssetLiveDataRefreshButton} from '../asset-data/AssetLiveDataProvider';
 import {LaunchAssetExecutionButton} from '../assets/LaunchAssetExecutionButton';
-import {LaunchAssetObservationButton} from '../assets/LaunchAssetObservationButton';
 import {AssetKey} from '../assets/types';
 import {DEFAULT_MAX_ZOOM, SVGViewport} from '../graph/SVGViewport';
 import {useAssetLayout} from '../graph/asyncGraphLayout';
@@ -734,14 +733,6 @@ const AssetGraphExplorerWithData = ({
                     />
                   </GraphQueryInputFlexWrap>
                   <AssetLiveDataRefreshButton />
-                  <LaunchAssetObservationButton
-                    preferredJobName={explorerPath.pipelineName}
-                    scope={
-                      selectedDefinitions.length
-                        ? {selected: selectedDefinitions.filter((a) => a.isObservable)}
-                        : {all: allDefinitionsForMaterialize.filter((a) => a.isObservable)}
-                    }
-                  />
                   <LaunchAssetExecutionButton
                     preferredJobName={explorerPath.pipelineName}
                     scope={

diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNodeMenu.tsx b/js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNodeMenu.tsx
@@ -23,6 +23,7 @@ export type AssetNodeMenuNode = {
     isMaterializable: boolean;
     isObservable: boolean;
     isExecutable: boolean;
+    isPartitioned: boolean;
     hasMaterializePermission: boolean;
   };
 };
@@ -45,10 +46,11 @@ export const useAssetNodeMenu = ({
   const upstream = graphData ? Object.keys(graphData.upstream[node.id] ?? {}) : [];
   const downstream = graphData ? Object.keys(graphData.downstream[node.id] ?? {}) : [];
 
-  const {executeItem, launchpadElement} = useExecuteAssetMenuItem(
-    node.assetKey.path,
-    node.definition,
+  const asset = React.useMemo(
+    () => ({assetKey: node.assetKey, ...node.definition}),
+    [node.definition, node.assetKey],
   );
+  const {executeItem, launchpadElement} = useExecuteAssetMenuItem(asset);
 
   const [showParents, setShowParents] = React.useState(false);
 

diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetActionMenu.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetActionMenu.tsx
@@ -1,18 +1,16 @@
 import {Button, Icon, Menu, MenuItem, Popover, Spinner, Tooltip} from '@dagster-io/ui-components';
-import {useContext} from 'react';
+import {useContext, useMemo} from 'react';
 
-import {
-  executionDisabledMessageForAssets,
-  useMaterializationAction,
-} from './LaunchAssetExecutionButton';
-import {useObserveAction} from './LaunchAssetObservationButton';
+import {optionsForExecuteButton, useMaterializationAction} from './LaunchAssetExecutionButton';
 import {assetDetailsPathForKey} from './assetDetailsPathForKey';
 import {AssetTableDefinitionFragment} from './types/AssetTableFragment.types';
 import {useDeleteDynamicPartitionsDialog} from './useDeleteDynamicPartitionsDialog';
+import {useObserveAction} from './useObserveAction';
 import {useReportEventsModal} from './useReportEventsModal';
 import {useWipeModal} from './useWipeModal';
 import {CloudOSSContext} from '../app/CloudOSSContext';
 import {showSharedToaster} from '../app/DomUtils';
+import {AssetKeyInput} from '../graphql/types';
 import {MenuLink} from '../ui/MenuLink';
 import {RepoAddress} from '../workspace/types';
 import {workspacePathFromAddress} from '../workspace/workspacePath';
@@ -30,7 +28,17 @@ export const AssetActionMenu = (props: Props) => {
     featureContext: {canSeeMaterializeAction},
   } = useContext(CloudOSSContext);
 
-  const {executeItem, launchpadElement} = useExecuteAssetMenuItem(path, definition);
+  // Because the asset catalog loads a list of keys, and then definitions for SDAs,
+  // we don't re-fetch the key inside the definition of each asset. Re-attach the
+  // assetKey to the definition for the hook below.
+  const asset = useMemo(
+    () =>
+      definition
+        ? {...definition, isPartitioned: !!definition?.partitionDefinition, assetKey: {path}}
+        : null,
+    [path, definition],
+  );
+  const {executeItem, launchpadElement} = useExecuteAssetMenuItem(asset);
 
   const deletePartitions = useDeleteDynamicPartitionsDialog(
     repoAddress && definition ? {repoAddress, assetKey: {path}, definition} : null,
@@ -120,49 +128,40 @@ export const AssetActionMenu = (props: Props) => {
 };
 
 export const useExecuteAssetMenuItem = (
-  path: string[],
   definition: {
+    assetKey: AssetKeyInput;
     isObservable: boolean;
     isExecutable: boolean;
+    isPartitioned: boolean;
     hasMaterializePermission: boolean;
   } | null,
 ) => {
-  const disabledMessage = definition
-    ? executionDisabledMessageForAssets([definition])
-    : 'Asset definition not found in a code location';
+  const {materializeOption, observeOption} = optionsForExecuteButton(
+    definition ? [definition] : [],
+    {skipAllTerm: true},
+  );
 
   const {
     featureContext: {canSeeMaterializeAction},
   } = useContext(CloudOSSContext);
 
   const materialize = useMaterializationAction();
   const observe = useObserveAction();
+  const loading = materialize.loading || observe.loading;
+
+  const [option, action] =
+    materializeOption.disabledReason && !observeOption.disabledReason
+      ? [observeOption, observe.onClick]
+      : [materializeOption, materialize.onClick];
+
+  const disabledMessage = definition
+    ? option.disabledReason
+    : 'Asset definition not found in a code location';
 
   if (!canSeeMaterializeAction) {
     return {executeItem: null, launchpadElement: null};
   }
 
-  if (definition?.isExecutable && definition.isObservable && definition.hasMaterializePermission) {
-    return {
-      launchpadElement: null,
-      executeItem: (
-        <MenuItem
-          text="Observe"
-          icon={observe.loading ? <Spinner purpose="body-text" /> : 'observation'}
-          disabled={observe.loading}
-          onClick={(e) => {
-            void showSharedToaster({
-              intent: 'primary',
-              message: 'Initiating observation',
-              icon: 'observation',
-            });
-            observe.onClick([{path}], e);
-          }}
-        />
-      ),
-    };
-  }
-
   return {
     launchpadElement: materialize.launchpadElement,
     executeItem: (
@@ -173,20 +172,19 @@ export const useExecuteAssetMenuItem = (
         useDisabledButtonTooltipFix
       >
         <MenuItem
-          text="Materialize"
-          icon={materialize.loading ? <Spinner purpose="body-text" /> : 'materialization'}
-          disabled={!!disabledMessage || materialize.loading}
+          text={option.label}
+          icon={loading ? <Spinner purpose="body-text" /> : option.icon}
+          disabled={!!disabledMessage || loading}
           onClick={(e) => {
             if (!definition) {
               return;
             }
             void showSharedToaster({
               intent: 'primary',
-              message: 'Initiating materialization',
-              icon: 'materialization',
+              message: `Initiating...`,
             });
 
-            materialize.onClick([{path}], e);
+            action([definition.assetKey], e);
           }}
         />
       </Tooltip>

diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetTable.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetTable.tsx
@@ -8,7 +8,6 @@ import {
   MenuItem,
   NonIdealState,
   Popover,
-  Tooltip,
 } from '@dagster-io/ui-components';
 import groupBy from 'lodash/groupBy';
 import * as React from 'react';
@@ -23,12 +22,15 @@ import {CloudOSSContext} from '../app/CloudOSSContext';
 import {useUnscopedPermissions} from '../app/Permissions';
 import {QueryRefreshCountdown, RefreshState} from '../app/QueryRefresh';
 import {useSelectionReducer} from '../hooks/useSelectionReducer';
-import {testId} from '../testing/testId';
 import {StaticSetFilter} from '../ui/BaseFilters/useStaticSetFilter';
 import {VirtualizedAssetTable} from '../workspace/VirtualizedAssetTable';
 
 type Asset = AssetTableFragment;
 
+type AssetWithDefinition = AssetTableFragment & {
+  definition: NonNullable<AssetTableFragment['definition']>;
+};
+
 interface Props {
   view: AssetViewType;
   assets: Asset[];
@@ -158,24 +160,13 @@ export const AssetTable = ({
           <div style={{flex: 1}} />
           <QueryRefreshCountdown refreshState={refreshState} />
           <Box flex={{alignItems: 'center', gap: 8}}>
-            {checkedAssets.some((c) => !c.definition) ? (
-              <Tooltip content="One or more selected assets are not software-defined and cannot be launched directly.">
-                <Button
-                  intent="primary"
-                  data-testid={testId('materialize-button')}
-                  icon={<Icon name="materialization" />}
-                  disabled
-                >
-                  {checkedAssets.length > 1
-                    ? `Materialize (${checkedAssets.length.toLocaleString()})`
-                    : 'Materialize'}
-                </Button>
-              </Tooltip>
-            ) : (
-              <LaunchAssetExecutionButton
-                scope={{selected: checkedAssets.map((a) => ({...a.definition!, assetKey: a.key}))}}
-              />
-            )}
+            <LaunchAssetExecutionButton
+              scope={{
+                selected: checkedAssets
+                  .filter((a): a is AssetWithDefinition => !!a.definition)
+                  .map((a) => ({...a.definition, assetKey: a.key})),
+              }}
+            />
             <MoreActionsDropdown
               selected={checkedAssets}
               clearSelection={() => onToggleAll(false)}

diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/AssetView.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/AssetView.tsx
@@ -19,7 +19,6 @@ import {useAllAssets} from './AssetsCatalogTable';
 import {AssetAutomaterializePolicyPage} from './AutoMaterializePolicyPage/AssetAutomaterializePolicyPage';
 import {ChangedReasonsTag} from './ChangedReasons';
 import {LaunchAssetExecutionButton} from './LaunchAssetExecutionButton';
-import {LaunchAssetObservationButton} from './LaunchAssetObservationButton';
 import {UNDERLYING_OPS_ASSET_NODE_FRAGMENT} from './UnderlyingOpsOrGraph';
 import {AssetChecks} from './asset-checks/AssetChecks';
 import {assetDetailsPathForKey} from './assetDetailsPathForKey';
@@ -279,7 +278,12 @@ export const AssetView = ({assetKey, headerBreadcrumbs, writeAssetVisit, current
   }, [path, selectedTab, setCurrentPage]);
 
   const wipe = useWipeModal(
-    definition ? {assetKey: definition.assetKey, repository: definition.repository} : null,
+    definition && !definition.isObservable
+      ? {
+          assetKey: definition.assetKey,
+          repository: definition.repository,
+        }
+      : null,
     refresh,
   );
 
@@ -292,7 +296,7 @@ export const AssetView = ({assetKey, headerBreadcrumbs, writeAssetVisit, current
   );
 
   const reportEvents = useReportEventsModal(
-    definition && repoAddress
+    definition && !definition.isObservable && repoAddress
       ? {
           assetKey: definition.assetKey,
           isPartitioned: definition.isPartitioned,
@@ -336,13 +340,8 @@ export const AssetView = ({assetKey, headerBreadcrumbs, writeAssetVisit, current
           </div>
         }
         right={
-          <Box flex={{direction: 'row'}}>
-            {cachedOrLiveDefinition && cachedOrLiveDefinition.isObservable ? (
-              <LaunchAssetObservationButton
-                primary
-                scope={{all: [cachedOrLiveDefinition], skipAllTerm: true}}
-              />
-            ) : cachedOrLiveDefinition && cachedOrLiveDefinition.jobNames.length > 0 && upstream ? (
+          <Box style={{margin: '-4px 0'}} flex={{direction: 'row', gap: 8}}>
+            {cachedOrLiveDefinition && cachedOrLiveDefinition.jobNames.length > 0 && upstream ? (
               <LaunchAssetExecutionButton
                 scope={{all: [cachedOrLiveDefinition]}}
                 showChangedAndMissingOption={false}