Azure · dpwatrous · Mar 27, 2023 · Mar 27, 2023
@@ -12,6 +12,9 @@ describe("Action tests", () => {
             subject: "planet",
         });
 
+        expect(action.actionName).toEqual("Hello");
+        expect(action.logger.context.area).toEqual("Action-Hello");
+
         // Test that we can call waitFor() functions even while execute/initialize
         // aren't running, and they return immediately
         await action.waitForInitialization();
@@ -132,6 +135,8 @@ describe("Action tests", () => {
     };
 
     class HelloAction extends AbstractAction<HelloFormValues> {
+        actionName = "Hello";
+
         message = "Hola world";
         onValidateCallCount = 0;
         subjectOnValidateCallCount = 0;

@@ -1,15 +1,75 @@
 import { Form, FormValues, ValidationStatus } from "../form";
-import { getLogger } from "../logging";
+import { getLogger, Logger } from "../logging";
 import { Deferred } from "../util";
 
 export interface Action<V extends FormValues = FormValues> {
-    form: Form<V>;
-    isInitialized: boolean;
+    /**
+     * A globally unique friendly name for this action. Used for logging, etc.
+     */
+    readonly actionName: string;
+
+    /**
+     * A logger which is pre-configured with the name of the action
+     */
+    readonly logger: Logger;
+
+    /**
+     * The form associated with this action.
+     */
+    readonly form: Form<V>;
+
+    /**
+     * True if initialization has finished.
+     */
+    readonly isInitialized: boolean;
+
+    /**
+     * Load data needed for the action and perform any other initialization.
+     * Calls the onInitialize() callback.
+     */
     initialize(): Promise<void>;
+
+    /**
+     * A callback which performs action initialization.
+     */
     onInitialize(): Promise<V>;
+
+    /**
+     * Constructs the form for the action.
+     *
+     * @param initialValues Initial values to populate the form with.
+     */
+    buildForm(initialValues: V): Form<V>;
+
+    /**
+     * Execute the action and return a result (which may be success or failure)
+     * @param formValues The form values to use to execute the action.
+     */
     execute(formValues: V): Promise<ActionExecutionResult>;
+
+    /**
+     * A callback which performs validation synchronously. Note that both
+     * onValidateSync and onValidateAsync will be called for every validation.
+     * The onValidateSync callback will simply return more quickly.
+     *
+     * @param formValues The form values to validate
+     */
     onValidateSync?(formValues: V): ValidationStatus;
+
+    /**
+     * A callback which performs validation asynchronously. Note that both
+     * onValidateSync and onValidateAsync will be called for every validation.
+     * The onValidateSync callback will simply return more quickly.
+     *
+     * @param formValues The form values to validate
+     */
     onValidateAsync?(formValues: V): Promise<ValidationStatus>;
+
+    /**
+     * A callback which performs action execution.
+     *
+     * @param formValues The form values to use
+     */
     onExecute(formValues: V): Promise<void>;
 
     /**
@@ -36,6 +96,22 @@ export type ActionExecutionResult = {
 export abstract class AbstractAction<V extends FormValues>
     implements Action<V>
 {
+    abstract actionName: string;
+
+    private _logger: Logger | undefined;
+
+    /**
+     * Get a logger for the action
+     */
+    get logger(): Logger {
+        // In practice this should never be undefined
+        const actionName = this.actionName ?? "UnknownAction";
+        if (!this._logger) {
+            this._logger = getLogger(`Action-${actionName}`);
+        }
+        return this._logger;
+    }
+
     protected _form?: Form<V>;
 
     private _isInitialized: boolean = false;
@@ -79,10 +155,11 @@ export abstract class AbstractAction<V extends FormValues>
             };
 
             this._isInitialized = true;
-            this._initializationDeferred.resolve();
-        } catch (e) {
-            this._initializationDeferred.reject(e);
         } finally {
+            // Always resolve rather than reject. Waiting for initialization
+            // isn't the right place to handle errors. Instead, handle rejections
+            // from initialize() itself.
+            this._initializationDeferred.resolve();
             this._isInitializing = false;
         }
     }
@@ -157,20 +234,19 @@ export abstract class AbstractAction<V extends FormValues>
                 executionResult.success = true;
             } catch (e) {
                 executionResult.error = e;
-                getLogger("AbstractAction").warn(
-                    "Action failed to execute:",
-                    e
-                );
+                this.logger.warn("Action failed to execute:", e);
             }
+        } finally {
+            this._isExecuting = false;
 
+            // Always resolve rather than reject. Waiting for execution
+            // isn't the right place to handle errors. Instead, handle rejections
+            // from execute() itself.
             this._executionDeferred.resolve();
-        } catch (e) {
-            this._executionDeferred.reject(e);
-        } finally {
+
             // Create a new deferred execution object since execution
             // can happen again and again
             this._executionDeferred = new Deferred();
-            this._isExecuting = false;
         }
         return executionResult;
     }
@@ -181,7 +257,7 @@ export abstract class AbstractAction<V extends FormValues>
 
     abstract onExecute(formValues: V): Promise<void>;
 
-    abstract onValidateSync(values: V): ValidationStatus;
+    onValidateSync?(values: V): ValidationStatus;
 
     onValidateAsync?(values: V): Promise<ValidationStatus>;
 }
@@ -210,7 +210,7 @@ export abstract class AbstractParameter<
     D extends ParameterDependencies<V> = ParameterDependencies<V>
 > implements Parameter<V, K, D>
 {
-    private _logger: Logger = getLogger("FormParameter");
+    private _logger: Logger;
 
     readonly parentForm: Form<V>;
     readonly parentSection?: Section<V>;
@@ -280,6 +280,8 @@ export abstract class AbstractParameter<
 
         this.name = name;
 
+        this._logger = getLogger(`FormParameter-${name}`);
+
         this._label = init?.label;
         this.description = init?.description;
         this.disabled = init?.disabled;

@@ -1,4 +1,3 @@
-import { getLogger } from "@batch/ui-common";
 import { AbstractAction, Action } from "@batch/ui-common/lib/action";
 import {
     Form,
@@ -29,7 +28,7 @@ type CarFormValues = {
 };
 
 class CreateOrUpdateCarAction extends AbstractAction<CarFormValues> {
-    private _logger = getLogger("CreateOrUpdateCarAction");
+    actionName = "CreateOrUpdateCar";
 
     async onInitialize(): Promise<CarFormValues> {
         return {
@@ -120,7 +119,7 @@ class CreateOrUpdateCarAction extends AbstractAction<CarFormValues> {
     }
 
     async onExecute(formValues: CarFormValues): Promise<void> {
-        this._logger.info("Execute called with values:" + formValues);
+        this.logger.info("Execute called with values:" + formValues);
     }
 }
 

@@ -29,6 +29,8 @@ export type CreateAccountFormValues = {
 };
 
 export class CreateAccountAction extends AbstractAction<CreateAccountFormValues> {
+    actionName = "CreateAccount";
+
     private _initialValues: CreateAccountFormValues = {};
 
     async onInitialize(): Promise<CreateAccountFormValues> {

@@ -7,7 +7,7 @@ import {
     StringParameter,
     ValidationStatus,
 } from "@batch/ui-common/lib/form";
-import { act, render, screen } from "@testing-library/react";
+import { act, render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import * as React from "react";
 import { initMockBrowserEnvironment } from "../../../environment";
@@ -28,6 +28,26 @@ describe("Action form tests", () => {
         expect(await runAxe(container)).toHaveNoViolations();
     });
 
+    test("Can handle initialization errors", async () => {
+        const action = new PetDogAction({
+            throwInitializationError: true,
+        });
+
+        let err: string | undefined;
+        render(
+            <ActionForm
+                action={action}
+                onError={(e) => {
+                    err = (e as Error).message;
+                }}
+            />
+        );
+
+        await waitFor(() => {
+            expect(err).toEqual("Fake initialization error");
+        });
+    });
+
     test("Can submit and reset using the built-in buttons", async () => {
         userEvent.setup();
 
@@ -78,9 +98,12 @@ type PetDogFormValues = {
     dogName?: string;
     numberOfPets?: number;
     giveTreat?: boolean;
+    throwInitializationError?: boolean;
 };
 
 class PetDogAction extends AbstractAction<PetDogFormValues> {
+    actionName = "PetDog";
+
     onPet?: (count: number) => void;
     private _initialValues: PetDogFormValues = {};
 
@@ -96,6 +119,9 @@ class PetDogAction extends AbstractAction<PetDogFormValues> {
     }
 
     async onInitialize(): Promise<PetDogFormValues> {
+        if (this._initialValues.throwInitializationError) {
+            throw new Error("Fake initialization error");
+        }
         return this._initialValues;
     }