Code improvements (#740)

nsemets · web-flow · commit f193b2b7d2ad · 2025-11-03T13:08:28.000-05:00
## Summary of Changes
1. Add missing `destroyRef`for `takeUntilDestroyed`.
2. Renamed analytics service to avoid duplication of services.
diff --git 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/resource-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
@@ -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)
diff --git 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
@@ -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<string, Partial<FileModel & { file_id: string }>[]> = {};
 
   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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,16 +20,18 @@ 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<Education[]>([]);
   readonly isContentVisible = signal(false);
 
   constructor() {
     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
@@ -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,16 +20,18 @@ 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<Employment[]>([]);
   readonly isContentVisible = signal(false);
 
   constructor() {
     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
@@ -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<ElementRef>('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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-export { AnalyticsService } from './analytics.service';`
	`1`	`+export { ResourceAnalyticsService } from './resource-analytics.service';`