-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Slack integration #9
base: master
Are you sure you want to change the base?
Conversation
Run |
@@ -0,0 +1,4 @@ | |||
export default { | |||
apiToken: process.env.SLACK_API_TOKEN, | |||
channel: process.env.SLACK_NOTIFICATIONS_CHANNEL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
channel: process.env.SLACK_NOTIFICATIONS_CHANNEL, | |
channel: process.env.SLACK_NOTIFICATIONS_CHANNEL || '', |
) { | ||
} | ||
|
||
@Post('/slack/to-users') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post by default returns 201 Created. I am not sure that this should do this.
Maybe 204 is better?
} | ||
|
||
@Post('/slack/to-channel') | ||
public async sendNotificationToChannel(@Query('date') date: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post by default returns 201 Created. I am not sure that this should do this.
Maybe 204 is better?
providers: [NotificationsService, SlackService], | ||
}) | ||
export class NotificationsModule { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be moved to the same line, but it's nothing special :)
@Post('/slack/to-channel') | ||
public async sendNotificationToChannel(@Query('date') date: string) { | ||
const lastWorkingDate = new Date(date); // todo get last working date | ||
const users: UserWorklogResult[] = await this.aggregatorService.aggregate(lastWorkingDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think, should we move worklogs aggregation to NotificationService? I think it's service work.
.map(result => [`:pisiorek: ${result.firstName} ${result.lastName}`]) | ||
.join('\n')}`; | ||
|
||
return this.slackService.sendToChannel(message, 'worklog-monitor-int'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded channel.
Co-Authored-By: SimonB407 <[email protected]>
# Conflicts: # .env.dist # package.json # src/app.module.ts
No description provided.