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.

Sign up for GitHub

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

Jump to bottom

[Console] Fixes Elasticsearch doc VIEW IN CONSOLE will clean local Kibana console form history. #127430

Merged
merged 8 commits into from
Apr 4, 2022
Merged

[Console] Fixes Elasticsearch doc VIEW IN CONSOLE will clean local Kibana console form history. #127430

merged 8 commits into from
Apr 4, 2022

Conversation

hro-maker
Copy link
Contributor

@hro-maker hro-maker commented Mar 10, 2022 •
edited
Loading

Closes: #12053

Summary

issue 1.click any command example's VIEW IN CONSOLE in Elasticsearch doc, such as https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-delete-index.html
the link is http://localhost:5601/app/kibana#/dev_tools/console?load_from=https://www.elastic.co/guide/en/elasticsearch/reference/current/snippets/indices-delete-index/1.json
2.then all my kibana's console history gone
if we have opened some tab
for example:
Screenshot 2022-03-14 at 12 18 18

and then if we open new tab with link similar this one
http://localhost:5620/app/dev_tools#/console?load_from=https://www.elastic.co/guide/en/elasticsearch/reference/current/snippets/2121.console
Screen:

before
Screenshot 2022-03-14 at 12 17 25

now

Screenshot 2022-03-14 at 12 18 30

@Kuznietsov Kuznietsov added Feature:Console Dev Tools Console Feature Feature:Dev Tools Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more backport:skip This commit does not require backporting impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:fix v8.2.0 labels Mar 10, 2022
maksimkovalev
maksimkovalev reviewed Mar 10, 2022
View reviewed changes
src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx Outdated
@@ -105,7 +105,7 @@ function EditorUI({ initialTextValue }: EditorProps) {

const loadBufferFromRemote = (url: string) => {
const coreEditor = editor.getCoreEditor();

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove spaces, because Checks has failed

maksimkovalev
maksimkovalev reviewed Mar 10, 2022
View reviewed changes
src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx Outdated
@@ -121,7 +121,7 @@ function EditorUI({ initialTextValue }: EditorProps) {

// Fire and forget.
$.ajax(loadFrom).done(async (data) => {
await editor.update(data, true);
await editor.update(`${initialTextValue }\n ${data}` , true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, remove empty space after initialTextValue

@alexwizp alexwizp removed loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Mar 14, 2022
@alexwizp
Copy link
Contributor

@hro-maker please make this PR green,
@maksimkovalev could you please validate that PR

@maksimkovalev maksimkovalev self-requested a review March 23, 2022 13:02
maksimkovalev
maksimkovalev approved these changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@maksimkovalev maksimkovalev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM with green CI.
Tested in chrome. Console history is saved when opening new tab.
Screenshot 2022-03-23 at 15 01 17

@hro-maker please remove empty space before comma ${data}' , to fix CI.

@Kuznietsov
Copy link
Contributor

@elasticmachine merge upstream

Kuznietsov
Kuznietsov reviewed Mar 28, 2022
View reviewed changes
Copy link
Contributor

@Kuznietsov Kuznietsov left a comment

Choose a reason for hiding this comment

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

Before merging, please, wait for the feedback from me this time) I have worries about that, autocomplete is going to be broken after those changes. Need to check it) Thanks)

Kuznietsov
Kuznietsov reviewed Mar 28, 2022
View reviewed changes
Copy link
Contributor

@Kuznietsov Kuznietsov left a comment

Choose a reason for hiding this comment

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

Autocomplete is working. Thanks for the changes Waiting for the fixes, which Maxim has requested, and then, Code LGTM 😃

@hro-maker
Copy link
Contributor Author

@elasticmachine merge upstream

@hro-maker hro-maker marked this pull request as ready for review April 1, 2022 07:09
@hro-maker hro-maker requested a review from a team as a code owner April 1, 2022 07:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@hro-maker hro-maker requested a review from mibragimov April 1, 2022 07:12
sabarasaba
sabarasaba approved these changes Apr 1, 2022
View reviewed changes
Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this issue @hro-maker! changes lgtm, tested locally

@hro-maker
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
console 380.1KB 380.1KB +12.0B

History

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

cc @hro-maker

@alexwizp alexwizp added v8.3.0 backport:skip This commit does not require backporting and removed backport:skip This commit does not require backporting v8.2.0 labels Apr 4, 2022
@alexwizp alexwizp merged commit e1c25c9 into elastic:main Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kuznietsov Kuznietsov Kuznietsov left review comments

@sabarasaba sabarasaba sabarasaba approved these changes

@maksimkovalev maksimkovalev maksimkovalev approved these changes

@alexwizp alexwizp alexwizp approved these changes

@mibragimov mibragimov Awaiting requested review from mibragimov

Assignees

@hro-maker hro-maker

Labels
backport:skip This commit does not require backporting Feature:Console Dev Tools Console Feature Feature:Dev Tools release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Elasticsearch doc VIEW IN CONSOLE will clean local Kibana console form history.
8 participants
@hro-maker @alexwizp @Kuznietsov @elasticmachine @kibanamachine @kibana-ci @sabarasaba @maksimkovalev