Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/one time user colors #735

Open
wants to merge 37 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2fed9ad
✨feat(addDefaultColor):
isVivek99 Sep 23, 2022
0f505b7
Merge branch 'develop' into feature/one-time-user-colors
vivek-geekyants Oct 4, 2022
cd2c96d
feat(user-colors):added code to add user colors
vivek-geekyants Oct 6, 2022
5bd0f9a
Merge branch 'develop' into feature/one-time-user-colors
vivek-geekyants Oct 6, 2022
5e4f75a
🐞 fix():removed console logs
vivek-geekyants Oct 6, 2022
04c43e7
fixes+chores():fixed PR review changes and changed the filenames plus…
isVivek99 Oct 13, 2022
964dd56
chore(): dele commented code
isVivek99 Oct 13, 2022
0f65a11
chore(one-time-user-color): chores
isVivek99 Oct 17, 2022
146304a
fix(fixtures): changed color property
isVivek99 Oct 17, 2022
01c8138
fix():changed file structure suggested in the PR
isVivek99 Oct 18, 2022
7a75444
fix: helpers.js
isVivek99 Oct 26, 2022
54c94ce
chore():fixed migration filenames
isVivek99 Nov 5, 2022
f4d60d3
fix(userMigrations):
isVivek99 Nov 5, 2022
64f56cc
chore():removed unwanted comment
isVivek99 Nov 5, 2022
b210076
fix(usercolorArray): added constant
isVivek99 Nov 10, 2022
804c64c
code fix
isVivek99 Jun 9, 2023
b2c9c3c
Merge branch 'develop' into feature/one-time-user-colors
isVivek99 Jul 17, 2023
b495022
added firestore batch changes and updated tests
isVivek99 Jul 24, 2023
807df54
Update controllers/userMigrations.js
isVivek99 Aug 5, 2023
2ecedb5
review changes
isVivek99 Aug 5, 2023
4b0df82
change route name to noun
isVivek99 Aug 5, 2023
cc10b82
(#712) change route name to noun
isVivek99 Aug 5, 2023
b8c7521
update response based on review
isVivek99 Aug 6, 2023
231cbef
update test based on review
isVivek99 Aug 6, 2023
48f6ef3
(#712) moved firestore interaction inside controller and updated name…
isVivek99 Aug 8, 2023
f322598
Merge branch 'develop' of https://github.com/Real-Dev-Squad/website-b…
isVivek99 Aug 14, 2023
018f829
(#712) added test for helpers
isVivek99 Aug 14, 2023
62fb581
updated tests for the model
isVivek99 Aug 14, 2023
542d504
updated tests for the model
isVivek99 Aug 14, 2023
7e0d00a
Update users.test.js
isVivek99 Aug 16, 2023
a522f0f
adress PR comments
isVivek99 Aug 16, 2023
c723578
remove unrequired check
isVivek99 Aug 16, 2023
d76b090
Merge branch 'feature/one-time-user-colors' of https://github.com/vic…
isVivek99 Aug 16, 2023
c87fe9c
added dataAccessLayer to fetch users
isVivek99 Aug 17, 2023
6aaafd7
address failing test
isVivek99 Aug 20, 2023
c2151cd
updated helper file
isVivek99 Aug 21, 2023
0852243
update helper
isVivek99 Aug 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions constants/cardColorIdArray.js
Original file line number Diff line number Diff line change
@@ -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 },
];
isVivek99 marked this conversation as resolved.
Show resolved Hide resolved
27 changes: 27 additions & 0 deletions controllers/migrations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const userQuery = require("../models/migrations");
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,
};
40 changes: 40 additions & 0 deletions models/migrations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
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<usersMigrated|Object>}
*/

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;
isVivek99 marked this conversation as resolved.
Show resolved Hide resolved
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,
};
1 change: 1 addition & 0 deletions routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
10 changes: 10 additions & 0 deletions routes/migrations.js
Original file line number Diff line number Diff line change
@@ -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;
3 changes: 3 additions & 0 deletions test/fixtures/user/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ module.exports = () => {
app_owner: true,
archived: true,
},
colors: {
color_id: 2,
isVivek99 marked this conversation as resolved.
Show resolved Hide resolved
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we creating an object to store color_id?
what else will we be storing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure ATM, we can make it just color_id, but if we are going to add more colors in the future then this would be better, should i change the property?

},
{
username: "mehul",
Expand Down
81 changes: 81 additions & 0 deletions test/integration/migrations.test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
});
1 change: 1 addition & 0 deletions test/integration/users.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe("Users", function () {
userId = await addUser();
jwt = authService.generateAuthToken({ userId });
superUserId = await addUser(superUser);

heyrandhir marked this conversation as resolved.
Show resolved Hide resolved
superUserAuthToken = authService.generateAuthToken({ userId: superUserId });
});

Expand Down
12 changes: 12 additions & 0 deletions utils/helpers.js
Original file line number Diff line number Diff line change
@@ -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);
};

module.exports = {
getRandomIndex,
};