From d8c00f1f2571b4ccc3e5c9022b8cbba8ab01df6a Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Tue, 26 Nov 2024 12:18:07 -0800 Subject: [PATCH] [ui] Stop using the job page as the home page when single job is defined (#26120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary & Motivation https://linear.app/dagster-labs/issue/FE-683/change-fallthrough-redirect-not-to-land-on-single-job This case is somewhat rare -- if a user has exactly one job, we had special cased it and opening the Dagster UI would land you directly on the job page. We think that this case is odd and it'd be better to direct to the Overview. ## How I Tested These Changes I added test coverage for all variants of the landing page behavior. 🫡 ## Changelog [ui] Opening Dagster's UI with a single job defined takes you to the Overview page rather than the Job page Co-authored-by: bengotow --- .../ui-core/src/app/BaseFallthroughRoot.tsx | 25 +--- .../useJobStateForNav.fixtures.tsx | 21 +++- .../__tests__/BaseFallthroughRoot.test.tsx | 107 ++++++++++++++++++ 3 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 js_modules/dagster-ui/packages/ui-core/src/app/__tests__/BaseFallthroughRoot.test.tsx diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/BaseFallthroughRoot.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/BaseFallthroughRoot.tsx index 13d3b96e46182..ff892650314ae 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/BaseFallthroughRoot.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/BaseFallthroughRoot.tsx @@ -6,7 +6,7 @@ import {Route} from './Route'; import {isHiddenAssetGroupJob} from '../asset-graph/Utils'; import {WorkspaceContext} from '../workspace/WorkspaceContext/WorkspaceContext'; import {DagsterRepoOption} from '../workspace/WorkspaceContext/util'; -import {workspacePath, workspacePipelinePath} from '../workspace/workspacePath'; +import {workspacePath} from '../workspace/workspacePath'; export const BaseFallthroughRoot = () => { return ( @@ -59,30 +59,11 @@ const FinalRedirectOrLoadingRoot = () => { ); } } - - // If we have exactly one repo with one job, route to the job overview - if (reposWithVisibleJobs.length === 1) { - const repo = reposWithVisibleJobs[0]!; - const visibleJobs = getVisibleJobs(repo); - if (visibleJobs.length === 1) { - const job = visibleJobs[0]!; - return ( - - ); - } - } - - // If we have more than one repo with a job, route to the instance overview if (reposWithVisibleJobs.length > 0) { return ; } + // Ben note: We only reach here if reposWithVisibleJobs === 0 AND there is no asset group. + // In this case, the overview would be blank so we go to the locations page. return ; }; 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 index 1784efe72f834..2b12949f400fb 100644 --- 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 @@ -27,6 +27,16 @@ export const workspaceWithJob = buildWorkspaceMocks([ }), ]); +export const workspaceWithNoRepos = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + name: 'some_workspace', + locationOrLoadError: buildRepositoryLocation({ + name: 'location_without_job', + repositories: [], + }), + }), +]); + export const workspaceWithNoJobs = buildWorkspaceMocks([ buildWorkspaceLocationEntry({ name: 'some_workspace', @@ -46,11 +56,16 @@ export const workspaceWithDunderJob = buildWorkspaceMocks([ buildWorkspaceLocationEntry({ name: 'some_workspace', locationOrLoadError: buildRepositoryLocation({ - name: 'location_without_job', + name: 'location_with_dunder_job', repositories: [ buildRepository({ - name: `${__ANONYMOUS_ASSET_JOB_PREFIX}_pseudo_job`, - pipelines: [], + name: `repo_with_pseudo_job`, + pipelines: [ + buildPipeline({ + name: `${__ANONYMOUS_ASSET_JOB_PREFIX}_pseudo_job`, + isJob: true, + }), + ], }), ], }), diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/BaseFallthroughRoot.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/BaseFallthroughRoot.test.tsx new file mode 100644 index 0000000000000..75722b7d0c578 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/app/__tests__/BaseFallthroughRoot.test.tsx @@ -0,0 +1,107 @@ +import {MockedProvider} from '@apollo/client/testing'; +import {render, waitFor} from '@testing-library/react'; +import {MemoryRouter, Route, Switch} from 'react-router'; + +import {__ANONYMOUS_ASSET_JOB_PREFIX} from '../../asset-graph/Utils'; +import { + buildAssetGroup, + buildPipeline, + buildRepository, + buildRepositoryLocation, + buildWorkspaceLocationEntry, +} from '../../graphql/types'; +import {testId} from '../../testing/testId'; +import {WorkspaceProvider} from '../../workspace/WorkspaceContext/WorkspaceContext'; +import {buildWorkspaceMocks} from '../../workspace/WorkspaceContext/__fixtures__/Workspace.fixtures'; +import {BaseFallthroughRoot} from '../BaseFallthroughRoot'; +import { + workspaceWithDunderJob, + workspaceWithJob, + workspaceWithNoRepos, +} from '../__fixtures__/useJobStateForNav.fixtures'; + +describe('BaseFallthroughRoot', () => { + const Test = () => ( + + + +
} /> +
} /> + + +
{m.location.pathname}
} /> + + + ); + + it('redirects to /locations if there are no repositories', async () => { + const {getByTestId} = render( + + + , + ); + await waitFor(() => { + expect(getByTestId('path').textContent).toEqual('/locations'); + }); + }); + + it('redirects to the asset graph if assetGroups are present and repos have no visible jobs', async () => { + const {getByTestId} = render( + + + , + ); + await waitFor(() => { + expect(getByTestId('path').textContent).toEqual( + '/locations/repo_with_pseudo_job@location_with_dunder_job/asset-groups/group1', + ); + }); + }); + + it('redirects to /overview if repos have visible jobs', async () => { + const {getByTestId} = render( + + + , + ); + await waitFor(() => { + expect(getByTestId('path').textContent).toEqual('/overview'); + }); + }); + + it('redirects to /locations if repos have no visible jobs and no asset groups', async () => { + const {getByTestId} = render( + + + , + ); + await waitFor(() => { + expect(getByTestId('path').textContent).toEqual('/locations'); + }); + }); +});