From f99cade14f074d6f8746e170505bf4dc79d26888 Mon Sep 17 00:00:00 2001 From: nsemets Date: Mon, 29 Jun 2026 17:41:44 +0300 Subject: [PATCH 1/2] fix(registration): fixed registration link for file with view only link --- .../registration-blocks-data.component.html | 7 +- ...registration-blocks-data.component.spec.ts | 75 +++++++++++++++---- .../registration-blocks-data.component.ts | 23 +++++- 3 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.html b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.html index 0f3e2636a..b318cbc51 100644 --- a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.html +++ b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.html @@ -25,7 +25,12 @@

@if (isDraft()) {

{{ file.file_name }}

} @else { - + {{ file.file_name }} } diff --git a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.spec.ts b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.spec.ts index bfc7847a0..b5c846f18 100644 --- a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.spec.ts +++ b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.spec.ts @@ -1,10 +1,15 @@ -import { TestBed } from '@angular/core/testing'; +import { MockProvider } from 'ng-mocks'; + +import { ComponentFixture, TestBed } from '@angular/core/testing'; import { FieldType } from '@osf/shared/enums/field-type.enum'; import { Question } from '@osf/shared/models/registration/page-schema.model'; +import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; import { MOCK_REVIEW } from '@testing/mocks/review.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; +import { provideRouterMock, RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock'; +import { ViewOnlyLinkHelperMock, ViewOnlyLinkHelperMockType } from '@testing/providers/view-only-link-helper.mock'; import { RegistrationBlocksDataComponent } from './registration-blocks-data.component'; @@ -13,39 +18,61 @@ const MOCK_QUESTIONS: Question[] = [ { id: '2', displayText: 'Q2', required: false, responseKey: 'question2', fieldType: FieldType.Checkbox }, ]; +interface SetupOverrides { + routerUrl?: string; + viewOnlyParam?: string | null; +} + describe('RegistrationBlocksDataComponent', () => { - beforeEach(() => { + let component: RegistrationBlocksDataComponent; + let fixture: ComponentFixture; + let routerMock: RouterMockType; + let viewOnlyHelper: ViewOnlyLinkHelperMockType; + + function setup(overrides: SetupOverrides = {}) { + routerMock = RouterMockBuilder.create() + .withUrl(overrides.routerUrl ?? '/') + .build(); + viewOnlyHelper = ViewOnlyLinkHelperMock.simple(); + viewOnlyHelper.getViewOnlyParamFromUrl.mockReturnValue( + overrides.viewOnlyParam !== undefined ? overrides.viewOnlyParam : null + ); + TestBed.configureTestingModule({ imports: [RegistrationBlocksDataComponent], - providers: [provideOSFCore()], + providers: [ + provideOSFCore(), + provideRouterMock(routerMock), + MockProvider(ViewOnlyLinkHelperService, viewOnlyHelper), + ], }); - }); - it('should create', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); + fixture = TestBed.createComponent(RegistrationBlocksDataComponent); + component = fixture.componentInstance; fixture.detectChanges(); + } + + beforeEach(() => setup()); - expect(fixture.componentInstance).toBeTruthy(); + it('should create', () => { + expect(component).toBeTruthy(); }); it('should compute updatedKeysMap from updatedFields', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('updatedFields', ['question1', 'question3']); fixture.detectChanges(); - expect(fixture.componentInstance.updatedKeysMap()).toEqual({ question1: true, question3: true }); + expect(component.updatedKeysMap()).toEqual({ question1: true, question3: true }); }); it('should return empty updatedKeysMap when updatedFields is empty', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('updatedFields', []); fixture.detectChanges(); - expect(fixture.componentInstance.updatedKeysMap()).toEqual({}); + expect(component.updatedKeysMap()).toEqual({}); }); it('should render questions with review data', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('questions', MOCK_QUESTIONS); fixture.componentRef.setInput('reviewData', MOCK_REVIEW); fixture.detectChanges(); @@ -56,7 +83,6 @@ describe('RegistrationBlocksDataComponent', () => { }); it('should show required error when question is required and no data on non-overview page', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('questions', MOCK_QUESTIONS); fixture.componentRef.setInput('reviewData', {}); fixture.componentRef.setInput('isOverviewPage', false); @@ -67,7 +93,6 @@ describe('RegistrationBlocksDataComponent', () => { }); it('should not show required error on overview page', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('questions', MOCK_QUESTIONS); fixture.componentRef.setInput('reviewData', {}); fixture.componentRef.setInput('isOverviewPage', true); @@ -78,7 +103,6 @@ describe('RegistrationBlocksDataComponent', () => { }); it('should show updated tag when field is updated and not original revision', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('questions', MOCK_QUESTIONS); fixture.componentRef.setInput('reviewData', MOCK_REVIEW); fixture.componentRef.setInput('updatedFields', ['question1']); @@ -90,7 +114,6 @@ describe('RegistrationBlocksDataComponent', () => { }); it('should not show updated tag on original revision', () => { - const fixture = TestBed.createComponent(RegistrationBlocksDataComponent); fixture.componentRef.setInput('questions', MOCK_QUESTIONS); fixture.componentRef.setInput('reviewData', MOCK_REVIEW); fixture.componentRef.setInput('updatedFields', ['question1']); @@ -100,4 +123,24 @@ describe('RegistrationBlocksDataComponent', () => { const tags = fixture.nativeElement.querySelectorAll('p-tag'); expect(tags.length).toBe(0); }); + + it('should return empty string when file url is missing', () => { + expect(component.getFileUrl()).toBe(''); + }); + + it('should return original file url when view_only param is absent', () => { + const htmlUrl = 'https://osf.io/abc12/'; + + expect(component.getFileUrl(htmlUrl)).toBe(htmlUrl); + }); + + it('should append view_only query param to file url', () => { + viewOnlyHelper.getViewOnlyParamFromUrl.mockReturnValue('token'); + + const htmlUrl = 'https://osf.io/abc12/'; + const result = component.getFileUrl(htmlUrl); + + expect(viewOnlyHelper.getViewOnlyParamFromUrl).toHaveBeenCalledWith('/'); + expect(result).toBe('https://osf.io/abc12/?view_only=token'); + }); }); diff --git a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.ts b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.ts index d1c49eb2f..908471dbc 100644 --- a/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.ts +++ b/src/app/shared/components/registration-blocks-data/registration-blocks-data.component.ts @@ -3,12 +3,14 @@ import { TranslatePipe } from '@ngx-translate/core'; import { Message } from 'primeng/message'; import { Tag } from 'primeng/tag'; -import { ChangeDetectionStrategy, Component, computed, input } from '@angular/core'; +import { ChangeDetectionStrategy, Component, computed, inject, input } from '@angular/core'; +import { Router } from '@angular/router'; import { INPUT_VALIDATION_MESSAGES } from '@osf/shared/constants/input-validation-messages.const'; import { FieldType } from '@osf/shared/enums/field-type.enum'; import { Question } from '@osf/shared/models/registration/page-schema.model'; import { FixSpecialCharPipe } from '@osf/shared/pipes/fix-special-char.pipe'; +import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; @Component({ selector: 'osf-registration-blocks-data', @@ -18,6 +20,9 @@ import { FixSpecialCharPipe } from '@osf/shared/pipes/fix-special-char.pipe'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class RegistrationBlocksDataComponent { + private readonly router = inject(Router); + private readonly viewOnlyService = inject(ViewOnlyLinkHelperService); + questions = input(); // eslint-disable-next-line @typescript-eslint/no-explicit-any reviewData = input>({}); @@ -34,4 +39,20 @@ export class RegistrationBlocksDataComponent { readonly FieldType = FieldType; readonly INPUT_VALIDATION_MESSAGES = INPUT_VALIDATION_MESSAGES; + + getFileUrl(htmlUrl?: string): string { + if (!htmlUrl) { + return ''; + } + + const viewOnly = this.viewOnlyService.getViewOnlyParamFromUrl(this.router.url); + + if (!viewOnly) { + return htmlUrl; + } + + const url = new URL(htmlUrl); + url.searchParams.set('view_only', viewOnly); + return url.toString(); + } } From 6e5150a88cb2c5c95f816381c490ec943a71f153 Mon Sep 17 00:00:00 2001 From: nsemets Date: Wed, 15 Jul 2026 11:31:20 +0300 Subject: [PATCH 2/2] fix(avol): fixed anonymous contributors for files detail --- .../file-resource-metadata.component.html | 5 +++- .../file-resource-metadata.component.spec.ts | 30 +++++-------------- .../file-resource-metadata.component.ts | 20 +++++-------- src/app/features/files/store/files.state.ts | 9 ++++-- src/app/shared/models/files/file.model.ts | 6 ++++ src/app/shared/services/files.service.ts | 6 ++-- src/testing/providers/files-service.mock.ts | 6 ++-- 7 files changed, 37 insertions(+), 45 deletions(-) diff --git a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.html b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.html index 541f7af8c..cb784e3b8 100644 --- a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.html +++ b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.html @@ -105,7 +105,10 @@

{{ 'common.labels.dateModified' | translate }}

{{ 'common.labels.contributors' | translate }}

- +
} } diff --git a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.spec.ts b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.spec.ts index fffe0f5b0..f2c66555a 100644 --- a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.spec.ts +++ b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.spec.ts @@ -1,31 +1,21 @@ -import { MockComponent, MockProvider } from 'ng-mocks'; +import { MockComponent } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { Router } from '@angular/router'; import { ContributorsListComponent } from '@osf/shared/components/contributors-list/contributors-list.component'; import { ResourceMetadata } from '@osf/shared/models/resource-metadata.model'; -import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; import { MOCK_CONTRIBUTOR, MOCK_CONTRIBUTOR_WITHOUT_HISTORY } from '@testing/mocks/contributors.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; -import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock'; import { BaseSetupOverrides, mergeSignalOverrides, provideMockStore } from '@testing/providers/store-provider.mock'; -import { ViewOnlyLinkHelperMock, ViewOnlyLinkHelperMockType } from '@testing/providers/view-only-link-helper.mock'; import { FilesSelectors } from '../../store'; import { FileResourceMetadataComponent } from './file-resource-metadata.component'; -interface SetupOverrides extends BaseSetupOverrides { - hasViewOnly?: boolean; -} - describe('FileResourceMetadataComponent', () => { let component: FileResourceMetadataComponent; let fixture: ComponentFixture; - let mockRouter: RouterMockType; - let viewOnlyService: ViewOnlyLinkHelperMockType; const mockResourceMetadata: ResourceMetadata = { title: 'Test Resource', @@ -45,18 +35,14 @@ describe('FileResourceMetadataComponent', () => { { selector: FilesSelectors.getContributors, value: mockContributors }, { selector: FilesSelectors.isResourceMetadataLoading, value: false }, { selector: FilesSelectors.isResourceContributorsLoading, value: false }, + { selector: FilesSelectors.isFilesAnonymous, value: false }, ]; - function setup(overrides: SetupOverrides = {}): void { - mockRouter = RouterMockBuilder.create().withUrl('/test').build(); - viewOnlyService = ViewOnlyLinkHelperMock.simple(overrides.hasViewOnly ?? false); - + function setup(overrides: BaseSetupOverrides = {}): void { TestBed.configureTestingModule({ imports: [FileResourceMetadataComponent, MockComponent(ContributorsListComponent)], providers: [ provideOSFCore(), - MockProvider(Router, mockRouter), - MockProvider(ViewOnlyLinkHelperService, viewOnlyService), provideMockStore({ signals: mergeSignalOverrides(defaultSignals, overrides.selectorOverrides) }), ], }); @@ -102,12 +88,12 @@ describe('FileResourceMetadataComponent', () => { expect(component.isResourceContributorsLoading()).toBe(true); }); - it('should set hasViewOnly based on helper service', () => { - setup({ hasViewOnly: true }); + it('should expose isAnonymous from store', () => { + setup({ + selectorOverrides: [{ selector: FilesSelectors.isFilesAnonymous, value: true }], + }); - expect(component.hasViewOnly).toBe(true); - expect(viewOnlyService.hasViewOnlyParam).toHaveBeenCalled(); - expect(viewOnlyService.hasViewOnlyParam).toHaveBeenCalledWith(expect.objectContaining({ url: '/test' })); + expect(component.isAnonymous()).toBe(true); }); it('should handle input changes', () => { diff --git a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.ts b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.ts index 6a56cc300..cd11f29b4 100644 --- a/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.ts +++ b/src/app/features/files/components/file-resource-metadata/file-resource-metadata.component.ts @@ -5,11 +5,9 @@ import { TranslatePipe } from '@ngx-translate/core'; import { Skeleton } from 'primeng/skeleton'; import { DatePipe } from '@angular/common'; -import { ChangeDetectionStrategy, Component, inject, input } from '@angular/core'; -import { Router } from '@angular/router'; +import { ChangeDetectionStrategy, Component, input } from '@angular/core'; import { ContributorsListComponent } from '@osf/shared/components/contributors-list/contributors-list.component'; -import { ViewOnlyLinkHelperService } from '@osf/shared/services/view-only-link-helper.service'; import { FilesSelectors } from '../../store'; @@ -21,15 +19,11 @@ import { FilesSelectors } from '../../store'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class FileResourceMetadataComponent { - private readonly router = inject(Router); - private readonly viewOnlyService = inject(ViewOnlyLinkHelperService); + readonly resourceType = input('nodes'); - resourceType = input('nodes'); - - resourceMetadata = select(FilesSelectors.getResourceMetadata); - contributors = select(FilesSelectors.getContributors); - isResourceMetadataLoading = select(FilesSelectors.isResourceMetadataLoading); - isResourceContributorsLoading = select(FilesSelectors.isResourceContributorsLoading); - - hasViewOnly = this.viewOnlyService.hasViewOnlyParam(this.router); + readonly resourceMetadata = select(FilesSelectors.getResourceMetadata); + readonly contributors = select(FilesSelectors.getContributors); + readonly isResourceMetadataLoading = select(FilesSelectors.isResourceMetadataLoading); + readonly isResourceContributorsLoading = select(FilesSelectors.isResourceContributorsLoading); + readonly isAnonymous = select(FilesSelectors.isFilesAnonymous); } diff --git a/src/app/features/files/store/files.state.ts b/src/app/features/files/store/files.state.ts index 5ba307e6d..9b84843c5 100644 --- a/src/app/features/files/store/files.state.ts +++ b/src/app/features/files/store/files.state.ts @@ -159,9 +159,12 @@ export class FilesState { ctx.patchState({ tags: { ...state.tags, isLoading: true, error: null } }); return this.filesService.getFileTarget(action.fileGuid).pipe( - tap((file) => { - ctx.patchState({ openedFile: { data: file, isLoading: false, error: null } }); - ctx.patchState({ tags: { data: file.tags, isLoading: false, error: null } }); + tap(({ file, meta }) => { + ctx.patchState({ + openedFile: { data: file, isLoading: false, error: null }, + tags: { data: file.tags, isLoading: false, error: null }, + isAnonymous: meta?.anonymous ?? false, + }); }), catchError((error) => handleSectionError(ctx, 'openedFile', error)) ); diff --git a/src/app/shared/models/files/file.model.ts b/src/app/shared/models/files/file.model.ts index f49bc8215..602165a68 100644 --- a/src/app/shared/models/files/file.model.ts +++ b/src/app/shared/models/files/file.model.ts @@ -1,3 +1,4 @@ +import { MetaJsonApi } from '../common/json-api.model'; import { BaseNodeModel } from '../nodes/base-node.model'; import { FileKind } from './../../enums/file-kind.enum'; @@ -32,6 +33,11 @@ export interface FileDetailsModel extends BaseFileModel { target: BaseNodeModel | null; } +export interface FileDetailsWithMeta { + file: FileDetailsModel; + meta?: MetaJsonApi; +} + export interface FileExtraModel { hashes: FileHashesModel; downloads: number; diff --git a/src/app/shared/services/files.service.ts b/src/app/shared/services/files.service.ts index 5d5becd79..4958da928 100644 --- a/src/app/shared/services/files.service.ts +++ b/src/app/shared/services/files.service.ts @@ -30,7 +30,7 @@ import { ConfiguredAddonModel } from '../models/addons/configured-addon.model'; import { ApiData, JsonApiResponse, MetaJsonApi } from '../models/common/json-api.model'; import { ContributorModel } from '../models/contributors/contributor.model'; import { ContributorsResponseJsonApi } from '../models/contributors/contributor-response-json-api.model'; -import { FileDetailsModel, FileModel } from '../models/files/file.model'; +import { FileDetailsModel, FileDetailsWithMeta, FileModel } from '../models/files/file.model'; import { FileFolderModel } from '../models/files/file-folder.model'; import { FileFolderDataJsonApi, @@ -178,10 +178,10 @@ export class FilesService { return `${link}${separator}zip=`; } - getFileTarget(fileGuid: string): Observable { + getFileTarget(fileGuid: string): Observable { return this.jsonApiService .get(`${this.apiUrl}/files/${fileGuid}/?embed=target`) - .pipe(map((response) => FilesMapper.getFileDetails(response.data))); + .pipe(map((response) => ({ file: FilesMapper.getFileDetails(response.data), meta: response.meta }))); } getFileGuid(id: string): Observable { diff --git a/src/testing/providers/files-service.mock.ts b/src/testing/providers/files-service.mock.ts index 37d32e6ee..026b3ca50 100644 --- a/src/testing/providers/files-service.mock.ts +++ b/src/testing/providers/files-service.mock.ts @@ -5,7 +5,7 @@ import { Mock, vi } from 'vitest'; import { HttpEvent } from '@angular/common/http'; import { MetaJsonApi } from '@osf/shared/models/common/json-api.model'; -import { FileDetailsModel, FileModel } from '@osf/shared/models/files/file.model'; +import { FileDetailsModel, FileDetailsWithMeta, FileModel } from '@osf/shared/models/files/file.model'; import { FileFolderModel } from '@osf/shared/models/files/file-folder.model'; import { FileVersionModel } from '@osf/shared/models/files/file-version.model'; import { PaginatedData } from '@osf/shared/models/paginated-data.model'; @@ -39,7 +39,7 @@ type MoveFileFn = ( replace?: boolean ) => Observable; type GetFolderDownloadLinkFn = (link: string) => string; -type GetFileTargetFn = (fileGuid: string) => Observable; +type GetFileTargetFn = (fileGuid: string) => Observable; type GetFileGuidFn = (id: string) => Observable; type GetFileByIdFn = (fileGuid: string) => Observable; type GetFileVersionsFn = (fileGuid: string) => Observable; @@ -113,7 +113,7 @@ export const FilesServiceMock = { renameEntry: vi.fn().mockReturnValue(of(file)), moveFile: vi.fn().mockReturnValue(of({})), getFolderDownloadLink: vi.fn().mockImplementation((link: string) => `${link}?zip=`), - getFileTarget: vi.fn().mockReturnValue(of(fileDetails)), + getFileTarget: vi.fn().mockReturnValue(of({ file: fileDetails })), getFileGuid: vi.fn().mockReturnValue(of(file)), getFileById: vi.fn().mockReturnValue(of(file)), getFileVersions: vi.fn().mockReturnValue(of([])),