From f4e602d59cc64f7f4a669c2bb9da1f553da8c864 Mon Sep 17 00:00:00 2001 From: nsemets Date: Mon, 3 Nov 2025 19:51:20 +0200 Subject: [PATCH 1/2] fix(unsubscribe): added missing destroy ref --- .../features/project/wiki/wiki.component.ts | 4 ++-- .../custom-step/custom-step.component.ts | 4 +++- .../components/drafts/drafts.component.ts | 4 +++- .../justification/justification.component.ts | 5 ++++- .../registry-make-decision.component.ts | 6 ++++-- .../registry-overview.component.ts | 2 +- .../registry-wiki/registry-wiki.component.ts | 4 ++-- .../education-history-dialog.component.ts | 8 +++++--- .../employment-history-dialog.component.ts | 8 +++++--- .../truncated-text/truncated-text.component.ts | 18 ++++++++++++++++-- .../shared/directives/scroll-top.directive.ts | 5 +++-- 11 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/app/features/project/wiki/wiki.component.ts b/src/app/features/project/wiki/wiki.component.ts index 71b31364e..4c8a6f08b 100644 --- a/src/app/features/project/wiki/wiki.component.ts +++ b/src/app/features/project/wiki/wiki.component.ts @@ -108,7 +108,7 @@ export class WikiComponent { this.actions .getWikiList(ResourceType.Project, this.projectId()) .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), tap(() => { if (!this.wikiIdFromQueryParams) { this.navigateToWiki(this.wikiList()?.[0]?.id || ''); @@ -124,7 +124,7 @@ export class WikiComponent { this.route.queryParams .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), map((params) => params['wiki']), filter((wikiId) => wikiId), tap((wikiId) => { diff --git a/src/app/features/registries/components/custom-step/custom-step.component.ts b/src/app/features/registries/components/custom-step/custom-step.component.ts index af774ec20..98900e02c 100644 --- a/src/app/features/registries/components/custom-step/custom-step.component.ts +++ b/src/app/features/registries/components/custom-step/custom-step.component.ts @@ -17,6 +17,7 @@ import { ChangeDetectionStrategy, Component, computed, + DestroyRef, effect, inject, input, @@ -82,6 +83,7 @@ export class CustomStepComponent implements OnDestroy { private readonly route = inject(ActivatedRoute); private readonly router = inject(Router); private readonly fb = inject(FormBuilder); + private readonly destroyRef = inject(DestroyRef); private toastService = inject(ToastService); readonly pages = select(RegistriesSelectors.getPagesSchema); @@ -105,7 +107,7 @@ export class CustomStepComponent implements OnDestroy { attachedFiles: Record[]> = {}; constructor() { - this.route.params.pipe(takeUntilDestroyed()).subscribe((params) => { + this.route.params.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((params) => { this.updateStepState(); this.step.set(+params['step']); }); diff --git a/src/app/features/registries/components/drafts/drafts.component.ts b/src/app/features/registries/components/drafts/drafts.component.ts index 5edbcd1b2..353456834 100644 --- a/src/app/features/registries/components/drafts/drafts.component.ts +++ b/src/app/features/registries/components/drafts/drafts.component.ts @@ -8,6 +8,7 @@ import { ChangeDetectionStrategy, Component, computed, + DestroyRef, effect, inject, OnDestroy, @@ -43,6 +44,7 @@ export class DraftsComponent implements OnDestroy { private readonly route = inject(ActivatedRoute); private readonly loaderService = inject(LoaderService); private readonly translateService = inject(TranslateService); + private readonly destroyRef = inject(DestroyRef); readonly pages = select(RegistriesSelectors.getPagesSchema); readonly draftRegistration = select(RegistriesSelectors.getDraftRegistration); @@ -127,7 +129,7 @@ export class DraftsComponent implements OnDestroy { constructor() { this.router.events .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), filter((event): event is NavigationEnd => event instanceof NavigationEnd) ) .subscribe(() => { diff --git a/src/app/features/registries/pages/justification/justification.component.ts b/src/app/features/registries/pages/justification/justification.component.ts index cc5a5e6f7..e196e9038 100644 --- a/src/app/features/registries/pages/justification/justification.component.ts +++ b/src/app/features/registries/pages/justification/justification.component.ts @@ -8,6 +8,7 @@ import { ChangeDetectionStrategy, Component, computed, + DestroyRef, effect, inject, OnDestroy, @@ -37,9 +38,11 @@ import { ClearState, FetchSchemaBlocks, FetchSchemaResponse, RegistriesSelectors export class JustificationComponent implements OnDestroy { private readonly route = inject(ActivatedRoute); private readonly router = inject(Router); + private readonly destroyRef = inject(DestroyRef); private readonly loaderService = inject(LoaderService); private readonly translateService = inject(TranslateService); + readonly pages = select(RegistriesSelectors.getPagesSchema); readonly stepsState = select(RegistriesSelectors.getStepsState); readonly schemaResponse = select(RegistriesSelectors.getSchemaResponse); @@ -109,7 +112,7 @@ export class JustificationComponent implements OnDestroy { constructor() { this.router.events .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), filter((event): event is NavigationEnd => event instanceof NavigationEnd) ) .subscribe(() => { diff --git a/src/app/features/registry/components/registry-make-decision/registry-make-decision.component.ts b/src/app/features/registry/components/registry-make-decision/registry-make-decision.component.ts index 25e80e226..ff1994ec1 100644 --- a/src/app/features/registry/components/registry-make-decision/registry-make-decision.component.ts +++ b/src/app/features/registry/components/registry-make-decision/registry-make-decision.component.ts @@ -11,7 +11,7 @@ import { Textarea } from 'primeng/textarea'; import { tap } from 'rxjs'; import { DatePipe } from '@angular/common'; -import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, DestroyRef, inject } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { FormBuilder, FormControl, FormGroup, FormsModule, ReactiveFormsModule, Validators } from '@angular/forms'; @@ -46,6 +46,8 @@ import { RegistryOverviewSelectors, SubmitDecision } from '../../store/registry- }) export class RegistryMakeDecisionComponent { private readonly fb = inject(FormBuilder); + private readonly destroyRef = inject(DestroyRef); + readonly config = inject(DynamicDialogConfig); readonly dialogRef = inject(DynamicDialogRef); @@ -118,7 +120,7 @@ export class RegistryMakeDecisionComponent { this.requestForm .get(ModerationDecisionFormControls.Action) - ?.valueChanges.pipe(takeUntilDestroyed()) + ?.valueChanges.pipe(takeUntilDestroyed(this.destroyRef)) .subscribe((action) => { this.updateCommentValidators(action); }); diff --git a/src/app/features/registry/pages/registry-overview/registry-overview.component.ts b/src/app/features/registry/pages/registry-overview/registry-overview.component.ts index 883bd397d..2f38cb0a9 100644 --- a/src/app/features/registry/pages/registry-overview/registry-overview.component.ts +++ b/src/app/features/registry/pages/registry-overview/registry-overview.component.ts @@ -235,7 +235,7 @@ export class RegistryOverviewComponent { this.actions.getBookmarksId(); this.route.queryParams .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), map((params) => ({ revisionId: params['revisionId'], mode: params['mode'] })), tap(({ revisionId, mode }) => { this.revisionId = revisionId; diff --git a/src/app/features/registry/pages/registry-wiki/registry-wiki.component.ts b/src/app/features/registry/pages/registry-wiki/registry-wiki.component.ts index fe74f18b7..322808dfd 100644 --- a/src/app/features/registry/pages/registry-wiki/registry-wiki.component.ts +++ b/src/app/features/registry/pages/registry-wiki/registry-wiki.component.ts @@ -87,7 +87,7 @@ export class RegistryWikiComponent { this.actions .getWikiList(ResourceType.Registration, this.resourceId) .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), tap(() => { if (!this.wikiIdFromQueryParams) { this.navigateToWiki(this.wikiList()?.[0]?.id || ''); @@ -100,7 +100,7 @@ export class RegistryWikiComponent { this.route.queryParams .pipe( - takeUntilDestroyed(), + takeUntilDestroyed(this.destroyRef), map((params) => params['wiki']), filter((wikiId) => wikiId), tap((wikiId) => { diff --git a/src/app/shared/components/education-history-dialog/education-history-dialog.component.ts b/src/app/shared/components/education-history-dialog/education-history-dialog.component.ts index 01c41709e..71398bed4 100644 --- a/src/app/shared/components/education-history-dialog/education-history-dialog.component.ts +++ b/src/app/shared/components/education-history-dialog/education-history-dialog.component.ts @@ -5,7 +5,7 @@ import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; import { timer } from 'rxjs'; -import { ChangeDetectionStrategy, Component, inject, signal } from '@angular/core'; +import { ChangeDetectionStrategy, Component, DestroyRef, inject, signal } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { Education } from '@osf/shared/models/user/education.model'; @@ -20,8 +20,10 @@ import { EducationHistoryComponent } from '../education-history/education-histor changeDetection: ChangeDetectionStrategy.OnPush, }) export class EducationHistoryDialogComponent { - private readonly config = inject(DynamicDialogConfig); dialogRef = inject(DynamicDialogRef); + private readonly config = inject(DynamicDialogConfig); + private readonly destroyRef = inject(DestroyRef); + readonly educationHistory = signal([]); readonly isContentVisible = signal(false); @@ -29,7 +31,7 @@ export class EducationHistoryDialogComponent { this.educationHistory.set(this.config.data); timer(0) - .pipe(takeUntilDestroyed()) + .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(() => this.isContentVisible.set(true)); } diff --git a/src/app/shared/components/employment-history-dialog/employment-history-dialog.component.ts b/src/app/shared/components/employment-history-dialog/employment-history-dialog.component.ts index f66595d9a..6b2918bee 100644 --- a/src/app/shared/components/employment-history-dialog/employment-history-dialog.component.ts +++ b/src/app/shared/components/employment-history-dialog/employment-history-dialog.component.ts @@ -5,7 +5,7 @@ import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; import { timer } from 'rxjs'; -import { ChangeDetectionStrategy, Component, inject, signal } from '@angular/core'; +import { ChangeDetectionStrategy, Component, DestroyRef, inject, signal } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { Employment } from '@osf/shared/models/user/employment.model'; @@ -20,8 +20,10 @@ import { EmploymentHistoryComponent } from '../employment-history/employment-his changeDetection: ChangeDetectionStrategy.OnPush, }) export class EmploymentHistoryDialogComponent { - private readonly config = inject(DynamicDialogConfig); dialogRef = inject(DynamicDialogRef); + private readonly destroyRef = inject(DestroyRef); + private readonly config = inject(DynamicDialogConfig); + readonly employmentHistory = signal([]); readonly isContentVisible = signal(false); @@ -29,7 +31,7 @@ export class EmploymentHistoryDialogComponent { this.employmentHistory.set(this.config.data); timer(0) - .pipe(takeUntilDestroyed()) + .pipe(takeUntilDestroyed(this.destroyRef)) .subscribe(() => this.isContentVisible.set(true)); } diff --git a/src/app/shared/components/truncated-text/truncated-text.component.ts b/src/app/shared/components/truncated-text/truncated-text.component.ts index 63d2e4afe..d821033db 100644 --- a/src/app/shared/components/truncated-text/truncated-text.component.ts +++ b/src/app/shared/components/truncated-text/truncated-text.component.ts @@ -4,7 +4,18 @@ import { Button } from 'primeng/button'; import { timer } from 'rxjs'; -import { AfterViewInit, Component, effect, ElementRef, inject, input, signal, viewChild } from '@angular/core'; +import { + AfterViewInit, + Component, + DestroyRef, + effect, + ElementRef, + inject, + input, + signal, + viewChild, +} from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { Router } from '@angular/router'; @Component({ @@ -24,6 +35,7 @@ export class TruncatedTextComponent implements AfterViewInit { readonly contentElement = viewChild('textContent'); private readonly router = inject(Router); + private readonly destroyRef = inject(DestroyRef); isTextExpanded = signal(false); hasOverflowingText = signal(false); @@ -41,7 +53,9 @@ export class TruncatedTextComponent implements AfterViewInit { const currentText = this.text(); if (currentText) { this.isTextExpanded.set(false); - timer(0).subscribe(() => this.checkTextOverflow()); + timer(0) + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(() => this.checkTextOverflow()); } }); } diff --git a/src/app/shared/directives/scroll-top.directive.ts b/src/app/shared/directives/scroll-top.directive.ts index ba9369496..bbd14ea78 100644 --- a/src/app/shared/directives/scroll-top.directive.ts +++ b/src/app/shared/directives/scroll-top.directive.ts @@ -1,6 +1,6 @@ import { filter } from 'rxjs'; -import { Directive, ElementRef, inject } from '@angular/core'; +import { DestroyRef, Directive, ElementRef, inject } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { NavigationEnd, Router } from '@angular/router'; @@ -10,12 +10,13 @@ import { NavigationEnd, Router } from '@angular/router'; export class ScrollTopOnRouteChangeDirective { private el = inject(ElementRef); private router = inject(Router); + private destroyRef = inject(DestroyRef); constructor() { this.router.events .pipe( filter((e) => e instanceof NavigationEnd), - takeUntilDestroyed() + takeUntilDestroyed(this.destroyRef) ) .subscribe(() => { let route = this.router.routerState.root; From 57542e4f0684856052569b87e295ab2cb3356443 Mon Sep 17 00:00:00 2001 From: nsemets Date: Mon, 3 Nov 2025 19:51:57 +0200 Subject: [PATCH 2/2] fix(analytics): renamed service to avoid duplicates --- src/app/features/analytics/services/index.ts | 2 +- ...ytics.service.ts => resource-analytics.service.ts} | 11 +++++++---- src/app/features/analytics/store/analytics.state.ts | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) rename src/app/features/analytics/services/{analytics.service.ts => resource-analytics.service.ts} (84%) diff --git a/src/app/features/analytics/services/index.ts b/src/app/features/analytics/services/index.ts index 58bd2917d..5afbc150a 100644 --- a/src/app/features/analytics/services/index.ts +++ b/src/app/features/analytics/services/index.ts @@ -1 +1 @@ -export { AnalyticsService } from './analytics.service'; +export { ResourceAnalyticsService } from './resource-analytics.service'; diff --git a/src/app/features/analytics/services/analytics.service.ts b/src/app/features/analytics/services/resource-analytics.service.ts similarity index 84% rename from src/app/features/analytics/services/analytics.service.ts rename to src/app/features/analytics/services/resource-analytics.service.ts index 715ad26ce..96a7e1bf6 100644 --- a/src/app/features/analytics/services/analytics.service.ts +++ b/src/app/features/analytics/services/resource-analytics.service.ts @@ -3,16 +3,19 @@ import { map, Observable } from 'rxjs'; import { inject, Injectable } from '@angular/core'; import { ENVIRONMENT } from '@core/provider/environment.provider'; +import { AnalyticsMetricsMapper, RelatedCountsMapper } from '@osf/features/analytics/mappers'; +import { + NodeAnalyticsModel, + NodeAnalyticsResponseJsonApi, + RelatedCountsGetResponse, +} from '@osf/features/analytics/models'; import { ResourceType } from '@osf/shared/enums/resource-type.enum'; import { JsonApiService } from '@osf/shared/services/json-api.service'; -import { AnalyticsMetricsMapper, RelatedCountsMapper } from '../mappers'; -import { NodeAnalyticsModel, NodeAnalyticsResponseJsonApi, RelatedCountsGetResponse } from '../models'; - @Injectable({ providedIn: 'root', }) -export class AnalyticsService { +export class ResourceAnalyticsService { private readonly jsonApiService = inject(JsonApiService); private readonly environment = inject(ENVIRONMENT); diff --git a/src/app/features/analytics/store/analytics.state.ts b/src/app/features/analytics/store/analytics.state.ts index 064c28310..d4092992e 100644 --- a/src/app/features/analytics/store/analytics.state.ts +++ b/src/app/features/analytics/store/analytics.state.ts @@ -8,7 +8,7 @@ import { inject, Injectable } from '@angular/core'; import { handleSectionError } from '@osf/shared/helpers/state-error.handler'; import { NodeAnalyticsModel, RelatedCountsModel } from '../models'; -import { AnalyticsService } from '../services'; +import { ResourceAnalyticsService } from '../services'; import { ClearAnalytics, GetMetrics, GetRelatedCounts } from './analytics.actions'; import { ANALYTICS_DEFAULT_STATE, AnalyticsStateModel } from './analytics.model'; @@ -19,7 +19,7 @@ import { ANALYTICS_DEFAULT_STATE, AnalyticsStateModel } from './analytics.model' }) @Injectable() export class AnalyticsState { - private readonly analyticsService = inject(AnalyticsService); + private readonly analyticsService = inject(ResourceAnalyticsService); private readonly REFRESH_INTERVAL = 5 * 60 * 1000; @Action(GetMetrics)