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

[Workspace][Bug] Check if workspaces exists when creating saved objects #8739

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

yubonluo
Copy link
Contributor

@yubonluo yubonluo commented Oct 29, 2024

Description

When calling create/bulkCreate/update/bulkUpdate/addToWorkspace save object with API, it needs to check if the passed workspaces exist.

Issues Resolved

Screenshot

Testing the changes

Create a saved object

  • option 1
curl -X POST -H "osd-xsrf: true" -H "Content-Type: application/json" \
-d '{
  "attributes": {
    "title": "Revenue Dashboard",
    "description": "Revenue dashboard",
    "panelsJSON": "[{\"version\":\"2.9.0\",\"gridData\":{\"x\":0,\"y\":0,\"w\":24,\"h\":15,\"i\":\"5db1d75d-f680-4869-a0e8-0f2b8b05b99c\"},\"panelIndex\":\"5db1d75d-f680-4869-a0e8-0f2b8b05b99c\",\"embeddableConfig\":{},\"panelRefName\":\"panel_0\"}]",
    "optionsJSON": "{\"hidePanelTitles\":false,\"useMargins\":true}",
    "version": 1,
    "timeRestore": true,
    "kibanaSavedObjectMeta": {
      "searchSourceJSON": "{\"query\":{\"language\":\"kuery\",\"query\":\"\"},\"filter\":[]}"
    }
  },
  "references": [
    {
      "id": "37cc8650-b882-11e8-a6d9-e546fe2bba5f",
      "name": "panel_0",
      "type": "visualization"
    }
  ]
}' \
"http://localhost:5601/w/non-exist-workspace/api/saved_objects/dashboard" -u 'admin:myStrongPassword123!' --insecure

{"statusCode":400,"error":"Bad Request","message":"Exist invalid workspaces"}%
  • option 2
curl -X POST -H "osd-xsrf: true" -H "Content-Type: application/json" \
-d '{
  "attributes": {
    "title": "Revenue Dashboard",
    "description": "Revenue dashboard",
    "panelsJSON": "[{\"version\":\"2.9.0\",\"gridData\":{\"x\":0,\"y\":0,\"w\":24,\"h\":15,\"i\":\"5db1d75d-f680-4869-a0e8-0f2b8b05b99c\"},\"panelIndex\":\"5db1d75d-f680-4869-a0e8-0f2b8b05b99c\",\"embeddableConfig\":{},\"panelRefName\":\"panel_0\"}]",
    "optionsJSON": "{\"hidePanelTitles\":false,\"useMargins\":true}",
    "version": 1,
    "timeRestore": true,
    "kibanaSavedObjectMeta": {
      "searchSourceJSON": "{\"query\":{\"language\":\"kuery\",\"query\":\"\"},\"filter\":[]}"
    }
  },
"workspaces": ["non-exist-workspace1", "non-exist-workspace2"],
  "references": [
    {
      "id": "37cc8650-b882-11e8-a6d9-e546fe2bba5f",
      "name": "panel_0",
      "type": "visualization"
    }
  ]
}' \
"http://localhost:5601/api/saved_objects/dashboard" -u 'admin:myStrongPassword123!' --insecure
{"statusCode":400,"error":"Bad Request","message":"Exist invalid workspaces"}% 

bulk create saved objects

curl -X POST -H "osd-xsrf: true" -H "Content-Type: application/json" \
-d '[
  {
    "id": "67a9021c-c97e-4499-8150-9722ab44edd4",
    "type": "visualization",
    "attributes": {
      "title": "vega-visualization",
      "fieldFormatMap": "{\"hour_of_day\":{}}",
      "fields": "[{\"name\":\"@timestamp\",\"type\":\"date\",\"esTypes\":[\"date\"],\"count\":0,\"scripted\":false,\"searchable\":true,\"aggregatable\":true,\"readFromDocValues\":true}]"
    },
    "version": "1",
    "migrationVersion": {},
    "references": [
      {
        "id": "ef71d6c1-8e6b-418d-9f7c-e5d9bbde9cf7",
        "type": "data-source",
        "name": "dataSource"
      }
    ],
    "initialNamespaces": [
      "default"
    ]
  }
]' \
"http://localhost:5601/w/non-exist-workspace/api/saved_objects/_bulk_create" -u 'admin:myStrongPassword123!' --insecure

{"statusCode":400,"error":"Bad Request","message":"Exist invalid workspaces"}% 

update saved object

curl -X PUT -H "osd-xsrf: true" -H "Content-Type: application/json" \
-d '{
  "attributes": {}
}' \
"http://localhost:5601/w/non-exist-workspace1/api/saved_objects/dashboard/id" -u 'admin:myStrongPassword123!' --insecure
{"statusCode":400,"error":"Bad Request","message":"Exist invalid workspaces"}%

bulk update saved objects

curl -X PUT -H "osd-xsrf: string" -H "Content-Type: application/json" \
-d '[
    {
      "type": "dashboard",
      "id": "my-dashboard",
      "attributes": {
        "title": "My Updated Dashboard",
        "description": "This is an updated dashboard"
      }
    }
  ]' \
"http://localhost:5601/w/non-exist-workspace1/api/saved_objects/_bulk_update" -u 'admin:myStrongPassword123!' --insecure

{"statusCode":400,"error":"Bad Request","message":"Exist invalid workspaces"}% 

Changelog

  • fix: [Workspace] [Bug] Check if workspaces exists when creating saved objects.

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

@yubonluo yubonluo changed the title [Workspace] [Bug] Check if workspaces exists when creating saved objects [Workspace][Bug] Check if workspaces exists when creating saved objects Oct 29, 2024
@@ -48,25 +49,70 @@ export class WorkspaceIdConsumerWrapper {
return type === UI_SETTINGS_SAVED_OBJECTS_TYPE;
}

private async checkWorkspacesExist(
Copy link
Member

Choose a reason for hiding this comment

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

Nice refactor.

: this.formatWorkspaceIdParams(wrapperOptions.request, options)
),
bulkCreate: <T = unknown>(
create: async <T>(type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => {
Copy link
Member

Choose a reason for hiding this comment

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

Shall we do the check for addToWorkspace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think we also need to check the update and bulk update api?

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be fine for update and bulkUpdate because these 2 APIs won't touch workspaces fields.

SuZhou-Joe
SuZhou-Joe previously approved these changes Oct 29, 2024
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

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

Project coverage is 60.75%. Comparing base (56eeab2) to head (5bcfd2d).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...ver/saved_objects/workspace_id_consumer_wrapper.ts 90.90% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8739      +/-   ##
==========================================
- Coverage   60.75%   60.75%   -0.01%     
==========================================
  Files        3798     3798              
  Lines       90679    90689      +10     
  Branches    14269    14272       +3     
==========================================
+ Hits        55093    55096       +3     
- Misses      32089    32095       +6     
- Partials     3497     3498       +1     
Flag Coverage Δ
Linux_1 29.06% <90.90%> (+<0.01%) ⬆️
Linux_2 56.39% <ø> (ø)
Linux_3 37.59% <ø> (-0.02%) ⬇️
Linux_4 29.82% <ø> (-0.01%) ⬇️
Windows_1 29.08% <90.90%> (+<0.01%) ⬆️
Windows_2 56.34% <ø> (ø)
Windows_3 37.60% <ø> (+<0.01%) ⬆️
Windows_4 29.82% <ø> (-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.

@ananzh
Copy link
Member

ananzh commented Oct 29, 2024

I think we also need to fix the tests. Then I could help to approve.

Signed-off-by: yubonluo <[email protected]>
…yubonluo/OpenSearch-Dashboards into 2.18/fix-create-saved-objects-api-issue
Signed-off-by: yubonluo <[email protected]>
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.

4 participants