Browse Source

fix(crons): Improve types for the schedule config (#44693)

Evan Purkhiser 2 years ago
parent
commit
9dd5b91e19
2 changed files with 52 additions and 15 deletions
  1. 16 12
      static/app/views/monitors/monitorForm.tsx
  2. 36 3
      static/app/views/monitors/types.tsx

+ 16 - 12
static/app/views/monitors/monitorForm.tsx

@@ -20,7 +20,7 @@ import commonTheme from 'sentry/utils/theme';
 import withPageFilters from 'sentry/utils/withPageFilters';
 import withProjects from 'sentry/utils/withProjects';
 
-import {Monitor, MonitorConfig, MonitorType, ScheduleType} from './types';
+import {IntervalConfig, Monitor, MonitorConfig, MonitorType, ScheduleType} from './types';
 
 const SCHEDULE_TYPES: SelectValue<ScheduleType>[] = [
   {value: ScheduleType.CRONTAB, label: 'Crontab'},
@@ -54,28 +54,32 @@ type TransformedData = {
 
 function transformData(_data: Record<string, any>, model: FormModel) {
   return model.fields.toJSON().reduce<TransformedData>((data, [k, v]) => {
-    if (k.indexOf('config.') !== 0) {
+    // We're only concerned with transforming the config
+    if (!k.startsWith('config.')) {
       data[k] = v;
       return data;
     }
 
-    if (!data.config) {
-      data.config = {};
-    }
+    // Default to empty object
+    data.config ??= {};
+
     if (k === 'config.schedule.frequency' || k === 'config.schedule.interval') {
       if (!Array.isArray(data.config.schedule)) {
-        data.config.schedule = [null, null];
+        data.config.schedule = [1, 'hour'];
       }
     }
 
-    if (k === 'config.schedule.frequency') {
-      data.config!.schedule![0] = parseInt(v as string, 10);
-    } else if (k === 'config.schedule.interval') {
-      data.config!.schedule![1] = v;
-    } else {
-      data.config[k.substr(7)] = v;
+    if (Array.isArray(data.config.schedule) && k === 'config.schedule.frequency') {
+      data.config.schedule![0] = parseInt(v as string, 10);
+      return data;
+    }
+
+    if (Array.isArray(data.config.schedule) && k === 'config.schedule.interval') {
+      data.config.schedule![1] = v as IntervalConfig['schedule'][1];
+      return data;
     }
 
+    data.config[k.substr(7)] = v;
     return data;
   }, {});
 }

+ 36 - 3
static/app/views/monitors/types.tsx

@@ -9,6 +9,14 @@ export enum MonitorType {
   UNKNOWN = 'unknown',
 }
 
+/**
+ * Some old monitor configuratiosn do NOT have a schedule_type
+ *
+ * TODO: This should be removed once we've cleaned up our old data and can
+ *       verify we don't have any config objects missing schedule_type
+ */
+type LegacyDefaultSchedule = undefined;
+
 export enum ScheduleType {
   CRONTAB = 'crontab',
   INTERVAL = 'interval',
@@ -29,14 +37,39 @@ export enum CheckInStatus {
   MISSED = 'missed',
 }
 
-export interface MonitorConfig {
+interface BaseConfig {
   checkin_margin: number;
   max_runtime: number;
-  schedule: unknown[];
-  schedule_type: ScheduleType;
   timezone: string;
 }
 
+/**
+ * The configuration object used when the schedule is a CRONTAB
+ */
+export interface CrontabConfig extends BaseConfig {
+  /**
+   * The crontab schedule
+   */
+  schedule: string;
+  schedule_type: ScheduleType.CRONTAB | LegacyDefaultSchedule;
+}
+
+/**
+ * The configuration object used when the schedule is an INTERVAL
+ */
+export interface IntervalConfig extends BaseConfig {
+  /**
+   * The interval style schedule
+   */
+  schedule: [
+    value: number,
+    interval: 'year' | 'month' | 'week' | 'day' | 'hour' | 'minute'
+  ];
+  schedule_type: ScheduleType.INTERVAL;
+}
+
+export type MonitorConfig = CrontabConfig | IntervalConfig;
+
 export interface Monitor {
   config: MonitorConfig;
   dateCreated: string;