Skip to content

Commit

Permalink
[Telemetry] Only show Opt-In banner when user can change settings (#7…
Browse files Browse the repository at this point in the history
…6883)

Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
3 people authored Sep 10, 2020
1 parent ad62fa0 commit 8ad4784
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/plugins/home/public/application/components/welcome.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class Welcome extends React.Component<Props> {
componentDidMount() {
const { telemetry } = this.props;
this.services.trackUiMetric(METRIC_TYPE.LOADED, 'welcomeScreenMount');
if (telemetry) {
if (telemetry?.telemetryService.userCanChangeSettings) {
telemetry.telemetryNotifications.setOptedInNoticeSeen();
}
document.addEventListener('keydown', this.hideOnEsc);
Expand All @@ -88,7 +88,7 @@ export class Welcome extends React.Component<Props> {

private renderTelemetryEnabledOrDisabledText = () => {
const { telemetry } = this.props;
if (!telemetry) {
if (!telemetry || !telemetry.telemetryService.userCanChangeSettings) {
return null;
}

Expand Down
1 change: 1 addition & 0 deletions src/plugins/telemetry/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function mockTelemetryService({
banner: true,
allowChangingOptInStatus: true,
telemetryNotifyUserAboutOptInDefault: true,
userCanChangeSettings: true,
...configOverride,
};

Expand Down
18 changes: 18 additions & 0 deletions src/plugins/telemetry/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
PluginInitializerContext,
SavedObjectsClientContract,
SavedObjectsBatchResponse,
ApplicationStart,
} from '../../../core/public';

import { TelemetrySender, TelemetryService, TelemetryNotifications } from './services';
Expand Down Expand Up @@ -61,6 +62,7 @@ export interface TelemetryPluginConfig {
optInStatusUrl: string;
sendUsageFrom: 'browser' | 'server';
telemetryNotifyUserAboutOptInDefault?: boolean;
userCanChangeSettings?: boolean;
}

export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPluginStart> {
Expand All @@ -69,6 +71,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
private telemetrySender?: TelemetrySender;
private telemetryNotifications?: TelemetryNotifications;
private telemetryService?: TelemetryService;
private canUserChangeSettings: boolean = true;

constructor(initializerContext: PluginInitializerContext<TelemetryPluginConfig>) {
this.currentKibanaVersion = initializerContext.env.packageInfo.version;
Expand All @@ -91,6 +94,9 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
throw Error('Telemetry plugin failed to initialize properly.');
}

this.canUserChangeSettings = this.getCanUserChangeSettings(application);
this.telemetryService.userCanChangeSettings = this.canUserChangeSettings;

this.telemetryNotifications = new TelemetryNotifications({
overlays,
telemetryService: this.telemetryService,
Expand Down Expand Up @@ -125,6 +131,17 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
};
}

/**
* Can the user edit the saved objects?
* This is a security feature, not included in the OSS build, so we need to fallback to `true`
* in case it is `undefined`.
* @param application CoreStart.application
* @private
*/
private getCanUserChangeSettings(application: ApplicationStart): boolean {
return (application.capabilities?.savedObjectsManagement?.edit as boolean | undefined) ?? true;
}

private getIsUnauthenticated(http: HttpStart) {
const { anonymousPaths } = http;
return anonymousPaths.isAnonymous(window.location.pathname);
Expand Down Expand Up @@ -196,6 +213,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
optIn,
sendUsageFrom,
telemetryNotifyUserAboutOptInDefault,
userCanChangeSettings: this.canUserChangeSettings,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,27 @@ describe('setOptedInNoticeSeen', () => {
expect(telemetryService.setUserHasSeenNotice).toBeCalledTimes(1);
});
});

describe('shouldShowOptedInNoticeBanner', () => {
it("should return true because a banner hasn't been shown, the notice hasn't been seen and the user has privileges to edit saved objects", () => {
const telemetryService = mockTelemetryService();
telemetryService.getUserShouldSeeOptInNotice = jest.fn().mockReturnValue(true);
const telemetryNotifications = mockTelemetryNotifications({ telemetryService });
expect(telemetryNotifications.shouldShowOptedInNoticeBanner()).toBe(true);
});

it('should return false because the banner is already on screen', () => {
const telemetryService = mockTelemetryService();
telemetryService.getUserShouldSeeOptInNotice = jest.fn().mockReturnValue(true);
const telemetryNotifications = mockTelemetryNotifications({ telemetryService });
telemetryNotifications['optedInNoticeBannerId'] = 'bruce-banner';
expect(telemetryNotifications.shouldShowOptedInNoticeBanner()).toBe(false);
});

it("should return false because the banner has already been seen or the user doesn't have privileges to change saved objects", () => {
const telemetryService = mockTelemetryService();
telemetryService.getUserShouldSeeOptInNotice = jest.fn().mockReturnValue(false);
const telemetryNotifications = mockTelemetryNotifications({ telemetryService });
expect(telemetryNotifications.shouldShowOptedInNoticeBanner()).toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ export class TelemetryNotifications {
}

public shouldShowOptedInNoticeBanner = (): boolean => {
const userHasSeenOptedInNotice = this.telemetryService.getUserHasSeenOptedInNotice();
const userShouldSeeOptInNotice = this.telemetryService.getUserShouldSeeOptInNotice();
const bannerOnScreen = typeof this.optedInNoticeBannerId !== 'undefined';
return !bannerOnScreen && userHasSeenOptedInNotice;
return !bannerOnScreen && userShouldSeeOptInNotice;
};

public renderOptedInNoticeBanner = (): void => {
Expand Down
31 changes: 26 additions & 5 deletions src/plugins/telemetry/public/services/telemetry_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,15 @@ describe('TelemetryService', () => {
describe('setUserHasSeenNotice', () => {
it('should hit the API and change the config', async () => {
const telemetryService = mockTelemetryService({
config: { telemetryNotifyUserAboutOptInDefault: undefined },
config: { telemetryNotifyUserAboutOptInDefault: undefined, userCanChangeSettings: true },
});

expect(telemetryService.userHasSeenOptedInNotice).toBe(undefined);
expect(telemetryService.getUserHasSeenOptedInNotice()).toBe(false);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(false);
await telemetryService.setUserHasSeenNotice();
expect(telemetryService['http'].put).toBeCalledTimes(1);
expect(telemetryService.userHasSeenOptedInNotice).toBe(true);
expect(telemetryService.getUserHasSeenOptedInNotice()).toBe(true);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(true);
});

it('should show a toast notification if the request fail', async () => {
Expand All @@ -207,12 +207,33 @@ describe('TelemetryService', () => {
});

expect(telemetryService.userHasSeenOptedInNotice).toBe(undefined);
expect(telemetryService.getUserHasSeenOptedInNotice()).toBe(false);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(false);
await telemetryService.setUserHasSeenNotice();
expect(telemetryService['http'].put).toBeCalledTimes(1);
expect(telemetryService['notifications'].toasts.addError).toBeCalledTimes(1);
expect(telemetryService.userHasSeenOptedInNotice).toBe(false);
expect(telemetryService.getUserHasSeenOptedInNotice()).toBe(false);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(false);
});
});

describe('getUserShouldSeeOptInNotice', () => {
it('returns whether the user can update the telemetry config (has SavedObjects access)', () => {
const telemetryService = mockTelemetryService({
config: { userCanChangeSettings: undefined },
});
expect(telemetryService.config.userCanChangeSettings).toBe(undefined);
expect(telemetryService.userCanChangeSettings).toBe(false);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(false);

telemetryService.userCanChangeSettings = false;
expect(telemetryService.config.userCanChangeSettings).toBe(false);
expect(telemetryService.userCanChangeSettings).toBe(false);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(false);

telemetryService.userCanChangeSettings = true;
expect(telemetryService.config.userCanChangeSettings).toBe(true);
expect(telemetryService.userCanChangeSettings).toBe(true);
expect(telemetryService.getUserShouldSeeOptInNotice()).toBe(true);
});
});
});
22 changes: 19 additions & 3 deletions src/plugins/telemetry/public/services/telemetry_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,25 @@ export class TelemetryService {
return telemetryUrl;
};

public getUserHasSeenOptedInNotice = () => {
return this.config.telemetryNotifyUserAboutOptInDefault || false;
};
/**
* Returns if an user should be shown the notice about Opt-In/Out telemetry.
* The decision is made based on whether any user has already dismissed the message or
* the user can't actually change the settings (in which case, there's no point on bothering them)
*/
public getUserShouldSeeOptInNotice(): boolean {
return (
(this.config.telemetryNotifyUserAboutOptInDefault && this.config.userCanChangeSettings) ??
false
);
}

public get userCanChangeSettings() {
return this.config.userCanChangeSettings ?? false;
}

public set userCanChangeSettings(userCanChangeSettings: boolean) {
this.config = { ...this.config, userCanChangeSettings };
}

public getIsOptedIn = () => {
return this.isOptedIn;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 8ad4784

Please sign in to comment.