Skip to content

Commit 3b0634f

Browse files
malek10xdevmalek10xdev
authored
feat: add select target email for google contacts sync (#2591)
* update zerobounce rate limits & set execute evenly to True for mailercheck & drop queue interval to 10ms * remove clean unverified contacts toggle * fix(mining-api): set clean unverified contacts to work by default * remove i18n related to the removed toggle * feat(google contact sync): add select target email dialog * fix: phone number not added on updateEmptyfieldsOnly & 400 error on overwrite * add more cases on unittest * fix linting & formatting * fix formatting frontend
1 parent 8e33e77 commit 3b0634f

File tree

4 files changed

+342
-60
lines changed

4 files changed

+342
-60
lines changed

‎backend/src/controllers/contacts.controller.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ async function validateRequest(
3939
};
4040

4141
if (exportType === ExportType.GOOGLE_CONTACTS) {
42+
const targetEmail = req.body.targetEmail || user.email;
4243
const oauthCredentials = (await miningSources.getCredentialsBySourceEmail(
4344
user.id,
44-
user.email as string
45+
targetEmail as string
4546
)) as OAuthMiningSourceCredentials;
4647

4748
googleContactsOptions = {

‎backend/src/services/export/exports/google/contacts-api.ts‎

Lines changed: 88 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -391,66 +391,99 @@ export default class GoogleContactsSession {
391391
const existingMemberships = existing?.memberships || [];
392392
const labels = [this.appName, ...(contact.tags ?? [])].filter(Boolean);
393393

394-
const person: people_v1.Schema$Person = {
395-
names: updateEmptyOnly
396-
? existingNames
397-
: [
398-
...existingNames.filter(
399-
(n) =>
400-
n.givenName !== contact.given_name ||
401-
n.familyName !== contact.family_name
402-
),
403-
{
404-
givenName: contact.given_name,
405-
familyName: contact.family_name,
406-
unstructuredName: contact.name
407-
}
408-
],
409-
emailAddresses: updateEmptyOnly
410-
? existingEmails
411-
: [
394+
// Helper function to check if a field is empty
395+
const isEmpty = (val: string | null | undefined) =>
396+
val === null || val === undefined || val === '';
397+
398+
let names: people_v1.Schema$Name[] = existingNames;
399+
if (!updateEmptyOnly || existingNames.length === 0) {
400+
const newName: people_v1.Schema$Name = {};
401+
if (!isEmpty(contact.given_name)) {
402+
newName.givenName = contact.given_name;
403+
}
404+
if (!isEmpty(contact.family_name)) {
405+
newName.familyName = contact.family_name;
406+
}
407+
if (!isEmpty(contact.name)) {
408+
newName.unstructuredName = contact.name;
409+
}
410+
411+
if (newName.givenName || newName.familyName || newName.unstructuredName) {
412+
names = [newName];
413+
}
414+
}
415+
416+
const emailAddresses =
417+
!updateEmptyOnly || existingEmails.length === 0
418+
? [
412419
...existingEmails.filter((e) => e.value !== contact.email),
413420
{ value: contact.email }
414-
],
421+
]
422+
: existingEmails;
423+
424+
let phoneNumbers: people_v1.Schema$PhoneNumber[] = existingPhones;
425+
const newPhones =
426+
contact.telephone
427+
?.filter((tel) => !isEmpty(tel))
428+
.map((tel) => ({ value: tel })) || [];
429+
if (newPhones.length > 0) {
430+
const existingValues = new Set(existingPhones.map((p) => p.value));
431+
const uniqueNewPhones = newPhones.filter(
432+
(p) => !existingValues.has(p.value)
433+
);
434+
if (uniqueNewPhones.length > 0) {
435+
phoneNumbers = [...existingPhones, ...uniqueNewPhones];
436+
}
437+
}
415438

416-
phoneNumbers: updateEmptyOnly
417-
? existingPhones
418-
: [
419-
...existingPhones.filter(
420-
(p) => !contact.telephone?.includes(p.value ?? '')
421-
),
422-
...(contact.telephone?.map((tel) => ({ value: tel })) || [])
423-
],
439+
let organizations: people_v1.Schema$Organization[] = existingOrgs;
440+
const newOrg: people_v1.Schema$Organization = {};
441+
if (!isEmpty(contact.works_for)) {
442+
newOrg.name = contact.works_for;
443+
}
444+
if (!isEmpty(contact.job_title)) {
445+
newOrg.title = contact.job_title;
446+
}
447+
if (newOrg.name || newOrg.title) {
448+
const isDuplicate = existingOrgs.some(
449+
(o) => o.name === newOrg.name && o.title === newOrg.title
450+
);
451+
if (!isDuplicate) {
452+
organizations = [...existingOrgs, newOrg];
453+
}
454+
}
424455

425-
organizations: updateEmptyOnly
426-
? existingOrgs
427-
: [
428-
...existingOrgs.filter(
429-
(o) =>
430-
o.name !== contact.works_for || o.title !== contact.job_title
431-
),
432-
{
433-
name: contact.works_for,
434-
title: contact.job_title
435-
}
436-
],
456+
let urls: people_v1.Schema$Url[] = existingUrls;
457+
const newUrls =
458+
contact.same_as
459+
?.filter((url) => !isEmpty(url))
460+
.map((url) => ({ value: url })) || [];
461+
if (newUrls.length > 0) {
462+
const existingValues = new Set(existingUrls.map((u) => u.value));
463+
const uniqueNewUrls = newUrls.filter((u) => !existingValues.has(u.value));
464+
if (uniqueNewUrls.length > 0) {
465+
urls = [...existingUrls, ...uniqueNewUrls];
466+
}
467+
}
437468

438-
urls: updateEmptyOnly
439-
? existingUrls
440-
: [
441-
...existingUrls.filter(
442-
(u) => !contact.same_as?.includes(u.value ?? '')
443-
),
444-
...(contact.same_as?.map((url) => ({ value: url })) || [])
445-
],
446-
addresses: updateEmptyOnly
447-
? existingAddresses
448-
: [
449-
...existingAddresses.filter(
450-
(a) => a.streetAddress !== contact.location
451-
),
452-
...(contact.location ? [{ streetAddress: contact.location }] : [])
453-
],
469+
let addresses: people_v1.Schema$Address[] = existingAddresses;
470+
if (!isEmpty(contact.location)) {
471+
const newAddress = { streetAddress: contact.location };
472+
const isDuplicate = existingAddresses.some(
473+
(a) => a.streetAddress === newAddress.streetAddress
474+
);
475+
if (!isDuplicate) {
476+
addresses = [...existingAddresses, newAddress];
477+
}
478+
}
479+
480+
const person: people_v1.Schema$Person = {
481+
names,
482+
emailAddresses,
483+
phoneNumbers,
484+
organizations,
485+
urls,
486+
addresses,
454487
memberships: [
455488
...existingMemberships.filter(
456489
(m) =>

‎backend/test/unit/contact-export/googleContacts.test.ts‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,169 @@ describe('GoogleContactsSession', () => {
830830
expect(person?.phoneNumbers).toHaveLength(1);
831831
expect(person?.phoneNumbers?.[0]?.value).toBe('123456789');
832832
});
833+
834+
it('should replace existing name when updating (singleton behavior)', async () => {
835+
const existing: people_v1.Schema$Person = {
836+
...createMockPerson('people/c1', 'test@test.com'),
837+
names: [
838+
{
839+
givenName: 'Old',
840+
familyName: 'Name',
841+
unstructuredName: 'Old Name'
842+
}
843+
]
844+
};
845+
846+
mockService.people.connections.list.mockResolvedValue(
847+
createMockRes({ connections: [existing] })
848+
);
849+
850+
const incoming = createMockContact({
851+
email: 'test@test.com',
852+
given_name: 'New',
853+
family_name: 'Name',
854+
name: 'New Name'
855+
});
856+
857+
await session.run([incoming], false);
858+
859+
const callArgs = mockService.people.batchUpdateContacts.mock.calls[0];
860+
if (!callArgs) {
861+
throw new Error('Expected batchUpdateContacts to be called');
862+
}
863+
864+
const params = callArgs[0];
865+
const person = params.requestBody?.contacts?.['people/c1'];
866+
867+
// Should have only ONE name (singleton), not append
868+
expect(person?.names).toHaveLength(1);
869+
expect(person?.names?.[0]).toMatchObject({
870+
givenName: 'New',
871+
familyName: 'Name',
872+
unstructuredName: 'New Name'
873+
});
874+
});
875+
876+
it('should add phone numbers when updateEmptyOnly=true and no existing phones', async () => {
877+
const existing: people_v1.Schema$Person = {
878+
...createMockPerson('people/c1', 'test@test.com'),
879+
phoneNumbers: []
880+
};
881+
882+
mockService.people.connections.list.mockResolvedValue(
883+
createMockRes({ connections: [existing] })
884+
);
885+
886+
const incoming = createMockContact({
887+
email: 'test@test.com',
888+
telephone: ['123456789']
889+
});
890+
891+
await session.run([incoming], true);
892+
893+
const callArgs = mockService.people.batchUpdateContacts.mock.calls[0];
894+
if (!callArgs) {
895+
throw new Error('Expected batchUpdateContacts to be called');
896+
}
897+
898+
const params = callArgs[0];
899+
const person = params.requestBody?.contacts?.['people/c1'];
900+
901+
// Should add phone number even with updateEmptyOnly=true when no existing phones
902+
expect(person?.phoneNumbers).toHaveLength(1);
903+
expect(person?.phoneNumbers?.[0]?.value).toBe('123456789');
904+
});
905+
906+
it('should not create name entry with only null values', async () => {
907+
const contact = createMockContact({
908+
email: 'test@example.com',
909+
name: null as unknown as undefined,
910+
given_name: null as unknown as undefined,
911+
family_name: null as unknown as undefined
912+
});
913+
914+
await session.run([contact], false);
915+
916+
const callArgs = mockService.people.batchCreateContacts.mock.calls[0];
917+
if (!callArgs) {
918+
throw new Error('Expected batchCreateContacts to be called');
919+
}
920+
921+
const params = callArgs[0];
922+
const person = params.requestBody?.contacts?.[0]?.contactPerson;
923+
924+
// Should not have names array if all name fields are null/empty
925+
expect(person?.names).toHaveLength(0);
926+
});
927+
928+
it('should deduplicate URLs correctly', async () => {
929+
const existing: people_v1.Schema$Person = {
930+
...createMockPerson('people/c1', 'test@test.com'),
931+
urls: [{ value: 'https://example.com' }]
932+
};
933+
934+
mockService.people.connections.list.mockResolvedValue(
935+
createMockRes({ connections: [existing] })
936+
);
937+
938+
const incoming = createMockContact({
939+
email: 'test@test.com',
940+
same_as: ['https://example.com', 'https://newsite.com']
941+
});
942+
943+
await session.run([incoming], false);
944+
945+
const callArgs = mockService.people.batchUpdateContacts.mock.calls[0];
946+
if (!callArgs) {
947+
throw new Error('Expected batchUpdateContacts to be called');
948+
}
949+
950+
const params = callArgs[0];
951+
const person = params.requestBody?.contacts?.['people/c1'];
952+
953+
// Should have both URLs without duplication
954+
expect(person?.urls).toHaveLength(2);
955+
expect(person?.urls?.some((u) => u.value === 'https://example.com')).toBe(
956+
true
957+
);
958+
expect(person?.urls?.some((u) => u.value === 'https://newsite.com')).toBe(
959+
true
960+
);
961+
});
962+
963+
it('should deduplicate organizations correctly', async () => {
964+
const existing: people_v1.Schema$Person = {
965+
...createMockPerson('people/c1', 'test@test.com'),
966+
organizations: [{ name: 'ACME Corp', title: 'Engineer' }]
967+
};
968+
969+
mockService.people.connections.list.mockResolvedValue(
970+
createMockRes({ connections: [existing] })
971+
);
972+
973+
const incoming = createMockContact({
974+
email: 'test@test.com',
975+
works_for: 'ACME Corp',
976+
job_title: 'Engineer'
977+
});
978+
979+
await session.run([incoming], false);
980+
981+
const callArgs = mockService.people.batchUpdateContacts.mock.calls[0];
982+
if (!callArgs) {
983+
throw new Error('Expected batchUpdateContacts to be called');
984+
}
985+
986+
const params = callArgs[0];
987+
const person = params.requestBody?.contacts?.['people/c1'];
988+
989+
// Should have only one organization (deduplicated)
990+
expect(person?.organizations).toHaveLength(1);
991+
expect(person?.organizations?.[0]).toMatchObject({
992+
name: 'ACME Corp',
993+
title: 'Engineer'
994+
});
995+
});
833996
});
834997

835998
describe('System Safety & Pagination', () => {

0 commit comments

Comments
 (0)