-
Notifications
You must be signed in to change notification settings - Fork 5
Update email in member DB when identity is also updated (PS-433) #18
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import { PrismaClient as TargetIdentityClient } from '@prisma/client'; | ||
| import { PrismaClient as SourceIdentityClient } from '../legacy_migrate/generated/source-identity'; | ||
|
|
||
| const DEFAULT_BATCH_SIZE = 1000; | ||
|
|
||
| const parseBoolFlag = (flag: string) => process.argv.includes(flag); | ||
|
|
||
| const requiredEnv = (key: string) => { | ||
| const value = process.env[key]; | ||
| if (!value) { | ||
| throw new Error(`Missing required environment variable: ${key}`); | ||
| } | ||
| return value; | ||
| }; | ||
|
|
||
| async function migrateUserSocialLogin() { | ||
| const dryRun = parseBoolFlag('--dry-run'); | ||
| const truncate = parseBoolFlag('--truncate'); | ||
| const batchSize = Number(process.env.USER_SOCIAL_LOGIN_BATCH_SIZE ?? DEFAULT_BATCH_SIZE); | ||
| const insertMissingOnly = parseBoolFlag('--insert-missing-only'); | ||
|
|
||
| if (Number.isNaN(batchSize) || batchSize <= 0) { | ||
| throw new Error(`Invalid USER_SOCIAL_LOGIN_BATCH_SIZE: ${process.env.USER_SOCIAL_LOGIN_BATCH_SIZE}`); | ||
| } | ||
|
|
||
| if (insertMissingOnly && truncate) { | ||
| throw new Error('Cannot use --insert-missing-only together with --truncate'); | ||
| } | ||
|
|
||
| const sourceDbUrl = requiredEnv('SOURCE_IDENTITY_PG_URL'); | ||
| const targetDbUrl = requiredEnv('IDENTITY_DB_URL'); | ||
|
|
||
| const sourceDb = new SourceIdentityClient({ | ||
| datasources: { | ||
| db: { url: sourceDbUrl }, | ||
| }, | ||
| }); | ||
|
|
||
| const targetDb = new TargetIdentityClient({ | ||
| datasources: { | ||
| db: { url: targetDbUrl }, | ||
| }, | ||
| }); | ||
|
|
||
| console.log( | ||
| `Starting user_social_login migration (dryRun=${dryRun}, truncate=${truncate}, insertMissingOnly=${insertMissingOnly}, batchSize=${batchSize})`, | ||
| ); | ||
|
|
||
| try { | ||
| const totalRows = await sourceDb.user_social_login.count(); | ||
| if (!totalRows) { | ||
| console.log('No rows found in source user_social_login. Nothing to migrate.'); | ||
| return; | ||
| } | ||
|
|
||
| console.log(`Found ${totalRows} rows to migrate from source user_social_login`); | ||
|
|
||
| if (truncate) { | ||
| if (dryRun) { | ||
| console.log('[Dry Run] Would truncate target identity.user_social_login'); | ||
| } else { | ||
| console.log('Truncating target identity.user_social_login before import...'); | ||
| await targetDb.$executeRaw`TRUNCATE TABLE identity.user_social_login RESTART IDENTITY CASCADE`; | ||
| } | ||
| } | ||
|
|
||
| for (let offset = 0; offset < totalRows; offset += batchSize) { | ||
| const batch = await sourceDb.user_social_login.findMany({ | ||
| orderBy: [{ user_id: 'asc' }, { social_login_provider_id: 'asc' }], | ||
| skip: offset, | ||
| take: batchSize, | ||
| }); | ||
|
|
||
| if (!batch.length) { | ||
| break; | ||
| } | ||
|
|
||
| if (dryRun) { | ||
| console.log(`[Dry Run] Would migrate ${batch.length} rows (offset ${offset})`); | ||
| continue; | ||
| } | ||
|
|
||
| const recordsToInsert = batch.map((record) => ({ | ||
| social_user_id: record.social_user_id ?? null, | ||
| user_id: Number(record.user_id), | ||
| social_login_provider_id: Number(record.social_login_provider_id), | ||
| social_user_name: record.social_user_name, | ||
| social_email: record.social_email ?? null, | ||
| social_email_verified: record.social_email_verified ?? null, | ||
| create_date: record.create_date ?? undefined, | ||
| modify_date: record.modify_date ?? undefined, | ||
| })); | ||
|
|
||
| if (insertMissingOnly) { | ||
| const existing = await targetDb.user_social_login.findMany({ | ||
| where: { | ||
| OR: recordsToInsert.map((rec) => ({ | ||
| user_id: rec.user_id, | ||
| social_login_provider_id: rec.social_login_provider_id, | ||
| })), | ||
| }, | ||
| select: { | ||
| user_id: true, | ||
| social_login_provider_id: true, | ||
| }, | ||
| }); | ||
|
|
||
| const existingKey = new Set(existing.map((row) => `${row.user_id}-${row.social_login_provider_id}`)); | ||
| const missing = recordsToInsert.filter( | ||
| (rec) => !existingKey.has(`${rec.user_id}-${rec.social_login_provider_id}`), | ||
| ); | ||
|
|
||
| if (!missing.length) { | ||
| console.log(`Batch at offset ${offset} skipped (all ${recordsToInsert.length} rows already present)`); | ||
| continue; | ||
| } | ||
|
|
||
| await targetDb.user_social_login.createMany({ | ||
| data: missing, | ||
| skipDuplicates: true, | ||
| }); | ||
|
|
||
| console.log( | ||
| `Migrated ${Math.min(offset + batch.length, totalRows)} / ${totalRows} rows (inserted ${missing.length}, skipped ${ | ||
| recordsToInsert.length - missing.length | ||
| })`, | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| await targetDb.user_social_login.createMany({ | ||
| data: recordsToInsert, | ||
| skipDuplicates: true, | ||
| }); | ||
|
|
||
| console.log(`Migrated ${Math.min(offset + batch.length, totalRows)} / ${totalRows} rows`); | ||
| } | ||
|
|
||
| console.log('user_social_login migration completed'); | ||
| } finally { | ||
| await Promise.allSettled([sourceDb.$disconnect(), targetDb.$disconnect()]); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| } | ||
| } | ||
|
|
||
| migrateUserSocialLogin().catch((error) => { | ||
| console.error('user_social_login migration failed:', error); | ||
| process.exit(1); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,11 +138,13 @@ const mockEventService: jest.Mocked<Partial<EventService>> = { | |
| postDirectBusMessage: jest.fn(), | ||
| }; | ||
|
|
||
| const mockMemberPrisma: Partial<MemberPrismaService> = { | ||
| const memberUpdateMock = jest.fn(); | ||
| const mockMemberPrisma: any = { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| // Only the parts used by UserService need to be mocked | ||
| member: { | ||
| create: jest.fn(), | ||
| } as any, | ||
| update: memberUpdateMock, | ||
| }, | ||
| }; | ||
|
|
||
| const mockConfigService = { | ||
|
|
@@ -1562,6 +1564,8 @@ describe('UserService', () => { | |
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
|
|
||
| memberUpdateMock.mockResolvedValue(undefined); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
|
|
||
| // Mock checkEmailAvailabilityForUser | ||
| mockCheckEmail = jest | ||
| .spyOn(service, 'checkEmailAvailabilityForUser') | ||
|
|
@@ -1652,6 +1656,10 @@ describe('UserService', () => { | |
| { userId: 1, handle: 'testuser' }, | ||
| ); | ||
| expect(result).toEqual(mockUser); | ||
| expect(memberUpdateMock).toHaveBeenCalledWith({ | ||
| where: { userId }, | ||
| data: { email: newEmail.toLowerCase() }, | ||
| }); | ||
| }); | ||
|
|
||
| it('should throw BadRequestException for invalid user ID format', async () => { | ||
|
|
@@ -1863,6 +1871,48 @@ describe('UserService', () => { | |
| ); | ||
| }); | ||
|
|
||
| it('should log an error if members.member update fails but continue', async () => { | ||
| const txMock = { | ||
| user: { | ||
| findUnique: jest.fn().mockResolvedValue(mockUser), | ||
| update: jest.fn().mockResolvedValue(mockUser), | ||
| }, | ||
| email: { | ||
| findFirst: jest | ||
| .fn() | ||
| .mockResolvedValueOnce(mockCurrentEmailRecord) | ||
| .mockResolvedValueOnce(null), | ||
| update: jest.fn().mockResolvedValue(mockUpdatedEmailRecord), | ||
| }, | ||
| }; | ||
| mockPrismaOltp.$transaction.mockImplementation( | ||
| <T>(callback): Promise<T> => { | ||
| const result = callback(txMock); | ||
| return result instanceof Promise ? result : Promise.resolve(result); | ||
| }, | ||
| ); | ||
|
|
||
| memberUpdateMock.mockRejectedValueOnce( | ||
| new Error('Member update failed'), | ||
| ); | ||
|
|
||
| const result = await service.updatePrimaryEmail( | ||
| userIdString, | ||
| newEmail, | ||
| mockAuthUser, | ||
| ); | ||
|
|
||
| expect(result).toEqual(mockUser); | ||
| expect(memberUpdateMock).toHaveBeenCalledWith({ | ||
| where: { userId }, | ||
| data: { email: newEmail.toLowerCase() }, | ||
| }); | ||
| expect(loggerErrorSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining('Failed to update members.member email'), | ||
| expect.any(String), | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle case when updated email record is not found after transaction', async () => { | ||
| // Set up transaction mock | ||
| const txMock = { | ||
|
|
@@ -1965,6 +2015,7 @@ describe('UserService', () => { | |
| const userIdString = '1'; | ||
| const oldStatus = 'U'; | ||
| const newStatus = 'A'; // Unverified to Active | ||
| const statusComment = 'Unit test comment'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [💡 |
||
| const mockExistingUser = createMockUserModel({ | ||
| user_id: new Decimal(userId), | ||
| status: oldStatus, | ||
|
|
@@ -1992,6 +2043,7 @@ describe('UserService', () => { | |
| userIdString, | ||
| newStatus, | ||
| mockUser, | ||
| statusComment, | ||
| ); | ||
|
|
||
| expect(prismaOltp.user.update).toHaveBeenCalledWith({ | ||
|
|
@@ -2023,7 +2075,7 @@ describe('UserService', () => { | |
| prismaOltp.user.findUnique.mockResolvedValue(fromActiveUser); | ||
| prismaOltp.user.update.mockResolvedValue(toInactiveUser); | ||
|
|
||
| await service.updateStatus(userIdString, 'I', mockUser); | ||
| await service.updateStatus(userIdString, 'I', mockUser, statusComment); | ||
| expect(mockEventService.postEnvelopedNotification).toHaveBeenCalledWith( | ||
| 'event.user.deactivated', | ||
| service.toCamelCase(toInactiveUser), | ||
|
|
@@ -2037,17 +2089,17 @@ describe('UserService', () => { | |
|
|
||
| it('should throw BadRequest for invalid user ID or status code', async () => { | ||
| await expect( | ||
| service.updateStatus('abc', newStatus, mockUser), | ||
| service.updateStatus('abc', newStatus, mockUser, statusComment), | ||
| ).rejects.toThrow(BadRequestException); | ||
| await expect( | ||
| service.updateStatus(userIdString, 'X', mockUser), | ||
| service.updateStatus(userIdString, 'X', mockUser, statusComment), | ||
| ).rejects.toThrow(BadRequestException); | ||
| }); | ||
|
|
||
| it('should throw NotFoundException if user not found', async () => { | ||
| prismaOltp.user.findUnique.mockResolvedValue(null); | ||
| await expect( | ||
| service.updateStatus(userIdString, newStatus, mockUser), | ||
| service.updateStatus(userIdString, newStatus, mockUser, statusComment), | ||
| ).rejects.toThrow(NotFoundException); | ||
| }); | ||
|
|
||
|
|
@@ -2056,6 +2108,7 @@ describe('UserService', () => { | |
| userIdString, | ||
| oldStatus, | ||
| mockUser, | ||
| statusComment, | ||
| ); | ||
| expect(prismaOltp.user.update).not.toHaveBeenCalled(); | ||
| expect(result).toEqual(mockExistingUser); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1451,6 +1451,8 @@ export class UserService { | |
| if (isNaN(userId)) { | ||
| throw new BadRequestException('Invalid user ID format.'); | ||
| } | ||
| const normalizedEmail = newEmail.toLowerCase(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| let emailChanged = false; | ||
|
|
||
| this.logger.log( | ||
| `Attempting to update primary email for user ID: ${userId} to ${newEmail} by admin ${authUser.userId}`, | ||
|
|
@@ -1512,10 +1514,11 @@ export class UserService { | |
| await tx.email.update({ | ||
| where: { email_id: currentPrimaryEmailRecord.email_id }, | ||
| data: { | ||
| address: newEmail.toLowerCase(), | ||
| address: normalizedEmail, | ||
| modify_date: new Date(), | ||
| }, | ||
| }); | ||
| emailChanged = true; | ||
|
|
||
| this.logger.log( | ||
| `Updated existing primary email record ${currentPrimaryEmailRecord.email_id.toNumber()} from ${oldEmail} to ${newEmail} for user ${userId}`, | ||
|
|
@@ -1554,6 +1557,23 @@ export class UserService { | |
| ); | ||
| } | ||
|
|
||
| if (emailChanged) { | ||
| try { | ||
| await this.memberPrisma.member.update({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [❗❗ |
||
| where: { userId }, | ||
| data: { email: normalizedEmail }, | ||
| }); | ||
| this.logger.log( | ||
| `Updated members.member email to ${normalizedEmail} for user ${userId}`, | ||
| ); | ||
| } catch (error) { | ||
| this.logger.error( | ||
| `Failed to update members.member email for user ${userId}: ${error.message}`, | ||
| error.stack, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return updatedUserInTx; | ||
| } | ||
|
|
||
|
|
@@ -2172,8 +2192,7 @@ export class UserService { | |
|
|
||
| // Send Welcome Email directly, matching legacy Java behavior | ||
| if (emailAddress && user?.handle) { | ||
| const domain = | ||
| this.configService.get<string>('APP_DOMAIN') || 'topcoder-dev.com'; | ||
| const domain = CommonUtils.getAppDomain(this.configService); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [ |
||
| const fromEmail = `Topcoder <noreply@${domain}>`; | ||
| let welcomeTemplateId = this.configService.get<string>( | ||
| 'SENDGRID_WELCOME_EMAIL_TEMPLATE_ID', | ||
|
|
@@ -2289,8 +2308,7 @@ export class UserService { | |
| email: string, | ||
| regSource?: string, | ||
| ) { | ||
| const domain = | ||
| this.configService.get<string>('APP_DOMAIN') || 'topcoder-dev.com'; | ||
| const domain = CommonUtils.getAppDomain(this.configService); | ||
| const fromEmail = `Topcoder <noreply@${domain}>`; | ||
| const sendGridTemplateId = this.configService.get<string>( | ||
| 'SENDGRID_RESEND_ACTIVATION_EMAIL_TEMPLATE_ID', | ||
|
|
@@ -2349,8 +2367,7 @@ export class UserService { | |
| async resendActivationEmailEvent(userOtp: UserOtpDto, primaryEmail: string) { | ||
| try { | ||
| // For activation email (resend), use postDirectBusMessage to match legacy Java structure | ||
| const domain = | ||
| this.configService.get<string>('APP_DOMAIN') || 'topcoder-dev.com'; | ||
| const domain = CommonUtils.getAppDomain(this.configService); | ||
| const fromEmail = `Topcoder <noreply@${domain}>`; | ||
| // Use the specific template ID for resending activation emails | ||
| const sendgridTemplateId = this.configService.get<string>( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗
security]Using
$executeRawfor SQL execution can be risky if any part of the query is constructed dynamically, as it may lead to SQL injection vulnerabilities. Ensure that the query is static or properly sanitized.