[DURACOM-339] fix downloads not working when matomo is disabled

Andrea Barbasso · vins01-4science · commit a099e1554ffd · 2025-05-30T13:29:00.000+02:00
diff --git a/src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.spec.ts b/src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.spec.ts
@@ -18,6 +18,7 @@ import { of as observableOf } from 'rxjs';
 import { getForbiddenRoute } from '../../app-routing-paths';
 import { AuthService } from '../../core/auth/auth.service';
 import { DSONameService } from '../../core/breadcrumbs/dso-name.service';
+import { ConfigurationDataService } from '../../core/data/configuration-data.service';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { SignpostingDataService } from '../../core/data/signposting-data.service';
 import { HardRedirectService } from '../../core/services/hard-redirect.service';
@@ -91,16 +92,16 @@ describe('BitstreamDownloadPageComponent', () => {
         self: { href: 'bitstream-self-link' },
       },
     });
-
     activatedRoute = {
       data: observableOf({
-        bitstream: createSuccessfulRemoteDataObject(
-          bitstream,
-        ),
+        bitstream: createSuccessfulRemoteDataObject(bitstream),
       }),
       params: observableOf({
         id: 'testid',
       }),
+      queryParams: observableOf({
+        accessToken: undefined,
+      }),
     };
 
     router = jasmine.createSpyObj('router', ['navigateByUrl']);
@@ -112,7 +113,10 @@ describe('BitstreamDownloadPageComponent', () => {
     signpostingDataService = jasmine.createSpyObj('SignpostingDataService', {
       getLinks: observableOf([mocklink, mocklink2]),
     });
-    matomoService = jasmine.createSpyObj('MatomoService', ['appendVisitorId']);
+    matomoService = jasmine.createSpyObj('MatomoService', {
+      appendVisitorId: observableOf(''),
+      isMatomoEnabled$: observableOf(true),
+    });
     matomoService.appendVisitorId.and.callFake((link) => observableOf(link));
   }
 
@@ -132,6 +136,7 @@ describe('BitstreamDownloadPageComponent', () => {
         { provide: PLATFORM_ID, useValue: 'server' },
         { provide: Location, useValue: location },
         { provide: DSONameService, useValue: dsoNameService },
+        { provide: ConfigurationDataService, useValue: {} },
       ],
     })
       .compileComponents();
@@ -227,4 +232,42 @@ describe('BitstreamDownloadPageComponent', () => {
       }));
     });
   });
+
+  describe('when Matomo is enabled', () => {
+    beforeEach(waitForAsync(() => {
+      init();
+      (matomoService.appendVisitorId as jasmine.Spy).and.callFake((link) => observableOf(link + '?visitorId=12345'));
+      initTestbed();
+    }));
+    beforeEach(() => {
+      fixture = TestBed.createComponent(BitstreamDownloadPageComponent);
+      component = fixture.componentInstance;
+      fixture.detectChanges();
+    });
+    it('should append visitor ID to the file link', waitForAsync(() => {
+      fixture.whenStable().then(() => {
+        expect(matomoService.appendVisitorId).toHaveBeenCalledWith('content-url-with-headers');
+        expect(hardRedirectService.redirect).toHaveBeenCalledWith('content-url-with-headers?visitorId=12345');
+      });
+    }));
+  });
+
+  describe('when Matomo is not enabled', () => {
+    beforeEach(waitForAsync(() => {
+      init();
+      (matomoService.isMatomoEnabled$ as jasmine.Spy).and.returnValue(observableOf(false));
+      initTestbed();
+    }));
+    beforeEach(() => {
+      fixture = TestBed.createComponent(BitstreamDownloadPageComponent);
+      component = fixture.componentInstance;
+      fixture.detectChanges();
+    });
+    it('should not append visitor ID to the file link', waitForAsync(() => {
+      fixture.whenStable().then(() => {
+        expect(matomoService.appendVisitorId).not.toHaveBeenCalled();
+        expect(hardRedirectService.redirect).toHaveBeenCalledWith('content-url-with-headers');
+      });
+    }));
+  });
 });
diff --git a/src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.ts b/src/app/bitstream-page/bitstream-download-page/bitstream-download-page.component.ts
@@ -6,11 +6,13 @@ import {
 import {
   Component,
   Inject,
+  inject,
   OnInit,
   PLATFORM_ID,
 } from '@angular/core';
 import {
   ActivatedRoute,
+  Params,
   Router,
 } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
@@ -29,6 +31,7 @@ import {
 import { getForbiddenRoute } from '../../app-routing-paths';
 import { AuthService } from '../../core/auth/auth.service';
 import { DSONameService } from '../../core/breadcrumbs/dso-name.service';
+import { ConfigurationDataService } from '../../core/data/configuration-data.service';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { RemoteData } from '../../core/data/remote-data';
@@ -63,6 +66,8 @@ export class BitstreamDownloadPageComponent implements OnInit {
   bitstream$: Observable<Bitstream>;
   bitstreamRD$: Observable<RemoteData<Bitstream>>;
 
+  configService = inject(ConfigurationDataService);
+
   constructor(
     private route: ActivatedRoute,
     protected router: Router,
@@ -85,6 +90,10 @@ export class BitstreamDownloadPageComponent implements OnInit {
   }
 
   ngOnInit(): void {
+    const accessToken$: Observable<string> = this.route.queryParams.pipe(
+      map((queryParams: Params) => queryParams?.accessToken || null),
+      take(1),
+    );
 
     this.bitstreamRD$ = this.route.data.pipe(
       map((data) => data.bitstream));
@@ -98,38 +107,49 @@ export class BitstreamDownloadPageComponent implements OnInit {
       switchMap((bitstream: Bitstream) => {
         const isAuthorized$ = this.authorizationService.isAuthorized(FeatureID.CanDownload, isNotEmpty(bitstream) ? bitstream.self : undefined);
         const isLoggedIn$ = this.auth.isAuthenticated();
-        return observableCombineLatest([isAuthorized$, isLoggedIn$, observableOf(bitstream)]);
+        const isMatomoEnabled$ = this.matomoService.isMatomoEnabled$();
+        return observableCombineLatest([isAuthorized$, isLoggedIn$, isMatomoEnabled$, accessToken$, observableOf(bitstream)]);
       }),
-      filter(([isAuthorized, isLoggedIn, bitstream]: [boolean, boolean, Bitstream]) => hasValue(isAuthorized) && hasValue(isLoggedIn)),
+      filter(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [boolean, boolean, boolean, string, Bitstream]) => (hasValue(isAuthorized) && hasValue(isLoggedIn)) || hasValue(accessToken)),
       take(1),
-      switchMap(([isAuthorized, isLoggedIn, bitstream]: [boolean, boolean, Bitstream]) => {
+      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [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, bitstream, fileLink];
+              return [isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink];
             }));
+        } else if (hasValue(accessToken)) {
+          return [[isAuthorized, !isLoggedIn, isMatomoEnabled, bitstream, '', accessToken]];
         } else {
-          return [[isAuthorized, isLoggedIn, bitstream, bitstream._links.content.href]];
+          return [[isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, bitstream._links.content.href]];
         }
       }),
-      switchMap(([isAuthorized, isLoggedIn, bitstream, fileLink]: [boolean, boolean, Bitstream, string]) =>
-        this.matomoService.appendVisitorId(fileLink)
-          .pipe(
-            map((fileLinkWithVisitorId) => [isAuthorized, isLoggedIn, bitstream, fileLinkWithVisitorId]),
-          ),
-      ),
-    ).subscribe(([isAuthorized, isLoggedIn, , fileLink]: [boolean, boolean, Bitstream, string]) => {
+      switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink, accessToken]: [boolean, boolean, boolean, Bitstream, string, string]) => {
+        if (isMatomoEnabled) {
+          return this.matomoService.appendVisitorId(fileLink).pipe(
+            map((fileLinkWithVisitorId) => [isAuthorized, isLoggedIn, bitstream, fileLinkWithVisitorId, accessToken]),
+          );
+        }
+        return observableOf([isAuthorized, isLoggedIn, bitstream, fileLink, accessToken]);
+      }),
+    ).subscribe(([isAuthorized, isLoggedIn, bitstream, fileLink, accessToken]: [boolean, boolean, Bitstream, string, string]) => {
       if (isAuthorized && isLoggedIn && isNotEmpty(fileLink)) {
         this.hardRedirectService.redirect(fileLink);
-      } else if (isAuthorized && !isLoggedIn) {
+      } else if (isAuthorized && !isLoggedIn && !hasValue(accessToken)) {
         this.hardRedirectService.redirect(fileLink);
-      } else if (!isAuthorized && isLoggedIn) {
-        this.router.navigateByUrl(getForbiddenRoute(), { skipLocationChange: true });
-      } else if (!isAuthorized && !isLoggedIn) {
-        this.auth.setRedirectUrl(this.router.url);
-        this.router.navigateByUrl('login');
+      } else if (!isAuthorized) {
+        // Either we have an access token, or we are logged in, or we are not logged in.
+        // For now, the access token does not care if we are logged in or not.
+        if (hasValue(accessToken)) {
+          this.hardRedirectService.redirect(bitstream._links.content.href + '?accessToken=' + accessToken);
+        } else if (isLoggedIn) {
+          this.router.navigateByUrl(getForbiddenRoute(), { skipLocationChange: true });
+        } else if (!isLoggedIn) {
+          this.auth.setRedirectUrl(this.router.url);
+          this.router.navigateByUrl('login');
+        }
       }
     });
   }
diff --git a/src/app/shared/cookies/browser-klaro.service.ts b/src/app/shared/cookies/browser-klaro.service.ts
@@ -26,6 +26,7 @@ import { EPerson } from '../../core/eperson/models/eperson.model';
 import { CAPTCHA_NAME } from '../../core/google-recaptcha/google-recaptcha.service';
 import { CookieService } from '../../core/services/cookie.service';
 import { getFirstCompletedRemoteData } from '../../core/shared/operators';
+import { MATOMO_ENABLED } from '../../statistics/matomo.service';
 import {
   hasValue,
   isEmpty,
@@ -86,7 +87,7 @@ export class BrowserKlaroService extends KlaroService {
 
   private readonly GOOGLE_ANALYTICS_SERVICE_NAME = 'google-analytics';
 
-  private readonly MATOMO_ENABLED = 'matomo.enabled';
+  private readonly MATOMO_ENABLED = MATOMO_ENABLED;
 
   /**
    * Initial Klaro configuration
diff --git a/src/app/statistics/matomo.service.ts b/src/app/statistics/matomo.service.ts
@@ -106,6 +106,9 @@ export class MatomoService {
    * @returns An Observable that emits the URL with the visitor ID appended.
    */
   appendVisitorId(url: string): Observable<string> {
+    if (!this.matomoTracker) {
+      return of(url);
+    }
     return fromPromise(this.matomoTracker?.getVisitorId())
       .pipe(
         map(visitorId => this.appendTrackerId(url, visitorId)),
