From 336a387a881cfa65a0a1ae781f06dd39f7cf5ffd Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Mon, 11 Nov 2024 21:37:01 -0800 Subject: [PATCH 01/15] Upgrade regex to cover replaces --- packages/core/services/DatabaseService/index.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index ec6d82c7..c8f2b1dd 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -326,11 +326,8 @@ export default abstract class DatabaseService { // Grab near matches to the pre-defined columns like "file_name" for "File Name" const matches = [...columnsOnDataSource].filter((column) => { const simplifiedColumn = column - .trim() .toLowerCase() // File Name -> file name - .replace("_", "") // file_path -> filepath - .replace(" ", "") // file path -> filepath - .replace("-", ""); // file-path -> filepath + .replaceAll(/\s|-|_/g, ""); // Matches any whitespace, hyphen, or underscore return simplifiedColumn === preDefinedColumnSimplified; }); From ea6a10de512f99f3102b500c218a0e57f75c38f7 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Mon, 11 Nov 2024 21:38:47 -0800 Subject: [PATCH 02/15] Add missing await --- packages/core/services/DatabaseService/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index c8f2b1dd..f03dbfcb 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -343,7 +343,7 @@ export default abstract class DatabaseService { if (matches.length === 1) { // Rename matching column name to new pre-defined column - this.execute(` + await this.execute(` ALTER TABLE "${dataSourceName}" RENAME COLUMN "${matches[0]}" TO '${preDefinedColumn}' `); From 71c3537b2f014386b274ffdfa61aa3c1690595ec Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Mon, 11 Nov 2024 21:52:43 -0800 Subject: [PATCH 03/15] Use enum for pre-defined columns --- .../core/services/DatabaseService/index.ts | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index f03dbfcb..133be927 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -7,14 +7,15 @@ import { Source } from "../../entity/FileExplorerURL"; import SQLBuilder from "../../entity/SQLBuilder"; import DataSourcePreparationError from "../../errors/DataSourcePreparationError"; -const PRE_DEFINED_COLUMNS = [ - "File ID", - "File Path", - "File Name", - "File Size", - "Thumbnail", - "Uploaded", -]; +enum PreDefinedColumn { + FILE_ID = "File ID", + FILE_PATH = "File Path", + FILE_NAME = "File Name", + FILE_SIZE = "File Size", + THUMBNAIL = "Thumbnail", + UPLOADED = "Uploaded", +} +const PRE_DEFINED_COLUMNS = Object.values(PreDefinedColumn); /** * Service reponsible for querying against a database @@ -167,10 +168,10 @@ export default abstract class DatabaseService { */ protected async addRequiredColumns(name: string): Promise { const dataSourceColumns = await this.getColumnsOnDataSource(name); - if (!dataSourceColumns.has("File ID")) { + if (!dataSourceColumns.has(PreDefinedColumn.FILE_ID)) { await this.execute(` ALTER TABLE "${name}" - ADD COLUMN IF NOT EXISTS "File ID" VARCHAR; + ADD COLUMN IF NOT EXISTS "${PreDefinedColumn.FILE_ID}" VARCHAR; `); // The data source's input date gets appended to the end // of the auto-generated name, that makes it kind of messy @@ -180,33 +181,33 @@ export default abstract class DatabaseService { // ordered by file path. await this.execute(` UPDATE "${name}" - SET "File ID" = CONCAT(SQ.row, '-', '${dataSourceNameWithoutDate}') + SET "${PreDefinedColumn.FILE_ID}" = CONCAT(SQ.row, '-', '${dataSourceNameWithoutDate}') FROM ( - SELECT "File Path", ROW_NUMBER() OVER (ORDER BY "File Path") AS row + SELECT "${PreDefinedColumn.FILE_PATH}", ROW_NUMBER() OVER (ORDER BY "${PreDefinedColumn.FILE_PATH}") AS row FROM "${name}" ) AS SQ - WHERE "${name}"."File Path" = SQ."File Path"; + WHERE "${name}"."${PreDefinedColumn.FILE_PATH}" = SQ."${PreDefinedColumn.FILE_PATH}"; `); } - if (!dataSourceColumns.has("File Name")) { + if (!dataSourceColumns.has(PreDefinedColumn.FILE_NAME)) { await this.execute(` ALTER TABLE "${name}" - ADD COLUMN IF NOT EXISTS "File Name" VARCHAR; + ADD COLUMN IF NOT EXISTS "${PreDefinedColumn.FILE_NAME}" VARCHAR; `); // Best shot attempt at auto-generating a "File Name" // from the "File Path", defaults to full path if this fails await this.execute(` UPDATE "${name}" - SET "File Name" = COALESCE( + SET "${PreDefinedColumn.FILE_NAME}" = COALESCE( NULLIF( REGEXP_REPLACE( - "File Path", + "${PreDefinedColumn.FILE_PATH}", '^.*/([^/]*?)(\\.[^/.]+)?$', '\\1', '' ), ''), - "File Path"); + "${PreDefinedColumn.FILE_PATH}"); `); } @@ -224,21 +225,24 @@ export default abstract class DatabaseService { const columnsOnTable = await this.getColumnsOnDataSource(name); // If a data source has a File ID it must also pass validation - const hasFileIdColumn = columnsOnTable.has("File ID"); + const hasFileIdColumn = columnsOnTable.has(PreDefinedColumn.FILE_ID); if (hasFileIdColumn) { // Check for empty or just whitespace File ID column values - const blankFileIdRows = await this.getRowsWhereColumnIsBlank(name, "File ID"); + const blankFileIdRows = await this.getRowsWhereColumnIsBlank( + name, + PreDefinedColumn.FILE_ID + ); if (blankFileIdRows.length > 0) { const rowNumbers = DatabaseService.truncateString(blankFileIdRows.join(", "), 100); errors.push( - `"File ID" column contains ${blankFileIdRows.length} empty or purely whitespace values at rows ${rowNumbers}.` + `"${PreDefinedColumn.FILE_ID}" column contains ${blankFileIdRows.length} empty or purely whitespace ids at rows ${rowNumbers}.` ); } // Check for duplicate File ID column values const duplicateFileIdRows = await this.getRowsWhereColumnIsNotUniqueOrBlank( name, - "File ID" + PreDefinedColumn.FILE_ID ); if (duplicateFileIdRows.length > 0) { const rowNumbers = DatabaseService.truncateString( @@ -246,21 +250,17 @@ export default abstract class DatabaseService { 100 ); errors.push( - `"File ID" column contains duplicates. Found ${duplicateFileIdRows.length} duplicate values at rows ${rowNumbers}.` + `"${PreDefinedColumn.FILE_ID}" column contains duplicates. Found ${duplicateFileIdRows.length} duplicate ids at rows ${rowNumbers}.` ); } } - if (!columnsOnTable.has("File Path")) { - let error = - '\ - "File Path" column is missing in the data source. \ - Check the data source header row for a "File Path" column name and try again.'; + if (!columnsOnTable.has(PreDefinedColumn.FILE_PATH)) { + let error = `"${PreDefinedColumn.FILE_PATH}" column is missing in the data source. + Check the data source header row for a "${PreDefinedColumn.FILE_PATH}" column name and try again.`; // Attempt to find a column with a similar name to "File Path" const columns = Array.from(columnsOnTable); - // TODO: In addition to doing this, on ingestion detect "file_path", "file path", etc. - // and convert to "File Path" in DB table const filePathLikeColumn = columns.find((column) => column.toLowerCase().includes("path")) || columns.find((column) => column.toLowerCase().includes("file")); @@ -274,14 +274,17 @@ export default abstract class DatabaseService { errors.push(error); } else { // Check for empty or just whitespace File Path column values - const blankFilePathRows = await this.getRowsWhereColumnIsBlank(name, "File Path"); + const blankFilePathRows = await this.getRowsWhereColumnIsBlank( + name, + PreDefinedColumn.FILE_PATH + ); if (blankFilePathRows.length > 0) { const rowNumbers = DatabaseService.truncateString( blankFilePathRows.join(", "), 100 ); errors.push( - `"File Path" column contains ${blankFilePathRows.length} empty or purely whitespace values at rows ${rowNumbers}.` + `"${PreDefinedColumn.FILE_PATH}" column contains ${blankFilePathRows.length} empty or purely whitespace paths at rows ${rowNumbers}.` ); } @@ -291,7 +294,7 @@ export default abstract class DatabaseService { // Check for duplicate File ID column values const duplicateFilePathRows = await this.getRowsWhereColumnIsNotUniqueOrBlank( name, - "File Path" + PreDefinedColumn.FILE_PATH ); if (duplicateFilePathRows.length > 0) { const rowNumbers = DatabaseService.truncateString( @@ -299,7 +302,7 @@ export default abstract class DatabaseService { 100 ); errors.push( - `"File Path" column contains duplicates, but has no "File ID" column to use as a unique identifier instead. Add a unique "File ID" column or make "File Path" values unique. Found ${duplicateFilePathRows.length} duplicate values at rows ${rowNumbers}.` + `"${PreDefinedColumn.FILE_PATH}" column contains duplicates, but has no "${PreDefinedColumn.FILE_ID}" column to use as a unique identifier instead. Add a unique "${PreDefinedColumn.FILE_ID}" column or make "${PreDefinedColumn.FILE_PATH}" values unique. Found ${duplicateFilePathRows.length} duplicate paths at rows ${rowNumbers}.` ); } } From 0ed66faaf44a685c498970b56e5f292ff71e60c9 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Mon, 11 Nov 2024 22:20:28 -0800 Subject: [PATCH 04/15] Combine alter commands and add hidden UID --- .../core/services/DatabaseService/index.ts | 92 +++++++++---------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index 133be927..f623eb21 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -23,6 +23,8 @@ const PRE_DEFINED_COLUMNS = Object.values(PreDefinedColumn); export default abstract class DatabaseService { protected readonly SOURCE_METADATA_TABLE = "source_metadata"; public static readonly LIST_DELIMITER = ","; + // Name of the hidden column BFF uses to uniquely identify rows + private static readonly HIDDEN_UID_COLUMN = "hidden_bff_uid"; // "Open file link" as a datatype must be hardcoded, and CAN NOT change // without BREAKING visibility in the dataset released in 2024 as part // of the EMT Data Release paper @@ -155,7 +157,7 @@ export default abstract class DatabaseService { ); } - protected async deleteDataSource(dataSource: string): Promise { + private async deleteDataSource(dataSource: string): Promise { this.existingDataSources.delete(dataSource); this.dataSourceToAnnotationsMap.delete(dataSource); await this.execute(`DROP TABLE IF EXISTS "${dataSource}"`); @@ -166,38 +168,24 @@ export default abstract class DatabaseService { MUST come after we check for errors so that we can rely on the table to at least be valid before modifying it further */ - protected async addRequiredColumns(name: string): Promise { - const dataSourceColumns = await this.getColumnsOnDataSource(name); - if (!dataSourceColumns.has(PreDefinedColumn.FILE_ID)) { - await this.execute(` + private async addRequiredColumns(name: string): Promise { + const commandsToExecute = [ + // Add hidden UID column to uniquely identify rows + ` ALTER TABLE "${name}" - ADD COLUMN IF NOT EXISTS "${PreDefinedColumn.FILE_ID}" VARCHAR; - `); - // The data source's input date gets appended to the end - // of the auto-generated name, that makes it kind of messy - // as an ID though so here lets remove it - const dataSourceNameWithoutDate = name.split(" ")[0]; - // Auto-generates a File ID based on the row number when - // ordered by file path. - await this.execute(` - UPDATE "${name}" - SET "${PreDefinedColumn.FILE_ID}" = CONCAT(SQ.row, '-', '${dataSourceNameWithoutDate}') - FROM ( - SELECT "${PreDefinedColumn.FILE_PATH}", ROW_NUMBER() OVER (ORDER BY "${PreDefinedColumn.FILE_PATH}") AS row - FROM "${name}" - ) AS SQ - WHERE "${name}"."${PreDefinedColumn.FILE_PATH}" = SQ."${PreDefinedColumn.FILE_PATH}"; - `); - } + ADD COLUMN IF NOT EXISTS "${DatabaseService.HIDDEN_UID_COLUMN}" INT NOT NULL PRIMARY KEY + `, + ]; + const dataSourceColumns = await this.getColumnsOnDataSource(name); if (!dataSourceColumns.has(PreDefinedColumn.FILE_NAME)) { - await this.execute(` + commandsToExecute.push(` ALTER TABLE "${name}" ADD COLUMN IF NOT EXISTS "${PreDefinedColumn.FILE_NAME}" VARCHAR; `); // Best shot attempt at auto-generating a "File Name" // from the "File Path", defaults to full path if this fails - await this.execute(` + commandsToExecute.push(` UPDATE "${name}" SET "${PreDefinedColumn.FILE_NAME}" = COALESCE( NULLIF( @@ -211,6 +199,8 @@ export default abstract class DatabaseService { `); } + await this.execute(commandsToExecute.join("; ")); + // Because we edited the column names this cache is now invalid this.dataSourceToAnnotationsMap.delete(name); } @@ -220,7 +210,7 @@ export default abstract class DatabaseService { the expectations around uniqueness/blankness for pre-defined columns like "File Path", "File ID", etc. */ - protected async checkDataSourceForErrors(name: string): Promise { + private async checkDataSourceForErrors(name: string): Promise { const errors: string[] = []; const columnsOnTable = await this.getColumnsOnDataSource(name); @@ -320,19 +310,21 @@ export default abstract class DatabaseService { private async normalizeDataSourceColumnNames(dataSourceName: string): Promise { const columnsOnDataSource = await this.getColumnsOnDataSource(dataSourceName); - for (const preDefinedColumn of PRE_DEFINED_COLUMNS) { + const combinedAlterCommands = PRE_DEFINED_COLUMNS // Filter out any pre-defined columns that are exact matches to columns on the data source // since those are already perfect - if (!columnsOnDataSource.has(preDefinedColumn)) { + .filter((preDefinedColumn) => !columnsOnDataSource.has(preDefinedColumn)) + // Map the rest to SQL alter table commands to rename the columns + .flatMap((preDefinedColumn) => { const preDefinedColumnSimplified = preDefinedColumn.toLowerCase().replace(" ", ""); // Grab near matches to the pre-defined columns like "file_name" for "File Name" - const matches = [...columnsOnDataSource].filter((column) => { - const simplifiedColumn = column - .toLowerCase() // File Name -> file name - .replaceAll(/\s|-|_/g, ""); // Matches any whitespace, hyphen, or underscore - return simplifiedColumn === preDefinedColumnSimplified; - }); + const matches = [...columnsOnDataSource].filter( + (column) => + preDefinedColumnSimplified === + // Matches regardless of caps, whitespace, hyphens, or underscores + column.toLowerCase().replaceAll(/\s|-|_/g, "") + ); // Doesn't seem like we should guess at a pre-defined column match in this case // so just toss up a user-actionable error to try to get them to retry @@ -344,14 +336,22 @@ export default abstract class DatabaseService { ); } - if (matches.length === 1) { - // Rename matching column name to new pre-defined column - await this.execute(` - ALTER TABLE "${dataSourceName}" - RENAME COLUMN "${matches[0]}" TO '${preDefinedColumn}' - `); + if (matches.length < 0) { + return []; // No-op essentially if no matches } - } + + // Rename matching column name to new pre-defined column + return [ + ` + ALTER TABLE "${dataSourceName}" + RENAME COLUMN "${matches[0]}" TO '${preDefinedColumn}' + `, + ]; + }) + .join("; "); + + if (combinedAlterCommands) { + await this.execute(combinedAlterCommands); } // Because we edited the column names this cache is now invalid @@ -388,19 +388,16 @@ export default abstract class DatabaseService { return duplicateColumnQueryResult.map((row) => row.row); } - // TODO: Triple check this is going to work still... private async aggregateDataSources(dataSources: Source[]): Promise { const viewName = dataSources .map((source) => source.name) .sort() .join(", "); - if (this.currentAggregateSource) { + if (this.currentAggregateSource === viewName) { // Prevent adding the same data source multiple times by shortcutting out here - if (this.currentAggregateSource === viewName) { - return; - } - + return; + } else if (this.currentAggregateSource) { // Otherwise, if an old aggregate exists, delete it await this.deleteDataSource(this.currentAggregateSource); } @@ -461,6 +458,7 @@ export default abstract class DatabaseService { .select("column_name, data_type") .from('information_schema"."columns') .where(`table_name = '${aggregateDataSourceName}'`) + .where(`column_name != '${DatabaseService.HIDDEN_UID_COLUMN}'`) .toSQL(); const rows = await this.query(sql); if (isEmpty(rows)) { From 69841103b5fc865b1748e7ecc77b7c63a04ad02c Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 09:39:44 -0800 Subject: [PATCH 05/15] Use UID instead of File iD --- .../components/DataSourcePrompt/index.tsx | 10 +-- .../components/FileList/LazilyRenderedRow.tsx | 2 +- .../FileList/LazilyRenderedThumbnail.tsx | 2 +- packages/core/components/FileRow/index.tsx | 2 +- packages/core/constants/index.ts | 7 +- packages/core/entity/FileDetail/index.ts | 10 ++- .../core/services/DatabaseService/index.ts | 74 +++++-------------- .../FileService/DatabaseFileService/index.ts | 59 +++++++-------- packages/core/state/interaction/logics.ts | 2 +- 9 files changed, 64 insertions(+), 104 deletions(-) diff --git a/packages/core/components/DataSourcePrompt/index.tsx b/packages/core/components/DataSourcePrompt/index.tsx index 0b004338..7e4af533 100644 --- a/packages/core/components/DataSourcePrompt/index.tsx +++ b/packages/core/components/DataSourcePrompt/index.tsx @@ -19,8 +19,8 @@ interface Props { const ADDITIONAL_COLUMN_DETAILS = [ 'If a "Thumbnail" column is present it should contain a web URL to a thumbnail image for the file. ', 'If a "File Name" column is present it should be the file\'s name (this will replace the "File Name" created by default from the path). ', - 'If a "File Size" column is present it should contain the size of the file in bytes. ', - 'If an "Uploaded" column is present it should contain the date the file was uploaded to the cloud storage and be formatted as YYYY-MM-DD HH:MM:SS.Z where Z is a timezone offset. ', + 'If a "File Size" column is present it should contain the size of the file in bytes. This is used for showing feedback to the user during downloads. ', + 'If an "Uploaded" column is present it should contain the date the file was uploaded to the storage and be formatted as YYYY-MM-DD HH:MM:SS.Z where Z is a timezone offset. ', ]; /** @@ -81,10 +81,8 @@ export default function DataSourcePrompt(props: Props) { <>
  • - The file must contain a "File Path" column & must be unique by - the "File Path" column or have a unique "File ID" - column. Any other columns are optional and will be available as - annotations to query by. + The file must contain a "File Path" column. Any other columns + are optional and will be available as metadata to query by.

Advanced:

diff --git a/packages/core/components/FileList/LazilyRenderedRow.tsx b/packages/core/components/FileList/LazilyRenderedRow.tsx index 81c21e8e..fccbb20a 100644 --- a/packages/core/components/FileList/LazilyRenderedRow.tsx +++ b/packages/core/components/FileList/LazilyRenderedRow.tsx @@ -67,7 +67,7 @@ export default function LazilyRenderedRow(props: LazilyRenderedRowProps) { displayValue: annotation.extractFromFile(file), width: columnWidths[annotation.name] || 1 / annotations.length, }))} - rowIdentifier={{ index, id: file.id }} + rowIdentifier={{ index, id: file.uid }} onSelect={onSelect} /> ); diff --git a/packages/core/components/FileList/LazilyRenderedThumbnail.tsx b/packages/core/components/FileList/LazilyRenderedThumbnail.tsx index 11506fd1..eca9f8c4 100644 --- a/packages/core/components/FileList/LazilyRenderedThumbnail.tsx +++ b/packages/core/components/FileList/LazilyRenderedThumbnail.tsx @@ -79,7 +79,7 @@ export default function LazilyRenderedThumbnail(props: LazilyRenderedThumbnailPr if (onSelect && file !== undefined) { onSelect( - { index: overallIndex, id: file.id }, + { index: overallIndex, id: file.uid }, { // Details on different OS keybindings // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent#Properties diff --git a/packages/core/components/FileRow/index.tsx b/packages/core/components/FileRow/index.tsx index 75325cee..662d7054 100644 --- a/packages/core/components/FileRow/index.tsx +++ b/packages/core/components/FileRow/index.tsx @@ -19,7 +19,7 @@ export interface CellConfig { interface FileRowProps { cells: CellConfig[]; className?: string; - rowIdentifier?: any; + rowIdentifier?: { index: number; id: string }; onContextMenu?: (evt: React.MouseEvent) => void; onResize?: (columnKey: string, nextWidth?: number) => void; onSelect?: OnSelect; diff --git a/packages/core/constants/index.ts b/packages/core/constants/index.ts index 2c7275e4..6fbcd0bd 100644 --- a/packages/core/constants/index.ts +++ b/packages/core/constants/index.ts @@ -15,18 +15,17 @@ export const TOP_LEVEL_FILE_ANNOTATIONS = [ new Annotation({ annotationDisplayName: "File ID", annotationName: AnnotationName.FILE_ID, - description: - "Auto or manually generated unique ID for file. Should not be used for collaboration or sharing as it may change.", + description: "ID for file.", type: AnnotationType.STRING, }), new Annotation({ annotationDisplayName: "File Name", annotationName: AnnotationName.FILE_NAME, - description: "Name of file", + description: "Name of file.", type: AnnotationType.STRING, }), new Annotation({ - annotationDisplayName: "File Path (Canonical)", + annotationDisplayName: "File Path", annotationName: AnnotationName.FILE_PATH, description: "Path to file in storage.", type: AnnotationType.STRING, diff --git a/packages/core/entity/FileDetail/index.ts b/packages/core/entity/FileDetail/index.ts index 4d8258d3..83ddc9f4 100644 --- a/packages/core/entity/FileDetail/index.ts +++ b/packages/core/entity/FileDetail/index.ts @@ -73,7 +73,8 @@ export interface FmsFile { * Facade for a FileDetailResponse. */ export default class FileDetail { - private fileDetail: FmsFile; + private readonly fileDetail: FmsFile; + private readonly uniqueId?: string; private static convertAicsDrivePathToAicsS3Path(path: string): string { const pathWithoutDrive = path.replace("/allen/programs/allencell/data/proj0", ""); @@ -82,14 +83,19 @@ export default class FileDetail { return `https://s3.us-west-2.amazonaws.com/production.files.allencell.org${pathWithoutDrive}`; } - constructor(fileDetail: FmsFile) { + constructor(fileDetail: FmsFile, uniqueId?: string) { this.fileDetail = fileDetail; + this.uniqueId = uniqueId; } public get details() { return this.fileDetail; } + public get uid(): string { + return this.uniqueId || this.id; + } + public get id(): string { const id = this.fileDetail.file_id || this.getFirstAnnotationValue("File ID"); if (id === undefined) { diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index f623eb21..382ae974 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -21,10 +21,10 @@ const PRE_DEFINED_COLUMNS = Object.values(PreDefinedColumn); * Service reponsible for querying against a database */ export default abstract class DatabaseService { - protected readonly SOURCE_METADATA_TABLE = "source_metadata"; public static readonly LIST_DELIMITER = ","; + public static readonly HIDDEN_UID_ANNOTATION = "hidden_bff_uid"; // Name of the hidden column BFF uses to uniquely identify rows - private static readonly HIDDEN_UID_COLUMN = "hidden_bff_uid"; + protected readonly SOURCE_METADATA_TABLE = "source_metadata"; // "Open file link" as a datatype must be hardcoded, and CAN NOT change // without BREAKING visibility in the dataset released in 2024 as part // of the EMT Data Release paper @@ -173,7 +173,19 @@ export default abstract class DatabaseService { // Add hidden UID column to uniquely identify rows ` ALTER TABLE "${name}" - ADD COLUMN IF NOT EXISTS "${DatabaseService.HIDDEN_UID_COLUMN}" INT NOT NULL PRIMARY KEY + ADD COLUMN ${DatabaseService.HIDDEN_UID_ANNOTATION} INT + `, + // Altering tables to add primary keys or serially generated columns + // isn't yet supported in DuckDB, so this does a serially generated + // column addition manually + ` + UPDATE "${name}" + SET "${DatabaseService.HIDDEN_UID_ANNOTATION}" = SQ.row + FROM ( + SELECT "${PreDefinedColumn.FILE_PATH}", ROW_NUMBER() OVER (ORDER BY "${PreDefinedColumn.FILE_PATH}") AS row + FROM "${name}" + ) AS SQ + WHERE "${name}"."${PreDefinedColumn.FILE_PATH}" = SQ."${PreDefinedColumn.FILE_PATH}"; `, ]; @@ -181,7 +193,7 @@ export default abstract class DatabaseService { if (!dataSourceColumns.has(PreDefinedColumn.FILE_NAME)) { commandsToExecute.push(` ALTER TABLE "${name}" - ADD COLUMN IF NOT EXISTS "${PreDefinedColumn.FILE_NAME}" VARCHAR; + ADD COLUMN "${PreDefinedColumn.FILE_NAME}" VARCHAR; `); // Best shot attempt at auto-generating a "File Name" // from the "File Path", defaults to full path if this fails @@ -214,37 +226,6 @@ export default abstract class DatabaseService { const errors: string[] = []; const columnsOnTable = await this.getColumnsOnDataSource(name); - // If a data source has a File ID it must also pass validation - const hasFileIdColumn = columnsOnTable.has(PreDefinedColumn.FILE_ID); - if (hasFileIdColumn) { - // Check for empty or just whitespace File ID column values - const blankFileIdRows = await this.getRowsWhereColumnIsBlank( - name, - PreDefinedColumn.FILE_ID - ); - if (blankFileIdRows.length > 0) { - const rowNumbers = DatabaseService.truncateString(blankFileIdRows.join(", "), 100); - errors.push( - `"${PreDefinedColumn.FILE_ID}" column contains ${blankFileIdRows.length} empty or purely whitespace ids at rows ${rowNumbers}.` - ); - } - - // Check for duplicate File ID column values - const duplicateFileIdRows = await this.getRowsWhereColumnIsNotUniqueOrBlank( - name, - PreDefinedColumn.FILE_ID - ); - if (duplicateFileIdRows.length > 0) { - const rowNumbers = DatabaseService.truncateString( - duplicateFileIdRows.join(", "), - 100 - ); - errors.push( - `"${PreDefinedColumn.FILE_ID}" column contains duplicates. Found ${duplicateFileIdRows.length} duplicate ids at rows ${rowNumbers}.` - ); - } - } - if (!columnsOnTable.has(PreDefinedColumn.FILE_PATH)) { let error = `"${PreDefinedColumn.FILE_PATH}" column is missing in the data source. Check the data source header row for a "${PreDefinedColumn.FILE_PATH}" column name and try again.`; @@ -277,25 +258,6 @@ export default abstract class DatabaseService { `"${PreDefinedColumn.FILE_PATH}" column contains ${blankFilePathRows.length} empty or purely whitespace paths at rows ${rowNumbers}.` ); } - - // "File Path" has to be unique when a unique File ID is not provided - // otherwise we can't cleanly auto-generate a File ID based on the File Path - if (!hasFileIdColumn) { - // Check for duplicate File ID column values - const duplicateFilePathRows = await this.getRowsWhereColumnIsNotUniqueOrBlank( - name, - PreDefinedColumn.FILE_PATH - ); - if (duplicateFilePathRows.length > 0) { - const rowNumbers = DatabaseService.truncateString( - duplicateFilePathRows.join(", "), - 100 - ); - errors.push( - `"${PreDefinedColumn.FILE_PATH}" column contains duplicates, but has no "${PreDefinedColumn.FILE_ID}" column to use as a unique identifier instead. Add a unique "${PreDefinedColumn.FILE_ID}" column or make "${PreDefinedColumn.FILE_PATH}" values unique. Found ${duplicateFilePathRows.length} duplicate paths at rows ${rowNumbers}.` - ); - } - } } return errors; @@ -336,7 +298,7 @@ export default abstract class DatabaseService { ); } - if (matches.length < 0) { + if (matches.length < 1) { return []; // No-op essentially if no matches } @@ -458,7 +420,7 @@ export default abstract class DatabaseService { .select("column_name, data_type") .from('information_schema"."columns') .where(`table_name = '${aggregateDataSourceName}'`) - .where(`column_name != '${DatabaseService.HIDDEN_UID_COLUMN}'`) + .where(`column_name != '${DatabaseService.HIDDEN_UID_ANNOTATION}'`) .toSQL(); const rows = await this.query(sql); if (isEmpty(rows)) { diff --git a/packages/core/services/FileService/DatabaseFileService/index.ts b/packages/core/services/FileService/DatabaseFileService/index.ts index dca2b9ed..4ec842bc 100644 --- a/packages/core/services/FileService/DatabaseFileService/index.ts +++ b/packages/core/services/FileService/DatabaseFileService/index.ts @@ -1,4 +1,4 @@ -import { isEmpty, isNil, omit, uniqueId } from "lodash"; +import { isEmpty, isNil, uniqueId } from "lodash"; import FileService, { GetFilesRequest, SelectionAggregationResult, Selection } from ".."; import DatabaseService from "../../DatabaseService"; @@ -25,36 +25,29 @@ export default class DatabaseFileService implements FileService { private readonly dataSourceNames: string[]; private static convertDatabaseRowToFileDetail(row: { [key: string]: string }): FileDetail { - const annotations = [ - { name: "File Path", values: [row["File Path"]] }, - { name: "File ID", values: [row["File ID"]] }, - { name: "File Name", values: [row["File Name"]] }, - ]; - if (!isNil(row["File Size"])) { - annotations.push({ name: "File Size", values: [row["File Size"]] }); + const uniqueId: string | undefined = row[DatabaseService.HIDDEN_UID_ANNOTATION]; + if (!uniqueId) { + throw new Error("Missing auto-generated unique ID"); } - if (row["Thumbnail"]) { - annotations.push({ name: "Thumbnail", values: [row["Thumbnail"]] }); - } - if (row["Uploaded"]) { - annotations.push({ name: "Uploaded", values: [row["Uploaded"]] }); - } - return new FileDetail({ - annotations: [ - ...Object.entries(omit(row, ...annotations.keys())).flatMap(([name, values]: any) => - values !== null - ? [ - { - name, - values: `${values}` - .split(DatabaseService.LIST_DELIMITER) - .map((value: string) => value.trim()), - }, - ] - : [] - ), - ], - }); + + return new FileDetail( + { + annotations: Object.entries(row) + .filter( + ([name, values]) => + !isNil(values) && + // Omit hidden UID annotation + name !== DatabaseService.HIDDEN_UID_ANNOTATION + ) + .map(([name, values]) => ({ + name, + values: `${values}` + .split(DatabaseService.LIST_DELIMITER) + .map((value: string) => value.trim()), + })), + }, + uniqueId + ); } constructor( @@ -134,7 +127,7 @@ export default class DatabaseFileService implements FileService { selections.forEach((selection) => { selection.indexRanges.forEach((indexRange) => { const subQuery = new SQLBuilder() - .select('"File Path"') + .select(`${DatabaseService.HIDDEN_UID_ANNOTATION}`) .from(this.dataSourceNames) .offset(indexRange.start) .limit(indexRange.end - indexRange.start + 1); @@ -156,7 +149,9 @@ export default class DatabaseFileService implements FileService { ); } - sqlBuilder.whereOr(`"File Path" IN (${subQuery.toSQL()})`); + sqlBuilder.whereOr( + `${DatabaseService.HIDDEN_UID_ANNOTATION} IN (${subQuery.toSQL()})` + ); }); }); diff --git a/packages/core/state/interaction/logics.ts b/packages/core/state/interaction/logics.ts index 9c7eb4eb..94ef5e22 100644 --- a/packages/core/state/interaction/logics.ts +++ b/packages/core/state/interaction/logics.ts @@ -213,7 +213,7 @@ const downloadFilesLogic = createLogic({ } else { const selectedFilesDetails = await fileSelection.fetchAllDetails(); filesToDownload = selectedFilesDetails.map((file) => ({ - id: file.id, + id: file.uid, name: file.name, size: file.size, path: file.downloadPath, From bcb08dce24e0310a16d161e375fcd46276323ec1 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 09:43:27 -0800 Subject: [PATCH 06/15] Use uid in more places --- packages/core/components/FileDetails/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/components/FileDetails/index.tsx b/packages/core/components/FileDetails/index.tsx index a7b02e21..fb43ce5a 100644 --- a/packages/core/components/FileDetails/index.tsx +++ b/packages/core/components/FileDetails/index.tsx @@ -99,7 +99,7 @@ export default function FileDetails(props: Props) { dispatch( interaction.actions.downloadFiles([ { - id: fileDetails.id, + id: fileDetails.uid, name: fileDetails.name, size: fileDetails.size, path: fileDetails.downloadPath, @@ -145,7 +145,7 @@ export default function FileDetails(props: Props) { - status.data.fileId?.includes(fileDetails.id) + status.data.fileId?.includes(fileDetails.uid) )} iconName="Download" text="Download" From b7a1438d7030a1ee5a2ec8349bd004decfbbd356 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 09:55:57 -0800 Subject: [PATCH 07/15] Use hidden id in tests --- .../DatabaseFileService/test/DatabaseFileService.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts b/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts index 27adaa2a..03b32f85 100644 --- a/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts +++ b/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts @@ -1,5 +1,6 @@ import { expect } from "chai"; +import DatabaseService from "../../../DatabaseService"; import FileSelection from "../../../../entity/FileSelection"; import FileSet from "../../../../entity/FileSet"; import NumericRange from "../../../../entity/NumericRange"; @@ -12,7 +13,7 @@ describe("DatabaseFileService", () => { const totalFileSize = 864452; const fileIds = ["abc123", "def456"]; const files = fileIds.map((file_id) => ({ - "File ID": file_id, + [DatabaseService.HIDDEN_UID_ANNOTATION]: file_id, "File Size": `${totalFileSize / 2}`, "File Path": "path/to/file", "File Name": "file", From 09858bd1f544232bce1be31583ebf50e2bb7a3c8 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 09:58:39 -0800 Subject: [PATCH 08/15] Add canonical path back --- packages/core/constants/index.ts | 2 +- .../DatabaseFileService/test/DatabaseFileService.test.ts | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/core/constants/index.ts b/packages/core/constants/index.ts index 6fbcd0bd..283b0b11 100644 --- a/packages/core/constants/index.ts +++ b/packages/core/constants/index.ts @@ -25,7 +25,7 @@ export const TOP_LEVEL_FILE_ANNOTATIONS = [ type: AnnotationType.STRING, }), new Annotation({ - annotationDisplayName: "File Path", + annotationDisplayName: "File Path (Canonical)", annotationName: AnnotationName.FILE_PATH, description: "Path to file in storage.", type: AnnotationType.STRING, diff --git a/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts b/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts index 03b32f85..3269add3 100644 --- a/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts +++ b/packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts @@ -44,10 +44,6 @@ describe("DatabaseFileService", () => { expect(data.length).to.equal(2); expect(data[0].details).to.deep.equal({ annotations: [ - { - name: "File ID", - values: ["abc123"], - }, { name: "File Size", values: ["432226"], From 00af6b800f08bd962fd0ee5eed2d86bcf3f69db4 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 11:03:45 -0800 Subject: [PATCH 09/15] Improve data source prompt explanation --- .../DataSourcePrompt.module.css | 9 +++++ .../components/DataSourcePrompt/index.tsx | 40 ++++++++++++++++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css b/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css index 74c8ec2f..78e3b530 100644 --- a/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css +++ b/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css @@ -53,6 +53,15 @@ justify-content: left; } +.table-example, .table-example > tr > :is(th, td) { + border: 2px solid var(--secondary-background-color); + border-collapse: collapse; +} + +.table-example > tr > :is(th, td) { + padding: 5px; +} + .title { margin-bottom: 6px; } diff --git a/packages/core/components/DataSourcePrompt/index.tsx b/packages/core/components/DataSourcePrompt/index.tsx index 7e4af533..453f6d0d 100644 --- a/packages/core/components/DataSourcePrompt/index.tsx +++ b/packages/core/components/DataSourcePrompt/index.tsx @@ -77,14 +77,42 @@ export default function DataSourcePrompt(props: Props) { To get started, load a CSV, Parquet, or JSON file containing metadata (annotations) about your files to view them.

+

+ The first row should be the metadata tags and each row proceeding it should be + metadata about a file. The only required column is "File Path" which is + intended to represent the path to a file in storage. Any other columns are optional + and will be available as metadata to query by. +

+

+ Example CSV: +

+ + + + + + + + + + + + + + + + +
File PathGene (Just an example)Color (Also an example)
/some/path/to/storage/somewhere.zarrCDH2Blue
/another/path/to/another/file.txtVIMGreen
{isDataSourceDetailExpanded ? ( <> -
    -
  • - The file must contain a "File Path" column. Any other columns - are optional and will be available as metadata to query by. -
    • -

Advanced:

  • From 2888c046854cf5d659b09f74158ad3fcc3ed7f19 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 11:27:51 -0800 Subject: [PATCH 10/15] Upgrade styling of csv example --- .../DataSourcePrompt/DataSourcePrompt.module.css | 11 ++++++++++- .../core/components/DataSourcePrompt/index.tsx | 14 +++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css b/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css index 78e3b530..776c4767 100644 --- a/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css +++ b/packages/core/components/DataSourcePrompt/DataSourcePrompt.module.css @@ -62,12 +62,21 @@ padding: 5px; } +.light-border.table-example, .light-border.table-example > tr > :is(th, td) { + border: 2px solid var(--primary-background-color); +} + +.left-text-align { + text-align: left !important; +} + .title { margin-bottom: 6px; } .datasource-subhead { - margin-bottom: 26px !important; + margin-bottom: 13px !important; + text-align: left; font-weight: 600; } diff --git a/packages/core/components/DataSourcePrompt/index.tsx b/packages/core/components/DataSourcePrompt/index.tsx index 453f6d0d..b3190e9f 100644 --- a/packages/core/components/DataSourcePrompt/index.tsx +++ b/packages/core/components/DataSourcePrompt/index.tsx @@ -82,19 +82,23 @@ export default function DataSourcePrompt(props: Props) { [styles.datasourceSubhead]: !props?.hideTitle, })} > - The first row should be the metadata tags and each row proceeding it should be - metadata about a file. The only required column is "File Path" which is - intended to represent the path to a file in storage. Any other columns are optional - and will be available as metadata to query by. + The first row should contain metadata tags, and each subsequent row includes + metadata for a file, with "File Path" being the only required column. + Other columns are optional and can be used for querying additional file metadata.

    Example CSV:

    - +
    From 8f6e19b18cb05602bca9a2600783ea2fe22a577c Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Wed, 13 Nov 2024 12:11:19 -0800 Subject: [PATCH 11/15] Add tbody/thead --- .../components/DataSourcePrompt/index.tsx | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/core/components/DataSourcePrompt/index.tsx b/packages/core/components/DataSourcePrompt/index.tsx index b3190e9f..b4a55e1f 100644 --- a/packages/core/components/DataSourcePrompt/index.tsx +++ b/packages/core/components/DataSourcePrompt/index.tsx @@ -99,21 +99,25 @@ export default function DataSourcePrompt(props: Props) { [styles.lightBorder]: !props?.hideTitle, })} > - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + +
    File Path Gene (Just an example)
    File PathGene (Just an example)Color (Also an example)
    /some/path/to/storage/somewhere.zarrCDH2Blue
    /another/path/to/another/file.txtVIMGreen
    File PathGene (Just an example)Color (Also an example)
    /some/path/to/storage/somewhere.zarrCDH2Blue
    /another/path/to/another/file.txtVIMGreen
    {isDataSourceDetailExpanded ? ( <> From 99d172e35b568cbab19eb35777fcd4f302c219e3 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Thu, 14 Nov 2024 12:48:41 -0800 Subject: [PATCH 12/15] Move comment --- packages/core/services/DatabaseService/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/services/DatabaseService/index.ts b/packages/core/services/DatabaseService/index.ts index 382ae974..c3f31788 100644 --- a/packages/core/services/DatabaseService/index.ts +++ b/packages/core/services/DatabaseService/index.ts @@ -22,8 +22,8 @@ const PRE_DEFINED_COLUMNS = Object.values(PreDefinedColumn); */ export default abstract class DatabaseService { public static readonly LIST_DELIMITER = ","; - public static readonly HIDDEN_UID_ANNOTATION = "hidden_bff_uid"; // Name of the hidden column BFF uses to uniquely identify rows + public static readonly HIDDEN_UID_ANNOTATION = "hidden_bff_uid"; protected readonly SOURCE_METADATA_TABLE = "source_metadata"; // "Open file link" as a datatype must be hardcoded, and CAN NOT change // without BREAKING visibility in the dataset released in 2024 as part From c901b9a7823abf7289e17afd8739e2bdf0ba2e12 Mon Sep 17 00:00:00 2001 From: SeanLeRoy Date: Thu, 14 Nov 2024 12:55:58 -0800 Subject: [PATCH 13/15] Typo --- packages/core/components/DataSourcePrompt/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/components/DataSourcePrompt/index.tsx b/packages/core/components/DataSourcePrompt/index.tsx index b4a55e1f..cfde006a 100644 --- a/packages/core/components/DataSourcePrompt/index.tsx +++ b/packages/core/components/DataSourcePrompt/index.tsx @@ -82,9 +82,9 @@ export default function DataSourcePrompt(props: Props) { [styles.datasourceSubhead]: !props?.hideTitle, })} > - The first row should contain metadata tags, and each subsequent row includes - metadata for a file, with "File Path" being the only required column. - Other columns are optional and can be used for querying additional file metadata. + The first row should contain metadata tags, and each subsequent row include metadata + for a file, with "File Path" being the only required column. Other columns + are optional and can be used for querying additional file metadata.

    Date: Mon, 18 Nov 2024 15:05:48 -0800 Subject: [PATCH 14/15] prevent button from refocusing on menu close --- packages/core/components/Buttons/BaseButton.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/components/Buttons/BaseButton.tsx b/packages/core/components/Buttons/BaseButton.tsx index 5dd608af..c5424cf1 100644 --- a/packages/core/components/Buttons/BaseButton.tsx +++ b/packages/core/components/Buttons/BaseButton.tsx @@ -1,5 +1,6 @@ import { DefaultButton, DirectionalHint, IContextualMenuItem, Icon } from "@fluentui/react"; import classNames from "classnames"; +import { noop } from "lodash"; import * as React from "react"; import useButtonMenu from "./useButtonMenu"; @@ -28,6 +29,7 @@ export default function BaseButton(props: Props) { const styledMenu = useButtonMenu({ items: props.menuItems, directionalHint: props.menuDirection, + onRestoreFocus: noop, // Prevent button from refocusing on menu close }); const content = ( From d7e9e998ad4c6dae20b36d91338e4e0082a012fe Mon Sep 17 00:00:00 2001 From: Anya Wallace Date: Mon, 18 Nov 2024 16:23:41 -0800 Subject: [PATCH 15/15] move refocus change to apply to all menu buttons --- packages/core/components/Buttons/BaseButton.tsx | 2 -- packages/core/components/Buttons/useButtonMenu.ts | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/components/Buttons/BaseButton.tsx b/packages/core/components/Buttons/BaseButton.tsx index c5424cf1..5dd608af 100644 --- a/packages/core/components/Buttons/BaseButton.tsx +++ b/packages/core/components/Buttons/BaseButton.tsx @@ -1,6 +1,5 @@ import { DefaultButton, DirectionalHint, IContextualMenuItem, Icon } from "@fluentui/react"; import classNames from "classnames"; -import { noop } from "lodash"; import * as React from "react"; import useButtonMenu from "./useButtonMenu"; @@ -29,7 +28,6 @@ export default function BaseButton(props: Props) { const styledMenu = useButtonMenu({ items: props.menuItems, directionalHint: props.menuDirection, - onRestoreFocus: noop, // Prevent button from refocusing on menu close }); const content = ( diff --git a/packages/core/components/Buttons/useButtonMenu.ts b/packages/core/components/Buttons/useButtonMenu.ts index 4164c3e7..42860cbb 100644 --- a/packages/core/components/Buttons/useButtonMenu.ts +++ b/packages/core/components/Buttons/useButtonMenu.ts @@ -1,4 +1,5 @@ import { ContextualMenuItemType, IContextualMenuItem, IContextualMenuProps } from "@fluentui/react"; +import { noop } from "lodash"; import * as React from "react"; import styles from "./useButtonMenu.module.css"; @@ -30,6 +31,7 @@ function normalizeButtonMenu(menu: IContextualMenuProps): IContextualMenuProps { className: styles.buttonMenu, calloutProps: { className: styles.buttonMenuCallout }, items: menu.items.map((item) => normalizeButtonMenuItem(item)), + onRestoreFocus: noop, // Prevent button from refocusing on menu close }; }