Fix moved model's collection isn't displayed until page refresh (#21178)

frontend/src/metabase/entities/questions.js

@@ -2,7 +2,7 @@ import { createEntity, undo } from "metabase/lib/entities";
 import * as Urls from "metabase/lib/urls";
 import { color } from "metabase/lib/colors";
 
-import {
+import Collections, {
   getCollectionType,
   normalizedCollection,
 } from "metabase/entities/collections";
@@ -34,12 +34,24 @@ const Questions = createEntity({
         ),
       ),
 
-    setCollection: ({ id, model }, collection, opts) =>
-      Questions.actions.update(
-        { id },
-        { collection_id: canonicalCollectionId(collection && collection.id) },
-        undo(opts, model === "dataset" ? "model" : "question", "moved"),
-      ),
+    setCollection: ({ id, model }, collection, opts) => {
+      return async dispatch => {
+        const result = await dispatch(
+          Questions.actions.update(
+            { id },
+            {
+              collection_id: canonicalCollectionId(collection && collection.id),
+            },
+            undo(opts, model === "dataset" ? "model" : "question", "moved"),
+          ),
+        );
+        dispatch(
+          Collections.actions.fetchList({ tree: true }, { reload: true }),
+        );
+
+        return result;
+      };
+    },
 
     setPinned: ({ id }, pinned, opts) =>
       Questions.actions.update(

frontend/src/metabase/entities/schemas.js

@@ -65,12 +65,28 @@ export default createEntity({
 
     if (type === Questions.actionTypes.UPDATE) {
       const { question } = payload;
-      const schema = getCollectionVirtualSchemaId(question.collection);
-      if (!state[schema]) {
+      const schemaId = getCollectionVirtualSchemaId(question.collection);
+
+      const virtualQuestionId = getQuestionVirtualTableId(question);
+      const previousSchemaContainingTheQuestion = getPreviousSchemaContainingTheQuestion(
+        state,
+        schemaId,
+        virtualQuestionId,
+      );
+
+      if (previousSchemaContainingTheQuestion) {
+        state = removeVirtualQuestionFromSchema(
+          state,
+          previousSchemaContainingTheQuestion.id,
+          virtualQuestionId,
+        );
+      }
+
+      if (!state[schemaId]) {
         return state;
       }
-      const virtualQuestionId = getQuestionVirtualTableId(question);
-      return updateIn(state, [schema, "tables"], tables => {
+
+      return updateIn(state, [schemaId, "tables"], tables => {
         if (question.archived) {
           return tables.filter(id => id !== virtualQuestionId);
         }
@@ -82,6 +98,26 @@ export default createEntity({
   },
 });
 
+function getPreviousSchemaContainingTheQuestion(
+  state,
+  schemaId,
+  virtualQuestionId,
+) {
+  return Object.values(state).find(schema => {
+    if (schema.id === schemaId) {
+      return false;
+    }
+
+    return (schema.tables || []).includes(virtualQuestionId);
+  });
+}
+
+function removeVirtualQuestionFromSchema(state, schemaId, virtualQuestionId) {
+  return updateIn(state, [schemaId, "tables"], tables =>
+    tables.filter(tableId => tableId !== virtualQuestionId),
+  );
+}
+
 function addTableAvoidingDuplicates(tables, tableId) {
   if (!Array.isArray(tables)) {
     return [tableId];

frontend/test/metabase/entities/schemas.unit.spec.js

@@ -227,6 +227,7 @@ describe("schema entity", () => {
       const nextState = Schemas.reducer(
         {
           [ROOT_COLLECTION_VIRTUAL_SCHEMA]: {
+            id: ROOT_COLLECTION_VIRTUAL_SCHEMA,
             tables: ["card__123", `card__${question.id}`],
           },
         },
@@ -235,6 +236,7 @@ describe("schema entity", () => {
 
       expect(nextState).toEqual({
         [ROOT_COLLECTION_VIRTUAL_SCHEMA]: {
+          id: ROOT_COLLECTION_VIRTUAL_SCHEMA,
           tables: ["card__123"],
         },
       });

frontend/test/metabase/scenarios/models/reproductions/19737-data-picker-not-showing-moved-model.cy.spec.js

@@ -2,7 +2,7 @@ import { restore, modal, popover } from "__support__/e2e/cypress";
 
 const modelName = "Orders Model";
 
-describe.skip("issue 19737", () => {
+describe("issue 19737", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();
@@ -13,30 +13,76 @@ describe.skip("issue 19737", () => {
   it("should show moved model in the data picker without refreshing (metabase#19737)", () => {
     cy.visit("/collection/root");
 
-    openEllipsisMenuFor(modelName);
-    popover()
-      .contains("Move")
+    moveModel(modelName, "My personal collection");
+
+    cy.findByText("Moved model");
+
+    cy.findByText("New").click();
+    cy.findByText("Question")
+      .should("be.visible")
       .click();
 
-    modal().within(() => {
-      cy.findByText("My personal collection").click();
-      cy.findByText("Move").click();
-    });
+    cy.findByText("Models").click();
+
+    cy.findByText("Your personal collection").click();
+    cy.findByText(modelName);
+  });
+
+  it("should not show duplicate models in the data picker after it's moved from a custom collection without refreshing (metabase#19737)", () => {
+    // move "Orders Model" to "First collection"
+    cy.visit("/collection/root");
+
+    moveModel(modelName, "First collection");
 
     cy.findByText("Moved model");
+    // Close the modal so the next time we move the model another model will always be shown
+    cy.icon("close:visible").click();
 
     cy.findByText("New").click();
     cy.findByText("Question")
       .should("be.visible")
       .click();
 
+    // Open question picker (this is crucial) so the collection list are loaded.
     cy.findByText("Models").click();
 
-    cy.findByText("Your personal collection").click();
+    cy.findByText("First collection").click();
     cy.findByText(modelName);
+
+    // Use back button to so the state is kept
+    cy.go("back");
+
+    // move "Orders Model" from a custom collection ("First collection") to another collection
+    cy.findByText("First collection").click();
+
+    moveModel(modelName, "My personal collection");
+
+    cy.findByText("Moved model");
+
+    cy.findByText("New").click();
+    cy.findByText("Question")
+      .should("be.visible")
+      .click();
+
+    cy.findByText("Models").click();
+
+    cy.findByText("First collection").click();
+    cy.findByText("Nothing here");
   });
 });
 
+function moveModel(modelName, collectionName) {
+  openEllipsisMenuFor(modelName);
+  popover()
+    .contains("Move")
+    .click();
+
+  modal().within(() => {
+    cy.findByText(collectionName).click();
+    cy.findByText("Move").click();
+  });
+}
+
 function openEllipsisMenuFor(item) {
   cy.findByText(item)
     .closest("tr")