switch dataContextFromURL to be async

v3/src/data-interactive/handlers/data-context-from-url-handler.test.ts

@@ -29,34 +29,28 @@ describe("DataInteractive DataContextHandler", () => {
   })
 
   describe("create" , () => {
-    it("handles invalid values", () => {
-      expect(handler.create?.({}).success).toBe(false)
-      expect(handler.create?.({}, 1).success).toBe(false)
-      expect(handler.create?.({}, ["hi"]).success).toBe(false)
-      expect(handler.create?.({}, {fake: "value"}).success).toBe(false)
-    })
+    it("handles invalid values", async () => {
 
-    it("returns success true", () => {
-      const result = handler.create!({}, {URL: "https://example.com"})
-      expect(mockedDownloadCsvFile).toHaveBeenCalled()
-      expect(result.success).toBe(true)
+      expect(await handler.create?.({})).toMatchObject({success: false})
+      expect(await handler.create?.({}, 1)).toMatchObject({success: false})
+      expect(await handler.create?.({}, ["hi"])).toMatchObject({success: false})
+      expect(await handler.create?.({}, {fake: "value"})).toMatchObject({success: false})
     })
 
-    it("handles onError from downloadCsvFile", () => {
+    it("handles onError from downloadCsvFile", async () => {
       mockedDownloadCsvFile.mockImplementation((url, onComplete, onError) => {
         onError({
             name: "",
             message: ""
         }, "")
       })
 
-      const result = handler.create!({}, {URL: "https://example.com"})
+      const result = await handler.create!({}, {URL: "https://example.com"})
       expect(mockedDownloadCsvFile).toHaveBeenCalled()
-      expect(alertSpy).toHaveBeenCalled()
-      expect(result.success).toBe(true)
+      expect(result.success).toBe(false)
     })
 
-    it("creates a dataset", () => {
+    it("creates a dataset", async () => {
       gDataBroker.setSharedModelManager(getSharedModelManager(appState.document)!)
 
       mockedDownloadCsvFile.mockImplementation((url, onComplete, onError) => {
@@ -75,10 +69,9 @@ describe("DataInteractive DataContextHandler", () => {
       })
 
       expect(gDataBroker.length).toBe(0)
-      const result = handler.create!({}, {URL: "https://example.com"})
+      const result = await handler.create!({}, {URL: "https://example.com"})
       expect(gDataBroker.length).toBe(1)
       expect(mockedDownloadCsvFile).toHaveBeenCalled()
-      expect(alertSpy).not.toHaveBeenCalled()
       expect(result.success).toBe(true)
     })
   })

v3/src/data-interactive/handlers/data-context-from-url-handler.ts

@@ -1,7 +1,7 @@
 import { appState } from "../../models/app-state"
 import { convertParsedCsvToDataSet, CsvParseResult, downloadCsvFile } from "../../utilities/csv-import"
 import { registerDIHandler } from "../data-interactive-handler"
-import { DIErrorResult, DIHandler, diNotImplementedYet, DIResources, DIUrl, DIValues } from "../data-interactive-types"
+import { DIAsyncHandler, DIErrorResult, DIResources, DIUrl, DIValues } from "../data-interactive-types"
 
 const kInvalidValuesError: DIErrorResult = {
   success: false,
@@ -14,48 +14,46 @@ export function getFilenameFromUrl(url: string) {
   return new URL(url, window.location.href).pathname.split("/").pop()
 }
 
-export const diDataContextFromURLHandler: DIHandler = {
+export const diDataContextFromURLHandler: DIAsyncHandler = {
 
   // The API tester has a template for this under the dataContext section.
-  // Because the download is async we won't know if this succeeded or failed until it
-  // has been downloaded. As a first pass we always say it succeeds. And then when if
-  // it fails we show an alert.
-  create(resources: DIResources, _values?: DIValues) {
+  // Because the download is async, this is an DIAsyncHandler which
+  // resolves with the API response.
+  async create(resources: DIResources, _values?: DIValues) {
     const values = _values as DIUrl | undefined
     if (!values || !(typeof values === "object") || !values.URL) return kInvalidValuesError
 
     const url = values.URL
 
-    const failureAlert = () => {
-      // Because the handlers don't support async calls we just show a dialog when it fails
-      appState.alert(`A plugin tried to import the URL ${url} and failed.`, "Import dataset failed")
-    }
-
-    try {
-      downloadCsvFile(url,
-        (results: CsvParseResult) => {
-          const filename = getFilenameFromUrl(url)
-          // TODO: look at v2 to figure out if it has a default name perhaps that is translated
-          const ds = convertParsedCsvToDataSet(results, filename || "Imported Data")
-          if (ds) {
-            appState.document.content?.importDataSet(ds, { createDefaultTile: true })
-          }
-          else {
-            // Because the handlers don't support async calls we just show a dialog when it fails
-            failureAlert()
+    return new Promise((resolve) => {
+      const errorResult = {
+        success: false,
+        values: { error: `Failed to download and import ${url}.`}
+      } as const
+      try {
+        downloadCsvFile(url,
+          (results: CsvParseResult) => {
+            const filename = getFilenameFromUrl(url)
+            // TODO: look at v2 to figure out if it has a default name perhaps that is translated
+            const ds = convertParsedCsvToDataSet(results, filename || "Imported Data")
+            if (ds) {
+              appState.document.content?.importDataSet(ds, { createDefaultTile: true })
+              resolve({ success: true })
+            }
+            else {
+              resolve(errorResult)
+            }
+          },
+          (error) => {
+            // It seems the error object returned by papaparse does not include useful information for
+            // the user.
+            resolve(errorResult)
           }
-        },
-        (error) => {
-          // It seems the error object returned by papaparse does not include useful information for
-          // the user.
-          failureAlert()
-        }
-      )
-
-      return { success: true }
-    } catch (e) {
-      return { success: false, values: { error: "Failed to download CSV file"}}
-    }
+        )
+      } catch (e) {
+        resolve(errorResult)
+      }
+    })
   }
 }