[WIP] Remote changes order

core/remote/change.js

@@ -233,6 +233,8 @@ const isTrash = (a /*: RemoteChange */) /*: boolean %checks */ =>
   a.type === 'DirTrashing' || a.type === 'FileTrashing'
 const isRestore = (a /*: RemoteChange */) /*: boolean %checks */ =>
   a.type === 'DirRestoration' || a.type === 'FileRestoration'
+const isIgnored = (a /*: RemoteChange */) /*: boolean %checks */ =>
+  a.type === 'IgnoredChange'
 
 function includeDescendant(
   parent /*: RemoteDirMove */,
@@ -260,6 +262,10 @@ const aFirst = -1
 const bFirst = 1
 
 const sorter = (a, b) => {
+  if (isIgnored(a) && !isIgnored(b)) return bFirst
+  if (isIgnored(b) && !isIgnored(a)) return aFirst
+  if (isIgnored(a) && isIgnored(b)) return 0
+
   if (areParentChild(createdId(a), deletedId(b))) return aFirst
   if (areParentChild(createdId(b), deletedId(a))) return bFirst
 
@@ -280,7 +286,12 @@ const sorter = (a, b) => {
 
   // if there isnt 2 add paths, sort by del path
   if (lower(deletedId(b), deletedId(a))) return aFirst
-  return bFirst
+  if (lower(deletedId(a), deletedId(b))) return bFirst
+
+  if (lower(createdId(a), deletedId(b))) return bFirst
+  if (lower(deletedId(a), createdId(b))) return aFirst
+
+  return 0
 }
 
 function sort(changes /*: Array<RemoteChange> */) /*: void */ {

test/integration/move.js

@@ -177,9 +177,8 @@ describe('Move', () => {
 
     it('local', async () => {
       const oldFolder = await pouch.byRemoteIdMaybeAsync(dir._id)
-      // FIXME: Why is this a file? And why does it break with a directory?
       const doc = builders
-        .metafile()
+        .metadir()
         .path('parent/dst/dir')
         .build()
 
@@ -228,17 +227,23 @@ describe('Move', () => {
       await cozy.files.updateAttributesById(dir._id, { dir_id: dst._id })
       await helpers.remote.pullChanges()
 
-      /* FIXME: Nondeterministic
-      should(helpers.putDocs('path', '_deleted', 'trashed')).deepEqual([
-        {path: 'parent/src/dir/subdir/file', _deleted: true},
-        {path: 'parent/src/dir/subdir', _deleted: true},
-        {path: 'parent/src/dir/empty-subdir', _deleted: true},
-        {path: 'parent/src/dir', _deleted: true},
-        {path: 'parent/dst/dir'},
-        {path: 'parent/dst/dir/subdir'},
-        {path: 'parent/dst/dir/empty-subdir'}
+      should(
+        helpers.putDocs('path', '_deleted', 'trashed', 'childMove')
+      ).deepEqual([
+        { path: 'parent/src/dir', _deleted: true },
+        { path: 'parent/dst/dir' },
+        {
+          path: 'parent/src/dir/empty-subdir',
+          _deleted: true,
+          childMove: true
+        },
+        { path: 'parent/dst/dir/empty-subdir' },
+        { path: 'parent/src/dir/subdir', _deleted: true, childMove: true },
+        { path: 'parent/dst/dir/subdir' },
+        { path: 'parent/src/dir/subdir/file', _deleted: true, childMove: true },
+        { path: 'parent/dst/dir/subdir/file' }
       ])
-      */
+
       await helpers.syncAll()
       should(await helpers.local.tree()).deepEqual([
         'parent/',
@@ -257,91 +262,47 @@ describe('Move', () => {
     })
 
     it('from remote client', async () => {
-      // FIXME: Ensure events occur in the same order as resulting from the
-      // local dir test
-      await helpers._remote.addFolderAsync(
-        _.defaults(
-          {
-            path: path.normalize('parent/dst/dir'),
-            updated_at: '2017-06-20T12:58:49.681Z'
-          },
-          await pouch.byRemoteIdAsync(dir._id)
-        )
-      )
-      await helpers._remote.addFolderAsync(
+      const oldFolderMetadata = await pouch.byRemoteIdAsync(dir._id)
+      await helpers._remote.moveFolderAsync(
         _.defaults(
           {
-            path: path.normalize('parent/dst/dir/empty-subdir'),
-            updated_at: '2017-06-20T12:58:49.817Z'
+            path: path.normalize('parent/dst/dir')
           },
-          await pouch.byRemoteIdAsync(emptySubdir._id)
-        )
-      )
-      await helpers._remote.addFolderAsync(
-        _.defaults(
-          {
-            path: path.normalize('parent/dst/dir/subdir'),
-            updated_at: '2017-06-20T12:58:49.873Z'
-          },
-          await pouch.byRemoteIdAsync(subdir._id)
-        )
-      )
-      const oldFileMetadata = await pouch.byRemoteIdAsync(file._id)
-      await helpers._remote.moveFileAsync(
-        _.defaults(
-          {
-            path: path.normalize('parent/dst/dir/subdir/file')
-            // FIXME: Why does moveFileAsync({updated_at: ...}) fail?
-            // updated_at: '2017-06-20T12:58:49.921Z'
-          },
-          oldFileMetadata
+          oldFolderMetadata
         ),
-        oldFileMetadata
-      )
-      const oldSubdirMetadata = await pouch.byRemoteIdAsync(subdir._id)
-      await helpers._remote.deleteFolderAsync(oldSubdirMetadata)
-      const oldEmptySubdirMetadata = await pouch.byRemoteIdAsync(
-        emptySubdir._id
+        oldFolderMetadata
       )
-      await helpers._remote.deleteFolderAsync(oldEmptySubdirMetadata)
-      const oldDirMetadata = await pouch.byRemoteIdAsync(dir._id)
-      await helpers._remote.deleteFolderAsync(oldDirMetadata)
 
       await helpers.remote.pullChanges()
 
-      /* FIXME: Nondeterministic
-      should(helpers.putDocs('path', '_deleted', 'trashed')).deepEqual([
-        // file 1/2
-        {path: path.normalize('parent/src/dir/subdir/file'), _deleted: true},
-        // file 2/2
-        {path: path.normalize('parent/dst/dir/subdir/file')},
-        // dir 2/2
-        {path: path.normalize('parent/dst/dir')},
-        // empty-subdir 2/2
-        {path: path.normalize('parent/dst/dir/empty-subdir')},
-        // subdir 2/3
-        {path: path.normalize('parent/dst/dir/subdir')},
-        // subdir 1/3
-        {path: path.normalize('parent/src/dir/subdir'), _deleted: true},
-        {path: path.normalize('parent/src/dir/subdir'), trashed: true},
-        // empty-subdir 1/2
-        {path: path.normalize('parent/src/dir/empty-subdir'), _deleted: true},
-        {path: path.normalize('parent/src/dir/empty-subdir'), trashed: true},
-        // dir 1/2
-        {path: path.normalize('parent/src/dir'), _deleted: true},
-        {path: path.normalize('parent/src/dir'), trashed: true}
+      should(
+        helpers.putDocs('path', '_deleted', 'trashed', 'childMove')
+      ).deepEqual([
+        { path: path.normalize('parent/src/dir'), _deleted: true },
+        { path: path.normalize('parent/dst/dir') },
+        {
+          path: path.normalize('parent/src/dir/empty-subdir'),
+          _deleted: true,
+          childMove: true
+        },
+        { path: path.normalize('parent/dst/dir/empty-subdir') },
+        {
+          path: path.normalize('parent/src/dir/subdir'),
+          _deleted: true,
+          childMove: true
+        },
+        { path: path.normalize('parent/dst/dir/subdir') },
+        {
+          path: path.normalize('parent/src/dir/subdir/file'),
+          _deleted: true,
+          childMove: true
+        },
+        { path: path.normalize('parent/dst/dir/subdir/file') }
       ])
-      */
 
       await helpers.syncAll()
 
-      should(
-        (await helpers.local.tree())
-          // FIXME: Sometimes a copy of the file ends up in the OS trash.
-          // Issue was already occurring from time to time, even before we start
-          // to permanently delete empty dirs.
-          .filter(path => path !== '/Trash/file')
-      ).deepEqual([
+      should(await helpers.local.tree()).deepEqual([
         'parent/',
         'parent/dst/',
         'parent/dst/dir/',

test/unit/remote/change.js

@@ -6,7 +6,7 @@ describe('remote change sort', () => {
   it('sort correctly move inside move', () => {
     const parent = {
       doc: { path: 'parent/dst/dir' },
-      type: 'FolderMove',
+      type: 'DirMove',
       was: { path: 'parent/src/dir' }
     }
     const child = {
@@ -41,5 +41,129 @@ describe('remote change sort', () => {
       remoteChange.sort(changes)
       changes.should.deepEqual([deleted, created])
     })
+
+    it('even with other changes', () => {
+      const netflixBillAddition = {
+        doc: {
+          path: 'Administratif/Netflix/email_2/2019-05-06_12,34.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const edfContract1ConflictCreation = {
+        doc: {
+          path:
+            'Administratif/EDF/email_1/Address 1/attestation de contrat-conflict-2019-05-06T12_34_56.012Z.pdf'
+        },
+        was: {
+          path: 'Administratif/EDF/email_1/Address 1/attestation de contrat.pdf'
+        },
+        type: 'FileMove'
+      }
+      const edfContract2ConflictCreation = {
+        doc: {
+          path:
+            'Administratif/EDF/email_1/Address 3/attestation de contrat-conflict-2019-05-06T12_34_56.345Z.pdf'
+        },
+        was: {
+          path: 'Administratif/EDF/email_1/Address 3/attestation de contrat.pdf'
+        },
+        type: 'FileMove'
+      }
+      const edfContract3Deletion = {
+        doc: {
+          path: 'Administratif/EDF/email_2/Address 2/attestation de contrat.pdf'
+        },
+        was: {
+          path: 'Administratif/EDF/email_2/Address 2/attestation de contrat.pdf'
+        },
+        type: 'FileDeletion'
+      }
+      const edfContract3Addition = {
+        doc: {
+          path: 'Administratif/EDF/email_2/Address 2/attestation de contrat.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const edfContract2Addition = {
+        doc: {
+          path: 'Administratif/EDF/email_1/Address 3/attestation de contrat.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const edfContract1Addition = {
+        doc: {
+          path: 'Administratif/EDF/email_1/Address 1/attestation de contrat.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const digipostBouyguesBill = {
+        doc: {
+          path:
+            'Administratif/Digiposte/email_2/Bouygues Telecom - Factures/Facture_2019-05-06.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const alanInsuranceCardDeletion = {
+        doc: {
+          path: 'Administratif/Alan/email_2/Carte_Mutuelle.pdf'
+        },
+        was: {
+          path: 'Administratif/Alan/email_2/Carte_Mutuelle.pdf'
+        },
+        type: 'FileDeletion'
+      }
+      const alanInsuranceCardAddition = {
+        doc: {
+          path: 'Administratif/Alan/email_2/Carte_Mutuelle.pdf'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+      const photoAddition = {
+        doc: {
+          path: 'Photos/Sauvegardées depuis mon mobile/20190506_123456.jpg'
+        },
+        was: null,
+        type: 'FileAddition'
+      }
+
+      const changes = [
+        netflixBillAddition,
+        edfContract1ConflictCreation,
+        edfContract2ConflictCreation,
+        edfContract3Deletion,
+        edfContract3Addition,
+        edfContract2Addition,
+        edfContract1Addition,
+        digipostBouyguesBill,
+        alanInsuranceCardDeletion,
+        alanInsuranceCardAddition,
+        photoAddition
+      ]
+      remoteChange.sort(changes)
+
+      // Sort order:
+      // - alphabetical order
+      // - conflicts before anything else on same id
+      // - deletion before addition on same id
+      changes.should.deepEqual([
+        edfContract3Deletion,
+        digipostBouyguesBill,
+        edfContract1ConflictCreation,
+        edfContract1Addition,
+        edfContract2ConflictCreation,
+        edfContract2Addition,
+        edfContract3Addition,
+        alanInsuranceCardDeletion,
+        alanInsuranceCardAddition,
+        netflixBillAddition,
+        photoAddition
+      ])
+    })
   })
 })