fix(cozy-sharing): Normalize recipient id in dedup helper

Added id/_id normalization into mergeAndDeduplicateRecipients
to have correct deduplication of recipients
Added unit tests for SharedDrive helpers.

Author: doubleface

packages/cozy-sharing/src/components/SharedDrive/helpers.js

@@ -5,17 +5,34 @@ const ContactModel = models.contact
 export const RECIPIENT_INDEX_PREFIX = 'virtual-shared-drive-sharing-'
 
 export const mergeAndDeduplicateRecipients = arrays => {
-  const combinedArray = arrays.flat()
+  const combinedArray = arrays.flat().map(item => ({
+    ...item,
+    id: item.id || item._id
+  }))
 
   const seenIds = new Set()
 
-  const uniqueArray = combinedArray.filter(item => {
-    if (!seenIds.has(item.id)) {
-      seenIds.add(item.id)
-      return true
+  const uniqueArray = combinedArray.reduce((acc, item) => {
+    const id = item.id || item._id
+
+    if (id == null) {
+      // Recipients without any identifier are kept as-is to avoid silent data loss
+      acc.push(item)
+      return acc
     }
-    return false
-  })
+
+    if (!seenIds.has(id)) {
+      seenIds.add(id)
+      // Ensure both id and _id exist for downstream compatibility
+      const normalizedItem =
+        !item.id || !item._id
+          ? { ...item, id: item.id || item._id, _id: item._id || item.id }
+          : item
+      acc.push(normalizedItem)
+    }
+
+    return acc
+  }, [])
 
   return uniqueArray
 }

packages/cozy-sharing/src/components/SharedDrive/helpers.spec.js

@@ -0,0 +1,161 @@
+import {
+  mergeAndDeduplicateRecipients,
+  moveRecipientToReadWrite,
+  moveRecipientToReadOnly,
+  formatRecipients,
+  RECIPIENT_INDEX_PREFIX
+} from './helpers'
+
+describe('mergeAndDeduplicateRecipients', () => {
+  it('should merge multiple arrays into one', () => {
+    const result = mergeAndDeduplicateRecipients([
+      [{ id: '1', name: 'Alice' }],
+      [{ id: '2', name: 'Bob' }]
+    ])
+    expect(result).toEqual([
+      { id: '1', _id: '1', name: 'Alice' },
+      { id: '2', _id: '2', name: 'Bob' }
+    ])
+  })
+
+  it('should deduplicate recipients with the same id', () => {
+    const result = mergeAndDeduplicateRecipients([
+      [{ id: '1', name: 'Alice' }],
+      [{ id: '1', name: 'Alice' }]
+    ])
+    expect(result).toEqual([{ id: '1', _id: '1', name: 'Alice' }])
+  })
+
+  it('should normalize _id to id for deduplication', () => {
+    const result = mergeAndDeduplicateRecipients([
+      [{ _id: '1', name: 'Alice' }],
+      [{ _id: '2', name: 'Bob' }]
+    ])
+    expect(result).toEqual([
+      { _id: '1', id: '1', name: 'Alice' },
+      { _id: '2', id: '2', name: 'Bob' }
+    ])
+  })
+
+  it('should deduplicate recipients when one has id and the other has _id', () => {
+    const result = mergeAndDeduplicateRecipients([
+      [{ id: '1', name: 'Alice' }],
+      [{ _id: '1', name: 'Alice' }]
+    ])
+    expect(result).toEqual([{ id: '1', _id: '1', name: 'Alice' }])
+  })
+
+  it('should prefer existing id over _id when both are present', () => {
+    const result = mergeAndDeduplicateRecipients([
+      [{ id: 'real-id', _id: 'other-id', name: 'Alice' }]
+    ])
+    expect(result).toEqual([{ id: 'real-id', _id: 'other-id', name: 'Alice' }])
+  })
+
+  it('should handle empty arrays', () => {
+    const result = mergeAndDeduplicateRecipients([[], []])
+    expect(result).toEqual([])
+  })
+})
+
+describe('moveRecipientToReadWrite', () => {
+  it('should move a recipient from readOnly to readWrite', () => {
+    const state = {
+      recipients: [{ _id: '1', name: 'Alice' }],
+      readOnlyRecipients: [{ _id: '2', name: 'Bob' }]
+    }
+    const result = moveRecipientToReadWrite(state, '2')
+    expect(result.recipients).toEqual([
+      { _id: '1', name: 'Alice' },
+      { _id: '2', name: 'Bob' }
+    ])
+    expect(result.readOnlyRecipients).toEqual([])
+  })
+
+  it('should return unchanged state if recipient not found', () => {
+    const state = {
+      recipients: [{ _id: '1', name: 'Alice' }],
+      readOnlyRecipients: []
+    }
+    const result = moveRecipientToReadWrite(state, '999')
+    expect(result).toBe(state)
+  })
+})
+
+describe('moveRecipientToReadOnly', () => {
+  it('should move a recipient from readWrite to readOnly', () => {
+    const state = {
+      recipients: [{ _id: '1', name: 'Alice' }],
+      readOnlyRecipients: [{ _id: '2', name: 'Bob' }]
+    }
+    const result = moveRecipientToReadOnly(state, '1')
+    expect(result.recipients).toEqual([])
+    expect(result.readOnlyRecipients).toEqual([
+      { _id: '2', name: 'Bob' },
+      { _id: '1', name: 'Alice' }
+    ])
+  })
+
+  it('should return unchanged state if recipient not found', () => {
+    const state = {
+      recipients: [],
+      readOnlyRecipients: [{ _id: '1', name: 'Alice' }]
+    }
+    const result = moveRecipientToReadOnly(state, '999')
+    expect(result).toBe(state)
+  })
+})
+
+describe('formatRecipients', () => {
+  it('should format readWrite and readOnly recipients with correct types', () => {
+    const input = {
+      recipients: [
+        {
+          _id: '1',
+          displayName: 'Alice',
+          email: [{ address: 'alice@example.com', primary: true }]
+        }
+      ],
+      readOnlyRecipients: [
+        {
+          _id: '2',
+          displayName: 'Bob',
+          email: [{ address: 'bob@example.com', primary: true }]
+        }
+      ]
+    }
+    const result = formatRecipients(input)
+    expect(result).toHaveLength(2)
+    expect(result[0]).toMatchObject({
+      index: `${RECIPIENT_INDEX_PREFIX}1`,
+      type: 'two-way',
+      public_name: 'Alice'
+    })
+    expect(result[1]).toMatchObject({
+      index: `${RECIPIENT_INDEX_PREFIX}2`,
+      type: 'one-way',
+      public_name: 'Bob'
+    })
+  })
+
+  it('should sort recipients by public_name', () => {
+    const input = {
+      recipients: [
+        {
+          _id: '1',
+          displayName: 'Zara',
+          email: [{ address: 'zara@example.com', primary: true }]
+        },
+        {
+          _id: '2',
+          displayName: 'Alice',
+          email: [{ address: 'alice@example.com', primary: true }]
+        }
+      ],
+      readOnlyRecipients: []
+    }
+    const result = formatRecipients(input)
+    expect(result[0].public_name).toBe('Alice')
+    expect(result[1].public_name).toBe('Zara')
+  })
+})