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

[Multiple DataSource] DataSource creation and edition page improvement to better support registered auth types #6122

Conversation

xinruiba
Copy link
Member

@xinruiba xinruiba commented Mar 12, 2024

Description

This is the PR to improve state handle for "CreateDataSource" and "EditDataSource" pages. With this change registered auth type with be better supported in editDataSource page by showing initial credential info in the edit page

Issues Resolved

Partially fixes #5692, #5838

Screenshot

CreateDataSourceWithTokenExchange.mov
EditTestingForTokenExchange.mov

Testing the changes

For the first video, we do following steps to test dataSource creation:

  1. Go to dataSource creation page
  2. Fill in auth credentials for registered auth type
  3. Test connection success
  4. Create datasource success

For the second video, we do following steps to test dataSource edition:

  1. Select a previously created dataSource
  2. Update the auth type to registered auth, and fill in credential info
  3. Test connection success
  4. Submit change success

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link

codecov bot commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 72.00000% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 67.18%. Comparing base (3073926) to head (41ca221).

Files Patch % Lines
...rce/components/edit_form/edit_data_source_form.tsx 68.42% 2 Missing and 4 partials ⚠️
...components/create_form/create_data_source_form.tsx 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6122   +/-   ##
=======================================
  Coverage   67.17%   67.18%           
=======================================
  Files        3328     3328           
  Lines       64448    64461   +13     
  Branches    10376    10376           
=======================================
+ Hits        43295    43306   +11     
- Misses      18621    18622    +1     
- Partials     2532     2533    +1     
Flag Coverage Δ
Linux_1 31.74% <ø> (ø)
Linux_2 55.29% <ø> (ø)
Linux_3 44.76% <72.00%> (+0.01%) ⬆️
Linux_4 35.05% <0.00%> (-0.01%) ⬇️
Windows_1 31.77% <ø> (ø)
Windows_2 55.26% <ø> (ø)
Windows_3 44.78% <72.00%> (+0.01%) ⬆️
Windows_4 35.05% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xinruiba xinruiba changed the title [Multiple DataSource] State update for createDataSource and edit DataSource pages [Multiple DataSource] Create DataSource form and Edit DataSource form state update and validation Mar 12, 2024
@xinruiba xinruiba changed the title [Multiple DataSource] Create DataSource form and Edit DataSource form state update and validation [Multiple DataSource] DataSource creation and edition page improvement to better support registered auth types Mar 12, 2024
Signed-off-by: Xinrui Bai <[email protected]>
@xinruiba xinruiba added v2.13.0 multiple datasource multiple datasource project labels Mar 13, 2024
Signed-off-by: Xinrui Bai <[email protected]>
bandinib-amzn
bandinib-amzn previously approved these changes Mar 13, 2024
@@ -380,5 +380,71 @@ describe('DataSourceManagement: Utils.ts', () => {

expect(deepEqual(registedAuthTypeCredentials, expectExtractedAuthCredentials));
});

test('Should inherit value from registered field when credentail state not have registered field', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Typo: credentail. Actually, there are only 8 files referenced by crendentialFormField. Can you please fix crendentialFormField in this PR, and then fix #6122 (comment) (31 files referenced)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, thanks for the comment.
Fixing.

Copy link
Member Author

@xinruiba xinruiba Mar 14, 2024

Choose a reason for hiding this comment

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

crendentialFormField typo resolved, thanks.

@Flyingliuhub Flyingliuhub merged commit 31e8481 into opensearch-project:main Mar 14, 2024
68 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 14, 2024
…t to better support registered auth types (#6122)

* [Token Exchange Unification] State update for createDataSource and editDataSource pages

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange Unification] rectify state for dataSource creation page and edit page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add more test cases for util functions

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange Unification] Update dataSource bottom banner control

Signed-off-by: Xinrui Bai <[email protected]>

* Update changefile.md

Signed-off-by: Xinrui Bai <[email protected]>

* Add comments

Signed-off-by: Xinrui Bai <[email protected]>

* Code review change, fix typo

Signed-off-by: Xinrui Bai <[email protected]>

* Resolve comments, update typo in test cases

Signed-off-by: Xinrui Bai <[email protected]>

---------

Signed-off-by: Xinrui Bai <[email protected]>
(cherry picked from commit 31e8481)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
Flyingliuhub pushed a commit that referenced this pull request Mar 14, 2024
…t to better support registered auth types (#6122) (#6145)

* [Token Exchange Unification] State update for createDataSource and editDataSource pages

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange Unification] rectify state for dataSource creation page and edit page

Signed-off-by: Xinrui Bai <[email protected]>

* [UT] Add more test cases for util functions

Signed-off-by: Xinrui Bai <[email protected]>

* [Token Exchange Unification] Update dataSource bottom banner control

Signed-off-by: Xinrui Bai <[email protected]>

* Update changefile.md

Signed-off-by: Xinrui Bai <[email protected]>

* Add comments

Signed-off-by: Xinrui Bai <[email protected]>

* Code review change, fix typo

Signed-off-by: Xinrui Bai <[email protected]>

* Resolve comments, update typo in test cases

Signed-off-by: Xinrui Bai <[email protected]>

---------

Signed-off-by: Xinrui Bai <[email protected]>
(cherry picked from commit 31e8481)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@xinruiba xinruiba deleted the xinrui_TokenExchangeUnification_FormValidation branch March 18, 2024 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Refactoring data source plugin to support add-on authentication method with plug-in module
5 participants