fix: apply changes to custom endpoint [IDE-36] (#429)

* refactor: pass in configuration as an argument

* fix: check analytics permitted in enqueueEvent

* fix: add userId argument to identify function

* fix: check analytics permitted in identify

* refactor: move check in a function

* chore: update CHANGELOG

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Snyk Security - Code and Open Source Dependencies Changelog
 
+## [2.2.4]
+
+### Fixes
+
+- Changing the custom endpoints has an effect on whether we sent Amplitude events or not
+
 ## [2.2.3]
 
 ### Fixed

src/snyk/common/analytics/itly.ts

@@ -8,7 +8,7 @@ import itly, {
  AnalysisIsTriggeredProperties as _AnalysisIsTriggeredProperties,
  QuickFixIsDisplayedProperties as _QuickFixIsDisplayedProperties,
 } from '../../../ampli';
-import { Configuration } from '../configuration/configuration';
+import { Configuration, IConfiguration } from '../configuration/configuration';
 import { SnykConfiguration } from '../configuration/snykConfiguration';
 import { IDE_NAME } from '../constants/general';
 import { ErrorHandler } from '../error/errorHandler';
@@ -37,7 +37,6 @@ export type QuickFixIsDisplayedProperties = _QuickFixIsDisplayedProperties & {
 export interface IAnalytics {
  load(): Iteratively | null;
  flush(): Promise<void>;
- setShouldReportEvents(shouldReportEvents: boolean): void;
  identify(userId: string): Promise<void>;
  logIssueInTreeIsClicked(properties: IssueInTreeIsClickedProperties): void;
  logAnalysisIsReady(properties: AnalysisIsReadyProperties): void;
@@ -63,23 +62,15 @@ export class Iteratively implements IAnalytics {
  constructor(
  private readonly user: User,
  private logger: ILog,
- private shouldReportEvents: boolean,
- private analyticsPermitted: boolean,
- private isDevelopment: boolean,
+ private configuration: IConfiguration,
  private snykConfiguration?: SnykConfiguration,
  ) {}
 
- setShouldReportEvents(shouldReportEvents: boolean): void {
- this.shouldReportEvents = shouldReportEvents;
- this.load();
- }
-
  load(): Iteratively | null {
- if (!this.shouldReportEvents || !this.analyticsPermitted) {
- this.logger.debug(`Analytics are disabled. No analytics will be collected.`);
+ if (!this.configuration.shouldReportEvents) {
+ this.logger.debug(`Analytics are disabled in Settings.`);
  return null;
  }
- this.logger.debug(`Analytics are enabled. Analytics will be collected.`);
 
  const segmentWriteKey = this.snykConfiguration?.segmentWriteKey;
  if (!segmentWriteKey) {
@@ -88,12 +79,12 @@ export class Iteratively implements IAnalytics {
  }
 
  const segment = new SegmentPlugin(segmentWriteKey);
- const isDevelopment = this.isDevelopment;
+ const isDevelopment = this.configuration.isDevelopment;
 
  if (!this.loaded) {
  try {
  itly.load({
- disabled: !this.shouldReportEvents,
+ disabled: !this.configuration.shouldReportEvents,
  environment: isDevelopment ? 'development' : 'production',
  plugins: [segment, new ItlyErrorPlugin(this.logger)],
  });
@@ -109,19 +100,14 @@ export class Iteratively implements IAnalytics {
 
  public flush = (): Promise<void> => itly.flush();
 
- async identify(): Promise<void> {
- if (!this.canReportEvents()) {
- return;
- }
-
- if (!this.user.authenticatedId) {
- this.logger.error('Tried to identify non-authenticated user');
+ async identify(userId: string): Promise<void> {
+ if (!this.allowedToReportEvents()) {
  return;
  }
 
  // Calling identify is the preferred way to merge authenticated user with anonymous one,
  // see https://snyk.slack.com/archives/C01U2SPRB3Q/p1624276750134700?thread_ts=1624030602.128900&cid=C01U2SPRB3Q
- itly.identify(this.user.authenticatedId, undefined, {
+ itly.identify(userId, undefined, {
  segment: {
  options: {
  anonymousId: this.user.anonymousId,
@@ -232,8 +218,8 @@ export class Iteratively implements IAnalytics {
  });
  }
 
- private enqueueEvent(eventFunction: () => void, mustBeAuthenticated = true): void {
- if (!this.canReportEvents()) {
+ enqueueEvent(eventFunction: () => void, mustBeAuthenticated = true): void {
+ if (!this.allowedToReportEvents()) {
  return;
  }
  if (mustBeAuthenticated && !this.user.authenticatedId) {
@@ -249,13 +235,17 @@ export class Iteratively implements IAnalytics {
  return false;
  }
 
- if (!this.shouldReportEvents) {
+ if (!this.configuration.shouldReportEvents) {
  return false;
  }
 
  return true;
  }
 
+ private allowedToReportEvents(): boolean {
+ return this.canReportEvents() && this.configuration.analyticsPermitted;
+ }
+
  private getAnonymousSegmentOptions(): TrackOptions {
  return {
  segment: {

src/snyk/common/watchers/configurationWatcher.ts

@@ -30,7 +30,7 @@ class ConfigurationWatcher implements IWatcher {
  if (key === ADVANCED_ADVANCED_MODE_SETTING) {
  return extension.checkAdvancedMode();
  } else if (key === YES_TELEMETRY_SETTING) {
- return this.analytics.setShouldReportEvents(configuration.shouldReportEvents);
+ this.analytics.load();
  } else if (key === OSS_ENABLED_SETTING) {
  extension.viewManagerService.refreshOssView();
  } else if (key === CODE_SECURITY_ENABLED_SETTING || key === CODE_QUALITY_ENABLED_SETTING) {

src/snyk/extension.ts

@@ -121,14 +121,7 @@ class SnykExtension extends SnykLib implements IExtension {
 
  this.user = await User.getAnonymous(this.context, Logger);
 
- this.analytics = new Iteratively(
- this.user,
- Logger,
- configuration.shouldReportEvents,
- configuration.analyticsPermitted,
- configuration.isDevelopment,
- snykConfiguration,
- );
+ this.analytics = new Iteratively(this.user, Logger, configuration, snykConfiguration);
 
  SecretStorageAdapter.init(vscodeContext);
 

src/test/unit/common/analytics/itly.test.ts

@@ -1,46 +1,26 @@
 import { strictEqual } from 'assert';
+import * as sinon from 'sinon';
+import itly from '../../../../ampli';
 import { Iteratively } from '../../../../snyk/common/analytics/itly';
+import { IConfiguration } from '../../../../snyk/common/configuration/configuration';
 import { SnykConfiguration } from '../../../../snyk/common/configuration/snykConfiguration';
 import { User } from '../../../../snyk/common/user';
 import { LoggerMock } from '../../mocks/logger.mock';
 
 suite('Iteratively', () => {
- const snykConfig = {} as SnykConfiguration;
- const isDevelopment = false;
+ const config = {
+ shouldReportEvents: false,
+ analyticsPermitted: false,
+ isDevelopment: false,
+ } as IConfiguration;
+ const snykConfig = {
+ segmentWriteKey: 'test',
+ } as SnykConfiguration;
 
  suite('.load()', () => {
- suite('when analytics are not permitted', () => {
- const analyticsPermitted = false;
- [true, false].forEach(shouldReportEvents => {
- test(`Returns "null" when shouldReportEvents == ${shouldReportEvents}`, () => {
- const iteratively = new Iteratively(
- new User(),
- new LoggerMock(),
- shouldReportEvents,
- analyticsPermitted,
- isDevelopment,
- snykConfig,
- );
-
- const result = iteratively.load();
-
- strictEqual(result, null);
- });
- });
- });
-
  suite('when analytics are permitted', () => {
- const analyticsPermitted = true;
-
  test('Returns "null" when shouldReportEvents == false', () => {
- const iteratively = new Iteratively(
- new User(),
- new LoggerMock(),
- false,
- analyticsPermitted,
- isDevelopment,
- snykConfig,
- );
+ const iteratively = new Iteratively(new User(), new LoggerMock(), config, snykConfig);
 
  const result = iteratively.load();
 
@@ -51,9 +31,10 @@ suite('Iteratively', () => {
  const iteratively = new Iteratively(
  new User(),
  new LoggerMock(),
- true,
- analyticsPermitted,
- isDevelopment,
+ {
+ ...config,
+ shouldReportEvents: true,
+ },
  snykConfig,
  );
 
@@ -63,4 +44,171 @@ suite('Iteratively', () => {
  });
  });
  });
+
+ suite('.enqueueEvent', () => {
+ test('Enqueues event if shouldReportEvents === true and analyticsPermitted === true', () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: true,
+ analyticsPermitted: true,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ let eventFunctionWasCalled = false;
+ iteratively.enqueueEvent(() => {
+ eventFunctionWasCalled = true;
+ }, false);
+ strictEqual(eventFunctionWasCalled, true);
+ });
+
+ test('Does not enqueue event if shouldReportEvents === false and analyticsPermitted === true', () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: false,
+ analyticsPermitted: true,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ let eventFunctionWasCalled = false;
+ iteratively.enqueueEvent(() => {
+ eventFunctionWasCalled = true;
+ }, false);
+ strictEqual(eventFunctionWasCalled, false);
+ });
+
+ test('Does not enqueue event if shouldReportEvents === true and analyticsPermitted === false', () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: true,
+ analyticsPermitted: false,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ let eventFunctionWasCalled = false;
+ iteratively.enqueueEvent(() => {
+ eventFunctionWasCalled = true;
+ }, false);
+ strictEqual(eventFunctionWasCalled, false);
+ });
+
+ test('Does not enqueue event if shouldReportEvents === false and analyticsPermitted === false', () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: false,
+ analyticsPermitted: false,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ let eventFunctionWasCalled = false;
+ iteratively.enqueueEvent(() => {
+ eventFunctionWasCalled = true;
+ }, false);
+ strictEqual(eventFunctionWasCalled, false);
+ });
+ });
+
+ suite('.identify', () => {
+ let identify: sinon.SinonSpy;
+ const user = new User();
+
+ setup(() => {
+ identify = sinon.spy(itly, 'identify');
+ });
+
+ teardown(() => {
+ identify.restore();
+ });
+
+ test('Identifies a user if shouldReportEvents === true and analyticsPermitted === true', async () => {
+ const iteratively = new Iteratively(
+ user,
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: true,
+ analyticsPermitted: true,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ await iteratively.identify('test');
+ strictEqual(identify.called, true);
+ strictEqual(identify.calledOnce, true);
+ });
+
+ test('Does not identify a user if shouldReportEvents === true and analyticsPermitted === false', async () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: true,
+ analyticsPermitted: false,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ await iteratively.identify('test');
+ strictEqual(identify.called, false);
+ strictEqual(identify.calledOnce, false);
+ });
+
+ test('Does not identify a user if shouldReportEvents === false and analyticsPermitted === true', async () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: false,
+ analyticsPermitted: true,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ await iteratively.identify('test');
+ strictEqual(identify.called, false);
+ strictEqual(identify.calledOnce, false);
+ });
+
+ test('Does not identify a user if shouldReportEvents === false and analyticsPermitted === false', async () => {
+ const iteratively = new Iteratively(
+ new User(),
+ new LoggerMock(),
+ {
+ ...config,
+ shouldReportEvents: false,
+ analyticsPermitted: false,
+ },
+ snykConfig,
+ );
+ iteratively.load();
+
+ await iteratively.identify('test');
+ strictEqual(identify.called, false);
+ strictEqual(identify.calledOnce, false);
+ });
+ });
 });