From 051a085f6335192433b5bdea69eb01f154928843 Mon Sep 17 00:00:00 2001 From: nsemets Date: Wed, 23 Sep 2026 18:01:11 +0300 Subject: [PATCH 1/4] fix(addons): updated account id logic --- .../connect-configured-addon.component.html | 40 ++++++++++--------- .../storage-item-selector.component.html | 2 +- .../google-file-picker.component.ts | 29 +++++++++----- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html index ee683bd95..f8b6e1fc3 100644 --- a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html +++ b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html @@ -134,25 +134,27 @@

{{ 'settings.addons.connectAddon.chooseExistingAccount' | trans

{{ 'settings.addons.connectAddon.configure' | translate }} {{ addon()?.displayName }}

- + @if (chosenAccountId()) { + + }
diff --git a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html index 0ed868bfd..bd2f4690c 100644 --- a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html +++ b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html @@ -1,7 +1,7 @@
{ + if (!this.isPickerConfigured || !this.accountId()) { + return; + } + + this.loadOauthToken(); + }); + effect(() => { const isReady = !this.isGFPDisabled(); const hasRootFolder = !!this.rootFolder(); @@ -77,7 +85,6 @@ export class GoogleFilePickerComponent implements OnInit { this.googlePicker.loadGapiModules().subscribe({ next: () => { this.initializePicker(); - this.loadOauthToken(); }, error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker auth' } }), }); @@ -146,16 +153,18 @@ export class GoogleFilePickerComponent implements OnInit { } private loadOauthToken(): void { - if (this.accountId()) { - this.store.dispatch(new GetAuthorizedStorageOauthToken(this.accountId(), this.currentAddonType())).subscribe({ - complete: () => { - this.accessToken.set( - this.store.selectSnapshot(AddonsSelectors.getAuthorizedStorageAddonOauthToken(this.accountId())) - ); - this.isGFPDisabled.set(!this.accessToken()); - }, - }); + const accountId = this.accountId(); + + if (!accountId) { + return; } + + this.store.dispatch(new GetAuthorizedStorageOauthToken(accountId, this.currentAddonType())).subscribe({ + complete: () => { + this.accessToken.set(this.store.selectSnapshot(AddonsSelectors.getAuthorizedStorageAddonOauthToken(accountId))); + this.isGFPDisabled.set(!this.accessToken()); + }, + }); } private filePickerCallback(data: GoogleFileDataModel) { From e1acc9bbc63f0667938011c6304f04e8d643cd47 Mon Sep 17 00:00:00 2001 From: nsemets Date: Thu, 24 Sep 2026 12:31:45 +0300 Subject: [PATCH 2/4] fix(project-addons): clean up code --- .../configure-addon.component.html | 6 ++-- .../configure-addon.component.ts | 16 ++------- ...rm-account-connection-modal.component.html | 2 +- ...rm-account-connection-modal.component.scss | 0 ...firm-account-connection-modal.component.ts | 15 ++++---- .../connect-configured-addon.component.html | 6 ++-- ...connect-configured-addon.component.spec.ts | 1 - .../connect-configured-addon.component.ts | 17 ++-------- .../disconnect-addon-modal.component.html | 2 +- .../disconnect-addon-modal.component.scss | 0 .../disconnect-addon-modal.component.ts | 18 +++++----- .../project-addons/components/index.ts | 4 --- .../models/addon-config-actions.model.ts | 4 ++- .../models/addon-config-map.type.ts | 3 -- .../project/project-addons/models/index.ts | 2 -- .../project-addons.component.ts | 34 +++++-------------- .../storage-item-selector.component.html | 2 +- .../storage-item-selector.component.scss | 12 ------- .../storage-item-selector.component.ts | 17 ++-------- .../google-file-picker.component.html | 2 +- .../google-file-picker.component.scss | 6 +--- 21 files changed, 46 insertions(+), 123 deletions(-) delete mode 100644 src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.scss delete mode 100644 src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.scss delete mode 100644 src/app/features/project/project-addons/components/index.ts delete mode 100644 src/app/features/project/project-addons/models/addon-config-map.type.ts delete mode 100644 src/app/features/project/project-addons/models/index.ts diff --git a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html index 59c97dc04..3e4446cc3 100644 --- a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html +++ b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.html @@ -28,7 +28,7 @@

} @@ -36,7 +36,7 @@

icon="fas fa-trash" severity="danger" text - (click)="handleDisconnectAccount()" + (onClick)="handleDisconnectAccount()" (keydown.enter)="handleDisconnectAccount()" >

@@ -63,7 +63,7 @@

diff --git a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.ts b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.ts index d4061482a..a11418d30 100644 --- a/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.ts +++ b/src/app/features/project/project-addons/components/configure-addon/configure-addon.component.ts @@ -2,7 +2,6 @@ import { createDispatchMap, select, Store } from '@ngxs/store'; import { TranslatePipe } from '@ngx-translate/core'; -import { BreadcrumbModule } from 'primeng/breadcrumb'; import { Button } from 'primeng/button'; import { Card } from 'primeng/card'; import { Skeleton } from 'primeng/skeleton'; @@ -19,7 +18,7 @@ import { PLATFORM_ID, signal, } from '@angular/core'; -import { FormControl, FormsModule, ReactiveFormsModule } from '@angular/forms'; +import { FormControl } from '@angular/forms'; import { ActivatedRoute, Router, RouterLink } from '@angular/router'; import { ENVIRONMENT } from '@core/provider/environment.provider'; @@ -46,18 +45,7 @@ import { AddonDialogService } from '../../services/addon-dialog.service'; @Component({ selector: 'osf-configure-addon', - imports: [ - SubHeaderComponent, - TranslatePipe, - Button, - RouterLink, - Card, - ReactiveFormsModule, - FormsModule, - Skeleton, - BreadcrumbModule, - StorageItemSelectorComponent, - ], + imports: [SubHeaderComponent, TranslatePipe, Button, RouterLink, Card, Skeleton, StorageItemSelectorComponent], templateUrl: './configure-addon.component.html', styleUrl: './configure-addon.component.scss', providers: [AddonDialogService], diff --git a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.html b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.html index b24357152..3cc02b29e 100644 --- a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.html +++ b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.html @@ -5,7 +5,7 @@ class="btn-full-width" [label]="'common.buttons.cancel' | translate" severity="info" - (click)="dialogRef.close()" + (onClick)="dialogRef.close()" [disabled]="isSubmitting()" data-test-addon-cancel-button /> diff --git a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.scss b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.scss deleted file mode 100644 index e69de29bb..000000000 diff --git a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts index ceb0de46f..69e140c33 100644 --- a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts +++ b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts @@ -6,7 +6,6 @@ import { Button } from 'primeng/button'; import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; -import { ReactiveFormsModule } from '@angular/forms'; import { OperationNames } from '@osf/shared/enums/operation-names.enum'; import { AddonOperationInvocationService } from '@osf/shared/services/addons/addon-operation-invocation.service'; @@ -14,19 +13,19 @@ import { AddonsSelectors, CreateAddonOperationInvocation } from '@osf/shared/sto @Component({ selector: 'osf-confirm-account-connection-modal', - imports: [Button, ReactiveFormsModule, TranslatePipe], + imports: [Button, TranslatePipe], templateUrl: './confirm-account-connection-modal.component.html', - styleUrl: './confirm-account-connection-modal.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class ConfirmAccountConnectionModalComponent { - private dialogConfig = inject(DynamicDialogConfig); - private operationInvocationService = inject(AddonOperationInvocationService); - dialogRef = inject(DynamicDialogRef); + private readonly dialogConfig = inject(DynamicDialogConfig); + private readonly operationInvocationService = inject(AddonOperationInvocationService); + readonly dialogRef = inject(DynamicDialogRef); + dialogMessage = this.dialogConfig.data.message || ''; - isSubmitting = select(AddonsSelectors.getOperationInvocationSubmitting); + readonly isSubmitting = select(AddonsSelectors.getOperationInvocationSubmitting); - actions = createDispatchMap({ + private readonly actions = createDispatchMap({ createAddonOperationInvocation: CreateAddonOperationInvocation, }); diff --git a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html index f8b6e1fc3..9ce5f1cb3 100644 --- a/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html +++ b/src/app/features/project/project-addons/components/connect-configured-addon/connect-configured-addon.component.html @@ -70,7 +70,7 @@

{{ loginOrChooseAccountText() }}

[label]="'common.buttons.back' | translate" severity="info" class="w-7rem btn-full-width" - (click)="activateCallback(AddonStepperValue.TERMS)" + (onClick)="activateCallback(AddonStepperValue.TERMS)" data-test-addon-back-button > @@ -78,14 +78,14 @@

{{ loginOrChooseAccountText() }}

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..c8904dafd 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 @@ -46,6 +46,5 @@ describe.skip('ConnectAddonComponent', () => { it('should create and initialize with addon data from router state', () => { expect(component).toBeTruthy(); expect(component['addon']()).toEqual(mockAddon); - expect(component['terms']().length).toBeGreaterThan(0); }); }); 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..3e9ba666f 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 @@ -3,15 +3,12 @@ import { createDispatchMap, select } from '@ngxs/store'; import { TranslatePipe, TranslateService } from '@ngx-translate/core'; import { Button } from 'primeng/button'; -import { DialogModule } from 'primeng/dialog'; -import { DynamicDialogModule } from 'primeng/dynamicdialog'; import { RadioButtonModule } from 'primeng/radiobutton'; import { StepPanel, StepPanels, Stepper } from 'primeng/stepper'; -import { TableModule } from 'primeng/table'; import { isPlatformBrowser } from '@angular/common'; import { Component, computed, DestroyRef, inject, PLATFORM_ID, signal, viewChild } from '@angular/core'; -import { FormControl, FormsModule, ReactiveFormsModule } from '@angular/forms'; +import { FormControl, FormsModule } from '@angular/forms'; import { ActivatedRoute, Router, RouterLink } from '@angular/router'; import { ENVIRONMENT } from '@core/provider/environment.provider'; @@ -25,7 +22,6 @@ import { OperationNames } from '@osf/shared/enums/operation-names.enum'; import { ProjectAddonsStepperValue } from '@osf/shared/enums/profile-addons-stepper.enum'; import { getAddonTypeString } from '@osf/shared/helpers/addon-type.helper'; import { AddonModel } from '@osf/shared/models/addons/addon.model'; -import { AddonTerm } from '@osf/shared/models/addons/addon-utils.model'; import { AuthorizedAccountModel } from '@osf/shared/models/addons/authorized-account.model'; import { AuthorizedAddonRequestJsonApi } from '@osf/shared/models/addons/authorized-addon-json-api.model'; import { AddonFormService } from '@osf/shared/services/addons/addon-form.service'; @@ -40,11 +36,9 @@ import { GetAuthorizedCitationAddons, GetAuthorizedLinkAddons, GetAuthorizedStorageAddons, - UpdateAuthorizedAddon, - UpdateConfiguredAddon, } from '@osf/shared/stores/addons'; -import { AddonConfigMap } from '../../models'; +import { AddonConfigMap } from '../../models/addon-config-actions.model'; import { AddonDialogService } from '../../services'; @Component({ @@ -55,17 +49,13 @@ import { AddonDialogService } from '../../services'; StepPanels, Stepper, Button, - TableModule, RouterLink, FormsModule, - ReactiveFormsModule, TranslatePipe, RadioButtonModule, StorageItemSelectorComponent, AddonTermsComponent, AddonSetupAccountFormComponent, - DialogModule, - DynamicDialogModule, ], templateUrl: './connect-configured-addon.component.html', providers: [AddonDialogService], @@ -93,7 +83,6 @@ export class ConnectConfiguredAddonComponent { readonly stepper = viewChild(Stepper); accountNameControl = new FormControl(''); - terms = signal([]); addon = signal(null); addonAuthUrl = signal('/settings/addons'); currentAuthorizedAddonAccounts = signal([]); @@ -129,8 +118,6 @@ export class ConnectConfiguredAddonComponent { getAuthorizedLinkAddons: GetAuthorizedLinkAddons, createAuthorizedAddon: CreateAuthorizedAddon, createConfiguredAddon: CreateConfiguredAddon, - updateConfiguredAddon: UpdateConfiguredAddon, - updateAuthorizedAddon: UpdateAuthorizedAddon, createAddonOperationInvocation: CreateAddonOperationInvocation, }); diff --git a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.html b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.html index 7fdcbf053..b77742e4e 100644 --- a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.html +++ b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.html @@ -12,7 +12,7 @@ class="btn-full-width" [label]="'common.buttons.cancel' | translate" severity="info" - (click)="dialogRef.close()" + (onClick)="dialogRef.close()" [disabled]="isSubmitting()" data-test-addon-cancel-button /> diff --git a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.scss b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.scss deleted file mode 100644 index e69de29bb..000000000 diff --git a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.ts b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.ts index 9395b4b6d..1cba5e35d 100644 --- a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.ts +++ b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.ts @@ -15,25 +15,27 @@ import { AddonsSelectors, DeleteConfiguredAddon } from '@osf/shared/stores/addon selector: 'osf-disconnect-addon-modal', imports: [Button, TranslatePipe], templateUrl: './disconnect-addon-modal.component.html', - styleUrl: './disconnect-addon-modal.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class DisconnectAddonModalComponent { - private dialogConfig = inject(DynamicDialogConfig); - dialogRef = inject(DynamicDialogRef); + private readonly dialogConfig = inject(DynamicDialogConfig); + readonly dialogRef = inject(DynamicDialogRef); + addon = this.dialogConfig.data.addon; dialogMessage = this.dialogConfig.data.message || ''; - isSubmitting = select(AddonsSelectors.getDeleteStorageAddonSubmitting); - selectedFolder = select(AddonsSelectors.getSelectedStorageItem); - selectedItemLabel = computed(() => { + + private readonly actions = createDispatchMap({ deleteConfiguredAddon: DeleteConfiguredAddon }); + + readonly isSubmitting = select(AddonsSelectors.getDeleteStorageAddonSubmitting); + readonly selectedFolder = select(AddonsSelectors.getSelectedStorageItem); + + readonly selectedItemLabel = computed(() => { const addonType = getAddonTypeString(this.addon); return addonType === AddonType.LINK ? 'settings.addons.configureAddon.linkedItem' : 'settings.addons.configureAddon.selectedFolder'; }); - actions = createDispatchMap({ deleteConfiguredAddon: DeleteConfiguredAddon }); - handleDisconnectAddonAccount(): void { if (!this.addon) return; diff --git a/src/app/features/project/project-addons/components/index.ts b/src/app/features/project/project-addons/components/index.ts deleted file mode 100644 index e7ac9e46a..000000000 --- a/src/app/features/project/project-addons/components/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -export { ConfigureAddonComponent } from './configure-addon/configure-addon.component'; -export { ConfirmAccountConnectionModalComponent } from './confirm-account-connection-modal/confirm-account-connection-modal.component'; -export { ConnectConfiguredAddonComponent } from './connect-configured-addon/connect-configured-addon.component'; -export { DisconnectAddonModalComponent } from './disconnect-addon-modal/disconnect-addon-modal.component'; diff --git a/src/app/features/project/project-addons/models/addon-config-actions.model.ts b/src/app/features/project/project-addons/models/addon-config-actions.model.ts index aa4b2645d..b6dc4c7be 100644 --- a/src/app/features/project/project-addons/models/addon-config-actions.model.ts +++ b/src/app/features/project/project-addons/models/addon-config-actions.model.ts @@ -2,7 +2,9 @@ import { Observable } from 'rxjs'; import { AuthorizedAccountModel } from '@osf/shared/models/addons/authorized-account.model'; -export interface AddonConfigActions { +export type AddonConfigMap = Record; + +interface AddonConfigActions { getAddons: () => Observable; getAuthorizedAddons: () => AuthorizedAccountModel[]; } diff --git a/src/app/features/project/project-addons/models/addon-config-map.type.ts b/src/app/features/project/project-addons/models/addon-config-map.type.ts deleted file mode 100644 index 1873fd2a9..000000000 --- a/src/app/features/project/project-addons/models/addon-config-map.type.ts +++ /dev/null @@ -1,3 +0,0 @@ -import { AddonConfigActions } from '.'; - -export type AddonConfigMap = Record; diff --git a/src/app/features/project/project-addons/models/index.ts b/src/app/features/project/project-addons/models/index.ts deleted file mode 100644 index 488a1c42c..000000000 --- a/src/app/features/project/project-addons/models/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from './addon-config-actions.model'; -export * from './addon-config-map.type'; diff --git a/src/app/features/project/project-addons/project-addons.component.ts b/src/app/features/project/project-addons/project-addons.component.ts index c26e7b0e6..a6dc4828f 100644 --- a/src/app/features/project/project-addons/project-addons.component.ts +++ b/src/app/features/project/project-addons/project-addons.component.ts @@ -21,7 +21,7 @@ import { untracked, } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; -import { FormControl, FormsModule } from '@angular/forms'; +import { FormControl } from '@angular/forms'; import { ActivatedRoute } from '@angular/router'; import { UserSelectors } from '@core/store/user'; @@ -42,7 +42,6 @@ import { AddonsQueryParamsService } from '@shared/services/addons-query-params.s import { AddonsSelectors, ClearConfiguredAddons, - DeleteAuthorizedAddon, GetAddonsResourceReference, GetAddonsUserReference, GetCitationAddons, @@ -67,7 +66,6 @@ import { CurrentResourceSelectors } from '@shared/stores/current-resource'; TabPanels, Tabs, TranslatePipe, - FormsModule, LoadingSpinnerComponent, SelectComponent, ], @@ -76,15 +74,16 @@ import { CurrentResourceSelectors } from '@shared/stores/current-resource'; changeDetection: ChangeDetectionStrategy.OnPush, }) export class ProjectAddonsComponent implements OnInit { - private route = inject(ActivatedRoute); - private destroyRef = inject(DestroyRef); - private queryParamsService = inject(AddonsQueryParamsService); - private platformId = inject(PLATFORM_ID); - private isBrowser = isPlatformBrowser(this.platformId); + private readonly route = inject(ActivatedRoute); + private readonly destroyRef = inject(DestroyRef); + private readonly queryParamsService = inject(AddonsQueryParamsService); + private readonly platformId = inject(PLATFORM_ID); + private readonly isBrowser = isPlatformBrowser(this.platformId); readonly tabOptions = ADDON_TAB_OPTIONS; readonly AddonTabValue = AddonTabValue; readonly defaultTabValue = AddonTabValue.ALL_ADDONS; + searchControl = new FormControl(''); searchValue = signal(''); selectedCategory = signal(AddonCategory.EXTERNAL_STORAGE_SERVICES); @@ -127,16 +126,6 @@ export class ProjectAddonsComponent implements OnInit { return ADDON_CATEGORY_OPTIONS; }); - isAddonsLoading = computed(() => { - return ( - this.isStorageAddonsLoading() || - this.isCitationAddonsLoading() || - this.isLinkAddonsLoading() || - this.isRedirectAddonsLoading() || - this.isUserReferenceLoading() || - this.isCurrentUserLoading() - ); - }); isConfiguredAddonsLoading = computed(() => { let categoryLoading; @@ -175,9 +164,7 @@ export class ProjectAddonsComponent implements OnInit { } }); - isAllAddonsTabLoading = computed(() => { - return this.currentAddonsLoading() || this.isConfiguredAddonsLoading(); - }); + isAllAddonsTabLoading = computed(() => this.currentAddonsLoading() || this.isConfiguredAddonsLoading()); actions = createDispatchMap({ getStorageAddons: GetStorageAddons, @@ -189,7 +176,6 @@ export class ProjectAddonsComponent implements OnInit { getConfiguredLinkAddons: GetConfiguredLinkAddons, getAddonsUserReference: GetAddonsUserReference, getAddonsResourceReference: GetAddonsResourceReference, - deleteAuthorizedAddon: DeleteAuthorizedAddon, clearConfiguredAddons: ClearConfiguredAddons, }); @@ -233,9 +219,7 @@ export class ProjectAddonsComponent implements OnInit { } }); - resourceReferenceId = computed(() => { - return this.addonsResourceReference()[0]?.id; - }); + resourceReferenceId = computed(() => this.addonsResourceReference()[0]?.id); currentAction = computed(() => { switch (this.selectedCategory()) { diff --git a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html index bd2f4690c..f9445a7cd 100644 --- a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html +++ b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html @@ -112,7 +112,7 @@

{{ 'settings.addons.configureAddon.selectFolder' | translate }}

{{ 'settings.addons.configureAddon.noFolders' | translate }}

} - @if (showLoadMoreButton() && !isOperationInvocationSubmitting()) { + @if (showLoadMoreButton()) {
.table-cell:first-child { - min-width: 0; - max-width: 95%; - } } &-row:last-child { diff --git a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.ts b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.ts index 26291fadc..2b5c683cd 100644 --- a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.ts +++ b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.ts @@ -91,8 +91,6 @@ export class StorageItemSelectorComponent implements OnInit { operationInvokeWithCursor = output(); save = output(); cancelSelection = output(); - readonly OperationNames = OperationNames; - readonly StorageItemType = StorageItemType; hasInputChanged = signal(false); hasFolderChanged = signal(false); hasResourceTypeChanged = signal(false); @@ -159,19 +157,8 @@ export class StorageItemSelectorComponent implements OnInit { } ngOnInit(): void { - this.initializeFormState(); - this.setupAccountNameTracking(); - } - - private initializeFormState(): void { this.initialResourceType.set(this.selectedResourceType()); - this.resetChangeFlags(); - } - - private resetChangeFlags(): void { - this.hasInputChanged.set(false); - this.hasFolderChanged.set(false); - this.hasResourceTypeChanged.set(false); + this.setupAccountNameTracking(); } private setupAccountNameTracking(): void { @@ -271,7 +258,7 @@ export class StorageItemSelectorComponent implements OnInit { id: itemId, label: itemName, state: { - operationName: mayContainRootCandidates ? OperationNames.LIST_CHILD_ITEMS : OperationNames.GET_ITEM_INFO, + operationName: OperationNames.LIST_CHILD_ITEMS, }, }; diff --git a/src/app/shared/components/google-file-picker/google-file-picker.component.html b/src/app/shared/components/google-file-picker/google-file-picker.component.html index a894d8383..7bafac660 100644 --- a/src/app/shared/components/google-file-picker/google-file-picker.component.html +++ b/src/app/shared/components/google-file-picker/google-file-picker.component.html @@ -1,4 +1,4 @@ -
+
@if (isFolderPicker()) {
Date: Thu, 24 Sep 2026 13:02:06 +0300 Subject: [PATCH 3/4] fix(project-addons): added tests for modals --- ...account-connection-modal.component.spec.ts | 172 +++++++++++++++++- ...firm-account-connection-modal.component.ts | 4 +- .../disconnect-addon-modal.component.spec.ts | 151 ++++++++++++++- 3 files changed, 313 insertions(+), 14 deletions(-) diff --git a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.spec.ts b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.spec.ts index 00acc6dca..14d29483a 100644 --- a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.spec.ts +++ b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.spec.ts @@ -1,25 +1,187 @@ +import { Store } from '@ngxs/store'; + +import { MockProvider } from 'ng-mocks'; + +import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; + +import { Mock, Mocked } from 'vitest'; + import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { OperationNames } from '@osf/shared/enums/operation-names.enum'; +import { OperationInvocationRequestJsonApi } from '@osf/shared/models/addons/addon-operations-json-api.model'; +import { AuthorizedAccountModel } from '@osf/shared/models/addons/authorized-account.model'; +import { AddonOperationInvocationService } from '@osf/shared/services/addons/addon-operation-invocation.service'; +import { AddonsSelectors, CreateAddonOperationInvocation } from '@osf/shared/stores/addons'; + +import { MOCK_ADDON } from '@testing/mocks/addon.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; +import { AddonOperationInvocationServiceMockFactory } from '@testing/providers/addon-operation-invocation.service.mock'; +import { provideDynamicDialogRefMock } from '@testing/providers/dynamic-dialog-ref.mock'; +import { + BaseSetupOverrides, + mergeSignalOverrides, + provideMockStore, + SignalOverride, +} from '@testing/providers/store-provider.mock'; import { ConfirmAccountConnectionModalComponent } from './confirm-account-connection-modal.component'; -describe.skip('ConfirmAccountConnectionModalComponent', () => { +describe('ConfirmAccountConnectionModalComponent', () => { let component: ConfirmAccountConnectionModalComponent; let fixture: ComponentFixture; + let store: Store; + let dialogRef: DynamicDialogRef; + let operationInvocationService: Mocked; + + const selectedAccount: AuthorizedAccountModel = { + ...MOCK_ADDON, + id: 'account-1', + displayName: 'Google Drive', + type: 'authorized-storage-accounts', + authUrl: null, + authorizedCapabilities: ['ACCESS'], + authorizedOperationNames: [OperationNames.LIST_ROOT_ITEMS], + credentialsAvailable: true, + apiBaseUrl: 'https://www.googleapis.com', + defaultRootFolder: '', + oauthToken: 'token', + accountOwnerId: 'owner-1', + externalStorageServiceId: 'service-1', + }; + + const invocationPayload = { + data: { + type: 'addon-operation-invocations', + attributes: { + invocation_status: null, + operation_name: OperationNames.LIST_ROOT_ITEMS, + operation_kwargs: {}, + operation_result: {}, + created: null, + modified: null, + }, + relationships: {}, + }, + } as OperationInvocationRequestJsonApi; + + const defaultSignals: SignalOverride[] = [ + { selector: AddonsSelectors.getOperationInvocationSubmitting, value: false }, + ]; + + interface SetupOverrides extends BaseSetupOverrides { + message?: string; + omitMessage?: boolean; + selectedAccount?: AuthorizedAccountModel | null; + isGoogleDrive?: boolean; + } + + function setup(overrides: SetupOverrides = {}) { + const account = overrides.selectedAccount === undefined ? selectedAccount : overrides.selectedAccount; + const data = { + ...(overrides.omitMessage ? {} : { message: overrides.message ?? 'Connect this account?' }), + selectedAccount: account, + isGoogleDrive: overrides.isGoogleDrive ?? false, + }; + const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides); + operationInvocationService = AddonOperationInvocationServiceMockFactory(); + operationInvocationService.createInitialOperationInvocationPayload.mockReturnValue(invocationPayload); - beforeEach(() => { TestBed.configureTestingModule({ imports: [ConfirmAccountConnectionModalComponent], - providers: [provideOSFCore()], + providers: [ + provideOSFCore(), + provideDynamicDialogRefMock(), + MockProvider(DynamicDialogConfig, { data }), + MockProvider(AddonOperationInvocationService, operationInvocationService), + provideMockStore({ signals }), + ], }); + store = TestBed.inject(Store); + dialogRef = TestBed.inject(DynamicDialogRef); fixture = TestBed.createComponent(ConfirmAccountConnectionModalComponent); component = fixture.componentInstance; fixture.detectChanges(); + } + + it('should read the message from dialog config', () => { + setup(); + + expect(component.dialogMessage).toBe('Connect this account?'); + }); + + it('should use an empty message when dialog data omits it', () => { + setup({ omitMessage: true }); + + expect(component.dialogMessage).toBe(''); + }); + + it('should expose the submitting state from the store', () => { + setup({ + selectorOverrides: [{ selector: AddonsSelectors.getOperationInvocationSubmitting, value: true }], + }); + + expect(component.isSubmitting()).toBe(true); + }); + + it('should render the confirmation message', () => { + setup(); + + expect(fixture.nativeElement.textContent).toContain('Connect this account?'); + }); + + it('should disable cancel while the connection is submitting', () => { + setup({ + selectorOverrides: [{ selector: AddonsSelectors.getOperationInvocationSubmitting, value: true }], + }); + + const buttons = fixture.nativeElement.querySelectorAll('button'); + expect(buttons[0].disabled).toBe(true); + }); + + it('should not connect when the selected account is missing', () => { + setup({ selectedAccount: null }); + (store.dispatch as Mock).mockClear(); + + component.handleConnectAddonAccount(); + + expect(operationInvocationService.createInitialOperationInvocationPayload).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalled(); + expect(dialogRef.close).not.toHaveBeenCalled(); }); - it('should create', () => { - expect(component).toBeTruthy(); + it('should close with success for Google Drive without creating an invocation', () => { + setup({ isGoogleDrive: true }); + (store.dispatch as Mock).mockClear(); + + component.handleConnectAddonAccount(); + + expect(dialogRef.close).toHaveBeenCalledWith({ success: true }); + expect(operationInvocationService.createInitialOperationInvocationPayload).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('should create a root-items invocation and close with success', () => { + setup(); + (store.dispatch as Mock).mockClear(); + + component.handleConnectAddonAccount(); + + expect(operationInvocationService.createInitialOperationInvocationPayload).toHaveBeenCalledWith( + OperationNames.LIST_ROOT_ITEMS, + selectedAccount + ); + expect(store.dispatch).toHaveBeenCalledWith(new CreateAddonOperationInvocation(invocationPayload)); + expect(dialogRef.close).toHaveBeenCalledWith({ success: true }); + }); + + it('should close without a result when cancel is clicked', () => { + setup(); + + const buttons = fixture.nativeElement.querySelectorAll('button'); + buttons[0].click(); + + expect(dialogRef.close).toHaveBeenCalledWith(); }); }); diff --git a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts index 69e140c33..f44314113 100644 --- a/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts +++ b/src/app/features/project/project-addons/components/confirm-account-connection-modal/confirm-account-connection-modal.component.ts @@ -25,9 +25,7 @@ export class ConfirmAccountConnectionModalComponent { dialogMessage = this.dialogConfig.data.message || ''; readonly isSubmitting = select(AddonsSelectors.getOperationInvocationSubmitting); - private readonly actions = createDispatchMap({ - createAddonOperationInvocation: CreateAddonOperationInvocation, - }); + private readonly actions = createDispatchMap({ createAddonOperationInvocation: CreateAddonOperationInvocation }); handleConnectAddonAccount(): void { const selectedAccount = this.dialogConfig.data.selectedAccount; diff --git a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.spec.ts b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.spec.ts index 9540ea0b4..1f6c9c712 100644 --- a/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.spec.ts +++ b/src/app/features/project/project-addons/components/disconnect-addon-modal/disconnect-addon-modal.component.spec.ts @@ -1,25 +1,164 @@ +import { Store } from '@ngxs/store'; + +import { MockProvider } from 'ng-mocks'; + +import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; + +import { Mock } from 'vitest'; + import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ConfiguredAddonType } from '@osf/shared/enums/addon-type.enum'; +import { ConfiguredAddonModel } from '@osf/shared/models/addons/configured-addon.model'; +import { AddonsSelectors, DeleteConfiguredAddon } from '@osf/shared/stores/addons'; + +import { MOCK_CONFIGURED_ADDON } from '@testing/mocks/configured-addon.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; +import { provideDynamicDialogRefMock } from '@testing/providers/dynamic-dialog-ref.mock'; +import { + BaseSetupOverrides, + mergeSignalOverrides, + provideMockStore, + SignalOverride, +} from '@testing/providers/store-provider.mock'; import { DisconnectAddonModalComponent } from './disconnect-addon-modal.component'; -describe.skip('DisconnectAddonModalComponent', () => { +describe('DisconnectAddonModalComponent', () => { let component: DisconnectAddonModalComponent; let fixture: ComponentFixture; + let store: Store; + let dialogRef: DynamicDialogRef; + + const storageAddon: ConfiguredAddonModel = { + ...MOCK_CONFIGURED_ADDON, + type: ConfiguredAddonType.STORAGE, + }; + + const linkAddon: ConfiguredAddonModel = { + ...MOCK_CONFIGURED_ADDON, + type: ConfiguredAddonType.LINK, + }; + + const defaultSignals: SignalOverride[] = [ + { selector: AddonsSelectors.getDeleteStorageAddonSubmitting, value: false }, + { selector: AddonsSelectors.getSelectedStorageItem, value: { itemName: 'Research Folder' } }, + ]; + + interface SetupOverrides extends BaseSetupOverrides { + addon?: ConfiguredAddonModel | null; + message?: string; + omitMessage?: boolean; + detectChanges?: boolean; + } + + function setup(overrides: SetupOverrides = {}) { + const addon = overrides.addon === undefined ? storageAddon : overrides.addon; + const data = overrides.omitMessage ? { addon } : { addon, message: overrides.message ?? 'Disconnect this addon?' }; + const signals = mergeSignalOverrides(defaultSignals, overrides.selectorOverrides); - beforeEach(() => { TestBed.configureTestingModule({ imports: [DisconnectAddonModalComponent], - providers: [provideOSFCore()], + providers: [ + provideOSFCore(), + provideDynamicDialogRefMock(), + MockProvider(DynamicDialogConfig, { data }), + provideMockStore({ signals }), + ], }); + store = TestBed.inject(Store); + dialogRef = TestBed.inject(DynamicDialogRef); fixture = TestBed.createComponent(DisconnectAddonModalComponent); component = fixture.componentInstance; - fixture.detectChanges(); + + if (overrides.detectChanges !== false) { + fixture.detectChanges(); + } + } + + it('should read addon and message from dialog config', () => { + setup(); + + expect(component.addon).toEqual(storageAddon); + expect(component.dialogMessage).toBe('Disconnect this addon?'); + }); + + it('should use an empty message when dialog data omits it', () => { + setup({ omitMessage: true }); + + expect(component.dialogMessage).toBe(''); + }); + + it('should label the selected item as a folder for storage addons', () => { + setup(); + + expect(component.selectedItemLabel()).toBe('settings.addons.configureAddon.selectedFolder'); + }); + + it('should label the selected item as a linked item for link addons', () => { + setup({ addon: linkAddon }); + + expect(component.selectedItemLabel()).toBe('settings.addons.configureAddon.linkedItem'); }); - it('should create', () => { - expect(component).toBeTruthy(); + it('should expose submitting state and the selected folder from the store', () => { + setup({ + selectorOverrides: [ + { selector: AddonsSelectors.getDeleteStorageAddonSubmitting, value: true }, + { selector: AddonsSelectors.getSelectedStorageItem, value: { itemName: 'Shared Drive' } }, + ], + }); + + expect(component.isSubmitting()).toBe(true); + expect(component.selectedFolder()?.itemName).toBe('Shared Drive'); + }); + + it('should render the message, account name, and selected folder', () => { + setup(); + + const text = fixture.nativeElement.textContent; + expect(text).toContain('Disconnect this addon?'); + expect(text).toContain(storageAddon.displayName); + expect(text).toContain('Research Folder'); + }); + + it('should disable actions while disconnect is submitting', () => { + setup({ + selectorOverrides: [{ selector: AddonsSelectors.getDeleteStorageAddonSubmitting, value: true }], + }); + + const buttons = fixture.nativeElement.querySelectorAll('button'); + expect(buttons[0].disabled).toBe(true); + expect(buttons[1].disabled).toBe(true); + }); + + it('should delete the configured addon and close with success', () => { + setup(); + (store.dispatch as Mock).mockClear(); + + component.handleDisconnectAddonAccount(); + + expect(store.dispatch).toHaveBeenCalledWith(new DeleteConfiguredAddon(storageAddon.id, storageAddon.type)); + expect(dialogRef.close).toHaveBeenCalledWith({ success: true }); + }); + + it('should not delete when the addon is missing', () => { + setup({ addon: null, detectChanges: false }); + (store.dispatch as Mock).mockClear(); + + component.handleDisconnectAddonAccount(); + + expect(store.dispatch).not.toHaveBeenCalled(); + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it('should close without a result when cancel is clicked', () => { + setup(); + + const buttons = fixture.nativeElement.querySelectorAll('button'); + buttons[0].click(); + + expect(dialogRef.close).toHaveBeenCalledWith(); }); }); From 2f4aafffd1ad23b535888b8fca79b63ff7832a42 Mon Sep 17 00:00:00 2001 From: nsemets Date: Fri, 25 Sep 2026 10:44:15 +0300 Subject: [PATCH 4/4] fix(addons): fixed google picker --- .../connect-addon.component.scss | 5 - .../connect-addon.component.spec.ts | 1 - .../connect-addon/connect-addon.component.ts | 7 - .../storage-item-selector.component.html | 34 ++-- .../google-file-picker.component.spec.ts | 58 ++++-- .../google-file-picker.component.ts | 185 ++++++++---------- 6 files changed, 140 insertions(+), 150 deletions(-) diff --git a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.scss b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.scss index 4e458d78a..ce7c4536b 100644 --- a/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.scss +++ b/src/app/features/settings/settings-addons/components/connect-addon/connect-addon.component.scss @@ -13,8 +13,3 @@ width: 100%; } } - -.folders-list { - border: 1px solid var(--grey-2); - border-radius: 0.5rem; -} 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..39e8068bb 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 @@ -34,6 +34,5 @@ describe.skip('ConnectAddonComponent', () => { it('should create and initialize with addon data from router state', () => { expect(component).toBeTruthy(); expect(component['addon']()).toEqual(MOCK_ADDON); - expect(component['terms']().length).toBeGreaterThan(0); }); }); 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..97bd9b8d3 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 @@ -4,11 +4,9 @@ import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; import { StepPanel, StepPanels, Stepper } from 'primeng/stepper'; -import { TableModule } from 'primeng/table'; import { isPlatformBrowser } from '@angular/common'; import { Component, computed, DestroyRef, effect, inject, PLATFORM_ID, signal, viewChild } from '@angular/core'; -import { FormsModule, ReactiveFormsModule } from '@angular/forms'; import { Router, RouterLink } from '@angular/router'; import { AddonSetupAccountFormComponent } from '@osf/shared/components/addons/addon-setup-account-form/addon-setup-account-form.component'; @@ -18,7 +16,6 @@ import { AddonServiceNames } from '@osf/shared/enums/addon-service-names.enum'; import { AddonType } from '@osf/shared/enums/addon-type.enum'; import { ProjectAddonsStepperValue } from '@osf/shared/enums/profile-addons-stepper.enum'; import { getAddonTypeString, isAuthorizedAddon } from '@osf/shared/helpers/addon-type.helper'; -import { AddonTerm } from '@osf/shared/models/addons/addon-utils.model'; 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'; @@ -34,10 +31,7 @@ import { AddonsSelectors, CreateAuthorizedAddon, UpdateAuthorizedAddon } from '@ StepPanels, Stepper, Button, - TableModule, RouterLink, - FormsModule, - ReactiveFormsModule, TranslatePipe, AddonTermsComponent, AddonSetupAccountFormComponent, @@ -57,7 +51,6 @@ export class ConnectAddonComponent { readonly AddonType = AddonType; readonly ProjectAddonsStepperValue = ProjectAddonsStepperValue; - terms = signal([]); addon = signal(null); addonAuthUrl = signal('/settings/addons'); diff --git a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html index f9445a7cd..40e23d2af 100644 --- a/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html +++ b/src/app/shared/components/addons/storage-item-selector/storage-item-selector.component.html @@ -1,21 +1,23 @@
- - - {{ item.label | translate }} - - + @if (!isGoogleFilePicker()) { + + + {{ item.label | translate }} + + + }

diff --git a/src/app/shared/components/google-file-picker/google-file-picker.component.spec.ts b/src/app/shared/components/google-file-picker/google-file-picker.component.spec.ts index 07dccc811..e8a9d2ad9 100644 --- a/src/app/shared/components/google-file-picker/google-file-picker.component.spec.ts +++ b/src/app/shared/components/google-file-picker/google-file-picker.component.spec.ts @@ -4,6 +4,7 @@ import { MockProvider } from 'ng-mocks'; import { throwError } from 'rxjs'; +import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { ENVIRONMENT } from '@core/provider/environment.provider'; @@ -11,8 +12,9 @@ import { SENTRY_TOKEN } from '@core/provider/sentry.provider'; import { AddonType } from '@shared/enums/addon-type.enum'; import { StorageItem } from '@shared/models/addons/storage-item.model'; import { GoogleFileDataModel } from '@shared/models/files/google-file-data.model'; +import { GoogleFilePickerModel } from '@shared/models/files/google-file-picker.model'; import { GoogleFilePickerDownloadService } from '@shared/services/google-file-picker.download.service'; -import { GetAuthorizedStorageOauthToken } from '@shared/stores/addons'; +import { AddonsSelectors, GetAuthorizedStorageOauthToken } from '@shared/stores/addons'; import { setupGooglePickerMock } from '@testing/mocks/google-picker.mock'; import { provideOSFCore } from '@testing/osf.testing.provider'; @@ -34,21 +36,38 @@ describe('GoogleFilePickerComponent', () => { let pickerBuilderMock: ReturnType['pickerBuilderMock']; let pickerSetVisibleMock: ReturnType['pickerSetVisibleMock']; + const registeredPickerCallback = (): ((data: GoogleFilePickerModel) => void) | undefined => { + const callback = pickerBuilderMock.setCallback.mock.calls.at(-1)?.[0]; + return typeof callback === 'function' ? callback : undefined; + }; + const rootFolder: StorageItem = { itemId: 'root-folder-id', itemName: 'Root Folder', }; - const setup = (options?: { accountId?: string; isFolderPicker?: boolean; googleFilePickerApiKey?: string }) => { + const setup = (options?: { + accountId?: string; + isFolderPicker?: boolean; + googleFilePickerApiKey?: string; + oauthToken?: string; + detectChanges?: boolean; + }) => { sentryMock = SentryMock.simple(); googlePickerDownloadServiceMock = GoogleFilePickerDownloadServiceMockBuilder.create().build(); ({ pickerBuilderMock, pickerSetVisibleMock } = setupGooglePickerMock()); + const authorizedStorageAddons = options?.oauthToken + ? [{ id: options.accountId ?? '', oauthToken: options.oauthToken }] + : []; + TestBed.configureTestingModule({ imports: [GoogleFilePickerComponent], providers: [ provideOSFCore(), - provideMockStore(), + provideMockStore({ + signals: [{ selector: AddonsSelectors.getAuthorizedStorageAddons, value: signal(authorizedStorageAddons) }], + }), { provide: SENTRY_TOKEN, useValue: sentryMock }, MockProvider(GoogleFilePickerDownloadService, googlePickerDownloadServiceMock), MockProvider(ENVIRONMENT, { @@ -66,7 +85,9 @@ describe('GoogleFilePickerComponent', () => { fixture.componentRef.setInput('rootFolder', rootFolder); fixture.componentRef.setInput('accountId', options?.accountId ?? ''); fixture.componentRef.setInput('currentAddonType', AddonType.STORAGE); - fixture.detectChanges(); + if (options?.detectChanges !== false) { + fixture.detectChanges(); + } }; it('should create', () => { @@ -78,8 +99,6 @@ describe('GoogleFilePickerComponent', () => { it('should disable picker when configuration is missing', () => { setup({ googleFilePickerApiKey: '' }); - component.ngOnInit(); - expect(component.isGFPDisabled()).toBe(true); expect(googlePickerDownloadServiceMock.loadScript).not.toHaveBeenCalled(); }); @@ -87,49 +106,44 @@ describe('GoogleFilePickerComponent', () => { it('should initialize and set folder picker visible on init', () => { setup({ isFolderPicker: true }); - component.ngOnInit(); - expect(googlePickerDownloadServiceMock.loadScript).toHaveBeenCalled(); expect(googlePickerDownloadServiceMock.loadGapiModules).toHaveBeenCalled(); expect(component.visible()).toBe(true); }); it('should capture Sentry error when script loading fails', () => { - setup(); + setup({ detectChanges: false }); const error = new Error('script fail'); googlePickerDownloadServiceMock.loadScript.mockReturnValue(throwError(() => error)); - component.ngOnInit(); + fixture.detectChanges(); expect(sentryMock.captureException).toHaveBeenCalledWith(error, { tags: { feature: 'google-picker load' } }); }); it('should capture Sentry error when gapi modules loading fails', () => { - setup(); + setup({ detectChanges: false }); const error = new Error('gapi fail'); googlePickerDownloadServiceMock.loadGapiModules.mockReturnValue(throwError(() => error)); - component.ngOnInit(); + fixture.detectChanges(); expect(sentryMock.captureException).toHaveBeenCalledWith(error, { tags: { feature: 'google-picker auth' } }); }); it('should dispatch token action and open picker for account id', () => { - setup({ accountId: 'account-1' }); - vi.spyOn(store, 'selectSnapshot').mockReturnValue('oauth-token'); + setup({ accountId: 'account-1', oauthToken: 'oauth-token' }); - component.ngOnInit(); component.createPicker(); expect(store.dispatch).toHaveBeenCalledWith(new GetAuthorizedStorageOauthToken('account-1', AddonType.STORAGE)); - expect(component.accessToken()).toBe('oauth-token'); expect(component.isGFPDisabled()).toBe(false); expect(pickerBuilderMock.setOAuthToken).toHaveBeenCalledWith('oauth-token'); expect(pickerSetVisibleMock).toHaveBeenCalledWith(true); }); it('should send selected item to handleFolderSelection on PICKED action', () => { - setup(); + setup({ accountId: 'account-1', oauthToken: 'oauth-token' }); const handleFolderSelection = vi.fn(); fixture.componentRef.setInput('handleFolderSelection', handleFolderSelection); fixture.detectChanges(); @@ -139,24 +153,26 @@ describe('GoogleFilePickerComponent', () => { id: 42, }; - component.pickerCallback({ + component.createPicker(); + registeredPickerCallback()?.({ action: 'picked', docs: [selectedDoc], }); expect(handleFolderSelection).toHaveBeenCalledWith({ itemName: 'Google Doc', - itemId: 42, + itemId: '42', }); }); it('should ignore callback when action is not PICKED', () => { - setup(); + setup({ accountId: 'account-1', oauthToken: 'oauth-token' }); const handleFolderSelection = vi.fn(); fixture.componentRef.setInput('handleFolderSelection', handleFolderSelection); fixture.detectChanges(); - component.pickerCallback({ + component.createPicker(); + registeredPickerCallback()?.({ action: 'cancel', docs: [{ name: 'Google Doc', id: 42 }], }); diff --git a/src/app/shared/components/google-file-picker/google-file-picker.component.ts b/src/app/shared/components/google-file-picker/google-file-picker.component.ts index 3dd48b0dc..bba91005b 100644 --- a/src/app/shared/components/google-file-picker/google-file-picker.component.ts +++ b/src/app/shared/components/google-file-picker/google-file-picker.component.ts @@ -1,17 +1,29 @@ -import { Store } from '@ngxs/store'; +import { createDispatchMap, select } from '@ngxs/store'; import { TranslatePipe, TranslateService } from '@ngx-translate/core'; import { Button } from 'primeng/button'; -import { ChangeDetectionStrategy, Component, effect, inject, input, OnInit, signal } from '@angular/core'; +import { catchError, EMPTY, switchMap } from 'rxjs'; + +import { + ChangeDetectionStrategy, + Component, + computed, + DestroyRef, + effect, + inject, + input, + OnInit, + signal, +} from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { ENVIRONMENT } from '@core/provider/environment.provider'; import { SENTRY_TOKEN } from '@core/provider/sentry.provider'; import { AddonType } from '@osf/shared/enums/addon-type.enum'; import { GoogleFilePickerDownloadService } from '@osf/shared/services/google-file-picker.download.service'; import { StorageItem } from '@shared/models/addons/storage-item.model'; -import { GoogleFileDataModel } from '@shared/models/files/google-file-data.model'; import { GoogleFilePickerModel } from '@shared/models/files/google-file-picker.model'; import { AddonsSelectors, GetAuthorizedStorageOauthToken } from '@shared/stores/addons'; @@ -24,29 +36,34 @@ import { AddonsSelectors, GetAuthorizedStorageOauthToken } from '@shared/stores/ }) export class GoogleFilePickerComponent implements OnInit { private readonly Sentry = inject(SENTRY_TOKEN); - private readonly store = inject(Store); + private readonly destroyRef = inject(DestroyRef); private readonly environment = inject(ENVIRONMENT); private readonly translateService = inject(TranslateService); private readonly googlePicker = inject(GoogleFilePickerDownloadService); private readonly apiKey = this.environment.googleFilePickerApiKey; private readonly appId = this.environment.googleFilePickerAppId; - private parentId = ''; - private isMultipleSelect!: boolean; - private title!: string; - - isFolderPicker = input.required(); - rootFolder = input(null); - accountId = input(''); - handleFolderSelection = input<(folder: StorageItem) => void>(); - currentAddonType = input(AddonType.STORAGE); - - accessToken = signal(null); - visible = signal(false); - isGFPDisabled = signal(true); - - private get isPickerConfigured() { - return !!this.apiKey && !!this.appId; - } + private readonly isPickerConfigured = !!this.apiKey && !!this.appId; + + private readonly authorizedStorageAddons = select(AddonsSelectors.getAuthorizedStorageAddons); + private readonly actions = createDispatchMap({ + getAuthorizedStorageOauthToken: GetAuthorizedStorageOauthToken, + }); + + readonly isFolderPicker = input.required(); + readonly rootFolder = input(null); + readonly accountId = input(''); + readonly handleFolderSelection = input<(folder: StorageItem) => void>(); + readonly currentAddonType = input(AddonType.STORAGE); + + private readonly scriptsLoaded = signal(false); + + private readonly accessToken = computed(() => { + const accountId = this.accountId(); + return this.authorizedStorageAddons()?.find((addon) => addon.id === accountId)?.oauthToken || null; + }); + + readonly visible = computed(() => this.isFolderPicker() && this.scriptsLoaded()); + readonly isGFPDisabled = computed(() => !this.isPickerConfigured || !this.scriptsLoaded() || !this.accessToken()); constructor() { effect(() => { @@ -56,89 +73,75 @@ export class GoogleFilePickerComponent implements OnInit { this.loadOauthToken(); }); - - effect(() => { - const isReady = !this.isGFPDisabled(); - const hasRootFolder = !!this.rootFolder(); - const isFilePicker = !this.isFolderPicker(); - - if (isReady && hasRootFolder && isFilePicker) { - this.createPicker(); - } - }); } ngOnInit(): void { if (!this.isPickerConfigured) { - this.isGFPDisabled.set(true); return; } - this.parentId = this.isFolderPicker() ? '' : this.rootFolder()?.itemId || ''; - this.title = this.isFolderPicker() - ? this.translateService.instant('settings.addons.configureAddon.googleFilePicker.rootFolderTitle') - : this.translateService.instant('settings.addons.configureAddon.googleFilePicker.fileFolderTitle'); - this.isMultipleSelect = !this.isFolderPicker(); - - this.googlePicker.loadScript().subscribe({ - next: () => { - this.googlePicker.loadGapiModules().subscribe({ - next: () => { - this.initializePicker(); - }, - error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker auth' } }), - }); - }, - error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker load' } }), - }); + this.googlePicker + .loadScript() + .pipe( + catchError((err) => { + this.Sentry.captureException(err, { tags: { feature: 'google-picker load' } }); + return EMPTY; + }), + switchMap(() => this.googlePicker.loadGapiModules()), + takeUntilDestroyed(this.destroyRef) + ) + .subscribe({ + next: () => this.scriptsLoaded.set(true), + error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker auth' } }), + }); } createPicker(): void { - if (!this.isPickerConfigured) return; + if (this.isGFPDisabled()) { + return; + } this.refreshOauthTokenAndOpenPicker(); } private refreshOauthTokenAndOpenPicker(): void { - if (!this.accountId()) { - this.openPickerWithCurrentToken(); - return; - } - - this.store.dispatch(new GetAuthorizedStorageOauthToken(this.accountId(), this.currentAddonType())).subscribe({ - complete: () => { - this.accessToken.set( - this.store.selectSnapshot(AddonsSelectors.getAuthorizedStorageAddonOauthToken(this.accountId())) - ); - this.isGFPDisabled.set(!this.accessToken()); - this.openPickerWithCurrentToken(); - }, - error: () => { - this.openPickerWithCurrentToken(); - }, - }); + this.actions + .getAuthorizedStorageOauthToken(this.accountId(), this.currentAddonType()) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe({ + complete: () => this.openPickerWithCurrentToken(), + error: () => undefined, + }); } private openPickerWithCurrentToken(): void { const google = window.google; + if (!google?.picker) { + return; + } + + const isFolderPicker = this.isFolderPicker(); + const titleKey = isFolderPicker + ? 'settings.addons.configureAddon.googleFilePicker.rootFolderTitle' + : 'settings.addons.configureAddon.googleFilePicker.fileFolderTitle'; const googlePickerView = new google.picker.DocsView(google.picker.ViewId.DOCS); googlePickerView.setSelectFolderEnabled(true); - if (this.isFolderPicker()) { + if (isFolderPicker) { googlePickerView.setMimeTypes('application/vnd.google-apps.folder'); } googlePickerView.setIncludeFolders(true); - googlePickerView.setParent(this.parentId); + googlePickerView.setParent(isFolderPicker ? '' : this.rootFolder()?.itemId || ''); const pickerBuilder = new google.picker.PickerBuilder() .setDeveloperKey(this.apiKey) - .setAppId(this.appId) + .setAppId(String(this.appId)) .addView(googlePickerView) - .setTitle(this.title) + .setTitle(this.translateService.instant(titleKey)) .setOAuthToken(this.accessToken()) .setCallback(this.pickerCallback.bind(this)); - if (this.isMultipleSelect) { + if (!isFolderPicker) { pickerBuilder.enableFeature(google.picker.Feature.MULTISELECT_ENABLED); } @@ -146,39 +149,21 @@ export class GoogleFilePickerComponent implements OnInit { picker.setVisible(true); } - private initializePicker() { - if (this.isFolderPicker()) { - this.visible.set(true); - } - } - private loadOauthToken(): void { - const accountId = this.accountId(); + this.actions.getAuthorizedStorageOauthToken(this.accountId(), this.currentAddonType()); + } - if (!accountId) { + private pickerCallback(data: GoogleFilePickerModel) { + if (data.action !== window.google.picker.Action.PICKED) { return; } - this.store.dispatch(new GetAuthorizedStorageOauthToken(accountId, this.currentAddonType())).subscribe({ - complete: () => { - this.accessToken.set(this.store.selectSnapshot(AddonsSelectors.getAuthorizedStorageAddonOauthToken(accountId))); - this.isGFPDisabled.set(!this.accessToken()); - }, - }); - } - - private filePickerCallback(data: GoogleFileDataModel) { - this.handleFolderSelection()?.( - Object({ - itemName: data.name, - itemId: data.id, - }) - ); - } - - pickerCallback(data: GoogleFilePickerModel) { - if (data.action === window.google.picker.Action.PICKED) { - this.filePickerCallback(data.docs[0]); + const handleFolderSelection = this.handleFolderSelection(); + for (const selectedFile of data.docs ?? []) { + handleFolderSelection?.({ + itemName: selectedFile.name, + itemId: String(selectedFile.id), + }); } } }