Skip to content

Commit

Permalink
[ui] Fix code location page status filters (#25229)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Fix `useCodeLocationPageFilters` when an invalid query param is used.

- If `status` is a non-array string, discard it. (This is the bug that
was reported, where perhaps the user just modified the URL manually?)
- Use a query parameter array instead of JSON encoding.

## How I Tested These Changes

View code location list, filter on status. Verify that it works
correctly and updates the URL accordingly, e.g.
`?status%5B%5D=Loaded&status%5B%5D=Failed`.

Modify the parameter manually to be `?status=failed`, verify that the
page loads correctly with no JS errors, and ignores the invalid
parameter.

## Changelog

[ui] Fix an issue in the code locations page where invalid query
parameters could crash the page.
  • Loading branch information
hellendag authored Oct 18, 2024
1 parent 79d60bf commit f7d0b30
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ export const useCodeLocationPageFilters = () => {
const queryString = searchValue.toLocaleLowerCase();

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Box, ButtonLink, Colors} from '@dagster-io/ui-components';
import qs from 'qs';
import {useCallback, useContext, useLayoutEffect, useMemo, useRef, useState} from 'react';
import {useHistory} from 'react-router-dom';
import {atom, useRecoilValue} from 'recoil';
Expand Down Expand Up @@ -35,7 +36,11 @@ export const useCodeLocationsStatus = (): StatusAndMessage | null => {
const [showSpinner, setShowSpinner] = useState(false);

const onClickViewButton = useCallback((statuses: CodeLocationRowStatusType[]) => {
historyRef.current.push(`/locations?status=${JSON.stringify(statuses)}`);
const params =
statuses.length > 0
? qs.stringify({status: statuses}, {arrayFormat: 'brackets', addQueryPrefix: true})
: '';
historyRef.current.push(`/locations${params}`);
}, []);

// Reload the workspace, but don't toast about it.
Expand Down

1 comment on commit f7d0b30

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

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

Please sign in to comment.