make custom fields case sensitive (#1242)

* remove custom field lowercasing

* don't allow custom field form to submit duplicate field

* remove unused

* update custom field tests

* also keep upper case when editing

* show key in UI

* also kep upper case when creating a new field

* fix test

* allow old forced case keys to stay as is

* allow space

* add extension to import

* supply initial key to CustomFieldForm

* refactor: improve element contrast

* refactor: style and composition

* consistent use of `customFieldLabelToKey`

* fix CopyTag

* remove log

---------

Co-authored-by: arc-alex <ac@omnivox.dk>
Co-authored-by: Carlos Valente <carlosvalente@pm.me>

apps/client/src/externals.ts

@@ -3,3 +3,4 @@ export const apiRepoLatest = 'https://api.github.com/repos/cpvalente/ontime/rele
 export const websiteUrl = 'https://www.getontime.no';
 
 export const documentationUrl = 'https://docs.getontime.no';
+export const customFieldsDocsUrl = 'https://docs.getontime.no/features/custom-fields/';

apps/client/src/features/app-settings/panel/feature-settings-panel/FeatureSettings.module.scss

@@ -1,3 +1,7 @@
+.halfWidth {
+  width: 50%;
+}
+
 .fullWidth {
   width: 100%;
 }
@@ -37,3 +41,9 @@
 .flex {
   display: flex;
 }
+
+.twoCols {
+  display: grid;
+  grid-template-columns: 1fr 1fr;
+  gap: 1rem;
+}

apps/client/src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFieldEntry.tsx

@@ -4,6 +4,7 @@ import { IoPencil } from '@react-icons/all-files/io5/IoPencil';
 import { IoTrash } from '@react-icons/all-files/io5/IoTrash';
 import { CustomField, CustomFieldLabel } from 'ontime-types';
 
+import CopyTag from '../../../../../common/components/copy-tag/CopyTag';
 import Swatch from '../../../../../common/components/input/colour-input/Swatch';
 
 import CustomFieldForm from './CustomFieldForm';
@@ -36,6 +37,7 @@ export default function CustomFieldEntry(props: CustomFieldEntryProps) {
             onSubmit={handleEdit}
             initialColour={colour}
             initialLabel={label}
+            initialKey={field}
           />
         </td>
       </tr>
@@ -47,7 +49,12 @@ export default function CustomFieldEntry(props: CustomFieldEntryProps) {
       <td>
         <Swatch color={colour} />
       </td>
-      <td className={style.fullWidth}>{label}</td>
+      <td className={style.halfWidth}>{label}</td>
+      <td className={style.fullWidth}>
+        <CopyTag label='Copy key to use in integrations' copyValue={field}>
+          {field}
+        </CopyTag>
+      </td>
       <td className={style.actions}>
         <IconButton
           size='sm'

apps/client/src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFieldForm.tsx

@@ -2,10 +2,11 @@ import { useEffect, useState } from 'react';
 import { useForm } from 'react-hook-form';
 import { Button, Input } from '@chakra-ui/react';
 import { CustomField } from 'ontime-types';
-import { isAlphanumeric } from 'ontime-utils';
+import { customFieldLabelToKey, isAlphanumericWithSpace } from 'ontime-utils';
 
 import { maybeAxiosError } from '../../../../../common/api/utils';
 import SwatchSelect from '../../../../../common/components/input/colour-input/SwatchSelect';
+import useCustomFields from '../../../../../common/hooks-query/useCustomFields';
 import * as Panel from '../../PanelUtils';
 
 import style from '../FeatureSettings.module.scss';
@@ -15,10 +16,13 @@ interface CustomFieldsFormProps {
   onCancel: () => void;
   initialColour?: string;
   initialLabel?: string;
+  initialKey?: string;
 }
 
 export default function CustomFieldForm(props: CustomFieldsFormProps) {
-  const { onSubmit, onCancel, initialColour, initialLabel } = props;
+  const { onSubmit, onCancel, initialColour, initialLabel, initialKey } = props;
+  const { data } = useCustomFields();
+
   // we use this to force an update
   const [_, setColour] = useState(initialColour || '');
 
@@ -31,7 +35,7 @@ export default function CustomFieldForm(props: CustomFieldsFormProps) {
     getValues,
     formState: { errors, isSubmitting, isValid, isDirty },
   } = useForm({
-    defaultValues: { label: initialLabel || '', colour: initialColour || '' },
+    defaultValues: { label: initialLabel || '', colour: initialColour || '', key: initialKey || '' },
     resetOptions: {
       keepDirtyValues: true,
     },
@@ -66,28 +70,38 @@ export default function CustomFieldForm(props: CustomFieldsFormProps) {
 
   return (
     <form onSubmit={handleSubmit(setupSubmit)} className={style.fieldForm}>
-      <div className={style.column}>
-        <Panel.Description>Label (only alphanumeric characters are allowed)</Panel.Description>
-        {errors.label && <Panel.Error>{errors.label.message}</Panel.Error>}
-        <Input
-          {...register('label', {
-            required: { value: true, message: 'Required field' },
-            validate: (value) => {
-              if (value.trim().length === 0) return 'Required field';
-              if (!isAlphanumeric(value)) return 'Only alphanumeric characters are allowed';
-              return true;
-            },
-          })}
-          size='sm'
-          variant='ontime-filled'
-          autoComplete='off'
-        />
+      <div className={style.twoCols}>
+        <div>
+          <Panel.Description>Label (only alphanumeric characters are allowed)</Panel.Description>
+          {errors.label && <Panel.Error>{errors.label.message}</Panel.Error>}
+          <Input
+            {...register('label', {
+              required: { value: true, message: 'Required field' },
+              onChange: () => setValue('key', customFieldLabelToKey(getValues('label')) ?? 'N/A'),
+              validate: (value) => {
+                if (value.trim().length === 0) return 'Required field';
+                if (!isAlphanumericWithSpace(value)) return 'Only alphanumeric characters and space are allowed';
+                if (Object.keys(data).includes(value)) return 'Custom fields must be unique';
+                return true;
+              },
+            })}
+            size='sm'
+            variant='ontime-filled'
+            autoComplete='off'
+          />
+        </div>
+
+        <div>
+          <Panel.Description>Key (auto-generated value for use in Integrations and API)</Panel.Description>
+          <Input {...register('key')} disabled size='sm' variant='ontime-filled' autoComplete='off' />
+        </div>
       </div>
 
       <div>
         <Panel.Description>Colour</Panel.Description>
         <SwatchSelect name='colour' value={colour} handleChange={(_field, value) => handleSelectColour(value)} />
       </div>
+
       {errors.root && <Panel.Error>{errors.root.message}</Panel.Error>}
       <div className={style.buttonRow}>
         <Button size='sm' variant='ontime-ghosted' onClick={onCancel}>

apps/client/src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFields.tsx

@@ -6,13 +6,12 @@ import { CustomField, CustomFieldLabel } from 'ontime-types';
 import { deleteCustomField, editCustomField, postCustomField } from '../../../../../common/api/customFields';
 import ExternalLink from '../../../../../common/components/external-link/ExternalLink';
 import useCustomFields from '../../../../../common/hooks-query/useCustomFields';
+import { customFieldsDocsUrl } from '../../../../../externals';
 import * as Panel from '../../PanelUtils';
 
 import CustomFieldEntry from './CustomFieldEntry';
 import CustomFieldForm from './CustomFieldForm';
 
-const customFieldsDocsUrl = 'https://docs.getontime.no/features/custom-fields/';
-
 export default function CustomFields() {
   const { data, refetch } = useCustomFields();
   const [isAdding, setIsAdding] = useState(false);
@@ -59,9 +58,14 @@ export default function CustomFields() {
           <Alert status='info' variant='ontime-on-dark-info'>
             <AlertIcon />
             <AlertDescription>
-              Custom fields allow for additional information to be added to an event (eg. light, sound, camera). <br />
+              Custom fields allow for additional information to be added to an event.
               <br />
-              This data is not used by Ontime.
+              <br />
+              This data is not used by Ontime, but provides place for cueing or department specific information (eg.
+              light, sound, camera).
+              <br />
+              <br />
+              Custom fields can be used width the Integrations feature using the generated key.
               <ExternalLink href={customFieldsDocsUrl}>See the docs</ExternalLink>
             </AlertDescription>
           </Alert>
@@ -73,6 +77,7 @@ export default function CustomFields() {
               <tr>
                 <th>Colour</th>
                 <th>Name</th>
+                <th>Key (used in Integrations)</th>
                 <th />
               </tr>
             </thead>

apps/client/src/theme/OntimeAlert.ts

@@ -1,7 +1,7 @@
 export const ontimeAlertOnDark = {
   container: {
     fontSize: 'calc(1rem - 1px)',
-    backgroundColor: '#1a1a1a', // $gray-1300
+    backgroundColor: '#202020', // $gray-1200
     color: '#e2e2e2', // $gray-200
     borderRadius: '3px',
   },

apps/server/src/api-data/custom-fields/customFields.validation.ts

@@ -1,4 +1,4 @@
-import { isAlphanumeric } from 'ontime-utils';
+import { isAlphanumericWithSpace } from 'ontime-utils';
 
 import { Request, Response, NextFunction } from 'express';
 import { body, param, validationResult } from 'express-validator';
@@ -9,7 +9,7 @@ export const validateCustomField = [
     .isString()
     .trim()
     .custom((value) => {
-      return isAlphanumeric(value);
+      return isAlphanumericWithSpace(value);
     }),
   body('type').exists().isString().trim(),
   body('colour').exists().isString().trim(),

apps/server/src/services/integration-service/OscIntegration.ts

@@ -85,7 +85,7 @@ export class OscIntegration implements IIntegration<OscSubscription, OSCSettings
   private initTX(enabledOut: boolean, targetIP: string, portOut: number, subscriptions: OscSubscription[]) {
     this.initSubscriptions(subscriptions);
 
-    if (!enabledOut && this.enabledOut) {
+    if (!enabledOut) {
       this.targetIP = targetIP;
       this.portOut = portOut;
       this.enabledOut = enabledOut;
@@ -104,6 +104,7 @@ export class OscIntegration implements IIntegration<OscSubscription, OSCSettings
 
     try {
       this.oscClient = new Client(targetIP, portOut);
+      logger.info(LogOrigin.Tx, `Starting OSC Clint on port: ${portOut}`);
     } catch (error) {
       this.oscClient = null;
       throw new Error(`Failed initialising OSC client: ${error}`);
@@ -111,7 +112,7 @@ export class OscIntegration implements IIntegration<OscSubscription, OSCSettings
   }
 
   private initRX(enabledIn: boolean, portIn: number) {
-    if (!enabledIn && this.enabledIn) {
+    if (!enabledIn) {
       this.shutdownRX();
       return;
     }

apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts

@@ -925,7 +925,7 @@ describe('custom fields', () => {
   describe('createCustomField()', () => {
     it('creates a field from given parameters', async () => {
       const expected = {
-        lighting: {
+        Lighting: {
           label: 'Lighting',
           type: 'string',
           colour: 'blue',
@@ -942,19 +942,19 @@ describe('custom fields', () => {
       await createCustomField({ label: 'Sound', type: 'string', colour: 'blue' });
 
       const expected = {
-        lighting: {
+        Lighting: {
           label: 'Lighting',
           type: 'string',
           colour: 'blue',
         },
-        sound: {
+        Sound: {
           label: 'Sound',
           type: 'string',
           colour: 'green',
         },
       };
 
-      const customField = await editCustomField('sound', { label: 'Sound', type: 'string', colour: 'green' });
+      const customField = await editCustomField('Sound', { label: 'Sound', type: 'string', colour: 'green' });
       expect(customFieldChangelog).toStrictEqual(new Map());
 
       expect(customField).toStrictEqual(expected);
@@ -964,17 +964,17 @@ describe('custom fields', () => {
       const created = await createCustomField({ label: 'Video', type: 'string', colour: 'red' });
 
       const expected = {
-        lighting: {
+        Lighting: {
           label: 'Lighting',
           type: 'string',
           colour: 'blue',
         },
-        sound: {
+        Sound: {
           label: 'Sound',
           type: 'string',
           colour: 'green',
         },
-        video: {
+        Video: {
           label: 'Video',
           type: 'string',
           colour: 'red',
@@ -984,17 +984,17 @@ describe('custom fields', () => {
       expect(created).toStrictEqual(expected);
 
       const expectedAfter = {
-        lighting: {
+        Lighting: {
           label: 'Lighting',
           type: 'string',
           colour: 'blue',
         },
-        sound: {
+        Sound: {
           label: 'Sound',
           type: 'string',
           colour: 'green',
         },
-        av: {
+        AV: {
           label: 'AV',
           type: 'string',
           colour: 'red',
@@ -1003,10 +1003,10 @@ describe('custom fields', () => {
 
       // We need to flush all scheduled tasks for the generate function to settle
       vi.useFakeTimers();
-      const customField = await editCustomField('video', { label: 'AV', type: 'string', colour: 'red' });
+      const customField = await editCustomField('Video', { label: 'AV', type: 'string', colour: 'red' });
       expect(customField).toStrictEqual(expectedAfter);
-      expect(customFieldChangelog).toStrictEqual(new Map([['video', 'av']]));
-      await editCustomField('av', { label: 'video' });
+      expect(customFieldChangelog).toStrictEqual(new Map([['Video', 'AV']]));
+      await editCustomField('AV', { label: 'Video' });
       vi.runAllTimers();
       expect(customFieldChangelog).toStrictEqual(new Map());
       vi.useRealTimers();
@@ -1016,19 +1016,19 @@ describe('custom fields', () => {
   describe('removeCustomField()', () => {
     it('deletes a field with a given label', async () => {
       const expected = {
-        lighting: {
+        Lighting: {
           label: 'Lighting',
           type: 'string',
           colour: 'blue',
         },
-        video: {
-          label: 'video',
+        Video: {
+          label: 'Video',
           type: 'string',
           colour: 'red',
         },
       };
 
-      const customField = await removeCustomField('sound');
+      const customField = await removeCustomField('Sound');
 
       expect(customField).toStrictEqual(expected);
     });

apps/server/src/services/rundown-service/rundownCache.ts

@@ -11,7 +11,15 @@ import {
   OntimeRundownEntry,
   PlayableEvent,
 } from 'ontime-types';
-import { generateId, insertAtIndex, reorderArray, swapEventData, getTimeFromPrevious, isNewLatest } from 'ontime-utils';
+import {
+  generateId,
+  insertAtIndex,
+  reorderArray,
+  swapEventData,
+  getTimeFromPrevious,
+  isNewLatest,
+  customFieldLabelToKey,
+} from 'ontime-utils';
 import { getDataProvider } from '../../classes/data-provider/DataProvider.js';
 import { createPatch } from '../../utils/parser.js';
 import { apply } from './delayUtils.js';
@@ -447,7 +455,7 @@ function scheduleCustomFieldPersist(persistedCustomFields: CustomFields) {
  */
 export const createCustomField = async (field: CustomField) => {
   const { label, type, colour } = field;
-  const key = label.toLowerCase();
+  const key = customFieldLabelToKey(label);
   // check if label already exists
   const alreadyExists = Object.hasOwn(persistedCustomFields, key);
 
@@ -479,7 +487,7 @@ export const editCustomField = async (key: string, newField: Partial<CustomField
     throw new Error('Change of field type is not allowed');
   }
 
-  const newKey = newField.label.toLowerCase();
+  const newKey = customFieldLabelToKey(newField.label);
   persistedCustomFields[newKey] = { ...existingField, ...newField };
 
   if (key !== newKey) {

apps/server/src/utils/__tests__/parserFunctions.test.ts

@@ -279,7 +279,7 @@ describe('sanitiseCustomFields()', () => {
     const customFields: CustomFields = {
       test: { label: 'test', type: 'string', colour: 'red' },
       test2: { label: 'test2', type: 'string', colour: 'green' },
-      test3: { label: 'Test3', type: 'string', colour: '' },
+      Test3: { label: 'Test3', type: 'string', colour: '' },
     };
     const sanitationResult = sanitiseCustomFields(customFields);
     expect(sanitationResult).toStrictEqual(customFields);
@@ -328,10 +328,32 @@ describe('sanitiseCustomFields()', () => {
 
   it('enforce name cohesion', () => {
     const customFields: CustomFields = {
-      test: { label: 'New Name', type: 'string', colour: 'red' },
+      test: { label: 'NewName', type: 'string', colour: 'red' },
     };
     const expectedCustomFields: CustomFields = {
-      'new name': { label: 'New Name', type: 'string', colour: 'red' },
+      NewName: { label: 'NewName', type: 'string', colour: 'red' },
+    };
+    const sanitationResult = sanitiseCustomFields(customFields);
+    expect(sanitationResult).toStrictEqual(expectedCustomFields);
+  });
+
+  it('allow old keys', () => {
+    const customFields: CustomFields = {
+      test: { label: 'Test', type: 'string', colour: 'red' },
+    };
+    const expectedCustomFields: CustomFields = {
+      test: { label: 'Test', type: 'string', colour: 'red' },
+    };
+    const sanitationResult = sanitiseCustomFields(customFields);
+    expect(sanitationResult).toStrictEqual(expectedCustomFields);
+  });
+
+  it('labels with space', () => {
+    const customFields: CustomFields = {
+      Test_with_Space: { label: 'Test with Space', type: 'string', colour: 'red' },
+    };
+    const expectedCustomFields: CustomFields = {
+      Test_with_Space: { label: 'Test with Space', type: 'string', colour: 'red' },
     };
     const sanitationResult = sanitiseCustomFields(customFields);
     expect(sanitationResult).toStrictEqual(expectedCustomFields);
@@ -342,12 +364,12 @@ describe('sanitiseCustomFields()', () => {
       test: { label: 'test', type: 'string', colour: 'red' },
       test2: { label: 'test2', type: 'string', colour: 'green' },
       bad: { label: '', type: 'string', colour: '' },
-      test3: { label: 'Test3', type: 'string', colour: '' },
+      Test3: { label: 'Test3', type: 'string', colour: '' },
     };
     const expectedCustomFields: CustomFields = {
       test: { label: 'test', type: 'string', colour: 'red' },
       test2: { label: 'test2', type: 'string', colour: 'green' },
-      test3: { label: 'Test3', type: 'string', colour: '' },
+      Test3: { label: 'Test3', type: 'string', colour: '' },
     };
     const sanitationResult = sanitiseCustomFields(customFields);
     expect(sanitationResult).toStrictEqual(expectedCustomFields);

apps/server/src/utils/parserFunctions.ts

@@ -19,7 +19,13 @@ import {
   isOntimeDelay,
   isOntimeEvent,
 } from 'ontime-types';
-import { generateId, getErrorMessage, getLastEvent } from 'ontime-utils';
+import {
+  customFieldLabelToKey,
+  generateId,
+  getErrorMessage,
+  getLastEvent,
+  isAlphanumericWithSpace,
+} from 'ontime-utils';
 
 import { dbModel } from '../models/dataModel.js';
 import { block as blockDef, delay as delayDef } from '../models/eventsDefinition.js';
@@ -305,13 +311,18 @@ export function parseCustomFields(data: Partial<DatabaseModel>, emitError?: Erro
 export function sanitiseCustomFields(data: object): CustomFields {
   const newCustomFields: CustomFields = {};
 
-  for (const [_key, field] of Object.entries(data)) {
+  for (const [originalKey, field] of Object.entries(data)) {
     if (!isValidField(field)) {
       continue;
     }
 
-    // make a new key to avoid mismatches
-    const key = field.label.toLowerCase();
+    if (!isAlphanumericWithSpace(field.label)) {
+      continue;
+    }
+
+    const keyFromLabel = customFieldLabelToKey(field.label);
+    //Test label and key cohesion, but allow old lowercased keys to stay
+    const key = originalKey.toLocaleLowerCase() === keyFromLabel.toLocaleLowerCase() ? originalKey : keyFromLabel;
     if (key in newCustomFields) {
       continue;
     }

packages/utils/index.ts

@@ -53,10 +53,12 @@ export {
   removeTrailingZero,
 } from './src/date-utils/timeFormatting.js';
 export { parseUserTime } from './src/date-utils/parseUserTime.js';
-export { isAlphanumeric } from './src/regex-utils/isAlphanumeric.js';
+export { isAlphanumeric, isAlphanumericWithSpace } from './src/regex-utils/isAlphanumeric.js';
 export { isColourHex } from './src/regex-utils/isColourHex.js';
 export { splitWhitespace } from './src/regex-utils/splitWhitespace.js';
 
+export { customFieldLabelToKey } from './src/customField-utils/customFieldLabelToKey.js';
+
 // helpers from externals
 export { deepmerge } from './src/externals/deepmerge.js';
 

packages/utils/src/customField-utils/customFieldLabelToKey.ts

@@ -0,0 +1,12 @@
+import { isAlphanumericWithSpace } from '../regex-utils/isAlphanumeric.js';
+
+/**
+ * @description Transforms a Custom field label into a valid key or returns null if not possible
+ * @returns {string | null}
+ */
+export const customFieldLabelToKey = (label: string): string | null => {
+  if (isAlphanumericWithSpace(label)) {
+    return label.trim().replaceAll(' ', '_');
+  }
+  return null;
+};

packages/utils/src/regex-utils/isAlphanumeric.ts

@@ -6,3 +6,12 @@ export const isAlphanumeric = (text: string): boolean => {
   const regex = /^[a-z0-9]+$/i;
   return regex.test(text);
 };
+
+/**
+ * @description Validates a alphanumeric string allow space
+ * @returns {boolean}
+ */
+export const isAlphanumericWithSpace = (text: string): boolean => {
+  const regex = /^[a-z0-9_ ]+$/i;
+  return regex.test(text);
+};