#2501 add reviewers comment to slack message#2872
Conversation
| } | ||
| }); | ||
|
|
||
| const newCR = await prisma.change_Request.findUnique({ |
There was a problem hiding this comment.
Why do you need to make a new variable instead of just using the updated changed request above, and editing its include args?
There was a problem hiding this comment.
I make a new variable as changing the updated variable's include args will mess with the sendSlackCRStatusToThread function call which uses updated.notificationSlackThreads.
There was a problem hiding this comment.
You can have the notificationSlackThreads and also the change request query args in the same include statement
src/backend/src/utils/slack.utils.ts
Outdated
| const msgs = []; | ||
| const fullMsg = `:tada: Your Change Request was just reviewed! Click the link to view! :tada:`; | ||
| let fullMsg; | ||
| if (!comments) { |
There was a problem hiding this comment.
Can you simplify this if-else block by using a ternary operator?
vsp05
left a comment
vsp05
left a comment
There was a problem hiding this comment.
Looks good to me, other than Alan's requested changes. When that is replaced with a ternary operator I'll approve. Also, I played around with the newCr a bit, and I think I agree with the way you did it.
src/backend/src/utils/slack.utils.ts
Outdated
| const msgs = []; | ||
| const fullMsg = `:tada: Your Change Request was just reviewed! Click the link to view! :tada:`; | ||
| // let fullMsg; | ||
| const fullMsg = !comments |
There was a problem hiding this comment.
The ternary can be inline in the string because in this case there is still repetition of the string when there is and isn't comments. For example "...reviewed!\n${comments ? 'some string with the comments' : ''}..."
| await this.reviewActivationChangeRequest(foundCR, reviewer); | ||
| } | ||
|
|
||
| const includeArgs = getChangeRequestQueryArgs(organization.organizationId).include; |
There was a problem hiding this comment.
This can just be in the include: {}, doesn't need a variable if it is only being used once
src/backend/src/utils/slack.utils.ts
Outdated
| if (process.env.NODE_ENV !== 'production') return; // don't send msgs unless in prod | ||
| const msgs = []; | ||
| const fullMsg = `:tada: Your Change Request was just reviewed! Click the link to view! :tada:`; | ||
| // let fullMsg; |
4 participants
Changes
Changed the existing method to add reviewers comment to the string that gets sent as a slack message
Test Cases
Tested by creating and reviewing a CR using the UI.
Screenshots
To Do
None
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes #2501