Edge case of merge members with similar activities resulting 500 error (

#388)
CrowdDotDev · Jan 5, 2023 · 3b54b93 · 3b54b93
1 parent fd7575b
commit 3b54b93
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 45 deletions.
diff --git a/backend/src/services/__tests__/memberService.test.ts b/backend/src/services/__tests__/memberService.test.ts
@@ -1765,7 +1765,7 @@ describe('MemberService tests', () => {
       expect(found).toStrictEqual(memberCreated)
     })
 
-    it('Should not duplicate activities - by timestamp', async () => {
+    it('Should not duplicate activities - by sourceId', async () => {
       const mockIRepositoryOptions = await SequelizeTestUtils.getTestIRepositoryOptions(db)
       const mas = new MemberAttributeSettingsService(mockIRepositoryOptions)
 
@@ -1792,18 +1792,8 @@ describe('MemberService tests', () => {
         sourceId: '#sourceId1',
       }
 
-      const aRepeated = {
-        timestamp: '2021-06-27T15:14:30Z',
-        type: 'activity',
-        member: createdMember.id,
-        platform: PlatformType.GITHUB,
-        sourceId: '#sourceId1',
-      }
-
       const a1Created = await ActivityRepository.create(a1, mockIRepositoryOptions)
 
-      const aCreatedRepeated1 = await ActivityRepository.create(aRepeated, mockIRepositoryOptions)
-
       const member2 = {
         username: {
           [PlatformType.GITHUB]: 'anil',
@@ -1814,10 +1804,9 @@ describe('MemberService tests', () => {
 
       const createdMember2 = await MemberRepository.create(member2, mockIRepositoryOptions)
 
-      aRepeated.member = createdMember2.id
-      aRepeated.sourceId = '#sourceId2'
+      a1.member = createdMember2.id
 
-      await ActivityRepository.create(aRepeated, mockIRepositoryOptions)
+      await ActivityRepository.create(a1, mockIRepositoryOptions)
 
       const a3Created = await ActivityRepository.create(
         {
@@ -1830,36 +1819,14 @@ describe('MemberService tests', () => {
         mockIRepositoryOptions,
       )
 
-      const foundActivities = await ActivityRepository.findAndCountAll(
-        {
-          filter: {
-            timestamp: aRepeated.timestamp,
-            type: aRepeated.type,
-            platform: aRepeated.platform,
-          },
-        },
-        mockIRepositoryOptions,
-      )
-
-      // Making sure the activity is indeed repeated
-      expect({
-        timestamp: foundActivities.rows[0].timestamp,
-        type: foundActivities.rows[0].type,
-        platform: foundActivities.rows[0].platform,
-      }).toStrictEqual({
-        timestamp: foundActivities.rows[1].timestamp,
-        type: foundActivities.rows[1].type,
-        platform: foundActivities.rows[1].platform,
-      })
-
       // Merge
       await memberService.merge(createdMember.id, createdMember2.id)
 
       const foundMergedActivities = (await memberService.findById(createdMember.id)).activities
         .map((a) => a.get({ plain: true }).id)
         .sort()
 
-      const expected = [aCreatedRepeated1.id, a1Created.id, a3Created.id].sort()
+      const expected = [a1Created.id, a3Created.id].sort()
       expect(foundMergedActivities).toStrictEqual(expected)
     })
 

diff --git a/backend/src/services/memberService.ts b/backend/src/services/memberService.ts
@@ -451,16 +451,11 @@ export default class MemberService extends LoggingBase {
         return oldEmail
       },
       // Get rid of activities that are the same and were in both members
-      activities: (oldActivities, newActivities) => {
-        oldActivities = oldActivities || []
+      activities: (_oldActivities, newActivities) => {
         newActivities = newActivities || []
         // A member cannot 2 different activities with same timestamp and platform and type
-        const uniq = lodash.uniqWith(
-          [...oldActivities, ...newActivities],
-          (act1, act2) =>
-            moment(act1.timestamp).utc().unix() === moment(act2.timestamp).utc().unix() &&
-            act1.platform === act2.platform,
-        )
+        const uniq = lodash.uniqWith(newActivities, (act1, act2) => act1.sourceId === act2.sourceId)
+
         return uniq.length > 0 ? uniq : null
       },
     })