Merge pull request #1260 from cpvalente/show-key

Show key and support spaces in label
cpvalente · Oct 13, 2024 · 27c45df · 27c45df
2 parents 3e11085 + 24de056
commit 27c45df
Show file tree

Hide file tree

Showing 12 changed files with 117 additions and 34 deletions.
diff --git a/...src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFieldEntry.tsx b/...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,18 +37,25 @@ export default function CustomFieldEntry(props: CustomFieldEntryProps) {
             onSubmit={handleEdit}
             initialColour={colour}
             initialLabel={label}
+            initialKey={field}
           />
         </td>
       </tr>
     );
   }
 
+  console.log(field)
+
   return (
     <tr>
       <td>
         <Swatch color={colour} />
       </td>
       <td className={style.fullWidth}>{label}</td>
+      <td className={style.fullWidth}>
+        {/* TODO: better description */}
+        <CopyTag label='The key can be used in Integrations and API'>{field}</CopyTag>
+      </td>
       <td className={style.actions}>
         <IconButton
           size='sm'

diff --git a/.../src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFieldForm.tsx b/.../src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFieldForm.tsx
@@ -2,7 +2,7 @@ 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 { isAlphanumericWithSpace } from 'ontime-utils';
 
 import { maybeAxiosError } from '../../../../../common/api/utils';
 import SwatchSelect from '../../../../../common/components/input/colour-input/SwatchSelect';
@@ -16,10 +16,11 @@ 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
@@ -34,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,
     },
@@ -75,9 +76,10 @@ export default function CustomFieldForm(props: CustomFieldsFormProps) {
         <Input
           {...register('label', {
             required: { value: true, message: 'Required field' },
+            onChange: () => setValue('key', getValues('label').replaceAll(' ', '_')),
             validate: (value) => {
               if (value.trim().length === 0) return 'Required field';
-              if (!isAlphanumeric(value)) return 'Only alphanumeric characters are allowed';
+              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;
             },
@@ -87,6 +89,11 @@ export default function CustomFieldForm(props: CustomFieldsFormProps) {
           autoComplete='off'
         />
       </div>
+      <div className={style.column}>
+        {/* TODO: better style and description */}
+        <Panel.Description>Key (The key is generated from the label for use in Integrations and API)</Panel.Description>
+        <Input {...register('key')} disabled size='sm' variant='ontime-filled' autoComplete='off' />
+      </div>
 
       <div>
         <Panel.Description>Colour</Panel.Description>

diff --git a/...ent/src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFields.tsx b/...ent/src/features/app-settings/panel/feature-settings-panel/custom-fields/CustomFields.tsx
@@ -73,6 +73,7 @@ export default function CustomFields() {
               <tr>
                 <th>Colour</th>
                 <th>Name</th>
+                <th></th>
                 <th />
               </tr>
             </thead>

diff --git a/apps/server/src/api-data/custom-fields/customFields.validation.ts b/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(),

diff --git a/apps/server/src/services/integration-service/OscIntegration.ts b/apps/server/src/services/integration-service/OscIntegration.ts
@@ -78,14 +78,15 @@ export class OscIntegration implements IIntegration<OscSubscription, OSCSettings
     //TODO: Look into using bundles
     const message = new Message(address);
     message.append(args);
+    console.log(message);
 
     this.oscClient.send(message);
   }
 
   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,14 +105,15 @@ 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}`);
     }
   }
 
   private initRX(enabledIn: boolean, portIn: number) {
-    if (!enabledIn && this.enabledIn) {
+    if (!enabledIn) {
       this.shutdownRX();
       return;
     }

diff --git a/apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts b/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);
     });

diff --git a/apps/server/src/services/rundown-service/rundownCache.ts b/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) {

diff --git a/apps/server/src/utils/__tests__/parserFunctions.test.ts b/apps/server/src/utils/__tests__/parserFunctions.test.ts
@@ -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);

diff --git a/apps/server/src/utils/parserFunctions.ts b/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,12 +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;
     }
 
-    const key = field.label;
+    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;
     }

diff --git a/packages/utils/index.ts b/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';
 

diff --git a/packages/utils/src/customField-utils/customFieldLabelToKey.ts b/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;
+};