Skip to content

Commit 0aaaa77

Browse files
fix: silently fail when we cannot get visitorId from matomo for bitstream download
1 parent 3753418 commit 0aaaa77

6 files changed

Lines changed: 99 additions & 9 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ describe('BitstreamDownloadPageComponent', () => {
116116
matomoService = jasmine.createSpyObj('MatomoService', {
117117
appendVisitorId: of(''),
118118
isMatomoEnabled$: of(true),
119+
isMatomoScriptLoaded$: of(true),
119120
});
120121
matomoService.appendVisitorId.and.callFake((link) => of(link));
121122
}

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,26 +107,27 @@ export class BitstreamDownloadPageComponent implements OnInit {
107107
const isAuthorized$ = this.authorizationService.isAuthorized(FeatureID.CanDownload, isNotEmpty(bitstream) ? bitstream.self : undefined);
108108
const isLoggedIn$ = this.auth.isAuthenticated();
109109
const isMatomoEnabled$ = this.matomoService.isMatomoEnabled$();
110-
return observableCombineLatest([isAuthorized$, isLoggedIn$, isMatomoEnabled$, accessToken$, of(bitstream)]);
110+
const isMatomoScriptLoaded$ = this.matomoService.isMatomoScriptLoaded$();
111+
return observableCombineLatest([isAuthorized$, isLoggedIn$, isMatomoEnabled$, isMatomoScriptLoaded$, accessToken$, of(bitstream)]);
111112
}),
112-
filter(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [boolean, boolean, boolean, string, Bitstream]) => (hasValue(isAuthorized) && hasValue(isLoggedIn)) || hasValue(accessToken)),
113+
filter(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, accessToken, bitstream]: [boolean, boolean, boolean, boolean, string, Bitstream]) => (hasValue(isAuthorized) && hasValue(isLoggedIn)) || hasValue(accessToken)),
113114
take(1),
114-
switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, accessToken, bitstream]: [boolean, boolean, boolean, string, Bitstream]) => {
115+
switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, accessToken, bitstream]: [boolean, boolean, boolean, boolean, string, Bitstream]) => {
115116
if (isAuthorized && isLoggedIn) {
116117
return this.fileService.retrieveFileDownloadLink(bitstream._links.content.href).pipe(
117118
filter((fileLink) => hasValue(fileLink)),
118119
take(1),
119120
map((fileLink) => {
120-
return [isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink];
121+
return [isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, fileLink];
121122
}));
122123
} else if (hasValue(accessToken)) {
123-
return [[isAuthorized, !isLoggedIn, isMatomoEnabled, bitstream, '', accessToken]];
124+
return [[isAuthorized, !isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, '', accessToken]];
124125
} else {
125-
return [[isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, bitstream._links.content.href]];
126+
return [[isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, bitstream._links.content.href]];
126127
}
127128
}),
128-
switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, bitstream, fileLink, accessToken]: [boolean, boolean, boolean, Bitstream, string, string]) => {
129-
if (isMatomoEnabled) {
129+
switchMap(([isAuthorized, isLoggedIn, isMatomoEnabled, isMatomoScriptLoaded, bitstream, fileLink, accessToken]: [boolean, boolean, boolean, boolean, Bitstream, string, string]) => {
130+
if (isMatomoEnabled && isMatomoScriptLoaded) {
130131
return this.matomoService.appendVisitorId(fileLink).pipe(
131132
map((fileLinkWithVisitorId) => [isAuthorized, isLoggedIn, bitstream, fileLinkWithVisitorId, accessToken]),
132133
);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { Injector } from '@angular/core';
2+
import { TestBed } from '@angular/core/testing';
3+
import { OrejimeService } from '@dspace/core/cookies/orejime.service';
4+
import { ConfigurationDataService } from '@dspace/core/data/configuration-data.service';
5+
import { NativeWindowService } from '@dspace/core/services/window.service';
6+
import {
7+
MatomoInitializerService,
8+
MatomoTracker,
9+
} from 'ngx-matomo-client';
10+
import { firstValueFrom } from 'rxjs';
11+
12+
import { customMatomoScriptFactory } from './matomo.factory';
13+
import { MatomoService } from './matomo.service';
14+
15+
describe('customMatomoScriptFactory', () => {
16+
let service: MatomoService;
17+
beforeEach(() => {
18+
TestBed.configureTestingModule({
19+
providers: [
20+
{ provide: MatomoTracker, useValue: {} },
21+
{ provide: MatomoInitializerService, useValue: {} },
22+
{ provide: OrejimeService, useValue: {} },
23+
{ provide: NativeWindowService, useValue: {} },
24+
{ provide: ConfigurationDataService, useValue: {} },
25+
{ provide: Injector, useValue: TestBed },
26+
],
27+
});
28+
29+
service = TestBed.inject(MatomoService);
30+
});
31+
32+
it('should notify when the script loads', async () => {
33+
const script = customMatomoScriptFactory(service)('', document);
34+
35+
script.dispatchEvent(new Event('load'));
36+
const isMatomoScriptLoaded = await firstValueFrom(service.isMatomoScriptLoaded$());
37+
38+
expect(isMatomoScriptLoaded).toBeTruthy();
39+
});
40+
41+
it('should notify when the script fails', async () => {
42+
const script = customMatomoScriptFactory(service)('', document);
43+
44+
script.dispatchEvent(new Event('error'));
45+
const isMatomoScriptLoaded = await firstValueFrom(service.isMatomoScriptLoaded$());
46+
47+
expect(isMatomoScriptLoaded).toBeFalsy();
48+
});
49+
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { createDefaultMatomoScriptElement } from 'ngx-matomo-client';
2+
3+
import { MatomoService } from './matomo.service';
4+
5+
export function customMatomoScriptFactory(matomoService: MatomoService) {
6+
return (scriptUrl: string, document: Document): HTMLScriptElement => {
7+
const script = createDefaultMatomoScriptElement(scriptUrl, document);
8+
9+
script.onload = () => matomoService.markAsLoaded();
10+
script.onerror = () => matomoService.markAsError();
11+
12+
return script;
13+
};
14+
}

src/app/statistics/matomo.service.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
from as fromPromise,
2626
Observable,
2727
of,
28+
ReplaySubject,
2829
} from 'rxjs';
2930
import {
3031
map,
@@ -45,7 +46,6 @@ import { environment } from '../../environments/environment';
4546
* Provides methods for initializing tracking, managing consent, and appending visitor identifiers.
4647
*/
4748
export class MatomoService {
48-
4949
/** Injects the MatomoInitializerService to initialize the Matomo tracker. */
5050
matomoInitializer: MatomoInitializerService;
5151

@@ -61,8 +61,19 @@ export class MatomoService {
6161
/** Injects the ConfigurationService. */
6262
configService = inject(ConfigurationDataService);
6363

64+
private statusSubject = new ReplaySubject<'loading' | 'loaded' | 'error'>(1);
65+
private status$ = this.statusSubject.asObservable();
66+
6467
constructor(private injector: EnvironmentInjector) {
68+
this.statusSubject.next('loading');
69+
}
6570

71+
markAsLoaded() {
72+
this.statusSubject.next('loaded');
73+
}
74+
75+
markAsError() {
76+
this.statusSubject.next('error');
6677
}
6778

6879
/**
@@ -165,6 +176,12 @@ export class MatomoService {
165176
);
166177
}
167178

179+
isMatomoScriptLoaded$(): Observable<boolean> {
180+
return this.status$.pipe(
181+
map(status => status === 'loaded'),
182+
);
183+
}
184+
168185
/**
169186
* Appends the visitor ID as a query parameter to the given URL.
170187
* @param url - The original URL to modify

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,13 @@ import {
5656
Angulartics2RouterlessModule,
5757
} from 'angulartics2';
5858
import {
59+
MATOMO_SCRIPT_FACTORY,
5960
provideMatomo,
6061
withRouteData,
6162
withRouter,
6263
} from 'ngx-matomo-client';
64+
import { customMatomoScriptFactory } from 'src/app/statistics/matomo.factory';
65+
import { MatomoService } from 'src/app/statistics/matomo.service';
6366

6467
import { commonAppConfig } from '../../app/app.config';
6568
import { storeModuleConfig } from '../../app/app.reducer';
@@ -169,5 +172,10 @@ export const browserAppConfig: ApplicationConfig = mergeApplicationConfig({
169172
withRouter(),
170173
withRouteData(),
171174
),
175+
{
176+
provide: MATOMO_SCRIPT_FACTORY,
177+
useFactory: customMatomoScriptFactory,
178+
deps: [MatomoService],
179+
},
172180
],
173181
}, commonAppConfig);

0 commit comments

Comments
 (0)