From e37e5990bba1704bb8a8a5c188a7833b49e110a7 Mon Sep 17 00:00:00 2001 From: Isaac Hellendag Date: Wed, 7 Aug 2024 14:40:21 -0500 Subject: [PATCH] [ui] Hide Jobs item from nav if there are no jobs [INTERNAL_BRANCH=dish/hide-jobs-nav-cloud] --- .../src/app/AppTopNav/AppTopNavLinks.tsx | 41 +++++++----- .../AppTopNav/AppTopNavRightOfLogo.oss.tsx | 4 +- .../src/app/AppTopNav/useJobStateForNav.tsx | 26 ++++++++ .../useJobStateForNav.fixtures.tsx | 58 +++++++++++++++++ .../app/__tests__/useJobStateForNav.test.tsx | 62 +++++++++++++++++++ .../src/workspace/WorkspaceContext.tsx | 1 + 6 files changed, 177 insertions(+), 15 deletions(-) create mode 100644 js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/useJobStateForNav.tsx create mode 100644 js_modules/dagster-ui/packages/ui-core/src/app/__fixtures__/useJobStateForNav.fixtures.tsx create mode 100644 js_modules/dagster-ui/packages/ui-core/src/app/__tests__/useJobStateForNav.test.tsx diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx index 64033efcfdedc..0e23731da7d8c 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavLinks.tsx @@ -1,5 +1,5 @@ import {Box} from '@dagster-io/ui-components'; -import {ReactNode} from 'react'; +import {ReactElement} from 'react'; import {useHistory} from 'react-router-dom'; import {FeatureFlag} from 'shared/app/FeatureFlags.oss'; @@ -11,6 +11,7 @@ import { jobsPathMatcher, locationPathMatcher, } from './activePathMatchers'; +import {JobStateForNav} from './useJobStateForNav'; import {DeploymentStatusIcon} from '../../nav/DeploymentStatusIcon'; import {featureEnabled} from '../Flags'; import {ShortcutHandler} from '../ShortcutHandler'; @@ -18,7 +19,7 @@ import {ShortcutHandler} from '../ShortcutHandler'; export type AppNavLinkType = { key: string; path: string; - element: ReactNode; + element: ReactElement; }; export const AppTopNavLinks = ({links}: {links: AppNavLinkType[]}) => { @@ -42,7 +43,13 @@ export const AppTopNavLinks = ({links}: {links: AppNavLinkType[]}) => { ); }; -export const navLinks = () => { +type Config = { + jobState?: JobStateForNav; +}; + +export const navLinks = (config: Config): AppNavLinkType[] => { + const {jobState = 'unknown'} = config; + const overview = { key: 'overview', path: '/overview', @@ -63,16 +70,6 @@ export const navLinks = () => { ), }; - const jobs = { - key: 'jobs', - path: '/jobs', - element: ( - - Jobs - - ), - }; - const assets = { key: 'assets', path: '/assets', @@ -98,6 +95,19 @@ export const navLinks = () => { }; if (featureEnabled(FeatureFlag.flagSettingsPage)) { + const jobs = + jobState === 'has-jobs' + ? { + key: 'jobs', + path: '/jobs', + element: ( + + Jobs + + ), + } + : null; + const deployment = { key: 'deployment', path: '/deployment', @@ -114,7 +124,10 @@ export const navLinks = () => { ), }; - return [overview, assets, jobs, automation, runs, deployment]; + + return [overview, runs, assets, jobs, automation, deployment].filter( + (link): link is AppNavLinkType => !!link, + ); } const deployment = { diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavRightOfLogo.oss.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavRightOfLogo.oss.tsx index 1e6c7cec794a3..f41af8886918c 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavRightOfLogo.oss.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/AppTopNavRightOfLogo.oss.tsx @@ -1,7 +1,9 @@ import {memo} from 'react'; import {AppTopNavLinks, navLinks} from './AppTopNavLinks'; +import {useJobStateForNav} from './useJobStateForNav'; export const AppTopNavRightOfLogo = memo(() => { - return ; + const jobState = useJobStateForNav(); + return ; }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/useJobStateForNav.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/useJobStateForNav.tsx new file mode 100644 index 0000000000000..9b90608c999e5 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav/useJobStateForNav.tsx @@ -0,0 +1,26 @@ +import {useMemo} from 'react'; + +import {isHiddenAssetGroupJob} from '../../asset-graph/Utils'; +import {useRepositoryOptions} from '../../workspace/WorkspaceContext'; + +export type JobStateForNav = 'unknown' | 'has-jobs' | 'no-jobs'; + +/** + * Determine whether the viewer has any jobs in any of their code locations. We use + * this information to determine whether to show the "Jobs" item in the top navigation + * at all. If there are no jobs, we won't show it. + */ +export const useJobStateForNav = () => { + const {options, loading} = useRepositoryOptions(); + return useMemo(() => { + if (loading) { + return 'unknown'; + } + + const hasJobs = options.some((option) => + option.repository.pipelines.some((job) => !isHiddenAssetGroupJob(job.name)), + ); + + return hasJobs ? 'has-jobs' : 'no-jobs'; + }, [options, loading]); +}; diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/__fixtures__/useJobStateForNav.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/__fixtures__/useJobStateForNav.fixtures.tsx new file mode 100644 index 0000000000000..e281ffd6ad515 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/app/__fixtures__/useJobStateForNav.fixtures.tsx @@ -0,0 +1,58 @@ +import {__ANONYMOUS_ASSET_JOB_PREFIX} from '../../asset-graph/Utils'; +import { + buildPipeline, + buildRepository, + buildRepositoryLocation, + buildWorkspaceLocationEntry, +} from '../../graphql/types'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; + +export const workspaceWithJob = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + name: 'some_workspace', + locationOrLoadError: buildRepositoryLocation({ + name: 'location_with_job', + repositories: [ + buildRepository({ + name: 'repo_with_job', + pipelines: [ + buildPipeline({ + name: 'some_job', + isJob: true, + }), + ], + }), + ], + }), + }), +]); + +export const workspaceWithNoJobs = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + name: 'some_workspace', + locationOrLoadError: buildRepositoryLocation({ + name: 'location_without_job', + repositories: [ + buildRepository({ + name: 'repo_without_job', + pipelines: [], + }), + ], + }), + }), +]); + +export const workspaceWithDunderJob = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + name: 'some_workspace', + locationOrLoadError: buildRepositoryLocation({ + name: 'location_without_job', + repositories: [ + buildRepository({ + name: `${__ANONYMOUS_ASSET_JOB_PREFIX}_pseudo_job`, + pipelines: [], + }), + ], + }), + }), +]); diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/useJobStateForNav.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/useJobStateForNav.test.tsx new file mode 100644 index 0000000000000..45ad57cc7d198 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/useJobStateForNav.test.tsx @@ -0,0 +1,62 @@ +import {MockedProvider} from '@apollo/client/testing'; +import {render, screen} from '@testing-library/react'; + +import {WorkspaceProvider} from '../../workspace/WorkspaceContext'; +import {useJobStateForNav} from '../AppTopNav/useJobStateForNav'; +import { + workspaceWithDunderJob, + workspaceWithJob, + workspaceWithNoJobs, +} from '../__fixtures__/useJobStateForNav.fixtures'; + +describe('useJobStateForNav', () => { + const Test = () => { + const value = useJobStateForNav(); + return
{value}
; + }; + + it('returns `unknown` if still loading, then finds jobs and returns `has-jobs`', async () => { + render( + + + + + , + ); + + expect(screen.getByText(/unknown/i)).toBeVisible(); + + const found = await screen.findByText(/has-jobs/i); + expect(found).toBeVisible(); + }); + + it('returns `no-jobs` if no jobs found after loading', async () => { + render( + + + + + , + ); + + expect(screen.getByText(/unknown/i)).toBeVisible(); + + const found = await screen.findByText(/no-jobs/i); + expect(found).toBeVisible(); + }); + + it('returns `no-jobs` if only dunder job found', async () => { + render( + + + + + , + ); + + expect(screen.getByText(/unknown/i)).toBeVisible(); + + const found = await screen.findByText(/no-jobs/i); + expect(found).toBeVisible(); + }); +}); diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx index b251cb1c2c57e..5024ff301c43b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx @@ -432,6 +432,7 @@ const useVisibleRepos = ( const getRepositoryOptionHash = (a: DagsterRepoOption) => `${a.repository.name}:${a.repositoryLocation.name}`; + export const useRepositoryOptions = () => { const {allRepos: options, loading} = React.useContext(WorkspaceContext); return {options, loading};