From c7e0899a35cdba5be131da25a2c14a879c3f06ba Mon Sep 17 00:00:00 2001 From: Favour Okerri <111712988+Favourokerri@users.noreply.github.com> Date: Mon, 2 Sep 2024 19:32:37 +0100 Subject: [PATCH 1/3] fixed getUserApplications to-fetch-latest-application (#2097) * fixed getUserApplications to-fetch-latest-application * fixed getUserApplications to return most recent --------- Co-authored-by: Vinit khandal <111434418+vinit717@users.noreply.github.com> Co-authored-by: Amit Prakash <34869115+iamitprakash@users.noreply.github.com> --- models/applications.ts | 6 +++++- test/unit/models/application.test.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/models/applications.ts b/models/applications.ts index e6595634e..995f4c2e3 100644 --- a/models/applications.ts +++ b/models/applications.ts @@ -91,7 +91,10 @@ const getApplicationsBasedOnStatus = async (status: string, limit: number, lastD const getUserApplications = async (userId: string) => { try { const applicationsResult = []; - const applications = await ApplicationsModel.where("userId", "==", userId).get(); + const applications = await ApplicationsModel.where("userId", "==", userId) + .orderBy("createdAt", "desc") + .limit(1) + .get(); applications.forEach((application) => { applicationsResult.push({ @@ -99,6 +102,7 @@ const getUserApplications = async (userId: string) => { ...application.data(), }); }); + return applicationsResult; } catch (err) { logger.log("error in getting user intro", err); diff --git a/test/unit/models/application.test.ts b/test/unit/models/application.test.ts index 0c9395299..96165c41f 100644 --- a/test/unit/models/application.test.ts +++ b/test/unit/models/application.test.ts @@ -48,7 +48,7 @@ describe("applications", function () { }); describe("getUserApplications", function () { - it("should return all the user applications", async function () { + it("should return users most recent application", async function () { const applications = await ApplicationModel.getUserApplications("kfasdjfkdlfjkasdjflsdjfk"); expect(applications).to.be.a("array"); expect(applications.length).to.be.equal(1); From 82a78d997958ebce8ec88c9b61cf454c47c3dc3f Mon Sep 17 00:00:00 2001 From: Vinit khandal <111434418+vinit717@users.noreply.github.com> Date: Sun, 8 Sep 2024 00:20:24 +0530 Subject: [PATCH 2/3] Migration script to update username (#2122) * feat: add migration script to update username * chore: add users with updated username in reponse * chore: fix code structure to make test run * chore: add integration test * chore: add check for super_user * chore: fix failing test * chore: add more test for migration --- controllers/users.js | 11 +++ models/users.js | 125 +++++++++++++++++++++++++++++++++ routes/users.js | 2 + test/integration/users.test.js | 54 ++++++++++++++ 4 files changed, 192 insertions(+) diff --git a/controllers/users.js b/controllers/users.js index 82c197ced..eefa62c6e 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -955,6 +955,16 @@ const getIdentityStats = async (req, res) => { }); }; +const updateUsernames = async (req, res) => { + try { + const response = await userQuery.updateUsersWithNewUsernames(); + return res.status(200).json(response); + } catch (error) { + logger.error("Error in username update script", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } +}; + module.exports = { verifyUser, generateChaincode, @@ -986,4 +996,5 @@ module.exports = { usersPatchHandler, isDeveloper, getIdentityStats, + updateUsernames, }; diff --git a/models/users.js b/models/users.js index 82a0b2c45..acf5c9382 100644 --- a/models/users.js +++ b/models/users.js @@ -913,6 +913,130 @@ const getNonNickNameSyncedUsers = async () => { } }; +const updateUsernamesInBatch = async (usersData) => { + const batch = firestore.batch(); + const usersBatch = []; + const summary = { + totalUpdatedUsernames: 0, + totalOperationsFailed: 0, + failedUserDetails: [], + }; + + usersData.forEach((user) => { + const updateUserData = { ...user, username: user.username }; + batch.update(userModel.doc(user.id), updateUserData); + usersBatch.push(user.id); + }); + + try { + await batch.commit(); + summary.totalUpdatedUsernames += usersData.length; + return { ...summary }; + } catch (err) { + logger.error("Firebase batch Operation Failed!"); + summary.totalOperationsFailed += usersData.length; + summary.failedUserDetails = [...usersBatch]; + return { ...summary }; + } +}; + +const sanitizeString = (str) => { + if (!str) return ""; + return str.replace(/[^a-zA-Z0-9-]/g, "-"); +}; + +const formatUsername = (firstName, lastName, suffix) => { + const actualFirstName = firstName.split(" ")[0].toLowerCase(); + const sanitizedFirstName = sanitizeString(actualFirstName); + const sanitizedLastName = sanitizeString(lastName).toLowerCase(); + + const baseUsername = `${sanitizedFirstName}-${sanitizedLastName}`; + const fullUsername = `${baseUsername}-${suffix}`; + + return fullUsername.length <= 32 + ? fullUsername + : `${baseUsername.slice(0, 32 - suffix.toString().length - 1)}-${suffix}`; +}; + +const updateUsersWithNewUsernames = async () => { + try { + const snapshot = await userModel.get(); + + const nonMemberUsers = snapshot.docs.filter((doc) => { + const roles = doc.data().roles; + return !(roles?.member === true || roles?.super_user === true); + }); + + const summary = { + totalUsers: nonMemberUsers.length, + totalUpdatedUsernames: 0, + totalOperationsFailed: 0, + failedUserDetails: [], + }; + + if (nonMemberUsers.length === 0) { + return summary; + } + + const usersToUpdate = []; + const nameToUsersMap = new Map(); + + nonMemberUsers.forEach((userDoc) => { + const userData = userDoc.data(); + const id = userDoc.id; + + const firstName = userData.first_name.split(" ")[0].toLowerCase(); + const lastName = userData.last_name.toLowerCase(); + const fullName = `${firstName}-${lastName}`; + + if (!nameToUsersMap.has(fullName)) { + nameToUsersMap.set(fullName, []); + } + + nameToUsersMap.get(fullName).push({ id, userData, createdAt: userData.created_at }); + }); + + for (const [, usersWithSameName] of nameToUsersMap.entries()) { + usersWithSameName.sort((a, b) => a.createdAt - b.createdAt); + + usersWithSameName.forEach((user, index) => { + const suffix = index + 1; + const formattedUsername = formatUsername(user.userData.first_name, user.userData.last_name, suffix); + + if (user.userData.username !== formattedUsername) { + usersToUpdate.push({ ...user.userData, id: user.id, username: formattedUsername }); + } + }); + } + + const userChunks = chunks(usersToUpdate, DOCUMENT_WRITE_SIZE); + + const updatedUsersPromises = await Promise.all( + userChunks.map(async (users) => { + const res = await updateUsernamesInBatch(users); + return res; + }) + ); + + updatedUsersPromises.forEach((res) => { + summary.totalUpdatedUsernames += res.totalUpdatedUsernames; + summary.totalOperationsFailed += res.totalOperationsFailed; + if (res.failedUserDetails.length > 0) { + summary.failedUserDetails.push(...res.failedUserDetails); + } + }); + + if (summary.totalOperationsFailed === summary.totalUsers) { + throw new Error("INTERNAL_SERVER_ERROR"); + } + + return summary; + } catch (error) { + logger.error(`Error in updating usernames: ${error}`); + throw error; + } +}; + module.exports = { addOrUpdate, fetchPaginatedUsers, @@ -942,4 +1066,5 @@ module.exports = { fetchUsersListForMultipleValues, fetchUserForKeyValue, getNonNickNameSyncedUsers, + updateUsersWithNewUsernames, }; diff --git a/routes/users.js b/routes/users.js index 9b29099d7..43b6dfd56 100644 --- a/routes/users.js +++ b/routes/users.js @@ -67,3 +67,5 @@ router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rej router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); router.get("/suggestedUsers/:skillId", authenticate, authorizeRoles([SUPERUSER]), users.getSuggestedUsers); module.exports = router; +router.post("/migration/update-usernames", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); +module.exports = router; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 92acc8325..93be6f94a 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -2423,4 +2423,58 @@ describe("Users", function () { }); }); }); + + describe("POST USERS MIGRATION", function () { + it("should run the migration and update usernames successfully", async function () { + const res = await chai + .request(app) + .post("/users/migration/update-usernames") + .set("cookie", `${cookieName}=${superUserAuthToken}`) + .send(); + + expect(res).to.have.status(200); + }); + + it("should not update usernames for super_user or member", async function () { + const res = await chai + .request(app) + .post("/users/migration/update-usernames") + .set("cookie", `${cookieName}=${superUserAuthToken}`) + .send(); + + expect(res).to.have.status(200); + const affectedUsers = res.body.totalUpdatedUsernames; + expect(affectedUsers).to.equal(0); + }); + + it("should return 401 for unauthorized user attempting migration", async function () { + const res = await chai + .request(app) + .post("/users/migration/update-usernames") + .set("cookie", `${cookieName}=${jwt}`) + .send(); + + expect(res).to.have.status(401); + }); + + it("should update username for users without member and super user true", async function () { + const userWithoutRoles = userData[2]; + const userId = await addUser(userWithoutRoles); + + const res = await chai + .request(app) + .post("/users/migration/update-usernames") + .set("cookie", `${cookieName}=${superUserAuthToken}`) + .send(); + + expect(res).to.have.status(200); + expect(res.body.totalUpdatedUsernames).to.be.greaterThan(0); + + const updatedUser = await firestore.collection("users").doc(userId).get(); + + expect(updatedUser.data().username).to.include( + `${userWithoutRoles.first_name.toLowerCase()}-${userWithoutRoles.last_name.toLowerCase()}-1` + ); + }); + }); }); From 0edca3dfb08ed50b857125ca8474cb70d073fdd5 Mon Sep 17 00:00:00 2001 From: Vinit khandal <111434418+vinit717@users.noreply.github.com> Date: Sun, 8 Sep 2024 16:22:15 +0530 Subject: [PATCH 3/3] Fix Username Migration script to run on users with no first/last name (#2130) --- models/users.js | 16 +++++++++++----- routes/users.js | 2 +- test/integration/users.test.js | 26 +++----------------------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/models/users.js b/models/users.js index acf5c9382..3d14c7636 100644 --- a/models/users.js +++ b/models/users.js @@ -963,8 +963,10 @@ const updateUsersWithNewUsernames = async () => { const snapshot = await userModel.get(); const nonMemberUsers = snapshot.docs.filter((doc) => { - const roles = doc.data().roles; - return !(roles?.member === true || roles?.super_user === true); + const userData = doc.data(); + const roles = userData.roles; + + return !(roles?.member === true || roles?.super_user === true || userData.incompleteUserDetails === true); }); const summary = { @@ -985,10 +987,14 @@ const updateUsersWithNewUsernames = async () => { const userData = userDoc.data(); const id = userDoc.id; - const firstName = userData.first_name.split(" ")[0].toLowerCase(); - const lastName = userData.last_name.toLowerCase(); - const fullName = `${firstName}-${lastName}`; + const firstName = userData.first_name?.split(" ")[0]?.toLowerCase(); + const lastName = userData.last_name?.toLowerCase(); + if (!firstName || !lastName) { + return; + } + + const fullName = `${firstName}-${lastName}`; if (!nameToUsersMap.has(fullName)) { nameToUsersMap.set(fullName, []); } diff --git a/routes/users.js b/routes/users.js index 43b6dfd56..32c906c2d 100644 --- a/routes/users.js +++ b/routes/users.js @@ -67,5 +67,5 @@ router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rej router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); router.get("/suggestedUsers/:skillId", authenticate, authorizeRoles([SUPERUSER]), users.getSuggestedUsers); module.exports = router; -router.post("/migration/update-usernames", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); +router.post("/batch-username-update", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); module.exports = router; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 93be6f94a..6e891e23e 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -2428,7 +2428,7 @@ describe("Users", function () { it("should run the migration and update usernames successfully", async function () { const res = await chai .request(app) - .post("/users/migration/update-usernames") + .post("/users/batch-username-update") .set("cookie", `${cookieName}=${superUserAuthToken}`) .send(); @@ -2438,7 +2438,7 @@ describe("Users", function () { it("should not update usernames for super_user or member", async function () { const res = await chai .request(app) - .post("/users/migration/update-usernames") + .post("/users/batch-username-update") .set("cookie", `${cookieName}=${superUserAuthToken}`) .send(); @@ -2450,31 +2450,11 @@ describe("Users", function () { it("should return 401 for unauthorized user attempting migration", async function () { const res = await chai .request(app) - .post("/users/migration/update-usernames") + .post("/users/batch-username-update") .set("cookie", `${cookieName}=${jwt}`) .send(); expect(res).to.have.status(401); }); - - it("should update username for users without member and super user true", async function () { - const userWithoutRoles = userData[2]; - const userId = await addUser(userWithoutRoles); - - const res = await chai - .request(app) - .post("/users/migration/update-usernames") - .set("cookie", `${cookieName}=${superUserAuthToken}`) - .send(); - - expect(res).to.have.status(200); - expect(res.body.totalUpdatedUsernames).to.be.greaterThan(0); - - const updatedUser = await firestore.collection("users").doc(userId).get(); - - expect(updatedUser.data().username).to.include( - `${userWithoutRoles.first_name.toLowerCase()}-${userWithoutRoles.last_name.toLowerCase()}-1` - ); - }); }); });