more refactor
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Has been cancelled

This commit is contained in:
2025-12-21 20:34:32 -08:00
parent 15f759cbc4
commit bc2c24bcff
46 changed files with 387 additions and 257 deletions

View File

@@ -303,26 +303,44 @@ describe('Flyer DB Service', () => {
});
describe('getFlyers', () => {
const expectedQuery = `
SELECT
f.*,
json_build_object(
'store_id', s.store_id,
'name', s.name,
'logo_url', s.logo_url
) as store
FROM public.flyers f
JOIN public.stores s ON f.store_id = s.store_id
ORDER BY f.created_at DESC LIMIT $1 OFFSET $2`;
it('should use default limit and offset when none are provided', async () => {
console.log('[TEST DEBUG] Running test: getFlyers > should use default limit and offset');
const mockFlyers: Flyer[] = [createMockFlyer({ flyer_id: 1 })];
mockPoolInstance.query.mockResolvedValue({ rows: mockFlyers });
await flyerRepo.getFlyers(mockLogger);
console.log('[TEST DEBUG] mockPoolInstance.query calls:', JSON.stringify(mockPoolInstance.query.mock.calls, null, 2));
expect(mockPoolInstance.query).toHaveBeenCalledWith(
'SELECT * FROM public.flyers ORDER BY created_at DESC LIMIT $1 OFFSET $2',
expectedQuery,
[20, 0] // Default values
);
});
it('should use provided limit and offset values', async () => {
console.log('[TEST DEBUG] Running test: getFlyers > should use provided limit and offset');
const mockFlyers: Flyer[] = [createMockFlyer({ flyer_id: 1 })];
mockPoolInstance.query.mockResolvedValue({ rows: mockFlyers });
await flyerRepo.getFlyers(mockLogger, 10, 5);
console.log('[TEST DEBUG] mockPoolInstance.query calls:', JSON.stringify(mockPoolInstance.query.mock.calls, null, 2));
expect(mockPoolInstance.query).toHaveBeenCalledWith(
'SELECT * FROM public.flyers ORDER BY created_at DESC LIMIT $1 OFFSET $2',
expectedQuery,
[10, 5] // Provided values
);
});

View File

@@ -47,12 +47,25 @@ describe('Personalization DB Service', () => {
describe('getAllMasterItems', () => {
it('should execute the correct query and return master items', async () => {
console.log('[TEST DEBUG] Running test: getAllMasterItems > should execute the correct query');
const mockItems: MasterGroceryItem[] = [createMockMasterGroceryItem({ master_grocery_item_id: 1, name: 'Apples' })];
mockQuery.mockResolvedValue({ rows: mockItems });
const result = await personalizationRepo.getAllMasterItems(mockLogger);
expect(mockQuery).toHaveBeenCalledWith('SELECT * FROM public.master_grocery_items ORDER BY name ASC');
const expectedQuery = `
SELECT
mgi.*,
c.name as category_name
FROM public.master_grocery_items mgi
LEFT JOIN public.categories c ON mgi.category_id = c.category_id
ORDER BY mgi.name ASC`;
console.log('[TEST DEBUG] mockQuery calls:', JSON.stringify(mockQuery.mock.calls, null, 2));
// The query string in the implementation has a lot of whitespace from the template literal.
// This updated expectation matches the new query exactly.
expect(mockQuery).toHaveBeenCalledWith(expectedQuery);
expect(result).toEqual(mockItems);
});

View File

@@ -1,7 +1,7 @@
// src/services/db/personalization.db.ts
import type { Pool, PoolClient } from 'pg';
import { getPool, withTransaction } from './connection.db';
import { UniqueConstraintError, ForeignKeyConstraintError } from './errors.db';
import { ForeignKeyConstraintError } from './errors.db';
import type { Logger } from 'pino';
import {
MasterGroceryItem,

View File

@@ -25,6 +25,7 @@ import { withTransaction } from './connection.db';
import { UserRepository, exportUserData } from './user.db';
import { mockPoolInstance } from '../../tests/setup/tests-setup-unit';
import { createMockUserProfile } from '../../tests/utils/mockFactories';
import { UniqueConstraintError, ForeignKeyConstraintError, NotFoundError } from './errors.db';
import type { Profile, ActivityLogItem, SearchQuery, UserProfile } from '../../types';
@@ -87,19 +88,22 @@ describe('User DB Service', () => {
describe('createUser', () => {
it('should execute a transaction to create a user and profile', async () => {
const mockUser = { user_id: 'new-user-id', email: 'new@example.com' };
const now = new Date().toISOString();
// This is the flat structure returned by the DB query inside createUser
const mockDbProfile = { user_id: 'new-user-id', email: 'new@example.com', role: 'user', full_name: 'New User', avatar_url: null, points: 0, preferences: null };
const mockDbProfile = {
user_id: 'new-user-id', email: 'new@example.com', role: 'user', full_name: 'New User',
avatar_url: null, points: 0, preferences: null, created_at: now, updated_at: now
};
// This is the nested structure the function is expected to return
const expectedProfile: UserProfile = {
user: { user_id: 'new-user-id', email: 'new@example.com' },
user_id: 'new-user-id',
full_name: 'New User',
avatar_url: null,
role: 'user',
points: 0,
preferences: null,
created_at: expect.any(String), // We can't know the exact timestamp from the DB function call in this test context easily, but we know it should be there.
updated_at: expect.any(String),
created_at: now,
updated_at: now,
};
vi.mocked(withTransaction).mockImplementation(async (callback) => {
@@ -113,11 +117,11 @@ describe('User DB Service', () => {
const result = await userRepo.createUser('new@example.com', 'hashedpass', { full_name: 'New User' }, mockLogger);
// Use objectContaining to match the structure, as created_at/updated_at are dynamic
expect(result).toEqual(expect.objectContaining({
...expectedProfile,
created_at: undefined, // The implementation doesn't actually return these from the mock above, so let's adjust the expectation or the mock.
}));
console.log('[TEST DEBUG] createUser - Result from function:', JSON.stringify(result, null, 2));
console.log('[TEST DEBUG] createUser - Expected result:', JSON.stringify(expectedProfile, null, 2));
// Use objectContaining because the real implementation might have other DB-generated fields.
expect(result).toEqual(expect.objectContaining(expectedProfile));
expect(withTransaction).toHaveBeenCalledTimes(1);
});
@@ -194,13 +198,50 @@ describe('User DB Service', () => {
describe('findUserWithProfileByEmail', () => {
it('should query for a user and their profile by email', async () => {
const mockUserWithProfile = { user_id: '123', email: 'test@example.com', full_name: 'Test User', role: 'user' };
mockPoolInstance.query.mockResolvedValue({ rows: [mockUserWithProfile] });
const now = new Date().toISOString();
const mockDbResult = {
user_id: '123',
email: 'test@example.com',
password_hash: 'hash',
refresh_token: 'token',
failed_login_attempts: 0,
last_failed_login: null,
full_name: 'Test User',
avatar_url: null,
role: 'user' as const,
points: 0,
preferences: null,
address_id: null,
created_at: now,
updated_at: now,
};
mockPoolInstance.query.mockResolvedValue({ rows: [mockDbResult] });
const expectedResult = {
user_id: '123',
full_name: 'Test User',
avatar_url: null,
role: 'user',
points: 0,
preferences: null,
address_id: null,
created_at: now,
updated_at: now,
user: { user_id: '123', email: 'test@example.com' },
email: 'test@example.com',
password_hash: 'hash',
failed_login_attempts: 0,
last_failed_login: null,
refresh_token: 'token',
};
const result = await userRepo.findUserWithProfileByEmail('test@example.com', mockLogger);
console.log('[TEST DEBUG] findUserWithProfileByEmail - Result from function:', JSON.stringify(result, null, 2));
console.log('[TEST DEBUG] findUserWithProfileByEmail - Expected result:', JSON.stringify(expectedResult, null, 2));
expect(mockPoolInstance.query).toHaveBeenCalledWith(expect.stringContaining('JOIN public.profiles'), ['test@example.com']);
expect(result).toEqual(mockUserWithProfile);
expect(result).toEqual(expect.objectContaining(expectedResult));
});
it('should return undefined if user is not found', async () => {
@@ -281,7 +322,7 @@ describe('User DB Service', () => {
describe('updateUserProfile', () => {
it('should execute an UPDATE query for the user profile', async () => {
const mockProfile: Profile = { user_id: '123', full_name: 'Updated Name', role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
const mockProfile: Profile = { full_name: 'Updated Name', role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
mockPoolInstance.query.mockResolvedValue({ rows: [mockProfile] });
await userRepo.updateUserProfile('123', { full_name: 'Updated Name' }, mockLogger);
@@ -290,7 +331,7 @@ describe('User DB Service', () => {
});
it('should execute an UPDATE query for avatar_url', async () => {
const mockProfile: Profile = { user_id: '123', avatar_url: 'new-avatar.png', role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
const mockProfile: Profile = { avatar_url: 'new-avatar.png', role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
mockPoolInstance.query.mockResolvedValue({ rows: [mockProfile] });
await userRepo.updateUserProfile('123', { avatar_url: 'new-avatar.png' }, mockLogger);
@@ -299,7 +340,7 @@ describe('User DB Service', () => {
});
it('should execute an UPDATE query for address_id', async () => {
const mockProfile: Profile = { user_id: '123', address_id: 99, role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
const mockProfile: Profile = { address_id: 99, role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
mockPoolInstance.query.mockResolvedValue({ rows: [mockProfile] });
await userRepo.updateUserProfile('123', { address_id: 99 }, mockLogger);
@@ -308,7 +349,7 @@ describe('User DB Service', () => {
});
it('should fetch the current profile if no update fields are provided', async () => {
const mockProfile: Profile = { user_id: '123', full_name: 'Current Name', role: 'user', points: 0, created_at: new Date().toISOString(), updated_at: new Date().toISOString() };
const mockProfile: Profile = createMockUserProfile({ user: { user_id: '123', email: '123@example.com' }, full_name: 'Current Name' });
// FIX: Instead of mocking `mockResolvedValue` on the instance method which might fail if not spied correctly,
// we mock the underlying `db.query` call that `findUserProfileById` makes.
mockPoolInstance.query.mockResolvedValue({ rows: [mockProfile] });
@@ -521,7 +562,7 @@ describe('User DB Service', () => {
const { PersonalizationRepository } = await import('./personalization.db');
const findProfileSpy = vi.spyOn(UserRepository.prototype, 'findUserProfileById');
findProfileSpy.mockResolvedValue({ user_id: '123' } as Profile);
findProfileSpy.mockResolvedValue(createMockUserProfile({ user: { user_id: '123', email: '123@example.com' } }));
const getWatchedItemsSpy = vi.spyOn(PersonalizationRepository.prototype, 'getWatchedItems');
getWatchedItemsSpy.mockResolvedValue([]);
const getShoppingListsSpy = vi.spyOn(ShoppingRepository.prototype, 'getShoppingLists');

View File

@@ -93,11 +93,11 @@ export class UserRepository {
// Construct the nested UserProfile object to match the type definition.
const fullUserProfile: UserProfile = {
// user_id is now correctly part of the nested user object, not at the top level.
user: {
user_id: flatProfile.user_id,
email: flatProfile.email,
},
user_id: flatProfile.user_id,
full_name: flatProfile.full_name,
avatar_url: flatProfile.avatar_url,
role: flatProfile.role,
@@ -127,7 +127,7 @@ export class UserRepository {
* @param email The email of the user to find.
* @returns A promise that resolves to the combined user and profile object or undefined if not found.
*/
async findUserWithProfileByEmail(email: string, logger: Logger): Promise<(UserProfile & DbUser) | undefined> {
async findUserWithProfileByEmail(email: string, logger: Logger): Promise<(UserProfile & Omit<DbUser, 'user_id' | 'email'>) | undefined> {
logger.debug({ email }, `[DB findUserWithProfileByEmail] Searching for user.`);
try {
const query = `
@@ -139,7 +139,7 @@ export class UserRepository {
JOIN public.profiles p ON u.user_id = p.user_id
WHERE u.email = $1;
`;
const res = await this.db.query<any>(query, [email]);
const res = await this.db.query<DbUser & Profile>(query, [email]);
const flatUser = res.rows[0];
if (!flatUser) {
@@ -147,8 +147,7 @@ export class UserRepository {
}
// Manually construct the nested UserProfile object and add auth fields
const authableProfile: UserProfile & DbUser = {
user_id: flatUser.user_id,
const authableProfile: UserProfile & Omit<DbUser, 'user_id' | 'email'> = {
full_name: flatUser.full_name,
avatar_url: flatUser.avatar_url,
role: flatUser.role,
@@ -161,7 +160,6 @@ export class UserRepository {
user_id: flatUser.user_id,
email: flatUser.email,
},
email: flatUser.email,
password_hash: flatUser.password_hash,
failed_login_attempts: flatUser.failed_login_attempts,
last_failed_login: flatUser.last_failed_login,
@@ -231,8 +229,7 @@ export class UserRepository {
async findUserProfileById(userId: string, logger: Logger): Promise<UserProfile> {
try {
const res = await this.db.query<UserProfile>(
`SELECT
p.user_id, p.full_name, p.avatar_url, p.address_id, p.preferences, p.role, p.points,
`SELECT p.full_name, p.avatar_url, p.address_id, p.preferences, p.role, p.points,
p.created_at, p.updated_at,
json_build_object(
'user_id', u.user_id,