rusackas commented on code in PR #44470:
URL: https://github.com/apache/superset/pull/44470#discussion_r4067145676


##########
superset-embedded-sdk/src/index.test.ts:
##########
@@ -210,4 +222,94 @@ describe("embedDashboard", () => {
       'Method "setDataMask" is not defined',
     );
   });
+  describe("reauthentication after a navigation internal to the dashboard", () 
=> {
+    const fakeToken = () => makeFakeJWT({ exp: Date.now() / 1000 + 300 });
+
+    // A link internal to the dashboard (side menu, tab, drill-down) navigates
+    // the iframe: same element, brand new document, which knows nothing of the
+    // channel the previous one was given.
+    function reload() {
+      mountPoint.querySelector("iframe")!.dispatchEvent(new Event("load"));
+    }
+
+    test("hands the reloaded document a new port and a fresh guest token", 
async () => {
+      const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
+      const dashboard = await embedDashboard({
+        id: "test-id",
+        supersetDomain: "https://superset.example.com";,
+        mountPoint,
+        fetchGuestToken: mockFetchGuestToken,
+      });
+      expect(mockFetchGuestToken).toHaveBeenCalledTimes(1);
+
+      reload();
+      // The token is fetched again rather than replayed from memory: the one 
in
+      // hand may be seconds from expiring, and the new page would take it
+      // straight into a 401.
+      await vi.waitFor(() =>
+        expect(switchboards[1].emit).toHaveBeenCalledWith("guestToken", {
+          guestToken: expect.any(String),
+        }),
+      );
+      expect(mockFetchGuestToken).toHaveBeenCalledTimes(2);
+
+      // A channel of its own, not the one the first document holds.
+      const constructions = vi.mocked(Switchboard).mock.calls;
+      expect(constructions).toHaveLength(2);
+      expect((constructions[1][0] as any).port).not.toBe(
+        (constructions[0][0] as any).port,
+      );
+      // Nothing is sent to the document that is gone.
+      expect(switchboards[0].emit).toHaveBeenCalledTimes(1);
+
+      dashboard.unmount();
+    });
+
+    test("replays the host's methods on the new port", async () => {
+      const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
+      const observer = vi.fn();
+      const dashboard = await embedDashboard({
+        id: "test-id",
+        supersetDomain: "https://superset.example.com";,
+        mountPoint,
+        fetchGuestToken: mockFetchGuestToken,
+        resolvePermalinkUrl: ({ key }) => `https://host.example.com/p/${key}`,
+      });
+      dashboard.observeDataMask(observer);
+
+      reload();
+      await vi.waitFor(() =>
+        expect(mockFetchGuestToken).toHaveBeenCalledTimes(2),
+      );
+
+      // The new document has never heard of either method.
+      const defined = vi
+        .mocked(switchboards[1].defineMethod)
+        .mock.calls.map(([name]) => name);
+      expect(defined).toContain("resolvePermalinkUrl");
+      expect(defined).toContain("observeDataMask");
+
+      dashboard.unmount();
+    });
+
+    test("ignores a load that arrives after unmount", async () => {
+      const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
+      const dashboard = await embedDashboard({
+        id: "test-id",
+        supersetDomain: "https://superset.example.com";,
+        mountPoint,
+        fetchGuestToken: mockFetchGuestToken,
+      });
+      const iframe = mountPoint.querySelector("iframe")!;
+
+      dashboard.unmount();
+      iframe.dispatchEvent(new Event("load"));
+      await new Promise((resolve) => {
+        setTimeout(resolve, 0);
+      });
+
+      expect(vi.mocked(Switchboard).mock.calls).toHaveLength(1);
+      expect(mockFetchGuestToken).toHaveBeenCalledTimes(1);
+    });
+  });

Review Comment:
   We usually keep this file flat (`test()` per case, no nesting) rather than 
wrapping related tests in a `describe()`. Same coverage, just flattened and 
renamed to match the `setDataMask ...`-style convention already used above.
   
   ```suggestion
     const fakeToken = () => makeFakeJWT({ exp: Date.now() / 1000 + 300 });
   
     // A link internal to the dashboard (side menu, tab, drill-down) navigates
     // the iframe: same element, brand new document, which knows nothing of the
     // channel the previous one was given.
     function reload() {
       mountPoint.querySelector("iframe")!.dispatchEvent(new Event("load"));
     }
   
     test("reload hands the reloaded document a new port and a fresh guest 
token", async () => {
       const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
       const dashboard = await embedDashboard({
         id: "test-id",
         supersetDomain: "https://superset.example.com";,
         mountPoint,
         fetchGuestToken: mockFetchGuestToken,
       });
       expect(mockFetchGuestToken).toHaveBeenCalledTimes(1);
   
       reload();
       // The token is fetched again rather than replayed from memory: the one 
in
       // hand may be seconds from expiring, and the new page would take it
       // straight into a 401.
       await vi.waitFor(() =>
         expect(switchboards[1].emit).toHaveBeenCalledWith("guestToken", {
           guestToken: expect.any(String),
         }),
       );
       expect(mockFetchGuestToken).toHaveBeenCalledTimes(2);
   
       // A channel of its own, not the one the first document holds.
       const constructions = vi.mocked(Switchboard).mock.calls;
       expect(constructions).toHaveLength(2);
       expect((constructions[1][0] as any).port).not.toBe(
         (constructions[0][0] as any).port,
       );
       // Nothing is sent to the document that is gone.
       expect(switchboards[0].emit).toHaveBeenCalledTimes(1);
   
       dashboard.unmount();
     });
   
     test("reload replays the host's methods on the new port", async () => {
       const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
       const observer = vi.fn();
       const dashboard = await embedDashboard({
         id: "test-id",
         supersetDomain: "https://superset.example.com";,
         mountPoint,
         fetchGuestToken: mockFetchGuestToken,
         resolvePermalinkUrl: ({ key }) => `https://host.example.com/p/${key}`,
       });
       dashboard.observeDataMask(observer);
   
       reload();
       await vi.waitFor(() =>
         expect(mockFetchGuestToken).toHaveBeenCalledTimes(2),
       );
   
       // The new document has never heard of either method.
       const defined = vi
         .mocked(switchboards[1].defineMethod)
         .mock.calls.map(([name]) => name);
       expect(defined).toContain("resolvePermalinkUrl");
       expect(defined).toContain("observeDataMask");
   
       dashboard.unmount();
     });
   
     test("reload ignores a load that arrives after unmount", async () => {
       const mockFetchGuestToken = vi.fn().mockResolvedValue(fakeToken());
       const dashboard = await embedDashboard({
         id: "test-id",
         supersetDomain: "https://superset.example.com";,
         mountPoint,
         fetchGuestToken: mockFetchGuestToken,
       });
       const iframe = mountPoint.querySelector("iframe")!;
   
       dashboard.unmount();
       iframe.dispatchEvent(new Event("load"));
       await new Promise((resolve) => {
         setTimeout(resolve, 0);
       });
   
       expect(vi.mocked(Switchboard).mock.calls).toHaveLength(1);
       expect(mockFetchGuestToken).toHaveBeenCalledTimes(1);
     });
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to