[Telemetry] Show opt-in changes for OSS users #50831

joelgriffith · 2019-11-15T22:04:19Z

Summary

Fixes #50773.

This ensure that we always show telemetry changes in 7.5, and saves the fact that we've served notice in our index.

Still pending are a tests and some final tweaks, but functionality is there.

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

…m/elastic/kibana into chore/oss-telemetry-banner-notice

joelgriffith · 2019-11-15T22:06:19Z

src/legacy/core_plugins/telemetry/public/components/opted_in_notice_banner.tsx

+ * under the License.
+ */
+
+/* eslint @elastic/eui/href-or-on-click:0 */


This is a bit left-handed, but we need to call internals that they've seen this notice when they're navigating via anchor tags.

It's not clear which line this comment setting refers to. Can this be done with an eslint-disable-line ?

…m/elastic/kibana into chore/oss-telemetry-banner-notice

tsullivan · 2019-11-15T23:14:40Z

src/legacy/core_plugins/telemetry/public/services/telemetry_opt_in.ts

  const allowChangingOptInStatus = npStart.core.injectedMetadata.getInjectedVar(
    'allowChangingOptInStatus'
  ) as boolean;

+  telemetryNotifyUserAboutOptInDefault = npStart.core.injectedMetadata.getInjectedVar(
+    'telemetryNotifyUserAboutOptInDefault'
+  ) as boolean;


I'm not so sure on this naming, usually boolean variables need an is/should prefix for clarity.

Maybe telemetryNotifyUserAboutOptInDefault ?

I think we want to keep the telemetry prefix because of the way the injectedVars are not namespaced by plugin

…m/elastic/kibana into chore/oss-telemetry-banner-notice

tsullivan · 2019-11-16T00:02:53Z

x-pack/test/api_integration/apis/telemetry/telemetry_optin_notice_seen.ts

+      } as GetParams);
+
+      expect(telemetry.userHasSeenNotice).to.be(true);
+    });


The only thing I think the test code is missing is a clean-up step to remove the telemetry:telemetry document after the test did its update

Also, I didn't see an easy way to test that this API works with a non-privileged user.

elasticmachine · 2019-11-16T01:06:44Z

💔 Build Failed

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

joelgriffith · 2019-11-16T02:43:47Z

@elasticmachine merge upstream

elasticmachine · 2019-11-16T04:01:17Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 790767b

elasticmachine · 2019-11-16T06:19:06Z

💚 Build Succeeded

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

pgayvallet

LGTM for platform changes

pgayvallet · 2019-11-16T07:27:25Z

src/legacy/core_plugins/kibana/public/home/components/welcome.tsx

@@ -43,6 +43,7 @@ import { SampleDataCard } from './sample_data';
 interface Props {
  urlBasePath: string;
  onSkip: () => void;
+  optInSeen: () => any;


NIT: naming. This is an event handler. optInSeen -> onOptInSeen (like onSkip here). Same goes for propagaging from home.js

src/legacy/core_plugins/kibana/public/home/components/home.js

…ry-banner-notice

…m/elastic/kibana into chore/oss-telemetry-banner-notice

elasticmachine · 2019-11-18T16:43:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 11122a1

Bamieh

LGTM

* WIP: Notice banner for OSS folks * Add telemetryNotifyUserAboutOptInDefault to injected vars * add userHasSeenNotice check * More WIP on banner notice component * Text changes on screens * make userHasSeenNotice flag work * Finalzed splash text + checking new flag * Consolidating banner calls and saving status of opt-in notice * Conditionally remove the banner and add some code docs * Fixing prior welcome tests * api integration test for user has seen opt in * change post method to put in ui * unit test for get_telemetry_notify_user_about_optin_default * Ignore TS woes * Adding new tests and snapshots for opt-in banner component * Notice banner test * Translation miss * More opt-in tests * increase types usage * roll back core server api change * update snapshot * Prop name change + snapshot updates

…-fallback * 'master' of github.com:elastic/kibana: (116 commits) [Maps] move apply global filter settting from layer to source (elastic#50523) [SIEM] Fix: Empty `Source` / `Destination` shown when only ports are populated (elastic#50843) [Maps] Delay vector tile layer syncing until spritesheet is loaded (elastic#48955) [Maps] prevent users from overflowing URL when filtering by shape (elastic#50747) [DOCS] Mark Beats central management as discontinued (elastic#49423) [page_objects/common_page] convert to ts (elastic#50771) [NP Kibana Migrations ] kibana plugin home (elastic#50444) [DOCS] Shareables naming convention (elastic#50497) [ML] DF Analytics - auto-populate model_memory_limit (elastic#50714) Increase alerting test stability and reduce flakiness (elastic#50246) [ML] Remaning new_job_new folder (elastic#50917) [Telemetry] Show opt-in changes for OSS users (elastic#50831) [ML] Fix lat_long anomalies table links menu and value formatting (elastic#50916) [Dev] Fix serialising a really big string (elastic#50915) Better explanation about the Prettier recommendation (extension vs. NPM module) (elastic#50629) [Monitoring] Use a basic monitoring user for tests (elastic#47865) [Monitoring] Gracefully handle issue with filebeat indices (elastic#48929) [Monitoring] Improve permissions required around setup mode (elastic#50421) Additional validation for elasticsearch username (elastic#48247) Revert changes to use_kibana_ui_setting (elastic#50877) ... # Conflicts: # src/legacy/core_plugins/console/server/request.test.ts

Joel Griffith and others added 9 commits November 15, 2019 11:13

WIP: Notice banner for OSS folks

64e58ec

Add telemetryNotifyUserAboutOptInDefault to injected vars

5481a23

add userHasSeenNotice check

08eb719

More WIP on banner notice component

d19bfda

Text changes on screens

30ff7ed

make userHasSeenNotice flag work

ddd4df5

Finalzed splash text + checking new flag

5ed66f4

Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…

e081ffb

…m/elastic/kibana into chore/oss-telemetry-banner-notice

Consolidating banner calls and saving status of opt-in notice

9668955

joelgriffith added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v7.6.0 labels Nov 15, 2019

joelgriffith commented Nov 15, 2019

View reviewed changes

Joel Griffith and others added 7 commits November 15, 2019 14:11

Conditionally remove the banner and add some code docs

e7677ab

Fixing prior welcome tests

caca75f

api integration test for user has seen opt in

45382f5

change post method to put in ui

9d7b316

unit test for get_telemetry_notify_user_about_optin_default

8be436c

Ignore TS woes

850e456

Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…

1d94868

…m/elastic/kibana into chore/oss-telemetry-banner-notice

tsullivan reviewed Nov 15, 2019

View reviewed changes

Joel Griffith and others added 5 commits November 15, 2019 15:18

Adding new tests and snapshots for opt-in banner component

f4cf63b

Notice banner test

a70928c

Translation miss

49dedba

More opt-in tests

4f4cb0f

increase types usage

602bb09

joelgriffith requested a review from a team as a code owner November 15, 2019 23:51

Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…

c5c4712

…m/elastic/kibana into chore/oss-telemetry-banner-notice

tsullivan reviewed Nov 16, 2019

View reviewed changes

Merge branch 'master' into chore/oss-telemetry-banner-notice

790767b

tsullivan added 2 commits November 15, 2019 21:07

roll back core server api change

08a1493

update snapshot

c8bc021

pgayvallet approved these changes Nov 16, 2019

View reviewed changes

Bamieh reviewed Nov 18, 2019

View reviewed changes

src/legacy/core_plugins/kibana/public/home/components/home.js Show resolved Hide resolved

Joel Griffith added 3 commits November 18, 2019 07:11

Merge remote-tracking branch 'upstream/master' into chore/oss-telemet…

30b5e8f

…ry-banner-notice

Prop name change + snapshot updates

37199bb

Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…

11122a1

…m/elastic/kibana into chore/oss-telemetry-banner-notice

joelgriffith merged commit 97c2bc9 into master Nov 18, 2019

Bamieh approved these changes Nov 18, 2019

View reviewed changes

This was referenced Nov 18, 2019

[7.x] [Telemetry] Show opt-in changes for OSS users (#50831) #50940

Merged

[7.5] [Telemetry] Show opt-in changes for OSS users (#50831) #50941

Merged

spalger deleted the chore/oss-telemetry-banner-notice branch December 4, 2019 19:03

alexfrancoeur mentioned this pull request Dec 16, 2019

Telemetry notice per splash screen needs to be removed. #50960

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Telemetry] Show opt-in changes for OSS users #50831

[Telemetry] Show opt-in changes for OSS users #50831

joelgriffith commented Nov 15, 2019

joelgriffith Nov 15, 2019

tsullivan Nov 15, 2019

tsullivan Nov 15, 2019

tsullivan Nov 16, 2019

tsullivan Nov 16, 2019

elasticmachine commented Nov 16, 2019

joelgriffith commented Nov 16, 2019

elasticmachine commented Nov 16, 2019

elasticmachine commented Nov 16, 2019

pgayvallet left a comment

pgayvallet Nov 16, 2019

elasticmachine commented Nov 18, 2019

Bamieh left a comment

[Telemetry] Show opt-in changes for OSS users #50831

[Telemetry] Show opt-in changes for OSS users #50831

Conversation

joelgriffith commented Nov 15, 2019

Summary

Checklist

joelgriffith Nov 15, 2019

Choose a reason for hiding this comment

tsullivan Nov 15, 2019

Choose a reason for hiding this comment

tsullivan Nov 15, 2019

Choose a reason for hiding this comment

tsullivan Nov 16, 2019

Choose a reason for hiding this comment

tsullivan Nov 16, 2019

Choose a reason for hiding this comment

elasticmachine commented Nov 16, 2019

💔 Build Failed

joelgriffith commented Nov 16, 2019

elasticmachine commented Nov 16, 2019

💔 Build Failed

elasticmachine commented Nov 16, 2019

💚 Build Succeeded

pgayvallet left a comment

Choose a reason for hiding this comment

pgayvallet Nov 16, 2019

Choose a reason for hiding this comment

elasticmachine commented Nov 18, 2019

💚 Build Succeeded

Bamieh left a comment

Choose a reason for hiding this comment