Commit 6b0c776f authored by Robert Knight's avatar Robert Knight

Reimplement service URL fetching

Change the way that links pointing into the Hypothesis service (eg. for
tags, users, account settings etc.) are fetched and generated. The new
implementation better matches patterns used elsewhere in the application
and ensures that UI components displaying these links re-render if the
`/api/links` response is received after the component is initially
rendered.

 - Move the logic for rendering the URL templates from the `serviceUrl`
   service into a selector in the `links` store module. Components then
   render links using `store.getLink(...)`. This ensures use of the existing
   mechanism that re-renders components when relevant store data changes.

 - Convert the `serviceUrl` service to an ES class

 - Move the logic for fetching URL templates from the `/api/links`
   endpoint into an `init` method of the `ServiceURLService` service.
   This matches the convention used by several other services.

 - Remove unused `serviceUrl` dependency from `groups` service
parent 47f24786
...@@ -20,7 +20,6 @@ import AnnotationUser from './AnnotationUser'; ...@@ -20,7 +20,6 @@ import AnnotationUser from './AnnotationUser';
/** /**
* @typedef {import("../../../types/api").Annotation} Annotation * @typedef {import("../../../types/api").Annotation} Annotation
* @typedef {import('../../services/service-url').ServiceUrlGetter} ServiceUrlGetter
* @typedef {import('../../../types/config').MergedConfig} MergedConfig * @typedef {import('../../../types/config').MergedConfig} MergedConfig
*/ */
...@@ -30,7 +29,6 @@ import AnnotationUser from './AnnotationUser'; ...@@ -30,7 +29,6 @@ import AnnotationUser from './AnnotationUser';
* @prop {boolean} [isEditing] - Whether the annotation is actively being edited * @prop {boolean} [isEditing] - Whether the annotation is actively being edited
* @prop {number} replyCount - How many replies this annotation currently has * @prop {number} replyCount - How many replies this annotation currently has
* @prop {boolean} threadIsCollapsed - Is this thread currently collapsed? * @prop {boolean} threadIsCollapsed - Is this thread currently collapsed?
* @prop {ServiceUrlGetter} serviceUrl - Injected service
* @prop {MergedConfig} settings - Injected * @prop {MergedConfig} settings - Injected
* *
*/ */
...@@ -47,7 +45,6 @@ function AnnotationHeader({ ...@@ -47,7 +45,6 @@ function AnnotationHeader({
isEditing, isEditing,
replyCount, replyCount,
threadIsCollapsed, threadIsCollapsed,
serviceUrl,
settings, settings,
}) { }) {
const store = useStoreProxy(); const store = useStoreProxy();
...@@ -63,7 +60,7 @@ function AnnotationHeader({ ...@@ -63,7 +60,7 @@ function AnnotationHeader({
const authorLink = (() => { const authorLink = (() => {
if (!isThirdParty) { if (!isThirdParty) {
return serviceUrl('user', { user: annotation.user }); return store.getLink('user', { user: annotation.user });
} else { } else {
return ( return (
(settings.usernameUrl && (settings.usernameUrl &&
...@@ -174,6 +171,6 @@ function AnnotationHeader({ ...@@ -174,6 +171,6 @@ function AnnotationHeader({
); );
} }
AnnotationHeader.injectedProps = ['serviceUrl', 'settings']; AnnotationHeader.injectedProps = ['settings'];
export default withServices(AnnotationHeader); export default withServices(AnnotationHeader);
...@@ -15,7 +15,6 @@ describe('AnnotationHeader', () => { ...@@ -15,7 +15,6 @@ describe('AnnotationHeader', () => {
let fakeIsReply; let fakeIsReply;
let fakeHasBeenEdited; let fakeHasBeenEdited;
let fakeIsPrivate; let fakeIsPrivate;
let fakeServiceUrl;
let fakeSettings; let fakeSettings;
let fakeStore; let fakeStore;
...@@ -26,7 +25,6 @@ describe('AnnotationHeader', () => { ...@@ -26,7 +25,6 @@ describe('AnnotationHeader', () => {
isEditing={false} isEditing={false}
replyCount={0} replyCount={0}
threadIsCollapsed={false} threadIsCollapsed={false}
serviceUrl={fakeServiceUrl}
settings={fakeSettings} settings={fakeSettings}
{...props} {...props}
/> />
...@@ -47,11 +45,11 @@ describe('AnnotationHeader', () => { ...@@ -47,11 +45,11 @@ describe('AnnotationHeader', () => {
fakeAnnotationDisplayName = sinon.stub().returns('Robbie Burns'); fakeAnnotationDisplayName = sinon.stub().returns('Robbie Burns');
fakeServiceUrl = sinon.stub().returns('http://example.com');
fakeSettings = { usernameUrl: 'http://foo.bar/' }; fakeSettings = { usernameUrl: 'http://foo.bar/' };
fakeStore = { fakeStore = {
defaultAuthority: sinon.stub().returns('foo.com'), defaultAuthority: sinon.stub().returns('foo.com'),
getLink: sinon.stub().returns('http://example.com'),
isFeatureEnabled: sinon.stub().returns(false), isFeatureEnabled: sinon.stub().returns(false),
route: sinon.stub().returns('sidebar'), route: sinon.stub().returns('sidebar'),
setExpanded: sinon.stub(), setExpanded: sinon.stub(),
......
...@@ -16,7 +16,6 @@ import GroupListSection from './GroupListSection'; ...@@ -16,7 +16,6 @@ import GroupListSection from './GroupListSection';
/** /**
* @typedef {import('../../../types/config').MergedConfig} MergedConfig * @typedef {import('../../../types/config').MergedConfig} MergedConfig
* @typedef {import('../../../types/api').Group} Group * @typedef {import('../../../types/api').Group} Group
* @typedef {import('../../services/service-url').ServiceUrlGetter} ServiceUrlGetter
*/ */
/** /**
...@@ -30,7 +29,6 @@ function publisherProvidedIcon(settings) { ...@@ -30,7 +29,6 @@ function publisherProvidedIcon(settings) {
/** /**
* @typedef GroupListProps * @typedef GroupListProps
* @prop {ServiceUrlGetter} serviceUrl
* @prop {MergedConfig} settings * @prop {MergedConfig} settings
*/ */
...@@ -40,7 +38,7 @@ function publisherProvidedIcon(settings) { ...@@ -40,7 +38,7 @@ function publisherProvidedIcon(settings) {
* *
* @param {GroupListProps} props * @param {GroupListProps} props
*/ */
function GroupList({ serviceUrl, settings }) { function GroupList({ settings }) {
const store = useStoreProxy(); const store = useStoreProxy();
const currentGroups = store.getCurrentlyViewingGroups(); const currentGroups = store.getCurrentlyViewingGroups();
const featuredGroups = store.getFeaturedGroups(); const featuredGroups = store.getFeaturedGroups();
...@@ -65,7 +63,7 @@ function GroupList({ serviceUrl, settings }) { ...@@ -65,7 +63,7 @@ function GroupList({ serviceUrl, settings }) {
const defaultAuthority = store.defaultAuthority(); const defaultAuthority = store.defaultAuthority();
const canCreateNewGroup = const canCreateNewGroup =
userid && !isThirdPartyUser(userid, defaultAuthority); userid && !isThirdPartyUser(userid, defaultAuthority);
const newGroupLink = serviceUrl('groups.new'); const newGroupLink = store.getLink('groups.new');
// The group whose submenu is currently open, or `null` if no group item is // The group whose submenu is currently open, or `null` if no group item is
// currently expanded. // currently expanded.
...@@ -150,6 +148,6 @@ function GroupList({ serviceUrl, settings }) { ...@@ -150,6 +148,6 @@ function GroupList({ serviceUrl, settings }) {
); );
} }
GroupList.injectedProps = ['serviceUrl', 'settings']; GroupList.injectedProps = ['settings'];
export default withServices(GroupList); export default withServices(GroupList);
...@@ -7,15 +7,12 @@ import mockImportedComponents from '../../../../test-util/mock-imported-componen ...@@ -7,15 +7,12 @@ import mockImportedComponents from '../../../../test-util/mock-imported-componen
describe('GroupList', () => { describe('GroupList', () => {
let fakeServiceConfig; let fakeServiceConfig;
let fakeServiceUrl;
let fakeSettings; let fakeSettings;
let fakeStore; let fakeStore;
let testGroup; let testGroup;
function createGroupList() { function createGroupList() {
return mount( return mount(<GroupList settings={fakeSettings} />);
<GroupList serviceUrl={fakeServiceUrl} settings={fakeSettings} />
);
} }
/** /**
...@@ -46,12 +43,12 @@ describe('GroupList', () => { ...@@ -46,12 +43,12 @@ describe('GroupList', () => {
organization: { id: 'testorg', name: 'Test Org' }, organization: { id: 'testorg', name: 'Test Org' },
}; };
fakeServiceUrl = sinon.stub();
fakeSettings = {}; fakeSettings = {};
fakeStore = { fakeStore = {
defaultAuthority: sinon.stub().returns('hypothes.is'), defaultAuthority: sinon.stub().returns('hypothes.is'),
getCurrentlyViewingGroups: sinon.stub().returns([]), getCurrentlyViewingGroups: sinon.stub().returns([]),
getFeaturedGroups: sinon.stub().returns([]), getFeaturedGroups: sinon.stub().returns([]),
getLink: sinon.stub().returns(''),
getMyGroups: sinon.stub().returns([]), getMyGroups: sinon.stub().returns([]),
focusedGroup: sinon.stub().returns(testGroup), focusedGroup: sinon.stub().returns(testGroup),
profile: sinon.stub().returns({ userid: null }), profile: sinon.stub().returns({ userid: null }),
...@@ -156,7 +153,7 @@ describe('GroupList', () => { ...@@ -156,7 +153,7 @@ describe('GroupList', () => {
}); });
it('opens new window at correct URL when "New private group" is clicked', () => { it('opens new window at correct URL when "New private group" is clicked', () => {
fakeServiceUrl fakeStore.getLink
.withArgs('groups.new') .withArgs('groups.new')
.returns('https://example.com/groups/new'); .returns('https://example.com/groups/new');
fakeStore.profile.returns({ userid: 'jsmith@hypothes.is' }); fakeStore.profile.returns({ userid: 'jsmith@hypothes.is' });
......
...@@ -22,7 +22,6 @@ import TopBar from './TopBar'; ...@@ -22,7 +22,6 @@ import TopBar from './TopBar';
/** /**
* @typedef {import('../../types/api').Profile} Profile * @typedef {import('../../types/api').Profile} Profile
* @typedef {import('../services/service-url').ServiceUrlGetter} ServiceUrlGetter
* @typedef {import('../../types/config').MergedConfig} MergedConfig * @typedef {import('../../types/config').MergedConfig} MergedConfig
* @typedef {import('../../shared/bridge').default} Bridge * @typedef {import('../../shared/bridge').default} Bridge
*/ */
...@@ -55,7 +54,6 @@ function authStateFromProfile(profile) { ...@@ -55,7 +54,6 @@ function authStateFromProfile(profile) {
* @typedef HypothesisAppProps * @typedef HypothesisAppProps
* @prop {import('../services/auth').AuthService} auth * @prop {import('../services/auth').AuthService} auth
* @prop {Bridge} bridge * @prop {Bridge} bridge
* @prop {ServiceUrlGetter} serviceUrl
* @prop {MergedConfig} settings * @prop {MergedConfig} settings
* @prop {Object} session * @prop {Object} session
* @prop {import('../services/toast-messenger').ToastMessengerService} toastMessenger * @prop {import('../services/toast-messenger').ToastMessengerService} toastMessenger
...@@ -69,14 +67,7 @@ function authStateFromProfile(profile) { ...@@ -69,14 +67,7 @@ function authStateFromProfile(profile) {
* *
* @param {HypothesisAppProps} props * @param {HypothesisAppProps} props
*/ */
function HypothesisApp({ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) {
auth,
bridge,
serviceUrl,
settings,
session,
toastMessenger,
}) {
const store = useStoreProxy(); const store = useStoreProxy();
const hasFetchedProfile = store.hasFetchedProfile(); const hasFetchedProfile = store.hasFetchedProfile();
const profile = store.profile(); const profile = store.profile();
...@@ -127,7 +118,7 @@ function HypothesisApp({ ...@@ -127,7 +118,7 @@ function HypothesisApp({
bridge.call(bridgeEvents.SIGNUP_REQUESTED); bridge.call(bridgeEvents.SIGNUP_REQUESTED);
return; return;
} }
window.open(serviceUrl('signup')); window.open(store.getLink('signup'));
}; };
const promptToLogout = async () => { const promptToLogout = async () => {
...@@ -211,7 +202,6 @@ function HypothesisApp({ ...@@ -211,7 +202,6 @@ function HypothesisApp({
HypothesisApp.injectedProps = [ HypothesisApp.injectedProps = [
'auth', 'auth',
'bridge', 'bridge',
'serviceUrl',
'session', 'session',
'settings', 'settings',
'toastMessenger', 'toastMessenger',
......
import { LinkButton, SvgIcon } from '@hypothesis/frontend-shared'; import { LinkButton, SvgIcon } from '@hypothesis/frontend-shared';
import { withServices } from '../service-context'; import { useStoreProxy } from '../store/use-store';
/** @typedef {import('../services/service-url').ServiceUrlGetter} ServiceUrlGetter */
/** /**
* @typedef LoggedOutMessageProps * @typedef LoggedOutMessageProps
* @prop {() => any} onLogin * @prop {() => any} onLogin
* @prop {ServiceUrlGetter} serviceUrl
*/ */
/** /**
...@@ -17,7 +14,9 @@ import { withServices } from '../service-context'; ...@@ -17,7 +14,9 @@ import { withServices } from '../service-context';
* *
* @param {LoggedOutMessageProps} props * @param {LoggedOutMessageProps} props
*/ */
function LoggedOutMessage({ onLogin, serviceUrl }) { function LoggedOutMessage({ onLogin }) {
const store = useStoreProxy();
return ( return (
<div className="LoggedOutMessage"> <div className="LoggedOutMessage">
<span> <span>
...@@ -25,7 +24,7 @@ function LoggedOutMessage({ onLogin, serviceUrl }) { ...@@ -25,7 +24,7 @@ function LoggedOutMessage({ onLogin, serviceUrl }) {
To reply or make your own annotations on this document,{' '} To reply or make your own annotations on this document,{' '}
<a <a
className="LoggedOutMessage__link" className="LoggedOutMessage__link"
href={serviceUrl('signup')} href={store.getLink('signup')}
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
> >
...@@ -54,6 +53,4 @@ function LoggedOutMessage({ onLogin, serviceUrl }) { ...@@ -54,6 +53,4 @@ function LoggedOutMessage({ onLogin, serviceUrl }) {
); );
} }
LoggedOutMessage.injectedProps = ['serviceUrl']; export default LoggedOutMessage;
export default withServices(LoggedOutMessage);
...@@ -2,7 +2,6 @@ import { useMemo } from 'preact/hooks'; ...@@ -2,7 +2,6 @@ import { useMemo } from 'preact/hooks';
import { useStoreProxy } from '../store/use-store'; import { useStoreProxy } from '../store/use-store';
import { isThirdPartyUser } from '../helpers/account-id'; import { isThirdPartyUser } from '../helpers/account-id';
import { withServices } from '../service-context';
/** @typedef {import('../../types/api').Annotation} Annotation */ /** @typedef {import('../../types/api').Annotation} Annotation */
...@@ -10,14 +9,13 @@ import { withServices } from '../service-context'; ...@@ -10,14 +9,13 @@ import { withServices } from '../service-context';
* @typedef TagListProps * @typedef TagListProps
* @prop {Annotation} annotation - Annotation that owns the tags. * @prop {Annotation} annotation - Annotation that owns the tags.
* @prop {string[]} tags - List of tags as strings. * @prop {string[]} tags - List of tags as strings.
* @prop {(a: string, b: Object<'tag', string>) => any} serviceUrl - Services
*/ */
/** /**
* Component to render an annotation's tags. * Component to render an annotation's tags.
* @param {TagListProps} props * @param {TagListProps} props
*/ */
function TagList({ annotation, serviceUrl, tags }) { function TagList({ annotation, tags }) {
const store = useStoreProxy(); const store = useStoreProxy();
const defaultAuthority = store.defaultAuthority(); const defaultAuthority = store.defaultAuthority();
const renderLink = useMemo( const renderLink = useMemo(
...@@ -32,7 +30,7 @@ function TagList({ annotation, serviceUrl, tags }) { ...@@ -32,7 +30,7 @@ function TagList({ annotation, serviceUrl, tags }) {
* @return {string} * @return {string}
*/ */
const createTagSearchURL = tag => { const createTagSearchURL = tag => {
return serviceUrl('search.tag', { tag: tag }); return store.getLink('search.tag', { tag: tag });
}; };
return ( return (
...@@ -63,6 +61,4 @@ function TagList({ annotation, serviceUrl, tags }) { ...@@ -63,6 +61,4 @@ function TagList({ annotation, serviceUrl, tags }) {
); );
} }
TagList.injectedProps = ['serviceUrl']; export default TagList;
export default withServices(TagList);
...@@ -12,7 +12,6 @@ import MenuItem from './MenuItem'; ...@@ -12,7 +12,6 @@ import MenuItem from './MenuItem';
import MenuSection from './MenuSection'; import MenuSection from './MenuSection';
/** /**
* @typedef {import('../services/service-url').ServiceUrlGetter} ServiceUrlGetter
* @typedef {import('../../types/config').MergedConfig} MergedConfig * @typedef {import('../../types/config').MergedConfig} MergedConfig
* / * /
...@@ -28,7 +27,6 @@ import MenuSection from './MenuSection'; ...@@ -28,7 +27,6 @@ import MenuSection from './MenuSection';
* @prop {AuthState} auth - object representing authenticated user and auth status * @prop {AuthState} auth - object representing authenticated user and auth status
* @prop {() => any} onLogout - onClick callback for the "log out" button * @prop {() => any} onLogout - onClick callback for the "log out" button
* @prop {Object} bridge * @prop {Object} bridge
* @prop {ServiceUrlGetter} serviceUrl
* @prop {MergedConfig} settings * @prop {MergedConfig} settings
*/ */
...@@ -40,7 +38,7 @@ import MenuSection from './MenuSection'; ...@@ -40,7 +38,7 @@ import MenuSection from './MenuSection';
* *
* @param {UserMenuProps} props * @param {UserMenuProps} props
*/ */
function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) { function UserMenu({ auth, bridge, onLogout, settings }) {
const store = useStoreProxy(); const store = useStoreProxy();
const defaultAuthority = store.defaultAuthority(); const defaultAuthority = store.defaultAuthority();
...@@ -77,7 +75,7 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) { ...@@ -77,7 +75,7 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) {
const props = {}; const props = {};
if (isSelectableProfile) { if (isSelectableProfile) {
if (!isThirdParty) { if (!isThirdParty) {
props.href = serviceUrl('user', { user: auth.username }); props.href = store.getLink('user', { user: auth.username });
} }
props.onClick = onProfileSelected; props.onClick = onProfileSelected;
} }
...@@ -109,7 +107,7 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) { ...@@ -109,7 +107,7 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) {
{!isThirdParty && ( {!isThirdParty && (
<MenuItem <MenuItem
label="Account settings" label="Account settings"
href={serviceUrl('account.settings')} href={store.getLink('account.settings')}
/> />
)} )}
{isNotebookEnabled && ( {isNotebookEnabled && (
...@@ -129,6 +127,6 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) { ...@@ -129,6 +127,6 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) {
); );
} }
UserMenu.injectedProps = ['bridge', 'serviceUrl', 'settings']; UserMenu.injectedProps = ['bridge', 'settings'];
export default withServices(UserMenu); export default withServices(UserMenu);
...@@ -14,7 +14,6 @@ describe('HypothesisApp', () => { ...@@ -14,7 +14,6 @@ describe('HypothesisApp', () => {
let fakeServiceConfig = null; let fakeServiceConfig = null;
let fakeSession = null; let fakeSession = null;
let fakeShouldAutoDisplayTutorial = null; let fakeShouldAutoDisplayTutorial = null;
let fakeServiceUrl = null;
let fakeSettings = null; let fakeSettings = null;
let fakeToastMessenger = null; let fakeToastMessenger = null;
...@@ -23,7 +22,6 @@ describe('HypothesisApp', () => { ...@@ -23,7 +22,6 @@ describe('HypothesisApp', () => {
<HypothesisApp <HypothesisApp
auth={fakeAuth} auth={fakeAuth}
bridge={fakeBridge} bridge={fakeBridge}
serviceUrl={fakeServiceUrl}
settings={fakeSettings} settings={fakeSettings}
session={fakeSession} session={fakeSession}
toastMessenger={fakeToastMessenger} toastMessenger={fakeToastMessenger}
...@@ -55,6 +53,8 @@ describe('HypothesisApp', () => { ...@@ -55,6 +53,8 @@ describe('HypothesisApp', () => {
}, },
}), }),
route: sinon.stub().returns('sidebar'), route: sinon.stub().returns('sidebar'),
getLink: sinon.stub(),
}; };
fakeAuth = {}; fakeAuth = {};
...@@ -65,8 +65,6 @@ describe('HypothesisApp', () => { ...@@ -65,8 +65,6 @@ describe('HypothesisApp', () => {
reload: sinon.stub().returns(Promise.resolve({ userid: null })), reload: sinon.stub().returns(Promise.resolve({ userid: null })),
}; };
fakeServiceUrl = sinon.stub();
fakeSettings = {}; fakeSettings = {};
fakeBridge = { fakeBridge = {
...@@ -244,7 +242,9 @@ describe('HypothesisApp', () => { ...@@ -244,7 +242,9 @@ describe('HypothesisApp', () => {
context('when not using a third-party service', () => { context('when not using a third-party service', () => {
it('opens the signup URL in a new tab', () => { it('opens the signup URL in a new tab', () => {
fakeServiceUrl.withArgs('signup').returns('https://ann.service/signup'); fakeStore.getLink
.withArgs('signup')
.returns('https://ann.service/signup');
const wrapper = createComponent(); const wrapper = createComponent();
clickSignUp(wrapper); clickSignUp(wrapper);
assert.calledWith(window.open, 'https://ann.service/signup'); assert.calledWith(window.open, 'https://ann.service/signup');
......
...@@ -7,18 +7,21 @@ import { checkAccessibility } from '../../../test-util/accessibility'; ...@@ -7,18 +7,21 @@ import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components'; import mockImportedComponents from '../../../test-util/mock-imported-components';
describe('LoggedOutMessage', () => { describe('LoggedOutMessage', () => {
let fakeStore;
const createLoggedOutMessage = props => { const createLoggedOutMessage = props => {
return mount( return mount(<LoggedOutMessage onLogin={sinon.stub()} {...props} />);
<LoggedOutMessage
onLogin={sinon.stub()}
serviceUrl={sinon.stub()}
{...props}
/>
);
}; };
beforeEach(() => { beforeEach(() => {
fakeStore = {
getLink: sinon.stub().returns('signup_link'),
};
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({
'../store/use-store': { useStoreProxy: () => fakeStore },
});
}); });
afterEach(() => { afterEach(() => {
...@@ -26,12 +29,11 @@ describe('LoggedOutMessage', () => { ...@@ -26,12 +29,11 @@ describe('LoggedOutMessage', () => {
}); });
it('should link to signup', () => { it('should link to signup', () => {
const fakeServiceUrl = sinon.stub().returns('signup_link'); const wrapper = createLoggedOutMessage();
const wrapper = createLoggedOutMessage({ serviceUrl: fakeServiceUrl });
const signupLink = wrapper.find('.LoggedOutMessage__link').at(0); const signupLink = wrapper.find('.LoggedOutMessage__link').at(0);
assert.calledWith(fakeServiceUrl, 'signup'); assert.calledWith(fakeStore.getLink, 'signup');
assert.equal(signupLink.prop('href'), 'signup_link'); assert.equal(signupLink.prop('href'), 'signup_link');
}); });
......
...@@ -7,30 +7,20 @@ import { checkAccessibility } from '../../../test-util/accessibility'; ...@@ -7,30 +7,20 @@ import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components'; import mockImportedComponents from '../../../test-util/mock-imported-components';
describe('TagList', () => { describe('TagList', () => {
let fakeServiceUrl;
let fakeIsThirdPartyUser; let fakeIsThirdPartyUser;
let fakeStore; let fakeStore;
const fakeTags = ['tag1', 'tag2']; const fakeTags = ['tag1', 'tag2'];
function createComponent(props) { function createComponent(props) {
return mount( return mount(<TagList annotation={{}} tags={fakeTags} {...props} />);
<TagList
// props
annotation={{}}
tags={fakeTags}
// service props
serviceUrl={fakeServiceUrl}
{...props}
/>
);
} }
beforeEach(() => { beforeEach(() => {
fakeServiceUrl = sinon.stub().returns('http://serviceurl.com');
fakeIsThirdPartyUser = sinon.stub().returns(false); fakeIsThirdPartyUser = sinon.stub().returns(false);
fakeStore = { fakeStore = {
defaultAuthority: sinon.stub().returns('hypothes.is'), defaultAuthority: sinon.stub().returns('hypothes.is'),
getLink: sinon.stub().returns('http://serviceurl.com'),
}; };
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
...@@ -66,10 +56,10 @@ describe('TagList', () => { ...@@ -66,10 +56,10 @@ describe('TagList', () => {
}); });
}); });
it('calls fakeServiceUrl()', () => { it('gets the links for tags', () => {
createComponent(); createComponent();
assert.calledWith(fakeServiceUrl, 'search.tag', { tag: 'tag1' }); assert.calledWith(fakeStore.getLink, 'search.tag', { tag: 'tag1' });
assert.calledWith(fakeServiceUrl, 'search.tag', { tag: 'tag2' }); assert.calledWith(fakeStore.getLink, 'search.tag', { tag: 'tag2' });
}); });
}); });
...@@ -87,9 +77,9 @@ describe('TagList', () => { ...@@ -87,9 +77,9 @@ describe('TagList', () => {
}); });
}); });
it('does not call fakeServiceUrl()', () => { it('does not fetch tag link', () => {
createComponent(); createComponent();
assert.notCalled(fakeServiceUrl); assert.notCalled(fakeStore.getLink);
}); });
}); });
......
...@@ -13,7 +13,6 @@ describe('UserMenu', () => { ...@@ -13,7 +13,6 @@ describe('UserMenu', () => {
let fakeIsThirdPartyUser; let fakeIsThirdPartyUser;
let fakeOnLogout; let fakeOnLogout;
let fakeServiceConfig; let fakeServiceConfig;
let fakeServiceUrl;
let fakeSettings; let fakeSettings;
let fakeStore; let fakeStore;
...@@ -23,7 +22,6 @@ describe('UserMenu', () => { ...@@ -23,7 +22,6 @@ describe('UserMenu', () => {
auth={fakeAuth} auth={fakeAuth}
bridge={fakeBridge} bridge={fakeBridge}
onLogout={fakeOnLogout} onLogout={fakeOnLogout}
serviceUrl={fakeServiceUrl}
settings={fakeSettings} settings={fakeSettings}
/> />
); );
...@@ -46,11 +44,11 @@ describe('UserMenu', () => { ...@@ -46,11 +44,11 @@ describe('UserMenu', () => {
fakeIsThirdPartyUser = sinon.stub(); fakeIsThirdPartyUser = sinon.stub();
fakeOnLogout = sinon.stub(); fakeOnLogout = sinon.stub();
fakeServiceConfig = sinon.stub(); fakeServiceConfig = sinon.stub();
fakeServiceUrl = sinon.stub();
fakeSettings = {}; fakeSettings = {};
fakeStore = { fakeStore = {
defaultAuthority: sinon.stub().returns('hypothes.is'), defaultAuthority: sinon.stub().returns('hypothes.is'),
focusedGroupId: sinon.stub().returns('mygroup'), focusedGroupId: sinon.stub().returns('mygroup'),
getLink: sinon.stub(),
isFeatureEnabled: sinon.stub().returns(false), isFeatureEnabled: sinon.stub().returns(false),
}; };
...@@ -72,7 +70,7 @@ describe('UserMenu', () => { ...@@ -72,7 +70,7 @@ describe('UserMenu', () => {
context('first-party user', () => { context('first-party user', () => {
beforeEach(() => { beforeEach(() => {
fakeIsThirdPartyUser.returns(false); fakeIsThirdPartyUser.returns(false);
fakeServiceUrl.returns('profile-link'); fakeStore.getLink.returns('profile-link');
}); });
it('should be enabled', () => { it('should be enabled', () => {
...@@ -174,7 +172,7 @@ describe('UserMenu', () => { ...@@ -174,7 +172,7 @@ describe('UserMenu', () => {
const accountMenuItem = findMenuItem(wrapper, 'Account settings'); const accountMenuItem = findMenuItem(wrapper, 'Account settings');
assert.isTrue(accountMenuItem.exists()); assert.isTrue(accountMenuItem.exists());
assert.calledWith(fakeServiceUrl, 'account.settings'); assert.calledWith(fakeStore.getLink, 'account.settings');
}); });
it('should not be present if third-party user', () => { it('should not be present if third-party user', () => {
......
...@@ -65,12 +65,19 @@ function setupRoute(groups, session, router) { ...@@ -65,12 +65,19 @@ function setupRoute(groups, session, router) {
* @param {import('./services/autosave').AutosaveService} autosaveService * @param {import('./services/autosave').AutosaveService} autosaveService
* @param {import('./services/features').FeaturesService} features * @param {import('./services/features').FeaturesService} features
* @param {import('./services/persisted-defaults').PersistedDefaultsService} persistedDefaults * @param {import('./services/persisted-defaults').PersistedDefaultsService} persistedDefaults
* @param {import('./services/service-url').ServiceURLService} serviceURL
* @inject * @inject
*/ */
function initServices(autosaveService, features, persistedDefaults) { function initServices(
autosaveService,
features,
persistedDefaults,
serviceURL
) {
autosaveService.init(); autosaveService.init();
features.init(); features.init();
persistedDefaults.init(); persistedDefaults.init();
serviceURL.init();
} }
// @inject // @inject
...@@ -107,7 +114,7 @@ import loadAnnotationsService from './services/load-annotations'; ...@@ -107,7 +114,7 @@ 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';
import { RouterService } from './services/router'; import { RouterService } from './services/router';
import serviceUrlService from './services/service-url'; import { ServiceURLService } from './services/service-url';
import sessionService from './services/session'; import sessionService from './services/session';
import { StreamFilter } from './services/stream-filter'; import { StreamFilter } from './services/stream-filter';
import streamerService from './services/streamer'; import streamerService from './services/streamer';
...@@ -145,7 +152,7 @@ function startApp(config, appEl) { ...@@ -145,7 +152,7 @@ function startApp(config, appEl) {
.register('localStorage', LocalStorageService) .register('localStorage', LocalStorageService)
.register('persistedDefaults', PersistedDefaultsService) .register('persistedDefaults', PersistedDefaultsService)
.register('router', RouterService) .register('router', RouterService)
.register('serviceUrl', serviceUrlService) .register('serviceURL', ServiceURLService)
.register('session', sessionService) .register('session', sessionService)
.register('streamer', streamerService) .register('streamer', streamerService)
.register('streamFilter', StreamFilter) .register('streamFilter', StreamFilter)
......
...@@ -29,7 +29,6 @@ const DEFAULT_ORGANIZATION = { ...@@ -29,7 +29,6 @@ const DEFAULT_ORGANIZATION = {
export default function groups( export default function groups(
store, store,
api, api,
serviceUrl,
session, session,
settings, settings,
toastMessenger, toastMessenger,
......
import * as urlUtil from '../util/url';
/** /**
* A function that returns an absolute URL given a link name and params, by * Service for fetching the data needed to render URLs that point to the H
* expanding named URL templates received from the annotation service's API. * service.
*
* The links object from the API is a map of link names to URL templates:
* *
* { * The H API has an `/api/links` endpoint that returns a map of link name to
* signup: "http://localhost:5000/signup", * URL template for URLs that point to the H API. This service fetches that
* user: "http://localhost:5000/u/:user", * data and persists it in the store.
* ...
* }
* *
* Given a link name (e.g. 'user') and params (e.g. {user: 'bob'}) return * To use a link within a UI component, use `store.getLink(name, params)`.
* an absolute URL by expanding the named URL template from the API with the
* given params (e.g. "http://localhost:5000/u/bob").
* *
* Before the links object has been received from the API this function
* always returns empty strings as the URLs. After the links object has been
* received from the API this function starts returning the real URLs.
*
* @callback ServiceUrlGetter
* @param {string} linkName - The name of the link to expand
* @param {object} [params] - The params with which to expand the link
* @returns {string} The expanded absolute URL, or an empty string if the
* links haven't been received from the API yet
* @throws {Error} If the links have been received from the API but the given
* linkName is unknown
* @throws {Error} If one or more of the params given isn't used in the URL
* template
*/
/**
* @param {import('../store').SidebarStore} store
* @param {import('./api-routes').APIRoutesService} apiRoutes
* @return {ServiceUrlGetter}
* @inject * @inject
*/ */
export default function serviceUrl(store, apiRoutes) { export class ServiceURLService {
apiRoutes /**
.links() * @param {import('./api-routes').APIRoutesService} apiRoutes
.then(store.updateLinks) * @param {import('../store').SidebarStore} store
.catch(function (error) { */
console.warn('The links API request was rejected: ' + error.message); constructor(apiRoutes, store) {
}); this._apiRoutes = apiRoutes;
this._store = store;
return function (linkName, params) { }
const links = store.getState().links;
/**
if (links === null) { * Fetch URL templates for links from the API and persist them in the store.
return ''; */
async init() {
try {
const links = await this._apiRoutes.links();
this._store.updateLinks(links);
} catch (error) {
console.warn('Failed to fetch Hypothesis links: ' + error.message);
} }
}
const path = links[linkName];
if (!path) {
throw new Error('Unknown link ' + linkName);
}
params = params || {};
const url = urlUtil.replaceURLParams(path, params);
const unused = Object.keys(url.params);
if (unused.length > 0) {
throw new Error('Unknown link parameters: ' + unused.join(', '));
}
return url.url;
};
} }
...@@ -43,7 +43,6 @@ describe('sidebar/services/groups', function () { ...@@ -43,7 +43,6 @@ describe('sidebar/services/groups', function () {
let fakeSession; let fakeSession;
let fakeSettings; let fakeSettings;
let fakeApi; let fakeApi;
let fakeServiceUrl;
let fakeMetadata; let fakeMetadata;
let fakeToastMessenger; let fakeToastMessenger;
...@@ -120,7 +119,6 @@ describe('sidebar/services/groups', function () { ...@@ -120,7 +119,6 @@ describe('sidebar/services/groups', function () {
}, },
}, },
}; };
fakeServiceUrl = sinon.stub();
fakeSettings = { group: null }; fakeSettings = { group: null };
$imports.$mock({ $imports.$mock({
...@@ -136,7 +134,6 @@ describe('sidebar/services/groups', function () { ...@@ -136,7 +134,6 @@ describe('sidebar/services/groups', function () {
return groups( return groups(
fakeStore, fakeStore,
fakeApi, fakeApi,
fakeServiceUrl,
fakeSession, fakeSession,
fakeSettings, fakeSettings,
fakeToastMessenger, fakeToastMessenger,
......
import serviceUrlFactory from '../service-url'; import { ServiceURLService } from '../service-url';
import { $imports } from '../service-url';
/** Return a fake store object. */ const links = {
function fakeStore() { 'account.settings': 'https://hypothes.is/account/settings',
let links = null; };
return {
updateLinks: function (newLinks) {
links = newLinks;
},
getState: function () {
return { links: links };
},
};
}
function createServiceUrl(linksPromise) { describe('ServiceURLService', () => {
const replaceURLParams = sinon let fakeStore;
.stub() let fakeAPIRoutes;
.returns({ url: 'EXPANDED_URL', params: {} });
$imports.$mock({ beforeEach(() => {
'../util/url': { replaceURLParams: replaceURLParams }, fakeStore = {
}); updateLinks: sinon.stub(),
};
const store = fakeStore(); fakeAPIRoutes = {
links: sinon.stub().resolves(links),
const apiRoutes = { };
links: sinon.stub().returns(linksPromise),
};
return {
store: store,
apiRoutes,
serviceUrl: serviceUrlFactory(store, apiRoutes),
replaceURLParams: replaceURLParams,
};
}
describe('sidebar.service-url', function () {
beforeEach(function () {
sinon.stub(console, 'warn'); sinon.stub(console, 'warn');
}); });
afterEach(function () { afterEach(() => {
console.warn.restore(); console.warn.restore();
$imports.$restore();
}); });
context('before the API response has been received', function () { describe('#init', () => {
let serviceUrl; it('fetches links and updates store with response', async () => {
let apiRoutes; const service = new ServiceURLService(fakeAPIRoutes, fakeStore);
beforeEach(function () { await service.init();
// Create a serviceUrl function with an unresolved Promise that will
// never be resolved - it never receives the links from store.links().
const parts = createServiceUrl(new Promise(function () {}));
serviceUrl = parts.serviceUrl;
apiRoutes = parts.apiRoutes;
});
it('sends one API request for the links at boot time', function () {
assert.calledOnce(apiRoutes.links);
assert.isTrue(apiRoutes.links.calledWithExactly());
});
it('returns an empty string for any link', function () { assert.calledWith(fakeStore.updateLinks, links);
assert.equal(serviceUrl('foo'), '');
}); });
it('returns an empty string even if link params are given', function () { it('logs a warning if links cannot be fetched', async () => {
assert.equal(serviceUrl('foo', { bar: 'bar' }), ''); fakeAPIRoutes.links.returns(Promise.reject(new Error('Fetch failed')));
}); const service = new ServiceURLService(fakeAPIRoutes, fakeStore);
});
context('after the API response has been received', function () {
let store;
let linksPromise;
let replaceURLParams;
let serviceUrl;
beforeEach(function () {
// The links Promise that store.links() will return.
linksPromise = Promise.resolve({
first_link: 'http://example.com/first_page/:foo',
second_link: 'http://example.com/second_page',
});
const parts = createServiceUrl(linksPromise);
store = parts.store;
serviceUrl = parts.serviceUrl;
replaceURLParams = parts.replaceURLParams;
});
it('updates store with the real links', function () {
return linksPromise.then(function (links) {
assert.deepEqual(store.getState(), { links: links });
});
});
it('calls replaceURLParams with the path and given params', function () {
return linksPromise.then(function () {
const params = { foo: 'bar' };
serviceUrl('first_link', params);
assert.calledOnce(replaceURLParams);
assert.deepEqual(replaceURLParams.args[0], [
'http://example.com/first_page/:foo',
params,
]);
});
});
it('passes an empty params object to replaceURLParams if no params are given', function () {
return linksPromise.then(function () {
serviceUrl('first_link');
assert.calledOnce(replaceURLParams);
assert.deepEqual(replaceURLParams.args[0][1], {});
});
});
it('returns the expanded URL from replaceURLParams', function () {
return linksPromise.then(function () {
const renderedUrl = serviceUrl('first_link');
assert.equal(renderedUrl, 'EXPANDED_URL');
});
});
it("throws an error if it doesn't have the requested link", function () {
return linksPromise.then(function () {
assert.throws(
function () {
serviceUrl('madeUpLinkName');
},
Error,
'Unknown link madeUpLinkName'
);
});
});
it('throws an error if replaceURLParams returns unused params', function () { await service.init();
const params = { unused_param_1: 'foo', unused_param_2: 'bar' };
replaceURLParams.returns({
url: 'EXPANDED_URL',
params: params,
});
return linksPromise.then(function () { assert.notCalled(fakeStore.updateLinks);
assert.throws( assert.calledWith(
function () { console.warn,
serviceUrl('first_link', params); 'Failed to fetch Hypothesis links: Fetch failed'
}, );
Error,
'Unknown link parameters: unused_param_1, unused_param_2'
);
});
}); });
}); });
}); });
import { storeModule } from '../create-store';
import { actionTypes } from '../util'; import { actionTypes } from '../util';
import { replaceURLParams } from '../../util/url';
import { storeModule } from '../create-store';
/**
* Reducer for storing a "links" object in the Redux state store.
*
* The links object is initially null, and can only be updated by completely
* replacing it with a new links object.
*
* Used by serviceUrl.
*/
/** Return the initial links. */
function init() { function init() {
return null; return null;
} }
...@@ -26,7 +16,11 @@ const update = { ...@@ -26,7 +16,11 @@ const update = {
const actions = actionTypes(update); const actions = actionTypes(update);
/** Return updated links based on the given current state and action object. */ /**
* Update links
*
* @param {object} newLinks - Link map returned by the `/api/links` endpoint
*/
function updateLinks(newLinks) { function updateLinks(newLinks) {
return { return {
type: actions.UPDATE_LINKS, type: actions.UPDATE_LINKS,
...@@ -34,6 +28,32 @@ function updateLinks(newLinks) { ...@@ -34,6 +28,32 @@ function updateLinks(newLinks) {
}; };
} }
/**
* Render a service link (URL) using the given `params`
*
* Returns an empty string if links have not been fetched yet.
*
* @param {string} linkName
* @param {Record<string,string>} params
* @return {string}
*/
function getLink(state, linkName, params = {}) {
if (!state) {
return '';
}
const template = state[linkName];
if (!template) {
throw new Error(`Unknown link "${linkName}"`);
}
const { url, params: unusedParams } = replaceURLParams(template, params);
if (Object.keys(unusedParams).length > 0) {
throw new Error(
`Unused parameters: ${Object.keys(unusedParams).join(', ')}`
);
}
return url;
}
export default storeModule({ export default storeModule({
init, init,
namespace: 'links', namespace: 'links',
...@@ -41,5 +61,7 @@ export default storeModule({ ...@@ -41,5 +61,7 @@ export default storeModule({
actions: { actions: {
updateLinks, updateLinks,
}, },
selectors: {}, selectors: {
getLink,
},
}); });
import createStore from '../../create-store';
import links from '../links'; import links from '../links';
const init = links.init; describe('sidebar/store/modules/links', () => {
const update = links.update.UPDATE_LINKS; let store;
const action = links.actions.updateLinks;
describe('sidebar/store/modules/links', function () { beforeEach(() => {
describe('#init()', function () { store = createStore([links]);
it('returns a null links object', function () {
assert.deepEqual(init(), null);
});
}); });
describe('#update.UPDATE_LINKS()', function () { function addLinks() {
it('returns the given newLinks as the links object', function () { // Snapshot of response from https://hypothes.is/api/links.
assert.deepEqual( const data = {
update('CURRENT_STATE', { newLinks: { NEW_LINK: 'http://new_link' } }), 'account.settings': 'https://hypothes.is/account/settings',
{ NEW_LINK: 'http://new_link' } 'forgot-password': 'https://hypothes.is/forgot-password',
'groups.new': 'https://hypothes.is/groups/new',
help: 'https://hypothes.is/docs/help',
'oauth.authorize': 'https://hypothes.is/oauth/authorize',
'oauth.revoke': 'https://hypothes.is/oauth/revoke',
'search.tag': 'https://hypothes.is/search?q=tag%3A%22:tag%22',
signup: 'https://hypothes.is/signup',
user: 'https://hypothes.is/u/:user',
};
store.updateLinks(data);
}
describe('#getLink', () => {
it('returns an empty string before links are loaded', () => {
assert.equal(store.getLink('account.settings'), '');
});
it('renders URLs once links are loaded', () => {
addLinks();
assert.equal(
store.getLink('account.settings'),
'https://hypothes.is/account/settings'
); );
}); });
});
describe('#actions.updateLinks()', function () { it('renders URLs with parameters', () => {
it('returns an UPDATE_LINKS action object for the given newLinks', function () { addLinks();
assert.deepEqual(action({ NEW_LINK: 'http://new_link' }), { assert.equal(
type: 'UPDATE_LINKS', store.getLink('user', { user: 'foobar' }),
newLinks: { NEW_LINK: 'http://new_link' }, 'https://hypothes.is/u/foobar'
}); );
});
it('throws an error if link name is invalid', () => {
addLinks();
assert.throws(() => {
store.getLink('unknown');
}, 'Unknown link "unknown"');
});
it('throws an error if unused link parameters are provided', () => {
addLinks();
assert.throws(() => {
store.getLink('account.settings', { unused: 'foo', unused2: 'bar' });
}, 'Unused parameters: unused, unused2');
}); });
}); });
}); });
...@@ -8,10 +8,11 @@ ...@@ -8,10 +8,11 @@
* {url: '/things/foo', params: {q: 'bar'}} * {url: '/things/foo', params: {q: 'bar'}}
* *
* @param {string} url * @param {string} url
* @param {Record<string, any>} params * @param {Record<string, string>} params
* @return {{ url: string, params: Record<string, any>}} * @return {{ url: string, params: Record<string, string>}}
*/ */
export function replaceURLParams(url, params) { export function replaceURLParams(url, params) {
/** @type {Record<string, string>} */
const unusedParams = {}; const unusedParams = {};
for (const param in params) { for (const param in params) {
if (params.hasOwnProperty(param)) { if (params.hasOwnProperty(param)) {
......
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