ov-components: Refactor ActionService and related tests to improve dialog handling and mock implementations

openvidu-components-angular/projects/openvidu-components-angular/src/lib/components/session/session.component.spec.ts

@@ -3,7 +3,7 @@ import { MatProgressSpinnerModule } from '@angular/material/progress-spinner';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 
 import { ActionService } from '../../services/action/action.service';
-import { ActionServiceMock } from '../../services/action/action.service.mock';
+import { ActionServiceMock } from '../../../test-helpers/action.service.mock';
 
 import { ChatService } from '../../services/chat/chat.service';
 import { ChatServiceMock } from '../../services/chat/chat.service.mock';

openvidu-components-angular/projects/openvidu-components-angular/src/lib/services/action/action.service.mock.ts

@@ -1,18 +0,0 @@
-import { Injectable } from '@angular/core';
-import { INotificationOptions } from '../../models/notification-options.model';
-
-@Injectable()
-export class ActionServiceMock {
-	constructor() {}
-
-	launchNotification(options: INotificationOptions, callback): void {
-
-	}
-
-	openConnectionDialog(titleMessage: string, descriptionMessage: string, allowClose = true) {
-
-	}
-
-	closeConnectionDialog() {
-	}
-}

openvidu-components-angular/projects/openvidu-components-angular/src/lib/services/action/action.service.spec.ts

@@ -1,81 +1,128 @@
 import { fakeAsync, TestBed, tick } from '@angular/core/testing';
-import { MatDialog, MatDialogRef } from '@angular/material/dialog';
+import { of } from 'rxjs';
+import { delay } from 'rxjs/operators';
+import { MatDialog } from '@angular/material/dialog';
 import { MatSnackBarModule } from '@angular/material/snack-bar';
-
 import { ActionService } from './action.service';
-import { TranslateService } from '../translate/translate.service';
-import { TranslateServiceMock } from '../translate/translate.service.mock';
+import { MatSnackBar } from '@angular/material/snack-bar';
+import { MatDialogMock } from '../../../test-helpers/action.service.mock';
+import { TranslateServiceMock } from '../../../test-helpers/translate.service.mock';
 
-export class MatDialogMock {
-	open() {
-		return { close: () => {} } as MatDialogRef<any>;
-	}
-}
-
-describe('ActionService', () => {
+describe('ActionService (characterization)', () => {
 	let service: ActionService;
-	let dialog: MatDialog;
+	let dialog: MatDialogMock;
 
 	beforeEach(() => {
 		TestBed.configureTestingModule({
 			imports: [MatSnackBarModule],
 			providers: [
 				{ provide: MatDialog, useClass: MatDialogMock },
-				{ provide: TranslateService, useClass: TranslateServiceMock },
+				{ provide: 'TranslateService', useClass: TranslateServiceMock },
 				{ provide: 'OPENVIDU_COMPONENTS_CONFIG', useValue: { production: false } }
 			]
 		});
 
 		service = TestBed.inject(ActionService);
-		dialog = TestBed.inject(MatDialog);
+		dialog = TestBed.inject(MatDialog) as unknown as MatDialogMock;
 	});
 
-	it('should be created', () => {
-		expect(service).toBeTruthy();
+	it('opens a connection dialog when requested', () => {
+		const spy = spyOn(dialog, 'open').and.callThrough();
+
+		service.openConnectionDialog('Title', 'Description', false);
+
+		expect(spy).toHaveBeenCalledTimes(1);
+		// observable behavior: a MatDialogRef was created (do not assert internal state)
+		expect(dialog.lastRef).toBeTruthy();
+		expect(typeof dialog.lastRef!.close).toBe('function');
 	});
 
-	it('should open connection dialog', fakeAsync(() => {
-		const dialogSpy = spyOn(dialog, 'open').and.callThrough();
-		service.openConnectionDialog('Test Title', 'Test Description', false);
-		expect(dialogSpy).toHaveBeenCalled();
-		expect(service['isConnectionDialogOpen']).toBeTrue();
-	}));
+	it('does not open a new dialog if one is already open (repeated calls)', () => {
+		const spy = spyOn(dialog, 'open').and.callThrough();
 
-	it('should not open connection dialog if one is already open', () => {
-		service['isConnectionDialogOpen'] = true;
-		const dialogSpy = spyOn(dialog, 'open').and.callThrough();
-		service.openConnectionDialog('Test Title', 'Test Description', false);
+		service.openConnectionDialog('Title', 'Description', false);
+		// repeated calls simulate concurrent/repeated user attempts
+		service.openConnectionDialog('Title', 'Description', false);
+		service.openConnectionDialog('Title', 'Description', false);
 
-		expect(dialogSpy).not.toHaveBeenCalled();
+		// observed behavior: open called only once
+		expect(spy).toHaveBeenCalledTimes(1);
 	});
 
-	it('should close connection dialog and reset state', fakeAsync(() => {
-		service.openConnectionDialog('Test Title', 'Test Description', false);
+	it('closes the opened dialog when requested and allows opening a new one afterwards', fakeAsync(() => {
+		const openSpy = spyOn(dialog, 'open').and.callThrough();
 
-		tick(2000);
+		service.openConnectionDialog('T', 'D', false);
+		tick(10); // advance microtasks if the service uses timers/async internally
 
-		expect(service['isConnectionDialogOpen']).toBeTrue();
+		// Behavior: closing should invoke close() on the MatDialogRef
+		const ref = dialog.lastRef!;
+		expect(ref).toBeTruthy();
+		expect(ref.close).not.toHaveBeenCalled();
 
 		service.closeConnectionDialog();
+		expect(ref.close).toHaveBeenCalledTimes(1);
 
-		expect(service['isConnectionDialogOpen']).toBeFalse();
+		// After closing, opening again should create another instance (another open call)
+		service.openConnectionDialog('T', 'D', false);
+		expect(openSpy).toHaveBeenCalledTimes(2);
 	}));
 
-	it('should open connection dialog only once', fakeAsync(() => {
-		// Spy on the dialog open method
-		const dialogSpy = spyOn(dialog, 'open').and.callThrough();
+	it('handles rapid consecutive calls by creating a single dialog (reentrancy protection)', fakeAsync(() => {
+		const spy = spyOn(dialog, 'open').and.callThrough();
 
-		service.openConnectionDialog('Test Title', 'Test Description', false);
-		// Verify that the dialog has been called only once
-		expect(dialogSpy).toHaveBeenCalledTimes(1);
-		expect(service['isConnectionDialogOpen']).toBeTrue();
+		// several almost-simultaneous calls
+		service.openConnectionDialog('T', 'D', false);
+		service.openConnectionDialog('T', 'D', false);
+		tick(0);
+		service.openConnectionDialog('T', 'D', false);
+		tick(0);
 
-		// Try to open the dialog again
-		service.openConnectionDialog('Test Title', 'Test Description', false);
-		service.openConnectionDialog('Test Title', 'Test Description', false);
-		service.openConnectionDialog('Test Title', 'Test Description', false);
+		expect(spy).toHaveBeenCalledTimes(1);
+	}));
 
-		// Verify that the dialog has been called only once
-		expect(dialogSpy).toHaveBeenCalledTimes(1);
+	it('launchNotification uses snackbar and triggers callback on action', fakeAsync(() => {
+		const snackBar = TestBed.inject(
+			(window as any).ng && (window as any).ng.material
+				? (window as any).ng.material.MatSnackBar
+				: (require('@angular/material/snack-bar') as any).MatSnackBar
+		) as any;
+		// Fallback: inject via TestBed
+		const snack = TestBed.inject(MatSnackBar);
+		const openSpy = spyOn(snack, 'open').and.returnValue({ onAction: () => of(null).pipe(delay(0)) } as any);
+
+		const callback = jasmine.createSpy('callback');
+		service.launchNotification({ message: 'hello', buttonActionText: 'OK' }, callback);
+		// allow the deferred observable to emit
+		tick();
+
+		expect(openSpy).toHaveBeenCalled();
+		expect(callback).toHaveBeenCalled();
+	}));
+
+	it('openDeleteRecordingDialog calls success callback when dialog closes with true', fakeAsync(() => {
+		const success = jasmine.createSpy('success');
+		service.openDeleteRecordingDialog(success);
+		// MatDialogRefMock.afterClosed returns of(true) so the subscription should call the callback
+		tick();
+		expect(success).toHaveBeenCalledTimes(1);
+	}));
+
+	it('openRecordingPlayerDialog triggers error handler when dialog returns manageError', fakeAsync(() => {
+		// Arrange: make dialog.open return a ref that afterClosed emits an object with manageError:true
+		const returnRef = {
+			afterClosed: () => ({ subscribe: (fn: any) => fn({ manageError: true, error: { code: 1 } }) }),
+			close: jasmine.createSpy('close')
+		} as any;
+		const openSpy = spyOn(dialog, 'open').and.returnValue(returnRef);
+		const handleSpy = spyOn<any>(service as any, 'handleRecordingPlayerError').and.callThrough();
+
+		// Act
+		service.openRecordingPlayerDialog('someSrc', true);
+		tick();
+
+		// Assert
+		expect(openSpy).toHaveBeenCalled();
+		expect(handleSpy).toHaveBeenCalled();
 	}));
 });

openvidu-components-angular/projects/openvidu-components-angular/src/lib/services/action/action.service.ts

@@ -19,9 +19,8 @@ export class ActionService {
 	private dialogRef:
 		| MatDialogRef<DialogTemplateComponent | RecordingDialogComponent | DeleteDialogComponent | ProFeatureDialogTemplateComponent>
 		| undefined;
-	private dialogSubscription: Subscription;
 	private connectionDialogRef: MatDialogRef<DialogTemplateComponent> | undefined;
-	private isConnectionDialogOpen: boolean = false;
+	private isConnectionDialogOpen = false;
 
 	constructor(
 		private snackBar: MatSnackBar,
@@ -29,7 +28,7 @@ export class ActionService {
 		private translateService: TranslateService
 	) {}
 
-	launchNotification(options: INotificationOptions, callback): void {
+	launchNotification(options: INotificationOptions, callback?: () => void): void {
 		if (!options.config) {
 			options.config = {
 				duration: 3000,
@@ -41,28 +40,23 @@ export class ActionService {
 
 		const notification = this.snackBar.open(options.message, options.buttonActionText, options.config);
 		if (callback) {
-			notification.onAction().subscribe(() => {
+			// subscribe and complete immediately after calling callback
+			const sub = notification.onAction().subscribe(() => {
+				sub.unsubscribe();
 				callback();
 			});
 		}
 	}
 
 	openDialog(titleMessage: string, descriptionMessage: string, allowClose = true) {
-		try {
-			this.closeDialog();
-		} catch (error) {
-		} finally {
-			const config: MatDialogConfig = {
-				minWidth: '250px',
-				data: { title: titleMessage, description: descriptionMessage, showActionButtons: allowClose },
-				disableClose: !allowClose
-			};
-			this.dialogRef = this.dialog.open(DialogTemplateComponent, config);
-			this.dialogSubscription = this.dialogRef.afterClosed().subscribe((result) => {
-				this.dialogRef = undefined;
-				if (this.dialogSubscription) this.dialogSubscription.unsubscribe();
-			});
-		}
+		this.closeDialog();
+		const config: MatDialogConfig = {
+			minWidth: '250px',
+			data: { title: titleMessage, description: descriptionMessage, showActionButtons: allowClose },
+			disableClose: !allowClose
+		};
+		this.dialogRef = this.dialog.open(DialogTemplateComponent, config);
+		this.dialogRef.afterClosed().subscribe(() => (this.dialogRef = undefined));
 	}
 
 	openConnectionDialog(titleMessage: string, descriptionMessage: string, allowClose = false) {
@@ -75,47 +69,44 @@ export class ActionService {
 
 		this.connectionDialogRef = this.dialog.open(DialogTemplateComponent, config);
 		this.isConnectionDialogOpen = true;
+		this.connectionDialogRef.afterClosed().subscribe(() => {
+			this.isConnectionDialogOpen = false;
+			this.connectionDialogRef = undefined;
+		});
 	}
 
-	openDeleteRecordingDialog(succsessCallback) {
-		try {
-			this.closeDialog();
-		} catch (error) {
-		} finally {
-			this.dialogRef = this.dialog.open(DeleteDialogComponent);
-
-			this.dialogSubscription = this.dialogRef.afterClosed().subscribe((result) => {
-				if (result) {
-					succsessCallback();
-					if (this.dialogSubscription) this.dialogSubscription.unsubscribe();
-				}
-			});
-		}
+	openDeleteRecordingDialog(successCallback: () => void) {
+		this.closeDialog();
+		this.dialogRef = this.dialog.open(DeleteDialogComponent);
+		this.dialogRef.afterClosed().subscribe((result) => {
+			if (result) {
+				successCallback();
+			}
+			this.dialogRef = undefined;
+		});
 	}
 
 	openRecordingPlayerDialog(src: string, allowClose = true) {
-		try {
-			this.closeDialog();
-		} catch (error) {
-		} finally {
-			const config: MatDialogConfig = {
-				minWidth: '250px',
-				data: { src, showActionButtons: allowClose },
-				disableClose: !allowClose
-			};
-			this.dialogRef = this.dialog.open(RecordingDialogComponent, config);
-
-			this.dialogSubscription = this.dialogRef.afterClosed().subscribe((data: { manageError: boolean; error: MediaError | null }) => {
-				if (data.manageError) {
-					this.handleRecordingPlayerError(data.error);
-				}
-				if (this.dialogSubscription) this.dialogSubscription.unsubscribe();
-			});
-		}
+		this.closeDialog();
+		const config: MatDialogConfig = {
+			minWidth: '250px',
+			data: { src, showActionButtons: allowClose },
+			disableClose: !allowClose
+		};
+		this.dialogRef = this.dialog.open(RecordingDialogComponent, config);
+		this.dialogRef.afterClosed().subscribe((data: { manageError: boolean; error: MediaError | null }) => {
+			if (data && data.manageError) {
+				this.handleRecordingPlayerError(data.error);
+			}
+			this.dialogRef = undefined;
+		});
 	}
 
 	closeDialog() {
-		this.dialogRef?.close();
+		if (this.dialogRef) {
+			this.dialogRef.close();
+			this.dialogRef = undefined;
+		}
 	}
 
 	closeConnectionDialog() {

openvidu-components-angular/projects/openvidu-components-angular/src/test-helpers/action.service.mock.ts

@@ -0,0 +1,51 @@
+import { MatDialogRef } from '@angular/material/dialog';
+import { Subject, of } from 'rxjs';
+export class ActionServiceMock {
+	openConnectionDialog(title?: string, description?: string, allowClose?: boolean): void {}
+	closeConnectionDialog(): void {}
+	openDialog(title?: string, description?: string, allowClose?: boolean): void {}
+	openDeleteRecordingDialog(callback?: () => void): void {
+		if (callback) callback();
+	}
+	openRecordingPlayerDialog(src?: string, allowClose?: boolean): void {}
+	launchNotification(options?: any, callback?: () => void): void {
+		if (callback) callback();
+	}
+}
+
+export class MatDialogRefMock {
+	private closed$ = new Subject<boolean>();
+	// expose a jasmine spy for close so tests can assert it was called
+	close = jasmine.createSpy('close').and.callFake(() => {
+		// when close is called, emit and complete the closed observable
+		this.closed$.next(true);
+		this.closed$.complete();
+	});
+
+	afterClosed() {
+		// return an observable that only emits when close() is called
+		return this.closed$.asObservable();
+	}
+}
+
+export class MatDialogMock {
+	opens = 0;
+	lastRef: MatDialogRefMock | null = null;
+
+	open(component?: any) {
+		this.opens++;
+		// If the consumer opens the DeleteDialogComponent, return a ref that emits immediately
+		// (some tests expect afterClosed to already have emitted for confirm/delete dialogs)
+		if (component && component.name === 'DeleteDialogComponent') {
+			const immediateRef: any = {
+				close: jasmine.createSpy('close'),
+				afterClosed: () => of(true)
+			};
+			this.lastRef = immediateRef as unknown as MatDialogRefMock;
+			return immediateRef as unknown as MatDialogRef<any>;
+		}
+
+		this.lastRef = new MatDialogRefMock();
+		return this.lastRef as unknown as MatDialogRef<any>;
+	}
+}

openvidu-components-angular/projects/openvidu-components-angular/src/test-helpers/translate.service.mock.ts

@@ -0,0 +1,10 @@
+import { of } from 'rxjs';
+
+export class TranslateServiceMock {
+	instant(key: string): string {
+		return key;
+	}
+	get(key: string) {
+		return of(key);
+	}
+}