From b15514e37b5d45dcd16ad9a27d5f158d1476e712 Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Thu, 29 Jun 2023 14:23:53 -0400 Subject: [PATCH] PORTALS-2701 - Send row selection to CAVATICA - QueryVisualizationWrapper: add rowSelectionPrimaryKey prop to determine how filtering on row selection should work. Defaults to `['id']` for file views and datasets. - SendToCavaticaConfirmationDialog: refactor and encapsulate query to get actions required in new hook `useGetActionsRequiredForTableEntity` - ConfirmationDialog: fix confirmButtonDisabled prop not propagating to button - add tests to confirm actions required blocks export to CAVATICA action --- .../ConfirmationDialog/ConfirmationDialog.tsx | 2 + .../components/QueryVisualizationWrapper.tsx | 28 +++++++++-- .../QueryWrapperPlotNav.tsx | 34 +++++++------ .../RowSelection/RowSelectionUI.tsx | 2 +- .../SendToCavaticaConfirmationDialog.tsx | 48 +++++++++---------- .../components/SynapseTable/SynapseTable.tsx | 16 ++----- .../SynapseTable/SynapseTableUtils.ts | 18 +++++++ .../entity/useActionsRequiredForTableQuery.ts | 46 ++++++++++++++++++ .../entity/useGetQueryResultBundle.ts | 39 ++++++++++----- .../SendToCavaticaConfirmationDialog.test.tsx | 46 +++++++++++++++++- 10 files changed, 209 insertions(+), 70 deletions(-) create mode 100644 packages/synapse-react-client/src/synapse-queries/entity/useActionsRequiredForTableQuery.ts diff --git a/packages/synapse-react-client/src/components/ConfirmationDialog/ConfirmationDialog.tsx b/packages/synapse-react-client/src/components/ConfirmationDialog/ConfirmationDialog.tsx index 3171939231..cea191d325 100644 --- a/packages/synapse-react-client/src/components/ConfirmationDialog/ConfirmationDialog.tsx +++ b/packages/synapse-react-client/src/components/ConfirmationDialog/ConfirmationDialog.tsx @@ -61,6 +61,7 @@ export function ConfirmationDialog(props: ConfirmationDialogProps) { confirmButtonClassName, confirmButtonColor, confirmButtonVariant, + confirmButtonDisabled, onConfirm, onCancel, hasCancelButton, @@ -75,6 +76,7 @@ export function ConfirmationDialog(props: ConfirmationDialogProps) { confirmButtonClassName={confirmButtonClassName} confirmButtonColor={confirmButtonColor} confirmButtonVariant={confirmButtonVariant} + confirmButtonDisabled={confirmButtonDisabled} onConfirm={onConfirm} onCancel={onCancel} hasCancelButton={hasCancelButton} diff --git a/packages/synapse-react-client/src/components/QueryVisualizationWrapper.tsx b/packages/synapse-react-client/src/components/QueryVisualizationWrapper.tsx index a3b8250d92..cbff9cb588 100644 --- a/packages/synapse-react-client/src/components/QueryVisualizationWrapper.tsx +++ b/packages/synapse-react-client/src/components/QueryVisualizationWrapper.tsx @@ -14,6 +14,7 @@ import ThisTableIsEmpty from './SynapseTable/TableIsEmpty' import { unCamelCase } from '../utils/functions/unCamelCase' import { ColumnType, Row } from '@sage-bionetworks/synapse-types' import { getDisplayValue } from '../utils/functions/getDataFromFromStorage' +import { isFileViewOrDataset } from './SynapseTable/SynapseTableUtils' export type QueryVisualizationContextType = { topLevelControlsState: TopLevelControlsState @@ -39,6 +40,10 @@ export type QueryVisualizationContextType = { setIsShowingExportToCavaticaModal: React.Dispatch< React.SetStateAction > + /** The set of columns that defines a uniqueness constraint on the table for the purposes of filtering based on row selection. + * Note that Synapse tables have no internal concept of a primary key. + */ + rowSelectionPrimaryKey?: string[] } /** @@ -94,6 +99,10 @@ export type QueryVisualizationWrapperProps = { /** Default is INTERACTIVE */ noContentPlaceholderType?: NoContentPlaceholderType isRowSelectionVisible?: boolean + /** The set of columns that defines a uniqueness constraint on the table for the purposes of filtering based on row selection. + * Note that Synapse tables have no internal concept of a primary key. + */ + rowSelectionPrimaryKey?: string[] } export type TopLevelControlsState = { @@ -117,12 +126,22 @@ export function QueryVisualizationWrapper( const { noContentPlaceholderType = NoContentPlaceholderType.INTERACTIVE, isRowSelectionVisible = false, + columnAliases = {}, } = props - const { data, getLastQueryRequest, isFacetsAvailable, hasResettableFilters } = - useQueryContext() - - const { columnAliases = {} } = props + const { + data, + entity, + getLastQueryRequest, + isFacetsAvailable, + hasResettableFilters, + } = useQueryContext() + + let { rowSelectionPrimaryKey } = props + if (!rowSelectionPrimaryKey && isFileViewOrDataset(entity)) { + // If the primary key isn't specified on a file view/dataset, we can safely use the 'id' column + rowSelectionPrimaryKey = ['id'] + } const [topLevelControlsState, setTopLevelControlsState] = useState({ @@ -212,6 +231,7 @@ export function QueryVisualizationWrapper( isRowSelectionVisible, isShowingExportToCavaticaModal, setIsShowingExportToCavaticaModal, + rowSelectionPrimaryKey, } /** * Render the children without any formatting diff --git a/packages/synapse-react-client/src/components/QueryWrapperPlotNav/QueryWrapperPlotNav.tsx b/packages/synapse-react-client/src/components/QueryWrapperPlotNav/QueryWrapperPlotNav.tsx index fa4d00d205..7def51865b 100644 --- a/packages/synapse-react-client/src/components/QueryWrapperPlotNav/QueryWrapperPlotNav.tsx +++ b/packages/synapse-react-client/src/components/QueryWrapperPlotNav/QueryWrapperPlotNav.tsx @@ -41,6 +41,7 @@ import FacetFilterControls, { import FilterAndView from './FilterAndView' import { NoContentPlaceholderType } from '../SynapseTable/NoContentPlaceholderType' import { Box } from '@mui/material' +import { SynapseErrorBoundary } from '../error/ErrorBanner' type QueryWrapperPlotNavOwnProps = { sql: string @@ -76,8 +77,9 @@ type QueryWrapperPlotNavOwnProps = { | 'rgbIndex' | 'showLastUpdatedOn' | 'noContentPlaceholderType' - | 'isRowSelectionVisible', - 'unitDescription' + | 'isRowSelectionVisible' + | 'unitDescription' + | 'rowSelectionPrimaryKey' > export type SearchParams = { @@ -119,6 +121,7 @@ const QueryWrapperPlotNav: React.FunctionComponent = ( customControls, isRowSelectionVisible, unitDescription, + rowSelectionPrimaryKey, } = props const entityId = parseEntityIdFromSqlStatement(sql) @@ -162,6 +165,7 @@ const QueryWrapperPlotNav: React.FunctionComponent = ( = ( queryVisualizationContext.setTopLevelControlsState } /> - + + + {isFaceted && ( <> 0 @@ -46,12 +45,18 @@ export default function SendToCavaticaConfirmationDialog( if (!hasSelectedRows) { return request } else { + if (!rowSelectionPrimaryKey || rowSelectionPrimaryKey.length !== 1) { + // TODO: Handle composite/undefined key + throw new Error( + 'rowSelectionPrimaryKey must be defined and have length 1', + ) + } // Add a filter that will just return the selected rows. const idColIndex = data?.columnModels?.findIndex(cm => cm.name === 'id') const idColumnFilter: ColumnSingleValueQueryFilter = { concreteType: 'org.sagebionetworks.repo.model.table.ColumnSingleValueQueryFilter', - columnName: 'id', + columnName: rowSelectionPrimaryKey[0], operator: ColumnSingleValueFilterOperator.IN, values: selectedRows!.map(row => row.values[idColIndex!]!), } @@ -67,25 +72,16 @@ export default function SendToCavaticaConfirmationDialog( cavaticaQueryRequest, data?.queryResult?.queryResults.headers, ) - const queryRequestCopy = useMemo(() => { - const request = cloneDeep(cavaticaQueryRequest) - const fileColumnId = getFileColumnModelId(data?.columnModels) - if (fileColumnId) { - request.query.selectFileColumn = Number(fileColumnId) - } - request.partMask = SynapseConstants.BUNDLE_MASK_ACTIONS_REQUIRED - return request - }, [cavaticaQueryRequest, data?.columnModels]) - const { data: asyncJobStatus, isLoading } = - useGetQueryResultBundleWithAsyncStatus(queryRequestCopy, { - enabled: fileColumnId !== undefined, + const { data: actions, isLoading } = useGetActionsRequiredForTableQuery( + cavaticaQueryRequest, + data?.columnModels as ColumnModel[], + undefined, + { useErrorBoundary: true, - }) - - const queryResultBundle = asyncJobStatus?.responseBody - const actions: ActionRequiredCount[] | undefined = - queryResultBundle?.actionsRequired + enabled: !!data?.columnModels, + }, + ) const confirmButtonText = `Send ${getNumberOfResultsToInvokeActionCopy( hasResettableFilters, @@ -168,7 +164,7 @@ export default function SendToCavaticaConfirmationDialog( <> You must also take these actions before sending the selected data to CAVATICA: @@ -183,7 +179,7 @@ export default function SendToCavaticaConfirmationDialog( You must take the following actions before we can send this data to CAVATICA. - + {actions.map((item: ActionRequiredCount, index) => { if (item) { return ( @@ -198,7 +194,7 @@ export default function SendToCavaticaConfirmationDialog( ) } else return false })} - + ) )} diff --git a/packages/synapse-react-client/src/components/SynapseTable/SynapseTable.tsx b/packages/synapse-react-client/src/components/SynapseTable/SynapseTable.tsx index e9c647702b..e8f30b1b09 100644 --- a/packages/synapse-react-client/src/components/SynapseTable/SynapseTable.tsx +++ b/packages/synapse-react-client/src/components/SynapseTable/SynapseTable.tsx @@ -4,7 +4,6 @@ import React from 'react' import { DialogBase } from '../DialogBase' import SynapseClient from '../../synapse-client' import { - hasFilesInView, isDataset, isDatasetCollection, isEntityView, @@ -48,6 +47,7 @@ import { ICON_STATE } from './SynapseTableConstants' import { getColumnIndicesWithType, getUniqueEntities, + isFileViewOrDataset, } from './SynapseTableUtils' import { TablePagination } from './TablePagination' import EntityIDColumnCopyIcon from './EntityIDColumnCopyIcon' @@ -426,15 +426,6 @@ export class SynapseTable extends React.Component< showDownloadColumn, } = this.props - /** - * i.e. the view may have FileEntities in it - * - * PORTALS-2010: Enhance change made for PORTALS-1973. File specific action will only be shown for rows that represent FileEntities. - */ - const isFileViewOrDataset = - entity && - ((isEntityView(entity) && hasFilesInView(entity)) || isDataset(entity)) - const isShowingAccessColumn: boolean | undefined = showAccessColumn && entity && @@ -443,7 +434,10 @@ export class SynapseTable extends React.Component< const isLoggedIn = !!this.props.synapseContext.accessToken const rowsAreDownloadable = - entity && isFileViewOrDataset && isLoggedIn && this.allRowsHaveId() + entity && + isFileViewOrDataset(entity) && + isLoggedIn && + this.allRowsHaveId() const isShowingAddToV2DownloadListColumn: boolean = !!( rowsAreDownloadable && !this.props.hideDownload diff --git a/packages/synapse-react-client/src/components/SynapseTable/SynapseTableUtils.ts b/packages/synapse-react-client/src/components/SynapseTable/SynapseTableUtils.ts index 8a2b8e3ef1..2f8787592b 100644 --- a/packages/synapse-react-client/src/components/SynapseTable/SynapseTableUtils.ts +++ b/packages/synapse-react-client/src/components/SynapseTable/SynapseTableUtils.ts @@ -4,9 +4,15 @@ import { ColumnTypeEnum, EntityHeader, QueryResultBundle, + Table, UserGroupHeader, UserProfile, } from '@sage-bionetworks/synapse-types' +import { + hasFilesInView, + isDataset, + isEntityView, +} from '../../utils/functions/EntityTypeUtils' export const getColumnIndicesWithType = ( data: QueryResultBundle | undefined, @@ -46,6 +52,18 @@ export const getUniqueEntities = ( return distinctEntities } +/** + * i.e. the view may have FileEntities in it + * + * PORTALS-2010: Enhance change made for PORTALS-1973. File specific action will only be shown for rows that represent FileEntities. + */ +export function isFileViewOrDataset(entity?: Table) { + return ( + entity && + ((isEntityView(entity) && hasFilesInView(entity)) || isDataset(entity)) + ) +} + export const getFileColumnModelId = ( columnModels?: ColumnModel[], ): string | undefined => { diff --git a/packages/synapse-react-client/src/synapse-queries/entity/useActionsRequiredForTableQuery.ts b/packages/synapse-react-client/src/synapse-queries/entity/useActionsRequiredForTableQuery.ts new file mode 100644 index 0000000000..b9e8159e7b --- /dev/null +++ b/packages/synapse-react-client/src/synapse-queries/entity/useActionsRequiredForTableQuery.ts @@ -0,0 +1,46 @@ +import { + ActionRequiredCount, + AsynchronousJobStatus, + ColumnModel, + QueryBundleRequest, + QueryResultBundle, +} from '@sage-bionetworks/synapse-types' +import { useMemo } from 'react' +import { cloneDeep } from 'lodash-es' +import { getFileColumnModelId } from '../../components/SynapseTable/SynapseTableUtils' +import { SynapseConstants } from '../../utils' +import { useGetQueryResultBundleWithAsyncStatus } from './useGetQueryResultBundle' +import { UseQueryOptions } from 'react-query' +import { SynapseClientError } from '../../utils/SynapseClientError' + +export function useGetActionsRequiredForTableQuery( + queryBundleRequest: QueryBundleRequest, + columnModels: ColumnModel[], + fileColumnModelId?: number, + options?: UseQueryOptions< + AsynchronousJobStatus, + SynapseClientError, + ActionRequiredCount[] + >, +) { + const queryRequestCopy = useMemo(() => { + const request = cloneDeep(queryBundleRequest) + const fileColumnId = fileColumnModelId || getFileColumnModelId(columnModels) + request.query.selectFileColumn = Number(fileColumnId) + request.partMask = SynapseConstants.BUNDLE_MASK_ACTIONS_REQUIRED + return request + }, [columnModels, fileColumnModelId, queryBundleRequest]) + + return useGetQueryResultBundleWithAsyncStatus( + queryRequestCopy, + { + ...options, + enabled: + (options?.enabled ?? true) && + queryRequestCopy.query.selectFileColumn !== undefined, + select: data => { + return data?.responseBody?.actionsRequired! + }, + }, + ) +} diff --git a/packages/synapse-react-client/src/synapse-queries/entity/useGetQueryResultBundle.ts b/packages/synapse-react-client/src/synapse-queries/entity/useGetQueryResultBundle.ts index c853b2a716..3df6f04b1b 100644 --- a/packages/synapse-react-client/src/synapse-queries/entity/useGetQueryResultBundle.ts +++ b/packages/synapse-react-client/src/synapse-queries/entity/useGetQueryResultBundle.ts @@ -50,11 +50,14 @@ export default function useGetQueryResultBundle( ) } -function _useGetQueryResultBundleWithAsyncStatus( +function _useGetQueryResultBundleWithAsyncStatus< + TData = AsynchronousJobStatus, +>( queryBundleRequest: QueryBundleRequest, options?: UseQueryOptions< AsynchronousJobStatus, - SynapseClientError + SynapseClientError, + TData >, setCurrentAsyncStatus?: ( status: AsynchronousJobStatus, @@ -64,7 +67,8 @@ function _useGetQueryResultBundleWithAsyncStatus( return useQuery< AsynchronousJobStatus, - SynapseClientError + SynapseClientError, + TData >( keyFactory.getEntityTableQueryResultWithAsyncStatusQueryKey( queryBundleRequest, @@ -83,11 +87,14 @@ function _useGetQueryResultBundleWithAsyncStatus( ) } -function useGetQueryRows( +function useGetQueryRows< + TData = AsynchronousJobStatus, +>( queryBundleRequest: QueryBundleRequest, options?: UseQueryOptions< AsynchronousJobStatus, - SynapseClientError + SynapseClientError, + TData >, setCurrentAsyncStatus?: ( status: AsynchronousJobStatus, @@ -103,7 +110,7 @@ function useGetQueryRows( const enableQuery = queryRowsBundleRequestMask > 0 ? options?.enabled : false - return _useGetQueryResultBundleWithAsyncStatus( + return _useGetQueryResultBundleWithAsyncStatus( rowsOnlyQueryBundleRequest, { ...options, @@ -113,11 +120,14 @@ function useGetQueryRows( ) } -function useGetQueryStats( +function useGetQueryStats< + TData = AsynchronousJobStatus, +>( queryBundleRequest: QueryBundleRequest, options?: UseQueryOptions< AsynchronousJobStatus, - SynapseClientError + SynapseClientError, + TData >, setCurrentAsyncStatus?: ( status: AsynchronousJobStatus, @@ -140,7 +150,7 @@ function useGetQueryStats( const enableQuery = queryStatsMask > 0 ? options?.enabled : false - return _useGetQueryResultBundleWithAsyncStatus( + return _useGetQueryResultBundleWithAsyncStatus( queryStatsRequest, { ...options, @@ -150,11 +160,14 @@ function useGetQueryStats( ) } -export function useGetQueryResultBundleWithAsyncStatus( +export function useGetQueryResultBundleWithAsyncStatus< + TData = AsynchronousJobStatus, +>( queryBundleRequest: QueryBundleRequest, options?: UseQueryOptions< AsynchronousJobStatus, - SynapseClientError + SynapseClientError, + TData >, setCurrentAsyncStatus?: ( status: AsynchronousJobStatus, @@ -165,12 +178,12 @@ export function useGetQueryResultBundleWithAsyncStatus( * - Query result rows, which will change each page * - Everything else, which does not change each page */ - const rowResult = useGetQueryRows( + const rowResult = useGetQueryRows( queryBundleRequest, options, setCurrentAsyncStatus, ) - const statsResult = useGetQueryStats( + const statsResult = useGetQueryStats( queryBundleRequest, options, setCurrentAsyncStatus, diff --git a/packages/synapse-react-client/test/containers/table/SendToCavaticaConfirmationDialog.test.tsx b/packages/synapse-react-client/test/containers/table/SendToCavaticaConfirmationDialog.test.tsx index c2ebcf3f83..2f22463e77 100644 --- a/packages/synapse-react-client/test/containers/table/SendToCavaticaConfirmationDialog.test.tsx +++ b/packages/synapse-react-client/test/containers/table/SendToCavaticaConfirmationDialog.test.tsx @@ -19,7 +19,10 @@ import { } from '../../../src/components/QueryVisualizationWrapper' import { ColumnSingleValueFilterOperator } from '@sage-bionetworks/synapse-types' import { cloneDeep } from 'lodash-es' +import { mockManagedACTAccessRequirement } from '../../../mocks/mockAccessRequirements' import * as UseExportToCavaticaModule from '../../../src/synapse-queries/entity/useExportToCavatica' +import * as UseActionsRequiredForTableQueryModule from '../../../src/synapse-queries/entity/useActionsRequiredForTableQuery' +import * as ActionRequiredListItem from '../../../src/components/DownloadCart/ActionRequiredListItem' const onExportToCavatica = jest.fn() @@ -29,6 +32,13 @@ const mockUseExportToCavatica = jest return onExportToCavatica }) +const mockUseGetActionsRequiredForTableQuery = jest + .spyOn( + UseActionsRequiredForTableQueryModule, + 'useGetActionsRequiredForTableQuery', + ) + .mockReturnValue({ isLoading: false, data: [] }) + function renderComponent( wrapperProps?: SynapseContextType, queryContextOverrides?: Partial, @@ -52,6 +62,7 @@ function renderComponent( selectedRows: [], resultsToExportToCavatica: 'ALL', unitDescription: 'result', + rowSelectionPrimaryKey: ['id'], ...queryVisualizationContextOverrides, }} > @@ -79,7 +90,7 @@ function setUp( return { component, user, sendToCavatica } } -describe('Export to CAVATICA Modal', () => { +describe('Send to CAVATICA Confirmation Dialog', () => { beforeEach(() => { jest.clearAllMocks() }) @@ -129,4 +140,37 @@ describe('Export to CAVATICA Modal', () => { mockQueryResultBundle.queryResult.queryResults.headers, ) }) + + it('blocks submission while loading actions required', async () => { + mockUseGetActionsRequiredForTableQuery.mockReturnValue({ + isLoading: true, + data: undefined, + }) + const { user, sendToCavatica } = setUp(undefined, undefined, { + isRowSelectionVisible: false, + selectedRows: [], + }) + + expect(sendToCavatica).toBeDisabled() + }) + it('blocks submission if actions required exist', async () => { + mockUseGetActionsRequiredForTableQuery.mockReturnValue({ + isLoading: false, + data: [ + { + action: { + concreteType: 'org.sagebionetworks.repo.model.download.EnableTwoFa', + accessRequirementId: mockManagedACTAccessRequirement.id, + }, + count: 1, + }, + ], + }) + const { user, sendToCavatica } = setUp(undefined, undefined, { + isRowSelectionVisible: false, + selectedRows: [], + }) + + expect(sendToCavatica).toBeDisabled() + }) })