Commit 8ca4773b authored by Lyza Danger Gardner's avatar Lyza Danger Gardner

Prevent errors arising from applying private permissions to anonymous annotations

As a convenience, the app retains the last permissions level used by a user when publishing an annotation. For example, if a user creates and saves and annotation that is set to “only me”/private, the app stashes that as a pseudo-preference in `localStorage` and the next time a user creates a new annotation, that will be the default permissions level applied to it. This makes it easier to create several subsequent annotations that have the same permissions/sharing setting.

There is a bug in this approach, however, which before this patch happened when:

* A Hypothesis user creates an annotation and sets the permissions to “private” (only me) and saves the annotation successfully to the `h` service, then;
* The Hypothesis user logs out, then;
* The same person in the same browser (now an anonymous user from Hypothesis’ perspective) selects some text and clicks “annotate., then;
* The user is shown an error message that they must log in to create annotations: they click “Log In”

Before this fix, the user then would see an error “Cannot read property ‘split’ of null” in an error flash and the login process would not successfully complete. The only workaround was to reload the browser window and try again.

After this fix, the user will not see an error after the sequence detailed above and the login process will complete.

This fix is achieved by preventing the app from attempting to apply default permissions of “only me”/private when no `userid` is available. Doing so creates corrupted permissions on the annotation which throw `TypeError`s when later iterated over.

There are several further fixes necessary to fully clean up this situation, but this patches over the immediate user-visible bug.

Fixes https://github.com/hypothesis/client/issues/1221
parent bf727705
......@@ -79,7 +79,14 @@ function Permissions(localStorage) {
* @return {Permissions}
*/
this.default = function(userid, groupId) {
if (defaultLevel() === 'private') {
// FIXME: The `&& userid` guard was put in place to protect against
// https://github.com/hypothesis/client/issues/1221 for the short term
// It prevents the setting of permissions to a private level, which
// will throw Errors if no `userid` is available (i.e. the annotation was
// just created by an anonymous user). Translation: default permissions for
// a newly-created (but not saved-to-server) annotation created by a
// non-authenticated (anonymous) user will always be shared. For now.
if (defaultLevel() === 'private' && userid) {
return self.private(userid);
} else {
return self.shared(userid, groupId);
......
......@@ -52,6 +52,17 @@ describe('permissions', function() {
);
});
it('returns shared permissions if the saved level is "private" but no `userid`', function() {
// FIXME: This test is necessary for the patch fix to prevent the "split-null" bug
// https://github.com/hypothesis/client/issues/1221 but should be removed when the
// code is refactored.
fakeLocalStorage.getItem.returns('private');
assert.deepEqual(
permissions.default(undefined, 'gid'),
permissions.shared(undefined, 'gid')
);
});
it('returns shared permissions if the saved level is "shared"', function() {
fakeLocalStorage.getItem.returns('shared');
assert.deepEqual(
......
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