Merge pull request DSpace#5001 from pzlakowski/fix/bitstream-downloads-fail-when-matomo-active

fix: silently fail when we cannot get visitorId from matomo for bitsream download

Author: tdonohue

src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.spec.ts

@@ -116,6 +116,7 @@ describe('BitstreamDownloadPageComponent', () => {
     matomoService = jasmine.createSpyObj('MatomoService', {
       appendVisitorId: of(''),
       isMatomoEnabled$: of(true),
+      isMatomoScriptLoaded$: of(true),
     });
     matomoService.appendVisitorId.and.callFake((link) => of(link));
   }

src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.ts

@@ -107,26 +107,27 @@ export class BitstreamDownloadPageComponent implements OnInit {
         const isAuthorized$ = this.authorizationService.isAuthorized(FeatureID.CanDownload, isNotEmpty(bitstream) ? bitstream.self : undefined);
         const isLoggedIn$ = this.auth.isAuthenticated();
         const isMatomoEnabled$ = this.matomoService.isMatomoEnabled$();
-        return observableCombineLatest([isAuthorized$, isLoggedIn$, isMatomoEnabled$, accessToken$, of(bitstream)]);
+        const isMatomoScriptLoaded$ = this.matomoService.isMatomoScriptLoaded$();
+        return observableCombineLatest([isAuthorized$, isLoggedIn$, isMatomoEnabled$, isMatomoScriptLoaded$, accessToken$, of(bitstream)]);
       }),
-      filter(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [boolean, boolean, boolean, string, Bitstream]) => (hasValue(isAuthorized) && hasValue(isLoggedIn)) || hasValue(accessToken)),
+      filter(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, accessToken, bitstream]: [boolean, boolean, boolean, boolean, string, Bitstream]) => (hasValue(isAuthorized) && hasValue(isLoggedIn)) || hasValue(accessToken)),
       take(1),
-      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [boolean, boolean, boolean, string, Bitstream]) => {
+      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, accessToken, bitstream]: [boolean, boolean, boolean, boolean, string, Bitstream]) => {
         if (isAuthorized && isLoggedIn) {
           return this.fileService.retrieveFileDownloadLink(bitstream._links.content.href).pipe(
             filter((fileLink) => hasValue(fileLink)),
             take(1),
             map((fileLink) => {
-              return [isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink];
+              return [isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, fileLink];
             }));
         } else if (hasValue(accessToken)) {
-          return [[isAuthorized, !isLoggedIn, isMatomoEnabled, bitstream, '', accessToken]];
+          return [[isAuthorized, !isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, '', accessToken]];
         } else {
-          return [[isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, bitstream._links.content.href]];
+          return [[isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, bitstream._links.content.href]];
         }
       }),
-      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink, accessToken]: [boolean, boolean, boolean, Bitstream, string, string]) => {
-        if (isMatomoEnabled) {
+      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, fileLink, accessToken]: [boolean, boolean, boolean, boolean, Bitstream, string, string]) => {
+        if (isMatomoEnabled && isMatomoScriptLoaded) {
           return this.matomoService.appendVisitorId(fileLink).pipe(
             map((fileLinkWithVisitorId) => [isAuthorized, isLoggedIn, bitstream, fileLinkWithVisitorId, accessToken]),
           );

src/app/statistics/matomo.factory.spec.ts

@@ -0,0 +1,49 @@
+import { Injector } from '@angular/core';
+import { TestBed } from '@angular/core/testing';
+import { OrejimeService } from '@dspace/core/cookies/orejime.service';
+import { ConfigurationDataService } from '@dspace/core/data/configuration-data.service';
+import { NativeWindowService } from '@dspace/core/services/window.service';
+import {
+  MatomoInitializerService,
+  MatomoTracker,
+} from 'ngx-matomo-client';
+import { firstValueFrom } from 'rxjs';
+
+import { customMatomoScriptFactory } from './matomo.factory';
+import { MatomoService } from './matomo.service';
+
+describe('customMatomoScriptFactory', () => {
+  let service: MatomoService;
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      providers: [
+        { provide: MatomoTracker, useValue: {} },
+        { provide: MatomoInitializerService, useValue: {} },
+        { provide: OrejimeService, useValue: {} },
+        { provide: NativeWindowService, useValue: {} },
+        { provide: ConfigurationDataService, useValue: {} },
+        { provide: Injector, useValue: TestBed },
+      ],
+    });
+
+    service = TestBed.inject(MatomoService);
+  });
+
+  it('should notify when the script loads', async () => {
+    const script = customMatomoScriptFactory(service)('', document);
+
+    script.dispatchEvent(new Event('load'));
+    const isMatomoScriptLoaded = await firstValueFrom(service.isMatomoScriptLoaded$());
+
+    expect(isMatomoScriptLoaded).toBeTruthy();
+  });
+
+  it('should notify when the script fails', async () => {
+    const script = customMatomoScriptFactory(service)('', document);
+
+    script.dispatchEvent(new Event('error'));
+    const isMatomoScriptLoaded = await firstValueFrom(service.isMatomoScriptLoaded$());
+
+    expect(isMatomoScriptLoaded).toBeFalsy();
+  });
+});

src/app/statistics/matomo.factory.ts

@@ -0,0 +1,28 @@
+import { createDefaultMatomoScriptElement } from 'ngx-matomo-client';
+
+import { MatomoService } from './matomo.service';
+
+/**
+ * Creates a custom script factory function that integrates with the `MatomoService`.
+ *
+ * @param matomoService - The instance of `MatomoService` used to track the loading state.
+ * @returns A function to initialize script to listen onload/onerror events by MatomoService
+ *
+ * @example
+ * // In your app config or module providers:
+ * {
+ * provide: MATOMO_SCRIPT_FACTORY,
+ * useFactory: customMatomoScriptFactory,
+ * deps: [MatomoService]
+ * }
+ */
+export function customMatomoScriptFactory(matomoService: MatomoService) {
+  return (scriptUrl: string, document: Document): HTMLScriptElement => {
+    const script = createDefaultMatomoScriptElement(scriptUrl, document);
+
+    script.onload = () => matomoService.markAsLoaded();
+    script.onerror = () => matomoService.markAsError();
+
+    return script;
+  };
+}

src/app/statistics/matomo.service.ts

@@ -25,6 +25,7 @@ import {
   from as fromPromise,
   Observable,
   of,
+  ReplaySubject,
 } from 'rxjs';
 import {
   map,
@@ -45,7 +46,6 @@ import { environment } from '../../environments/environment';
  * Provides methods for initializing tracking, managing consent, and appending visitor identifiers.
  */
 export class MatomoService {
-
   /** Injects the MatomoInitializerService to initialize the Matomo tracker. */
   matomoInitializer: MatomoInitializerService;
 
@@ -61,8 +61,25 @@ export class MatomoService {
   /** Injects the ConfigurationService. */
   configService = inject(ConfigurationDataService);
 
+  private statusSubject = new ReplaySubject<'loading' | 'loaded' | 'error'>(1);
+  private status$ = this.statusSubject.asObservable();
+
   constructor(private injector: EnvironmentInjector) {
+    this.statusSubject.next('loading');
+  }
 
+  /**
+   * This method indicates that the Matomo script loaded successfully thus we set state to loaded
+   */
+  markAsLoaded() {
+    this.statusSubject.next('loaded');
+  }
+
+  /**
+   * This method indicates that the Matomo script failed to download or execute and sets state to error
+   */
+  markAsError() {
+    this.statusSubject.next('error');
   }
 
   /**
@@ -165,6 +182,16 @@ export class MatomoService {
       );
   }
 
+  /**
+   * Checks if Matomo script loaded correctly
+   * @returns An Observable that emits a boolean indicating whether Matomo script loaded correctly.
+   */
+  isMatomoScriptLoaded$(): Observable<boolean> {
+    return this.status$.pipe(
+      map(status => status === 'loaded'),
+    );
+  }
+
   /**
    * Appends the visitor ID as a query parameter to the given URL.
    * @param url - The original URL to modify

src/modules/app/browser-app.config.ts

@@ -56,10 +56,13 @@ import {
   Angulartics2RouterlessModule,
 } from 'angulartics2';
 import {
+  MATOMO_SCRIPT_FACTORY,
   provideMatomo,
   withRouteData,
   withRouter,
 } from 'ngx-matomo-client';
+import { customMatomoScriptFactory } from 'src/app/statistics/matomo.factory';
+import { MatomoService } from 'src/app/statistics/matomo.service';
 
 import { commonAppConfig } from '../../app/app.config';
 import { storeModuleConfig } from '../../app/app.reducer';
@@ -169,5 +172,10 @@ export const browserAppConfig: ApplicationConfig = mergeApplicationConfig({
       withRouter(),
       withRouteData(),
     ),
+    {
+      provide: MATOMO_SCRIPT_FACTORY,
+      useFactory: customMatomoScriptFactory,
+      deps: [MatomoService],
+    },
   ],
 }, commonAppConfig);