Refactor: Refactor makeResult function and form handling in Trial com…

…ponent (#1254)

* type: Refactor checkBreakRound function to use explicit types for parameters

* refactor: Move `form` related expressions to inside the `feedback_form` check as form is only utilized from within that if statement

* refactor: Utilize early return if `feedback_form` is not defined

* refactor: Simplify `form` assignment as we can be sure that `feedback_form` is always defined here

* refactor: Initialize form as empty array if undefined and add test scenarios for it

* fix: Fix assertions in tests

* refactor: Simplify break round condition in Trial component

frontend/src/components/Trial/Trial.test.tsx

@@ -158,4 +158,67 @@ describe('Trial', () => {
             );
         });
     });
+
+    it("calls onResult when form is not defined", async () => {
+        const formless_feedback_form = {
+            ...feedback_form,
+            form: undefined
+        };
+        render(<Trial
+            config={defaultConfig}
+            onNext={mockOnNext}
+            onResult={mockOnResult}
+            feedback_form={formless_feedback_form}
+        />);
+        fireEvent.click(screen.getByTestId('mock-feedback-form'));
+        await waitFor(() => {
+            expect(mockOnResult).toHaveBeenCalled();
+        });
+    });
+
+    it("calls onResult and onNext when form is not defined and break_round_on is met", async () => {
+        const formless_feedback_form = {
+            ...feedback_form,
+            form: undefined
+        };
+        const config = {
+            ...defaultConfig,
+            break_round_on: { NOT: ['fast'] }
+        };
+        render(<Trial
+            config={config}
+            onNext={mockOnNext}
+            onResult={mockOnResult}
+            feedback_form={formless_feedback_form}
+        />);
+        fireEvent.click(screen.getByTestId('mock-feedback-form'));
+        await waitFor(() => {
+            expect(mockOnResult).toHaveBeenCalled();
+            expect(mockOnNext).toHaveBeenCalled();
+        });
+    });
+
+    it("calls only onResult when form is not defined and break_round_on is NOT met", async () => {
+        const formless_feedback_form = {
+            ...feedback_form,
+            form: undefined
+        };
+        const config = {
+            ...defaultConfig,
+            break_round_on: { EQUALS: ['slow'] }
+        };
+        render(<Trial
+            config={config}
+            onNext={mockOnNext}
+            onResult={mockOnResult}
+            feedback_form={formless_feedback_form}
+        />);
+        fireEvent.click(screen.getByTestId('mock-feedback-form'));
+
+        await waitFor(() => {
+            expect(mockOnResult).toHaveBeenCalled();
+        });
+
+        expect(mockOnNext).not.toHaveBeenCalled();
+    });
 });

frontend/src/components/Trial/Trial.tsx

@@ -63,47 +63,15 @@ const Trial = (props: TrialProps) => {
     // Create result data
     const makeResult = useCallback(
         async (result: { type: 'time_passed' }) => {
+
             // Prevent multiple submissions
             if (submitted.current) {
                 return;
             }
-            submitted.current = true;
-
-            // TODO: Check if we can find another solution for
-            // the default value of form than [{}]
-            const form = feedback_form ? feedback_form.form : [{}];
-
-            if (result.type === "time_passed") {
-                form.map((formElement) => (formElement.value = "TIMEOUT"));
-            }
-
-            if (feedback_form) {
-
-                if (feedback_form.is_skippable) {
-                    form.map((formElement => (formElement.value = formElement.value || '')))
-                }
-
-                const breakRoundOn = config.break_round_on;
-                const shouldBreakRound = breakRoundOn && checkBreakRound(form.map((formElement) => formElement.value), breakRoundOn);
-                const shouldCallOnNextInOnResult = !shouldBreakRound
-
-                await onResult(
-                    {
-                        decision_time: getAndStoreDecisionTime(),
-                        form,
-                        config
-                    },
-                    false,
-                    // if we break the round, we don't want to call onNext in onResult
-                    shouldCallOnNextInOnResult
-                );
-
-                if (shouldBreakRound) {
-                    onNext(true);
-                }
 
-            } else {
+            submitted.current = true;
 
+            if (!feedback_form) {
 
                 if (result_id) {
                     onResult({
@@ -114,18 +82,56 @@ const Trial = (props: TrialProps) => {
                     onNext();
                 }
 
+                return;
+            }
+
+            const { form = [] } = feedback_form;
+
+            if (result.type === "time_passed") {
+                form.map((formElement) => (formElement.value = "TIMEOUT"));
+            }
+
+            if (feedback_form.is_skippable) {
+                form.map((formElement => (formElement.value = formElement.value || '')))
             }
+
+            const breakRoundConditions = config.break_round_on;
+            const shouldBreakRound = breakRoundConditions && checkBreakRound(form.map((formElement) => formElement.value), breakRoundConditions);
+
+            await onResult(
+                {
+                    decision_time: getAndStoreDecisionTime(),
+                    form,
+                    config
+                },
+                false,
+                // if we break the round, we don't want to call `onNext` in `onResult`
+                // as it does not allow us to pass a `breakRound` flag
+                !shouldBreakRound
+            );
+
+            if (shouldBreakRound) {
+                onNext(true);
+            }
+
         },
         [feedback_form, config, onNext, onResult, result_id]
     );
 
-    const checkBreakRound = (values, breakConditions) => {
+    const checkBreakRound = (
+        values: string[],
+        breakConditions: TrialConfig['break_round_on']
+    ) => {
         switch (Object.keys(breakConditions)[0]) {
             case 'EQUALS':
-                return values.some(val => breakConditions['EQUALS']
-                    .includes(val));
+                return values
+                    .some(
+                        val => breakConditions['EQUALS']!.includes(val)
+                    );
             case 'NOT':
-                return !values.some(val => breakConditions['NOT'].includes(val));
+                return !values.some(
+                    val => breakConditions['NOT']!.includes(val)
+                );
             default:
                 return false;
         }

frontend/src/types/Trial.ts

@@ -1,11 +1,16 @@
+interface BreakRoundOn {
+    EQUALS?: string[];
+    NOT?: string[];
+}
+
 export interface TrialConfig {
     response_time: number;
     auto_advance: boolean;
     listen_first: boolean;
     show_continue_button: boolean;
     continue_label: string;
     style: string;
-    break_round_on: any;
+    break_round_on: BreakRoundOn;
 
     auto_advance_timer: number | null;
 }