Skip to content

Commit

Permalink
[Multiple DataSource]Refactor authentication type selection in create…
Browse files Browse the repository at this point in the history
… and edit data source forms (#3693)

* Refactor authentication type selection in create and edit data source forms to use EuiSelect instead of EuiRadioGroup for better user experience

Signed-off-by: Su <[email protected]>
  • Loading branch information
zhongnansu authored Apr 5, 2023
1 parent 6b42669 commit 44e2e34
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Vis Builder] Removed Hard Coded Strings and Used i18n to transalte([#2867](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2867))
- [Console] Replace jQuery.ajax with core.http when calling OSD APIs in console ([#3080](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3080))
- [I18n] Fix Listr type errors and error handlers ([#3629](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3629))
- [Multiple DataSource] Present the authentication type choices in a drop-down ([#3693](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3693))

### 🔩 Tests

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,12 @@ describe('Datasource Management: Create Datasource form', () => {
component.find(testSubjId).last().simulate('blur');
};

const setAuthTypeValue = (
comp: ReactWrapper<any, Readonly<{}>, React.Component<{}, {}, any>>,
value: AuthType
) => {
// @ts-ignore
comp
.find(authTypeIdentifier)
.first()
// @ts-ignore
.prop('onChange')(value);
comp.update();
const setAuthTypeValue = (testSubjId: string, value: string) => {
component.find(testSubjId).last().simulate('change', {
target: {
value,
},
});
};

beforeEach(() => {
Expand Down Expand Up @@ -98,15 +93,15 @@ describe('Datasource Management: Create Datasource form', () => {
/* Change option: No Authentication */
test('should validate when auth type changed & previously submit button clicked', () => {
/* Update Eui Super Select Value to No Auth*/
setAuthTypeValue(component, AuthType.NoAuth);
setAuthTypeValue(authTypeIdentifier, AuthType.NoAuth);
component.update();

/* Click on submit without any user input */
findTestSubject(component, 'createDataSourceButton').simulate('click');

const { authType, username, password } = getFields(component);

expect(authType.prop('idSelected')).toBe(AuthType.NoAuth);
expect(authType.prop('value')).toBe(AuthType.NoAuth);
expect(username.exists()).toBeFalsy(); // username field does not exist when No Auth option is selected
expect(password.exists()).toBeFalsy(); // password field does not exist when No Auth option is selected
});
Expand Down Expand Up @@ -138,7 +133,7 @@ describe('Datasource Management: Create Datasource form', () => {
/* Username & Password */
test('should create data source with username & password when all fields are valid', () => {
/* set form fields */
setAuthTypeValue(component, AuthType.UsernamePasswordType);
setAuthTypeValue(authTypeIdentifier, AuthType.UsernamePasswordType);
changeTextFieldValue(titleIdentifier, 'test');
changeTextFieldValue(descriptionIdentifier, 'test');
changeTextFieldValue(endpointIdentifier, 'https://test.com');
Expand All @@ -155,7 +150,7 @@ describe('Datasource Management: Create Datasource form', () => {
/* No Auth - Username & Password */
test('should create data source with No Auth when all fields are valid', () => {
/* set form fields */
setAuthTypeValue(component, AuthType.NoAuth); // No auth
setAuthTypeValue(authTypeIdentifier, AuthType.NoAuth); // No auth
changeTextFieldValue(titleIdentifier, 'test');
changeTextFieldValue(descriptionIdentifier, 'test');
changeTextFieldValue(endpointIdentifier, 'https://test.com');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
EuiForm,
EuiFormRow,
EuiPageContent,
EuiRadioGroup,
EuiSelect,
EuiSpacer,
EuiText,
} from '@elastic/eui';
Expand Down Expand Up @@ -120,8 +120,8 @@ export class CreateDataSourceForm extends React.Component<
});
};

onChangeAuthType = (value: string) => {
this.setState({ auth: { ...this.state.auth, type: value as AuthType } });
onChangeAuthType = (e: React.ChangeEvent<HTMLSelectElement>) => {
this.setState({ auth: { ...this.state.auth, type: e.target.value as AuthType } });
};

onChangeUsername = (e: { target: { value: any } }) => {
Expand Down Expand Up @@ -555,10 +555,10 @@ export class CreateDataSourceForm extends React.Component<
{/* Credential source */}
<EuiSpacer size="l" />
<EuiFormRow>
<EuiRadioGroup
<EuiSelect
options={credentialSourceOptions}
idSelected={this.state.auth.type}
onChange={(id) => this.onChangeAuthType(id)}
value={this.state.auth.type}
onChange={(e) => this.onChangeAuthType(e)}
name="Credential"
data-test-subj="createDataSourceFormAuthTypeSelect"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const titleFormRowIdentifier = '[data-test-subj="editDataSourceTitleFormRow"]';
const endpointFieldIdentifier = '[data-test-subj="editDatasourceEndpointField"]';
const descriptionFieldIdentifier = 'dataSourceDescription';
const descriptionFormRowIdentifier = '[data-test-subj="editDataSourceDescriptionFormRow"]';
const authTypeRadioIdentifier = '[data-test-subj="editDataSourceSelectAuthType"]';
const authTypeSelectIdentifier = '[data-test-subj="editDataSourceSelectAuthType"]';
const usernameFieldIdentifier = 'datasourceUsername';
const usernameFormRowIdentifier = '[data-test-subj="editDatasourceUsernameFormRow"]';
const passwordFieldIdentifier = '[data-test-subj="updateDataSourceFormPasswordField"]';
Expand All @@ -49,6 +49,14 @@ describe('Datasource Management: Edit Datasource Form', () => {
comp.update();
};

const setAuthTypeValue = (testSubjId: string, value: string) => {
component.find(testSubjId).last().simulate('change', {
target: {
value,
},
});
};

describe('Case 1: With Username & Password', () => {
beforeEach(() => {
component = mount(
Expand Down Expand Up @@ -147,10 +155,7 @@ describe('Datasource Management: Edit Datasource Form', () => {
expect(component.find('UpdatePasswordModal').exists()).toBe(false);
});
test("should hide username & password fields when 'No Authentication' is selected as the credential type", () => {
act(() => {
// @ts-ignore
component.find(authTypeRadioIdentifier).first().prop('onChange')(AuthType.NoAuth);
});
setAuthTypeValue(authTypeSelectIdentifier, AuthType.NoAuth);
component.update();
expect(component.find(usernameFormRowIdentifier).exists()).toBe(false);
expect(component.find(passwordFieldIdentifier).exists()).toBe(false);
Expand Down Expand Up @@ -255,13 +260,7 @@ describe('Datasource Management: Edit Datasource Form', () => {
/* functionality */

test("should show username & password fields when 'Username & Password' is selected as the credential type", () => {
act(() => {
// @ts-ignore
component.find(authTypeRadioIdentifier).first().prop('onChange')(
// @ts-ignore
AuthType.UsernamePasswordType
);
});
setAuthTypeValue(authTypeSelectIdentifier, AuthType.UsernamePasswordType);
component.update();
expect(component.find(usernameFormRowIdentifier).exists()).toBe(true);
expect(component.find(passwordFieldIdentifier).exists()).toBe(true);
Expand All @@ -270,13 +269,7 @@ describe('Datasource Management: Edit Datasource Form', () => {
/* validation - Password */

test('should validate password as required field', () => {
act(() => {
// @ts-ignore
component.find(authTypeRadioIdentifier).first().prop('onChange')(
// @ts-ignore
AuthType.UsernamePasswordType
);
});
setAuthTypeValue(authTypeSelectIdentifier, AuthType.UsernamePasswordType);
component.update();

/* Validate empty username - required */
Expand All @@ -297,7 +290,7 @@ describe('Datasource Management: Edit Datasource Form', () => {
});

/* Save Changes */
test('should update the form with NoAUth on click save changes', async () => {
test('should update the form with NoAuth on click save changes', async () => {
await new Promise((resolve) =>
setTimeout(() => {
updateInputFieldAndBlur(component, descriptionFieldIdentifier, '');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
EuiFormRow,
EuiHorizontalRule,
EuiPanel,
EuiRadioGroup,
EuiSelect,
EuiSpacer,
EuiText,
} from '@elastic/eui';
Expand Down Expand Up @@ -162,8 +162,8 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
});
};

onChangeAuthType = (value: string) => {
this.setState({ auth: { ...this.state.auth, type: value as AuthType } }, () => {
onChangeAuthType = (e: React.ChangeEvent<HTMLSelectElement>) => {
this.setState({ auth: { ...this.state.auth, type: e.target.value as AuthType } }, () => {
this.onChangeFormValues();
});
};
Expand Down Expand Up @@ -738,10 +738,10 @@ export class EditDataSourceForm extends React.Component<EditDataSourceProps, Edi
defaultMessage: 'Credential',
})}
>
<EuiRadioGroup
<EuiSelect
options={credentialSourceOptions}
idSelected={this.state.auth.type}
onChange={(id) => this.onChangeAuthType(id)}
value={this.state.auth.type}
onChange={this.onChangeAuthType}
name="Credential"
data-test-subj="editDataSourceSelectAuthType"
/>
Expand Down
22 changes: 19 additions & 3 deletions src/plugins/data_source_management/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from 'src/core/public';
import { ManagementAppMountParams } from 'src/plugins/management/public';
import { SavedObjectAttributes } from 'src/core/types';
import { i18n } from '@osd/i18n';
import { OpenSearchDashboardsReactContextValue } from '../../opensearch_dashboards_react/public';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand Down Expand Up @@ -57,9 +58,24 @@ export enum AuthType {
}

export const credentialSourceOptions = [
{ id: AuthType.NoAuth, label: 'No authentication' },
{ id: AuthType.UsernamePasswordType, label: 'Username & Password' },
{ id: AuthType.SigV4, label: 'AWS SigV4' },
{
value: AuthType.NoAuth,
text: i18n.translate('dataSourceManagement.credentialSourceOptions.NoAuthentication', {
defaultMessage: 'No authentication',
}),
},
{
value: AuthType.UsernamePasswordType,
text: i18n.translate('dataSourceManagement.credentialSourceOptions.UsernamePassword', {
defaultMessage: 'Username & Password',
}),
},
{
value: AuthType.SigV4,
text: i18n.translate('dataSourceManagement.credentialSourceOptions.AwsSigV4', {
defaultMessage: 'AWS SigV4',
}),
},
];

export interface DataSourceAttributes extends SavedObjectAttributes {
Expand Down

0 comments on commit 44e2e34

Please sign in to comment.