updated get userById to knex (#1491)

* updated get userById to knex * Changed userManager controller and testing. Migrate to Knex * Minor changes to adapt data transfer for knex migration * package json & changelog update * comment deleted * chagelog date updated
IT-Academy-BCN · Oct 15, 2024 · 69cbf10 · 69cbf10
1 parent a8e042b
commit 69cbf10
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 81 deletions.
diff --git a/services/sso/CHANGELOG.md b/services/sso/CHANGELOG.md
@@ -2,6 +2,12 @@ All notable changes to this project will be documented in this file.
 
 # Changelog
 
+## [1.43.0] - 2024-10-14
+
+### Changed
+
+- Update userManager controller. Migrate to Knex.
+
 ## [1.42.0] - 2024-10-07
 
 ### Changed

diff --git a/services/sso/package.json b/services/sso/package.json
@@ -1,6 +1,6 @@
 {
   "name": "sso-service",
-  "version": "1.42.0",
+  "version": "1.43.0",
   "description": "",
   "directories": {
     "test": "test"

diff --git a/services/sso/src/__tests__/unit/utils/userManager.test.ts b/services/sso/src/__tests__/unit/utils/userManager.test.ts
@@ -4,13 +4,11 @@ import {
   expect,
   vi,
   beforeEach,
-  Mock,
   beforeAll,
-  afterAll,
+  afterEach,
 } from 'vitest'
 import { createTracker, MockClient, Tracker } from 'knex-mock-client'
 import knex from 'knex'
-import { client } from '../../../db/client'
 import { userManager } from '../../../db/managers/userManager'
 import { UserStatus } from '../../../schemas/users/userSchema'
 import db from '../../../db/knexClient'
@@ -21,61 +19,61 @@ type MockUser = {
   status: string
   deletedAt?: null | Date
 }
+let tracker: Tracker
+beforeAll(() => {
+  tracker = createTracker(db)
+})
+beforeEach(() => {
+  vi.clearAllMocks()
+})
+afterEach(() => {
+  tracker.reset()
+})
 
-vi.mock('../../../db/client', () => ({
-  client: {
-    query: vi.fn(),
-  },
-}))
 vi.mock('../../../db/knexClient', async () => {
-  const bd = knex({ client: MockClient })
+  const bd = knex({
+    client: MockClient,
+    dialect: 'pg',
+  })
   return {
     default: bd,
   }
 })
 describe('userManager.getUser', () => {
   const userId = '123'
 
-  beforeEach(() => {
-    vi.clearAllMocks()
-  })
-
   it('should retrieve a user with the specified fields', async () => {
     const mockUser: MockUser = {
       id: userId,
       email: 'user@example.cat',
       status: UserStatus.ACTIVE,
     }
-    ;(client.query as Mock).mockResolvedValue({ rows: [mockUser] })
+    tracker.on.select('user').response([mockUser])
 
     const user = await userManager.getUser(userId, {
       fields: ['id', 'email', 'status'],
     })
-
-    expect(client.query).toHaveBeenCalledWith(
-      `SELECT id, email, status FROM "user" WHERE id = $1 AND deleted_at IS NULL`,
-      [userId]
+    expect(tracker.history.select[0].sql).toEqual(
+      'select "id", "email", "status" from "user" where "id" in ($1) and "deleted_at" is null'
     )
     expect(user).toEqual(mockUser)
   })
 
   it('should return null if user is not found', async () => {
-    ;(client.query as Mock).mockResolvedValue({ rows: [] })
+    tracker.on.select('user').response([])
 
     const user = await userManager.getUser(userId, {
       fields: ['id', 'email', 'status'],
     })
-
-    expect(client.query).toHaveBeenCalledWith(
-      `SELECT id, email, status FROM "user" WHERE id = $1 AND deleted_at IS NULL`,
-      [userId]
+    expect(tracker.history.select[0].sql).toEqual(
+      'select "id", "email", "status" from "user" where "id" in ($1) and "deleted_at" is null'
     )
     expect(user).toBeNull()
   })
 
   it('should handle SQL query errors', async () => {
     const errorMessage = 'SQL query failed'
-    ;(client.query as Mock).mockRejectedValue(new Error(errorMessage))
+    tracker.on.select('user').simulateError(errorMessage)
 
     await expect(
       userManager.getUser(userId, {
@@ -86,10 +84,6 @@ describe('userManager.getUser', () => {
 })
 
 describe('userManager.getUsers', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-  })
-
   it('should retrieve a list of users with the specified fields and active status', async () => {
     const mockUsers: MockUser[] = [
       {
@@ -103,18 +97,17 @@ describe('userManager.getUsers', () => {
         status: UserStatus.ACTIVE,
       },
     ]
-    ;(client.query as Mock).mockResolvedValue({ rows: mockUsers })
 
+    tracker.on.select('user').response(mockUsers)
     const users = await userManager.getUsersByIds(
       { fields: ['id', 'email', 'status'] },
       true,
       ['123', '456']
     )
-
-    expect(client.query).toHaveBeenCalledWith(
-      `SELECT id, email, status FROM "user" WHERE deleted_at IS NULL AND status = $1 AND id IN ($2, $3)`,
-      ['ACTIVE', '123', '456']
+    expect(tracker.history.all[0].sql).toEqual(
+      'select "id", "email", "status" from "user" where "deleted_at" is null and "status" = $1 and "id" in ($2, $3)'
     )
+    expect(tracker.history.all[0].bindings).toEqual(['ACTIVE', '123', '456'])
     expect(users).toEqual(mockUsers)
   })
 
@@ -131,46 +124,36 @@ describe('userManager.getUsers', () => {
         status: UserStatus.PENDING,
       },
     ]
-    ;(client.query as Mock).mockResolvedValue({ rows: mockUsers })
+
+    tracker.on.select('user').response(mockUsers)
 
     const users = await userManager.getUsersByIds(
       { fields: ['id', 'email', 'status'] },
       false,
       ['123', '456']
     )
 
-    expect(client.query).toHaveBeenCalledWith(
-      `SELECT id, email, status FROM "user" WHERE deleted_at IS NULL AND id IN ($1, $2)`,
-      ['123', '456']
+    expect(tracker.history.all[0].sql).toEqual(
+      'select "id", "email", "status" from "user" where "deleted_at" is null and "id" in ($1, $2)'
     )
+    expect(tracker.history.all[0].bindings).toEqual(['123', '456'])
+
     expect(users).toEqual(mockUsers)
   })
 
   it('should return an empty array if no users are found', async () => {
-    ;(client.query as Mock).mockResolvedValue({ rows: [] })
+    tracker.on.select('user').response([])
 
     const users = await userManager.getUsersByIds({
       fields: ['id', 'email', 'status'],
     })
-
-    expect(client.query).toHaveBeenCalledWith(
-      `SELECT id, email, status FROM "user" WHERE deleted_at IS NULL`,
-      []
+    expect(tracker.history.all[0].sql).toEqual(
+      'select "id", "email", "status" from "user" where "deleted_at" is null'
     )
     expect(users).toEqual([])
   })
 })
-let tracker: Tracker
-beforeAll(() => {
-  tracker = createTracker(db)
-})
-beforeEach(() => {
-  vi.clearAllMocks()
-  tracker.reset()
-})
-afterAll(() => {
-  tracker.reset()
-})
+
 describe('userManager.findById', () => {
   const userId = '123'
 

diff --git a/services/sso/src/db/managers/userManager.ts b/services/sso/src/db/managers/userManager.ts
@@ -1,6 +1,5 @@
 import { User, UserPatch } from '../../schemas'
 import { ItinerayList } from '../../schemas/itineraries/itinerariesListSchema'
-import { client } from '../client'
 import db from '../knexClient'
 
 /**
@@ -81,13 +80,14 @@ export const userManager = {
   ): Promise<Pick<User, T> | null> {
     const snakeCase = await getSnakeCase()
     const camelCase = await getCamelCase()
-    const fieldsToSelect = options.fields.map((f) => snakeCase(f)).join(', ')
-    const query = `SELECT ${fieldsToSelect} FROM "user" WHERE id = $1 AND deleted_at IS NULL`
+    const fieldsToSelect = options.fields.map((f) => snakeCase(f))
     const values = [id]
-    const result = await client.query(query, values)
-
-    if (result.rows.length) {
-      const row = result.rows[0]
+    const result = await db('user')
+      .select(fieldsToSelect)
+      .whereIn('id', values)
+      .andWhere('deleted_at', null)
+    if (result.length) {
+      const row = result[0]
       const camelCaseRow = Object.keys(row).reduce((acc: any, key) => {
         acc[camelCase(key)] = row[key]
         return acc
@@ -194,33 +194,30 @@ export const userManager = {
   ): Promise<Pick<User, T>[]> {
     const snakeCase = await getSnakeCase()
     const camelCase = await getCamelCase()
-    const fieldsToSelect = options.fields.map((f) => snakeCase(f)).join(', ')
-    let query = `SELECT ${fieldsToSelect} FROM "user" WHERE deleted_at IS NULL`
-    const values: string[] = []
-
+    const fieldsToSelect = options.fields.map((f) => snakeCase(f))
+    const query = db<User>('user')
+      .select(fieldsToSelect)
+      .where('deleted_at', null)
     if (activeOnly) {
-      query += ` AND status = $1`
-      values.push('ACTIVE')
+      query.where('status', 'ACTIVE')
     }
 
     if (ids && ids.length > 0) {
-      const idsPlaceholders = ids
-        .map((_, index) => `$${values.length + index + 1}`)
-        .join(', ')
-      query += ` AND id IN (${idsPlaceholders})`
-      values.push(...ids)
+      query.whereIn('id', ids)
     }
-
-    const result = await client.query(query, values)
-
-    if (result.rows.length) {
-      return result.rows.map((row) => {
-        const camelCaseRow = Object.keys(row).reduce((acc: any, key) => {
-          acc[camelCase(key)] = row[key]
-          return acc
-        }, {} as Pick<User, T>)
+    const result = await query
+    if (result.length) {
+      const res: Pick<User, T>[] = result.map((row) => {
+        const camelCaseRow = Object.entries(row).reduce(
+          (acc: any, [key, value]) => {
+            acc[camelCase(key)] = value
+            return acc
+          },
+          {} as Pick<User, T>
+        )
         return camelCaseRow
       })
+      return res
     }
     return []
   },