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

Refactor association table in create page #4

Draft
wants to merge 3 commits into
base: 2.17/support-DQC
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export interface AssociationDataSourceModalProps {
savedObjects: SavedObjectsStart;
assignedConnections: DataSourceConnection[];
closeModal: () => void;
handleAssignDataSourceConnections: (connections: DataSourceConnection[]) => Promise<void>;
handleAssignDataSourceConnections: (connections: DataSourceConnection[]) => Promise<void> | void;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my perspective, can remove Promise<void>?

Copy link
Author

@Kapian1234 Kapian1234 Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const handleAssignDataSourceConnections = async ( in detail page may need this

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try to remove it. The type should work fine. Because we doesn't await handleAssignDataSourceConnections inside modal component.

}

export const AssociationDataSourceModal = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,31 @@ import S3Logo from '../../assets/s3_logo.svg';

interface OpenSearchConnectionTableProps {
isDashboardAdmin: boolean;
connectionType: string;
connectionType?: string;
dataSourceConnections: DataSourceConnection[];
handleUnassignDataSources: (dataSources: DataSourceConnection[]) => Promise<void>;
inCreatePage?: boolean;
handleUnassignDataSources: (dataSources: DataSourceConnection[]) => Promise<void> | void;
getSelectedItems?: (dataSources: DataSourceConnection[]) => void;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer rename to onSelectedItemsChange

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}

export const OpenSearchConnectionTable = ({
isDashboardAdmin,
connectionType,
dataSourceConnections,
handleUnassignDataSources,
getSelectedItems,
inCreatePage = false,
}: OpenSearchConnectionTableProps) => {
const [selectedItems, setSelectedItems] = useState<DataSourceConnection[]>([]);
const [modalVisible, setModalVisible] = useState(false);
const [popoversState, setPopoversState] = useState<Record<string, boolean>>({});

useEffect(() => {
if (inCreatePage && getSelectedItems) {
getSelectedItems(selectedItems);
}
}, [selectedItems, getSelectedItems, inCreatePage]);

useEffect(() => {
// Reset selected items when connectionType changes
setSelectedItems([]);
Expand Down Expand Up @@ -127,7 +137,7 @@ export const OpenSearchConnectionTable = ({
{
field: 'name',
name: i18n.translate('workspace.detail.dataSources.table.title', {
defaultMessage: 'Title',
defaultMessage: 'Data source',
}),
truncateText: true,
render: (name: string, record) => {
Expand Down Expand Up @@ -250,19 +260,31 @@ export const OpenSearchConnectionTable = ({

return (
<>
<EuiInMemoryTable
items={filteredDataSources}
itemId="id"
columns={columns}
selection={selection}
search={search}
key={connectionType}
isSelectable={true}
pagination={{
initialPageSize: 10,
pageSizeOptions: [10, 20, 30],
}}
/>
{inCreatePage ? (
<EuiInMemoryTable
items={filteredDataSources}
itemId="id"
columns={columns}
selection={selection}
key={connectionType}
isSelectable={true}
/>
) : (
<EuiInMemoryTable
items={filteredDataSources}
itemId="id"
columns={columns}
selection={selection}
search={search}
key={connectionType}
isSelectable={true}
pagination={{
initialPageSize: 10,
pageSizeOptions: [10, 20, 30],
}}
/>
)}

<EuiSpacer />
{modalVisible && (
<EuiConfirmModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useCallback, useEffect, useMemo, useState } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import {
EuiText,
EuiTitle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,149 +3,131 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useCallback, useEffect, useState } from 'react';
import React, { useState } from 'react';
import {
EuiSmallButton,
EuiCompressedFormRow,
EuiSpacer,
EuiFlexGroup,
EuiFlexItem,
EuiButtonIcon,
EuiCompressedComboBox,
EuiComboBoxOptionOption,
EuiFormLabel,
EuiText,
EuiFlexItem,
EuiSmallButton,
EuiFlexGroup,
} from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { SavedObjectsStart } from '../../../../../core/public';
import { getDataSourcesList } from '../../utils';
import { DataSource } from '../../../common/types';
import { SavedObjectsStart, CoreStart } from '../../../../../core/public';
import { DataSourceConnection } from '../../../common/types';
import { WorkspaceFormError } from './types';
import { AssociationDataSourceModal } from '../workspace_detail/association_data_source_modal';
import { OpenSearchConnectionTable } from '../workspace_detail/opensearch_connections_table';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { WorkspaceClient } from '../../workspace_client';

export interface SelectDataSourcePanelProps {
errors?: { [key: number]: WorkspaceFormError };
savedObjects: SavedObjectsStart;
selectedDataSources: DataSource[];
onChange: (value: DataSource[]) => void;
assignedDataSources: DataSourceConnection[];
onChange: (value: DataSourceConnection[]) => void;
isDashboardAdmin: boolean;
}

export const SelectDataSourcePanel = ({
errors,
onChange,
selectedDataSources,
assignedDataSources,
savedObjects,
isDashboardAdmin,
}: SelectDataSourcePanelProps) => {
const [dataSourcesOptions, setDataSourcesOptions] = useState<EuiComboBoxOptionOption[]>([]);
useEffect(() => {
if (!savedObjects) return;
getDataSourcesList(savedObjects.client, ['*']).then((result) => {
const options = result.map(({ title, id }) => ({
label: title,
value: id,
}));
setDataSourcesOptions(options);
});
}, [savedObjects, setDataSourcesOptions]);
const handleAddNewOne = useCallback(() => {
onChange?.([
...selectedDataSources,
{
title: '',
id: '',
},
]);
}, [onChange, selectedDataSources]);
const [modalVisible, setModalVisible] = useState(false);
const [selectedItems, setSelectedItems] = useState<DataSourceConnection[]>([]);
const {
services: { notifications, http },
} = useOpenSearchDashboards<{ CoreStart: CoreStart; workspaceClient: WorkspaceClient }>();

const handleSelect = useCallback(
(selectedOptions, index) => {
const newOption = selectedOptions[0]
? // Select new data source
{
title: selectedOptions[0].label,
id: selectedOptions[0].value,
}
: // Click reset button
{
title: '',
id: '',
};
const newSelectedOptions = [...selectedDataSources];
newSelectedOptions.splice(index, 1, newOption);
const handleAssignDataSources = (dataSources: DataSourceConnection[]) => {
setModalVisible(false);
const savedDataSources: DataSourceConnection[] = [...assignedDataSources, ...dataSources];
onChange(savedDataSources);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By this onChange, the selected DQC won't be displayed. How do we support this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the table already supported it by getting relatedConnections from datasources

};

onChange(newSelectedOptions);
},
[onChange, selectedDataSources]
);
const handleUnassignDataSources = (dataSources: DataSourceConnection[]) => {
const savedDataSources = (assignedDataSources ?? [])?.filter(
({ id }: DataSourceConnection) => !dataSources.some((item) => item.id === id)
);
onChange(savedDataSources);
};

const renderTableContent = () => {
return (
<OpenSearchConnectionTable
isDashboardAdmin={isDashboardAdmin}
dataSourceConnections={assignedDataSources}
handleUnassignDataSources={handleUnassignDataSources}
getSelectedItems={getSelectedItems}
inCreatePage={true}
connectionType="openSearchConnections"
/>
);
};

const handleDelete = useCallback(
(index) => {
const newSelectedOptions = [...selectedDataSources];
newSelectedOptions.splice(index, 1);
const associationButton = (
<EuiSmallButton
iconType="plusInCircle"
onClick={() => setModalVisible(true)}
data-test-subj="workspace-creator-dataSources-assign-button"
>
{i18n.translate('workspace.form.selectDataSourcePanel.addNew', {
defaultMessage: 'Add data sources',
})}
</EuiSmallButton>
);

onChange(newSelectedOptions);
},
[onChange, selectedDataSources]
const removeButton = (
<EuiSmallButton
iconType="unlink"
color="danger"
onClick={() => {
handleUnassignDataSources(selectedItems);
}}
data-test-subj="workspace-creator-dataSources-assign-button"
>
{i18n.translate('workspace.form.selectDataSourcePanel.remove', {
defaultMessage: 'Remove selected',
})}
</EuiSmallButton>
);

const getSelectedItems = (currentSelectedItems: DataSourceConnection[]) =>
setSelectedItems(currentSelectedItems);

return (
<div>
<EuiFormLabel>
{i18n.translate('workspace.form.selectDataSource.subTitle', {
defaultMessage: 'Data source',
})}
<EuiText size="xs">
{i18n.translate('workspace.form.selectDataSource.subTitle', {
defaultMessage: 'Add data sources that will be available in the workspace',
})}
</EuiText>
</EuiFormLabel>
<EuiSpacer size="s" />
{selectedDataSources.map(({ id, title }, index) => (
<EuiCompressedFormRow
key={index}
isInvalid={!!errors?.[index]}
error={errors?.[index]?.message}
fullWidth
>
<EuiFlexGroup alignItems="flexEnd" gutterSize="m">
<EuiFlexItem style={{ maxWidth: 400 }}>
<EuiCompressedComboBox
data-test-subj="workspaceForm-select-dataSource-comboBox"
singleSelection
options={dataSourcesOptions}
selectedOptions={
id
? [
{
label: title,
value: id,
},
]
: []
}
onChange={(selectedOptions) => handleSelect(selectedOptions, index)}
placeholder="Select"
/>
</EuiFlexItem>
<EuiFlexItem style={{ maxWidth: 332 }}>
<EuiButtonIcon
color="danger"
aria-label="Delete data source"
iconType="trash"
display="empty"
size="m"
onClick={() => handleDelete(index)}
isDisabled={false}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiCompressedFormRow>
))}

<EuiSmallButton
fill
fullWidth={false}
onClick={handleAddNewOne}
data-test-subj={`workspaceForm-select-dataSource-addNew`}
>
{i18n.translate('workspace.form.selectDataSourcePanel.addNew', {
defaultMessage: 'Add New',
})}
</EuiSmallButton>
<EuiSpacer size="m" />
<EuiFlexGroup alignItems="center">
{isDashboardAdmin && selectedItems.length > 0 && assignedDataSources.length > 0 && (
<EuiFlexItem grow={false}>{removeButton}</EuiFlexItem>
)}
{isDashboardAdmin && <EuiFlexItem grow={false}>{associationButton}</EuiFlexItem>}
</EuiFlexGroup>
<EuiSpacer size="xs" />
<EuiFlexItem style={{ maxWidth: 800 }}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the maxWidth was 768 in design.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

{assignedDataSources.length > 0 && renderTableContent()}
</EuiFlexItem>
{modalVisible && (
<AssociationDataSourceModal
savedObjects={savedObjects}
assignedConnections={assignedDataSources}
closeModal={() => setModalVisible(false)}
handleAssignDataSourceConnections={handleAssignDataSources}
http={http}
notifications={notifications}
/>
)}
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import type { ApplicationStart, SavedObjectsStart } from '../../../../../core/public';
import type { WorkspacePermissionMode } from '../../../common/constants';
import type { DetailTab, WorkspaceOperationType, WorkspacePermissionItemType } from './constants';
import { DataSource } from '../../../common/types';
import { DataSourceConnection } from '../../../common/types';
import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public';
import { WorkspaceUseCase } from '../../types';

Expand Down Expand Up @@ -34,7 +34,7 @@ export interface WorkspaceFormSubmitData {
features?: string[];
color?: string;
permissionSettings?: WorkspacePermissionSetting[];
selectedDataSources?: DataSource[];
selectedDataSources?: DataSourceConnection[];
}

export interface WorkspaceFormData extends WorkspaceFormSubmitData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
getUseCaseFeatureConfig,
isUseCaseFeatureConfig,
} from '../../utils';
import { DataSource } from '../../../common/types';
import { DataSourceConnection } from '../../../common/types';
import { WorkspaceFormProps, WorkspaceFormErrors, WorkspacePermissionSetting } from './types';
import {
appendDefaultFeatureIds,
Expand Down Expand Up @@ -52,7 +52,7 @@ export const useWorkspaceForm = ({
Array<Pick<WorkspacePermissionSetting, 'id'> & Partial<WorkspacePermissionSetting>>
>(initialPermissionSettingsRef.current);

const [selectedDataSources, setSelectedDataSources] = useState<DataSource[]>(
const [selectedDataSources, setSelectedDataSources] = useState<DataSourceConnection[]>(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep using DataSource[] as data type here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't find related changes in that branch, can you paste me the code line by URL?

Copy link
Author

@Kapian1234 Kapian1234 Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies, I made a mistake.
WorkspaceForm would get selectedDataSources and setSelectedDataSources, then pass them to SelectDataSourcePanel. SelectDataSourcePanel would need them
https://github.com/Kapian1234/OpenSearch-Dashboards/blob/f22c7ac958ec5835c44a45e3447bd32d6b29b45b/src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx#L28

defaultValues?.selectedDataSources && defaultValues.selectedDataSources.length > 0
? defaultValues.selectedDataSources
: []
Expand Down
Loading
Loading