[Logs UI] Set streamLive false in URL state when arriving from link-to by Zacqary · Pull Request #56329 · elastic/kibana

Zacqary · 2020-01-29T18:46:07Z

Summary

~~Removes the streamLive param from the log position URL state whenever it's false, and only adds it back in when it's true.~~

Forces the streamLive param to false when replacing a link-to search string in the logs stream URL.

This fixes a flaky test.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~[ ] 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
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2020-01-29T18:46:10Z

Pinging @elastic/kibana-test-triage (failed-test)

elasticmachine · 2020-01-29T18:46:11Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

Zacqary · 2020-01-29T22:52:36Z

@elasticmachine merge upstream

afgomez

I'm thinking out loud.

I understand the reason why this test was flaky was because the streamLive parameter in the URL sometimes was false, and sometimes wasn't there, and the test was only covering the false case.

streamLive is a 3-state boolean (true, false, and not-there). In practice false and not-there have the same effect so we can remove one of them.

This PR removes false leaving only two states: true and not-there. I think that makes it a bit harder to reason about it that if it had true and false as valid states.

I haven't dug into the code, but I think that could be made with this change.

-const mapToStreamLiveUrlState = (value: any) => (typeof value === 'boolean' ? value : undefined);	
+const mapToStreamLiveUrlState = (value: any) => (typeof value === 'boolean' ? value : false);

Maybe you tried that already and it didn't work correctly because sometimes it wasn't being called in the right order during initialisation and the test was still flaky, but I think it might be worth a try. Otherwise I can go ahead and approve the PR

weltenwort · 2020-01-30T10:04:32Z

I would agree that we should prefer true/false as the two states in the URL. I wonder why the value passed to mapToStreamLiveUrlState seems to not be a boolean in the first place 🤔 maybe the cleaner fix would be to ensure that it is by making sure the source of that value is false by default and that false always gets passed down.

afgomez · 2020-01-30T10:58:08Z

I would agree that we should prefer true/false as the two states in the URL. I wonder why the value passed to mapToStreamLiveUrlState seems to not be a boolean in the first place

I think that happens on page load, when the URL has no values.

…url-fix

Zacqary · 2020-01-30T17:59:50Z

Just pushed another approach, I think this should actually fix it for real.

afgomez

👍 Perfect!

Zacqary · 2020-02-06T16:54:02Z

@elasticmachine merge upstream

Zacqary · 2020-02-06T18:23:45Z

@elasticmachine merge upstream

elasticmachine · 2020-02-06T19:51:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c2b597d

History

💚 Build #24084 succeeded 2f06bd5
💔 Build #23841 failed f80615e
💔 Build #23820 failed 19c51b0
💔 Build #23601 failed b4d8112
💚 Build #23420 succeeded 9abff9a

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

elastic#56329) * [Logs UI] Remove streamLive false from URL state * Force false livestream boolean from link-to * Update type def * Update snapshots * Fix snapshot typo * Fix snapshot again * Fix node test snapshot Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

#56329) (#57115) * [Logs UI] Remove streamLive false from URL state * Force false livestream boolean from link-to * Update type def * Update snapshots * Fix snapshot typo * Fix snapshot again * Fix node test snapshot Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

#56329) (#57114) * [Logs UI] Remove streamLive false from URL state * Force false livestream boolean from link-to * Update type def * Update snapshots * Fix snapshot typo * Fix snapshot again * Fix node test snapshot Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

…t-state * upstream/master: (96 commits) top nav ts arg support (elastic#56984) [SIEM][detection engine] Limit network rules to filebeat source semantics (elastic#57130) Add docs for alerting and action settings (elastic#57035) Add Test to Verify Endpoint App Landing Page (elastic#57129) Update `markdown-to-jsx` (`6.9.3` → `6.11.0`) and `url-parse` (`1.4.4` → `1.4.7`) dependencies. (elastic#57126) chore(NA): removes use of parallel option in the terser minimizer (elastic#57077) [ML] New Platform server shim: update file data visualizer routes to use new platform router (elastic#56972) Specifying valid licenses for the Graph feature (elastic#55911) [APM][docs] Add troubleshooting for non-indexed fields (elastic#54948) [ML] DF Analytics creation: update schema definition for create route (elastic#56979) Remove Kibana a11y guide in favor of EUI (elastic#57021) [Logs UI] Set streamLive false in URL state when arriving from link-to (elastic#56329) [docs] Fix spaces api example json (elastic#50411) Add new config for filebeat index name (elastic#56920) [Metrics-UI] Fix toolbar popover for metrics table row (elastic#56796) Saved Objects testing (elastic#56965) Disabled categorization stats validation (elastic#57087) [Rollups] Server NP migration (elastic#55606) [Metrics UI] Limit group by selector to only 2 fields (elastic#56800) fix auto closing new vis modal when navigating to lens or when navigating away with browser history (elastic#56998) ...

* master: (96 commits) top nav ts arg support (elastic#56984) [SIEM][detection engine] Limit network rules to filebeat source semantics (elastic#57130) Add docs for alerting and action settings (elastic#57035) Add Test to Verify Endpoint App Landing Page (elastic#57129) Update `markdown-to-jsx` (`6.9.3` → `6.11.0`) and `url-parse` (`1.4.4` → `1.4.7`) dependencies. (elastic#57126) chore(NA): removes use of parallel option in the terser minimizer (elastic#57077) [ML] New Platform server shim: update file data visualizer routes to use new platform router (elastic#56972) Specifying valid licenses for the Graph feature (elastic#55911) [APM][docs] Add troubleshooting for non-indexed fields (elastic#54948) [ML] DF Analytics creation: update schema definition for create route (elastic#56979) Remove Kibana a11y guide in favor of EUI (elastic#57021) [Logs UI] Set streamLive false in URL state when arriving from link-to (elastic#56329) [docs] Fix spaces api example json (elastic#50411) Add new config for filebeat index name (elastic#56920) [Metrics-UI] Fix toolbar popover for metrics table row (elastic#56796) Saved Objects testing (elastic#56965) Disabled categorization stats validation (elastic#57087) [Rollups] Server NP migration (elastic#55606) [Metrics UI] Limit group by selector to only 2 fields (elastic#56800) fix auto closing new vis modal when navigating to lens or when navigating away with browser history (elastic#56998) ...

[Logs UI] Remove streamLive false from URL state

a8a3fcb

Zacqary requested a review from a team January 29, 2020 18:46

Merge branch 'master' into 55652-url-fix

9abff9a

afgomez self-requested a review January 30, 2020 08:28

afgomez reviewed Jan 30, 2020

View reviewed changes

Zacqary added 2 commits January 30, 2020 11:56

Force false livestream boolean from link-to

3b4f32f

Merge branch '55652-url-fix' of github.com:Zacqary/kibana into 55652-…

5139b58

…url-fix

Zacqary changed the title ~~[Logs UI] Remove streamLive false from URL state~~ [Logs UI] Set streamLive false in URL state when arriving from link-to Jan 30, 2020

Zacqary added 6 commits January 30, 2020 12:00

Update type def

b4d8112

Update snapshots

515488f

Fix snapshot typo

19c51b0

Fix snapshot again

f80615e

Fix node test snapshot

ec16be6

Merge remote-tracking branch 'upstream/master' into 55652-url-fix

2f06bd5

afgomez approved these changes Feb 4, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into 55652-url-fix

58b087d

Merge branch 'master' into 55652-url-fix

c2b597d

elastic deleted a comment from kibanamachine Feb 6, 2020

Zacqary merged commit 13944fa into elastic:master Feb 7, 2020

Zacqary mentioned this pull request Feb 7, 2020

[7.x] [Logs UI] Set streamLive false in URL state when arriving from link-to (#56329) #57114

Merged

Zacqary mentioned this pull request Feb 7, 2020

[7.6] [Logs UI] Set streamLive false in URL state when arriving from link-to (#56329) #57115

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Logs UI] Set streamLive false in URL state when arriving from link-to#56329

[Logs UI] Set streamLive false in URL state when arriving from link-to#56329
Zacqary merged 12 commits intoelastic:masterfrom
Zacqary:55652-url-fix

Zacqary commented Jan 29, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Jan 29, 2020

Uh oh!

elasticmachine commented Jan 29, 2020

Uh oh!

Zacqary commented Jan 29, 2020

Uh oh!

afgomez left a comment

Uh oh!

weltenwort commented Jan 30, 2020

Uh oh!

afgomez commented Jan 30, 2020

Uh oh!

Zacqary commented Jan 30, 2020

Uh oh!

afgomez left a comment

Uh oh!

Zacqary commented Feb 6, 2020

Uh oh!

Zacqary commented Feb 6, 2020

Uh oh!

elasticmachine commented Feb 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Zacqary commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Jan 29, 2020

Uh oh!

elasticmachine commented Jan 29, 2020

Uh oh!

Zacqary commented Jan 29, 2020

Uh oh!

afgomez left a comment

Choose a reason for hiding this comment

Uh oh!

weltenwort commented Jan 30, 2020

Uh oh!

afgomez commented Jan 30, 2020

Uh oh!

Zacqary commented Jan 30, 2020

Uh oh!

afgomez left a comment

Choose a reason for hiding this comment

Uh oh!

Zacqary commented Feb 6, 2020

Uh oh!

Zacqary commented Feb 6, 2020

Uh oh!

elasticmachine commented Feb 6, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Zacqary commented Jan 29, 2020 •

edited

Loading