[Telemetry] Only show Opt-In banner when user can change settings (#76883)

Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Alejandro Fernández Haro 2020-09-10 14:49:09 +01:00 committed by GitHub
parent ad62fa0a6e
commit 8ad47846ca
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 92 additions and 14 deletions

View file

@ -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);
@ -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;
}

View file

@ -48,6 +48,7 @@ export function mockTelemetryService({
banner: true,
allowChangingOptInStatus: true,
telemetryNotifyUserAboutOptInDefault: true,
userCanChangeSettings: true,
...configOverride,
};

View file

@ -25,6 +25,7 @@ import {
PluginInitializerContext,
SavedObjectsClientContract,
SavedObjectsBatchResponse,
ApplicationStart,
} from '../../../core/public';
import { TelemetrySender, TelemetryService, TelemetryNotifications } from './services';
@ -61,6 +62,7 @@ export interface TelemetryPluginConfig {
optInStatusUrl: string;
sendUsageFrom: 'browser' | 'server';
telemetryNotifyUserAboutOptInDefault?: boolean;
userCanChangeSettings?: boolean;
}
export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPluginStart> {
@ -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;
@ -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,
@ -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);
@ -196,6 +213,7 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
optIn,
sendUsageFrom,
telemetryNotifyUserAboutOptInDefault,
userCanChangeSettings: this.canUserChangeSettings,
};
}

View file

@ -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);
});
});

View file

@ -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 => {

View file

@ -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 () => {
@ -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);
});
});
});

View file

@ -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;

View file

@ -228,7 +228,6 @@ exports[`TelemetryManagementSectionComponent renders null because allowChangingO
"getIsOptedIn": [Function],
"getOptInStatusUrl": [Function],
"getTelemetryUrl": [Function],
"getUserHasSeenOptedInNotice": [Function],
"http": Object {
"addLoadingCountSource": [MockFunction],
"anonymousPaths": Object {
@ -430,7 +429,6 @@ exports[`TelemetryManagementSectionComponent renders null because query does not
"getIsOptedIn": [Function],
"getOptInStatusUrl": [Function],
"getTelemetryUrl": [Function],
"getUserHasSeenOptedInNotice": [Function],
"http": Object {
"addLoadingCountSource": [MockFunction],
"anonymousPaths": Object {