From b964657327c67e01143a5f1897e12fbde1290d5d Mon Sep 17 00:00:00 2001
From: Connor Lanigan <dev@connorlanigan.com>
Date: Tue, 18 Jul 2023 12:05:43 +0200
Subject: [PATCH] fix: Emit funnelStepComplete after completion, but not after
 cancellation (#1329)

---
 src/form/__tests__/analytics.test.tsx         | 24 ++++++++++---------
 .../components/analytics-funnel.test.tsx      | 12 ++--------
 .../analytics/components/analytics-funnel.tsx |  4 ++--
 src/internal/analytics/hooks/use-funnel.ts    |  2 +-
 4 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/src/form/__tests__/analytics.test.tsx b/src/form/__tests__/analytics.test.tsx
index 9df956cd69..3b6815901e 100644
--- a/src/form/__tests__/analytics.test.tsx
+++ b/src/form/__tests__/analytics.test.tsx
@@ -45,20 +45,12 @@ describe('Form Analytics', () => {
     );
   });
 
-  test('sends a funnelStepComplete metric when Form is unmounted', () => {
+  test('does not send a funnelStepComplete metric when Form is unmounted', () => {
     const { unmount } = render(<Form />);
 
     unmount();
 
-    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledTimes(1);
-    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledWith(
-      expect.objectContaining({
-        stepNumber: 1,
-        stepNameSelector: expect.any(String),
-        subStepAllSelector: expect.any(String),
-        funnelInteractionId: expect.any(String),
-      })
-    );
+    expect(FunnelMetrics.funnelStepComplete).not.toHaveBeenCalled();
   });
 
   test('sends a funnelCancelled metric when Form is unmounted', () => {
@@ -119,7 +111,7 @@ describe('Form Analytics', () => {
    * 1. The submit button is the only primary button in the form
    * 2. The submit button is clicked before the Form is unmounted
    */
-  test('sends a funnelComplete metric when Form is unmounted after clicking a primary button in the actions slot', () => {
+  test('sends a funnelComplete and funnelStepComplete metric when Form is unmounted after clicking a primary button in the actions slot', () => {
     const { container, unmount } = render(
       <Form
         actions={
@@ -141,6 +133,16 @@ describe('Form Analytics', () => {
         funnelInteractionId: expect.any(String),
       })
     );
+
+    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledTimes(1);
+    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledWith(
+      expect.objectContaining({
+        stepNumber: 1,
+        funnelInteractionId: expect.any(String),
+        stepNameSelector: expect.any(String),
+        subStepAllSelector: expect.any(String),
+      })
+    );
   });
 
   test('increments the submissionAttempt counter when clicking Submit', () => {
diff --git a/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx b/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx
index a374e64461..4c2f1e05ef 100644
--- a/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx
+++ b/src/internal/analytics/__tests__/components/analytics-funnel.test.tsx
@@ -341,7 +341,7 @@ describe('AnalyticsFunnelStep', () => {
     });
   });
 
-  test('calls funnelStepComplete with the correct arguments when the everything unmounts', () => {
+  test('does not call funnelStepComplete when the funnel unmounts without submitting', () => {
     const stepNumber = 1;
     const stepNameSelector = '.step-name-selector';
 
@@ -353,17 +353,9 @@ describe('AnalyticsFunnelStep', () => {
       </AnalyticsFunnel>
     );
 
-    expect(FunnelMetrics.funnelStepComplete).not.toHaveBeenCalled();
-
     unmount();
 
-    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledTimes(1);
-    expect(FunnelMetrics.funnelStepComplete).toHaveBeenCalledWith({
-      funnelInteractionId: mockedFunnelInteractionId,
-      stepNumber,
-      stepNameSelector,
-      subStepAllSelector: expect.any(String),
-    });
+    expect(FunnelMetrics.funnelStepComplete).not.toHaveBeenCalled();
   });
 
   test('calls funnelStepStart and funnelStepComplete when stepNumber changes', () => {
diff --git a/src/internal/analytics/components/analytics-funnel.tsx b/src/internal/analytics/components/analytics-funnel.tsx
index f1218149a2..e9f2cb7408 100644
--- a/src/internal/analytics/components/analytics-funnel.tsx
+++ b/src/internal/analytics/components/analytics-funnel.tsx
@@ -85,7 +85,7 @@ export const AnalyticsFunnel = ({ children, ...props }: AnalyticsFunnelProps) =>
         FunnelMetrics.funnelSuccessful({ funnelInteractionId });
       } else {
         FunnelMetrics.funnelCancelled({ funnelInteractionId });
-        funnelState.current === 'cancelled';
+        funnelState.current = 'cancelled';
       }
     };
   }, []);
@@ -180,7 +180,7 @@ export const AnalyticsFunnelStep = ({ children, stepNumber, stepNameSelector }:
 
     return () => {
       //eslint-disable-next-line react-hooks/exhaustive-deps
-      if (funnelInteractionId && funnelState.current === 'default') {
+      if (funnelInteractionId && funnelState.current !== 'cancelled') {
         FunnelMetrics.funnelStepComplete({
           funnelInteractionId,
           stepNumber,
diff --git a/src/internal/analytics/hooks/use-funnel.ts b/src/internal/analytics/hooks/use-funnel.ts
index 44742ef508..97a96cf2a2 100644
--- a/src/internal/analytics/hooks/use-funnel.ts
+++ b/src/internal/analytics/hooks/use-funnel.ts
@@ -49,7 +49,7 @@ export const useFunnelSubStep = () => {
       funnelInteractionId &&
       subStepRef.current &&
       !subStepRef.current.contains(event.relatedTarget) &&
-      funnelState.current === 'default'
+      funnelState.current !== 'cancelled'
     ) {
       FunnelMetrics.funnelSubStepComplete({
         funnelInteractionId,