Skip to content

Commit 7dc4e27

Browse files
committed
feat(registration-recent-activity): code fixes regarding comments
1 parent 38f9232 commit 7dc4e27

15 files changed

+333
-106
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { inject } from '@angular/core';
2+
import { CanActivateFn, Router, UrlTree } from '@angular/router';
3+
4+
export const requireRegistrationIdGuard: CanActivateFn = (route): boolean | UrlTree => {
5+
const id = route.paramMap.get('id');
6+
if (id) return true;
7+
8+
return inject(Router).parseUrl('/registries/discover');
9+
};

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ export class RecentActivityComponent {
2626

2727
readonly pageSize = input.required<number>();
2828
currentPage = signal<number>(1);
29+
2930
activityLogs = select(ActivityLogsSelectors.getActivityLogs);
3031
totalCount = select(ActivityLogsSelectors.getActivityLogsTotalCount);
3132
isLoading = select(ActivityLogsSelectors.getActivityLogsLoading);
33+
3234
firstIndex = computed(() => (this.currentPage() - 1) * this.pageSize());
3335

34-
actions = createDispatchMap({
35-
getActivityLogs: GetActivityLogs,
36-
});
36+
actions = createDispatchMap({ getActivityLogs: GetActivityLogs });
3737

3838
formattedActivityLogs = computed(() => {
3939
const logs = this.activityLogs();
@@ -50,7 +50,7 @@ export class RecentActivityComponent {
5050

5151
const projectId = this.route.snapshot.params['id'] || this.route.parent?.snapshot.params['id'];
5252
if (projectId) {
53-
this.actions.getActivityLogs(projectId, pageNumber.toString(), this.pageSize().toString());
53+
this.actions.getActivityLogs(projectId, pageNumber, this.pageSize());
5454
}
5555
}
5656
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export class ProjectOverviewComponent extends DataciteTrackerComponent implement
246246
this.actions.getComponents(projectId);
247247
this.actions.getComponentsTree(projectId, ResourceType.Project);
248248
this.actions.getLinkedProjects(projectId);
249-
this.actions.getActivityLogs(projectId, this.activityDefaultPage.toString(), this.activityPageSize.toString());
249+
this.actions.getActivityLogs(projectId, this.activityDefaultPage, this.activityPageSize);
250250
this.setupDataciteViewTrackerEffect().subscribe();
251251
}
252252
}
Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,42 @@
1-
<div class="activities p-3 flex flex-column gap-3">
2-
<h2 class="mb-2">{{ 'project.overview.recentActivity.title' | translate }}</h2>
1+
<div
2+
class="p-3 flex flex-column gap-3 border-1 surface-border border-round-xl text-color"
3+
role="region"
4+
aria-labelledby="recent-activity-title"
5+
>
6+
<h2 id="recent-activity-title" class="mb-2" data-test="recent-activity-title">
7+
{{ 'project.overview.recentActivity.title' | translate }}
8+
</h2>
39

4-
@if (!isLoading()) {
5-
@if (formattedActivityLogs().length) {
10+
@defer (when !isLoading()) {
11+
<div role="list" data-test="recent-activity-list">
612
@for (activityLog of formattedActivityLogs(); track activityLog.id) {
7-
<div class="activities-activity flex justify-content-between gap-3 pb-2 align-items-center">
8-
<div [innerHTML]="activityLog.formattedActivity"></div>
9-
<p class="hidden activity-date sm:block text-right">
13+
<div
14+
class="flex justify-content-between gap-3 pb-2 align-items-center border-bottom-1 surface-border"
15+
role="listitem"
16+
data-test="recent-activity-item"
17+
>
18+
<div [innerHTML]="activityLog.formattedActivity" data-test="recent-activity-item-content"></div>
19+
20+
<p class="hidden sm:block text-right white-space-nowrap flex-shrink-0" data-test="recent-activity-item-date">
1021
{{ activityLog.date | date: 'MMM d, y hh:mm a' }}
1122
</p>
1223
</div>
24+
} @empty {
25+
<div class="text-center text-muted-color" role="status" aria-live="polite" data-test="recent-activity-empty">
26+
{{ 'project.overview.recentActivity.noActivity' | translate }}
27+
</div>
1328
}
14-
} @else {
15-
<div class="text-center text-muted-color">
16-
{{ 'project.overview.recentActivity.noActivity' | translate }}
17-
</div>
18-
}
29+
</div>
1930

2031
@if (totalCount() > pageSize) {
2132
<osf-custom-paginator
33+
data-test="recent-activity-paginator"
2234
[showFirstLastIcon]="true"
2335
[totalCount]="totalCount()"
2436
[rows]="pageSize"
2537
[first]="firstIndex()"
2638
(pageChanged)="onPageChange($event)"
2739
/>
2840
}
29-
} @else {
30-
<div class="flex flex-column gap-2">
31-
<p-skeleton width="100%" height="2rem" />
32-
<p-skeleton width="100%" height="2rem" />
33-
<p-skeleton width="100%" height="2rem" />
34-
<p-skeleton width="100%" height="2rem" />
35-
<p-skeleton width="100%" height="2rem" />
36-
</div>
3741
}
3842
</div>
Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +0,0 @@
1-
@use "/assets/styles/variables" as var;
2-
@use "/assets/styles/mixins" as mix;
3-
4-
.activities {
5-
border: 1px solid var.$grey-2;
6-
border-radius: mix.rem(12px);
7-
color: var.$dark-blue-1;
8-
9-
&-activity {
10-
border-bottom: 1px solid var.$grey-2;
11-
12-
.activity-date {
13-
width: 30%;
14-
}
15-
}
16-
}
Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { NgxsModule, Store } from '@ngxs/store';
1+
import { provideStore, Store } from '@ngxs/store';
22

33
import { ComponentFixture, TestBed } from '@angular/core/testing';
44
import { ActivatedRoute } from '@angular/router';
@@ -14,36 +14,84 @@ describe('RegistrationRecentActivityComponent', () => {
1414

1515
beforeEach(async () => {
1616
await TestBed.configureTestingModule({
17-
imports: [NgxsModule.forRoot([ActivityLogsState]), RegistrationRecentActivityComponent],
18-
providers: [{ provide: ActivatedRoute, useValue: { snapshot: { params: { id: 'reg123' } }, parent: null } }],
17+
imports: [RegistrationRecentActivityComponent],
18+
providers: [
19+
provideStore([ActivityLogsState]),
20+
{ provide: ActivatedRoute, useValue: { snapshot: { params: { id: 'reg123' } }, parent: null } },
21+
],
1922
}).compileComponents();
2023

2124
store = TestBed.inject(Store);
22-
spyOn(store, 'dispatch').and.callThrough();
25+
jest.spyOn(store, 'dispatch');
2326

2427
fixture = TestBed.createComponent(RegistrationRecentActivityComponent);
2528
fixture.detectChanges();
2629
});
2730

28-
it('creates and dispatches initial registration logs fetch', () => {
29-
expect(fixture.componentInstance).toBeTruthy();
30-
expect(store.dispatch).toHaveBeenCalledWith(jasmine.any(GetRegistrationActivityLogs));
31-
const action = (store.dispatch as jasmine.Spy).calls.mostRecent().args[0] as GetRegistrationActivityLogs;
31+
it('dispatches initial registration logs fetch', () => {
32+
const dispatchSpy = store.dispatch as jest.Mock;
33+
expect(dispatchSpy).toHaveBeenCalledWith(expect.any(GetRegistrationActivityLogs));
34+
const action = dispatchSpy.mock.calls.at(-1)?.[0] as GetRegistrationActivityLogs;
3235
expect(action.registrationId).toBe('reg123');
33-
expect(action.page).toBe('1');
36+
expect(action.page).toBe(1);
37+
});
38+
39+
it('renders empty state when no logs and not loading', () => {
40+
store.reset({
41+
activityLogs: {
42+
activityLogs: { data: [], isLoading: false, error: null, totalCount: 0 },
43+
},
44+
} as any);
45+
fixture.detectChanges();
46+
47+
const empty = fixture.nativeElement.querySelector('[data-test="recent-activity-empty"]');
48+
expect(empty).toBeTruthy();
49+
});
50+
51+
it('renders item & paginator when logs exist and totalCount > pageSize', () => {
52+
store.reset({
53+
activityLogs: {
54+
activityLogs: {
55+
data: [
56+
{
57+
id: 'log1',
58+
date: '2024-01-01T00:00:00Z',
59+
formattedActivity: '<b>formatted</b>',
60+
},
61+
],
62+
isLoading: false,
63+
error: null,
64+
totalCount: 25,
65+
},
66+
},
67+
} as any);
68+
fixture.detectChanges();
69+
70+
const item = fixture.nativeElement.querySelector('[data-test="recent-activity-item"]');
71+
const content = fixture.nativeElement.querySelector('[data-test="recent-activity-item-content"]');
72+
const paginator = fixture.nativeElement.querySelector('[data-test="recent-activity-paginator"]');
73+
74+
expect(item).toBeTruthy();
75+
expect(content?.innerHTML).toContain('formatted');
76+
expect(paginator).toBeTruthy();
3477
});
3578

3679
it('dispatches on page change', () => {
37-
(store.dispatch as jasmine.Spy).calls.reset();
80+
const dispatchSpy = store.dispatch as jest.Mock;
81+
dispatchSpy.mockClear();
82+
3883
fixture.componentInstance.onPageChange({ page: 2 } as any);
39-
expect(store.dispatch).toHaveBeenCalledWith(jasmine.any(GetRegistrationActivityLogs));
40-
const action = (store.dispatch as jasmine.Spy).calls.mostRecent().args[0] as GetRegistrationActivityLogs;
41-
expect(action.page).toBe('3');
84+
expect(dispatchSpy).toHaveBeenCalledWith(expect.any(GetRegistrationActivityLogs));
85+
86+
const action = dispatchSpy.mock.calls.at(-1)?.[0] as GetRegistrationActivityLogs;
87+
expect(action.page).toBe(3);
4288
});
4389

4490
it('clears store on destroy', () => {
45-
(store.dispatch as jasmine.Spy).calls.reset();
91+
const dispatchSpy = store.dispatch as jest.Mock;
92+
dispatchSpy.mockClear();
93+
4694
fixture.destroy();
47-
expect(store.dispatch).toHaveBeenCalledWith(jasmine.any(ClearActivityLogsStore));
95+
expect(dispatchSpy).toHaveBeenCalledWith(expect.any(ClearActivityLogsStore));
4896
});
4997
});

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

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,67 +3,60 @@ import { createDispatchMap, select } from '@ngxs/store';
33
import { TranslatePipe } from '@ngx-translate/core';
44

55
import { PaginatorState } from 'primeng/paginator';
6-
import { Skeleton } from 'primeng/skeleton';
76

87
import { DatePipe } from '@angular/common';
98
import { ChangeDetectionStrategy, Component, computed, inject, OnDestroy, signal } from '@angular/core';
109
import { ActivatedRoute } from '@angular/router';
1110

1211
import { CustomPaginatorComponent } from '@shared/components';
13-
import { ActivityLogDisplayService } from '@shared/services';
1412
import {
1513
ActivityLogsSelectors,
1614
ClearActivityLogsStore,
1715
GetRegistrationActivityLogs,
1816
} from '@shared/stores/activity-logs';
1917

18+
import { environment } from 'src/environments/environment';
19+
2020
@Component({
2121
selector: 'osf-registration-recent-activity',
22-
imports: [TranslatePipe, Skeleton, DatePipe, CustomPaginatorComponent],
22+
imports: [TranslatePipe, DatePipe, CustomPaginatorComponent],
2323
templateUrl: './registration-recent-activity.component.html',
2424
styleUrl: './registration-recent-activity.component.scss',
2525
changeDetection: ChangeDetectionStrategy.OnPush,
2626
})
2727
export class RegistrationRecentActivityComponent implements OnDestroy {
28-
private readonly activityLogDisplayService = inject(ActivityLogDisplayService);
2928
private readonly route = inject(ActivatedRoute);
3029

31-
readonly pageSize = 10;
30+
private static readonly DEFAULT_PAGE_SIZE = 10;
31+
readonly pageSize =
32+
(environment as unknown as { activityLogs?: { pageSize?: number } })?.activityLogs?.pageSize ??
33+
RegistrationRecentActivityComponent.DEFAULT_PAGE_SIZE;
34+
35+
private readonly registrationId: string = (this.route.snapshot.params['id'] ??
36+
this.route.parent?.snapshot.params['id']) as string;
3237

3338
protected currentPage = signal<number>(1);
34-
protected activityLogs = select(ActivityLogsSelectors.getActivityLogs);
39+
40+
protected formattedActivityLogs = select(ActivityLogsSelectors.getFormattedActivityLogs);
3541
protected totalCount = select(ActivityLogsSelectors.getActivityLogsTotalCount);
3642
protected isLoading = select(ActivityLogsSelectors.getActivityLogsLoading);
43+
3744
protected firstIndex = computed(() => (this.currentPage() - 1) * this.pageSize);
3845

3946
protected actions = createDispatchMap({
4047
getRegistrationActivityLogs: GetRegistrationActivityLogs,
4148
clearActivityLogsStore: ClearActivityLogsStore,
4249
});
4350

44-
protected formattedActivityLogs = computed(() => {
45-
const logs = this.activityLogs();
46-
return logs.map((log) => ({
47-
...log,
48-
formattedActivity: this.activityLogDisplayService.getActivityDisplay(log),
49-
}));
50-
});
51-
5251
constructor() {
53-
const registrationId = this.route.snapshot.params['id'] || this.route.parent?.snapshot.params['id'];
54-
if (registrationId) {
55-
this.actions.getRegistrationActivityLogs(registrationId, '1', String(this.pageSize));
56-
}
52+
this.actions.getRegistrationActivityLogs(this.registrationId, 1, this.pageSize);
5753
}
5854

5955
onPageChange(event: PaginatorState) {
6056
if (event.page !== undefined) {
6157
const pageNumber = event.page + 1;
6258
this.currentPage.set(pageNumber);
63-
const registrationId = this.route.snapshot.params['id'] || this.route.parent?.snapshot.params['id'];
64-
if (registrationId) {
65-
this.actions.getRegistrationActivityLogs(registrationId, String(pageNumber), String(this.pageSize));
66-
}
59+
this.actions.getRegistrationActivityLogs(this.registrationId, pageNumber, this.pageSize);
6760
}
6861
}
6962

src/app/features/registry/registry.routes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { provideStates } from '@ngxs/store';
2+
23
import { Routes } from '@angular/router';
34

45
import { viewOnlyGuard } from '@osf/core/guards';
6+
import { requireRegistrationIdGuard } from '@osf/core/guards/require-registration-id.guard';
57
import { ResourceType } from '@osf/shared/enums';
68
import { LicensesService } from '@osf/shared/services';
79
import {
@@ -115,6 +117,7 @@ export const registryRoutes: Routes = [
115117
},
116118
{
117119
path: 'recent-activity',
120+
canActivate: [requireRegistrationIdGuard],
118121
loadComponent: () =>
119122
import('./pages/recent-activity/registration-recent-activity.component').then(
120123
(c) => c.RegistrationRecentActivityComponent
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { HttpTestingController } from '@angular/common/http/testing';
2+
import { inject, TestBed } from '@angular/core/testing';
3+
4+
import { ActivityLogDisplayService } from '@shared/services';
5+
6+
import { ActivityLogsService } from './activity-logs.service';
7+
8+
import { getActivityLogsResponse } from '@testing/data/activity-logs/activity-logs.data';
9+
import { OSFTestingStoreModule } from '@testing/osf.testing.module';
10+
import { environment } from 'src/environments/environment';
11+
12+
describe('Service: ActivityLogs', () => {
13+
let service: ActivityLogsService;
14+
15+
beforeEach(() => {
16+
TestBed.configureTestingModule({
17+
imports: [OSFTestingStoreModule],
18+
providers: [
19+
ActivityLogsService,
20+
{ provide: ActivityLogDisplayService, useValue: { getActivityDisplay: jest.fn().mockReturnValue('FMT') } },
21+
],
22+
});
23+
service = TestBed.inject(ActivityLogsService);
24+
});
25+
26+
it('fetchRegistrationLogs maps + formats', inject([HttpTestingController], (httpMock: HttpTestingController) => {
27+
let result: any;
28+
service.fetchRegistrationLogs('reg1', 1, 10).subscribe((res) => (result = res));
29+
30+
const req = httpMock.expectOne(
31+
(r) => r.method === 'GET' && r.url === `${environment.apiUrl}/registrations/reg1/logs/`
32+
);
33+
expect(req.request.method).toBe('GET');
34+
expect(req.request.params.get('page')).toBe('1');
35+
expect(req.request.params.get('page[size]')).toBe('10');
36+
37+
req.flush(getActivityLogsResponse());
38+
39+
expect(result.totalCount).toBe(2);
40+
expect(result.data[0].formattedActivity).toBe('FMT');
41+
42+
httpMock.verify();
43+
}));
44+
45+
it('fetchLogs maps + formats', inject([HttpTestingController], (httpMock: HttpTestingController) => {
46+
let result: any;
47+
service.fetchLogs('proj1', 2, 5).subscribe((res) => (result = res));
48+
49+
const req = httpMock.expectOne((r) => r.method === 'GET' && r.url === `${environment.apiUrl}/nodes/proj1/logs/`);
50+
expect(req.request.method).toBe('GET');
51+
expect(req.request.params.get('page')).toBe('2');
52+
expect(req.request.params.get('page[size]')).toBe('5');
53+
54+
req.flush(getActivityLogsResponse());
55+
56+
expect(result.data.length).toBe(2);
57+
expect(result.data[1].formattedActivity).toBe('FMT');
58+
59+
httpMock.verify();
60+
}));
61+
});

0 commit comments

Comments
 (0)