Skip to content

Commit 85fefea

Browse files
authored
[ENG-10707] update project count metrics if user is not contributor or is not authenticated (#929)
- Ticket: https://openscience.atlassian.net/browse/ENG-10707 - Feature flag: n/a ## Purpose When contributors (Admin, Write, or Read) view their own projects, the unique view count increases — which it should not. This inflates analytics data for project owners. ## Summary of Changes add check to update project count metrics (count_usage call) if user is not contributor or is not authenticated
1 parent aacfd54 commit 85fefea

4 files changed

Lines changed: 54 additions & 35 deletions

File tree

src/app/features/project/project.component.spec.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Store } from '@ngxs/store';
22

33
import { MockProvider } from 'ng-mocks';
44

5-
import { TestBed } from '@angular/core/testing';
5+
import { fakeAsync, TestBed } from '@angular/core/testing';
66
import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
77

88
import { HelpScoutService } from '@core/services/help-scout.service';
@@ -95,6 +95,7 @@ function setup(overrides: SetupOverrides = {}) {
9595
{ selector: ContributorsSelectors.getBibliographicContributors, value: [] },
9696
{ selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false },
9797
{ selector: CurrentResourceSelectors.getCurrentResource, value: null },
98+
{ selector: CurrentResourceSelectors.hasNoPermissions, value: false },
9899
];
99100

100101
const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides);
@@ -227,16 +228,20 @@ describe('Component: Project', () => {
227228
expect(metaTagsService.updateMetaTags).not.toHaveBeenCalled();
228229
});
229230

230-
it('should send analytics on NavigationEnd', () => {
231-
const { routerBuilder, analyticsService } = setup();
232-
231+
it('should send analytics on NavigationEnd', fakeAsync(() => {
232+
const mockResource = { id: 'project-1' };
233+
const { routerBuilder, analyticsService } = setup({
234+
selectorOverrides: [
235+
{ selector: CurrentResourceSelectors.getCurrentResource, value: mockResource },
236+
{ selector: CurrentResourceSelectors.hasNoPermissions, value: true },
237+
],
238+
});
233239
routerBuilder.emit(new NavigationEnd(1, '/project-1', '/project-1/overview'));
234-
235240
expect(analyticsService.sendCountedUsageForRegistrationAndProjects).toHaveBeenCalledWith(
236241
'/project-1/overview',
237-
null
242+
mockResource
238243
);
239-
});
244+
}));
240245

241246
it('should call unsetResourceType on destroy', () => {
242247
const { component, helpScoutService } = setup();

src/app/features/project/project.component.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export class ProjectComponent implements OnDestroy {
5252
private readonly router = inject(Router);
5353
private readonly analyticsService = inject(AnalyticsService);
5454

55+
readonly hasNoPermissions = select(CurrentResourceSelectors.hasNoPermissions);
5556
readonly currentResource = select(CurrentResourceSelectors.getCurrentResource);
5657
readonly currentProject = select(ProjectOverviewSelectors.getProject);
5758
readonly isProjectLoading = select(ProjectOverviewSelectors.getProjectLoading);
@@ -143,10 +144,12 @@ export class ProjectComponent implements OnDestroy {
143144
.subscribe((event: NavigationEnd) => {
144145
this.canonicalPath.set(this.getCanonicalPathFromSnapshot());
145146
this.isFileDetailRoute.set(this.isFileDetailRouteFromSnapshot());
146-
this.analyticsService.sendCountedUsageForRegistrationAndProjects(
147-
event.urlAfterRedirects,
148-
this.currentResource()
149-
);
147+
if (this.currentResource()?.id && this.hasNoPermissions()) {
148+
this.analyticsService.sendCountedUsageForRegistrationAndProjects(
149+
event.urlAfterRedirects,
150+
this.currentResource()
151+
);
152+
}
150153
});
151154
}
152155

src/app/features/registry/registry.component.spec.ts

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Store } from '@ngxs/store';
33
import { MockProvider } from 'ng-mocks';
44

55
import { PLATFORM_ID, Provider } from '@angular/core';
6-
import { TestBed } from '@angular/core/testing';
6+
import { fakeAsync, TestBed } from '@angular/core/testing';
77
import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
88

99
import { HelpScoutService } from '@core/services/help-scout.service';
@@ -32,14 +32,15 @@ import { MetaTagsBuilderServiceMockFactory } from '@testing/providers/meta-tags-
3232
import { PrerenderReadyServiceMockFactory } from '@testing/providers/prerender-ready.service.mock';
3333
import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock';
3434
import { RouterMockBuilder } from '@testing/providers/router-provider.mock';
35-
import { provideMockStore } from '@testing/providers/store-provider.mock';
35+
import { mergeSignalOverrides, provideMockStore, SignalOverride } from '@testing/providers/store-provider.mock';
3636

3737
interface SetupOverrides {
3838
registryId?: string;
3939
registry?: RegistrationOverviewModel | null;
4040
identifiers?: IdentifierModel[];
4141
canonicalPath?: string;
4242
platform?: string;
43+
selectorOverrides?: SignalOverride[];
4344
}
4445

4546
function setup(overrides: SetupOverrides = {}) {
@@ -72,6 +73,20 @@ function setup(overrides: SetupOverrides = {}) {
7273
}) as MetaTagsData
7374
);
7475

76+
const defaultSignals: SignalOverride[] = [
77+
{ selector: RegistrySelectors.getRegistry, value: registry },
78+
{ selector: RegistrySelectors.isRegistryLoading, value: false },
79+
{ selector: RegistrySelectors.getIdentifiers, value: identifiers },
80+
{ selector: RegistrySelectors.getLicense, value: { name: 'MIT' } },
81+
{ selector: RegistrySelectors.isLicenseLoading, value: false },
82+
{ selector: ContributorsSelectors.getBibliographicContributors, value: [] },
83+
{ selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false },
84+
{ selector: CurrentResourceSelectors.getCurrentResource, value: null },
85+
{ selector: CurrentResourceSelectors.hasNoPermissions, value: false },
86+
];
87+
88+
const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides);
89+
7590
const providers: Provider[] = [
7691
provideOSFCore(),
7792
MockProvider(HelpScoutService, helpScoutService),
@@ -82,18 +97,7 @@ function setup(overrides: SetupOverrides = {}) {
8297
MockProvider(AnalyticsService, analyticsService),
8398
MockProvider(ActivatedRoute, routeBuilder.build()),
8499
MockProvider(Router, mockRouter),
85-
provideMockStore({
86-
signals: [
87-
{ selector: RegistrySelectors.getRegistry, value: registry },
88-
{ selector: RegistrySelectors.isRegistryLoading, value: false },
89-
{ selector: RegistrySelectors.getIdentifiers, value: identifiers },
90-
{ selector: RegistrySelectors.getLicense, value: { name: 'MIT' } },
91-
{ selector: RegistrySelectors.isLicenseLoading, value: false },
92-
{ selector: ContributorsSelectors.getBibliographicContributors, value: [] },
93-
{ selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false },
94-
{ selector: CurrentResourceSelectors.getCurrentResource, value: null },
95-
],
96-
}),
100+
provideMockStore({ signals }),
97101
];
98102

99103
if (overrides.platform) {
@@ -225,16 +229,20 @@ describe('RegistryComponent', () => {
225229
expect(metaTagsService.updateMetaTags).not.toHaveBeenCalled();
226230
});
227231

228-
it('should send analytics on NavigationEnd event', () => {
229-
const { routerBuilder, analyticsService } = setup();
230-
232+
it('should send analytics on NavigationEnd event', fakeAsync(() => {
233+
const mockResource = { id: 'registry-1' };
234+
const { routerBuilder, analyticsService } = setup({
235+
selectorOverrides: [
236+
{ selector: CurrentResourceSelectors.getCurrentResource, value: mockResource },
237+
{ selector: CurrentResourceSelectors.hasNoPermissions, value: true },
238+
],
239+
});
231240
routerBuilder.emit(new NavigationEnd(1, '/registries/registry-1', '/registries/registry-1'));
232-
233241
expect(analyticsService.sendCountedUsageForRegistrationAndProjects).toHaveBeenCalledWith(
234242
'/registries/registry-1',
235-
null
243+
mockResource
236244
);
237-
});
245+
}));
238246

239247
it('should unset helpScout and clear provider on destroy', () => {
240248
const { component, store, helpScoutService } = setup();

src/app/features/registry/registry.component.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export class RegistryComponent implements OnDestroy {
6565
});
6666

6767
private readonly registryId = toSignal(this.route.params.pipe(map((params) => params['id'])));
68+
readonly hasNoPermissions = select(CurrentResourceSelectors.hasNoPermissions);
6869
private readonly currentResource = select(CurrentResourceSelectors.getCurrentResource);
6970
private readonly registry = select(RegistrySelectors.getRegistry);
7071
private readonly isRegistryLoading = select(RegistrySelectors.isRegistryLoading);
@@ -141,10 +142,12 @@ export class RegistryComponent implements OnDestroy {
141142
.subscribe((event) => {
142143
this.canonicalPath.set(this.getCanonicalPathFromSnapshot());
143144
this.isFileDetailRoute.set(this.isFileDetailRouteFromSnapshot());
144-
this.analyticsService.sendCountedUsageForRegistrationAndProjects(
145-
event.urlAfterRedirects,
146-
this.currentResource()
147-
);
145+
if (this.currentResource()?.id && this.hasNoPermissions()) {
146+
this.analyticsService.sendCountedUsageForRegistrationAndProjects(
147+
event.urlAfterRedirects,
148+
this.currentResource()
149+
);
150+
}
148151
});
149152
}
150153

0 commit comments

Comments
 (0)