PM-2222 - send notification when ai workflow run has compelted #122
Conversation
vas3a
commented
Oct 10, 2025
vas3a
commented
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| private readonly prisma: PrismaService, | ||
| private readonly scheduler: QueueSchedulerService, | ||
| private readonly giteaService: GiteaService, | ||
| private readonly eventBusService: EventBusService, |
There was a problem hiding this comment.
The EventBusService is added as a dependency but there is no indication in this diff how it is used. Ensure that it is utilized appropriately in the class to justify its inclusion.
| await this.sendWorkflowRunCompletedNotification(aiWorkflowRun); | ||
| } catch (e) { | ||
| this.logger.log( | ||
| `Failed to send workflowRun compelted notification for aiWorkflowRun ${aiWorkflowRun.id}. Got error ${e.message ?? e}!`, |
There was a problem hiding this comment.
Typo in the log message: 'compelted' should be 'completed'.
|
|
||
| if (!submission) { | ||
| this.logger.log( | ||
| `Failed to send workflowRun compelted notification for aiWorkflowRun ${aiWorkflowRun.id}. Submission ${aiWorkflowRun.submissionId} is missing!`, |
There was a problem hiding this comment.
Typo in the log message: 'compelted' should be 'completed'.
|
|
||
| if (!challenge) { | ||
| this.logger.log( | ||
| `Failed to send workflowRun compelted notification for aiWorkflowRun ${aiWorkflowRun.id}. Challenge ${submission.challengeId} couldn't be fetched!`, |
There was a problem hiding this comment.
Typo in the log message: 'compelted' should be 'completed'.
|
|
||
| if (!user) { | ||
| this.logger.log( | ||
| `Failed to send workflowRun compelted notification for aiWorkflowRun ${aiWorkflowRun.id}. User ${submission.memberId} couldn't be fetched!`, |
There was a problem hiding this comment.
Typo in the log message: 'compelted' should be 'completed'.
| recipients: [user.email], | ||
| data: { | ||
| userName: | ||
| [user.firstName, user.lastName].filter(Boolean).join(' ') ?? |
There was a problem hiding this comment.
The use of filter(Boolean).join(' ') ?? user.handle is incorrect. The nullish coalescing operator ?? will not be reached because join(' ') will always return a string. Consider using a conditional check to handle cases where both firstName and lastName are undefined.
| 'd-7d14d986ba0a4317b449164b73939910', | ||
| }, | ||
| ui: { | ||
| reviewUIUrl: |
There was a problem hiding this comment.
Consider renaming reviewUIUrl to reviewUiUrl to maintain consistent camelCase naming convention.
| recipients: [user.email], | ||
| data: { | ||
| userName: | ||
| [user.firstName, user.lastName].filter(Boolean).join(' ') || |
There was a problem hiding this comment.
Consider using the nullish coalescing operator (??) instead of the logical OR operator (||) for better handling of null or undefined values. The nullish coalescing operator will only fall back to user.handle if the result of the join operation is null or undefined, whereas the logical OR operator will also fall back if the result is an empty string.
| return; | ||
| } | ||
|
|
||
| const [user] = await this.membersPrisma.$queryRaw< |
There was a problem hiding this comment.
Consider renaming membersPrisma to something more descriptive if it represents a specific database connection or context. This will improve code readability and maintainability.
|
@kkartunov ok to merge this? |
2 participants
