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

Data Sources component Improvements and bug fixes #1551

Merged
merged 3 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 2 additions & 2 deletions common/types/data_connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
name: string;
database: string;
type: AccelerationIndexType | 'table';
accelerations: Acceleration[];
columns?: TableColumn[];
accelerations: CachedAcceleration[];
columns?: CachedColumn[];
}

export type Role = EuiComboBoxOptionOption;
Expand Down Expand Up @@ -70,7 +70,7 @@
interface AsyncApiDataResponse {
status: string;
schema?: Array<{ name: string; type: string }>;
datarows?: any;

Check warning on line 73 in common/types/data_connections.ts

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type
total?: number;
size?: number;
error?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
]
}
data-test-subj="associatedObjectsTable"
hasActions={true}
items={
Array [
Object {
Expand Down Expand Up @@ -773,7 +774,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
Object {
"box": Object {
"incremental": true,
"placeholder": "database:database_1 database: database_2 accelerations:skipping_index_1",
"placeholder": "accelerations:skipping_index_1",
"schema": Object {
"fields": Object {
"database": Object {
Expand Down Expand Up @@ -815,14 +816,14 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
},
}
}
tableLayout="fixed"
tableLayout="auto"
>
<div>
<EuiSearchBar
box={
Object {
"incremental": true,
"placeholder": "database:database_1 database: database_2 accelerations:skipping_index_1",
"placeholder": "accelerations:skipping_index_1",
"schema": Object {
"fields": Object {
"database": Object {
Expand Down Expand Up @@ -877,7 +878,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
incremental={true}
isInvalid={false}
onSearch={[Function]}
placeholder="database:database_1 database: database_2 accelerations:skipping_index_1"
placeholder="accelerations:skipping_index_1"
query=""
>
<EuiFieldSearch
Expand All @@ -891,7 +892,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
isInvalid={false}
isLoading={false}
onSearch={[Function]}
placeholder="database:database_1 database: database_2 accelerations:skipping_index_1"
placeholder="accelerations:skipping_index_1"
>
<EuiFormControlLayout
compressed={false}
Expand All @@ -913,7 +914,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
className="euiFieldSearch euiFieldSearch--fullWidth"
defaultValue=""
onKeyUp={[Function]}
placeholder="database:database_1 database: database_2 accelerations:skipping_index_1"
placeholder="accelerations:skipping_index_1"
type="search"
/>
</EuiValidatableControl>
Expand Down Expand Up @@ -1253,6 +1254,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
]
}
data-test-subj="associatedObjectsTable"
hasActions={true}
items={
Array [
Object {
Expand Down Expand Up @@ -1345,7 +1347,7 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
},
}
}
tableLayout="fixed"
tableLayout="auto"
>
<div
className="euiBasicTable"
Expand Down Expand Up @@ -1528,10 +1530,10 @@ exports[`AssociatedObjectsTab Component renders correctly with associated object
<EuiTable
id="random_html_id"
responsive={true}
tableLayout="fixed"
tableLayout="auto"
>
<table
className="euiTable euiTable--responsive"
className="euiTable euiTable--responsive euiTable--auto"
id="random_html_id"
tabIndex={-1}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ jest.mock('../../../../framework/catalog_cache/cache_loader', () => ({

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

describe('AccelerationTable Component', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import { AssociatedObjectsDetailsFlyout } from '../manage/associated_objects/ass
import * as plugin from '../../../../plugin';
import { act } from '@testing-library/react';
import { mockAssociatedObjects } from '../../../../../test/datasources';
import { getAccelerationName } from '../manage/accelerations/utils/acceleration_utils';

configure({ adapter: new Adapter() });

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

describe('AssociatedObjectsDetailsFlyout Integration Tests', () => {
Expand All @@ -26,7 +28,9 @@ describe('AssociatedObjectsDetailsFlyout Integration Tests', () => {
});

it('renders acceleration details correctly and triggers flyout on click', () => {
const wrapper = mount(<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} />);
const wrapper = mount(
<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} datasourceName="flint_s3" />
);
expect(wrapper.find('EuiInMemoryTable').at(0).find('EuiLink').length).toBeGreaterThan(0);

wrapper.find('EuiInMemoryTable').at(0).find('EuiLink').first().simulate('click');
Expand All @@ -38,13 +42,16 @@ describe('AssociatedObjectsDetailsFlyout Integration Tests', () => {
...mockTableDetail,
accelerations: [],
columns: [
{ name: 'column1', dataType: 'string' },
{ name: 'column2', dataType: 'number' },
{ fieldName: 'column1', dataType: 'string' },
{ fieldName: 'column2', dataType: 'number' },
],
};

const wrapper = mount(
<AssociatedObjectsDetailsFlyout tableDetail={mockDetailNoAccelerations} />
<AssociatedObjectsDetailsFlyout
tableDetail={mockDetailNoAccelerations}
datasourceName="flint_s3"
/>
);

expect(wrapper.text()).toContain('You have no accelerations');
Expand All @@ -54,24 +61,28 @@ describe('AssociatedObjectsDetailsFlyout Integration Tests', () => {
});

it('renders schema table correctly with column data', () => {
const wrapper = mount(<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} />);
const wrapper = mount(
<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} datasourceName="flint_s3" />
);

expect(wrapper.find('EuiInMemoryTable').at(1).exists()).toBe(true);
expect(wrapper.find('EuiInMemoryTable').at(1).text()).toContain(
mockTableDetail.columns[0].name
mockTableDetail.columns[0].fieldName
);
});

it('triggers details flyout on acceleration link click', async () => {
const wrapper = mount(<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} />);
const wrapper = mount(
<AssociatedObjectsDetailsFlyout tableDetail={mockTableDetail} datasourceName="flint_s3" />
);

await act(async () => {
// Wait a tick for async updates
await new Promise((resolve) => setTimeout(resolve, 0));
wrapper.update();
});

const accName = mockTableDetail.accelerations[0]?.name;
const accName = getAccelerationName(mockTableDetail.accelerations[0], 'flint_s3');
const accLink = wrapper
.find('EuiLink')
.findWhere((node) => node.text() === accName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
jest.mock('../../../../plugin', () => ({
getRenderAccelerationDetailsFlyout: jest.fn(() => jest.fn()),
getRenderAssociatedObjectsDetailsFlyout: jest.fn(() => jest.fn()),
getRenderCreateAccelerationFlyout: jest.fn(() => jest.fn()),
}));

describe('AssociatedObjectsTab Component', () => {
Expand All @@ -37,7 +38,7 @@

beforeAll(() => {
const originalDate = Date;
global.Date = jest.fn(() => new originalDate('2024-03-14T12:00:00Z')) as any;

Check warning on line 41 in public/components/datasources/components/__tests__/associated_objects_tab.test.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type

global.Date.UTC = originalDate.UTC;
global.Date.parse = originalDate.parse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
} from '../../../../../../common/types/data_connections';
import { DirectQueryLoadingStatus } from '../../../../../../common/types/explorer';
import { isCatalogCacheFetching } from '../associated_objects/utils/associated_objects_tab_utils';
import { getRenderAccelerationDetailsFlyout } from '../../../../../plugin';
import {
getRenderAccelerationDetailsFlyout,
getRenderCreateAccelerationFlyout,
} from '../../../../../plugin';
import {
ACC_LOADING_MSG,
ACC_PANEL_DESC,
Expand All @@ -40,7 +43,7 @@

interface AccelerationTableProps {
dataSourceName: string;
cacheLoadingHooks: any;

Check warning on line 46 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
}

export const AccelerationTable = ({
Expand Down Expand Up @@ -77,7 +80,7 @@
setAccelerations(cachedDataSource.accelerations);
setUpdatedTime(cachedDataSource.lastUpdated);
}
}, []);

Check warning on line 83 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 @@ -93,7 +96,7 @@
setIsRefreshing(false);
console.log('Refresh process is failed.');
}
}, [accelerationsLoadStatus]);

Check warning on line 99 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 = () => {
console.log('Initiating refresh...');
Expand All @@ -118,11 +121,9 @@
};

const CreateButton = () => {
// TODO: Create button should call create_acceleration.tsx, which will be brought
// over from dashboards-query-workbench/public/components/acceleration/create/create_accelerations.tsx
return (
<>
<EuiButton onClick={() => console.log('clicked on create accelerations button')} fill>
<EuiButton onClick={() => renderCreateAccelerationFlyout(dataSourceName)} fill>

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

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/accelerations/acceleration_table.tsx#L126

Added line #L126 was not covered by tests
Create acceleration
</EuiButton>
</>
Expand Down Expand Up @@ -204,16 +205,12 @@
name: 'Name',
sortable: true,
render: (indexName: string, acceleration: CachedAcceleration) => {
const displayName = getAccelerationName(indexName, acceleration, dataSourceName);
const displayName = getAccelerationName(acceleration, dataSourceName);
return (
<EuiLink
onClick={() => {
console.log(displayName);
renderAccelerationDetailsFlyout({
index: displayName,
acceleration,
dataSourceName,
});
renderAccelerationDetailsFlyout(displayName, acceleration, dataSourceName);

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

View check run for this annotation

Codecov / codecov/patch

public/components/datasources/components/manage/accelerations/acceleration_table.tsx#L213

Added line #L213 was not covered by tests
}}
>
{displayName}
Expand Down Expand Up @@ -284,7 +281,7 @@
name: 'Actions',
actions: tableActions,
},
] as Array<EuiBasicTableColumn<any>>;

Check warning on line 284 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 All @@ -299,6 +296,7 @@
};

const renderAccelerationDetailsFlyout = getRenderAccelerationDetailsFlyout();
const renderCreateAccelerationFlyout = getRenderCreateAccelerationFlyout();

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
}

export const AccelerationSchemaTab = ({ mappings, indexInfo }: AccelerationSchemaTabProps) => {
console.log('mappings', mappings);
console.log('indexInfo', indexInfo);
sejli marked this conversation as resolved.
Show resolved Hide resolved
const indexName = indexInfo.data[0]?.index;
const indexData = mappings.data[indexName]?.mappings._meta?.indexedColumns;
const indexType = mappings.data[indexName]?.mappings._meta?.kind;
Expand All @@ -33,7 +35,7 @@
field: 'data_type',
name: 'Data type',
},
] as Array<EuiTableFieldDataColumnType<any>>;

Check warning on line 38 in public/components/datasources/components/manage/accelerations/flyout_modules/accelerations_schema_tab.tsx

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type

if (isSkippingIndex) {
columns.push({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@
'Accelerations optimize query performance by indexing external data into OpenSearch.';
export const ACC_LOADING_MSG = 'Loading/Refreshing accelerations...';

export const getAccelerationName = (
indexName: string,
acceleration: CachedAcceleration,
datasource: string
) => {
export const getAccelerationName = (acceleration: CachedAcceleration, datasource: string) => {
return (
indexName || `${datasource}_${acceleration.database}_${acceleration.table}`.replace(/\s+/g, '_')
acceleration.indexName ||
`${datasource}_${acceleration.database}_${acceleration.table}`.replace(/\s+/g, '_')
);
};

Expand Down Expand Up @@ -80,7 +77,7 @@
return 'inputOutput';
};

export const onRefreshButtonClick = (acceleration: any) => {

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

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type
// TODO: send request to refresh
console.log('refreshing', acceleration.name);
};
Expand All @@ -100,7 +97,7 @@
}
};

export const onDeleteButtonClick = (acceleration: any) => {

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

View workflow job for this annotation

GitHub Actions / Lint

Unexpected any. Specify a different type
// TODO: delete acceleration
console.log('deleting', acceleration.name);
};
Loading
Loading