refactor(modules/history): move complex logic to service

src/modules/history/history.module.ts

@@ -6,6 +6,7 @@ import { History } from './history.entity'
 import { HistoryRepository } from './history.repository'
 import { HistoryScheduler } from './history.scheduler'
 import { HistoryTracksModule } from './tracks'
+import { HistoryService } from './history.service'
 
 import { UsersModule } from '@modules/users'
 import { SpotifyModule } from '@modules/spotify'
@@ -22,7 +23,7 @@ import { AlbumsModule } from '@modules/albums'
     AlbumsModule,
     HistoryTracksModule,
   ],
-  providers: [HistoryRepository, HistoryScheduler],
-  exports: [HistoryRepository, HistoryScheduler],
+  providers: [HistoryRepository, HistoryScheduler, HistoryService],
+  exports: [HistoryRepository, HistoryScheduler, HistoryService],
 })
 export class HistoryModule {}

src/modules/history/history.repository.spec.ts

@@ -1,18 +1,14 @@
 import { Test } from '@nestjs/testing'
 import { DataSource } from 'typeorm'
 import { MockProxy, mock } from 'vitest-mock-extended'
-import { PlayHistory } from '@spotify/web-api-ts-sdk'
 
 import { HistoryRepository } from './history.repository'
 import { History } from './history.entity'
-import { HistoryTracksRepository } from './tracks'
-import { HistoryTrack } from './tracks'
 
 describe('HistoryRepository', () => {
   const userId = 'userId'
 
   let historyRepository: HistoryRepository
-  let historyTracksRepository: HistoryTracksRepository
 
   let historyEntityMock: MockProxy<History>
 
@@ -25,19 +21,12 @@ describe('HistoryRepository', () => {
             createEntityManager: vi.fn(),
           },
         },
-        {
-          provide: HistoryTracksRepository,
-          useValue: {
-            createHistoryTracksFromPlayHistory: vi.fn(),
-            findLastHistoryTrackByUser: vi.fn(),
-          },
-        },
+
         HistoryRepository,
       ],
     }).compile()
 
     historyRepository = module.get(HistoryRepository)
-    historyTracksRepository = module.get(HistoryTracksRepository)
 
     historyEntityMock = mock<History>()
   })
@@ -63,115 +52,4 @@ describe('HistoryRepository', () => {
       },
     })
   })
-
-  describe('updateOrCreateHistoryByUser', () => {
-    let historyTrack: MockProxy<HistoryTrack>
-    let historyTracks: MockProxy<HistoryTrack[]>
-    let playHistoryMock: MockProxy<PlayHistory[]>
-
-    beforeEach(() => {
-      historyTrack = mock<HistoryTrack>()
-      historyTracks = [historyTrack]
-      playHistoryMock = mock<PlayHistory[]>()
-
-      historyEntityMock.historyTracks = historyTracks
-    })
-
-    test('should create new history', async () => {
-      const findHistoryByUserSpy = vi
-        .spyOn(historyRepository, 'findHistoryByUser')
-        .mockResolvedValue(null)
-      const createHistoryTracksFromPlayHistorySpy = vi
-        .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory')
-        .mockResolvedValue(historyTracks)
-      const createSpy = vi
-        .spyOn(historyRepository, 'create')
-        .mockReturnValue(historyEntityMock)
-      const saveSpy = vi
-        .spyOn(historyRepository, 'save')
-        .mockResolvedValue(historyEntityMock)
-
-      expect(
-        await historyRepository.updateOrCreateHistoryByUser(
-          historyEntityMock.user,
-          playHistoryMock
-        )
-      ).toEqual(historyEntityMock)
-
-      expect(findHistoryByUserSpy).toHaveBeenCalledWith(
-        historyEntityMock.user.id
-      )
-      expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalledWith(
-        playHistoryMock,
-        historyEntityMock.user
-      )
-      expect(createSpy).toHaveBeenCalledWith({
-        user: historyEntityMock.user,
-        historyTracks,
-      })
-      expect(saveSpy).toHaveBeenCalledWith(historyEntityMock)
-    })
-
-    test('should update existing history if historyTracks is not empty', async () => {
-      const findHistoryByUserSpy = vi
-        .spyOn(historyRepository, 'findHistoryByUser')
-        .mockResolvedValue(historyEntityMock)
-      const findLastHistoryTrackByUserSpy = vi
-        .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser')
-        .mockResolvedValue(historyTrack)
-      const createHistoryTracksFromPlayHistorySpy = vi
-        .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory')
-        .mockResolvedValue(historyTracks)
-      const saveSpy = vi
-        .spyOn(historyRepository, 'save')
-        .mockResolvedValue(historyEntityMock)
-
-      expect(
-        await historyRepository.updateOrCreateHistoryByUser(
-          historyEntityMock.user,
-          playHistoryMock
-        )
-      ).toEqual(historyEntityMock)
-
-      expect(findHistoryByUserSpy).toHaveBeenCalledWith(
-        historyEntityMock.user.id
-      )
-      expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith(
-        historyEntityMock.user.id
-      )
-      expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled()
-      expect(saveSpy).toHaveBeenCalledWith(historyEntityMock)
-    })
-
-    test('should update existing history if historyTracks is empty', async () => {
-      const findHistoryByUserSpy = vi
-        .spyOn(historyRepository, 'findHistoryByUser')
-        .mockResolvedValue(historyEntityMock)
-      const findLastHistoryTrackByUserSpy = vi
-        .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser')
-        .mockResolvedValue(null)
-      const createHistoryTracksFromPlayHistorySpy = vi
-        .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory')
-        .mockResolvedValue(historyTracks)
-      const saveSpy = vi
-        .spyOn(historyRepository, 'save')
-        .mockResolvedValue(historyEntityMock)
-
-      expect(
-        await historyRepository.updateOrCreateHistoryByUser(
-          historyEntityMock.user,
-          playHistoryMock
-        )
-      ).toEqual(historyEntityMock)
-
-      expect(findHistoryByUserSpy).toHaveBeenCalledWith(
-        historyEntityMock.user.id
-      )
-      expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith(
-        historyEntityMock.user.id
-      )
-      expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled()
-      expect(saveSpy).toHaveBeenCalledWith(historyEntityMock)
-    })
-  })
 })

src/modules/history/history.repository.ts

@@ -1,18 +1,11 @@
 import { Injectable } from '@nestjs/common'
 import { DataSource, Repository } from 'typeorm'
-import { PlayHistory } from '@spotify/web-api-ts-sdk'
 
 import { History } from './history.entity'
-import { HistoryTracksRepository } from './tracks'
-
-import { User } from '@modules/users'
 
 @Injectable()
 export class HistoryRepository extends Repository<History> {
-  constructor(
-    private readonly dataSource: DataSource,
-    private readonly historyTracksRepository: HistoryTracksRepository
-  ) {
+  constructor(private readonly dataSource: DataSource) {
     super(History, dataSource.createEntityManager())
   }
 
@@ -25,61 +18,4 @@ export class HistoryRepository extends Repository<History> {
       },
     })
   }
-
-  async updateOrCreateHistoryByUser(user: User, playHistory: PlayHistory[]) {
-    const foundHistory = await this.findHistoryByUser(user.id)
-
-    if (!foundHistory) {
-      const historyTracks =
-        await this.historyTracksRepository.createHistoryTracksFromPlayHistory(
-          playHistory,
-          user
-        )
-
-      const newHistory = this.create({
-        user,
-        historyTracks,
-      })
-
-      return this.save(newHistory)
-    }
-
-    const lastHistoryTrack =
-      await this.historyTracksRepository.findLastHistoryTrackByUser(
-        foundHistory.user.id
-      )
-
-    if (lastHistoryTrack) {
-      const latestPlayHistory = playHistory.filter(
-        (_, index) =>
-          index <
-          playHistory.findIndex(
-            ({ track, played_at }) =>
-              track.id === lastHistoryTrack.track.externalId &&
-              new Date(played_at).getTime() ===
-                lastHistoryTrack.playedAt.getTime()
-          )
-      )
-
-      const newHistoryTracks =
-        await this.historyTracksRepository.createHistoryTracksFromPlayHistory(
-          latestPlayHistory,
-          user
-        )
-
-      foundHistory.historyTracks.push(...newHistoryTracks)
-
-      return this.save(foundHistory)
-    }
-
-    const newHistoryTracks =
-      await this.historyTracksRepository.createHistoryTracksFromPlayHistory(
-        playHistory,
-        user
-      )
-
-    foundHistory.historyTracks.push(...newHistoryTracks)
-
-    return this.save(foundHistory)
-  }
 }

src/modules/history/history.scheduler.spec.ts

@@ -6,8 +6,8 @@ import { AccessToken, MaxInt } from '@spotify/web-api-ts-sdk'
 import { ConfigService } from '@nestjs/config'
 
 import { HistoryScheduler } from './history.scheduler'
-import { HistoryRepository } from './history.repository'
 import { History } from './history.entity'
+import { HistoryService } from './history.service'
 
 import { UsersRepository } from '@modules/users'
 import { SpotifyAuthService } from '@modules/spotify/auth'
@@ -21,7 +21,7 @@ describe('HistoryScheduler', () => {
   let usersRepository: UsersRepository
   let spotifyAuthService: SpotifyAuthService
   let spotifyPlayerService: SpotifyPlayerService
-  let historyRepository: HistoryRepository
+  let historyService: HistoryService
   let schedulerRegistry: SchedulerRegistry
 
   beforeEach(async () => {
@@ -52,9 +52,9 @@ describe('HistoryScheduler', () => {
           },
         },
         {
-          provide: HistoryRepository,
+          provide: HistoryService,
           useValue: {
-            updateOrCreateHistoryByUser: vi.fn(),
+            updateOrCreate: vi.fn(),
           },
         },
         {
@@ -77,7 +77,7 @@ describe('HistoryScheduler', () => {
     usersRepository = module.get(UsersRepository)
     spotifyAuthService = module.get(SpotifyAuthService)
     spotifyPlayerService = module.get(SpotifyPlayerService)
-    historyRepository = module.get(HistoryRepository)
+    historyService = module.get(HistoryService)
     schedulerRegistry = module.get(SchedulerRegistry)
   })
 
@@ -134,7 +134,7 @@ describe('HistoryScheduler', () => {
       >
     ).mockResolvedValue(recentlyPlayedTracksPageMock)
     const updateOrCreateHistoryByUserSpy = vi
-      .spyOn(historyRepository, 'updateOrCreateHistoryByUser')
+      .spyOn(historyService, 'updateOrCreate')
       .mockResolvedValue(mock<History>())
 
     await historyScheduler.fetchUserHistory(userMock)

src/modules/history/history.scheduler.ts

@@ -9,7 +9,7 @@ import { SchedulerRegistry } from '@nestjs/schedule'
 import { ConfigService } from '@nestjs/config'
 import ms from 'ms'
 
-import { HistoryRepository } from './history.repository'
+import { HistoryService } from './history.service'
 
 import { User, UsersRepository } from '@modules/users'
 import { SpotifyAuthService } from '@modules/spotify/auth'
@@ -27,7 +27,7 @@ export class HistoryScheduler implements OnModuleInit {
     private readonly usersRepository: UsersRepository,
     private readonly spotifyAuthService: SpotifyAuthService,
     private readonly spotifyPlayerService: SpotifyPlayerService,
-    private readonly historyRepository: HistoryRepository,
+    private readonly historyService: HistoryService,
     private readonly schedulerRegistry: SchedulerRegistry,
     private readonly configService: ConfigService
   ) {}
@@ -73,6 +73,6 @@ export class HistoryScheduler implements OnModuleInit {
       false
     )
 
-    await this.historyRepository.updateOrCreateHistoryByUser(user, items)
+    await this.historyService.updateOrCreate(user, items)
   }
 }