-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Conversation
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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
Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
There was a problem hiding this 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 🚀
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Canvas Sharable Runtime
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
…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
…ve (elastic#183643) ## Summary Closes elastic#181066 Closes epic elastic/kibana-team#735 Closes elastic#183752 - [x] Based on PR elastic#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 elastic#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
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.