Commit ef9426db authored by Robert Knight's avatar Robert Knight

Add missing types to session store module

This found apparent missing checks for a null `profile.userid` (ie. current user
is logged out) in several places. These places should not be reachable
when the user is logged out due to checks elsewhere, but add explicit
checks for the user being logged in.
parent 2269088f
...@@ -88,7 +88,7 @@ function Annotation({ ...@@ -88,7 +88,7 @@ function Annotation({
!isReply && !isEditing && !hasAppliedFilter && replyCount > 0; !isReply && !isEditing && !hasAppliedFilter && replyCount > 0;
const onReply = () => { const onReply = () => {
if (annotation && isSaved(annotation)) { if (annotation && isSaved(annotation) && userid) {
annotationsService.reply(annotation, userid); annotationsService.reply(annotation, userid);
} }
}; };
......
...@@ -22,7 +22,7 @@ export function parseAccountID(user) { ...@@ -22,7 +22,7 @@ export function parseAccountID(user) {
/** /**
* Returns the username part of an account ID or an empty string. * Returns the username part of an account ID or an empty string.
* *
* @param {string} user * @param {string|null} user
*/ */
export function username(user) { export function username(user) {
const account = parseAccountID(user); const account = parseAccountID(user);
......
...@@ -68,6 +68,10 @@ export class AnnotationsService { ...@@ -68,6 +68,10 @@ export class AnnotationsService {
} }
const userid = profile.userid; const userid = profile.userid;
if (!userid) {
throw new Error('Cannot create annotation when logged out');
}
const userInfo = profile.user_info; const userInfo = profile.user_info;
// We need a unique local/app identifier for this new annotation such // We need a unique local/app identifier for this new annotation such
......
...@@ -14,6 +14,14 @@ describe('AnnotationsService', () => { ...@@ -14,6 +14,14 @@ describe('AnnotationsService', () => {
let svc; let svc;
function setLoggedIn(loggedIn) {
const profile = loggedIn
? { userid: 'acct:foo@bar.com', user_info: {} }
: { userid: null };
fakeStore.profile.returns(profile);
fakeStore.isLoggedIn.returns(loggedIn);
}
beforeEach(() => { beforeEach(() => {
fakeAnnotationActivity = { fakeAnnotationActivity = {
reportActivity: sinon.stub(), reportActivity: sinon.stub(),
...@@ -59,6 +67,8 @@ describe('AnnotationsService', () => { ...@@ -59,6 +67,8 @@ describe('AnnotationsService', () => {
updateFlagStatus: sinon.stub(), updateFlagStatus: sinon.stub(),
}; };
setLoggedIn(true);
$imports.$mock({ $imports.$mock({
'../helpers/annotation-metadata': fakeMetadata, '../helpers/annotation-metadata': fakeMetadata,
'../helpers/permissions': { '../helpers/permissions': {
...@@ -90,10 +100,6 @@ describe('AnnotationsService', () => { ...@@ -90,10 +100,6 @@ describe('AnnotationsService', () => {
now = new Date(); now = new Date();
fakeStore.focusedGroupId.returns('mygroup'); fakeStore.focusedGroupId.returns('mygroup');
fakeStore.profile.returns({
userid: 'acct:foo@bar.com',
user_info: {},
});
}); });
it('extends the provided annotation object with defaults', () => { it('extends the provided annotation object with defaults', () => {
...@@ -232,6 +238,14 @@ describe('AnnotationsService', () => { ...@@ -232,6 +238,14 @@ describe('AnnotationsService', () => {
assert.calledWith(fakeStore.setExpanded, 'yetanotherancestor', true); assert.calledWith(fakeStore.setExpanded, 'yetanotherancestor', true);
}); });
it('throws if the user is not logged in', () => {
setLoggedIn(false);
assert.throws(() => {
svc.create(fixtures.newAnnotation(), now);
}, 'Cannot create annotation when logged out');
});
it('throws an error if there is no focused group', () => { it('throws an error if there is no focused group', () => {
fakeStore.focusedGroupId.returns(null); fakeStore.focusedGroupId.returns(null);
...@@ -243,7 +257,7 @@ describe('AnnotationsService', () => { ...@@ -243,7 +257,7 @@ describe('AnnotationsService', () => {
describe('createPageNote', () => { describe('createPageNote', () => {
it('should open the login-prompt panel if the user is not logged in', () => { it('should open the login-prompt panel if the user is not logged in', () => {
fakeStore.isLoggedIn.returns(false); setLoggedIn(false);
svc.createPageNote(); svc.createPageNote();
......
import * as util from '../util'; import { createStoreModule, makeAction } from '../create-store';
import { createStoreModule } from '../create-store';
/** /**
* @typedef {import('../../../types/api').Profile} Profile * @typedef {import('../../../types/api').Profile} Profile
...@@ -49,28 +47,28 @@ function initialState(settings) { ...@@ -49,28 +47,28 @@ function initialState(settings) {
} }
const reducers = { const reducers = {
UPDATE_PROFILE: function (state, action) { /**
* @param {State} state
* @param {{ profile: Profile }} action
*/
UPDATE_PROFILE(state, action) {
return { return {
profile: { ...action.profile }, profile: { ...action.profile },
}; };
}, },
}; };
const actions = util.actionTypes(reducers);
/** /**
* Update the profile information for the current user. * Update the profile information for the current user.
*
* @param {Profile} profile
*/ */
function updateProfile(profile) { function updateProfile(profile) {
return { return makeAction(reducers, 'UPDATE_PROFILE', { profile });
type: actions.UPDATE_PROFILE,
profile,
};
} }
/** /**
* * @param {State} state
* @return {string}
*/ */
function defaultAuthority(state) { function defaultAuthority(state) {
return state.defaultAuthority; return state.defaultAuthority;
...@@ -79,7 +77,7 @@ function defaultAuthority(state) { ...@@ -79,7 +77,7 @@ function defaultAuthority(state) {
/** /**
* Return true if a user is logged in and false otherwise. * Return true if a user is logged in and false otherwise.
* *
* @param {object} state - The application state * @param {State} state
*/ */
function isLoggedIn(state) { function isLoggedIn(state) {
return state.profile.userid !== null; return state.profile.userid !== null;
...@@ -88,7 +86,7 @@ function isLoggedIn(state) { ...@@ -88,7 +86,7 @@ function isLoggedIn(state) {
/** /**
* Return true if a given feature flag is enabled for the current user. * Return true if a given feature flag is enabled for the current user.
* *
* @param {object} state - The application state * @param {State} state
* @param {string} feature - The name of the feature flag. This matches the * @param {string} feature - The name of the feature flag. This matches the
* name of the feature flag as declared in the Hypothesis service. * name of the feature flag as declared in the Hypothesis service.
*/ */
...@@ -100,6 +98,8 @@ function isFeatureEnabled(state, feature) { ...@@ -100,6 +98,8 @@ function isFeatureEnabled(state, feature) {
* Return true if the user's profile has been fetched. This can be used to * Return true if the user's profile has been fetched. This can be used to
* distinguish the dummy profile returned by `profile()` on startup from a * distinguish the dummy profile returned by `profile()` on startup from a
* logged-out user profile returned by the server. * logged-out user profile returned by the server.
*
* @param {State} state
*/ */
function hasFetchedProfile(state) { function hasFetchedProfile(state) {
return state.profile !== initialProfile; return state.profile !== initialProfile;
...@@ -112,6 +112,8 @@ function hasFetchedProfile(state) { ...@@ -112,6 +112,8 @@ function hasFetchedProfile(state) {
* *
* If the profile has not yet been fetched yet, a dummy logged-out profile is * If the profile has not yet been fetched yet, a dummy logged-out profile is
* returned. This allows code to skip a null check. * returned. This allows code to skip a null check.
*
* @param {State} state
*/ */
function profile(state) { function profile(state) {
return state.profile; return state.profile;
......
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