Skip to content

Commit e944bda

Browse files
committed
fix: lowercase email-shaped identity values (CM-1349) (#4412)
Signed-off-by: Yeganathan S <63534555+skwowet@users.noreply.github.com>
1 parent c3cf218 commit e944bda

10 files changed

Lines changed: 126 additions & 102 deletions

File tree

backend/src/api/public/v1/members/createMember.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,6 @@ export async function createMember(req: Request, res: Response): Promise<void> {
5454
identities.map((identity) => ({
5555
...identity,
5656
memberId: dbMember.id,
57-
value:
58-
identity.type === MemberIdentityType.EMAIL
59-
? identity.value.trim().toLowerCase()
60-
: identity.value.trim(),
6157
})),
6258
true,
6359
true,

backend/src/api/public/v1/members/identities/createMemberIdentity.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,6 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
4646
throw new NotFoundError('Member not found')
4747
}
4848

49-
// Normalize emails to lowercase; keep username preferred casing from the caller.
50-
const normalizedValue =
51-
data.type === MemberIdentityType.EMAIL ? data.value.trim().toLowerCase() : data.value.trim()
52-
5349
let result!: IMemberIdentity
5450
let alreadyExisted = false
5551

@@ -59,7 +55,7 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
5955
captureOldState({})
6056

6157
await qx.tx(async (tx) => {
62-
const existing = await findMemberIdentitiesByValue(tx, memberId, normalizedValue, {
58+
const existing = await findMemberIdentitiesByValue(tx, memberId, data.value, {
6359
type: data.type,
6460
})
6561
const exactMatch = existing.find((i) => i.platform === data.platform)
@@ -75,7 +71,7 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
7571
{
7672
memberId,
7773
platform: data.platform,
78-
value: normalizedValue,
74+
value: data.value,
7975
type: data.type,
8076
source: data.source,
8177
verified: data.verified,
@@ -105,7 +101,7 @@ export async function createMemberIdentity(req: Request, res: Response): Promise
105101
}
106102
}
107103
} catch (error) {
108-
const ctx = { platform: data.platform, value: normalizedValue, type: data.type }
104+
const ctx = { platform: data.platform, value: data.value, type: data.type }
109105
rethrowDbConflict(error, ctx)
110106
}
111107

backend/src/database/repositories/memberRepository.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
Error409,
1515
RawQueryParser,
1616
groupBy,
17+
normalizeMemberIdentityValue,
1718
} from '@crowd/common'
1819
import { BotDetectionService, CommonMemberService } from '@crowd/common_services'
1920
import {
@@ -1852,6 +1853,7 @@ class MemberRepository {
18521853
const transaction = SequelizeRepository.getTransaction(options)
18531854

18541855
const seq = SequelizeRepository.getSequelize(options)
1856+
const normalizedValue = normalizeMemberIdentityValue(value)
18551857

18561858
const query = `
18571859
insert into "memberIdentities"("memberId", platform, type, value, "tenantId", verified)
@@ -1863,7 +1865,7 @@ class MemberRepository {
18631865
await seq.query(query, {
18641866
replacements: {
18651867
memberId,
1866-
value,
1868+
value: normalizedValue,
18671869
type,
18681870
platform,
18691871
tenantId: DEFAULT_TENANT_ID,

backend/src/services/member/memberIdentityService.ts

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,14 @@ import {
1313
updateMemberIdentity,
1414
} from '@crowd/data-access-layer/src/members'
1515
import { LoggerBase } from '@crowd/logging'
16-
import { IMemberIdentity, MemberIdentityType, NewMemberIdentity } from '@crowd/types'
16+
import { IMemberIdentity, NewMemberIdentity } from '@crowd/types'
1717

1818
import { IRepositoryOptions } from '@/database/repositories/IRepositoryOptions'
1919
import SequelizeRepository from '@/database/repositories/sequelizeRepository'
2020
import { optionsQx } from '@/database/sequelizeQueryExecutor'
2121

2222
import { IServiceOptions } from '../IServiceOptions'
2323

24-
function normalizeIdentityValue(type: string, value: string): string {
25-
const trimmed = value.trim()
26-
return type === MemberIdentityType.EMAIL ? trimmed.toLowerCase() : trimmed
27-
}
28-
2924
export default class MemberIdentityService extends LoggerBase {
3025
options: IServiceOptions
3126

@@ -63,16 +58,11 @@ export default class MemberIdentityService extends LoggerBase {
6358

6459
const qx = SequelizeRepository.getQueryExecutor(repoOptions)
6560

66-
const identityData = {
67-
...data,
68-
value: normalizeIdentityValue(data.type, data.value),
69-
}
70-
7161
// Check if identity already exists
7262
const conflict = await findMemberIdentityConflict(qx, {
73-
value: identityData.value,
74-
platform: identityData.platform,
75-
type: identityData.type,
63+
value: data.value,
64+
platform: data.platform,
65+
type: data.type,
7666
})
7767

7868
if (conflict) {
@@ -87,7 +77,7 @@ export default class MemberIdentityService extends LoggerBase {
8777
}
8878

8979
// Create member identity
90-
await insertMemberIdentities(qx, [{ ...identityData, memberId }])
80+
await insertMemberIdentities(qx, [{ ...data, memberId }])
9181

9282
await touchMemberUpdatedAt(qx, memberId)
9383

@@ -141,12 +131,7 @@ export default class MemberIdentityService extends LoggerBase {
141131
const qx = SequelizeRepository.getQueryExecutor(repoOptions)
142132

143133
// Check if any of the identities already exist
144-
const normalizedData = data.map((identity) => ({
145-
...identity,
146-
value: normalizeIdentityValue(identity.type, identity.value),
147-
}))
148-
149-
for (const identity of normalizedData) {
134+
for (const identity of data) {
150135
const conflict = await findMemberIdentityConflict(qx, {
151136
value: identity.value,
152137
platform: identity.platform,
@@ -168,7 +153,7 @@ export default class MemberIdentityService extends LoggerBase {
168153
// Create member identities
169154
await insertMemberIdentities(
170155
qx,
171-
normalizedData.map((identity) => ({ ...identity, memberId })),
156+
data.map((identity) => ({ ...identity, memberId })),
172157
)
173158

174159
await touchMemberUpdatedAt(qx, memberId)
@@ -226,10 +211,7 @@ export default class MemberIdentityService extends LoggerBase {
226211
throw new Error404(this.options.language, 'errors.notFound.message')
227212
}
228213

229-
const value = normalizeIdentityValue(
230-
data.type ?? currentIdentity.type,
231-
data.value ?? currentIdentity.value,
232-
)
214+
const value = data.value ?? currentIdentity.value
233215
const platform = data.platform ?? currentIdentity.platform
234216
const type = data.type ?? currentIdentity.type
235217

docs/adr/0015-how-cdp-stores-member-identities.md

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
**Date**: 2026-07-28
44
**Status**: accepted
55
**Deciders**: Yeganathan S
6-
**Related**: CM-1349
76

87
## Context
98

@@ -20,18 +19,18 @@ Historically, `memberIdentities` uniqueness was defined on raw `value` (case-sen
2019

2120
Lookups used `lower(value)`, but inserts often did not. Data-sink `mergeData` matched with exact `value ===`, so a self-serve lowercase `willsonhg` plus a later GitHub ingest of `WillsonHG` produced two rows for the same identity — often both verified on the same member. Prod had tens of thousands of GitHub case-variant groups.
2221

23-
Ticket CM-1349 proposed soft-deleting / auto-verifying case variants at verification time. That treats a write-path bug as a product special case.
22+
Soft-deleting or auto-verifying case variants only at verification time would treat a write-path bug as a product special case.
2423

2524
## Decision
2625

2726
**Mental model**
2827

29-
| Kind | Store | Compare / unique on |
30-
| --- | --- | --- |
31-
| username | preferred casing from the source/integration | `lower(value)` |
32-
| email | always lowercase | `lower(value)` (same as stored) |
28+
| Kind | Store | Compare / unique on |
29+
| -------- | -------------------------------- | ------------------------------- |
30+
| username | preferred casing from the source | `lower(value)` |
31+
| email | always lowercase | `lower(value)` (same as stored) |
3332

34-
Identity equality in CDP is `(platform, type, lower(value))`. The `value` column keeps what the source sent for usernames; we do not rewrite GitHub `login` casing on ingest.
33+
Identity equality in CDP is `(platform, type, lower(value))`. Email vs username for storage casing is inferred from the value with `isValidEmail`, not from `type` (git often stores emails as `type=username`). Non-email usernames keep preferred casing from the source.
3534

3635
**Enforcement**
3736

@@ -42,7 +41,7 @@ Identity equality in CDP is `(platform, type, lower(value))`. The `value` column
4241

4342
## Alternatives Considered
4443

45-
### Alternative 1: Soft-delete / auto-verify case variants at identity verification time (CM-1349 as written)
44+
### Alternative 1: Soft-delete / auto-verify case variants at identity verification time
4645

4746
- **Pros**: Fixes the user-visible self-serve pain quickly; no schema change.
4847
- **Cons**: Case variants keep being inserted by ingest/enrichment; verify path becomes a mop; duplicates still break uniqueness and analytics.

services/apps/data_sink_worker/src/service/activity.service.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111
generateUUIDv1,
1212
isDomainExcluded,
1313
isValidEmail,
14+
normalizeMemberIdentities,
15+
normalizeMemberIdentityValue,
1416
parseGitHubNoreplyEmail,
1517
single,
1618
singleOrDefault,
@@ -287,7 +289,16 @@ export default class ActivityService extends LoggerBase {
287289
}
288290

289291
let member = activity.member
290-
const username = activity.username ? activity.username.trim() : undefined
292+
if (member?.identities) {
293+
member.identities = normalizeMemberIdentities(member.identities)
294+
}
295+
296+
const username = activity.username
297+
? normalizeMemberIdentityValue(activity.username)
298+
: undefined
299+
if (username) {
300+
activity.username = username
301+
}
291302
if (!member && username) {
292303
member = {
293304
identities: [
@@ -317,17 +328,19 @@ export default class ActivityService extends LoggerBase {
317328
)
318329
if (platformIdentity && platformIdentity.value !== username) {
319330
this.log.debug(
320-
{ platform, originalUsername: username, correctedUsername: platformIdentity.value },
331+
{
332+
platform,
333+
originalUsername: username,
334+
correctedUsername: platformIdentity.value,
335+
},
321336
'Overriding activity.username with member platform identity value',
322337
)
323338
activity.username = platformIdentity.value
324339
}
325340
}
326341

327-
member.identities = member.identities.filter((i) => i.value)
328-
329-
if (!username) {
330-
const identities = activity.member.identities.filter(
342+
if (!activity.username) {
343+
const identities = (member?.identities ?? []).filter(
331344
(i) => i.platform === platform && i.type === MemberIdentityType.USERNAME,
332345
)
333346

@@ -337,12 +350,12 @@ export default class ActivityService extends LoggerBase {
337350
// Fall back to same-platform email identity — handles old gerrit records where
338351
// only a type:email identity was stored (before the gerrit integration
339352
// gained the email-as-username fallback).
340-
const emailFallback = activity.member.identities.find(
353+
const emailFallback = (member?.identities ?? []).find(
341354
(i) => i.platform === platform && i.type === MemberIdentityType.EMAIL && i.value,
342355
)
343356
if (emailFallback && emailFallback.verified) {
344357
activity.username = emailFallback.value
345-
activity.member.identities.push({
358+
member.identities.push({
346359
platform,
347360
type: MemberIdentityType.USERNAME,
348361
value: emailFallback.value,
@@ -388,12 +401,15 @@ export default class ActivityService extends LoggerBase {
388401
}
389402

390403
const objectMemberUsername = activity.objectMemberUsername
391-
? activity.objectMemberUsername.trim()
404+
? normalizeMemberIdentityValue(activity.objectMemberUsername)
392405
: undefined
406+
if (objectMemberUsername) {
407+
activity.objectMemberUsername = objectMemberUsername
408+
}
393409
let objectMember = activity.objectMember
394410

395-
if (objectMember) {
396-
objectMember.identities = objectMember.identities.filter((i) => i.value)
411+
if (objectMember?.identities) {
412+
objectMember.identities = normalizeMemberIdentities(objectMember.identities)
397413
}
398414

399415
if (objectMember && !objectMemberUsername) {
@@ -443,6 +459,11 @@ export default class ActivityService extends LoggerBase {
443459
}
444460
}
445461

462+
activity.member = member
463+
if (objectMember) {
464+
activity.objectMember = objectMember
465+
}
466+
446467
results.set(resultId, { success: true })
447468
}
448469

services/apps/data_sink_worker/src/service/member.service.ts

Lines changed: 8 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import {
88
getEarliestValidDate,
99
getProperDisplayName,
1010
isDomainExcluded,
11-
isEmail,
1211
isObjectEmpty,
1312
isSameMemberIdentity,
13+
normalizeMemberIdentities,
1414
singleOrDefault,
1515
} from '@crowd/common'
1616
import {
@@ -136,15 +136,7 @@ export default class MemberService extends LoggerBase {
136136
// Deduplicate by (platform, type, lower(value)) — prefer verified=true when the same
137137
// identity appears with both flags. Prevents per-member unique identity index
138138
// from firing when a payload contains the same identity twice with different verified values.
139-
const seen = new Map<string, IMemberIdentity>()
140-
for (const id of identities) {
141-
const key = `${id.platform}:${id.type}:${id.value.trim().toLowerCase()}`
142-
const existing = seen.get(key)
143-
if (!existing || (!existing.verified && id.verified)) {
144-
seen.set(key, id)
145-
}
146-
}
147-
const deduped = Array.from(seen.values())
139+
const deduped = normalizeMemberIdentities(identities)
148140

149141
try {
150142
await this.memberRepo.insertIdentities(memberId, integrationId, deduped, true)
@@ -346,8 +338,9 @@ export default class MemberService extends LoggerBase {
346338
)
347339
}
348340

349-
// validate emails
350-
data.identities = this.validateEmails(data.identities)
341+
data.identities = normalizeMemberIdentities(data.identities, {
342+
dropInvalidEmails: true,
343+
})
351344

352345
data.displayName = getProperDisplayName(data.displayName)
353346

@@ -586,11 +579,9 @@ export default class MemberService extends LoggerBase {
586579
)
587580
}
588581

589-
// prevent empty identity handles
590-
data.identities = data.identities.filter((i) => i.value)
591-
592-
// validate emails
593-
data.identities = this.validateEmails(data.identities)
582+
data.identities = normalizeMemberIdentities(data.identities, {
583+
dropInvalidEmails: true,
584+
})
594585

595586
// make sure displayName is proper
596587
if (data.displayName) {
@@ -952,31 +943,6 @@ export default class MemberService extends LoggerBase {
952943
}
953944
}
954945

955-
private validateEmails(identities: IMemberIdentity[]): IMemberIdentity[] {
956-
const toReturn: IMemberIdentity[] = []
957-
958-
for (const identity of identities) {
959-
if (identity.type === MemberIdentityType.EMAIL) {
960-
const lowerValue = identity.value.toLowerCase()
961-
if (
962-
isEmail(identity.value) &&
963-
toReturn.find(
964-
(i) =>
965-
i.type === MemberIdentityType.EMAIL &&
966-
i.value === lowerValue &&
967-
i.platform === identity.platform,
968-
) === undefined
969-
) {
970-
toReturn.push({ ...identity, value: lowerValue })
971-
}
972-
} else {
973-
toReturn.push(identity)
974-
}
975-
}
976-
977-
return toReturn
978-
}
979-
980946
private mergeData(
981947
dbMember: IDbMember,
982948
dbIdentities: IMemberIdentity[],

0 commit comments

Comments
 (0)