Code review

InseeFrLab · Dec 6, 2024 · 573fd03 · 573fd03
1 parent 813780f
commit 573fd03
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 98 deletions.
diff --git a/web/src/core/adapters/s3Client/s3Client.ts b/web/src/core/adapters/s3Client/s3Client.ts
@@ -596,7 +596,7 @@ export function createS3Client(
             return downloadUrl;
         },
 
-        headObject: async ({ path }) => {
+        getFileMetadata: async ({ path }) => {
             const { bucketName, objectName } = bucketNameAndObjectNameFromS3Path(path);
 
             const { getAwsS3Client } = await prApi;

diff --git a/web/src/core/ports/S3Client.ts b/web/src/core/ports/S3Client.ts
@@ -59,7 +59,7 @@ export type S3Client = {
         validityDurationSecond: number;
     }) => Promise<string>;
 
-    headObject: (params: { path: string }) => Promise<{
+    getFileMetadata: (params: { path: string }) => Promise<{
         contentType: string | undefined;
         metadata: Record<string, string> | undefined;
     }>;

diff --git a/web/src/core/usecases/dataExplorer/state.ts b/web/src/core/usecases/dataExplorer/state.ts
@@ -1,5 +1,5 @@
 import { createUsecaseActions } from "clean-architecture";
-import { assert } from "tsafe";
+import { assert } from "tsafe/assert";
 import { id } from "tsafe/id";
 
 export const name = "dataExplorer";

diff --git a/web/src/core/usecases/dataExplorer/thunks.ts b/web/src/core/usecases/dataExplorer/thunks.ts
@@ -148,70 +148,121 @@ const privateThunks = {
                 })
             );
         },
-    detectFileType:
-        (params: { sourceUrl: string }) =>
-        async (...args) => {
-            const { sourceUrl } = params;
-            const [dispatch] = args;
+    detectFileType: (params: { sourceUrl: string }) => async () => {
+        const { sourceUrl } = params;
+        //const [dispatch] = args;
 
-            const extension = (() => {
-                const validExtensions = ["parquet", "csv", "json"] as const;
-                type ValidExtension = (typeof validExtensions)[number];
+        const extension = (() => {
+            const validExtensions = ["parquet", "csv", "json"] as const;
+            type ValidExtension = (typeof validExtensions)[number];
 
-                const isValidExtension = (ext: string): ext is ValidExtension =>
-                    validExtensions.includes(ext as ValidExtension);
+            const isValidExtension = (ext: string): ext is ValidExtension =>
+                validExtensions.includes(ext as ValidExtension);
 
-                let pathname: string;
+            let pathname: string;
 
-                try {
-                    pathname = new URL(sourceUrl).pathname;
-                } catch {
-                    return undefined;
-                }
-                const match = pathname.match(/\.(\w+)$/);
+            try {
+                pathname = new URL(sourceUrl).pathname;
+            } catch {
+                return undefined;
+            }
+            const match = pathname.match(/\.(\w+)$/);
 
-                if (match === null) {
-                    return undefined;
-                }
+            if (match === null) {
+                return undefined;
+            }
 
-                const [, extension] = match;
+            const [, extension] = match;
 
-                return isValidExtension(extension) ? extension : undefined;
-            })();
+            return isValidExtension(extension) ? extension : undefined;
+        })();
 
-            if (extension) {
-                return extension;
-            }
+        if (extension) {
+            return extension;
+        }
 
+        /*
             const contentType = await (async () => {
                 const fileDownloadUrl = await dispatch(
                     privateThunks.getFileDonwloadUrl({
                         sourceUrl
                     })
                 );
             })();
+            */
+    },
+    updateDataSource:
+        (params: {
+            queryParams: {
+                sourceUrl: string;
+                rowsPerPage: number | undefined;
+                page: number | undefined;
+            };
+            shouldVerifyUrl: boolean;
+        }) =>
+        async (...args) => {
+            const {
+                queryParams: { sourceUrl, rowsPerPage = 25, page = 1 },
+                shouldVerifyUrl
+            } = params;
+
+            const [dispatch, getState, rootContext] = args;
+
+            if (sourceUrl === "") {
+                if (getState()[name].queryParams !== undefined) {
+                    dispatch(actions.restoreState());
+                }
+                return;
+            }
+            if (getState()[name].isQuerying) {
+                dispatch(actions.queryCanceled());
+            }
+
+            if (getState()[name].queryParams?.sourceUrl === sourceUrl) {
+                return;
+            }
+
+            const { waitForDebounce } = getContext(rootContext);
+
+            await waitForDebounce();
+
+            dispatch(
+                privateThunks.performQuery({
+                    queryParams: {
+                        sourceUrl,
+                        rowsPerPage,
+                        page
+                    },
+                    shouldVerifyUrl
+                })
+            );
         }
 } satisfies Thunks;
 
 export const thunks = {
     initialize:
         (params: {
-            sourceUrl: string;
-            rowsPerPage: number;
-            page: number;
+            sourceUrl: string | undefined;
+            rowsPerPage: number | undefined;
+            page: number | undefined;
             selectedRowIndex: number | undefined;
-            columnVisibility: Record<string, boolean>;
+            columnVisibility: Record<string, boolean> | undefined;
         }) =>
         async (...args) => {
-            const { sourceUrl, columnVisibility, page, rowsPerPage, selectedRowIndex } =
-                params;
+            const {
+                sourceUrl,
+                columnVisibility = {},
+                page,
+                rowsPerPage,
+                selectedRowIndex
+            } = params;
             const [dispatch, getState, rootContext] = args;
 
             const { sqlOlap } = rootContext;
 
             sqlOlap.getConfiguredAsyncDuckDb();
 
-            if (sourceUrl === "") {
+            if (sourceUrl === undefined) {
                 return;
             }
 
@@ -230,7 +281,7 @@ export const thunks = {
             await waitForDebounce();
 
             await dispatch(
-                thunks.updateDataSource({
+                privateThunks.updateDataSource({
                     queryParams: { sourceUrl, rowsPerPage, page },
                     shouldVerifyUrl: true
                 })
@@ -272,40 +323,22 @@ export const thunks = {
         return true;
     },
     updateDataSource:
-        (params: {
-            queryParams: {
-                sourceUrl: string;
-                rowsPerPage: number;
-                page: number;
-            };
-            shouldVerifyUrl: boolean;
-        }) =>
+        (params: { sourceUrl: string }) =>
         async (...args) => {
-            const { queryParams, shouldVerifyUrl } = params;
-
-            const [dispatch, getState, rootContext] = args;
-
-            if (queryParams.sourceUrl === "") {
-                if (getState()[name].queryParams !== undefined) {
-                    dispatch(actions.restoreState());
-                }
-                return;
-            }
-            if (getState()[name].isQuerying) {
-                dispatch(actions.queryCanceled());
-            }
-
-            const { sourceUrl } = queryParams;
-
-            if (getState()[name].queryParams?.sourceUrl === sourceUrl) {
-                return;
-            }
-
-            const { waitForDebounce } = getContext(rootContext);
+            const { sourceUrl } = params;
 
-            await waitForDebounce();
+            const [dispatch] = args;
 
-            dispatch(privateThunks.performQuery({ queryParams, shouldVerifyUrl }));
+            await dispatch(
+                privateThunks.updateDataSource({
+                    queryParams: {
+                        sourceUrl,
+                        rowsPerPage: undefined,
+                        page: undefined
+                    },
+                    shouldVerifyUrl: false
+                })
+            );
         },
     updatePaginationModel:
         (params: { rowsPerPage: number; page: number }) =>

diff --git a/web/src/ui/pages/dataExplorer/DataExplorer.tsx b/web/src/ui/pages/dataExplorer/DataExplorer.tsx
@@ -1,6 +1,6 @@
 import { useEffect, useState } from "react";
 import { tss } from "tss";
-import { DEFAULT_QUERY_PARAMS, type PageRoute } from "./route";
+import type { PageRoute } from "./route";
 import { routes } from "ui/routes";
 import { useCore, useCoreState } from "core";
 import { Alert } from "onyxia-ui/Alert";
@@ -34,7 +34,7 @@ export default function DataExplorer(props: Props) {
 
     useEffect(() => {
         dataExplorer.initialize({
-            sourceUrl: route.params.source ?? "",
+            sourceUrl: route.params.source,
             rowsPerPage: route.params.rowsPerPage,
             page: route.params.page,
             selectedRowIndex: route.params.selectedRow,
@@ -124,16 +124,14 @@ export default function DataExplorer(props: Props) {
                     })
                 }
                 onUrlChange={value => {
-                    dataExplorer.updateDataSource({
-                        queryParams: {
-                            sourceUrl: value,
-                            rowsPerPage: DEFAULT_QUERY_PARAMS.rowsPerPage,
-                            page: DEFAULT_QUERY_PARAMS.page
-                        },
-                        shouldVerifyUrl: false
-                    });
+                    dataExplorer.updateDataSource({ sourceUrl: value });
                 }}
-                url={queryParams?.sourceUrl ?? DEFAULT_QUERY_PARAMS.source}
+                // NOTE: So that we show the URL in the search bar while it's being queried
+                url={
+                    queryParams === undefined
+                        ? (route.params.source ?? "")
+                        : queryParams.sourceUrl
+                }
             />
             <div className={classes.mainArea}>
                 {(() => {

diff --git a/web/src/ui/pages/dataExplorer/route.ts b/web/src/ui/pages/dataExplorer/route.ts
@@ -2,30 +2,19 @@ import { createRouter, defineRoute, param, createGroup, type Route } from "type-
 import { id } from "tsafe/id";
 import type { ValueSerializer } from "type-route";
 
-export const DEFAULT_QUERY_PARAMS = {
-    source: "",
-    rowsPerPage: 25,
-    page: 1,
-    selectedRow: undefined,
-    columnVisibility: {}
-};
 export const routeDefs = {
     dataExplorer: defineRoute(
         {
             source: param.query.optional.string,
-            rowsPerPage: param.query.optional.number.default(
-                DEFAULT_QUERY_PARAMS.rowsPerPage
-            ),
-            page: param.query.optional.number.default(DEFAULT_QUERY_PARAMS.page),
+            rowsPerPage: param.query.optional.number,
+            page: param.query.optional.number,
             selectedRow: param.query.optional.number,
-            columnVisibility: param.query.optional
-                .ofType(
-                    id<ValueSerializer<Record<string, boolean>>>({
-                        parse: raw => JSON.parse(raw),
-                        stringify: value => JSON.stringify(value)
-                    })
-                )
-                .default(DEFAULT_QUERY_PARAMS.columnVisibility)
+            columnVisibility: param.query.optional.ofType(
+                id<ValueSerializer<Record<string, boolean>>>({
+                    parse: raw => JSON.parse(raw),
+                    stringify: value => JSON.stringify(value)
+                })
+            )
         },
         () => `/data-explorer`
     )