Skip to content

Commit 6be61b0

Browse files
committed
feat(platform-notifications): PR Fixes
1 parent e373906 commit 6be61b0

File tree

7 files changed

+118
-32
lines changed

7 files changed

+118
-32
lines changed

apps/webapp/app/components/navigation/NotificationPanel.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,23 @@ function NotificationCard({
261261
)}
262262
</div>
263263

264-
{image && (
264+
{image && isSafeImageUrl(image) && (
265265
<img src={image} alt="" className="mt-1.5 rounded" />
266266
)}
267267
</div>
268268
</Wrapper>
269269
);
270270
}
271271

272+
function isSafeImageUrl(url: string): boolean {
273+
try {
274+
const parsed = new URL(url);
275+
return parsed.protocol === "https:" || parsed.protocol === "http:";
276+
} catch {
277+
return false;
278+
}
279+
}
280+
272281
function getMarkdownComponents(onLinkClick: () => void) {
273282
return {
274283
p: ({ children }: { children?: React.ReactNode }) => (
@@ -280,7 +289,10 @@ function getMarkdownComponents(onLinkClick: () => void) {
280289
target="_blank"
281290
rel="noopener noreferrer"
282291
className="text-indigo-400 underline hover:text-indigo-300 transition-colors"
283-
onClick={onLinkClick}
292+
onClick={(e) => {
293+
e.stopPropagation();
294+
onLinkClick();
295+
}}
284296
>
285297
{children}
286298
</a>

apps/webapp/app/routes/admin.api.v1.platform-notifications.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ export async function action({ request }: ActionFunctionArgs) {
4242
return json({ error: message }, { status });
4343
}
4444

45-
const body = await request.json();
45+
let body: unknown;
46+
try {
47+
body = await request.json();
48+
} catch {
49+
return json({ error: "Invalid JSON body" }, { status: 400 });
50+
}
4651
const result = await createPlatformNotification(body as CreatePlatformNotificationInput);
4752

4853
if (result.isErr()) {

apps/webapp/app/routes/admin.notifications.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -712,8 +712,7 @@ function NotificationPreviewCard({
712712
)}
713713
</div>
714714

715-
{/* lgtm[js/xss-through-dom] React JSX sets src via setAttribute, not raw HTML interpolation — safe from XSS */}
716-
{image && (
715+
{image && isSafeImageUrl(image) && (
717716
<img src={image} alt="" className="mt-1.5 rounded px-2 pb-2" />
718717
)}
719718
</div>
@@ -793,6 +792,15 @@ function defaultEndsAt(): string {
793792
return toDatetimeLocalUTC(new Date(Date.now() + 30 * 24 * 60 * 60 * 1000));
794793
}
795794

795+
function isSafeImageUrl(url: string): boolean {
796+
try {
797+
const parsed = new URL(url);
798+
return parsed.protocol === "https:" || parsed.protocol === "http:";
799+
} catch {
800+
return false;
801+
}
802+
}
803+
796804
type NotificationStatus = "active" | "pending" | "expired" | "archived";
797805

798806
function getNotificationStatus(n: {

apps/webapp/app/routes/resources.platform-notifications.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ const POLL_INTERVAL_MS = 60000; // 1 minute
3434

3535
export function usePlatformNotifications(organizationId: string, projectId: string) {
3636
const fetcher = useFetcher<typeof loader>();
37-
const hasInitiallyFetched = useRef(false);
37+
const lastLoadedUrl = useRef<string | null>(null);
3838

3939
useEffect(() => {
4040
const url = `/resources/platform-notifications?organizationId=${encodeURIComponent(organizationId)}&projectId=${encodeURIComponent(projectId)}`;
4141

42-
if (!hasInitiallyFetched.current && fetcher.state === "idle") {
43-
hasInitiallyFetched.current = true;
42+
if (lastLoadedUrl.current !== url && fetcher.state === "idle") {
43+
lastLoadedUrl.current = url;
4444
fetcher.load(url);
4545
}
4646

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { Redis } from "ioredis";
2+
import { env } from "~/env.server";
3+
import { singleton } from "~/utils/singleton";
4+
import { logger } from "./logger.server";
5+
6+
const KEY_PREFIX = "cli-notif-ctr:";
7+
const MAX_COUNTER = 1000;
8+
9+
function initializeRedis(): Redis | undefined {
10+
const host = env.CACHE_REDIS_HOST;
11+
if (!host) return undefined;
12+
13+
return new Redis({
14+
connectionName: "platformNotificationCounter",
15+
host,
16+
port: env.CACHE_REDIS_PORT,
17+
username: env.CACHE_REDIS_USERNAME,
18+
password: env.CACHE_REDIS_PASSWORD,
19+
keyPrefix: "tr:",
20+
enableAutoPipelining: true,
21+
...(env.CACHE_REDIS_TLS_DISABLED === "true" ? {} : { tls: {} }),
22+
});
23+
}
24+
25+
const redis = singleton("platformNotificationCounter", initializeRedis);
26+
27+
/** Increment and return the user's CLI request counter (1-based, wraps at 1001→1). */
28+
export async function incrementCliRequestCounter(userId: string): Promise<number> {
29+
if (!redis) return 1;
30+
31+
try {
32+
const key = `${KEY_PREFIX}${userId}`;
33+
const value = await redis.incr(key);
34+
35+
if (value > MAX_COUNTER) {
36+
await redis.set(key, "1");
37+
return 1;
38+
}
39+
40+
return value;
41+
} catch (error) {
42+
logger.error("Failed to increment CLI notification counter", { userId, error });
43+
return 1;
44+
}
45+
}

apps/webapp/app/services/platformNotifications.server.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from "zod";
22
import { errAsync, fromPromise, type ResultAsync } from "neverthrow";
33
import { prisma } from "~/db.server";
44
import { type PlatformNotificationScope, type PlatformNotificationSurface } from "@trigger.dev/database";
5+
import { incrementCliRequestCounter } from "./platformNotificationCounter.server";
56

67
// --- Payload schema (spec v1) ---
78

@@ -275,6 +276,9 @@ export async function recordNotificationClicked({
275276
// --- Read: recent changelogs (for Help & Feedback) ---
276277

277278
export async function getRecentChangelogs({ limit = 2 }: { limit?: number } = {}) {
279+
// NOTE: Intentionally not filtering by archivedAt, startsAt, or endsAt.
280+
// We want to show archived and expired changelogs in the "What's new" section
281+
// so users can still find recent release notes.
278282
const notifications = await prisma.platformNotification.findMany({
279283
where: {
280284
surface: "WEBAPP",
@@ -398,6 +402,11 @@ export async function getNextCliNotification({
398402

399403
const sorted = [...notifications].sort(compareNotifications);
400404

405+
// Global per-user request counter stored in Redis, used for cliShowEvery modulo.
406+
// This is independent of per-notification showCount so that cliMaxShowCount
407+
// correctly tracks actual displays, not API encounters.
408+
const requestCounter = await incrementCliRequestCounter(userId);
409+
401410
for (const n of sorted) {
402411
const interaction = n.interactions[0] ?? null;
403412

@@ -407,6 +416,12 @@ export async function getNextCliNotification({
407416
const parsed = PayloadV1Schema.safeParse(n.payload);
408417
if (!parsed.success) continue;
409418

419+
// Check cliShowEvery using the global request counter
420+
if (n.cliShowEvery !== null && requestCounter % n.cliShowEvery !== 0) {
421+
continue;
422+
}
423+
424+
// Only increment showCount when the notification will actually be displayed
410425
const updated = await prisma.platformNotificationInteraction.upsert({
411426
where: { notificationId_userId: { notificationId: n.id, userId } },
412427
update: { showCount: { increment: 1 } },
@@ -418,11 +433,6 @@ export async function getNextCliNotification({
418433
},
419434
});
420435

421-
// If cliShowEvery is set, only return on every N-th request
422-
if (n.cliShowEvery !== null && updated.showCount % n.cliShowEvery !== 0) {
423-
continue;
424-
}
425-
426436
return {
427437
id: n.id,
428438
payload: parsed.data,

packages/cli-v3/src/utilities/platformNotifications.ts

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,26 +95,32 @@ export async function awaitAndDisplayPlatformNotification(
9595
): Promise<void> {
9696
if (!notificationPromise) return;
9797

98-
// Race against a short delay — if the promise resolves quickly, skip the spinner
99-
const pending = Symbol("pending");
100-
const raceResult = await Promise.race([
101-
notificationPromise,
102-
new Promise<typeof pending>((resolve) => setTimeout(() => resolve(pending), SPINNER_DELAY_MS)),
103-
]);
104-
105-
if (raceResult !== pending) {
106-
displayPlatformNotification(raceResult);
107-
return;
108-
}
98+
try {
99+
// Race against a short delay — if the promise resolves quickly, skip the spinner
100+
const pending = Symbol("pending");
101+
const raceResult = await Promise.race([
102+
notificationPromise,
103+
new Promise<typeof pending>((resolve) =>
104+
setTimeout(() => resolve(pending), SPINNER_DELAY_MS)
105+
),
106+
]);
107+
108+
if (raceResult !== pending) {
109+
displayPlatformNotification(raceResult);
110+
return;
111+
}
109112

110-
// Still pending after delay — show a spinner while waiting
111-
const $spinner = spinner();
112-
$spinner.start("Checking for notifications");
113-
const notification = await notificationPromise;
113+
// Still pending after delay — show a spinner while waiting
114+
const $spinner = spinner();
115+
$spinner.start("Checking for notifications");
116+
const notification = await notificationPromise;
114117

115-
if (notification) {
116-
$spinner.stop(formatNotificationMessage(notification));
117-
} else {
118-
$spinner.stop("No new notifications");
118+
if (notification) {
119+
$spinner.stop(formatNotificationMessage(notification));
120+
} else {
121+
$spinner.stop("No new notifications");
122+
}
123+
} catch (error) {
124+
logger.debug("Platform notification display failed silently", { error });
119125
}
120126
}

0 commit comments

Comments
 (0)