[Workspace]Move set default data source before workspace update

[wanglam]

Description

This PR is for fixing dev server crash after un-assign self as a workspace owner. In previous implementation, the set default data source will be called after permission updated. The user won't have write permission to the workspace. So the set default data source operation will be failed and throw exception. Then there will be a unhandled promise rejection. Move set default data source before workspace update to avoid this issue.

Issues Resolved

#7639

Screenshot

No UI Changes.

Testing the changes

• Clone branch and run yarn osd bootstrap
• Add below configs in config/opensearch_dashboards.yml

workspace.enabled: true
savedObjects.permission.enabled: true
data_source.enabled: true
uiSettings:
  overrides:
    'home:useNewHomePage': true
opensearchDashboards.dashboardAdmin.users: ['admin']

• Run yarn start --no-base-path
• Login with admin user and create a test user with kibanauser role
• Create a workspace and set test user as a workspace owner
• Login with test user and go to workspace detail page
• Remove test user in Collaborator tab
• Click save button and then it will show success toasts and redirect workspace not found page

Changelog

• fix: [Workspace] Move set default source order to avoid dev server crash

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.68%. Comparing base (a65a8aa) to head (f1658dd).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7636      +/-   ##
==========================================
+ Coverage   63.66%   63.68%   +0.01%     
==========================================
  Files        3633     3633              
  Lines       80065    80107      +42     
  Branches    12685    12691       +6     
==========================================
+ Hits        50973    51013      +40     
- Misses      25986    25987       +1     
- Partials     3106     3107       +1     

Flag	Coverage Δ
Linux_1	30.60% <100.00%> (+0.05%)	⬆️
Linux_2	55.58% <ø> (ø)
Linux_3	40.56% <ø> (-0.01%)	⬇️
Linux_4	31.30% <ø> (+<0.01%)	⬆️
Windows_1	30.62% <100.00%> (+0.05%)	⬆️
Windows_2	55.53% <ø> (ø)
Windows_3	40.57% <ø> (+<0.01%)	⬆️
Windows_4	31.30% <ø> (+<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.

[Hailong-am]

So the set default data source operation will be failed and throw exception. Then there will be a unhandled promise rejection

but we have try catch for whole update operation, not sure why it does not catch it and cause server crash

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/workspace/server/workspace_client.ts#L221-L293

[wanglam]

So the set default data source operation will be failed and throw exception. Then there will be a unhandled promise rejection

but we have try catch for whole update operation, not sure why it does not catch it and cause server crash

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/workspace/server/workspace_client.ts#L221-L293

The checkAndSetDefaultDataSource‎ is an async function but we didn't wait it complete in the workspaceClient.update method. The error occurs after workspaceClient.update method executed complete, so there will be an unhandled promise rejection.

[Hailong-am]

So the set default data source operation will be failed and throw exception. Then there will be a unhandled promise rejection

but we have try catch for whole update operation, not sure why it does not catch it and cause server crash
https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/workspace/server/workspace_client.ts#L221-L293

The checkAndSetDefaultDataSource‎ is an async function but we didn't wait it complete in the workspaceClient.update method. The error occurs after workspaceClient.update method executed complete, so there will be an unhandled promise rejection.

I see, thanks!

[ruanyl]

@wanglam I guess the issue only happens when user self-removed from the workspace and assign/un-assign data source at the same time?

[wanglam]

@wanglam I guess the issue only happens when user self-removed from the workspace and assign/un-assign data source at the same time?

@ruanyl I think it's not related to the assign/un-assign data source, the default data source will always be set to undefined if passed data sources is empty. In this case, I didn't change the data sources of workspace.