feat(registration-recent-activity): second round of code fixes regarding review comments

Author: sh-andriy

src/app/core/constants/environment.token.ts

@@ -1,8 +1,9 @@
 import { InjectionToken } from '@angular/core';
 
 import { environment } from 'src/environments/environment';
+import { AppEnvironment } from '@shared/models/environment.model';
 
-export const ENVIRONMENT = new InjectionToken<typeof environment>('App Environment', {
+export const ENVIRONMENT = new InjectionToken<AppEnvironment>('App Environment', {
   providedIn: 'root',
-  factory: () => environment,
+  factory: () => environment as AppEnvironment,
 });

src/app/core/guards/require-registration-id.guard.ts

@@ -1,22 +1,98 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { provideStore, Store } from '@ngxs/store';
+import { ActivatedRoute } from '@angular/router';
+
+import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
+import { provideHttpClientTesting } from '@angular/common/http/testing';
+
+import { TranslateService } from '@ngx-translate/core';
+import { of } from 'rxjs';
 
 import { RecentActivityComponent } from './recent-activity.component';
+import { ActivityLogDisplayService } from '@shared/services';
+import { GetActivityLogs } from '@shared/stores/activity-logs';
+import { ActivityLogsState } from '@shared/stores/activity-logs/activity-logs.state';
 
 describe('RecentActivityComponent', () => {
-  let component: RecentActivityComponent;
   let fixture: ComponentFixture<RecentActivityComponent>;
+  let store: Store;
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
       imports: [RecentActivityComponent],
+      providers: [
+        provideStore([ActivityLogsState]),
+
+        provideHttpClient(withInterceptorsFromDi()),
+        provideHttpClientTesting(),
+
+        {
+          provide: TranslateService,
+          useValue: {
+            instant: (k: string) => k,
+            get: () => of(''),
+            stream: () => of(''),
+            onLangChange: of({}),
+            onDefaultLangChange: of({}),
+            onTranslationChange: of({}),
+          },
+        },
+
+        { provide: ActivatedRoute, useValue: { snapshot: { params: { id: 'proj123' } }, parent: null } },
+        { provide: ActivityLogDisplayService, useValue: { getActivityDisplay: jest.fn().mockReturnValue('FMT') } },
+      ],
     }).compileComponents();
 
+    store = TestBed.inject(Store);
+    store.reset({
+      activityLogs: {
+        activityLogs: { data: [], isLoading: false, error: null, totalCount: 0 },
+      },
+    } as any);
+
     fixture = TestBed.createComponent(RecentActivityComponent);
-    component = fixture.componentInstance;
+    fixture.componentRef.setInput('pageSize', 10);
     fixture.detectChanges();
   });
 
   it('should create', () => {
-    expect(component).toBeTruthy();
+    expect(fixture.componentInstance).toBeTruthy();
+  });
+
+  it('formats activity logs using ActivityLogDisplayService', () => {
+    store.reset({
+      activityLogs: {
+        activityLogs: {
+          data: [{ id: 'log1', date: '2024-01-01T00:00:00Z' }],
+          isLoading: false,
+          error: null,
+          totalCount: 1,
+        },
+      },
+    } as any);
+
+    fixture.detectChanges();
+
+    const formatted = fixture.componentInstance.formattedActivityLogs();
+    expect(formatted.length).toBe(1);
+    expect(formatted[0].formattedActivity).toBe('FMT');
+  });
+
+  it('dispatches GetActivityLogs with numeric page and pageSize on page change', () => {
+    const dispatchSpy = jest.spyOn(store, 'dispatch');
+    fixture.componentInstance.onPageChange({ page: 2 } as any);
+
+    expect(dispatchSpy).toHaveBeenCalled();
+    const action = dispatchSpy.mock.calls.at(-1)?.[0] as GetActivityLogs;
+
+    expect(action).toBeInstanceOf(GetActivityLogs);
+    expect(action.projectId).toBe('proj123');
+    expect(action.page).toBe(3);
+    expect(action.pageSize).toBe(10);
+  });
+
+  it('computes firstIndex correctly', () => {
+    fixture.componentInstance['currentPage'].set(3);
+    expect(fixture.componentInstance['firstIndex']()).toBe(20);
   });
 });

src/app/features/project/overview/components/recent-activity/recent-activity.component.spec.ts

@@ -1,26 +1,69 @@
-import { MockComponent } from 'ng-mocks';
-
 import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { RouterTestingModule } from '@angular/router/testing';
+import { provideStore, Store } from '@ngxs/store';
+import { ActivatedRoute } from '@angular/router';
+import { of } from 'rxjs';
 
-import { SubHeaderComponent } from '@osf/shared/components';
+import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
+import { provideHttpClientTesting } from '@angular/common/http/testing';
 
 import { ProjectOverviewComponent } from './project-overview.component';
+import { GetActivityLogs } from '@shared/stores/activity-logs';
+
+import { DataciteService } from '@osf/shared/services';
+import { DialogService } from 'primeng/dynamicdialog';
+import { TranslateService } from '@ngx-translate/core';
+import { ToastService } from '@osf/shared/services';
 
 describe('ProjectOverviewComponent', () => {
-  let component: ProjectOverviewComponent;
   let fixture: ComponentFixture<ProjectOverviewComponent>;
+  let component: ProjectOverviewComponent;
+  let store: Store;
 
   beforeEach(async () => {
+    TestBed.overrideComponent(ProjectOverviewComponent, { set: { template: '' } });
+
     await TestBed.configureTestingModule({
-      imports: [ProjectOverviewComponent, MockComponent(SubHeaderComponent)],
+      imports: [ProjectOverviewComponent, RouterTestingModule],
+      providers: [
+        provideStore([]),
+
+        { provide: ActivatedRoute, useValue: { snapshot: { params: { id: 'proj123' } }, parent: null } },
+
+        provideHttpClient(withInterceptorsFromDi()),
+        provideHttpClientTesting(),
+
+        { provide: DataciteService, useValue: {} },
+        { provide: DialogService, useValue: { open: () => ({ onClose: of(null) }) } },
+        { provide: TranslateService, useValue: { instant: (k: string) => k } },
+        { provide: ToastService, useValue: { showSuccess: jest.fn() } },
+      ],
     }).compileComponents();
 
+    store = TestBed.inject(Store);
     fixture = TestBed.createComponent(ProjectOverviewComponent);
     component = fixture.componentInstance;
-    fixture.detectChanges();
   });
 
   it('should create', () => {
     expect(component).toBeTruthy();
   });
+
+  it('dispatches GetActivityLogs with numeric page and pageSize on init', () => {
+    const dispatchSpy = jest.spyOn(store, 'dispatch');
+
+    jest.spyOn(component as any, 'setupDataciteViewTrackerEffect').mockReturnValue(of(null));
+
+    component.ngOnInit();
+
+    const actions = dispatchSpy.mock.calls.map((c) => c[0]);
+    const activityAction = actions.find((a) => a instanceof GetActivityLogs) as GetActivityLogs;
+
+    expect(activityAction).toBeDefined();
+    expect(activityAction.projectId).toBe('proj123');
+    expect(activityAction.page).toBe(1);
+    expect(activityAction.pageSize).toBe(5);
+    expect(typeof activityAction.page).toBe('number');
+    expect(typeof activityAction.pageSize).toBe('number');
+  });
 });

src/app/features/project/overview/project-overview.component.spec.ts

@@ -7,7 +7,7 @@ <h2 id="recent-activity-title" class="mb-2" data-test="recent-activity-title">
     {{ 'project.overview.recentActivity.title' | translate }}
   </h2>
 
-  @defer (when !isLoading()) {
+  @if (!isLoading()) {
     <div role="list" data-test="recent-activity-list">
       @for (activityLog of formattedActivityLogs(); track activityLog.id) {
         <div
@@ -38,5 +38,13 @@ <h2 id="recent-activity-title" class="mb-2" data-test="recent-activity-title">
         (pageChanged)="onPageChange($event)"
       />
     }
+  } @else {
+    <div class="flex flex-column gap-2" data-test="recent-activity-skeleton">
+      <p-skeleton width="100%" height="2rem" />
+      <p-skeleton width="100%" height="2rem" />
+      <p-skeleton width="100%" height="2rem" />
+      <p-skeleton width="100%" height="2rem" />
+      <p-skeleton width="100%" height="2rem" />
+    </div>
   }
 </div>

src/app/features/registry/pages/recent-activity/registration-recent-activity.component.html

@@ -1,3 +1,9 @@
+import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http';
+import { provideHttpClientTesting } from '@angular/common/http/testing';
+import { TranslateService } from '@ngx-translate/core';
+import { of } from 'rxjs';
+import { ActivityLogDisplayService } from '@shared/services';
+
 import { provideStore, Store } from '@ngxs/store';
 
 import { ComponentFixture, TestBed } from '@angular/core/testing';
@@ -17,6 +23,27 @@ describe('RegistrationRecentActivityComponent', () => {
       imports: [RegistrationRecentActivityComponent],
       providers: [
         provideStore([ActivityLogsState]),
+
+        provideHttpClient(withInterceptorsFromDi()),
+        provideHttpClientTesting(),
+
+        {
+          provide: TranslateService,
+          useValue: {
+            instant: (k: string) => k,
+            get: () => of(''),
+            stream: () => of(''),
+            onLangChange: of({}),
+            onDefaultLangChange: of({}),
+            onTranslationChange: of({}),
+          },
+        },
+
+        {
+          provide: ActivityLogDisplayService,
+          useValue: { getActivityDisplay: jest.fn(() => '<b>formatted</b>') },
+        },
+
         { provide: ActivatedRoute, useValue: { snapshot: { params: { id: 'reg123' } }, parent: null } },
       ],
     }).compileComponents();
@@ -55,7 +82,7 @@ describe('RegistrationRecentActivityComponent', () => {
           data: [
             {
               id: 'log1',
-              date: '2024-01-01T00:00:00Z',
+              date: '2024-01-01T12:34:00Z',
               formattedActivity: '<b>formatted</b>',
             },
           ],
@@ -70,10 +97,29 @@ describe('RegistrationRecentActivityComponent', () => {
     const item = fixture.nativeElement.querySelector('[data-test="recent-activity-item"]');
     const content = fixture.nativeElement.querySelector('[data-test="recent-activity-item-content"]');
     const paginator = fixture.nativeElement.querySelector('[data-test="recent-activity-paginator"]');
+    const dateText = fixture.nativeElement.querySelector('[data-test="recent-activity-item-date"]')?.textContent ?? '';
 
     expect(item).toBeTruthy();
     expect(content?.innerHTML).toContain('formatted');
     expect(paginator).toBeTruthy();
+    expect(dateText).toMatch(/\w{3} \d{1,2}, \d{4} \d{1,2}:\d{2} [AP]M/);
+  });
+
+  it('does not render paginator when totalCount <= pageSize', () => {
+    store.reset({
+      activityLogs: {
+        activityLogs: {
+          data: [{ id: 'log1', date: '2024-01-01T12:34:00Z', formattedActivity: '<b>formatted</b>' }],
+          isLoading: false,
+          error: null,
+          totalCount: 10,
+        },
+      },
+    } as any);
+    fixture.detectChanges();
+
+    const paginator = fixture.nativeElement.querySelector('[data-test="recent-activity-paginator"]');
+    expect(paginator).toBeFalsy();
   });
 
   it('dispatches on page change', () => {
@@ -87,11 +133,64 @@ describe('RegistrationRecentActivityComponent', () => {
     expect(action.page).toBe(3);
   });
 
+  it('does not dispatch when page change event has undefined page', () => {
+    const dispatchSpy = store.dispatch as jest.Mock;
+    dispatchSpy.mockClear();
+
+    fixture.componentInstance.onPageChange({} as any);
+    expect(dispatchSpy).not.toHaveBeenCalled();
+  });
+
+  it('computes firstIndex correctly after page change', () => {
+    fixture.componentInstance.onPageChange({ page: 1 } as any);
+    const firstIndex = (fixture.componentInstance as any)['firstIndex']();
+    expect(firstIndex).toBe(10);
+  });
+
   it('clears store on destroy', () => {
     const dispatchSpy = store.dispatch as jest.Mock;
     dispatchSpy.mockClear();
 
     fixture.destroy();
     expect(dispatchSpy).toHaveBeenCalledWith(expect.any(ClearActivityLogsStore));
   });
+
+  it('shows skeleton while loading', () => {
+    store.reset({
+      activityLogs: {
+        activityLogs: { data: [], isLoading: true, error: null, totalCount: 0 },
+      },
+    } as any);
+
+    fixture.detectChanges();
+
+    expect(fixture.nativeElement.querySelector('[data-test="recent-activity-skeleton"]')).toBeTruthy();
+    expect(fixture.nativeElement.querySelector('[data-test="recent-activity-list"]')).toBeFalsy();
+    expect(fixture.nativeElement.querySelector('[data-test="recent-activity-paginator"]')).toBeFalsy();
+  });
+
+  it('renders expected ARIA roles/labels', () => {
+    store.reset({
+      activityLogs: {
+        activityLogs: {
+          data: [{ id: 'log1', date: '2024-01-01T12:34:00Z', formattedActivity: '<b>formatted</b>' }],
+          isLoading: false,
+          error: null,
+          totalCount: 1,
+        },
+      },
+    } as any);
+    fixture.detectChanges();
+
+    const region = fixture.nativeElement.querySelector('[role="region"]');
+    const heading = fixture.nativeElement.querySelector('#recent-activity-title');
+    const list = fixture.nativeElement.querySelector('[role="list"]');
+    const listitem = fixture.nativeElement.querySelector('[role="listitem"]');
+
+    expect(region).toBeTruthy();
+    expect(region.getAttribute('aria-labelledby')).toBe('recent-activity-title');
+    expect(heading).toBeTruthy();
+    expect(list).toBeTruthy();
+    expect(listitem).toBeTruthy();
+  });
 });