From bd7a97f7482808cbca72e575fbf3b195cbcd9f79 Mon Sep 17 00:00:00 2001 From: Mark McIntosh Date: Tue, 24 Feb 2026 01:43:37 -0500 Subject: [PATCH] fix: move user_profiles migration from app-level to core package The user_profiles table was introduced in upstream PRs #508/#510 as an app-level migration (my-sonicjs-app/migrations/018_user_profiles.sql), but PR #512 added raw SQL queries against it in the core package (admin-users.ts) without a corresponding core migration. This caused 500 errors on the user edit page for any app using @sonicjs-cms/core that hadn't independently applied the app-level migration. - Add core migration 032_user_profiles.sql (IF NOT EXISTS for idempotency) - Add auto-detection in MigrationService for existing tables - Un-skip E2E test 38-user-profile-edit.spec.ts (5 tests) - Add 8 unit tests for user profile query logic on edit/update routes --- .../core/migrations/032_user_profiles.sql | 36 ++ .../routes/admin-users-profile.test.ts | 410 ++++++++++++++++++ packages/core/src/db/migrations-bundle.ts | 9 +- packages/core/src/services/migrations.ts | 15 + tests/e2e/38-user-profile-edit.spec.ts | 4 +- 5 files changed, 470 insertions(+), 4 deletions(-) create mode 100644 packages/core/migrations/032_user_profiles.sql create mode 100644 packages/core/src/__tests__/routes/admin-users-profile.test.ts diff --git a/packages/core/migrations/032_user_profiles.sql b/packages/core/migrations/032_user_profiles.sql new file mode 100644 index 000000000..2e8a05075 --- /dev/null +++ b/packages/core/migrations/032_user_profiles.sql @@ -0,0 +1,36 @@ +-- User Profiles Table (Core Migration) +-- Stores extended user profile data separate from auth concerns +-- Required by admin-users.ts for user edit page profile management +-- +-- Originally introduced as app-level migration (my-sonicjs-app/migrations/018_user_profiles.sql) +-- in upstream PR #508. Core routes (admin-users.ts) were updated to query this table in PR #512, +-- but no corresponding core migration was added. This migration corrects that gap. +-- +-- IF NOT EXISTS guards ensure idempotency for databases that already have the table +-- from the app-level migration. + +CREATE TABLE IF NOT EXISTS user_profiles ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL UNIQUE REFERENCES users(id) ON DELETE CASCADE, + + display_name TEXT, + bio TEXT, + company TEXT, + job_title TEXT, + website TEXT, + location TEXT, + date_of_birth INTEGER, + + created_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now') * 1000), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now') * 1000) +); + +-- Index for fast user lookups +CREATE INDEX IF NOT EXISTS idx_user_profiles_user_id ON user_profiles(user_id); + +-- Trigger to auto-update updated_at timestamp +CREATE TRIGGER IF NOT EXISTS user_profiles_updated_at + AFTER UPDATE ON user_profiles +BEGIN + UPDATE user_profiles SET updated_at = strftime('%s', 'now') * 1000 WHERE id = NEW.id; +END; diff --git a/packages/core/src/__tests__/routes/admin-users-profile.test.ts b/packages/core/src/__tests__/routes/admin-users-profile.test.ts new file mode 100644 index 000000000..f0e9ecbfd --- /dev/null +++ b/packages/core/src/__tests__/routes/admin-users-profile.test.ts @@ -0,0 +1,410 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { Hono } from 'hono' + +// Helper to create mock user with specific role +const createMockUser = (role: string = 'admin') => ({ + userId: 'admin-1', + email: 'admin@test.com', + role +}) + +// Mock the requireAuth middleware to bypass authentication in tests +vi.mock('../../middleware', () => ({ + requireAuth: () => { + return async (c: any, next: any) => { + c.set('user', createMockUser()) + await next() + } + }, + logActivity: vi.fn(), + AuthManager: { + generateToken: vi.fn(), + verifyToken: vi.fn(), + hashPassword: vi.fn() + } +})) + +// Mock sanitizeInput to pass through (tested elsewhere) +vi.mock('../../utils/sanitize', () => ({ + sanitizeInput: (val: any) => val +})) + +// Mock template modules — return JSON-stringified data for inspection +vi.mock('../../templates/pages/admin-user-edit.template', () => ({ + renderUserEditPage: (data: any) => JSON.stringify(data), + UserEditPageData: {}, + UserEditData: {}, + UserProfileData: {} +})) + +vi.mock('../../templates/pages/admin-profile.template', () => ({ + renderProfilePage: (data: any) => JSON.stringify(data), + renderAvatarImage: () => '', + UserProfile: {}, + ProfilePageData: {} +})) + +vi.mock('../../templates/components/alert.template', () => ({ + renderAlert: (data: any) => JSON.stringify(data) +})) + +vi.mock('../../templates/pages/admin-activity-logs.template', () => ({ + renderActivityLogsPage: (data: any) => JSON.stringify(data), + ActivityLogsPageData: {}, + ActivityLog: {} +})) + +vi.mock('../../templates/pages/admin-user-new.template', () => ({ + renderUserNewPage: (data: any) => JSON.stringify(data), + UserNewPageData: {} +})) + +vi.mock('../../templates/pages/admin-users-list.template', () => ({ + renderUsersListPage: (data: any) => JSON.stringify(data), + UsersListPageData: {}, + User: {} +})) + +import { userRoutes } from '../../routes/admin-users' + +// Create call-order based D1 mock +// Each db.prepare() call returns a distinct mock chain based on call index +const createOrderedMockDb = (results: Array<{ first?: any; run?: any; all?: any }>) => { + let callIndex = 0 + return { + prepare: vi.fn().mockImplementation(() => { + const result = results[callIndex++] || {} + return { + bind: vi.fn().mockReturnValue({ + first: vi.fn().mockResolvedValue(result.first ?? null), + run: vi.fn().mockResolvedValue(result.run ?? { success: true }), + all: vi.fn().mockResolvedValue(result.all ?? { results: [] }) + }) + } + }) + } +} + +// Standard mock user record +const mockUserRecord = { + id: 'user-123', + email: 'test@example.com', + username: 'testuser', + first_name: 'Test', + last_name: 'User', + phone: null, + avatar_url: null, + role: 'viewer', + is_active: 1, + email_verified: 0, + two_factor_enabled: 0, + created_at: Date.now(), + last_login_at: null +} + +// Standard mock profile record +const mockProfileRecord = { + display_name: 'Test Display', + bio: 'A test bio', + company: 'Test Corp', + job_title: 'Engineer', + website: 'https://example.com', + location: 'San Francisco', + date_of_birth: 631152000000 +} + +describe('Admin Users - Profile on Edit Page', () => { + let app: Hono + let mockDb: any + + beforeEach(() => { + vi.clearAllMocks() + }) + + const createApp = (db: any) => { + app = new Hono() + app.route('/admin', userRoutes) + return app + } + + describe('GET /admin/users/:id/edit', () => { + it('should render edit page with profile data when profile exists', async () => { + mockDb = createOrderedMockDb([ + { first: mockUserRecord }, // call 0: SELECT FROM users + { first: mockProfileRecord } // call 1: SELECT FROM user_profiles + ]) + + app = createApp(mockDb) + + const res = await app.request('/admin/users/user-123/edit', {}, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(200) + + const body = await res.text() + const data = JSON.parse(body) + + // Verify profile data is mapped to camelCase interface + expect(data.userToEdit.profile).toBeDefined() + expect(data.userToEdit.profile.displayName).toBe('Test Display') + expect(data.userToEdit.profile.bio).toBe('A test bio') + expect(data.userToEdit.profile.company).toBe('Test Corp') + expect(data.userToEdit.profile.jobTitle).toBe('Engineer') + expect(data.userToEdit.profile.website).toBe('https://example.com') + expect(data.userToEdit.profile.location).toBe('San Francisco') + + // Verify db.prepare was called twice (user + profile) + expect(mockDb.prepare).toHaveBeenCalledTimes(2) + }) + + it('should render edit page with undefined profile when no profile exists', async () => { + mockDb = createOrderedMockDb([ + { first: mockUserRecord }, // call 0: SELECT FROM users + { first: null } // call 1: SELECT FROM user_profiles — no profile + ]) + + app = createApp(mockDb) + + const res = await app.request('/admin/users/user-123/edit', {}, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(200) + + const body = await res.text() + const data = JSON.parse(body) + + // Profile should be undefined, page should still render + expect(data.userToEdit.profile).toBeUndefined() + expect(data.userToEdit.id).toBe('user-123') + }) + + it('should return 404 when user not found', async () => { + mockDb = createOrderedMockDb([ + { first: null } // call 0: SELECT FROM users — not found + ]) + + app = createApp(mockDb) + + const res = await app.request('/admin/users/nonexistent/edit', {}, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(404) + + const body = await res.text() + const data = JSON.parse(body) + expect(data.type).toBe('error') + expect(data.message).toContain('not found') + }) + + it('should return 500 on database error', async () => { + mockDb = { + prepare: vi.fn().mockImplementation(() => { + throw new Error('D1 database error') + }) + } + + app = createApp(mockDb) + + const res = await app.request('/admin/users/user-123/edit', {}, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(500) + + const body = await res.text() + const data = JSON.parse(body) + expect(data.type).toBe('error') + expect(data.message).toContain('Failed to load user') + }) + }) + + describe('PUT /admin/users/:id', () => { + const createFormBody = (fields: Record) => { + const params = new URLSearchParams() + for (const [key, value] of Object.entries(fields)) { + params.append(key, value) + } + return params.toString() + } + + const baseUserFields = { + first_name: 'Test', + last_name: 'User', + username: 'testuser', + email: 'test@example.com', + role: 'viewer', + is_active: '1' + } + + it('should update existing profile when profile record exists', async () => { + mockDb = createOrderedMockDb([ + { first: null }, // call 0: SELECT id FROM users WHERE (username=? OR email=?) — uniqueness check, no conflict + { run: { success: true } }, // call 1: UPDATE users SET ... + { first: { id: 'profile-existing' } }, // call 2: SELECT id FROM user_profiles WHERE user_id=? + { run: { success: true } } // call 3: UPDATE user_profiles SET ... + ]) + + app = createApp(mockDb) + + const body = createFormBody({ + ...baseUserFields, + profile_display_name: 'Updated Name', + profile_company: 'New Corp' + }) + + const res = await app.request('/admin/users/user-123', { + method: 'PUT', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body + }, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(200) + + const responseBody = await res.text() + const data = JSON.parse(responseBody) + expect(data.type).toBe('success') + + // Verify all 4 prepare calls were made (uniqueness + update + profile check + profile update) + expect(mockDb.prepare.mock.calls.length).toBeGreaterThanOrEqual(4) + }) + + it('should create new profile when no profile record exists', async () => { + mockDb = createOrderedMockDb([ + { first: null }, // call 0: SELECT id FROM users — uniqueness check, no conflict + { run: { success: true } }, // call 1: UPDATE users SET ... + { first: null }, // call 2: SELECT id FROM user_profiles — not found + { run: { success: true } } // call 3: INSERT INTO user_profiles + ]) + + app = createApp(mockDb) + + const body = createFormBody({ + ...baseUserFields, + profile_display_name: 'New Profile', + profile_bio: 'A new bio' + }) + + const res = await app.request('/admin/users/user-123', { + method: 'PUT', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body + }, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(200) + + const responseBody = await res.text() + const data = JSON.parse(responseBody) + expect(data.type).toBe('success') + + // Verify all 4 prepare calls were made (uniqueness + update + profile check + profile insert) + expect(mockDb.prepare.mock.calls.length).toBeGreaterThanOrEqual(4) + }) + + it('should skip profile queries when no profile fields are submitted', async () => { + mockDb = createOrderedMockDb([ + { first: null }, // call 0: SELECT id FROM users — uniqueness check, no conflict + { run: { success: true } } // call 1: UPDATE users SET ... + ]) + + app = createApp(mockDb) + + // Submit only user fields — no profile_* keys at all + const body = createFormBody(baseUserFields) + + const res = await app.request('/admin/users/user-123', { + method: 'PUT', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body + }, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + expect(res.status).toBe(200) + + const responseBody = await res.text() + const data = JSON.parse(responseBody) + expect(data.type).toBe('success') + + // With no profile fields, hasProfileData is falsy (all null via || null). + // The profile block (SELECT/INSERT/UPDATE on user_profiles) should be skipped entirely. + // Only 2 prepare calls: uniqueness check + user update. + // logActivity is mocked so it won't add more. + expect(mockDb.prepare.mock.calls.length).toBe(2) + }) + + it('should return error on profile database failure', async () => { + mockDb = createOrderedMockDb([ + { first: null }, // call 0: SELECT id FROM users — uniqueness check, no conflict + { run: { success: true } }, // call 1: UPDATE users SET ... + {} // call 2: SELECT id FROM user_profiles — will be overridden below + ]) + + // Override call 2 to throw an error on .first() + let callCount = 0 + mockDb.prepare = vi.fn().mockImplementation(() => { + callCount++ + if (callCount === 3) { + // 3rd prepare call: profile existence check — throw error + return { + bind: vi.fn().mockReturnValue({ + first: vi.fn().mockRejectedValue(new Error('D1 profile table error')), + run: vi.fn().mockRejectedValue(new Error('D1 profile table error')), + all: vi.fn().mockResolvedValue({ results: [] }) + }) + } + } + // Calls 1-2: uniqueness check (return null) and user update (success) + return { + bind: vi.fn().mockReturnValue({ + first: vi.fn().mockResolvedValue(null), + run: vi.fn().mockResolvedValue({ success: true }), + all: vi.fn().mockResolvedValue({ results: [] }) + }) + } + }) + + app = createApp(mockDb) + + const body = createFormBody({ + ...baseUserFields, + profile_display_name: 'Will Fail' + }) + + const res = await app.request('/admin/users/user-123', { + method: 'PUT', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body + }, { + DB: mockDb, + KV: {}, + CACHE_KV: {} + }) + + const responseBody = await res.text() + const data = JSON.parse(responseBody) + expect(data.type).toBe('error') + expect(data.message).toContain('Failed to update user') + }) + }) +}) diff --git a/packages/core/src/db/migrations-bundle.ts b/packages/core/src/db/migrations-bundle.ts index c37a00010..dbac282db 100644 --- a/packages/core/src/db/migrations-bundle.ts +++ b/packages/core/src/db/migrations-bundle.ts @@ -1,7 +1,7 @@ /** * AUTO-GENERATED FILE - DO NOT EDIT * Generated by: scripts/generate-migrations.ts - * Generated at: 2026-01-30T06:55:27.442Z + * Generated at: 2026-02-24T06:42:50.837Z * * This file contains all migration SQL bundled for use in Cloudflare Workers * where filesystem access is not available at runtime. @@ -225,6 +225,13 @@ export const bundledMigrations: BundledMigration[] = [ filename: '031_ai_search_plugin.sql', description: 'Migration 031: Ai Search Plugin', sql: "-- AI Search plugin settings\nCREATE TABLE IF NOT EXISTS ai_search_settings (\n id INTEGER PRIMARY KEY AUTOINCREMENT,\n enabled BOOLEAN DEFAULT 0,\n ai_mode_enabled BOOLEAN DEFAULT 1,\n selected_collections TEXT, -- JSON array of collection IDs to index\n dismissed_collections TEXT, -- JSON array of collection IDs user chose not to index\n autocomplete_enabled BOOLEAN DEFAULT 1,\n cache_duration INTEGER DEFAULT 1, -- hours\n results_limit INTEGER DEFAULT 20,\n index_media BOOLEAN DEFAULT 0,\n index_status TEXT, -- JSON object with status per collection\n last_indexed_at INTEGER,\n created_at INTEGER DEFAULT (strftime('%s', 'now') * 1000),\n updated_at INTEGER DEFAULT (strftime('%s', 'now') * 1000)\n);\n\n-- Search history/analytics\nCREATE TABLE IF NOT EXISTS ai_search_history (\n id INTEGER PRIMARY KEY AUTOINCREMENT,\n query TEXT NOT NULL,\n mode TEXT, -- 'ai' or 'keyword'\n results_count INTEGER,\n user_id INTEGER,\n created_at INTEGER DEFAULT (strftime('%s', 'now') * 1000)\n);\n\n-- Index metadata tracking (per collection)\nCREATE TABLE IF NOT EXISTS ai_search_index_meta (\n id INTEGER PRIMARY KEY AUTOINCREMENT,\n collection_id INTEGER NOT NULL,\n collection_name TEXT NOT NULL, -- Cache collection name for display\n total_items INTEGER DEFAULT 0,\n indexed_items INTEGER DEFAULT 0,\n last_sync_at INTEGER,\n status TEXT DEFAULT 'pending', -- 'pending', 'indexing', 'completed', 'error'\n error_message TEXT,\n UNIQUE(collection_id)\n);\n\n-- Indexes for performance\nCREATE INDEX IF NOT EXISTS idx_ai_search_history_created_at ON ai_search_history(created_at);\nCREATE INDEX IF NOT EXISTS idx_ai_search_history_mode ON ai_search_history(mode);\nCREATE INDEX IF NOT EXISTS idx_ai_search_index_meta_collection_id ON ai_search_index_meta(collection_id);\nCREATE INDEX IF NOT EXISTS idx_ai_search_index_meta_status ON ai_search_index_meta(status);\n" + }, + { + id: '032', + name: 'User Profiles', + filename: '032_user_profiles.sql', + description: 'Migration 032: User Profiles', + sql: "-- User Profiles Table (Core Migration)\n-- Stores extended user profile data separate from auth concerns\n-- Required by admin-users.ts for user edit page profile management\n--\n-- Originally introduced as app-level migration (my-sonicjs-app/migrations/018_user_profiles.sql)\n-- in upstream PR #508. Core routes (admin-users.ts) were updated to query this table in PR #512,\n-- but no corresponding core migration was added. This migration corrects that gap.\n--\n-- IF NOT EXISTS guards ensure idempotency for databases that already have the table\n-- from the app-level migration.\n\nCREATE TABLE IF NOT EXISTS user_profiles (\n id TEXT PRIMARY KEY,\n user_id TEXT NOT NULL UNIQUE REFERENCES users(id) ON DELETE CASCADE,\n\n display_name TEXT,\n bio TEXT,\n company TEXT,\n job_title TEXT,\n website TEXT,\n location TEXT,\n date_of_birth INTEGER,\n\n created_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now') * 1000),\n updated_at INTEGER NOT NULL DEFAULT (strftime('%s', 'now') * 1000)\n);\n\n-- Index for fast user lookups\nCREATE INDEX IF NOT EXISTS idx_user_profiles_user_id ON user_profiles(user_id);\n\n-- Trigger to auto-update updated_at timestamp\nCREATE TRIGGER IF NOT EXISTS user_profiles_updated_at\n AFTER UPDATE ON user_profiles\nBEGIN\n UPDATE user_profiles SET updated_at = strftime('%s', 'now') * 1000 WHERE id = NEW.id;\nEND;\n" } ] diff --git a/packages/core/src/services/migrations.ts b/packages/core/src/services/migrations.ts index c39d44046..e6a684f3e 100644 --- a/packages/core/src/services/migrations.ts +++ b/packages/core/src/services/migrations.ts @@ -227,6 +227,21 @@ export class MigrationService { await this.markMigrationApplied('018', 'Settings Table', '018_settings_table.sql') } } + + // Check if user_profiles table exists (migration 032) + // Table may already exist from app-level migration (my-sonicjs-app/migrations/018_user_profiles.sql) + if (!appliedMigrations.has('032')) { + const hasUserProfilesTable = await this.checkTablesExist(['user_profiles']) + if (hasUserProfilesTable) { + appliedMigrations.set('032', { + id: '032', + applied_at: new Date().toISOString(), + name: 'User Profiles', + filename: '032_user_profiles.sql' + }) + await this.markMigrationApplied('032', 'User Profiles', '032_user_profiles.sql') + } + } } /** diff --git a/tests/e2e/38-user-profile-edit.spec.ts b/tests/e2e/38-user-profile-edit.spec.ts index af409c396..19bee16e9 100644 --- a/tests/e2e/38-user-profile-edit.spec.ts +++ b/tests/e2e/38-user-profile-edit.spec.ts @@ -1,9 +1,7 @@ import { test, expect } from '@playwright/test'; import { loginAsAdmin, waitForHTMX, ADMIN_CREDENTIALS } from './utils/test-helpers'; -// Skip entire test suite in CI - requires user_profiles table migration -// which may not be applied to the CI database. Feature works locally. -test.describe.skip('User Profile Edit on User Edit Page', () => { +test.describe('User Profile Edit on User Edit Page', () => { let testUserId: string | undefined; let authToken: string; let setupFailed = false;