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] Handle form values(request payload) if the selected type is available in the authentication registry. #6049

Conversation

xinruiba
Copy link
Member

@xinruiba xinruiba commented Mar 6, 2024

Description

Render credential form for registered from AuthMethod

  • “Create DataSource” and "Submit Change" Button enable / disable

Issues Resolved

Partially fixes #5692, #5838

Screenshot

AuthCredentialFieldSubmit.mov

Testing the changes

Test steps in this video:

  1. Enable MD
  2. Navigate to the DataSource creation page:
    • Ensured that the Registered Auth Option appears under the creation form.
    • Confirmed that the "Create data source" button is initially disabled when some fields are empty.
    • Validated that the "Create data source" button becomes enabled when all fields are filled.
  3. Proceed to the DataSource Edit page:
    • Verified that the "Registered Auth" option appears under the edition form.
    • Ensured that the "Save changes" button is initially disabled when some fields are empty.
    • Confirmed that the "Save changes" button becomes enabled when all fields are filled.

Additionally, E2E tests have been conducted, and the request payload has been verified as correct for registered Auth Types.

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

…taSource creation and dataSource edition

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

codecov bot commented Mar 6, 2024

Codecov Report

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

Project coverage is 67.11%. Comparing base (9f3a689) to head (a93ac4b).

Files Patch % Lines
...omponents/validation/datasource_form_validation.ts 50.00% 1 Missing and 4 partials ⚠️
...components/create_form/create_data_source_form.tsx 75.00% 0 Missing and 2 partials ⚠️
...rce/components/edit_form/edit_data_source_form.tsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6049      +/-   ##
==========================================
+ Coverage   67.09%   67.11%   +0.01%     
==========================================
  Files        3322     3322              
  Lines       64238    64258      +20     
  Branches    10321    10329       +8     
==========================================
+ Hits        43101    43126      +25     
+ Misses      18621    18610      -11     
- Partials     2516     2522       +6     
Flag Coverage Δ
Linux_1 31.63% <ø> (ø)
Linux_2 55.20% <ø> (ø)
Linux_3 44.67% <70.37%> (+<0.01%) ⬆️
Linux_4 35.10% <16.66%> (-0.01%) ⬇️
Windows_1 31.68% <ø> (+0.02%) ⬆️
Windows_2 55.17% <ø> (ø)
Windows_3 44.69% <70.37%> (+0.01%) ⬆️
Windows_4 35.10% <16.66%> (-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 [Token Exchange Unification] Send registered credential field for dataSource creation and edition [Token Exchange Unification] Send registered credentials while dataSource creation and edition Mar 7, 2024
@xinruiba xinruiba changed the title [Token Exchange Unification] Send registered credentials while dataSource creation and edition [MD] Token Exchange Unification: Send registered credentials while dataSource creation and edition Mar 7, 2024
@xinruiba xinruiba changed the title [MD] Token Exchange Unification: Send registered credentials while dataSource creation and edition [MD] Token Exchange Unification: send registered auth credentials during dataSource creation and edition Mar 7, 2024
@xinruiba xinruiba changed the title [MD] Token Exchange Unification: send registered auth credentials during dataSource creation and edition [Multiple Datasource] send registered auth credentials during dataSource creation and edition process Mar 7, 2024
@xinruiba xinruiba changed the title [Multiple Datasource] send registered auth credentials during dataSource creation and edition process [Multiple Datasource] Send registered authentication credentials during the dataSource creation and editing processes Mar 7, 2024
Signed-off-by: Xinrui Bai <[email protected]>
@xinruiba xinruiba added the multiple datasource multiple datasource project label Mar 7, 2024
@Flyingliuhub
Copy link
Member

Flyingliuhub commented Mar 8, 2024

Testing the changes: @xinruiba Can you add detail in this section? Thanks

@xinruiba
Copy link
Member Author

xinruiba commented Mar 8, 2024

Testing the changes: @xinruiba Can you add detail in this section? Thanks

@Flyingliuhub Thanks for the comment, details added.

CHANGELOG.md Outdated
@@ -24,7 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Multiple Datasource] Refactoring create and edit form to use authentication registry ([#6002](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6002))
- [Multiple Datasource] Expose a few properties for customize the appearance of the data source selector component ([#6057](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6057))
- [Multiple Datasource] Create data source menu component able to be mount to nav bar ([#6082](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6082))

- [Multiple Datasource] Send registered authentication credentials during the dataSource creation and editing processes ([#6049](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6049))
Copy link
Member

Choose a reason for hiding this comment

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

I would say something simpler, e.g. "Handle form values(request payload) if the selected type is available in the authentication registry." or something like that. Same for PR title.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comment~
Will using the suggested title.

setTimeout(() => {
updateInputFieldAndBlur(component, descriptionFieldIdentifier, '');
expect(
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

why do we need //@ts-ignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, will remove it.

@bandinib-amzn
Copy link
Member

Please update with rebase.

@xinruiba xinruiba changed the title [Multiple Datasource] Send registered authentication credentials during the dataSource creation and editing processes [Multiple Datasource] Handle form values(request payload) if the selected type is available in the authentication registry. Mar 8, 2024
Signed-off-by: Xinrui Bai <[email protected]>
@Flyingliuhub Flyingliuhub merged commit 6f4d814 into opensearch-project:main Mar 8, 2024
68 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 8, 2024
…cted type is available in the authentication registry. (#6049)

* [Token Exchange Unification] Send registered crendentail field for dataSource creation and dataSource edition

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

* [UT] Add unit test for extractRegisteredAuthTypeCredentials newly defined in untils.ts

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

* [UT] add more test cases for util.ts

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

* [UT] Add test cases for datasource creation form and datasource edition form

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

* [Token Exchange Unification + UT] Button create-data-source and save-changes enable / disable control and unit test cases

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

* Update changelog file

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

* Addressing comments

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

---------

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

# Conflicts:
#	CHANGELOG.md
@xinruiba xinruiba deleted the xinrui_Token_Exchange_DataSourceSubmit branch March 9, 2024 00:53
ZilongX pushed a commit that referenced this pull request Mar 11, 2024
…cted type is available in the authentication registry. (#6049) (#6099)

* [Token Exchange Unification] Send registered crendentail field for dataSource creation and dataSource edition

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

* [UT] Add unit test for extractRegisteredAuthTypeCredentials newly defined in untils.ts

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

* [UT] add more test cases for util.ts

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

* [UT] Add test cases for datasource creation form and datasource edition form

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

* [Token Exchange Unification + UT] Button create-data-source and save-changes enable / disable control and unit test cases

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

* Update changelog file

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

* Addressing comments

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

---------

Signed-off-by: Xinrui Bai <[email protected]>
(cherry picked from commit 6f4d814)
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>
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
6 participants