fix: engagement dashboard becoming unresponsive when clicking "See on Engagement Dashboard" (RocketChat#40058)

Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
Co-authored-by: Douglas Fabris <devfabris@gmail.com>

Author: abhinavkrin

.changeset/cool-parents-buy.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/meteor': patch
+'@rocket.chat/mock-providers': patch
+---
+
+Fixed UI becoming unresponsive after clicking "See on Engagement Dashboard" from the workspace info card, which required a manual page refresh to recover.

apps/meteor/client/views/admin/engagementDashboard/EngagementDashboardRoute.spec.tsx

@@ -0,0 +1,137 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import type { RouterContextValue } from '@rocket.chat/ui-contexts';
+import { render } from '@testing-library/react';
+
+import EngagementDashboardRoute from './EngagementDashboardRoute';
+import { useUpsellActions } from '../../../components/GenericUpsellModal/hooks';
+import { useHasLicenseModule } from '../../../hooks/useHasLicenseModule';
+
+jest.mock('../../../hooks/useHasLicenseModule', () => ({ useHasLicenseModule: jest.fn() }));
+jest.mock('../../../components/GenericUpsellModal/hooks', () => ({ useUpsellActions: jest.fn() }));
+jest.mock('./EngagementDashboardPage', () => () => null);
+jest.mock('../../../components/PageSkeleton', () => () => null);
+jest.mock('../../notAuthorized/NotAuthorizedPage', () => () => null);
+jest.mock('../../../../app/utils/client/getURL', () => ({ getURL: (path: string) => path }));
+
+const buildWrapper = (router: Partial<RouterContextValue>, tab?: string) => {
+	const builder = mockAppRoot()
+		.withPermission('view-engagement-dashboard')
+		.withEndpoint('POST', '/v1/statistics.telemetry', () => ({}))
+		.withRouter(router);
+
+	if (tab) {
+		builder.withRouteParameter('tab', tab);
+	}
+
+	return builder.build();
+};
+
+const createSubscribeTracker = () => {
+	const unsubscribes: jest.Mock[] = [];
+
+	const subscribeToRouteChange = jest.fn((_cb: () => void) => {
+		const unsub = jest.fn();
+		unsubscribes.push(unsub);
+		return unsub;
+	});
+
+	const allUnsubscribed = () => unsubscribes.length > 0 && unsubscribes.every((u) => u.mock.calls.length > 0);
+
+	const noneUnsubscribed = () => unsubscribes.every((u) => u.mock.calls.length === 0);
+
+	const getLastRouteChangeCallback = () => {
+		const callback = subscribeToRouteChange.mock.calls.at(-1)?.[0];
+		expect(callback).toBeDefined();
+		return callback as () => void;
+	};
+
+	return { subscribeToRouteChange, allUnsubscribed, noneUnsubscribed, getLastRouteChangeCallback };
+};
+
+beforeEach(() => {
+	jest.clearAllMocks();
+
+	jest.mocked(useHasLicenseModule).mockReturnValue({
+		isPending: false,
+		data: true,
+	} as unknown as ReturnType<typeof useHasLicenseModule>);
+
+	jest.mocked(useUpsellActions).mockReturnValue({
+		shouldShowUpsell: false,
+		cloudWorkspaceHadTrial: false,
+		handleManageSubscription: jest.fn(),
+		handleTalkToSales: jest.fn(),
+	});
+});
+
+describe('EngagementDashboardRoute', () => {
+	it('subscribes to route changes on mount', () => {
+		const { subscribeToRouteChange } = createSubscribeTracker();
+
+		render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange }, 'users'),
+		});
+
+		expect(subscribeToRouteChange).toHaveBeenCalled();
+	});
+
+	it('unsubscribes all listeners on unmount', () => {
+		const { subscribeToRouteChange, allUnsubscribed } = createSubscribeTracker();
+
+		const { unmount } = render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange }, 'users'),
+		});
+
+		unmount();
+		expect(allUnsubscribed()).toBe(true);
+	});
+
+	it('does not redirect when the tab is valid', () => {
+		const navigate = jest.fn();
+		const { subscribeToRouteChange, getLastRouteChangeCallback } = createSubscribeTracker();
+
+		render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange, navigate }, 'users'),
+		});
+
+		const routeChangeCallback = getLastRouteChangeCallback();
+		routeChangeCallback();
+
+		expect(navigate).not.toHaveBeenCalled();
+	});
+
+	it('redirects to the users tab when the tab is invalid', () => {
+		const navigate = jest.fn();
+		const { subscribeToRouteChange, getLastRouteChangeCallback } = createSubscribeTracker();
+
+		render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange, navigate }),
+		});
+
+		const routeChangeCallback = getLastRouteChangeCallback();
+		routeChangeCallback();
+
+		expect(navigate).toHaveBeenCalledWith({ pattern: '/admin/engagement/:tab?', params: { tab: 'users' } }, { replace: true });
+	});
+
+	it('cleans up all subscriptions on unmount and resubscribes on remount', () => {
+		const first = createSubscribeTracker();
+
+		const { unmount } = render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange: first.subscribeToRouteChange }, 'users'),
+		});
+
+		expect(first.subscribeToRouteChange).toHaveBeenCalled();
+		unmount();
+		expect(first.allUnsubscribed()).toBe(true);
+
+		const second = createSubscribeTracker();
+
+		render(<EngagementDashboardRoute />, {
+			wrapper: buildWrapper({ subscribeToRouteChange: second.subscribeToRouteChange }, 'messages'),
+		});
+
+		expect(second.subscribeToRouteChange).toHaveBeenCalled();
+		expect(second.noneUnsubscribed()).toBe(true);
+	});
+});

apps/meteor/client/views/admin/engagementDashboard/EngagementDashboardRoute.tsx

@@ -50,8 +50,10 @@ const EngagementDashboardRoute = (): ReactElement | null => {
 				/>,
 			);
 		}
+	}, [shouldShowUpsell, setModal, t, handleManageSubscription]);
 
-		router.subscribeToRouteChange(() => {
+	useEffect(() => {
+		return router.subscribeToRouteChange(() => {
 			if (!isValidTab(tab)) {
 				router.navigate(
 					{
@@ -62,7 +64,7 @@ const EngagementDashboardRoute = (): ReactElement | null => {
 				);
 			}
 		});
-	}, [shouldShowUpsell, router, tab, setModal, t, handleManageSubscription]);
+	}, [router, tab]);
 
 	if (isModalOpen || isPending) {
 		return <PageSkeleton />;

packages/mock-providers/src/MockedAppRootBuilder.tsx

@@ -474,6 +474,17 @@ export class MockedAppRootBuilder {
 		return this;
 	}
 
+	withRouter(overrides: Partial<ContextType<typeof RouterContext>>): this {
+		this.router = { ...this.router, ...overrides };
+		return this;
+	}
+
+	withRouteParameter(name: string, value: string): this {
+		const innerFn = this.router.getRouteParameters;
+		this.router.getRouteParameters = () => ({ ...innerFn(), [name]: value });
+		return this;
+	}
+
 	withRole(role: string): this {
 		if (!this.user.user) {
 			throw new Error('user is not defined');