Skip to content

Commit

Permalink
Disable Asset daemon global toggle if using AMP sensors. (#20348)
Browse files Browse the repository at this point in the history 
## Summary & Motivation

As titled

## How I Tested These Changes

jest
  • Loading branch information
@salazarm
salazarm committed Mar 8, 2024
1 parent 995fb0f commit c490734
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 24 deletions.
4 changes: 2 additions & 2 deletions js_modules/dagster-ui/packages/ui-core/src/assets/AutoMaterializeSensorFlag.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ type FlagState = 'unknown' | 'has-sensor-amp' | 'has-global-amp';

export const useAutoMaterializeSensorFlag = (): FlagState => {
const {data} = useQuery<AutoMaterializeSensorFlagQuery, AutoMaterializeSensorFlagQueryVariables>(
AUTO_MATERIALIZE_POLICY_SENSOR_FLAG,
AUTO_MATERIALIZE_POLICY_SENSOR_FLAG_QUERY,
);
if (!data) {
return 'unknown';
}
return data?.instance.useAutoMaterializeSensors ? 'has-sensor-amp' : 'has-global-amp';
};

const AUTO_MATERIALIZE_POLICY_SENSOR_FLAG = gql`
export const AUTO_MATERIALIZE_POLICY_SENSOR_FLAG_QUERY = gql`
query AutoMaterializeSensorFlag {
instance {
id
Expand Down
12 changes: 9 additions & 3 deletions js_modules/dagster-ui/packages/ui-core/src/instance/DaemonList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {useUnscopedPermissions} from '../app/Permissions';
import {PYTHON_ERROR_FRAGMENT} from '../app/PythonErrorFragment';
import {Timestamp} from '../app/time/Timestamp';
import {AutoMaterializeExperimentalTag} from '../assets/AutoMaterializePolicyPage/AutoMaterializeExperimentalBanner';
import {useAutoMaterializeSensorFlag} from '../assets/AutoMaterializeSensorFlag';
import {useAutomaterializeDaemonStatus} from '../assets/useAutomaterializeDaemonStatus';
import {testId} from '../testing/testId';
import {TimeFromNow} from '../ui/TimeFromNow';

interface DaemonLabelProps {
Expand Down Expand Up @@ -47,6 +49,8 @@ export const DaemonList = ({daemonStatuses, showTimestampColumn = true}: Props)
const assetDaemon = daemonStatuses?.filter((daemon) => daemon.daemonType === 'ASSET')[0];
const nonAssetDaemons = daemonStatuses?.filter((daemon) => daemon.daemonType !== 'ASSET');

const hasGlobalAMP = useAutoMaterializeSensorFlag() === 'has-global-amp';

const confirm = useConfirmation();

const {permissions: {canToggleAutoMaterialize} = {}} = useUnscopedPermissions();
Expand All @@ -70,8 +74,10 @@ export const DaemonList = ({daemonStatuses, showTimestampColumn = true}: Props)
<AutoMaterializeExperimentalTag />
</Box>
{automaterialize.loading ? (
<Spinner purpose="body-text" />
) : (
<div data-testid={testId('loading-spinner')}>
<Spinner purpose="body-text" />
</div>
) : hasGlobalAMP ? (
<Checkbox
format="switch"
checked={!automaterialize.paused}
Expand All @@ -88,7 +94,7 @@ export const DaemonList = ({daemonStatuses, showTimestampColumn = true}: Props)
automaterialize.setPaused(!checked);
}}
/>
)}
) : null}
</Box>
</td>
<td>
Expand Down
87 changes: 68 additions & 19 deletions js_modules/dagster-ui/packages/ui-core/src/instance/__tests__/DaemonList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,21 @@ import {render, screen, waitFor} from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import {CustomConfirmationProvider} from '../../app/CustomConfirmationProvider';
import {GetAutoMaterializePausedQuery} from '../../assets/types/useAutomaterializeDaemonStatus.types';
import {AUTO_MATERIALIZE_POLICY_SENSOR_FLAG_QUERY} from '../../assets/AutoMaterializeSensorFlag';
import {
AutoMaterializeSensorFlagQuery,
AutoMaterializeSensorFlagQueryVariables,
} from '../../assets/types/AutoMaterializeSensorFlag.types';
import {
GetAutoMaterializePausedQuery,
GetAutoMaterializePausedQueryVariables,
} from '../../assets/types/useAutomaterializeDaemonStatus.types';
import {
AUTOMATERIALIZE_PAUSED_QUERY,
SET_AUTOMATERIALIZE_PAUSED_MUTATION,
} from '../../assets/useAutomaterializeDaemonStatus';
import {buildDaemonStatus, buildInstance} from '../../graphql/types';
import {buildQueryMock, getMockResultFn} from '../../testing/mocking';
import {DaemonList} from '../DaemonList';

jest.mock('../../app/Permissions', () => ({
Expand All @@ -17,9 +26,29 @@ jest.mock('../../app/Permissions', () => ({
},
}));

jest.mock('../../assets/AutoMaterializeSensorFlag', () => ({
useAutoMaterializeSensorFlag: jest.fn().mockReturnValue('has-global-amp'),
}));
const GlobalAMPMock = buildQueryMock<
AutoMaterializeSensorFlagQuery,
AutoMaterializeSensorFlagQueryVariables
>({
query: AUTO_MATERIALIZE_POLICY_SENSOR_FLAG_QUERY,
data: {
instance: buildInstance({
useAutoMaterializeSensors: false,
}),
},
});

const SensorAMPMock = buildQueryMock<
AutoMaterializeSensorFlagQuery,
AutoMaterializeSensorFlagQueryVariables
>({
query: AUTO_MATERIALIZE_POLICY_SENSOR_FLAG_QUERY,
data: {
instance: buildInstance({
useAutoMaterializeSensors: true,
}),
},
});

const mockDaemons = [
buildDaemonStatus({
Expand Down Expand Up @@ -49,27 +78,25 @@ const mockDaemons = [
];

function autoMaterializePausedMock(paused: boolean): MockedResponse<GetAutoMaterializePausedQuery> {
return {
request: {
query: AUTOMATERIALIZE_PAUSED_QUERY,
const mock = buildQueryMock<
GetAutoMaterializePausedQuery,
GetAutoMaterializePausedQueryVariables
>({
query: AUTOMATERIALIZE_PAUSED_QUERY,
data: {
instance: buildInstance({
autoMaterializePaused: paused,
}),
},
result: jest.fn(() => {
return {
data: {
__typename: 'Query',
instance: buildInstance({
autoMaterializePaused: paused,
}),
},
};
}),
};
});
getMockResultFn(mock);
return mock;
}

describe('DaemonList', () => {
it('renders daemons correctly', async () => {
const {findByText, queryByText} = render(
<MockedProvider mocks={[autoMaterializePausedMock(false)]}>
<MockedProvider mocks={[autoMaterializePausedMock(false), GlobalAMPMock]}>
<DaemonList daemonStatuses={mockDaemons} />
</MockedProvider>,
);
Expand Down Expand Up @@ -99,6 +126,7 @@ describe('DaemonList', () => {
autoMaterializePausedMock(false),
setAutoMaterializePausedMock,
autoMaterializePausedMock(true),
GlobalAMPMock,
]}
>
<CustomConfirmationProvider>
Expand All @@ -107,6 +135,8 @@ describe('DaemonList', () => {
</MockedProvider>,
);

await waitFor(() => expect(screen.queryByTestId('loading-spinner')).toBe(null));

const switchButton = await screen.findByRole('checkbox');
expect(switchButton).toBeChecked();
expect(switchButton).toBeEnabled();
Expand Down Expand Up @@ -137,6 +167,7 @@ describe('DaemonList', () => {
autoMaterializePausedMock(true),
setAutoMaterializePausedMock,
autoMaterializePausedMock(false),
GlobalAMPMock,
]}
>
<CustomConfirmationProvider>
Expand All @@ -145,11 +176,29 @@ describe('DaemonList', () => {
</MockedProvider>,
);

await waitFor(() => expect(screen.queryByTestId('loading-spinner')).toBe(null));

const switchButton = await screen.findByRole('checkbox');
expect(switchButton).not.toBeChecked();
expect(switchButton).toBeEnabled();

await userEvent.click(switchButton);
await waitFor(() => expect(setAutoMaterializePausedMock.result).toHaveBeenCalled());
});

it('hides Auto-materializing toggle ifusing automaterialize sensors', async () => {
render(
<MockedProvider mocks={[autoMaterializePausedMock(false), SensorAMPMock]}>
<CustomConfirmationProvider>
<DaemonList daemonStatuses={mockDaemons} />
</CustomConfirmationProvider>
</MockedProvider>,
);

await waitFor(() => expect(screen.queryByTestId('loading-spinner')).toBe(null));

const switchButton = await screen.queryByRole('checkbox');

expect(switchButton).toBe(null);
});
});

1 comment on commit c490734

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

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

Please sign in to comment.