diff --git a/src/app/account/verify-additional-email/verify-additional-email.component.html b/src/app/account/verify-additional-email/verify-additional-email.component.html new file mode 100644 index 0000000000..d3eb27cc8a --- /dev/null +++ b/src/app/account/verify-additional-email/verify-additional-email.component.html @@ -0,0 +1,23 @@ +
+ +

Additional notification email

+

{{ message }}

+ + @if (state === 'verified') { + + } @else if (state === 'error') { + + } +
diff --git a/src/app/account/verify-additional-email/verify-additional-email.component.scss b/src/app/account/verify-additional-email/verify-additional-email.component.scss new file mode 100644 index 0000000000..e53a48e581 --- /dev/null +++ b/src/app/account/verify-additional-email/verify-additional-email.component.scss @@ -0,0 +1,38 @@ +:host { + display: block; + min-height: 100dvh; + padding: clamp(16px, 5vw, 48px); +} + +.verification { + align-items: center; + border: 1px solid var(--ot-color-border); + border-radius: 16px; + display: flex; + flex-direction: column; + gap: 12px; + margin: min(15vh, 96px) auto 0; + max-width: 36rem; + padding: clamp(24px, 7vw, 48px); + text-align: center; + background: var(--ot-color-surface); + color: var(--ot-color-text); + + mat-icon { + color: var(--ot-color-link); + font-size: 48px; + height: 48px; + width: 48px; + } + + h1, + p { + margin: 0; + overflow-wrap: anywhere; + } + + button { + margin-top: 8px; + min-height: 44px; + } +} diff --git a/src/app/account/verify-additional-email/verify-additional-email.component.spec.ts b/src/app/account/verify-additional-email/verify-additional-email.component.spec.ts new file mode 100644 index 0000000000..8f1c36027e --- /dev/null +++ b/src/app/account/verify-additional-email/verify-additional-email.component.spec.ts @@ -0,0 +1,133 @@ +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatButtonModule} from '@angular/material/button'; +import {MatIconModule} from '@angular/material/icon'; +import {Router} from '@angular/router'; +import {Subject, of, throwError} from 'rxjs'; +import {AdditionalNotificationEmailService} from 'src/app/api/services/additional-notification-email.service'; +import {AuthenticationService} from 'src/app/api/services/authentication.service'; +import { + captureAndScrubAdditionalEmailVerification, + consumeAdditionalEmailVerificationToken, +} from 'src/app/security/additional-email-verification-callback'; +import {VerifyAdditionalEmailComponent} from './verify-additional-email.component'; + +describe('VerifyAdditionalEmailComponent', () => { + let fixture: ComponentFixture; + let component: VerifyAdditionalEmailComponent; + const service = {verify: vi.fn()}; + const authentication = {isAuthenticated: vi.fn()}; + const router = {navigateByUrl: vi.fn()}; + let token: string | null = 'private-token'; + + const create = async (): Promise => { + await TestBed.configureTestingModule({ + declarations: [VerifyAdditionalEmailComponent], + imports: [MatButtonModule, MatIconModule], + providers: [ + {provide: AdditionalNotificationEmailService, useValue: service}, + {provide: AuthenticationService, useValue: authentication}, + {provide: Router, useValue: router}, + ], + }).compileComponents(); + fixture = TestBed.createComponent(VerifyAdditionalEmailComponent); + component = fixture.componentInstance; + if (token) { + captureAndScrubAdditionalEmailVerification( + `https://ontrack.example/verify_additional_email#token=${token}`, + vi.fn(), + ); + } + fixture.detectChanges(); + }; + + const rendered = (selector: string): HTMLElement | null => + (fixture.nativeElement as HTMLElement).querySelector(selector); + + beforeEach(() => { + TestBed.resetTestingModule(); + vi.clearAllMocks(); + consumeAdditionalEmailVerificationToken(); + token = 'private-token'; + service.verify.mockReturnValue(of(undefined)); + authentication.isAuthenticated.mockReturnValue(false); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('consumes the pre-bootstrap token once and reports success', async () => { + await create(); + + expect(service.verify).toHaveBeenCalledWith('private-token'); + expect(component.state).toBe('verified'); + expect(consumeAdditionalEmailVerificationToken()).toBeNull(); + }); + + it('shows the success message and next step when the response arrives after first render', async () => { + const response: Subject = new Subject(); + service.verify.mockReturnValue(response.asObservable()); + await create(); + expect(rendered('p')?.textContent).toContain('Verifying'); + expect(rendered('button')).toBeNull(); + + response.next(); + response.complete(); + fixture.detectChanges(); + + expect(rendered('p')?.textContent).toContain('Your additional notification email is verified.'); + expect(rendered('button')?.textContent).toContain('Review notification settings'); + }); + + it('shows the error message and recovery step when a late response fails', async () => { + const response: Subject = new Subject(); + service.verify.mockReturnValue(response.asObservable()); + await create(); + + response.error(new Error('expired')); + fixture.detectChanges(); + + expect(rendered('p')?.textContent).toContain('invalid, expired, or has already been used'); + expect(rendered('button')?.textContent).toContain('Request a new verification link'); + }); + + it('reloads the profile for a signed-out verifier so startup can restore a saved session', async () => { + const assign = vi.fn(); + vi.stubGlobal('location', {assign}); + await create(); + + rendered('button')?.click(); + + expect(assign).toHaveBeenCalledWith('/edit_profile'); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + + it('opens the profile directly for an already authenticated user', async () => { + const assign = vi.fn(); + vi.stubGlobal('location', {assign}); + authentication.isAuthenticated.mockReturnValue(true); + await create(); + + rendered('button')?.click(); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/edit_profile'); + expect(assign).not.toHaveBeenCalled(); + }); + + it('does not call the API for an incomplete link', async () => { + token = null; + await create(); + + expect(service.verify).not.toHaveBeenCalled(); + expect(component.state).toBe('error'); + }); + + it('shows one safe error for expired or replayed links', async () => { + service.verify.mockReturnValue(throwError(() => new Error('expired'))); + await create(); + + expect(component.state).toBe('error'); + expect(component.message).toContain('invalid, expired, or has already been used'); + }); +}); diff --git a/src/app/account/verify-additional-email/verify-additional-email.component.ts b/src/app/account/verify-additional-email/verify-additional-email.component.ts new file mode 100644 index 0000000000..17c42bbb0d --- /dev/null +++ b/src/app/account/verify-additional-email/verify-additional-email.component.ts @@ -0,0 +1,61 @@ +import {ChangeDetectorRef, Component, OnInit} from '@angular/core'; +import {Router} from '@angular/router'; +import {AdditionalNotificationEmailService} from 'src/app/api/services/additional-notification-email.service'; +import {AuthenticationService} from 'src/app/api/services/authentication.service'; +import {consumeAdditionalEmailVerificationToken} from 'src/app/security/additional-email-verification-callback'; + +type VerificationState = 'verifying' | 'verified' | 'error'; + +@Component({ + selector: 'f-verify-additional-email', + templateUrl: './verify-additional-email.component.html', + styleUrl: './verify-additional-email.component.scss', + standalone: false, +}) +export class VerifyAdditionalEmailComponent implements OnInit { + public state: VerificationState = 'verifying'; + public message = 'Verifying your additional notification email…'; + + constructor( + private additionalEmailService: AdditionalNotificationEmailService, + private authentication: AuthenticationService, + private router: Router, + private changeDetector: ChangeDetectorRef, + ) {} + + public openProfile(): void { + if (this.authentication.isAuthenticated()) { + void this.router.navigateByUrl('/edit_profile'); + } else { + // Startup skips the refresh-token login on this route, so a saved session + // is never restored here. A full load of the profile page runs the normal + // startup, which restores that session or saves the profile as the return + // URL and sends the user to sign in. + window.location.assign('/edit_profile'); + } + } + + public ngOnInit(): void { + const token = consumeAdditionalEmailVerificationToken(); + if (!token) { + this.state = 'error'; + this.message = 'This verification link is incomplete.'; + return; + } + + // The component is OnPush by default, so a response that arrives after the + // first render has to mark the view or the page stays on "Verifying". + this.additionalEmailService.verify(token).subscribe({ + next: () => { + this.state = 'verified'; + this.message = 'Your additional notification email is verified.'; + this.changeDetector.markForCheck(); + }, + error: () => { + this.state = 'error'; + this.message = 'This verification link is invalid, expired, or has already been used.'; + this.changeDetector.markForCheck(); + }, + }); + } +} diff --git a/src/app/admin/institution-settings/campuses/campus-list/campus-list.component.html b/src/app/admin/institution-settings/campuses/campus-list/campus-list.component.html index 16bcd8b53b..db61b89d90 100644 --- a/src/app/admin/institution-settings/campuses/campus-list/campus-list.component.html +++ b/src/app/admin/institution-settings/campuses/campus-list/campus-list.component.html @@ -174,7 +174,7 @@

Campuses

-
@@ -183,6 +183,15 @@

Campuses

+ + + + + diff --git a/src/app/admin/institution-settings/overseer-images/overseer-image-list.component.scss b/src/app/admin/institution-settings/overseer-images/overseer-image-list.component.scss index a67ed67ef3..3423fd28ea 100644 --- a/src/app/admin/institution-settings/overseer-images/overseer-image-list.component.scss +++ b/src/app/admin/institution-settings/overseer-images/overseer-image-list.component.scss @@ -42,7 +42,7 @@ td.mat-column-options { width: 95%; } .status-icon { - background-color: white; + background-color: var(--ot-color-surface); border: none; margin-top: 8px; } diff --git a/src/app/admin/states/teaching-periods/teaching-period-list/teaching-period-list.component.html b/src/app/admin/states/teaching-periods/teaching-period-list/teaching-period-list.component.html index ccc2216d73..5c999a38bb 100644 --- a/src/app/admin/states/teaching-periods/teaching-period-list/teaching-period-list.component.html +++ b/src/app/admin/states/teaching-periods/teaching-period-list/teaching-period-list.component.html @@ -65,6 +65,15 @@

Teaching periods

+ + + + + Import Units Into {{ data.teachingPeriod.name }} + + + + +
diff --git a/src/app/admin/states/units/units.component.html b/src/app/admin/states/units/units.component.html index b7ef11e74c..19681c7b72 100644 --- a/src/app/admin/states/units/units.component.html +++ b/src/app/admin/states/units/units.component.html @@ -1,133 +1,183 @@ -
-
-
-
-

{{ title }}

-
-
- - Search - - search - -
+ +
+
+

+ {{ title }} +

+

+ {{ description }} +

- - - - - - + @if (mode === 'admin') { + + } + - - - - - +
+
+ + Search units + + + + @if (!loading || dataSource.data.length > 0) { +

{{ summary }}

+ } +
- - -
- - + @if (loading) { + + } - - - - - + @if (loadError) { + + } - - - - - + +
+
Unit Code - - Name{{ element.name }} - Unit Role - - {{ element.unit_role }} - Teaching Period - {{ element.teaching_period }} - Start Date{{ element.start_date | date: 'EEE d MMM y' }}
+ + + + - - - - - + + + + - - - - + + + + + + + + + + + + + + + + + + + + + - + + - - @if (mode === 'tutor') { + + - } - @if (mode === 'admin') { - - } - @if (mode === 'student') { - - } -
Code + + + {{ element.unit_code }} + + End Date{{ element.end_date | date: 'EEE d MMM y' }}Name{{ element.name }}Active - - @if (element.teachingPeriod) { - @if (element.teachingPeriod.active && element.active) { - done + + Role + @if (element.unit_role) { + {{ element.unit_role }} + } @else { + None } - @if (!element.teachingPeriod.active || !element.active) { - close + Teaching period + @if (element.teaching_period) { + {{ element.teaching_period }} + } @else { + Custom dates } - } @else { + Starts + {{ element.start_date | date: 'd MMM y' }} + Ends + {{ element.end_date | date: 'd MMM y' }} + Status @if (element.active) { - done + + + Active + + } @else { + + + Inactive + } - @if (!element.active) { - close - } - } - - -
- - - - @if (mode === 'admin') { - - } - -
-
+ + + @if (loading) { + + } @else if (isFiltered) { + + } @else { + + } + + + +
+ + + + diff --git a/src/app/admin/states/units/units.component.scss b/src/app/admin/states/units/units.component.scss index bb619a90b4..1604d01b25 100644 --- a/src/app/admin/states/units/units.component.scss +++ b/src/app/admin/states/units/units.component.scss @@ -1,8 +1,23 @@ -.icon_display { - transform: scale(2); +// The search field runs at 48px, like the toolbar fields on the student My units page, +// so it sits level with the summary beside it instead of towering over it. +.units-search { + --mat-form-field-container-height: 48px; + --mat-form-field-container-vertical-padding: 12px; } -.mat-column-unit_code { - max-width: 130px !important; - min-width: 130px !important; +// A calm error note inside the card, in the error tone at low strength. +.units-callout { + display: flex; + align-items: flex-start; + gap: 0.75rem; + padding: 0.75rem 1rem; + border: 1px solid color-mix(in srgb, var(--ot-color-error) 35%, var(--ot-color-divider)); + border-radius: var(--ot-radius-md); + background: color-mix(in srgb, var(--ot-color-error) 8%, var(--ot-color-surface)); + color: var(--ot-color-text); + + > .mat-icon { + flex: none; + color: var(--ot-color-error); + } } diff --git a/src/app/admin/states/units/units.component.spec.ts b/src/app/admin/states/units/units.component.spec.ts index b5ca421a3a..9d7cdd1b98 100644 --- a/src/app/admin/states/units/units.component.spec.ts +++ b/src/app/admin/states/units/units.component.spec.ts @@ -1,39 +1,241 @@ -import {beforeEach, describe, expect, it} from 'vitest'; +import {EntityCache} from 'ngx-entity-service'; +import {beforeEach, describe, expect, it, vi} from 'vitest'; import {NO_ERRORS_SCHEMA} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; -import {ActivatedRoute} from '@angular/router'; +import {MatSortModule} from '@angular/material/sort'; +import {MatTableModule} from '@angular/material/table'; +import {ActivatedRoute, RouterModule, provideRouter} from '@angular/router'; +import {BehaviorSubject, Subject, of, throwError} from 'rxjs'; +import {Project} from 'src/app/api/models/project'; +import {TeachingPeriod} from 'src/app/api/models/teaching-period'; +import {Unit} from 'src/app/api/models/unit'; +import {UnitRole} from 'src/app/api/models/unit-role'; +import {ProjectService} from 'src/app/api/services/project.service'; import {UnitService} from 'src/app/api/services/unit.service'; +import {EmptyStateComponent} from 'src/app/common/empty-state/empty-state.component'; import {GlobalStateService} from 'src/app/projects/states/index/global-state.service'; import {CreateNewUnitModal} from '../../modals/create-new-unit-modal/create-new-unit-modal.component'; import {FUnitsComponent} from './units.component'; -const emptyProvider = {}; +function unit(id: number, code: string, extra: Partial = {}): Unit { + return Object.assign(new Unit(), { + id, + code, + name: `${code} name`, + active: true, + startDate: new Date(2026, 6, 6), + endDate: new Date(2026, 9, 30), + ...extra, + }); +} + +function roleIn(id: number, taught: Unit, role = 'Tutor'): UnitRole { + return Object.assign(new UnitRole(), {id, role, unit: taught}); +} + +function projectIn(id: number, studied: Unit): Project { + return Object.assign(new Project(studied), {id}); +} describe('FUnitsComponent', () => { - let component: FUnitsComponent; let fixture: ComponentFixture; + let component: FUnitsComponent; + let unitRoles$: BehaviorSubject; + let units$: BehaviorSubject; + let projects$: BehaviorSubject; + let unitService: {query: ReturnType}; + let projectService: {query: ReturnType}; + let allProjects: Project[]; - beforeEach(async () => { + async function create(mode: 'admin' | 'tutor' | 'student') { await TestBed.configureTestingModule({ declarations: [FUnitsComponent], + imports: [EmptyStateComponent, MatSortModule, MatTableModule, RouterModule], providers: [ - {provide: CreateNewUnitModal, useValue: emptyProvider}, - {provide: GlobalStateService, useValue: emptyProvider}, - {provide: UnitService, useValue: emptyProvider}, - {provide: ActivatedRoute, useValue: emptyProvider}, + provideRouter([]), + {provide: CreateNewUnitModal, useValue: {show: vi.fn()}}, + { + provide: GlobalStateService, + useValue: { + onLoad: (run: () => void) => run(), + loadedUnitRoles: {values: unitRoles$}, + loadedUnits: {values: units$}, + currentUserProjects: {values: projects$}, + }, + }, + {provide: UnitService, useValue: unitService}, + {provide: ProjectService, useValue: projectService}, + {provide: ActivatedRoute, useValue: {snapshot: {data: {mode}}}}, ], schemas: [NO_ERRORS_SCHEMA], - }) - .overrideComponent(FUnitsComponent, {set: {template: ''}}) - .compileComponents(); - }); + }).compileComponents(); - beforeEach(() => { fixture = TestBed.createComponent(FUnitsComponent); component = fixture.componentInstance; + fixture.detectChanges(); + } + + function page(): HTMLElement { + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + function rows(): HTMLElement[] { + return Array.from(page().querySelectorAll('tr.mat-mdc-row')); + } + + beforeEach(() => { + unitRoles$ = new BehaviorSubject([]); + units$ = new BehaviorSubject([]); + projects$ = new BehaviorSubject([]); + allProjects = []; + unitService = {query: vi.fn(() => of([]))}; + projectService = { + // Stands in for the entity service, which fills the cache it is handed. + query: vi.fn((_ids: unknown, options: {cache: EntityCache}) => { + allProjects.forEach((project) => options.cache.add(project)); + return of(allProjects); + }), + }; }); - it('should create', () => { - expect(component).toBeTruthy(); + describe('as a tutor', () => { + beforeEach(async () => { + await create('tutor'); + }); + + it('titles the page for a tutor and links each unit code to its inbox', () => { + unitRoles$.next([roleIn(11, unit(1, 'SIT374'), 'Convenor')]); + + expect(page().querySelector('h1')?.textContent).toContain('Units you teach'); + const [row] = rows(); + const link = row.querySelector('a'); + expect(link?.textContent).toContain('SIT374'); + expect(link?.getAttribute('href')).toBe('/units/1/tasks/inbox'); + // The row stays clickable but out of the tab order; the link is the keyboard way in. + expect(row.getAttribute('tabindex')).toBe('-1'); + expect(row.textContent).toContain('Convenor'); + }); + + // The Active column tested element.teachingPeriod, a field the rows never had, so a + // unit whose teaching period had ended still showed as active. + it('shows a unit whose teaching period is over as inactive', () => { + const over = Object.assign(new TeachingPeriod(), {period: 'T1', year: '2026', active: false}); + unitRoles$.next([ + roleIn(11, unit(1, 'SIT374', {teachingPeriod: over})), + roleIn(12, unit(2, 'SIT111')), + ]); + + const [first, second] = rows(); + expect(first.textContent).toContain('Inactive'); + expect(second.textContent).toContain('Active'); + expect(second.textContent).not.toContain('Inactive'); + expect(page().textContent).toContain('2 units · 1 active'); + }); + + // Each visit left a live subscription to the unit role cache behind, still writing + // into the destroyed page's table. + it('stops listening to the unit roles once the page is gone', () => { + unitRoles$.next([roleIn(11, unit(1, 'SIT374'))]); + fixture.destroy(); + + unitRoles$.next([roleIn(11, unit(1, 'SIT374')), roleIn(12, unit(2, 'SIT111'))]); + + expect(component.dataSource.data.length).toBe(1); + }); + + it('tells a search with no match apart from an empty list', () => { + unitRoles$.next([roleIn(11, unit(1, 'SIT374'))]); + const input = page().querySelector('input') as HTMLInputElement; + + input.value = 'zzz'; + input.dispatchEvent(new Event('input')); + + expect(page().textContent).toContain('No units match your search'); + + input.value = '374'; + input.dispatchEvent(new Event('input')); + expect(rows().length).toBe(1); + }); + + it('sorts a missing date without breaking and text without regard to case', () => { + const [row] = component.mapUnitOrProjectsToColumns([ + roleIn(11, unit(1, 'sit374', {startDate: undefined})), + ]); + + expect(component.sortValue(row, 'start_date')).toBe(0); + expect(component.sortValue(row, 'unit_code')).toBe('sit374'); + expect(component.sortValue({...row, unit_code: 'SIT374'}, 'unit_code')).toBe('sit374'); + }); + }); + + describe('as a student', () => { + // The page is reached from "View previous", but it only listed the active units the + // global state had loaded, so a previous unit never appeared on it. + it('lists earlier units as well as current ones', async () => { + const current = unit(1, 'SIT374'); + const earlier = unit(2, 'SIT111', {active: false}); + projects$.next([projectIn(21, current)]); + allProjects = [projectIn(21, current), projectIn(22, earlier)]; + + await create('student'); + + expect(projectService.query).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ + params: {include_inactive: true, include_task_definitions: true}, + }), + ); + const listed = rows(); + expect(listed.length).toBe(2); + expect(listed.map((row) => row.querySelector('a')?.getAttribute('href'))).toEqual( + expect.arrayContaining(['/projects/21/dashboard', '/projects/22/dashboard']), + ); + expect(page().querySelector('th')?.textContent).not.toContain('Role'); + }); + + // A student's own project gets its user later, so searching before then called + // matches() on an undefined student and threw out of the table filter. + it('searches a project that has no student yet without throwing', async () => { + allProjects = [projectIn(21, unit(1, 'SIT374'))]; + await create('student'); + + const [row] = component.dataSource.data; + expect(() => row.matches('nothing like this')).not.toThrow(); + expect(row.matches('374')).toBe(true); + }); + + it('keeps the current units and offers a retry when the full list fails', async () => { + projects$.next([projectIn(21, unit(1, 'SIT374'))]); + projectService.query.mockReturnValueOnce(throwError(() => new Error('offline'))); + + await create('student'); + + expect(rows().length).toBe(1); + const alert = page().querySelector('[role="alert"]'); + expect(alert?.textContent).toContain('Your earlier units could not be loaded'); + + (alert?.querySelector('button') as HTMLButtonElement).click(); + expect(projectService.query).toHaveBeenCalledTimes(2); + expect(page().querySelector('[role="alert"]')).toBeNull(); + }); + }); + + describe('as an admin', () => { + it('says it is loading, not that there are no units, while they are fetched', async () => { + unitService.query.mockReturnValue(new Subject()); + await create('admin'); + + expect(page().textContent).toContain('Loading units...'); + expect(page().textContent).not.toContain('No units yet'); + }); + + it('offers the create button and routes rows to the unit admin page', async () => { + units$.next([unit(1, 'SIT374')]); + await create('admin'); + + expect(page().querySelector('header button')?.textContent).toContain('Create unit'); + expect(rows()[0].querySelector('a')?.getAttribute('href')).toBe('/units/1/admin'); + }); }); }); diff --git a/src/app/admin/states/units/units.component.ts b/src/app/admin/states/units/units.component.ts index 86f03ab90d..4c58ecb094 100644 --- a/src/app/admin/states/units/units.component.ts +++ b/src/app/admin/states/units/units.component.ts @@ -1,19 +1,24 @@ +import {EntityCache} from 'ngx-entity-service'; import { AfterViewInit, ChangeDetectionStrategy, Component, + DestroyRef, Input, OnInit, ViewChild, + inject, } from '@angular/core'; +import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; import {MatPaginator} from '@angular/material/paginator'; -import {MatSort, Sort} from '@angular/material/sort'; +import {MatSort} from '@angular/material/sort'; import {MatTable, MatTableDataSource} from '@angular/material/table'; import {ActivatedRoute} from '@angular/router'; import {Project} from 'src/app/api/models/project'; import {Unit} from 'src/app/api/models/unit'; import {UnitRole} from 'src/app/api/models/unit-role'; import {User} from 'src/app/api/models/user/user'; +import {ProjectService} from 'src/app/api/services/project.service'; import {UnitService} from 'src/app/api/services/unit.service'; import {GlobalStateService} from 'src/app/projects/states/index/global-state.service'; import {CreateNewUnitModal} from '../../modals/create-new-unit-modal/create-new-unit-modal.component'; @@ -27,6 +32,8 @@ interface IUnitOrProject { teaching_period: string; start_date: Date; end_date: Date; + // Whether the unit is running now: its own active flag, and its teaching period if it + // has one. The Active column always meant this, but it read a field the rows never had. active: boolean; user?: User; unit?: Unit; @@ -36,6 +43,26 @@ interface IUnitOrProject { matches: (filter: string) => boolean; } +type UnitsMode = 'admin' | 'tutor' | 'student'; + +const PAGE_COPY: Record = { + tutor: { + title: 'Units you teach', + description: 'Every unit you have taught, including earlier teaching periods.', + noun: 'units you teach', + }, + admin: { + title: 'Units', + description: 'Every unit, active or not. Open one to manage it.', + noun: 'units', + }, + student: { + title: 'Your units', + description: 'Every unit you have studied, including earlier teaching periods.', + noun: 'units', + }, +}; + @Component({ selector: 'f-units', templateUrl: './units.component.html', @@ -48,7 +75,7 @@ export class FUnitsComponent implements OnInit, AfterViewInit { @ViewChild(MatSort, {static: false}) sort: MatSort; @ViewChild(MatPaginator, {static: false}) paginator: MatPaginator; - @Input({required: true}) mode: 'admin' | 'tutor' | 'student'; + @Input({required: true}) mode: UnitsMode; displayedColumns: string[] = [ 'unit_code', @@ -64,6 +91,20 @@ export class FUnitsComponent implements OnInit, AfterViewInit { dataSource: MatTableDataSource = new MatTableDataSource([]); title: string; + description: string; + + /** True until the first list of units arrives. */ + loading = true; + /** Set when the request behind this list fails, so the page can offer to retry. */ + loadError = false; + + filterText = ''; + + private readonly destroyRef = inject(DestroyRef); + + // A student's own project list in the global state only holds active units, so the + // full history is fetched into a cache of its own, the way the dashboard does it. + private readonly allProjectsCache: EntityCache = new EntityCache(); shouldShowUnitRoleColumn(): boolean { return this.mode === 'admin' || this.mode === 'tutor'; @@ -73,6 +114,7 @@ export class FUnitsComponent implements OnInit, AfterViewInit { private createUnitDialog: CreateNewUnitModal, private globalStateService: GlobalStateService, private unitService: UnitService, + private projectService: ProjectService, private route: ActivatedRoute, ) {} @@ -80,45 +122,84 @@ export class FUnitsComponent implements OnInit, AfterViewInit { ngOnInit(): void { this.mode = this.mode ?? this.route.snapshot.data.mode; - if (this.mode === 'tutor') { - this.title = 'View all units you teach'; + const copy = PAGE_COPY[this.mode] ?? PAGE_COPY.student; + this.title = copy.title; + this.description = copy.description; + if (!this.shouldShowUnitRoleColumn()) { + this.displayedColumns = this.displayedColumns.filter((column) => column !== 'unit_role'); + } + + if (this.mode === 'tutor') { this.globalStateService.onLoad(() => { - this.globalStateService.loadedUnitRoles.values.subscribe({ - next: (unitRoles) => { - this.dataSource.data = this.mapUnitOrProjectsToColumns(unitRoles); - }, - }); + this.globalStateService.loadedUnitRoles.values + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe({ + next: (unitRoles) => this.showRows(unitRoles), + }); }); } if (this.mode === 'admin') { - this.title = 'Administer units'; - this.globalStateService.onLoad(() => { - this.unitService.query(undefined, {params: {include_in_active: true}}).subscribe({ - next: () => { - this.globalStateService.loadedUnits.values.subscribe( - (loadedUnits) => - (this.dataSource.data = this.mapUnitOrProjectsToColumns(loadedUnits)), - ); - }, - }); - - this.globalStateService.loadedUnits.values.subscribe( - (units) => (this.dataSource.data = this.mapUnitOrProjectsToColumns(units)), - ); + this.globalStateService.loadedUnits.values + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((units) => this.showRows(units, false)); + this.loadAllUnits(); }); } else if (this.mode === 'student') { - this.title = 'View all your units'; - this.globalStateService.onLoad(() => { - this.globalStateService.currentUserProjects.values.subscribe( - (projects) => (this.dataSource.data = this.mapUnitOrProjectsToColumns(projects)), - ); + this.globalStateService.currentUserProjects.values + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((projects) => { + // Show the active units straight away, until the full history arrives. + if (this.allProjectsCache.size === 0) { + this.showRows(projects, false); + } + }); + this.allProjectsCache.values + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((projects) => { + if (this.allProjectsCache.size > 0) { + this.showRows(projects); + } + }); + this.loadAllProjects(); }); } } + get noun(): string { + return (PAGE_COPY[this.mode] ?? PAGE_COPY.student).noun; + } + + get summary(): string { + const rows = this.dataSource.data; + const active = rows.filter((row) => row.active).length; + return `${rows.length} ${rows.length === 1 ? 'unit' : 'units'} · ${active} active`; + } + + get isFiltered(): boolean { + return this.filterText.trim().length > 0; + } + + retry(): void { + if (this.mode === 'admin') { + this.loadAllUnits(); + } else if (this.mode === 'student') { + this.loadAllProjects(); + } + } + + routeFor(row: IUnitOrProject): (string | number)[] { + if (this.mode === 'admin') { + return ['/units', row.id, 'admin']; + } + if (this.mode === 'student') { + return ['/projects', row.id, 'dashboard']; + } + return ['/units', row.id, 'tasks', 'inbox']; + } + mapUnitSourceToColumn(unitOrProject: Unit | Project | UnitRole): IUnitOrProject { if (unitOrProject instanceof Unit) { return { @@ -130,8 +211,8 @@ export class FUnitsComponent implements OnInit, AfterViewInit { teaching_period: unitOrProject.teachingPeriod?.name || 'Custom', start_date: unitOrProject.startDate, end_date: unitOrProject.endDate, - active: unitOrProject.active, - matches: unitOrProject.matches, + active: unitOrProject.isActive, + matches: (filter: string) => unitOrProject.matches(filter), }; } else if (unitOrProject instanceof Project) { return { @@ -142,14 +223,16 @@ export class FUnitsComponent implements OnInit, AfterViewInit { teaching_period: unitOrProject.unit.teachingPeriod?.name, start_date: unitOrProject.unit.startDate, end_date: unitOrProject.unit.endDate, - active: unitOrProject.unit.active, + active: unitOrProject.unit.isActive, student: unitOrProject.student, matchesTutorialEnrolments: unitOrProject.matchesTutorialEnrolments, matchesGroup: unitOrProject.matchesGroup, matches: (filter: string) => { + // A student's own project carries a user id rather than a student, and the + // user is filled in later, so it may not be there yet when someone searches. return ( unitOrProject.unit.matches(filter) || - unitOrProject.student.matches(filter) || + !!unitOrProject.student?.matches(filter) || unitOrProject.matchesTutorialEnrolments(filter) || unitOrProject.matchesGroup(filter) ); @@ -165,23 +248,27 @@ export class FUnitsComponent implements OnInit, AfterViewInit { teaching_period: unitOrProject.unit.teachingPeriod?.name, start_date: unitOrProject.unit.startDate, end_date: unitOrProject.unit.endDate, - active: unitOrProject.unit.active, + active: unitOrProject.unit.isActive, user: unitOrProject.user, unit: unitOrProject.unit, - matches: unitOrProject.matches, + matches: (filter: string) => + unitOrProject.unit.matches(filter) || !!unitOrProject.user?.matches(filter), }; } } mapUnitOrProjectsToColumns(unitOrProjects: readonly (Unit | Project | UnitRole)[]) { - // copy the array of units/projects/unitRole and map each unit through the mapUnitSourceToColumn function - return [...unitOrProjects].map((unitOrProject) => this.mapUnitSourceToColumn(unitOrProject)); + // Skip anything that has not been mapped far enough to have a unit yet. + return [...unitOrProjects] + .filter((source) => (source instanceof Unit ? source.code : source?.unit?.code)) + .map((unitOrProject) => this.mapUnitSourceToColumn(unitOrProject)); } ngAfterViewInit(): void { this.dataSource.paginator = this.paginator; this.dataSource.sort = this.sort; this.dataSource.filterPredicate = (data, filter: string) => data.matches(filter); + this.dataSource.sortingDataAccessor = (data, column) => this.sortValue(data, column); } createUnit() { @@ -189,51 +276,66 @@ export class FUnitsComponent implements OnInit, AfterViewInit { } applyFilter(event: Event) { - const filterValue = (event.target as HTMLInputElement).value; - this.dataSource.filter = filterValue.trim().toLowerCase(); + this.filterText = (event.target as HTMLInputElement).value ?? ''; + this.dataSource.filter = this.filterText.trim().toLowerCase(); if (this.dataSource.paginator) { this.dataSource.paginator.firstPage(); } } - private sortCompare(aValue: number | string, bValue: number | string, isAsc: boolean) { - return (aValue < bValue ? -1 : 1) * (isAsc ? 1 : -1); + /** + * The value the table sorts a column on. Text sorts without regard to case, dates by + * time, and a missing value sorts as empty rather than breaking the comparison. + */ + sortValue(data: IUnitOrProject, column: string): string | number { + switch (column) { + case 'start_date': + case 'end_date': { + const date = data[column]; + return date instanceof Date ? date.getTime() : 0; + } + case 'active': + return data.active ? 1 : 0; + default: { + const value = data[column as keyof IUnitOrProject]; + return typeof value === 'string' ? value.toLowerCase() : ''; + } + } } - sortTableData(sort: Sort) { - if (!sort.active || sort.direction === '') { - return; + private showRows(sources: readonly (Unit | Project | UnitRole)[], finishedLoading = true) { + this.dataSource.data = this.mapUnitOrProjectsToColumns(sources ?? []); + if (finishedLoading) { + this.loading = false; } - this.dataSource.data = this.dataSource.data.sort((a, b) => { - switch (sort.active) { - case 'unit_code': - return this.sortCompare(a.unit_code, b.unit_code, sort.direction === 'asc'); - case 'name': - return this.sortCompare(a.name, b.name, sort.direction === 'asc'); - case 'unit_role': - return this.sortCompare(a.unit_role, b.unit_role, sort.direction === 'asc'); - case 'teaching_period': { - return this.sortCompare(a.teaching_period, b.teaching_period, sort.direction === 'asc'); - } - case 'start_date': { - return this.sortCompare( - a.start_date.getTime(), - b.start_date.getTime(), - sort.direction === 'asc', - ); - } - case 'end_date': { - return this.sortCompare( - a.end_date.getTime(), - b.end_date.getTime(), - sort.direction === 'asc', - ); - } - case 'active': - return this.sortCompare(+!!a.active, +!!b.active, sort.direction === 'asc'); - default: - return 0; - } + } + + private loadAllUnits(): void { + this.loading = true; + this.loadError = false; + this.unitService.query(undefined, {params: {include_in_active: true}}).subscribe({ + next: () => (this.loading = false), + error: () => { + this.loading = false; + this.loadError = true; + }, }); } + + private loadAllProjects(): void { + this.loading = true; + this.loadError = false; + this.projectService + .query(undefined, { + cache: this.allProjectsCache, + params: {include_inactive: true, include_task_definitions: true}, + }) + .subscribe({ + next: () => (this.loading = false), + error: () => { + this.loading = false; + this.loadError = true; + }, + }); + } } diff --git a/src/app/admin/states/users/users.component.spec.ts b/src/app/admin/states/users/users.component.spec.ts new file mode 100644 index 0000000000..a0c847418f --- /dev/null +++ b/src/app/admin/states/users/users.component.spec.ts @@ -0,0 +1,81 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {of, throwError} from 'rxjs'; +import {FUsersComponent} from './users.component'; + +describe('FUsersComponent CSV import result', () => { + let component: FUsersComponent; + + let userService: { + fetchAll: ReturnType; + }; + + let alerts: { + success: ReturnType; + error: ReturnType; + }; + + beforeEach(() => { + userService = { + fetchAll: vi.fn(() => of([])), + }; + + alerts = { + success: vi.fn(), + error: vi.fn(), + }; + + component = new FUsersComponent( + userService as unknown as ConstructorParameters[0], + {} as ConstructorParameters[1], + {} as ConstructorParameters[2], + {} as ConstructorParameters[3], + alerts as unknown as ConstructorParameters[4], + ); + }); + + it('shows a success alert and refreshes the table when the CSV import has no errors', () => { + component['onUserUploadSuccess']({ + body: { + errors: [], + success: Array(50).fill({}), + ignored: [], + }, + }); + + expect(alerts.success).toHaveBeenCalledWith( + '50 users successfully updated, 0 users ignored, 0 users contained an error in the CSV...', + ); + expect(alerts.error).not.toHaveBeenCalled(); + expect(userService.fetchAll).toHaveBeenCalledOnce(); + }); + + it('shows an error alert when the CSV import contains errors', () => { + component['onUserUploadSuccess']({ + body: { + errors: [{message: 'Invalid user'}], + success: [], + ignored: [], + }, + }); + + expect(alerts.error).toHaveBeenCalledWith( + '0 users successfully updated, 0 users ignored, 1 users contained an error in the CSV...Invalid user\n', + ); + expect(alerts.success).not.toHaveBeenCalled(); + expect(userService.fetchAll).toHaveBeenCalledOnce(); + }); + + it('surfaces an error alert when the refresh request fails', () => { + userService.fetchAll = vi.fn(() => throwError(() => 'refresh failed')); + + component['onUserUploadSuccess']({ + body: { + errors: [], + success: Array(1).fill({}), + ignored: [], + }, + }); + + expect(alerts.error).toHaveBeenCalledWith('refresh failed'); + }); +}); diff --git a/src/app/api/models/model-guards.spec.ts b/src/app/api/models/model-guards.spec.ts new file mode 100644 index 0000000000..0a3608906e --- /dev/null +++ b/src/app/api/models/model-guards.spec.ts @@ -0,0 +1,72 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {Observable, throwError} from 'rxjs'; +import {ProjectService} from 'src/app/api/services/project.service'; +import {AlertService} from 'src/app/common/services/alert.service'; +import {provideAppInjectorForTests} from 'src/app/testing/app-injector-stub'; +import {Project} from './project'; +import {Tutorial} from './tutorial/tutorial'; +import {Unit} from './unit'; + +describe('Project.isEnrolledIn', () => { + it('is false for a group left without a tutorial, instead of throwing', () => { + const project = new Project(new Unit()); + + expect(project.isEnrolledIn(undefined)).toBe(false); + }); +}); + +describe('Tutorial.description', () => { + it('still describes a tutorial saved without a meeting day', () => { + const tutorial = new Tutorial(new Unit()); + tutorial.meetingTime = '10:00'; + tutorial.meetingLocation = 'Room 4'; + + expect(tutorial.description).toBe('No day set at 10:00 by in Room 4'); + }); +}); + +// The models look their services up through AppInjector. Each test registers its +// stand-ins with the shared spec stub. +const projectService: {loadStudents?: unknown; update?: unknown} = {}; +const alerts = {error: vi.fn(), success: vi.fn()}; + +beforeEach(() => { + provideAppInjectorForTests([ + [ProjectService, projectService], + [AlertService, alerts], + ]); +}); + +describe('Unit.refreshStudents', () => { + it('sends the request, which it used to build and never subscribe to', () => { + let subscribed = false; + const loadStudents = vi.fn( + () => + new Observable(() => { + subscribed = true; + }), + ); + projectService.loadStudents = loadStudents; + + const unit = new Unit(); + unit.refreshStudents(true); + + expect(loadStudents).toHaveBeenCalledWith(unit, true, true); + expect(subscribed).toBe(true); + }); +}); + +describe('Project.assignGrade', () => { + it('puts back the old rationale as well as the old grade when the save fails', () => { + projectService.update = vi.fn(() => throwError(() => 'no connection')); + const project = new Project(new Unit()); + project.grade = 70; + project.gradeRationale = 'Met every distinction criterion.'; + + project.assignGrade(80, 'Now meets the high distinction criteria.'); + + expect(project.grade).toBe(70); + expect(project.gradeRationale).toBe('Met every distinction criterion.'); + expect(alerts.error).toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/models/task-prerequisite.ts b/src/app/api/models/task-prerequisite.ts index 4e26df0917..8e7a208c1b 100644 --- a/src/app/api/models/task-prerequisite.ts +++ b/src/app/api/models/task-prerequisite.ts @@ -83,7 +83,7 @@ export class TaskPrerequisite extends Entity { .pipe( tap({ next: () => { - AppInjector.get(AlertService).error('Successfully deleted prerequisite', 4000); + AppInjector.get(AlertService).success('Prerequisite removed', 4000); this.taskDefinition.taskPrerequisitesCache.delete(this.id); }, error: (error) => { diff --git a/src/app/api/models/task.spec.ts b/src/app/api/models/task.spec.ts new file mode 100644 index 0000000000..1af6067bc7 --- /dev/null +++ b/src/app/api/models/task.spec.ts @@ -0,0 +1,157 @@ +import {beforeEach, describe, expect, it} from 'vitest'; +import {HttpClient} from '@angular/common/http'; +import {of} from 'rxjs'; +import {DoubtfireConstants} from 'src/app/config/constants/doubtfire-constants'; +import {provideAppInjectorForTests} from 'src/app/testing/app-injector-stub'; +import {Project} from './project'; +import {SubmissionProcessingResponse, Task} from './task'; +import {TaskDefinition} from './task-definition'; + +function taskWithUploads(uploads: number = 1): Task { + const task = new Task(); + task.definition = { + id: 2, + uploadRequirements: Array.from({length: uploads}, (_, i) => ({key: `file${i}`})), + } as unknown as TaskDefinition; + return task; +} + +describe('Task submission history', () => { + it('distinguishes a first submission from current and returned submission states', () => { + const task = taskWithUploads(); + task.status = 'not_started'; + expect(task.hasSubmissionHistory()).toBe(false); + + task.status = 'ready_for_feedback'; + expect(task.hasSubmissionHistory()).toBe(true); + + task.status = 'redo'; + expect(task.hasSubmissionHistory()).toBe(true); + }); + + it('retains history while status changes when a timestamp or artifact exists', () => { + const task = taskWithUploads(); + task.status = 'working_on_it'; + + task.submissionDate = new Date('2026-08-31T00:00:00Z'); + expect(task.hasSubmissionHistory()).toBe(true); + + task.submissionDate = undefined; + task.hasPdf = true; + expect(task.hasSubmissionHistory()).toBe(true); + + task.hasPdf = false; + task.processingPdf = true; + expect(task.hasSubmissionHistory()).toBe(true); + + task.processingPdf = false; + task.submissionProcessingState = 'failed'; + expect(task.hasSubmissionHistory()).toBe(true); + }); + + it('does not treat an invalid submission date as history', () => { + const task = taskWithUploads(); + task.status = 'working_on_it'; + task.submissionDate = new Date(Number.NaN); + + expect(task.hasSubmissionHistory()).toBe(false); + }); + + it.each(['complete', 'ready_for_feedback', 'redo'] as const)( + 'has no submission history in %s when the task takes no uploads', + (status) => { + const task = taskWithUploads(0); + task.status = status; + task.submissionDate = new Date('2026-08-31T00:00:00Z'); + + expect(task.hasSubmissionHistory()).toBe(false); + }, + ); +}); + +describe('Task submission details', () => { + let response: SubmissionProcessingResponse; + + beforeEach(() => { + provideAppInjectorForTests([ + [HttpClient, {get: () => of(response)}], + [DoubtfireConstants, {API_URL: 'http://localhost:3000/api'}], + ]); + }); + + function loadDetails(task: Task): Task { + let loaded: Task; + task.getSubmissionDetails().subscribe((result) => { + loaded = result; + }); + return loaded; + } + + function unsubmittedTask(): Task { + const task = taskWithUploads(); + const project = new Project(); + project.id = 1; + task.project = project; + task.status = 'not_started'; + return task; + } + + it.each([ + { + shape: 'an existing task that was never submitted', + details: { + has_pdf: false, + pdf_ready: false, + submission_files_ready: false, + processing_pdf: false, + processing_state: 'not_submitted', + submission_date: null, + task_status: 'not_started', + } as SubmissionProcessingResponse, + }, + { + shape: 'a task with no submission date key', + details: { + has_pdf: false, + pdf_ready: false, + submission_files_ready: false, + processing_pdf: false, + processing_state: 'not_submitted', + } as SubmissionProcessingResponse, + }, + ])('keeps no submission history after loading details for $shape', ({details}) => { + const task = unsubmittedTask(); + expect(task.hasSubmissionHistory()).toBe(false); + + response = details; + loadDetails(task); + + expect(task.hasSubmissionHistory()).toBe(false); + expect(task.submissionDate).toBeUndefined(); + }); + + it('maps a real submission date from the details response', () => { + const task = unsubmittedTask(); + task.status = 'working_on_it'; + + response = { + processing_state: 'ready', + has_pdf: true, + submission_date: '2026-08-31T00:00:00.000Z', + }; + loadDetails(task); + + expect(task.submissionDate?.toISOString()).toBe('2026-08-31T00:00:00.000Z'); + expect(task.hasSubmissionHistory()).toBe(true); + }); + + it('keeps the loaded submission date when the details response leaves the key out', () => { + const task = unsubmittedTask(); + task.submissionDate = new Date('2026-08-31T00:00:00.000Z'); + + response = {processing_state: 'not_submitted'}; + loadDetails(task); + + expect(task.submissionDate?.toISOString()).toBe('2026-08-31T00:00:00.000Z'); + }); +}); diff --git a/src/app/api/models/tutorial/tutorial.ts b/src/app/api/models/tutorial/tutorial.ts index 164b8df832..7955cbcbbf 100644 --- a/src/app/api/models/tutorial/tutorial.ts +++ b/src/app/api/models/tutorial/tutorial.ts @@ -58,7 +58,9 @@ export class Tutorial extends Entity { campusPart = ''; } - return `${this.meetingDay.slice(0, 3)} at ${this.meetingTime} by ${this.tutorName} in ${ + // A tutorial can be saved without a meeting day, which made slice() throw. + const day = this.meetingDay ? this.meetingDay.slice(0, 3) : 'No day set'; + return `${day} at ${this.meetingTime} by ${this.tutorName} in ${ this.meetingLocation }${campusPart}`; } diff --git a/src/app/api/models/unit.reviewed.spec.ts b/src/app/api/models/unit.reviewed.spec.ts new file mode 100644 index 0000000000..1a1023fdaa --- /dev/null +++ b/src/app/api/models/unit.reviewed.spec.ts @@ -0,0 +1,35 @@ +import {describe, expect, it, vi} from 'vitest'; +import {Project} from './project'; +import {Unit} from './unit'; + +describe('Unit.findStudent', () => { + it('returns the same project instance as studentCache.get', () => { + const unit = new Unit(); + const project = new Project(unit); + + project.id = 42; + unit.studentCache.add(project); + + expect(unit.findStudent(42)).toBe(unit.studentCache.get(42)); + }); + + it('does not scan the students array', () => { + const unit = new Unit(); + const project = new Project(unit); + + project.id = 42; + unit.studentCache.add(project); + + vi.spyOn(unit, 'students', 'get').mockImplementation(() => { + throw new Error('students array was scanned'); + }); + + expect(unit.findStudent(42)).toBe(project); + }); + + it('returns undefined when the student does not exist', () => { + const unit = new Unit(); + + expect(unit.findStudent(999)).toBeUndefined(); + }); +}); diff --git a/src/app/api/services/additional-notification-email.service.ts b/src/app/api/services/additional-notification-email.service.ts new file mode 100644 index 0000000000..c706e047d9 --- /dev/null +++ b/src/app/api/services/additional-notification-email.service.ts @@ -0,0 +1,63 @@ +import {HttpClient} from '@angular/common/http'; +import {Injectable} from '@angular/core'; +import {Observable, map} from 'rxjs'; +import API_URL from 'src/app/config/constants/apiUrl'; + +export type AdditionalNotificationEmailStatus = 'none' | 'pending' | 'verified'; + +export interface AdditionalNotificationEmailState { + status: AdditionalNotificationEmailStatus; + email: string | null; + verificationExpiresAt: string | null; +} + +interface AdditionalNotificationEmailResponse { + status: AdditionalNotificationEmailStatus; + email: string | null; + verification_expires_at: string | null; +} + +@Injectable({providedIn: 'root'}) +export class AdditionalNotificationEmailService { + constructor(private http: HttpClient) {} + + public get(userId: number): Observable { + return this.http + .get(this.url(userId)) + .pipe(map((response) => this.mapState(response))); + } + + public request(userId: number, email: string): Observable { + return this.http + .put(this.url(userId), {email}) + .pipe(map((response) => this.mapState(response))); + } + + public resend(userId: number): Observable { + return this.http + .post(`${this.url(userId)}/resend`, {}) + .pipe(map((response) => this.mapState(response))); + } + + public remove(userId: number): Observable { + return this.http.delete(this.url(userId)); + } + + public verify(token: string): Observable { + return this.http.post(`${API_URL}/additional_notification_emails/verify`, {token}); + } + + private url(userId: number): string { + return `${API_URL}/users/${userId}/additional_notification_email`; + } + + private mapState( + response: AdditionalNotificationEmailResponse, + ): AdditionalNotificationEmailState { + return { + status: response.status, + email: response.email, + verificationExpiresAt: response.verification_expires_at, + }; + } +} diff --git a/src/app/api/services/notification-feedback-route-intent.service.ts b/src/app/api/services/notification-feedback-route-intent.service.ts new file mode 100644 index 0000000000..036d657499 --- /dev/null +++ b/src/app/api/services/notification-feedback-route-intent.service.ts @@ -0,0 +1,70 @@ +import {Injectable} from '@angular/core'; +import {Observable, Subject} from 'rxjs'; + +export interface NotificationFeedbackRouteTarget { + projectId: number; + taskAbbreviation: string; +} + +export interface NotificationFeedbackRouteIntent extends NotificationFeedbackRouteTarget { + requestId: number; +} + +/** + * Carries a validated feedback-route intent until the project resolves its + * task definition. + * + * Notification links contain the public task abbreviation, while Batch 02's + * conversation hook deliberately accepts the internal task-definition id. A + * small one-shot coordinator keeps either layer from guessing the other's + * identity and also handles a click on the feedback route that is already open. + */ +@Injectable({providedIn: 'root'}) +export class NotificationFeedbackRouteIntentService { + private nextRequestId = 0; + private pendingIntent: NotificationFeedbackRouteIntent | null = null; + private readonly requestSubject: Subject = new Subject(); + + readonly requests$: Observable = + this.requestSubject.asObservable(); + + request(target: NotificationFeedbackRouteTarget): NotificationFeedbackRouteIntent { + const request: NotificationFeedbackRouteIntent = { + ...target, + requestId: ++this.nextRequestId, + }; + this.pendingIntent = request; + this.requestSubject.next(request); + return request; + } + + consume(target: NotificationFeedbackRouteTarget): NotificationFeedbackRouteIntent | null { + if (!this.matches(this.pendingIntent, target)) { + return null; + } + + const request = this.pendingIntent; + this.pendingIntent = null; + return request; + } + + cancel(request: NotificationFeedbackRouteIntent): void { + if (this.pendingIntent?.requestId === request.requestId) { + this.pendingIntent = null; + } + } + + clear(): void { + this.pendingIntent = null; + } + + private matches( + request: NotificationFeedbackRouteIntent | null, + target: NotificationFeedbackRouteTarget, + ): request is NotificationFeedbackRouteIntent { + return ( + request?.projectId === target.projectId && + request.taskAbbreviation === target.taskAbbreviation + ); + } +} diff --git a/src/app/api/services/project.service.spec.ts b/src/app/api/services/project.service.spec.ts new file mode 100644 index 0000000000..7baa98c4d3 --- /dev/null +++ b/src/app/api/services/project.service.spec.ts @@ -0,0 +1,90 @@ +import {describe, expect, it, vi} from 'vitest'; +import {Observable, Subject, of, throwError} from 'rxjs'; +import {Project} from '../models/project'; +import {Unit} from '../models/unit'; +import {User} from '../models/user/user'; +import {ProjectService} from './project.service'; + +// The api sends a project's user_id, and only a student may read their own user +// record. Staff are refused (403), so a tutor on a student's page had no student at +// all and the moderation notes tab threw on every render. +describe('ProjectService student lookup', () => { + function serviceWith(getUser: () => Observable): ProjectService { + const userService = {get: getUser}; + return new ProjectService( + {} as never, + {} as never, + userService as never, + {} as never, + {} as never, + {} as never, + ); + } + + function listedProject(unit: Unit, student: User): Project { + const listed = new Project(unit); + listed.id = 7; + listed.student = student; + return listed; + } + + const student = {name: 'Chloe Wilson'} as unknown as User; + + it('keeps the user record when the student can read it', () => { + const service = serviceWith(() => of(student)); + const project = new Project(new Unit()); + + project.updateFromJson({id: 7, user_id: 560}, service.mapping); + + expect(project.student).toBe(student); + }); + + it('takes the student from the unit list when staff are refused the user record', () => { + const service = serviceWith(() => throwError(() => ({status: 403}))); + const unit = new Unit(); + unit.studentCache.add(listedProject(unit, student)); + const loadStudents = vi.spyOn(service, 'loadStudents'); + const project = new Project(unit); + + project.updateFromJson({id: 7, user_id: 560}, service.mapping); + + expect(project.student).toBe(student); + expect(loadStudents).not.toHaveBeenCalled(); + }); + + it('loads the unit list when a staff page has not loaded it yet', () => { + const service = serviceWith(() => throwError(() => ({status: 403}))); + const unit = new Unit(); + const loadStudents = vi + .spyOn(service, 'loadStudents') + .mockReturnValue(of([listedProject(new Unit(), student)])); + const project = new Project(unit); + + project.updateFromJson({id: 7, user_id: 560}, service.mapping); + + expect(loadStudents).toHaveBeenCalledWith(unit); + expect(project.student).toBe(student); + }); + + it('shares one student list request between students opened at the same time', () => { + const service = serviceWith(() => throwError(() => ({status: 403}))); + const unit = new Unit(); + const response: Subject = new Subject(); + const loadStudents = vi.spyOn(service, 'loadStudents').mockReturnValue(response); + const first = new Project(unit); + const second = new Project(unit); + const other = {name: 'Ethan Brown'} as unknown as User; + const otherListed = new Project(new Unit()); + otherListed.id = 8; + otherListed.student = other; + + first.updateFromJson({id: 7, user_id: 560}, service.mapping); + second.updateFromJson({id: 8, user_id: 561}, service.mapping); + response.next([listedProject(new Unit(), student), otherListed]); + response.complete(); + + expect(loadStudents).toHaveBeenCalledOnce(); + expect(first.student).toBe(student); + expect(second.student).toBe(other); + }); +}); diff --git a/src/app/api/services/project.service.ts b/src/app/api/services/project.service.ts index 4156e29484..95ab69942d 100644 --- a/src/app/api/services/project.service.ts +++ b/src/app/api/services/project.service.ts @@ -1,7 +1,7 @@ import {CachedEntityService, MappingProcess, RequestOptions} from 'ngx-entity-service'; import {HttpClient} from '@angular/common/http'; import {Injectable} from '@angular/core'; -import {Observable} from 'rxjs'; +import {Observable, finalize, shareReplay} from 'rxjs'; import { CampusService, Project, @@ -60,10 +60,16 @@ export class ProjectService extends CachedEntityService { toEntityOp: (data: object, key: string, entity: Project) => { const userId = data['user_id']; + // A student can read their own user record, but the api refuses staff (403), + // so a tutor or convenor on a student's page never had the student. For them + // the student comes from the unit's student list, which staff can read. this.userService.get(userId).subscribe({ next: (user) => { entity.student = user; }, + error: () => { + this.loadStudentFromUnit(entity); + }, }); }, }, @@ -104,7 +110,9 @@ export class ProjectService extends CachedEntityService { }, { key: 'not_started', - value: Math.round((values['grey_pct'] || 1) * 100), + // ?? not ||: a student with nothing left to start has a grey share + // of 0, and || turned that into a bar that was all grey. + value: Math.round((values['grey_pct'] ?? 1) * 100), }, { key: 'working_on_it', @@ -251,6 +259,46 @@ export class ProjectService extends CachedEntityService { } } + // One student list request per unit while it is in flight, so opening several + // students of the same unit at once does not load the whole list each time. + private readonly studentListLoads: Map> = new Map(); + + // Reuses the unit's student list when a staff page has already loaded it (the inbox + // and the students list do), and otherwise loads it once. + private loadStudentFromUnit(project: Project): void { + const unit = project.unit; + if (!unit) { + return; + } + + const known = unit.studentCache.get(project.id)?.student; + if (known) { + project.student = known; + return; + } + + let studentList = this.studentListLoads.get(unit); + if (!studentList) { + studentList = this.loadStudents(unit).pipe( + finalize(() => this.studentListLoads.delete(unit)), + shareReplay({bufferSize: 1, refCount: false}), + ); + this.studentListLoads.set(unit, studentList); + } + + studentList.subscribe({ + next: (students) => { + const match = students.find((candidate) => candidate.id === project.id); + if (match?.student) { + project.student = match.student; + } + }, + error: () => { + // Nothing more to try. The pages leave the student's name out instead. + }, + }); + } + public loadProject( proj: Project | number, unit: Unit, diff --git a/src/app/api/services/spec/additional-notification-email.service.spec.ts b/src/app/api/services/spec/additional-notification-email.service.spec.ts new file mode 100644 index 0000000000..648b7a34f9 --- /dev/null +++ b/src/app/api/services/spec/additional-notification-email.service.spec.ts @@ -0,0 +1,76 @@ +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {provideHttpClient} from '@angular/common/http'; +import {HttpTestingController, provideHttpClientTesting} from '@angular/common/http/testing'; +import {TestBed} from '@angular/core/testing'; +import {AdditionalNotificationEmailService} from '../additional-notification-email.service'; + +describe('AdditionalNotificationEmailService', () => { + let service: AdditionalNotificationEmailService; + let httpMock: HttpTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + AdditionalNotificationEmailService, + provideHttpClient(), + provideHttpClientTesting(), + ], + }); + service = TestBed.inject(AdditionalNotificationEmailService); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => httpMock.verify()); + + it('maps pending state without exposing a verification token', () => { + service.get(12).subscribe((state) => { + expect(state).toEqual({ + status: 'pending', + email: 'secondary@example.org', + verificationExpiresAt: '2026-09-01T00:00:00Z', + }); + expect(state).not.toHaveProperty('token'); + }); + + const request = httpMock.expectOne( + 'http://localhost:3000/api/users/12/additional_notification_email', + ); + expect(request.request.method).toBe('GET'); + request.flush({ + status: 'pending', + email: 'secondary@example.org', + verification_expires_at: '2026-09-01T00:00:00Z', + }); + }); + + it('uses explicit request, resend, removal, and body-token verification endpoints', () => { + service.request(12, 'secondary@example.org').subscribe(); + let request = httpMock.expectOne( + 'http://localhost:3000/api/users/12/additional_notification_email', + ); + expect(request.request.method).toBe('PUT'); + expect(request.request.body).toEqual({email: 'secondary@example.org'}); + request.flush({status: 'pending', email: 'secondary@example.org'}); + + service.resend(12).subscribe(); + request = httpMock.expectOne( + 'http://localhost:3000/api/users/12/additional_notification_email/resend', + ); + expect(request.request.method).toBe('POST'); + request.flush({status: 'pending', email: 'secondary@example.org'}); + + service.remove(12).subscribe(); + request = httpMock.expectOne( + 'http://localhost:3000/api/users/12/additional_notification_email', + ); + expect(request.request.method).toBe('DELETE'); + request.flush(null); + + service.verify('secret-link-token').subscribe(); + request = httpMock.expectOne('http://localhost:3000/api/additional_notification_emails/verify'); + expect(request.request.method).toBe('POST'); + expect(request.request.body).toEqual({token: 'secret-link-token'}); + expect(request.request.url).not.toContain('secret-link-token'); + request.flush({status: 'verified'}); + }); +}); diff --git a/src/app/api/services/spec/notification-feedback-route-intent.service.spec.ts b/src/app/api/services/spec/notification-feedback-route-intent.service.spec.ts new file mode 100644 index 0000000000..08e52ee63f --- /dev/null +++ b/src/app/api/services/spec/notification-feedback-route-intent.service.spec.ts @@ -0,0 +1,44 @@ +import {describe, expect, it, vi} from 'vitest'; +import {NotificationFeedbackRouteIntentService} from '../notification-feedback-route-intent.service'; + +describe('NotificationFeedbackRouteIntentService', () => { + const target = {projectId: 7, taskAbbreviation: '1.1P'}; + + it('carries one intent until the matching resolved task consumes it', () => { + const service = new NotificationFeedbackRouteIntentService(); + const request = service.request(target); + + expect(service.consume({projectId: 7, taskAbbreviation: 'OTHER'})).toBeNull(); + expect(service.consume(target)).toBe(request); + expect(service.consume(target)).toBeNull(); + }); + + it('notifies an already-mounted project when the same feedback route is clicked', () => { + const service = new NotificationFeedbackRouteIntentService(); + const observed = vi.fn(); + service.requests$.subscribe(observed); + + const request = service.request(target); + + expect(observed).toHaveBeenCalledWith(request); + }); + + it('retains only the latest intent and cannot cancel it with an older request', () => { + const service = new NotificationFeedbackRouteIntentService(); + const older = service.request(target); + const latest = service.request({projectId: 8, taskAbbreviation: '2.1P'}); + + service.cancel(older); + + expect(service.consume({projectId: 8, taskAbbreviation: '2.1P'})).toBe(latest); + }); + + it('clears account-scoped state on sign out', () => { + const service = new NotificationFeedbackRouteIntentService(); + service.request(target); + + service.clear(); + + expect(service.consume(target)).toBeNull(); + }); +}); diff --git a/src/app/common/additional-notification-email/additional-notification-email.component.html b/src/app/common/additional-notification-email/additional-notification-email.component.html new file mode 100644 index 0000000000..2e1b490d7c --- /dev/null +++ b/src/app/common/additional-notification-email/additional-notification-email.component.html @@ -0,0 +1,85 @@ + diff --git a/src/app/common/additional-notification-email/additional-notification-email.component.scss b/src/app/common/additional-notification-email/additional-notification-email.component.scss new file mode 100644 index 0000000000..c944a90577 --- /dev/null +++ b/src/app/common/additional-notification-email/additional-notification-email.component.scss @@ -0,0 +1,97 @@ +@use '../edit-profile-form/profile-form' as profile; + +@include profile.theme; + +:host { + display: block; + min-width: 0; + width: 100%; +} + +.additional-email { + @include profile.card; + + // empty live regions stay in the tree for screen readers but take no gap + > p:empty { + margin-top: -16px; + } + + &__heading { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 12px; + + > div { + @include profile.card-heading; + } + } + + &__status { + flex: 0 0 auto; + padding: 4px 10px; + border-radius: var(--ot-radius-pill); + background: var(--ot-color-warning); + color: var(--ot-color-on-warning); + font-size: 0.78rem; + font-weight: 600; + line-height: 1.4; + + &--verified { + background: var(--ot-status-complete); + color: var(--ot-status-complete-on); + } + } + + mat-form-field { + width: 100%; + } + + &__actions { + display: flex; + flex-wrap: wrap; + gap: 8px; + + button { + min-height: 44px; + } + } + + &__help, + &__message, + &__error { + margin: 0; + font-size: 0.875rem; + line-height: 1.45; + } + + &__help { + color: var(--ot-color-text-muted); + } + + &__message { + color: var(--ot-color-success); + } + + &__error { + color: var(--ot-color-error); + font-weight: 600; + } +} + +@media (max-width: 599px) { + .additional-email { + &__heading { + flex-direction: column; + align-items: stretch; + } + + &__status { + align-self: flex-start; + } + + &__actions button { + flex: 1 1 100%; + } + } +} diff --git a/src/app/common/additional-notification-email/additional-notification-email.component.spec.ts b/src/app/common/additional-notification-email/additional-notification-email.component.spec.ts new file mode 100644 index 0000000000..0d747a092c --- /dev/null +++ b/src/app/common/additional-notification-email/additional-notification-email.component.spec.ts @@ -0,0 +1,160 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {NO_ERRORS_SCHEMA} from '@angular/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {FormsModule} from '@angular/forms'; +import {of, throwError} from 'rxjs'; +import {User} from 'src/app/api/models/user/user'; +import {AdditionalNotificationEmailService} from 'src/app/api/services/additional-notification-email.service'; +import {AdditionalNotificationEmailComponent} from './additional-notification-email.component'; + +describe('AdditionalNotificationEmailComponent', () => { + let fixture: ComponentFixture; + let component: AdditionalNotificationEmailComponent; + const service = { + get: vi.fn(), + request: vi.fn(), + resend: vi.fn(), + remove: vi.fn(), + }; + + beforeEach(async () => { + vi.restoreAllMocks(); + service.get.mockReturnValue(of({status: 'none', email: null, verificationExpiresAt: null})); + service.request.mockReturnValue( + of({ + status: 'pending', + email: 'secondary@example.org', + verificationExpiresAt: '2026-09-01T00:00:00Z', + }), + ); + service.resend.mockReturnValue( + of({ + status: 'pending', + email: 'secondary@example.org', + verificationExpiresAt: '2026-09-01T00:00:00Z', + }), + ); + service.remove.mockReturnValue(of(undefined)); + + await TestBed.configureTestingModule({ + declarations: [AdditionalNotificationEmailComponent], + providers: [{provide: AdditionalNotificationEmailService, useValue: service}], + }) + .overrideComponent(AdditionalNotificationEmailComponent, {set: {template: ''}}) + .compileComponents(); + + fixture = TestBed.createComponent(AdditionalNotificationEmailComponent); + component = fixture.componentInstance; + component.user = {id: 12} as User; + fixture.detectChanges(); + }); + + it('loads state for only the signed-in profile user', () => { + expect(service.get).toHaveBeenCalledWith(12); + expect(component.state.status).toBe('none'); + }); + + it('keeps normal copies off while a newly requested address is pending', () => { + component.draftEmail = 'secondary@example.org'; + component.requestVerification(); + + expect(service.request).toHaveBeenCalledWith(12, 'secondary@example.org'); + expect(component.state.status).toBe('pending'); + expect(component.message).toContain('No notification copies are sent yet'); + }); + + it('preserves the address and exposes controlled failure feedback', () => { + service.request.mockReturnValueOnce( + throwError(() => ({error: {error: 'Too many verification requests.'}})), + ); + component.draftEmail = 'secondary@example.org'; + component.requestVerification(); + + expect(component.draftEmail).toBe('secondary@example.org'); + expect(component.errorMessage).toBe('Too many verification requests.'); + }); + + it('requires confirmation before removal', () => { + component.state = { + status: 'verified', + email: 'secondary@example.org', + verificationExpiresAt: null, + }; + vi.spyOn(window, 'confirm').mockReturnValue(false); + + component.remove(); + expect(service.remove).not.toHaveBeenCalled(); + + vi.mocked(window.confirm).mockReturnValue(true); + component.remove(); + expect(service.remove).toHaveBeenCalledWith(12); + expect(component.state.status).toBe('none'); + }); +}); + +// The profile page renders this block inside the profile
. Without its own +// Enter handling, the browser's implicit submission presses Save profile and no +// verification is requested. Renders the real template so the key binding on +// the input is what gets checked. +describe('AdditionalNotificationEmailComponent Enter key', () => { + let fixture: ComponentFixture; + const service = { + get: vi.fn(), + request: vi.fn(), + resend: vi.fn(), + remove: vi.fn(), + }; + + beforeEach(async () => { + vi.clearAllMocks(); + service.get.mockReturnValue(of({status: 'none', email: null, verificationExpiresAt: null})); + service.request.mockReturnValue( + of({ + status: 'pending', + email: 'secondary@example.org', + verificationExpiresAt: '2026-09-01T00:00:00Z', + }), + ); + + await TestBed.configureTestingModule({ + declarations: [AdditionalNotificationEmailComponent], + imports: [FormsModule], + providers: [{provide: AdditionalNotificationEmailService, useValue: service}], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(AdditionalNotificationEmailComponent); + fixture.componentInstance.user = {id: 12} as User; + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + }); + + const typeAndPressEnter = async (value: string): Promise => { + const input: HTMLInputElement = fixture.nativeElement.querySelector( + 'input[name="additional_notification_email"]', + ); + input.value = value; + input.dispatchEvent(new Event('input')); + fixture.detectChanges(); + await fixture.whenStable(); + + const enter = new KeyboardEvent('keydown', {key: 'Enter', bubbles: true, cancelable: true}); + input.dispatchEvent(enter); + return enter; + }; + + it('requests verification and stops the enclosing form from submitting', async () => { + const enter = await typeAndPressEnter('secondary@example.org'); + + expect(enter.defaultPrevented).toBe(true); + expect(service.request).toHaveBeenCalledWith(12, 'secondary@example.org'); + }); + + it('does not request verification for an address the button would refuse', async () => { + const enter = await typeAndPressEnter(`${'a'.repeat(250)}@example.org`); + + expect(enter.defaultPrevented).toBe(true); + expect(service.request).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/common/additional-notification-email/additional-notification-email.component.ts b/src/app/common/additional-notification-email/additional-notification-email.component.ts new file mode 100644 index 0000000000..36ddbe6ee7 --- /dev/null +++ b/src/app/common/additional-notification-email/additional-notification-email.component.ts @@ -0,0 +1,149 @@ +import {HttpErrorResponse} from '@angular/common/http'; +import {ChangeDetectorRef, Component, Input, OnInit} from '@angular/core'; +import {User} from 'src/app/api/models/user/user'; +import { + AdditionalNotificationEmailService, + AdditionalNotificationEmailState, +} from 'src/app/api/services/additional-notification-email.service'; + +@Component({ + selector: 'f-additional-notification-email', + templateUrl: './additional-notification-email.component.html', + styleUrl: './additional-notification-email.component.scss', + standalone: false, +}) +export class AdditionalNotificationEmailComponent implements OnInit { + @Input({required: true}) user!: User; + + public state: AdditionalNotificationEmailState = { + status: 'none', + email: null, + verificationExpiresAt: null, + }; + public draftEmail = ''; + public loading = true; + public busy = false; + public message = ''; + public errorMessage = ''; + + constructor( + private additionalEmailService: AdditionalNotificationEmailService, + private changeDetector: ChangeDetectorRef, + ) {} + + public ngOnInit(): void { + this.additionalEmailService.get(this.user.id).subscribe({ + next: (state) => { + this.applyState(state); + this.loading = false; + this.changeDetector.markForCheck(); + }, + error: (error: HttpErrorResponse) => { + this.loading = false; + this.errorMessage = this.errorText(error, 'Could not load additional email settings.'); + this.changeDetector.markForCheck(); + }, + }); + } + + public get changed(): boolean { + return this.draftEmail.trim().toLowerCase() !== (this.state.email ?? '').toLowerCase(); + } + + public get requestLabel(): string { + if (this.state.status === 'verified') { + return 'Change email and send verification'; + } + return this.state.status === 'pending' + ? 'Update email and send verification' + : 'Send verification email'; + } + + public requestVerification(): void { + const email = this.draftEmail.trim(); + if (this.busy || !email || !this.changed) { + return; + } + + this.startRequest(); + this.additionalEmailService.request(this.user.id, email).subscribe({ + next: (state) => { + this.applyState(state); + this.busy = false; + this.message = 'Verification email requested. No notification copies are sent yet.'; + this.changeDetector.markForCheck(); + }, + error: (error: HttpErrorResponse) => this.fail(error, 'Could not request verification.'), + }); + } + + /** + * This block sits inside the profile form, so Enter in the email field would + * otherwise submit the whole profile and request nothing. Treat Enter as the + * verification button instead, and do nothing when that button is disabled. + */ + public requestVerificationOnEnter(event: Event, invalid: boolean): void { + event.preventDefault(); + if (!invalid) { + this.requestVerification(); + } + } + + public resend(): void { + if (this.busy || this.state.status !== 'pending') { + return; + } + + this.startRequest(); + this.additionalEmailService.resend(this.user.id).subscribe({ + next: (state) => { + this.applyState(state); + this.busy = false; + this.message = 'A new verification email was requested. Earlier links no longer work.'; + this.changeDetector.markForCheck(); + }, + error: (error: HttpErrorResponse) => this.fail(error, 'Could not resend verification.'), + }); + } + + public remove(): void { + if (this.busy || this.state.status === 'none') { + return; + } + if (!window.confirm('Remove this additional notification email? Future copies will stop.')) { + return; + } + + this.startRequest(); + this.additionalEmailService.remove(this.user.id).subscribe({ + next: () => { + this.applyState({status: 'none', email: null, verificationExpiresAt: null}); + this.busy = false; + this.message = 'Additional notification email removed.'; + this.changeDetector.markForCheck(); + }, + error: (error: HttpErrorResponse) => this.fail(error, 'Could not remove the email.'), + }); + } + + private applyState(state: AdditionalNotificationEmailState): void { + this.state = state; + this.draftEmail = state.email ?? ''; + } + + private startRequest(): void { + this.busy = true; + this.message = ''; + this.errorMessage = ''; + } + + private fail(error: HttpErrorResponse, fallback: string): void { + this.busy = false; + this.errorMessage = this.errorText(error, fallback); + this.changeDetector.markForCheck(); + } + + private errorText(error: HttpErrorResponse, fallback: string): string { + return typeof error.error?.error === 'string' ? error.error.error : fallback; + } +} diff --git a/src/app/common/archive-viewer/archive-viewer.component.html b/src/app/common/archive-viewer/archive-viewer.component.html index 1ff2f8290e..21afcaa95b 100644 --- a/src/app/common/archive-viewer/archive-viewer.component.html +++ b/src/app/common/archive-viewer/archive-viewer.component.html @@ -1,21 +1,21 @@
@if (isLoading) { -
+
Loading archive...
} @else if (errorMessage) { -
+
error_outline {{ errorMessage }}
} @else if (!archiveFile) { -
+
folder_zip Select an archive to preview.
} @else if (!hasFiles) { -
+
folder_off No files to display.
@@ -38,9 +38,9 @@ } @if (navigationMode === 'tree') { -
+