-
Notifications
You must be signed in to change notification settings - Fork 22
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
Log percentage of users without selected_sites alongside the total count #3582
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## staging #3582 +/- ##
===========================================
+ Coverage 29.66% 29.68% +0.01%
===========================================
Files 185 185
Lines 25016 25016
Branches 3315 3315
===========================================
+ Hits 7421 7426 +5
+ Misses 17471 17466 -5
Partials 124 124 |
Auth-service changes in this PR available for preview here |
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
🧹 Outside diff range and nitpick comments (3)
src/auth-service/bin/jobs/preferences-log-job.js (3)
44-47
: Nicely refactored user filtering logic.The use of the
filter
method here is a great improvement. It makes the code more readable and functional. Well done!A small optimization suggestion: Consider using
Array.prototype.some()
for a slight performance boost, as it can short-circuit the iteration:const usersWithoutSelectedSites = users.filter((user) => { const preference = preferencesMap.get(user._id.toString()); return !preference || !preference.selected_sites.some(site => !isEmpty(site)); });This change would stop checking
selected_sites
as soon as it finds a non-empty site.
57-61
: Well-implemented percentage calculation.The percentage calculation logic is sound and includes a safeguard against division by zero. Rounding to two decimal places provides a good balance between precision and readability.
To enhance readability further, consider extracting the calculation into a separate function:
const calculatePercentage = (part, whole) => whole > 0 ? ((part / whole) * 100).toFixed(2) : '0.00'; // Usage const percentageWithoutSelectedSites = calculatePercentage( totalCountWithoutSelectedSites, totalUsersProcessed );This abstraction would make the main function body cleaner and the calculation reusable if needed elsewhere.
64-64
: Informative and comprehensive logging statement.The updated log message successfully incorporates both the total count and the percentage of users without selected sites. This enhancement significantly improves our ability to monitor and analyze user engagement.
To further improve clarity, consider slightly restructuring the message:
logger.info( `💔💔 Users without selected_sites: ${totalCountWithoutSelectedSites} (${percentageWithoutSelectedSites}%) out of ${totalUsersProcessed} total Analytics users.` );This format provides a clear breakdown of the statistics and explicitly mentions the total number of users processed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/auth-service/bin/jobs/preferences-log-job.js (2 hunks)
🔇 Additional comments (2)
src/auth-service/bin/jobs/preferences-log-job.js (2)
17-17
: Excellent addition of thetotalUsersProcessed
variable.This new variable is a thoughtful inclusion that aligns perfectly with the PR's objective. It will enable the calculation of the percentage of users without selected sites, enhancing our analytics capabilities.
51-51
: Proper increment oftotalUsersProcessed
.This line correctly increments the total count of processed users after each batch. It's well-placed within the loop to ensure an accurate count. This addition is crucial for calculating the percentage later on.
Description
To log the percentage of users without selected_sites alongside the total count
Changes Made
totalUsersProcessed
to keep track of the total number of users processed.selected_sites
Testing
Affected Services
API Documentation Updated?
Additional Notes
With these modifications, the script will now log not only the count of users without
selected_sites
but also what percentage that count represents of all processed analytics users.Summary by CodeRabbit
New Features
Bug Fixes
Documentation