[Workspace]Fix error toasts in sample data page #8842

wanglam · 2024-11-11T08:00:56Z

Description

This PR addresses the issue of error toasts appearing on the sample data page when installing or uninstalling sample data. The root cause was that the user did not have permission to update UI settings at the workspace level. The changes in this PR include:

Remove the default index UI setting first if deleting an index pattern in a specific workspace.
Remove index patterns first when uninstalling sample data.
Disable the behavior of updating the default index pattern UI setting when installing or uninstalling sample data inside a workspace.

Screenshot

No UI Changes

Testing the changes

Clone the branch and run yarn osd bootstrap --single-version loose.

Add the following configurations to config/opensearch_dashboards.yml:

savedObjects.permission.enabled: true
workspace.enabled: true
uiSettings:
  overrides:
    'home:useNewHomePage': true

Run yarn start --no-base-path.
Log in with the admin user and create a new workspace named "test1".
Visit the sample data page of the "test1" workspace.
Click the "Add data" and "Remove" buttons on the sample data cards to ensure you can install and uninstall sample data without any issues.
Create a new user with the "admin" backend_role in the "Internal users" page.
Assign the "Read and write" access level to the new user inside the "test1" workspace.
Open an incognito window and log in with the new user.
Visit the sample data page of the "test1" workspace and click the "Add data" button. Verify that no error toasts appear.
Switch back to the admin user tab, visit the same workspace, and set the "flights" sample data as the default index pattern in the index patterns page.
Switch to the new user tab and click the "Remove" button on the "flights" sample data card. Verify that an error toast appears, similar to the provided screenshot.

13. Click the "Remove" buttons on other sample data cards and ensure no error toasts appear. 14. Switch to the admin user tab and set the new user as an "Admin" of the "test1" workspace. 15. Switch to the new user tab and click the "Remove" button on the "flights" sample data card. Verify that no error toasts appear. 16. Test with the workspace disabled (for regression testing): - Remove the newly added configurations. - Visit the home page and click the "Add sample data" button. - Click the "Add data" buttons. Sample data should be installed without error toasts. - Click the "Remove" buttons. Sample data should be uninstalled without error toasts.

Changelog

fix: [Workspace]Fix error toasts in sample data page

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

Signed-off-by: Lin Wang <[email protected]>

codecov · 2024-11-11T08:32:03Z

Codecov Report

Attention: Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.

Project coverage is 60.90%. Comparing base (1c744d6) to head (f537c6f).

Files with missing lines	Patch %	Lines
...me/server/services/sample_data/routes/uninstall.ts	83.33%	1 Missing and 1 partial ⚠️
...ed_objects/workspace_ui_settings_client_wrapper.ts	94.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8842      +/-   ##
==========================================
+ Coverage   60.87%   60.90%   +0.02%     
==========================================
  Files        3802     3802              
  Lines       91074    91105      +31     
  Branches    14376    14384       +8     
==========================================
+ Hits        55445    55487      +42     
+ Misses      32086    32074      -12     
- Partials     3543     3544       +1

Flag	Coverage Δ
Linux_1	`29.03% <94.11%> (+0.01%)`	⬆️
Linux_2	`56.38% <ø> (ø)`
Linux_3	`37.90% <ø> (ø)`
Linux_4	`29.04% <87.50%> (+0.04%)`	⬆️
Windows_1	`29.05% <94.11%> (+0.01%)`	⬆️
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.90% <ø> (ø)`
Windows_4	`29.04% <87.50%> (+0.04%)`	⬆️

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.

This reverts commit b92b333. Signed-off-by: Lin Wang <[email protected]>

Signed-off-by: Lin Wang <[email protected]>

…admin

SuZhou-Joe · 2024-11-28T09:21:07Z

src/plugins/home/public/application/sample_data_client.js

@@ -41,11 +41,16 @@ export async function listSampleDataSets(dataSourceId) {
  return await getServices().http.get(sampleDataUrl, { query });
 }

+const isWorkspaceEnabled = () => {


I am thinking if it is workspaceEnabled or permissionControlEnabled.

SuZhou-Joe · 2024-11-28T09:27:51Z

src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts

@@ -144,6 +148,59 @@ export class WorkspaceUiSettingsClientWrapper {
      return wrapperOptions.client.update(type, id, attributes, options);
    };

+    const deleteUiSettingsWithWorkspace = async (


Suggested change

const deleteUiSettingsWithWorkspace = async (

const deleteWithDefaultIndexPatternCheck = async (

SuZhou-Joe · 2024-11-28T09:34:24Z

src/plugins/home/public/application/sample_data_client.js

 export async function installSampleDataSet(id, sampleDataDefaultIndex, dataSourceId) {
  const query = buildQuery(dataSourceId);
  await getServices().http.post(`${sampleDataUrl}/${id}`, { query });

-  if (getServices().uiSettings.isDefault('defaultIndex')) {
+  if (!isWorkspaceEnabled() && getServices().uiSettings.isDefault('defaultIndex')) {


Do we still need to check the feature flag? I am thinking line 51 will throw permission error if user happens to delete the default index pattern.

can we add a unit test to cover this condition check?

Signed-off-by: Lin Wang <[email protected]>

…admin

Hailong-am · 2024-11-29T07:54:18Z

src/plugins/home/public/application/sample_data_client.js

 export async function installSampleDataSet(id, sampleDataDefaultIndex, dataSourceId) {
  const query = buildQuery(dataSourceId);
  await getServices().http.post(`${sampleDataUrl}/${id}`, { query });

-  if (getServices().uiSettings.isDefault('defaultIndex')) {
+  if (!isWorkspaceEnabled() && getServices().uiSettings.isDefault('defaultIndex')) {


can we add a unit test to cover this condition check?

Hailong-am · 2024-11-29T07:58:52Z

src/plugins/home/public/application/sample_data_client.js

  if (
+    !isWorkspaceEnabled() &&
    !uiSettings.isDefault('defaultIndex') &&
    uiSettings.get('defaultIndex') === sampleDataDefaultIndex


can we add a comment here, workspace_ui_settings_client_wrapper will handle default index pattern reset?

Hailong-am · 2024-11-29T08:20:32Z

src/plugins/workspace/server/saved_objects/workspace_ui_settings_client_wrapper.ts

+      const defaultIndexPatternWorkspaces = workspaceObjects.saved_objects.filter(
+        ({ attributes }) => attributes?.uiSettings?.[DEFAULT_INDEX_PATTERN_UI_SETTINGS_ID] === id
+      );
+      try {
+        await this.getWorkspaceTypeEnabledClient(wrapperOptions.request).bulkUpdate(
+          defaultIndexPatternWorkspaces.map((savedObject) => ({


can we check defaultIndexPatternWorkspaces is not empty first?

also will bulkUpdate an empty list throw errors?

The bulkUpdate won't throw errors for empty list. But you're right, we can add a empty check logic for it.

Signed-off-by: Lin Wang <[email protected]>

Fix error toasts in sample data page

b92b333

Signed-off-by: Lin Wang <[email protected]>

github-actions bot added the distinguished-contributor label Nov 11, 2024

Changeset file for PR opensearch-project#8842 created/updated

f199290

wanglam marked this pull request as ready for review November 11, 2024 09:25

SuZhou-Joe previously approved these changes Nov 12, 2024

View reviewed changes

SuZhou-Joe added backport 2.x workspace labels Nov 12, 2024

wanglam marked this pull request as draft November 21, 2024 06:03

wanglam added 4 commits November 22, 2024 12:09

Revert "Fix error toasts in sample data page"

d76a356

This reverts commit b92b333. Signed-off-by: Lin Wang <[email protected]>

Update default index pattern setting before index pattern deleted

bea08d1

Signed-off-by: Lin Wang <[email protected]>

Remove index pattern first when uninstall sample data inside workspace

7cb72a2

Signed-off-by: Lin Wang <[email protected]>

Set default index pattern when workspace disabled

9119767

Signed-off-by: Lin Wang <[email protected]>

wanglam dismissed SuZhou-Joe’s stale review via 9119767 November 22, 2024 04:10

Merge branch 'main' into fix-default-index-setting-for-non-workspace-…

c156004

…admin

wanglam marked this pull request as ready for review November 22, 2024 06:28

wanglam requested a review from SuZhou-Joe November 22, 2024 10:05

Merge branch 'main' into fix-default-index-setting-for-non-workspace-…

a6e4ea4

…admin

SuZhou-Joe reviewed Nov 28, 2024

View reviewed changes

wanglam added 2 commits November 29, 2024 11:45

Refactor with deleteWithDefaultIndexPatternCheck

fa38231

Signed-off-by: Lin Wang <[email protected]>

Merge branch 'main' into fix-default-index-setting-for-non-workspace-…

80fbe13

…admin

Hailong-am reviewed Nov 29, 2024

View reviewed changes

Address PR comments and add more UTs

f537c6f

Signed-off-by: Lin Wang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workspace]Fix error toasts in sample data page #8842

[Workspace]Fix error toasts in sample data page #8842

wanglam commented Nov 11, 2024 •

edited

Loading

codecov bot commented Nov 11, 2024 •

edited

Loading

SuZhou-Joe Nov 28, 2024

SuZhou-Joe Nov 28, 2024

SuZhou-Joe Nov 28, 2024

Hailong-am Nov 29, 2024

Hailong-am Nov 29, 2024

Hailong-am Nov 29, 2024

Hailong-am Nov 29, 2024 •

edited

Loading

wanglam Dec 2, 2024

	const deleteUiSettingsWithWorkspace = async (
	const deleteWithDefaultIndexPatternCheck = async (

[Workspace]Fix error toasts in sample data page #8842

Are you sure you want to change the base?

[Workspace]Fix error toasts in sample data page #8842

Conversation

wanglam commented Nov 11, 2024 • edited Loading

Description

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Nov 11, 2024 • edited Loading

Codecov Report

SuZhou-Joe Nov 28, 2024

Choose a reason for hiding this comment

SuZhou-Joe Nov 28, 2024

Choose a reason for hiding this comment

SuZhou-Joe Nov 28, 2024

Choose a reason for hiding this comment

Hailong-am Nov 29, 2024

Choose a reason for hiding this comment

Hailong-am Nov 29, 2024

Choose a reason for hiding this comment

Hailong-am Nov 29, 2024

Choose a reason for hiding this comment

Hailong-am Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

wanglam Dec 2, 2024

Choose a reason for hiding this comment

wanglam commented Nov 11, 2024 •

edited

Loading

codecov bot commented Nov 11, 2024 •

edited

Loading

Hailong-am Nov 29, 2024 •

edited

Loading