From 71f3a65c456c79dfcfbd688770401b8a19e469e1 Mon Sep 17 00:00:00 2001 From: nsemets Date: Thu, 5 Mar 2026 16:09:12 +0200 Subject: [PATCH 1/3] fix(commands): updated commands for angular serve --- angular.json | 17 +++++++++++++++++ package.json | 1 + 2 files changed, 18 insertions(+) diff --git a/angular.json b/angular.json index 45489de17..1457228db 100644 --- a/angular.json +++ b/angular.json @@ -117,6 +117,20 @@ "namedChunks": true }, "development": { + "outputMode": "static", + "server": false, + "ssr": false, + "optimization": false, + "extractLicenses": false, + "sourceMap": true, + "fileReplacements": [ + { + "replace": "src/environments/environment.ts", + "with": "src/environments/environment.development.ts" + } + ] + }, + "dev-ssr": { "optimization": false, "extractLicenses": false, "sourceMap": true, @@ -164,6 +178,9 @@ "development": { "buildTarget": "osf:build:development" }, + "dev-ssr": { + "buildTarget": "osf:build:dev-ssr" + }, "docker": { "buildTarget": "osf:build:docker" }, diff --git a/package.json b/package.json index 80657c032..fafb56652 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ "ngxs:store": "ng generate @ngxs/store:store --name --path", "prepare": "husky", "start": "ng serve", + "start:ssr": "ng serve --configuration dev-ssr", "start:docker": "npm run check:config && ng serve --host 0.0.0.0 --port 4200 --poll 2000 --configuration development", "start:docker:local": "npm run check:config && ng serve --host 0.0.0.0 --port 4200 --poll 2000 --configuration docker", "test": "jest", From 0b4570697dab5fe400818b8ebd93fd00b8664ddf Mon Sep 17 00:00:00 2001 From: nsemets Date: Fri, 13 Mar 2026 14:23:06 +0200 Subject: [PATCH 2/3] feat(canonical-links): added canonical links to guid pages --- src/app/features/files/files.routes.ts | 9 +- .../file-detail/file-detail.component.ts | 33 +-- src/app/features/metadata/metadata.routes.ts | 1 + .../preprint-details.component.spec.ts | 27 +- .../preprint-details.component.ts | 40 +-- .../project/project.component.spec.ts | 274 ++++++++++++++---- src/app/features/project/project.component.ts | 92 +++--- src/app/features/project/project.routes.ts | 19 +- .../registry/registry.component.spec.ts | 87 +++++- .../features/registry/registry.component.ts | 69 ++--- src/app/features/registry/registry.routes.ts | 16 +- .../shared/helpers/canonical-path.helper.ts | 76 +++++ .../models/meta-tags/head-tag-def.model.ts | 21 +- .../models/meta-tags/meta-tags-data.model.ts | 1 + .../meta-tags-builder.service.spec.ts | 179 ++++++++++++ .../services/meta-tags-builder.service.ts | 139 +++++++++ .../shared/services/meta-tags.service.spec.ts | 124 ++++++++ src/app/shared/services/meta-tags.service.ts | 175 +++++------ .../services/metadata-records.service.spec.ts | 52 ++++ .../services/metadata-records.service.ts | 13 +- .../meta-tags-builder.service.mock.ts | 16 + 21 files changed, 1142 insertions(+), 321 deletions(-) create mode 100644 src/app/shared/helpers/canonical-path.helper.ts create mode 100644 src/app/shared/services/meta-tags-builder.service.spec.ts create mode 100644 src/app/shared/services/meta-tags-builder.service.ts create mode 100644 src/app/shared/services/meta-tags.service.spec.ts create mode 100644 src/app/shared/services/metadata-records.service.spec.ts create mode 100644 src/testing/providers/meta-tags-builder.service.mock.ts diff --git a/src/app/features/files/files.routes.ts b/src/app/features/files/files.routes.ts index 5c93c2baf..8b4cb2b77 100644 --- a/src/app/features/files/files.routes.ts +++ b/src/app/features/files/files.routes.ts @@ -18,6 +18,7 @@ export const filesRoutes: Routes = [ { path: ':fileProvider', canMatch: [isFileProvider], + data: { canonicalPathTemplate: 'files/:fileProvider' }, loadComponent: () => import('@osf/features/files/pages/files/files.component').then((c) => c.FilesComponent), }, { @@ -27,18 +28,12 @@ export const filesRoutes: Routes = [ }, { path: ':fileGuid', + data: { canonicalPathTemplate: 'files/:fileGuid' }, loadComponent: () => { return import('@osf/features/files/pages/file-detail/file-detail.component').then( (c) => c.FileDetailComponent ); }, - children: [ - { - path: 'metadata', - loadChildren: () => import('@osf/features/metadata/metadata.routes').then((mod) => mod.metadataRoutes), - data: { resourceType: ResourceType.File }, - }, - ], }, ], }, diff --git a/src/app/features/files/pages/file-detail/file-detail.component.ts b/src/app/features/files/pages/file-detail/file-detail.component.ts index 9dc06ff04..a9c4ec849 100644 --- a/src/app/features/files/pages/file-detail/file-detail.component.ts +++ b/src/app/features/files/pages/file-detail/file-detail.component.ts @@ -1,6 +1,6 @@ import { createDispatchMap, select, Store } from '@ngxs/store'; -import { TranslatePipe, TranslateService } from '@ngx-translate/core'; +import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; import { Menu } from 'primeng/menu'; @@ -10,7 +10,6 @@ import { Tab, TabList, Tabs } from 'primeng/tabs'; import { switchMap } from 'rxjs'; import { Clipboard } from '@angular/cdk/clipboard'; -import { DatePipe } from '@angular/common'; import { ChangeDetectionStrategy, Component, @@ -43,10 +42,10 @@ import { MetadataTabsComponent } from '@osf/shared/components/metadata-tabs/meta import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; import { MetadataResourceEnum } from '@osf/shared/enums/metadata-resource.enum'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; -import { pathJoin } from '@osf/shared/helpers/path-join.helper'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ToastService } from '@osf/shared/services/toast.service'; import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; import { FileDetailsModel } from '@shared/models/files/file.model'; @@ -92,7 +91,6 @@ import { templateUrl: './file-detail.component.html', styleUrl: './file-detail.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, - providers: [DatePipe], }) export class FileDetailComponent { @HostBinding('class') classes = 'flex flex-column flex-1 w-full h-full'; @@ -106,16 +104,13 @@ export class FileDetailComponent { readonly customConfirmationService = inject(CustomConfirmationService); private readonly metaTags = inject(MetaTagsService); - private readonly datePipe = inject(DatePipe); + private readonly metaTagsBuilder = inject(MetaTagsBuilderService); private readonly viewOnlyService = inject(ViewOnlyLinkHelperService); - private readonly translateService = inject(TranslateService); private readonly environment = inject(ENVIRONMENT); private readonly clipboard = inject(Clipboard); readonly dataciteService = inject(DataciteService); - private readonly webUrl = this.environment.webUrl; - private readonly actions = createDispatchMap({ getFile: GetFile, getFileRevisions: GetFileRevisions, @@ -204,24 +199,14 @@ export class FileDetailComponent { } const file = this.file(); + if (!file) return null; - return { - osfGuid: file.guid, - title: this.fileCustomMetadata()?.title || file.name, - type: this.fileCustomMetadata()?.resourceTypeGeneral, - description: - this.fileCustomMetadata()?.description ?? this.translateService.instant('files.metaTagDescriptionPlaceholder'), - url: pathJoin(this.webUrl, this.fileGuid), - publishedDate: this.datePipe.transform(file.dateCreated, 'yyyy-MM-dd'), - modifiedDate: this.datePipe.transform(file.dateModified, 'yyyy-MM-dd'), - language: this.fileCustomMetadata()?.language, - contributors: this.resourceContributors()?.map((contributor) => ({ - fullName: contributor.fullName, - givenName: contributor.givenName, - familyName: contributor.familyName, - })), - }; + return this.metaTagsBuilder.buildFileMetaTagsData({ + file, + fileMetadata: this.fileCustomMetadata(), + contributors: this.resourceContributors() ?? [], + }); }); constructor() { diff --git a/src/app/features/metadata/metadata.routes.ts b/src/app/features/metadata/metadata.routes.ts index cc8c4097e..5c8e80eac 100644 --- a/src/app/features/metadata/metadata.routes.ts +++ b/src/app/features/metadata/metadata.routes.ts @@ -14,6 +14,7 @@ export const metadataRoutes: Routes = [ }, { path: ':recordId', + data: { canonicalPathTemplate: 'metadata/:recordId' }, component: MetadataComponent, }, ]; diff --git a/src/app/features/preprints/pages/preprint-details/preprint-details.component.spec.ts b/src/app/features/preprints/pages/preprint-details/preprint-details.component.spec.ts index d9cc1d1a1..6823577f4 100644 --- a/src/app/features/preprints/pages/preprint-details/preprint-details.component.spec.ts +++ b/src/app/features/preprints/pages/preprint-details/preprint-details.component.spec.ts @@ -13,9 +13,11 @@ import { ActivatedRoute, Router } from '@angular/router'; import { HelpScoutService } from '@core/services/help-scout.service'; import { PrerenderReadyService } from '@core/services/prerender-ready.service'; import { ClearCurrentProvider } from '@core/store/provider'; +import { MetaTagsData } from '@osf/shared/models/meta-tags/meta-tags-data.model'; import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ToastService } from '@osf/shared/services/toast.service'; import { ContributorsSelectors } from '@osf/shared/stores/contributors'; @@ -55,6 +57,7 @@ import { provideOSFCore } from '@testing/osf.testing.provider'; import { CustomDialogServiceMockBuilder } from '@testing/providers/custom-dialog-provider.mock'; import { HelpScoutServiceMockFactory } from '@testing/providers/help-scout.service.mock'; import { MetaTagsServiceMockFactory } from '@testing/providers/meta-tags.service.mock'; +import { MetaTagsBuilderServiceMockFactory } from '@testing/providers/meta-tags-builder.service.mock'; import { PrerenderReadyServiceMockFactory } from '@testing/providers/prerender-ready.service.mock'; import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock'; @@ -70,6 +73,7 @@ describe('PreprintDetailsComponent', () => { let prerenderReadyServiceMock: jest.Mocked; let dataciteServiceMock: ReturnType; let metaTagsServiceMock: ReturnType; + let metaTagsBuilderServiceMock: ReturnType; let customDialogServiceMock: ReturnType; let toastService: ToastServiceMockType; @@ -122,6 +126,13 @@ describe('PreprintDetailsComponent', () => { prerenderReadyServiceMock = PrerenderReadyServiceMockFactory(); dataciteServiceMock = DataciteMockFactory(); metaTagsServiceMock = MetaTagsServiceMockFactory(); + metaTagsBuilderServiceMock = MetaTagsBuilderServiceMockFactory(); + metaTagsBuilderServiceMock.buildPreprintMetaTagsData.mockImplementation( + ({ providerId, preprint }) => + ({ + canonicalUrl: `http://localhost:4200/preprints/${providerId}/${preprint?.id}`, + }) as MetaTagsData + ); toastService = ToastServiceMock.simple(); customDialogServiceMock = overrides?.dialogReturnsCloseValue === false @@ -167,6 +178,7 @@ describe('PreprintDetailsComponent', () => { MockProvider(PrerenderReadyService, prerenderReadyServiceMock), MockProvider(DataciteService, dataciteServiceMock), MockProvider(MetaTagsService, metaTagsServiceMock), + MockProvider(MetaTagsBuilderService, metaTagsBuilderServiceMock), MockProvider(CustomDialogService, customDialogServiceMock), provideMockStore({ signals }), ], @@ -199,7 +211,19 @@ describe('PreprintDetailsComponent', () => { it('should update meta tags when preprint and contributors are loaded', () => { setup(); - expect(metaTagsServiceMock.updateMetaTags).toHaveBeenCalled(); + expect(metaTagsBuilderServiceMock.buildPreprintMetaTagsData).toHaveBeenCalledWith( + expect.objectContaining({ + providerId: 'osf', + preprint: expect.objectContaining({ id: 'preprint-1' }), + }) + ); + + expect(metaTagsServiceMock.updateMetaTags).toHaveBeenCalledWith( + expect.objectContaining({ + canonicalUrl: 'http://localhost:4200/preprints/osf/preprint-1', + }), + expect.anything() + ); }); it('should not fetch moderation actions when not moderator and no permissions', () => { @@ -532,6 +556,7 @@ describe('PreprintDetailsComponent SSR', () => { MockProvider(Router, routerMock), MockProvider(CustomDialogService, CustomDialogServiceMockBuilder.create().withDefaultOpen().build()), MockProvider(DataciteService, DataciteMockFactory()), + MockProvider(MetaTagsBuilderService, MetaTagsBuilderServiceMockFactory()), MockProvider(MetaTagsService, MetaTagsServiceMockFactory()), MockProvider(PrerenderReadyService, PrerenderReadyServiceMockFactory()), MockProvider(HelpScoutService, helpScoutServiceMock), diff --git a/src/app/features/preprints/pages/preprint-details/preprint-details.component.ts b/src/app/features/preprints/pages/preprint-details/preprint-details.component.ts index a904cff99..df896e0da 100644 --- a/src/app/features/preprints/pages/preprint-details/preprint-details.component.ts +++ b/src/app/features/preprints/pages/preprint-details/preprint-details.component.ts @@ -7,7 +7,7 @@ import { Skeleton } from 'primeng/skeleton'; import { catchError, EMPTY, filter, map } from 'rxjs'; -import { DatePipe, isPlatformBrowser } from '@angular/common'; +import { isPlatformBrowser } from '@angular/common'; import { HttpErrorResponse } from '@angular/common/http'; import { ChangeDetectionStrategy, @@ -30,10 +30,10 @@ import { PrerenderReadyService } from '@core/services/prerender-ready.service'; import { ClearCurrentProvider } from '@core/store/provider'; import { UserSelectors } from '@core/store/user'; import { ReviewPermissions } from '@osf/shared/enums/review-permissions.enum'; -import { pathJoin } from '@osf/shared/helpers/path-join.helper'; import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ToastService } from '@osf/shared/services/toast.service'; import { ContributorsSelectors } from '@osf/shared/stores/contributors'; @@ -82,7 +82,6 @@ import { CreateNewVersion, PreprintStepperSelectors } from '../../store/preprint ], templateUrl: './preprint-details.component.html', styleUrl: './preprint-details.component.scss', - providers: [DatePipe], changeDetection: ChangeDetectionStrategy.OnPush, }) export class PreprintDetailsComponent implements OnInit, OnDestroy { @@ -97,14 +96,13 @@ export class PreprintDetailsComponent implements OnInit, OnDestroy { private readonly customDialogService = inject(CustomDialogService); private readonly translateService = inject(TranslateService); private readonly metaTags = inject(MetaTagsService); - private readonly datePipe = inject(DatePipe); + private readonly metaTagsBuilder = inject(MetaTagsBuilderService); private readonly dataciteService = inject(DataciteService); private readonly prerenderReady = inject(PrerenderReadyService); - private readonly platformId = inject(PLATFORM_ID); private readonly environment = inject(ENVIRONMENT); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); - private readonly isBrowser = isPlatformBrowser(this.platformId); - + readonly providerId = toSignal(this.route.params.pipe(map((params) => params['providerId']))); private readonly preprintId = toSignal(this.route.params.pipe(map((params) => params['id']))); private readonly actions = createDispatchMap({ @@ -118,7 +116,6 @@ export class PreprintDetailsComponent implements OnInit, OnDestroy { clearCurrentProvider: ClearCurrentProvider, }); - readonly providerId = toSignal(this.route.params.pipe(map((params) => params['providerId']))); currentUser = select(UserSelectors.getCurrentUser); preprintProvider = select(PreprintProvidersSelectors.getPreprintProviderDetails(this.providerId())); isPreprintProviderLoading = select(PreprintProvidersSelectors.isPreprintProviderDetailsLoading); @@ -410,26 +407,13 @@ export class PreprintDetailsComponent implements OnInit, OnDestroy { } private setMetaTags() { - this.metaTags.updateMetaTags( - { - osfGuid: this.preprint()?.id, - title: this.preprint()?.title, - description: this.preprint()?.description, - publishedDate: this.datePipe.transform(this.preprint()?.datePublished, 'yyyy-MM-dd'), - modifiedDate: this.datePipe.transform(this.preprint()?.dateModified, 'yyyy-MM-dd'), - url: pathJoin(this.environment.webUrl, this.preprint()?.id ?? ''), - doi: this.preprint()?.doi, - keywords: this.preprint()?.tags, - siteName: 'OSF', - license: this.preprint()?.embeddedLicense?.name, - contributors: this.contributors().map((contributor) => ({ - fullName: contributor.fullName, - givenName: contributor.givenName, - familyName: contributor.familyName, - })), - }, - this.destroyRef - ); + const metaTags = this.metaTagsBuilder.buildPreprintMetaTagsData({ + providerId: this.providerId(), + preprint: this.preprint(), + contributors: this.contributors(), + }); + + this.metaTags.updateMetaTags(metaTags, this.destroyRef); } private checkAndSetVersionToTheUrl() { diff --git a/src/app/features/project/project.component.spec.ts b/src/app/features/project/project.component.spec.ts index 72d6774ba..dac59c787 100644 --- a/src/app/features/project/project.component.spec.ts +++ b/src/app/features/project/project.component.spec.ts @@ -1,83 +1,247 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { ActivatedRoute } from '@angular/router'; +import { Store } from '@ngxs/store'; + +import { MockProvider } from 'ng-mocks'; + +import { TestBed } from '@angular/core/testing'; +import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { HelpScoutService } from '@core/services/help-scout.service'; import { PrerenderReadyService } from '@core/services/prerender-ready.service'; +import { ProjectOverviewModel } from '@osf/features/project/overview/models'; +import { IdentifierModel } from '@osf/shared/models/identifiers/identifier.model'; +import { MetaTagsData } from '@osf/shared/models/meta-tags/meta-tags-data.model'; +import { AnalyticsService } from '@osf/shared/services/analytics.service'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ContributorsSelectors } from '@osf/shared/stores/contributors'; import { CurrentResourceSelectors } from '@osf/shared/stores/current-resource'; -import { ProjectOverviewSelectors } from './overview/store'; +import { GetProjectById, GetProjectIdentifiers, GetProjectLicense, ProjectOverviewSelectors } from './overview/store'; import { ProjectComponent } from './project.component'; import { DataciteMockFactory } from '@testing/mocks/datacite.service.mock'; -import { OSFTestingModule } from '@testing/osf.testing.module'; +import { MOCK_PROJECT_OVERVIEW } from '@testing/mocks/project-overview.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; +import { AnalyticsServiceMockFactory } from '@testing/providers/analytics.service.mock'; import { HelpScoutServiceMockFactory } from '@testing/providers/help-scout.service.mock'; import { MetaTagsServiceMockFactory } from '@testing/providers/meta-tags.service.mock'; +import { MetaTagsBuilderServiceMockFactory } from '@testing/providers/meta-tags-builder.service.mock'; import { PrerenderReadyServiceMockFactory } from '@testing/providers/prerender-ready.service.mock'; import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; -import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; +import { + BaseSetupOverrides, + mergeSignalOverrides, + provideMockStore, + SignalOverride, +} from '@testing/providers/store-provider.mock'; -describe('Component: Project', () => { - let component: ProjectComponent; - let fixture: ComponentFixture; - let helpScoutService: ReturnType; - let metaTagsService: ReturnType; - let dataciteService: ReturnType; - let prerenderReadyService: ReturnType; - let mockActivatedRoute: ReturnType; - - beforeEach(async () => { - mockActivatedRoute = ActivatedRouteMockBuilder.create().withParams({ id: 'project-1' }).build(); - - helpScoutService = HelpScoutServiceMockFactory(); - metaTagsService = MetaTagsServiceMockFactory(); - dataciteService = DataciteMockFactory(); - prerenderReadyService = PrerenderReadyServiceMockFactory(); - - await TestBed.configureTestingModule({ - imports: [ProjectComponent, OSFTestingModule], - providers: [ - { provide: HelpScoutService, useValue: helpScoutService }, - { provide: MetaTagsService, useValue: metaTagsService }, - { provide: DataciteService, useValue: dataciteService }, - { provide: PrerenderReadyService, useValue: prerenderReadyService }, - { provide: ActivatedRoute, useValue: mockActivatedRoute }, - provideMockStore({ - signals: [ - { selector: ProjectOverviewSelectors.getProject, value: null }, - { selector: ProjectOverviewSelectors.getProjectLoading, value: false }, - { selector: ProjectOverviewSelectors.getIdentifiers, value: [] }, - { selector: ProjectOverviewSelectors.getLicense, value: null }, - { selector: ProjectOverviewSelectors.isLicenseLoading, value: false }, - { selector: ContributorsSelectors.getBibliographicContributors, value: [] }, - { selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false }, - { selector: CurrentResourceSelectors.getCurrentResource, value: null }, - ], - }), - ], - }).compileComponents(); - - fixture = TestBed.createComponent(ProjectComponent); - component = fixture.componentInstance; - fixture.detectChanges(); +interface SetupOverrides extends BaseSetupOverrides { + projectId?: string; + selectorOverrides?: SignalOverride[]; + childCanonicalPath?: string; + childCanonicalPathTemplate?: string; + childParams?: Record; +} + +function setup(overrides: SetupOverrides = {}) { + const projectId = overrides.projectId ?? 'project-1'; + const helpScoutService = HelpScoutServiceMockFactory(); + const analyticsService = AnalyticsServiceMockFactory(); + const metaTagsService = MetaTagsServiceMockFactory(); + const metaTagsBuilderService = MetaTagsBuilderServiceMockFactory(); + const dataciteService = DataciteMockFactory(); + const prerenderReadyService = PrerenderReadyServiceMockFactory(); + const routerBuilder = RouterMockBuilder.create(); + const routerMock = routerBuilder.build(); + + const routeBuilder = ActivatedRouteMockBuilder.create().withParams(overrides.routeParams ?? { id: projectId }); + + if (overrides.hasParent === false) { + routeBuilder.withNoParent(); + } + + if (overrides.childCanonicalPath || overrides.childCanonicalPathTemplate || overrides.childParams) { + routeBuilder.withFirstChild((builder) => { + if (overrides.childCanonicalPath) { + builder.withData({ canonicalPath: overrides.childCanonicalPath }); + } + if (overrides.childCanonicalPathTemplate) { + builder.withData({ canonicalPathTemplate: overrides.childCanonicalPathTemplate }); + } + if (overrides.childParams) { + builder.withParams(overrides.childParams); + } + }); + } + + const mockActivatedRoute = routeBuilder.build(); + + metaTagsBuilderService.buildProjectMetaTagsData.mockImplementation( + ({ project, canonicalPath }): MetaTagsData => + ({ + osfGuid: project.id, + canonicalUrl: `http://localhost:4200/${project.id}/${canonicalPath}`, + }) as MetaTagsData + ); + + const defaultSignals: SignalOverride[] = [ + { selector: ProjectOverviewSelectors.getProject, value: null }, + { selector: ProjectOverviewSelectors.getProjectLoading, value: false }, + { selector: ProjectOverviewSelectors.getIdentifiers, value: [] }, + { selector: ProjectOverviewSelectors.getLicense, value: { name: 'MIT' } }, + { selector: ProjectOverviewSelectors.isLicenseLoading, value: false }, + { selector: ContributorsSelectors.getBibliographicContributors, value: [] }, + { selector: ContributorsSelectors.isBibliographicContributorsLoading, value: false }, + { selector: CurrentResourceSelectors.getCurrentResource, value: null }, + ]; + + const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides); + + TestBed.configureTestingModule({ + imports: [ProjectComponent], + providers: [ + provideOSFCore(), + MockProvider(HelpScoutService, helpScoutService), + MockProvider(AnalyticsService, analyticsService), + MockProvider(MetaTagsService, metaTagsService), + MockProvider(MetaTagsBuilderService, metaTagsBuilderService), + MockProvider(DataciteService, dataciteService), + MockProvider(PrerenderReadyService, prerenderReadyService), + MockProvider(ActivatedRoute, mockActivatedRoute), + MockProvider(Router, routerMock), + provideMockStore({ signals }), + ], }); - it('should call the helpScoutService', () => { + const store = TestBed.inject(Store); + const fixture = TestBed.createComponent(ProjectComponent); + const component = fixture.componentInstance; + fixture.detectChanges(); + + return { + component, + fixture, + store, + routerBuilder, + routerMock, + helpScoutService, + analyticsService, + metaTagsService, + metaTagsBuilderService, + dataciteService, + prerenderReadyService, + }; +} + +describe('Component: Project', () => { + it('should set helpScout resource type and prerender not ready on init', () => { + const { helpScoutService, prerenderReadyService } = setup(); + expect(helpScoutService.setResourceType).toHaveBeenCalledWith('project'); + expect(prerenderReadyService.setNotReady).toHaveBeenCalled(); }); - it('should call unsetResourceType on destroy', () => { - component.ngOnDestroy(); - expect(helpScoutService.unsetResourceType).toHaveBeenCalled(); + it('should dispatch init actions when project id is available', () => { + const { store } = setup(); + + expect(store.dispatch).toHaveBeenCalledWith(new GetProjectById('project-1')); + expect(store.dispatch).toHaveBeenCalledWith(new GetProjectIdentifiers('project-1')); }); - it('should call prerenderReady.setNotReady in constructor', () => { - expect(prerenderReadyService.setNotReady).toHaveBeenCalled(); + it('should dispatch get license when project has licenseId', () => { + const project = { ...MOCK_PROJECT_OVERVIEW, licenseId: 'license-1' } as ProjectOverviewModel; + const { store } = setup({ + selectorOverrides: [{ selector: ProjectOverviewSelectors.getProject, value: project }], + }); + + expect(store.dispatch).toHaveBeenCalledWith(new GetProjectLicense('license-1')); }); - it('should call dataciteService.logIdentifiableView', () => { + it('should call datacite tracking on init', () => { + const { dataciteService } = setup(); + expect(dataciteService.logIdentifiableView).toHaveBeenCalled(); }); + + it('should map identifiers to null when identifiers are empty', () => { + const { dataciteService } = setup(); + const identifiers$ = (dataciteService.logIdentifiableView as jest.Mock).mock.calls[0][0]; + let emitted: unknown; + + identifiers$.subscribe((value: unknown) => { + emitted = value; + }); + + expect(emitted).toBeNull(); + }); + + it('should map identifiers to payload when identifiers exist', () => { + const identifiers = [ + { + id: 'identifier-1', + type: 'identifiers', + category: 'doi', + value: '10.1234/osf.test', + }, + ] as IdentifierModel[]; + const { dataciteService } = setup({ + selectorOverrides: [{ selector: ProjectOverviewSelectors.getIdentifiers, value: identifiers }], + }); + const identifiers$ = (dataciteService.logIdentifiableView as jest.Mock).mock.calls[0][0]; + let emitted: unknown; + + identifiers$.subscribe((value: unknown) => { + emitted = value; + }); + + expect(emitted).toEqual({ identifiers }); + }); + + it('should build and update meta tags with canonical path from active subroute', () => { + const { metaTagsBuilderService, metaTagsService } = setup({ + selectorOverrides: [{ selector: ProjectOverviewSelectors.getProject, value: MOCK_PROJECT_OVERVIEW }], + childCanonicalPath: 'files/osfstorage', + }); + + expect(metaTagsBuilderService.buildProjectMetaTagsData).toHaveBeenCalledWith( + expect.objectContaining({ + project: expect.objectContaining({ id: 'project-1' }), + canonicalPath: 'files/osfstorage', + }) + ); + expect(metaTagsService.updateMetaTags).toHaveBeenCalledWith( + expect.objectContaining({ + canonicalUrl: 'http://localhost:4200/project-1/files/osfstorage', + }), + expect.anything() + ); + }); + + it('should not build or update meta tags when current project is null', () => { + const { metaTagsBuilderService, metaTagsService } = setup(); + + expect(metaTagsBuilderService.buildProjectMetaTagsData).not.toHaveBeenCalled(); + expect(metaTagsService.updateMetaTags).not.toHaveBeenCalled(); + }); + + it('should send analytics on NavigationEnd', () => { + const { routerBuilder, analyticsService } = setup(); + + routerBuilder.emit(new NavigationEnd(1, '/project-1', '/project-1/overview')); + + expect(analyticsService.sendCountedUsageForRegistrationAndProjects).toHaveBeenCalledWith( + '/project-1/overview', + null + ); + }); + + it('should call unsetResourceType on destroy', () => { + const { component, helpScoutService } = setup(); + + component.ngOnDestroy(); + + expect(helpScoutService.unsetResourceType).toHaveBeenCalled(); + }); }); diff --git a/src/app/features/project/project.component.ts b/src/app/features/project/project.component.ts index 0ec8ced6b..27cc4df93 100644 --- a/src/app/features/project/project.component.ts +++ b/src/app/features/project/project.component.ts @@ -2,7 +2,6 @@ import { createDispatchMap, select } from '@ngxs/store'; import { filter, map } from 'rxjs'; -import { DatePipe } from '@angular/common'; import { ChangeDetectionStrategy, Component, @@ -20,8 +19,13 @@ import { ActivatedRoute, NavigationEnd, Router, RouterOutlet } from '@angular/ro import { HelpScoutService } from '@core/services/help-scout.service'; import { PrerenderReadyService } from '@core/services/prerender-ready.service'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; +import { + getDeepestCanonicalPathTemplateFromSnapshot, + resolveCanonicalPathFromSnapshot, +} from '@osf/shared/helpers/canonical-path.helper'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ContributorsSelectors, GetBibliographicContributors } from '@osf/shared/stores/contributors'; import { AnalyticsService } from '@shared/services/analytics.service'; import { CurrentResourceSelectors } from '@shared/stores/current-resource'; @@ -34,26 +38,21 @@ import { GetProjectById, GetProjectIdentifiers, GetProjectLicense, ProjectOvervi templateUrl: './project.component.html', styleUrl: './project.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, - providers: [DatePipe], }) export class ProjectComponent implements OnDestroy { @HostBinding('class') classes = 'flex flex-1 flex-column w-full'; private readonly helpScoutService = inject(HelpScoutService); private readonly metaTags = inject(MetaTagsService); + private readonly metaTagsBuilder = inject(MetaTagsBuilderService); private readonly dataciteService = inject(DataciteService); private readonly destroyRef = inject(DestroyRef); private readonly route = inject(ActivatedRoute); - private readonly datePipe = inject(DatePipe); private readonly prerenderReady = inject(PrerenderReadyService); private readonly router = inject(Router); private readonly analyticsService = inject(AnalyticsService); - private readonly currentResource = select(CurrentResourceSelectors.getCurrentResource); - - readonly identifiersForDatacite$ = toObservable(select(ProjectOverviewSelectors.getIdentifiers)).pipe( - map((identifiers) => (identifiers?.length ? { identifiers } : null)) - ); + readonly currentResource = select(CurrentResourceSelectors.getCurrentResource); readonly currentProject = select(ProjectOverviewSelectors.getProject); readonly isProjectLoading = select(ProjectOverviewSelectors.getProjectLoading); readonly bibliographicContributors = select(ContributorsSelectors.getBibliographicContributors); @@ -61,8 +60,20 @@ export class ProjectComponent implements OnDestroy { readonly license = select(ProjectOverviewSelectors.getLicense); readonly isLicenseLoading = select(ProjectOverviewSelectors.isLicenseLoading); - private readonly lastMetaTagsProjectId = signal(null); + readonly identifiersForDatacite$ = toObservable(select(ProjectOverviewSelectors.getIdentifiers)).pipe( + map((identifiers) => (identifiers?.length ? { identifiers } : null)) + ); + + private readonly actions = createDispatchMap({ + getProject: GetProjectById, + getLicense: GetProjectLicense, + getIdentifiers: GetProjectIdentifiers, + getBibliographicContributors: GetBibliographicContributors, + }); + private readonly projectId = toSignal(this.route.params.pipe(map((params) => params['id']))); + private readonly canonicalPath = signal(this.getCanonicalPathFromSnapshot()); + private readonly isFileDetailRoute = signal(this.isFileDetailRouteFromSnapshot()); private readonly allDataLoaded = computed( () => @@ -72,13 +83,6 @@ export class ProjectComponent implements OnDestroy { !!this.currentProject() ); - private readonly actions = createDispatchMap({ - getProject: GetProjectById, - getLicense: GetProjectLicense, - getIdentifiers: GetProjectIdentifiers, - getBibliographicContributors: GetBibliographicContributors, - }); - constructor() { this.prerenderReady.setNotReady(); this.helpScoutService.setResourceType('project'); @@ -94,19 +98,19 @@ export class ProjectComponent implements OnDestroy { }); effect(() => { - const project = this.currentProject(); + const licenseId = this.currentProject()?.licenseId; - if (project?.licenseId) { - this.actions.getLicense(project.licenseId); + if (licenseId) { + this.actions.getLicense(licenseId); } }); effect(() => { if (this.allDataLoaded()) { - const currentProjectId = this.projectId(); - const lastSetProjectId = this.lastMetaTagsProjectId(); + const currentProjectId = this.currentProject()?.id; + const currentCanonicalPath = this.canonicalPath(); - if (currentProjectId && currentProjectId !== lastSetProjectId) { + if (currentProjectId && currentCanonicalPath && !this.isFileDetailRoute()) { this.setMetaTags(); } } @@ -123,6 +127,8 @@ export class ProjectComponent implements OnDestroy { takeUntilDestroyed(this.destroyRef) ) .subscribe((event: NavigationEnd) => { + this.canonicalPath.set(this.getCanonicalPathFromSnapshot()); + this.isFileDetailRoute.set(this.isFileDetailRouteFromSnapshot()); this.analyticsService.sendCountedUsageForRegistrationAndProjects( event.urlAfterRedirects, this.currentResource() @@ -135,29 +141,29 @@ export class ProjectComponent implements OnDestroy { } private setMetaTags(): void { - const project = this.currentProject(); - if (!project) return; - - const keywords = [...(project.tags || []), ...(project.category ? [project.category] : [])]; - - const metaTagsData = { - osfGuid: project.id, - title: project.title, - description: project.description, - url: project.links?.iri, - license: this.license.name, - publishedDate: this.datePipe.transform(project.dateCreated, 'yyyy-MM-dd'), - modifiedDate: this.datePipe.transform(project.dateModified, 'yyyy-MM-dd'), - keywords, - contributors: this.bibliographicContributors().map((contributor) => ({ - fullName: contributor.fullName, - givenName: contributor.givenName, - familyName: contributor.familyName, - })), - }; + const project = this.currentProject()!; + + const metaTagsData = this.metaTagsBuilder.buildProjectMetaTagsData({ + project, + canonicalPath: this.canonicalPath(), + contributors: this.bibliographicContributors(), + licenseName: this.license.name, + }); this.metaTags.updateMetaTags(metaTagsData, this.destroyRef); + } + + private getCanonicalPathFromSnapshot(): string { + return resolveCanonicalPathFromSnapshot(this.route.snapshot, { + fallbackPath: 'overview', + paramDefaults: { + fileProvider: 'osfstorage', + recordId: 'osf', + }, + }); + } - this.lastMetaTagsProjectId.set(project.id); + private isFileDetailRouteFromSnapshot(): boolean { + return getDeepestCanonicalPathTemplateFromSnapshot(this.route.snapshot) === 'files/:fileGuid'; } } diff --git a/src/app/features/project/project.routes.ts b/src/app/features/project/project.routes.ts index 8c2861788..a4f4b153d 100644 --- a/src/app/features/project/project.routes.ts +++ b/src/app/features/project/project.routes.ts @@ -36,6 +36,7 @@ export const projectRoutes: Routes = [ path: 'overview', loadComponent: () => import('../project/overview/project-overview.component').then((mod) => mod.ProjectOverviewComponent), + data: { canonicalPath: 'overview' }, providers: [ provideStates([ NodeLinksState, @@ -52,13 +53,13 @@ export const projectRoutes: Routes = [ path: 'metadata', loadChildren: () => import('@osf/features/metadata/metadata.routes').then((mod) => mod.metadataRoutes), providers: [provideStates([SubjectsState, CollectionsState])], - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'metadata/osf' }, canActivate: [viewOnlyGuard], }, { path: 'files', loadChildren: () => import('@osf/features/files/files.routes').then((mod) => mod.filesRoutes), - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'files/osfstorage' }, }, { path: 'registrations', @@ -66,29 +67,31 @@ export const projectRoutes: Routes = [ providers: [provideStates([RegistrationsState])], loadComponent: () => import('../project/registrations/registrations.component').then((mod) => mod.RegistrationsComponent), + data: { canonicalPath: 'registrations' }, }, { path: 'settings', canActivate: [viewOnlyGuard], loadComponent: () => import('../project/settings/settings.component').then((mod) => mod.SettingsComponent), + data: { canonicalPath: 'settings' }, providers: [provideStates([SettingsState, ViewOnlyLinkState])], }, { path: 'contributors', canActivate: [viewOnlyGuard], loadComponent: () => import('../contributors/contributors.component').then((mod) => mod.ContributorsComponent), - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'contributors' }, providers: [provideStates([ViewOnlyLinkState])], }, { path: 'analytics', loadComponent: () => import('../analytics/analytics.component').then((mod) => mod.AnalyticsComponent), - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'analytics' }, providers: [provideStates([AnalyticsState])], }, { path: 'analytics/linked-projects', - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'analytics/linked-projects' }, loadComponent: () => import('../analytics/components/view-linked-projects/view-linked-projects.component').then( (mod) => mod.ViewLinkedProjectsComponent @@ -97,7 +100,7 @@ export const projectRoutes: Routes = [ }, { path: 'analytics/duplicates', - data: { resourceType: ResourceType.Project }, + data: { resourceType: ResourceType.Project, canonicalPath: 'analytics/duplicates' }, loadComponent: () => import('../analytics/components/view-duplicates/view-duplicates.component').then( (mod) => mod.ViewDuplicatesComponent @@ -108,20 +111,24 @@ export const projectRoutes: Routes = [ path: 'wiki/:wikiName', loadComponent: () => import('../project/wiki/legacy-wiki-redirect.component').then((m) => m.LegacyWikiRedirectComponent), + data: { canonicalPath: 'wiki' }, }, { path: 'wiki', loadComponent: () => import('../project/wiki/wiki.component').then((mod) => mod.WikiComponent), + data: { canonicalPath: 'wiki' }, }, { path: 'addons', canActivate: [viewOnlyGuard], + data: { canonicalPath: 'addons' }, loadChildren: () => import('@osf/features/project/project-addons/project-addons.routes').then((mod) => mod.projectAddonsRoutes), }, { path: 'links', canActivate: [viewOnlyGuard], + data: { canonicalPath: 'links' }, loadComponent: () => import('../project/linked-services/linked-services.component').then((mod) => mod.LinkedServicesComponent), }, diff --git a/src/app/features/registry/registry.component.spec.ts b/src/app/features/registry/registry.component.spec.ts index 5c59c5f26..78d79a98e 100644 --- a/src/app/features/registry/registry.component.spec.ts +++ b/src/app/features/registry/registry.component.spec.ts @@ -9,9 +9,12 @@ import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { HelpScoutService } from '@core/services/help-scout.service'; import { PrerenderReadyService } from '@core/services/prerender-ready.service'; import { ClearCurrentProvider } from '@core/store/provider'; +import { IdentifierModel } from '@osf/shared/models/identifiers/identifier.model'; +import { MetaTagsData } from '@osf/shared/models/meta-tags/meta-tags-data.model'; import { AnalyticsService } from '@osf/shared/services/analytics.service'; import { DataciteService } from '@osf/shared/services/datacite/datacite.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ContributorsSelectors } from '@osf/shared/stores/contributors'; import { CurrentResourceSelectors } from '@shared/stores/current-resource'; @@ -25,6 +28,7 @@ import { provideOSFCore } from '@testing/osf.testing.provider'; import { AnalyticsServiceMockFactory } from '@testing/providers/analytics.service.mock'; import { HelpScoutServiceMockFactory } from '@testing/providers/help-scout.service.mock'; import { MetaTagsServiceMockFactory } from '@testing/providers/meta-tags.service.mock'; +import { MetaTagsBuilderServiceMockFactory } from '@testing/providers/meta-tags-builder.service.mock'; import { PrerenderReadyServiceMockFactory } from '@testing/providers/prerender-ready.service.mock'; import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; @@ -33,35 +37,56 @@ import { provideMockStore } from '@testing/providers/store-provider.mock'; interface SetupOverrides { registryId?: string; registry?: RegistrationOverviewModel | null; + identifiers?: IdentifierModel[]; + canonicalPath?: string; platform?: string; } function setup(overrides: SetupOverrides = {}) { const registryId = overrides.registryId ?? 'registry-1'; const registry = overrides.registry ?? null; + const identifiers = overrides.identifiers ?? []; + const canonicalPath = overrides.canonicalPath; const helpScoutService = HelpScoutServiceMockFactory(); const metaTagsService = MetaTagsServiceMockFactory(); + const metaTagsBuilderService = MetaTagsBuilderServiceMockFactory(); const dataciteService = DataciteMockFactory(); const prerenderReadyService = PrerenderReadyServiceMockFactory(); const analyticsService = AnalyticsServiceMockFactory(); const routerBuilder = RouterMockBuilder.create(); const mockRouter = routerBuilder.build(); + const routeBuilder = ActivatedRouteMockBuilder.create().withParams({ id: registryId }); + if (canonicalPath) { + routeBuilder.withFirstChild((child) => child.withData({ canonicalPath })); + } + metaTagsBuilderService.buildRegistryMetaTagsData.mockImplementation( + ({ registry: currentRegistry, canonicalPath }) => + ({ + osfGuid: currentRegistry.id, + title: currentRegistry.title, + description: currentRegistry.description, + url: `http://localhost:4200/${currentRegistry.id}`, + canonicalUrl: `http://localhost:4200/${currentRegistry.id}/${canonicalPath}`, + siteName: 'OSF', + }) as MetaTagsData + ); const providers: Provider[] = [ provideOSFCore(), MockProvider(HelpScoutService, helpScoutService), MockProvider(MetaTagsService, metaTagsService), + MockProvider(MetaTagsBuilderService, metaTagsBuilderService), MockProvider(DataciteService, dataciteService), MockProvider(PrerenderReadyService, prerenderReadyService), MockProvider(AnalyticsService, analyticsService), - MockProvider(ActivatedRoute, ActivatedRouteMockBuilder.create().withParams({ id: registryId }).build()), + MockProvider(ActivatedRoute, routeBuilder.build()), MockProvider(Router, mockRouter), provideMockStore({ signals: [ { selector: RegistrySelectors.getRegistry, value: registry }, { selector: RegistrySelectors.isRegistryLoading, value: false }, - { selector: RegistrySelectors.getIdentifiers, value: [] }, + { selector: RegistrySelectors.getIdentifiers, value: identifiers }, { selector: RegistrySelectors.getLicense, value: { name: 'MIT' } }, { selector: RegistrySelectors.isLicenseLoading, value: false }, { selector: ContributorsSelectors.getBibliographicContributors, value: [] }, @@ -91,6 +116,7 @@ function setup(overrides: SetupOverrides = {}) { routerBuilder, helpScoutService, metaTagsService, + metaTagsBuilderService, dataciteService, prerenderReadyService, analyticsService, @@ -111,6 +137,38 @@ describe('RegistryComponent', () => { expect(dataciteService.logIdentifiableView).toHaveBeenCalled(); }); + it('should map identifiers to null when identifiers are empty', () => { + const { dataciteService } = setup({ identifiers: [] }); + const identifiers$ = (dataciteService.logIdentifiableView as jest.Mock).mock.calls[0][0]; + let emitted: unknown; + + identifiers$.subscribe((value: unknown) => { + emitted = value; + }); + + expect(emitted).toBeNull(); + }); + + it('should map identifiers to payload when identifiers exist', () => { + const identifiers: IdentifierModel[] = [ + { + id: 'identifier-1', + type: 'identifiers', + category: 'doi', + value: '10.1234/osf.test', + }, + ]; + const { dataciteService } = setup({ identifiers }); + const identifiers$ = (dataciteService.logIdentifiableView as jest.Mock).mock.calls[0][0]; + let emitted: unknown; + + identifiers$.subscribe((value: unknown) => { + emitted = value; + }); + + expect(emitted).toEqual({ identifiers }); + }); + it('should dispatch init actions when registryId is available', () => { const { store } = setup(); @@ -124,19 +182,42 @@ describe('RegistryComponent', () => { }); it('should call setMetaTags when all data is loaded', () => { - const { metaTagsService } = setup({ registry: MOCK_REGISTRATION_OVERVIEW_MODEL }); + const { metaTagsService, metaTagsBuilderService } = setup({ registry: MOCK_REGISTRATION_OVERVIEW_MODEL }); + + expect(metaTagsBuilderService.buildRegistryMetaTagsData).toHaveBeenCalledWith( + expect.objectContaining({ + registry: expect.objectContaining({ id: MOCK_REGISTRATION_OVERVIEW_MODEL.id }), + canonicalPath: 'overview', + }) + ); expect(metaTagsService.updateMetaTags).toHaveBeenCalledWith( expect.objectContaining({ osfGuid: MOCK_REGISTRATION_OVERVIEW_MODEL.id, title: MOCK_REGISTRATION_OVERVIEW_MODEL.title, description: MOCK_REGISTRATION_OVERVIEW_MODEL.description, + url: `http://localhost:4200/${MOCK_REGISTRATION_OVERVIEW_MODEL.id}`, + canonicalUrl: `http://localhost:4200/${MOCK_REGISTRATION_OVERVIEW_MODEL.id}/overview`, siteName: 'OSF', }), expect.anything() ); }); + it('should set canonicalUrl using active subroute path', () => { + const { metaTagsService } = setup({ + registry: MOCK_REGISTRATION_OVERVIEW_MODEL, + canonicalPath: 'files/osfstorage', + }); + + expect(metaTagsService.updateMetaTags).toHaveBeenLastCalledWith( + expect.objectContaining({ + canonicalUrl: `http://localhost:4200/${MOCK_REGISTRATION_OVERVIEW_MODEL.id}/files/osfstorage`, + }), + expect.anything() + ); + }); + it('should not call setMetaTags when registry is null', () => { const { metaTagsService } = setup({ registry: null }); diff --git a/src/app/features/registry/registry.component.ts b/src/app/features/registry/registry.component.ts index 75e90c022..3c726583c 100644 --- a/src/app/features/registry/registry.component.ts +++ b/src/app/features/registry/registry.component.ts @@ -2,7 +2,7 @@ import { createDispatchMap, select } from '@ngxs/store'; import { filter, map } from 'rxjs'; -import { DatePipe, isPlatformBrowser } from '@angular/common'; +import { isPlatformBrowser } from '@angular/common'; import { ChangeDetectionStrategy, Component, @@ -18,14 +18,17 @@ import { import { takeUntilDestroyed, toObservable, toSignal } from '@angular/core/rxjs-interop'; import { ActivatedRoute, NavigationEnd, Router, RouterOutlet } from '@angular/router'; -import { ENVIRONMENT } from '@core/provider/environment.provider'; import { HelpScoutService } from '@core/services/help-scout.service'; import { PrerenderReadyService } from '@core/services/prerender-ready.service'; import { ClearCurrentProvider } from '@core/store/provider'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; -import { pathJoin } from '@osf/shared/helpers/path-join.helper'; +import { + getDeepestCanonicalPathTemplateFromSnapshot, + resolveCanonicalPathFromSnapshot, +} from '@osf/shared/helpers/canonical-path.helper'; import { AnalyticsService } from '@osf/shared/services/analytics.service'; import { MetaTagsService } from '@osf/shared/services/meta-tags.service'; +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; import { ContributorsSelectors, GetBibliographicContributors } from '@osf/shared/stores/contributors'; import { DataciteService } from '@shared/services/datacite/datacite.service'; import { CurrentResourceSelectors } from '@shared/stores/current-resource'; @@ -38,19 +41,17 @@ import { GetRegistryIdentifiers, GetRegistryWithRelatedData, RegistrySelectors } templateUrl: './registry.component.html', styleUrl: './registry.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, - providers: [DatePipe], }) export class RegistryComponent implements OnDestroy { @HostBinding('class') classes = 'flex-1 flex flex-column'; private readonly metaTags = inject(MetaTagsService); - private readonly datePipe = inject(DatePipe); + private readonly metaTagsBuilder = inject(MetaTagsBuilderService); private readonly dataciteService = inject(DataciteService); private readonly destroyRef = inject(DestroyRef); private readonly route = inject(ActivatedRoute); private readonly router = inject(Router); private readonly helpScoutService = inject(HelpScoutService); - private readonly environment = inject(ENVIRONMENT); private readonly prerenderReady = inject(PrerenderReadyService); private readonly analyticsService = inject(AnalyticsService); private readonly platformId = inject(PLATFORM_ID); @@ -77,7 +78,8 @@ export class RegistryComponent implements OnDestroy { private readonly license = select(RegistrySelectors.getLicense); private readonly isLicenseLoading = select(RegistrySelectors.isLicenseLoading); - private readonly lastMetaTagsRegistryId = signal(null); + private readonly canonicalPath = signal(this.getCanonicalPathFromSnapshot()); + private readonly isFileDetailRoute = signal(this.isFileDetailRouteFromSnapshot()); private readonly allDataLoaded = computed( () => @@ -103,11 +105,10 @@ export class RegistryComponent implements OnDestroy { effect(() => { if (this.allDataLoaded()) { - const currentRegistry = this.registry(); - const currentRegistryId = currentRegistry?.id ?? null; - const lastSetRegistryId = this.lastMetaTagsRegistryId(); + const currentRegistryId = this.registry()?.id; + const currentCanonicalPath = this.canonicalPath(); - if (currentRegistryId && currentRegistryId !== lastSetRegistryId) { + if (currentRegistryId && currentCanonicalPath && !this.isFileDetailRoute()) { this.setMetaTags(); } } @@ -124,6 +125,8 @@ export class RegistryComponent implements OnDestroy { takeUntilDestroyed(this.destroyRef) ) .subscribe((event) => { + this.canonicalPath.set(this.getCanonicalPathFromSnapshot()); + this.isFileDetailRoute.set(this.isFileDetailRouteFromSnapshot()); this.analyticsService.sendCountedUsageForRegistrationAndProjects( event.urlAfterRedirects, this.currentResource() @@ -140,31 +143,29 @@ export class RegistryComponent implements OnDestroy { } private setMetaTags(): void { - const currentRegistry = this.registry(); - if (!currentRegistry) return; - - const metaTagsData = { - osfGuid: currentRegistry.id, - title: currentRegistry.title, - description: currentRegistry.description, - publishedDate: this.datePipe.transform(currentRegistry.dateRegistered, 'yyyy-MM-dd'), - modifiedDate: this.datePipe.transform(currentRegistry.dateModified, 'yyyy-MM-dd'), - url: pathJoin(this.environment.webUrl, currentRegistry.id ?? ''), - identifier: currentRegistry.id, - doi: currentRegistry.articleDoi, - keywords: currentRegistry.tags, - siteName: 'OSF', - license: this.license()?.name, - contributors: - this.bibliographicContributors()?.map((contributor) => ({ - fullName: contributor.fullName, - givenName: contributor.givenName, - familyName: contributor.familyName, - })) ?? [], - }; + const currentRegistry = this.registry()!; + + const metaTagsData = this.metaTagsBuilder.buildRegistryMetaTagsData({ + registry: currentRegistry, + canonicalPath: this.canonicalPath(), + contributors: this.bibliographicContributors() ?? [], + licenseName: this.license()?.name, + }); this.metaTags.updateMetaTags(metaTagsData, this.destroyRef); + } + + private getCanonicalPathFromSnapshot(): string { + return resolveCanonicalPathFromSnapshot(this.route.snapshot, { + fallbackPath: 'overview', + paramDefaults: { + fileProvider: 'osfstorage', + recordId: 'osf', + }, + }); + } - this.lastMetaTagsRegistryId.set(currentRegistry.id); + private isFileDetailRouteFromSnapshot(): boolean { + return getDeepestCanonicalPathTemplateFromSnapshot(this.route.snapshot) === 'files/:fileGuid'; } } diff --git a/src/app/features/registry/registry.routes.ts b/src/app/features/registry/registry.routes.ts index 752cbf703..06109e3b9 100644 --- a/src/app/features/registry/registry.routes.ts +++ b/src/app/features/registry/registry.routes.ts @@ -35,13 +35,14 @@ export const registryRoutes: Routes = [ path: 'overview', loadComponent: () => import('./pages/registry-overview/registry-overview.component').then((c) => c.RegistryOverviewComponent), + data: { canonicalPath: 'overview' }, providers: [provideStates([SubjectsState, CitationsState])], }, { path: 'metadata', loadChildren: () => import('@osf/features/metadata/metadata.routes').then((mod) => mod.metadataRoutes), providers: [provideStates([SubjectsState])], - data: { resourceType: ResourceType.Registration }, + data: { resourceType: ResourceType.Registration, canonicalPath: 'metadata/osf' }, canActivate: [viewOnlyGuard], }, { @@ -49,24 +50,25 @@ export const registryRoutes: Routes = [ canActivate: [viewOnlyGuard], loadComponent: () => import('./pages/registry-links/registry-links.component').then((c) => c.RegistryLinksComponent), + data: { canonicalPath: 'links' }, providers: [provideStates([RegistryLinksState])], }, { path: 'contributors', canActivate: [viewOnlyGuard], loadComponent: () => import('../contributors/contributors.component').then((mod) => mod.ContributorsComponent), - data: { resourceType: ResourceType.Registration }, + data: { resourceType: ResourceType.Registration, canonicalPath: 'contributors' }, providers: [provideStates([ViewOnlyLinkState])], }, { path: 'analytics', loadComponent: () => import('../analytics/analytics.component').then((mod) => mod.AnalyticsComponent), - data: { resourceType: ResourceType.Registration }, + data: { resourceType: ResourceType.Registration, canonicalPath: 'analytics' }, providers: [provideStates([AnalyticsState])], }, { path: 'analytics/duplicates', - data: { resourceType: ResourceType.Registration }, + data: { resourceType: ResourceType.Registration, canonicalPath: 'analytics/duplicates' }, loadComponent: () => import('../analytics/components/view-duplicates/view-duplicates.component').then( (mod) => mod.ViewDuplicatesComponent @@ -76,7 +78,7 @@ export const registryRoutes: Routes = [ { path: 'files', loadChildren: () => import('@osf/features/files/files.routes').then((mod) => mod.filesRoutes), - data: { resourceType: ResourceType.Registration }, + data: { resourceType: ResourceType.Registration, canonicalPath: 'files/osfstorage' }, }, { path: 'components', @@ -85,6 +87,7 @@ export const registryRoutes: Routes = [ import('./pages/registry-components/registry-components.component').then( (c) => c.RegistryComponentsComponent ), + data: { canonicalPath: 'components' }, providers: [provideStates([RegistryComponentsState, RegistryLinksState])], }, { @@ -94,12 +97,14 @@ export const registryRoutes: Routes = [ import('./pages/registry-resources/registry-resources.component').then( (mod) => mod.RegistryResourcesComponent ), + data: { canonicalPath: 'resources' }, providers: [provideStates([RegistryResourcesState])], }, { path: 'wiki', loadComponent: () => import('./pages/registry-wiki/registry-wiki.component').then((c) => c.RegistryWikiComponent), + data: { canonicalPath: 'wiki' }, }, { path: 'recent-activity', @@ -107,6 +112,7 @@ export const registryRoutes: Routes = [ import('./pages/registration-recent-activity/registration-recent-activity.component').then( (c) => c.RegistrationRecentActivityComponent ), + data: { canonicalPath: 'recent-activity' }, providers: [provideStates([ActivityLogsState])], }, ], diff --git a/src/app/shared/helpers/canonical-path.helper.ts b/src/app/shared/helpers/canonical-path.helper.ts new file mode 100644 index 000000000..6ef61fcfc --- /dev/null +++ b/src/app/shared/helpers/canonical-path.helper.ts @@ -0,0 +1,76 @@ +import { ActivatedRouteSnapshot } from '@angular/router'; + +interface ResolveCanonicalPathOptions { + fallbackPath?: string; + paramDefaults?: Record; +} + +export function resolveCanonicalPathFromSnapshot( + routeSnapshot: ActivatedRouteSnapshot | null, + options: ResolveCanonicalPathOptions = {} +): string { + const fallbackPath = options.fallbackPath ?? 'overview'; + let current = routeSnapshot?.firstChild ?? null; + let resolvedPath = fallbackPath; + + while (current) { + const canonicalPath = current.data['canonicalPath'] as string | undefined; + const canonicalPathTemplate = current.data['canonicalPathTemplate'] as string | undefined; + + if (canonicalPathTemplate) { + const templatePath = resolveTemplatePath(current, canonicalPathTemplate, options.paramDefaults); + if (templatePath) { + resolvedPath = templatePath; + } + } else if (canonicalPath) { + resolvedPath = canonicalPath; + } + + current = current.firstChild ?? null; + } + + return resolvedPath; +} + +export function getDeepestCanonicalPathTemplateFromSnapshot( + routeSnapshot: ActivatedRouteSnapshot | null +): string | null { + let current = routeSnapshot?.firstChild ?? null; + let template: string | null = null; + + while (current) { + const currentTemplate = current.data['canonicalPathTemplate'] as string | undefined; + + if (currentTemplate) { + template = currentTemplate; + } + + current = current.firstChild ?? null; + } + + return template; +} + +function resolveTemplatePath( + snapshot: ActivatedRouteSnapshot, + template: string, + paramDefaults?: Record +): string { + const params = snapshot.pathFromRoot.reduce>((acc, segment) => { + const segmentParams = segment.params as Record; + Object.entries(segmentParams).forEach(([key, value]) => { + if (value !== undefined && value !== null) { + acc[key] = String(value); + } + }); + return acc; + }, {}); + + return template + .replace(/:([A-Za-z0-9_]+)/g, (_match, paramName: string) => { + const value = params[paramName] ?? paramDefaults?.[paramName] ?? ''; + return encodeURIComponent(String(value)); + }) + .replace(/\/+/g, '/') + .replace(/\/$/, ''); +} diff --git a/src/app/shared/models/meta-tags/head-tag-def.model.ts b/src/app/shared/models/meta-tags/head-tag-def.model.ts index c0b61e605..9cce95a1c 100644 --- a/src/app/shared/models/meta-tags/head-tag-def.model.ts +++ b/src/app/shared/models/meta-tags/head-tag-def.model.ts @@ -1,7 +1,18 @@ import { MetaDefinition } from '@angular/platform-browser'; -export interface HeadTagDef { - type: 'meta' | 'link' | 'script'; - attrs: MetaDefinition; - content?: string; -} +export type HeadTagAttrs = Record; + +export type HeadTagDef = + | { + type: 'meta'; + attrs: MetaDefinition; + } + | { + type: 'link'; + attrs: HeadTagAttrs; + } + | { + type: 'script'; + attrs: HeadTagAttrs; + content?: string; + }; diff --git a/src/app/shared/models/meta-tags/meta-tags-data.model.ts b/src/app/shared/models/meta-tags/meta-tags-data.model.ts index 20af37136..aa86ec61a 100644 --- a/src/app/shared/models/meta-tags/meta-tags-data.model.ts +++ b/src/app/shared/models/meta-tags/meta-tags-data.model.ts @@ -10,6 +10,7 @@ export interface MetaTagsData { type?: DataContent; description?: DataContent; url?: DataContent; + canonicalUrl?: DataContent; doi?: DataContent; identifier?: DataContent; publishedDate?: DataContent; diff --git a/src/app/shared/services/meta-tags-builder.service.spec.ts b/src/app/shared/services/meta-tags-builder.service.spec.ts new file mode 100644 index 000000000..9cbe20743 --- /dev/null +++ b/src/app/shared/services/meta-tags-builder.service.spec.ts @@ -0,0 +1,179 @@ +import { TranslateService } from '@ngx-translate/core'; +import { MockProvider } from 'ng-mocks'; + +import { LOCALE_ID } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { OsfFileCustomMetadata } from '@osf/features/files/models'; +import { FileKind } from '@osf/shared/enums/file-kind.enum'; +import { FileDetailsModel } from '@osf/shared/models/files/file.model'; + +import { MetaTagsBuilderService } from './meta-tags-builder.service'; + +import { MOCK_CONTRIBUTOR } from '@testing/mocks/contributors.mock'; +import { PREPRINT_MOCK } from '@testing/mocks/preprint.mock'; +import { MOCK_PROJECT_OVERVIEW } from '@testing/mocks/project-overview.mock'; +import { MOCK_REGISTRATION_OVERVIEW_MODEL } from '@testing/mocks/registration-overview-model.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; + +function buildFile(overrides: Partial = {}): FileDetailsModel { + return { + id: 'file-id', + guid: 'file-guid', + name: 'file-name.pdf', + kind: FileKind.File, + path: '/file-name.pdf', + size: 100, + materializedPath: '/file-name.pdf', + dateModified: '2024-01-05T00:00:00Z', + extra: { + hashes: { + md5: 'md5', + sha256: 'sha256', + }, + downloads: 1, + }, + lastTouched: null, + dateCreated: '2024-01-04T00:00:00Z', + tags: [], + currentVersion: 1, + showAsUnviewed: false, + links: { + info: 'info', + move: 'move', + upload: 'upload', + delete: 'delete', + download: 'download', + render: 'render', + html: 'html', + self: 'self', + }, + target: MOCK_PROJECT_OVERVIEW, + ...overrides, + }; +} + +describe('MetaTagsBuilderService', () => { + let service: MetaTagsBuilderService; + let translateService: TranslateService; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [MetaTagsBuilderService, MockProvider(LOCALE_ID, 'en-US'), provideOSFCore()], + }); + + service = TestBed.inject(MetaTagsBuilderService); + translateService = TestBed.inject(TranslateService); + }); + + it('buildProjectMetaTagsData should build canonical, keywords and contributors', () => { + const meta = service.buildProjectMetaTagsData({ + project: { ...MOCK_PROJECT_OVERVIEW, tags: ['tag-1', 'tag-2'], category: 'analysis' }, + canonicalPath: 'overview', + contributors: [MOCK_CONTRIBUTOR], + licenseName: 'MIT', + }); + + expect(meta).toEqual( + expect.objectContaining({ + osfGuid: 'project-1', + title: 'Test Project', + description: 'Test Description', + url: 'http://localhost:4200/project-1/overview', + canonicalUrl: 'http://localhost:4200/project-1/overview', + license: 'MIT', + publishedDate: '2023-01-01', + modifiedDate: '2023-01-01', + keywords: ['tag-1', 'tag-2', 'analysis'], + contributors: [{ fullName: 'John Doe', givenName: 'John Doe', familyName: 'John Doe' }], + }) + ); + }); + + it('buildRegistryMetaTagsData should include doi and identifier', () => { + const meta = service.buildRegistryMetaTagsData({ + registry: MOCK_REGISTRATION_OVERVIEW_MODEL, + canonicalPath: 'overview', + contributors: [MOCK_CONTRIBUTOR], + licenseName: 'CC-BY', + }); + + expect(meta).toEqual( + expect.objectContaining({ + osfGuid: 'test-registry-id', + identifier: 'test-registry-id', + doi: '10.1234/test', + url: 'http://localhost:4200/test-registry-id', + canonicalUrl: 'http://localhost:4200/test-registry-id/overview', + siteName: 'OSF', + license: 'CC-BY', + contributors: [{ fullName: 'John Doe', givenName: 'John Doe', familyName: 'John Doe' }], + }) + ); + }); + + it('buildPreprintMetaTagsData should build provider canonical url', () => { + const meta = service.buildPreprintMetaTagsData({ + providerId: PREPRINT_MOCK.providerId, + preprint: PREPRINT_MOCK, + contributors: [MOCK_CONTRIBUTOR], + }); + + expect(meta).toEqual( + expect.objectContaining({ + osfGuid: PREPRINT_MOCK.id, + title: PREPRINT_MOCK.title, + canonicalUrl: `http://localhost:4200/preprints/${PREPRINT_MOCK.providerId}/${PREPRINT_MOCK.id}`, + doi: PREPRINT_MOCK.doi, + license: PREPRINT_MOCK.embeddedLicense?.name, + contributors: [{ fullName: 'John Doe', givenName: 'John Doe', familyName: 'John Doe' }], + }) + ); + }); + + it('buildFileMetaTagsData should prefer custom metadata values', () => { + const fileMetadata: OsfFileCustomMetadata = { + id: 'metadata-1', + language: 'en', + resourceTypeGeneral: 'Dataset', + title: 'Custom file title', + description: 'Custom file description', + }; + + const meta = service.buildFileMetaTagsData({ + file: buildFile(), + fileMetadata, + contributors: [MOCK_CONTRIBUTOR], + }); + + expect(meta).toEqual( + expect.objectContaining({ + osfGuid: 'file-guid', + title: 'Custom file title', + description: 'Custom file description', + type: 'Dataset', + language: 'en', + url: 'http://localhost:4200/file-guid', + canonicalUrl: 'http://localhost:4200/project-1/files/file-guid', + }) + ); + }); + + it('buildFileMetaTagsData should fallback when custom metadata is null', () => { + const file = buildFile({ target: null as unknown as FileDetailsModel['target'] }); + const meta = service.buildFileMetaTagsData({ + file, + fileMetadata: null, + contributors: [MOCK_CONTRIBUTOR], + }); + + expect(translateService.instant).toHaveBeenCalledWith('files.metaTagDescriptionPlaceholder'); + expect(meta).toEqual( + expect.objectContaining({ + title: 'file-name.pdf', + description: 'files.metaTagDescriptionPlaceholder', + canonicalUrl: 'http://localhost:4200/file-guid', + }) + ); + }); +}); diff --git a/src/app/shared/services/meta-tags-builder.service.ts b/src/app/shared/services/meta-tags-builder.service.ts new file mode 100644 index 000000000..28dd524f4 --- /dev/null +++ b/src/app/shared/services/meta-tags-builder.service.ts @@ -0,0 +1,139 @@ +import { TranslateService } from '@ngx-translate/core'; + +import { formatDate } from '@angular/common'; +import { inject, Injectable, LOCALE_ID } from '@angular/core'; + +import { ENVIRONMENT } from '@core/provider/environment.provider'; +import { OsfFileCustomMetadata } from '@osf/features/files/models'; +import { PreprintModel } from '@osf/features/preprints/models'; +import { ProjectOverviewModel } from '@osf/features/project/overview/models'; +import { RegistrationOverviewModel } from '@osf/features/registry/models'; + +import { pathJoin } from '../helpers/path-join.helper'; +import { ContributorModel } from '../models/contributors/contributor.model'; +import { FileDetailsModel } from '../models/files/file.model'; +import { MetaTagsData } from '../models/meta-tags/meta-tags-data.model'; + +@Injectable({ + providedIn: 'root', +}) +export class MetaTagsBuilderService { + private readonly environment = inject(ENVIRONMENT); + private readonly locale = inject(LOCALE_ID); + private readonly translateService = inject(TranslateService); + + private readonly dateFormat = 'yyyy-MM-dd'; + + buildProjectMetaTagsData(params: { + project: ProjectOverviewModel; + canonicalPath: string; + contributors: ContributorModel[]; + licenseName?: string; + }): MetaTagsData { + const { project, canonicalPath, contributors, licenseName } = params; + const keywords = [...(project.tags || []), ...(project.category ? [project.category] : [])]; + + return { + osfGuid: project.id, + title: project.title, + description: project.description, + url: pathJoin(this.environment.webUrl, project.id, 'overview'), + canonicalUrl: pathJoin(this.environment.webUrl, project.id, canonicalPath), + license: licenseName, + publishedDate: this.formatDate(project.dateCreated), + modifiedDate: this.formatDate(project.dateModified), + keywords, + contributors: contributors.map((contributor) => ({ + fullName: contributor.fullName, + givenName: contributor.givenName, + familyName: contributor.familyName, + })), + }; + } + + buildRegistryMetaTagsData(params: { + registry: RegistrationOverviewModel; + canonicalPath: string; + contributors: ContributorModel[]; + licenseName?: string; + }): MetaTagsData { + const { registry, canonicalPath, contributors, licenseName } = params; + + return { + osfGuid: registry.id, + title: registry.title, + description: registry.description, + publishedDate: this.formatDate(registry.dateRegistered), + modifiedDate: this.formatDate(registry.dateModified), + url: pathJoin(this.environment.webUrl, registry.id), + canonicalUrl: pathJoin(this.environment.webUrl, registry.id, canonicalPath), + identifier: registry.id, + doi: registry.articleDoi, + keywords: registry.tags, + siteName: 'OSF', + license: licenseName, + contributors: this.mapContributors(contributors), + }; + } + + buildPreprintMetaTagsData(params: { + providerId?: string; + preprint: PreprintModel | null; + contributors: ContributorModel[]; + }): MetaTagsData { + const { providerId, preprint, contributors } = params; + + return { + osfGuid: preprint?.id, + title: preprint?.title, + description: preprint?.description, + publishedDate: this.formatDate(preprint?.datePublished), + modifiedDate: this.formatDate(preprint?.dateModified), + url: pathJoin(this.environment.webUrl, preprint?.id ?? ''), + canonicalUrl: pathJoin(this.environment.webUrl, 'preprints', providerId ?? '', preprint?.id ?? ''), + doi: preprint?.doi, + keywords: preprint?.tags, + siteName: 'OSF', + license: preprint?.embeddedLicense?.name, + contributors: this.mapContributors(contributors), + }; + } + + buildFileMetaTagsData(params: { + file: FileDetailsModel; + fileMetadata: OsfFileCustomMetadata | null; + contributors: ContributorModel[]; + }): MetaTagsData { + const { file, fileMetadata, contributors } = params; + + const targetId = file.target?.id; + const fileGuid = file.guid ?? ''; + + return { + osfGuid: file.guid, + title: fileMetadata?.title || file.name, + type: fileMetadata?.resourceTypeGeneral, + description: fileMetadata?.description ?? this.translateService.instant('files.metaTagDescriptionPlaceholder'), + url: pathJoin(this.environment.webUrl, fileGuid), + canonicalUrl: targetId + ? pathJoin(this.environment.webUrl, targetId, 'files', fileGuid) + : pathJoin(this.environment.webUrl, fileGuid), + publishedDate: this.formatDate(file.dateCreated), + modifiedDate: this.formatDate(file.dateModified), + language: fileMetadata?.language, + contributors: this.mapContributors(contributors), + }; + } + + private mapContributors(contributors: ContributorModel[]) { + return contributors.map(({ fullName, givenName, familyName }) => ({ fullName, givenName, familyName })); + } + + private formatDate(value?: string | Date | null) { + if (!value) { + return null; + } + + return formatDate(value, this.dateFormat, this.locale); + } +} diff --git a/src/app/shared/services/meta-tags.service.spec.ts b/src/app/shared/services/meta-tags.service.spec.ts new file mode 100644 index 000000000..010ad8bda --- /dev/null +++ b/src/app/shared/services/meta-tags.service.spec.ts @@ -0,0 +1,124 @@ +import { MockProvider } from 'ng-mocks'; + +import { of } from 'rxjs'; + +import { DestroyRef } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { PrerenderReadyService } from '@core/services/prerender-ready.service'; + +import { MetaTagsService } from './meta-tags.service'; +import { MetadataRecordsService } from './metadata-records.service'; + +import { provideOSFCore } from '@testing/osf.testing.provider'; + +describe('MetaTagsService', () => { + let service: MetaTagsService; + let metadataRecordsMock: { getMetadataRecord: jest.Mock }; + + beforeEach(() => { + metadataRecordsMock = { + getMetadataRecord: jest.fn().mockReturnValue(of('')), + }; + + TestBed.configureTestingModule({ + providers: [ + provideOSFCore(), + MockProvider(MetadataRecordsService, metadataRecordsMock), + MockProvider(PrerenderReadyService, { + setReady: jest.fn(), + setNotReady: jest.fn(), + }), + ], + }); + + service = TestBed.inject(MetaTagsService); + }); + + afterEach(() => { + service.clearMetaTags(); + }); + + it('adds canonical link from url', () => { + const destroyRef = { + onDestroy: jest.fn(), + } as unknown as DestroyRef; + + service.updateMetaTags( + { + title: 'Title', + url: 'http://localhost:4200/ezcuj/overview', + }, + destroyRef + ); + + const canonical = document.head.querySelector('link[rel="canonical"]'); + expect(canonical?.getAttribute('href')).toBe('http://localhost:4200/ezcuj/overview'); + }); + + it('uses canonicalUrl when it differs from url', () => { + const destroyRef = { + onDestroy: jest.fn(), + } as unknown as DestroyRef; + + service.updateMetaTags( + { + title: 'Title', + url: 'http://localhost:4200/ezcuj', + canonicalUrl: 'http://localhost:4200/ezcuj/overview', + }, + destroyRef + ); + + const canonical = document.head.querySelector('link[rel="canonical"]'); + expect(canonical?.getAttribute('href')).toBe('http://localhost:4200/ezcuj/overview'); + }); + + it('replaces canonical link when updated again', () => { + const destroyRef = { + onDestroy: jest.fn(), + } as unknown as DestroyRef; + + service.updateMetaTags( + { + title: 'Title 1', + url: 'http://localhost:4200/ezcuj/overview', + }, + destroyRef + ); + + service.updateMetaTags( + { + title: 'Title 2', + url: 'http://localhost:4200/abcd1/overview', + }, + destroyRef + ); + + const canonicalLinks = document.head.querySelectorAll('link[rel="canonical"]'); + expect(canonicalLinks.length).toBe(1); + expect(canonicalLinks[0]?.getAttribute('href')).toBe('http://localhost:4200/abcd1/overview'); + }); + + it('removes canonical link on destroy callback', () => { + let destroyCallback: (() => void) | undefined; + const destroyRef = { + onDestroy: jest.fn((cb: () => void) => { + destroyCallback = cb; + }), + } as unknown as DestroyRef; + + service.updateMetaTags( + { + title: 'Title', + url: 'http://localhost:4200/ezcuj/overview', + }, + destroyRef + ); + + destroyCallback?.(); + + const canonical = document.head.querySelector('link[rel="canonical"]'); + expect(canonical).toBeNull(); + }); +}); diff --git a/src/app/shared/services/meta-tags.service.ts b/src/app/shared/services/meta-tags.service.ts index a3bc9e727..83b2ca1f8 100644 --- a/src/app/shared/services/meta-tags.service.ts +++ b/src/app/shared/services/meta-tags.service.ts @@ -1,7 +1,7 @@ import { catchError, map, Observable, of, switchMap, tap } from 'rxjs'; import { DOCUMENT, isPlatformBrowser } from '@angular/common'; -import { DestroyRef, effect, Inject, inject, Injectable, PLATFORM_ID, signal } from '@angular/core'; +import { DestroyRef, effect, inject, Injectable, PLATFORM_ID, signal } from '@angular/core'; import { Meta, MetaDefinition, Title } from '@angular/platform-browser'; import { ENVIRONMENT } from '@core/provider/environment.provider'; @@ -19,10 +19,13 @@ import { MetadataRecordsService } from './metadata-records.service'; providedIn: 'root', }) export class MetaTagsService { - private readonly metadataRecords: MetadataRecordsService = inject(MetadataRecordsService); + private readonly metadataRecords = inject(MetadataRecordsService); private readonly environment = inject(ENVIRONMENT); private readonly prerenderReady = inject(PrerenderReadyService); private readonly platformId = inject(PLATFORM_ID); + private readonly document = inject(DOCUMENT); + private readonly title = inject(Title); + private readonly meta = inject(Meta); get webUrl() { return this.environment.webUrl; @@ -53,16 +56,11 @@ export class MetaTagsService { }; private readonly metaTagClass = 'osf-dynamic-meta'; - private metaTagStack: { metaTagsData: MetaTagsData; componentDestroyRef: DestroyRef }[] = []; areMetaTagsApplied = signal(false); - constructor( - private meta: Meta, - private title: Title, - @Inject(DOCUMENT) private document: Document - ) { + constructor() { effect(() => { if (this.areMetaTagsApplied()) { this.prerenderReady.setReady(); @@ -80,26 +78,12 @@ export class MetaTagsService { } clearMetaTags(): void { - if (!isPlatformBrowser(this.platformId)) { - this.areMetaTagsApplied.set(false); - this.prerenderReady.setNotReady(); - return; - } - - const elementsToRemove = this.document.querySelectorAll(`.${this.metaTagClass}`); - - if (elementsToRemove.length === 0) { + if (!isPlatformBrowser(this.platformId) || this.removeDynamicMetaTags() === 0) { this.areMetaTagsApplied.set(false); this.prerenderReady.setNotReady(); return; } - elementsToRemove.forEach((element) => { - if (element.parentNode) { - element.parentNode.removeChild(element); - } - }); - this.title.setTitle(String(this.defaultMetaTags.siteName)); this.areMetaTagsApplied.set(false); this.prerenderReady.setNotReady(); @@ -111,6 +95,7 @@ export class MetaTagsService { private applyNearestMetaTags() { const nearest = this.metaTagStack.at(-1); + if (nearest) { this.applyMetaTagsData(nearest.metaTagsData); } else { @@ -118,21 +103,20 @@ export class MetaTagsService { } } - private applyMetaTagsData(metaTagsData: MetaTagsData) { + private applyMetaTagsData(metaTagsData: MetaTagsData): void { this.areMetaTagsApplied.set(false); this.prerenderReady.setNotReady(); + this.removeDynamicMetaTags(); + const combinedData = { ...this.defaultMetaTags, ...metaTagsData }; const headTags = this.getHeadTags(combinedData); + of(metaTagsData.osfGuid) .pipe( switchMap((osfid) => osfid ? this.getSchemaDotOrgJsonLdHeadTag(osfid).pipe( - tap((jsonLdHeadTag) => { - if (jsonLdHeadTag) { - headTags.push(jsonLdHeadTag); - } - }), + tap((jsonLdHeadTag) => jsonLdHeadTag && headTags.push(jsonLdHeadTag)), catchError(() => of(null)) ) : of(null) @@ -146,26 +130,26 @@ export class MetaTagsService { .subscribe(); } + private removeDynamicMetaTags(): number { + const elements = this.document.querySelectorAll(`.${this.metaTagClass}`); + elements.forEach((el) => el.parentNode?.removeChild(el)); + return elements.length; + } + private getSchemaDotOrgJsonLdHeadTag(osfid: string): Observable { - return this.metadataRecords.getMetadataRecord(osfid, MetadataRecordFormat.SchemaDotOrgDataset).pipe( - map((jsonLd) => - jsonLd - ? { - type: 'script' as const, - attrs: { type: 'application/ld+json' }, - content: jsonLd, - } - : null - ) - ); + return this.metadataRecords + .getMetadataRecord(osfid, MetadataRecordFormat.SchemaDotOrgDataset) + .pipe( + map((jsonLd) => + jsonLd ? { type: 'script' as const, attrs: { type: 'application/ld+json' }, content: jsonLd } : null + ) + ); } private getHeadTags(metaTagsData: MetaTagsData): HeadTagDef[] { - const identifiers = this.toArray(metaTagsData.url) - .concat(this.toArray(metaTagsData.doi)) - .concat(this.toArray(metaTagsData.identifier)); + const identifiers = [metaTagsData.url, metaTagsData.doi, metaTagsData.identifier].flatMap((v) => this.toArray(v)); - const metaTagsDefs = { + const metaTagsDefs: Record = { // Citation citation_title: metaTagsData.title, citation_doi: metaTagsData.doi, @@ -188,7 +172,7 @@ export class MetaTagsService { 'dc.creator': metaTagsData.contributors, 'dc.subject': metaTagsData.keywords, - // Open Graph/Facebook + // Open Graph / Facebook 'fb:app_id': metaTagsData.fbAppId, 'og:ttl': 345600, 'og:title': metaTagsData.title, @@ -213,37 +197,44 @@ export class MetaTagsService { 'twitter:image:alt': metaTagsData.imageAlt, }; - return Object.entries(metaTagsDefs) - .reduce((acc: HeadTagDef[], [name, content]) => { - if (content) { - const contentArray = this.toArray(content); - return acc.concat( - contentArray - .filter((contentItem) => contentItem) - .map((contentItem) => ({ - type: 'meta' as const, - attrs: this.makeMetaTagAttrs(name, this.buildMetaTagContent(name, contentItem)), - })) - ); - } - return acc; - }, []) + const tags: HeadTagDef[] = Object.entries(metaTagsDefs) + .flatMap(([name, content]) => { + if (!content) return []; + + return this.toArray(content as DataContent) + .filter(Boolean) + .map((item) => ({ + type: 'meta' as const, + attrs: this.makeMetaTagAttrs(name, this.buildMetaTagContent(name, item)), + })); + }) .filter((tag) => tag.attrs.content); + + const canonicalUrl = this.toArray(metaTagsData.canonicalUrl || metaTagsData.url).find(Boolean); + + if (canonicalUrl) { + tags.push({ + type: 'link', + attrs: { rel: 'canonical', href: String(canonicalUrl), class: this.metaTagClass } as MetaDefinition, + }); + } + + return tags; } private buildMetaTagContent(name: string, content: Content): Content { if (['citation_author', 'dc.creator'].includes(name) && typeof content === 'object') { - const author = content as MetaTagAuthor; - return `${author.familyName}, ${author.givenName}`; + const { familyName, givenName } = content as MetaTagAuthor; + return `${familyName}, ${givenName}`; } return content; } private makeMetaTagAttrs(name: string, content: Content): MetaDefinition { - if (['fb:', 'og:'].includes(name.substring(0, 3))) { - return { property: name, content: String(content), class: this.metaTagClass }; - } - return { name, content: String(content), class: this.metaTagClass } as MetaDefinition; + const isProperty = name.startsWith('og:') || name.startsWith('fb:'); + return isProperty + ? { property: name, content: String(content), class: this.metaTagClass } + : ({ name, content: String(content), class: this.metaTagClass } as MetaDefinition); } private toArray(content: DataContent): Content[] { @@ -254,49 +245,31 @@ export class MetaTagsService { headTags.forEach((tag) => { if (tag.type === 'meta') { this.meta.addTag(tag.attrs); - } else if (tag.type === 'link') { - const link = this.document.createElement('link'); - link.className = this.metaTagClass; - Object.entries(tag.attrs).forEach(([key, value]) => { - link.setAttribute(key, String(value)); - }); - - this.document.head.appendChild(link); - } else if (tag.type === 'script') { - const script = this.document.createElement('script'); - script.className = this.metaTagClass; - Object.entries(tag.attrs).forEach(([key, value]) => { - script.setAttribute(key, String(value)); - }); - - if (tag.content) { - script.textContent = tag.content; - } - - this.document.head.appendChild(script); + return; } - }); - if (headTags.some((tag) => tag.attrs.name === 'citation_title')) { - const titleTag = headTags.find((tag) => tag.attrs.name === 'citation_title'); + const el = this.document.createElement(tag.type); + el.className = this.metaTagClass; + Object.entries(tag.attrs).forEach(([key, value]) => el.setAttribute(key, String(value))); - if (titleTag?.attrs.content) { - const title = `${String(this.defaultMetaTags.siteName)} | ${String(titleTag.attrs.content)}`; - this.title.setTitle(replaceBadEncodedChars(title)); + if (tag.type === 'script' && tag.content) { + el.textContent = tag.content; } + + this.document.head.appendChild(el); + }); + + const citationTitle = headTags.find((tag) => tag.attrs.name === 'citation_title')?.attrs.content; + if (citationTitle) { + this.title.setTitle( + replaceBadEncodedChars(`${String(this.defaultMetaTags.siteName)} | ${String(citationTitle)}`) + ); } } private dispatchZoteroEvent(): void { - if (!isPlatformBrowser(this.platformId)) { - return; - } - - const event = new Event('ZoteroItemUpdated', { - bubbles: true, - cancelable: true, - }); + if (!isPlatformBrowser(this.platformId)) return; - this.document.dispatchEvent(event); + this.document.dispatchEvent(new Event('ZoteroItemUpdated', { bubbles: true, cancelable: true })); } } diff --git a/src/app/shared/services/metadata-records.service.spec.ts b/src/app/shared/services/metadata-records.service.spec.ts new file mode 100644 index 000000000..21b05fc58 --- /dev/null +++ b/src/app/shared/services/metadata-records.service.spec.ts @@ -0,0 +1,52 @@ +import { HttpTestingController } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; + +import { ENVIRONMENT } from '@core/provider/environment.provider'; + +import { MetadataRecordFormat } from '../enums/metadata-record-format.enum'; + +import { MetadataRecordsService } from './metadata-records.service'; + +import { provideOSFCore, provideOSFHttp } from '@testing/osf.testing.provider'; + +describe('MetadataRecordsService', () => { + let service: MetadataRecordsService; + let httpMock: HttpTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideOSFCore(), provideOSFHttp()], + }); + + service = TestBed.inject(MetadataRecordsService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it('returns webUrl from environment', () => { + const environment = TestBed.inject(ENVIRONMENT); + expect(service.webUrl).toBe(environment.webUrl); + }); + + it('requests metadata record in text format', () => { + const osfid = 'ezcuj'; + let responseBody = ''; + + service.getMetadataRecord(osfid, MetadataRecordFormat.SchemaDotOrgDataset).subscribe((result) => { + responseBody = result; + }); + + const request = httpMock.expectOne( + `http://localhost:4200/metadata/${osfid}/?format=${MetadataRecordFormat.SchemaDotOrgDataset}` + ); + expect(request.request.method).toBe('GET'); + expect(request.request.responseType).toBe('text'); + + request.flush('{"@context":"https://schema.org"}'); + + expect(responseBody).toBe('{"@context":"https://schema.org"}'); + }); +}); diff --git a/src/app/shared/services/metadata-records.service.ts b/src/app/shared/services/metadata-records.service.ts index 64e31ef2c..993ece886 100644 --- a/src/app/shared/services/metadata-records.service.ts +++ b/src/app/shared/services/metadata-records.service.ts @@ -1,5 +1,3 @@ -import { Observable } from 'rxjs'; - import { HttpClient } from '@angular/common/http'; import { inject, Injectable } from '@angular/core'; @@ -11,18 +9,15 @@ import { MetadataRecordFormat } from '../enums/metadata-record-format.enum'; providedIn: 'root', }) export class MetadataRecordsService { - private readonly http: HttpClient = inject(HttpClient); + private readonly http = inject(HttpClient); private readonly environment = inject(ENVIRONMENT); get webUrl() { return this.environment.webUrl; } - metadataRecordUrl(osfid: string, format: MetadataRecordFormat): string { - return `${this.webUrl}/metadata/${osfid}/?format=${format}`; - } - - getMetadataRecord(osfid: string, format: MetadataRecordFormat): Observable { - return this.http.get(this.metadataRecordUrl(osfid, format), { responseType: 'text' }); + getMetadataRecord(osfid: string, format: MetadataRecordFormat) { + const url = `${this.webUrl}/metadata/${osfid}/?format=${format}`; + return this.http.get(url, { responseType: 'text' }); } } diff --git a/src/testing/providers/meta-tags-builder.service.mock.ts b/src/testing/providers/meta-tags-builder.service.mock.ts new file mode 100644 index 000000000..beee832a5 --- /dev/null +++ b/src/testing/providers/meta-tags-builder.service.mock.ts @@ -0,0 +1,16 @@ +import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service'; + +type MetaTagsBuilderMethods = + | 'buildProjectMetaTagsData' + | 'buildRegistryMetaTagsData' + | 'buildPreprintMetaTagsData' + | 'buildFileMetaTagsData'; + +export function MetaTagsBuilderServiceMockFactory() { + return { + buildProjectMetaTagsData: jest.fn(), + buildRegistryMetaTagsData: jest.fn(), + buildPreprintMetaTagsData: jest.fn(), + buildFileMetaTagsData: jest.fn(), + } as unknown as jest.Mocked>; +} From 570de894f7cb1e10625d7802569d4191511533c6 Mon Sep 17 00:00:00 2001 From: nsemets Date: Fri, 13 Mar 2026 14:41:45 +0200 Subject: [PATCH 3/3] fix(meta-tags): fixed multiple trigger for meta tags --- .../project/project.component.spec.ts | 1 + src/app/features/project/project.component.ts | 26 ++++++++++++++----- .../registry/registry.component.spec.ts | 1 + .../features/registry/registry.component.ts | 26 ++++++++++++++----- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/app/features/project/project.component.spec.ts b/src/app/features/project/project.component.spec.ts index dac59c787..88b558a8c 100644 --- a/src/app/features/project/project.component.spec.ts +++ b/src/app/features/project/project.component.spec.ts @@ -217,6 +217,7 @@ describe('Component: Project', () => { }), expect.anything() ); + expect(metaTagsService.updateMetaTags).toHaveBeenCalledTimes(1); }); it('should not build or update meta tags when current project is null', () => { diff --git a/src/app/features/project/project.component.ts b/src/app/features/project/project.component.ts index 27cc4df93..bbcec1410 100644 --- a/src/app/features/project/project.component.ts +++ b/src/app/features/project/project.component.ts @@ -74,6 +74,7 @@ export class ProjectComponent implements OnDestroy { private readonly projectId = toSignal(this.route.params.pipe(map((params) => params['id']))); private readonly canonicalPath = signal(this.getCanonicalPathFromSnapshot()); private readonly isFileDetailRoute = signal(this.isFileDetailRouteFromSnapshot()); + private readonly lastMetaTagsKey = signal(null); private readonly allDataLoaded = computed( () => @@ -106,14 +107,27 @@ export class ProjectComponent implements OnDestroy { }); effect(() => { - if (this.allDataLoaded()) { - const currentProjectId = this.currentProject()?.id; - const currentCanonicalPath = this.canonicalPath(); + if (!this.allDataLoaded()) { + this.lastMetaTagsKey.set(null); + return; + } + + const currentProjectId = this.currentProject()?.id; + const currentCanonicalPath = this.canonicalPath(); + + if (!currentProjectId || !currentCanonicalPath || this.isFileDetailRoute()) { + this.lastMetaTagsKey.set(null); + return; + } + + const metaTagsKey = `${currentProjectId}:${currentCanonicalPath}`; - if (currentProjectId && currentCanonicalPath && !this.isFileDetailRoute()) { - this.setMetaTags(); - } + if (this.lastMetaTagsKey() === metaTagsKey) { + return; } + + this.lastMetaTagsKey.set(metaTagsKey); + this.setMetaTags(); }); this.dataciteService diff --git a/src/app/features/registry/registry.component.spec.ts b/src/app/features/registry/registry.component.spec.ts index 78d79a98e..2c8ab38a9 100644 --- a/src/app/features/registry/registry.component.spec.ts +++ b/src/app/features/registry/registry.component.spec.ts @@ -202,6 +202,7 @@ describe('RegistryComponent', () => { }), expect.anything() ); + expect(metaTagsService.updateMetaTags).toHaveBeenCalledTimes(1); }); it('should set canonicalUrl using active subroute path', () => { diff --git a/src/app/features/registry/registry.component.ts b/src/app/features/registry/registry.component.ts index 3c726583c..6b37a90c7 100644 --- a/src/app/features/registry/registry.component.ts +++ b/src/app/features/registry/registry.component.ts @@ -80,6 +80,7 @@ export class RegistryComponent implements OnDestroy { private readonly canonicalPath = signal(this.getCanonicalPathFromSnapshot()); private readonly isFileDetailRoute = signal(this.isFileDetailRouteFromSnapshot()); + private readonly lastMetaTagsKey = signal(null); private readonly allDataLoaded = computed( () => @@ -104,14 +105,27 @@ export class RegistryComponent implements OnDestroy { }); effect(() => { - if (this.allDataLoaded()) { - const currentRegistryId = this.registry()?.id; - const currentCanonicalPath = this.canonicalPath(); + if (!this.allDataLoaded()) { + this.lastMetaTagsKey.set(null); + return; + } + + const currentRegistryId = this.registry()?.id; + const currentCanonicalPath = this.canonicalPath(); + + if (!currentRegistryId || !currentCanonicalPath || this.isFileDetailRoute()) { + this.lastMetaTagsKey.set(null); + return; + } + + const metaTagsKey = `${currentRegistryId}:${currentCanonicalPath}`; - if (currentRegistryId && currentCanonicalPath && !this.isFileDetailRoute()) { - this.setMetaTags(); - } + if (this.lastMetaTagsKey() === metaTagsKey) { + return; } + + this.lastMetaTagsKey.set(metaTagsKey); + this.setMetaTags(); }); this.dataciteService