Feat/GitHub duplication #1818
Feat/GitHub duplication #1818
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
joyguptaa
requested review from
prakashchoudhary07,
iamitprakash,
sahsisunny,
satyam73 and
Ajeyakrishna-k
controllers/users.js
Outdated
| const userId = userDoc.id; | ||
| batchWrite.update(userDoc.ref, { github_user_id: null }); | ||
|
|
||
| const promise = fetch(`https://api.github.com/users/${githubUsername}`, requestOptions) |
There was a problem hiding this comment.
Have we considered GitHub's rate limits?
There was a problem hiding this comment.
The doc says that it is 5000 requests per hour for authenticated users. And I do not think that this will cause any issue as we anyway have less than 1k users.
There was a problem hiding this comment.
It also says we cannot make more than 900 calls per minute on a single route. I think we have around 3000 users docs in our prod db.
https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#about-secondary-rate-limits
Done |
…-Squad/website-backend into feat/github-duplication
controllers/users.js
Outdated
| const userId = userDoc.id; | ||
| batchWrite.update(userDoc.ref, { github_user_id: null }); | ||
|
|
||
| const promise = fetch(`https://api.github.com/users/${githubUsername}`, requestOptions) |
There was a problem hiding this comment.
It also says we cannot make more than 900 calls per minute on a single route. I think we have around 3000 users docs in our prod db.
https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#about-secondary-rate-limits
| // WARNING!! - One time Script/Route to do migration | ||
| router.post( | ||
| "/migrations", | ||
| authenticate, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
…-Squad/website-backend into feat/github-duplication
There was a problem hiding this comment.
- Unit tests should have been written for models and middleware instead of only having integration tests.
Please also get approvals from @prakashchoudhary07 / @iamitprakash.
Date:
28 Dec 2023Developer Name:
Joy GuptaIssue Ticket Number:-
Reference PR:-
Description:
github_user_idinto user schema to avoid user duplicationgithub_user_idupon logging if it does not existsIs Under the Feature Flag
Database changes
Breaking changes (If your feature is breaking/missing something please mention pending tickets)
Is Development Tested?
Tested in staging?
Test Cases
Test Stats
Add relevant Screenshot below
1. Github user id should be added upon logging in
Screen.Recording.2023-12-28.at.9.59.42.AM.1.mov
2. Should add github_user_id after executing the script
Before
Execution log
Updated Users