-
Notifications
You must be signed in to change notification settings - Fork 13.1k
chore: remove rate limiter for functions #38354
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
Conversation
…rate limiter for setEmail function
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdded per-route Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
c27cda4 to
846055a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38354 +/- ##
===========================================
- Coverage 70.78% 70.73% -0.05%
===========================================
Files 3159 3159
Lines 109364 109364
Branches 19671 19704 +33
===========================================
- Hits 77415 77362 -53
- Misses 29920 29965 +45
- Partials 2029 2037 +8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
846055a to
2a2334b
Compare
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.
No issues found across 9 files
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/app/lib/server/functions/setEmail.ts (1)
44-79: Update error message function references.The function has been renamed from
_setEmailtosetEmail, but the error objects still reference'_setEmail'in thefunctionfield (lines 54, 58, 65, 76). Update these for consistency and accurate error tracking.Proposed fix
- throw new Meteor.Error('error-invalid-user', 'Invalid user', { function: '_setEmail' }); + throw new Meteor.Error('error-invalid-user', 'Invalid user', { function: 'setEmail' });Apply similar changes to lines 58, 65, and 76-77.
🧹 Nitpick comments (1)
apps/meteor/app/lib/server/functions/setStatusText.ts (1)
26-26: Consider replacing deprecatedsubstrwithsubstring.
String.prototype.substr()is deprecated. While functionally equivalent here, usingsubstring(0, 120)orslice(0, 120)is preferred.Proposed fix
- statusText = statusText.trim().substr(0, 120); + statusText = statusText.trim().substring(0, 120);
2a2334b to
ab65fef
Compare
….js and update tests accordingly
ab65fef to
37e5399
Compare
33bba73 to
6b9b1e7
Compare
code extracted from #38017
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.