Skip to content

Commit 9ec5dfa

Browse files
andrii-iandrii-i
committed
Utilize more targeted model update function for NotificationsConfig
1 parent 7755333 commit 9ec5dfa

File tree

3 files changed

+60
-63
lines changed

3 files changed

+60
-63
lines changed

‎src/components/notifications-config.tsx

Lines changed: 36 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -11,83 +11,67 @@ import {
1111
Switch,
1212
TextField
1313
} from '@mui/material';
14-
import { ICreateJobModel } from '../model';
14+
import { INotificationsConfig } from '../model';
1515
import Select, { SelectChangeEvent } from '@mui/material/Select';
1616
import { Stack } from './stack';
1717
import { useTranslator } from '../hooks';
1818

1919
type NotificationsConfigProps = {
2020
notificationEvents: string[];
2121
id: string;
22-
model: ICreateJobModel;
23-
handleModelChange: (model: ICreateJobModel) => void;
22+
notificationsConfig: INotificationsConfig | undefined;
23+
notificationsConfigChange: (
24+
updatedFields: Partial<INotificationsConfig>
25+
) => void;
2426
};
2527

2628
export function NotificationsConfig(
2729
props: NotificationsConfigProps
2830
): JSX.Element | null {
2931
const trans = useTranslator('jupyterlab');
3032
const [selectedEvents, setSelectedEvents] = useState<string[]>(
31-
props.model.notificationsConfig?.selectedEvents || []
32-
);
33-
const [enableNotification, setEnableNotification] = useState<boolean>(
34-
props.model.notificationsConfig?.enableNotification ?? true
33+
props.notificationsConfig?.selectedEvents || []
3534
);
3635
const [sendToInput, setSendToInput] = useState<string>(
37-
props.model.notificationsConfig?.sendTo?.join(', ') || ''
36+
props.notificationsConfig?.sendTo?.join(', ') || ''
3837
);
3938
const [includeOutput, setIncludeOutput] = useState<boolean>(
40-
props.model.notificationsConfig?.includeOutput || false
39+
props.notificationsConfig?.includeOutput || false
4140
);
4241

43-
const enableNotificationChange = (e: React.ChangeEvent<HTMLInputElement>) => {
44-
const updatedEnableNotification = e.target.checked;
45-
setEnableNotification(updatedEnableNotification);
46-
props.handleModelChange({
47-
...props.model,
48-
notificationsConfig: {
49-
...props.model.notificationsConfig,
50-
enableNotification: updatedEnableNotification
51-
}
42+
function enableNotificationChange(e: React.ChangeEvent<HTMLInputElement>) {
43+
props.notificationsConfigChange({
44+
enableNotification: e.target.checked
5245
});
53-
};
46+
}
5447

5548
const selectChange = useCallback(
5649
(e: SelectChangeEvent<string>) => {
5750
const newEvent = e.target.value;
5851
if (!selectedEvents.includes(newEvent)) {
5952
const updatedEvents = [...selectedEvents, newEvent];
6053
setSelectedEvents(updatedEvents);
61-
props.handleModelChange({
62-
...props.model,
63-
notificationsConfig: {
64-
...props.model.notificationsConfig,
65-
selectedEvents: updatedEvents
66-
}
54+
props.notificationsConfigChange({
55+
selectedEvents: updatedEvents
6756
});
6857
}
6958
},
70-
[selectedEvents, props.model, props.handleModelChange]
59+
[selectedEvents, props.notificationsConfig]
7160
);
7261

73-
const sendToChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
62+
function sendToChange(e: React.ChangeEvent<HTMLInputElement>) {
7463
setSendToInput(e.target.value);
75-
}, []);
64+
}
7665

7766
const blur = useCallback(() => {
7867
const emailArray = sendToInput
7968
.split(',')
8069
.map(email => email.trim())
8170
.filter(email => email);
82-
const updatedNotification = {
83-
...props.model.notificationsConfig,
71+
props.notificationsConfigChange({
8472
sendTo: emailArray
85-
};
86-
props.handleModelChange({
87-
...props.model,
88-
notificationsConfig: updatedNotification
8973
});
90-
}, [sendToInput, props.model, props.handleModelChange]);
74+
}, [sendToInput, props.notificationsConfigChange]);
9175

9276
const keyDown = useCallback(
9377
(e: React.KeyboardEvent) => {
@@ -99,33 +83,28 @@ export function NotificationsConfig(
9983
[blur]
10084
);
10185

102-
const includeOutputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
103-
const updatedValue = event.target.checked;
104-
setIncludeOutput(updatedValue);
105-
props.handleModelChange({
106-
...props.model,
107-
notificationsConfig: {
108-
...props.model.notificationsConfig,
86+
const includeOutputChange = useCallback(
87+
(event: React.ChangeEvent<HTMLInputElement>) => {
88+
const updatedValue = event.target.checked;
89+
setIncludeOutput(updatedValue);
90+
props.notificationsConfigChange({
10991
includeOutput: updatedValue
110-
}
111-
});
112-
};
92+
});
93+
},
94+
[props.notificationsConfigChange]
95+
);
11396

11497
const deleteSelectedEvent = useCallback(
11598
(eventToDelete: string) => () => {
11699
const updatedEvents = selectedEvents.filter(
117100
event => event !== eventToDelete
118101
);
119102
setSelectedEvents(updatedEvents);
120-
props.handleModelChange({
121-
...props.model,
122-
notifications: {
123-
...props.model.notificationsConfig,
124-
selectedEvents: updatedEvents
125-
}
103+
props.notificationsConfigChange({
104+
selectedEvents: updatedEvents
126105
});
127106
},
128-
[selectedEvents, props.model, props.handleModelChange]
107+
[selectedEvents, props.notificationsConfigChange]
129108
);
130109

131110
if (!props.notificationEvents.length) {
@@ -138,7 +117,7 @@ export function NotificationsConfig(
138117
<FormControlLabel
139118
control={
140119
<Switch
141-
checked={enableNotification}
120+
checked={props.notificationsConfig?.enableNotification ?? true}
142121
onChange={enableNotificationChange}
143122
/>
144123
}
@@ -152,27 +131,27 @@ export function NotificationsConfig(
152131
onChange={sendToChange}
153132
onBlur={blur}
154133
onKeyDown={keyDown}
155-
disabled={!enableNotification}
134+
disabled={!props.notificationsConfig?.enableNotification}
156135
/>
157136
<NotificationEventsSelect
158137
id={props.id}
159138
value={props.notificationEvents.filter(
160139
e => !selectedEvents.includes(e)
161140
)}
162141
onChange={selectChange}
163-
disabled={!enableNotification}
142+
disabled={!props.notificationsConfig?.enableNotification}
164143
/>
165144
<SelectedEventsChips
166145
value={selectedEvents}
167146
onChange={deleteSelectedEvent}
168-
disabled={!enableNotification}
147+
disabled={!props.notificationsConfig?.enableNotification}
169148
/>
170149
<FormControlLabel
171150
control={
172151
<Checkbox
173152
checked={includeOutput}
174153
onChange={includeOutputChange}
175-
disabled={!enableNotification}
154+
disabled={!props.notificationsConfig?.enableNotification}
176155
/>
177156
}
178157
label={trans.__('Include output')}

‎src/mainviews/create-job.tsx

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,19 @@ import { Cluster } from '../components/cluster';
1111
import { ComputeTypePicker } from '../components/compute-type-picker';
1212
import { CreateScheduleOptions } from '../components/create-schedule-options';
1313
import { EnvironmentPicker } from '../components/environment-picker';
14-
import { NotificationsConfig } from '../components/notifications-config';
1514
import {
1615
OutputFormatPicker,
1716
outputFormatsForEnvironment
1817
} from '../components/output-format-picker';
1918
import { ParametersPicker } from '../components/parameters-picker';
2019
import { Scheduler, SchedulerService } from '../handler';
2120
import { useEventLogger, useTranslator } from '../hooks';
22-
import { ICreateJobModel, IJobParameter, JobsView } from '../model';
21+
import {
22+
ICreateJobModel,
23+
IJobParameter,
24+
INotificationsConfig,
25+
JobsView
26+
} from '../model';
2327
import { Scheduler as SchedulerTokens } from '../tokens';
2428
import { NameError } from '../util/job-name-validation';
2529

@@ -42,6 +46,7 @@ import {
4246
} from '@mui/material';
4347

4448
import { Box, Stack } from '@mui/system';
49+
import { NotificationsConfig } from '../components/notifications-config';
4550

4651
export interface ICreateJobProps {
4752
model: ICreateJobModel;
@@ -424,6 +429,19 @@ export function CreateJob(props: ICreateJobProps): JSX.Element {
424429
});
425430
};
426431

432+
function notificationsConfigChange(
433+
updatedFields: Partial<INotificationsConfig>
434+
) {
435+
const newModel = {
436+
...props.model,
437+
notificationsConfig: {
438+
...props.model.notificationsConfig,
439+
...updatedFields
440+
}
441+
};
442+
props.handleModelChange(newModel);
443+
}
444+
427445
const removeParameter = (idx: number) => {
428446
const newParams = props.model.parameters || [];
429447
newParams.splice(idx, 1);
@@ -547,8 +565,8 @@ export function CreateJob(props: ICreateJobProps): JSX.Element {
547565
envsByName[props.model.environment].notification_events
548566
}
549567
id={`${formPrefix}parameters`}
550-
model={props.model}
551-
handleModelChange={props.handleModelChange}
568+
notificationsConfig={props.model.notificationsConfig}
569+
notificationsConfigChange={notificationsConfigChange}
552570
/>
553571
)}
554572
<ParametersPicker

‎src/model.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export type ModelWithScheduleFields = {
7474
scheduleMinute: string;
7575
};
7676

77-
export type NotificationsConfig = {
77+
export type INotificationsConfig = {
7878
sendTo?: string[];
7979
includeOutput?: boolean;
8080
enableNotification?: boolean;
@@ -107,7 +107,7 @@ export interface ICreateJobModel
107107
tags?: string[];
108108
// Is the create button disabled due to a submission in progress?
109109
createInProgress?: boolean;
110-
notificationsConfig?: NotificationsConfig;
110+
notificationsConfig?: INotificationsConfig;
111111
}
112112

113113
export const defaultScheduleFields: ModelWithScheduleFields = {

0 commit comments

Comments
 (0)