Skip to content

Commit

Permalink
Feat: Implement APIs for Tracking Departed Users with Assigned Tasks (R…
Browse files Browse the repository at this point in the history
…eal-Dev-Squad#2268)

* feat: Departed users api changes.

* feat: Added test cases for departed users api changes.

* Refactor: Update fetchIncompleteTasksByUserIds to use batch queries and filter in-memory

- Replaced Firestore query with batch query to fetch incomplete tasks for multiple users at once.
- Filter tasks by user IDs and completed statuses in-memory for improved efficiency.
- Updated return structure to return an array directly instead of an object with `docs` property.
- Updated test cases related to the same.
  • Loading branch information
VinuB-Dev authored and listiclehub1 committed Dec 2, 2024
1 parent c32ff5f commit 26ec1df
Show file tree
Hide file tree
Showing 9 changed files with 273 additions and 48 deletions.
25 changes: 25 additions & 0 deletions controllers/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const { addLog } = require("../models/logs");
const { getUserStatus } = require("../models/userStatus");
const config = require("config");
const { generateUniqueUsername } = require("../services/users");
const userService = require("../services/users");
const discordDeveloperRoleId = config.get("discordDeveloperRoleId");

const verifyUser = async (req, res) => {
Expand Down Expand Up @@ -191,6 +192,30 @@ const getUsers = async (req, res) => {
}
}

const isDeparted = req.query.departed === "true";

if (isDeparted) {
if (!dev) {
return res.boom.notFound("Route not found");
}
try {
const result = await dataAccess.retrieveUsers({ query: req.query });
const departedUsers = await userService.getUsersWithIncompleteTasks(result.users);
if (departedUsers.length === 0) return res.status(204).send();
return res.json({
message: "Users with abandoned tasks fetched successfully",
users: departedUsers,
links: {
next: result.nextId ? getPaginationLink(req.query, "next", result.nextId) : "",
prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "",
},
});
} catch (error) {
logger.error("Error when fetching users who abandoned tasks:", error);
return res.boom.badImplementation(INTERNAL_SERVER_ERROR);
}
}

if (transformedQuery?.filterBy === OVERDUE_TASKS) {
try {
const tasksData = await getOverdueTasks(days);
Expand Down
1 change: 1 addition & 0 deletions middlewares/validators/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ async function getUsers(req, res, next) {
filterBy: joi.string().optional(),
days: joi.string().optional(),
dev: joi.string().optional(),
departed: joi.string().optional(),
roles: joi.optional().custom((value, helpers) => {
if (value !== "member") {
return helpers.message("only member role is supported");
Expand Down
6 changes: 3 additions & 3 deletions models/tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,9 +752,9 @@ const fetchIncompleteTasksByUserIds = async (userIds) => {
try {
const incompleteTasksQuery = await tasksModel.where("assigneeId", "in", userIds).get();

const incompleteTaskForUsers = incompleteTasksQuery.docs.filter((task) => {
return !COMPLETED_STATUSES.includes(task.data().status);
});
const incompleteTaskForUsers = incompleteTasksQuery.docs.filter(
(task) => !COMPLETED_STATUSES.includes(task.data().status)
);

return incompleteTaskForUsers;
} catch (error) {
Expand Down
65 changes: 60 additions & 5 deletions models/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ const firestore = require("../utils/firestore");
const { fetchWallet, createWallet } = require("../models/wallets");
const { updateUserStatus } = require("../models/userStatus");
const { arraysHaveCommonItem, chunks } = require("../utils/array");
const { archiveUsers } = require("../services/users");
const { ALLOWED_FILTER_PARAMS, FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users");
const {
ALLOWED_FILTER_PARAMS,
FIRESTORE_IN_CLAUSE_SIZE,
USERS_PATCH_HANDLER_SUCCESS_MESSAGES,
USERS_PATCH_HANDLER_ERROR_MESSAGES,
} = require("../constants/users");
const { DOCUMENT_WRITE_SIZE } = require("../constants/constants");
const { userState } = require("../constants/userStatus");
const { BATCH_SIZE_IN_CLAUSE } = require("../constants/firebase");
Expand All @@ -27,6 +31,52 @@ const { formatUsername } = require("../utils/username");
const { logType } = require("../constants/logs");
const { addLog } = require("../services/logService");

/**
* Archive users by setting the roles.archived field to true.
* This function commits the write in batches to avoid reaching the maximum number of writes per batch.
* @param {Array} usersData - An array of user objects with the following properties: id, first_name, last_name
* @returns {Promise} - A promise that resolves with a summary object containing the number of users updated and failed, and an array of updated and failed user details.
*/
const archiveUsers = async (usersData) => {
const batch = firestore.batch();
const usersBatch = [];
const summary = {
totalUsersArchived: 0,
totalOperationsFailed: 0,
updatedUserDetails: [],
failedUserDetails: [],
};

usersData.forEach((user) => {
const { id, first_name: firstName, last_name: lastName } = user;
const updatedUserData = {
...user,
roles: {
...user.roles,
archived: true,
},
updated_at: Date.now(),
};
batch.update(userModel.doc(id), updatedUserData);
usersBatch.push({ id, firstName, lastName });
});

try {
await batch.commit();
summary.totalUsersArchived += usersData.length;
summary.updatedUserDetails = [...usersBatch];
return {
message: USERS_PATCH_HANDLER_SUCCESS_MESSAGES.ARCHIVE_USERS.SUCCESSFULLY_COMPLETED_BATCH_UPDATES,
...summary,
};
} catch (err) {
logger.error("Firebase batch Operation Failed!");
summary.totalOperationsFailed += usersData.length;
summary.failedUserDetails = [...usersBatch];
return { message: USERS_PATCH_HANDLER_ERROR_MESSAGES.ARCHIVE_USERS.BATCH_DATA_UPDATED_FAILED, ...summary };
}
};

/**
* Adds or updates the user data
*
Expand Down Expand Up @@ -218,11 +268,11 @@ const getSuggestedUsers = async (skill) => {
*/
const fetchPaginatedUsers = async (query) => {
const isDevMode = query.dev === "true";

try {
const size = parseInt(query.size) || 100;
const doc = (query.next || query.prev) && (await userModel.doc(query.next || query.prev).get());

const isArchived = query.departed === "true";
let dbQuery;
/**
* !!NOTE : At the time of writing we only support member in the role query
Expand All @@ -231,9 +281,9 @@ const fetchPaginatedUsers = async (query) => {
* if you're making changes to this code remove the archived check in the role query, example: role=archived,member
*/
if (query.roles === "member") {
dbQuery = userModel.where("roles.archived", "==", false).where("roles.member", "==", true);
dbQuery = userModel.where("roles.archived", "==", isArchived).where("roles.member", "==", true);
} else {
dbQuery = userModel.where("roles.archived", "==", false).orderBy("username");
dbQuery = userModel.where("roles.archived", "==", isArchived).orderBy("username");
}

let compositeQuery = [dbQuery];
Expand All @@ -253,6 +303,10 @@ const fetchPaginatedUsers = async (query) => {
}

if (Object.keys(query).length) {
if (query.departed) {
compositeQuery = compositeQuery.map((query) => query.where("roles.in_discord", "==", false));
dbQuery = dbQuery.where("roles.in_discord", "==", false);
}
if (query.search) {
const searchValue = query.search.toLowerCase().trim();
dbQuery = dbQuery.startAt(searchValue).endAt(searchValue + "\uf8ff");
Expand Down Expand Up @@ -1092,6 +1146,7 @@ const fetchUsersNotInDiscordServer = async () => {
};

module.exports = {
archiveUsers,
addOrUpdate,
fetchPaginatedUsers,
fetchUser,
Expand Down
56 changes: 20 additions & 36 deletions services/users.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,28 @@
const { USERS_PATCH_HANDLER_SUCCESS_MESSAGES, USERS_PATCH_HANDLER_ERROR_MESSAGES } = require("../constants/users");
const firestore = require("../utils/firestore");
const { formatUsername } = require("../utils/username");
const userModel = firestore.collection("users");
const archiveUsers = async (usersData) => {
const batch = firestore.batch();
const usersBatch = [];
const summary = {
totalUsersArchived: 0,
totalOperationsFailed: 0,
updatedUserDetails: [],
failedUserDetails: [],
};
const tasksModel = require("../models/tasks");

usersData.forEach((user) => {
const { id, first_name: firstName, last_name: lastName } = user;
const updatedUserData = {
...user,
roles: {
...user.roles,
archived: true,
},
updated_at: Date.now(),
};
batch.update(userModel.doc(id), updatedUserData);
usersBatch.push({ id, firstName, lastName });
});
const getUsersWithIncompleteTasks = async (users) => {
if (users.length === 0) return [];

try {
await batch.commit();
summary.totalUsersArchived += usersData.length;
summary.updatedUserDetails = [...usersBatch];
return {
message: USERS_PATCH_HANDLER_SUCCESS_MESSAGES.ARCHIVE_USERS.SUCCESSFULLY_COMPLETED_BATCH_UPDATES,
...summary,
};
} catch (err) {
logger.error("Firebase batch Operation Failed!");
summary.totalOperationsFailed += usersData.length;
summary.failedUserDetails = [...usersBatch];
return { message: USERS_PATCH_HANDLER_ERROR_MESSAGES.ARCHIVE_USERS.BATCH_DATA_UPDATED_FAILED, ...summary };
const userIds = users.map((user) => user.id);

const abandonedTasksQuerySnapshot = await tasksModel.fetchIncompleteTasksByUserIds(userIds);

if (abandonedTasksQuerySnapshot.empty) {
return [];
}

const userIdsWithIncompleteTasks = new Set(abandonedTasksQuerySnapshot.map((doc) => doc.data().assigneeId));

const eligibleUsersWithTasks = users.filter((user) => userIdsWithIncompleteTasks.has(user.id));

return eligibleUsersWithTasks;
} catch (error) {
logger.error(`Error in getting users who abandoned tasks: ${error}`);
throw error;
}
};

Expand All @@ -63,6 +47,6 @@ const generateUniqueUsername = async (firstName, lastName) => {
};

module.exports = {
archiveUsers,
generateUniqueUsername,
getUsersWithIncompleteTasks,
};
58 changes: 57 additions & 1 deletion test/integration/users.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ const { userPhotoVerificationData } = require("../fixtures/user/photo-verificati
const Sinon = require("sinon");
const { INTERNAL_SERVER_ERROR, SOMETHING_WENT_WRONG } = require("../../constants/errorMessages");
const photoVerificationModel = firestore.collection("photo-verification");

const userModel = firestore.collection("users");
const taskModel = firestore.collection("tasks");
const {
usersData: abandonedUsersData,
tasksData: abandonedTasksData,
} = require("../fixtures/abandoned-tasks/departed-users");
const userService = require("../../services/users");
chai.use(chaiHttp);

describe("Users", function () {
Expand Down Expand Up @@ -1441,6 +1447,56 @@ describe("Users", function () {
});
});

describe("GET /users?departed", function () {
beforeEach(async function () {
await cleanDb();
const userPromises = abandonedUsersData.map((user) => userModel.doc(user.id).set(user));
await Promise.all(userPromises);

const taskPromises = abandonedTasksData.map((task) => taskModel.add(task));
await Promise.all(taskPromises);
});

afterEach(async function () {
Sinon.restore();
await cleanDb();
});

it("should return a list of users with abandoned tasks", async function () {
const res = await chai.request(app).get("/users?dev=true&departed=true");
expect(res).to.have.status(200);
expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully");
expect(res.body).to.have.property("users").to.be.an("array").with.lengthOf(2);
});

it("should return an empty array when no users have abandoned tasks", async function () {
await cleanDb();
const user = abandonedUsersData[2];
await userModel.add(user);

const task = abandonedTasksData[3];
await taskModel.add(task);
const res = await chai.request(app).get("/users?dev=true&departed=true");

expect(res).to.have.status(204);
});

it("should fail if dev flag is not passed", async function () {
const res = await chai.request(app).get("/users?departed=true");
expect(res).to.have.status(404);
expect(res.body.message).to.be.equal("Route not found");
});

it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () {
Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(INTERNAL_SERVER_ERROR));

const res = await chai.request(app).get("/users?departed=true&dev=true");

expect(res).to.have.status(500);
expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR);
});
});

describe("PUT /users/self/intro", function () {
let userStatusData;

Expand Down
1 change: 0 additions & 1 deletion test/unit/models/tasks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,6 @@ describe("tasks", function () {

it("should return an empty array if there are no tasks incomplete for the user", async function () {
await cleanDb();

const activeUser = abandonedUsersData[2];
const incompleteTasks = await tasks.fetchIncompleteTasksByUserIds([activeUser.id]);
expect(incompleteTasks.length).to.be.equal(0);
Expand Down
44 changes: 44 additions & 0 deletions test/unit/models/users.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,50 @@ describe("users", function () {
});
});

describe("fetchPaginatedUsers - Departed Users", function () {
beforeEach(async function () {
await cleanDb();

const userPromises = abandonedUsersData.map((user) => userModel.add(user));
await Promise.all(userPromises);
});

afterEach(async function () {
await cleanDb();
sinon.restore();
});

it("should fetch users not in discord server", async function () {
const result = await users.fetchPaginatedUsers({ departed: "true" });
expect(result.allUsers.length).to.be.equal(2);
});

it("should return no users if departed flag is false", async function () {
const result = await users.fetchPaginatedUsers({ departed: "false" });
expect(result.allUsers.length).to.be.equal(0);
});

it("should return an empty array if there are no departed users in the database", async function () {
await cleanDb();
const activeUser = abandonedUsersData[2];
await userModel.add(activeUser);

const result = await users.fetchPaginatedUsers({ departed: "true" });
expect(result.allUsers.length).to.be.equal(0);
});

it("should handle errors gracefully if the database query fails", async function () {
sinon.stub(users, "fetchPaginatedUsers").throws(new Error("Database query failed"));

try {
await users.fetchPaginatedUsers();
expect.fail("Expected function to throw an error");
} catch (error) {
expect(error.message).to.equal("Database query failed");
}
});
});

describe("fetchUsersNotInDiscordServer", function () {
beforeEach(async function () {
await cleanDb();
Expand Down
Loading

0 comments on commit 26ec1df

Please sign in to comment.