[Lens] Share link feature

[dej611]

Summary

Fixes #75316

• Lens
  • Refactored Top nav actions code to be more modular
  • Created new Locator object for Lens
    • Enabled server side to make Short URL work with it
    • Added unit tests for it
  • Extended getEditPath to support filters and refreshInterval
  • Extended mounting code in Lens to handle a new type of incoming context
• Add new Share action
  • Added new objectTypeTitle prop to have custom titles on Share popover
  • Replaced the Download CSV action and move it as menu item
    • Refactor code into share item provider + (lazy) panel content
    • Add debug flag to make CSV download testable
    • Add functional tests for CSV download
  • Add Permalink action
    • Integrate Permalink with Short URL service
      • Tweaked Permalink action to work with SO custom URL
      • Tweaked Permalink action to handle disabled state
      • Updated unit tests with new features
      • Added (basic) caching logic to avoid too many snapshot duplicate Short URLs
• New share function test suite created
  • Extended browser service with a new method to have a blank tab in browser
  • New helper functions in Lens to test Share feature

Notes

Short URL requirement

This feature strictly requires the ShortURL service to be enabled to work, otherwise the permalink feature is disabled for snapshot sharing. This requirement is not clearly stated in the Share menu (yet) like other app do as the Sharing flow had to be customised in Lens due to some other technical challenges.
Would it be ok to workout a UI improvement as follow up?

Context tech debt

The way the locator works as injecting the shared state into the context produced a discrete amount of branching, due to inconsistency of the context type coming from different sources (Discover, Agg-based/TSVB, Lens itself...). Perhaps it's worth discussing having a refactoring of the context type here?

Missing locator service

Due to the way the sharing logic works in Lens the locator has not been exported from the plugin functions. I thought to add a custom function for it, but perhaps we could investigate a bit better whether this is needed and eventually its implementation in a follow up task.

How is the snapshot URL generated?

sequenceDiagram
    actor User
    User->>Share Snapshot URL: click
    Share Snapshot URL->> Lens ShortUrlService: Lens state
    Lens ShortUrlService->> Lens ShortUrlService: Check cache based on state
    Lens ShortUrlService->> ShortUrlService: Generate a Short URL
    ShortUrlService->> Lens ShortUrlService: new Short URL
    Lens ShortUrlService->> Application `getUrlForApp`: Build absolute URL
    Application `getUrlForApp`->> Lens ShortUrlService: final URL
    Lens ShortUrlService->>Share Snapshot URL: final URL
    Share Snapshot URL->>User: final URL copied in clipboard

Loading

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

[bhavyarm]

LGTM. I looked over the functional test. It covers pretty much everything I would test for share menu.

[stratoula]

I love it!

@dej611 I feel that now we are adding more clicks to download the csv, is it needed the extra menu?

[dej611]

I love it!

@dej611 I feel that now we are adding more clicks to download the csv, is it needed the extra menu?

I see what you mean.
The nested menu comes from the ShareMenu component which triggers that nested layout structure if there is more than 1 action available. Maybe we could propose some changes upstream as a follow up? I'm just worried to get this PR stuck on upstream changes (already made) right now.

[stratoula]

@dej611 I found one bug. I have shared a link and I save the visualization. I see a blank screen

If I reload, the visualization is depicted correctly.

Also if I share a visualization, copy and paste my url to another tab the time is not the same (defaults to 15 min). Shouldn't it carried over too?

Also sharing a Lens visualization with an adhoc dataview doesn't work. It fails (dataview not found). Can we make it work?

[dej611]

@dej611 I found one bug. I have shared a link and I save the visualization. I see a blank screen If I reload, the visualization is depicted correctly.

Can you recall the configuration of the original chart?

Also if I share a visualization, copy and paste my url to another tab the time is not the same (defaults to 15 min). Shouldn't it carried over too?

It should.

Also sharing a Lens visualization with an adhoc dataview doesn't work. It fails (dataview not found). Can we make it work?

It has to work. I might have broken it accidentally, as I've tested it previously. Will fix it, thanks for checking

[stratoula]

Can you recall the configuration of the original chart?

@dej611 I just created a XY chart by dragging and dropping the records, then I shared the url, open in new tab and save it to Library (not dashboard, it works)

[majagrubic]

nit: how about setting shareableUrlForSavedObject to default value of '', and then this whole line can be simplified to: url = this.props.shareableUrlForSavedObject? Or even better:
const url = this.props.shareableUrlForSavedObject ?? (this.props.shareableUrl) || window.location.href)

[dej611]

Not sure I understand exactly what you propose.
Given a default value of '' for shareableUrlForSavedObject, and this line changing to:

const url = this.props.shareableUrlForSavedObject ?? (this.props.shareableUrl) || window.location.href)

Then any snapshot url would depend on shareableUrlForSavedObject, which is not what I need. The shareableUrlForSavedObject should be used only when the forSavedObject flag is on, not every time a snapshot URL is asked.

[majagrubic]

Ah fair, I thought forSavedObject was always set to true, because other lines were collapsed 😬

[dej611]

@stratoula ad hoc dataviews are now working and text-based sources too.
I've added also functional tests for both to detect in case I forget again to commit code 😅

[stratoula]

@dej611 are the tests flaky or there is a bug here?

[dej611]

@stratoula managed to reproduce your bug on save and pushed a fix to it.
The functional tests have been updated and now they should work, just waiting for the text-based ones which are working locally for me but I saw on CI there are some issues.

[dej611]

Since this is the Lens UI, we can remove Lens and simplify to:

Share the visualization

Done ✅

Instead of Can't share as saved object until the lens_visualization has been saved., how about:

To share as a saved object, save the visualization.

❌ This requires a change in the Share plugin and will have a wider impact than just in Lens. :(

Permalink -> Get link

❌ Also this one requires a Share plugin change.

CSV Download -> Export to CSV

Done ✅

[dej611]

@KOTungseth I can prepare a follow up with your suggestions for the Share menu if that's ok for you.

[drewdaemon]

Tested—works great! 🥳

[drewdaemon]

Suggested change

	export const downloadCsvShareProvider = ({
	export const getDownloadCsvShareProvider = ({

[drewdaemon]

Is this a bug fix? Does it relate to the rest of the changes? (Just asking to understand)

[dej611]

This is a bug fix which appears when a visualization started from a shared URL is saved, then the Lens editor would reload to change the URL using the SO id.
I suspect there's a race condition as activeData is available at that point but the dataViews cache has not been populated yet, but the editor is trying to load the Explore in Discover feature, and fails because of the lack of referenced dataView.

For now I've stopped the loading of the feature via this check, but perhaps it would be worth to either stop the editor reloading on SO saving or track down the race condition.

[stratoula]

This works great now Marco! The only thing missing is the timepicker, As long as I can share my Lens conf with the filters and search query I would also like to keep my timepicker selection.

[majagrubic]

shared-ux changes look good

[dej611]

This is a fix for a bug on the functional tests suite.

Sometimes the secondary tab with discover was kept open (didn't understand why...) and the following suites were carried on on that tab, leaving the first one open, and confusing other suite who usually have code like that to switch between two tabs:

const [currentLensTab, otherTabs] = await browser.getAllWindowHandles();

but because the first tab is no longer the current Lens instance, rather a stale dashboard page, then all checks fail.

[stratoula]

This works great! I tested it Safari and Chrome with many scenarios and everything seems to be shared fine! LGTM! Great job Marco!

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 1238515

Failed CI Steps

• x-pack/test/spaces_api_integration/security_and_spaces/config_basic.ts

Test Failures

• [job] [logs] x-pack/test/spaces_api_integration/security_and_spaces/config_basic.ts / spaces api with security copy to spaces user with no access from the default space single-namespace types "after each" hook for "should return 403 when copying to space without conflicts or references"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
lens	887	892	+5

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	599	602	+3
share	56	59	+3
total			+6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
lens	1.3MB	1.3MB	+5.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
lens	30.5KB	33.9KB	+3.4KB
share	50.8KB	51.3KB	+429.0B
total			+3.8KB

Unknown metric groups

API count

id	before	after	diff
lens	695	698	+3
share	115	118	+3
total			+6

async chunk count

id	before	after	diff
lens	14	15	+1

History

• 💔 Build #100942 failed 2cf9fdc
• 💚 Build #100921 succeeded 4df7ab8
• 💚 Build #100778 succeeded 1daa5e3
• 💚 Build #100777 succeeded dd8bb76
• 💔 Build #100668 failed d707b94

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream