Skip to content
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

logging is consistent with the new group-specific preference management #4080

Merged
merged 1 commit into from
Dec 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 33 additions & 12 deletions src/auth-service/bin/jobs/preferences-log-job.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const cron = require("node-cron");
const mongoose = require("mongoose");
const UserModel = require("@models/User");
const PreferenceModel = require("@models/Preference");
const constants = require("@config/constants");
Expand All @@ -13,25 +14,41 @@ const logUserPreferences = async () => {
try {
const batchSize = 100;
let skip = 0;
let totalCountWithoutSelectedSites = 0; // To keep track of total count
let totalUsersProcessed = 0; // To keep track of total users processed
let totalCountWithoutSelectedSites = 0;
let totalUsersProcessed = 0;

// Use the default group ID
const defaultGroupId = mongoose.Types.ObjectId(constants.DEFAULT_GROUP);

while (true) {
// Fetch users with their group_roles
const users = await UserModel("airqo")
.find()
.limit(batchSize)
.skip(skip)
.select("_id email")
.select("_id email group_roles")
.lean();

if (users.length === 0) {
break;
}

// Fetch existing preferences for users in batch
const userIds = users.map((user) => user._id);
// Filter users who are members of the default group
const validUsers = users.filter(
(user) =>
user.group_roles &&
user.group_roles.some(
(role) => role.group.toString() === defaultGroupId.toString()
)
);
Comment on lines +24 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize group role filtering and add safety checks.

The user fetching logic is well-structured, but there are a few improvements we can make:

  1. Use optional chaining as suggested by static analysis:
-          user.group_roles &&
-          user.group_roles.some(
+          user.group_roles?.some(
  1. Add index hint for better query performance:
 const users = await UserModel("airqo")
   .find()
+  .hint({ group_roles: 1 })
   .limit(batchSize)
   .skip(skip)
   .select("_id email group_roles")
   .lean();
  1. Consider caching the defaultGroupId.toString() result:
+      const defaultGroupIdString = defaultGroupId.toString();
       const validUsers = users.filter(
         (user) =>
           user.group_roles?.some(
-            (role) => role.group.toString() === defaultGroupId.toString()
+            (role) => role.group.toString() === defaultGroupIdString
           )
       );

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (1.9.4)

[error] 39-42: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


// Fetch existing preferences for valid users in the default group
const userIds = validUsers.map((user) => user._id);
const preferences = await PreferenceModel("airqo")
.find({ user_id: { $in: userIds } })
.find({
user_id: { $in: userIds },
group_id: defaultGroupId,
})
.select("_id user_id selected_sites")
.lean();

Expand All @@ -40,31 +57,33 @@ const logUserPreferences = async () => {
preferencesMap.set(pref.user_id.toString(), pref);
});

// Collect IDs of users without selected_sites
const usersWithoutSelectedSites = users.filter((user) => {
// Collect IDs of users without selected_sites in the default group
const usersWithoutSelectedSites = validUsers.filter((user) => {
const preference = preferencesMap.get(user._id.toString());
return !preference || isEmpty(preference.selected_sites);
});

// Aggregate results
totalCountWithoutSelectedSites += usersWithoutSelectedSites.length;
totalUsersProcessed += users.length; // Increment total processed users
totalUsersProcessed += validUsers.length;

skip += batchSize;
}

// Log the aggregated results once after processing all users
if (totalUsersProcessed > 0 && totalCountWithoutSelectedSites > 0) {
if (totalUsersProcessed > 0) {
const percentageWithoutSelectedSites = (
(totalCountWithoutSelectedSites / totalUsersProcessed) *
100
).toFixed(2);

logger.info(
`💔💔 Total count of users without any Customised Locations: ${totalCountWithoutSelectedSites}, which is ${percentageWithoutSelectedSites}% of all Analytics users.`
`💔💔 Total count of users without Customised Locations in the default group: ${totalCountWithoutSelectedSites}, which is ${percentageWithoutSelectedSites}% of processed users.`
);
} else {
logger.info(`😎🎉✅ All Analytics users have Customised Locations.`);
logger.info(
`😎🎉✅ No users processed or all users have Customised Locations.`
);
}
} catch (error) {
logger.error(`🐛🐛 Error in logUserPreferences: ${stringify(error)}`);
Expand All @@ -76,3 +95,5 @@ cron.schedule(schedule, logUserPreferences, {
scheduled: true,
timezone: "Africa/Nairobi",
});

module.exports = { logUserPreferences };
Loading