Connect app to the c0d3r (Discord Bot) #949

JasirZaeem · 2021-07-10T12:50:56Z

Part of #843

As a part of the effort to integrate the discord server with c0d3.com I had created a Discord bot, c0d3r, here which exposes a REST Api to interact with Discord.

This PR adds utility functions to the app that allows interaction with this bot, right now this lets us send messages to Discord from the app and more features can be added later as required.

Full integration to achieve the same that we had mattermost can be done once the OAuth integration, under process, is completed.

Until then the following notifications have been added in this PR which the app will send to Discord.

On challenge submission

When a new challenge is submitted the app will call the bot to send a message to the channel of the next lesson to notify reviewers. e.g. to the JS2 channel for submissions in lesson JS1 (submissions for the last lesson will be notified in its channel itself, currently this is JS9)

Example

On lesson completion

When a student completes a lesson two notifications will be sent, one to the completed lesson to notify that the student has completed the lesson, and one to the next lesson's channel to notify that a new student is beginning the lesson, as was the case with mattermost. In case of the last lesson (currently JS9) the second notification will be skipped.

Example

1st notification

2nd notification

On receiving a star

When a reviewer receives a star a notification is sent to the channel for the lesson they received the star for.

Example

TODO

Once the OAuth integration is complete and the app has knowledge of the user's discord id more ways to communicate with discord through the bot can be implemented. For example challenge submission notifications can mention the discord user with their correct username/server nickname (if set) once we have their id like this

Other changes

After discussion with @songz , added discord.js to be able to use it's types for better type safety.
c0d3r now immediately responds to requests after a firing a request to discord (for valid requests) by default, an additional parameter includeDetails can be passed in json bodies of requests, requests with these will be responded to after the message is sent to discord with the id of the message. Relevant PR
added .idea to .gitignore to ignore files created by the jetbrains ides like webstorm.

vercel · 2021-07-10T12:51:00Z

@JasirZaeem is attempting to deploy a commit to a Personal Account owned by @garageScript on Vercel.

@garageScript first needs to authorize it.

codecov · 2021-07-10T12:54:09Z

Codecov Report

Merging #949 (4656e5c) into master (ec824e7) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #949   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          115       116    +1     
  Lines         1974      2000   +26     
  Branches       485       487    +2     
=========================================
+ Hits          1974      2000   +26

Impacted Files	Coverage Δ
helpers/controllers/starsController.ts	`100.00% <100.00%> (ø)`
helpers/controllers/submissionController.ts	`100.00% <100.00%> (ø)`
helpers/discordBot.ts	`100.00% <100.00%> (ø)`
helpers/updateSubmission.ts	`100.00% <100.00%> (ø)`

vercel · 2021-07-10T13:02:25Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/c0d3/c0d3-app/3F31RSxk1t8upFVLZi982LzwQevj
✅ Preview: https://c0d3-app-git-fork-jasirzaeem-add-discord-bot-c0d3.vercel.app

The review link was for the same lesson as the lesson to send notification to, fixed

ggwadera · 2021-07-10T14:32:37Z

helpers/discordBot.ts

+  fetch(`${messagesEndpoint}/channel/${channelId}`, {
+    method: 'POST',
+    headers,
+    body: JSON.stringify({
+      message,
+      embed
+    })
+  }).then(r => r.json())


Should probably add a .catch() on these fetches to handle unexpected errors that could interrupt the API call.

Could also maybe extract a helper function here with the fetch call since the parameters are mostly the same for all the functions.

Well there is actually another issue were we need to rememeber we are running serverless functions so unless you await every async call there will be no work finished after the user gets there response.

But I don't think we really want our users waiting for a discord msg to go through so not awaiting is good in this case.

I think the real proper pattern we need to implement is if we want to catch all the errors is either auto respond with a msg received update and await that or just send it and know all error reporting is responsible via the receiver

(We could still add a catch but I dont believe the errors would ever make it to our logs or would do so inconsistently)

Well there is actually another issue were we need to remember we are running serverless functions so unless you await every async call there will be no work finished after the user gets there response.

Yeah, when solving the issue with emails not being sent I believe part of the problem was that the promise was not being awaited. In this case the await should be put on the function that will be calling this, because the return of these functions are going to be 'Promise' even with the await.

For the error catching, in the case of these notifications I think they should be handled gracefully and not propagate up the call stack. Since these notifications are not crucial for the app they can be ignored and just be logged so us on the engineering team can look into it, but the response for the client should still be successful.

My understanding is all these discord functions should be fire-and-forget. They send a command to the bot and that's it, this is to ensure that requests, of which these are called a part of, don't wait on response from discord as that is not necessary.

And so, keeping this in mind, like @tomrule007 pointed out, the serverless invocation that was handling a request may have already ended before discord responds.

Considering this, I'm not sure what a catch function would do here. The intended use is to call these without awaiting them or adding .then, if the caller needs the returned response then it would be their responsibility to take care of the error handling by either adding .catch then or if using async/await then surrounding it in a try catch block.

Does this make sense? if not then I'd need some suggestion as to what to do inside a .catch if a attach it to these.

@ggwadera So is your suggestion then to await where they are called, and add a catch to these functions which silently logs the error on failure and lets the rest of the caller function run like usual ?

Hmmm good idea. The bot could reply immediately since we don't to need to wait for the Discord API call to finish it here. After the bot sends the response back it could then process the notification normally.

Detailed error logs could be done on the bot side too, here we could just leave a generic catch to avoid halting the execution.

Yeah I am working on that, I will change the default behaviour of the bot to respond immediately without waiting on discord, while providing an option to get details of the message if needed, as these details can be used to perform tasks like replying to these messages, editing them reacting to them and so on.
So the default behaviour of the bot will be to respond without waiting on discord, but if requested it will wait for the message sending to complete and respond with the details of the message, this can then be used in the future to build more features.
Adding logging in the bot itself is a task in progress.

If this is supposed to be fire and forget function, then why do you need .then(r=>r.json()) here?

That's because in most cases you would just fire and forget it, but, it does return a json body with any errors and (optionally) details of the message the was created at discord, so if a function wants to use any of these it can just acquire an object.

Calls to dicord functions are now awaited.

Common fetch calls in discord functions have been moved into a separate function.

The function calling fetch for c0d3r is now chained by .catch to prevent promise rejections from affecting called functions.

c0d3r will immediately respond to requests by default after Make bot respond to api requests without waiting for discord. c0d3r#11

Ulisseus · 2021-07-10T19:25:59Z

@types/discordBot.d.ts

@@ -0,0 +1,70 @@
+import { BotErrorType, IdType } from '../helpers/discordBot'
+
+export type snowflake = string


I don't understand this snowflake type. Why not use string instead?

It's just for convenience, to make it clear at a glance that it'll be something like '828783458469675019' and not an arbitrary string https://en.wikipedia.org/wiki/Snowflake_ID .

It's just an type alias. This way we can know that this string type actually has a snowflake id format.

TIL about snowflake ids. This wiki link should be in a comment IMO.

discord.js has been added as dependency, snowflake type alias is now imported from it. A comment describing it has been added in discordBot.d.ts

@types/discordBot.d.ts

Ulisseus · 2021-07-10T19:29:21Z

@types/discordBot.d.ts

+  color?: any
+  fields?: any[]
+  files?: any[]
+  author?: any
+  thumbnail?: any
+  image?: any
+  video?: any
+  footer?: any


Is it possible to type these fields?

https://github.com/discordjs/discord.js/blob/master/typings/index.d.ts#L3546-L3558 it depends on many other types I'm not sure how deep to go

could you add a comment with the link?

couldn't we import discord.js's types instead of recreating this? Of no, then perhaps instead of any, we just type the specific resource that we need. If in the future someone needs more, they can add it in by following your comment

As discussed, discord.js has been added as a dependency and relevant types are now imported from it rather than been manually recreated.

Ulisseus · 2021-07-10T19:49:33Z

helpers/discordBot.ts

+  fetch(`${messagesEndpoint}/channel/${channelId}`, {
+    method: 'POST',
+    headers,
+    body: JSON.stringify({
+      message,
+      embed
+    })
+  }).then(r => r.json())


If this is supposed to be fire and forget function, then why do you need .then(r=>r.json()) here?

hwong0305 · 2021-07-13T20:29:21Z

helpers/discordBot.test.js

+const channelId = '1234'
+const message = 'hello'
+const embed = { description: 'an embed' }
+const lessonId = 1
+const notificationLessonId = 2
+const idType = IdType.C0D3
+const userName = 'test_user'
+const discordUserId = '9898'
+const challengeTitle = 'test challenge'


Can you please make this into an object instead? I think it's neater than having separate variables for everything.

Might not look the neatest but it does make all the code below it very easy to read IMO

const mockData = { channelId: '1234', message: 'hello', embed: { description: 'an embed' }, lessonId: 1, notificationLessonId: 2, idType: IdType.C0D3, userName: 'test_user', discordUserId: '9898', challengeTitle: 'test challenge' }

Is this fine? variable names will be longer where used but should be readable

songz · 2021-07-13T22:33:56Z

@JasirZaeem I assigned this issue to you perhaps you could take a look afterward? Its sort of related to this feature:
#954

tomrule007

Looks good, just got the one question about discord.js possibly being a dev dependency if it is only used for Types? (I'm still a ts noob so i dont know how that all works)

And lastly I thought you could remove some extra database calls by doing some if the id look ups on the c0d3r side. Not really sure how much time an extra database call roundtrip adds.

package.json

helpers/controllers/submissionController.ts

hwong0305 · 2021-07-14T03:34:46Z

.gitignore

@@ -116,3 +116,6 @@ dist

 #removing package-lock as the master branch has yarn.lock
 package-lock.json
+
+# Jetbrains ide config files
+.idea


This is the first time I've seen someone use Jetbrains as an IDE for JS/TS.

I find WebStorm very convenient :D

JasirZaeem · 2021-07-14T05:46:21Z

@JasirZaeem I assigned this issue to you perhaps you could take a look afterward? Its sort of related to this feature:
#954

Yeah, sure, I'll work on it.

JasirZaeem added 6 commits July 10, 2021 12:08

Add example env vars for c0d3r

8441fd6

ignore jetbrains config files

dfad158

Create types and functions to interact with c0d3r

d056c13

Send notifications to discord when a new submission is created

fcabbce

Send notifications when a star is given

dfdce83

Send notifications when a student completes a lesson

1c3cf65

vercel bot deployed to Preview July 10, 2021 13:02 View deployment

JasirZaeem added 2 commits July 10, 2021 13:35

Fix submission notification

4cedaf0

The review link was for the same lesson as the lesson to send notification to, fixed

Update discordBot.test.js for the fix

b39cf7f

vercel bot deployed to Preview July 10, 2021 13:56 View deployment

ggwadera reviewed Jul 10, 2021

View reviewed changes

Ulisseus reviewed Jul 10, 2021

View reviewed changes

tomrule007 mentioned this pull request Jul 10, 2021

Handling 3rd party services with serverless hosting #950

Open

Ulisseus reviewed Jul 10, 2021

View reviewed changes

JasirZaeem added 5 commits July 12, 2021 22:06

Merge branch 'master' into add-discord-bot

98b5f18

Add discord to enable use of discord types for type safety

0d02aaa

Remove common fetch calls into a function, use discord.js types.

4a988df

Await calls to c0d3r

b363a99

Update MessageResponse type to add sending status response

99c3754

vercel bot deployed to Preview July 12, 2021 17:30 View deployment

songz approved these changes Jul 13, 2021

View reviewed changes

hwong0305 reviewed Jul 13, 2021

View reviewed changes

ggwadera approved these changes Jul 13, 2021

View reviewed changes

tomrule007 approved these changes Jul 14, 2021

View reviewed changes

package.json Show resolved Hide resolved

helpers/controllers/submissionController.ts Show resolved Hide resolved

hwong0305 reviewed Jul 14, 2021

View reviewed changes

hwong0305 approved these changes Jul 14, 2021

View reviewed changes

Ulisseus approved these changes Jul 14, 2021

View reviewed changes

Collect test data into an object in discordBot.test.js

cadf410

vercel bot deployed to Preview July 14, 2021 09:30 View deployment

JasirZaeem added 2 commits July 14, 2021 12:20

Bold star receiver name in discord message

25791ee

Update tests for completed lesson notification

4656e5c

vercel bot deployed to Preview July 14, 2021 12:36 View deployment

JasirZaeem merged commit 756a3e8 into garageScript:master Jul 16, 2021

JasirZaeem deleted the add-discord-bot branch July 22, 2021 07:00

JasirZaeem mentioned this pull request May 3, 2022

Improved Discord notifications #1826

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connect app to the c0d3r (Discord Bot) #949

Connect app to the c0d3r (Discord Bot) #949

JasirZaeem commented Jul 10, 2021 •

edited

Loading

vercel bot commented Jul 10, 2021

codecov bot commented Jul 10, 2021 •

edited

Loading

vercel bot commented Jul 10, 2021 •

edited

Loading

ggwadera Jul 10, 2021 •

edited

Loading

tomrule007 Jul 10, 2021 •

edited

Loading

ggwadera Jul 10, 2021

JasirZaeem Jul 10, 2021

JasirZaeem Jul 10, 2021

ggwadera Jul 10, 2021 •

edited

Loading

JasirZaeem Jul 10, 2021

Ulisseus Jul 10, 2021

JasirZaeem Jul 10, 2021

JasirZaeem Jul 12, 2021 •

edited

Loading

Ulisseus Jul 10, 2021

JasirZaeem Jul 10, 2021 •

edited

Loading

ggwadera Jul 10, 2021

Ulisseus Jul 10, 2021 •

edited

Loading

JasirZaeem Jul 12, 2021

Ulisseus Jul 10, 2021

JasirZaeem Jul 10, 2021

songz Jul 12, 2021

JasirZaeem Jul 12, 2021

Ulisseus Jul 10, 2021

hwong0305 Jul 13, 2021

tomrule007 Jul 14, 2021

JasirZaeem Jul 14, 2021

songz commented Jul 13, 2021

tomrule007 left a comment

hwong0305 Jul 14, 2021

JasirZaeem Jul 14, 2021

JasirZaeem commented Jul 14, 2021

		@@ -0,0 +1,70 @@
		import { BotErrorType, IdType } from '../helpers/discordBot'

		export type snowflake = string

Connect app to the c0d3r (Discord Bot) #949

Connect app to the c0d3r (Discord Bot) #949

Conversation

JasirZaeem commented Jul 10, 2021 • edited Loading

On challenge submission

Example

On lesson completion

Example

On receiving a star

Example

TODO

Other changes

vercel bot commented Jul 10, 2021

codecov bot commented Jul 10, 2021 • edited Loading

Codecov Report

vercel bot commented Jul 10, 2021 • edited Loading

ggwadera Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

tomrule007 Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggwadera Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasirZaeem Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasirZaeem Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ulisseus Jul 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

songz commented Jul 13, 2021

tomrule007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasirZaeem commented Jul 14, 2021

JasirZaeem commented Jul 10, 2021 •

edited

Loading

codecov bot commented Jul 10, 2021 •

edited

Loading

vercel bot commented Jul 10, 2021 •

edited

Loading

ggwadera Jul 10, 2021 •

edited

Loading

tomrule007 Jul 10, 2021 •

edited

Loading

ggwadera Jul 10, 2021 •

edited

Loading

JasirZaeem Jul 12, 2021 •

edited

Loading

JasirZaeem Jul 10, 2021 •

edited

Loading

Ulisseus Jul 10, 2021 •

edited

Loading