Feat: Added /discord/:id endpoint to allow connection to UniCS Discord Bot. #145

SamJHirst · 2020-12-22T17:13:35Z

Resolves #138

…n in the future

codecov · 2020-12-22T17:15:33Z

Codecov Report

Merging #145 (12ce63e) into main (d61fcf1) will decrease coverage by 0.59%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
- Coverage   79.16%   78.57%   -0.60%     
==========================================
  Files          51       51              
  Lines        1349     1363      +14     
  Branches      144      189      +45     
==========================================
+ Hits         1068     1071       +3     
- Misses        281      292      +11

Impacted Files	Coverage Δ
src/services/DiscordService.ts	`30.13% <0.00%> (-1.75%)`	⬇️
src/controllers/DiscordController.ts	`32.00% <30.00%> (-5.50%)`	⬇️
src/routes/UserRoutes.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d61fcf1...f04f7a4. Read the comment docs.

amishshah

A change for security, and maybe a change for the route naming too? Maybe the following instead:

/discord/users/:id
/users/discord/:id

I think these give more clarity.

amishshah · 2020-12-23T20:34:42Z

src/routes/UserRoutes.ts

@@ -29,6 +29,8 @@ export class UserRoutes {

 		router.post('/reset_password', getUser(TokenType.PasswordReset), this.userController.resetPassword.bind(this.userController));

+		router.get('/discord/:id', this.discordController.getDiscordUser.bind(this.discordController));


This is sensitive info, I'd say that the route should be restricted to authorised admins.

Sam Hirst and others added 2 commits December 22, 2020 17:02

feat: added /discord/:id endpoint to allow for discord bot integratio…

351abfb

…n in the future

style: missing whitespace

6e0709c

amishshah suggested changes Dec 23, 2020

View reviewed changes

fix: added security middleware to discord id endpoint

f04f7a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Added /discord/:id endpoint to allow connection to UniCS Discord Bot. #145

Feat: Added /discord/:id endpoint to allow connection to UniCS Discord Bot. #145

SamJHirst commented Dec 22, 2020

codecov bot commented Dec 22, 2020 •

edited

Loading

amishshah left a comment

amishshah Dec 23, 2020

		@@ -29,6 +29,8 @@ export class UserRoutes {

		router.post('/reset_password', getUser(TokenType.PasswordReset), this.userController.resetPassword.bind(this.userController));

		router.get('/discord/:id', this.discordController.getDiscordUser.bind(this.discordController));

Feat: Added /discord/:id endpoint to allow connection to UniCS Discord Bot. #145

Are you sure you want to change the base?

Feat: Added /discord/:id endpoint to allow connection to UniCS Discord Bot. #145

Conversation

SamJHirst commented Dec 22, 2020

codecov bot commented Dec 22, 2020 • edited Loading

Codecov Report

amishshah left a comment

Choose a reason for hiding this comment

amishshah Dec 23, 2020

Choose a reason for hiding this comment

codecov bot commented Dec 22, 2020 •

edited

Loading