Make CSV "Start Exploring" Link Actually Start Exploring (#30421)

* Return the created model ID when uploading a CSV

* fix upload complete link

* update e2e test

* update unit tests

---------

Co-authored-by: Tim Macdonald <tim@tsmacdonald.com>

e2e/test/scenarios/collections/uploads.cy.spec.js

@@ -67,6 +67,13 @@ describe("CSV Uploading", { tags: ["@external", "@actions"] }, () => {
             cy.findByText(testFile.tableName); // TODO: we should humanize model names
           });
 
+          cy.findByRole("status").within(() => {
+            cy.findByText("Start exploring").click();
+          });
+
+          cy.url().should("include", `/model/4`);
+          cy.findByTestId("TableInteractive-root");
+
           const tableQuery = `SELECT * FROM information_schema.tables WHERE table_name LIKE 'upload_${testFile.tableName}_%' ORDER BY table_name DESC LIMIT 1;`;
 
           queryWritableDB(tableQuery, dialect).then(result => {

frontend/src/metabase/redux/uploads.ts

@@ -27,7 +27,7 @@ export const UPLOAD_FILE_TO_COLLECTION_CLEAR =
 const MAX_UPLOAD_SIZE = 200 * 1024 * 1024; // 200MB
 const MAX_UPLOAD_STRING = "200MB";
 
-const CLEAR_AFTER_MS = 5000;
+const CLEAR_AFTER_MS = 8000;
 
 const uploadStart = createAction(UPLOAD_FILE_TO_COLLECTION_START);
 const uploadEnd = createAction(UPLOAD_FILE_TO_COLLECTION_END);
@@ -76,7 +76,7 @@ export const uploadFile = createThunkAction(
         dispatch(
           uploadEnd({
             id,
-            modelId: response.model_id,
+            modelId: response,
           }),
         );
 

frontend/src/metabase/redux/uploads.unit.spec.js

@@ -10,13 +10,13 @@ import {
 
 const now = Date.now();
 
+const NOTIFICATION_DELAY = 9000;
+
 const mockUploadCSV = (valid = true) => {
   fetchMock.post(
     "path:/api/card/from-csv",
     valid
-      ? {
-          model_id: 3,
-        }
+      ? "3"
       : {
           throws: { data: { message: "It's dead Jim" } },
         },
@@ -47,7 +47,7 @@ describe("csv uploads", () => {
       mockUploadCSV();
 
       await uploadFile(file, "root")(dispatch);
-      jest.advanceTimersByTime(6000);
+      jest.advanceTimersByTime(NOTIFICATION_DELAY);
 
       expect(dispatch).toHaveBeenCalledWith({
         type: UPLOAD_FILE_TO_COLLECTION_START,
@@ -78,7 +78,7 @@ describe("csv uploads", () => {
       mockUploadCSV(false);
 
       await uploadFile(file, "root")(dispatch);
-      jest.advanceTimersByTime(6000);
+      jest.advanceTimersByTime(NOTIFICATION_DELAY);
 
       expect(dispatch).toHaveBeenCalledWith({
         type: UPLOAD_FILE_TO_COLLECTION_START,
@@ -109,7 +109,7 @@ describe("csv uploads", () => {
       const bigFile = new File([""], "test.csv");
       Object.defineProperty(bigFile, "size", { value: 200 * 1024 * 1024 + 1 });
       await uploadFile(bigFile, "root")(dispatch);
-      jest.advanceTimersByTime(6000);
+      jest.advanceTimersByTime(NOTIFICATION_DELAY);
 
       expect(dispatch).toHaveBeenCalledWith({
         type: UPLOAD_FILE_TO_COLLECTION_START,

frontend/src/metabase/status/components/FileUploadStatus/FileUploadStatus.unit.spec.tsx

@@ -70,7 +70,7 @@ describe("FileUploadStatus", () => {
   });
 
   it("Should show a start exploring link on completion", async () => {
-    fetchMock.post("path:/api/card/from-csv", { model_id: 3 }, { delay: 1000 });
+    fetchMock.post("path:/api/card/from-csv", "3", { delay: 1000 });
 
     renderWithProviders(
       <Route

frontend/src/metabase/status/components/FileUploadStatusLarge/FileUploadStatusLarge.tsx

@@ -27,7 +27,7 @@ const FileUploadLarge = ({
     title: getTitle(uploads, collection),
     items: uploads.map(upload => ({
       id: upload.id,
-      title: upload.name,
+      title: getName(upload),
       icon: "model",
       description: getDescription(upload),
       isInProgress: isUploadInProgress(upload),
@@ -39,6 +39,13 @@ const FileUploadLarge = ({
   return <StatusLarge status={status} isActive={isActive} />;
 };
 
+const getName = (upload: FileUpload) => {
+  if (upload.status === "complete") {
+    return <Link to={`/model/${upload.modelId}`}>{upload.name}</Link>;
+  }
+  return upload.name;
+};
+
 const getTitle = (uploads: FileUpload[], collection: Collection) => {
   const isDone = uploads.every(isUploadCompleted);
   const isError = uploads.some(isUploadAborted);

frontend/src/metabase/status/components/StatusLarge/StatusLarge.tsx

@@ -24,7 +24,7 @@ type Status = {
 
 type StatusItem = {
   id?: number;
-  title: string;
+  title: string | JSX.Element;
   icon: string;
   description?: string | JSX.Element;
   isInProgress: boolean;

src/metabase/api/card.clj

@@ -978,7 +978,8 @@ saved later when it is ready."
 
 (defn upload-csv!
   "Main entry point for CSV uploading. Coordinates detecting the schema, inserting it into an appropriate database,
-  syncing and scanning the new data, and creating an appropriate model. May throw validation or DB errors."
+  syncing and scanning the new data, and creating an appropriate model which is then returned. May throw validation or
+  DB errors."
   [collection-id filename csv-file]
   (when (not (public-settings/uploads-enabled))
     (throw (Exception. "Uploads are not enabled.")))
@@ -1012,12 +1013,13 @@ saved later when it is ready."
       :visualization_settings {}})))
 
 (api/defendpoint ^:multipart POST "/from-csv"
-  "Create a table and model populated with the values from the attached CSV."
+  "Create a table and model populated with the values from the attached CSV. Returns the model ID if successful."
   [:as {raw-params :params}]
   ;; parse-long returns nil with "root", which is what we want anyway
-  (upload-csv! (parse-long (get raw-params "collection_id"))
-               (get-in raw-params ["file" :filename])
-               (get-in raw-params ["file" :tempfile]))
-  {:status 200})
+  (let [model-id (:id (upload-csv! (parse-long (get raw-params "collection_id"))
+                                   (get-in raw-params ["file" :filename])
+                                   (get-in raw-params ["file" :tempfile])))]
+    {:status 200
+     :body   model-id}))
 
 (api/define-routes)