Unverified Commit cc081d11 authored by Robert Knight's avatar Robert Knight Committed by GitHub

Merge pull request #2185 from hypothesis/remove-oauth-tokens-changed-event

Remove `OAUTH_TOKENS_CHANGED` event
parents db3f3f17 5b140712
/**
* This module defines the set of global events that are dispatched
* on $rootScope
*/
export default {
// Session state changes
/**
* API tokens were fetched and saved to local storage by another client
* instance.
*/
OAUTH_TOKENS_CHANGED: 'oauthTokensChanged',
};
...@@ -138,7 +138,6 @@ import store from './store'; ...@@ -138,7 +138,6 @@ import store from './store';
// Utilities. // Utilities.
import { Injector } from '../shared/injector'; import { Injector } from '../shared/injector';
import EventEmitter from 'tiny-emitter';
function startApp(config) { function startApp(config) {
const container = new Injector(); const container = new Injector();
...@@ -172,15 +171,6 @@ function startApp(config) { ...@@ -172,15 +171,6 @@ function startApp(config) {
.register('viewFilter', viewFilterService) .register('viewFilter', viewFilterService)
.register('store', store); .register('store', store);
// Register a dummy `$rootScope` pub-sub service for services that still
// use it.
const emitter = new EventEmitter();
const dummyRootScope = {
$on: (event, callback) => emitter.on(event, data => callback({}, data)),
$broadcast: (event, data) => emitter.emit(event, data),
};
container.register('$rootScope', { value: dummyRootScope });
// Register utility values/classes. // Register utility values/classes.
// //
// nb. In many cases these can be replaced by direct imports in the services // nb. In many cases these can be replaced by direct imports in the services
......
import events from '../events'; import EventEmitter from 'tiny-emitter';
import serviceConfig from '../service-config'; import serviceConfig from '../service-config';
import OAuthClient from '../util/oauth-client'; import OAuthClient from '../util/oauth-client';
import { resolve } from '../util/url'; import { resolve } from '../util/url';
...@@ -23,7 +24,6 @@ import { resolve } from '../util/url'; ...@@ -23,7 +24,6 @@ import { resolve } from '../util/url';
* the `OAuthClient` class. * the `OAuthClient` class.
*/ */
export default function auth( export default function auth(
$rootScope,
$window, $window,
apiRoutes, apiRoutes,
localStorage, localStorage,
...@@ -127,6 +127,8 @@ export default function auth( ...@@ -127,6 +127,8 @@ export default function auth(
}); });
} }
const emitter = new EventEmitter();
/** /**
* Listen for updated access & refresh tokens saved by other instances of the * Listen for updated access & refresh tokens saved by other instances of the
* client. * client.
...@@ -137,7 +139,7 @@ export default function auth( ...@@ -137,7 +139,7 @@ export default function auth(
// Reset cached token information. Tokens will be reloaded from storage // Reset cached token information. Tokens will be reloaded from storage
// on the next call to `tokenGetter()`. // on the next call to `tokenGetter()`.
tokenInfoPromise = null; tokenInfoPromise = null;
$rootScope.$broadcast(events.OAUTH_TOKENS_CHANGED); emitter.emit('oauthTokensChanged');
} }
}); });
} }
...@@ -290,18 +292,19 @@ export default function auth( ...@@ -290,18 +292,19 @@ export default function auth(
listenForTokenStorageEvents(); listenForTokenStorageEvents();
return { // The returned object `extends` the EventEmitter. We could rework this
// service to be a class in future to do this more explicitly.
return Object.assign(emitter, {
login, login,
logout, logout,
tokenGetter, tokenGetter,
}; });
} }
// `$inject` is added manually rather than using `@ngInject` to work around // `$inject` is added manually rather than using `@ngInject` to work around
// a conflict between the transform-async-to-promises and angularjs-annotate // a conflict between the transform-async-to-promises and angularjs-annotate
// Babel plugins. // Babel plugins.
auth.$inject = [ auth.$inject = [
'$rootScope',
'$window', '$window',
'apiRoutes', 'apiRoutes',
'localStorage', 'localStorage',
......
import events from '../events';
import serviceConfig from '../service-config'; import serviceConfig from '../service-config';
import * as retryUtil from '../util/retry'; import * as retryUtil from '../util/retry';
import * as sentry from '../util/sentry'; import * as sentry from '../util/sentry';
...@@ -21,7 +20,6 @@ const CACHE_TTL = 5 * 60 * 1000; // 5 minutes ...@@ -21,7 +20,6 @@ const CACHE_TTL = 5 * 60 * 1000; // 5 minutes
* @ngInject * @ngInject
*/ */
export default function session( export default function session(
$rootScope,
analytics, analytics,
store, store,
api, api,
...@@ -160,7 +158,7 @@ export default function session( ...@@ -160,7 +158,7 @@ export default function session(
return load(); return load();
} }
$rootScope.$on(events.OAUTH_TOKENS_CHANGED, () => { auth.on('oauthTokensChanged', () => {
reload(); reload();
}); });
......
import events from '../../events';
import { Injector } from '../../../shared/injector'; import { Injector } from '../../../shared/injector';
import FakeWindow from '../../util/test/fake-window'; import FakeWindow from '../../util/test/fake-window';
import authFactory, { $imports } from '../oauth-auth'; import authFactory, { $imports } from '../oauth-auth';
...@@ -6,8 +5,7 @@ import authFactory, { $imports } from '../oauth-auth'; ...@@ -6,8 +5,7 @@ import authFactory, { $imports } from '../oauth-auth';
const DEFAULT_TOKEN_EXPIRES_IN_SECS = 1000; const DEFAULT_TOKEN_EXPIRES_IN_SECS = 1000;
const TOKEN_KEY = 'hypothesis.oauth.hypothes%2Eis.token'; const TOKEN_KEY = 'hypothesis.oauth.hypothes%2Eis.token';
describe('sidebar.oauth-auth', function () { describe('sidebar/services/oauth-auth', function () {
let $rootScope;
let FakeOAuthClient; let FakeOAuthClient;
let auth; let auth;
let nowStub; let nowStub;
...@@ -92,10 +90,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -92,10 +90,7 @@ describe('sidebar.oauth-auth', function () {
'../util/oauth-client': FakeOAuthClient, '../util/oauth-client': FakeOAuthClient,
}); });
$rootScope = { $broadcast: sinon.stub() };
auth = new Injector() auth = new Injector()
.register('$rootScope', { value: $rootScope })
.register('$window', { value: fakeWindow }) .register('$window', { value: fakeWindow })
.register('apiRoutes', { value: fakeApiRoutes }) .register('apiRoutes', { value: fakeApiRoutes })
.register('localStorage', { value: fakeLocalStorage }) .register('localStorage', { value: fakeLocalStorage })
...@@ -516,9 +511,12 @@ describe('sidebar.oauth-auth', function () { ...@@ -516,9 +511,12 @@ describe('sidebar.oauth-auth', function () {
}); });
it('notifies other services about the change', () => { it('notifies other services about the change', () => {
const tokenChanged = sinon.stub();
auth.on('oauthTokensChanged', tokenChanged);
notifyStoredTokenChange(); notifyStoredTokenChange();
assert.calledWith($rootScope.$broadcast, events.OAUTH_TOKENS_CHANGED); assert.called(tokenChanged);
}); });
}); });
......
import EventEmitter from 'tiny-emitter'; import EventEmitter from 'tiny-emitter';
import events from '../../events';
import { events as analyticsEvents } from '../analytics'; import { events as analyticsEvents } from '../analytics';
import sessionFactory from '../session'; import sessionFactory from '../session';
import { $imports } from '../session'; import { $imports } from '../session';
import { Injector } from '../../../shared/injector'; import { Injector } from '../../../shared/injector';
describe('sidebar/services/session', function () { describe('sidebar/services/session', function () {
let $rootScope;
let fakeAnalytics; let fakeAnalytics;
let fakeAuth; let fakeAuth;
let fakeSentry; let fakeSentry;
...@@ -39,10 +36,10 @@ describe('sidebar/services/session', function () { ...@@ -39,10 +36,10 @@ describe('sidebar/services/session', function () {
currentProfile = newProfile; currentProfile = newProfile;
}), }),
}; };
fakeAuth = { fakeAuth = Object.assign(new EventEmitter(), {
login: sandbox.stub().returns(Promise.resolve()), login: sandbox.stub().returns(Promise.resolve()),
logout: sinon.stub().resolves(), logout: sinon.stub().resolves(),
}; });
fakeSentry = { fakeSentry = {
setUserInfo: sandbox.spy(), setUserInfo: sandbox.spy(),
}; };
...@@ -63,14 +60,7 @@ describe('sidebar/services/session', function () { ...@@ -63,14 +60,7 @@ describe('sidebar/services/session', function () {
'../util/sentry': fakeSentry, '../util/sentry': fakeSentry,
}); });
const emitter = new EventEmitter();
$rootScope = {
$on: (event, cb) => emitter.on(event, data => cb(null, data)),
$broadcast: (event, data) => emitter.emit(event, data),
};
session = new Injector() session = new Injector()
.register('$rootScope', { value: $rootScope })
.register('analytics', { value: fakeAnalytics }) .register('analytics', { value: fakeAnalytics })
.register('store', { value: fakeStore }) .register('store', { value: fakeStore })
.register('api', { value: fakeApi }) .register('api', { value: fakeApi })
...@@ -328,7 +318,7 @@ describe('sidebar/services/session', function () { ...@@ -328,7 +318,7 @@ describe('sidebar/services/session', function () {
userid: 'acct:different_user@hypothes.is', userid: 'acct:different_user@hypothes.is',
}) })
); );
$rootScope.$broadcast(events.OAUTH_TOKENS_CHANGED); fakeAuth.emit('oauthTokensChanged');
fakeStore.updateProfile.resetHistory(); fakeStore.updateProfile.resetHistory();
return session.load(); return session.load();
......
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