Skip to content

[Telemetry] Fix inconsistent search behaviour in Advanced Settings #64510

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
Merged
merged 4 commits into from
Apr 29, 2020
Merged

[Telemetry] Fix inconsistent search behaviour in Advanced Settings #64510

merged 4 commits into from
Apr 29, 2020

Conversation

@afharo
Copy link
Member

@afharo afharo commented Apr 27, 2020 •
edited
Loading

Summary

Fix inconsistent search behaviour in Advanced Settings when telemetry.allowChangingOptInStatus: false.
Closes #50676

Fix the logic to calculate searchTermMatches in the Advanced Settings for the Telemetry Management section.

The change is basically fixing the logic to calculate searchTermMatches by also taking into consideration the telemetry.allowChangingOptInStatus flag.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@afharo afharo self-assigned this Apr 27, 2020
@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 27, 2020

Pinging @elastic/pulse (Team:Pulse)

@afharo afharo force-pushed the telemetry/fix-search-in-advanced-settings branch from aec1817 to 9de64cd Compare April 27, 2020 18:09
@afharo
Copy link
Member Author

afharo commented Apr 28, 2020

@elasticmachine merge upstream

@afharo afharo marked this pull request as ready for review April 28, 2020 11:20
@afharo afharo requested a review from a team as a code owner April 28, 2020 11:20
@afharo
Copy link
Member Author

afharo commented Apr 28, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 28, 2020

💚 Build Succeeded

History

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

TinaHeiligers
TinaHeiligers requested changes Apr 28, 2020
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

The code looks good and the tests indicate that having telemetry.allowChangingOptInStatus:false should render the second image (without the 'No settings found' tag) from #50676.
However, when I add telemetry.allowChangingOptInStatus: false to my local kibana.dev.yml file, I still see the undesired effect:
Screen Shot 2020-04-28 at 13 38 17

@TinaHeiligers TinaHeiligers self-requested a review April 28, 2020 21:03
TinaHeiligers
TinaHeiligers approved these changes Apr 28, 2020
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

My original review was based off of an incorrect interpretation of what was meant by

The behaviour of the second one should be used when searching for "telemetry" while telemetry opting UI is disabled.

in #50676. I understood the second option reference to be to the images and not the listed items.

The code looks great and I tested the fix locally. The fix shows the correct search behavior:
Screen Shot 2020-04-28 at 14 01 20

LGTM!

@afharo afharo merged commit fc79c06 into elastic:master Apr 29, 2020
@afharo afharo deleted the telemetry/fix-search-in-advanced-settings branch April 29, 2020 09:40
@afharo afharo mentioned this pull request Apr 29, 2020
Merged
afharo added a commit that referenced this pull request Apr 29, 2020
@afharo
[7.x] [Telemetry] Fix inconsistent search behaviour in Advanced Setti…
3085731 
…ngs (#64510) (#64751)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 29, 2020
@gmmorris
Merge branch 'master' into alerting/np-migration
e0373bb 
* master:
  [ML] Changes Machine learning overview UI text (elastic#64625)
  [Uptime] Migrate client to New Platform (elastic#55086)
  Slim vis type timeseries (elastic#64631)
  [Telemetry] Fix inconsistent search behaviour in Advanced Settings (elastic#64510)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 29, 2020
@gmmorris
Merge branch 'alerting/np-migration' into alerting/np-migration-tests
8523508 
* alerting/np-migration: (64 commits)
  [ML] Changes Machine learning overview UI text (elastic#64625)
  [Uptime] Migrate client to New Platform (elastic#55086)
  Slim vis type timeseries (elastic#64631)
  [Telemetry] Fix inconsistent search behaviour in Advanced Settings (elastic#64510)
  removed unneeded dep and file
  [SIEM] Create template timeline (elastic#63136)
  load react component lazily in so management section (elastic#64285)
  Cleanup .eslingignore and add target (elastic#64617)
  [Ingest] Support yaml variables in datasource (elastic#64459)
  typescript-ify portions of src/optimize (elastic#64688)
  [ngSanitize] add explicit dependencies to all uses of `ngSanitize` angular module (elastic#64546)
  Consolidate downloading plugin bundles to bootstrap script (elastic#64685)
  [Maps] disable edit layer button when flyout is open for add layer or map settings (elastic#64230)
  chore(NA): add async import into infra plugin to reduce apm bundle size (elastic#63292)
  [Maps] fix edit filter (elastic#64586)
  [SIEM][Detections] Adds large list support using REST endpoints
  Replace a number of any-ed styled(eui*) with accurate types (elastic#64555)
  [Endpoint] Recursive resolver children (elastic#61914)
  [ML] Fix new job wizard with multiple indices (elastic#64567)
  Use short URLs for legacy plugin deprecation warning (elastic#64540)
  ...
@lukeelmers lukeelmers added the Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// label Oct 1, 2021
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 1, 2021

Pinging @elastic/kibana-core (Team:Core)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TinaHeiligers TinaHeiligers TinaHeiligers approved these changes

Assignees

@afharo afharo

Labels

Feature:Telemetry release_note:fix Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.8.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Inconsistent search behaviour in advanced settings for Telemetry

5 participants

@afharo @elasticmachine @kibanamachine @TinaHeiligers @lukeelmers