Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow adding people back into a group #94

Closed
wants to merge 10 commits into from
14 changes: 5 additions & 9 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,12 @@ function init (ssb, config) {
...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)
processAuthors(groupId, authors, m.value.author, cb)
})
} else {
// TODO: only add if we're one of the people being added by this msg
Powersource marked this conversation as resolved.
Show resolved Hide resolved
// TODO: i think getting re-added might not work atm, since the keyring might only mark you un-excluded if you add a *new* key, but we're just adding the same one. so prob need to patch keyring
Powersource marked this conversation as resolved.
Show resolved Hide resolved
return keystore.group.add(groupId, { key: groupKey, root }, (err) => {
if (err) return cb(err)
processAuthors(groupId, authors, m.value.author, cb)
}
})
})
}),
pull.drain(() => {}, (err) => {
Expand Down
30 changes: 25 additions & 5 deletions method/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 this crude version won't work sorry D:

The problem is if I add you to a Batts Group at the same time as Staltz adds you, then you've technically been added "twice", even though that addition was idempotent. Then I remove you ... then you've been added twice and removed once... so according to this you're in the group, even though you may have honoured the exclusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mm ok i based this solution on a discussion we had at some point.

a simple solution could be to check this algorithm also whenever we receive something in the mutateMembers stream? to have the 2 systems match up, avoiding mismatches.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try the tangle stuff in a separate pr instead #95

//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)
})
Expand Down
16 changes: 14 additions & 2 deletions test/api/exclude-members.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const { promisify: p } = require('util')
const { Server, replicate, FeedId } = require('../helpers')

test('tribes.excludeMembers', async t => {
const kaitiaki = Server()
const newPerson = Server()
const kaitiaki = Server({ name: 'kaitiaki' })
const newPerson = Server({ name: 'newPerson' })

const name = id => {
if (id === kaitiaki.id) return 'kaitiaki '
Expand Down Expand Up @@ -55,6 +55,18 @@ test('tribes.excludeMembers', async t => {

const excludedList = await p(newPerson.tribes.list)()
t.deepEqual(excludedList, [], "new person can't list any groups anymore")

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')
} catch (err) {
t.fail(err)
}
Expand Down
Loading