refactor: clean up meteor-run-as-user package#38373
Conversation
…les and simplifying code structure
|
WalkthroughThis PR removes deprecated public APIs from the meteor-run-as-user package (Meteor.isRestricted, Meteor.runRestricted, Meteor.isAdmin, etc.) and eliminates a Meteor version compatibility polyfill. It refactors collection operation handling to use LocalCollection iteration helpers instead of Mongo.Collection monkey-patching. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38373 +/- ##
========================================
Coverage 70.78% 70.78%
========================================
Files 3159 3159
Lines 109397 109396 -1
Branches 19669 19666 -3
========================================
Hits 77437 77437
+ Misses 29933 29930 -3
- Partials 2027 2029 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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/packages/meteor-run-as-user/package.js (1)
1-7: Update package summary to reflect removed API.The summary still references
Meteor.isRestricted()which has been removed in this refactor. Consider updating it to accurately describe the current API surface.Suggested diff
Package.describe({ name: 'dispatch:run-as-user', version: '1.1.1', - summary: 'Adds Meteor.runAsUser(user, f) and Meteor.isRestricted()', + summary: 'Adds Meteor.runAsUser(user, f)', git: 'https://github.com/DispatchMe/Meteor-run-as-user.git', documentation: 'README.md', });
🧹 Nitpick comments (2)
apps/meteor/packages/meteor-run-as-user/lib/collection.overwrites.js (2)
11-13: Consider removing inline comments.Per coding guidelines, implementation code should avoid comments. The change from
!fn(doc,id)to=== falseis self-explanatory for the explicit boolean check pattern.Suggested diff
- if (doc && (await fn(doc, id)) === false) { - // Changed from `!fn(doc,id)` + if (doc && (await fn(doc, id)) === false) { break; }
28-30: Same comment applies here.The inline comment on line 29 can be removed for consistency with the async version and to follow the coding guidelines.
Suggested diff
- if (doc && fn(doc, id) === false) { - // Changed from `!fn(doc,id)` + if (doc && fn(doc, id) === false) { break; }
extracted from #38354
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Breaking Changes
Meteor.isRestricted(),Meteor.runRestricted(),Meteor.runUnrestricted(),Meteor.runAsRestrictedUser(),Meteor.isAdmin(),Meteor.runAsAdmin(), andMeteor.runOutsideInvocation().Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.