diff --git a/README.md b/README.md index 2960168c5..7d086f241 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ take up to 60 seconds once the docker build finishes. - [i18n](docs/i18n.md). - [NGXS Conventions](docs/ngxs.md). - [Testing Strategy](docs/testing.md). +- [Sentry error filtering](docs/sentry.md). ### Optional diff --git a/docs/sentry.md b/docs/sentry.md new file mode 100644 index 000000000..27a1ae607 --- /dev/null +++ b/docs/sentry.md @@ -0,0 +1,86 @@ +# Sentry error filtering + +Sentry collects JavaScript errors from the OSF Angular app in the browser. Many of those events are not application bugs: flaky networks, cancelled requests, missing/deleted API resources, browser extensions, and stale tabs after a deploy. + +Filtering happens on the client when Sentry starts. Change the lists in `src/app/core/helpers/sentry-filter.helper.ts`. That file is passed into `Sentry.init` from `src/app/core/provider/application.initialization.provider.ts`. + +[Sentry filtering docs](https://docs.sentry.io/platforms/javascript/configuration/filtering/) + +## How to read Sentry after this + +If an issue disappears from Sentry, it was probably filtered here. It does not mean the user stopped hitting the error. + +Server failures (HTTP 500–599) and real JavaScript exceptions are still sent. + +## What we drop + +Three independent checks. An event is dropped if **any** of them match. + +### 1. Error message (`ignoreErrors`) + +Sentry treats each string as a **substring**. `Failed to fetch` also matches `Failed to fetch dynamically imported module`. + +| You will stop seeing | Typical cause | +| ------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | +| Handled unknown error | Sentry could not extract a real Error from Angular | +| Non-Error promise rejection captured… | A promise rejected with `undefined` / `null` / a plain object | +| no elements in sequence | RxJS `EmptyError` (empty observable used with `first()` / `single()`) | +| ResizeObserver loop… | Browser layout warning | +| Failed to fetch | Chrome/Edge: offline, CORS, blocked request, including `api.osf.io` / `addons.osf.io` | +| Load failed | Safari equivalent of failed fetch (including `files.osf.io`) | +| NetworkError when attempting to fetch resource | Firefox equivalent of failed fetch | +| Failed to fetch dynamically imported module | Chrome/Edge: lazy chunk failed to load (often an old tab after deploy) | +| error loading dynamically imported module | Firefox: same lazy-chunk failure | +| Importing a module script failed | Safari: same lazy-chunk failure | +| ChunkLoadError / Loading chunk … failed | Webpack/Vite chunk load failure after deploy | +| AbortError / The operation was aborted / The user aborted a request | Request cancelled (navigation, timeout, user abort) | +| Beacon is not defined | Extension or third-party script; not OSF (`navigator.sendBeacon` is a different API) | + +### 2. Script URL (`denyUrls`) + +Errors whose stack frames come from a **browser extension**, not from OSF code: + +- `extensions/` +- `chrome://` +- `chrome-extension://` +- `moz-extension://` +- `safari-extension://` +- `safari-web-extension://` +- `ms-browser-extension://` + +### 3. HTTP status below 500 (`beforeSend`) + +If the event is an HTTP response and the status is **0–499**, it is dropped. Status is read from: + +- Angular `HttpErrorResponse` (including nested `ngOriginalError` / `rejection` / `cause`) +- `Http failure response for …: 410` +- `Server returned code 404 with body "…"` +- `Object captured as exception with keys: …` or `Non-Error exception captured with keys: …` when the keys look like an HTTP response + +| Status | Meaning | Dropped? | +| ------------- | -------------------------------------- | ------------------- | +| 0 | No response (offline, CORS, cancelled) | Yes | +| 401, 403 | Not signed in / not allowed | Yes | +| 404, 410 | Missing or deleted resource | Yes | +| 409, 422, 429 | Conflict, validation, rate limit | Yes | +| Other 4xx | Client/request errors | Yes | +| 500–599 | Server error | **No — still sent** | + +This includes noisy issues such as `Http failure response for https://api.osf.io/v2/users/…: 410` and `Object captured as exception with keys: error, headers, … status … url` when the status is below 500. + +**Side effect:** a 4xx that is actually a frontend bug is also dropped (for example a request URL that contains `undefined`). + +## What still goes to Sentry + +- HTTP 500–599 +- TypeError / ReferenceError / other exceptions that are not in the ignore list and have no HTTP status +- HTTP-looking events where a status cannot be read + +## Changing the filters + +1. Open `src/app/core/helpers/sentry-filter.helper.ts`. +2. Add a **string** to `SENTRY_IGNORE_ERRORS` for a stable message substring, or a **RegExp** for a pattern. +3. Add to `SENTRY_DENY_URLS` only for third-party script origins. +4. Change `sentryBeforeSend` only if the HTTP status rule should change (for example keep 404s that contain `undefined` in the URL). + +After a release, confirm in the Sentry project that volume dropped and that 5xx / real exceptions still appear. diff --git a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts index 4b09dcfcb..7d6024103 100644 --- a/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts +++ b/src/app/core/components/osf-banners/tos-consent-banner/tos-consent-banner.component.ts @@ -58,7 +58,7 @@ export class TosConsentBannerComponent { * if user is authenticated we check whether is accepted terms of service to hide banner or show if not * otherwise user is not authenticated we hide banner always */ - return user ? user.acceptedTermsOfService : true; + return user?.id ? user.acceptedTermsOfService : true; }); /** diff --git a/src/app/core/helpers/sentry-filter.helper.ts b/src/app/core/helpers/sentry-filter.helper.ts new file mode 100644 index 000000000..230c11b9d --- /dev/null +++ b/src/app/core/helpers/sentry-filter.helper.ts @@ -0,0 +1,158 @@ +import { HttpErrorResponse } from '@angular/common/http'; + +import type { ErrorEvent, EventHint } from '@sentry/angular'; + +export const SENTRY_IGNORE_ERRORS: (string | RegExp)[] = [ + 'Handled unknown error', + 'Non-Error promise rejection captured', + 'no elements in sequence', + /ResizeObserver loop/, + 'error loading dynamically imported module', + 'Importing a module script failed', + 'Failed to fetch', + 'Load failed', + 'NetworkError when attempting to fetch resource', + 'AbortError', + 'The operation was aborted', + 'The user aborted a request', + 'ChunkLoadError', + /Loading chunk [\w.-]+ failed/, + 'Beacon is not defined', +]; + +export const SENTRY_DENY_URLS: (string | RegExp)[] = [ + /extensions\//i, + /^chrome:\/\//i, + /^chrome-extension:\/\//i, + /^moz-extension:\/\//i, + /^safari-extension:\/\//i, + /^safari-web-extension:\/\//i, + /^ms-browser-extension:\/\//i, +]; + +const MIN_REPORTED_STATUS = 500; +const MAX_UNWRAP_DEPTH = 4; + +const STATUS_MESSAGE_PATTERNS = [ + /Http failure response for .*: (\d{1,3})(?:\s|$)/, + /Server returned code (\d{1,3})(?:\s|$)/, +]; + +const CAPTURED_OBJECT_KEYS = /(?:Object captured as exception|Non-Error exception captured) with keys: (.+)/; + +const WRAPPER_KEYS = ['ngOriginalError', 'rejection', 'cause'] as const; +const HTTP_RESPONSE_KEYS = ['url', 'statusText', 'headers', 'ok'] as const; + +function isHttpResponseLike(value: object): value is { status: number } { + const hasNumericStatus = 'status' in value && typeof (value as { status: unknown }).status === 'number'; + + return hasNumericStatus && HTTP_RESPONSE_KEYS.some((key) => key in value); +} + +function describesHttpResponse(message: string | undefined): boolean { + const keys = message + ?.match(CAPTURED_OBJECT_KEYS)?.[1] + .split(',') + .map((key) => key.trim()); + + if (!keys?.includes('status')) { + return false; + } + + return HTTP_RESPONSE_KEYS.some((key) => keys.includes(key)); +} + +function getStatusFromMessage(message: string | undefined): number | null { + if (!message) { + return null; + } + + for (const pattern of STATUS_MESSAGE_PATTERNS) { + const match = message.match(pattern); + + if (match) { + return Number(match[1]); + } + } + + return null; +} + +function getStatusFromError(error: unknown, depth = 0): number | null { + if (error instanceof HttpErrorResponse) { + return error.status; + } + + if (typeof error === 'string') { + return getStatusFromMessage(error); + } + + if (!error || typeof error !== 'object') { + return null; + } + + if (isHttpResponseLike(error)) { + return error.status; + } + + if (depth >= MAX_UNWRAP_DEPTH) { + return null; + } + + for (const key of WRAPPER_KEYS) { + const status = getStatusFromError((error as Record)[key], depth + 1); + + if (status !== null) { + return status; + } + } + + return null; +} + +function getErrorMessage(error: unknown, event: ErrorEvent): string | undefined { + if (typeof error === 'string') { + return error; + } + + if (error && typeof error === 'object' && 'message' in error && typeof error.message === 'string') { + return error.message; + } + + const values = event.exception?.values; + + return values?.[values.length - 1]?.value; +} + +function getStatusFromSerialized(event: ErrorEvent, message: string | undefined): number | null { + const serialized = event.extra?.['__serialized__']; + + if (!serialized || typeof serialized !== 'object') { + return null; + } + + const status = 'status' in serialized ? serialized.status : null; + + if (typeof status === 'number' && (isHttpResponseLike(serialized) || describesHttpResponse(message))) { + return status; + } + + if ('message' in serialized && typeof serialized.message === 'string') { + return getStatusFromMessage(serialized.message); + } + + return null; +} + +function resolveHttpStatus(error: unknown, event: ErrorEvent): number | null { + const message = getErrorMessage(error, event); + + return getStatusFromError(error) ?? getStatusFromSerialized(event, message) ?? getStatusFromMessage(message); +} + +export function sentryBeforeSend(event: ErrorEvent, hint: EventHint): ErrorEvent | null { + const status = resolveHttpStatus(hint.originalException, event); + const isReportable = status === null || status >= MIN_REPORTED_STATUS; + + return isReportable ? event : null; +} diff --git a/src/app/core/provider/application.initialization.provider.ts b/src/app/core/provider/application.initialization.provider.ts index 10e11a5d9..ed08425c7 100644 --- a/src/app/core/provider/application.initialization.provider.ts +++ b/src/app/core/provider/application.initialization.provider.ts @@ -1,6 +1,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 { OSFConfigService } from '@core/services/osf-config.service'; import { ENVIRONMENT } from './environment.provider'; @@ -43,7 +44,9 @@ export function initializeApplication() { environment: environment.production ? 'production' : 'development', maxBreadcrumbs: 50, sampleRate: 1.0, - integrations: [], + ignoreErrors: SENTRY_IGNORE_ERRORS, + denyUrls: SENTRY_DENY_URLS, + beforeSend: sentryBeforeSend, }); } } diff --git a/src/app/features/home/home.component.html b/src/app/features/home/home.component.html index d54df5233..ee5536702 100644 --- a/src/app/features/home/home.component.html +++ b/src/app/features/home/home.component.html @@ -1,5 +1,7 @@
-
+ + +
diff --git a/src/app/features/home/home.component.spec.ts b/src/app/features/home/home.component.spec.ts index 0b0c3ee7b..ddd1a6ca5 100644 --- a/src/app/features/home/home.component.spec.ts +++ b/src/app/features/home/home.component.spec.ts @@ -3,6 +3,7 @@ import { MockComponents, MockProvider } from 'ng-mocks'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { ActivatedRoute, Router } from '@angular/router'; +import { ScheduledBannerComponent } from '@osf/core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { SearchInputComponent } from '@osf/shared/components/search-input/search-input.component'; @@ -23,7 +24,7 @@ describe('HomeComponent', () => { activatedRouteMock = ActivatedRouteMockBuilder.create().build(); TestBed.configureTestingModule({ - imports: [HomeComponent, ...MockComponents(SearchInputComponent, IconComponent)], + imports: [HomeComponent, ...MockComponents(SearchInputComponent, IconComponent, ScheduledBannerComponent)], providers: [provideOSFCore(), MockProvider(Router, routerMock), MockProvider(ActivatedRoute, activatedRouteMock)], }); diff --git a/src/app/features/home/home.component.ts b/src/app/features/home/home.component.ts index 28f3d25fd..e3bddd499 100644 --- a/src/app/features/home/home.component.ts +++ b/src/app/features/home/home.component.ts @@ -8,6 +8,7 @@ import { Component, inject } from '@angular/core'; import { FormControl } from '@angular/forms'; import { Router, RouterLink } from '@angular/router'; +import { ScheduledBannerComponent } from '@osf/core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { SearchInputComponent } from '@osf/shared/components/search-input/search-input.component'; @@ -19,6 +20,7 @@ import { INTEGRATION_ICONS, SLIDES } from './constants/data'; Carousel, Button, SearchInputComponent, + ScheduledBannerComponent, IconComponent, NgOptimizedImage, NgTemplateOutlet, diff --git a/src/app/features/institutions/pages/institutions-list/institutions-list.component.html b/src/app/features/institutions/pages/institutions-list/institutions-list.component.html index edc495ac4..0f3016e4d 100644 --- a/src/app/features/institutions/pages/institutions-list/institutions-list.component.html +++ b/src/app/features/institutions/pages/institutions-list/institutions-list.component.html @@ -6,8 +6,6 @@ [icon]="'custom-icon-institutions'" /> - -
diff --git a/src/app/features/institutions/pages/institutions-list/institutions-list.component.spec.ts b/src/app/features/institutions/pages/institutions-list/institutions-list.component.spec.ts index 6c9df80bb..57ad0a423 100644 --- a/src/app/features/institutions/pages/institutions-list/institutions-list.component.spec.ts +++ b/src/app/features/institutions/pages/institutions-list/institutions-list.component.spec.ts @@ -8,7 +8,6 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { FormControl } from '@angular/forms'; import { provideRouter } from '@angular/router'; -import { ScheduledBannerComponent } from '@core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; import { SearchInputComponent } from '@osf/shared/components/search-input/search-input.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; @@ -31,7 +30,7 @@ describe('InstitutionsListComponent', () => { TestBed.configureTestingModule({ imports: [ InstitutionsListComponent, - ...MockComponents(SubHeaderComponent, SearchInputComponent, LoadingSpinnerComponent, ScheduledBannerComponent), + ...MockComponents(SubHeaderComponent, SearchInputComponent, LoadingSpinnerComponent), ], providers: [ provideOSFCore(), diff --git a/src/app/features/institutions/pages/institutions-list/institutions-list.component.ts b/src/app/features/institutions/pages/institutions-list/institutions-list.component.ts index 135588f15..1cb874e33 100644 --- a/src/app/features/institutions/pages/institutions-list/institutions-list.component.ts +++ b/src/app/features/institutions/pages/institutions-list/institutions-list.component.ts @@ -10,7 +10,6 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { FormControl } from '@angular/forms'; import { RouterLink } from '@angular/router'; -import { ScheduledBannerComponent } from '@core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; import { SearchInputComponent } from '@osf/shared/components/search-input/search-input.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; @@ -25,7 +24,6 @@ import { FetchInstitutions, InstitutionsSelectors } from '@osf/shared/stores/ins SubHeaderComponent, SearchInputComponent, LoadingSpinnerComponent, - ScheduledBannerComponent, ], templateUrl: './institutions-list.component.html', changeDetection: ChangeDetectionStrategy.OnPush, diff --git a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.html b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.html index 4ebbd829b..918210498 100644 --- a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.html +++ b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.html @@ -143,18 +143,6 @@

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

[cedarTemplates]="cedarTemplates()" > -
-

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

- - -
- -
-

{{ 'shared.tags.title' | translate }}

- - -
- @if (!isAnonymous()) { { - const licenseId = this.currentProject()?.licenseId; - - if (licenseId) { - this.actions.getLicense(licenseId); - } + this.actions.getLicense(this.currentProject()?.licenseId); }); effect(() => { diff --git a/src/app/features/registries/pages/registries-landing/registries-landing.component.html b/src/app/features/registries/pages/registries-landing/registries-landing.component.html index 1b9b67583..5ae7387e5 100644 --- a/src/app/features/registries/pages/registries-landing/registries-landing.component.html +++ b/src/app/features/registries/pages/registries-landing/registries-landing.component.html @@ -1,21 +1,19 @@
-
- - - -
+ + + diff --git a/src/app/features/registries/pages/registries-landing/registries-landing.component.spec.ts b/src/app/features/registries/pages/registries-landing/registries-landing.component.spec.ts index 0bf2e4fd3..50b1adefe 100644 --- a/src/app/features/registries/pages/registries-landing/registries-landing.component.spec.ts +++ b/src/app/features/registries/pages/registries-landing/registries-landing.component.spec.ts @@ -8,7 +8,6 @@ import { PLATFORM_ID } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; -import { ScheduledBannerComponent } from '@core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { ClearCurrentProvider } from '@core/store/provider'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; import { ResourceCardComponent } from '@osf/shared/components/resource-card/resource-card.component'; @@ -42,8 +41,7 @@ describe('RegistriesLandingComponent', () => { RegistryServicesComponent, ResourceCardComponent, LoadingSpinnerComponent, - SubHeaderComponent, - ScheduledBannerComponent + SubHeaderComponent ), ], providers: [ diff --git a/src/app/features/registries/pages/registries-landing/registries-landing.component.ts b/src/app/features/registries/pages/registries-landing/registries-landing.component.ts index 917caf2fc..bc30381dc 100644 --- a/src/app/features/registries/pages/registries-landing/registries-landing.component.ts +++ b/src/app/features/registries/pages/registries-landing/registries-landing.component.ts @@ -9,7 +9,6 @@ import { ChangeDetectionStrategy, Component, inject, OnDestroy, OnInit, PLATFORM import { FormControl } from '@angular/forms'; import { Router } from '@angular/router'; -import { ScheduledBannerComponent } from '@core/components/osf-banners/scheduled-banner/scheduled-banner.component'; import { ENVIRONMENT } from '@core/provider/environment.provider'; import { ClearCurrentProvider } from '@core/store/provider'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; @@ -33,7 +32,6 @@ import { GetRegistries, RegistriesSelectors } from '../../store'; ResourceCardComponent, LoadingSpinnerComponent, SubHeaderComponent, - ScheduledBannerComponent, ], templateUrl: './registries-landing.component.html', styleUrl: './registries-landing.component.scss', diff --git a/src/app/features/registry/models/registry-overview.model.ts b/src/app/features/registry/models/registry-overview.model.ts index 2de035367..bab443fd7 100644 --- a/src/app/features/registry/models/registry-overview.model.ts +++ b/src/app/features/registry/models/registry-overview.model.ts @@ -4,7 +4,7 @@ import { RegistrationNodeModel } from '@shared/models/registration/registration- export interface RegistrationOverviewModel extends RegistrationNodeModel { associatedProjectId?: string; forksCount: number; - licenseId: string; + licenseId?: string; providerId: string; registrationSchemaLink: string; rootParentId?: string; diff --git a/src/app/features/registry/pages/registry-resources/registry-resources.component.html b/src/app/features/registry/pages/registry-resources/registry-resources.component.html index cf0747a89..89dd5037a 100644 --- a/src/app/features/registry/pages/registry-resources/registry-resources.component.html +++ b/src/app/features/registry/pages/registry-resources/registry-resources.component.html @@ -6,21 +6,21 @@ (buttonClick)="addResource()" /> -@if (isResourcesLoading()) { - -} @else { -
-

- @if (addButtonVisible()) { - {{ 'resources.linkDoi' | translate }} - } +

+

+ @if (addButtonVisible()) { + {{ 'resources.linkDoi' | translate }} + } - {{ 'resources.description' | translate }} - - {{ 'common.labels.learnMore' | translate }} - -

+ {{ 'resources.description' | translate }} + + {{ 'common.labels.learnMore' | translate }} + +

+ @if (isResourcesLoading()) { + + } @else {
@for (resource of resources(); track resource.id) {
@@ -60,5 +60,15 @@

{{ getResourceTypeTranslationKey(resource.type) | translate }}

}
-
-} + } + + @if (resourcesTotalCount() > rows()) { + + } +
diff --git a/src/app/features/registry/pages/registry-resources/registry-resources.component.spec.ts b/src/app/features/registry/pages/registry-resources/registry-resources.component.spec.ts index bcccca4b6..a251a0542 100644 --- a/src/app/features/registry/pages/registry-resources/registry-resources.component.spec.ts +++ b/src/app/features/registry/pages/registry-resources/registry-resources.component.spec.ts @@ -2,6 +2,9 @@ import { Store } from '@ngxs/store'; import { MockComponents, MockProvider } from 'ng-mocks'; +import { Button } from 'primeng/button'; +import { DynamicDialogRef } from 'primeng/dynamicdialog'; + import { Subject, throwError } from 'rxjs'; import { Mock } from 'vitest'; @@ -9,9 +12,11 @@ import { Mock } from 'vitest'; import { TestBed } from '@angular/core/testing'; import { ActivatedRoute } from '@angular/router'; +import { CustomPaginatorComponent } from '@osf/shared/components/custom-paginator/custom-paginator.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; +import { DEFAULT_TABLE_PARAMS } from '@osf/shared/constants/default-table-params.constants'; import { RegistryResourceType } from '@osf/shared/enums/registry-resource.enum'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; @@ -59,6 +64,7 @@ function setup(overrides: BaseSetupOverrides = {}) { const defaultSignals = [ { selector: RegistryResourcesSelectors.getResources, value: [] }, + { selector: RegistryResourcesSelectors.getResourcesTotalCount, value: 0 }, { selector: RegistryResourcesSelectors.isResourcesLoading, value: false }, { selector: RegistryResourcesSelectors.getCurrentResource, value: null }, { selector: RegistrySelectors.getRegistry, value: null }, @@ -71,7 +77,7 @@ function setup(overrides: BaseSetupOverrides = {}) { TestBed.configureTestingModule({ imports: [ RegistryResourcesComponent, - ...MockComponents(LoadingSpinnerComponent, SubHeaderComponent, IconComponent), + ...MockComponents(Button, LoadingSpinnerComponent, SubHeaderComponent, IconComponent, CustomPaginatorComponent), ], providers: [ provideOSFCore(), @@ -100,79 +106,71 @@ function setup(overrides: BaseSetupOverrides = {}) { } describe('RegistryResourcesComponent', () => { - it('should create with default values', () => { - const { component } = setup(); + it('should initialize defaults and load the first page', () => { + const { component, store, fixture } = setup(); - expect(component).toBeTruthy(); expect(component.isAddingResource()).toBe(false); - expect(component.doiDomain).toBe('https://doi.org/'); + expect(component.first()).toBe(0); + expect(component.rows()).toBe(DEFAULT_TABLE_PARAMS.rows); + expect(component.addButtonVisible()).toBe(true); + expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({ registryId: 'reg-1', page: 1 })); + expect(fixture.nativeElement.querySelector('osf-custom-paginator')).toBeFalsy(); }); - it('should dispatch getResources when registryId is available', () => { - const { store } = setup(); + it('should skip resource actions when registryId is missing', () => { + const { component, store, mockDialogService, mockConfirmationService } = setup({ hasParent: false }); - expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({ registryId: 'reg-1' })); - }); - - it('should not dispatch getResources when registryId is not available', () => { - const { store } = setup({ hasParent: false }); + (store.dispatch as Mock).mockClear(); + component.addResource(); + component.updateResource(MOCK_RESOURCE); + component.deleteResource('res-1'); + component.onPageChange({ page: 1, first: 10, rows: 10 }); expect(store.dispatch).not.toHaveBeenCalled(); + expect(mockDialogService.open).not.toHaveBeenCalled(); + expect(mockConfirmationService.confirmDelete).not.toHaveBeenCalled(); + expect(component.isAddingResource()).toBe(false); + expect(component.first()).toBe(10); }); - it('should compute addButtonVisible when identifiers exist and canEdit', () => { - const { component } = setup(); - - expect(component.addButtonVisible()).toBe(true); - }); - - it('should compute addButtonVisible as false when no identifiers', () => { - const { component } = setup({ + it('should hide add button when identifiers or write access are missing', () => { + const { component: withoutIdentifiers } = setup({ selectorOverrides: [{ selector: RegistrySelectors.getIdentifiers, value: [] }], }); - - expect(component.addButtonVisible()).toBe(false); - }); - - it('should compute addButtonVisible as false when canEdit is false', () => { - const { component } = setup({ + const { component: withoutWriteAccess } = setup({ selectorOverrides: [{ selector: RegistrySelectors.hasWriteAccess, value: false }], }); - expect(component.addButtonVisible()).toBe(false); + expect(withoutIdentifiers.addButtonVisible()).toBe(false); + expect(withoutWriteAccess.addButtonVisible()).toBe(false); }); - it('should add resource and show success toast on dialog confirm', () => { + it('should add a resource, reset pagination, and show a success toast', () => { const { component, dialogClose$, mockDialogService, mockToastService, store } = setup(); (store.dispatch as Mock).mockClear(); + component.first.set(20); component.addResource(); - - expect(component.isAddingResource()).toBe(true); - expect(store.dispatch).toHaveBeenCalled(); - expect(mockDialogService.open).toHaveBeenCalled(); - dialogClose$.next(true); dialogClose$.complete(); + expect(mockDialogService.open).toHaveBeenCalled(); expect(mockToastService.showSuccess).toHaveBeenCalledWith('resources.toastMessages.addResourceSuccess'); expect(component.isAddingResource()).toBe(false); + expect(component.first()).toBe(0); }); - it('should reset isAddingResource when dialog is dismissed', () => { + it('should reset isAddingResource when the add dialog is dismissed', () => { const { component, dialogClose$ } = setup(); component.addResource(); - - expect(component.isAddingResource()).toBe(true); - dialogClose$.next(null); dialogClose$.complete(); expect(component.isAddingResource()).toBe(false); }); - it('should show error toast when addResource dispatch errors', () => { + it('should show an error toast when addResource fails', () => { const { component, store, mockToastService } = setup(); vi.spyOn(store, 'dispatch').mockReturnValue(throwError(() => new Error('fail'))); @@ -181,21 +179,12 @@ describe('RegistryResourcesComponent', () => { expect(mockToastService.showError).toHaveBeenCalledWith('resources.toastMessages.addResourceError'); }); - it('should not add resource when registryId is not available', () => { - const { component, store, mockDialogService } = setup({ hasParent: false }); - - (store.dispatch as Mock).mockClear(); - component.addResource(); - - expect(component.isAddingResource()).toBe(false); - expect(store.dispatch).not.toHaveBeenCalled(); - expect(mockDialogService.open).not.toHaveBeenCalled(); - }); - - it('should open edit dialog on updateResource', () => { - const { component, mockDialogService } = setup(); + it('should update a resource and show a success toast', () => { + const { component, dialogClose$, mockDialogService, mockToastService } = setup(); component.updateResource(MOCK_RESOURCE); + dialogClose$.next(true); + dialogClose$.complete(); expect(mockDialogService.open).toHaveBeenCalledWith( expect.any(Function), @@ -204,40 +193,29 @@ describe('RegistryResourcesComponent', () => { data: { id: 'reg-1', resource: MOCK_RESOURCE }, }) ); - }); - - it('should show success toast on updateResource dialog confirm', () => { - const { component, dialogClose$, mockToastService } = setup(); - - component.updateResource(MOCK_RESOURCE); - dialogClose$.next(true); - dialogClose$.complete(); - expect(mockToastService.showSuccess).toHaveBeenCalledWith('resources.toastMessages.updatedResourceSuccess'); }); - it('should show error toast when updateResource dialog errors', () => { + it('should show an error toast when updateResource fails', () => { const errorSubject = new Subject(); const { component, mockDialogService, mockToastService } = setup(); - mockDialogService.open.mockReturnValue({ onClose: errorSubject.pipe() } as any); + mockDialogService.open.mockReturnValue({ + onClose: errorSubject.pipe(), + close: vi.fn(), + } as unknown as DynamicDialogRef); component.updateResource(MOCK_RESOURCE); errorSubject.error(new Error('fail')); expect(mockToastService.showError).toHaveBeenCalledWith('resources.toastMessages.updateResourceError'); }); - it('should not update resource when registryId is not available', () => { - const { component, mockDialogService } = setup({ hasParent: false }); - - component.updateResource(MOCK_RESOURCE); - - expect(mockDialogService.open).not.toHaveBeenCalled(); - }); - - it('should delete resource with confirmation', () => { - const { component, mockConfirmationService } = setup(); + it('should delete a resource, reset pagination, and show a success toast', () => { + const { component, mockConfirmationService, mockToastService, store } = setup(); + mockConfirmationService.confirmDelete.mockImplementation(({ onConfirm }: { onConfirm: () => void }) => onConfirm()); + (store.dispatch as Mock).mockClear(); + component.first.set(20); component.deleteResource('res-1'); expect(mockConfirmationService.confirmDelete).toHaveBeenCalledWith( @@ -245,43 +223,49 @@ describe('RegistryResourcesComponent', () => { headerKey: 'resources.delete', messageKey: 'resources.deleteText', acceptLabelKey: 'common.buttons.remove', - onConfirm: expect.any(Function), }) ); - }); - - it('should dispatch delete and show toast on confirm', () => { - const { component, mockConfirmationService, mockToastService, store } = setup(); - - mockConfirmationService.confirmDelete.mockImplementation(({ onConfirm }: { onConfirm: () => void }) => onConfirm()); - - (store.dispatch as Mock).mockClear(); - component.deleteResource('res-1'); - expect(store.dispatch).toHaveBeenCalled(); expect(mockToastService.showSuccess).toHaveBeenCalledWith('resources.toastMessages.deletedResourceSuccess'); + expect(component.first()).toBe(0); }); - it('should not delete resource when registryId is not available', () => { - const { component, mockConfirmationService } = setup({ hasParent: false }); - - component.deleteResource('res-1'); - - expect(mockConfirmationService.confirmDelete).not.toHaveBeenCalled(); - }); - - it('should return translation key for known resource type', () => { + it('should resolve resource type labels', () => { const { component } = setup(); expect(component.getResourceTypeTranslationKey(RegistryResourceType.Data)).toBe('resourceCard.resources.data'); expect(component.getResourceTypeTranslationKey(RegistryResourceType.Code)).toBe( 'resourceCard.resources.analyticCode' ); + expect(component.getResourceTypeTranslationKey('unknown')).toBe(''); }); - it('should return empty string for unknown resource type', () => { - const { component } = setup(); + it('should load the selected page and keep current rows when rows are omitted', () => { + const { component, store } = setup(); - expect(component.getResourceTypeTranslationKey('unknown')).toBe(''); + (store.dispatch as Mock).mockClear(); + component.rows.set(25); + component.onPageChange({ page: 1, first: 25, rows: undefined }); + + expect(component.first()).toBe(25); + expect(component.rows()).toBe(25); + expect(store.dispatch).toHaveBeenCalledWith(expect.objectContaining({ registryId: 'reg-1', page: 2 })); + }); + + it('should not load a page when the paginator page is undefined', () => { + const { component, store } = setup(); + + (store.dispatch as Mock).mockClear(); + component.onPageChange({ page: undefined, first: 0, rows: 10 }); + + expect(store.dispatch).not.toHaveBeenCalled(); + }); + + it('should render the paginator when total count exceeds page size', () => { + const { fixture } = setup({ + selectorOverrides: [{ selector: RegistryResourcesSelectors.getResourcesTotalCount, value: 25 }], + }); + + expect(fixture.nativeElement.querySelector('osf-custom-paginator')).toBeTruthy(); }); }); diff --git a/src/app/features/registry/pages/registry-resources/registry-resources.component.ts b/src/app/features/registry/pages/registry-resources/registry-resources.component.ts index c8cc676f5..00f559dbd 100644 --- a/src/app/features/registry/pages/registry-resources/registry-resources.component.ts +++ b/src/app/features/registry/pages/registry-resources/registry-resources.component.ts @@ -3,6 +3,7 @@ import { createDispatchMap, select } from '@ngxs/store'; import { TranslatePipe } from '@ngx-translate/core'; import { Button } from 'primeng/button'; +import { PaginatorState } from 'primeng/paginator'; import { filter, finalize, map, of, switchMap } from 'rxjs'; @@ -19,9 +20,11 @@ import { import { takeUntilDestroyed, toSignal } from '@angular/core/rxjs-interop'; import { ActivatedRoute } from '@angular/router'; +import { CustomPaginatorComponent } from '@osf/shared/components/custom-paginator/custom-paginator.component'; import { IconComponent } from '@osf/shared/components/icon/icon.component'; import { LoadingSpinnerComponent } from '@osf/shared/components/loading-spinner/loading-spinner.component'; import { SubHeaderComponent } from '@osf/shared/components/sub-header/sub-header.component'; +import { DEFAULT_TABLE_PARAMS } from '@osf/shared/constants/default-table-params.constants'; import { RegistryResourceType } from '@osf/shared/enums/registry-resource.enum'; import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service'; import { CustomDialogService } from '@osf/shared/services/custom-dialog.service'; @@ -41,13 +44,21 @@ import { @Component({ selector: 'osf-registry-resources', - imports: [Button, SubHeaderComponent, LoadingSpinnerComponent, IconComponent, TranslatePipe], + imports: [ + Button, + SubHeaderComponent, + LoadingSpinnerComponent, + IconComponent, + TranslatePipe, + CustomPaginatorComponent, + ], templateUrl: './registry-resources.component.html', styleUrl: './registry-resources.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) export class RegistryResourcesComponent { @HostBinding('class') classes = 'flex-1 flex flex-column w-full h-full'; + private readonly route = inject(ActivatedRoute); private readonly customDialogService = inject(CustomDialogService); private readonly toastService = inject(ToastService); @@ -55,10 +66,12 @@ export class RegistryResourcesComponent { private readonly destroyRef = inject(DestroyRef); readonly resources = select(RegistryResourcesSelectors.getResources); + readonly resourcesTotalCount = select(RegistryResourcesSelectors.getResourcesTotalCount); readonly isResourcesLoading = select(RegistryResourcesSelectors.isResourcesLoading); readonly currentResource = select(RegistryResourcesSelectors.getCurrentResource); readonly registry = select(RegistrySelectors.getRegistry); readonly identifiers = select(RegistrySelectors.getIdentifiers); + readonly canEdit = select(RegistrySelectors.hasWriteAccess); private readonly registryId = toSignal( this.route.parent?.params.pipe(map((params) => params['id'])) ?? of(undefined) @@ -66,6 +79,8 @@ export class RegistryResourcesComponent { isAddingResource = signal(false); doiDomain = 'https://doi.org/'; + first = signal(0); + rows = signal(DEFAULT_TABLE_PARAMS.rows); private readonly actions = createDispatchMap({ getResources: GetRegistryResources, @@ -75,8 +90,6 @@ export class RegistryResourcesComponent { readonly RegistryResourceType = RegistryResourceType; - canEdit = select(RegistrySelectors.hasWriteAccess); - addButtonVisible = computed(() => !!this.identifiers().length && this.canEdit()); getResourceTypeTranslationKey(type: string): string { @@ -88,6 +101,7 @@ export class RegistryResourcesComponent { const registryId = this.registryId(); if (registryId) { + this.resetPagination(); this.actions.getResources(registryId); } }); @@ -108,7 +122,10 @@ export class RegistryResourcesComponent { takeUntilDestroyed(this.destroyRef) ) .subscribe({ - next: () => this.toastService.showSuccess('resources.toastMessages.addResourceSuccess'), + next: () => { + this.resetPagination(); + this.toastService.showSuccess('resources.toastMessages.addResourceSuccess'); + }, error: () => this.toastService.showError('resources.toastMessages.addResourceError'), }); } @@ -145,11 +162,32 @@ export class RegistryResourcesComponent { this.actions .deleteResource(id, registryId) .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(() => this.toastService.showSuccess('resources.toastMessages.deletedResourceSuccess')); + .subscribe(() => { + this.resetPagination(); + this.toastService.showSuccess('resources.toastMessages.deletedResourceSuccess'); + }); }, }); } + onPageChange(event: PaginatorState) { + this.first.set(event.first ?? 0); + this.rows.set(event.rows ?? this.rows()); + + if (event.page === undefined) { + return; + } + + const registryId = this.registryId(); + if (!registryId) return; + + this.actions.getResources(registryId, event.page + 1); + } + + private resetPagination() { + this.first.set(0); + } + private openAddResourceDialog(registryId: string) { return this.customDialogService.open(AddResourceDialogComponent, { header: 'resources.add', diff --git a/src/app/features/registry/services/registry-resources.service.spec.ts b/src/app/features/registry/services/registry-resources.service.spec.ts new file mode 100644 index 000000000..d6118670f --- /dev/null +++ b/src/app/features/registry/services/registry-resources.service.spec.ts @@ -0,0 +1,115 @@ +import { HttpTestingController } from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; + +import { DEFAULT_TABLE_PARAMS } from '@osf/shared/constants/default-table-params.constants'; +import { RegistryResourceType } from '@osf/shared/enums/registry-resource.enum'; +import { PaginatedData } from '@osf/shared/models/paginated-data.model'; + +import { provideOSFCore, provideOSFHttp } from '@testing/osf.testing.provider'; +import { EnvironmentTokenMock } from '@testing/providers/environment.token.mock'; + +import { GetRegistryResourcesJsonApi, RegistryResource, RegistryResourceDataJsonApi } from '../models'; + +import { RegistryResourcesService } from './registry-resources.service'; + +const apiResource: RegistryResourceDataJsonApi = { + id: 'res-1', + type: 'resources', + attributes: { + description: 'Dataset description', + finalized: true, + pid: '10.123/test', + resource_type: RegistryResourceType.Data, + }, +}; + +const mappedResource: RegistryResource = { + id: 'res-1', + description: 'Dataset description', + finalized: true, + type: RegistryResourceType.Data, + pid: '10.123/test', +}; + +describe('RegistryResourcesService', () => { + let service: RegistryResourcesService; + let httpMock: HttpTestingController; + const apiBase = `${EnvironmentTokenMock.useValue.apiDomainUrl}/v2`; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideOSFCore(), provideOSFHttp(), RegistryResourcesService], + }); + service = TestBed.inject(RegistryResourcesService); + httpMock = TestBed.inject(HttpTestingController); + }); + + it('should get resources with default pagination params and map the response', () => { + const response: GetRegistryResourcesJsonApi = { + data: [apiResource], + meta: { total: 12, per_page: 10 }, + }; + let result: PaginatedData | undefined; + + service.getResources('reg-1').subscribe((value) => (result = value)); + + const req = httpMock.expectOne( + (request) => + request.url === `${apiBase}/registrations/reg-1/resources/` && + request.params.get('fields[resources]') === 'description,finalized,resource_type,pid' && + request.params.get('page') === '1' && + request.params.get('page[size]') === String(DEFAULT_TABLE_PARAMS.rows) + ); + expect(req.request.method).toBe('GET'); + req.flush(response); + + expect(result).toEqual({ + data: [mappedResource], + totalCount: 12, + pageSize: 10, + }); + httpMock.verify(); + }); + + it('should get resources with custom page and page size', () => { + const response: GetRegistryResourcesJsonApi = { + data: [apiResource], + meta: { total: 25, per_page: 5 }, + }; + let result: PaginatedData | undefined; + + service.getResources('reg-1', 3, 5).subscribe((value) => (result = value)); + + const req = httpMock.expectOne( + (request) => + request.url === `${apiBase}/registrations/reg-1/resources/` && + request.params.get('page') === '3' && + request.params.get('page[size]') === '5' + ); + expect(req.request.method).toBe('GET'); + req.flush(response); + + expect(result).toEqual({ + data: [mappedResource], + totalCount: 25, + pageSize: 5, + }); + httpMock.verify(); + }); + + it('should fall back to default page size when per_page is missing', () => { + const response: GetRegistryResourcesJsonApi = { + data: [apiResource], + meta: { total: 1 }, + }; + let result: PaginatedData | undefined; + + service.getResources('reg-1').subscribe((value) => (result = value)); + + const req = httpMock.expectOne((request) => request.url === `${apiBase}/registrations/reg-1/resources/`); + req.flush(response); + + expect(result?.pageSize).toBe(DEFAULT_TABLE_PARAMS.rows); + httpMock.verify(); + }); +}); diff --git a/src/app/features/registry/services/registry-resources.service.ts b/src/app/features/registry/services/registry-resources.service.ts index 39366b50b..61d2d3305 100644 --- a/src/app/features/registry/services/registry-resources.service.ts +++ b/src/app/features/registry/services/registry-resources.service.ts @@ -3,6 +3,8 @@ import { map, Observable } from 'rxjs'; import { inject, Injectable } from '@angular/core'; import { ENVIRONMENT } from '@core/provider/environment.provider'; +import { DEFAULT_TABLE_PARAMS } from '@osf/shared/constants/default-table-params.constants'; +import { PaginatedData } from '@osf/shared/models/paginated-data.model'; import { JsonApiService } from '@osf/shared/services/json-api.service'; import { MapAddResourceRequest, MapRegistryResource, toAddResourceRequestBody } from '../mappers'; @@ -26,14 +28,26 @@ export class RegistryResourcesService { return `${this.environment.apiDomainUrl}/v2`; } - getResources(registryId: string): Observable { + getResources( + registryId: string, + page = 1, + pageSize = DEFAULT_TABLE_PARAMS.rows + ): Observable> { const params = { 'fields[resources]': 'description,finalized,resource_type,pid', + page, + 'page[size]': pageSize, }; return this.jsonApiService - .get(`${this.apiUrl}/registrations/${registryId}/resources/?page=1`, params) - .pipe(map((response) => response.data.map((resource) => MapRegistryResource(resource)))); + .get(`${this.apiUrl}/registrations/${registryId}/resources/`, params) + .pipe( + map((response) => ({ + data: response.data.map((resource) => MapRegistryResource(resource)), + totalCount: response.meta.total, + pageSize: response.meta.per_page ?? DEFAULT_TABLE_PARAMS.rows, + })) + ); } addRegistryResource(registryId: string): Observable { diff --git a/src/app/features/registry/store/registry-resources/registry-resources.actions.ts b/src/app/features/registry/store/registry-resources/registry-resources.actions.ts index 11109ed33..9960a6ebc 100644 --- a/src/app/features/registry/store/registry-resources/registry-resources.actions.ts +++ b/src/app/features/registry/store/registry-resources/registry-resources.actions.ts @@ -3,7 +3,10 @@ import { AddResource, ConfirmAddResource } from '../../models'; export class GetRegistryResources { static readonly type = '[Registry Resources] Get Registry Resources'; - constructor(public registryId: string) {} + constructor( + public registryId: string, + public page = 1 + ) {} } export class AddRegistryResource { diff --git a/src/app/features/registry/store/registry-resources/registry-resources.model.ts b/src/app/features/registry/store/registry-resources/registry-resources.model.ts index f51349805..349eae373 100644 --- a/src/app/features/registry/store/registry-resources/registry-resources.model.ts +++ b/src/app/features/registry/store/registry-resources/registry-resources.model.ts @@ -1,10 +1,12 @@ import { AsyncStateModel } from '@osf/shared/models/store/async-state.model'; +import { AsyncStateWithTotalCount } from '@osf/shared/models/store/async-state-with-total-count.model'; import { RegistryResource } from '../../models'; export interface RegistryResourcesStateModel { - resources: AsyncStateModel; + resources: AsyncStateWithTotalCount; currentResource: AsyncStateModel; + currentPage: number; } export const REGISTRY_RESOURCES_STATE_DEFAULTS = { @@ -12,10 +14,12 @@ export const REGISTRY_RESOURCES_STATE_DEFAULTS = { data: null, isLoading: false, error: null, + totalCount: 0, }, currentResource: { data: null, isLoading: false, error: null, }, + currentPage: 1, }; diff --git a/src/app/features/registry/store/registry-resources/registry-resources.selectors.ts b/src/app/features/registry/store/registry-resources/registry-resources.selectors.ts index 447c95c19..9ed29d6ed 100644 --- a/src/app/features/registry/store/registry-resources/registry-resources.selectors.ts +++ b/src/app/features/registry/store/registry-resources/registry-resources.selectors.ts @@ -11,6 +11,11 @@ export class RegistryResourcesSelectors { return state.resources.data; } + @Selector([RegistryResourcesState]) + static getResourcesTotalCount(state: RegistryResourcesStateModel): number { + return state.resources.totalCount; + } + @Selector([RegistryResourcesState]) static isResourcesLoading(state: RegistryResourcesStateModel): boolean { return state.resources.isLoading; diff --git a/src/app/features/registry/store/registry-resources/registry-resources.state.spec.ts b/src/app/features/registry/store/registry-resources/registry-resources.state.spec.ts new file mode 100644 index 000000000..3a87f9289 --- /dev/null +++ b/src/app/features/registry/store/registry-resources/registry-resources.state.spec.ts @@ -0,0 +1,132 @@ +import { provideStore, Store } from '@ngxs/store'; + +import { MockProvider } from 'ng-mocks'; + +import { firstValueFrom, of, Subject, throwError } from 'rxjs'; + +import { TestBed } from '@angular/core/testing'; + +import { RegistryResourceType } from '@osf/shared/enums/registry-resource.enum'; +import { PaginatedData } from '@osf/shared/models/paginated-data.model'; + +import { RegistryResource } from '../../models'; +import { RegistryResourcesService } from '../../services'; + +import { + ConfirmAddRegistryResource, + DeleteResource, + GetRegistryResources, + UpdateResource, +} from './registry-resources.actions'; +import { RegistryResourcesSelectors } from './registry-resources.selectors'; +import { RegistryResourcesState } from './registry-resources.state'; + +const MOCK_RESOURCE: RegistryResource = { + id: 'res-1', + description: 'Test resource', + finalized: true, + type: RegistryResourceType.Data, + pid: '10.123/test', +}; + +const MOCK_PAGINATED_RESOURCES: PaginatedData = { + data: [MOCK_RESOURCE], + totalCount: 21, + pageSize: 10, +}; + +describe('RegistryResourcesState', () => { + let store: Store; + let getResourcesMock: ReturnType>; + let deleteResourceMock: ReturnType>; + let confirmAddingResourceMock: ReturnType>; + let updateResourceMock: ReturnType>; + + beforeEach(() => { + getResourcesMock = vi.fn().mockReturnValue(of(MOCK_PAGINATED_RESOURCES)); + deleteResourceMock = vi.fn().mockReturnValue(of(undefined)); + confirmAddingResourceMock = vi + .fn() + .mockReturnValue(of(MOCK_RESOURCE)); + updateResourceMock = vi.fn().mockReturnValue(of(undefined)); + + const mockService: Pick< + RegistryResourcesService, + 'getResources' | 'deleteResource' | 'confirmAddingResource' | 'updateResource' + > = { + getResources: getResourcesMock, + deleteResource: deleteResourceMock, + confirmAddingResource: confirmAddingResourceMock, + updateResource: updateResourceMock, + }; + + TestBed.configureTestingModule({ + providers: [provideStore([RegistryResourcesState]), MockProvider(RegistryResourcesService, mockService)], + }); + + store = TestBed.inject(Store); + }); + + it('should fetch resources for a page and update total count', async () => { + const subject = new Subject>(); + getResourcesMock.mockReturnValue(subject.asObservable()); + + const dispatchPromise = firstValueFrom(store.dispatch(new GetRegistryResources('reg-1', 2))); + + expect(store.selectSnapshot(RegistryResourcesSelectors.isResourcesLoading)).toBe(true); + expect(getResourcesMock).toHaveBeenCalledWith('reg-1', 2); + + subject.next(MOCK_PAGINATED_RESOURCES); + subject.complete(); + await dispatchPromise; + + expect(store.selectSnapshot(RegistryResourcesSelectors.getResources)).toEqual([MOCK_RESOURCE]); + expect(store.selectSnapshot(RegistryResourcesSelectors.getResourcesTotalCount)).toBe(21); + expect(store.selectSnapshot(RegistryResourcesSelectors.isResourcesLoading)).toBe(false); + expect(store.snapshot().registryResources.currentPage).toBe(2); + }); + + it('should handle get resources error', async () => { + getResourcesMock.mockReturnValue(throwError(() => new Error('Failed to fetch resources'))); + + await expect(firstValueFrom(store.dispatch(new GetRegistryResources('reg-1', 1)))).rejects.toThrow( + 'Failed to fetch resources' + ); + + const snapshot = store.snapshot().registryResources.resources; + expect(snapshot.data).toBeNull(); + expect(snapshot.error).toBe('Failed to fetch resources'); + expect(snapshot.isLoading).toBe(false); + }); + + it('should refetch the first page after delete', async () => { + await firstValueFrom(store.dispatch(new DeleteResource('res-1', 'reg-1'))); + + expect(deleteResourceMock).toHaveBeenCalledWith('res-1'); + expect(getResourcesMock).toHaveBeenCalledWith('reg-1', 1); + }); + + it('should refetch the first page after confirm add', async () => { + await firstValueFrom(store.dispatch(new ConfirmAddRegistryResource({ finalized: true }, 'res-1', 'reg-1'))); + + expect(confirmAddingResourceMock).toHaveBeenCalledWith('res-1', { finalized: true }); + expect(getResourcesMock).toHaveBeenCalledWith('reg-1', 1); + }); + + it('should refetch the current page after update', async () => { + await firstValueFrom(store.dispatch(new GetRegistryResources('reg-1', 3))); + getResourcesMock.mockClear(); + + await firstValueFrom( + store.dispatch( + new UpdateResource('reg-1', 'res-1', { + pid: '10.123/updated', + resource_type: RegistryResourceType.Data, + }) + ) + ); + + expect(updateResourceMock).toHaveBeenCalled(); + expect(getResourcesMock).toHaveBeenCalledWith('reg-1', 3); + }); +}); diff --git a/src/app/features/registry/store/registry-resources/registry-resources.state.ts b/src/app/features/registry/store/registry-resources/registry-resources.state.ts index c54223f86..98fffdef7 100644 --- a/src/app/features/registry/store/registry-resources/registry-resources.state.ts +++ b/src/app/features/registry/store/registry-resources/registry-resources.state.ts @@ -38,14 +38,16 @@ export class RegistryResourcesState { }, }); - return this.registryResourcesService.getResources(action.registryId).pipe( + return this.registryResourcesService.getResources(action.registryId, action.page).pipe( tap((resources) => { ctx.patchState({ resources: { - data: resources, + data: resources.data, isLoading: false, error: null, + totalCount: resources.totalCount, }, + currentPage: action.page, }); }), catchError((err) => handleSectionError(ctx, 'resources', err)) @@ -105,7 +107,7 @@ export class RegistryResourcesState { confirmAddRegistryResource(ctx: StateContext, action: ConfirmAddRegistryResource) { return this.registryResourcesService.confirmAddingResource(action.resourceId, action.resource).pipe( tap(() => { - ctx.dispatch(new GetRegistryResources(action.registryId)); + ctx.dispatch(new GetRegistryResources(action.registryId, 1)); }), catchError((err) => handleSectionError(ctx, 'resources', err)) ); @@ -123,7 +125,7 @@ export class RegistryResourcesState { return this.registryResourcesService.deleteResource(action.resourceId).pipe( tap(() => { - ctx.dispatch(new GetRegistryResources(action.registryId)); + ctx.dispatch(new GetRegistryResources(action.registryId, 1)); }), catchError((err) => handleSectionError(ctx, 'resources', err)) ); @@ -152,7 +154,7 @@ export class RegistryResourcesState { isLoading: false, }, }); - ctx.dispatch(new GetRegistryResources(action.registryId)); + ctx.dispatch(new GetRegistryResources(action.registryId, ctx.getState().currentPage)); }), catchError((err) => handleSectionError(ctx, 'resources', err)) ); diff --git a/src/app/features/registry/store/registry/registry.actions.ts b/src/app/features/registry/store/registry/registry.actions.ts index 8a32ffe8c..70b702ffb 100644 --- a/src/app/features/registry/store/registry/registry.actions.ts +++ b/src/app/features/registry/store/registry/registry.actions.ts @@ -27,7 +27,7 @@ export class GetRegistryIdentifiers { export class GetRegistryLicense { static readonly type = '[Registry] Get Registry License'; - constructor(public licenseId: string) {} + constructor(public licenseId: string | undefined) {} } export class GetSchemaBlocks { diff --git a/src/app/features/registry/store/registry/registry.state.ts b/src/app/features/registry/store/registry/registry.state.ts index bceed584d..778fcb967 100644 --- a/src/app/features/registry/store/registry/registry.state.ts +++ b/src/app/features/registry/store/registry/registry.state.ts @@ -91,9 +91,8 @@ export class RegistryState { if (registryOverview.providerId) { ctx.dispatch(new GetRegistryProvider(registryOverview.providerId)); } - if (registryOverview.licenseId) { - ctx.dispatch(new GetRegistryLicense(registryOverview.licenseId)); - } + + ctx.dispatch(new GetRegistryLicense(registryOverview.licenseId)); }), catchError((error) => handleSectionError(ctx, 'registry', error)) ); @@ -149,6 +148,10 @@ export class RegistryState { @Action(GetRegistryLicense) getRegistryLicense(ctx: StateContext, action: GetRegistryLicense) { + if (!action.licenseId) { + return; + } + const state = ctx.getState(); ctx.patchState({ license: { diff --git a/src/app/features/settings/profile-settings/components/authenticated-identity/authenticated-identity.component.html b/src/app/features/settings/profile-settings/components/authenticated-identity/authenticated-identity.component.html index ecd67c295..85cd7979d 100644 --- a/src/app/features/settings/profile-settings/components/authenticated-identity/authenticated-identity.component.html +++ b/src/app/features/settings/profile-settings/components/authenticated-identity/authenticated-identity.component.html @@ -1,7 +1,7 @@

- {{ 'settings.profileSettings.social.labels.authenticatedIdentity' | translate }} + {{ 'settings.profileSettings.social.authenticatedIdentity' | translate }}

@@ -23,7 +23,7 @@

} @else { orcid -

+

{{ 'settings.profileSettings.social.orcidWarning' | translate }}

{{ 'common.hint.viewOnlyLinksBanner' | translate }}

-
diff --git a/src/app/shared/components/view-only-link-message/view-only-link-message.component.spec.ts b/src/app/shared/components/view-only-link-message/view-only-link-message.component.spec.ts index 438914f53..eb2383f6a 100644 --- a/src/app/shared/components/view-only-link-message/view-only-link-message.component.spec.ts +++ b/src/app/shared/components/view-only-link-message/view-only-link-message.component.spec.ts @@ -1,54 +1,26 @@ -import { MockProvider } from 'ng-mocks'; - -import { PLATFORM_ID } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { Router } from '@angular/router'; import { provideOSFCore } from '@testing/osf.testing.provider'; -import { RouterMockBuilder, RouterMockType } from '@testing/providers/router-provider.mock'; import { ViewOnlyLinkMessageComponent } from './view-only-link-message.component'; describe('ViewOnlyLinkMessageComponent', () => { let fixture: ComponentFixture; - let component: ViewOnlyLinkMessageComponent; - let routerMock: RouterMockType; - - function setup(platformId: 'browser' | 'server' = 'browser') { - routerMock = RouterMockBuilder.create().build(); + beforeEach(() => { TestBed.configureTestingModule({ imports: [ViewOnlyLinkMessageComponent], - providers: [provideOSFCore(), MockProvider(Router, routerMock), MockProvider(PLATFORM_ID, platformId)], + providers: [provideOSFCore()], }); fixture = TestBed.createComponent(ViewOnlyLinkMessageComponent); - component = fixture.componentInstance; fixture.detectChanges(); - } - - it('should create', () => { - setup(); - - expect(component).toBeTruthy(); - }); - - it('should navigate with merged query params in browser', () => { - setup(); - - component.handleLeaveViewOnlyView(); - - expect(routerMock.navigate).toHaveBeenCalledWith([], { - queryParams: { view_only: null }, - queryParamsHandling: 'merge', - }); }); - it('should not navigate on server platform', () => { - setup('server'); - - component.handleLeaveViewOnlyView(); + it('should render the view-only links banner', () => { + const message = fixture.nativeElement.querySelector('p-message[severity="info"]'); - expect(routerMock.navigate).not.toHaveBeenCalled(); + expect(message).toBeTruthy(); + expect(fixture.nativeElement.textContent).toContain('common.hint.viewOnlyLinksBanner'); }); }); diff --git a/src/app/shared/components/view-only-link-message/view-only-link-message.component.ts b/src/app/shared/components/view-only-link-message/view-only-link-message.component.ts index 25b52ea7d..d7a68dfaa 100644 --- a/src/app/shared/components/view-only-link-message/view-only-link-message.component.ts +++ b/src/app/shared/components/view-only-link-message/view-only-link-message.component.ts @@ -1,33 +1,14 @@ import { TranslatePipe } from '@ngx-translate/core'; -import { Button } from 'primeng/button'; import { Message } from 'primeng/message'; -import { isPlatformBrowser } from '@angular/common'; -import { ChangeDetectionStrategy, Component, inject, PLATFORM_ID } from '@angular/core'; -import { Router } from '@angular/router'; +import { ChangeDetectionStrategy, Component } from '@angular/core'; @Component({ selector: 'osf-view-only-link-message', - imports: [Message, TranslatePipe, Button], + imports: [Message, TranslatePipe], templateUrl: './view-only-link-message.component.html', styleUrl: './view-only-link-message.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) -export class ViewOnlyLinkMessageComponent { - private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); - private readonly router = inject(Router); - - handleLeaveViewOnlyView(): void { - if (!this.isBrowser) { - return; - } - - this.router - .navigate([], { - queryParams: { view_only: null }, - queryParamsHandling: 'merge', - }) - .then(() => window.location.reload()); - } -} +export class ViewOnlyLinkMessageComponent {} diff --git a/src/app/shared/constants/social-share.config.ts b/src/app/shared/constants/social-share.config.ts index 7a0e2bfaf..a44557fd1 100644 --- a/src/app/shared/constants/social-share.config.ts +++ b/src/app/shared/constants/social-share.config.ts @@ -3,7 +3,7 @@ export const SOCIAL_SHARE_URLS = { x: { preview_url: 'https://x.com/intent/tweet', viaHandle: 'OsfFramework' }, facebook: 'https://www.facebook.com/sharer/sharer.php', facebookShare: 'https://www.facebook.com/dialog/share', - linkedIn: 'https://www.linkedin.com/sharing/share-offsite', + linkedIn: 'https://www.linkedin.com/feed/', mastodon: 'https://mastodonshare.com', bluesky: 'https://bsky.app/intent/compose', }; diff --git a/src/app/shared/services/social-share.service.ts b/src/app/shared/services/social-share.service.ts index faa5db904..72f357199 100644 --- a/src/app/shared/services/social-share.service.ts +++ b/src/app/shared/services/social-share.service.ts @@ -92,9 +92,9 @@ export class SocialShareService { } private generateLinkedInLink(content: SocialShareContentModel): string { - const url = encodeURIComponent(content.url); + const text = encodeURIComponent(content.url); - return `${SOCIAL_SHARE_URLS.linkedIn}?url=${url}`; + return `${SOCIAL_SHARE_URLS.linkedIn}?shareActive=true&text=${text}`; } private generateMastodonLink(content: SocialShareContentModel): string { diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json index d2532757d..c041a41cc 100644 --- a/src/assets/i18n/en.json +++ b/src/assets/i18n/en.json @@ -2821,8 +2821,13 @@ "suffix": "Suffix (Optional)" }, "social": { + "authenticatedIdentity": "Authenticated identity", "successUpdate": "Social successfully updated.", - "title": "Social Link {{index}}" + "title": "Social Link {{index}}", + "connectOrcid": "Connect ORCID", + "disconnectOrcid": "Disconnect ORCID", + "orcidDescription": "Link your ORCID. ORCID is a free, unique, persistent identifier (PID) for individuals to use as they engage in research, scholarship, and innovation activities. Learn how ORCID can help you spend more time conducting your research and less time managing it. Learn more about ORCID.", + "orcidWarning": "NOTE: This will log you out of your current OSF profile. Your OSF account email and ORCID email need to match to correctly link your ORCID to your OSF profile." }, "tabs": { "education": "Education",