Skip to content

Commit

Permalink
[ui] Stop using the job page as the home page when single job is defi…
Browse files Browse the repository at this point in the history
…ned (#26120)

## 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 <[email protected]>
  • Loading branch information
bengotow and bengotow authored Nov 26, 2024
1 parent d67be67 commit d8c00f1
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 (
<Redirect
to={workspacePipelinePath({
repoName: repo.repository.name,
repoLocation: repo.repositoryLocation.name,
pipelineName: job.name,
isJob: job.isJob,
})}
/>
);
}
}

// If we have more than one repo with a job, route to the instance overview
if (reposWithVisibleJobs.length > 0) {
return <Redirect to="/overview" />;
}

// 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 <Redirect to="/locations" />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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,
}),
],
}),
],
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = () => (
<MemoryRouter initialEntries={[{pathname: '/'}]}>
<WorkspaceProvider>
<Switch>
<Route path="/overview" render={() => <div />} />
<Route path="/locations" render={() => <div />} />
<BaseFallthroughRoot />
</Switch>
<Route render={(m) => <div data-testid={testId('path')}>{m.location.pathname}</div>} />
</WorkspaceProvider>
</MemoryRouter>
);

it('redirects to /locations if there are no repositories', async () => {
const {getByTestId} = render(
<MockedProvider mocks={[...workspaceWithNoRepos]}>
<Test />
</MockedProvider>,
);
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(
<MockedProvider
mocks={buildWorkspaceMocks([
buildWorkspaceLocationEntry({
name: 'some_workspace',
locationOrLoadError: buildRepositoryLocation({
name: 'location_with_dunder_job',
repositories: [
buildRepository({
name: `repo_with_pseudo_job`,
assetGroups: [
buildAssetGroup({
groupName: 'group1',
}),
],
pipelines: [
buildPipeline({
name: `${__ANONYMOUS_ASSET_JOB_PREFIX}_pseudo_job`,
isJob: true,
}),
],
}),
],
}),
}),
])}
>
<Test />
</MockedProvider>,
);
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(
<MockedProvider mocks={[...workspaceWithJob]}>
<Test />
</MockedProvider>,
);
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(
<MockedProvider mocks={[...workspaceWithDunderJob]}>
<Test />
</MockedProvider>,
);
await waitFor(() => {
expect(getByTestId('path').textContent).toEqual('/locations');
});
});
});

1 comment on commit d8c00f1

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-3k44l8rix-elementl.vercel.app

Built with commit d8c00f1.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.