Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate jsx files that affect run/task selection to tsx #24509

Merged
merged 10 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions airflow/www/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"stylelint": "^13.6.1",
"stylelint-config-standard": "^20.0.0",
"terser-webpack-plugin": "<5.0.0",
"ts-loader": "^8.2.0",
"typescript": "^4.6.3",
"url-loader": "4.1.0",
"webpack": "^5.73.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { useGridData } from './api';

const detailsPanelKey = 'hideDetailsPanel';

const Main = () => {
const Main: React.FC = () => {
const { data: { groups }, isLoading } = useGridData();
const isPanelOpen = localStorage.getItem(detailsPanelKey) !== 'true';
const { isOpen, onToggle } = useDisclosure({ defaultIsOpen: isPanelOpen });
Expand All @@ -47,10 +47,10 @@ const Main = () => {

const onPanelToggle = () => {
if (!isOpen) {
localStorage.setItem(detailsPanelKey, false);
localStorage.setItem(detailsPanelKey, 'false');
} else {
clearSelection();
localStorage.setItem(detailsPanelKey, true);
localStorage.setItem(detailsPanelKey, 'true');
}
onToggle();
};
Expand Down
8 changes: 4 additions & 4 deletions airflow/www/static/js/grid/ToggleGroups.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ const getGroupIds = (groups) => {
};

const ToggleGroups = ({ groups, openGroupIds, onToggleGroups }) => {
// Don't show button if the DAG has no task groups
const hasGroups = groups.children && groups.children.find((c) => !!c.children);
if (!hasGroups) return null;

const allGroupIds = getGroupIds(groups.children);

const isExpandDisabled = allGroupIds.length === openGroupIds.length;
const isCollapseDisabled = !openGroupIds.length;

// Don't show button if the DAG has no task groups
const hasGroups = groups.children.find((c) => !!c.children);
if (!hasGroups) return null;

const onExpand = () => {
onToggleGroups(allGroupIds);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import axios from 'axios';
import axios, { AxiosResponse } from 'axios';
import camelcaseKeys from 'camelcase-keys';

import useTasks from './useTasks';
Expand All @@ -35,7 +35,7 @@ import useGridData from './useGridData';
import useMappedInstances from './useMappedInstances';

axios.interceptors.response.use(
(res) => (res.data ? camelcaseKeys(res.data, { deep: true }) : res),
(res: AxiosResponse) => (res.data ? camelcaseKeys(res.data, { deep: true }) : res),
);

axios.defaults.headers.common.Accept = 'application/json';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@
* under the License.
*/

/* global autoRefreshInterval */

import { useQuery } from 'react-query';
import axios from 'axios';
import axios, { AxiosResponse } from 'axios';

import { getMetaValue } from '../../utils';
import { useAutoRefresh } from '../context/autorefresh';
import useErrorToast from '../utils/useErrorToast';
import useFilters, {
BASE_DATE_PARAM, NUM_RUNS_PARAM, RUN_STATE_PARAM, RUN_TYPE_PARAM, now,
} from '../utils/useFilters';
import type { Task, DagRun } from '../types';

const DAG_ID_PARAM = 'dag_id';

Expand All @@ -36,12 +35,21 @@ const dagId = getMetaValue(DAG_ID_PARAM);
const gridDataUrl = getMetaValue('grid_data_url') || '';
const urlRoot = getMetaValue('root');

const emptyData = {
interface GridData {
dagRuns: DagRun[];
groups: Task;
}

const emptyGridData: GridData = {
dagRuns: [],
groups: {},
groups: {
id: null,
label: null,
instances: [],
},
};

export const areActiveRuns = (runs = []) => runs.filter((run) => ['queued', 'running', 'scheduled'].includes(run.state)).length > 0;
export const areActiveRuns = (runs: DagRun[] = []) => runs.filter((run) => ['queued', 'running', 'scheduled'].includes(run.state)).length > 0;

const useGridData = () => {
const { isRefreshOn, stopRefresh } = useAutoRefresh();
Expand All @@ -52,8 +60,9 @@ const useGridData = () => {
},
} = useFilters();

return useQuery(['gridData', baseDate, numRuns, runType, runState], async () => {
try {
const query = useQuery(
['gridData', baseDate, numRuns, runType, runState],
async () => {
const params = {
root: urlRoot || undefined,
[DAG_ID_PARAM]: dagId,
Expand All @@ -62,24 +71,29 @@ const useGridData = () => {
[RUN_TYPE_PARAM]: runType,
[RUN_STATE_PARAM]: runState,
};
const newData = await axios.get(gridDataUrl, { params });
const response = await axios.get<AxiosResponse, GridData>(gridDataUrl, { params });
// turn off auto refresh if there are no active runs
if (!areActiveRuns(newData.dagRuns)) stopRefresh();
return newData;
} catch (error) {
stopRefresh();
errorToast({
title: 'Auto-refresh Error',
error,
});
throw (error);
}
}, {
placeholderData: emptyData,
// only refetch if the refresh switch is on
refetchInterval: isRefreshOn && autoRefreshInterval * 1000,
keepPreviousData: true,
});
if (!areActiveRuns(response.dagRuns)) stopRefresh();
return response;
},
{
// only refetch if the refresh switch is on
refetchInterval: isRefreshOn && (autoRefreshInterval || 1) * 1000,
keepPreviousData: true,
onError: (error) => {
stopRefresh();
errorToast({
title: 'Auto-refresh Error',
error,
});
throw (error);
},
},
);
return {
...query,
data: query.data ?? emptyGridData,
};
};

export default useGridData;
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,25 @@
* under the License.
*/

import axios from 'axios';
import axios, { AxiosResponse } from 'axios';
import { useQuery } from 'react-query';
import { getMetaValue } from '../../utils';

interface TaskData {
tasks: any[];
totalEntries: number;
}

export default function useTasks() {
return useQuery(
const query = useQuery<TaskData>(
'tasks',
() => {
const tasksUrl = getMetaValue('tasks_api');
return axios.get(tasksUrl);
},
{
initialData: { tasks: [], totalEntries: 0 },
return axios.get<AxiosResponse, TaskData>(tasksUrl || '');
},
);
return {
...query,
data: query.data || { tasks: [], totalEntries: 0 },
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,21 @@ import { render } from '@testing-library/react';

import InstanceTooltip from './InstanceTooltip';
import { Wrapper } from '../utils/testUtils';
import type { TaskState } from '../types';

const instance = {
startDate: new Date(),
endDate: new Date(),
state: 'success',
startDate: new Date().toISOString(),
endDate: new Date().toISOString(),
state: 'success' as TaskState,
runId: 'run',
taskId: 'task',
};

describe('Test Task InstanceTooltip', () => {
test('Displays a normal task', () => {
const { getByText } = render(
<InstanceTooltip
group={{}}
group={{ id: 'task', label: 'task', instances: [] }}
instance={instance}
/>,
{ wrapper: Wrapper },
Expand All @@ -48,7 +50,9 @@ describe('Test Task InstanceTooltip', () => {
test('Displays a mapped task with overall status', () => {
const { getByText } = render(
<InstanceTooltip
group={{ isMapped: true }}
group={{
id: 'task', label: 'task', instances: [], isMapped: true,
}}
instance={{ ...instance, mappedStates: { success: 2 } }}
/>,
{ wrapper: Wrapper },
Expand All @@ -63,12 +67,20 @@ describe('Test Task InstanceTooltip', () => {
const { getByText, queryByText } = render(
<InstanceTooltip
group={{
id: 'task',
label: 'task',
instances: [],
children: [
{
id: 'child_task',
label: 'child_task',
instances: [
{
taskId: 'child_task',
runId: 'run',
state: 'success',
startDate: '',
endDate: '',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,33 @@ import { Box, Text } from '@chakra-ui/react';
import { finalStatesMap } from '../../utils';
import { formatDuration, getDuration } from '../../datetime_utils';
import Time from './Time';
import type { TaskInstance, Task } from '../types';

const InstanceTooltip = ({
interface Props {
group: Task;
instance: TaskInstance;
}

const InstanceTooltip: React.FC<Props> = ({
Copy link
Member

Choose a reason for hiding this comment

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

Any reason we favor React.FC over:

const MyComponent  = ({}: Props): JSX.Element => {}

React.FC was removed in CRA:
facebook/create-react-app#8177

React.FC seems discouraged:
https://github.com/typescript-cheatsheets/react#function-components

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. You're right. As of React 18, it doesn't make sense anymore. I'll update that.

group,
instance: {
startDate, endDate, state, runId, mappedStates,
},
}) => {
if (!group) return null;
const isGroup = !!group.children;
const { isMapped } = group;
const summary = [];
const summary: React.ReactNode[] = [];

const isMapped = group?.isMapped;

const numMap = finalStatesMap();
let numMapped = 0;
if (isGroup) {
if (isGroup && group.children) {
group.children.forEach((child) => {
const taskInstance = child.instances.find((ti) => ti.runId === runId);
if (taskInstance) {
const stateKey = taskInstance.state == null ? 'no_status' : taskInstance.state;
if (numMap.has(stateKey)) numMap.set(stateKey, numMap.get(stateKey) + 1);
if (numMap.has(stateKey)) numMap.set(stateKey, (numMap.get(stateKey) || 0) + 1);
}
});
} else if (isMapped && mappedStates) {
Expand Down Expand Up @@ -88,7 +96,7 @@ const InstanceTooltip = ({
<Text>
Started:
{' '}
<Time dateTime={startDate} />
<Time dateTime={startDate || ''} />
</Text>
<Text>
Duration:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,46 @@
* under the License.
*/

/* global stateColors */

import React from 'react';
import { isEqual } from 'lodash';
import {
Box,
useTheme,
BoxProps,
} from '@chakra-ui/react';

import Tooltip from './Tooltip';
import InstanceTooltip from './InstanceTooltip';
import { useContainerRef } from '../context/containerRef';
import type { Task, TaskInstance, TaskState } from '../types';
import type { SelectionProps } from '../utils/useSelection';

export const boxSize = 10;
export const boxSizePx = `${boxSize}px`;

export const SimpleStatus = ({ state, ...rest }) => (
interface SimpleStatusProps extends BoxProps {
state: TaskState;
}

export const SimpleStatus: React.FC<SimpleStatusProps> = ({ state, ...rest }) => (
<Box
width={boxSizePx}
height={boxSizePx}
backgroundColor={stateColors[state] || 'white'}
backgroundColor={state && stateColors[state] ? stateColors[state] : 'white'}
borderRadius="2px"
borderWidth={state ? 0 : 1}
{...rest}
/>
);

const StatusBox = ({
interface Props {
group: Task;
instance: TaskInstance;
onSelect: (selection: SelectionProps) => void;
isActive: boolean;
}

const StatusBox: React.FC<Props> = ({
group, instance, onSelect, isActive,
}) => {
const containerRef = useContainerRef();
Expand All @@ -54,15 +66,19 @@ const StatusBox = ({

// Fetch the corresponding column element and set its background color when hovering
const onMouseEnter = () => {
[...containerRef.current.getElementsByClassName(`js-${runId}`)]
.forEach((e) => {
// Don't apply hover if it is already selected
if (e.getAttribute('data-selected') === 'false') e.style.backgroundColor = hoverBlue;
});
if (containerRef && containerRef.current) {
([...containerRef.current.getElementsByClassName(`js-${runId}`)] as HTMLElement[])
.forEach((e) => {
// Don't apply hover if it is already selected
if (e.getAttribute('data-selected') === 'false') e.style.backgroundColor = hoverBlue;
});
}
};
const onMouseLeave = () => {
[...containerRef.current.getElementsByClassName(`js-${runId}`)]
.forEach((e) => { e.style.backgroundColor = null; });
if (containerRef && containerRef.current) {
([...containerRef.current.getElementsByClassName(`js-${runId}`)] as HTMLElement[])
.forEach((e) => { e.style.backgroundColor = ''; });
}
};

const onClick = () => {
Expand Down Expand Up @@ -97,8 +113,8 @@ const StatusBox = ({
// The default equality function is a shallow comparison and json objects will return false
// This custom compare function allows us to do a deeper comparison
const compareProps = (
prevProps,
nextProps,
prevProps: Props,
nextProps: Props,
) => (
isEqual(prevProps.group, nextProps.group)
&& isEqual(prevProps.instance, nextProps.instance)
Expand Down
Loading