force select only the matching guide when forced guide key is set

Author: thomaswhyyou

packages/client/src/clients/guide/client.ts

@@ -157,29 +157,11 @@ const select = (state: StoreState, filters: SelectFilterParams = {}) => {
   const defaultGroup = findDefaultGroup(state.guideGroups);
   if (!defaultGroup) return result;
 
-  const displaySequence = [...defaultGroup.display_sequence];
+  const displaySequence = defaultGroup.display_sequence;
   const location = state.location;
 
-  // If in debug mode, put the forced guide at the beginning of the display sequence.
-  if (state.debug.forcedGuideKey) {
-    const forcedKeyIndex = displaySequence.indexOf(state.debug.forcedGuideKey);
-    if (forcedKeyIndex > -1) {
-      displaySequence.splice(forcedKeyIndex, 1);
-    }
-    displaySequence.unshift(state.debug.forcedGuideKey);
-  }
-
   for (const [index, guideKey] of displaySequence.entries()) {
-    let guide = state.guides[guideKey];
-
-    // Use preview guide if it exists and matches the forced guide key
-    if (
-      state.debug.forcedGuideKey === guideKey &&
-      state.previewGuides[guideKey]
-    ) {
-      guide = state.previewGuides[guideKey];
-    }
-
+    const guide = state.previewGuides[guideKey] || state.guides[guideKey];
     if (!guide) continue;
 
     const affirmed = predicate(guide, {
@@ -215,11 +197,11 @@ const predicate = (
     return false;
   }
 
-  // Bypass filtering if the debugged guide matches the given filters.
-  // This should always run AFTER checking the filters but BEFORE
-  // checking archived status and location rules.
-  if (debug.forcedGuideKey === guide.key) {
-    return true;
+  // If in debug mode with a forced guide key, bypass other filtering and always
+  // return true for that guide only. This should always run AFTER checking the
+  // filters but BEFORE checking archived status and location rules.
+  if (debug.forcedGuideKey) {
+    return debug.forcedGuideKey === guide.key;
   }
 
   if (!guide.active) {
@@ -736,17 +718,8 @@ export class KnockGuideClient {
     // callback to a setTimeout, but just to be safe.
     this.ensureClearTimeout();
 
-    // If in debug mode, try to resolve the forced guide, otherwise return the first non-undefined guide.
-    let resolved = undefined;
-    if (this.store.state.debug.forcedGuideKey) {
-      resolved = this.stage.ordered.find(
-        (x) => x === this.store.state.debug.forcedGuideKey,
-      );
-    }
-
-    if (!resolved) {
-      resolved = this.stage.ordered.find((x) => x !== undefined);
-    }
+    // Resolve to the first non-undefined guide in the stage.
+    const resolved = this.stage.ordered.find((x) => x !== undefined);
 
     this.knock.log(
       `[Guide] Closing the current group stage: resolved=${resolved}`,

packages/client/test/clients/guide/guide.test.ts

@@ -1758,7 +1758,7 @@ describe("KnockGuideClient", () => {
       expect(result!.type).not.toBe("regular-type");
     });
 
-    test("doesn't return the preview guide when filtered by a different key", () => {
+    test("doesn't return any guide when filtered by a different key than forced guide", () => {
       const previewGuide = {
         ...mockGuideTwo,
         type: "preview-type",
@@ -1791,10 +1791,12 @@ describe("KnockGuideClient", () => {
         key: "onboarding",
       });
 
-      expect(result!.key).toBe("onboarding");
+      // When forcedGuideKey is set, only that guide is considered, so filtering
+      // by a different key returns undefined
+      expect(result).toBeUndefined();
     });
 
-    test("doesn't return the preview guide when filtered by a different type", () => {
+    test("doesn't return any guide when filtered by a different type than forced guide", () => {
       const previewGuide = {
         ...mockGuideTwo,
         type: "preview-type",
@@ -1827,7 +1829,9 @@ describe("KnockGuideClient", () => {
         type: "banner",
       });
 
-      expect(result!.key).toBe("system_status");
+      // When forcedGuideKey is set, only that guide is considered, so filtering
+      // by a different type returns undefined
+      expect(result).toBeUndefined();
     });
 
     test("does not return a guide inside a throttle window ", () => {
@@ -2131,7 +2135,7 @@ describe("KnockGuideClient", () => {
       expect(result[0]!.key).toBe(mockGuideTwo.key);
     });
 
-    test("does not return an inactive guide when forced guide key is set", () => {
+    test("returns the target guide even if inactive when forced guide key is set", () => {
       const stateWithGuides = {
         guideGroups: [mockDefaultGroup],
         guideGroupDisplayLogs: {},
@@ -2147,17 +2151,41 @@ describe("KnockGuideClient", () => {
         location: undefined,
         counter: 0,
         debug: {
-          forcedGuideKey: mockGuideThree.key,
+          forcedGuideKey: mockGuideTwo.key,
           previewSessionId: "test-session-id",
         },
       };
 
       const client = new KnockGuideClient(mockKnock, channelId);
-      const result = client["_selectGuides"](stateWithGuides);
+      const result = client["_selectGuides"](stateWithGuides, {
+        key: mockGuideTwo.key
+      });
 
-      expect(result).toHaveLength(2);
-      expect(result[0]!.key).toBe(mockGuideThree.key);
-      expect(result[1]!.key).toBe(mockGuideOne.key);
+      expect(result[0]!.key).toBe(mockGuideTwo.key);
+    });
+
+    test("returns only the forced guide when forced guide key is set", () => {
+      const stateWithGuides = {
+        guideGroups: [mockDefaultGroup],
+        guideGroupDisplayLogs: {},
+        guides: mockGuides,
+        previewGuides: {},
+        queries: {},
+        location: undefined,
+        counter: 0,
+        debug: {
+          forcedGuideKey: mockGuideOne.key,
+          previewSessionId: "test-session-id",
+        },
+      };
+
+      const client = new KnockGuideClient(mockKnock, channelId);
+      const result = client["_selectGuides"](stateWithGuides, {
+        type: "card"
+      });
+
+      expect(result).toHaveLength(1);
+      expect(result[0]!.key).toBe(mockGuideOne.key);
     });
 
     test("returns empty array when inside throttle window by default", () => {