From 2fed9ad8ca0ac47a6898094165963ed926cdc362 Mon Sep 17 00:00:00 2001 From: vivek lokhande <68243599+vickydonor-99@users.noreply.github.com> Date: Fri, 23 Sep 2022 22:02:16 +0530 Subject: [PATCH 01/32] =?UTF-8?q?=E2=9C=A8feat(addDefaultColor):=20added?= =?UTF-8?q?=20code=20to=20add=20color=20property=20to=20exisiting=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- controllers/users.js | 22 ++++++++++++++++++++++ models/users.js | 32 ++++++++++++++++++++++++++++++++ routes/users.js | 1 + 3 files changed, 55 insertions(+) diff --git a/controllers/users.js b/controllers/users.js index 45293b0c0..a99458d7d 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -292,6 +292,27 @@ const rejectProfileDiff = async (req, res) => { } }; +/** + * Returns the lists of usernames where default colors were added + * + * @param req {Object} - Express request object + * @param res {Object} - Express response object + */ + +const addDefaultColors = async (req, res) => { + try { + const addedDefaultColorsData = await userQuery.addDefaultColors(); + + return res.json({ + message: "User colors updated successfully!", + ...addedDefaultColorsData, + }); + } catch (error) { + logger.error(`Error adding default colors to users: ${error}`); + return res.boom.badImplementation("Something went wrong. Please contact admin"); + } +}; + module.exports = { verifyUser, generateChaincode, @@ -305,4 +326,5 @@ module.exports = { rejectProfileDiff, getUserById, profileURL, + addDefaultColors, }; diff --git a/models/users.js b/models/users.js index 7b2c87d69..71fbcf040 100644 --- a/models/users.js +++ b/models/users.js @@ -186,6 +186,37 @@ const fetchUserImage = async (users) => { return images; }; +/** + * Adds default archived role + * @return {Promise} + */ + +const addDefaultColors = async () => { + try { + const usersSnapshot = await userModel.get(); + const migratedUsers = []; + const updateUserPromises = []; + const usersArr = []; + + usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); + + for (const user of usersArr) { + const colors = user.colors ? user.colors : {}; + if (user.color === undefined) { + colors.primary_color = ""; + colors.secondary_color = ""; + updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); + migratedUsers.push(user.username); + } + } + await Promise.all(updateUserPromises); + return { count: migratedUsers.length, users: migratedUsers }; + } catch (err) { + logger.error("Error adding default colors to users", err); + throw err; + } +}; + module.exports = { addOrUpdate, fetchUsers, @@ -194,4 +225,5 @@ module.exports = { initializeUser, updateUserPicture, fetchUserImage, + addDefaultColors, }; diff --git a/routes/users.js b/routes/users.js index d81501a5c..615324f2d 100644 --- a/routes/users.js +++ b/routes/users.js @@ -19,6 +19,7 @@ router.get("/:username", users.getUser); router.post("/picture", authenticate, upload.single("profile"), users.postUserPicture); router.patch("/profileURL", authenticate, userValidator.updateProfileURL, users.profileURL); router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rejectProfileDiff); +router.patch("/addDefaultColorProperty", authenticate, authorizeRoles([SUPERUSER]), users.addDefaultColors); router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); module.exports = router; From cd2c96dfcbd7eb3de1dc0391ab480a53ca67e1aa Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 6 Oct 2022 21:31:45 +0530 Subject: [PATCH 02/32] feat(user-colors):added code to add user colors --- constants/cardColorArray.js | 12 ++++++++ controllers/migrations.js | 27 ++++++++++++++++++ controllers/users.js | 22 --------------- models/users.js | 12 +++++--- routes/index.js | 1 + routes/migrations.js | 10 +++++++ routes/users.js | 1 - test/fixtures/user/user.js | 4 +++ test/integration/users.test.js | 50 +++++++++++++++++++++++++++++++++- utils/users.js | 10 +++++++ 10 files changed, 121 insertions(+), 28 deletions(-) create mode 100644 constants/cardColorArray.js create mode 100644 controllers/migrations.js create mode 100644 routes/migrations.js diff --git a/constants/cardColorArray.js b/constants/cardColorArray.js new file mode 100644 index 000000000..559e43d87 --- /dev/null +++ b/constants/cardColorArray.js @@ -0,0 +1,12 @@ +module.exports = [ + { color_primary: "#DB1212", color_secondary: "#F88181" }, + { color_primary: "#E9C46A", color_secondary: "#FFE094" }, + { color_primary: "#F4A261", color_secondary: "#FFBF8C" }, + { color_primary: "#2A9D8F", color_secondary: "#42E0CD" }, + { color_primary: "#165692", color_secondary: "#3D8BD3" }, + { color_primary: "#264653", color_secondary: "#387892" }, + { color_primary: "#B93160", color_secondary: "#D75281" }, + { color_primary: "#A5C9CA", color_secondary: "#E7F6F2" }, + { color_primary: "#6E85B7", color_secondary: "#B2C8DF" }, + { color_primary: "#FBA1A1", color_secondary: "#FBC5C5" }, +]; diff --git a/controllers/migrations.js b/controllers/migrations.js new file mode 100644 index 000000000..4a218ea55 --- /dev/null +++ b/controllers/migrations.js @@ -0,0 +1,27 @@ +const userQuery = require("../models/users"); +const logger = require("../utils/logger"); + +/** + * Returns the lists of usernames where default colors were added + * + * @param req {Object} - Express request object + * @param res {Object} - Express response object + */ + +const addDefaultColors = async (req, res) => { + try { + const addedDefaultColorsData = await userQuery.addDefaultColors(); + + return res.json({ + message: "User colors updated successfully!", + ...addedDefaultColorsData, + }); + } catch (error) { + logger.error(`Error adding default colors to users: ${error}`); + return res.boom.badImplementation("Something went wrong. Please contact admin"); + } +}; + +module.exports = { + addDefaultColors, +}; diff --git a/controllers/users.js b/controllers/users.js index a99458d7d..45293b0c0 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -292,27 +292,6 @@ const rejectProfileDiff = async (req, res) => { } }; -/** - * Returns the lists of usernames where default colors were added - * - * @param req {Object} - Express request object - * @param res {Object} - Express response object - */ - -const addDefaultColors = async (req, res) => { - try { - const addedDefaultColorsData = await userQuery.addDefaultColors(); - - return res.json({ - message: "User colors updated successfully!", - ...addedDefaultColorsData, - }); - } catch (error) { - logger.error(`Error adding default colors to users: ${error}`); - return res.boom.badImplementation("Something went wrong. Please contact admin"); - } -}; - module.exports = { verifyUser, generateChaincode, @@ -326,5 +305,4 @@ module.exports = { rejectProfileDiff, getUserById, profileURL, - addDefaultColors, }; diff --git a/models/users.js b/models/users.js index 9d921466c..663f99f15 100644 --- a/models/users.js +++ b/models/users.js @@ -7,7 +7,8 @@ const walletConstants = require("../constants/wallets"); const firestore = require("../utils/firestore"); const { fetchWallet, createWallet } = require("../models/wallets"); const userModel = firestore.collection("users"); - +const cardColorArray = require("../constants/cardColorArray"); +const { addColorsProperty } = require("../utils/users"); /** * Adds or updates the user data * @@ -204,9 +205,12 @@ const addDefaultColors = async () => { for (const user of usersArr) { const colors = user.colors ? user.colors : {}; - if (user.color === undefined) { - colors.primary_color = ""; - colors.secondary_color = ""; + if (user.colors === undefined) { + const userColorIndex = addColorsProperty(cardColorArray); + // eslint-disable-next-line no-console + console.log("index:", userColorIndex); + colors.primary_color = cardColorArray[parseInt(userColorIndex)].color_primary; + colors.secondary_color = cardColorArray[parseInt(userColorIndex)].color_secondary; updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); migratedUsers.push(user.username); } diff --git a/routes/index.js b/routes/index.js index 87592d632..a9827f25e 100644 --- a/routes/index.js +++ b/routes/index.js @@ -17,5 +17,6 @@ app.use("/trade", require("./trading.js")); app.use("/users", require("./users.js")); app.use("/profileDiffs", require("./profileDiffs.js")); app.use("/wallet", require("./wallets.js")); +app.use("/migrations", require("./migrations.js")); module.exports = app; diff --git a/routes/migrations.js b/routes/migrations.js new file mode 100644 index 000000000..894eb0a9e --- /dev/null +++ b/routes/migrations.js @@ -0,0 +1,10 @@ +const express = require("express"); +const router = express.Router(); +const authenticate = require("../middlewares/authenticate"); +const authorizeRoles = require("../middlewares/authorizeRoles"); +const { SUPERUSER } = require("../constants/roles"); +const migrations = require("../controllers/migrations"); + +router.patch("/addDefaultColorProperty", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); + +module.exports = router; diff --git a/routes/users.js b/routes/users.js index 615324f2d..d81501a5c 100644 --- a/routes/users.js +++ b/routes/users.js @@ -19,7 +19,6 @@ router.get("/:username", users.getUser); router.post("/picture", authenticate, upload.single("profile"), users.postUserPicture); router.patch("/profileURL", authenticate, userValidator.updateProfileURL, users.profileURL); router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rejectProfileDiff); -router.patch("/addDefaultColorProperty", authenticate, authorizeRoles([SUPERUSER]), users.addDefaultColors); router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); module.exports = router; diff --git a/test/fixtures/user/user.js b/test/fixtures/user/user.js index b09c9e812..62704bfd8 100644 --- a/test/fixtures/user/user.js +++ b/test/fixtures/user/user.js @@ -115,6 +115,10 @@ module.exports = () => { app_owner: true, archived: true, }, + colors: { + primary_color: "#DB1212", + secondary_color: "#F88181", + }, }, { username: "mehul", diff --git a/test/integration/users.test.js b/test/integration/users.test.js index dcb2b5a92..43b5fe7cf 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -11,6 +11,9 @@ const cleanDb = require("../utils/cleanDb"); const userData = require("../fixtures/user/user")(); const profileDiffData = require("../fixtures/profileDiffs/profileDiffs")(); const superUser = userData[4]; +const nonSuperUser = userData[0]; +const userWithColorProperty = [userData[5].username]; +const colorBearingUsernames = [superUser.username, nonSuperUser.username]; const config = require("config"); const cookieName = config.get("userToken.cookieName"); @@ -22,11 +25,12 @@ describe("Users", function () { let superUserId; let superUserAuthToken; let userId = ""; - + let nonSuperUserId = ""; beforeEach(async function () { userId = await addUser(); jwt = authService.generateAuthToken({ userId }); superUserId = await addUser(superUser); + nonSuperUserId = await addUser(nonSuperUser); superUserAuthToken = authService.generateAuthToken({ userId: superUserId }); }); @@ -565,4 +569,48 @@ describe("Users", function () { }); }); }); + + describe("PATCH /users/addDefaultColorProperty", function () { + it("Should return 401 if user is not a super user", function (done) { + const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); + chai + .request(app) + .patch(`/users/addDefaultColorProperty`) + .set("cookie", `${cookieName}=${nonSuperUserJwt}`) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(401); + expect(res.body).to.be.a("object"); + expect(res.body.message).to.equal("You are not authorized for this action."); + return done(); + }); + }); + it("Should add default color property to all users,using authorized user (super_user)", function (done) { + chai + .request(app) + .patch(`/users/addDefaultColorProperty`) + .set("cookie", `${cookieName}=${superUserAuthToken}`) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(200); + expect(res.body.count).to.be.equal(colorBearingUsernames.length); + const migratedUsernames = res.body.users; + + expect(migratedUsernames).to.include( + colorBearingUsernames[0], + "Should add default color property to user without color property" + ); + + expect(migratedUsernames).to.not.include.any.members( + userWithColorProperty, + "Should not modify color property of user with color object" + ); + return done(); + }); + }); + }); }); diff --git a/utils/users.js b/utils/users.js index aa45dad1f..c89dfef3a 100644 --- a/utils/users.js +++ b/utils/users.js @@ -81,9 +81,19 @@ const getParticipantUserIds = async (participantArray) => { } }; +/** + * Returns a random object from the array of colors to user + * @param cardColorArray {array} : color containing array + * @returns userColorIndex number : index between the range 0 to cardColorArray.length + */ +const addColorsProperty = (cardColorArray) => { + return Math.floor(Math.random() * (cardColorArray.length - 0) + 0); +}; + module.exports = { getUserId, getUsername, getParticipantUserIds, getParticipantUsernames, + addColorsProperty, }; From 5e4f75a33afd6c701b4ecf6ffc78060a233b521f Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 6 Oct 2022 21:41:27 +0530 Subject: [PATCH 03/32] =?UTF-8?q?=F0=9F=90=9E=20fix():removed=20console=20?= =?UTF-8?q?logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- models/users.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/users.js b/models/users.js index 663f99f15..9920f4238 100644 --- a/models/users.js +++ b/models/users.js @@ -207,8 +207,7 @@ const addDefaultColors = async () => { const colors = user.colors ? user.colors : {}; if (user.colors === undefined) { const userColorIndex = addColorsProperty(cardColorArray); - // eslint-disable-next-line no-console - console.log("index:", userColorIndex); + colors.primary_color = cardColorArray[parseInt(userColorIndex)].color_primary; colors.secondary_color = cardColorArray[parseInt(userColorIndex)].color_secondary; updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); From 04c43e77f61e3f60873ce384120311ff40ead595 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 13 Oct 2022 09:45:21 +0530 Subject: [PATCH 04/32] fixes+chores():fixed PR review changes and changed the filenames plus added migration files to required folders --- constants/cardColorArray.js | 20 +++--- controllers/migrations.js | 2 +- models/migrations.js | 41 +++++++++++++ models/users.js | 37 +----------- test/integration/migrations.test.js | 81 +++++++++++++++++++++++++ test/integration/users.test.js | 94 ++++++++++++++--------------- utils/users.js | 10 +-- 7 files changed, 185 insertions(+), 100 deletions(-) create mode 100644 models/migrations.js create mode 100644 test/integration/migrations.test.js diff --git a/constants/cardColorArray.js b/constants/cardColorArray.js index 559e43d87..04598f1bf 100644 --- a/constants/cardColorArray.js +++ b/constants/cardColorArray.js @@ -1,12 +1,12 @@ module.exports = [ - { color_primary: "#DB1212", color_secondary: "#F88181" }, - { color_primary: "#E9C46A", color_secondary: "#FFE094" }, - { color_primary: "#F4A261", color_secondary: "#FFBF8C" }, - { color_primary: "#2A9D8F", color_secondary: "#42E0CD" }, - { color_primary: "#165692", color_secondary: "#3D8BD3" }, - { color_primary: "#264653", color_secondary: "#387892" }, - { color_primary: "#B93160", color_secondary: "#D75281" }, - { color_primary: "#A5C9CA", color_secondary: "#E7F6F2" }, - { color_primary: "#6E85B7", color_secondary: "#B2C8DF" }, - { color_primary: "#FBA1A1", color_secondary: "#FBC5C5" }, + { COLOR_PRIMARY: "#DB1212", COLOR_SECONDARY: "#F88181" }, + { COLOR_PRIMARY: "#E9C46A", COLOR_SECONDARY: "#FFE094" }, + { COLOR_PRIMARY: "#F4A261", COLOR_SECONDARY: "#FFBF8C" }, + { COLOR_PRIMARY: "#2A9D8F", COLOR_SECONDARY: "#42E0CD" }, + { COLOR_PRIMARY: "#165692", COLOR_SECONDARY: "#3D8BD3" }, + { COLOR_PRIMARY: "#264653", COLOR_SECONDARY: "#387892" }, + { COLOR_PRIMARY: "#B93160", COLOR_SECONDARY: "#D75281" }, + { COLOR_PRIMARY: "#A5C9CA", COLOR_SECONDARY: "#E7F6F2" }, + { COLOR_PRIMARY: "#6E85B7", COLOR_SECONDARY: "#B2C8DF" }, + { COLOR_PRIMARY: "#FBA1A1", COLOR_SECONDARY: "#FBC5C5" }, ]; diff --git a/controllers/migrations.js b/controllers/migrations.js index 4a218ea55..2e6adef2b 100644 --- a/controllers/migrations.js +++ b/controllers/migrations.js @@ -1,4 +1,4 @@ -const userQuery = require("../models/users"); +const userQuery = require("../models/migrations"); const logger = require("../utils/logger"); /** diff --git a/models/migrations.js b/models/migrations.js new file mode 100644 index 000000000..aa4d2e2cc --- /dev/null +++ b/models/migrations.js @@ -0,0 +1,41 @@ +const firestore = require("../utils/firestore"); +const userModel = firestore.collection("users"); +const cardColorArray = require("../constants/cardColorArray"); +const { getRandomIndex } = require("../utils/users"); +/** + * Adds default colors property + * @return {Promise} + */ + +const addDefaultColors = async () => { + try { + const usersSnapshot = await userModel.get(); + const migratedUsers = []; + const updateUserPromises = []; + const usersArr = []; + + usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); + + for (const user of usersArr) { + const colors = user.colors ? user.colors : {}; + if (user.colors === undefined) { + const userColorIndex = getRandomIndex(cardColorArray); + + colors.primary_color = cardColorArray[parseInt(userColorIndex)].COLOR_PRIMARY; + colors.secondary_color = cardColorArray[parseInt(userColorIndex)].COLOR_SECONDARY; + updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); + migratedUsers.push(user.username); + } + } + + await Promise.all(updateUserPromises); + return { count: migratedUsers.length, users: migratedUsers }; + } catch (err) { + logger.error("Error adding default colors to users", err); + throw err; + } +}; + +module.exports = { + addDefaultColors, +}; diff --git a/models/users.js b/models/users.js index 9920f4238..2c2dbfd1a 100644 --- a/models/users.js +++ b/models/users.js @@ -7,8 +7,7 @@ const walletConstants = require("../constants/wallets"); const firestore = require("../utils/firestore"); const { fetchWallet, createWallet } = require("../models/wallets"); const userModel = firestore.collection("users"); -const cardColorArray = require("../constants/cardColorArray"); -const { addColorsProperty } = require("../utils/users"); + /** * Adds or updates the user data * @@ -189,39 +188,6 @@ const fetchUserImage = async (users) => { return images; }; -/** - * Adds default archived role - * @return {Promise} - */ - -const addDefaultColors = async () => { - try { - const usersSnapshot = await userModel.get(); - const migratedUsers = []; - const updateUserPromises = []; - const usersArr = []; - - usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); - - for (const user of usersArr) { - const colors = user.colors ? user.colors : {}; - if (user.colors === undefined) { - const userColorIndex = addColorsProperty(cardColorArray); - - colors.primary_color = cardColorArray[parseInt(userColorIndex)].color_primary; - colors.secondary_color = cardColorArray[parseInt(userColorIndex)].color_secondary; - updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); - migratedUsers.push(user.username); - } - } - await Promise.all(updateUserPromises); - return { count: migratedUsers.length, users: migratedUsers }; - } catch (err) { - logger.error("Error adding default colors to users", err); - throw err; - } -}; - module.exports = { addOrUpdate, fetchUsers, @@ -230,5 +196,4 @@ module.exports = { initializeUser, updateUserPicture, fetchUserImage, - addDefaultColors, }; diff --git a/test/integration/migrations.test.js b/test/integration/migrations.test.js new file mode 100644 index 000000000..8a4f62444 --- /dev/null +++ b/test/integration/migrations.test.js @@ -0,0 +1,81 @@ +const chai = require("chai"); +const { expect } = chai; +const chaiHttp = require("chai-http"); + +const app = require("../../server"); +const authService = require("../../services/authService"); +const addUser = require("../utils/addUser"); +const cleanDb = require("../utils/cleanDb"); +// Import fixtures +const userData = require("../fixtures/user/user")(); +const superUser = userData[4]; +const nonSuperUser = userData[0]; +const userWithColorProperty = [userData[5].username]; +const colorBearingUsernames = [superUser.username, nonSuperUser.username]; + +const config = require("config"); +const cookieName = config.get("userToken.cookieName"); + +chai.use(chaiHttp); + +describe("Migrations", function () { + let superUserId; + let superUserAuthToken; + let userId = ""; + let nonSuperUserId = ""; + beforeEach(async function () { + userId = await addUser(); + superUserId = await addUser(superUser); + nonSuperUserId = userId; + superUserAuthToken = authService.generateAuthToken({ userId: superUserId }); + }); + + afterEach(async function () { + await cleanDb(); + }); + + describe("PATCH /migrations/addDefaultColorProperty", function () { + it("Should return 401 if user is not a super user", function (done) { + const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); + chai + .request(app) + .patch(`/migrations/addDefaultColorProperty`) + .set("cookie", `${cookieName}=${nonSuperUserJwt}`) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(401); + expect(res.body).to.be.a("object"); + expect(res.body.message).to.equal("You are not authorized for this action."); + return done(); + }); + }); + it("Should add default color property to all users,using authorized user (super_user)", function (done) { + chai + .request(app) + .patch(`/migrations/addDefaultColorProperty`) + .set("cookie", `${cookieName}=${superUserAuthToken}`) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(200); + expect(res.body.count).to.be.equal(colorBearingUsernames.length); + const migratedUsernames = res.body.users; + + expect(migratedUsernames).to.include( + colorBearingUsernames[0], + "Should add default color property to user without color property" + ); + + expect(migratedUsernames).to.not.include.any.members( + userWithColorProperty, + "Should not modify color property of user with color object" + ); + return done(); + }); + }); + }); +}); diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 43b5fe7cf..5c096912a 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -11,9 +11,6 @@ const cleanDb = require("../utils/cleanDb"); const userData = require("../fixtures/user/user")(); const profileDiffData = require("../fixtures/profileDiffs/profileDiffs")(); const superUser = userData[4]; -const nonSuperUser = userData[0]; -const userWithColorProperty = [userData[5].username]; -const colorBearingUsernames = [superUser.username, nonSuperUser.username]; const config = require("config"); const cookieName = config.get("userToken.cookieName"); @@ -25,12 +22,12 @@ describe("Users", function () { let superUserId; let superUserAuthToken; let userId = ""; - let nonSuperUserId = ""; + beforeEach(async function () { userId = await addUser(); jwt = authService.generateAuthToken({ userId }); superUserId = await addUser(superUser); - nonSuperUserId = await addUser(nonSuperUser); + superUserAuthToken = authService.generateAuthToken({ userId: superUserId }); }); @@ -570,47 +567,48 @@ describe("Users", function () { }); }); - describe("PATCH /users/addDefaultColorProperty", function () { - it("Should return 401 if user is not a super user", function (done) { - const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); - chai - .request(app) - .patch(`/users/addDefaultColorProperty`) - .set("cookie", `${cookieName}=${nonSuperUserJwt}`) - .end((err, res) => { - if (err) { - return done(err); - } - expect(res).to.have.status(401); - expect(res.body).to.be.a("object"); - expect(res.body.message).to.equal("You are not authorized for this action."); - return done(); - }); - }); - it("Should add default color property to all users,using authorized user (super_user)", function (done) { - chai - .request(app) - .patch(`/users/addDefaultColorProperty`) - .set("cookie", `${cookieName}=${superUserAuthToken}`) - .end((err, res) => { - if (err) { - return done(err); - } - expect(res).to.have.status(200); - expect(res.body.count).to.be.equal(colorBearingUsernames.length); - const migratedUsernames = res.body.users; - - expect(migratedUsernames).to.include( - colorBearingUsernames[0], - "Should add default color property to user without color property" - ); - - expect(migratedUsernames).to.not.include.any.members( - userWithColorProperty, - "Should not modify color property of user with color object" - ); - return done(); - }); - }); - }); + // describe("PATCH /migrations/addDefaultColorProperty", function () { + // it("Should return 401 if user is not a super user", function (done) { + // const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); + // chai + // .request(app) + // .patch(`/migrations/addDefaultColorProperty`) + // .set("cookie", `${cookieName}=${nonSuperUserJwt}`) + // .end((err, res) => { + // if (err) { + // return done(err); + // } + // expect(res).to.have.status(401); + // expect(res.body).to.be.a("object"); + // expect(res.body.message).to.equal("You are not authorized for this action."); + // return done(); + // }); + // }); + // it("Should add default color property to all users,using authorized user (super_user)", function (done) { + // chai + // .request(app) + // .patch(`/migrations/addDefaultColorProperty`) + // .set("cookie", `${cookieName}=${superUserAuthToken}`) + // .end((err, res) => { + // if (err) { + // return done(err); + // } + + // expect(res).to.have.status(200); + // expect(res.body.count).to.be.equal(colorBearingUsernames.length); + // const migratedUsernames = res.body.users; + + // expect(migratedUsernames).to.include( + // colorBearingUsernames[0], + // "Should add default color property to user without color property" + // ); + + // expect(migratedUsernames).to.not.include.any.members( + // userWithColorProperty, + // "Should not modify color property of user with color object" + // ); + // return done(); + // }); + // }); + // }); }); diff --git a/utils/users.js b/utils/users.js index c89dfef3a..0493dbc5e 100644 --- a/utils/users.js +++ b/utils/users.js @@ -83,11 +83,11 @@ const getParticipantUserIds = async (participantArray) => { /** * Returns a random object from the array of colors to user - * @param cardColorArray {array} : color containing array - * @returns userColorIndex number : index between the range 0 to cardColorArray.length + * @param array {array} : array containing objects + * @returns random Index number : index between the range 0 to array.length */ -const addColorsProperty = (cardColorArray) => { - return Math.floor(Math.random() * (cardColorArray.length - 0) + 0); +const getRandomIndex = (array = []) => { + return Math.floor(Math.random() * (array.length - 0) + 0); }; module.exports = { @@ -95,5 +95,5 @@ module.exports = { getUsername, getParticipantUserIds, getParticipantUsernames, - addColorsProperty, + getRandomIndex, }; From 964dd56b30433b5c668e7d3121f7ae81c54c2745 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 13 Oct 2022 09:48:13 +0530 Subject: [PATCH 05/32] chore(): dele commented code --- test/integration/users.test.js | 45 ---------------------------------- 1 file changed, 45 deletions(-) diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 5c096912a..ca21a1fb6 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -566,49 +566,4 @@ describe("Users", function () { }); }); }); - - // describe("PATCH /migrations/addDefaultColorProperty", function () { - // it("Should return 401 if user is not a super user", function (done) { - // const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); - // chai - // .request(app) - // .patch(`/migrations/addDefaultColorProperty`) - // .set("cookie", `${cookieName}=${nonSuperUserJwt}`) - // .end((err, res) => { - // if (err) { - // return done(err); - // } - // expect(res).to.have.status(401); - // expect(res.body).to.be.a("object"); - // expect(res.body.message).to.equal("You are not authorized for this action."); - // return done(); - // }); - // }); - // it("Should add default color property to all users,using authorized user (super_user)", function (done) { - // chai - // .request(app) - // .patch(`/migrations/addDefaultColorProperty`) - // .set("cookie", `${cookieName}=${superUserAuthToken}`) - // .end((err, res) => { - // if (err) { - // return done(err); - // } - - // expect(res).to.have.status(200); - // expect(res.body.count).to.be.equal(colorBearingUsernames.length); - // const migratedUsernames = res.body.users; - - // expect(migratedUsernames).to.include( - // colorBearingUsernames[0], - // "Should add default color property to user without color property" - // ); - - // expect(migratedUsernames).to.not.include.any.members( - // userWithColorProperty, - // "Should not modify color property of user with color object" - // ); - // return done(); - // }); - // }); - // }); }); From 0f65a110317bc8d211c3bcd460fa6319eddedc55 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 17 Oct 2022 15:46:31 +0530 Subject: [PATCH 06/32] chore(one-time-user-color): chores --- constants/cardColorArray.js | 20 ++++++++++---------- models/migrations.js | 3 +-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/constants/cardColorArray.js b/constants/cardColorArray.js index 04598f1bf..ae354685a 100644 --- a/constants/cardColorArray.js +++ b/constants/cardColorArray.js @@ -1,12 +1,12 @@ module.exports = [ - { COLOR_PRIMARY: "#DB1212", COLOR_SECONDARY: "#F88181" }, - { COLOR_PRIMARY: "#E9C46A", COLOR_SECONDARY: "#FFE094" }, - { COLOR_PRIMARY: "#F4A261", COLOR_SECONDARY: "#FFBF8C" }, - { COLOR_PRIMARY: "#2A9D8F", COLOR_SECONDARY: "#42E0CD" }, - { COLOR_PRIMARY: "#165692", COLOR_SECONDARY: "#3D8BD3" }, - { COLOR_PRIMARY: "#264653", COLOR_SECONDARY: "#387892" }, - { COLOR_PRIMARY: "#B93160", COLOR_SECONDARY: "#D75281" }, - { COLOR_PRIMARY: "#A5C9CA", COLOR_SECONDARY: "#E7F6F2" }, - { COLOR_PRIMARY: "#6E85B7", COLOR_SECONDARY: "#B2C8DF" }, - { COLOR_PRIMARY: "#FBA1A1", COLOR_SECONDARY: "#FBC5C5" }, + { COLOR_ID: 1, COLOR_PRIMARY: "#DB1212", COLOR_SECONDARY: "#F88181" }, + { COLOR_ID: 2, COLOR_PRIMARY: "#F4A261", COLOR_SECONDARY: "#FFBF8C" }, + { COLOR_ID: 3, COLOR_PRIMARY: "#2A9D8F", COLOR_SECONDARY: "#42E0CD" }, + { COLOR_ID: 4, COLOR_PRIMARY: "#165692", COLOR_SECONDARY: "#3D8BD3" }, + { COLOR_ID: 5, COLOR_PRIMARY: "#E9C46A", COLOR_SECONDARY: "#FFE094" }, + { COLOR_ID: 6, COLOR_PRIMARY: "#264653", COLOR_SECONDARY: "#387892" }, + { COLOR_ID: 7, COLOR_PRIMARY: "#B93160", COLOR_SECONDARY: "#D75281" }, + { COLOR_ID: 8, COLOR_PRIMARY: "#A5C9CA", COLOR_SECONDARY: "#E7F6F2" }, + { COLOR_ID: 9, COLOR_PRIMARY: "#6E85B7", COLOR_SECONDARY: "#B2C8DF" }, + { COLOR_ID: 10, COLOR_PRIMARY: "#FBA1A1", COLOR_SECONDARY: "#FBC5C5" }, ]; diff --git a/models/migrations.js b/models/migrations.js index aa4d2e2cc..4d54bdfcb 100644 --- a/models/migrations.js +++ b/models/migrations.js @@ -21,8 +21,7 @@ const addDefaultColors = async () => { if (user.colors === undefined) { const userColorIndex = getRandomIndex(cardColorArray); - colors.primary_color = cardColorArray[parseInt(userColorIndex)].COLOR_PRIMARY; - colors.secondary_color = cardColorArray[parseInt(userColorIndex)].COLOR_SECONDARY; + colors.color_id = cardColorArray[parseInt(userColorIndex)].COLOR_ID; updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); migratedUsers.push(user.username); } From 146304a8f3171a7210fc176c0c6f4c295201665b Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 17 Oct 2022 15:57:20 +0530 Subject: [PATCH 07/32] fix(fixtures): changed color property --- test/fixtures/user/user.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/fixtures/user/user.js b/test/fixtures/user/user.js index 62704bfd8..f7d7a0a8c 100644 --- a/test/fixtures/user/user.js +++ b/test/fixtures/user/user.js @@ -116,8 +116,7 @@ module.exports = () => { archived: true, }, colors: { - primary_color: "#DB1212", - secondary_color: "#F88181", + color_id: 2, }, }, { From 01c81386f17e5d42a8234bb3a612027d00824856 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Tue, 18 Oct 2022 10:56:18 +0530 Subject: [PATCH 08/32] fix():changed file structure suggested in the PR --- constants/cardColorArray.js | 12 ------------ constants/cardColorIdArray.js | 12 ++++++++++++ models/migrations.js | 8 ++++---- utils/helpers.js | 12 ++++++++++++ utils/users.js | 10 ---------- 5 files changed, 28 insertions(+), 26 deletions(-) delete mode 100644 constants/cardColorArray.js create mode 100644 constants/cardColorIdArray.js create mode 100644 utils/helpers.js diff --git a/constants/cardColorArray.js b/constants/cardColorArray.js deleted file mode 100644 index ae354685a..000000000 --- a/constants/cardColorArray.js +++ /dev/null @@ -1,12 +0,0 @@ -module.exports = [ - { COLOR_ID: 1, COLOR_PRIMARY: "#DB1212", COLOR_SECONDARY: "#F88181" }, - { COLOR_ID: 2, COLOR_PRIMARY: "#F4A261", COLOR_SECONDARY: "#FFBF8C" }, - { COLOR_ID: 3, COLOR_PRIMARY: "#2A9D8F", COLOR_SECONDARY: "#42E0CD" }, - { COLOR_ID: 4, COLOR_PRIMARY: "#165692", COLOR_SECONDARY: "#3D8BD3" }, - { COLOR_ID: 5, COLOR_PRIMARY: "#E9C46A", COLOR_SECONDARY: "#FFE094" }, - { COLOR_ID: 6, COLOR_PRIMARY: "#264653", COLOR_SECONDARY: "#387892" }, - { COLOR_ID: 7, COLOR_PRIMARY: "#B93160", COLOR_SECONDARY: "#D75281" }, - { COLOR_ID: 8, COLOR_PRIMARY: "#A5C9CA", COLOR_SECONDARY: "#E7F6F2" }, - { COLOR_ID: 9, COLOR_PRIMARY: "#6E85B7", COLOR_SECONDARY: "#B2C8DF" }, - { COLOR_ID: 10, COLOR_PRIMARY: "#FBA1A1", COLOR_SECONDARY: "#FBC5C5" }, -]; diff --git a/constants/cardColorIdArray.js b/constants/cardColorIdArray.js new file mode 100644 index 000000000..394354a07 --- /dev/null +++ b/constants/cardColorIdArray.js @@ -0,0 +1,12 @@ +module.exports = [ + { COLOR_ID: 1 }, + { COLOR_ID: 2 }, + { COLOR_ID: 3 }, + { COLOR_ID: 4 }, + { COLOR_ID: 5 }, + { COLOR_ID: 6 }, + { COLOR_ID: 7 }, + { COLOR_ID: 8 }, + { COLOR_ID: 9 }, + { COLOR_ID: 10 }, +]; diff --git a/models/migrations.js b/models/migrations.js index 4d54bdfcb..281603a62 100644 --- a/models/migrations.js +++ b/models/migrations.js @@ -1,7 +1,7 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); -const cardColorArray = require("../constants/cardColorArray"); -const { getRandomIndex } = require("../utils/users"); +const cardColorIdArray = require("../constants/cardColorIdArray"); +const { getRandomIndex } = require("../utils/helpers"); /** * Adds default colors property * @return {Promise} @@ -19,9 +19,9 @@ const addDefaultColors = async () => { for (const user of usersArr) { const colors = user.colors ? user.colors : {}; if (user.colors === undefined) { - const userColorIndex = getRandomIndex(cardColorArray); + const userColorIndex = getRandomIndex(cardColorIdArray); - colors.color_id = cardColorArray[parseInt(userColorIndex)].COLOR_ID; + colors.color_id = cardColorIdArray[parseInt(userColorIndex)].COLOR_ID; updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); migratedUsers.push(user.username); } diff --git a/utils/helpers.js b/utils/helpers.js new file mode 100644 index 000000000..169e88e51 --- /dev/null +++ b/utils/helpers.js @@ -0,0 +1,12 @@ +/** + * Returns a random object from the array of colors to user + * @param array {array} : array containing objects + * @returns random Index number : index between the range 0 to array.length + */ +const getRandomIndex = (array = []) => { + return Math.floor(Math.random() * (array.length - 0) + 0); +}; + +module.exports = { + getRandomIndex, +}; diff --git a/utils/users.js b/utils/users.js index 0493dbc5e..aa45dad1f 100644 --- a/utils/users.js +++ b/utils/users.js @@ -81,19 +81,9 @@ const getParticipantUserIds = async (participantArray) => { } }; -/** - * Returns a random object from the array of colors to user - * @param array {array} : array containing objects - * @returns random Index number : index between the range 0 to array.length - */ -const getRandomIndex = (array = []) => { - return Math.floor(Math.random() * (array.length - 0) + 0); -}; - module.exports = { getUserId, getUsername, getParticipantUserIds, getParticipantUsernames, - getRandomIndex, }; From 7a754449dd69f8c2d72be303e3be9700103e3e05 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Wed, 26 Oct 2022 10:10:20 +0530 Subject: [PATCH 09/32] fix: helpers.js --- utils/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/helpers.js b/utils/helpers.js index 169e88e51..7a77e021b 100644 --- a/utils/helpers.js +++ b/utils/helpers.js @@ -4,7 +4,7 @@ * @returns random Index number : index between the range 0 to array.length */ const getRandomIndex = (array = []) => { - return Math.floor(Math.random() * (array.length - 0) + 0); + return Math.floor(Math.random() * array.length); }; module.exports = { From 54c94ce655f2c6ef060f163e70cde84f6d127bc7 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Nov 2022 10:03:49 +0530 Subject: [PATCH 10/32] chore():fixed migration filenames --- controllers/{migrations.js => userMigrations.js} | 2 +- models/{migrations.js => userMigrations.js} | 0 routes/index.js | 2 +- routes/{migrations.js => userMigrations.js} | 2 +- test/integration/{migrations.test.js => userMigrations.test.js} | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename controllers/{migrations.js => userMigrations.js} (92%) rename models/{migrations.js => userMigrations.js} (100%) rename routes/{migrations.js => userMigrations.js} (86%) rename test/integration/{migrations.test.js => userMigrations.test.js} (98%) diff --git a/controllers/migrations.js b/controllers/userMigrations.js similarity index 92% rename from controllers/migrations.js rename to controllers/userMigrations.js index 2e6adef2b..66e9f415f 100644 --- a/controllers/migrations.js +++ b/controllers/userMigrations.js @@ -1,4 +1,4 @@ -const userQuery = require("../models/migrations"); +const userQuery = require("../models/userMigrations"); const logger = require("../utils/logger"); /** diff --git a/models/migrations.js b/models/userMigrations.js similarity index 100% rename from models/migrations.js rename to models/userMigrations.js diff --git a/routes/index.js b/routes/index.js index a9827f25e..5c17f38ff 100644 --- a/routes/index.js +++ b/routes/index.js @@ -17,6 +17,6 @@ app.use("/trade", require("./trading.js")); app.use("/users", require("./users.js")); app.use("/profileDiffs", require("./profileDiffs.js")); app.use("/wallet", require("./wallets.js")); -app.use("/migrations", require("./migrations.js")); +app.use("/migrations", require("./userMigrations.js")); module.exports = app; diff --git a/routes/migrations.js b/routes/userMigrations.js similarity index 86% rename from routes/migrations.js rename to routes/userMigrations.js index 894eb0a9e..9b4aabc3c 100644 --- a/routes/migrations.js +++ b/routes/userMigrations.js @@ -3,7 +3,7 @@ const router = express.Router(); const authenticate = require("../middlewares/authenticate"); const authorizeRoles = require("../middlewares/authorizeRoles"); const { SUPERUSER } = require("../constants/roles"); -const migrations = require("../controllers/migrations"); +const migrations = require("../controllers/userMigrations"); router.patch("/addDefaultColorProperty", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); diff --git a/test/integration/migrations.test.js b/test/integration/userMigrations.test.js similarity index 98% rename from test/integration/migrations.test.js rename to test/integration/userMigrations.test.js index 8a4f62444..2474f5e57 100644 --- a/test/integration/migrations.test.js +++ b/test/integration/userMigrations.test.js @@ -18,7 +18,7 @@ const cookieName = config.get("userToken.cookieName"); chai.use(chaiHttp); -describe("Migrations", function () { +describe("userColorMigrations", function () { let superUserId; let superUserAuthToken; let userId = ""; From f4d60d3beeffeeb90d349f67e43c62af7ecaa5ca Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Nov 2022 12:08:55 +0530 Subject: [PATCH 11/32] fix(userMigrations): changed file structure requested in the code review by ankush --- constants/cardColorIdArray.js | 12 ------------ controllers/userMigrations.js | 22 +++++++++++++++++++--- models/userMigrations.js | 19 ++----------------- test/integration/userMigrations.test.js | 4 ++-- utils/helpers.js | 4 ++-- 5 files changed, 25 insertions(+), 36 deletions(-) delete mode 100644 constants/cardColorIdArray.js diff --git a/constants/cardColorIdArray.js b/constants/cardColorIdArray.js deleted file mode 100644 index 394354a07..000000000 --- a/constants/cardColorIdArray.js +++ /dev/null @@ -1,12 +0,0 @@ -module.exports = [ - { COLOR_ID: 1 }, - { COLOR_ID: 2 }, - { COLOR_ID: 3 }, - { COLOR_ID: 4 }, - { COLOR_ID: 5 }, - { COLOR_ID: 6 }, - { COLOR_ID: 7 }, - { COLOR_ID: 8 }, - { COLOR_ID: 9 }, - { COLOR_ID: 10 }, -]; diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 66e9f415f..a9b60ea63 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,6 +1,7 @@ const userQuery = require("../models/userMigrations"); const logger = require("../utils/logger"); - +const cardColorIdArray = require("../constants/cardColorIdArray"); +const { getRandomIndex } = require("../utils/helpers"); /** * Returns the lists of usernames where default colors were added * @@ -10,11 +11,26 @@ const logger = require("../utils/logger"); const addDefaultColors = async (req, res) => { try { - const addedDefaultColorsData = await userQuery.addDefaultColors(); + const { usersArr, userModel } = await userQuery.addDefaultColors(); + const migratedUsers = []; + const updateUserPromises = []; + + for (const user of usersArr) { + const colors = user.colors ? user.colors : {}; + if (user.colors === undefined) { + const userColorIndex = getRandomIndex(cardColorIdArray.length); + colors.color_id = userColorIndex; + updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); + migratedUsers.push(user.username); + } + } + + await Promise.all(updateUserPromises); + // eslint-disable-next-line no-console return res.json({ message: "User colors updated successfully!", - ...addedDefaultColorsData, + usersDetails: { count: migratedUsers.length, users: migratedUsers }, }); } catch (error) { logger.error(`Error adding default colors to users: ${error}`); diff --git a/models/userMigrations.js b/models/userMigrations.js index 281603a62..226939162 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,7 +1,6 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); -const cardColorIdArray = require("../constants/cardColorIdArray"); -const { getRandomIndex } = require("../utils/helpers"); + /** * Adds default colors property * @return {Promise} @@ -10,25 +9,11 @@ const { getRandomIndex } = require("../utils/helpers"); const addDefaultColors = async () => { try { const usersSnapshot = await userModel.get(); - const migratedUsers = []; - const updateUserPromises = []; const usersArr = []; usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); - for (const user of usersArr) { - const colors = user.colors ? user.colors : {}; - if (user.colors === undefined) { - const userColorIndex = getRandomIndex(cardColorIdArray); - - colors.color_id = cardColorIdArray[parseInt(userColorIndex)].COLOR_ID; - updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); - migratedUsers.push(user.username); - } - } - - await Promise.all(updateUserPromises); - return { count: migratedUsers.length, users: migratedUsers }; + return { usersArr, userModel }; } catch (err) { logger.error("Error adding default colors to users", err); throw err; diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 2474f5e57..772e7e49b 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -62,8 +62,8 @@ describe("userColorMigrations", function () { } expect(res).to.have.status(200); - expect(res.body.count).to.be.equal(colorBearingUsernames.length); - const migratedUsernames = res.body.users; + expect(res.body.usersDetails.count).to.be.equal(colorBearingUsernames.length); + const migratedUsernames = res.body.usersDetails.users; expect(migratedUsernames).to.include( colorBearingUsernames[0], diff --git a/utils/helpers.js b/utils/helpers.js index 7a77e021b..8e3c48cc3 100644 --- a/utils/helpers.js +++ b/utils/helpers.js @@ -3,8 +3,8 @@ * @param array {array} : array containing objects * @returns random Index number : index between the range 0 to array.length */ -const getRandomIndex = (array = []) => { - return Math.floor(Math.random() * array.length); +const getRandomIndex = (arrayLength = 10) => { + return Math.floor(Math.random() * arrayLength); }; module.exports = { From 64f56ccc23d1a341bed94be696c347a3a0e26153 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Nov 2022 12:09:41 +0530 Subject: [PATCH 12/32] chore():removed unwanted comment --- controllers/userMigrations.js | 1 - 1 file changed, 1 deletion(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index a9b60ea63..aa8e2dca6 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -26,7 +26,6 @@ const addDefaultColors = async (req, res) => { } await Promise.all(updateUserPromises); - // eslint-disable-next-line no-console return res.json({ message: "User colors updated successfully!", From b21007673c66a64fc87ef9c9d89ce6f74b3ae60a Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 10 Nov 2022 09:26:12 +0530 Subject: [PATCH 13/32] fix(usercolorArray): added constant --- controllers/userMigrations.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index aa8e2dca6..a0a7c79de 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,7 +1,7 @@ const userQuery = require("../models/userMigrations"); const logger = require("../utils/logger"); -const cardColorIdArray = require("../constants/cardColorIdArray"); const { getRandomIndex } = require("../utils/helpers"); +const userColors = 10; /** * Returns the lists of usernames where default colors were added * @@ -18,7 +18,7 @@ const addDefaultColors = async (req, res) => { for (const user of usersArr) { const colors = user.colors ? user.colors : {}; if (user.colors === undefined) { - const userColorIndex = getRandomIndex(cardColorIdArray.length); + const userColorIndex = getRandomIndex(userColors); colors.color_id = userColorIndex; updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); migratedUsers.push(user.username); From 804c64c5e4940677cb369420d4f2dac15545b09f Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Fri, 9 Jun 2023 06:48:15 +0530 Subject: [PATCH 14/32] code fix --- controllers/userMigrations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index a0a7c79de..84ea025e3 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -16,7 +16,7 @@ const addDefaultColors = async (req, res) => { const updateUserPromises = []; for (const user of usersArr) { - const colors = user.colors ? user.colors : {}; + const colors = user.colors ?? {}; if (user.colors === undefined) { const userColorIndex = getRandomIndex(userColors); colors.color_id = userColorIndex; From b495022025e4695373b389c5a25f946c22c64455 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 24 Jul 2023 06:45:29 +0530 Subject: [PATCH 15/32] added firestore batch changes and updated tests --- controllers/userMigrations.js | 33 ++++++++++++++++++------- test/integration/userMigrations.test.js | 1 - 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 84ea025e3..03fcacaa5 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,7 +1,10 @@ +const firestore = require("../utils/firestore"); const userQuery = require("../models/userMigrations"); const logger = require("../utils/logger"); const { getRandomIndex } = require("../utils/helpers"); -const userColors = 10; +const USER_COLORS = 10; +const MAX_TRANSACTION_WRITES = 499; + /** * Returns the lists of usernames where default colors were added * @@ -12,24 +15,36 @@ const userColors = 10; const addDefaultColors = async (req, res) => { try { const { usersArr, userModel } = await userQuery.addDefaultColors(); - const migratedUsers = []; - const updateUserPromises = []; + + const batchArray = []; + const users = []; + batchArray.push(firestore.batch()); + let operationCounter = 0; + let batchIndex = 0; + let totalCount = 0; for (const user of usersArr) { const colors = user.colors ?? {}; if (user.colors === undefined) { - const userColorIndex = getRandomIndex(userColors); + const userColorIndex = getRandomIndex(USER_COLORS); colors.color_id = userColorIndex; - updateUserPromises.push(userModel.doc(user.id).set({ ...user, colors })); - migratedUsers.push(user.username); + const docId = userModel.doc(user.id); + batchArray[parseInt(batchIndex)].set(docId, { ...user, colors }); + operationCounter++; + totalCount++; + users.push(user.username); + if (operationCounter === MAX_TRANSACTION_WRITES) { + batchArray.push(firestore.batch()); + batchIndex++; + operationCounter = 0; + } } } - - await Promise.all(updateUserPromises); + batchArray.forEach(async (batch) => await batch.commit()); return res.json({ message: "User colors updated successfully!", - usersDetails: { count: migratedUsers.length, users: migratedUsers }, + usersDetails: { count: totalCount, users }, }); } catch (error) { logger.error(`Error adding default colors to users: ${error}`); diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 772e7e49b..663719f61 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -64,7 +64,6 @@ describe("userColorMigrations", function () { expect(res).to.have.status(200); expect(res.body.usersDetails.count).to.be.equal(colorBearingUsernames.length); const migratedUsernames = res.body.usersDetails.users; - expect(migratedUsernames).to.include( colorBearingUsernames[0], "Should add default color property to user without color property" From 807df549a0f0d9ba8e3550d705f2c8cf9afe96aa Mon Sep 17 00:00:00 2001 From: vivek lokhande <68243599+isVivek99@users.noreply.github.com> Date: Sat, 5 Aug 2023 11:40:05 +0530 Subject: [PATCH 16/32] Update controllers/userMigrations.js Co-authored-by: Randhir Kumar Singh <97341921+heyrandhir@users.noreply.github.com> --- controllers/userMigrations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 03fcacaa5..63339fe4c 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -25,7 +25,7 @@ const addDefaultColors = async (req, res) => { for (const user of usersArr) { const colors = user.colors ?? {}; - if (user.colors === undefined) { + if (!user.colors) { const userColorIndex = getRandomIndex(USER_COLORS); colors.color_id = userColorIndex; const docId = userModel.doc(user.id); From 2ecedb571baa18ce73f5cbb9020ccd0e1d004ea4 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Aug 2023 14:13:01 +0530 Subject: [PATCH 17/32] review changes --- controllers/userMigrations.js | 3 ++- models/userMigrations.js | 8 ++++---- routes/userMigrations.js | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 63339fe4c..7ea6f3284 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,4 +1,5 @@ const firestore = require("../utils/firestore"); +const userModel = firestore.collection("users"); const userQuery = require("../models/userMigrations"); const logger = require("../utils/logger"); const { getRandomIndex } = require("../utils/helpers"); @@ -14,7 +15,7 @@ const MAX_TRANSACTION_WRITES = 499; const addDefaultColors = async (req, res) => { try { - const { usersArr, userModel } = await userQuery.addDefaultColors(); + const { usersArr } = await userQuery.returnUsers(); const batchArray = []; const users = []; diff --git a/models/userMigrations.js b/models/userMigrations.js index 226939162..0446dd136 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -3,17 +3,17 @@ const userModel = firestore.collection("users"); /** * Adds default colors property - * @return {Promise} + * @return {Promise} */ -const addDefaultColors = async () => { +const returnUsers = async () => { try { const usersSnapshot = await userModel.get(); const usersArr = []; usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); - return { usersArr, userModel }; + return { usersArr }; } catch (err) { logger.error("Error adding default colors to users", err); throw err; @@ -21,5 +21,5 @@ const addDefaultColors = async () => { }; module.exports = { - addDefaultColors, + returnUsers, }; diff --git a/routes/userMigrations.js b/routes/userMigrations.js index 9b4aabc3c..07ffa4b5d 100644 --- a/routes/userMigrations.js +++ b/routes/userMigrations.js @@ -5,6 +5,6 @@ const authorizeRoles = require("../middlewares/authorizeRoles"); const { SUPERUSER } = require("../constants/roles"); const migrations = require("../controllers/userMigrations"); -router.patch("/addDefaultColorProperty", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); +router.patch("/userDefaultColor", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); module.exports = router; From 4b0df8283c24902dcdcc0d3ee40f56715ec0d937 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Aug 2023 18:08:39 +0530 Subject: [PATCH 18/32] change route name to noun --- test/integration/userMigrations.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 663719f61..878ca3e08 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -18,7 +18,7 @@ const cookieName = config.get("userToken.cookieName"); chai.use(chaiHttp); -describe("userColorMigrations", function () { +describe.only("userColorMigrations", function () { let superUserId; let superUserAuthToken; let userId = ""; @@ -34,12 +34,12 @@ describe("userColorMigrations", function () { await cleanDb(); }); - describe("PATCH /migrations/addDefaultColorProperty", function () { + describe("PATCH /migrations/userDefaultColor", function () { it("Should return 401 if user is not a super user", function (done) { const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); chai .request(app) - .patch(`/migrations/addDefaultColorProperty`) + .patch(`/migrations/userDefaultColor`) .set("cookie", `${cookieName}=${nonSuperUserJwt}`) .end((err, res) => { if (err) { @@ -54,7 +54,7 @@ describe("userColorMigrations", function () { it("Should add default color property to all users,using authorized user (super_user)", function (done) { chai .request(app) - .patch(`/migrations/addDefaultColorProperty`) + .patch(`/migrations/userDefaultColor`) .set("cookie", `${cookieName}=${superUserAuthToken}`) .end((err, res) => { if (err) { From cc10b824db5dd9a1a1495a2d2d62bd008b7ed4fb Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sat, 5 Aug 2023 18:09:38 +0530 Subject: [PATCH 19/32] (#712) change route name to noun --- test/integration/userMigrations.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 878ca3e08..42b307d36 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -18,7 +18,7 @@ const cookieName = config.get("userToken.cookieName"); chai.use(chaiHttp); -describe.only("userColorMigrations", function () { +describe("userColorMigrations", function () { let superUserId; let superUserAuthToken; let userId = ""; From b8c7521c8c207d9b1f71f1e1aa3ffd14004535da Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sun, 6 Aug 2023 16:19:36 +0530 Subject: [PATCH 20/32] update response based on review --- controllers/userMigrations.js | 7 +++++-- test/integration/userMigrations.test.js | 19 +++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 7ea6f3284..f34f74a6c 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,7 +1,6 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); const userQuery = require("../models/userMigrations"); -const logger = require("../utils/logger"); const { getRandomIndex } = require("../utils/helpers"); const USER_COLORS = 10; const MAX_TRANSACTION_WRITES = 499; @@ -45,7 +44,11 @@ const addDefaultColors = async (req, res) => { return res.json({ message: "User colors updated successfully!", - usersDetails: { count: totalCount, users }, + usersDetails: { + totalUsersFetched: usersArr.length, + totalUsersUpdated: totalCount, + totalUsersUnaffected: usersArr.length - totalCount, + }, }); } catch (error) { logger.error(`Error adding default colors to users: ${error}`); diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 42b307d36..430cbdeda 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -10,7 +10,6 @@ const cleanDb = require("../utils/cleanDb"); const userData = require("../fixtures/user/user")(); const superUser = userData[4]; const nonSuperUser = userData[0]; -const userWithColorProperty = [userData[5].username]; const colorBearingUsernames = [superUser.username, nonSuperUser.username]; const config = require("config"); @@ -18,13 +17,13 @@ const cookieName = config.get("userToken.cookieName"); chai.use(chaiHttp); -describe("userColorMigrations", function () { +describe.only("userColorMigrations", function () { let superUserId; let superUserAuthToken; let userId = ""; let nonSuperUserId = ""; beforeEach(async function () { - userId = await addUser(); + userId = await addUser(nonSuperUser); superUserId = await addUser(superUser); nonSuperUserId = userId; superUserAuthToken = authService.generateAuthToken({ userId: superUserId }); @@ -62,17 +61,9 @@ describe("userColorMigrations", function () { } expect(res).to.have.status(200); - expect(res.body.usersDetails.count).to.be.equal(colorBearingUsernames.length); - const migratedUsernames = res.body.usersDetails.users; - expect(migratedUsernames).to.include( - colorBearingUsernames[0], - "Should add default color property to user without color property" - ); - - expect(migratedUsernames).to.not.include.any.members( - userWithColorProperty, - "Should not modify color property of user with color object" - ); + expect(res.body.usersDetails.totalUsersFetched).to.be.equal(colorBearingUsernames.length); + expect(res.body.usersDetails.totalUsersUpdated).to.be.equal(colorBearingUsernames.length); + expect(res.body.usersDetails.totalUsersUnaffected).to.be.equal(0); return done(); }); }); From 231cbef5c148bdeccf2667c6314a430d36563777 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sun, 6 Aug 2023 16:20:02 +0530 Subject: [PATCH 21/32] update test based on review --- test/integration/userMigrations.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index 430cbdeda..edf4de29f 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -17,7 +17,7 @@ const cookieName = config.get("userToken.cookieName"); chai.use(chaiHttp); -describe.only("userColorMigrations", function () { +describe("userColorMigrations", function () { let superUserId; let superUserAuthToken; let userId = ""; From 48f6ef3135d25e450a6fa4a335ebb85c37012066 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Tue, 8 Aug 2023 17:46:34 +0530 Subject: [PATCH 22/32] (#712) moved firestore interaction inside controller and updated name of endpoint --- controllers/userMigrations.js | 39 ++------------------- models/userMigrations.js | 45 ++++++++++++++++++++++--- routes/userMigrations.js | 2 +- test/integration/userMigrations.test.js | 6 ++-- 4 files changed, 46 insertions(+), 46 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index f34f74a6c..652238875 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,9 +1,4 @@ -const firestore = require("../utils/firestore"); -const userModel = firestore.collection("users"); const userQuery = require("../models/userMigrations"); -const { getRandomIndex } = require("../utils/helpers"); -const USER_COLORS = 10; -const MAX_TRANSACTION_WRITES = 499; /** * Returns the lists of usernames where default colors were added @@ -14,41 +9,11 @@ const MAX_TRANSACTION_WRITES = 499; const addDefaultColors = async (req, res) => { try { - const { usersArr } = await userQuery.returnUsers(); - - const batchArray = []; - const users = []; - batchArray.push(firestore.batch()); - let operationCounter = 0; - let batchIndex = 0; - let totalCount = 0; - - for (const user of usersArr) { - const colors = user.colors ?? {}; - if (!user.colors) { - const userColorIndex = getRandomIndex(USER_COLORS); - colors.color_id = userColorIndex; - const docId = userModel.doc(user.id); - batchArray[parseInt(batchIndex)].set(docId, { ...user, colors }); - operationCounter++; - totalCount++; - users.push(user.username); - if (operationCounter === MAX_TRANSACTION_WRITES) { - batchArray.push(firestore.batch()); - batchIndex++; - operationCounter = 0; - } - } - } - batchArray.forEach(async (batch) => await batch.commit()); + const usersDetails = await userQuery.addDefaultColors(); return res.json({ message: "User colors updated successfully!", - usersDetails: { - totalUsersFetched: usersArr.length, - totalUsersUpdated: totalCount, - totalUsersUnaffected: usersArr.length - totalCount, - }, + usersDetails, }); } catch (error) { logger.error(`Error adding default colors to users: ${error}`); diff --git a/models/userMigrations.js b/models/userMigrations.js index 0446dd136..f4a5e09d8 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,19 +1,54 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); +const { getRandomIndex } = require("../utils/helpers"); +const USER_COLORS = 10; +const MAX_TRANSACTION_WRITES = 499; /** - * Adds default colors property - * @return {Promise} + * Returns the object with details about users to whom user color was added + * + * @param req {Object} - Express request object + * @param res {Object} - Express response object */ -const returnUsers = async () => { +const addDefaultColors = async () => { try { const usersSnapshot = await userModel.get(); const usersArr = []; usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); - return { usersArr }; + const batchArray = []; + const users = []; + batchArray.push(firestore.batch()); + let operationCounter = 0; + let batchIndex = 0; + let totalCount = 0; + + for (const user of usersArr) { + const colors = user.colors ?? {}; + if (!user.colors) { + const userColorIndex = getRandomIndex(USER_COLORS); + colors.color_id = userColorIndex; + const docId = userModel.doc(user.id); + batchArray[parseInt(batchIndex)].set(docId, { ...user, colors }); + operationCounter++; + totalCount++; + users.push(user.username); + if (operationCounter === MAX_TRANSACTION_WRITES) { + batchArray.push(firestore.batch()); + batchIndex++; + operationCounter = 0; + } + } + } + batchArray.forEach(async (batch) => await batch.commit()); + + return { + totalUsersFetched: usersArr.length, + totalUsersUpdated: totalCount, + totalUsersUnaffected: usersArr.length - totalCount, + }; } catch (err) { logger.error("Error adding default colors to users", err); throw err; @@ -21,5 +56,5 @@ const returnUsers = async () => { }; module.exports = { - returnUsers, + addDefaultColors, }; diff --git a/routes/userMigrations.js b/routes/userMigrations.js index 07ffa4b5d..e6e2519fb 100644 --- a/routes/userMigrations.js +++ b/routes/userMigrations.js @@ -5,6 +5,6 @@ const authorizeRoles = require("../middlewares/authorizeRoles"); const { SUPERUSER } = require("../constants/roles"); const migrations = require("../controllers/userMigrations"); -router.patch("/userDefaultColor", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); +router.patch("/user-default-color", authenticate, authorizeRoles([SUPERUSER]), migrations.addDefaultColors); module.exports = router; diff --git a/test/integration/userMigrations.test.js b/test/integration/userMigrations.test.js index edf4de29f..4713722da 100644 --- a/test/integration/userMigrations.test.js +++ b/test/integration/userMigrations.test.js @@ -33,12 +33,12 @@ describe("userColorMigrations", function () { await cleanDb(); }); - describe("PATCH /migrations/userDefaultColor", function () { + describe("PATCH /migrations/user-default-color", function () { it("Should return 401 if user is not a super user", function (done) { const nonSuperUserJwt = authService.generateAuthToken({ userId: nonSuperUserId }); chai .request(app) - .patch(`/migrations/userDefaultColor`) + .patch(`/migrations/user-default-color`) .set("cookie", `${cookieName}=${nonSuperUserJwt}`) .end((err, res) => { if (err) { @@ -53,7 +53,7 @@ describe("userColorMigrations", function () { it("Should add default color property to all users,using authorized user (super_user)", function (done) { chai .request(app) - .patch(`/migrations/userDefaultColor`) + .patch(`/migrations/user-default-color`) .set("cookie", `${cookieName}=${superUserAuthToken}`) .end((err, res) => { if (err) { From 018f829280cbc66e6036c89ba085ffaa2b6384b2 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 14 Aug 2023 12:52:44 +0530 Subject: [PATCH 23/32] (#712) added test for helpers --- controllers/userMigrations.js | 3 ++- test/unit/utils/helpers.test.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 test/unit/utils/helpers.test.js diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 652238875..968383fb6 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,4 +1,5 @@ const userQuery = require("../models/userMigrations"); +const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); /** * Returns the lists of usernames where default colors were added @@ -17,7 +18,7 @@ const addDefaultColors = async (req, res) => { }); } catch (error) { logger.error(`Error adding default colors to users: ${error}`); - return res.boom.badImplementation("Something went wrong. Please contact admin"); + return res.boom.badImplementation(SOMETHING_WENT_WRONG); } }; diff --git a/test/unit/utils/helpers.test.js b/test/unit/utils/helpers.test.js new file mode 100644 index 000000000..c43a258c3 --- /dev/null +++ b/test/unit/utils/helpers.test.js @@ -0,0 +1,20 @@ +const chai = require("chai"); +const { getRandomIndex } = require("../../../utils/helpers"); +const { expect } = chai; + +describe("helpers", function () { + describe("getRandom Index from function", function () { + it("should return a random number between 0 and 10 excluding 10 if no index is passed", function () { + const result = getRandomIndex(); + expect(result).to.be.at.least(0); + expect(result).to.be.below(10); + }); + + it("expect a number between 0 and passed number", function () { + const delimiter = 100; + const result = getRandomIndex(delimiter); + expect(result).to.be.at.least(0); + expect(result).to.be.below(delimiter); + }); + }); +}); From 62fb581ce7b29db7817e052a6053fab07e707f4a Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 14 Aug 2023 21:53:47 +0530 Subject: [PATCH 24/32] updated tests for the model --- controllers/userMigrations.js | 4 +- models/userMigrations.js | 7 ++- test/unit/models/userMigrations.test.js | 58 +++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 test/unit/models/userMigrations.test.js diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 968383fb6..4033d6a52 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,6 +1,6 @@ const userQuery = require("../models/userMigrations"); const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); - +const MAX_TRANSACTION_WRITES = 499; /** * Returns the lists of usernames where default colors were added * @@ -10,7 +10,7 @@ const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); const addDefaultColors = async (req, res) => { try { - const usersDetails = await userQuery.addDefaultColors(); + const usersDetails = await userQuery.addDefaultColors(MAX_TRANSACTION_WRITES); return res.json({ message: "User colors updated successfully!", diff --git a/models/userMigrations.js b/models/userMigrations.js index f4a5e09d8..202ba8766 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,8 +1,8 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); const { getRandomIndex } = require("../utils/helpers"); +export const MAX_TRANSACTION_WRITES = 499; const USER_COLORS = 10; -const MAX_TRANSACTION_WRITES = 499; /** * Returns the object with details about users to whom user color was added @@ -11,8 +11,11 @@ const MAX_TRANSACTION_WRITES = 499; * @param res {Object} - Express response object */ -const addDefaultColors = async () => { +const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { try { + if (batchSize > MAX_TRANSACTION_WRITES) { + throw new Error(`Error cannot add more than ${batchSize} users at once .`); + } const usersSnapshot = await userModel.get(); const usersArr = []; diff --git a/test/unit/models/userMigrations.test.js b/test/unit/models/userMigrations.test.js new file mode 100644 index 000000000..8b9355631 --- /dev/null +++ b/test/unit/models/userMigrations.test.js @@ -0,0 +1,58 @@ +import { MAX_TRANSACTION_WRITES } from "../../../models/userMigrations"; +const chai = require("chai"); +const { expect } = chai; +const firestore = require("../../../utils/firestore"); +const userModel = firestore.collection("users"); +const cleanDb = require("../../utils/cleanDb"); +const userMigrationModel = require("../../../models/userMigrations"); +const userData = require("../../fixtures/user/user")(); +const addUser = require("../../utils/addUser"); + +describe.only("userColorMigrations", function () { + beforeEach(async function () { + await addUser(userData[0]); + await addUser(userData[1]); + await addUser(userData[2]); + await addUser(userData[3]); + await addUser(userData[4]); + await addUser(userData[5]); + }); + afterEach(async function () { + await cleanDb(); + }); + + it("should throw an error on passing invalid max transactions", async function () { + try { + await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES + 1); + } catch (err) { + expect(err).to.be.a("error"); + expect(err.message).to.be.equal("Error cannot add more than 499 users at once ."); + } + }); + it("should add color property to added users which dont have a color property", async function () { + const response = await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES); + expect(response.totalUsersFetched).to.equal(6); + expect(response.totalUsersUpdated).to.equal(5); + expect(response.totalUsersUnaffected).to.equal(1); + }); + it("should make sure that batch updates are working properly by passing smaller batch size", async function () { + const SMALL_BATCH_SIZE = 2; + const response = await userMigrationModel.addDefaultColors(SMALL_BATCH_SIZE); + expect(response.totalUsersFetched).to.equal(6); + expect(response.totalUsersUpdated).to.equal(5); + expect(response.totalUsersUnaffected).to.equal(1); + }); + it.only("should not affect users already having color property", async function () { + // Manually add a color property to a user + const userId = await addUser(userData[0]); + await userModel.doc(userId).update({ colors: { color_id: 3 } }); + const response = await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES); + expect(response.totalUsersFetched).to.equal(6); + expect(response.totalUsersUpdated).to.equal(4); + expect(response.totalUsersUnaffected).to.equal(2); + + // Check that the user with a color property was unaffected + const updatedUser = await userModel.doc(userId).get(); + expect(updatedUser.data().colors.color_id).to.equal(3); + }); +}); From 542d504e011a6a402d715bc614aacbb51f105316 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 14 Aug 2023 21:55:04 +0530 Subject: [PATCH 25/32] updated tests for the model --- test/unit/models/userMigrations.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/models/userMigrations.test.js b/test/unit/models/userMigrations.test.js index 8b9355631..cc15778c8 100644 --- a/test/unit/models/userMigrations.test.js +++ b/test/unit/models/userMigrations.test.js @@ -8,7 +8,7 @@ const userMigrationModel = require("../../../models/userMigrations"); const userData = require("../../fixtures/user/user")(); const addUser = require("../../utils/addUser"); -describe.only("userColorMigrations", function () { +describe("userColorMigrations", function () { beforeEach(async function () { await addUser(userData[0]); await addUser(userData[1]); @@ -42,7 +42,7 @@ describe.only("userColorMigrations", function () { expect(response.totalUsersUpdated).to.equal(5); expect(response.totalUsersUnaffected).to.equal(1); }); - it.only("should not affect users already having color property", async function () { + it("should not affect users already having color property", async function () { // Manually add a color property to a user const userId = await addUser(userData[0]); await userModel.doc(userId).update({ colors: { color_id: 3 } }); From 7e0d00a331a5e874f130317ba4004186feeef43d Mon Sep 17 00:00:00 2001 From: vivek lokhande <68243599+isVivek99@users.noreply.github.com> Date: Wed, 16 Aug 2023 14:20:50 +0530 Subject: [PATCH 26/32] Update users.test.js --- test/integration/users.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 72b0d263e..cbe8c5f90 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -50,7 +50,6 @@ describe("Users", function () { userId = await addUser(); jwt = authService.generateAuthToken({ userId }); superUserId = await addUser(superUser); - superUserAuthToken = authService.generateAuthToken({ userId: superUserId }); const userDocRef = photoVerificationModel.doc(); From a522f0f0c6554fb20eb101965a7e38301a88463a Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Wed, 16 Aug 2023 14:39:45 +0530 Subject: [PATCH 27/32] adress PR comments --- controllers/userMigrations.js | 4 ++-- models/userMigrations.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/controllers/userMigrations.js b/controllers/userMigrations.js index 4033d6a52..968383fb6 100644 --- a/controllers/userMigrations.js +++ b/controllers/userMigrations.js @@ -1,6 +1,6 @@ const userQuery = require("../models/userMigrations"); const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); -const MAX_TRANSACTION_WRITES = 499; + /** * Returns the lists of usernames where default colors were added * @@ -10,7 +10,7 @@ const MAX_TRANSACTION_WRITES = 499; const addDefaultColors = async (req, res) => { try { - const usersDetails = await userQuery.addDefaultColors(MAX_TRANSACTION_WRITES); + const usersDetails = await userQuery.addDefaultColors(); return res.json({ message: "User colors updated successfully!", diff --git a/models/userMigrations.js b/models/userMigrations.js index 202ba8766..a4a98fa31 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,7 +1,7 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); const { getRandomIndex } = require("../utils/helpers"); -export const MAX_TRANSACTION_WRITES = 499; +export const MAX_TRANSACTION_WRITES = 500; const USER_COLORS = 10; /** @@ -34,7 +34,8 @@ const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { const userColorIndex = getRandomIndex(USER_COLORS); colors.color_id = userColorIndex; const docId = userModel.doc(user.id); - batchArray[parseInt(batchIndex)].set(docId, { ...user, colors }); + user.colors = colors; + batchArray[parseInt(batchIndex)].set(docId, user); operationCounter++; totalCount++; users.push(user.username); From c7235782c2b0138011d8dcb5bda7937e411da2e1 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Wed, 16 Aug 2023 15:23:01 +0530 Subject: [PATCH 28/32] remove unrequired check --- models/userMigrations.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/models/userMigrations.js b/models/userMigrations.js index a4a98fa31..d96518d77 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -13,9 +13,6 @@ const USER_COLORS = 10; const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { try { - if (batchSize > MAX_TRANSACTION_WRITES) { - throw new Error(`Error cannot add more than ${batchSize} users at once .`); - } const usersSnapshot = await userModel.get(); const usersArr = []; From c87fe9cc77debad9b1b083e5ff41a08eaef3890f Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Thu, 17 Aug 2023 21:07:30 +0530 Subject: [PATCH 29/32] added dataAccessLayer to fetch users --- models/userMigrations.js | 9 +++++---- test/unit/models/userMigrations.test.js | 11 ++--------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/models/userMigrations.js b/models/userMigrations.js index d96518d77..12ec3a303 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,7 +1,9 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); const { getRandomIndex } = require("../utils/helpers"); -export const MAX_TRANSACTION_WRITES = 500; +const dataAccess = require("../services/dataAccessLayer"); +const MAX_TRANSACTION_WRITES = 500; +const MAX_USERS_SIZE = 10_000; const USER_COLORS = 10; /** @@ -13,10 +15,9 @@ const USER_COLORS = 10; const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { try { - const usersSnapshot = await userModel.get(); + const usersSnapshot = await dataAccess.retrieveUsers({ query: { size: MAX_USERS_SIZE } }); const usersArr = []; - - usersSnapshot.forEach((doc) => usersArr.push({ id: doc.id, ...doc.data() })); + usersSnapshot.users.forEach((doc) => usersArr.push({ id: doc.id, ...doc })); const batchArray = []; const users = []; diff --git a/test/unit/models/userMigrations.test.js b/test/unit/models/userMigrations.test.js index cc15778c8..2681e782e 100644 --- a/test/unit/models/userMigrations.test.js +++ b/test/unit/models/userMigrations.test.js @@ -1,4 +1,3 @@ -import { MAX_TRANSACTION_WRITES } from "../../../models/userMigrations"; const chai = require("chai"); const { expect } = chai; const firestore = require("../../../utils/firestore"); @@ -9,6 +8,8 @@ const userData = require("../../fixtures/user/user")(); const addUser = require("../../utils/addUser"); describe("userColorMigrations", function () { + const MAX_TRANSACTION_WRITES = 500; + beforeEach(async function () { await addUser(userData[0]); await addUser(userData[1]); @@ -21,14 +22,6 @@ describe("userColorMigrations", function () { await cleanDb(); }); - it("should throw an error on passing invalid max transactions", async function () { - try { - await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES + 1); - } catch (err) { - expect(err).to.be.a("error"); - expect(err.message).to.be.equal("Error cannot add more than 499 users at once ."); - } - }); it("should add color property to added users which dont have a color property", async function () { const response = await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES); expect(response.totalUsersFetched).to.equal(6); From 6aaafd7827059f19d446bbd1681e068034100fb6 Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Sun, 20 Aug 2023 12:19:45 +0530 Subject: [PATCH 30/32] address failing test --- models/userMigrations.js | 9 +++++---- test/fixtures/user/user.js | 6 +++--- test/unit/models/userMigrations.test.js | 5 +++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/models/userMigrations.js b/models/userMigrations.js index 12ec3a303..11d385366 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -15,9 +15,9 @@ const USER_COLORS = 10; const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { try { - const usersSnapshot = await dataAccess.retrieveUsers({ query: { size: MAX_USERS_SIZE } }); - const usersArr = []; - usersSnapshot.users.forEach((doc) => usersArr.push({ id: doc.id, ...doc })); + const usersSnapshotArr = await dataAccess.retrieveUsers({ query: { size: MAX_USERS_SIZE } }); + const usersArr = usersSnapshotArr.users; + // usersSnapshot.users.forEach((doc) => usersArr.push({ id: doc.id, ...doc })); const batchArray = []; const users = []; @@ -28,6 +28,7 @@ const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { for (const user of usersArr) { const colors = user.colors ?? {}; + if (!user.colors) { const userColorIndex = getRandomIndex(USER_COLORS); colors.color_id = userColorIndex; @@ -37,7 +38,7 @@ const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { operationCounter++; totalCount++; users.push(user.username); - if (operationCounter === MAX_TRANSACTION_WRITES) { + if (operationCounter === batchSize) { batchArray.push(firestore.batch()); batchIndex++; operationCounter = 0; diff --git a/test/fixtures/user/user.js b/test/fixtures/user/user.js index 27d63fcd4..22f006f2d 100644 --- a/test/fixtures/user/user.js +++ b/test/fixtures/user/user.js @@ -145,9 +145,6 @@ module.exports = () => { app_owner: true, archived: true, }, - colors: { - color_id: 2, - }, picture: { publicId: "profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar", url: "https://res.cloudinary.com/realdevsquad/image/upload/v1667685133/profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar.jpg", @@ -173,6 +170,9 @@ module.exports = () => { archived: false, in_discord: true, }, + colors: { + color_id: 2, + }, picture: { publicId: "profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar", url: "https://res.cloudinary.com/realdevsquad/image/upload/v1667685133/profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar.jpg", diff --git a/test/unit/models/userMigrations.test.js b/test/unit/models/userMigrations.test.js index 2681e782e..31f5dc4c4 100644 --- a/test/unit/models/userMigrations.test.js +++ b/test/unit/models/userMigrations.test.js @@ -16,14 +16,15 @@ describe("userColorMigrations", function () { await addUser(userData[2]); await addUser(userData[3]); await addUser(userData[4]); - await addUser(userData[5]); + await addUser(userData[6]); }); afterEach(async function () { await cleanDb(); }); it("should add color property to added users which dont have a color property", async function () { - const response = await userMigrationModel.addDefaultColors(MAX_TRANSACTION_WRITES); + const response = await userMigrationModel.addDefaultColors(); + expect(response.totalUsersFetched).to.equal(6); expect(response.totalUsersUpdated).to.equal(5); expect(response.totalUsersUnaffected).to.equal(1); From c2151cd09ca87c4a80cfa2fd8e47956848bb7f2a Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 21 Aug 2023 09:36:48 +0530 Subject: [PATCH 31/32] updated helper file --- models/userMigrations.js | 3 +-- test/unit/utils/helpers.test.js | 2 +- utils/helper.js | 10 ++++++++++ utils/helpers.js | 12 ------------ 4 files changed, 12 insertions(+), 15 deletions(-) delete mode 100644 utils/helpers.js diff --git a/models/userMigrations.js b/models/userMigrations.js index 11d385366..4e78fcb9a 100644 --- a/models/userMigrations.js +++ b/models/userMigrations.js @@ -1,6 +1,6 @@ const firestore = require("../utils/firestore"); const userModel = firestore.collection("users"); -const { getRandomIndex } = require("../utils/helpers"); +const { getRandomIndex } = require("../utils/helper"); const dataAccess = require("../services/dataAccessLayer"); const MAX_TRANSACTION_WRITES = 500; const MAX_USERS_SIZE = 10_000; @@ -17,7 +17,6 @@ const addDefaultColors = async (batchSize = MAX_TRANSACTION_WRITES) => { try { const usersSnapshotArr = await dataAccess.retrieveUsers({ query: { size: MAX_USERS_SIZE } }); const usersArr = usersSnapshotArr.users; - // usersSnapshot.users.forEach((doc) => usersArr.push({ id: doc.id, ...doc })); const batchArray = []; const users = []; diff --git a/test/unit/utils/helpers.test.js b/test/unit/utils/helpers.test.js index c43a258c3..dac7ad1ef 100644 --- a/test/unit/utils/helpers.test.js +++ b/test/unit/utils/helpers.test.js @@ -1,5 +1,5 @@ const chai = require("chai"); -const { getRandomIndex } = require("../../../utils/helpers"); +const { getRandomIndex } = require("../../../utils/helper"); const { expect } = chai; describe("helpers", function () { diff --git a/utils/helper.js b/utils/helper.js index 5ee6636f9..d1d29cad7 100644 --- a/utils/helper.js +++ b/utils/helper.js @@ -67,8 +67,18 @@ const getPaginatedLink = ({ return paginatedLink; }; +/** + * Returns a random object from the array of colors to user + * @param array {array} : array containing objects + * @returns random Index number : index between the range 0 to array.length + */ +const getRandomIndex = (maxLength = 10) => { + return Math.floor(Math.random() * maxLength); +}; + module.exports = { getQualifiers, getDateTimeRangeForPRs, getPaginatedLink, + getRandomIndex, }; diff --git a/utils/helpers.js b/utils/helpers.js deleted file mode 100644 index 8e3c48cc3..000000000 --- a/utils/helpers.js +++ /dev/null @@ -1,12 +0,0 @@ -/** - * Returns a random object from the array of colors to user - * @param array {array} : array containing objects - * @returns random Index number : index between the range 0 to array.length - */ -const getRandomIndex = (arrayLength = 10) => { - return Math.floor(Math.random() * arrayLength); -}; - -module.exports = { - getRandomIndex, -}; From 0852243aa6ee1a440bc555e0ce9b4fbb626ae30c Mon Sep 17 00:00:00 2001 From: vivek lokhande Date: Mon, 21 Aug 2023 09:38:58 +0530 Subject: [PATCH 32/32] update helper --- utils/helper.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/utils/helper.js b/utils/helper.js index d1d29cad7..10f129317 100644 --- a/utils/helper.js +++ b/utils/helper.js @@ -73,6 +73,14 @@ const getPaginatedLink = ({ * @returns random Index number : index between the range 0 to array.length */ const getRandomIndex = (maxLength = 10) => { + if (typeof maxLength !== "number") { + throw new Error("maxLength must be a number"); + } + + if (maxLength <= 0) { + throw new Error("maxLength must be a positive number"); + } + return Math.floor(Math.random() * maxLength); };