fix(ui): handle parsing errors properly in object editor (argoproj#13931)

Signed-off-by: Mason Malone <[email protected]>
(cherry picked from commit fe8df34)

Author: MasonM

ui/src/shared/use-editable-object.test.ts

@@ -1,4 +1,4 @@
-import {createInitialState, reducer} from './use-editable-object';
+import {createInitialState, reducer, setObjectActionCreator} from './use-editable-object';
 
 describe('createInitialState', () => {
     test('without object', () => {
@@ -59,8 +59,8 @@ describe('reducer', () => {
         });
     });
 
-    test('setObject with string', () => {
-        const newState = reducer(testState, {type: 'setObject', payload: 'a: 2'});
+    test('setObject with object and string', () => {
+        const newState = reducer(testState, {type: 'setObject', payload: {object: {a: 2}, serialization: 'a: 2'}});
         expect(newState).toEqual({
             object: {a: 2},
             serialization: 'a: 2',
@@ -70,8 +70,8 @@ describe('reducer', () => {
         });
     });
 
-    test('setObject with object', () => {
-        const newState = reducer(testState, {type: 'setObject', payload: {a: 2}});
+    test('setObject with only object', () => {
+        const newState = reducer(testState, {type: 'setObject', payload: {object: {a: 2}}});
         expect(newState).toEqual({
             object: {a: 2},
             serialization: 'a: 2\n',
@@ -81,17 +81,6 @@ describe('reducer', () => {
         });
     });
 
-    test('resetObject with string', () => {
-        const newState = reducer(testState, {type: 'resetObject', payload: 'a: 2'});
-        expect(newState).toEqual({
-            object: {a: 2},
-            serialization: 'a: 2',
-            lang: 'yaml',
-            initialSerialization: 'a: 2',
-            edited: false
-        });
-    });
-
     test('resetObject with object', () => {
         const newState = reducer(testState, {type: 'resetObject', payload: {a: 2}});
         expect(newState).toEqual({
@@ -103,3 +92,23 @@ describe('reducer', () => {
         });
     });
 });
+
+describe('setObjectActionCreator', () => {
+    test('with object', () => {
+        expect(setObjectActionCreator({a: 2})).toEqual({
+            type: 'setObject',
+            payload: {object: {a: 2}}
+        });
+    });
+
+    test('with string', () => {
+        expect(setObjectActionCreator('a: 2')).toEqual({
+            type: 'setObject',
+            payload: {object: {a: 2}, serialization: 'a: 2'}
+        });
+    });
+
+    test("with string that can't be parsed", () => {
+        expect(() => setObjectActionCreator('@')).toThrow('Plain value cannot start with reserved character @');
+    });
+});

ui/src/shared/use-editable-object.ts

@@ -3,7 +3,8 @@ import {useReducer} from 'react';
 import {Lang, parse, stringify} from '../shared/components/object-parser';
 import {ScopedLocalStorage} from '../shared/scoped-local-storage';
 
-type Action<T> = {type: 'setLang'; payload: Lang} | {type: 'setObject'; payload: string | T} | {type: 'resetObject'; payload: string | T};
+type SetObjectAction<T> = {type: 'setObject'; payload: {object: T; serialization?: string}};
+type Action<T> = {type: 'setLang'; payload: Lang} | SetObjectAction<T> | {type: 'resetObject'; payload: T};
 
 interface State<T> {
     /** The parsed form of the object, kept in sync with "serialization" */
@@ -24,20 +25,17 @@ const storage = new ScopedLocalStorage('object-editor');
 export function reducer<T>(state: State<T>, action: Action<T>) {
     const newState = {...state};
     switch (action.type) {
-        case 'resetObject':
         case 'setObject':
-            if (typeof action.payload === 'string') {
-                newState.object = parse(action.payload);
-                newState.serialization = action.payload;
-            } else {
-                newState.object = action.payload;
-                newState.serialization = stringify(action.payload, newState.lang);
-            }
-            if (action.type === 'resetObject') {
-                newState.initialSerialization = newState.serialization;
-            }
+            newState.object = action.payload.object;
+            newState.serialization = action.payload.serialization ?? stringify(newState.object, newState.lang);
             newState.edited = newState.initialSerialization !== newState.serialization;
             return newState;
+        case 'resetObject':
+            newState.object = action.payload;
+            newState.serialization = stringify(newState.object, newState.lang);
+            newState.initialSerialization = newState.serialization;
+            newState.edited = false;
+            return newState;
         case 'setLang':
             newState.lang = action.payload;
             storage.setItem('lang', newState.lang, defaultLang);
@@ -61,19 +59,31 @@ export function createInitialState<T>(object?: T): State<T> {
     };
 }
 
+/**
+ * Action creator for setObject that can accept a string and parse it.
+ * The reason the parsing logic isn't in the reducer is because we want parse
+ * errors to be propagated to the caller.
+ */
+export function setObjectActionCreator<T>(value: string | T): SetObjectAction<T> {
+    return {
+        type: 'setObject',
+        payload: typeof value === 'string' ? {object: parse<T>(value), serialization: value} : {object: value}
+    };
+}
+
 /**
  * useEditableObject is a React hook to manage the state of an object that can be serialized and edited, encapsulating the logic to
  * parse/stringify the object as necessary.
  */
 export function useEditableObject<T>(object?: T): State<T> & {
     setObject: (value: string | T) => void;
-    resetObject: (value: string | T) => void;
+    resetObject: (value: T) => void;
     setLang: (lang: Lang) => void;
 } {
     const [state, dispatch] = useReducer(reducer<T>, object, createInitialState);
     return {
         ...state,
-        setObject: value => dispatch({type: 'setObject', payload: value}),
+        setObject: value => dispatch(setObjectActionCreator<T>(value)),
         resetObject: value => dispatch({type: 'resetObject', payload: value}),
         setLang: value => dispatch({type: 'setLang', payload: value})
     };