[PER-10778] Video stuck in processing

src/app/shared/components/video/video.component.html

@@ -1,6 +1,10 @@
 <div
-	class="pr-video-wrapper loading"
-	[ngClass]="{ tall: item.imageRatio > 1, processing: isProcessing }"
+	class="pr-video-wrapper"
+	[ngClass]="{
+		tall: item.imageRatio > 1,
+		loading: isLoading,
+		processing: isProcessing
+	}"
 >
 	<video
 		[attr.aria-label]="item | getAltText"

src/app/shared/components/video/video.component.spec.ts

@@ -3,13 +3,30 @@ import * as Testing from '@root/test/testbedConfig';
 import { cloneDeep } from 'lodash';
 
 import { RecordVO } from '@root/app/models';
+import { FileFormat, PermanentFile } from '@models/file-vo';
 import { GetAltTextPipe } from '../../pipes/get-alt-text.pipe';
 import { VideoComponent } from './video.component';
 
 describe('VideoComponent', () => {
 	let component: VideoComponent;
 	let fixture: ComponentFixture<VideoComponent>;
 
+	function makeTestFile(params: Partial<PermanentFile> = {}): PermanentFile {
+		return {
+			fileId: 0,
+			size: 0,
+			format: FileFormat.Original,
+			fileURL: 'https://example.test/original.mp4',
+			downloadURL: 'https://example.test/original.mp4',
+			type: 'video/mp4',
+			...params,
+		};
+	}
+
+	function processingMessage(): HTMLElement | null {
+		return fixture.nativeElement.querySelector('.message');
+	}
+
 	beforeEach(async () => {
 		const config = cloneDeep(Testing.BASE_TEST_CONFIG);
 
@@ -29,4 +46,58 @@ describe('VideoComponent', () => {
 	it('should create', () => {
 		expect(component).toBeTruthy();
 	});
+
+	it('should show the processing message when the record has no files', () => {
+		expect(component.isProcessing).toBeTrue();
+		expect(processingMessage()).not.toBeNull();
+	});
+
+	it('should play the archivematica access copy when there is one', () => {
+		component.item = new RecordVO({
+			displayName: 'test video',
+			FileVOs: [
+				makeTestFile(),
+				makeTestFile({
+					format: FileFormat.ArchivematicaAccess,
+					fileURL: 'https://example.test/access.mp4',
+				}),
+			],
+		});
+		fixture.detectChanges();
+
+		expect(component.isProcessing).toBeFalse();
+		expect(component.videoSrc).toBe('https://example.test/access.mp4');
+		expect(processingMessage()).toBeNull();
+	});
+
+	it('should fall back to the original when there is no access copy', () => {
+		component.item = new RecordVO({
+			displayName: 'test video',
+			FileVOs: [makeTestFile()],
+		});
+		fixture.detectChanges();
+
+		expect(component.isProcessing).toBeFalse();
+		expect(component.videoSrc).toBe('https://example.test/original.mp4');
+	});
+
+	it('should clear the processing message when the files arrive after init', () => {
+		expect(component.isProcessing).toBeTrue();
+
+		component.item.update({ FileVOs: [makeTestFile()] });
+		fixture.detectChanges();
+
+		expect(component.isProcessing).toBeFalse();
+		expect(component.videoSrc).toBe('https://example.test/original.mp4');
+		expect(processingMessage()).toBeNull();
+	});
+
+	it('should show the loader again while a newly arrived file loads', () => {
+		expect(component.isLoading).toBeFalse();
+
+		component.item.update({ FileVOs: [makeTestFile()] });
+		fixture.detectChanges();
+
+		expect(component.isLoading).toBeTrue();
+	});
 });

src/app/shared/components/video/video.component.ts

@@ -1,4 +1,4 @@
-import { Component, OnInit, Input, ElementRef, Renderer2 } from '@angular/core';
+import { Component, OnInit, DoCheck, Input, ElementRef } from '@angular/core';
 import { gsap } from 'gsap';
 
 import { RecordVO } from '@root/app/models';
@@ -12,27 +12,22 @@ const FADE_IN_DURATION = 0.3;
 	styleUrls: ['./video.component.scss'],
 	standalone: false,
 })
-export class VideoComponent implements OnInit {
+export class VideoComponent implements OnInit, DoCheck {
 	@Input() item: RecordVO;
 
-	private videoWrapperElem: Element;
 	private videoElem: Element;
 	public videoSrc: string;
 	public isProcessing: boolean;
+	public isLoading = true;
 
-	constructor(
-		private elementRef: ElementRef,
-		private renderer: Renderer2,
-	) {}
+	constructor(private elementRef: ElementRef) {}
 
 	ngOnInit() {
 		this.videoElem = this.elementRef.nativeElement.querySelector('video');
-		this.videoWrapperElem =
-			this.elementRef.nativeElement.querySelector('.pr-video-wrapper');
 
-		this.videoElem.addEventListener('loadstart', (event) => {
+		this.videoElem.addEventListener('loadstart', () => {
 			setTimeout(() => {
-				this.renderer.removeClass(this.videoWrapperElem, 'loading');
+				this.isLoading = false;
 				gsap.from(this.videoElem, {
 					duration: FADE_IN_DURATION,
 					opacity: 0,
@@ -41,14 +36,22 @@ export class VideoComponent implements OnInit {
 			}, 250);
 		});
 
-		const accessFile = GetAccessFile(this.item);
+		this.applyAccessFile();
+	}
+
+	ngDoCheck() {
+		this.applyAccessFile();
+	}
+
+	private applyAccessFile(): void {
+		const accessFileUrl = GetAccessFile(this.item)?.fileURL ?? null;
+		const hasAccessFileChanged = accessFileUrl !== this.videoSrc;
+
+		this.videoSrc = accessFileUrl;
+		this.isProcessing = accessFileUrl === null;
 
-		if (accessFile) {
-			this.videoSrc = accessFile.fileURL;
-			this.isProcessing = false;
-		} else {
-			this.renderer.removeClass(this.videoWrapperElem, 'loading');
-			this.isProcessing = true;
+		if (hasAccessFileChanged) {
+			this.isLoading = accessFileUrl !== null;
 		}
 	}
 }

src/app/shared/services/data/data.service.spec.ts

@@ -5,7 +5,10 @@ import { HttpV2Service } from '@shared/services/http-v2/http-v2.service';
 
 import { DataService } from '@shared/services/data/data.service';
 import { FolderVO, FolderVOData, RecordVO } from '@root/app/models';
-import { FolderResponse } from '@shared/services/api/index.repo';
+import {
+	FolderResponse,
+	RecordResponse,
+} from '@shared/services/api/index.repo';
 import { of } from 'rxjs';
 import { DataStatus } from '@models/data-status.enum';
 
@@ -234,6 +237,104 @@ describe('DataService', () => {
 		await service.fetchFullItems([]);
 	});
 
+	describe('fetchFullItems', () => {
+		const buildRecordResponse = (recordsData: object[]) =>
+			new RecordResponse({
+				isSuccessful: true,
+				Results: recordsData.map((recordData) => ({
+					data: [{ RecordVO: recordData }],
+				})),
+			});
+
+		const buildFolderVOsResponse = (foldersData: FolderVOData[]) =>
+			new FolderResponse({
+				isSuccessful: true,
+				Results: foldersData.map((folderData) => ({
+					data: [{ FolderVO: folderData }],
+				})),
+			});
+
+		let service: DataService;
+		let recordGet: jasmine.Spy;
+		let getStelaFolderVOs: jasmine.Spy;
+
+		beforeEach(() => {
+			service = TestBed.inject(DataService);
+			const api = TestBed.inject(ApiService);
+			recordGet = spyOn(api.record, 'get');
+			getStelaFolderVOs = spyOn(api.folder, 'getStelaFolderVOs');
+			service.setCurrentFolder(testFolder);
+		});
+
+		it('should match records to the response by recordId, not by position', async () => {
+			const firstRecord = new RecordVO({ recordId: '11', archiveNbr: 'a' });
+			const secondRecord = new RecordVO({ recordId: '22', archiveNbr: 'b' });
+			recordGet.and.resolveTo(
+				buildRecordResponse([
+					{ recordId: '22', displayName: 'second' },
+					{ recordId: '11', displayName: 'first' },
+				]),
+			);
+
+			await service.fetchFullItems([firstRecord, secondRecord]);
+
+			expect(firstRecord.displayName).toBe('first');
+			expect(secondRecord.displayName).toBe('second');
+		});
+
+		it('should leave a record the response skipped below Full so it can be fetched again', async () => {
+			const returnedRecord = new RecordVO({ recordId: '11', archiveNbr: 'a' });
+			const skippedRecord = new RecordVO({
+				recordId: '22',
+				archiveNbr: 'b',
+				displayName: 'lean name',
+			});
+			recordGet.and.resolveTo(
+				buildRecordResponse([{ recordId: '11', displayName: 'first' }]),
+			);
+
+			await service.fetchFullItems([returnedRecord, skippedRecord]);
+
+			expect(returnedRecord.dataStatus).toBe(DataStatus.Full);
+			expect(skippedRecord.dataStatus).toBeLessThan(DataStatus.Full);
+			expect(skippedRecord.displayName).toBe('lean name');
+		});
+
+		it('should match folders to the response by folderId, not by position', async () => {
+			const folderWithoutId = new FolderVO({ folder_linkId: 1 });
+			const folderWithId = new FolderVO({ folderId: '33', folder_linkId: 2 });
+			getStelaFolderVOs.and.resolveTo(
+				buildFolderVOsResponse([{ folderId: '33', displayName: 'real name' }]),
+			);
+
+			await service.fetchFullItems([folderWithoutId, folderWithId]);
+
+			expect(folderWithId.displayName).toBe('real name');
+			expect(folderWithoutId.displayName).toBeUndefined();
+			expect(folderWithoutId.dataStatus).toBeLessThan(DataStatus.Full);
+		});
+
+		it('should clear isFetching once the items have been fetched', async () => {
+			const record = new RecordVO({ recordId: '11', archiveNbr: 'a' });
+			recordGet.and.resolveTo(
+				buildRecordResponse([{ recordId: '11', displayName: 'first' }]),
+			);
+
+			await service.fetchFullItems([record]);
+
+			expect(record.isFetching).toBeFalse();
+		});
+
+		it('should clear isFetching when the request fails', async () => {
+			const record = new RecordVO({ recordId: '11', archiveNbr: 'a' });
+			recordGet.and.rejectWith(new Error('nope'));
+
+			await service.fetchFullItems([record]);
+
+			expect(record.isFetching).toBeFalse();
+		});
+	});
+
 	describe('refreshCurrentFolder', () => {
 		const berlinFolderData = {
 			folderId: '1',

src/app/shared/services/data/data.service.ts

@@ -40,6 +40,22 @@ const isSameId = (a: ItemId, b: ItemId): boolean => {
 	return String(a) === String(b);
 };
 
+const indexItemsById = <T>(
+	items: T[] = [],
+	getId: (item: T) => ItemId,
+): Map<string, T> => {
+	const itemsById = new Map<string, T>();
+
+	items.forEach((item) => {
+		const id = getId(item);
+		if (id !== null && id !== undefined) {
+			itemsById.set(String(id), item);
+		}
+	});
+
+	return itemsById;
+};
+
 export type SelectedItemsSet = Set<ItemVO>;
 
 export interface SelectKeyEvent {
@@ -375,6 +391,7 @@ export class DataService {
 				itemResolves.push(resolve);
 				itemRejects.push(reject);
 			});
+			item.fetched.catch(noop);
 
 			if (item.isRecord) {
 				records.push(item);
@@ -418,23 +435,42 @@ export class DataService {
 					fullFolders = folderResponse.getFolderVOs();
 				}
 
-				for (let i = 0; i < records.length; i += 1) {
-					records[i].update(fullRecords[i]);
-					records[i].dataStatus = DataStatus.Full;
-					this.tags.checkTagsOnItem(records[i]);
-				}
+				const fullRecordsById = indexItemsById(
+					fullRecords,
+					(fullRecord) => fullRecord.recordId,
+				);
+				const fullFoldersById = indexItemsById(
+					fullFolders,
+					(fullFolder) => fullFolder.folderId,
+				);
+
+				records.forEach((record: RecordVO) => {
+					const fullRecord = fullRecordsById.get(String(record.recordId));
+					if (!fullRecord) {
+						return;
+					}
+
+					record.update(fullRecord);
+					record.dataStatus = DataStatus.Full;
+					this.tags.checkTagsOnItem(record);
+				});
+
+				folders.forEach((folder: FolderVO) => {
+					const fullFolder = fullFoldersById.get(String(folder.folderId));
+					if (!fullFolder) {
+						return;
+					}
 
-				for (let i = 0; i < folders.length; i += 1) {
-					const folder = folders[i] as FolderVO;
 					folder.update(
-						fullFolders[i] as FolderVOData,
-						folders[i] === this.currentFolder,
+						fullFolder as FolderVOData,
+						folder === this.currentFolder,
 					);
 					folder.dataStatus = DataStatus.Full;
-					this.tags.checkTagsOnItem(folders[i]);
-				}
+					this.tags.checkTagsOnItem(folder);
+				});
 
 				itemResolves.forEach((resolve, index) => {
+					items[index].isFetching = false;
 					items[index].fetched = null;
 					this.byArchiveNbr[items[index].archiveNbr] = items[index];
 					resolve();
@@ -446,6 +482,7 @@ export class DataService {
 			})
 			.catch(() => {
 				itemRejects.forEach((reject, index) => {
+					items[index].isFetching = false;
 					items[index].fetched = null;
 					reject();
 				});