From a0981f80e44e3aaca551e768947a3d0397a44b6c Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Mon, 8 Sep 2025 14:23:47 -0500 Subject: [PATCH 1/9] chore(config-service): added a config service with tests --- .gitignore | 1 + package-lock.json | 96 +++++++++++++++++++ package.json | 1 + src/app/core/models/config.model.ts | 6 ++ .../core/services/osf-config.service.spec.ts | 40 ++++++++ src/app/core/services/osf-config.service.ts | 69 +++++++++++++ src/assets/config/.git-keep | 0 7 files changed, 213 insertions(+) create mode 100644 src/app/core/models/config.model.ts create mode 100644 src/app/core/services/osf-config.service.spec.ts create mode 100644 src/app/core/services/osf-config.service.ts create mode 100644 src/assets/config/.git-keep diff --git a/.gitignore b/.gitignore index 2f85265bd..5598168a1 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ /tmp /out-tsc /bazel-out +/src/assets/config # Node /node_modules diff --git a/package-lock.json b/package-lock.json index 912834b45..1a9e23bff 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ "@ngxs/logger-plugin": "^19.0.0", "@ngxs/store": "^19.0.0", "@primeng/themes": "^19.0.9", + "@sentry/angular": "^10.10.0", "ace-builds": "^1.42.0", "cedar-artifact-viewer": "^0.9.5", "cedar-embeddable-editor": "^1.5.0", @@ -7406,6 +7407,101 @@ "yarn": ">= 1.13.0" } }, + "node_modules/@sentry-internal/browser-utils": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry-internal/browser-utils/-/browser-utils-10.10.0.tgz", + "integrity": "sha512-209QN9vsQBwJcS+9DU7B4yl9mb4OqCt2kdL3LYDvqsuOdpICpwfowdK3RMn825Ruf4KLJa0KHM1scQbXZCc4lw==", + "license": "MIT", + "dependencies": { + "@sentry/core": "10.10.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@sentry-internal/feedback": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry-internal/feedback/-/feedback-10.10.0.tgz", + "integrity": "sha512-oSU4F/ebOsJA9Eof0me9hLpSDTSelpnEY6gmhU9sHyIG+U7hJRuCfeGICxQOzBtteepWRhAaZEv4s9ZBh3iD2w==", + "license": "MIT", + "dependencies": { + "@sentry/core": "10.10.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@sentry-internal/replay": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry-internal/replay/-/replay-10.10.0.tgz", + "integrity": "sha512-sKFYWBaft0ET6gd5B0pThR6gYTjaUECXCzVAnSYxy64a2/PK6lV93BtnA1C2Q34Yhv/0scdyIbZtfTnSsEgwUg==", + "license": "MIT", + "dependencies": { + "@sentry-internal/browser-utils": "10.10.0", + "@sentry/core": "10.10.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@sentry-internal/replay-canvas": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry-internal/replay-canvas/-/replay-canvas-10.10.0.tgz", + "integrity": "sha512-mJBNB0EBbE3vzL7lgd8lDoWWhRaRwxXdI4Kkx3r39u2+1qTdJP/xHbJDihyemCaw7gRL1FR/GC44JLipzEfkKQ==", + "license": "MIT", + "dependencies": { + "@sentry-internal/replay": "10.10.0", + "@sentry/core": "10.10.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@sentry/angular": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry/angular/-/angular-10.10.0.tgz", + "integrity": "sha512-QlaVlkZHwAsZGWaWbCKAwrjFHB78IbExybVGl4wpuaJtZHUm7hS595jndTNeMW7yOjTXGINTlW5xRiSuuZ3tlw==", + "license": "MIT", + "dependencies": { + "@sentry/browser": "10.10.0", + "@sentry/core": "10.10.0", + "tslib": "^2.4.1" + }, + "engines": { + "node": ">=18" + }, + "peerDependencies": { + "@angular/common": ">= 14.x <= 20.x", + "@angular/core": ">= 14.x <= 20.x", + "@angular/router": ">= 14.x <= 20.x", + "rxjs": "^6.5.5 || ^7.x" + } + }, + "node_modules/@sentry/browser": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry/browser/-/browser-10.10.0.tgz", + "integrity": "sha512-STBs29meUk0CvluIOXXnnRGRtjKsJN9fAHS3dUu3GMjmow4rxKBiBbAwoPYftAVdfvGypT7zQCQ+K30dbRxp0g==", + "license": "MIT", + "dependencies": { + "@sentry-internal/browser-utils": "10.10.0", + "@sentry-internal/feedback": "10.10.0", + "@sentry-internal/replay": "10.10.0", + "@sentry-internal/replay-canvas": "10.10.0", + "@sentry/core": "10.10.0" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/@sentry/core": { + "version": "10.10.0", + "resolved": "https://registry.npmjs.org/@sentry/core/-/core-10.10.0.tgz", + "integrity": "sha512-4O1O6my/vYE98ZgfEuLEwOOuHzqqzfBT6IdRo1yiQM7/AXcmSl0H/k4HJtXCiCTiHm+veEuTDBHp0GQZmpIbtA==", + "license": "MIT", + "engines": { + "node": ">=18" + } + }, "node_modules/@sigstore/bundle": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/@sigstore/bundle/-/bundle-3.1.0.tgz", diff --git a/package.json b/package.json index 9d0b3e1b9..1fd57c920 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "@ngxs/logger-plugin": "^19.0.0", "@ngxs/store": "^19.0.0", "@primeng/themes": "^19.0.9", + "@sentry/angular": "^10.10.0", "ace-builds": "^1.42.0", "cedar-artifact-viewer": "^0.9.5", "cedar-embeddable-editor": "^1.5.0", diff --git a/src/app/core/models/config.model.ts b/src/app/core/models/config.model.ts new file mode 100644 index 000000000..b5df579a7 --- /dev/null +++ b/src/app/core/models/config.model.ts @@ -0,0 +1,6 @@ +export type ConfigModelType = string | number | boolean | null; + +export interface ConfigModel { + sentryDsn: string; + [key: string]: ConfigModelType; +} diff --git a/src/app/core/services/osf-config.service.spec.ts b/src/app/core/services/osf-config.service.spec.ts new file mode 100644 index 000000000..ea7120b3f --- /dev/null +++ b/src/app/core/services/osf-config.service.spec.ts @@ -0,0 +1,40 @@ +import { HttpTestingController } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; + +import { ConfigModel } from '@core/models/config.model'; + +import { OSFConfigService } from './osf-config.service'; + +import { OSFTestingModule } from '@testing/osf.testing.module'; + +describe('Service: Config', () => { + let service: OSFConfigService; + let httpMock: HttpTestingController; + + const mockConfig: ConfigModel = { + apiUrl: 'https://api.example.com', + environment: 'staging', + featureToggle: true, + customKey: 'customValue', + } as any; // Cast to any if index signature isn’t added + + beforeEach(async () => { + jest.clearAllMocks(); + await TestBed.configureTestingModule({ + imports: [OSFTestingModule], + providers: [OSFConfigService], + }).compileComponents(); + + service = TestBed.inject(OSFConfigService); + httpMock = TestBed.inject(HttpTestingController); + }); + + it('should return a value with get()', () => { + const apiUrl = service.get('apiUrl'); + expect(apiUrl()).toBeNull(); + httpMock.expectOne('/assets/config/config.json').flush(mockConfig); + expect(apiUrl()).toBe('https://api.example.com'); + expect(service.get('featureToggle')()).toBe(true); + expect(service.get('nonexistentKey')()).toBeNull(); + }); +}); diff --git a/src/app/core/services/osf-config.service.ts b/src/app/core/services/osf-config.service.ts new file mode 100644 index 000000000..5b2a79d15 --- /dev/null +++ b/src/app/core/services/osf-config.service.ts @@ -0,0 +1,69 @@ +import { HttpClient } from '@angular/common/http'; +import { inject, Injectable, Signal, signal, WritableSignal } from '@angular/core'; + +import { ConfigModel, ConfigModelType } from '@core/models/config.model'; + +/** + * Service for managing and accessing configuration values loaded from a JSON file. + * Config values are stored as signals to enable reactive access in Angular components. + */ +@Injectable({ providedIn: 'root' }) +export class OSFConfigService { + /** + * Internal map of config keys to reactive signals. + * Each key corresponds to a configuration field, and its value is a WritableSignal holding the config value. + */ + private configMap = new Map>(); + + /** + * HTTP client used to fetch the configuration file. + */ + private http: HttpClient = inject(HttpClient); + + /** + * Flag to ensure config is only loaded once. + */ + private isLoaded = false; + + /** + * Loads the configuration file from `/assets/config/config.json`. + * Populates `configMap` with keys and reactive signals. + * If a key already exists, it updates the signal value instead. + */ + private load(): void { + this.http.get('/assets/config/config.json').subscribe({ + next: (config: ConfigModel) => { + for (const key of Object.keys(config) as (keyof ConfigModel)[]) { + const keyString = key as string; + const value = config[key]; + + if (this.configMap.has(keyString)) { + this.configMap.get(keyString)!.set(value); + } else { + this.configMap.set(keyString, signal(value)); + } + this.isLoaded = true; + } + }, + }); + } + + /** + * Retrieves a reactive signal for a given config key. + * If the config is not yet loaded, it triggers the loading process. + * If the key does not exist, it initializes the signal with `null`. + * + * @param key - The configuration key to retrieve. + * @returns A `Signal` representing the value of the key. + */ + get(key: string): Signal { + if (!this.isLoaded) { + this.load(); + } + + if (!this.configMap.has(key)) { + this.configMap.set(key, signal(null)); + } + return this.configMap.get(key)!; + } +} diff --git a/src/assets/config/.git-keep b/src/assets/config/.git-keep new file mode 100644 index 000000000..e69de29bb From 9ca933c57f3b0aa86bcadc95427bdd07537acdb1 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Mon, 8 Sep 2025 16:22:22 -0500 Subject: [PATCH 2/9] feat(sentry): added sentry to the app and state-error handler with tests --- src/app/app.config.ts | 10 ++- src/app/core/factory/sentry.factory.spec.ts | 65 +++++++++++++++++++ src/app/core/factory/sentry.factory.ts | 60 +++++++++++++++++ .../helpers/state-error.handler.spec.ts | 52 +++++++++++++++ src/app/shared/helpers/state-error.handler.ts | 7 ++ src/main.ts | 5 +- 6 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 src/app/core/factory/sentry.factory.spec.ts create mode 100644 src/app/core/factory/sentry.factory.ts create mode 100644 src/app/shared/helpers/state-error.handler.spec.ts diff --git a/src/app/app.config.ts b/src/app/app.config.ts index 4f619d1f6..a91939df5 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -12,13 +12,15 @@ import { provideAnimations } from '@angular/platform-browser/animations'; import { provideRouter } from '@angular/router'; import { STATES } from '@core/constants'; +import { SENTRY_PROVIDER } from '@core/factory/sentry.factory'; import { provideTranslation } from '@core/helpers'; -import { GlobalErrorHandler } from './core/handlers'; import { authInterceptor, errorInterceptor, viewOnlyInterceptor } from './core/interceptors'; import CustomPreset from './core/theme/custom-preset'; import { routes } from './app.routes'; +import * as Sentry from '@sentry/angular'; + export const appConfig: ApplicationConfig = { providers: [ provideZoneChangeDetection({ eventCoalescing: true }), @@ -41,6 +43,10 @@ export const appConfig: ApplicationConfig = { importProvidersFrom(TranslateModule.forRoot(provideTranslation())), ConfirmationService, MessageService, - { provide: ErrorHandler, useClass: GlobalErrorHandler }, + SENTRY_PROVIDER, + { + provide: ErrorHandler, + useFactory: () => Sentry.createErrorHandler({ showDialog: false }), + }, ], }; diff --git a/src/app/core/factory/sentry.factory.spec.ts b/src/app/core/factory/sentry.factory.spec.ts new file mode 100644 index 000000000..adaf6e284 --- /dev/null +++ b/src/app/core/factory/sentry.factory.spec.ts @@ -0,0 +1,65 @@ +import { runInInjectionContext, signal } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { OSFConfigService } from '@core/services/osf-config.service'; + +import { initializeSentry } from './sentry.factory'; + +import * as Sentry from '@sentry/angular'; +import { OSFTestingModule } from '@testing/osf.testing.module'; + +jest.mock('@sentry/angular', () => ({ + init: jest.fn(), + createErrorHandler: jest.fn(() => 'mockErrorHandler'), +})); + +describe('factory: sentry', () => { + const configServiceMock = { + get: jest.fn(), + } as unknown as jest.Mocked; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [OSFTestingModule], + providers: [ + { + provide: OSFConfigService, + useValue: configServiceMock, + }, + ], + }).compileComponents(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should initialize Sentry if DSN is provided', async () => { + const service = TestBed.inject(OSFConfigService); + jest.spyOn(service, 'get').mockReturnValue(signal('https://dsn.url')); + await runInInjectionContext(TestBed, async () => { + await initializeSentry()(); + }); + + configServiceMock.get.mockReturnValue(signal('https://dsn.url')); + + expect(Sentry.init).toHaveBeenCalledWith({ + dsn: 'https://dsn.url', + integrations: [], + environment: 'development', + maxBreadcrumbs: 50, + sampleRate: 1, + }); + }); + + it('should initialize Sentry if DSN is missind', async () => { + const service = TestBed.inject(OSFConfigService); + jest.spyOn(service, 'get').mockReturnValue(signal(null)); + await runInInjectionContext(TestBed, async () => { + await initializeSentry(0)(); + }); + + expect(service.get).toHaveBeenCalledTimes(20); + expect(Sentry.init).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/core/factory/sentry.factory.ts b/src/app/core/factory/sentry.factory.ts new file mode 100644 index 000000000..f0c2b4135 --- /dev/null +++ b/src/app/core/factory/sentry.factory.ts @@ -0,0 +1,60 @@ +import { inject, provideAppInitializer } from '@angular/core'; + +import { ENVIRONMENT } from '@core/constants/environment.token'; +import { OSFConfigService } from '@core/services/osf-config.service'; + +import * as Sentry from '@sentry/angular'; + +/** + * Asynchronous initializer function that loads the Sentry DSN from the config service + * and initializes Sentry at application bootstrap. + * + * This function is meant to be used with `provideAppInitializer`, which blocks Angular + * bootstrap until the Promise resolves. This avoids race conditions when reading config. + * + * @param timeout - Optional timeout in milliseconds to wait for the DSN to be available. + * + * @returns A Promise that resolves once Sentry is initialized (or skipped if no DSN) + */ +export function initializeSentry(timeout = 100) { + return async () => { + const configService = inject(OSFConfigService); + const environment = inject(ENVIRONMENT); + + // Await the signal being populated (polling every 100ms) + let dsn: string | null = null; + for (let i = 0; i < 20; i++) { + dsn = configService.get('sentryDsn')()?.toString() || null; + if (dsn) break; + await new Promise((r) => setTimeout(r, timeout)); + } + + if (!dsn) { + return; + } + + // More Options + // https://docs.sentry.io/platforms/javascript/guides/angular/configuration/options/ + Sentry.init({ + dsn, + environment: environment.production ? 'production' : 'development', + maxBreadcrumbs: 50, + sampleRate: 1.0, // error sample rate + integrations: [], + }); + }; +} + +/** + * Provides the Sentry initialization logic during Angular's application bootstrap phase. + * + * This uses `provideAppInitializer` to block application startup until Sentry is initialized. + * It ensures that the Sentry DSN (fetched from OSFConfigService) is available and configured + * before any errors are handled or reported by the app. + * + * `initializeSentry` is a function that returns a Promise which resolves after Sentry is fully initialized. + * + * @see https://docs.sentry.io/platforms/javascript/guides/angular/ + * @see Angular's `provideAppInitializer`: https://angular.io/api/core/provideAppInitializer + */ +export const SENTRY_PROVIDER = provideAppInitializer(initializeSentry()); diff --git a/src/app/shared/helpers/state-error.handler.spec.ts b/src/app/shared/helpers/state-error.handler.spec.ts new file mode 100644 index 000000000..ec4a4ba4d --- /dev/null +++ b/src/app/shared/helpers/state-error.handler.spec.ts @@ -0,0 +1,52 @@ +import { StateContext } from '@ngxs/store'; + +import { firstValueFrom } from 'rxjs'; + +import { handleSectionError } from './state-error.handler'; // adjust path as needed + +import * as Sentry from '@sentry/angular'; + +jest.mock('@sentry/angular'); + +describe('Helper: State Error Handler', () => { + interface TestState { + mySection: { + isLoading: boolean; + isSubmitting: boolean; + error?: string; + otherField?: string; + }; + } + + it('should patch the state and throw the error', async () => { + const patchState = jest.fn(); + const ctx: StateContext = { + getState: () => ({ + mySection: { + isLoading: true, + isSubmitting: true, + otherField: 'someValue', + }, + }), + patchState, + setState: jest.fn(), + dispatch: jest.fn(), + }; + + const error = new Error('Something went wrong'); + + const result$ = handleSectionError(ctx, 'mySection', error); + + expect(patchState).toHaveBeenCalledWith({ + mySection: { + isLoading: false, + isSubmitting: false, + error: 'Something went wrong', + otherField: 'someValue', + }, + }); + + expect(Sentry.captureException).toHaveBeenCalledWith(error); + await expect(firstValueFrom(result$)).rejects.toThrow('Something went wrong'); + }); +}); diff --git a/src/app/shared/helpers/state-error.handler.ts b/src/app/shared/helpers/state-error.handler.ts index 5af177f08..6b2fa3dcf 100644 --- a/src/app/shared/helpers/state-error.handler.ts +++ b/src/app/shared/helpers/state-error.handler.ts @@ -2,7 +2,13 @@ import { StateContext } from '@ngxs/store'; import { throwError } from 'rxjs'; +import * as Sentry from '@sentry/angular'; + export function handleSectionError(ctx: StateContext, section: keyof T, error: Error) { + // Report error to Sentry + Sentry.captureException(error); + + // Patch the state to update loading/submitting flags and set the error message ctx.patchState({ [section]: { ...ctx.getState()[section], @@ -11,5 +17,6 @@ export function handleSectionError(ctx: StateContext, section: keyof T, er error: error.message, }, } as Partial); + // Rethrow the error as an observable return throwError(() => error); } diff --git a/src/main.ts b/src/main.ts index dde5c92e3..62b5a3c94 100644 --- a/src/main.ts +++ b/src/main.ts @@ -8,4 +8,7 @@ import 'cedar-embeddable-editor'; bootstrapApplication(AppComponent, { providers: [...appConfig.providers], -}).catch((err) => console.error(err)); +}).catch((err) => + // eslint-disable-next-line no-console + console.error(err) +); From 2ae9daffc40425c61846afa2fbe2e0f3c65c1c88 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Tue, 9 Sep 2025 16:17:33 -0500 Subject: [PATCH 3/9] feat(promise): added a promise for application loading --- src/app/core/factory/sentry.factory.spec.ts | 11 +-- src/app/core/factory/sentry.factory.ts | 12 +-- .../core/services/osf-config.service.spec.ts | 11 +-- src/app/core/services/osf-config.service.ts | 77 ++++++++----------- 4 files changed, 45 insertions(+), 66 deletions(-) diff --git a/src/app/core/factory/sentry.factory.spec.ts b/src/app/core/factory/sentry.factory.spec.ts index adaf6e284..adc546af5 100644 --- a/src/app/core/factory/sentry.factory.spec.ts +++ b/src/app/core/factory/sentry.factory.spec.ts @@ -1,4 +1,4 @@ -import { runInInjectionContext, signal } from '@angular/core'; +import { runInInjectionContext } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { OSFConfigService } from '@core/services/osf-config.service'; @@ -36,13 +36,11 @@ describe('factory: sentry', () => { it('should initialize Sentry if DSN is provided', async () => { const service = TestBed.inject(OSFConfigService); - jest.spyOn(service, 'get').mockReturnValue(signal('https://dsn.url')); + jest.spyOn(service, 'get').mockResolvedValue('https://dsn.url'); await runInInjectionContext(TestBed, async () => { await initializeSentry()(); }); - configServiceMock.get.mockReturnValue(signal('https://dsn.url')); - expect(Sentry.init).toHaveBeenCalledWith({ dsn: 'https://dsn.url', integrations: [], @@ -54,12 +52,11 @@ describe('factory: sentry', () => { it('should initialize Sentry if DSN is missind', async () => { const service = TestBed.inject(OSFConfigService); - jest.spyOn(service, 'get').mockReturnValue(signal(null)); + jest.spyOn(service, 'get').mockResolvedValue(null); await runInInjectionContext(TestBed, async () => { - await initializeSentry(0)(); + await initializeSentry()(); }); - expect(service.get).toHaveBeenCalledTimes(20); expect(Sentry.init).not.toHaveBeenCalled(); }); }); diff --git a/src/app/core/factory/sentry.factory.ts b/src/app/core/factory/sentry.factory.ts index f0c2b4135..a3b229f43 100644 --- a/src/app/core/factory/sentry.factory.ts +++ b/src/app/core/factory/sentry.factory.ts @@ -12,22 +12,14 @@ import * as Sentry from '@sentry/angular'; * This function is meant to be used with `provideAppInitializer`, which blocks Angular * bootstrap until the Promise resolves. This avoids race conditions when reading config. * - * @param timeout - Optional timeout in milliseconds to wait for the DSN to be available. - * * @returns A Promise that resolves once Sentry is initialized (or skipped if no DSN) */ -export function initializeSentry(timeout = 100) { +export function initializeSentry() { return async () => { const configService = inject(OSFConfigService); const environment = inject(ENVIRONMENT); - // Await the signal being populated (polling every 100ms) - let dsn: string | null = null; - for (let i = 0; i < 20; i++) { - dsn = configService.get('sentryDsn')()?.toString() || null; - if (dsn) break; - await new Promise((r) => setTimeout(r, timeout)); - } + const dsn = await configService.get('sentryDsn'); if (!dsn) { return; diff --git a/src/app/core/services/osf-config.service.spec.ts b/src/app/core/services/osf-config.service.spec.ts index ea7120b3f..65fb6dac0 100644 --- a/src/app/core/services/osf-config.service.spec.ts +++ b/src/app/core/services/osf-config.service.spec.ts @@ -29,12 +29,13 @@ describe('Service: Config', () => { httpMock = TestBed.inject(HttpTestingController); }); - it('should return a value with get()', () => { + it('should return a value with get()', async () => { const apiUrl = service.get('apiUrl'); - expect(apiUrl()).toBeNull(); httpMock.expectOne('/assets/config/config.json').flush(mockConfig); - expect(apiUrl()).toBe('https://api.example.com'); - expect(service.get('featureToggle')()).toBe(true); - expect(service.get('nonexistentKey')()).toBeNull(); + expect(await apiUrl).toBe('https://api.example.com'); + expect(await service.get('featureToggle')).toBe(true); + expect(await service.get('nonexistentKey')).toBeNull(); + + expect(httpMock.verify()).toBeUndefined(); }); }); diff --git a/src/app/core/services/osf-config.service.ts b/src/app/core/services/osf-config.service.ts index 5b2a79d15..a08a1e9b7 100644 --- a/src/app/core/services/osf-config.service.ts +++ b/src/app/core/services/osf-config.service.ts @@ -1,69 +1,58 @@ +import { lastValueFrom, shareReplay } from 'rxjs'; + import { HttpClient } from '@angular/common/http'; -import { inject, Injectable, Signal, signal, WritableSignal } from '@angular/core'; +import { inject, Injectable } from '@angular/core'; -import { ConfigModel, ConfigModelType } from '@core/models/config.model'; +import { ConfigModel } from '@core/models/config.model'; /** - * Service for managing and accessing configuration values loaded from a JSON file. - * Config values are stored as signals to enable reactive access in Angular components. + * Service for loading and accessing configuration values + * from the static JSON file at `/assets/config/config.json`. + * + * This service ensures that the configuration is only fetched once + * and made available application-wide via promise-based access. + * + * Consumers must call `get()` or `has()` using `await` to ensure + * that config values are available after loading completes. */ @Injectable({ providedIn: 'root' }) export class OSFConfigService { /** - * Internal map of config keys to reactive signals. - * Each key corresponds to a configuration field, and its value is a WritableSignal holding the config value. - */ - private configMap = new Map>(); - - /** - * HTTP client used to fetch the configuration file. + * Angular's HttpClient used to fetch the configuration JSON. + * Injected via Angular's dependency injection system. */ private http: HttpClient = inject(HttpClient); /** - * Flag to ensure config is only loaded once. + * Stores the loaded configuration object after it is fetched from the server. + * Remains `null` until `load()` is successfully called. */ - private isLoaded = false; + private config: ConfigModel | null = null; /** - * Loads the configuration file from `/assets/config/config.json`. - * Populates `configMap` with keys and reactive signals. - * If a key already exists, it updates the signal value instead. + * Loads the configuration from the JSON file if not already loaded. + * Ensures that only one request is made. */ - private load(): void { - this.http.get('/assets/config/config.json').subscribe({ - next: (config: ConfigModel) => { - for (const key of Object.keys(config) as (keyof ConfigModel)[]) { - const keyString = key as string; - const value = config[key]; + private async load(): Promise { + if (!this.config) { + this.config = await lastValueFrom( + this.http.get('/assets/config/config.json').pipe(shareReplay(1)) + ); + } - if (this.configMap.has(keyString)) { - this.configMap.get(keyString)!.set(value); - } else { - this.configMap.set(keyString, signal(value)); - } - this.isLoaded = true; - } - }, - }); + return this.config; } /** - * Retrieves a reactive signal for a given config key. - * If the config is not yet loaded, it triggers the loading process. - * If the key does not exist, it initializes the signal with `null`. - * - * @param key - The configuration key to retrieve. - * @returns A `Signal` representing the value of the key. + * Retrieves a configuration value by key after ensuring the config is loaded. + * @param key The key to look up in the config. + * @returns A promise that resolves to the value of the configuration key. */ - get(key: string): Signal { - if (!this.isLoaded) { - this.load(); + async get(key: T): Promise { + if (!this.config) { + await this.load(); } - if (!this.configMap.has(key)) { - this.configMap.set(key, signal(null)); - } - return this.configMap.get(key)!; + return this.config?.[key] || null; } } From 732f32070b0fbef66bc6e8b36d964501a2ef7e9a Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Tue, 9 Sep 2025 16:22:27 -0500 Subject: [PATCH 4/9] refactor(rename): renamed the files and consts to be more explicit --- src/app/app.config.ts | 4 ++-- ...y.spec.ts => application.initialization.factory.spec.ts} | 6 +++--- ...try.factory.ts => application.initialization.factory.ts} | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/app/core/factory/{sentry.factory.spec.ts => application.initialization.factory.spec.ts} (91%) rename src/app/core/factory/{sentry.factory.ts => application.initialization.factory.ts} (92%) diff --git a/src/app/app.config.ts b/src/app/app.config.ts index a91939df5..50f34dbfd 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -12,7 +12,7 @@ import { provideAnimations } from '@angular/platform-browser/animations'; import { provideRouter } from '@angular/router'; import { STATES } from '@core/constants'; -import { SENTRY_PROVIDER } from '@core/factory/sentry.factory'; +import { APPLICATION_INITIALIZATION_PROVIDER } from '@core/factory/application.initialization.factory'; import { provideTranslation } from '@core/helpers'; import { authInterceptor, errorInterceptor, viewOnlyInterceptor } from './core/interceptors'; @@ -43,7 +43,7 @@ export const appConfig: ApplicationConfig = { importProvidersFrom(TranslateModule.forRoot(provideTranslation())), ConfirmationService, MessageService, - SENTRY_PROVIDER, + APPLICATION_INITIALIZATION_PROVIDER, { provide: ErrorHandler, useFactory: () => Sentry.createErrorHandler({ showDialog: false }), diff --git a/src/app/core/factory/sentry.factory.spec.ts b/src/app/core/factory/application.initialization.factory.spec.ts similarity index 91% rename from src/app/core/factory/sentry.factory.spec.ts rename to src/app/core/factory/application.initialization.factory.spec.ts index adc546af5..83e6a9a67 100644 --- a/src/app/core/factory/sentry.factory.spec.ts +++ b/src/app/core/factory/application.initialization.factory.spec.ts @@ -3,7 +3,7 @@ import { TestBed } from '@angular/core/testing'; import { OSFConfigService } from '@core/services/osf-config.service'; -import { initializeSentry } from './sentry.factory'; +import { initializeApplication } from './application.initialization.factory'; import * as Sentry from '@sentry/angular'; import { OSFTestingModule } from '@testing/osf.testing.module'; @@ -38,7 +38,7 @@ describe('factory: sentry', () => { const service = TestBed.inject(OSFConfigService); jest.spyOn(service, 'get').mockResolvedValue('https://dsn.url'); await runInInjectionContext(TestBed, async () => { - await initializeSentry()(); + await initializeApplication()(); }); expect(Sentry.init).toHaveBeenCalledWith({ @@ -54,7 +54,7 @@ describe('factory: sentry', () => { const service = TestBed.inject(OSFConfigService); jest.spyOn(service, 'get').mockResolvedValue(null); await runInInjectionContext(TestBed, async () => { - await initializeSentry()(); + await initializeApplication()(); }); expect(Sentry.init).not.toHaveBeenCalled(); diff --git a/src/app/core/factory/sentry.factory.ts b/src/app/core/factory/application.initialization.factory.ts similarity index 92% rename from src/app/core/factory/sentry.factory.ts rename to src/app/core/factory/application.initialization.factory.ts index a3b229f43..926c00650 100644 --- a/src/app/core/factory/sentry.factory.ts +++ b/src/app/core/factory/application.initialization.factory.ts @@ -14,7 +14,7 @@ import * as Sentry from '@sentry/angular'; * * @returns A Promise that resolves once Sentry is initialized (or skipped if no DSN) */ -export function initializeSentry() { +export function initializeApplication() { return async () => { const configService = inject(OSFConfigService); const environment = inject(ENVIRONMENT); @@ -49,4 +49,4 @@ export function initializeSentry() { * @see https://docs.sentry.io/platforms/javascript/guides/angular/ * @see Angular's `provideAppInitializer`: https://angular.io/api/core/provideAppInitializer */ -export const SENTRY_PROVIDER = provideAppInitializer(initializeSentry()); +export const APPLICATION_INITIALIZATION_PROVIDER = provideAppInitializer(initializeApplication()); From 38968879de64765adef93f2d8ff720e809cd4669 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Tue, 9 Sep 2025 16:38:14 -0500 Subject: [PATCH 5/9] feat(google-tag-manager): added a google tag manager factor --- src/app/app.config.ts | 2 + .../google-tag-manager.factory.spec.ts | 33 +++++++++++++++++ .../factory/google-tag-manager.factory.ts | 37 +++++++++++++++++++ src/app/core/models/config.model.ts | 32 ++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 src/app/core/factory/google-tag-manager.factory.spec.ts create mode 100644 src/app/core/factory/google-tag-manager.factory.ts diff --git a/src/app/app.config.ts b/src/app/app.config.ts index 50f34dbfd..dda4595de 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -13,6 +13,7 @@ import { provideRouter } from '@angular/router'; import { STATES } from '@core/constants'; import { APPLICATION_INITIALIZATION_PROVIDER } from '@core/factory/application.initialization.factory'; +import { GOOGLE_TAG_MANAGER_ID_PROVIDER } from '@core/factory/google-tag-manager.factory'; import { provideTranslation } from '@core/helpers'; import { authInterceptor, errorInterceptor, viewOnlyInterceptor } from './core/interceptors'; @@ -44,6 +45,7 @@ export const appConfig: ApplicationConfig = { ConfirmationService, MessageService, APPLICATION_INITIALIZATION_PROVIDER, + GOOGLE_TAG_MANAGER_ID_PROVIDER, { provide: ErrorHandler, useFactory: () => Sentry.createErrorHandler({ showDialog: false }), diff --git a/src/app/core/factory/google-tag-manager.factory.spec.ts b/src/app/core/factory/google-tag-manager.factory.spec.ts new file mode 100644 index 000000000..00b6fc169 --- /dev/null +++ b/src/app/core/factory/google-tag-manager.factory.spec.ts @@ -0,0 +1,33 @@ +import { TestBed } from '@angular/core/testing'; + +import { OSFConfigService } from '@core/services/osf-config.service'; + +import { GOOGLE_TAG_MANAGER_ID, GOOGLE_TAG_MANAGER_ID_PROVIDER } from './google-tag-manager.factory'; + +describe('factory: google tag manager', () => { + const mockGtmId = 'GTM-TEST123'; + + let configServiceMock: jest.Mocked; + + beforeEach(async () => { + configServiceMock = { + get: jest.fn().mockResolvedValue(mockGtmId), + } as unknown as jest.Mocked; + + await TestBed.configureTestingModule({ + providers: [ + { + provide: OSFConfigService, + useValue: configServiceMock, + }, + GOOGLE_TAG_MANAGER_ID_PROVIDER, + ], + }).compileComponents(); + }); + + it('should return the GTM ID from the config service', async () => { + const gtmId = await TestBed.inject(GOOGLE_TAG_MANAGER_ID); + expect(gtmId).toBe(mockGtmId); + expect(configServiceMock.get).toHaveBeenCalledWith('googleTagManagerId'); + }); +}); diff --git a/src/app/core/factory/google-tag-manager.factory.ts b/src/app/core/factory/google-tag-manager.factory.ts new file mode 100644 index 000000000..41ee75a18 --- /dev/null +++ b/src/app/core/factory/google-tag-manager.factory.ts @@ -0,0 +1,37 @@ +import { inject, InjectionToken, Provider } from '@angular/core'; + +import { OSFConfigService } from '@core/services/osf-config.service'; + +/** + * Injection token used to provide the Google Tag Manager ID (GTM ID). + * + * This token allows the GTM ID to be injected anywhere in the Angular application + * using Angular's dependency injection system. + * + * Example usage: + * ```ts + * const gtmId = inject(GOOGLE_TAG_MANAGER_ID); + * ``` + */ +export const GOOGLE_TAG_MANAGER_ID = new InjectionToken('googleTagManagerId'); + +/** + * Angular provider that asynchronously retrieves the Google Tag Manager ID from + * the OSFConfigService and registers it under the `GOOGLE_TAG_MANAGER_ID` token. + * + * This provider should be used when the GTM ID is stored in a configuration file + * loaded at app initialization. It ensures that the ID can be injected and used + * in components or services after configuration has been loaded. + * + * Example: + * ```ts + * providers: [GOOGLE_TAG_MANAGER_ID_PROVIDER] + * ``` + */ +export const GOOGLE_TAG_MANAGER_ID_PROVIDER: Provider = { + provide: GOOGLE_TAG_MANAGER_ID, + useFactory: async () => { + const configService = inject(OSFConfigService); + return await configService.get('googleTagManagerId'); + }, +}; diff --git a/src/app/core/models/config.model.ts b/src/app/core/models/config.model.ts index b5df579a7..e8e7d152a 100644 --- a/src/app/core/models/config.model.ts +++ b/src/app/core/models/config.model.ts @@ -1,6 +1,38 @@ export type ConfigModelType = string | number | boolean | null; +/** + * Interface representing the application-wide configuration model + * loaded from `assets/config/config.json`. + * + * This config supports both strongly typed properties and dynamic keys. + * + */ export interface ConfigModel { + /** + * The DSN (Data Source Name) used to configure Sentry for error tracking. + * This string is provided by Sentry and uniquely identifies your project. + * + * @example "https://1234567890abcdef.ingest.sentry.io/1234567" + */ sentryDsn: string; + + /** + * The Google Tag Manager ID used to embed GTM scripts for analytics tracking. + * This ID typically starts with "GTM-". + * + * @example "GTM-ABCDE123" + */ + googleTagManagerId: string; + + /** + * A catch-all for additional configuration keys not explicitly defined. + * Each dynamic property maps to a `ConfigModelType` value. + * + * @example + * { + * "featureToggle": true, + * "apiUrl": "https://api.example.com" + * } + */ [key: string]: ConfigModelType; } From e5ed8a45a7da2e2976b9a443c13dae021a8daa23 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Tue, 9 Sep 2025 18:39:46 -0500 Subject: [PATCH 6/9] feat(gtm): added the logic to get the google tag manager working --- package-lock.json | 14 +++++++ package.json | 1 + src/app/app.component.spec.ts | 40 +++++++++++++++---- src/app/app.component.ts | 23 ++++++++++- src/app/app.config.ts | 3 +- ...application.initialization.factory.spec.ts | 2 +- .../application.initialization.factory.ts | 8 +++- .../google-tag-manager.factory.spec.ts | 33 --------------- .../factory/google-tag-manager.factory.ts | 37 ----------------- .../core/services/osf-config.service.spec.ts | 14 ++++--- src/app/core/services/osf-config.service.ts | 12 ++---- 11 files changed, 90 insertions(+), 97 deletions(-) delete mode 100644 src/app/core/factory/google-tag-manager.factory.spec.ts delete mode 100644 src/app/core/factory/google-tag-manager.factory.ts diff --git a/package-lock.json b/package-lock.json index 1a9e23bff..66961e984 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27,6 +27,7 @@ "@primeng/themes": "^19.0.9", "@sentry/angular": "^10.10.0", "ace-builds": "^1.42.0", + "angular-google-tag-manager": "^1.11.0", "cedar-artifact-viewer": "^0.9.5", "cedar-embeddable-editor": "^1.5.0", "chart.js": "^4.4.9", @@ -9083,6 +9084,19 @@ "typescript-eslint": "^8.0.0" } }, + "node_modules/angular-google-tag-manager": { + "version": "1.11.0", + "resolved": "https://registry.npmjs.org/angular-google-tag-manager/-/angular-google-tag-manager-1.11.0.tgz", + "integrity": "sha512-r9sHS+LO9LUoQsiqPo05yTfGRpA3oODc/0AmL0QA1SbeboHKBkCRZIUHkv5w6+GGmWR/G+ZR52eHNLWcgTwIAA==", + "license": "MIT", + "dependencies": { + "tslib": "^2.5.0" + }, + "peerDependencies": { + "@angular/common": "^19.0.0", + "@angular/compiler": "^19.0.0" + } + }, "node_modules/angularx-qrcode": { "version": "19.0.0", "resolved": "https://registry.npmjs.org/angularx-qrcode/-/angularx-qrcode-19.0.0.tgz", diff --git a/package.json b/package.json index 1fd57c920..e3b38309f 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "@primeng/themes": "^19.0.9", "@sentry/angular": "^10.10.0", "ace-builds": "^1.42.0", + "angular-google-tag-manager": "^1.11.0", "cedar-artifact-viewer": "^0.9.5", "cedar-embeddable-editor": "^1.5.0", "chart.js": "^4.4.9", diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 3edc6c1e5..2db622176 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -2,10 +2,13 @@ import { provideStore, Store } from '@ngxs/store'; import { MockComponents } from 'ng-mocks'; +import { Subject } from 'rxjs'; + import { provideHttpClient } from '@angular/common/http'; import { provideHttpClientTesting } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; +import { NavigationEnd, Router } from '@angular/router'; import { GetCurrentUser, UserState } from '@core/store/user'; import { UserEmailsState } from '@core/store/user-emails'; @@ -14,11 +17,21 @@ import { FullScreenLoaderComponent, ToastComponent } from './shared/components'; import { TranslateServiceMock } from './shared/mocks'; import { AppComponent } from './app.component'; -describe('AppComponent', () => { - let component: AppComponent; +import { GoogleTagManagerService } from 'angular-google-tag-manager'; + +describe('Component: App', () => { + let routerEvents$: Subject; + let gtmServiceMock: jest.Mocked; + let fixture: ComponentFixture; beforeEach(async () => { + routerEvents$ = new Subject(); + + gtmServiceMock = { + pushTag: jest.fn(), + } as any; + await TestBed.configureTestingModule({ imports: [AppComponent, ...MockComponents(ToastComponent, FullScreenLoaderComponent)], providers: [ @@ -26,18 +39,20 @@ describe('AppComponent', () => { provideHttpClient(), provideHttpClientTesting(), TranslateServiceMock, + { provide: GoogleTagManagerService, useValue: gtmServiceMock }, + { + provide: Router, + useValue: { + events: routerEvents$.asObservable(), + }, + }, ], }).compileComponents(); fixture = TestBed.createComponent(AppComponent); - component = fixture.componentInstance; fixture.detectChanges(); }); - it('should create', () => { - expect(component).toBeTruthy(); - }); - it('should dispatch GetCurrentUser action on initialization', () => { const store = TestBed.inject(Store); const dispatchSpy = jest.spyOn(store, 'dispatch'); @@ -49,4 +64,15 @@ describe('AppComponent', () => { const routerOutlet = fixture.debugElement.query(By.css('router-outlet')); expect(routerOutlet).toBeTruthy(); }); + + it('should push GTM tag on NavigationEnd', () => { + const event = new NavigationEnd(1, '/previous', '/current'); + + routerEvents$.next(event); + + expect(gtmServiceMock.pushTag).toHaveBeenCalledWith({ + event: 'page', + pageName: '/current', + }); + }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 983562162..4398b5950 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -4,8 +4,11 @@ import { TranslateService } from '@ngx-translate/core'; import { DialogService } from 'primeng/dynamicdialog'; -import { ChangeDetectionStrategy, Component, effect, inject, OnInit } from '@angular/core'; -import { Router, RouterOutlet } from '@angular/router'; +import { filter } from 'rxjs'; + +import { ChangeDetectionStrategy, Component, DestroyRef, effect, inject, OnInit } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { NavigationEnd, Router, RouterOutlet } from '@angular/router'; import { GetCurrentUser } from '@core/store/user'; import { GetEmails, UserEmailsSelectors } from '@core/store/user-emails'; @@ -13,6 +16,8 @@ import { ConfirmEmailComponent } from '@shared/components'; import { FullScreenLoaderComponent, ToastComponent } from './shared/components'; +import { GoogleTagManagerService } from 'angular-google-tag-manager'; + @Component({ selector: 'osf-root', imports: [RouterOutlet, ToastComponent, FullScreenLoaderComponent], @@ -22,6 +27,8 @@ import { FullScreenLoaderComponent, ToastComponent } from './shared/components'; providers: [DialogService], }) export class AppComponent implements OnInit { + private readonly googleTagManagerService = inject(GoogleTagManagerService); + private readonly destroyRef = inject(DestroyRef); private readonly dialogService = inject(DialogService); private readonly router = inject(Router); private readonly translateService = inject(TranslateService); @@ -41,6 +48,18 @@ export class AppComponent implements OnInit { ngOnInit(): void { this.actions.getCurrentUser(); this.actions.getEmails(); + + this.router.events + .pipe( + filter((event) => event instanceof NavigationEnd), + takeUntilDestroyed(this.destroyRef) + ) + .subscribe((event: NavigationEnd) => { + this.googleTagManagerService.pushTag({ + event: 'page', + pageName: event.urlAfterRedirects, + }); + }); } private showEmailDialog() { diff --git a/src/app/app.config.ts b/src/app/app.config.ts index dda4595de..a606ce22b 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -13,7 +13,6 @@ import { provideRouter } from '@angular/router'; import { STATES } from '@core/constants'; import { APPLICATION_INITIALIZATION_PROVIDER } from '@core/factory/application.initialization.factory'; -import { GOOGLE_TAG_MANAGER_ID_PROVIDER } from '@core/factory/google-tag-manager.factory'; import { provideTranslation } from '@core/helpers'; import { authInterceptor, errorInterceptor, viewOnlyInterceptor } from './core/interceptors'; @@ -44,8 +43,8 @@ export const appConfig: ApplicationConfig = { importProvidersFrom(TranslateModule.forRoot(provideTranslation())), ConfirmationService, MessageService, + APPLICATION_INITIALIZATION_PROVIDER, - GOOGLE_TAG_MANAGER_ID_PROVIDER, { provide: ErrorHandler, useFactory: () => Sentry.createErrorHandler({ showDialog: false }), diff --git a/src/app/core/factory/application.initialization.factory.spec.ts b/src/app/core/factory/application.initialization.factory.spec.ts index 83e6a9a67..0e1703d81 100644 --- a/src/app/core/factory/application.initialization.factory.spec.ts +++ b/src/app/core/factory/application.initialization.factory.spec.ts @@ -52,7 +52,7 @@ describe('factory: sentry', () => { it('should initialize Sentry if DSN is missind', async () => { const service = TestBed.inject(OSFConfigService); - jest.spyOn(service, 'get').mockResolvedValue(null); + jest.spyOn(service, 'get').mockResolvedValue(''); await runInInjectionContext(TestBed, async () => { await initializeApplication()(); }); diff --git a/src/app/core/factory/application.initialization.factory.ts b/src/app/core/factory/application.initialization.factory.ts index 926c00650..c4acf5336 100644 --- a/src/app/core/factory/application.initialization.factory.ts +++ b/src/app/core/factory/application.initialization.factory.ts @@ -4,6 +4,7 @@ import { ENVIRONMENT } from '@core/constants/environment.token'; import { OSFConfigService } from '@core/services/osf-config.service'; import * as Sentry from '@sentry/angular'; +import { GoogleTagManagerConfiguration } from 'angular-google-tag-manager'; /** * Asynchronous initializer function that loads the Sentry DSN from the config service @@ -17,9 +18,14 @@ import * as Sentry from '@sentry/angular'; export function initializeApplication() { return async () => { const configService = inject(OSFConfigService); + const googleTagManagerConfiguration = inject(GoogleTagManagerConfiguration); const environment = inject(ENVIRONMENT); - const dsn = await configService.get('sentryDsn'); + await configService.load(); + + googleTagManagerConfiguration.set({ id: configService.get('googleTagManagerId') }); + + const dsn = configService.get('sentryDsn'); if (!dsn) { return; diff --git a/src/app/core/factory/google-tag-manager.factory.spec.ts b/src/app/core/factory/google-tag-manager.factory.spec.ts deleted file mode 100644 index 00b6fc169..000000000 --- a/src/app/core/factory/google-tag-manager.factory.spec.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { TestBed } from '@angular/core/testing'; - -import { OSFConfigService } from '@core/services/osf-config.service'; - -import { GOOGLE_TAG_MANAGER_ID, GOOGLE_TAG_MANAGER_ID_PROVIDER } from './google-tag-manager.factory'; - -describe('factory: google tag manager', () => { - const mockGtmId = 'GTM-TEST123'; - - let configServiceMock: jest.Mocked; - - beforeEach(async () => { - configServiceMock = { - get: jest.fn().mockResolvedValue(mockGtmId), - } as unknown as jest.Mocked; - - await TestBed.configureTestingModule({ - providers: [ - { - provide: OSFConfigService, - useValue: configServiceMock, - }, - GOOGLE_TAG_MANAGER_ID_PROVIDER, - ], - }).compileComponents(); - }); - - it('should return the GTM ID from the config service', async () => { - const gtmId = await TestBed.inject(GOOGLE_TAG_MANAGER_ID); - expect(gtmId).toBe(mockGtmId); - expect(configServiceMock.get).toHaveBeenCalledWith('googleTagManagerId'); - }); -}); diff --git a/src/app/core/factory/google-tag-manager.factory.ts b/src/app/core/factory/google-tag-manager.factory.ts deleted file mode 100644 index 41ee75a18..000000000 --- a/src/app/core/factory/google-tag-manager.factory.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { inject, InjectionToken, Provider } from '@angular/core'; - -import { OSFConfigService } from '@core/services/osf-config.service'; - -/** - * Injection token used to provide the Google Tag Manager ID (GTM ID). - * - * This token allows the GTM ID to be injected anywhere in the Angular application - * using Angular's dependency injection system. - * - * Example usage: - * ```ts - * const gtmId = inject(GOOGLE_TAG_MANAGER_ID); - * ``` - */ -export const GOOGLE_TAG_MANAGER_ID = new InjectionToken('googleTagManagerId'); - -/** - * Angular provider that asynchronously retrieves the Google Tag Manager ID from - * the OSFConfigService and registers it under the `GOOGLE_TAG_MANAGER_ID` token. - * - * This provider should be used when the GTM ID is stored in a configuration file - * loaded at app initialization. It ensures that the ID can be injected and used - * in components or services after configuration has been loaded. - * - * Example: - * ```ts - * providers: [GOOGLE_TAG_MANAGER_ID_PROVIDER] - * ``` - */ -export const GOOGLE_TAG_MANAGER_ID_PROVIDER: Provider = { - provide: GOOGLE_TAG_MANAGER_ID, - useFactory: async () => { - const configService = inject(OSFConfigService); - return await configService.get('googleTagManagerId'); - }, -}; diff --git a/src/app/core/services/osf-config.service.spec.ts b/src/app/core/services/osf-config.service.spec.ts index 65fb6dac0..65f80469f 100644 --- a/src/app/core/services/osf-config.service.spec.ts +++ b/src/app/core/services/osf-config.service.spec.ts @@ -30,11 +30,15 @@ describe('Service: Config', () => { }); it('should return a value with get()', async () => { - const apiUrl = service.get('apiUrl'); - httpMock.expectOne('/assets/config/config.json').flush(mockConfig); - expect(await apiUrl).toBe('https://api.example.com'); - expect(await service.get('featureToggle')).toBe(true); - expect(await service.get('nonexistentKey')).toBeNull(); + let loadPromise = service.load(); + const request = httpMock.expectOne('/assets/config/config.json'); + request.flush(mockConfig); + await loadPromise; + expect(service.get('apiUrl')).toBe('https://api.example.com'); + expect(service.get('featureToggle')).toBe(true); + loadPromise = service.load(); + await loadPromise; + expect(service.get('nonexistentKey')).toBeNull(); expect(httpMock.verify()).toBeUndefined(); }); diff --git a/src/app/core/services/osf-config.service.ts b/src/app/core/services/osf-config.service.ts index a08a1e9b7..b42d2998e 100644 --- a/src/app/core/services/osf-config.service.ts +++ b/src/app/core/services/osf-config.service.ts @@ -33,26 +33,20 @@ export class OSFConfigService { * Loads the configuration from the JSON file if not already loaded. * Ensures that only one request is made. */ - private async load(): Promise { + async load(): Promise { if (!this.config) { this.config = await lastValueFrom( this.http.get('/assets/config/config.json').pipe(shareReplay(1)) ); } - - return this.config; } /** * Retrieves a configuration value by key after ensuring the config is loaded. * @param key The key to look up in the config. - * @returns A promise that resolves to the value of the configuration key. + * @returns The value of the configuration key. */ - async get(key: T): Promise { - if (!this.config) { - await this.load(); - } - + get(key: T): ConfigModel[T] | null { return this.config?.[key] || null; } } From 76c3eeb38844e058523d6f63272adb7619de625b Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Wed, 10 Sep 2025 11:11:33 -0500 Subject: [PATCH 7/9] feat(pr-review): add code from pr --- .../core/factory/application.initialization.factory.spec.ts | 4 ++++ src/assets/config/template.json | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 src/assets/config/template.json diff --git a/src/app/core/factory/application.initialization.factory.spec.ts b/src/app/core/factory/application.initialization.factory.spec.ts index 0e1703d81..8fdcbadc9 100644 --- a/src/app/core/factory/application.initialization.factory.spec.ts +++ b/src/app/core/factory/application.initialization.factory.spec.ts @@ -36,6 +36,8 @@ describe('factory: sentry', () => { it('should initialize Sentry if DSN is provided', async () => { const service = TestBed.inject(OSFConfigService); + // eslint-disable-next-line + // @ts-ignore jest.spyOn(service, 'get').mockResolvedValue('https://dsn.url'); await runInInjectionContext(TestBed, async () => { await initializeApplication()(); @@ -52,6 +54,8 @@ describe('factory: sentry', () => { it('should initialize Sentry if DSN is missind', async () => { const service = TestBed.inject(OSFConfigService); + // eslint-disable-next-line + // @ts-ignore jest.spyOn(service, 'get').mockResolvedValue(''); await runInInjectionContext(TestBed, async () => { await initializeApplication()(); diff --git a/src/assets/config/template.json b/src/assets/config/template.json new file mode 100644 index 000000000..d407164e3 --- /dev/null +++ b/src/assets/config/template.json @@ -0,0 +1,4 @@ +{ + "sentryDsn": "", + "googleTagManagerId": "" +} From 2e826786bcfaa53aa763c9470fd5ceb40a9913b3 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Wed, 10 Sep 2025 12:22:59 -0500 Subject: [PATCH 8/9] feat(eng-8741): added conditions if the config variables are not present --- README.md | 7 ++ src/app/app.component.spec.ts | 68 +++++++++++++------ src/app/app.component.ts | 24 ++++--- ...application.initialization.factory.spec.ts | 31 ++++++--- .../application.initialization.factory.ts | 28 ++++---- .../core/services/osf-config.service.spec.ts | 14 ++++ src/app/core/services/osf-config.service.ts | 20 ++++++ 7 files changed, 137 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index b2c6bb3e9..8b4fbb3be 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ take up to 60 seconds once the docker build finishes. - Install git commit template: [Commit Template](docs/commit.template.md). - [Volta](#volta) +- 3rd-party tokens [Configuration](#configuration) ### Recommended @@ -59,3 +60,9 @@ npm run test:check-coverage-thresholds OSF uses volta to manage node and npm versions inside of the repository Install Volta from [volta](https://volta.sh/) and it will automatically pin Node/npm per the repo toolchain. + +## Configuration + +OSF uses an `assets/config/config.json` file for any 3rd-party tokens. This file is not committed to the repo. + +There is a `assets/config/template.json` file that can be copied to `assets/config/config.json` to store any 3rd-party tokens locally. diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 2db622176..8e4fc542f 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -4,12 +4,11 @@ import { MockComponents } from 'ng-mocks'; import { Subject } from 'rxjs'; -import { provideHttpClient } from '@angular/common/http'; -import { provideHttpClientTesting } from '@angular/common/http/testing'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { NavigationEnd, Router } from '@angular/router'; +import { OSFConfigService } from '@core/services/osf-config.service'; import { GetCurrentUser, UserState } from '@core/store/user'; import { UserEmailsState } from '@core/store/user-emails'; @@ -17,12 +16,13 @@ import { FullScreenLoaderComponent, ToastComponent } from './shared/components'; import { TranslateServiceMock } from './shared/mocks'; import { AppComponent } from './app.component'; +import { OSFTestingModule } from '@testing/osf.testing.module'; import { GoogleTagManagerService } from 'angular-google-tag-manager'; describe('Component: App', () => { let routerEvents$: Subject; let gtmServiceMock: jest.Mocked; - + let osfConfigServiceMock: OSFConfigService; let fixture: ComponentFixture; beforeEach(async () => { @@ -33,11 +33,9 @@ describe('Component: App', () => { } as any; await TestBed.configureTestingModule({ - imports: [AppComponent, ...MockComponents(ToastComponent, FullScreenLoaderComponent)], + imports: [OSFTestingModule, AppComponent, ...MockComponents(ToastComponent, FullScreenLoaderComponent)], providers: [ provideStore([UserState, UserEmailsState]), - provideHttpClient(), - provideHttpClientTesting(), TranslateServiceMock, { provide: GoogleTagManagerService, useValue: gtmServiceMock }, { @@ -46,33 +44,59 @@ describe('Component: App', () => { events: routerEvents$.asObservable(), }, }, + { + provide: OSFConfigService, + useValue: { + has: jest.fn(), + }, + }, ], }).compileComponents(); + osfConfigServiceMock = TestBed.inject(OSFConfigService); fixture = TestBed.createComponent(AppComponent); - fixture.detectChanges(); }); - it('should dispatch GetCurrentUser action on initialization', () => { - const store = TestBed.inject(Store); - const dispatchSpy = jest.spyOn(store, 'dispatch'); - store.dispatch(GetCurrentUser); - expect(dispatchSpy).toHaveBeenCalledWith(GetCurrentUser); - }); + describe('detect changes', () => { + beforeEach(() => { + fixture.detectChanges(); + }); - it('should render router outlet', () => { - const routerOutlet = fixture.debugElement.query(By.css('router-outlet')); - expect(routerOutlet).toBeTruthy(); + it('should dispatch GetCurrentUser action on initialization', () => { + const store = TestBed.inject(Store); + const dispatchSpy = jest.spyOn(store, 'dispatch'); + store.dispatch(GetCurrentUser); + expect(dispatchSpy).toHaveBeenCalledWith(GetCurrentUser); + }); + + it('should render router outlet', () => { + const routerOutlet = fixture.debugElement.query(By.css('router-outlet')); + expect(routerOutlet).toBeTruthy(); + }); }); - it('should push GTM tag on NavigationEnd', () => { - const event = new NavigationEnd(1, '/previous', '/current'); + describe('Google Tag Manager', () => { + it('should push GTM tag on NavigationEnd with google tag id', () => { + jest.spyOn(osfConfigServiceMock, 'has').mockReturnValue(true); + fixture.detectChanges(); + const event = new NavigationEnd(1, '/previous', '/current'); + + routerEvents$.next(event); + + expect(gtmServiceMock.pushTag).toHaveBeenCalledWith({ + event: 'page', + pageName: '/current', + }); + }); + + it('should not push GTM tag on NavigationEnd with google tag id', () => { + jest.spyOn(osfConfigServiceMock, 'has').mockReturnValue(false); + fixture.detectChanges(); + const event = new NavigationEnd(1, '/previous', '/current'); - routerEvents$.next(event); + routerEvents$.next(event); - expect(gtmServiceMock.pushTag).toHaveBeenCalledWith({ - event: 'page', - pageName: '/current', + expect(gtmServiceMock.pushTag).not.toHaveBeenCalled(); }); }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 4398b5950..8bae9ec75 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -10,6 +10,7 @@ import { ChangeDetectionStrategy, Component, DestroyRef, effect, inject, OnInit import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { NavigationEnd, Router, RouterOutlet } from '@angular/router'; +import { OSFConfigService } from '@core/services/osf-config.service'; import { GetCurrentUser } from '@core/store/user'; import { GetEmails, UserEmailsSelectors } from '@core/store/user-emails'; import { ConfirmEmailComponent } from '@shared/components'; @@ -32,6 +33,7 @@ export class AppComponent implements OnInit { private readonly dialogService = inject(DialogService); private readonly router = inject(Router); private readonly translateService = inject(TranslateService); + private readonly osfConfigService = inject(OSFConfigService); private readonly actions = createDispatchMap({ getCurrentUser: GetCurrentUser, getEmails: GetEmails }); @@ -49,17 +51,19 @@ export class AppComponent implements OnInit { this.actions.getCurrentUser(); this.actions.getEmails(); - this.router.events - .pipe( - filter((event) => event instanceof NavigationEnd), - takeUntilDestroyed(this.destroyRef) - ) - .subscribe((event: NavigationEnd) => { - this.googleTagManagerService.pushTag({ - event: 'page', - pageName: event.urlAfterRedirects, + if (this.osfConfigService.has('googleTagManagerId')) { + this.router.events + .pipe( + filter((event) => event instanceof NavigationEnd), + takeUntilDestroyed(this.destroyRef) + ) + .subscribe((event: NavigationEnd) => { + this.googleTagManagerService.pushTag({ + event: 'page', + pageName: event.urlAfterRedirects, + }); }); - }); + } } private showEmailDialog() { diff --git a/src/app/core/factory/application.initialization.factory.spec.ts b/src/app/core/factory/application.initialization.factory.spec.ts index 8fdcbadc9..9e8222020 100644 --- a/src/app/core/factory/application.initialization.factory.spec.ts +++ b/src/app/core/factory/application.initialization.factory.spec.ts @@ -7,6 +7,7 @@ import { initializeApplication } from './application.initialization.factory'; import * as Sentry from '@sentry/angular'; import { OSFTestingModule } from '@testing/osf.testing.module'; +import { GoogleTagManagerConfiguration } from 'angular-google-tag-manager'; jest.mock('@sentry/angular', () => ({ init: jest.fn(), @@ -14,7 +15,10 @@ jest.mock('@sentry/angular', () => ({ })); describe('factory: sentry', () => { + let osfConfigServiceMock: OSFConfigService; + let googleTagManagerConfigurationMock: GoogleTagManagerConfiguration; const configServiceMock = { + load: jest.fn(), get: jest.fn(), } as unknown as jest.Mocked; @@ -26,8 +30,17 @@ describe('factory: sentry', () => { provide: OSFConfigService, useValue: configServiceMock, }, + { + provide: GoogleTagManagerConfiguration, + useValue: { + set: jest.fn(), + }, + }, ], }).compileComponents(); + + osfConfigServiceMock = TestBed.inject(OSFConfigService); + googleTagManagerConfigurationMock = TestBed.inject(GoogleTagManagerConfiguration); }); afterEach(() => { @@ -35,10 +48,7 @@ describe('factory: sentry', () => { }); it('should initialize Sentry if DSN is provided', async () => { - const service = TestBed.inject(OSFConfigService); - // eslint-disable-next-line - // @ts-ignore - jest.spyOn(service, 'get').mockResolvedValue('https://dsn.url'); + jest.spyOn(osfConfigServiceMock, 'get').mockReturnValueOnce('google-id').mockReturnValueOnce('https://dsn.url'); await runInInjectionContext(TestBed, async () => { await initializeApplication()(); }); @@ -50,17 +60,20 @@ describe('factory: sentry', () => { maxBreadcrumbs: 50, sampleRate: 1, }); + + expect(googleTagManagerConfigurationMock.set).toHaveBeenCalledWith({ + id: 'google-id', + }); }); - it('should initialize Sentry if DSN is missind', async () => { - const service = TestBed.inject(OSFConfigService); - // eslint-disable-next-line - // @ts-ignore - jest.spyOn(service, 'get').mockResolvedValue(''); + it('should initialize Sentry if DSN is missing', async () => { + jest.spyOn(osfConfigServiceMock, 'get').mockReturnValueOnce(null).mockReturnValueOnce(null); await runInInjectionContext(TestBed, async () => { await initializeApplication()(); }); expect(Sentry.init).not.toHaveBeenCalled(); + + expect(googleTagManagerConfigurationMock.set).not.toHaveBeenCalled(); }); }); diff --git a/src/app/core/factory/application.initialization.factory.ts b/src/app/core/factory/application.initialization.factory.ts index c4acf5336..7c17bfcaa 100644 --- a/src/app/core/factory/application.initialization.factory.ts +++ b/src/app/core/factory/application.initialization.factory.ts @@ -23,23 +23,23 @@ export function initializeApplication() { await configService.load(); - googleTagManagerConfiguration.set({ id: configService.get('googleTagManagerId') }); + const googleTagManagerId = configService.get('googleTagManagerId'); + if (googleTagManagerId) { + googleTagManagerConfiguration.set({ id: googleTagManagerId }); + } const dsn = configService.get('sentryDsn'); - - if (!dsn) { - return; + if (dsn) { + // More Options + // https://docs.sentry.io/platforms/javascript/guides/angular/configuration/options/ + Sentry.init({ + dsn, + environment: environment.production ? 'production' : 'development', + maxBreadcrumbs: 50, + sampleRate: 1.0, // error sample rate + integrations: [], + }); } - - // More Options - // https://docs.sentry.io/platforms/javascript/guides/angular/configuration/options/ - Sentry.init({ - dsn, - environment: environment.production ? 'production' : 'development', - maxBreadcrumbs: 50, - sampleRate: 1.0, // error sample rate - integrations: [], - }); }; } diff --git a/src/app/core/services/osf-config.service.spec.ts b/src/app/core/services/osf-config.service.spec.ts index 65f80469f..8068ef17f 100644 --- a/src/app/core/services/osf-config.service.spec.ts +++ b/src/app/core/services/osf-config.service.spec.ts @@ -42,4 +42,18 @@ describe('Service: Config', () => { expect(httpMock.verify()).toBeUndefined(); }); + + it('should return a value with ahs()', async () => { + let loadPromise = service.load(); + const request = httpMock.expectOne('/assets/config/config.json'); + request.flush(mockConfig); + await loadPromise; + expect(service.has('apiUrl')).toBeTruthy(); + expect(service.has('featureToggle')).toBeTruthy(); + loadPromise = service.load(); + await loadPromise; + expect(service.has('nonexistentKey')).toBeFalsy(); + + expect(httpMock.verify()).toBeUndefined(); + }); }); diff --git a/src/app/core/services/osf-config.service.ts b/src/app/core/services/osf-config.service.ts index b42d2998e..872a84a50 100644 --- a/src/app/core/services/osf-config.service.ts +++ b/src/app/core/services/osf-config.service.ts @@ -49,4 +49,24 @@ export class OSFConfigService { get(key: T): ConfigModel[T] | null { return this.config?.[key] || null; } + + /** + * Checks whether a specific configuration key exists and has a truthy value. + * + * This method inspects the currently loaded configuration object and determines + * if the given key is present and evaluates to a truthy value (e.g., non-null, non-undefined, not false/0/empty string). + * + * @template T - A key of the `ConfigModel` interface. + * @param {T} key - The key to check within the configuration object. + * @returns {boolean} - Returns `true` if the key exists and its value is truthy; otherwise, returns `false`. + * + * @example + * if (configService.has('sentryDsn')) { + * const dsn = configService.get('sentryDsn'); + * Sentry.init({ dsn }); + * } + */ + has(key: T): boolean { + return this.config?.[key] ? true : false; + } } From d23e43c08f65c0b51e6dddb3c8af6e8a15252ad9 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Wed, 10 Sep 2025 15:35:32 -0500 Subject: [PATCH 9/9] chore(nit-pick-for-brian-g): add a gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 5598168a1..fca5b5041 100644 --- a/.gitignore +++ b/.gitignore @@ -6,7 +6,7 @@ /tmp /out-tsc /bazel-out -/src/assets/config +/src/assets/config/config.json # Node /node_modules