Commit 82d434df authored by Robert Knight's avatar Robert Knight

Convert `groups` service to an ES class

 - Convert the `groups` service to an ES class and the many closures in
   the constructor to private or public methods.

 - Remove the unused `all` and `get` methods of the groups service.
   These were trivial wrappers around store methods. UI components
   should use the store methods directly.
parent 87956ee3
...@@ -16,7 +16,7 @@ import MenuItem from '../MenuItem'; ...@@ -16,7 +16,7 @@ import MenuItem from '../MenuItem';
* @prop {boolean} [isExpanded] - Whether the submenu for this group is expanded * @prop {boolean} [isExpanded] - Whether the submenu for this group is expanded
* @prop {(expand: boolean) => any} onExpand - * @prop {(expand: boolean) => any} onExpand -
* Callback invoked to expand or collapse the current group * Callback invoked to expand or collapse the current group
* @prop {Object} groups - Injected service * @prop {import('../../services/groups').GroupsService} groups
* @prop {import('../../services/toast-messenger').ToastMessengerService} toastMessenger * @prop {import('../../services/toast-messenger').ToastMessengerService} toastMessenger
*/ */
......
...@@ -113,7 +113,7 @@ import { AuthService } from './services/auth'; ...@@ -113,7 +113,7 @@ import { AuthService } from './services/auth';
import { AutosaveService } from './services/autosave'; import { AutosaveService } from './services/autosave';
import { FeaturesService } from './services/features'; import { FeaturesService } from './services/features';
import { FrameSyncService } from './services/frame-sync'; import { FrameSyncService } from './services/frame-sync';
import groupsService from './services/groups'; import { GroupsService } from './services/groups';
import { LoadAnnotationsService } from './services/load-annotations'; import { LoadAnnotationsService } from './services/load-annotations';
import { LocalStorageService } from './services/local-storage'; import { LocalStorageService } from './services/local-storage';
import { PersistedDefaultsService } from './services/persisted-defaults'; import { PersistedDefaultsService } from './services/persisted-defaults';
...@@ -151,7 +151,7 @@ function startApp(config, appEl) { ...@@ -151,7 +151,7 @@ function startApp(config, appEl) {
.register('bridge', bridgeService) .register('bridge', bridgeService)
.register('features', FeaturesService) .register('features', FeaturesService)
.register('frameSync', FrameSyncService) .register('frameSync', FrameSyncService)
.register('groups', groupsService) .register('groups', GroupsService)
.register('loadAnnotationsService', LoadAnnotationsService) .register('loadAnnotationsService', LoadAnnotationsService)
.register('localStorage', LocalStorageService) .register('localStorage', LocalStorageService)
.register('persistedDefaults', PersistedDefaultsService) .register('persistedDefaults', PersistedDefaultsService)
......
...@@ -21,74 +21,57 @@ const DEFAULT_ORGANIZATION = { ...@@ -21,74 +21,57 @@ const DEFAULT_ORGANIZATION = {
}; };
/** /**
* @param {import('../store').SidebarStore} store * For any group that does not have an associated organization, populate with
* @param {import('./api').APIService} api * the default Hypothesis organization.
* @param {import('./toast-messenger').ToastMessengerService} toastMessenger *
* @param {import('./auth').AuthService} auth * Mutates group objects in place
*
* @param {Group[]} groups
*/ */
// @inject function injectOrganizations(groups) {
export default function groups( groups.forEach(group => {
store, if (!group.organization || typeof group.organization !== 'object') {
api, group.organization = DEFAULT_ORGANIZATION;
session, }
settings, });
toastMessenger, }
auth
) { // `expand` parameter for various groups API calls.
const svc = serviceConfig(settings); const expandParam = ['organization', 'scopes'];
const authority = svc ? svc.authority : null;
// `expand` parameter for various groups API calls.
const expandParam = ['organization', 'scopes'];
/**
* Service for fetching groups from the API and adding them to the store.
*
* The service also provides a `focus` method for switching the active group
* and `leave` method to remove the current user from a private group.
*
* @inject
*/
export class GroupsService {
/** /**
* Return the main document URI that is used to fetch groups associated with * @param {import('../store').SidebarStore} store
* the site that the user is on. * @param {import('./api').APIService} api
* @param {import('./session').SessionService} session
* @param {import('./toast-messenger').ToastMessengerService} toastMessenger
* @param {import('./auth').AuthService} auth
*/ */
function mainUri() { constructor(store, api, session, settings, toastMessenger, auth) {
const mainFrame = store.mainFrame(); this._api = api;
if (!mainFrame) { this._auth = auth;
return null; this._settings = settings;
} this._store = store;
return mainFrame.uri; this._toastMessenger = toastMessenger;
}
this._serviceConfig = serviceConfig(settings);
function getDocumentUriForGroupSearch() { this._reloadSetUp = false;
return awaitStateChange(store, mainUri);
} }
/** /**
* Update the focused group. Update the store, then check to see if * Return the main document URI that is used to fetch groups associated with
* there are any new (unsaved) annotations—if so, update those annotations * the site that the user is on.
* such that they are associated with the newly-focused group.
*/ */
function focus(groupId) { _mainURI() {
const prevGroupId = store.focusedGroupId(); return this._store.mainFrame()?.uri ?? null;
store.focusGroup(groupId);
const newGroupId = store.focusedGroupId();
const groupHasChanged = prevGroupId !== newGroupId && prevGroupId !== null;
if (groupHasChanged) {
// Move any top-level new annotations to the newly-focused group.
// Leave replies where they are.
const updatedAnnotations = [];
store.newAnnotations().forEach(annot => {
if (!isReply(annot)) {
updatedAnnotations.push(
Object.assign({}, annot, { group: newGroupId })
);
}
});
if (updatedAnnotations.length) {
store.addAnnotations(updatedAnnotations);
}
// Persist this group as the last focused group default
store.setDefault('focusedGroup', newGroupId);
}
} }
/** /**
...@@ -103,7 +86,7 @@ export default function groups( ...@@ -103,7 +86,7 @@ export default function groups(
* @param {string|null} directLinkedGroupId * @param {string|null} directLinkedGroupId
* @return {Promise<Group[]>} * @return {Promise<Group[]>}
*/ */
async function filterGroups( async _filterGroups(
groups, groups,
isLoggedIn, isLoggedIn,
directLinkedAnnotationGroupId, directLinkedAnnotationGroupId,
...@@ -119,7 +102,7 @@ export default function groups( ...@@ -119,7 +102,7 @@ export default function groups(
directLinkedGroup.scopes.enforced directLinkedGroup.scopes.enforced
) { ) {
groups = groups.filter(g => g.id !== directLinkedGroupId); groups = groups.filter(g => g.id !== directLinkedGroupId);
store.setDirectLinkedGroupFetchFailed(); this._store.setDirectLinkedGroupFetchFailed();
directLinkedGroupId = null; directLinkedGroupId = null;
} }
} }
...@@ -151,80 +134,101 @@ export default function groups( ...@@ -151,80 +134,101 @@ export default function groups(
return groups.filter(g => g.id !== '__world__'); return groups.filter(g => g.id !== '__world__');
} }
/**
* For any group that does not have an associated organization, populate with
* the default Hypothesis organization.
*
* Mutates group objects in place
*
* @param {Group[]} groups
*/
function injectOrganizations(groups) {
groups.forEach(group => {
if (!group.organization || typeof group.organization !== 'object') {
group.organization = DEFAULT_ORGANIZATION;
}
});
}
/**
* Fetch a specific group.
*
* @param {string} id
* @return {Promise<Group>}
*/
async function fetchGroup(id) {
return api.group.read({ id, expand: expandParam });
}
let reloadSetUp = false;
/** /**
* Set up automatic re-fetching of groups in response to various events * Set up automatic re-fetching of groups in response to various events
* in the sidebar. * in the sidebar.
*/ */
function setupAutoReload() { _setupAutoReload() {
if (reloadSetUp) { if (this._reloadSetUp) {
return; return;
} }
reloadSetUp = true; this._reloadSetUp = true;
// Reload groups when main document URI changes. // Reload groups when main document URI changes.
watch(store.subscribe, mainUri, () => { watch(
load(); this._store.subscribe,
}); () => this._mainURI(),
() => {
this.load();
}
);
// Reload groups when user ID changes. This is a bit inefficient since it // Reload groups when user ID changes. This is a bit inefficient since it
// means we are not fetching the groups and profile concurrently after // means we are not fetching the groups and profile concurrently after
// logging in or logging out. // logging in or logging out.
watch( watch(
store.subscribe, this._store.subscribe,
[() => store.hasFetchedProfile(), () => store.profile().userid], [
() => this._store.hasFetchedProfile(),
() => this._store.profile().userid,
],
(_, [prevFetchedProfile]) => { (_, [prevFetchedProfile]) => {
if (!prevFetchedProfile) { if (!prevFetchedProfile) {
// Ignore the first time that the profile is loaded. // Ignore the first time that the profile is loaded.
return; return;
} }
load(); this.load();
} }
); );
} }
/**
* Add groups to the store and set the initial focused group.
*
* @param {Group[]} groups
* @param {string|null} groupToFocus
*/
_addGroupsToStore(groups, groupToFocus) {
// Add a default organization to groups that don't have one. The organization
// provides the logo to display when the group is selected and is also used
// to order groups.
injectOrganizations(groups);
const isFirstLoad = this._store.allGroups().length === 0;
const prevFocusedGroup = this._store.getDefault('focusedGroup');
this._store.loadGroups(groups);
if (isFirstLoad) {
if (groupToFocus && groups.some(g => g.id === groupToFocus)) {
this.focus(groupToFocus);
} else if (
prevFocusedGroup &&
groups.some(g => g.id === prevFocusedGroup)
) {
this.focus(prevFocusedGroup);
}
}
}
/**
* Fetch a specific group.
*
* @param {string} id
* @return {Promise<Group>}
*/
_fetchGroup(id) {
return this._api.group.read({ id, expand: expandParam });
}
/** /**
* Fetch the groups associated with the current user and document, as well * Fetch the groups associated with the current user and document, as well
* as any groups that have been direct-linked to. * as any groups that have been direct-linked to.
* *
* @return {Promise<Group[]>} * @return {Promise<Group[]>}
*/ */
async function loadGroupsForUserAndDocument() { async _loadGroupsForUserAndDocument() {
const getDocumentUriForGroupSearch = () =>
awaitStateChange(this._store, () => this._mainURI());
// Step 1: Get the URI of the active document, so we can fetch groups // Step 1: Get the URI of the active document, so we can fetch groups
// associated with that document. // associated with that document.
let documentUri = null; let documentUri = null;
if (store.route() === 'sidebar') { if (this._store.route() === 'sidebar') {
documentUri = await getDocumentUriForGroupSearch(); documentUri = await getDocumentUriForGroupSearch();
} }
setupAutoReload(); this._setupAutoReload();
// Step 2: Concurrently fetch the groups the user is a member of, // Step 2: Concurrently fetch the groups the user is a member of,
// the groups associated with the current document and the annotation // the groups associated with the current document and the annotation
...@@ -233,10 +237,10 @@ export default function groups( ...@@ -233,10 +237,10 @@ export default function groups(
// If there is a direct-linked annotation, fetch the annotation in case // If there is a direct-linked annotation, fetch the annotation in case
// the associated group has not already been fetched and we need to make // the associated group has not already been fetched and we need to make
// an additional request for it. // an additional request for it.
const directLinkedAnnId = store.directLinkedAnnotationId(); const directLinkedAnnId = this._store.directLinkedAnnotationId();
let directLinkedAnnApi = null; let directLinkedAnnApi = null;
if (directLinkedAnnId) { if (directLinkedAnnId) {
directLinkedAnnApi = api.annotation directLinkedAnnApi = this._api.annotation
.get({ id: directLinkedAnnId }) .get({ id: directLinkedAnnId })
.catch(() => { .catch(() => {
// If the annotation does not exist or the user doesn't have permission. // If the annotation does not exist or the user doesn't have permission.
...@@ -247,17 +251,17 @@ export default function groups( ...@@ -247,17 +251,17 @@ export default function groups(
// If there is a direct-linked group, add an API request to get that // If there is a direct-linked group, add an API request to get that
// particular group since it may not be in the set of groups that are // particular group since it may not be in the set of groups that are
// fetched by other requests. // fetched by other requests.
const directLinkedGroupId = store.directLinkedGroupId(); const directLinkedGroupId = this._store.directLinkedGroupId();
let directLinkedGroupApi = null; let directLinkedGroupApi = null;
if (directLinkedGroupId) { if (directLinkedGroupId) {
directLinkedGroupApi = fetchGroup(directLinkedGroupId) directLinkedGroupApi = this._fetchGroup(directLinkedGroupId)
.then(group => { .then(group => {
store.clearDirectLinkedGroupFetchFailed(); this._store.clearDirectLinkedGroupFetchFailed();
return group; return group;
}) })
.catch(() => { .catch(() => {
// If the group does not exist or the user doesn't have permission. // If the group does not exist or the user doesn't have permission.
store.setDirectLinkedGroupFetchFailed(); this._store.setDirectLinkedGroupFetchFailed();
return null; return null;
}); });
} }
...@@ -265,6 +269,7 @@ export default function groups( ...@@ -265,6 +269,7 @@ export default function groups(
const listParams = { const listParams = {
expand: expandParam, expand: expandParam,
}; };
const authority = this._serviceConfig?.authority;
if (authority) { if (authority) {
listParams.authority = authority; listParams.authority = authority;
} }
...@@ -279,9 +284,9 @@ export default function groups( ...@@ -279,9 +284,9 @@ export default function groups(
directLinkedAnn, directLinkedAnn,
directLinkedGroup, directLinkedGroup,
] = await Promise.all([ ] = await Promise.all([
api.profile.groups.read({ expand: expandParam }), this._api.profile.groups.read({ expand: expandParam }),
api.groups.list(listParams), this._api.groups.list(listParams),
auth.getAccessToken(), this._auth.getAccessToken(),
directLinkedAnnApi, directLinkedAnnApi,
directLinkedGroupApi, directLinkedGroupApi,
]); ]);
...@@ -314,10 +319,14 @@ export default function groups( ...@@ -314,10 +319,14 @@ export default function groups(
if (!directLinkedAnnGroup) { if (!directLinkedAnnGroup) {
try { try {
const directLinkedAnnGroup = await fetchGroup(directLinkedAnn.group); const directLinkedAnnGroup = await this._fetchGroup(
directLinkedAnn.group
);
featuredGroups.push(directLinkedAnnGroup); featuredGroups.push(directLinkedAnnGroup);
} catch (e) { } catch (e) {
toastMessenger.error('Unable to fetch group for linked annotation'); this._toastMessenger.error(
'Unable to fetch group for linked annotation'
);
} }
} }
} }
...@@ -325,8 +334,8 @@ export default function groups( ...@@ -325,8 +334,8 @@ export default function groups(
// Step 4. Combine all the groups into a single list and set additional // Step 4. Combine all the groups into a single list and set additional
// metadata on them that will be used elsewhere in the app. // metadata on them that will be used elsewhere in the app.
const isLoggedIn = token !== null; const isLoggedIn = token !== null;
const groups = await filterGroups( const groups = await this._filterGroups(
combineGroups(myGroups, featuredGroups, documentUri, settings), combineGroups(myGroups, featuredGroups, documentUri, this._settings),
isLoggedIn, isLoggedIn,
directLinkedAnnotationGroupId, directLinkedAnnotationGroupId,
directLinkedGroupId directLinkedGroupId
...@@ -334,7 +343,7 @@ export default function groups( ...@@ -334,7 +343,7 @@ export default function groups(
const groupToFocus = const groupToFocus =
directLinkedAnnotationGroupId || directLinkedGroupId || null; directLinkedAnnotationGroupId || directLinkedGroupId || null;
addGroupsToStore(groups, groupToFocus); this._addGroupsToStore(groups, groupToFocus);
return groups; return groups;
} }
...@@ -344,19 +353,21 @@ export default function groups( ...@@ -344,19 +353,21 @@ export default function groups(
* *
* @param {string[]} groupIds - `id` or `groupid`s of groups to fetch * @param {string[]} groupIds - `id` or `groupid`s of groups to fetch
*/ */
async function loadServiceSpecifiedGroups(groupIds) { async _loadServiceSpecifiedGroups(groupIds) {
// Fetch the groups that the user is a member of in one request and then // Fetch the groups that the user is a member of in one request and then
// fetch any other groups not returned in that request directly. // fetch any other groups not returned in that request directly.
// //
// This reduces the number of requests to the backend on the assumption // This reduces the number of requests to the backend on the assumption
// that most or all of the group IDs that the service configures the client // that most or all of the group IDs that the service configures the client
// to show are groups that the user is a member of. // to show are groups that the user is a member of.
const userGroups = await api.profile.groups.read({ expand: expandParam }); const userGroups = await this._api.profile.groups.read({
expand: expandParam,
});
let error; let error;
const tryFetchGroup = async id => { const tryFetchGroup = async id => {
try { try {
return await fetchGroup(id); return await this._fetchGroup(id);
} catch (e) { } catch (e) {
error = e; error = e;
return null; return null;
...@@ -372,13 +383,13 @@ export default function groups( ...@@ -372,13 +383,13 @@ export default function groups(
); );
// Optional direct linked group id. This is used in the Notebook context. // Optional direct linked group id. This is used in the Notebook context.
const focusedGroupId = store.directLinkedGroupId(); const focusedGroupId = this._store.directLinkedGroupId();
addGroupsToStore(groups, focusedGroupId); this._addGroupsToStore(groups, focusedGroupId);
if (error) { if (error) {
// @ts-ignore - TS can't track the type of `error` here. // @ts-ignore - TS can't track the type of `error` here.
toastMessenger.error(`Unable to fetch groups: ${error.message}`, { this._toastMessenger.error(`Unable to fetch groups: ${error.message}`, {
autoDismiss: false, autoDismiss: false,
}); });
} }
...@@ -386,32 +397,6 @@ export default function groups( ...@@ -386,32 +397,6 @@ export default function groups(
return groups; return groups;
} }
/**
* Add groups to the store and set the initial focused group.
*
* @param {Group[]} groups
* @param {string|null} [groupToFocus]
*/
function addGroupsToStore(groups, groupToFocus) {
// Add a default organization to groups that don't have one. The organization
// provides the logo to display when the group is selected and is also used
// to order groups.
injectOrganizations(groups);
const isFirstLoad = store.allGroups().length === 0;
const prevFocusedGroup = store.getDefault('focusedGroup');
store.loadGroups(groups);
if (isFirstLoad) {
if (groups.some(g => g.id === groupToFocus)) {
focus(groupToFocus);
} else if (groups.some(g => g.id === prevFocusedGroup)) {
focus(prevFocusedGroup);
}
}
}
/** /**
* Fetch groups from the API, load them into the store and set the focused * Fetch groups from the API, load them into the store and set the focused
* group. * group.
...@@ -430,54 +415,71 @@ export default function groups( ...@@ -430,54 +415,71 @@ export default function groups(
* *
* @return {Promise<Group[]>} * @return {Promise<Group[]>}
*/ */
async function load() { async load() {
let groups; if (this._serviceConfig?.groups) {
if (svc && svc.groups) {
let groupIds = []; let groupIds = [];
try { try {
groupIds = await svc.groups; groupIds = await this._serviceConfig.groups;
} catch (e) { } catch (e) {
toastMessenger.error( this._toastMessenger.error(
`Unable to fetch group configuration: ${e.message}` `Unable to fetch group configuration: ${e.message}`
); );
} }
groups = await loadServiceSpecifiedGroups(groupIds); return this._loadServiceSpecifiedGroups(groupIds);
} else { } else {
groups = await loadGroupsForUserAndDocument(); return this._loadGroupsForUserAndDocument();
} }
return groups;
} }
function all() { /**
return store.allGroups(); * Update the focused group. Update the store, then check to see if
} * there are any new (unsaved) annotations—if so, update those annotations
* such that they are associated with the newly-focused group.
*
* @param {string} groupId
*/
focus(groupId) {
const prevGroupId = this._store.focusedGroupId();
this._store.focusGroup(groupId);
const newGroupId = this._store.focusedGroupId();
const groupHasChanged = prevGroupId !== newGroupId && prevGroupId !== null;
if (groupHasChanged) {
// Move any top-level new annotations to the newly-focused group.
// Leave replies where they are.
const updatedAnnotations = [];
this._store.newAnnotations().forEach(annot => {
if (!isReply(annot)) {
updatedAnnotations.push(
Object.assign({}, annot, { group: newGroupId })
);
}
});
// Return the full object for the group with the given id. if (updatedAnnotations.length) {
function get(id) { this._store.addAnnotations(updatedAnnotations);
return store.getGroup(id); }
// Persist this group as the last focused group default
this._store.setDefault('focusedGroup', newGroupId);
}
} }
/** /**
* Leave the group with the given ID. * Request to remove the current user from a group.
* Returns a promise which resolves when the action completes. *
* @param {string} id - The group ID
* @return {Promise<void>}
*/ */
function leave(id) { leave(id) {
// The groups list will be updated in response to a session state // The groups list will be updated in response to a session state
// change notification from the server. We could improve the UX here // change notification from the server. We could improve the UX here
// by optimistically updating the session state // by optimistically updating the session state
return api.group.member.delete({ return this._api.group.member.delete({
pubid: id, pubid: id,
userid: 'me', userid: 'me',
}); });
} }
return {
all: all,
get: get,
leave: leave,
load: load,
focus: focus,
};
} }
...@@ -12,7 +12,7 @@ import { watch } from '../util/watch'; ...@@ -12,7 +12,7 @@ import { watch } from '../util/watch';
* *
* @param {import('../store').SidebarStore} store * @param {import('../store').SidebarStore} store
* @param {import('./auth').AuthService} auth * @param {import('./auth').AuthService} auth
* @param {ReturnType<import('./groups').default>} groups * @param {import('./groups').GroupsService} groups
* @param {import('./session').SessionService} session * @param {import('./session').SessionService} session
* @param {Record<string, any>} settings * @param {Record<string, any>} settings
*/ */
......
import fakeReduxStore from '../../test/fake-redux-store'; import fakeReduxStore from '../../test/fake-redux-store';
import groups, { $imports } from '../groups'; import { GroupsService, $imports } from '../groups';
import { waitFor } from '../../../test-util/wait'; import { waitFor } from '../../../test-util/wait';
/** /**
...@@ -37,7 +37,7 @@ const dummyGroups = [ ...@@ -37,7 +37,7 @@ const dummyGroups = [
{ name: 'Group 3', id: 'id3' }, { name: 'Group 3', id: 'id3' },
]; ];
describe('sidebar/services/groups', function () { describe('GroupsService', function () {
let fakeAuth; let fakeAuth;
let fakeStore; let fakeStore;
let fakeSession; let fakeSession;
...@@ -130,8 +130,8 @@ describe('sidebar/services/groups', function () { ...@@ -130,8 +130,8 @@ describe('sidebar/services/groups', function () {
$imports.$restore(); $imports.$restore();
}); });
function service() { function createService() {
return groups( return new GroupsService(
fakeStore, fakeStore,
fakeApi, fakeApi,
fakeSession, fakeSession,
...@@ -143,7 +143,7 @@ describe('sidebar/services/groups', function () { ...@@ -143,7 +143,7 @@ describe('sidebar/services/groups', function () {
describe('#focus', () => { describe('#focus', () => {
it('updates the focused group in the store', () => { it('updates the focused group in the store', () => {
const svc = service(); const svc = createService();
fakeStore.focusedGroupId.returns('whatever'); fakeStore.focusedGroupId.returns('whatever');
svc.focus('whatnot'); svc.focus('whatnot');
...@@ -166,7 +166,7 @@ describe('sidebar/services/groups', function () { ...@@ -166,7 +166,7 @@ describe('sidebar/services/groups', function () {
fakeMetadata.isReply.returns(false); fakeMetadata.isReply.returns(false);
fakeStore.newAnnotations.returns(fakeAnnotations); fakeStore.newAnnotations.returns(fakeAnnotations);
const svc = service(); const svc = createService();
svc.focus('newgroup'); svc.focus('newgroup');
assert.calledWith( assert.calledWith(
...@@ -190,7 +190,7 @@ describe('sidebar/services/groups', function () { ...@@ -190,7 +190,7 @@ describe('sidebar/services/groups', function () {
{ $tag: '2', group: 'groupB' }, { $tag: '2', group: 'groupB' },
]); ]);
const svc = service(); const svc = createService();
svc.focus('newgroup'); svc.focus('newgroup');
assert.calledTwice(fakeMetadata.isReply); assert.calledTwice(fakeMetadata.isReply);
...@@ -198,7 +198,7 @@ describe('sidebar/services/groups', function () { ...@@ -198,7 +198,7 @@ describe('sidebar/services/groups', function () {
}); });
it('updates the focused-group default', () => { it('updates the focused-group default', () => {
const svc = service(); const svc = createService();
svc.focus('newgroup'); svc.focus('newgroup');
assert.calledOnce(fakeStore.setDefault); assert.calledOnce(fakeStore.setDefault);
...@@ -209,25 +209,16 @@ describe('sidebar/services/groups', function () { ...@@ -209,25 +209,16 @@ describe('sidebar/services/groups', function () {
it('does not update the focused-group default if the group has not changed', () => { it('does not update the focused-group default if the group has not changed', () => {
fakeStore.focusedGroupId.returns('samegroup'); fakeStore.focusedGroupId.returns('samegroup');
const svc = service(); const svc = createService();
svc.focus('samegroup'); svc.focus('samegroup');
assert.notCalled(fakeStore.setDefault); assert.notCalled(fakeStore.setDefault);
}); });
}); });
describe('#all', function () {
it('returns all groups from store.allGroups', () => {
const svc = service();
fakeStore.allGroups = sinon.stub().returns(dummyGroups);
assert.deepEqual(svc.all(), dummyGroups);
assert.called(fakeStore.allGroups);
});
});
describe('#load', function () { describe('#load', function () {
it('filters out direct-linked groups that are out of scope and scope enforced', () => { it('filters out direct-linked groups that are out of scope and scope enforced', () => {
const svc = service(); const svc = createService();
fakeStore.getDefault.returns(dummyGroups[0].id); fakeStore.getDefault.returns(dummyGroups[0].id);
const outOfScopeEnforcedGroup = { const outOfScopeEnforcedGroup = {
id: 'oos', id: 'oos',
...@@ -247,7 +238,7 @@ describe('sidebar/services/groups', function () { ...@@ -247,7 +238,7 @@ describe('sidebar/services/groups', function () {
}); });
it('catches error from api.group.read request', () => { it('catches error from api.group.read request', () => {
const svc = service(); const svc = createService();
fakeStore.getDefault.returns(dummyGroups[0].id); fakeStore.getDefault.returns(dummyGroups[0].id);
fakeStore.directLinkedGroupId.returns('does-not-exist'); fakeStore.directLinkedGroupId.returns('does-not-exist');
...@@ -268,7 +259,7 @@ describe('sidebar/services/groups', function () { ...@@ -268,7 +259,7 @@ describe('sidebar/services/groups', function () {
}); });
it('combines groups from both endpoints', function () { it('combines groups from both endpoints', function () {
const svc = service(); const svc = createService();
const groups = [ const groups = [
{ id: 'groupa', name: 'GroupA' }, { id: 'groupa', name: 'GroupA' },
...@@ -286,7 +277,7 @@ describe('sidebar/services/groups', function () { ...@@ -286,7 +277,7 @@ describe('sidebar/services/groups', function () {
// TODO: Add a de-dup test for the direct-linked annotation. // TODO: Add a de-dup test for the direct-linked annotation.
it('does not duplicate groups if the direct-linked group is also a featured group', () => { it('does not duplicate groups if the direct-linked group is also a featured group', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns(dummyGroups[0].id); fakeStore.directLinkedGroupId.returns(dummyGroups[0].id);
fakeApi.group.read.returns(Promise.resolve(dummyGroups[0])); fakeApi.group.read.returns(Promise.resolve(dummyGroups[0]));
...@@ -302,7 +293,7 @@ describe('sidebar/services/groups', function () { ...@@ -302,7 +293,7 @@ describe('sidebar/services/groups', function () {
}); });
it('combines groups from all 3 endpoints if there is a selectedGroup', () => { it('combines groups from all 3 endpoints if there is a selectedGroup', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns('selected-id'); fakeStore.directLinkedGroupId.returns('selected-id');
...@@ -322,7 +313,7 @@ describe('sidebar/services/groups', function () { ...@@ -322,7 +313,7 @@ describe('sidebar/services/groups', function () {
}); });
it('passes the direct-linked group id from the store to the api.group.read call', () => { it('passes the direct-linked group id from the store to the api.group.read call', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns('selected-id'); fakeStore.directLinkedGroupId.returns('selected-id');
...@@ -343,7 +334,7 @@ describe('sidebar/services/groups', function () { ...@@ -343,7 +334,7 @@ describe('sidebar/services/groups', function () {
}); });
it('loads all available groups', function () { it('loads all available groups', function () {
const svc = service(); const svc = createService();
return svc.load().then(() => { return svc.load().then(() => {
assert.calledWith(fakeStore.loadGroups, dummyGroups); assert.calledWith(fakeStore.loadGroups, dummyGroups);
...@@ -351,7 +342,7 @@ describe('sidebar/services/groups', function () { ...@@ -351,7 +342,7 @@ describe('sidebar/services/groups', function () {
}); });
it('sends `expand` parameter', function () { it('sends `expand` parameter', function () {
const svc = service(); const svc = createService();
fakeApi.groups.list.returns( fakeApi.groups.list.returns(
Promise.resolve([{ id: 'groupa', name: 'GroupA' }]) Promise.resolve([{ id: 'groupa', name: 'GroupA' }])
); );
...@@ -374,7 +365,7 @@ describe('sidebar/services/groups', function () { ...@@ -374,7 +365,7 @@ describe('sidebar/services/groups', function () {
}); });
it('sets the focused group from the value saved in local storage', () => { it('sets the focused group from the value saved in local storage', () => {
const svc = service(); const svc = createService();
fakeStore.getDefault.returns(dummyGroups[1].id); fakeStore.getDefault.returns(dummyGroups[1].id);
return svc.load().then(() => { return svc.load().then(() => {
assert.calledWith(fakeStore.focusGroup, dummyGroups[1].id); assert.calledWith(fakeStore.focusGroup, dummyGroups[1].id);
...@@ -382,7 +373,7 @@ describe('sidebar/services/groups', function () { ...@@ -382,7 +373,7 @@ describe('sidebar/services/groups', function () {
}); });
it("sets the direct-linked annotation's group to take precedence over the group saved in local storage and the direct-linked group", () => { it("sets the direct-linked annotation's group to take precedence over the group saved in local storage and the direct-linked group", () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
fakeStore.directLinkedGroupId.returns(dummyGroups[1].id); fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
...@@ -400,7 +391,7 @@ describe('sidebar/services/groups', function () { ...@@ -400,7 +391,7 @@ describe('sidebar/services/groups', function () {
}); });
it("sets the focused group to the direct-linked annotation's group", () => { it("sets the focused group to the direct-linked annotation's group", () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
fakeApi.groups.list.returns(Promise.resolve(dummyGroups)); fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
...@@ -417,7 +408,7 @@ describe('sidebar/services/groups', function () { ...@@ -417,7 +408,7 @@ describe('sidebar/services/groups', function () {
}); });
it('sets the direct-linked group to take precedence over the group saved in local storage', () => { it('sets the direct-linked group to take precedence over the group saved in local storage', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns(dummyGroups[1].id); fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
fakeStore.getDefault.returns(dummyGroups[0].id); fakeStore.getDefault.returns(dummyGroups[0].id);
...@@ -428,7 +419,7 @@ describe('sidebar/services/groups', function () { ...@@ -428,7 +419,7 @@ describe('sidebar/services/groups', function () {
}); });
it('sets the focused group to the direct-linked group', () => { it('sets the focused group to the direct-linked group', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns(dummyGroups[1].id); fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
fakeApi.groups.list.returns(Promise.resolve(dummyGroups)); fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
...@@ -438,7 +429,7 @@ describe('sidebar/services/groups', function () { ...@@ -438,7 +429,7 @@ describe('sidebar/services/groups', function () {
}); });
it('clears the directLinkedGroupFetchFailed state if loading a direct-linked group', () => { it('clears the directLinkedGroupFetchFailed state if loading a direct-linked group', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns(dummyGroups[1].id); fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
fakeApi.groups.list.returns(Promise.resolve(dummyGroups)); fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
...@@ -450,7 +441,7 @@ describe('sidebar/services/groups', function () { ...@@ -450,7 +441,7 @@ describe('sidebar/services/groups', function () {
[null, 'some-group-id'].forEach(groupId => { [null, 'some-group-id'].forEach(groupId => {
it('does not set the focused group if not present in the groups list', () => { it('does not set the focused group if not present in the groups list', () => {
const svc = service(); const svc = createService();
fakeStore.getDefault.returns(groupId); fakeStore.getDefault.returns(groupId);
return svc.load().then(() => { return svc.load().then(() => {
assert.notCalled(fakeStore.focusGroup); assert.notCalled(fakeStore.focusGroup);
...@@ -460,7 +451,7 @@ describe('sidebar/services/groups', function () { ...@@ -460,7 +451,7 @@ describe('sidebar/services/groups', function () {
context('in the sidebar', () => { context('in the sidebar', () => {
it('waits for the document URL to be determined', () => { it('waits for the document URL to be determined', () => {
const svc = service(); const svc = createService();
fakeStore.setState({ frames: [null] }); fakeStore.setState({ frames: [null] });
const loaded = svc.load(); const loaded = svc.load();
...@@ -482,7 +473,7 @@ describe('sidebar/services/groups', function () { ...@@ -482,7 +473,7 @@ describe('sidebar/services/groups', function () {
it('does not wait for the document URL', () => { it('does not wait for the document URL', () => {
fakeStore.setState({ frames: [null] }); fakeStore.setState({ frames: [null] });
const svc = service(); const svc = createService();
return svc.load().then(() => { return svc.load().then(() => {
assert.calledWith(fakeApi.groups.list, { assert.calledWith(fakeApi.groups.list, {
expand: ['organization', 'scopes'], expand: ['organization', 'scopes'],
...@@ -493,7 +484,7 @@ describe('sidebar/services/groups', function () { ...@@ -493,7 +484,7 @@ describe('sidebar/services/groups', function () {
it('passes authority argument when using a third-party authority', () => { it('passes authority argument when using a third-party authority', () => {
fakeSettings.services = [{ authority: 'publisher.org' }]; fakeSettings.services = [{ authority: 'publisher.org' }];
const svc = service(); const svc = createService();
return svc.load().then(() => { return svc.load().then(() => {
assert.calledWith( assert.calledWith(
fakeApi.groups.list, fakeApi.groups.list,
...@@ -503,7 +494,7 @@ describe('sidebar/services/groups', function () { ...@@ -503,7 +494,7 @@ describe('sidebar/services/groups', function () {
}); });
it('injects a default organization if group is missing an organization', function () { it('injects a default organization if group is missing an organization', function () {
const svc = service(); const svc = createService();
const groups = [{ id: '39r39f', name: 'Ding Dong!' }]; const groups = [{ id: '39r39f', name: 'Ding Dong!' }];
fakeApi.groups.list.returns(Promise.resolve(groups)); fakeApi.groups.list.returns(Promise.resolve(groups));
return svc.load().then(groups => { return svc.load().then(groups => {
...@@ -513,7 +504,7 @@ describe('sidebar/services/groups', function () { ...@@ -513,7 +504,7 @@ describe('sidebar/services/groups', function () {
}); });
it('catches error when fetching the direct-linked annotation', () => { it('catches error when fetching the direct-linked annotation', () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
fakeApi.profile.groups.read.returns(Promise.resolve([])); fakeApi.profile.groups.read.returns(Promise.resolve([]));
...@@ -536,7 +527,7 @@ describe('sidebar/services/groups', function () { ...@@ -536,7 +527,7 @@ describe('sidebar/services/groups', function () {
}); });
it("catches error when fetching the direct-linked annotation's group", () => { it("catches error when fetching the direct-linked annotation's group", () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
...@@ -572,7 +563,7 @@ describe('sidebar/services/groups', function () { ...@@ -572,7 +563,7 @@ describe('sidebar/services/groups', function () {
}); });
it("includes the direct-linked annotation's group when it is not in the normal list of groups", () => { it("includes the direct-linked annotation's group when it is not in the normal list of groups", () => {
const svc = service(); const svc = createService();
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
...@@ -604,7 +595,7 @@ describe('sidebar/services/groups', function () { ...@@ -604,7 +595,7 @@ describe('sidebar/services/groups', function () {
it('both groups are in the final groups list when an annotation and a group are linked to', () => { it('both groups are in the final groups list when an annotation and a group are linked to', () => {
// This can happen if the linked to annotation and group are configured by // This can happen if the linked to annotation and group are configured by
// the frame embedding the client. // the frame embedding the client.
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns('out-of-scope'); fakeStore.directLinkedGroupId.returns('out-of-scope');
fakeStore.directLinkedAnnotationId.returns('ann-id'); fakeStore.directLinkedAnnotationId.returns('ann-id');
...@@ -641,7 +632,7 @@ describe('sidebar/services/groups', function () { ...@@ -641,7 +632,7 @@ describe('sidebar/services/groups', function () {
// Set up the test under conditions that would otherwise // Set up the test under conditions that would otherwise
// not return the Public group. Aka: the user is logged // not return the Public group. Aka: the user is logged
// out and there are associated groups. // out and there are associated groups.
const svc = service(); const svc = createService();
fakeStore.directLinkedGroupId.returns('__world__'); fakeStore.directLinkedGroupId.returns('__world__');
...@@ -667,7 +658,7 @@ describe('sidebar/services/groups', function () { ...@@ -667,7 +658,7 @@ describe('sidebar/services/groups', function () {
truthTable(3).forEach( truthTable(3).forEach(
([loggedIn, pageHasAssociatedGroups, directLinkToPublicAnnotation]) => { ([loggedIn, pageHasAssociatedGroups, directLinkToPublicAnnotation]) => {
it('excludes the "Public" group if user logged out and page has associated groups', () => { it('excludes the "Public" group if user logged out and page has associated groups', () => {
const svc = service(); const svc = createService();
const shouldShowPublicGroup = const shouldShowPublicGroup =
loggedIn || loggedIn ||
!pageHasAssociatedGroups || !pageHasAssociatedGroups ||
...@@ -720,7 +711,7 @@ describe('sidebar/services/groups', function () { ...@@ -720,7 +711,7 @@ describe('sidebar/services/groups', function () {
setServiceConfigGroups(['id-a', 'groupid-b']); setServiceConfigGroups(['id-a', 'groupid-b']);
fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]); fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]);
const svc = service(); const svc = createService();
const groups = await svc.load(); const groups = await svc.load();
assert.deepEqual( assert.deepEqual(
...@@ -733,7 +724,7 @@ describe('sidebar/services/groups', function () { ...@@ -733,7 +724,7 @@ describe('sidebar/services/groups', function () {
setServiceConfigGroups(Promise.resolve(['id-a', 'groupid-b'])); setServiceConfigGroups(Promise.resolve(['id-a', 'groupid-b']));
fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]); fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]);
const svc = service(); const svc = createService();
const groups = await svc.load(); const groups = await svc.load();
assert.deepEqual( assert.deepEqual(
...@@ -754,7 +745,7 @@ describe('sidebar/services/groups', function () { ...@@ -754,7 +745,7 @@ describe('sidebar/services/groups', function () {
return group; return group;
}); });
const svc = service(); const svc = createService();
const groups = await svc.load(); const groups = await svc.load();
const expand = ['organization', 'scopes']; const expand = ['organization', 'scopes'];
...@@ -770,7 +761,7 @@ describe('sidebar/services/groups', function () { ...@@ -770,7 +761,7 @@ describe('sidebar/services/groups', function () {
setServiceConfigGroups(Promise.resolve(['id-a', 'groupid-b'])); setServiceConfigGroups(Promise.resolve(['id-a', 'groupid-b']));
fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]); fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]);
const svc = service(); const svc = createService();
await svc.load(); await svc.load();
assert.notCalled(fakeApi.group.read); assert.notCalled(fakeApi.group.read);
...@@ -781,7 +772,7 @@ describe('sidebar/services/groups', function () { ...@@ -781,7 +772,7 @@ describe('sidebar/services/groups', function () {
fakeApi.profile.groups.read.resolves([groupA]); fakeApi.profile.groups.read.resolves([groupA]);
fakeApi.group.read.rejects(new Error('Not Found')); fakeApi.group.read.rejects(new Error('Not Found'));
const svc = service(); const svc = createService();
const groups = await svc.load(); const groups = await svc.load();
assert.calledWith( assert.calledWith(
...@@ -801,7 +792,7 @@ describe('sidebar/services/groups', function () { ...@@ -801,7 +792,7 @@ describe('sidebar/services/groups', function () {
Promise.reject(new Error('Something went wrong')) Promise.reject(new Error('Something went wrong'))
); );
const svc = service(); const svc = createService();
const groups = await svc.load(); const groups = await svc.load();
assert.calledWith( assert.calledWith(
...@@ -816,7 +807,7 @@ describe('sidebar/services/groups', function () { ...@@ -816,7 +807,7 @@ describe('sidebar/services/groups', function () {
fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]); fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]);
fakeStore.directLinkedGroupId.returns('id-c'); fakeStore.directLinkedGroupId.returns('id-c');
const svc = service(); const svc = createService();
await svc.load(); await svc.load();
assert.calledWith(fakeStore.focusGroup, 'id-c'); assert.calledWith(fakeStore.focusGroup, 'id-c');
...@@ -827,7 +818,7 @@ describe('sidebar/services/groups', function () { ...@@ -827,7 +818,7 @@ describe('sidebar/services/groups', function () {
fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]); fakeApi.profile.groups.read.resolves([groupA, groupB, groupC]);
fakeStore.directLinkedGroupId.returns(null); fakeStore.directLinkedGroupId.returns(null);
const svc = service(); const svc = createService();
await svc.load(); await svc.load();
assert.notCalled(fakeStore.focusGroup); assert.notCalled(fakeStore.focusGroup);
...@@ -835,18 +826,9 @@ describe('sidebar/services/groups', function () { ...@@ -835,18 +826,9 @@ describe('sidebar/services/groups', function () {
}); });
}); });
describe('#get', function () {
it('returns the requested group', function () {
const svc = service();
fakeStore.getGroup.withArgs('foo').returns(dummyGroups[1]);
assert.equal(svc.get('foo'), dummyGroups[1]);
});
});
describe('#leave', function () { describe('#leave', function () {
it('should call the group leave API', function () { it('should call the group leave API', function () {
const s = service(); const s = createService();
return s.leave('id2').then(() => { return s.leave('id2').then(() => {
assert.calledWithMatch(fakeApi.group.member.delete, { assert.calledWithMatch(fakeApi.group.member.delete, {
pubid: 'id2', pubid: 'id2',
...@@ -860,7 +842,7 @@ describe('sidebar/services/groups', function () { ...@@ -860,7 +842,7 @@ describe('sidebar/services/groups', function () {
describe('automatic re-fetching', function () { describe('automatic re-fetching', function () {
it('refetches groups when the logged-in user changes', async () => { it('refetches groups when the logged-in user changes', async () => {
const svc = service(); const svc = createService();
// Load groups before profile fetch has completed. // Load groups before profile fetch has completed.
fakeStore.hasFetchedProfile.returns(false); fakeStore.hasFetchedProfile.returns(false);
...@@ -889,7 +871,7 @@ describe('sidebar/services/groups', function () { ...@@ -889,7 +871,7 @@ describe('sidebar/services/groups', function () {
context('when a new frame connects', () => { context('when a new frame connects', () => {
it('should refetch groups if main frame URL has changed', async () => { it('should refetch groups if main frame URL has changed', async () => {
const svc = service(); const svc = createService();
fakeStore.setState({ fakeStore.setState({
frames: [{ uri: 'https://domain.com/page-a' }], frames: [{ uri: 'https://domain.com/page-a' }],
...@@ -908,7 +890,7 @@ describe('sidebar/services/groups', function () { ...@@ -908,7 +890,7 @@ describe('sidebar/services/groups', function () {
}); });
it('should not refetch groups if main frame URL has not changed', async () => { it('should not refetch groups if main frame URL has not changed', async () => {
const svc = service(); const svc = createService();
fakeStore.setState({ fakeStore.setState({
frames: [{ uri: 'https://domain.com/page-a' }], frames: [{ uri: 'https://domain.com/page-a' }],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment