fix: Unread notificaitons on startup
This commit is contained in:
@@ -25,6 +25,13 @@ Durable rules for AI agents working on this project. Read this file at session s
|
|||||||
|
|
||||||
## Lessons
|
## Lessons
|
||||||
|
|
||||||
|
### Re-clear visible notification channels after recompute [notifications] [startup]
|
||||||
|
|
||||||
|
- **Trigger:** fixing startup unread badges by only changing read-marker writes or initial hydration.
|
||||||
|
- **Rule:** also check later `loadMessagesSuccess` and `syncMessages` recomputes, and re-clear the focused visible channel after applying derived unread counts.
|
||||||
|
- **Why:** the startup-selected server can load or sync messages after it was marked read, reintroducing a channel unread badge even though the user is viewing that channel.
|
||||||
|
- **Example:** `NotificationsService.refreshRoomUnreadFromMessages(...)` should clear `activeChannelId` for `currentRoom` after recalculating counts from a startup message batch.
|
||||||
|
|
||||||
### Disambiguate nested chat cards [chat] [ui]
|
### Disambiguate nested chat cards [chat] [ui]
|
||||||
|
|
||||||
- **Trigger:** removing a visual treatment from chat history when a system message has both an outer row wrapper and an inner pill/card.
|
- **Trigger:** removing a visual treatment from chat history when a system message has both an outer row wrapper and an inner pill/card.
|
||||||
|
|||||||
@@ -143,6 +143,8 @@ Unread state is modeled as a combination of persisted read markers plus in-memor
|
|||||||
|
|
||||||
Important design constraint: unread counters are intentionally not persisted. The persisted state stores only the user-controlled settings and the read markers needed to derive unread counts again.
|
Important design constraint: unread counters are intentionally not persisted. The persisted state stores only the user-controlled settings and the read markers needed to derive unread counts again.
|
||||||
|
|
||||||
|
Read markers are written with the same server-adjusted clock that chat messages use, so a visible channel does not become unread again after reload when the signaling server clock is ahead of the local desktop clock.
|
||||||
|
|
||||||
### Why baselines exist
|
### Why baselines exist
|
||||||
|
|
||||||
The domain must avoid marking an entire historical backlog as unread the first time a room appears.
|
The domain must avoid marking an entire historical backlog as unread the first time a room appears.
|
||||||
@@ -176,6 +178,7 @@ Unread is cleared by channel, not globally.
|
|||||||
|
|
||||||
- `markCurrentChannelReadIfActive()` only runs when the window is focused and the document is visible.
|
- `markCurrentChannelReadIfActive()` only runs when the window is focused and the document is visible.
|
||||||
- If a live message arrives in the currently visible text channel, the domain immediately marks that channel read instead of incrementing unread.
|
- If a live message arrives in the currently visible text channel, the domain immediately marks that channel read instead of incrementing unread.
|
||||||
|
- When a startup message load or sync batch recomputes unread counts for the viewed server, the currently visible text channel is re-cleared and its read marker advances to the newest visible message in that batch.
|
||||||
- Window focus and document visibility changes both clear taskbar attention and mark the active channel read.
|
- Window focus and document visibility changes both clear taskbar attention and mark the active channel read.
|
||||||
- `pruneUnreadState()` removes deleted or non-text channels from the unread maps and re-derives room totals from channel totals.
|
- `pruneUnreadState()` removes deleted or non-text channels from the unread maps and re-derives room totals from channel totals.
|
||||||
|
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import type {
|
|||||||
User
|
User
|
||||||
} from '../../../../shared-kernel';
|
} from '../../../../shared-kernel';
|
||||||
import { NotificationAudioService } from '../../../../core/services/notification-audio.service';
|
import { NotificationAudioService } from '../../../../core/services/notification-audio.service';
|
||||||
|
import { TimeSyncService } from '../../../../core/services/time-sync.service';
|
||||||
import { DatabaseService } from '../../../../infrastructure/persistence';
|
import { DatabaseService } from '../../../../infrastructure/persistence';
|
||||||
import {
|
import {
|
||||||
selectActiveChannelId,
|
selectActiveChannelId,
|
||||||
@@ -33,6 +34,15 @@ const alice: User = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
describe('NotificationsService', () => {
|
describe('NotificationsService', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.setSystemTime(1_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
|
||||||
it('keeps channel read markers when startup room metadata has no channels yet', async () => {
|
it('keeps channel read markers when startup room metadata has no channels yet', async () => {
|
||||||
const roomWithoutChannels = createRoom({ channels: [] });
|
const roomWithoutChannels = createRoom({ channels: [] });
|
||||||
const context = createServiceContext({
|
const context = createServiceContext({
|
||||||
@@ -53,12 +63,92 @@ describe('NotificationsService', () => {
|
|||||||
|
|
||||||
expect(context.service.settings().lastReadByChannel['room-1']?.['channel-1']).toBe(100);
|
expect(context.service.settings().lastReadByChannel['room-1']?.['channel-1']).toBe(100);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('uses the server-adjusted clock when marking the active channel read', async () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const context = createServiceContext({
|
||||||
|
activeChannelId: 'channel-1',
|
||||||
|
currentRoom: room,
|
||||||
|
currentUser: alice,
|
||||||
|
savedRooms: [room],
|
||||||
|
settings: {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
},
|
||||||
|
timeNow: 3_000
|
||||||
|
});
|
||||||
|
|
||||||
|
await context.service.initialize();
|
||||||
|
|
||||||
|
expect(context.service.settings().lastReadByChannel['room-1']?.['channel-1']).toBe(3_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the visible active channel read when startup message loads recompute unread', async () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const context = createServiceContext({
|
||||||
|
activeChannelId: 'channel-1',
|
||||||
|
currentRoom: room,
|
||||||
|
currentUser: alice,
|
||||||
|
savedRooms: [room],
|
||||||
|
settings: {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
},
|
||||||
|
timeNow: 100
|
||||||
|
});
|
||||||
|
|
||||||
|
await context.service.initialize();
|
||||||
|
|
||||||
|
context.service.refreshRoomUnreadFromMessages('room-1', [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 200 })]);
|
||||||
|
|
||||||
|
expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(0);
|
||||||
|
expect(context.service.roomUnreadCount('room-1')).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not compute unread from messages before it is initialised', () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const context = createServiceContext({
|
||||||
|
currentUser: alice,
|
||||||
|
savedRooms: [room],
|
||||||
|
settings: createDefaultNotificationSettings()
|
||||||
|
});
|
||||||
|
|
||||||
|
context.service.refreshRoomUnreadFromMessages('room-1', [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 5_000 })]);
|
||||||
|
|
||||||
|
expect(context.service.roomUnreadCount('room-1')).toBe(0);
|
||||||
|
expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('hydrates unread counts from persisted read markers on initialize', async () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const context = createServiceContext({
|
||||||
|
activeChannelId: 'other',
|
||||||
|
currentRoom: null,
|
||||||
|
currentUser: alice,
|
||||||
|
dbMessages: [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 5_000 })],
|
||||||
|
savedRooms: [room],
|
||||||
|
settings: {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
},
|
||||||
|
timeNow: 10_000
|
||||||
|
});
|
||||||
|
|
||||||
|
await context.service.initialize();
|
||||||
|
|
||||||
|
expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(1);
|
||||||
|
expect(context.service.roomUnreadCount('room-1')).toBe(1);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
interface ServiceContextOptions {
|
interface ServiceContextOptions {
|
||||||
|
activeChannelId?: string | null;
|
||||||
|
currentRoom?: Room | null;
|
||||||
currentUser: User | null;
|
currentUser: User | null;
|
||||||
|
dbMessages?: Message[];
|
||||||
savedRooms: Room[];
|
savedRooms: Room[];
|
||||||
settings: NotificationsSettings;
|
settings: NotificationsSettings;
|
||||||
|
timeNow?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface ServiceContext {
|
interface ServiceContext {
|
||||||
@@ -68,8 +158,8 @@ interface ServiceContext {
|
|||||||
function createServiceContext(options: ServiceContextOptions): ServiceContext {
|
function createServiceContext(options: ServiceContextOptions): ServiceContext {
|
||||||
const currentUser = signal<User | null>(options.currentUser);
|
const currentUser = signal<User | null>(options.currentUser);
|
||||||
const savedRooms = signal<Room[]>(options.savedRooms);
|
const savedRooms = signal<Room[]>(options.savedRooms);
|
||||||
const currentRoom = signal<Room | null>(null);
|
const currentRoom = signal<Room | null>(options.currentRoom ?? null);
|
||||||
const activeChannelId = signal<string | null>(null);
|
const activeChannelId = signal<string | null>(options.activeChannelId ?? null);
|
||||||
|
|
||||||
let storedSettings = options.settings;
|
let storedSettings = options.settings;
|
||||||
|
|
||||||
@@ -78,7 +168,7 @@ function createServiceContext(options: ServiceContextOptions): ServiceContext {
|
|||||||
{
|
{
|
||||||
provide: DatabaseService,
|
provide: DatabaseService,
|
||||||
useValue: {
|
useValue: {
|
||||||
getMessagesSince: vi.fn(async (): Promise<Message[]> => [])
|
getMessagesSince: vi.fn(async (): Promise<Message[]> => options.dbMessages ?? [])
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -96,6 +186,12 @@ function createServiceContext(options: ServiceContextOptions): ServiceContext {
|
|||||||
play: vi.fn()
|
play: vi.fn()
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
provide: TimeSyncService,
|
||||||
|
useValue: {
|
||||||
|
now: vi.fn(() => options.timeNow ?? Date.now())
|
||||||
|
}
|
||||||
|
},
|
||||||
{
|
{
|
||||||
provide: NotificationSettingsStorageService,
|
provide: NotificationSettingsStorageService,
|
||||||
useValue: {
|
useValue: {
|
||||||
@@ -154,3 +250,18 @@ function createRoom(overrides: Partial<Room> = {}): Room {
|
|||||||
...overrides
|
...overrides
|
||||||
} as Room;
|
} as Room;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function createMessage(overrides: Partial<Message> = {}): Message {
|
||||||
|
return {
|
||||||
|
id: 'message-1',
|
||||||
|
roomId: 'room-1',
|
||||||
|
channelId: 'channel-1',
|
||||||
|
senderId: 'bob',
|
||||||
|
senderName: 'Bob',
|
||||||
|
content: 'hello',
|
||||||
|
timestamp: 1,
|
||||||
|
reactions: [],
|
||||||
|
isDeleted: false,
|
||||||
|
...overrides
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import {
|
|||||||
import { Store } from '@ngrx/store';
|
import { Store } from '@ngrx/store';
|
||||||
import type { Message, Room } from '../../../../shared-kernel';
|
import type { Message, Room } from '../../../../shared-kernel';
|
||||||
import { NotificationAudioService, AppSound } from '../../../../core/services/notification-audio.service';
|
import { NotificationAudioService, AppSound } from '../../../../core/services/notification-audio.service';
|
||||||
|
import { TimeSyncService } from '../../../../core/services/time-sync.service';
|
||||||
import { DatabaseService } from '../../../../infrastructure/persistence';
|
import { DatabaseService } from '../../../../infrastructure/persistence';
|
||||||
import {
|
import {
|
||||||
selectActiveChannelId,
|
selectActiveChannelId,
|
||||||
@@ -47,6 +48,7 @@ export class NotificationsService {
|
|||||||
private readonly store = inject(Store);
|
private readonly store = inject(Store);
|
||||||
private readonly db = inject(DatabaseService);
|
private readonly db = inject(DatabaseService);
|
||||||
private readonly audio = inject(NotificationAudioService);
|
private readonly audio = inject(NotificationAudioService);
|
||||||
|
private readonly timeSync = inject(TimeSyncService);
|
||||||
private readonly desktopNotifications = inject(DesktopNotificationService);
|
private readonly desktopNotifications = inject(DesktopNotificationService);
|
||||||
private readonly storage = inject(NotificationSettingsStorageService);
|
private readonly storage = inject(NotificationSettingsStorageService);
|
||||||
|
|
||||||
@@ -80,6 +82,7 @@ export class NotificationsService {
|
|||||||
this.registerWindowListeners();
|
this.registerWindowListeners();
|
||||||
this.registerWindowStateListener();
|
this.registerWindowStateListener();
|
||||||
this.syncRoomCatalog(this.savedRooms());
|
this.syncRoomCatalog(this.savedRooms());
|
||||||
|
await this.hydrateUnreadCounts(this.savedRooms());
|
||||||
this.markCurrentChannelReadIfActive();
|
this.markCurrentChannelReadIfActive();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -150,6 +153,10 @@ export class NotificationsService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
refreshRoomUnreadFromMessages(roomId: string, messages: Message[]): void {
|
refreshRoomUnreadFromMessages(roomId: string, messages: Message[]): void {
|
||||||
|
if (!this.initialised) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const room = getRoomById(this.savedRooms(), roomId);
|
const room = getRoomById(this.savedRooms(), roomId);
|
||||||
|
|
||||||
if (!room) {
|
if (!room) {
|
||||||
@@ -169,6 +176,7 @@ export class NotificationsService {
|
|||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
this.markVisibleChannelReadFromMessages(roomId, messages);
|
||||||
this.syncWindowAttention();
|
this.syncWindowAttention();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -462,8 +470,8 @@ export class NotificationsService {
|
|||||||
this.syncWindowAttention();
|
this.syncWindowAttention();
|
||||||
}
|
}
|
||||||
|
|
||||||
private markChannelRead(roomId: string, channelId: string, timestamp = Date.now()): void {
|
private markChannelRead(roomId: string, channelId: string, timestamp = this.timeSync.now()): void {
|
||||||
const nextReadAt = Math.max(timestamp, Date.now(), this.getChannelLastReadAt(roomId, channelId));
|
const nextReadAt = Math.max(timestamp, this.timeSync.now(), this.getChannelLastReadAt(roomId, channelId));
|
||||||
|
|
||||||
this.setSettings({
|
this.setSettings({
|
||||||
...this._settings(),
|
...this._settings(),
|
||||||
@@ -497,6 +505,23 @@ export class NotificationsService {
|
|||||||
this.syncWindowAttention();
|
this.syncWindowAttention();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private markVisibleChannelReadFromMessages(roomId: string, messages: Message[]): void {
|
||||||
|
if (!this._windowFocused() || !this._documentVisible() || this.currentRoom()?.id !== roomId) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const channelId = this.activeChannelId() || DEFAULT_TEXT_CHANNEL_ID;
|
||||||
|
const latestVisibleMessageTimestamp = messages.reduce((latestTimestamp, message) => {
|
||||||
|
if (resolveMessageChannelId(message) !== channelId) {
|
||||||
|
return latestTimestamp;
|
||||||
|
}
|
||||||
|
|
||||||
|
return Math.max(latestTimestamp, message.timestamp);
|
||||||
|
}, 0);
|
||||||
|
|
||||||
|
this.markChannelRead(roomId, channelId, latestVisibleMessageTimestamp);
|
||||||
|
}
|
||||||
|
|
||||||
private getChannelLastReadAt(roomId: string, channelId: string): number {
|
private getChannelLastReadAt(roomId: string, channelId: string): number {
|
||||||
return this._settings().lastReadByChannel[roomId]?.[channelId]
|
return this._settings().lastReadByChannel[roomId]?.[channelId]
|
||||||
?? this._settings().roomBaselines[roomId]
|
?? this._settings().roomBaselines[roomId]
|
||||||
|
|||||||
@@ -0,0 +1,71 @@
|
|||||||
|
import type { Message, Room } from '../../../../shared-kernel';
|
||||||
|
import { createDefaultNotificationSettings } from '../models/notification.model';
|
||||||
|
import { calculateUnreadForRoom } from './notification.logic';
|
||||||
|
|
||||||
|
function createRoom(overrides: Partial<Room> = {}): Room {
|
||||||
|
return {
|
||||||
|
id: 'room-1',
|
||||||
|
name: 'Room',
|
||||||
|
hostId: 'host',
|
||||||
|
isPrivate: false,
|
||||||
|
createdAt: 0,
|
||||||
|
userCount: 1,
|
||||||
|
channels: [{ id: 'text-1', name: 'lobby', type: 'text', position: 0 }],
|
||||||
|
...overrides
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMessage(overrides: Partial<Message> = {}): Message {
|
||||||
|
return {
|
||||||
|
id: 'message-1',
|
||||||
|
roomId: 'room-1',
|
||||||
|
senderId: 'bob',
|
||||||
|
senderName: 'Bob',
|
||||||
|
content: 'hello',
|
||||||
|
timestamp: 100,
|
||||||
|
reactions: [],
|
||||||
|
isDeleted: false,
|
||||||
|
...overrides
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('calculateUnreadForRoom', () => {
|
||||||
|
it('ignores messages whose channel is not part of the room catalog', () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const settings = {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
};
|
||||||
|
const messages = [createMessage({ id: 'm-1', channelId: 'general', timestamp: 200 })];
|
||||||
|
const counts = calculateUnreadForRoom(room, messages, settings, new Set());
|
||||||
|
|
||||||
|
expect(counts.roomCount).toBe(0);
|
||||||
|
expect(counts.channelCounts['general']).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('counts messages for known text channels', () => {
|
||||||
|
const room = createRoom();
|
||||||
|
const settings = {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
};
|
||||||
|
const messages = [createMessage({ id: 'm-1', channelId: 'text-1', timestamp: 200 })];
|
||||||
|
const counts = calculateUnreadForRoom(room, messages, settings, new Set());
|
||||||
|
|
||||||
|
expect(counts.roomCount).toBe(1);
|
||||||
|
expect(counts.channelCounts['text-1']).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('treats empty channel ids as the default channel when the room has no catalog', () => {
|
||||||
|
const room = createRoom({ channels: [] });
|
||||||
|
const settings = {
|
||||||
|
...createDefaultNotificationSettings(),
|
||||||
|
roomBaselines: { 'room-1': 0 }
|
||||||
|
};
|
||||||
|
const messages = [createMessage({ id: 'm-1', channelId: undefined, timestamp: 200 })];
|
||||||
|
const counts = calculateUnreadForRoom(room, messages, settings, new Set());
|
||||||
|
|
||||||
|
expect(counts.roomCount).toBe(1);
|
||||||
|
expect(counts.channelCounts['general']).toBe(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -130,11 +130,15 @@ export function calculateUnreadForRoom(
|
|||||||
|
|
||||||
const channelId = resolveMessageChannelId(message);
|
const channelId = resolveMessageChannelId(message);
|
||||||
|
|
||||||
|
if (!(channelId in channelCounts)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if (message.timestamp <= getChannelLastReadAt(settings, room.id, channelId)) {
|
if (message.timestamp <= getChannelLastReadAt(settings, room.id, channelId)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
channelCounts[channelId] = (channelCounts[channelId] ?? 0) + 1;
|
channelCounts[channelId] += 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user