Improve messaging and dashboard reliability
This commit is contained in:
1 parent
345ec99348
commit
5a5a30a1d4
20 files changed
+406
-56
No files matched your search
@@ -6,7 +6,11 @@ import { runMigrations } from '../src/database/client.js';
|
||||
import { RedditUsersRepository } from '../src/database/repositories.js';
|
||||
import { ChatService } from '../src/services/chat/service.js';
|
||||
import { ChatCache } from '../src/services/chat/cache.js';
|
||||
import { MatrixAuthError, MatrixClient } from '../src/services/chat/matrix/client.js';
|
||||
import {
|
||||
MatrixAuthError,
|
||||
MatrixClient,
|
||||
MatrixFeatureDisabledError,
|
||||
} from '../src/services/chat/matrix/client.js';
|
||||
import { createMockLogger } from './mockLogger.js';
|
||||
import { authenticatedAccountAuth } from './testUtils.js';
|
||||
|
||||
@@ -232,4 +236,54 @@ describe('ChatService username persistence', () => {
|
||||
expect(matrix.getRoomFullState).toHaveBeenCalledOnce();
|
||||
expect(matrix.getRoomMessages).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('stops room state lookups after the homeserver reports the feature is disabled', async () => {
|
||||
const logger = createMockLogger();
|
||||
const cache = tmpCache(logger);
|
||||
cache.set(['first', 'second'].map((name) => ({
|
||||
username: 'unknown',
|
||||
roomName: 'unknown',
|
||||
lastMessageTime: Date.now(),
|
||||
lastMessageSnippet: '',
|
||||
isFromUs: false,
|
||||
unread: false,
|
||||
roomId: `!${name}:reddit.com`,
|
||||
messages: [],
|
||||
})));
|
||||
|
||||
const matrix = buildMatrix();
|
||||
matrix.getRoomFullState = vi.fn().mockRejectedValue(
|
||||
new MatrixFeatureDisabledError('disabled by configuration'),
|
||||
);
|
||||
matrix.getRoomMessages = vi.fn().mockImplementation(async (roomId: string) => ({
|
||||
start: 'start',
|
||||
chunk: [{
|
||||
event_id: `$${roomId}`,
|
||||
type: 'm.room.message',
|
||||
sender: `@t2_${roomId.includes('first') ? 'first' : 'second'}:reddit.com`,
|
||||
origin_server_ts: Date.now(),
|
||||
content: { msgtype: 'm.text', body: 'hello' },
|
||||
}],
|
||||
}));
|
||||
const usernameResolver = { resolveT2Ids: vi.fn().mockResolvedValue({}) };
|
||||
const svc = new ChatService(
|
||||
matrix,
|
||||
logger,
|
||||
undefined, undefined, undefined, usernameResolver,
|
||||
cache,
|
||||
undefined,
|
||||
undefined,
|
||||
authenticatedAccountAuth,
|
||||
);
|
||||
|
||||
await svc.enrichUsernames();
|
||||
|
||||
expect(matrix.getRoomFullState).toHaveBeenCalledOnce();
|
||||
expect(matrix.getRoomMessages).toHaveBeenCalledTimes(2);
|
||||
expect(logger.logs.filter((entry) =>
|
||||
entry.message.includes('room state lookup is disabled upstream'),
|
||||
)).toHaveLength(1);
|
||||
expect((await svc.getCachedConversations()).map((chat) => chat.username))
|
||||
.toEqual(['t2_first', 't2_second']);
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,7 @@ import { ConversationsPage } from '../../apps/dashboard/src/features/conversatio
|
||||
interface ChatFixtureOptions {
|
||||
username: string;
|
||||
roomId: string;
|
||||
lastMessageTime?: number;
|
||||
unread?: boolean;
|
||||
isFromUs?: boolean;
|
||||
lastMessageSnippet?: string;
|
||||
@@ -24,7 +25,7 @@ function chatFixture(options: ChatFixtureOptions) {
|
||||
roomName: options.username,
|
||||
roomId: options.roomId,
|
||||
url: `https://chat.reddit.com/room/${options.roomId}`,
|
||||
lastMessageTime: 1_786_000_000_000,
|
||||
lastMessageTime: options.lastMessageTime ?? 1_786_000_000_000,
|
||||
lastMessageSnippet: options.lastMessageSnippet ?? `Latest from ${options.username}`,
|
||||
isFromUs: options.isFromUs ?? false,
|
||||
unread: options.unread ?? false,
|
||||
@@ -141,6 +142,36 @@ describe('ConversationsPage', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('orders conversations by latest message activity', async () => {
|
||||
const oldest = chatFixture({ username: 'oldest', roomId: 'room-oldest', lastMessageTime: 100 });
|
||||
const newest = chatFixture({ username: 'newest', roomId: 'room-newest', lastMessageTime: 300 });
|
||||
const middle = chatFixture({ username: 'middle', roomId: 'room-middle', lastMessageTime: 200 });
|
||||
server.use(http.get('/api/conversations', () => HttpResponse.json([oldest, newest, middle])));
|
||||
renderConversations();
|
||||
|
||||
await screen.findByRole('button', { name: /u\/newest/ });
|
||||
expect(screen.getAllByRole('button', { name: /u\/(?:oldest|newest|middle)/ }).map((button) => button.getAttribute('aria-label')))
|
||||
.toEqual([
|
||||
expect.stringContaining('u/newest'),
|
||||
expect.stringContaining('u/middle'),
|
||||
expect.stringContaining('u/oldest'),
|
||||
]);
|
||||
});
|
||||
|
||||
it('shows short relative times for the latest message', async () => {
|
||||
const now = Date.now();
|
||||
const minutesAgo = chatFixture({ username: 'minutes', roomId: 'room-minutes', lastMessageTime: now - (5 * 60_000) });
|
||||
const daysAgo = chatFixture({ username: 'days', roomId: 'room-days', lastMessageTime: now - (2 * 86_400_000) });
|
||||
server.use(http.get('/api/conversations', () => HttpResponse.json([daysAgo, minutesAgo])));
|
||||
renderConversations();
|
||||
|
||||
const recentTime = await screen.findByText('5m ago');
|
||||
expect(recentTime.tagName).toBe('TIME');
|
||||
expect(recentTime).toHaveAttribute('dateTime', new Date(minutesAgo.lastMessageTime).toISOString());
|
||||
expect(recentTime).toHaveAttribute('title');
|
||||
expect(screen.getByText('2d ago')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('marks and filters conversations with received images and renders the images in chat', async () => {
|
||||
const imageChat = chatFixture({
|
||||
username: 'alice',
|
||||
|
||||
@@ -150,6 +150,20 @@ describe('media page', () => {
|
||||
expect(screen.getByText('No media matches these filters.')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('orders media by received time', async () => {
|
||||
server.use(
|
||||
http.get('/api/media', () => HttpResponse.json([mediaItems[2], mediaItems[0], mediaItems[1]])),
|
||||
http.get('/api/media/sync', () => HttpResponse.json(idleStatus)),
|
||||
);
|
||||
renderMedia();
|
||||
|
||||
expect((await screen.findAllByRole('img')).map((item) => item.getAttribute('alt'))).toEqual([
|
||||
'Media from alice',
|
||||
'Media from charlie',
|
||||
'Media from dave',
|
||||
]);
|
||||
});
|
||||
|
||||
it('shows empty and recoverable error states', async () => {
|
||||
let requests = 0;
|
||||
server.use(
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, it, expect, vi } from 'vitest';
|
||||
import {
|
||||
MatrixAuthError,
|
||||
MatrixClient,
|
||||
MatrixFeatureDisabledError,
|
||||
MatrixRateLimitError,
|
||||
} from '../src/services/chat/matrix/client.js';
|
||||
import { createMockLogger } from './mockLogger.js';
|
||||
@@ -153,6 +154,18 @@ describe('MatrixClient direct room registration', () => {
|
||||
});
|
||||
|
||||
describe('MatrixClient authentication classification', () => {
|
||||
it('classifies an explicitly disabled endpoint as an unavailable feature', async () => {
|
||||
const client = new MatrixClient(creds, createMockLogger(), {
|
||||
minRequestIntervalMs: 0,
|
||||
fetchImpl: vi.fn().mockResolvedValue(new Response(JSON.stringify({
|
||||
message: 'disabled by configuration',
|
||||
}), { status: 403, headers: { 'content-type': 'application/json' } })),
|
||||
});
|
||||
|
||||
await expect(client.getRoomFullState('!room:example.test'))
|
||||
.rejects.toBeInstanceOf(MatrixFeatureDisabledError);
|
||||
});
|
||||
|
||||
it('does not classify a room-level 403 as an expired account token', async () => {
|
||||
const client = new MatrixClient(creds, createMockLogger(), {
|
||||
minRequestIntervalMs: 0,
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { RecipientDeduplicator } from '../src/services/recipient-deduplicator.js';
|
||||
|
||||
describe('RecipientDeduplicator', () => {
|
||||
it('reserves a recipient once across username casing variants', () => {
|
||||
const history = { hasSentTo: vi.fn(() => false) };
|
||||
const deduplicator = new RecipientDeduplicator(history);
|
||||
|
||||
expect(deduplicator.tryReserve('Some_User')).toBe(true);
|
||||
expect(deduplicator.hasSentTo('some_user')).toBe(true);
|
||||
expect(deduplicator.tryReserve('SOME_USER')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects recipients found in persisted cross-account history', () => {
|
||||
const history = { hasSentTo: vi.fn((recipient: string) => recipient === 'contacted') };
|
||||
const deduplicator = new RecipientDeduplicator(history);
|
||||
|
||||
expect(deduplicator.tryReserve('contacted')).toBe(false);
|
||||
expect(deduplicator.tryReserve('new-user')).toBe(true);
|
||||
});
|
||||
|
||||
it('allows another account to retry after a failed send releases its reservation', () => {
|
||||
const deduplicator = new RecipientDeduplicator({ hasSentTo: () => false });
|
||||
|
||||
expect(deduplicator.tryReserve('retry-user')).toBe(true);
|
||||
deduplicator.release('RETRY-USER');
|
||||
expect(deduplicator.tryReserve('retry-user')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
DeliveryOutboxRepository,
|
||||
MessagesRepository,
|
||||
PostsRepository,
|
||||
RecipientHistoryRepository,
|
||||
SuccessfulMessagesRepository,
|
||||
} from '../src/database/repositories.js';
|
||||
import { buildConversationState } from '../src/services/conversation-state.js';
|
||||
@@ -75,6 +76,8 @@ describe('account-bound domain repositories', () => {
|
||||
expect(messagesB.getByPostId('same-post')).toMatchObject({ id: messageB, message: 'from B' });
|
||||
expect(messagesA.hasSentTo('only-a')).toBe(true);
|
||||
expect(messagesB.hasSentTo('only-a')).toBe(false);
|
||||
const recipientHistory = new RecipientHistoryRepository(db);
|
||||
expect(recipientHistory.hasSentTo('ONLY-A')).toBe(true);
|
||||
expect(messagesA.getByChatRoomId('same-room')?.recipient).toBe('only-a');
|
||||
expect(messagesB.getByChatRoomId('same-room')).toBeUndefined();
|
||||
|
||||
|
||||
@@ -122,7 +122,7 @@ describe('createExpressSseHandler', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('replays the full available window when Last-Event-ID is absent', () => {
|
||||
it('starts at the live edge when Last-Event-ID is absent', () => {
|
||||
const hub = createHub();
|
||||
hub.publish('log.created', { level: 'info', message: 'one' });
|
||||
hub.publish('log.created', { level: 'warn', message: 'two' });
|
||||
@@ -134,10 +134,12 @@ describe('createExpressSseHandler', () => {
|
||||
vi.fn(),
|
||||
);
|
||||
|
||||
expect(response.frames).toEqual([': connected\n\n']);
|
||||
|
||||
hub.publish('log.created', { level: 'info', message: 'live' });
|
||||
expect(response.frames.map((frame) => frame.split('\n')[0])).toEqual([
|
||||
': connected',
|
||||
'id: event-1',
|
||||
'id: event-2',
|
||||
'id: event-3',
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user