Fix ontime aux actions (#1684)

* switch to base ui

* ensure all action types is part of the valitation

* update test

* atempt to do it in the base-ui way

* fix fixing

* allow undefide visible on set timer message

* fix set time not showing

* correct time data for aux set automation

---------

Co-authored-by: Carlos Valente <carlosvalente@pm.me>

apps/client/src/common/components/select/Select.module.scss

@@ -34,6 +34,11 @@
     cursor: not-allowed;
   }
 
+  &:focus-visible {
+    outline: 2px solid $blue-500;
+    outline-offset: 2px;
+  }
+
   &.fluid {
     width: 100%;
   }

apps/client/src/common/components/select/Select.tsx

@@ -11,6 +11,7 @@ interface SelectProps<T> extends Omit<BaseSelect.Root.Props<T>, 'items'> {
   options: {
     value: T;
     label: string;
+    disabled?: boolean;
   }[];
   fluid?: boolean;
 }
@@ -28,8 +29,8 @@ export default function Select<T>({ options, fluid, ...selectRootProps }: Select
         <BaseSelect.Positioner side='bottom' align='start'>
           <BaseSelect.ScrollUpArrow className={styles.scrollArrow} />
           <BaseSelect.Popup className={styles.popup}>
-            {options.map(({ label, value }) => (
-              <BaseSelect.Item key={String(value)} className={styles.item} value={value}>
+            {options.map(({ disabled, label, value }) => (
+              <BaseSelect.Item key={String(value)} className={styles.item} value={value} disabled={disabled}>
                 <BaseSelect.ItemIndicator className={styles.itemIndicator}>
                   <IoCheckmark className={styles.itemIndicatorIcon} />
                 </BaseSelect.ItemIndicator>

apps/client/src/features/app-settings/panel/automations-panel/AutomationForm.tsx

@@ -55,6 +55,7 @@ export default function AutomationForm(props: AutomationFormProps) {
     setError,
     setFocus,
     setValue,
+    watch,
     formState: { errors, isSubmitting, isDirty, isValid },
   } = useForm<AutomationDTO>({
     mode: 'onChange',
@@ -106,8 +107,7 @@ export default function AutomationForm(props: AutomationFormProps) {
   };
 
   const handleAddnewOntimeAction = () => {
-    // @ts-expect-error -- we dont want to choose an action
-    appendOutput({ type: 'ontime', action: undefined });
+    appendOutput({ type: 'ontime', action: 'aux1-start' });
   };
 
   const handleTestOSCOutput = async (index: number) => {
@@ -428,6 +428,7 @@ export default function AutomationForm(props: AutomationFormProps) {
                   register={register}
                   rowErrors={rowErrors}
                   setValue={setValue}
+                  watch={watch}
                 >
                   <span>&nbsp;</span>
                   <Panel.InlineElements relation='inner'>

apps/client/src/features/app-settings/panel/automations-panel/OntimeActionForm.tsx

@@ -1,10 +1,9 @@
 import { PropsWithChildren, useState } from 'react';
-import { UseFormRegister, UseFormSetValue } from 'react-hook-form';
-import { Select } from '@chakra-ui/react';
-import { AutomationDTO, OntimeAction } from 'ontime-types';
+import { UseFormRegister, UseFormSetValue, UseFormWatch } from 'react-hook-form';
+import { AutomationDTO, OntimeAction, OntimeActionKey, SecondarySource } from 'ontime-types';
 
 import Input from '../../../../common/components/input/input/Input';
-import { cx } from '../../../../common/utils/styleUtils';
+import Select from '../../../../common/components/select/Select';
 import * as Panel from '../../panel-utils/PanelUtils';
 
 import style from './AutomationForm.module.scss';
@@ -20,53 +19,58 @@ interface OntimeActionFormProps {
     secondarySource?: { message?: string };
   };
   value: OntimeAction['action'];
+  watch: UseFormWatch<AutomationDTO>;
   setValue: UseFormSetValue<AutomationDTO>;
 }
 
 export default function OntimeActionForm(props: PropsWithChildren<OntimeActionFormProps>) {
-  const { index, register, setValue, rowErrors, value, children } = props;
-  const [selectedAction, setSelectedAction] = useState<OntimeAction['action']>(value || 'aux-start');
+  const { index, register, setValue, rowErrors, value, children, watch } = props;
+  const [selectedAction, setSelectedAction] = useState<string>(value);
 
-  const updateSelectedAction = (value: string) => {
-    setSelectedAction(value as OntimeAction['action']);
-    setValue(`outputs.${index}.action`, value as OntimeAction['action']);
+  const handleSetAction = (value: OntimeActionKey) => {
+    setValue(`outputs.${index}.action`, value, { shouldDirty: true });
+    setSelectedAction(value);
   };
 
   return (
-    <div className={cx([style.actionSection, selectedAction && style[selectedAction]])}>
-      <input type='hidden' {...register(`outputs.${index}.action`)} value={selectedAction} />
+    <div className={style.actionSection}>
       <label>
         Action
         <Select
-          variant='ontime'
-          size='sm'
-          value={selectedAction}
-          onChange={(event) => updateSelectedAction(event.target.value)}
-        >
-          <option value='aux-start'>Aux 1: start</option>
-          <option value='aux-pause'>Aux 1: pause</option>
-          <option value='aux-stop'>Aux 1: stop</option>
-          <option value='aux-set'>Aux 2: set</option>
-          <option value='aux-start'>Aux 2: start</option>
-          <option value='aux-pause'>Aux 2: pause</option>
-          <option value='aux-stop'>Aux 2: stop</option>
-          <option value='aux-set'>Aux 2: set</option>
-          <option value='aux-start'>Aux 3: start</option>
-          <option value='aux-pause'>Aux 3: pause</option>
-          <option value='aux-stop'>Aux 3: stop</option>
-          <option value='aux-set'>Aux 3: set</option>
-          <option value='message-set'>Timer: timer message</option>
-          <option value='message-secondary'>Timer: timer secondary</option>
-        </Select>
+          onValueChange={(value) => {
+            handleSetAction(value as OntimeActionKey);
+          }}
+          value={watch(`outputs.${index}.action`)}
+          options={[
+            { value: 'aux1-pause', label: 'Aux 1: pause' },
+            { value: 'aux2-pause', label: 'Aux 2: pause' },
+            { value: 'aux3-pause', label: 'Aux 3: pause' },
+
+            { value: 'aux1-start', label: 'Aux 1: start' },
+            { value: 'aux2-start', label: 'Aux 2: start' },
+            { value: 'aux3-start', label: 'Aux 3: start' },
+
+            { value: 'aux1-stop', label: 'Aux 1: stop' },
+            { value: 'aux2-stop', label: 'Aux 2: stop' },
+            { value: 'aux3-stop', label: 'Aux 3: stop' },
+
+            { value: 'aux1-set', label: 'Aux 1: set' },
+            { value: 'aux2-set', label: 'Aux 2: set' },
+            { value: 'aux3-set', label: 'Aux 3: set' },
+
+            { value: 'message-set', label: 'Primary Message: set' },
+            { value: 'message-secondary', label: 'Secondary Message: source' },
+          ]}
+        />
         <Panel.Error>{rowErrors?.action?.message}</Panel.Error>
       </label>
 
-      {selectedAction === 'aux1-set' && (
+      {selectedAction.startsWith('aux') && selectedAction.endsWith('set') && (
         <label>
           New time
           <Input
             {...register(`outputs.${index}.time`, {
-              required: { value: true, message: 'Required field' },
+              required: { value: true, message: 'Required field' },  //TODO:(automation set aux) not sure what way around to have the string and where to have the ms value
             })}
             fluid
             placeholder='eg: 10m5s'
@@ -84,11 +88,19 @@ export default function OntimeActionForm(props: PropsWithChildren<OntimeActionFo
           </label>
           <label>
             Visibility
-            <Select variant='ontime' size='sm' {...register(`outputs.${index}.visible`)}>
-              <option value=''>Untouched</option>
-              <option value='true'>Show</option>
-              <option value='false'>Hide</option>
-            </Select>
+            <Select
+              onValueChange={(value) => {
+                // we need to translate the undefined value to 'untouched'
+                const translatedValue = value === 'untouched' ? undefined : (value as boolean | undefined);
+                setValue(`outputs.${index}.visible`, translatedValue, { shouldDirty: true });
+              }}
+              value={watch(`outputs.${index}.visible`) === undefined ? 'untouched' : watch(`outputs.${index}.visible`)}
+              options={[
+                { value: 'untouched', label: 'Untouched' },
+                { value: true, label: 'Show' },
+                { value: false, label: 'Hide' },
+              ]}
+            />
             <Panel.Error>{rowErrors?.visible?.message}</Panel.Error>
           </label>
         </>
@@ -97,11 +109,20 @@ export default function OntimeActionForm(props: PropsWithChildren<OntimeActionFo
       {selectedAction === 'message-secondary' && (
         <label>
           Timer secondary source
-          <Select variant='ontime' size='sm' {...register(`outputs.${index}.secondarySource`)}>
-            <option value='aux'>Auxiliary timer</option>
-            <option value='external'>External</option>
-            <option value='null'>None</option>
-          </Select>
+          <Select
+            onValueChange={(value) => {
+              setValue(`outputs.${index}.secondarySource`, value as SecondarySource, { shouldDirty: true });
+            }}
+            value={watch(`outputs.${index}.secondarySource`)}
+            options={[
+              { value: null, label: 'Select secondary source', disabled: true },
+              { value: 'aux1', label: 'Auxiliary timer 1' },
+              { value: 'aux2', label: 'Auxiliary timer 2' },
+              { value: 'aux3', label: 'Auxiliary timer 3' },
+              { value: 'external', label: 'External' },
+              { value: 'null', label: 'None' },
+            ]}
+          />
           <Panel.Error>{rowErrors?.secondarySource?.message}</Panel.Error>
         </label>
       )}

apps/server/src/api-data/automation/__tests__/automation.validation.test.ts

@@ -46,17 +46,17 @@ describe('parseOutput', () => {
     it('parses a valid payload', () => {
       const auxStart = {
         type: 'ontime',
-        action: 'aux-start',
+        action: 'aux1-start',
       };
       expect(parseOutput(auxStart)).toStrictEqual(auxStart);
       const auxStop = {
         type: 'ontime',
-        action: 'aux-stop',
+        action: 'aux3-stop',
       };
       expect(parseOutput(auxStop)).toStrictEqual(auxStop);
       const auxPause = {
         type: 'ontime',
-        action: 'aux-pause',
+        action: 'aux2-pause',
       };
       expect(parseOutput(auxPause)).toStrictEqual(auxPause);
     });
@@ -65,12 +65,12 @@ describe('parseOutput', () => {
       expect(
         parseOutput({
           type: 'ontime',
-          action: 'aux-start',
+          action: 'aux1-start',
           time: 10,
         }),
       ).toStrictEqual({
         type: 'ontime',
-        action: 'aux-start',
+        action: 'aux1-start',
       });
     });
 
@@ -88,7 +88,7 @@ describe('parseOutput', () => {
           type: 'ontime',
           action: 'message-set',
           text: 'test',
-          visible: 'true',
+          visible: true,
         }),
       ).toMatchObject({
         text: 'test',
@@ -99,7 +99,7 @@ describe('parseOutput', () => {
           type: 'ontime',
           action: 'message-set',
           text: '',
-          visible: 'false',
+          visible: false,
         }),
       ).toMatchObject({
         text: undefined,
@@ -110,7 +110,6 @@ describe('parseOutput', () => {
           type: 'ontime',
           action: 'message-set',
           text: '',
-          visible: '',
         }),
       ).toMatchObject({
         text: undefined,

apps/server/src/api-data/automation/automation.utils.ts

@@ -1,4 +1,4 @@
-import { EntryId, FilterRule, isOntimeEvent, MaybeNumber, OntimeAction, Rundown } from 'ontime-types';
+import { EntryId, FilterRule, isOntimeEvent, MaybeNumber, OntimeAction, ontimeActionKeyValues, Rundown } from 'ontime-types';
 import { millisToString, removeLeadingZero, splitWhitespace, getPropertyFromPath } from 'ontime-utils';
 import type { OscArgOrArrayInput, OscArgInput } from 'osc-min';
 
@@ -13,7 +13,7 @@ export function isFilterRule(value: string): value is FilterRule {
 }
 
 export function isOntimeActionAction(value: string): value is OntimeAction['action'] {
-  return ['aux-start', 'aux-stop', 'aux-pause', 'aux-set', 'message-set', 'message-secondary'].includes(value);
+  return ontimeActionKeyValues.includes(value);
 }
 
 function toOscValue(argString: string): OscArgInput {

apps/server/src/api-data/automation/automation.validation.ts

@@ -8,7 +8,6 @@ import {
   SecondarySource,
   timerLifecycleValues,
 } from 'ontime-types';
-import { parseUserTime } from 'ontime-utils';
 
 import { body, oneOf, param } from 'express-validator';
 
@@ -200,20 +199,24 @@ function parseOntimeAction(maybeOntimeAction: object): OntimeAction {
     return {
       type: 'ontime',
       action: maybeOntimeAction.action,
-      time: parseUserTime(maybeOntimeAction.time),
+      time: maybeOntimeAction.time, //TODO:(automation set aux) not sure what way around to have the string and where to have the ms value
     };
   }
 
   if (maybeOntimeAction.action === 'message-set') {
-    assert.hasKeys(maybeOntimeAction, ['text', 'visible']);
+    assert.hasKeys(maybeOntimeAction, ['text']);
     assert.isString(maybeOntimeAction.text);
-    assert.isString(maybeOntimeAction.visible);
+    let visible: boolean | undefined = undefined;
+    if ('visible' in maybeOntimeAction) {
+      assert.isBoolean(maybeOntimeAction.visible);
+      visible = maybeOntimeAction.visible;
+    }
 
     return {
       type: 'ontime',
       action: 'message-set',
       text: indeterminateText(maybeOntimeAction.text),
-      visible: indeterminateBooleanString(maybeOntimeAction.visible),
+      visible,
     };
   }
 
@@ -243,16 +246,6 @@ function indeterminateText(value: string): string | undefined {
   return value === '' ? undefined : value;
 }
 
-/**
- * Helper function to parse boolean values in transit
- * "true" -> true
- * "false" -> false
- * "" | "null" -> undefined
- */
-function indeterminateBooleanString(value: string): boolean | undefined {
-  return value === '' ? undefined : value === 'true';
-}
-
 /**
  * Helper function to validate the secondary source
  */

apps/server/src/api-data/automation/clients/ontime.client.ts

@@ -3,6 +3,7 @@ import { LogOrigin, OntimeAction } from 'ontime-types';
 import { logger } from '../../../classes/Logger.js';
 import { auxTimerService } from '../../../services/aux-timer-service/AuxTimerService.js';
 import * as messageService from '../../../services/message-service/message.service.js';
+import { parseUserTime } from 'ontime-utils';
 
 export function toOntimeAction(action: OntimeAction) {
   const actionType = action.action;
@@ -15,7 +16,8 @@ export function toOntimeAction(action: OntimeAction) {
     case 'aux1-pause':
       return auxTimerService.pause(1);
     case 'aux1-set': {
-      return auxTimerService.setTime(action.time, 1);
+      const time = parseUserTime(action.time);
+      return auxTimerService.setTime(time, 1);
     }
     case 'aux2-start':
       return auxTimerService.start(2);
@@ -24,7 +26,8 @@ export function toOntimeAction(action: OntimeAction) {
     case 'aux2-pause':
       return auxTimerService.pause(2);
     case 'aux2-set': {
-      return auxTimerService.setTime(action.time, 2);
+      const time = parseUserTime(action.time);
+      return auxTimerService.setTime(time, 2);
     }
     case 'aux3-start':
       return auxTimerService.start(3);
@@ -33,7 +36,8 @@ export function toOntimeAction(action: OntimeAction) {
     case 'aux3-pause':
       return auxTimerService.pause(3);
     case 'aux3-set': {
-      return auxTimerService.setTime(action.time, 3);
+      const time = parseUserTime(action.time);
+      return auxTimerService.setTime(time, 3);
     }
 
     // Message actions

apps/server/src/utils/assert.ts

@@ -1,5 +1,11 @@
 import { is } from './is.js';
 
+export function isBoolean(value: unknown): asserts value is boolean {
+  if (!is.boolean(value)) {
+    throw new Error(`Unexpected payload type: ${String(value)}`);
+  }
+}
+
 export function isString(value: unknown): asserts value is string {
   if (!is.string(value)) {
     throw new Error(`Unexpected payload type: ${String(value)}`);

apps/server/src/utils/is.ts

@@ -1,4 +1,5 @@
 export const is = {
+  boolean: (value: unknown): value is boolean => typeof value === 'boolean',
   string: (value: unknown): value is string => typeof value === 'string',
   number: (value: unknown): value is number => typeof value === 'number',
   defined: <T>(value: T | undefined): value is T => value !== undefined,

packages/types/src/definitions/core/Automation.type.ts

@@ -54,33 +54,52 @@ export type HTTPOutput = {
   url: string;
 };
 
+const ontimeAuxTriggerAction = [
+  'aux1-start',
+  'aux1-stop',
+  'aux1-pause',
+  'aux2-start',
+  'aux2-stop',
+  'aux2-pause',
+  'aux3-start',
+  'aux3-stop',
+  'aux3-pause',
+] as const;
+
+const ontimeAuxSetAction = ['aux1-set', 'aux2-set', 'aux3-set'] as const;
+
+type OntimeAuxTriggerAction = (typeof ontimeAuxTriggerAction)[number];
+type OntimeAuxSetAction = (typeof ontimeAuxSetAction)[number];
+type OntimeMessageSet = 'message-set';
+type OntimeMessageSecondary = 'message-secondary';
+
+export type OntimeActionKey = OntimeAuxTriggerAction | OntimeAuxSetAction | OntimeMessageSet | OntimeMessageSecondary;
+
+export const ontimeActionKeyValues = [
+  ...ontimeAuxTriggerAction,
+  ...ontimeAuxSetAction,
+  'message-set',
+  'message-secondary',
+];
+
 export type OntimeAction =
   | {
       type: 'ontime';
-      action:
-        | 'aux1-start'
-        | 'aux1-stop'
-        | 'aux1-pause'
-        | 'aux2-start'
-        | 'aux2-stop'
-        | 'aux2-pause'
-        | 'aux3-start'
-        | 'aux3-stop'
-        | 'aux3-pause';
+      action: OntimeAuxTriggerAction;
     }
   | {
       type: 'ontime';
-      action: 'aux1-set' | 'aux2-set' | 'aux3-set';
-      time: number;
+      action: OntimeAuxSetAction;
+      time: string;  //TODO:(automation set aux) not sure what way around to have the string and where to have the ms value
     }
   | {
       type: 'ontime';
-      action: 'message-set';
+      action: OntimeMessageSet;
       text?: string;
       visible?: boolean;
     }
   | {
       type: 'ontime';
-      action: 'message-secondary';
+      action: OntimeMessageSecondary;
       secondarySource: SecondarySource;
     };

packages/types/src/index.ts

@@ -24,7 +24,9 @@ export { TimerType } from './definitions/TimerType.type.js';
 export type { OntimeReport, OntimeEventReport } from './definitions/core/Report.type.js';
 
 // ---> Automations
+export { ontimeActionKeyValues } from './definitions/core/Automation.type.js';
 export type {
+  OntimeActionKey,
   Automation,
   AutomationDTO,
   AutomationFilter,