diff --git a/src/app/core/guards/is-file-provider.guard.spec.ts b/src/app/core/guards/is-file-provider.guard.spec.ts index 49e107f84..fce7d2131 100644 --- a/src/app/core/guards/is-file-provider.guard.spec.ts +++ b/src/app/core/guards/is-file-provider.guard.spec.ts @@ -1,49 +1,78 @@ -import { ParamMap, UrlSegment } from '@angular/router'; +import { MockProvider } from 'ng-mocks'; +import { Mock } from 'vitest'; + +import { TestBed } from '@angular/core/testing'; +import { Route, UrlSegment } from '@angular/router'; + +import { FileProviderRegistryService } from '@core/services/file-provider-registry.service'; import { FileProvider } from '@osf/features/files/constants'; import { isFileProvider } from './is-file-provider.guard'; describe('isFileProvider', () => { - const createMockParamMap = (): ParamMap => ({ - get: () => null, - getAll: () => [], - has: () => false, - keys: [], - }); + let registry: { isValidProvider: Mock }; - const createMockSegment = (path: string): UrlSegment => ({ - path, - parameters: {}, - parameterMap: createMockParamMap(), - }); + const FOREIGN_PROVIDER = 's3compat'; + const route: Route = {}; + + const createSegments = (...paths: string[]): UrlSegment[] => paths.map((path) => new UrlSegment(path, {})); - const createMockSegments = (path: string) => [createMockSegment(path)]; + const runGuard = (segments: UrlSegment[]) => TestBed.runInInjectionContext(() => isFileProvider(route, segments)); - it('should return true when id matches a FileProvider value', () => { + beforeEach(() => { + const validProviders: string[] = [...Object.values(FileProvider), FOREIGN_PROVIDER]; + + registry = { + isValidProvider: vi.fn((providerName: string) => validProviders.includes(providerName.toLowerCase())), + }; + + TestBed.configureTestingModule({ + providers: [MockProvider(FileProviderRegistryService, registry)], + }); + }); + + it('should return true when id matches a built-in FileProvider value', () => { Object.values(FileProvider).forEach((provider) => { - const result = isFileProvider({} as any, createMockSegments(provider)); - expect(result).toBe(true); + expect(runGuard(createSegments(provider))).toBe(true); + expect(registry.isValidProvider).toHaveBeenCalledWith(provider); }); }); - it('should return false when id does not match any FileProvider value', () => { - const result = isFileProvider({} as any, createMockSegments('invalid-provider')); - expect(result).toBe(false); + it('should return true when id matches an external provider registered in gravyvalet', () => { + expect(runGuard(createSegments(FOREIGN_PROVIDER))).toBe(true); + expect(registry.isValidProvider).toHaveBeenCalledWith(FOREIGN_PROVIDER); + }); + + it('should return false when id does not match any registered provider', () => { + expect(runGuard(createSegments('invalid-provider'))).toBe(false); + expect(registry.isValidProvider).toHaveBeenCalledWith('invalid-provider'); + }); + + it('should return false when the registry has no providers', () => { + registry.isValidProvider.mockReturnValue(false); + + expect(runGuard(createSegments(FileProvider.OsfStorage))).toBe(false); + }); + + it('should only check the first segment', () => { + expect(runGuard(createSegments(FileProvider.GoogleDrive, 'subfolder', 'file.txt'))).toBe(true); + expect(registry.isValidProvider).toHaveBeenCalledTimes(1); + expect(registry.isValidProvider).toHaveBeenCalledWith(FileProvider.GoogleDrive); }); it('should return false when segments array is empty', () => { - const result = isFileProvider({} as any, []); - expect(result).toBe(false); + expect(runGuard([])).toBe(false); + expect(registry.isValidProvider).not.toHaveBeenCalled(); }); it('should return false when first segment has no path', () => { - const result = isFileProvider({} as any, [createMockSegment('')]); - expect(result).toBe(false); + expect(runGuard(createSegments(''))).toBe(false); + expect(registry.isValidProvider).not.toHaveBeenCalled(); }); it('should return false when first segment is undefined', () => { - const result = isFileProvider({} as any, [undefined as any]); - expect(result).toBe(false); + expect(runGuard([undefined as unknown as UrlSegment])).toBe(false); + expect(registry.isValidProvider).not.toHaveBeenCalled(); }); }); diff --git a/src/app/core/guards/is-file-provider.guard.ts b/src/app/core/guards/is-file-provider.guard.ts index 0dd88d330..9025b9602 100644 --- a/src/app/core/guards/is-file-provider.guard.ts +++ b/src/app/core/guards/is-file-provider.guard.ts @@ -1,9 +1,20 @@ +import { inject } from '@angular/core'; import { CanMatchFn, Route, UrlSegment } from '@angular/router'; -import { FileProvider } from '@osf/features/files/constants'; +import { FileProviderRegistryService } from '@core/services/file-provider-registry.service'; +/** + * Route guard that checks if a file provider is valid. + * Supports both built-in providers (osfstorage, googledrive, etc.) and + * dynamically discovered external storage services (foreign addons like s3compat). + */ export const isFileProvider: CanMatchFn = (route: Route, segments: UrlSegment[]) => { const id = segments[0]?.path; + if (!id) { + return false; + } - return !!(id && Object.values(FileProvider).some((provider) => provider === id)); + const registry = inject(FileProviderRegistryService); + + return registry.isValidProvider(id); }; diff --git a/src/app/core/provider/application.initialization.provider.spec.ts b/src/app/core/provider/application.initialization.provider.spec.ts index 05899ea1d..388e2d608 100644 --- a/src/app/core/provider/application.initialization.provider.spec.ts +++ b/src/app/core/provider/application.initialization.provider.spec.ts @@ -3,6 +3,7 @@ import { MockProvider } from 'ng-mocks'; import { PLATFORM_ID } from '@angular/core'; import { TestBed } from '@angular/core/testing'; +import { FileProviderRegistryService } from '@core/services/file-provider-registry.service'; import { OSFConfigService } from '@core/services/osf-config.service'; import { EnvironmentModel } from '@osf/shared/models/environment.model'; @@ -26,6 +27,7 @@ vi.mock('@sentry/angular', () => { describe('initializeApplication', () => { let configServiceMock: { load: ReturnType }; + let fileProviderRegistryMock: { initialize: ReturnType }; let googleTagManagerConfigurationMock: { set: ReturnType }; let environment: EnvironmentModel; let sentryInitMock: ReturnType; @@ -33,6 +35,7 @@ describe('initializeApplication', () => { function setup(platformId: 'browser' | 'server', environmentOverrides: Partial = {}) { configServiceMock = { load: vi.fn().mockResolvedValue(undefined) }; + fileProviderRegistryMock = { initialize: vi.fn().mockResolvedValue(undefined) }; googleTagManagerConfigurationMock = { set: vi.fn() }; sentryMock = SentryMock.simple(); @@ -41,6 +44,7 @@ describe('initializeApplication', () => { provideOSFCore(), MockProvider(PLATFORM_ID, platformId), { provide: OSFConfigService, useValue: configServiceMock }, + { provide: FileProviderRegistryService, useValue: fileProviderRegistryMock }, { provide: GoogleTagManagerConfiguration, useValue: googleTagManagerConfigurationMock }, { provide: SENTRY_TOKEN, useValue: sentryMock }, ], @@ -61,6 +65,7 @@ describe('initializeApplication', () => { await TestBed.runInInjectionContext(async () => initializeApplication()()); expect(configServiceMock.load).toHaveBeenCalled(); + expect(fileProviderRegistryMock.initialize).toHaveBeenCalled(); expect(googleTagManagerConfigurationMock.set).toHaveBeenCalledWith({ id: 'GTM-TEST' }); expect(sentryInitMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -89,7 +94,52 @@ describe('initializeApplication', () => { await TestBed.runInInjectionContext(async () => initializeApplication()()); expect(configServiceMock.load).toHaveBeenCalled(); + expect(fileProviderRegistryMock.initialize).toHaveBeenCalled(); expect(googleTagManagerConfigurationMock.set).not.toHaveBeenCalled(); expect(sentryInitMock).not.toHaveBeenCalled(); }); + + it('should initialize the file provider registry only after the config is loaded', async () => { + setup('browser'); + let resolveConfig: () => void = () => undefined; + configServiceMock.load.mockReturnValue( + new Promise((resolve) => { + resolveConfig = resolve; + }) + ); + + const initialization = TestBed.runInInjectionContext(async () => initializeApplication()()); + await new Promise((resolve) => setTimeout(resolve)); + + expect(configServiceMock.load).toHaveBeenCalled(); + expect(fileProviderRegistryMock.initialize).not.toHaveBeenCalled(); + + resolveConfig(); + await initialization; + + expect(fileProviderRegistryMock.initialize).toHaveBeenCalledTimes(1); + }); + + it('should not complete before the file provider registry is initialized', async () => { + setup('browser'); + let resolveRegistry: () => void = () => undefined; + fileProviderRegistryMock.initialize.mockReturnValue( + new Promise((resolve) => { + resolveRegistry = resolve; + }) + ); + let completed = false; + + const initialization = TestBed.runInInjectionContext(async () => initializeApplication()()).then(() => { + completed = true; + }); + await new Promise((resolve) => setTimeout(resolve)); + + expect(completed).toBe(false); + + resolveRegistry(); + await initialization; + + expect(completed).toBe(true); + }); }); diff --git a/src/app/core/provider/application.initialization.provider.ts b/src/app/core/provider/application.initialization.provider.ts index ed08425c7..f36e8c906 100644 --- a/src/app/core/provider/application.initialization.provider.ts +++ b/src/app/core/provider/application.initialization.provider.ts @@ -2,6 +2,7 @@ import { isPlatformBrowser } from '@angular/common'; import { inject, PLATFORM_ID, provideAppInitializer } from '@angular/core'; import { SENTRY_DENY_URLS, SENTRY_IGNORE_ERRORS, sentryBeforeSend } from '@core/helpers/sentry-filter.helper'; +import { FileProviderRegistryService } from '@core/services/file-provider-registry.service'; import { OSFConfigService } from '@core/services/osf-config.service'; import { ENVIRONMENT } from './environment.provider'; @@ -24,9 +25,14 @@ export function initializeApplication() { const configService = inject(OSFConfigService); const googleTagManagerConfiguration = inject(GoogleTagManagerConfiguration); const environment = inject(ENVIRONMENT); + const fileProviderRegistry = inject(FileProviderRegistryService); await configService.load(); + // Initialize the file provider registry to fetch external storage services + // This enables foreign addon support + const registryPromise = fileProviderRegistry.initialize(); + if (isPlatformBrowser(platformId)) { const googleTagManagerId = environment.googleTagManagerId; @@ -79,6 +85,8 @@ export function initializeApplication() { }; new BrowserAgent(newRelicConfig); } + + await registryPromise; }; } diff --git a/src/app/core/services/file-provider-registry.service.spec.ts b/src/app/core/services/file-provider-registry.service.spec.ts new file mode 100644 index 000000000..c23f0ad90 --- /dev/null +++ b/src/app/core/services/file-provider-registry.service.spec.ts @@ -0,0 +1,152 @@ +import { HttpTestingController } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; + +import { BYPASS_ERROR_INTERCEPTOR } from '@core/interceptors/error-interceptor.tokens'; +import { FileProvider } from '@osf/features/files/constants'; +import { AddonGetListResponseJsonApi } from '@osf/shared/models/addons/external-addon-json-api.model'; + +import { getAddonsExternalStorageData } from '@testing/data/addons/addons.external-storage.data'; +import { provideOSFCore, provideOSFHttp } from '@testing/osf.testing.provider'; + +import { FILE_PROVIDER_REGISTRY_TIMEOUT_MS, FileProviderRegistryService } from './file-provider-registry.service'; + +describe('FileProviderRegistryService', () => { + let service: FileProviderRegistryService; + let httpMock: HttpTestingController; + + const FOREIGN_PROVIDER = 's3compat'; + const expectRequest = () => + httpMock.expectOne( + (request) => request.url === 'http://addons.localhost:8000/external-storage-services' && request.method === 'GET' + ); + + // The listing gravyvalet serves, with one service that is not a built-in provider + const getListing = () => getAddonsExternalStorageData() as unknown as AddonGetListResponseJsonApi; + + const getListingWithForeignProvider = (externalServiceName: string = FOREIGN_PROVIDER) => { + const listing = getListing(); + listing.data[0].attributes.external_service_name = externalServiceName; + return listing; + }; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideOSFCore(), provideOSFHttp()], + }); + + service = TestBed.inject(FileProviderRegistryService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpMock.verify(); + }); + + it('should not be initialized before initialize is called', () => { + expect(service.isInitialized()).toBe(false); + expect(service.isValidProvider(FileProvider.OsfStorage)).toBe(false); + expect(service.getValidProviders()).toEqual([]); + }); + + it('should request the names of the external storage services outside the error interceptor', async () => { + const initialization = service.initialize(); + + const request = expectRequest(); + expect(request.request.params.get('fields[external-storage-services]')).toBe('external_service_name'); + expect(request.request.context.get(BYPASS_ERROR_INTERCEPTOR)).toBe(true); + request.flush(getListingWithForeignProvider()); + await initialization; + + expect(service.isInitialized()).toBe(true); + }); + + it('should accept every built-in provider after initialization', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListingWithForeignProvider()); + await initialization; + + Object.values(FileProvider).forEach((provider) => { + expect(service.isValidProvider(provider)).toBe(true); + }); + }); + + it('should accept external providers registered in gravyvalet', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListingWithForeignProvider()); + await initialization; + + expect(service.isValidProvider(FOREIGN_PROVIDER)).toBe(true); + expect(service.getValidProviders()).toEqual(expect.arrayContaining([FOREIGN_PROVIDER, FileProvider.OsfStorage])); + }); + + it('should match provider names case-insensitively', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListingWithForeignProvider('S3Compat')); + await initialization; + + expect(service.isValidProvider('s3compat')).toBe(true); + expect(service.isValidProvider('S3COMPAT')).toBe(true); + expect(service.isValidProvider('OsfStorage')).toBe(true); + }); + + it('should reject unknown providers', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListingWithForeignProvider()); + await initialization; + + expect(service.isValidProvider('unknownprovider')).toBe(false); + }); + + it('should list a provider once when it is both built-in and external', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListing()); + await initialization; + + expect(service.getValidProviders()).toHaveLength(Object.values(FileProvider).length); + }); + + it('should skip external services without a service name', async () => { + const initialization = service.initialize(); + expectRequest().flush(getListingWithForeignProvider('')); + await initialization; + + expect(service.isValidProvider('')).toBe(false); + expect(service.getValidProviders()).toHaveLength(Object.values(FileProvider).length); + }); + + it('should fall back to built-in providers when the request fails', async () => { + const initialization = service.initialize(); + expectRequest().flush('Service Unavailable', { status: 503, statusText: 'Service Unavailable' }); + await initialization; + + expect(service.isInitialized()).toBe(true); + expect(service.isValidProvider(FileProvider.OsfStorage)).toBe(true); + expect(service.isValidProvider(FOREIGN_PROVIDER)).toBe(false); + }); + + it('should fall back to built-in providers when the request does not answer in time', async () => { + vi.useFakeTimers(); + + const initialization = service.initialize(); + const request = expectRequest(); + await vi.advanceTimersByTimeAsync(FILE_PROVIDER_REGISTRY_TIMEOUT_MS); + await initialization; + + expect(request.cancelled).toBe(true); + expect(service.isInitialized()).toBe(true); + expect(service.isValidProvider(FileProvider.OsfStorage)).toBe(true); + expect(service.isValidProvider(FOREIGN_PROVIDER)).toBe(false); + + vi.useRealTimers(); + }); + + it('should send one request for concurrent and repeated initializations', async () => { + const initializations = Promise.all([service.initialize(), service.initialize(), service.initialize()]); + expectRequest().flush(getListingWithForeignProvider()); + await initializations; + + await service.initialize(); + + expect(service.isInitialized()).toBe(true); + }); +}); diff --git a/src/app/core/services/file-provider-registry.service.ts b/src/app/core/services/file-provider-registry.service.ts new file mode 100644 index 000000000..adb9dc5f7 --- /dev/null +++ b/src/app/core/services/file-provider-registry.service.ts @@ -0,0 +1,104 @@ +import { firstValueFrom, timeout } from 'rxjs'; + +import { HttpContext } from '@angular/common/http'; +import { inject, Injectable, signal } from '@angular/core'; + +import { BYPASS_ERROR_INTERCEPTOR } from '@core/interceptors/error-interceptor.tokens'; +import { ENVIRONMENT } from '@core/provider/environment.provider'; +import { FileProvider } from '@osf/features/files/constants'; +import { AddonGetListResponseJsonApi } from '@osf/shared/models/addons/external-addon-json-api.model'; +import { JsonApiService } from '@osf/shared/services/json-api.service'; + +/** Application bootstrap waits for the registry, so the request must not hang. */ +export const FILE_PROVIDER_REGISTRY_TIMEOUT_MS = 5000; + +/** + * Registry service that maintains a set of valid file providers. + * Combines built-in providers with dynamically discovered external storage services. + * + * This enables foreign addon support - external storage services registered in + * gravyvalet are automatically recognized as valid file providers in angular-osf. + */ +@Injectable({ + providedIn: 'root', +}) +export class FileProviderRegistryService { + private readonly jsonApiService = inject(JsonApiService); + private readonly environment = inject(ENVIRONMENT); + + // Set of valid provider names (lowercase) + private readonly _validProviders = signal>(new Set()); + + // Initialization state + private readonly _initialized = signal(false); + private _initPromise: Promise | null = null; + + /** + * Check if a provider name is valid (built-in or external) + */ + isValidProvider(providerName: string): boolean { + return this._validProviders().has(providerName.toLowerCase()); + } + + /** + * Get all valid provider names + */ + getValidProviders(): string[] { + return Array.from(this._validProviders()); + } + + /** + * Check if initialization is complete + */ + isInitialized(): boolean { + return this._initialized(); + } + + /** + * Initialize the registry with built-in and external providers. + * Safe to call multiple times - subsequent calls return the same promise. + */ + async initialize(): Promise { + if (this._initPromise) { + return this._initPromise; + } + + this._initPromise = this._doInitialize(); + return this._initPromise; + } + + private async _doInitialize(): Promise { + // Start with built-in providers + const providers = new Set(Object.values(FileProvider).map((p) => p.toLowerCase())); + + // Fetch the names of the external storage services from gravyvalet. + // The request runs during application bootstrap on every page, so it must stay out of the + // global error interceptor (no toast, no redirect) and must not block the bootstrap for long. + try { + const context = new HttpContext().set(BYPASS_ERROR_INTERCEPTOR, true); + const params = { 'fields[external-storage-services]': 'external_service_name' }; + const response = await firstValueFrom( + this.jsonApiService + .get( + `${this.environment.addonsApiUrl}/external-storage-services`, + params, + context + ) + .pipe(timeout(FILE_PROVIDER_REGISTRY_TIMEOUT_MS)) + ); + + for (const service of response.data) { + const serviceName = service.attributes.external_service_name; + + if (serviceName) { + providers.add(serviceName.toLowerCase()); + } + } + } catch { + // Built-in providers still work; external storage services stay unknown until the next load + } + + this._validProviders.set(providers); + this._initialized.set(true); + } +} diff --git a/src/app/features/files/store/files.model.ts b/src/app/features/files/store/files.model.ts index 9c5dcb4b1..9ba38d3b9 100644 --- a/src/app/features/files/store/files.model.ts +++ b/src/app/features/files/store/files.model.ts @@ -11,6 +11,20 @@ import { FileProvider } from '../constants'; import { OsfFileCustomMetadata } from '../models/file-custom-metadata.model'; import { OsfFileRevision } from '../models/file-revisions.model'; +/** + * Type alias for built-in file providers. + * `FileProvider` is declared without `as const`, so this resolves to `string` today. + */ +type BuiltInFileProvider = (typeof FileProvider)[keyof typeof FileProvider]; + +/** + * A file provider name: a built-in one, or a dynamic one that gravyvalet lists + * (e.g., s3compat from a foreign addon). + * Resolves to `string` today. `(string & {})` keeps dynamic names accepted, and + * lets the IDE suggest the built-in names, if `FileProvider` is ever declared `as const`. + */ +export type FileProviderType = BuiltInFileProvider | (string & {}); + export interface FilesStateModel { files: AsyncStateWithTotalCount; moveDialogFiles: AsyncStateWithTotalCount; @@ -18,7 +32,7 @@ export interface FilesStateModel { moveDialogCurrentFolder: FileFolderModel | null; search: string; sort: string; - provider: (typeof FileProvider)[keyof typeof FileProvider]; + provider: FileProviderType; openedFile: AsyncStateModel; fileMetadata: AsyncStateModel; resourceMetadata: AsyncStateModel; diff --git a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.spec.ts b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.spec.ts index 0cef8ccfe..9212c4062 100644 --- a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.spec.ts +++ b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.spec.ts @@ -1,51 +1,112 @@ +import { Store } from '@ngxs/store'; + import { MockComponents, MockProvider } from 'ng-mocks'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { DialogService } from 'primeng/dynamicdialog'; + +import { TestBed } from '@angular/core/testing'; import { ActivatedRoute, Router } from '@angular/router'; import { AddonSetupAccountFormComponent } from '@osf/shared/components/addons/addon-setup-account-form/addon-setup-account-form.component'; +import { AddonTermsComponent } from '@osf/shared/components/addons/addon-terms/addon-terms.component'; import { StorageItemSelectorComponent } from '@osf/shared/components/addons/storage-item-selector/storage-item-selector.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; -import { CredentialsFormat } from '@osf/shared/enums/addons-credentials-format.enum'; +import { AddonCategory } from '@osf/shared/enums/addons-category.enum'; import { AddonModel } from '@osf/shared/models/addons/addon.model'; +import { ToastService } from '@osf/shared/services/toast.service'; +import { AddonsSelectors, CreateConfiguredAddon } from '@osf/shared/stores/addons'; +import { MOCK_ADDON } from '@testing/mocks/addon.mock'; +import { MOCK_CONFIGURED_ADDON } from '@testing/mocks/configured-addon.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; +import { ActivatedRouteMockBuilder } from '@testing/providers/route-provider.mock'; +import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; import { ConnectConfiguredAddonComponent } from './connect-configured-addon.component'; -describe.skip('ConnectAddonComponent', () => { - let component: ConnectConfiguredAddonComponent; - let fixture: ComponentFixture; - - const mockAddon: AddonModel = { - id: 'test-addon-id', - type: 'external-storage-services', - displayName: 'Test Addon', - credentialsFormat: CredentialsFormat.OAUTH2, - supportedFeatures: ['ACCESS'], - providerName: 'Test Provider', - authUrl: 'https://test.com/auth', - externalServiceName: 'test-service', - }; - - beforeEach(() => { +describe('ConnectConfiguredAddonComponent', () => { + const storageAddon: AddonModel = { ...MOCK_ADDON, type: AddonCategory.EXTERNAL_STORAGE_SERVICES }; + + function setup(addon: AddonModel = storageAddon) { + const toastService = ToastServiceMock.simple(); + const mockRouter = { + ...RouterMockBuilder.create().withUrl('/abc12/addons/connect-addon').build(), + getCurrentNavigation: vi.fn().mockReturnValue({ extras: { state: { addon } } }), + }; + TestBed.configureTestingModule({ imports: [ ConnectConfiguredAddonComponent, - ...MockComponents(SubHeaderComponent, AddonSetupAccountFormComponent, StorageItemSelectorComponent), + ...MockComponents( + SubHeaderComponent, + AddonTermsComponent, + AddonSetupAccountFormComponent, + StorageItemSelectorComponent + ), + ], + providers: [ + provideOSFCore(), + provideMockStore({ + signals: [ + { selector: AddonsSelectors.getAddonsUserReference, value: [{ id: 'user-reference-id', userUri: '' }] }, + { selector: AddonsSelectors.getCreatedOrUpdatedConfiguredAddon, value: MOCK_CONFIGURED_ADDON }, + ], + }), + MockProvider(Router, mockRouter), + MockProvider(ActivatedRoute, ActivatedRouteMockBuilder.create().withParams({ id: 'abc12' }).build()), + MockProvider(ToastService, toastService), + MockProvider(DialogService), ], - providers: [provideOSFCore(), provideMockStore(), MockProvider(Router), MockProvider(ActivatedRoute)], }); - fixture = TestBed.createComponent(ConnectConfiguredAddonComponent); - component = fixture.componentInstance; + const store = TestBed.inject(Store); + const fixture = TestBed.createComponent(ConnectConfiguredAddonComponent); fixture.detectChanges(); - }); + + return { component: fixture.componentInstance, store, toastService, mockRouter }; + } it('should create and initialize with addon data from router state', () => { + const { component } = setup(); + expect(component).toBeTruthy(); - expect(component['addon']()).toEqual(mockAddon); - expect(component['terms']().length).toBeGreaterThan(0); + expect(component.addon()).toEqual(storageAddon); + }); + + it('should create the configured addon and return to the addons list', () => { + const { component, store, mockRouter } = setup(); + + component.handleCreateConfiguredAddon(); + + expect(store.dispatch).toHaveBeenCalledWith(expect.any(CreateConfiguredAddon)); + expect(mockRouter.navigate).toHaveBeenCalledWith(['/abc12/addons'], { + queryParams: { activeTab: 1, addonType: 'storage' }, + }); + }); + + it('should name a built-in service in the success toast', () => { + const { component, toastService } = setup({ ...storageAddon, externalServiceName: 'dropbox' }); + + component.handleCreateConfiguredAddon(); + + expect(toastService.showSuccess).toHaveBeenCalledWith('settings.addons.toast.createSuccess', { + addonName: 'Dropbox', + }); + }); + + it('should fall back to the provider name in the success toast when the service has no built-in name', () => { + const { component, toastService } = setup({ + ...storageAddon, + externalServiceName: 's3compat', + providerName: 'S3 Compatible Storage', + }); + + component.handleCreateConfiguredAddon(); + + expect(toastService.showSuccess).toHaveBeenCalledWith('settings.addons.toast.createSuccess', { + addonName: 'S3 Compatible Storage', + }); }); }); diff --git a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.ts b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.ts index b7907d439..99b72bd01 100644 --- a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.ts +++ b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.ts @@ -216,7 +216,8 @@ export class ConnectConfiguredAddonComponent { }, }); this.toastService.showSuccess('settings.addons.toast.createSuccess', { - addonName: AddonServiceNames[addon.externalServiceName as keyof typeof AddonServiceNames], + addonName: + AddonServiceNames[addon.externalServiceName as keyof typeof AddonServiceNames] || addon.providerName, }); } }, diff --git a/src/app/features/project/project-addons/services/addon-dialog.service.spec.ts b/src/app/features/project/project-addons/services/addon-dialog.service.spec.ts new file mode 100644 index 000000000..1b29fba46 --- /dev/null +++ b/src/app/features/project/project-addons/services/addon-dialog.service.spec.ts @@ -0,0 +1,90 @@ +import { TranslateService } from '@ngx-translate/core'; +import { MockProvider } from 'ng-mocks'; + +import { DialogService } from 'primeng/dynamicdialog'; + +import { Subject } from 'rxjs'; + +import { Mock } from 'vitest'; + +import { TestBed } from '@angular/core/testing'; + +import { ConfiguredAddonModel } from '@shared/models/addons/configured-addon.model'; + +import { MOCK_CONFIGURED_ADDON } from '@testing/mocks/configured-addon.mock'; +import { provideOSFCore } from '@testing/osf.testing.provider'; + +import { DisconnectAddonModalComponent } from '../components/disconnect-addon-modal/disconnect-addon-modal.component'; + +import { AddonDialogService } from './addon-dialog.service'; + +describe('AddonDialogService', () => { + let service: AddonDialogService; + let translateService: TranslateService; + let dialogService: { open: Mock }; + let dialogClose$: Subject<{ success: boolean }>; + + const builtInAddon: ConfiguredAddonModel = { + ...MOCK_CONFIGURED_ADDON, + externalServiceName: 'dropbox', + displayName: 'My Dropbox folder', + }; + + const foreignAddon: ConfiguredAddonModel = { + ...MOCK_CONFIGURED_ADDON, + externalServiceName: 's3compat', + displayName: 'S3 Compatible Storage', + }; + + beforeEach(() => { + dialogClose$ = new Subject<{ success: boolean }>(); + dialogService = { open: vi.fn().mockReturnValue({ onClose: dialogClose$ }) }; + + TestBed.configureTestingModule({ + providers: [provideOSFCore(), MockProvider(DialogService, dialogService)], + }); + + service = TestBed.inject(AddonDialogService); + translateService = TestBed.inject(TranslateService); + }); + + it('should open the disconnect dialog for the given addon', () => { + service.openDisconnectDialog(builtInAddon); + + expect(dialogService.open).toHaveBeenCalledTimes(1); + const [component, config] = dialogService.open.mock.calls[0]; + expect(component).toBe(DisconnectAddonModalComponent); + expect(config.data.addon).toBe(builtInAddon); + }); + + it('should name a built-in service in the disconnect dialog header', () => { + service.openDisconnectDialog(builtInAddon); + + expect(translateService.instant).toHaveBeenCalledWith('settings.addons.configureAddon.disconnect', { + addonName: 'Dropbox', + }); + }); + + it('should fall back to the addon display name when the service has no built-in name', () => { + service.openDisconnectDialog(foreignAddon); + + expect(translateService.instant).toHaveBeenCalledWith('settings.addons.configureAddon.disconnect', { + addonName: 'S3 Compatible Storage', + }); + }); + + it('should return the result of the disconnect dialog', () => { + const results: { success: boolean }[] = []; + + service.openDisconnectDialog(foreignAddon).subscribe((result) => results.push(result)); + dialogClose$.next({ success: true }); + + expect(results).toEqual([{ success: true }]); + }); + + it('should throw when the disconnect dialog cannot be opened', () => { + dialogService.open.mockReturnValue(null); + + expect(() => service.openDisconnectDialog(foreignAddon)).toThrow('common.errorMessages.dialogOpenError'); + }); +}); diff --git a/src/app/features/project/project-addons/services/addon-dialog.service.ts b/src/app/features/project/project-addons/services/addon-dialog.service.ts index 15b551576..cad0428bc 100644 --- a/src/app/features/project/project-addons/services/addon-dialog.service.ts +++ b/src/app/features/project/project-addons/services/addon-dialog.service.ts @@ -24,7 +24,7 @@ export class AddonDialogService { const dialogRef = this.dialogService.open(DisconnectAddonModalComponent, { focusOnShow: false, header: this.translateService.instant('settings.addons.configureAddon.disconnect', { - addonName: AddonServiceNames[addon.externalServiceName as keyof typeof AddonServiceNames], + addonName: AddonServiceNames[addon.externalServiceName as keyof typeof AddonServiceNames] || addon.displayName, }), closeOnEscape: true, modal: true, diff --git a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.spec.ts b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.spec.ts index 1729311f2..dbdfc7203 100644 --- a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.spec.ts +++ b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.spec.ts @@ -1,39 +1,171 @@ -import { MockComponents } from 'ng-mocks'; +import { Store } from '@ngxs/store'; -import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { provideRouter } from '@angular/router'; +import { MockComponents, MockProvider } from 'ng-mocks'; + +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; import { AddonSetupAccountFormComponent } from '@osf/shared/components/addons/addon-setup-account-form/addon-setup-account-form.component'; import { AddonTermsComponent } from '@osf/shared/components/addons/addon-terms/addon-terms.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; +import { AuthorizedAccountType } from '@osf/shared/enums/addon-type.enum'; +import { AddonCategory } from '@osf/shared/enums/addons-category.enum'; +import { AuthorizedAddonRequestJsonApi } from '@osf/shared/models/addons/authorized-addon-json-api.model'; +import { AddonOAuthService } from '@osf/shared/services/addons/addon-oauth.service'; +import { ToastService } from '@osf/shared/services/toast.service'; +import { AddonModel } from '@shared/models/addons/addon.model'; +import { AuthorizedAccountModel } from '@shared/models/addons/authorized-account.model'; +import { AddonsSelectors, CreateAuthorizedAddon, UpdateAuthorizedAddon } from '@shared/stores/addons'; import { MOCK_ADDON } from '@testing/mocks/addon.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; +import { RouterMockBuilder } from '@testing/providers/router-provider.mock'; import { provideMockStore } from '@testing/providers/store-provider.mock'; +import { ToastServiceMock } from '@testing/providers/toast-provider.mock'; import { ConnectAddonComponent } from './connect-addon.component'; -describe.skip('ConnectAddonComponent', () => { - let component: ConnectAddonComponent; - let fixture: ComponentFixture; +interface SetupOverrides { + addon?: AddonModel | AuthorizedAccountModel; + createdAccount?: AuthorizedAccountModel; +} + +describe('ConnectAddonComponent', () => { + const storageAddon: AddonModel = { ...MOCK_ADDON, type: AddonCategory.EXTERNAL_STORAGE_SERVICES }; + + const authorizedAccount: AuthorizedAccountModel = { + ...MOCK_ADDON, + type: AuthorizedAccountType.STORAGE, + id: 'account-id', + displayName: 'My account', + authUrl: null, + authorizedCapabilities: ['ACCESS', 'UPDATE'], + authorizedOperationNames: ['list_root_items'], + credentialsAvailable: true, + apiBaseUrl: '', + defaultRootFolder: '', + oauthToken: '', + accountOwnerId: 'user-reference-id', + externalStorageServiceId: 'service-id', + }; + + const payload: AuthorizedAddonRequestJsonApi = { + data: { + type: AuthorizedAccountType.STORAGE, + attributes: { + api_base_url: '', + auth_url: null, + authorized_capabilities: ['ACCESS', 'UPDATE'], + credentials: {}, + credentials_available: false, + display_name: 'My account', + initiate_oauth: false, + }, + relationships: { + account_owner: { data: { type: 'user-references', id: 'user-reference-id' } }, + external_storage_service: { data: { type: 'external-storage-services', id: 'service-id' } }, + }, + }, + }; + + function setup(overrides: SetupOverrides = {}) { + const addon = overrides.addon ?? storageAddon; + const toastService = ToastServiceMock.simple(); + const mockRouter = { + ...RouterMockBuilder.create().withUrl('/settings/addons/connect-addon').build(), + getCurrentNavigation: vi.fn().mockReturnValue({ extras: { state: { addon } } }), + }; - beforeEach(() => { TestBed.configureTestingModule({ imports: [ ConnectAddonComponent, ...MockComponents(SubHeaderComponent, AddonTermsComponent, AddonSetupAccountFormComponent), ], - providers: [provideOSFCore(), provideRouter([]), provideMockStore()], + providers: [ + provideOSFCore(), + provideMockStore({ + signals: [ + { selector: AddonsSelectors.getAddonsUserReference, value: [{ id: 'user-reference-id', userUri: '' }] }, + { + selector: AddonsSelectors.getCreatedOrUpdatedAuthorizedAddon, + value: overrides.createdAccount ?? authorizedAccount, + }, + ], + }), + MockProvider(Router, mockRouter), + MockProvider(ToastService, toastService), + MockProvider(AddonOAuthService, { startOAuthTracking: vi.fn(), stopOAuthTracking: vi.fn() }), + ], }); - fixture = TestBed.createComponent(ConnectAddonComponent); - component = fixture.componentInstance; + const store = TestBed.inject(Store); + const fixture = TestBed.createComponent(ConnectAddonComponent); fixture.detectChanges(); - }); + + return { component: fixture.componentInstance, store, toastService, mockRouter }; + } it('should create and initialize with addon data from router state', () => { + const { component } = setup(); + expect(component).toBeTruthy(); - expect(component['addon']()).toEqual(MOCK_ADDON); - expect(component['terms']().length).toBeGreaterThan(0); + expect(component.addon()).toEqual(storageAddon); + }); + + it('should create the authorized addon and return to the addons list', () => { + const { component, store, mockRouter } = setup(); + + component.handleConnectAuthorizedAddon(payload); + + expect(store.dispatch).toHaveBeenCalledWith(new CreateAuthorizedAddon(payload, 'storage')); + expect(mockRouter.navigate).toHaveBeenCalledWith(['/settings/addons'], { + queryParams: { activeTab: 1, addonType: 'storage' }, + }); + }); + + it('should update the authorized addon when the page was opened for an account', () => { + const { component, store } = setup({ addon: authorizedAccount }); + + component.handleConnectAuthorizedAddon(payload); + + expect(store.dispatch).toHaveBeenCalledWith(new UpdateAuthorizedAddon(payload, 'storage', 'account-id')); + }); + + it('should name a built-in service in the success toast', () => { + const { component, toastService } = setup({ + createdAccount: { ...authorizedAccount, externalServiceName: 'dropbox' }, + }); + + component.handleConnectAuthorizedAddon(payload); + + expect(toastService.showSuccess).toHaveBeenCalledWith('settings.addons.toast.createSuccess', { + addonName: 'Dropbox', + }); + }); + + it('should fall back to the provider name of the addon being connected when the service has no built-in name', () => { + const { component, toastService } = setup({ + addon: { ...storageAddon, externalServiceName: 's3compat', providerName: 'S3 Compatible Storage' }, + createdAccount: { ...authorizedAccount, externalServiceName: 's3compat', providerName: '' }, + }); + + component.handleConnectAuthorizedAddon(payload); + + expect(toastService.showSuccess).toHaveBeenCalledWith('settings.addons.toast.createSuccess', { + addonName: 'S3 Compatible Storage', + }); + }); + + it('should fall back to the provider name of the updated account when the page was opened for an account', () => { + const { component, toastService } = setup({ + addon: { ...authorizedAccount, externalServiceName: 's3compat', providerName: '' }, + createdAccount: { ...authorizedAccount, externalServiceName: 's3compat', providerName: 'S3 Compatible Storage' }, + }); + + component.handleConnectAuthorizedAddon(payload); + + expect(toastService.showSuccess).toHaveBeenCalledWith('settings.addons.toast.createSuccess', { + addonName: 'S3 Compatible Storage', + }); }); }); diff --git a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.ts b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.ts index 95d65c98a..059f576e7 100644 --- a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.ts +++ b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.ts @@ -161,7 +161,10 @@ export class ConnectAddonComponent { }, }); this.toastService.showSuccess('settings.addons.toast.createSuccess', { - addonName: AddonServiceNames[createdAddon?.externalServiceName as keyof typeof AddonServiceNames], + addonName: + AddonServiceNames[createdAddon?.externalServiceName as keyof typeof AddonServiceNames] || + this.addon()?.providerName || + createdAddon?.providerName, }); } }