feat(apps/server/rest): find-video history#107
Conversation
…hentication with x-chrome-extension-id header
There was a problem hiding this comment.
Pull Request Overview
Adds a new “find-video” history feature to the REST API, including endpoints to create, list, and delete user history entries, as well as a daily batch job to clean up old deleted logs. It also updates CORS settings, deployment scripts, and the Prisma schema to support these new models.
- Introduce HistoryService, HistoryController, DTOs, and entities for CRUD of video/image history.
- Add a scheduled batch job to remove “deleted” logs older than one month.
- Integrate the new module into the app’s main module and update CORS origins and deploy scripts.
Reviewed Changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/find-video/history/history.service.ts | Implements CRUD operations for user history via Prisma |
| src/modules/find-video/history/history.controller.ts | Exposes REST endpoints for history create, get, and delete |
| src/modules/find-video/history/dto/*.dto.ts | Defines request/query DTOs for history APIs |
| src/modules/find-video/history/entities/history.entity.ts | Defines response entities for history API |
| prisma/schema.prisma | Adds FindVideoUser and FindVideoLog models |
| src/modules/find-video/batch/find-video-batch.service.ts | Adds a cron job to purge old deleted logs |
| src/main.ts | Updates CORS to allow additional Chrome extension origins |
| .scripts/deploy.sh | Enhances deploy script to support a “--clean” flag |
| src/modules/auth/guards/api-key.guard.ts | Refactors API key guard initialization |
| app.module.ts | Registers the new FindVideoModule |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
apps/server/rest/src/modules/find-video/history/history.service.ts:16
- [nitpick] The parameter name
historysis a nonstandard pluralization. Consider renaming it tohistoriesfor clarity and consistency.
async createHistoryBulk({ historys }: { historys: HistoryDto[] }) {
apps/server/rest/src/modules/find-video/history/history.controller.ts:85
userIdis a query parameter, but@ApiParamdocuments path parameters. Switch to@ApiQueryfor accurate Swagger docs.
@ApiParam({ name: 'userId', required: true })
apps/server/rest/src/modules/find-video/history/dto/get-history.dto.ts:5
GetHistoryDtoproperties currently lack validation decorators. Addclass-validatorannotations (e.g.,@IsString(),@IsNotEmpty(),@IsNumber()) or use@Type()pipes for robust DTO validation.
userId: string;
| */ | ||
| async deleteHistory({ userId, historyId }: { userId: string; historyId: string }) { | ||
| try { | ||
| await this.prismaService.findVideoLog.update({ |
There was a problem hiding this comment.
Prisma's update requires a unique identifier for where. Specifying both id and userId without a compound unique index will error. Use updateMany or add a compound unique constraint on (id, userId).
| @ApiResponse({ status: HttpStatus.OK, type: HistoryDto }) | ||
| @ApiInternalServerErrorResponse({ description: 'Internal server error' }) | ||
| @ApiBadRequestResponse({ description: 'Bad request body is required' }) | ||
| async createHistory(@Res() res: Response, @Body() body: HistoryDto[] | HistoryDto, @Query('bulk') bulk: boolean) { |
There was a problem hiding this comment.
Query parameters remain strings by default. Consider using @Query('bulk', ParseBoolPipe) bulk: boolean to ensure bulk is properly parsed to a boolean.
| const oneMonthAgo = now; | ||
| oneMonthAgo.setMonth(now.getMonth() - 1); | ||
| const fifteenDaysAgo = now; |
There was a problem hiding this comment.
oneMonthAgo and fifteenDaysAgo are both assigned the same Date object, so mutations affect both. Clone the date before adjusting, e.g. const oneMonthAgo = new Date(now);.
| const oneMonthAgo = now; | |
| oneMonthAgo.setMonth(now.getMonth() - 1); | |
| const fifteenDaysAgo = now; | |
| const oneMonthAgo = new Date(now); | |
| oneMonthAgo.setMonth(now.getMonth() - 1); | |
| const fifteenDaysAgo = new Date(now); |
| const fifteenDaysAgo = now; | ||
| fifteenDaysAgo.setDate(now.getDate() - 15); | ||
|
|
There was a problem hiding this comment.
The variable fifteenDaysAgo is defined but never used. Consider removing it to avoid confusion.
| const fifteenDaysAgo = now; | |
| fifteenDaysAgo.setDate(now.getDate() - 15); |
No description provided.