Skip to content

Comments

[Lens] Add internal CRUD api routes#223296

Merged
nickofthyme merged 19 commits intoelastic:mainfrom
nickofthyme:new-lens-api
Jun 25, 2025
Merged

[Lens] Add internal CRUD api routes#223296
nickofthyme merged 19 commits intoelastic:mainfrom
nickofthyme:new-lens-api

Conversation

@nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Jun 10, 2025

Summary

This adds basic Lens CRUD api routes using the Content Management system.

Operation URI
Create POST api/lens/visualizations
Get GET api/lens/visualizations/{id}
Search GET api/lens/visualizations?query=test
Update PUT api/lens/visualizations/{id}
Delete DELETE api/lens/visualizations/{id}

Changes to Lens Content Management

The custom update method uses soClient.create under the hood for reasons (i.e. #160116). However, doing this acts as an update or create method with the provided id. I changed this behavior so now any update where the id is not found will return a 404 error.

Closes #221941
Closes #221942 - OpenAPI docs auto generate from route schema

Testing

You can testing this locally in kibana dev console like so...

GET kbn:/api/lens/visualizations/<id>?apiVersion=1

The apiVersion query param is needed to test internal api routes.

Checklist

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
  • Unit or functional tests were updated or added to match the most common scenarios
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.

Copy link
Contributor Author

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

A few thoughts to consider.

Comment on lines 225 to 233
perPage: schema.maybe(
schema.number({
meta: {
description:
'The number of dashboards to display on each page (max 1000). Default is "20".',
},
defaultValue: 20,
min: 1,
max: 1000,
})
),
perPage: schema.number({
meta: {
description:
'The number of dashboards to display on each page (max 1000). Default is "20".',
},
defaultValue: 20,
min: 1,
max: 1000,
}),
Copy link
Contributor Author

@nickofthyme nickofthyme Jun 10, 2025

Choose a reason for hiding this comment

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

Using a defaultValue is enough to treat this as optional, but when also paired with schema.maybe the resulting validated value of perPage will be undefined instead of 20.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly for @nickpeihl

Comment on lines 80 to 87
try {
({ result } = await client.create(data, options));
} catch (e) {
// TODO prevent duplicate titles?

if (e.isBoom && e.output.statusCode === 403) {
return res.forbidden();
}
Copy link
Contributor Author

@nickofthyme nickofthyme Jun 10, 2025

Choose a reason for hiding this comment

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

Do we want to prohibit duplicate titles at the API level? Currently we block this in the UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we today ? (on the UI level ? )

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question.
@nickpeihl do you have an existing logic for that in presentation? I would like consistency on the API side with the UI, hence keep the title validation as well, but let's sync with the entire API ecosystem as I think it has priority over UI.
@ppisljar yes we do. It is not possible to save a visualisation with an existing title.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have that logic to prohibit duplicate titles on the API. We also don't prohibit it in the UI. We first warn users that saving a dashboard will create a duplicate title. But they can click the save button again to create the dashboard.

Screenshot 2025-06-26 at 10 55 03 AM

It is my understanding that Lens also allows creating a duplicative title in the same way.

Screenshot 2025-06-26 at 10 57 48 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I never realized that was the flow. I figured the save was disabled.

@nickofthyme nickofthyme changed the title feat: add internal lens api routes [Lens] Add internal CRUD api routes Jun 10, 2025
@nickofthyme nickofthyme added Feature:Lens backport:version Backport to applied version labels v8.19.0 v9.2.0 v9.1.0 release_note:feature Makes this part of the condensed release notes and removed v9.2.0 labels Jun 10, 2025
@nickofthyme nickofthyme marked this pull request as ready for review June 18, 2025 01:23
@nickofthyme nickofthyme requested review from a team as code owners June 18, 2025 01:23
Copy link
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

nice, LGTM!

@nickofthyme nickofthyme removed the request for review from afharo June 24, 2025 00:56
- fix paths
- update files after move
- move fixtures
- add lens test config
- update CO file with move
@nickofthyme nickofthyme requested a review from a team as a code owner June 24, 2025 01:46
Copy link
Contributor

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

.buildkite/ftr_platform_stateful_configs.yml LGTM

@nickofthyme nickofthyme removed the request for review from a team June 25, 2025 17:28
@nickofthyme nickofthyme enabled auto-merge (squash) June 25, 2025 17:28
@nickofthyme nickofthyme merged commit 17c2556 into elastic:main Jun 25, 2025
10 checks passed
@nickofthyme nickofthyme deleted the new-lens-api branch June 25, 2025 19:01
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/15884860731

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 561 563 +2
Unknown metric groups

API count

id before after diff
lens 660 662 +2

History

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.19:
- [ska] relocate x-pack/test/cases_api_integration (#225056)
- [ska] [xpack] relocate platform tests (#225223)
- Move apps/security and apps/spaces tests under platform

Manual backport

To create the backport manually run:

node scripts/backport --pr 223296

Questions ?

Please refer to the Backport tool documentation

@nickofthyme
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 27, 2025
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.
cc: @nickofthyme

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.
cc: @nickofthyme

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 223296 locally
cc: @nickofthyme

@nickofthyme nickofthyme added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:version Backport to applied version labels labels Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Lens release_note:feature Makes this part of the condensed release notes v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Lens as code] Write OpenAPI docs [Lens as code] Expose public endpoint

10 participants