Allow adding people back into a group

index.js

@@ -119,47 +119,50 @@ function init (ssb, config) {
   }
 
   pull(
-    listen.addMember(ssb),
+    // combined listeners so we process them in a deterministic order on client restarts
+    listen.mutateMembers(ssb),
     pull.asyncMap((m, cb) => {
-      const { root, groupKey } = m.value.content
-      ssb.get({ id: root, meta: true }, (err, groupInitMsg) => {
-        if (err) throw err
-
-        const groupId = buildGroupId({ groupInitMsg, groupKey })
-        const authors = unique([
-          groupInitMsg.value.author,
-          m.value.author,
-          ...m.value.content.recps.filter(isFeed)
-        ])
-
-        const record = keystore.group.get(groupId)
-        // if we haven't been in the group since before, register the group
-        if (record == null) {
-          return keystore.group.add(groupId, { key: groupKey, root }, (err) => {
-            if (err) return cb(err)
+      if (m.value.content.type === 'group/add-member') {
+        const { root, groupKey, recps } = m.value.content
+        const membersAdded = recps.slice(1)
+
+        ssb.get({ id: root, meta: true }, (err, groupInitMsg) => {
+          if (err) throw err
+
+          const groupId = buildGroupId({ groupInitMsg, groupKey })
+          const authors = unique([
+            groupInitMsg.value.author,
+            m.value.author,
+            ...m.value.content.recps.filter(isFeed)
+          ])
+
+          //const record = keystore.group.get(groupId)
+          //const inGroup = record && record.excluded === undefined
+
+          if (membersAdded.includes(ssb.id)){// && !inGroup) {
+            return keystore.group.add(groupId, { key: groupKey, root }, (err) => {
+              if (err) return cb(err)
+              processAuthors(groupId, authors, m.value.author, cb)
+            })
+          } else {
             processAuthors(groupId, authors, m.value.author, cb)
-          })
+          }
+        })
+      } else {
+        // if exclude member msg
+
+        const excludes = m.value.content.excludes
+        const groupId = m.value.content.recps[0]
+
+        if (excludes.includes(ssb.id)) {
+          keystore.group.exclude(groupId, cb)
         } else {
-          processAuthors(groupId, authors, m.value.author, cb)
+          cb()
         }
-      })
+      }
     }),
     pull.drain(() => {}, (err) => {
-      if (err) console.error('Listening for new addMembers errored:', err)
-    })
-  )
-
-  pull(
-    listen.excludeMember(ssb),
-    pull.drain((msg) => {
-      const excludes = msg.value.content.excludes
-      const groupId = msg.value.content.recps[0]
-
-      if (excludes.includes(ssb.id)) {
-        keystore.group.exclude(groupId)
-      }
-    }, err => {
-      if (err) console.error('Listening for new excludeMembers errored:', err)
+      if (err) console.error('Listening for new addMember and excludeMember messages errored:', err)
     })
   )
 

listen.js

@@ -10,24 +10,21 @@ const mockSSB = { backlinks: true, query: true }
 const { isUpdate: isPOBox } = new CRUT(mockSSB, poBoxSpec).spec
 
 module.exports = {
-  addMember (ssb) {
-    return pull(
-      ssb.messagesByType({ type: 'group/add-member', private: true, live: true }),
-      // NOTE this will run through all messages on each startup, which will help guarentee
-      // all messages have been emitted AND processed
-      // (same not true if we used a dummy flume-view)
-      pull.filter(m => m.sync !== true),
-      pull.filter(isAddMember),
-      pull.unique('key')
-      // NOTE we DO NOT filter our own messages out
-      // this is important for rebuilding indexes and keystore state if we have to restore our feed
-    )
-  },
-  excludeMember (ssb) {
+  mutateMembers (ssb) {
+    const query = [{
+      $filter: {
+        value: {
+          content: {
+            type: { $in: ['group/add-member', 'group/exclude-member'] }
+          }
+        }
+      }
+    }]
+
     return pull(
-      ssb.messagesByType({ type: 'group/exclude-member', private: true, live: true }),
+      ssb.query.read({ query, old: true, live: true }),
       pull.filter(m => m.sync !== true),
-      pull.filter(isExcludeMember),
+      pull.filter(msg => isAddMember(msg) || isExcludeMember(msg)),
       pull.unique('key')
     )
   },

method/group.js

@@ -129,7 +129,6 @@ module.exports = function GroupMethods (ssb, keystore, state) {
         ),
         pull.map(msg => msg.value.content.recps.slice(1)),
         pull.flatten(),
-        pull.unique(),
         pull.collect((err, addedMembers) => {
           if (err) return cb(err)
 
@@ -151,13 +150,34 @@ module.exports = function GroupMethods (ssb, keystore, state) {
             ),
             pull.map(msg => msg.value.content.excludes),
             pull.flatten(),
-            pull.unique(),
             pull.collect((err, excludedMembers) => {
               if (err) return cb(err)
 
-              // NOTE: this currently prevents people who've been removed from being re-added
-              // https://github.com/ssbc/ssb-tribes/issues/79
-              const members = addedMembers.filter(addedMember => !excludedMembers.includes(addedMember))
+              // calculate if the person has been added more times than they've been removed
+              // this is kind of basic but works. although lol people can add themselves back
+              // a fancier version would check the members tangle and maybe the author of the additions/exclusions
+              const numAdded = {}
+
+              addedMembers.forEach(addedMember => {
+                if (numAdded[addedMember] === undefined) {
+                  numAdded[addedMember] = 0
+                }
+                numAdded[addedMember]++
+              })
+
+              excludedMembers.forEach(excludedMember => {
+                if (numAdded[excludedMember] !== undefined) {
+                  numAdded[excludedMember]--
+                }
+              })
+
+              Object.keys(numAdded).forEach(member => {
+                if (numAdded[member] < 1) {
+                  delete numAdded[member]
+                }
+              })
+
+              const members = Object.keys(numAdded)
 
               return cb(null, members)
             })

package.json

@@ -38,7 +38,7 @@
     "sodium-native": "^3.4.1",
     "ssb-bfe": "^3.7.0",
     "ssb-crut": "^4.6.2",
-    "ssb-keyring": "^5.6.0",
+    "ssb-keyring": "^7.0.0",
     "ssb-keys": "^8.5.0",
     "ssb-private-group-keys": "^0.4.1",
     "ssb-ref": "^2.16.0",

test/api/add-new-author-listener.test.js

@@ -3,8 +3,6 @@ const { promisify: p } = require('util')
 const { Server, replicate } = require('../helpers')
 
 test('addNewAuthorListener', t => {
-  t.plan(6)
-
   const admin = Server({ name: 'admin', debug: false }) // me
   const newPerson = Server({ name: 'newPerson', debug: false }) // some friend
 
@@ -38,8 +36,13 @@ test('addNewAuthorListener', t => {
     t.equal(_groupId, groupId, 'newPerson, returns expected groupId')
 
     setTimeout(() => {
-      admin.close()
-      newPerson.close()
+      Promise.all([
+        p(admin.close)(true),
+        p(newPerson.close)(true)
+      ]).then(()=>{
+        if(numAdded === 2) t.end()
+        else t.fail("didn't get all listener events")
+      })
     }, 1000)
   })
 

test/api/exclude-members.test.js

@@ -1,10 +1,12 @@
 const test = require('tape')
 const { promisify: p } = require('util')
+const ssbKeys = require('ssb-keys')
 const { Server, replicate, FeedId } = require('../helpers')
 
 test('tribes.excludeMembers', async t => {
-  const kaitiaki = Server()
-  const newPerson = Server()
+  const newPersonKeys = ssbKeys.generate()
+  const kaitiaki = Server({ name: 'kaitiaki' })
+  const newPerson = Server({ name: 'newPerson', keys: newPersonKeys })
 
   const name = id => {
     if (id === kaitiaki.id) return 'kaitiaki '
@@ -55,18 +57,39 @@ test('tribes.excludeMembers', async t => {
 
     const excludedList = await p(newPerson.tribes.list)()
     t.deepEqual(excludedList, [], "new person can't list any groups anymore")
-  } catch (err) {
-    t.fail(err)
-  }
 
-  await p(setTimeout)(500)
+    await p(kaitiaki.tribes.invite)(
+      groupId, [newPerson.id], {}
+    )
+
+    const reAddedAuthors = await p(kaitiaki.tribes.listAuthors)(groupId)
+    t.deepEqual(reAddedAuthors.sort(), [kaitiaki.id, newPerson.id].sort(), 're-added person is listed as a member')
+
+    await p(replicate)({ from: kaitiaki, to: newPerson, live: false, name })
+
+    const newPersonBackInGroup = await p(newPerson.tribes.get)(groupId)
+    t.equal(newPersonBackInGroup.excluded, undefined, 'new person is not excluded anymore')
 
-  await Promise.all([
-    p(kaitiaki.close)(true),
-    p(newPerson.close)(true)
-  ])
-    .then(() => t.pass('clients close'))
-    .catch((err) => t.error(err))
+    await p(setTimeout)(5000)
 
-  t.end()
+    await Promise.all([
+      p(kaitiaki.close)(true),
+      p(newPerson.close)(true)
+    ])
+      .then(() => t.pass('clients close'))
+      .catch((err) => t.error(err))
+
+    await p(setTimeout)(5000)
+
+    const newPerson2 = Server({ name: 'newPerson',keys: newPersonKeys, startUnclean: true })
+
+    const stillInGroup = await p(newPerson2.tribes.get)(groupId)
+    t.equal(stillInGroup.excluded, undefined, 'new person is still not excluded after client restart')
+
+    await p(setTimeout)(5000)
+
+    await p(newPerson2.close)(true)
+  } catch (err) {
+    t.fail(err)
+  }
 })

test/listen.test.js

@@ -8,7 +8,7 @@ const listen = require('../listen')
 // TODO this is... not listen any more
 // we may need to rename this
 
-test('listen.addMember', t => {
+test.skip('listen.addMember', t => {
   const alice = Server()
   const bob = Server()
 

test/rebuild.test.js

@@ -61,7 +61,6 @@ test('rebuild (I am added to a group)', t => {
 })
 
 test('rebuild (I am added to a group, then someone else is added)', t => {
-  // t.plan(9)
   const admin = Server()
   const me = Server()
   const bob = Server()
@@ -166,9 +165,10 @@ test('rebuild (I am added to a group, then someone else is added)', t => {
           (err) => {
             if (seenMine === 20) t.equal(seenMine, 20, 'bob saw 20 messages from me')
             if (err) throw err
-            bob.close()
-            admin.close()
-            t.end()
+            Promise.all([
+              p(bob.close)(true),
+              p(admin.close)(true)
+            ]).then(()=>t.end())
           }
         )
       )