Skip to content

Commit dcfe028

Browse files
nsemetsnsemets
committed
Merge remote-tracking branch 'upstream/feature/pbs-26-2' into pbs-26-2
2 parents e8b9929 + fe1fc61 commit dcfe028

32 files changed

Lines changed: 1509 additions & 429 deletions

‎src/app/features/files/files.routes.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export const filesRoutes: Routes = [
1818
{
1919
path: ':fileProvider',
2020
canMatch: [isFileProvider],
21+
data: { canonicalPathTemplate: 'files/:fileProvider' },
2122
loadComponent: () => import('@osf/features/files/pages/files/files.component').then((c) => c.FilesComponent),
2223
},
2324
{
@@ -27,18 +28,12 @@ export const filesRoutes: Routes = [
2728
},
2829
{
2930
path: ':fileGuid',
31+
data: { canonicalPathTemplate: 'files/:fileGuid' },
3032
loadComponent: () => {
3133
return import('@osf/features/files/pages/file-detail/file-detail.component').then(
3234
(c) => c.FileDetailComponent
3335
);
3436
},
35-
children: [
36-
{
37-
path: 'metadata',
38-
loadChildren: () => import('@osf/features/metadata/metadata.routes').then((mod) => mod.metadataRoutes),
39-
data: { resourceType: ResourceType.File },
40-
},
41-
],
4237
},
4338
],
4439
},

‎src/app/features/files/pages/file-detail/file-detail.component.ts‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createDispatchMap, select, Store } from '@ngxs/store';
22

3-
import { TranslatePipe, TranslateService } from '@ngx-translate/core';
3+
import { TranslatePipe } from '@ngx-translate/core';
44

55
import { Button } from 'primeng/button';
66
import { Menu } from 'primeng/menu';
@@ -10,7 +10,6 @@ import { Tab, TabList, Tabs } from 'primeng/tabs';
1010
import { switchMap } from 'rxjs';
1111

1212
import { Clipboard } from '@angular/cdk/clipboard';
13-
import { DatePipe } from '@angular/common';
1413
import {
1514
ChangeDetectionStrategy,
1615
Component,
@@ -43,10 +42,10 @@ import { MetadataTabsComponent } from '@osf/shared/components/metadata-tabs/meta
4342
import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component';
4443
import { MetadataResourceEnum } from '@osf/shared/enums/metadata-resource.enum';
4544
import { ResourceType } from '@osf/shared/enums/resource-type.enum';
46-
import { pathJoin } from '@osf/shared/helpers/path-join.helper';
4745
import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service';
4846
import { DataciteService } from '@osf/shared/services/datacite/datacite.service';
4947
import { MetaTagsService } from '@osf/shared/services/meta-tags.service';
48+
import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service';
5049
import { ToastService } from '@osf/shared/services/toast.service';
5150
import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service';
5251
import { FileDetailsModel } from '@shared/models/files/file.model';
@@ -92,7 +91,6 @@ import {
9291
templateUrl: './file-detail.component.html',
9392
styleUrl: './file-detail.component.scss',
9493
changeDetection: ChangeDetectionStrategy.OnPush,
95-
providers: [DatePipe],
9694
})
9795
export class FileDetailComponent {
9896
@HostBinding('class') classes = 'flex flex-column flex-1 w-full h-full';
@@ -106,16 +104,13 @@ export class FileDetailComponent {
106104
readonly customConfirmationService = inject(CustomConfirmationService);
107105

108106
private readonly metaTags = inject(MetaTagsService);
109-
private readonly datePipe = inject(DatePipe);
107+
private readonly metaTagsBuilder = inject(MetaTagsBuilderService);
110108
private readonly viewOnlyService = inject(ViewOnlyLinkHelperService);
111-
private readonly translateService = inject(TranslateService);
112109
private readonly environment = inject(ENVIRONMENT);
113110
private readonly clipboard = inject(Clipboard);
114111

115112
readonly dataciteService = inject(DataciteService);
116113

117-
private readonly webUrl = this.environment.webUrl;
118-
119114
private readonly actions = createDispatchMap({
120115
getFile: GetFile,
121116
getFileRevisions: GetFileRevisions,
@@ -204,24 +199,14 @@ export class FileDetailComponent {
204199
}
205200

206201
const file = this.file();
202+
207203
if (!file) return null;
208204

209-
return {
210-
osfGuid: file.guid,
211-
title: this.fileCustomMetadata()?.title || file.name,
212-
type: this.fileCustomMetadata()?.resourceTypeGeneral,
213-
description:
214-
this.fileCustomMetadata()?.description ?? this.translateService.instant('files.metaTagDescriptionPlaceholder'),
215-
url: pathJoin(this.webUrl, this.fileGuid),
216-
publishedDate: this.datePipe.transform(file.dateCreated, 'yyyy-MM-dd'),
217-
modifiedDate: this.datePipe.transform(file.dateModified, 'yyyy-MM-dd'),
218-
language: this.fileCustomMetadata()?.language,
219-
contributors: this.resourceContributors()?.map((contributor) => ({
220-
fullName: contributor.fullName,
221-
givenName: contributor.givenName,
222-
familyName: contributor.familyName,
223-
})),
224-
};
205+
return this.metaTagsBuilder.buildFileMetaTagsData({
206+
file,
207+
fileMetadata: this.fileCustomMetadata(),
208+
contributors: this.resourceContributors() ?? [],
209+
});
225210
});
226211

227212
constructor() {

‎src/app/features/files/pages/files/files.component.spec.ts‎

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { DialogService } from 'primeng/dynamicdialog';
44

55
import { signal } from '@angular/core';
66
import { ComponentFixture, TestBed } from '@angular/core/testing';
7-
import { ActivatedRoute } from '@angular/router';
7+
import { ActivatedRoute, Router } from '@angular/router';
88

99
import { SENTRY_TOKEN } from '@core/provider/sentry.provider';
1010
import { FileUploadDialogComponent } from '@osf/shared/components/file-upload-dialog/file-upload-dialog.component';
@@ -18,8 +18,10 @@ import { CustomConfirmationService } from '@osf/shared/services/custom-confirmat
1818
import { FilesService } from '@osf/shared/services/files.service';
1919
import { CurrentResourceSelectors } from '@osf/shared/stores/current-resource';
2020
import { GoogleFilePickerComponent } from '@shared/components/google-file-picker/google-file-picker.component';
21+
import { FileLabelModel } from '@shared/models/files/file-label.model';
2122

2223
import { FilesSelectionActionsComponent } from '../../components';
24+
import { FileProvider } from '../../constants';
2325
import { FilesSelectors } from '../../store';
2426

2527
import { FilesComponent } from './files.component';
@@ -29,6 +31,8 @@ import { getNodeFilesMappedData } from '@testing/data/files/node.data';
2931
import { testNode } from '@testing/mocks/base-node.mock';
3032
import { OSFTestingModule } from '@testing/osf.testing.module';
3133
import { MockComponentWithSignal } from '@testing/providers/component-provider.mock';
34+
import { ActivatedRouteMock } from '@testing/providers/route-provider.mock';
35+
import { provideRouterMock, RouterMockType } from '@testing/providers/router-provider.mock';
3236
import { provideMockStore } from '@testing/providers/store-provider.mock';
3337

3438
describe('Component: Files', () => {
@@ -188,4 +192,138 @@ describe('Component: Files', () => {
188192
expect(() => component.updateFilesList()).not.toThrow();
189193
});
190194
});
195+
196+
describe('handleRootFolderChange', () => {
197+
it('should preserve view_only query param when switching storage providers', () => {
198+
const router = TestBed.inject(Router);
199+
const navigateSpy = jest.spyOn(router, 'navigate').mockResolvedValue(true);
200+
201+
const selectedFolder: FileLabelModel = {
202+
label: 'Dropbox',
203+
folder: { provider: FileProvider.Dropbox } as any,
204+
};
205+
206+
component.handleRootFolderChange(selectedFolder);
207+
208+
expect(navigateSpy).toHaveBeenCalledWith([`/${component.resourceId()}/files`, FileProvider.Dropbox], {
209+
queryParamsHandling: 'preserve',
210+
});
211+
});
212+
});
213+
214+
describe('invalid provider fallback effect', () => {
215+
let innerComponent: FilesComponent;
216+
let innerFixture: ComponentFixture<FilesComponent>;
217+
let routerMock: RouterMockType;
218+
219+
beforeEach(async () => {
220+
jest.clearAllMocks();
221+
routerMock = {
222+
...TestBed.inject(Router),
223+
navigate: jest.fn().mockResolvedValue(true),
224+
url: '/abc123/files/unknownprovider?view_only=testtoken',
225+
} as RouterMockType;
226+
227+
await TestBed.configureTestingModule({
228+
imports: [
229+
FilesComponent,
230+
OSFTestingModule,
231+
...MockComponents(
232+
FileUploadDialogComponent,
233+
FormSelectComponent,
234+
GoogleFilePickerComponent,
235+
LoadingSpinnerComponent,
236+
SearchInputComponent,
237+
SubHeaderComponent,
238+
ViewOnlyLinkMessageComponent,
239+
GoogleFilePickerComponent,
240+
FilesSelectionActionsComponent
241+
),
242+
],
243+
providers: [
244+
FilesService,
245+
MockProvider(CustomConfirmationService),
246+
DialogService,
247+
{
248+
provide: SENTRY_TOKEN,
249+
useValue: {
250+
captureException: jest.fn(),
251+
captureMessage: jest.fn(),
252+
setUser: jest.fn(),
253+
},
254+
},
255+
{
256+
provide: ActivatedRoute,
257+
useValue: ActivatedRouteMock.withParams({ fileProvider: 'unknownprovider' }).build(),
258+
},
259+
provideRouterMock(routerMock),
260+
provideMockStore({
261+
signals: [
262+
{
263+
selector: CurrentResourceSelectors.getResourceDetails,
264+
value: testNode,
265+
},
266+
{
267+
selector: FilesSelectors.getRootFolders,
268+
value: getNodeFilesMappedData(),
269+
},
270+
{
271+
selector: FilesSelectors.getCurrentFolder,
272+
value: getNodeFilesMappedData(0),
273+
},
274+
{
275+
selector: FilesSelectors.getConfiguredStorageAddons,
276+
value: getConfiguredAddonsMappedData(),
277+
},
278+
{
279+
selector: FilesSelectors.getProvider,
280+
value: 'osfstorage',
281+
},
282+
{
283+
selector: FilesSelectors.getStorageSupportedFeatures,
284+
value: {
285+
osfstorage: ['AddUpdateFiles', 'DownloadAsZip', 'DeleteFiles', 'CopyInto'],
286+
},
287+
},
288+
],
289+
}),
290+
],
291+
})
292+
.overrideComponent(FilesComponent, {
293+
remove: {
294+
imports: [FilesTreeComponent],
295+
},
296+
add: {
297+
imports: [
298+
MockComponentWithSignal('osf-files-tree', [
299+
'files',
300+
'currentFolder',
301+
'isLoading',
302+
'viewOnly',
303+
'resourceId',
304+
'provider',
305+
'storage',
306+
'totalCount',
307+
'allowedMenuActions',
308+
'supportUpload',
309+
'selectedFiles',
310+
'scrollHeight',
311+
]),
312+
],
313+
},
314+
})
315+
.compileComponents();
316+
317+
innerFixture = TestBed.createComponent(FilesComponent);
318+
innerComponent = innerFixture.componentInstance;
319+
innerFixture.detectChanges();
320+
});
321+
322+
it('should preserve view_only query param when redirecting to osfstorage for invalid provider', () => {
323+
expect(routerMock.navigate).toHaveBeenCalledWith(
324+
[`/${innerComponent.resourceId()}/files`, FileProvider.OsfStorage],
325+
{ queryParamsHandling: 'preserve' }
326+
);
327+
});
328+
});
191329
});

‎src/app/features/files/pages/files/files.component.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,9 @@ export class FilesComponent {
292292
const rootFoldersOption = rootFoldersOptions.find((option) => option.folder.provider === providerName);
293293

294294
if (!rootFoldersOption) {
295-
this.router.navigate([`/${this.resourceId()}/files`, FileProvider.OsfStorage]);
295+
this.router.navigate([`/${this.resourceId()}/files`, FileProvider.OsfStorage], {
296+
queryParamsHandling: 'preserve',
297+
});
296298
} else {
297299
this.currentRootFolder.set({
298300
label: rootFoldersOption.label,
@@ -688,6 +690,6 @@ export class FilesComponent {
688690
handleRootFolderChange(selectedFolder: FileLabelModel) {
689691
const provider = selectedFolder.folder?.provider;
690692
const resourceId = this.resourceId();
691-
this.router.navigate([`/${resourceId}/files`, provider]);
693+
this.router.navigate([`/${resourceId}/files`, provider], { queryParamsHandling: 'preserve' });
692694
}
693695
}

‎src/app/features/metadata/metadata.routes.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export const metadataRoutes: Routes = [
1414
},
1515
{
1616
path: ':recordId',
17+
data: { canonicalPathTemplate: 'metadata/:recordId' },
1718
component: MetadataComponent,
1819
},
1920
];

‎src/app/features/preprints/pages/preprint-details/preprint-details.component.spec.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ import { ActivatedRoute, Router } from '@angular/router';
1313
import { HelpScoutService } from '@core/services/help-scout.service';
1414
import { PrerenderReadyService } from '@core/services/prerender-ready.service';
1515
import { ClearCurrentProvider } from '@core/store/provider';
16+
import { MetaTagsData } from '@osf/shared/models/meta-tags/meta-tags-data.model';
1617
import { CustomDialogService } from '@osf/shared/services/custom-dialog.service';
1718
import { DataciteService } from '@osf/shared/services/datacite/datacite.service';
1819
import { MetaTagsService } from '@osf/shared/services/meta-tags.service';
20+
import { MetaTagsBuilderService } from '@osf/shared/services/meta-tags-builder.service';
1921
import { ToastService } from '@osf/shared/services/toast.service';
2022
import { ContributorsSelectors } from '@osf/shared/stores/contributors';
2123

@@ -55,6 +57,7 @@ import { provideOSFCore } from '@testing/osf.testing.provider';
5557
import { CustomDialogServiceMockBuilder } from '@testing/providers/custom-dialog-provider.mock';
5658
import { HelpScoutServiceMockFactory } from '@testing/providers/help-scout.service.mock';
5759
import { MetaTagsServiceMockFactory } from '@testing/providers/meta-tags.service.mock';
60+
import { MetaTagsBuilderServiceMockFactory } from '@testing/providers/meta-tags-builder.service.mock';
5861
import { PrerenderReadyServiceMockFactory } from '@testing/providers/prerender-ready.service.mock';
5962
import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock';
6063
import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock';
@@ -70,6 +73,7 @@ describe('PreprintDetailsComponent', () => {
7073
let prerenderReadyServiceMock: jest.Mocked<PrerenderReadyService>;
7174
let dataciteServiceMock: ReturnType<typeof DataciteMockFactory>;
7275
let metaTagsServiceMock: ReturnType<typeof MetaTagsServiceMockFactory>;
76+
let metaTagsBuilderServiceMock: ReturnType<typeof MetaTagsBuilderServiceMockFactory>;
7377
let customDialogServiceMock: ReturnType<CustomDialogServiceMockBuilder['build']>;
7478
let toastService: ToastServiceMockType;
7579

@@ -122,6 +126,13 @@ describe('PreprintDetailsComponent', () => {
122126
prerenderReadyServiceMock = PrerenderReadyServiceMockFactory();
123127
dataciteServiceMock = DataciteMockFactory();
124128
metaTagsServiceMock = MetaTagsServiceMockFactory();
129+
metaTagsBuilderServiceMock = MetaTagsBuilderServiceMockFactory();
130+
metaTagsBuilderServiceMock.buildPreprintMetaTagsData.mockImplementation(
131+
({ providerId, preprint }) =>
132+
({
133+
canonicalUrl: `http://localhost:4200/preprints/${providerId}/${preprint?.id}`,
134+
}) as MetaTagsData
135+
);
125136
toastService = ToastServiceMock.simple();
126137
customDialogServiceMock =
127138
overrides?.dialogReturnsCloseValue === false
@@ -167,6 +178,7 @@ describe('PreprintDetailsComponent', () => {
167178
MockProvider(PrerenderReadyService, prerenderReadyServiceMock),
168179
MockProvider(DataciteService, dataciteServiceMock),
169180
MockProvider(MetaTagsService, metaTagsServiceMock),
181+
MockProvider(MetaTagsBuilderService, metaTagsBuilderServiceMock),
170182
MockProvider(CustomDialogService, customDialogServiceMock),
171183
provideMockStore({ signals }),
172184
],
@@ -199,7 +211,19 @@ describe('PreprintDetailsComponent', () => {
199211
it('should update meta tags when preprint and contributors are loaded', () => {
200212
setup();
201213

202-
expect(metaTagsServiceMock.updateMetaTags).toHaveBeenCalled();
214+
expect(metaTagsBuilderServiceMock.buildPreprintMetaTagsData).toHaveBeenCalledWith(
215+
expect.objectContaining({
216+
providerId: 'osf',
217+
preprint: expect.objectContaining({ id: 'preprint-1' }),
218+
})
219+
);
220+
221+
expect(metaTagsServiceMock.updateMetaTags).toHaveBeenCalledWith(
222+
expect.objectContaining({
223+
canonicalUrl: 'http://localhost:4200/preprints/osf/preprint-1',
224+
}),
225+
expect.anything()
226+
);
203227
});
204228

205229
it('should not fetch moderation actions when not moderator and no permissions', () => {
@@ -532,6 +556,7 @@ describe('PreprintDetailsComponent SSR', () => {
532556
MockProvider(Router, routerMock),
533557
MockProvider(CustomDialogService, CustomDialogServiceMockBuilder.create().withDefaultOpen().build()),
534558
MockProvider(DataciteService, DataciteMockFactory()),
559+
MockProvider(MetaTagsBuilderService, MetaTagsBuilderServiceMockFactory()),
535560
MockProvider(MetaTagsService, MetaTagsServiceMockFactory()),
536561
MockProvider(PrerenderReadyService, PrerenderReadyServiceMockFactory()),
537562
MockProvider(HelpScoutService, helpScoutServiceMock),

0 commit comments

Comments
 (0)