Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ import { setupOutlookLogger } from './outlookCalendar/logger';
import { handleDesktopCapturerGetSources } from './screenSharing/desktopCapturerCache';
import { setupScreenSharing } from './screenSharing/main';
import { startServerViewScreenSharingHandler } from './screenSharing/serverViewScreenSharing';
import { handleClearCacheDialog } from './servers/cache';
import {
handleClearCacheDialog,
handleUserLoggedOutDataClearing,
} from './servers/cache';
import { setupServers } from './servers/main';
import { checkSupportedVersionServers } from './servers/supportedVersions/main';
import { setupSpellChecking } from './spellChecking/main';
Expand Down Expand Up @@ -155,6 +158,7 @@ const start = async (): Promise<void> => {
handleJitsiDesktopCapturerGetSources();
handleDesktopCapturerGetSources();
handleClearCacheDialog();
handleUserLoggedOutDataClearing();
startDocumentViewerHandler();
startBrowserHandler();
checkSupportedVersionServers();
Expand Down
26 changes: 26 additions & 0 deletions src/servers/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { listen } from '../store';
import {
CLEAR_CACHE_DIALOG_DELETE_LOGIN_DATA_CLICKED,
CLEAR_CACHE_DIALOG_KEEP_LOGIN_DATA_CLICKED,
WEBVIEW_USER_LOGGED_IN,
} from '../ui/actions';
import { getWebContentsByServerUrl } from '../ui/main/serverView';
import type { Server } from './common';

export const clearWebviewStorageKeepingLoginData = async (
guestWebContents: WebContents
Expand Down Expand Up @@ -51,3 +54,26 @@ export const handleClearCacheDialog = () => {
await clearWebviewStorageDeletingLoginData(guestWebContents);
});
};

const previousUserLoggedInByUrl = new Map<
Server['url'],
Server['userLoggedIn']
>();

export const handleUserLoggedOutDataClearing = (): void => {
listen(WEBVIEW_USER_LOGGED_IN, async (action) => {
const { url, userLoggedIn } = action.payload;
const wasLoggedIn = previousUserLoggedInByUrl.get(url);
previousUserLoggedInByUrl.set(url, userLoggedIn);

if (wasLoggedIn !== true || userLoggedIn !== false) {
return;
}

const guestWebContents = getWebContentsByServerUrl(url);
if (!guestWebContents) {
return;
}
await clearWebviewStorageDeletingLoginData(guestWebContents);
});
};
101 changes: 101 additions & 0 deletions src/servers/main/cache.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import type { WebContents } from 'electron';

import { listen } from '../../store';
import type { RootAction } from '../../store/actions';
import { WEBVIEW_USER_LOGGED_IN } from '../../ui/actions';
import { getWebContentsByServerUrl } from '../../ui/main/serverView';
import { handleUserLoggedOutDataClearing } from '../cache';

jest.mock('electron', () => ({
webContents: {
fromId: jest.fn(),
},
}));

jest.mock('../../store', () => ({
listen: jest.fn(),
}));

jest.mock('../../ui/main/serverView', () => ({
getWebContentsByServerUrl: jest.fn(),
}));

describe('servers/cache handleUserLoggedOutDataClearing', () => {
const mockListen = listen as unknown as jest.Mock<
() => void,
[string, (action: RootAction) => void]
>;
const mockGetWebContentsByServerUrl =
getWebContentsByServerUrl as jest.MockedFunction<
typeof getWebContentsByServerUrl
>;

const url = 'https://open.rocket.chat/';

const createMockWebContents = (): WebContents =>
({
session: {
clearCache: jest.fn().mockResolvedValue(undefined),
clearStorageData: jest.fn().mockResolvedValue(undefined),
},
reloadIgnoringCache: jest.fn(),
}) as unknown as WebContents;

const dispatchUserLoggedIn = async (userLoggedIn: boolean) => {
const [, listener] = mockListen.mock.calls.find(
([type]) => type === WEBVIEW_USER_LOGGED_IN
)!;
await listener({
type: WEBVIEW_USER_LOGGED_IN,
payload: { url, userLoggedIn },
} as any);
};

beforeEach(() => {
jest.clearAllMocks();
handleUserLoggedOutDataClearing();
});
Comment on lines +54 to +57

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reset previousUserLoggedInByUrl between tests for proper isolation.

jest.clearAllMocks() resets mock call history but does not reset the module-level previousUserLoggedInByUrl Map in cache.ts. After test 1 dispatches true then false, the map retains { url: false }. Test 2 ("does not clear on the initial logged-out state at startup/attach") then runs with wasLoggedIn = false instead of undefined — it passes, but doesn't exercise the actual initial-state path described in its name.

If the transition condition were ever changed to distinguish undefined from false, this test would give false confidence. Use jest.isolateModules to get a fresh module registry per test:

🧪 Proposed fix for test isolation
 beforeEach(() => {
   jest.clearAllMocks();
-  handleUserLoggedOutDataClearing();
+  jest.isolateModules(() => {
+    // eslint-disable-next-line `@typescript-eslint/no-var-requires`
+    const { handleUserLoggedOutDataClearing } = require('../cache');
+    handleUserLoggedOutDataClearing();
+  });
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/servers/main/cache.spec.ts` around lines 54 - 57, The cache.spec.ts setup
only clears mocks, but the module-level previousUserLoggedInByUrl state in
cache.ts persists across tests and can hide the initial-state path. Update the
tests to load the cache module with fresh module state per case, using
jest.isolateModules around the imports that define
handleUserLoggedOutDataClearing, so previousUserLoggedInByUrl starts empty for
each test and the “initial logged-out state” scenario truly exercises the
undefined transition.


it('clears webview storage on a logged-in -> logged-out transition', async () => {
const mockWebContents = createMockWebContents();
mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents);

await dispatchUserLoggedIn(true);
await dispatchUserLoggedIn(false);

expect(mockGetWebContentsByServerUrl).toHaveBeenCalledWith(url);
expect(mockWebContents.session.clearCache).toHaveBeenCalled();
expect(mockWebContents.session.clearStorageData).toHaveBeenCalledWith();
expect(mockWebContents.reloadIgnoringCache).toHaveBeenCalled();
});

it('does not clear on the initial logged-out state at startup/attach', async () => {
const mockWebContents = createMockWebContents();
mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents);

await dispatchUserLoggedIn(false);

expect(mockGetWebContentsByServerUrl).not.toHaveBeenCalled();
expect(mockWebContents.session.clearStorageData).not.toHaveBeenCalled();
});

it('does not clear on a logged-out -> logged-in transition', async () => {
const mockWebContents = createMockWebContents();
mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents);

await dispatchUserLoggedIn(false);
await dispatchUserLoggedIn(true);

expect(mockGetWebContentsByServerUrl).not.toHaveBeenCalled();
expect(mockWebContents.session.clearStorageData).not.toHaveBeenCalled();
});

it('is a safe no-op when no webContents is found for the url', async () => {
mockGetWebContentsByServerUrl.mockReturnValue(undefined);

await dispatchUserLoggedIn(true);
await expect(dispatchUserLoggedIn(false)).resolves.not.toThrow();

expect(mockGetWebContentsByServerUrl).toHaveBeenCalledWith(url);
});
});
Loading