Skip to content

Commit

Permalink
Data sources bug fixes and UI improvements (opensearch-project#1565) (o…
Browse files Browse the repository at this point in the history
…pensearch-project#1566)

* more bug fixes and UI changes



* adding failure states, improving refresh accelerations



---------


(cherry picked from commit 31d506a)

Signed-off-by: Sean Li <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent dc0245f commit a707810
Show file tree
Hide file tree
Showing 16 changed files with 275 additions and 431 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,24 @@ exports[`Data Connection Page test Renders S3 data connection page with data 1`]
</span>
</button>
</div>
<div
class="euiFlexItem euiFlexItem--flexGrowZero"
>
<button
class="euiButton euiButton--primary euiButton--fill"
type="button"
>
<span
class="euiButtonContent euiButton__content"
>
<span
class="euiButton__text"
>
Create acceleration
</span>
</span>
</button>
</div>
</div>
<hr
class="euiHorizontalRule euiHorizontalRule--full euiHorizontalRule--marginLarge"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const accelerationCache = {
status: 'refreshing',
},
],
lastUpdated: 'Thu, 14 Mar 2024 04:05:53 GMT',
lastUpdated: 'Thu, 14 Mar 2024 04:05:53',
status: 'Updated',
};

Expand Down Expand Up @@ -177,9 +177,7 @@ describe('AccelerationTable Component', () => {
});
wrapper!.update();

const expectedLocalizedTime = accelerationCache.lastUpdated
? new Date(accelerationCache.lastUpdated).toLocaleString()
: '';
const expectedLocalizedTime = 'Thu, 14 Mar 2024 04:05:53';

expect(wrapper!.text()).toContain(expectedLocalizedTime);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ jest.mock('react-router-dom', () => ({
}),
}));

jest.mock('../../../../plugin', () => ({
getRenderCreateAccelerationFlyout: jest.fn(() => jest.fn()),
}));

describe('Manage Data Connections Table test', () => {
configure({ adapter: new Adapter() });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ACC_PANEL_DESC,
getAccelerationName,
AccelerationActionType,
CreateAccelerationFlyoutButton,
} from './utils/acceleration_utils';
import { getRenderAccelerationDetailsFlyout } from '../../../../../plugin';
import { CatalogCacheManager } from '../../../../../framework/catalog_cache/cache_manager';
Expand Down Expand Up @@ -149,18 +150,6 @@ export const AccelerationTable = ({
);
};

const CreateButton = () => {
return (
<>
<EuiButton onClick={() => renderCreateAccelerationFlyout(dataSourceName)} fill>
Create acceleration
</EuiButton>
</>
);
};

const localUpdatedTime = updatedTime ? new Date(updatedTime).toLocaleString() : '';

const AccelerationTableHeader = () => {
return (
<>
Expand All @@ -174,7 +163,10 @@ export const AccelerationTable = ({
<EuiFlexItem grow={false}>
<EuiFlexGroup direction="rowReverse" alignItems="flexEnd">
<EuiFlexItem grow={false}>
<CreateButton />
<CreateAccelerationFlyoutButton
dataSourceName={dataSourceName}
renderCreateAccelerationFlyout={renderCreateAccelerationFlyout}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<RefreshButton data-test-subj="refreshButton" />
Expand All @@ -185,7 +177,7 @@ export const AccelerationTable = ({
{'Last updated'}
</EuiText>
<EuiText textAlign="right" color="subdued" size="xs">
{localUpdatedTime}
{updatedTime}
</EuiText>
</EuiFlexItem>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ Array [
<span
class="euiButtonEmpty__text"
>
Close
Cancel
</span>
</span>
</button>
Expand Down Expand Up @@ -2791,7 +2791,7 @@ Array [
<span
className="euiButtonEmpty__text"
>
Close
Cancel
</span>
</span>
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export const CreateAcceleration = ({
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiButtonEmpty iconType="cross" onClick={resetFlyout} flush="left">
Close
Cancel
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import React from 'react';
import { EuiHealth } from '@elastic/eui';
import { EuiButton, EuiHealth } from '@elastic/eui';
import { CachedAcceleration } from '../../../../../../../common/types/data_connections';
import {
redirectToExplorerOSIdx,
Expand Down Expand Up @@ -77,6 +77,22 @@ export const generateAccelerationOperationQuery = (
}
};

export const CreateAccelerationFlyoutButton = ({
dataSourceName,
renderCreateAccelerationFlyout,
}: {
dataSourceName: string;
renderCreateAccelerationFlyout: (dataSourceName: string) => void;
}) => {
return (
<>
<EuiButton onClick={() => renderCreateAccelerationFlyout(dataSourceName)} fill>
Create acceleration
</EuiButton>
</>
);
};

export const AccelerationStatus = ({ status }: { status: string }) => {
const label = status;
let color;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,7 @@ export const AssociatedObjectsDetailsFlyout = ({
const name = getAccelerationName(item, datasourceName);
return (
<EuiLink
onClick={() =>
renderAccelerationDetailsFlyout(name, item, datasourceName, handleRefresh)
}
onClick={() => renderAccelerationDetailsFlyout(item, datasourceName, handleRefresh)}
>
{name}
</EuiLink>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ import { AssociatedObjectsTabLoading } from './utils/associated_objects_tab_load
import { AssociatedObjectsRefreshButton } from './utils/associated_objects_refresh_button';
import { CatalogCacheManager } from '../../../../../../public/framework/catalog_cache/cache_manager';
import { AssociatedObjectsTable } from './modules/associated_objects_table';
import { getAccelerationName } from '../accelerations/utils/acceleration_utils';
import {
CreateAccelerationFlyoutButton,
getAccelerationName,
} from '../accelerations/utils/acceleration_utils';
import { getRenderCreateAccelerationFlyout } from '../../../../../../public/plugin';
import { AssociatedObjectsTabFailure } from './utils/associated_objects_tab_failure';

export interface AssociatedObjectsTabProps {
datasource: DatasourceDetails;
Expand All @@ -57,6 +62,8 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
const [cachedAccelerations, setCachedAccelerations] = useState<CachedAcceleration[]>([]);
const [associatedObjects, setAssociatedObjects] = useState<AssociatedObject[]>([]);
const [isFirstTimeLoading, setIsFirstTimeLoading] = useState<boolean>(true);
const [databasesLoadFailed, setDatabasesLoadFailed] = useState<boolean>(false);
const [associatedObjectsLoadFailed, setAssociatedObjectsLoadFailed] = useState<boolean>(false);

const {
databasesLoadStatus,
Expand Down Expand Up @@ -92,7 +99,6 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
const onRefreshButtonClick = () => {
if (!isCatalogCacheFetching(databasesLoadStatus, tablesLoadStatus, accelerationsLoadStatus)) {
startLoadingDatabases(datasource.name);
startLoadingAccelerations(datasource.name);
setIsRefreshing(true);
}
};
Expand Down Expand Up @@ -139,6 +145,12 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
onClick={onRefreshButtonClick}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<CreateAccelerationFlyoutButton
dataSourceName={datasource.name}
renderCreateAccelerationFlyout={renderCreateAccelerationFlyout}
/>
</EuiFlexItem>
</EuiFlexGroup>
);
};
Expand Down Expand Up @@ -166,6 +178,12 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
if (status === DirectQueryLoadingStatus.SUCCESS) {
setCachedDatabases(datasourceCache.databases);
setIsFirstTimeLoading(false);
} else if (
status === DirectQueryLoadingStatus.FAILED ||
status === DirectQueryLoadingStatus.CANCELED
) {
setDatabasesLoadFailed(true);
setIsFirstTimeLoading(false);
}
}, [datasource.name, databasesLoadStatus]);

Expand Down Expand Up @@ -200,7 +218,7 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
setCachedTables(databaseCache.tables);
}
if (
accelerationsCache.status === CachedDataSourceStatus.Empty &&
(accelerationsCache.status === CachedDataSourceStatus.Empty || isRefreshing) &&
!isCatalogCacheFetching(accelerationsLoadStatus)
) {
startLoadingAccelerations(datasource.name);
Expand All @@ -222,9 +240,23 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
);
if (tablesStatus === DirectQueryLoadingStatus.SUCCESS) {
setCachedTables(databaseCache.tables);
} else if (
tablesStatus === DirectQueryLoadingStatus.FAILED ||
tablesStatus === DirectQueryLoadingStatus.CANCELED
) {
setAssociatedObjectsLoadFailed(true);
setIsRefreshing(false);
setIsObjectsLoading(false);
}
if (accelerationsStatus === DirectQueryLoadingStatus.SUCCESS) {
setCachedAccelerations(accelerationsCache.accelerations);
} else if (
accelerationsStatus === DirectQueryLoadingStatus.FAILED ||
accelerationsStatus === DirectQueryLoadingStatus.CANCELED
) {
setAssociatedObjectsLoadFailed(true);
setIsRefreshing(false);
setIsObjectsLoading(false);
}
handleObjectsLoad(databaseCache, accelerationsCache);
}
Expand Down Expand Up @@ -280,13 +312,14 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
database: acceleration.database,
type: ACCELERATION_INDEX_TYPES.find((accelType) => accelType.value === acceleration.type)!
.value,
// Temporary dummy array
accelerations: [],
columns: undefined,
}));
setAssociatedObjects([...tableObjects, ...accelerationObjects]);
}, [selectedDatabase, cachedTables, cachedAccelerations]);

const renderCreateAccelerationFlyout = getRenderCreateAccelerationFlyout();

return (
<>
<EuiSpacer />
Expand All @@ -297,15 +330,17 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
<EuiHorizontalRule />
{isFirstTimeLoading ? (
<AssociatedObjectsTabLoading objectType="databases" warningMessage={false} />
) : databasesLoadFailed ? (
<AssociatedObjectsTabFailure type="databases" />
) : (
<>
{cachedDatabases.length === 0 ? (
<AssociatedObjectsTabEmpty cacheType="databases" />
) : (
<>
<EuiSpacer />
<EuiSpacer size="xs" />
<EuiFlexGroup direction="row">
<EuiFlexItem grow={false}>
<EuiFlexItem grow={false} className="database-selector">
<EuiSelectable
searchable={true}
singleSelection="always"
Expand All @@ -322,8 +357,10 @@ export const AssociatedObjectsTab: React.FC<AssociatedObjectsTabProps> = (props)
</EuiSelectable>
</EuiFlexItem>
<EuiFlexItem>
{isObjectsLoading && isFirstTimeLoading ? (
{isFirstTimeLoading || (isObjectsLoading && !isRefreshing) ? (
<AssociatedObjectsTabLoading objectType="tables" warningMessage={true} />
) : associatedObjectsLoadFailed ? (
<AssociatedObjectsTabFailure type="objects" />
) : (
<>
{cachedTables.length > 0 || cachedAccelerations.length > 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,7 @@ export const AssociatedObjectsTable = (props: AssociatedObjectsTableProps) => {
} else {
const acceleration = cachedAccelerations.find((acc) => acc.indexName === item.id);
if (acceleration) {
renderAccelerationDetailsFlyout(
getAccelerationName(acceleration),
acceleration,
datasourceName
);
renderAccelerationDetailsFlyout(acceleration, datasourceName);
}
}
}}
Expand All @@ -87,13 +83,6 @@ export const AssociatedObjectsTable = (props: AssociatedObjectsTableProps) => {
</EuiLink>
),
},
{
field: 'database',
name: i18n.translate('datasources.associatedObjectsTab.column.database', {
defaultMessage: 'Database',
}),
sortable: true,
},
{
field: 'type',
name: i18n.translate('datasources.associatedObjectsTab.column.type', {
Expand All @@ -119,7 +108,7 @@ export const AssociatedObjectsTable = (props: AssociatedObjectsTableProps) => {
return (
<EuiLink
onClick={() => {
renderAccelerationDetailsFlyout(name, accelerations[0], datasourceName);
renderAccelerationDetailsFlyout(accelerations[0], datasourceName);
}}
>
{name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
import { EuiEmptyPrompt } from '@elastic/eui';
import React from 'react';

export const AssociatedObjectsTabEmpty: React.FC = () => {
interface AssociatedObjectsTabFailureProps {
type: string;
}

export const AssociatedObjectsTabFailure = (props: AssociatedObjectsTabFailureProps) => {
const { type } = props;
return (
<EuiEmptyPrompt
iconType="alert"
title={<h3>Error</h3>}
body={<p>There was an error loading your databases.</p>}
/>
<EuiEmptyPrompt iconType="alert" title={<h3>Error</h3>} body={<p>Error loading {type}</p>} />
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const ASSC_OBJ_TABLE_SUBJ = 'associatedObjectsTable';

export const ASSC_OBJ_TABLE_ACC_COLUMN_NAME = 'accelerations';

export const ASSC_OBJ_TABLE_SEARCH_HINT = 'accelerations:skipping_index_1';
export const ASSC_OBJ_TABLE_SEARCH_HINT = 'Search for objects';

export const ASSC_OBJ_PANEL_TITLE = 'Associated objects';

Expand Down
Loading

0 comments on commit a707810

Please sign in to comment.