Reset join condition state when the rhs table changes (#48951)

e2e/test/scenarios/joins/joins-reproductions.cy.spec.js

@@ -5,6 +5,7 @@ import {
   assertQueryBuilderRowCount,
   cartesianChartCircle,
   chartPathWithFillColor,
+  createQuestion,
   dashboardGrid,
   echartsContainer,
   enterCustomColumnDetails,
@@ -27,6 +28,7 @@ import {
   selectSavedQuestionsToJoin,
   startNewQuestion,
   summarize,
+  tableInteractive,
   visitDashboard,
   visitQuestionAdhoc,
   visualize,
@@ -1361,3 +1363,78 @@ describe("issue 45300", () => {
     );
   });
 });
+
+describe("issue 46675", () => {
+  const questionDetails = {
+    query: {
+      "source-table": ORDERS_ID,
+    },
+  };
+
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+
+    cy.log("create draft state with a rhs table and a lhs column");
+    createQuestion(questionDetails, { visitQuestion: true });
+    openNotebook();
+    getNotebookStep("data").findByLabelText("Join data").click();
+    entityPickerModal().within(() => {
+      entityPickerModalTab("Tables").click();
+      cy.findByText("Reviews").click();
+    });
+    popover().findByText("ID").click();
+  });
+
+  it("should reset the draft join state when the source table changes (metabase#46675)", () => {
+    cy.log("change the source table and verify that the state was reset");
+    getNotebookStep("data").findByText("Orders").click();
+    entityPickerModal().within(() => {
+      entityPickerModalTab("Tables").click();
+      cy.findByText("Products").click();
+    });
+    getNotebookStep("join").within(() => {
+      cy.findByLabelText("Left table").should("have.text", "Products");
+      cy.findByLabelText("Right table").should("have.text", "Pick data…");
+      cy.findByLabelText("Left column").should("not.exist");
+    });
+
+    cy.log("complete the join and make sure the query can be executed");
+    getNotebookStep("join")
+      .findByLabelText("Right table")
+      .findByText("Pick data…")
+      .click();
+    entityPickerModal().within(() => {
+      entityPickerModalTab("Tables").click();
+      cy.findByText("Orders").click();
+    });
+    visualize();
+    tableInteractive().should("be.visible");
+  });
+
+  it("should reset the draft join state when the source table changes (metabase#46675)", () => {
+    cy.log("change the rhs table and verify that the state was reset");
+    getNotebookStep("join")
+      .findByLabelText("Right table")
+      .findByText("Reviews")
+      .click();
+    entityPickerModal().within(() => {
+      entityPickerModalTab("Tables").click();
+      cy.findByText("Orders").click();
+    });
+    getNotebookStep("join").within(() => {
+      cy.findByLabelText("Left table").should("have.text", "Orders");
+      cy.findByLabelText("Right table").should("have.text", "Orders");
+      cy.findByLabelText("Left column").should(
+        "contain.text",
+        "Pick a column…",
+      );
+    });
+
+    cy.log("complete the join and make sure the query can be executed");
+    popover().findByText("ID").click();
+    popover().findByText("ID").click();
+    visualize();
+    tableInteractive().should("be.visible");
+  });
+});

frontend/src/metabase/querying/notebook/components/JoinStep/JoinComplete/JoinComplete.tsx

@@ -69,6 +69,7 @@ export function JoinComplete({
     } else {
       onDraftRhsTableChange(newTable);
     }
+    setIsAddingNewCondition(false);
   };
 
   const handleAddCondition = (newCondition: Lib.JoinCondition) => {

frontend/src/metabase/querying/notebook/components/JoinStep/JoinConditionDraft/JoinConditionDraft.tsx

-import { useState } from "react";
+import { useLayoutEffect, useState } from "react";
 
 import { Box, Flex } from "metabase/ui";
 import * as Lib from "metabase-lib";
@@ -16,6 +16,7 @@ interface JoinConditionDraftProps {
   stageIndex: number;
   joinable: Lib.JoinOrJoinable;
   lhsTableName: string;
+  rhsTable?: Lib.Joinable;
   rhsTableName: string | undefined;
   isReadOnly: boolean;
   isRemovable: boolean;
@@ -29,6 +30,7 @@ export function JoinConditionDraft({
   stageIndex,
   joinable,
   lhsTableName,
+  rhsTable,
   rhsTableName,
   isReadOnly,
   isRemovable,
@@ -77,6 +79,13 @@ export function JoinConditionDraft({
     handleColumnChange(lhsColumn, newRhsColumn);
   };
 
+  useLayoutEffect(() => {
+    setLhsColumn(undefined);
+    setRhsColumn(undefined);
+    setIsLhsOpened(true);
+    setIsRhsOpened(false);
+  }, [rhsTable]);
+
   return (
     <JoinConditionRoot>
       <Flex align="center" gap="xs" mih="47px" p="xs">

frontend/src/metabase/querying/notebook/components/JoinStep/JoinDraft/JoinDraft.tsx

-import { useEffect, useMemo, useState } from "react";
+import { useLayoutEffect, useMemo, useState } from "react";
 import { useLatest } from "react-use";
 import { t } from "ttag";
 
@@ -33,7 +33,7 @@ export function JoinDraft({
   isReadOnly,
   onJoinChange,
 }: JoinDraftProps) {
-  const databaseId = Lib.databaseID(query);
+  const sourceTableId = Lib.sourceTableOrCardId(query);
   const [strategy, setStrategy] = useState(
     () => initialStrategy ?? getDefaultJoinStrategy(query, stageIndex),
   );
@@ -87,23 +87,21 @@ export function JoinDraft({
     }
   };
 
-  const resetStateRef = useLatest(() => {
+  const handleReset = () => {
     const rhsTableColumns = initialRhsTable
       ? Lib.joinableColumns(query, stageIndex, initialRhsTable)
       : [];
-
     setStrategy(initialStrategy ?? getDefaultJoinStrategy(query, stageIndex));
     setRhsTable(initialRhsTable);
     setRhsTableColumns(rhsTableColumns);
     setSelectedRhsTableColumns(rhsTableColumns);
     setLhsColumn(undefined);
-  });
+  };
 
-  useEffect(
-    function resetStateOnDatabaseChange() {
-      resetStateRef.current();
-    },
-    [databaseId, resetStateRef],
+  const handleResetRef = useLatest(handleReset);
+  useLayoutEffect(
+    () => handleResetRef.current(),
+    [sourceTableId, handleResetRef],
   );
 
   return (
@@ -150,6 +148,7 @@ export function JoinDraft({
               stageIndex={stageIndex}
               joinable={rhsTable}
               lhsTableName={lhsTableName}
+              rhsTable={rhsTable}
               rhsTableName={rhsTableName}
               isReadOnly={isReadOnly}
               isRemovable={false}

frontend/src/metabase/querying/notebook/components/JoinStep/JoinStep.unit.spec.tsx

@@ -559,6 +559,28 @@ describe("Notebook Editor > Join Step", () => {
     expect(condition.operator.shortName).toBe("!=");
   });
 
+  it("should reset the draft join condition state when the rhs table is changed", async () => {
+    setup();
+    const rhsTablePicker = screen.getByLabelText("Right table");
+    await userEvent.click(within(rhsTablePicker).getByRole("button"));
+    const entityPickerModal = await screen.findByTestId("entity-picker-modal");
+    await waitForLoaderToBeRemoved();
+    await userEvent.click(within(entityPickerModal).getByText("Reviews"));
+    const lhsColumnPicker = await screen.findByTestId("lhs-column-picker");
+    await userEvent.click(within(lhsColumnPicker).getByText("ID"));
+
+    const newRhsTablePicker = screen.getByLabelText("Right table");
+    await userEvent.click(within(newRhsTablePicker).getByText("Reviews"));
+    const newEntityPickerModal = await screen.findByTestId(
+      "entity-picker-modal",
+    );
+    await waitForLoaderToBeRemoved();
+    await userEvent.click(within(newEntityPickerModal).getByText("Orders"));
+    const lhsColumn = screen.getByLabelText("Left column");
+    expect(within(lhsColumn).getByText("Pick a column…")).toBeInTheDocument();
+    expect(within(lhsColumn).queryByText("ID")).not.toBeInTheDocument();
+  });
+
   describe("join strategies", () => {
     it("should be able to change the join strategy for a new join clause", async () => {
       const { getRecentJoin } = setup();

frontend/src/metabase/querying/notebook/components/NotebookStepList/NotebookStepList.tsx

 import { useCallback, useMemo, useState } from "react";
 
-import { useSelector } from "metabase/lib/redux";
 import { getQuestionSteps } from "metabase/querying/notebook/utils/steps";
-import { getMetadata } from "metabase/selectors/metadata";
 import type { Query } from "metabase-lib";
 import * as Lib from "metabase-lib";
 import type Question from "metabase-lib/v1/Question";
@@ -39,7 +37,7 @@ export function NotebookStepList({
   updateQuestion,
   readOnly = false,
 }: NotebookStepListProps) {
-  const metadata = useSelector(getMetadata);
+  const metadata = question.metadata();
   const [openSteps, setOpenSteps] = useState<OpenSteps>(
     getInitialOpenSteps(question, readOnly),
   );