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.

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

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

Merged
merged 28 commits into from
Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
64e58ec
WIP: Notice banner for OSS folks
Nov 15, 2019
5481a23
Add telemetryNotifyUserAboutOptInDefault to injected vars
tsullivan Nov 15, 2019
08eb719
add userHasSeenNotice check
tsullivan Nov 15, 2019
d19bfda
More WIP on banner notice component
Nov 15, 2019
30ff7ed
Text changes on screens
Nov 15, 2019
ddd4df5
make userHasSeenNotice flag work
tsullivan Nov 15, 2019
5ed66f4
Finalzed splash text + checking new flag
Nov 15, 2019
e081ffb
Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…
Nov 15, 2019
9668955
Consolidating banner calls and saving status of opt-in notice
Nov 15, 2019
e7677ab
Conditionally remove the banner and add some code docs
Nov 15, 2019
caca75f
Fixing prior welcome tests
Nov 15, 2019
45382f5
api integration test for user has seen opt in
tsullivan Nov 15, 2019
9d7b316
change post method to put in ui
tsullivan Nov 15, 2019
8be436c
unit test for get_telemetry_notify_user_about_optin_default
tsullivan Nov 15, 2019
850e456
Ignore TS woes
Nov 15, 2019
1d94868
Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…
Nov 15, 2019
f4cf63b
Adding new tests and snapshots for opt-in banner component
Nov 15, 2019
a70928c
Notice banner test
Nov 15, 2019
49dedba
Translation miss
Nov 15, 2019
4f4cb0f
More opt-in tests
Nov 15, 2019
602bb09
increase types usage
tsullivan Nov 15, 2019
c5c4712
Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…
Nov 15, 2019
790767b
Merge branch 'master' into chore/oss-telemetry-banner-notice
elasticmachine Nov 16, 2019
08a1493
roll back core server api change
tsullivan Nov 16, 2019
c8bc021
update snapshot
tsullivan Nov 16, 2019
30b5e8f
Merge remote-tracking branch 'upstream/master' into chore/oss-telemet…
Nov 18, 2019
37199bb
Prop name change + snapshot updates
Nov 18, 2019
11122a1
Merge branch 'chore/oss-telemetry-banner-notice' of https://github.co…
Nov 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class Home extends Component {
getServices().getInjected('disableWelcomeScreen') ||
props.localStorage.getItem(KEY_ENABLE_WELCOME) === 'false'
);
const showTelemetryDisclaimer = getServices().getInjected('allowChangingOptInStatus');
const showTelemetryDisclaimer = getServices().getInjected('telemetryNotifyUserAboutOptInDefault');
Bamieh marked this conversation as resolved.
Show resolved Hide resolved

this.state = {
// If welcome is enabled, we wait for loading to complete
Expand Down Expand Up @@ -231,6 +231,7 @@ export class Home extends Component {
onSkip={this.skipWelcome}
urlBasePath={this.props.urlBasePath}
showTelemetryDisclaimer={this.state.showTelemetryDisclaimer}
optInSeen={this.props.optInSeen}
/>
);
}
Expand Down Expand Up @@ -269,4 +270,5 @@ Home.propTypes = {
localStorage: PropTypes.object.isRequired,
urlBasePath: PropTypes.string.isRequired,
mlEnabled: PropTypes.bool.isRequired,
optInSeen: PropTypes.func.isRequired,
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export function HomeApp({ directories }) {
savedObjectsClient,
getBasePath,
addBasePath,
telemetryOptInProvider: {
setOptInNoticeSeen,
},
} = getServices();

const isCloudEnabled = getInjected('isCloudEnabled', false);
Expand Down Expand Up @@ -83,6 +86,7 @@ export function HomeApp({ directories }) {
find={savedObjectsClient.find}
localStorage={localStorage}
urlBasePath={getBasePath()}
optInSeen={setOptInNoticeSeen}
/>
</Route>
<Route path="/home">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { SampleDataCard } from './sample_data';
interface Props {
urlBasePath: string;
onSkip: () => void;
optInSeen: () => any;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

showTelemetryDisclaimer: boolean;
}

Expand Down Expand Up @@ -75,6 +76,7 @@ export class Welcome extends React.Component<Props> {

componentDidMount() {
this.services.trackUiMetric(this.services.METRIC_TYPE.LOADED, 'welcomeScreenMount');
this.props.optInSeen();
document.addEventListener('keydown', this.hideOnEsc);
}

Expand Down Expand Up @@ -132,17 +134,17 @@ export class Welcome extends React.Component<Props> {
>
<FormattedMessage
id="kbn.home.dataManagementDisclaimerPrivacyLink"
defaultMessage="Privacy Policy."
defaultMessage="Privacy Statement."
/>
</EuiLink>
<FormattedMessage
id="kbn.home.dataManagementDisableCollection"
defaultMessage=" To disable collection, "
defaultMessage=" To stop collection, "
/>
<EuiLink href="#/management/kibana/settings">
<FormattedMessage
id="kbn.home.dataManagementDisableCollectionLink"
defaultMessage="click here."
defaultMessage="disable usage data here."
/>
</EuiLink>
</EuiTextColor>
Expand Down
1 change: 1 addition & 0 deletions src/legacy/core_plugins/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const telemetry = (kibana: any) => {
telemetryOptInStatusUrl: config.get('telemetry.optInStatusUrl'),
allowChangingOptInStatus: config.get('telemetry.allowChangingOptInStatus'),
telemetrySendUsageFrom: config.get('telemetry.sendUsageFrom'),
telemetryNotifyUserAboutOptInDefault: false,
};
},
hacks: ['plugins/telemetry/hacks/telemetry_init', 'plugins/telemetry/hacks/telemetry_opt_in'],
Expand Down
3 changes: 3 additions & 0 deletions src/legacy/core_plugins/telemetry/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
"lastVersionChecked": {
"ignore_above": 256,
"type": "keyword"
},
"userHasSeenNotice": {
"type": "boolean"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/* eslint @elastic/eui/href-or-on-click:0 */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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


import * as React from 'react';
import { EuiButton, EuiLink, EuiCallOut, EuiSpacer } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

interface Props {
onSeenBanner: () => any;
}

/**
* React component for displaying the Telemetry opt-in banner.
*/
export class OptedInBanner extends React.PureComponent<Props> {
onLinkClick = () => {
this.props.onSeenBanner();
return;
};

render() {
return (
<EuiCallOut title="Help us improve the Elastic Stack">
<FormattedMessage
id="telemetry.telemetryOptedInNoticeDescription"
defaultMessage="To learn about how usage data helps us manage and improve our products and services, see our {privacyStatementLink}. To stop collection, {disableLink}."
values={{
privacyStatementLink: (
<EuiLink
onClick={this.onLinkClick}
href="https://www.elastic.co/legal/privacy-statement"
target="_blank"
rel="noopener"
>
<FormattedMessage
id="telemetry.telemetryOptedInPrivacyStatement"
defaultMessage="Privacy Statement"
/>
</EuiLink>
),
disableLink: (
<EuiLink href="#/management/kibana/settings" onClick={this.onLinkClick}>
<FormattedMessage
id="telemetry.telemetryOptedInDisableUsage"
defaultMessage="disable usage data here"
/>
</EuiLink>
),
}}
/>
<EuiSpacer size="s" />
<EuiButton size="s" onClick={this.props.onSeenBanner}>
<FormattedMessage
id="telemetry.welcomeBanner.enableButtonLabel"
defaultMessage="Dismiss"
/>
</EuiButton>
</EuiCallOut>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import chrome from 'ui/chrome';

import { fetchTelemetry } from '../fetch_telemetry';
import { renderBanner } from './render_banner';
import { renderOptedInBanner } from './render_notice_banner';
import { shouldShowBanner } from './should_show_banner';
import { shouldShowOptInBanner } from './should_show_opt_in_banner';
import { TelemetryOptInProvider, isUnauthenticated } from '../../services';
import { npStart } from 'ui/new_platform';

Expand All @@ -48,12 +50,16 @@ async function asyncInjectBanner($injector) {
return;
}

const $http = $injector.get('$http');

// determine if the banner should be displayed
if (await shouldShowBanner(telemetryOptInProvider, config)) {
const $http = $injector.get('$http');

renderBanner(telemetryOptInProvider, () => fetchTelemetry($http, { unencrypted: true }));
}

if (await shouldShowOptInBanner(telemetryOptInProvider, config)) {
renderOptedInBanner(telemetryOptInProvider, () => fetchTelemetry($http, { unencrypted: true }));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';

import { banners } from 'ui/notify';
import { OptedInBanner } from '../../components/opted_in_notice_banner';

/**
* Render the Telemetry Opt-in banner.
*
* @param {Object} telemetryOptInProvider The telemetry opt-in provider.
* @param {Function} fetchTelemetry Function to pull telemetry on demand.
* @param {Object} _banners Banners singleton, which can be overridden for tests.
*/
export function renderOptedInBanner(telemetryOptInProvider, { _banners = banners } = {}) {
const bannerId = _banners.add({
component: (
<OptedInBanner
onSeenBanner={telemetryOptInProvider.setOptInNoticeSeen}
/>
),
priority: 10000
});

telemetryOptInProvider.setOptInBannerNoticeId(bannerId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/**
* Determine if the banner should be displayed.
*
* This method can have side-effects related to deprecated config settings.
*
* @param {Object} config The advanced settings config object.
* @param {Object} _handleOldSettings handleOldSettings function, but overridable for tests.
* @return {Boolean} {@code true} if the banner should be displayed. {@code false} otherwise.
*/
export async function shouldShowOptInBanner(telemetryOptInProvider) {
return telemetryOptInProvider.notifyUserAboutOptInDefault();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@

import moment from 'moment';
import { setCanTrackUiMetrics } from 'ui/ui_metric';
import { toastNotifications } from 'ui/notify';
import { banners, toastNotifications } from 'ui/notify';
import { npStart } from 'ui/new_platform';
import { i18n } from '@kbn/i18n';

let bannerId: string | null = null;
let optInBannerNoticeId: string | null = null;
let currentOptInStatus = false;
let telemetryNotifyUserAboutOptInDefault = true;

async function sendOptInStatus($injector: any, chrome: any, enabled: boolean) {
const telemetryOptInStatusUrl = npStart.core.injectedMetadata.getInjectedVar(
Expand Down Expand Up @@ -57,18 +59,56 @@ async function sendOptInStatus($injector: any, chrome: any, enabled: boolean) {
}
export function TelemetryOptInProvider($injector: any, chrome: any, sendOptInStatusChange = true) {
currentOptInStatus = npStart.core.injectedMetadata.getInjectedVar('telemetryOptedIn') as boolean;

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

telemetryNotifyUserAboutOptInDefault = npStart.core.injectedMetadata.getInjectedVar(
'telemetryNotifyUserAboutOptInDefault'
) as boolean;
Copy link
Member

Choose a reason for hiding this comment

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

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


setCanTrackUiMetrics(currentOptInStatus);

const provider = {
getBannerId: () => bannerId,
getOptInBannerNoticeId: () => optInBannerNoticeId,
getOptIn: () => currentOptInStatus,
canChangeOptInStatus: () => allowChangingOptInStatus,
notifyUserAboutOptInDefault: () => telemetryNotifyUserAboutOptInDefault,
setBannerId(id: string) {
bannerId = id;
},
setOptInBannerNoticeId(id: string) {
optInBannerNoticeId = id;
},
setOptInNoticeSeen: async () => {
const $http = $injector.get('$http');

// If they've seen the notice don't spam the API
if (!telemetryNotifyUserAboutOptInDefault) {
return telemetryNotifyUserAboutOptInDefault;
}

banners.remove(optInBannerNoticeId);

try {
await $http.post(chrome.addBasePath('/api/telemetry/v2/userHasSeenNotice'));
telemetryNotifyUserAboutOptInDefault = false;
} catch (error) {
toastNotifications.addError(error, {
title: i18n.translate('telemetry.optInNoticeSeenErrorTitle', {
defaultMessage: 'Error',
}),
toastMessage: i18n.translate('telemetry.optInNoticeSeenErrorToastText', {
defaultMessage: 'An error occurred dismissing the notice',
}),
});
telemetryNotifyUserAboutOptInDefault = true;
}

return telemetryNotifyUserAboutOptInDefault;
},
setOptIn: async (enabled: boolean) => {
if (!allowChangingOptInStatus) {
return;
Expand All @@ -88,7 +128,8 @@ export function TelemetryOptInProvider($injector: any, chrome: any, sendOptInSta
defaultMessage: 'Error',
}),
toastMessage: i18n.translate('telemetry.optInErrorToastText', {
defaultMessage: 'An error occured while trying to set the usage statistics preference.',
defaultMessage:
'An error occurred while trying to set the usage statistics preference.',
}),
});
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/legacy/core_plugins/telemetry/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { CoreSetup } from 'src/core/server';
import { registerTelemetryOptInRoutes } from './telemetry_opt_in';
import { registerTelemetryUsageStatsRoutes } from './telemetry_usage_stats';
import { registerTelemetryOptInStatsRoutes } from './telemetry_opt_in_stats';
import { registerTelemetryUserHasSeenNotice } from './telemetry_user_has_seen_notice';

interface RegisterRoutesParams {
core: CoreSetup;
Expand All @@ -31,4 +32,5 @@ export function registerRoutes({ core, currentKibanaVersion }: RegisterRoutesPar
registerTelemetryOptInRoutes({ core, currentKibanaVersion });
registerTelemetryUsageStatsRoutes(core);
registerTelemetryOptInStatsRoutes(core);
registerTelemetryUserHasSeenNotice(core);
}
Loading