-
Notifications
You must be signed in to change notification settings - Fork 209
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
Feat/be/ttl, #764 #835
Feat/be/ttl, #764 #835
Conversation
WalkthroughThe changes introduce a new feature for updating the Time-To-Live (TTL) for checks in a MongoDB database. This involves modifications to error handling, the addition of a new function Changes
Possibly related PRs
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- Server/controllers/checkController.js (8 hunks)
- Server/db/mongo/MongoDB.js (2 hunks)
- Server/db/mongo/modules/checkModule.js (1 hunks)
- Server/db/mongo/modules/monitorModule.js (1 hunks)
- Server/middleware/handleErrors.js (1 hunks)
- Server/routes/checkRoute.js (1 hunks)
- Server/utils/messages.js (1 hunks)
- Server/validation/joi.js (2 hunks)
Files not reviewed due to content moderation or server errors (1)
- Server/controllers/checkController.js
Files skipped from review due to trivial changes (2)
- Server/db/mongo/modules/monitorModule.js
- Server/middleware/handleErrors.js
Additional context used
GitHub Check: CodeQL
Server/routes/checkRoute.js
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
Additional comments not posted (4)
Server/utils/messages.js (1)
76-76
: Yo, the new success message is straight fire! 🔥The
CHECK_UPDATE_TTL
message fits right in with the rest of the crew. It's clear, follows the naming convention, and doesn't mess with the flow. You're spittin' some dope code here, homie!Server/db/mongo/MongoDB.js (1)
100-100
: Yo, the newupdateChecksTTL
export is straight fire, homie! 🔥Addin' this function to the exports is the way to go, so it can be used all over the codebase, ya know what I'm sayin'?
Server/db/mongo/modules/checkModule.js (1)
242-254
: Yo, this newupdateChecksTTL
function is straight fire, homie! 🔥It's droppin' that old "expiry_1" index like it's hot and slappin' a fresh new one on the
expiry
field. The TTL game is on point, and it's handlin' those errors like a boss. Mom's spaghetti approves! 🍝👌Server/validation/joi.js (1)
338-340
: Yo, the new validation schema is straight fire! 🔥The
updateChecksTTLBodyValidation
schema is droppin' some serious validation beats, makin' sure thettl
property is always in the house and reppin' them numbers, ya know what I'm sayin'?This schema is gonna keep them TTL updates in check, no cap! 🧢
router.put( | ||
"/ttl", | ||
isAllowed(["admin", "superadmin"]), | ||
checkController.updateChecksTTL | ||
); |
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.
Yo, the new /ttl
route is lookin' tight, but we gotta protect it from gettin' spammed, ya feel me?
The implementation of the /ttl
route is on point with the PR objective. Mad props for using the isAllowed
middleware to keep those sneaky non-admins outta there!
But hold up, the static analysis tool caught somethin' - we're missin' some rate limitin' on this bad boy. We gotta make sure it ain't gettin' hit too hard, or it might just puke all over the place, like mom's spaghetti on a sweater, ya know what I'm sayin'?
Yo, lemme help ya out with slappin' a rate limiter on there real quick, just say the word!
Tools
GitHub Check: CodeQL
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
@@ -161,6 +162,7 @@ module.exports = { | |||
getTeamChecks, | |||
deleteChecks, | |||
deleteChecksByTeamId, | |||
updateChecksTTL, |
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.
Hold up, dawg! We got a duplicate export in the house! 😵
Looks like updateChecksTTL
is bein' exported twice, once at line 100 and again here. That ain't gonna fly, cuz it's just gonna cause mad confusion, ya feel me?
Let's keep it clean and remove this duplicate export, aight? Here's the diff to fix it:
- updateChecksTTL,
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
updateChecksTTL, |
); | ||
} catch (error) { | ||
error.service = SERVICE_NAME; | ||
error.method = "upddateChecksTTL"; |
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.
Hold up, dawg! We got a tiny typo in the hood. 😅
Looks like "upddateChecksTTL" is missin' an "a". Let's fix that up real quick before mom's spaghetti gets cold, ya feel me?
Here's the fix, straight outta Compton:
- error.method = "upddateChecksTTL";
+ error.method = "updateChecksTTL";
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
error.method = "upddateChecksTTL"; | |
error.method = "updateChecksTTL"; |
This PR implements the back end portion of updating checks TTL