Skip to content
Open
Changes from 1 commit
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
52 changes: 30 additions & 22 deletions apps/code/src/renderer/utils/notifications.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useSettingsStore } from "@features/settings/stores/settingsStore";
import { trpcClient } from "@renderer/trpc/client";
import { useNavigationStore } from "@stores/navigationStore";
import { logger } from "@utils/logger";
import { playCompletionSound } from "@utils/sounds";

Expand All @@ -12,6 +13,15 @@ function truncateTitle(title: string): string {
return `${title.slice(0, MAX_TITLE_LENGTH)}...`;
}

function shouldNotifyForTask(taskId?: string): boolean {
if (!document.hasFocus()) return true;
if (!taskId) return false;
const view = useNavigationStore.getState().view;
const viewedTaskId =
view.type === "task-detail" ? (view.data?.id ?? view.taskId) : undefined;
return viewedTaskId !== taskId;
}
Comment on lines +16 to +23
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.

P2 Missing automated tests for shouldNotifyForTask

The new helper has three distinct, clearly-testable branches (unfocused → always notify; focused + no taskId → never notify; focused + taskId → compare with viewed task), yet no test file exists for notifications.ts. Given the vitest infrastructure already present (see navigationStore.test.ts for the mocking pattern), and simplicity rule #1 ("Passes all the tests"), these branches should have parameterised unit tests covering at minimum: window unfocused, window focused with matching taskId, window focused with different taskId, and window focused with no taskId.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/utils/notifications.ts
Line: 16-23

Comment:
**Missing automated tests for `shouldNotifyForTask`**

The new helper has three distinct, clearly-testable branches (unfocused → always notify; focused + no taskId → never notify; focused + taskId → compare with viewed task), yet no test file exists for `notifications.ts`. Given the vitest infrastructure already present (see `navigationStore.test.ts` for the mocking pattern), and simplicity rule #1 ("Passes all the tests"), these branches should have parameterised unit tests covering at minimum: window unfocused, window focused with matching taskId, window focused with different taskId, and window focused with no taskId.

How can I resolve this? If you propose a fix, please make it concise.


function sendDesktopNotification(
title: string,
body: string,
Expand Down Expand Up @@ -52,8 +62,7 @@ export function notifyPromptComplete(
dockBounceNotifications,
} = useSettingsStore.getState();

const isWindowFocused = document.hasFocus();
if (isWindowFocused) return;
if (!shouldNotifyForTask(taskId)) return;

const willPlayCustomSound = completionSound !== "none";
playCompletionSound(completionSound, completionVolume);
Expand Down Expand Up @@ -85,25 +94,24 @@ export function notifyPermissionRequest(
dockBadgeNotifications,
dockBounceNotifications,
} = useSettingsStore.getState();
const isWindowFocused = document.hasFocus();

if (!isWindowFocused) {
const willPlayCustomSound = completionSound !== "none";
playCompletionSound(completionSound, completionVolume);

if (desktopNotifications) {
sendDesktopNotification(
"PostHog Code",
`"${truncateTitle(taskTitle)}" needs your input`,
willPlayCustomSound,
taskId,
);
}
if (dockBadgeNotifications) {
showDockBadge();
}
if (dockBounceNotifications) {
bounceDock();
}

if (!shouldNotifyForTask(taskId)) return;

const willPlayCustomSound = completionSound !== "none";
playCompletionSound(completionSound, completionVolume);

if (desktopNotifications) {
sendDesktopNotification(
"PostHog Code",
`"${truncateTitle(taskTitle)}" needs your input`,
willPlayCustomSound,
taskId,
);
}
if (dockBadgeNotifications) {
showDockBadge();
}
if (dockBounceNotifications) {
bounceDock();
}
}
Loading