Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Data sources bug fixes and UI improvements #1566

Merged
merged 1 commit into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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 @@
ACC_PANEL_DESC,
getAccelerationName,
AccelerationActionType,
CreateAccelerationFlyoutButton,
} from './utils/acceleration_utils';
import { getRenderAccelerationDetailsFlyout } from '../../../../../plugin';
import { CatalogCacheManager } from '../../../../../framework/catalog_cache/cache_manager';
Expand All @@ -41,7 +42,7 @@

interface AccelerationTableProps {
dataSourceName: string;
cacheLoadingHooks: any;

Check warning on line 45 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type
}

interface ModalState {
Expand Down Expand Up @@ -72,7 +73,7 @@
if (operationSuccess) {
handleRefresh();
}
}, [operationSuccess]);

Check warning on line 76 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has a missing dependency: 'handleRefresh'. Either include it or remove the dependency array

const handleActionClick = (
actionType: ModalState['actionType'],
Expand Down Expand Up @@ -112,7 +113,7 @@
setAccelerations(cachedDataSource.accelerations);
setUpdatedTime(cachedDataSource.lastUpdated);
}
}, []);

Check warning on line 116 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has missing dependencies: 'accelerationsLoadStatus', 'dataSourceName', and 'startLoadingAccelerations'. Either include them or remove the dependency array

useEffect(() => {
if (accelerationsLoadStatus === DirectQueryLoadingStatus.SUCCESS) {
Expand All @@ -126,14 +127,14 @@
if (accelerationsLoadStatus === DirectQueryLoadingStatus.FAILED) {
setIsRefreshing(false);
}
}, [accelerationsLoadStatus]);

Check warning on line 130 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has a missing dependency: 'dataSourceName'. Either include it or remove the dependency array

const handleRefresh = useCallback(() => {
if (!isCatalogCacheFetching(accelerationsLoadStatus)) {
setIsRefreshing(true);
startLoadingAccelerations(dataSourceName);
}
}, [accelerationsLoadStatus]);

Check warning on line 137 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useCallback has missing dependencies: 'dataSourceName' and 'startLoadingAccelerations'. Either include them or remove the dependency array

const RefreshButton = () => {
return (
Expand All @@ -149,18 +150,6 @@
);
};

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 @@
<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 @@
{'Last updated'}
</EuiText>
<EuiText textAlign="right" color="subdued" size="xs">
{localUpdatedTime}
{updatedTime}
</EuiText>
</EuiFlexItem>
)}
Expand Down Expand Up @@ -320,7 +312,7 @@
name: 'Actions',
actions: tableActions,
},
] as Array<EuiBasicTableColumn<any>>;

Check warning on line 315 in public/components/datasources/components/manage/accelerations/acceleration_table.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type

const pagination = {
initialPageSize: 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ Array [
<span
class="euiButtonEmpty__text"
>
Close
Cancel
</span>
</span>
</button>
Expand Down Expand Up @@ -2790,7 +2790,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 @@ -138,7 +138,7 @@
accelerationFormData.dataTable
);
}
}, [databaseName, tableName]);

Check warning on line 141 in public/components/datasources/components/manage/accelerations/create_accelerations_flyout/create/create_acceleration.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has missing dependencies: 'accelerationFormData.dataSource', 'accelerationFormData.dataTable', 'accelerationFormData.database', and 'initiateColumnLoad'. Either include them or remove the dependency array

useEffect(() => {
const status = loadStatus.toLowerCase();
Expand All @@ -156,7 +156,7 @@
) {
setTableFieldsLoading(false);
}
}, [loadStatus]);

Check warning on line 159 in public/components/datasources/components/manage/accelerations/create_accelerations_flyout/create/create_acceleration.tsx

View workflow job for this annotation

GitHub Actions / Lint

React Hook useEffect has missing dependencies: 'accelerationFormData.dataSource', 'accelerationFormData.dataTable', 'accelerationFormData.database', and 'loadColumnsToAccelerationForm'. Either include them or remove the dependency array

const dataSourcesPreselected = databaseName !== undefined && tableName !== undefined;

Expand Down Expand Up @@ -213,7 +213,7 @@
<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 CreateAccelerationFlyoutButton = ({
dataSourceName,
renderCreateAccelerationFlyout,
}: {
dataSourceName: string;
renderCreateAccelerationFlyout: (dataSourceName: string) => void;
}) => {
return (
<>
<EuiButton onClick={() => renderCreateAccelerationFlyout(dataSourceName)} fill>

Check warning on line 89 in public/components/datasources/components/manage/accelerations/utils/acceleration_utils.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/accelerations/utils/acceleration_utils.tsx#L89

Added line #L89 was not covered by tests
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 @@ -64,7 +64,7 @@
}: AssociatedObjectsFlyoutProps) => {
const { loadStatus, startLoading } = useLoadTableColumnsToCache();
const [tableColumns, setTableColumns] = useState<CachedColumn[] | undefined>([]);
const [schemaData, setSchemaData] = useState<any>([]);

Check warning on line 67 in public/components/datasources/components/manage/associated_objects/associated_objects_details_flyout.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type

const DiscoverButton = () => {
// TODO: display button if can be sent to discover
Expand Down Expand Up @@ -98,7 +98,7 @@
);
};

const DetailComponent = (detailProps: { title: string; description: any }) => {

Check warning on line 101 in public/components/datasources/components/manage/associated_objects/associated_objects_details_flyout.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type
const { title, description } = detailProps;
return (
<EuiFlexItem>
Expand Down Expand Up @@ -146,9 +146,7 @@
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 { 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 @@
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 @@
const onRefreshButtonClick = () => {
if (!isCatalogCacheFetching(databasesLoadStatus, tablesLoadStatus, accelerationsLoadStatus)) {
startLoadingDatabases(datasource.name);
startLoadingAccelerations(datasource.name);
setIsRefreshing(true);
}
};
Expand Down Expand Up @@ -139,6 +145,12 @@
onClick={onRefreshButtonClick}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<CreateAccelerationFlyoutButton
dataSourceName={datasource.name}
renderCreateAccelerationFlyout={renderCreateAccelerationFlyout}
/>
</EuiFlexItem>
</EuiFlexGroup>
);
};
Expand Down Expand Up @@ -166,6 +178,12 @@
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 @@
setCachedTables(databaseCache.tables);
}
if (
accelerationsCache.status === CachedDataSourceStatus.Empty &&
(accelerationsCache.status === CachedDataSourceStatus.Empty || isRefreshing) &&
!isCatalogCacheFetching(accelerationsLoadStatus)
) {
startLoadingAccelerations(datasource.name);
Expand All @@ -222,9 +240,23 @@
);
if (tablesStatus === DirectQueryLoadingStatus.SUCCESS) {
setCachedTables(databaseCache.tables);
} else if (
tablesStatus === DirectQueryLoadingStatus.FAILED ||
tablesStatus === DirectQueryLoadingStatus.CANCELED
) {
setAssociatedObjectsLoadFailed(true);
setIsRefreshing(false);
setIsObjectsLoading(false);

Check warning on line 249 in public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx#L247-L249

Added lines #L247 - L249 were not covered by tests
}
if (accelerationsStatus === DirectQueryLoadingStatus.SUCCESS) {
setCachedAccelerations(accelerationsCache.accelerations);
} else if (
accelerationsStatus === DirectQueryLoadingStatus.FAILED ||
accelerationsStatus === DirectQueryLoadingStatus.CANCELED
) {
setAssociatedObjectsLoadFailed(true);
setIsRefreshing(false);
setIsObjectsLoading(false);

Check warning on line 259 in public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx#L257-L259

Added lines #L257 - L259 were not covered by tests
}
handleObjectsLoad(databaseCache, accelerationsCache);
}
Expand Down Expand Up @@ -280,13 +312,14 @@
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 @@
<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 @@
</EuiSelectable>
</EuiFlexItem>
<EuiFlexItem>
{isObjectsLoading && isFirstTimeLoading ? (
{isFirstTimeLoading || (isObjectsLoading && !isRefreshing) ? (
<AssociatedObjectsTabLoading objectType="tables" warningMessage={true} />
) : associatedObjectsLoadFailed ? (
<AssociatedObjectsTabFailure type="objects" />

Check warning on line 363 in public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/associated_objects/associated_objects_tab.tsx#L363

Added line #L363 was not covered by tests
) : (
<>
{cachedTables.length > 0 || cachedAccelerations.length > 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,7 @@
} else {
const acceleration = cachedAccelerations.find((acc) => acc.indexName === item.id);
if (acceleration) {
renderAccelerationDetailsFlyout(
getAccelerationName(acceleration),
acceleration,
datasourceName
);
renderAccelerationDetailsFlyout(acceleration, datasourceName);

Check warning on line 77 in public/components/datasources/components/manage/associated_objects/modules/associated_objects_table.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/associated_objects/modules/associated_objects_table.tsx#L77

Added line #L77 was not covered by tests
}
}
}}
Expand All @@ -87,13 +83,6 @@
</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 @@
return (
<EuiLink
onClick={() => {
renderAccelerationDetailsFlyout(name, accelerations[0], datasourceName);
renderAccelerationDetailsFlyout(accelerations[0], datasourceName);

Check warning on line 111 in public/components/datasources/components/manage/associated_objects/modules/associated_objects_table.tsx

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/associated_objects/modules/associated_objects_table.tsx#L111

Added line #L111 was not covered by tests
}}
>
{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
Loading