removed user read call in sessions and added organization key to model #960

rakeshSgr · 2024-10-07T13:22:57Z

No description provided.

aks30

Reviewed on 11 Oct

src/database/models/userExtension.js

aks30 · 2024-10-11T09:26:18Z

src/database/migrations/20240813060340-update-sessions-columns-drop-views-create-again copy.js

+require('dotenv').config()
+const materializedViewsService = require('@generics/materializedViews')
+
+/** @type {import('sequelize-cli').Migration} */


@rakeshSgr Remove copy in name, see if some better name can be given, please add comment in file what this migration does.

@aks30 its invalid file, not required

src/database/migrations/20241007083318-add organisation column to userExtention.js

aks30 · 2024-10-11T10:08:46Z

src/services/users.js

@@ -127,6 +165,8 @@ module.exports = class UserHelper {
 			competency: userDetails.competency,
 			designation: userDetails.designation,
 			language: userDetails.language,
+			organization_name: userDetails.organization.name,
+			image: userDetails.image,


@rakeshSgr we can add this in column i am thinking, anyways in ELEVATE we have user image

aks30

Reviewed on 14 Oct

aks30 · 2024-10-14T17:03:02Z

src/constants/interface-routes/sunbird-mentoring.json

@@ -0,0 +1,3335 @@
+{


@rakeshSgr Let's move these files in project root, in sunbird-mentoring and mentoring

aks30 · 2024-10-14T17:03:57Z

src/database/migrations/20241007083318-add organisation column to OrgExt.js

+		try {
+			const { STRING } = Sequelize
+
+			await queryInterface.addColumn('organization_extension', 'organization_name', {


@rakeshSgr let's call it name only

src/database/migrations/20241007083318-add organisation column to OrgExt.js

aks30 · 2024-10-14T17:06:28Z

src/database/models/organizationExtension.js

@@ -39,6 +39,7 @@ module.exports = (sequelize, DataTypes) => {
 			},
 			mentee_visibility_policy: { type: DataTypes.STRING },
 			external_mentee_visibility_policy: { type: DataTypes.STRING },
+			organization_name: { type: DataTypes.STRING },


@rakeshSgr rename it to just name please, Also update the corresponding DB diagrams for documentation as well.

aks30 · 2024-10-14T17:09:48Z

src/database/migrations/20241007083318-add organisation column to OrgExt.js

+		try {
+			const { STRING } = Sequelize
+
+			await queryInterface.addColumn('organization_extension', 'organization_name', {


@rakeshSgr name can be an index if its fetched very often

aks30

Reviewed on 16 Oct

interface-routes/sunbird-mentoring.json

src/services/sessions.js

aks30

Reviewed partially on 13 Nov

aks30 · 2024-11-12T19:36:55Z

src/utils/emailEncryption.js

@@ -24,6 +24,16 @@ const decrypt = async (encryptedEmail) => {
 		throw err
 	}
 }
-const emailEncryption = { encrypt, decrypt }
+
+async function decryptAndValidate(data) {


@rakeshSgr What validation happens here? Can we start adding function signature, name, description, argument examples and response signature.

we are trying to decrypt the email if success it returns decrypted value else it returns false

aks30 · 2024-11-12T19:45:29Z

src/database/queries/userExtension.js

+		}
+	}
+
+	static async getUsersByEmailIds(emailIds) {


@rakeshSgr Can we start adding function signatures, along with sample arguments and responses?

aks30 · 2024-11-12T19:45:57Z

src/database/queries/userExtension.js

+	static async getUsersByEmailIds(emailIds) {
+		try {
+			const userFilterClause =
+				emailIds.length === 0 ? '' : `email IN (${emailIds.map((id) => `'${id}'`).join(',')})`


@rakeshSgr is the email column indexed? Is it encrypted?

email is not indexed. yes its encrypted.

should we add index for email ?

aks30 · 2024-11-12T20:00:45Z

src/requests/user.js

 	try {
-		let orgReadUrl
+		if (organizationId && db == true) {
+			const organizationDetails = await organisationExtensionQueries.findOne({ organization_id: organizationId })


@rakeshSgr don't we need org code based query as below?

currently we are not storing org code in OrganizationExtension

aks30 · 2024-11-12T20:03:51Z

src/requests/user.js


-		if (userId) profileUrl += `/${userId}`
+			if (userDetails.image) {
+				userDetails.image = await getDownloadableUrl(userDetails.image).result


@rakeshSgr don't we have to check whether we have http path of just storage bucket path before making a call to user service even when db is true?

added check in the getDownloadableUrl function

aks30 · 2024-11-12T20:05:42Z

src/requests/user.js

@@ -103,38 +142,90 @@ const fetchUserDetails = async ({ token, userId }) => {
 * @returns
 */

-const getListOfUserDetails = function (userIds, excludeDeletedRecords = false) {
+const getListOfUserDetails = function (userIds, db = false, excludeDeletedRecords = false) {


@rakeshSgr Why default value is false here and true at other places? Also let's define function signatures in all functions we modify going forward?

this function has used in migrations so I kept db = false as default.

aks30 · 2024-11-12T20:08:27Z

src/requests/user.js

+				// Enrich user details with roles and organization info
+				userDetails.forEach(async (user) => {
+					if (user.image) {
+						user.image = await getDownloadableUrl(user.image).result


@rakeshSgr Why do we have to make a API call in loop?

currently we can get only one path at time from user service API

rakeshSgr added 9 commits October 7, 2024 18:51

removed user read call in sessions and added organization key to model

a98437c

removed console logs

6d65f14

kb optimisation changes

aff250c

user respose updated

b57f9ad

await added

7ef4c6c

logs added

d4b4ac5

logs added

ddc7913

logs added

a3110f4

removed logs

180d2df

aks30 requested changes Oct 11, 2024

View reviewed changes

rakeshSgr added 3 commits October 14, 2024 15:20

kb optimisation pr comments

b715ef6

api route updated

2381d68

remove unwanted code

1325fe6

aks30 requested changes Oct 14, 2024

View reviewed changes

rakeshSgr added 2 commits October 15, 2024 17:02

pr comments changes

abd9ecc

interface logs moved to root folder

79b8cb5

aks30 requested changes Oct 15, 2024

View reviewed changes

interface-routes/sunbird-mentoring.json Show resolved Hide resolved

interface-routes/sunbird-mentoring.json Outdated Show resolved Hide resolved

src/services/sessions.js Outdated Show resolved Hide resolved

rakeshSgr added 5 commits October 18, 2024 12:56

pr comments changes

dd587d8

kb optimisation changes

d0dba5f

removing console logs

1dc44d3

routes added

c8b11bd

api added

1f49afe

aks30 requested changes Nov 12, 2024

View reviewed changes

adding method desription and examples for the functions

9c2b9c3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed user read call in sessions and added organization key to model #960

removed user read call in sessions and added organization key to model #960

rakeshSgr commented Oct 7, 2024

aks30 left a comment

aks30 Oct 11, 2024

rakeshSgr Oct 11, 2024

aks30 Oct 11, 2024

rakeshSgr Oct 11, 2024

aks30 left a comment

aks30 Oct 14, 2024

aks30 Oct 14, 2024

aks30 Oct 14, 2024

aks30 Oct 14, 2024

aks30 left a comment

aks30 left a comment

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

aks30 Nov 12, 2024

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

aks30 Nov 12, 2024

rakeshSgr Nov 15, 2024

removed user read call in sessions and added organization key to model #960

Are you sure you want to change the base?

removed user read call in sessions and added organization key to model #960

Conversation

rakeshSgr commented Oct 7, 2024

aks30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aks30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aks30 left a comment

Choose a reason for hiding this comment

aks30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment