feat: dont run the date update in the transaction

isomerpages · Apr 9, 2024 · addbe74 · addbe74
1 parent 7022a5b
commit addbe74
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 22 deletions.
diff --git a/src/services/identity/UsersService.ts b/src/services/identity/UsersService.ts
@@ -136,28 +136,32 @@ class UsersService {
   }
 
   async login(githubId: string): Promise<User> {
-    return this.sequelize.transaction<User>(async (transaction) => {
-      const [user] = await this.repository.findOrCreate({
-        where: { githubId },
-        transaction,
-      })
-      user.lastLoggedIn = new Date()
-
-      return user.save({ transaction })
+    const loginUpdate = { lastLoggedIn: new Date() }
+    const [user, created] = await this.repository.findOrCreate({
+      where: { githubId },
+      defaults: loginUpdate,
     })
+
+    if (!created) {
+      await user.update(loginUpdate)
+    }
+
+    return user
   }
 
   async loginWithEmail(email: string): Promise<User> {
     const parsedEmail = email.toLowerCase()
-    return this.sequelize.transaction<User>(async (transaction) => {
-      const [user] = await this.repository.findOrCreate({
-        where: { email: parsedEmail },
-        transaction,
-      })
-      user.lastLoggedIn = new Date()
-
-      return user.save({ transaction })
+    const loginUpdate = { lastLoggedIn: new Date() }
+    const [user, created] = await this.repository.findOrCreate({
+      where: { email: parsedEmail },
+      defaults: loginUpdate,
     })
+
+    if (!created) {
+      await user.update(loginUpdate)
+    }
+
+    return user
   }
 
   async canSendEmailOtp(email: string) {

diff --git a/src/services/identity/__tests__/UsersService.spec.ts b/src/services/identity/__tests__/UsersService.spec.ts
@@ -102,8 +102,9 @@ describe("User Service", () => {
 
   it("should call `findOrCreate` on the db model and set the lastLoggedIn", async () => {
     // Arrange
+    const testTime = Date.now()
     const mockDbUser = {
-      save: jest.fn().mockReturnThis(),
+      update: jest.fn().mockReturnThis(),
       githubId: mockGithubId,
     }
     MockRepository.findOrCreate.mockResolvedValue([mockDbUser])
@@ -112,16 +113,13 @@ describe("User Service", () => {
     const actual = await UsersService.login(mockGithubId)
 
     // Assert
-    expect(MockSequelize.transaction).toBeCalled()
     expect(MockRepository.findOrCreate).toBeCalledWith({
       where: { githubId: mockGithubId },
-      transaction: "transaction",
+      defaults: { lastLoggedIn: expect.any(Date) },
     })
     expect(actual.lastLoggedIn).toBeDefined()
+    expect(actual.lastLoggedIn.getTime()).toBeGreaterThanOrEqual(testTime)
     expect(actual.githubId).toBe(mockGithubId)
-    expect(actual.save).toBeCalledWith({
-      transaction: "transaction",
-    })
   })
 
   it("should allow whitelisted emails", async () => {