diff --git a/src/sidebar/helpers/groups.ts b/src/sidebar/helpers/groups.ts index 1aeaefe80b4..ecf0eb979f4 100644 --- a/src/sidebar/helpers/groups.ts +++ b/src/sidebar/helpers/groups.ts @@ -5,10 +5,10 @@ import type { SidebarSettings } from '../../types/config'; import { serviceConfig } from '../config/service-config'; /** - * Should users be able to leave private groups of which they - * are a member? Users may leave private groups unless - * explicitly disallowed in the service configuration of the - * `settings` object. + * Return true if users are allowed to leave groups. + * + * Third-party authorities currently have to opt-in to enabling this, since + * users may not have a way to rejoin a group after leaving. */ function allowLeavingGroups(settings: SidebarSettings): boolean { const config = serviceConfig(settings); @@ -42,19 +42,16 @@ export function combineGroups( const myGroupIds = userGroups.map(g => g.id); featuredGroups = featuredGroups.filter(g => !myGroupIds.includes(g.id)); - // Set the isMember property indicating user membership. + // Set flag indicating whether user is a member of the group. featuredGroups.forEach(group => (group.isMember = false)); userGroups.forEach(group => (group.isMember = true)); const groups = userGroups.concat(featuredGroups); - // Set the `canLeave` property. Groups can be left if they are private unless - // the global `allowLeavingGroups` value is false in the config settings object. - groups.forEach(group => { - group.canLeave = !allowLeavingGroups(settings) - ? false - : group.type === 'private'; - }); + // Set flag indicating whether user can leave group. + for (const group of groups) { + group.canLeave = allowLeavingGroups(settings) && group.isMember; + } // Add isScopedToUri property indicating whether a group is within scope // of the given uri. If the scope check cannot be performed, isScopedToUri diff --git a/src/sidebar/helpers/test/groups-test.js b/src/sidebar/helpers/test/groups-test.js index 7abedf44197..dafb8c58d35 100644 --- a/src/sidebar/helpers/test/groups-test.js +++ b/src/sidebar/helpers/test/groups-test.js @@ -2,6 +2,7 @@ import { combineGroups, normalizeGroupIds, $imports } from '../groups'; describe('sidebar/helpers/groups', () => { let fakeServiceConfig; + describe('combineGroups', () => { beforeEach(() => { fakeServiceConfig = sinon.stub().returns(null); @@ -22,54 +23,79 @@ describe('sidebar/helpers/groups', () => { assert.equal(groupA.isMember, true); }); - it('sets `canLeave` to true if a group is private and `allowLeavingGroups` is null', () => { - const userGroups = [{ id: 'groupa', name: 'GroupA', type: 'private' }]; - const featuredGroups = [{ id: 'groupb', name: 'GroupB', type: 'open' }]; - const groups = combineGroups( - userGroups, - featuredGroups, - 'https://foo.com/bar', - ); - const groupA = groups.find(g => g.id === 'groupa'); - const groupB = groups.find(g => g.id === 'groupb'); - assert.equal(groupA.canLeave, true); - assert.equal(groupB.canLeave, false); - }); + // `allowLeavingGroups` defaults to true for first party users and false + // for third-party users. + [null, { allowLeavingGroups: true }, { allowLeavingGroups: 1 }].forEach( + serviceConfig => { + it('sets `canLeave` to true if user is a member and leaving groups is enabled', () => { + fakeServiceConfig.returns(serviceConfig); + const userGroups = [ + { id: 'groupa', name: 'GroupA', type: 'private' }, + { id: 'groupc', name: 'GroupC', type: 'open' }, + { id: 'groupd', name: 'GroupD', type: 'restricted' }, + ]; + const featuredGroups = [ + { id: 'groupb', name: 'GroupB', type: 'open' }, + ]; + const groups = combineGroups( + userGroups, + featuredGroups, + 'https://foo.com/bar', + ); - it('sets `canLeave` to true if a group is private and `allowLeavingGroups` is not a boolean', () => { - fakeServiceConfig.returns({ - allowLeavingGroups: () => {}, - }); - const userGroups = [{ id: 'groupa', name: 'GroupA', type: 'private' }]; - const featuredGroups = [{ id: 'groupb', name: 'GroupB', type: 'open' }]; - const groups = combineGroups( - userGroups, - featuredGroups, - 'https://foo.com/bar', - ); - const groupA = groups.find(g => g.id === 'groupa'); - const groupB = groups.find(g => g.id === 'groupb'); + const expected = [ + { + id: 'groupa', + canLeave: true, + }, + { + id: 'groupb', + canLeave: false, + }, + { + id: 'groupc', + canLeave: true, + }, + { + id: 'groupd', + canLeave: true, + }, + ]; - assert.equal(groupA.canLeave, true); - assert.equal(groupB.canLeave, false); - }); + for (const { id, canLeave } of expected) { + const group = groups.find(g => g.id === id); + assert.strictEqual( + group.canLeave, + canLeave, + `incorrect canLeave for group ${id}`, + ); + } + }); + }, + ); - it('sets `canLeave` to false for all groups if `allowLeavingGroups` is false', () => { - fakeServiceConfig.returns({ - allowLeavingGroups: false, - }); - const userGroups = [{ id: 'groupa', name: 'GroupA', type: 'private' }]; - const featuredGroups = [{ id: 'groupb', name: 'GroupB', type: 'open' }]; - const groups = combineGroups( - userGroups, - featuredGroups, - 'https://foo.com/bar', - ); - const groupA = groups.find(g => g.id === 'groupa'); - const groupB = groups.find(g => g.id === 'groupb'); - assert.equal(groupA.canLeave, false); - assert.equal(groupB.canLeave, false); - }); + [{}, { allowLeavingGroups: false }, { allowLeavingGroups: null }].forEach( + serviceConfig => { + it('sets `canLeave` to false for all groups if leaving groups is disabled', () => { + fakeServiceConfig.returns(serviceConfig); + const userGroups = [ + { id: 'groupa', name: 'GroupA', type: 'private' }, + ]; + const featuredGroups = [ + { id: 'groupb', name: 'GroupB', type: 'open' }, + ]; + const groups = combineGroups( + userGroups, + featuredGroups, + 'https://foo.com/bar', + ); + const groupA = groups.find(g => g.id === 'groupa'); + const groupB = groups.find(g => g.id === 'groupb'); + assert.equal(groupA.canLeave, false); + assert.equal(groupB.canLeave, false); + }); + }, + ); it('combines groups from both lists uniquely', () => { const userGroups = [