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

[Tests] Share Modal Redesign clean up and tests #180406

Merged
merged 193 commits into from
May 15, 2024
Merged

Conversation

rshen91
Copy link
Contributor

@rshen91 rshen91 commented Apr 9, 2024

Summary

This PR makes the share redesign modal work the primary share context paradigm (excluding Canvas) by removing the share plugin config that had share.new_version.enabled for testing and implementation.
This PR cleans up the FTRs.

Closes #151523
As a result of defaulting to short urls, some tests were removed since they are now obsolete.
One fix in this PR to avoid customer known issues is to allow reporting (if license is permitted) for watcher users. Refer to https://github.com/elastic/sdh-kibana/issues/4481#issuecomment-2012969470.

I've opened a separate issue to track any skipped or deleted tests as a result of short urls by default here #181066

Checklist

Release Note

The share menu is updated for a more streamlined user experience. Users can navigate through a tabbed modal to copy links for discover, dashboard, and lens.

@rshen91 rshen91 added the release_note:feature Makes this part of the condensed release notes label Apr 9, 2024
@rshen91 rshen91 self-assigned this Apr 9, 2024
@rshen91 rshen91 added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Apr 9, 2024
@eokoneyo eokoneyo self-assigned this Apr 9, 2024
@rshen91 rshen91 removed the request for review from a team May 13, 2024 20:30
@rshen91 rshen91 removed the v8.14.0 label May 14, 2024
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Close to the finish line, left some review comments.

x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2024-05-15 at 10 25 51

The share.link.unsaved message feels misleading in combination with the new callout.
I would propose to either change its message - cc @amyjtechwriter :

There are unsaved changes. The generated link is a temporary link.

Or maybe just remove this tooltip altogether as it is redundant with the new callout.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok to improve this as a follow up of this PR

rshen91 and others added 2 commits May 15, 2024 07:57
Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
@rshen91 rshen91 requested a review from dej611 May 15, 2024 13:58
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested latest version and it works ok 👍
Great work @rshen91 🚀

@rshen91 rshen91 enabled auto-merge (squash) May 15, 2024 17:43
@kibana-ci
Copy link
Collaborator

kibana-ci commented May 15, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #57 / dashboard Reporting Dashboard Reporting Screenshots Preserve Layout downloads a PDF file with saved search given EuiDataGrid enabled

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 1473 1471 -2
reporting 120 117 -3
share 94 92 -2
total -7

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
@kbn/reporting-public 104 107 +3
share 77 59 -18
total -15

Async chunks

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

id before after diff
apm 3.3MB 3.3MB +95.0B
dashboard 493.1KB 493.3KB +116.0B
discover 722.4KB 722.5KB +112.0B
lens 1.5MB 1.5MB -1.2KB
reporting 68.6KB 60.9KB -7.7KB
share 3.6KB 4.2KB +581.0B
visualizations 285.6KB 285.8KB +125.0B
total -7.9KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5405 +5405
total size - 8.8MB +8.8MB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
share 11 12 +1

Page load bundle

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

id before after diff
lens 50.0KB 49.3KB -705.0B
reporting 56.0KB 52.8KB -3.2KB
share 71.1KB 55.9KB -15.2KB
total -19.1KB
Unknown metric groups

API count

id before after diff
@kbn/reporting-public 110 113 +3
share 136 120 -16
total -13

async chunk count

id before after diff
lens 25 24 -1
reporting 4 3 -1
total -2

ESLint disabled line counts

id before after diff
@kbn/reporting-public 2 3 +1

References to deprecated APIs

id before after diff
@kbn/reporting-public 0 2 +2
discover 16 17 +1
total +3

Total ESLint disabled count

id before after diff
@kbn/reporting-public 2 3 +1

History

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

cc @eokoneyo @rshen91

@rshen91 rshen91 merged commit dc1fd5a into elastic:main May 15, 2024
26 checks passed
@rshen91 rshen91 deleted the tests-redesign branch May 15, 2024 18:52
rshen91 added a commit that referenced this pull request Jun 12, 2024
…ve (#183643)

## Summary

Closes #181066
Closes epic elastic/kibana-team#735
Closes #183752

- [x] Based on PR #180406, the
test 'generates a report with single timefilter' was skipped in
x-pack/test/functional/apps/discover/reporting.ts because short URLs are
the default and make the test fail.
- [x] In addition test/functional/apps/dashboard/group5/share.ts was
skipped for similar reasons
- [x] x-pack/test/functional/apps/lens/group4/share.ts 'should preserve
filter and query when sharing'
- [x]
[x-pack/test_serverless/functional/test_suites/common/discover/x_pack/reporting.ts](https://github.com/elastic/kibana/pull/180406/files/03f8d94aedd6d76bcaf7cfa4db714c7665269807#diff-f8df59654e26e509d3e2b9fd3b998da7ea0f7b1d02bddced1acbd588d6b55883)
refactoring
- [x]
[x-pack/test/functional/apps/discover/feature_controls/discover_security.ts](https://github.com/elastic/kibana/pull/180406/files/ec0bec8387e8b75de2e57adb34517741052e18c4#diff-1efa1c849e2a1f9e206702cafa82c5d5d7b1a446add207b693f8fbebc992b59d)
- [x] Add lens by-value FTR to confirm share link in lens is passing a
share link that will open to a Lens visualization. For reference check
out
#180406 (review)


### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:feature Makes this part of the condensed release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Unsaved work error on CSV reports for read-only user