From 425a43d96a45742b441867b68c50f0e52e930817 Mon Sep 17 00:00:00 2001 From: Scott Cytacki Date: Fri, 20 Dec 2024 10:21:44 -0500 Subject: [PATCH] switch dataContextFromURL to be async --- .../data-context-from-url-handler.test.ts | 27 +++----- .../handlers/data-context-from-url-handler.ts | 68 +++++++++---------- 2 files changed, 43 insertions(+), 52 deletions(-) diff --git a/v3/src/data-interactive/handlers/data-context-from-url-handler.test.ts b/v3/src/data-interactive/handlers/data-context-from-url-handler.test.ts index 0a17752b9..973e28458 100644 --- a/v3/src/data-interactive/handlers/data-context-from-url-handler.test.ts +++ b/v3/src/data-interactive/handlers/data-context-from-url-handler.test.ts @@ -29,20 +29,15 @@ 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: "", @@ -50,13 +45,12 @@ describe("DataInteractive DataContextHandler", () => { }, "") }) - 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) }) }) diff --git a/v3/src/data-interactive/handlers/data-context-from-url-handler.ts b/v3/src/data-interactive/handlers/data-context-from-url-handler.ts index fc1a1e885..3c5b72ec3 100644 --- a/v3/src/data-interactive/handlers/data-context-from-url-handler.ts +++ b/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) + } + }) } }