From fbf76036f57b2b158b23da038a402aba61c3463c Mon Sep 17 00:00:00 2001 From: "borja (aider)" Date: Mon, 5 May 2025 15:08:35 +0200 Subject: [PATCH] feat: implement ensureUserExists, add foreign keys and tests --- src/db.ts | 105 ++++++++++++++++---- tests/unit/db.test.ts | 221 ++++++++++++++++++++++++++++++++---------- 2 files changed, 253 insertions(+), 73 deletions(-) diff --git a/src/db.ts b/src/db.ts index c68f197..dc78d6b 100644 --- a/src/db.ts +++ b/src/db.ts @@ -1,4 +1,5 @@ import { Database } from 'bun:sqlite'; +import { normalizeWhatsAppId } from './utils/whatsapp'; // Import the utility // Function to get a database instance. Defaults to 'tasks.db' export function getDb(filename: string = 'tasks.db'): Database { @@ -10,6 +11,30 @@ export const db = getDb(); // Initialize function now accepts a database instance export function initializeDatabase(instance: Database) { + // Enable foreign key constraints + instance.exec(`PRAGMA foreign_keys = ON;`); + + // Create users table first as others depend on it + instance.exec(` + CREATE TABLE IF NOT EXISTS users ( + id TEXT PRIMARY KEY, -- WhatsApp user ID (normalized) + first_seen TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + last_seen TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ); + `); + + // Create groups table + instance.exec(` + CREATE TABLE IF NOT EXISTS groups ( + id TEXT PRIMARY KEY, -- Group ID (normalized) + community_id TEXT NOT NULL, + name TEXT, + last_verified TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + active BOOLEAN DEFAULT TRUE + ); + `); + + // Create tasks table instance.exec(` CREATE TABLE IF NOT EXISTS tasks ( id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -18,32 +43,72 @@ export function initializeDatabase(instance: Database) { due_date TIMESTAMP NULL, completed BOOLEAN DEFAULT FALSE, completed_at TIMESTAMP NULL, - group_id TEXT NULL, - created_by TEXT NOT NULL, - FOREIGN KEY (created_by) REFERENCES users(id) + group_id TEXT NULL, -- Normalized group ID + created_by TEXT NOT NULL, -- Normalized user ID + FOREIGN KEY (created_by) REFERENCES users(id) ON DELETE CASCADE, + FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE SET NULL -- Optional: Link task to group ); + `); + // Create task_assignments table + instance.exec(` CREATE TABLE IF NOT EXISTS task_assignments ( task_id INTEGER NOT NULL, - user_id TEXT NOT NULL, - assigned_by TEXT NOT NULL, + user_id TEXT NOT NULL, -- Normalized user ID + assigned_by TEXT NOT NULL, -- Normalized user ID assigned_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, PRIMARY KEY (task_id, user_id), - FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE - ); - - CREATE TABLE IF NOT EXISTS users ( - id TEXT PRIMARY KEY, -- WhatsApp user ID (normalized phone number without +) - first_seen TIMESTAMP DEFAULT CURRENT_TIMESTAMP, - last_seen TIMESTAMP DEFAULT CURRENT_TIMESTAMP - ); - - CREATE TABLE IF NOT EXISTS groups ( - id TEXT PRIMARY KEY, - community_id TEXT NOT NULL, - name TEXT, - last_verified TIMESTAMP DEFAULT CURRENT_TIMESTAMP, - active BOOLEAN DEFAULT TRUE + FOREIGN KEY (task_id) REFERENCES tasks(id) ON DELETE CASCADE, + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + FOREIGN KEY (assigned_by) REFERENCES users(id) ON DELETE CASCADE ); `); } + +/** + * Ensures a user exists in the database based on their raw WhatsApp ID. + * If the user exists, updates their last_seen timestamp. + * If the user does not exist, creates them. + * Uses the normalizeWhatsAppId utility. + * + * @param rawUserId The raw WhatsApp ID (e.g., '12345@s.whatsapp.net'). + * @param instance The database instance to use (defaults to the main db). + * @returns The normalized user ID if successful, otherwise null. + */ +export function ensureUserExists(rawUserId: string | null | undefined, instance: Database = db): string | null { + const normalizedId = normalizeWhatsAppId(rawUserId); + + if (!normalizedId) { + console.error(`[ensureUserExists] Could not normalize or invalid user ID provided: ${rawUserId}`); + return null; + } + + try { + // Use INSERT OR IGNORE to add the user only if they don't exist, + // then UPDATE their last_seen timestamp regardless. + // This is often more efficient than SELECT followed by INSERT/UPDATE. + const insertStmt = instance.prepare(` + INSERT INTO users (id, first_seen, last_seen) + VALUES (?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + ON CONFLICT(id) DO NOTHING; + `); + + const updateStmt = instance.prepare(` + UPDATE users + SET last_seen = CURRENT_TIMESTAMP + WHERE id = ?; + `); + + // Run as transaction for atomicity + instance.transaction(() => { + insertStmt.run(normalizedId); + updateStmt.run(normalizedId); + })(); // Immediately invoke the transaction + + return normalizedId; + + } catch (error) { + console.error(`[ensureUserExists] Database error for user ID ${normalizedId}:`, error); + return null; + } +} diff --git a/tests/unit/db.test.ts b/tests/unit/db.test.ts index 39179d2..009ac6e 100644 --- a/tests/unit/db.test.ts +++ b/tests/unit/db.test.ts @@ -1,13 +1,18 @@ import { Database } from 'bun:sqlite'; -import { initializeDatabase } from '../../src/db'; +import { initializeDatabase, ensureUserExists } from '../../src/db'; // Import ensureUserExists import { describe, test, expect, beforeEach, afterAll, beforeAll } from 'bun:test'; +// Helper function to introduce slight delay for timestamp checks +const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + describe('Database', () => { let testDb: Database; // Create an in-memory database before any tests run beforeAll(() => { testDb = new Database(':memory:'); + // Enable foreign keys for the test database instance + testDb.exec('PRAGMA foreign_keys = ON;'); }); // Close the database connection after all tests have run @@ -17,8 +22,8 @@ describe('Database', () => { beforeEach(() => { // Reset database schema between tests by dropping tables and re-initializing + testDb.exec('DROP TABLE IF EXISTS task_assignments'); // Drop dependent tables first testDb.exec('DROP TABLE IF EXISTS tasks'); - testDb.exec('DROP TABLE IF EXISTS task_assignments'); testDb.exec('DROP TABLE IF EXISTS users'); testDb.exec('DROP TABLE IF EXISTS groups'); // Initialize schema on the test database instance @@ -26,15 +31,18 @@ describe('Database', () => { }); describe('Table Creation', () => { - test('should create all required tables', () => { + test('should create all required tables in correct order', () => { const tables = testDb .query("SELECT name FROM sqlite_master WHERE type='table'") .all() .map((t: any) => t.name); - const expectedTables = ['groups', 'task_assignments', 'tasks', 'users']; + // Order matters if foreign keys are involved during creation, though SQLite is flexible + const expectedTables = ['users', 'groups', 'tasks', 'task_assignments']; const userTables = tables.filter(t => !t.startsWith('sqlite_')); - expect(userTables.sort()).toEqual(expectedTables.sort()); + // Check if all expected tables exist, order might vary slightly depending on execution + expect(userTables).toHaveLength(expectedTables.length); + expectedTables.forEach(table => expect(userTables).toContain(table)); }); }); @@ -57,6 +65,14 @@ describe('Database', () => { expect(columns).toContain('group_id'); expect(columns).toContain('created_by'); }); + + test('task_assignments table should have correct columns', () => { + const columns = testDb + .query("PRAGMA table_info(task_assignments)") + .all() + .map((c: any) => c.name); + expect(columns).toEqual(['task_id', 'user_id', 'assigned_by', 'assigned_at']); + }); test('groups table should have active flag default to true', () => { testDb.exec(` @@ -69,73 +85,172 @@ describe('Database', () => { }); describe('Foreign Keys', () => { - test('task_assignments should reference tasks', () => { - const fkInfo = testDb - .query("PRAGMA foreign_key_list(task_assignments)") - .all(); - expect(fkInfo.length).toBe(1); - expect(fkInfo[0].from).toBe('task_id'); - expect(fkInfo[0].to).toBe('id'); - expect(fkInfo[0].table).toBe('tasks'); + test('tasks should reference users via created_by', () => { + const fkInfo = testDb.query("PRAGMA foreign_key_list(tasks)").all(); + const createdByFk = fkInfo.find(fk => fk.from === 'created_by'); + expect(createdByFk).toBeDefined(); + expect(createdByFk.table).toBe('users'); + expect(createdByFk.to).toBe('id'); }); - test('tasks should reference users via created_by', () => { - const fkInfo = testDb - .query("PRAGMA foreign_key_list(tasks)") - .all(); - expect(fkInfo.length).toBe(1); - expect(fkInfo[0].from).toBe('created_by'); - expect(fkInfo[0].to).toBe('id'); - expect(fkInfo[0].table).toBe('users'); + test('tasks should reference groups via group_id', () => { + const fkInfo = testDb.query("PRAGMA foreign_key_list(tasks)").all(); + const groupIdFk = fkInfo.find(fk => fk.from === 'group_id'); + expect(groupIdFk).toBeDefined(); + expect(groupIdFk.table).toBe('groups'); + expect(groupIdFk.to).toBe('id'); + expect(groupIdFk.on_delete).toBe('SET NULL'); + }); + + test('task_assignments should reference tasks via task_id', () => { + const fkInfo = testDb.query("PRAGMA foreign_key_list(task_assignments)").all(); + const taskIdFk = fkInfo.find(fk => fk.from === 'task_id'); + expect(taskIdFk).toBeDefined(); + expect(taskIdFk.table).toBe('tasks'); + expect(taskIdFk.to).toBe('id'); + expect(taskIdFk.on_delete).toBe('CASCADE'); + }); + + test('task_assignments should reference users via user_id', () => { + const fkInfo = testDb.query("PRAGMA foreign_key_list(task_assignments)").all(); + const userIdFk = fkInfo.find(fk => fk.from === 'user_id'); + expect(userIdFk).toBeDefined(); + expect(userIdFk.table).toBe('users'); + expect(userIdFk.to).toBe('id'); + expect(userIdFk.on_delete).toBe('CASCADE'); + }); + + test('task_assignments should reference users via assigned_by', () => { + const fkInfo = testDb.query("PRAGMA foreign_key_list(task_assignments)").all(); + const assignedByFk = fkInfo.find(fk => fk.from === 'assigned_by'); + expect(assignedByFk).toBeDefined(); + expect(assignedByFk.table).toBe('users'); + expect(assignedByFk.to).toBe('id'); + expect(assignedByFk.on_delete).toBe('CASCADE'); + }); + + test('should prevent inserting task with non-existent user', () => { + expect(() => { + testDb.prepare(` + INSERT INTO tasks (description, created_by) + VALUES ('Task for non-existent user', 'nonexistentuser') + `).run(); + }).toThrow(); // Foreign key constraint failure + }); + + test('should prevent inserting assignment with non-existent user', () => { + // Need a valid user and task first + testDb.exec(`INSERT INTO users (id) VALUES ('user1')`); + const taskResult = testDb.prepare(`INSERT INTO tasks (description, created_by) VALUES ('Test Task', 'user1') RETURNING id`).get(); + const taskId = taskResult.id; + + expect(() => { + testDb.prepare(` + INSERT INTO task_assignments (task_id, user_id, assigned_by) + VALUES (?, 'nonexistentuser', 'user1') + `).run(taskId); + }).toThrow(); // Foreign key constraint failure on user_id + + expect(() => { + testDb.prepare(` + INSERT INTO task_assignments (task_id, user_id, assigned_by) + VALUES (?, 'user1', 'nonexistentassigner') + `).run(taskId); + }).toThrow(); // Foreign key constraint failure on assigned_by }); }); - describe('User Operations', () => { - test('should reject duplicate user IDs', () => { - // First insert should succeed - const firstInsert = testDb.prepare(` - INSERT INTO users (id) VALUES ('34650112233') - `).run(); - expect(firstInsert.changes).toBe(1); - - // Second insert with same ID should fail due to PRIMARY KEY constraint - expect(() => { - testDb.prepare(` - INSERT INTO users (id) VALUES ('34650112233') - `).run(); - }).toThrow(); // Bun's SQLite driver throws an error on constraint violation - - // Verify only one record exists - const countResult = testDb.prepare(` - SELECT COUNT(*) as count FROM users WHERE id = '34650112233' - `).get(); - expect(countResult.count).toBe(1); + describe('ensureUserExists', () => { + const rawUserId = '1234567890@s.whatsapp.net'; + const normalizedId = '1234567890'; + + test('should create a new user if they do not exist', () => { + const resultId = ensureUserExists(rawUserId, testDb); + expect(resultId).toBe(normalizedId); + + const user = testDb.query("SELECT * FROM users WHERE id = ?").get(normalizedId); + expect(user).toBeDefined(); + expect(user.id).toBe(normalizedId); + expect(user.first_seen).toBeDefined(); + expect(user.last_seen).toBe(user.first_seen); // Initially they are the same + }); + + test('should update last_seen for an existing user', async () => { + // First call creates the user + ensureUserExists(rawUserId, testDb); + const initialUser = testDb.query("SELECT * FROM users WHERE id = ?").get(normalizedId); + const initialLastSeen = initialUser.last_seen; + + // Wait a bit to ensure timestamp changes + await sleep(50); + + // Second call should update last_seen + const resultId = ensureUserExists(rawUserId, testDb); + expect(resultId).toBe(normalizedId); + + const updatedUser = testDb.query("SELECT * FROM users WHERE id = ?").get(normalizedId); + expect(updatedUser.last_seen).toBeDefined(); + expect(updatedUser.last_seen).not.toBe(initialLastSeen); // last_seen should be updated + expect(updatedUser.first_seen).toBe(initialUser.first_seen); // first_seen should NOT be updated + }); + + test('should return the normalized ID on success', () => { + const resultId = ensureUserExists('9876543210@s.whatsapp.net', testDb); + expect(resultId).toBe('9876543210'); + }); + + test('should return null for invalid raw user ID', () => { + const resultId = ensureUserExists('invalid-id!', testDb); + expect(resultId).toBeNull(); + const userCount = testDb.query("SELECT COUNT(*) as count FROM users").get(); + expect(userCount.count).toBe(0); // No user should be created + }); + + test('should return null for null input', () => { + const resultId = ensureUserExists(null, testDb); + expect(resultId).toBeNull(); + }); + + test('should return null for undefined input', () => { + const resultId = ensureUserExists(undefined, testDb); + expect(resultId).toBeNull(); + }); + + test('should handle user ID with participant correctly', () => { + const resultId = ensureUserExists('1122334455:12@s.whatsapp.net', testDb); + expect(resultId).toBe('1122334455'); + const user = testDb.query("SELECT * FROM users WHERE id = ?").get('1122334455'); + expect(user).toBeDefined(); }); }); - describe('Data Operations', () => { + // Keep existing Data Operations tests, but ensure users/groups are created first + describe('Data Operations (with user checks)', () => { + const user1 = '34650112233'; + const group1 = 'test-group-123'; + beforeEach(() => { - // Seed necessary data for these tests into the test database + // Ensure users and groups exist before task operations + ensureUserExists(`${user1}@s.whatsapp.net`, testDb); testDb.exec(` - INSERT INTO users (id) VALUES ('34650112233'); - INSERT INTO groups (id, community_id, name) - VALUES ('test-group', 'test-community', 'Test Group') - `); + INSERT OR IGNORE INTO groups (id, community_id, name) + VALUES (?, 'test-community', 'Test Group') + `, [group1]); }); - test('should allow inserting group tasks', () => { + test('should allow inserting group tasks with existing user and group', () => { const result = testDb.prepare(` INSERT INTO tasks (description, created_by, group_id) - VALUES ('Test task', '34650112233', 'test-group') - `).run(); + VALUES ('Test task', ?, ?) + `).run(user1, group1); expect(result.changes).toBe(1); }); - test('should allow inserting private tasks', () => { + test('should allow inserting private tasks with existing user', () => { const result = testDb.prepare(` INSERT INTO tasks (description, created_by) - VALUES ('Private task', '34650112233') - `).run(); + VALUES ('Private task', ?) + `).run(user1); expect(result.changes).toBe(1); }); });