Skip to content

Commit

Permalink
Bugfix/throw toggle around experiment metrics view with warning msg (#…
Browse files Browse the repository at this point in the history
…1698)

* disable-analysis-metrics-view-with-env-toggle

* toggle the view was opposite condition

* flip-flop the direction of the boolean naming and logic, avoid double negative yuck

* version bump
  • Loading branch information
danoswaltCL authored Jun 26, 2024
1 parent 35dc094 commit 1b75dbb
Show file tree
Hide file tree
Showing 28 changed files with 147 additions and 75 deletions.
4 changes: 2 additions & 2 deletions backend/package-lock.json

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

2 changes: 1 addition & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ab_testing_backend",
"version": "5.1.15",
"version": "5.1.16",
"description": "Backend for A/B Testing Project",
"scripts": {
"install:all": "npm ci && cd packages/Scheduler && npm ci && cd ../Upgrade && npm ci",
Expand Down
4 changes: 2 additions & 2 deletions backend/packages/Scheduler/package-lock.json

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

2 changes: 1 addition & 1 deletion backend/packages/Scheduler/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ppl-upgrade-serverless",
"version": "5.1.15",
"version": "5.1.16",
"description": "Serverless webpack example using Typescript",
"main": "handler.js",
"scripts": {
Expand Down
4 changes: 2 additions & 2 deletions backend/packages/Upgrade/package-lock.json

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

2 changes: 1 addition & 1 deletion backend/packages/Upgrade/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ab_testing_backend",
"version": "5.1.15",
"version": "5.1.16",
"description": "Backend for A/B Testing Project",
"main": "index.js",
"scripts": {
Expand Down
2 changes: 1 addition & 1 deletion clientlibs/java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
at the same time that happen to rev to the same new version will be caught
by a merge conflict. -->

<version>5.1.15</version>
<version>5.1.16</version>
<build>
<plugins>
<plugin>
Expand Down
4 changes: 2 additions & 2 deletions clientlibs/js/package-lock.json

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

2 changes: 1 addition & 1 deletion clientlibs/js/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "upgrade_client_lib",
"version": "5.1.15",
"version": "5.1.16",
"description": "Client library to communicate with the Upgrade server",
"files": [
"dist/*"
Expand Down
4 changes: 2 additions & 2 deletions frontend/package-lock.json

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

2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ab-testing",
"version": "5.1.15",
"version": "5.1.16",
"license": "MIT",
"scripts": {
"ng": "ng",
Expand Down
5 changes: 4 additions & 1 deletion frontend/projects/upgrade/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ export const getEnvironmentConfig = (http: HttpClient, env: Environment) => {
.then((config: RuntimeEnvironmentConfig) => {
env.apiBaseUrl = config.endpointApi || config.apiBaseUrl;
env.googleClientId = config.gapiClientId || config.googleClientId;
env.withinSubjectExperimentSupportToggle = config.withinSubjectExperimentSupportToggle ?? env.withinSubjectExperimentSupportToggle ?? false;
env.withinSubjectExperimentSupportToggle =
config.withinSubjectExperimentSupportToggle ?? env.withinSubjectExperimentSupportToggle ?? false;
env.metricAnalyticsExperimentDisplayToggle =
config.metricAnalyticsExperimentDisplayToggle ?? env.metricAnalyticsExperimentDisplayToggle ?? false;
})
.catch((error) => {
console.log({ error });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
actionUpsertMetrics,
} from './store/analysis.actions';
import { UpsertMetrics } from './store/analysis.models';
import { Environment } from '../../../environments/environment-types';

const mockStateStore$ = new BehaviorSubject({});
(mockStateStore$ as any).dispatch = jest.fn();
Expand All @@ -22,10 +23,12 @@ jest.mock('./store/analysis.selectors', () => ({

describe('AnalysisService', () => {
const mockStore: any = mockStateStore$;
let mockEnvironment: Environment = { metricAnalyticsExperimentDisplayToggle: true } as Environment;
let service: AnalysisService;

beforeEach(() => {
service = new AnalysisService(mockStore);
service = new AnalysisService(mockStore, mockEnvironment);
jest.resetAllMocks();
});

describe('#queryResultById$', () => {
Expand Down Expand Up @@ -78,12 +81,34 @@ describe('AnalysisService', () => {
});

describe('#executeQuery', () => {
it('should dispatch executeQuery with the supplied string input array', () => {
let originalEnvironment;

beforeEach(() => {
// Save the original environment to restore it after tests
originalEnvironment = { ...mockEnvironment };
});

afterEach(() => {
// Restore the original environment after each test
mockEnvironment = { ...originalEnvironment };
});

it('should dispatch executeQuery with the supplied string input array when metricAnalyticsExperimentDisplayToggle is true', () => {
mockEnvironment = { metricAnalyticsExperimentDisplayToggle: true } as Environment;
const mockQueryIds = ['test', 'test2'];

service.executeQuery(mockQueryIds);

expect(mockStore.dispatch).toHaveBeenLastCalledWith(actionExecuteQuery({ queryIds: mockQueryIds }));
expect(mockStore.dispatch).toHaveBeenCalledWith(actionExecuteQuery({ queryIds: mockQueryIds }));
});

it('should not dispatch executeQuery and log a warning when metricAnalyticsExperimentDisplayToggle is false', () => {
mockEnvironment = { metricAnalyticsExperimentDisplayToggle: false } as Environment;
const mockQueryIds = ['test3', 'test4'];

service.executeQuery(mockQueryIds);

expect(mockStore.dispatch).not.toHaveBeenCalledWith(mockQueryIds);
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from '@angular/core';
import { Inject, Injectable } from '@angular/core';
import { AppState } from '../core.module';
import { Store, select } from '@ngrx/store';
import {
Expand All @@ -11,10 +11,11 @@ import {
import * as AnalysisActions from './store/analysis.actions';
import { UpsertMetrics } from './store/analysis.models';
import { selectExperimentQueries } from '../experiments/store/experiments.selectors';
import { ENV, Environment } from '../../../environments/environment-types';

@Injectable()
export class AnalysisService {
constructor(private store$: Store<AppState>) {}
constructor(private store$: Store<AppState>, @Inject(ENV) private environment: Environment) {}

isMetricsLoading$ = this.store$.pipe(select(selectIsMetricsLoading));
isQueryExecuting$ = this.store$.pipe(select(selectIsQueryExecuting));
Expand All @@ -35,7 +36,14 @@ export class AnalysisService {
}

executeQuery(queryIds: string[]) {
this.store$.dispatch(AnalysisActions.actionExecuteQuery({ queryIds }));
if (this.environment.metricAnalyticsExperimentDisplayToggle) {
this.store$.dispatch(AnalysisActions.actionExecuteQuery({ queryIds }));
} else {
console.warn(
'executeQuery is currently disabled via metricAnalyticsExperimentDisplayToggle:',
this.environment.metricAnalyticsExperimentDisplayToggle
);
}
}

experimentQueryResult$(experimentId: string) {
Expand Down
Loading

0 comments on commit 1b75dbb

Please sign in to comment.