[DEV-2064] Refactor resync error handler (#1285)

* sync user wip * sync user * changeset * Update .changeset/cuddly-cats-retire.md Co-authored-by: Marco Ponchia <[email protected]> * Update packages/active-campaign-client/src/index.ts Co-authored-by: Marco Ponchia <[email protected]> * Update packages/active-campaign-client/src/index.ts Co-authored-by: Marco Ponchia <[email protected]> * Update packages/active-campaign-client/src/helpers/resyncUser.ts Co-authored-by: Marco Ponchia <[email protected]> * pr comments * bulk add contact * bulk add contact * resync user * changeset * Update packages/active-campaign-client/src/clients/activeCampaignClient.ts Co-authored-by: Marco Ponchia <[email protected]> * Update packages/active-campaign-client/src/handlers/resyncUserHandler.ts Co-authored-by: Marco Ponchia <[email protected]> * pr changes * Update packages/active-campaign-client/src/helpers/fetchSubscribedWebinarsFromDynamo.ts Co-authored-by: Marco Ponchia <[email protected]> * Update packages/active-campaign-client/.env.example Co-authored-by: marcobottaro <[email protected]> * Update packages/active-campaign-client/src/handlers/resyncUserHandler.ts Co-authored-by: Marco Ponchia <[email protected]> * pr changes * add makeContactPayload * Add types * refactor AC client * add getListByName return type * refactor getListIdByName and add error manage to sqsQueueHandler * Add addOrUpdateContact helper * add getNewWebinarsAndUnsubsriptionLists * add addArrayOfListToContact and removeArrayOfListFromContact helpers * Update resync user handlers * Fix manageError * Fix after merge * add changeset * Fix list ac client call * Refactor env var for test * fix make payload * fix resync function * Apply suggestions from code review Co-authored-by: marcobottaro <[email protected]> * Fix after review * Apply suggestions from code review Co-authored-by: marcobottaro <[email protected]> --------- Co-authored-by: t <[email protected]> Co-authored-by: tommaso1 <[email protected]> Co-authored-by: marcobottaro <[email protected]>
pagopa · Dec 20, 2024 · 3f202f7 · 3f202f7
1 parent fb6d962
commit 3f202f7
Show file tree

Hide file tree

Showing 17 changed files with 344 additions and 139 deletions.
diff --git a/.changeset/khaki-knives-unite.md b/.changeset/khaki-knives-unite.md
@@ -0,0 +1,5 @@
+---
+"active-campaign-client": minor
+---
+
+Refactor resyncUserHandler to align contacts and subscriptions in Active Campaign
diff --git a/packages/active-campaign-client/.env.example b/packages/active-campaign-client/.env.example
@@ -1,10 +1,11 @@
-AC_BASE_URL=your_account_url
-AC_API_KEY=your_api_key
 SENDER_URL=localhost:3000
 AWS_REGION="region"
-AWS_USER_POOL_ID="region_DFWF81fRa"
-COGNITO_USER_ID=66ae52a0-f051-7080-04a1-465b3a4f44cc
-LIST_NAME=test-webinar-1732097286071
+COGNITO_USER_POOL_ID="your_region"
 AC_BASE_URL_PARAM='/ac/base_url'
 AC_API_KEY_PARAM='/ac/api_key'
+
+TEST_AC_BASE_URL=your_account_url
+TEST_AC_API_KEY=your_api_key
 TEST_AC_LIST_ID=28
+TEST_COGNITO_USER_ID=66ae52a0-f051-7080-04a1-465b3a4f44cc
+TEST_LIST_NAME=test-webinar-1732097286071
diff --git a/packages/active-campaign-client/src/__tests__/helpers/manageListSubscription.test.ts b/packages/active-campaign-client/src/__tests__/helpers/manageListSubscription.test.ts
@@ -1,12 +1,12 @@
 // remove .skip to run the test, be aware it does real API calls
 import {
   addContactToList,
-  removeContactToList,
+  removeContactFromList,
 } from '../../helpers/manageListSubscription';
 
 describe.skip('manage list subscription', () => {
-  const cognitoUserId = process.env.COGNITO_USER_ID || '';
-  const listName = process.env.LIST_NAME || '';
+  const cognitoUserId = process.env.TEST_COGNITO_USER_ID || '';
+  const listName = process.env.TEST_LIST_NAME || '';
 
   it('should subscribe the contact to the list', async () => {
     const result = await addContactToList(cognitoUserId, listName);
@@ -15,7 +15,7 @@ describe.skip('manage list subscription', () => {
   });
 
   it('should unsubscribe the contact from the list', async () => {
-    const result = await removeContactToList(cognitoUserId, listName);
+    const result = await removeContactFromList(cognitoUserId, listName);
     expect(result.statusCode).toBe(200);
   });
 });
diff --git a/packages/active-campaign-client/src/clients/activeCampaignClient.ts b/packages/active-campaign-client/src/clients/activeCampaignClient.ts
@@ -4,6 +4,13 @@ import { ContactPayload } from '../types/contactPayload';
 import { ListPayload } from '../types/listPayload';
 import { ListStatusPayload } from '../types/listStatusPayload';
 import { BulkAddContactPayload } from '../types/bulkAddContactPayload';
+import {
+  ContactResponse,
+  ContactResponseWithLists,
+} from '../types/contactResponse';
+import { ActiveCampaignList } from '../types/activeCampaignList';
+
+const MAX_NUMBER_OF_LISTS = '1000';
 
 async function getParameter(
   paramName: string,
@@ -52,8 +59,9 @@ export class ActiveCampaignClient {
     params?: Record<string, string>
   ): Promise<T> {
     const [apiKey, baseUrl] = await Promise.all([
-      getParameter(this.apiKeyParam, this.ssm, process.env.AC_API_KEY),
-      getParameter(this.baseUrlParam, this.ssm, process.env.AC_BASE_URL),
+      // Fallback env variable exists only for manual testing purposes
+      getParameter(this.apiKeyParam, this.ssm, process.env.TEST_AC_API_KEY),
+      getParameter(this.baseUrlParam, this.ssm, process.env.TEST_AC_BASE_URL),
     ]);
     return new Promise((resolve, reject) => {
       // Parse the base URL to get hostname and path and remove any trailing slashes from the baseUrl
@@ -109,29 +117,42 @@ export class ActiveCampaignClient {
   }
 
   async createContact(data: ContactPayload) {
-    return this.makeRequest('POST', '/api/3/contacts', data);
+    return this.makeRequest<ContactResponse>('POST', '/api/3/contacts', data);
   }
 
   async updateContact(contactId: string, data: ContactPayload) {
-    return this.makeRequest('PUT', `/api/3/contacts/${contactId}`, data);
+    return this.makeRequest<ContactResponse>(
+      'PUT',
+      `/api/3/contacts/${contactId}`,
+      data
+    );
   }
 
   async deleteContact(contactId: string) {
     return this.makeRequest('DELETE', `/api/3/contacts/${contactId}`);
   }
 
-  async getContactByCognitoId(cognitoId: string) {
+  async getContactByCognitoUsername(cognitoUsername: string) {
     const response = await this.makeRequest<{
       readonly contacts: ReadonlyArray<{ readonly id: string }>;
-    }>('GET', '/api/3/contacts', undefined, { phone: `cognito:${cognitoId}` });
+    }>('GET', '/api/3/contacts', undefined, {
+      phone: `cognito:${cognitoUsername}`,
+    });
     return response?.contacts?.[0]?.id;
   }
 
+  async getContact(id: string) {
+    return await this.makeRequest<ContactResponseWithLists>(
+      'GET',
+      `/api/3/contacts/${id}`
+    );
+  }
+
   async createList(data: ListPayload) {
     return this.makeRequest('POST', '/api/3/lists', data);
   }
 
-  async getListIdByName(name: string) {
+  async getListIdByName(name: string): Promise<number | undefined> {
     const response = await this.makeRequest<{
       readonly lists: ReadonlyArray<{ readonly id: number }>;
     }>('GET', '/api/3/lists', undefined, { 'filters[name][eq]': name });
@@ -142,6 +163,18 @@ export class ActiveCampaignClient {
     return this.makeRequest('DELETE', `/api/3/lists/${id}`);
   }
 
+  async getLists(ids?: readonly string[]) {
+    const limitParams = { limit: MAX_NUMBER_OF_LISTS };
+    return this.makeRequest<{ readonly lists: readonly ActiveCampaignList[] }>(
+      'GET',
+      '/api/3/lists',
+      undefined,
+      ids && ids.length > 0
+        ? { ids: ids.join(','), ...limitParams }
+        : limitParams
+    );
+  }
+
   async bulkAddContactToList(
     contacts: readonly (ContactPayload & {
       readonly listIds: readonly number[];

diff --git a/packages/active-campaign-client/src/handlers/resyncUserHandler.ts b/packages/active-campaign-client/src/handlers/resyncUserHandler.ts
@@ -1,74 +1,65 @@
 import { deleteContact } from '../helpers/deleteContact';
 import { getUserFromCognitoUsername } from '../helpers/getUserFromCognito';
-import { getSubscribedWebinars } from '../helpers/getSubscribedWebinars';
-import { addContact } from '../helpers/addContact';
 import { APIGatewayProxyResult, SQSEvent } from 'aws-lambda';
-import { addContactToList } from '../helpers/manageListSubscription';
 import { queueEventParser } from '../helpers/queueEventParser';
+import { addOrUpdateContact } from '../helpers/addOrUpdateContact';
+import { getNewWebinarsAndUnsubsriptionLists } from '../helpers/getNewWebinarsAndUnsubsriptionLists';
+import { addArrayOfListToContact } from '../helpers/addArrayOfListToContact';
+import { removeArrayOfListFromContact } from '../helpers/removeArrayOfListFromContact';
 
 export async function resyncUserHandler(event: {
   readonly Records: SQSEvent['Records'];
 }): Promise<APIGatewayProxyResult> {
   try {
+    console.log('resyncUserHandler: Event:', event); // TODO: Remove after testing
     const queueEvent = queueEventParser(event);
     const cognitoUsername = queueEvent.detail.additionalEventData.sub;
-    const deletionResult = await deleteContact(cognitoUsername);
-    if (
-      deletionResult.statusCode !== 200 &&
-      deletionResult.statusCode !== 404
-    ) {
-      // eslint-disable-next-line functional/no-throw-statements
-      throw new Error('Error adding contact');
-    }
 
     const user = await getUserFromCognitoUsername(cognitoUsername);
 
+    console.log('user:', user); // TODO: Remove after testing
+
     if (!user) {
-      console.log(
-        `User: ${cognitoUsername} not present on Cognito, sync done.`
-      );
-      return {
-        statusCode: 200,
-        body: JSON.stringify({
-          message: 'User not present on Cognito, sync done.',
-        }),
-      };
-    }
+      const deletionResult = await deleteContact(cognitoUsername);
+      if (
+        deletionResult.statusCode !== 200 &&
+        deletionResult.statusCode !== 404
+      ) {
+        // eslint-disable-next-line functional/no-throw-statements
+        throw new Error('Error deleting contact');
+      }
+    } else {
+      const contactResponse = await addOrUpdateContact(user);
 
-    const userWebinarsSubscriptions = await getSubscribedWebinars(
-      cognitoUsername
-    );
+      console.log('contactResponse:', contactResponse); // TODO: Remove after testing
 
-    const webinarIds = JSON.parse(userWebinarsSubscriptions.body)
-      .map(
-        (webinar: { readonly webinarId: { readonly S: string } }) =>
-          webinar?.webinarId?.S
-      )
-      .filter(Boolean);
+      const { listsToUnsubscribe, newWebinarSlugs } =
+        await getNewWebinarsAndUnsubsriptionLists(
+          contactResponse,
+          cognitoUsername
+        );
 
-    const res = await addContact(user);
-    if (res.statusCode !== 200) {
-      // eslint-disable-next-line functional/no-throw-statements
-      throw new Error('Error adding contact');
-    }
+      const resyncTimeoutMilliseconds: number = parseInt(
+        process.env.AC_RESYNC_TIMEOUT_IN_MS || '1000'
+      );
 
-    await webinarIds.reduce(
-      async (
-        prevPromise: Promise<APIGatewayProxyResult>,
-        webinarId: string
-      ) => {
-        await prevPromise;
-        try {
-          const result = await addContactToList(cognitoUsername, webinarId);
-          console.log('Add contact to list result:', result, webinarId); // TODO: Remove after testing
-          await new Promise((resolve) => setTimeout(resolve, 1000)); // wait 1 sec to avoid rate limiting
-        } catch (e) {
-          console.error('Error adding contact to list', e); // TODO: Remove after testing
-        }
-      },
-      Promise.resolve()
-    );
+      const subscriptionsresult = await addArrayOfListToContact({
+        webinarSlugs: newWebinarSlugs,
+        cognitoUsername: cognitoUsername,
+        resyncTimeoutMilliseconds,
+      });
 
+      const unsubscriptionsResult = await removeArrayOfListFromContact({
+        listsToUnsubscribe,
+        contactId: contactResponse.contact.id,
+        resyncTimeoutMilliseconds,
+      });
+
+      if (!subscriptionsresult || !unsubscriptionsResult) {
+        // eslint-disable-next-line functional/no-throw-statements
+        throw new Error('Error managing list subscriptions');
+      }
+    }
     return {
       statusCode: 200,
       body: JSON.stringify({ message: 'User resynced' }),

diff --git a/packages/active-campaign-client/src/handlers/sqsQueueHandler.ts b/packages/active-campaign-client/src/handlers/sqsQueueHandler.ts
@@ -6,9 +6,21 @@ import { deleteContact } from '../helpers/deleteContact';
 import { queueEventParser } from '../helpers/queueEventParser';
 import {
   addContactToList,
-  removeContactToList,
+  removeContactFromList,
 } from '../helpers/manageListSubscription';
 
+function manageError(result: APIGatewayProxyResult) {
+  if (result.statusCode === 500) {
+    // eslint-disable-next-line functional/no-throw-statements
+    throw new Error('Internal server error');
+  }
+
+  return {
+    statusCode: 200,
+    body: JSON.stringify(result),
+  };
+}
+
 export async function sqsQueueHandler(event: {
   readonly Records: SQSEvent['Records'];
 }): Promise<APIGatewayProxyResult> {
@@ -17,20 +29,30 @@ export async function sqsQueueHandler(event: {
     const queueEvent = queueEventParser(event);
     switch (queueEvent.detail.eventName) {
       case 'ConfirmSignUp':
-        return await addContact(await getUserFromCognito(queueEvent));
+        return manageError(
+          await addContact(await getUserFromCognito(queueEvent))
+        );
       case 'UpdateUserAttributes':
-        return await updateContact(await getUserFromCognito(queueEvent));
+        return manageError(
+          await updateContact(await getUserFromCognito(queueEvent))
+        );
       case 'DeleteUser':
-        return await deleteContact(queueEvent.detail.additionalEventData.sub);
+        return manageError(
+          await deleteContact(queueEvent.detail.additionalEventData.sub)
+        );
       case 'DynamoINSERT':
-        return await addContactToList(
-          queueEvent.detail.additionalEventData.sub,
-          queueEvent.webinarId || ''
+        return manageError(
+          await addContactToList(
+            queueEvent.detail.additionalEventData.sub,
+            queueEvent.webinarId || ''
+          )
         );
       case 'DynamoREMOVE':
-        return await removeContactToList(
-          queueEvent.detail.additionalEventData.sub,
-          queueEvent.webinarId || ''
+        return manageError(
+          await removeContactFromList(
+            queueEvent.detail.additionalEventData.sub,
+            queueEvent.webinarId || ''
+          )
         );
       default:
         // eslint-disable-next-line functional/no-throw-statements

diff --git a/packages/active-campaign-client/src/helpers/addArrayOfListToContact.ts b/packages/active-campaign-client/src/helpers/addArrayOfListToContact.ts
@@ -0,0 +1,36 @@
+import { addContactToList } from './manageListSubscription';
+
+export async function addArrayOfListToContact(event: {
+  readonly webinarSlugs: ReadonlyArray<string>;
+  readonly cognitoUsername: string;
+  readonly resyncTimeoutMilliseconds: number;
+}) {
+  const { webinarSlugs, cognitoUsername, resyncTimeoutMilliseconds } = event;
+  // eslint-disable-next-line functional/prefer-readonly-type
+  const subscriptionsWithErrors: string[] = [];
+  await webinarSlugs.reduce(
+    async (prevPromise: Promise<void>, webinarSlug: string) => {
+      await prevPromise;
+      try {
+        const result = await addContactToList(cognitoUsername, webinarSlug);
+        console.log('Add contact to list result:', result, webinarSlug); // TODO: Remove after testing
+        await new Promise((resolve) =>
+          setTimeout(resolve, resyncTimeoutMilliseconds)
+        ); // wait to avoid rate limiting
+      } catch (e) {
+        subscriptionsWithErrors.push(webinarSlug);
+      }
+    },
+    Promise.resolve()
+  );
+
+  if (subscriptionsWithErrors.length > 0) {
+    console.error(
+      'Error adding contact to list',
+      subscriptionsWithErrors.join(',')
+    );
+    return false;
+  }
+
+  return true;
+}
diff --git a/packages/active-campaign-client/src/helpers/addContact.ts b/packages/active-campaign-client/src/helpers/addContact.ts
@@ -1,34 +1,11 @@
 import { APIGatewayProxyResult } from 'aws-lambda';
 import { acClient } from '../clients/activeCampaignClient';
-import { ContactPayload } from '../types/contactPayload';
 import { User } from '../types/user';
+import { makeContactPayload } from './makeContactPayload';
 
 export async function addContact(user: User): Promise<APIGatewayProxyResult> {
   try {
-    // Transform to AC payload
-    const acPayload: ContactPayload = {
-      contact: {
-        email: user.email,
-        firstName: user.given_name,
-        lastName: user.family_name,
-        phone: `cognito:${user.username}`,
-        fieldValues: [
-          {
-            field: '2',
-            value: user['custom:company_type'],
-          },
-          {
-            field: '1',
-            value: user['custom:job_role'],
-          },
-          {
-            field: '3',
-            value:
-              user['custom:mailinglist_accepted'] === 'true' ? 'TRUE' : 'FALSE',
-          },
-        ],
-      },
-    };
+    const acPayload = makeContactPayload(user);
 
     const response = await acClient.createContact(acPayload);