Skip to content

Commit

Permalink
Fix duplicate code location error toasts + Add status filter to code …
Browse files Browse the repository at this point in the history
…locations page (#22540)

## Summary & Motivation

Recent changes to the WorkspaceContext where we split fetched by
location caused us show the code location error toast multiple times.
Once for each code location update after we've detected an errored code
location. The fix is to instead fire the effect off of the errored code
locations and preserve the reference if they haven't changed.

Also adding a "state" filter to the code locations page for use in the
error toast so that if you have 132 locations you can easily see which
ones failed.


## How I Tested These Changes

Loaded an org with 132 code locations and saw a single error toast.
Clicked on "view definitions" and saw it took me to the code location
page with status = Failed as the filter.

<img width="394" alt="Screenshot 2024-06-13 at 4 25 03 PM"
src="https://github.com/dagster-io/dagster/assets/2286579/92fcadc6-f844-4cd9-a48c-100eec9aba42">
<img width="1724" alt="Screenshot 2024-06-13 at 4 24 53 PM"
src="https://github.com/dagster-io/dagster/assets/2286579/fb000b48-d778-4d06-b339-8fc96816c9de">
  • Loading branch information
salazarm authored Jun 13, 2024
1 parent 26e03b8 commit ababe1a
Show file tree
Hide file tree
Showing 5 changed files with 208 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,20 @@ import * as React from 'react';

import {InstancePageContext} from './InstancePageContext';
import {InstanceTabs} from './InstanceTabs';
import {flattenCodeLocationRows} from './flattenCodeLocationRows';
import {useCodeLocationPageFilters} from './useCodeLocationPageFilters';
import {useTrackPageView} from '../app/analytics';
import {useDocumentTitle} from '../hooks/useDocumentTitle';
import {ReloadAllButton} from '../workspace/ReloadAllButton';
import {RepositoryLocationsList} from '../workspace/RepositoryLocationsList';
import {WorkspaceContext} from '../workspace/WorkspaceContext';

const SEARCH_THRESHOLD = 10;

export const CodeLocationsPageContent = () => {
useTrackPageView();
useDocumentTitle('Code locations');

const {locationEntries, loading} = React.useContext(WorkspaceContext);

const [searchValue, setSearchValue] = React.useState('');

const onChangeSearch = React.useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
setSearchValue(e.target.value);
}, []);

const queryString = searchValue.toLocaleLowerCase();
const {flattened, filtered} = React.useMemo(() => {
return flattenCodeLocationRows(locationEntries, queryString);
}, [locationEntries, queryString]);
const {activeFiltersJsx, flattened, button, loading, filtered, onChangeSearch, searchValue} =
useCodeLocationPageFilters();

const entryCount = flattened.length;
const showSearch = entryCount > SEARCH_THRESHOLD;
Expand All @@ -47,17 +36,20 @@ export const CodeLocationsPageContent = () => {
flex={{direction: 'row', justifyContent: 'space-between', alignItems: 'center'}}
style={{height: '64px'}}
>
{showSearch ? (
<TextInput
icon="search"
value={searchValue}
onChange={onChangeSearch}
placeholder="Filter code locations by name…"
style={{width: '400px'}}
/>
) : (
<Subheading id="repository-locations">{subheadingText()}</Subheading>
)}
<Box flex={{direction: 'row', gap: 8, alignItems: 'center'}}>
{button}
{showSearch ? (
<TextInput
icon="search"
value={searchValue}
onChange={onChangeSearch}
placeholder="Filter code locations by name…"
style={{width: '400px'}}
/>
) : (
<Subheading id="repository-locations">{subheadingText()}</Subheading>
)}
</Box>
<Box flex={{direction: 'row', gap: 12, alignItems: 'center'}}>
{showSearch ? <div>{`${entryCount} code locations`}</div> : null}
<ReloadAllButton />
Expand All @@ -68,6 +60,14 @@ export const CodeLocationsPageContent = () => {
codeLocations={filtered}
searchValue={searchValue}
/>
{activeFiltersJsx.length ? (
<Box
flex={{direction: 'row', alignItems: 'center', gap: 4}}
padding={{top: 8, horizontal: 24}}
>
{activeFiltersJsx}
</Box>
) : null}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,56 @@
import {CodeLocationRowType} from '../workspace/VirtualizedCodeLocationRow';
import {
CodeLocationRowStatusType,
CodeLocationRowType,
} from '../workspace/VirtualizedCodeLocationRow';
import {WorkspaceLocationNodeFragment} from '../workspace/types/WorkspaceQueries.types';

const flatten = (locationEntries: WorkspaceLocationNodeFragment[]) => {
// Consider each loaded repository to be a "code location".
const all: CodeLocationRowType[] = [];
for (const locationNode of locationEntries) {
const {locationOrLoadError} = locationNode;
let status: CodeLocationRowStatusType;
if (locationNode.loadStatus === 'LOADING') {
if (locationOrLoadError) {
status = 'Updating';
} else {
status = 'Loading';
}
} else {
if (locationOrLoadError?.__typename === 'PythonError') {
status = 'Failed';
} else {
status = 'Loaded';
}
}
if (!locationOrLoadError || locationOrLoadError?.__typename === 'PythonError') {
all.push({type: 'error' as const, node: locationNode});
all.push({type: 'error' as const, node: locationNode, status});
} else {
locationOrLoadError.repositories.forEach((repo) => {
all.push({type: 'repository' as const, codeLocation: locationNode, repository: repo});
all.push({
type: 'repository' as const,
codeLocation: locationNode,
repository: repo,
status,
});
});
}
}
return all;
};

const filterBySearch = (flattened: CodeLocationRowType[], searchValue: string) => {
const filterRows = (
flattened: CodeLocationRowType[],
searchValue: string,
filters: CodeLocationFilters,
) => {
const queryString = searchValue.toLocaleLowerCase();
return flattened.filter((row) => {
if (filters.status?.length) {
if (!filters.status.includes(row.status)) {
return false;
}
}
if (row.type === 'error') {
return row.node.name.toLocaleLowerCase().includes(queryString);
}
Expand All @@ -30,12 +61,15 @@ const filterBySearch = (flattened: CodeLocationRowType[], searchValue: string) =
});
};

export type CodeLocationFilters = Partial<{status: CodeLocationRowStatusType[]}>;

export const flattenCodeLocationRows = (
locationEntries: WorkspaceLocationNodeFragment[],
searchValue: string = '',
filters: CodeLocationFilters = {},
) => {
const flattened = flatten(locationEntries);
const filtered = filterBySearch(flattened, searchValue);
const filtered = filterRows(flattened, searchValue, filters);

return {
flattened,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React, {useCallback, useContext, useMemo, useState} from 'react';

import {CodeLocationFilters, flattenCodeLocationRows} from './flattenCodeLocationRows';
import {useQueryPersistedState} from '../hooks/useQueryPersistedState';
import {TruncatedTextWithFullTextOnHover} from '../nav/getLeftNavItemsForOption';
import {useFilters} from '../ui/Filters';
import {useStaticSetFilter} from '../ui/Filters/useStaticSetFilter';
import {CodeLocationRowStatusType} from '../workspace/VirtualizedCodeLocationRow';
import {WorkspaceContext} from '../workspace/WorkspaceContext';

export const useCodeLocationPageFilters = () => {
const {locationEntries, loading} = useContext(WorkspaceContext);

const [searchValue, setSearchValue] = useState('');

const onChangeSearch = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
setSearchValue(e.target.value);
}, []);

const queryString = searchValue.toLocaleLowerCase();

const [filters, setFilters] = useQueryPersistedState<CodeLocationFilters>({
encode: ({status}) => ({
status: status?.length ? JSON.stringify(status) : undefined,
}),
decode: (qs) => {
return {
status: qs.status ? JSON.parse(qs.status) : [],
};
},
});

const {flattened, filtered} = useMemo(() => {
return flattenCodeLocationRows(locationEntries, queryString, filters);
}, [locationEntries, queryString, filters]);

const statusFilter = useStaticSetFilter<CodeLocationRowStatusType>({
name: 'Status',
icon: 'tag',
allValues: useMemo(
() =>
(['Failed', 'Loaded', 'Updating', 'Loading'] as const).map((value) => ({
key: value,
value,
match: [value],
})),
[],
),
menuWidth: '300px',
renderLabel: ({value}) => {
return <TruncatedTextWithFullTextOnHover text={value} />;
},
getStringValue: (value) => value,
state: filters.status,
onStateChanged: (values) => {
setFilters({status: Array.from(values)});
},
matchType: 'all-of',
canSelectAll: false,
allowMultipleSelections: true,
});

const {button, activeFiltersJsx} = useFilters({filters: [statusFilter]});

return {
button,
activeFiltersJsx,
onChangeSearch,
loading,
flattened,
filtered,
searchValue,
};
};
Loading

1 comment on commit ababe1a

@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-9r5tru2w6-elementl.vercel.app

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

Please sign in to comment.