Skip to content

C
coopwire-hypothesis
Unverified Commit 248458e4 authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub
Browse files
Options

Merge pull request #1672 from hypothesis/annotation-init 

Move new-annotation initialization into the store
parents 765fafc5 71fd83ff
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 340 additions and 294 deletions
src/sidebar/components/annotation.js
View file @ 248458e4
......@@ -92,25 +92,13 @@ function AnnotationController(
*/
newlyCreatedByHighlightButton = self.annotation.$highlight || false;
// New annotations (just created locally by the client, rather then
// received from the server) have some fields missing. Add them.
//
// FIXME: This logic should go in the `addAnnotations` Redux action once all
// required state is in the store.
self.annotation.user = self.annotation.user || session.state.userid;
self.annotation.user_info =
self.annotation.user_info || session.state.user_info;
self.annotation.group = self.annotation.group || groups.focused().id;
// FIXME: This logic needs to move into the `annotations` store module
if (!self.annotation.permissions) {
self.annotation.permissions = permissions.default(
self.annotation.user,
self.annotation.group
);
}
self.annotation.text = self.annotation.text || '';
if (!Array.isArray(self.annotation.tags)) {
self.annotation.tags = [];
}
// Automatically save new highlights to the server when they're created.
// Note that this line also gets called when the user logs in (since
......
src/sidebar/components/test/annotation-test.js
View file @ 248458e4
......@@ -245,35 +245,14 @@ describe('annotation', function() {
});
describe('initialization', function() {
it("sets the user of annotations that don't have one", function() {
// You can create annotations while logged out and then login.
// When you login a new AnnotationController instance is created for
// each of your annotations, and on initialization it will set the
// annotation's user to your username from the session.
const annotation = fixtures.newAnnotation();
annotation.user = undefined;
fakeSession.state.userid = 'acct:bill@localhost';
fakeSession.state.user_info = {
display_name: 'Bill Jones',
};
createDirective(annotation);
assert.equal(annotation.user, 'acct:bill@localhost');
assert.deepEqual(annotation.user_info, {
display_name: 'Bill Jones',
});
});
it('sets the permissions of new annotations', function() {
// You can create annotations while logged out and then login.
// When you login a new AnnotationController instance is created for
// each of your annotations, and on initialization it will set the
// annotation's permissions using your username from the session.
const annotation = fixtures.newAnnotation();
annotation.user = annotation.permissions = undefined;
annotation.permissions = undefined;
annotation.group = '__world__';
fakeSession.state.userid = 'acct:bill@localhost';
fakePermissions.default = function(userid, group) {
return {
read: [userid, group],
......@@ -284,19 +263,10 @@ describe('annotation', function() {
assert.deepEqual(
annotation.permissions,
fakePermissions.default(fakeSession.state.userid, annotation.group)
fakePermissions.default(annotation.user, annotation.group)
);
});
it('sets the tags and text fields for new annotations', function() {
const annotation = fixtures.newAnnotation();
delete annotation.tags;
delete annotation.text;
createDirective(annotation);
assert.equal(annotation.text, '');
assert.deepEqual(annotation.tags, []);
});
it('preserves the permissions of existing annotations', function() {
const annotation = fixtures.newAnnotation();
annotation.permissions = {
......@@ -795,14 +765,6 @@ describe('annotation', function() {
assert.notCalled(fakeStore.removeDraft);
});
});
it("sets the annotation's group to the focused group", function() {
fakeGroups.focused = function() {
return { id: 'test-id' };
};
const controller = createDirective(fixtures.newAnnotation()).controller;
assert.equal(controller.annotation.group, 'test-id');
});
});
describe('saving an edited an annotation', function() {
......
src/sidebar/store/modules/annotations.js
View file @ 248458e4
......@@ -48,17 +48,21 @@ function findByTag(annotations, tag) {
}
/**
* Initialize the status flags and properties of a new annotation.
* Set custom private fields on an annotation object about to be added to the
* store's collection of `annotations`.
*
* `annotation` may either be new (unsaved) or a persisted annotation retrieved
* from the service.
*
* @param {Object} annotation
* @param {Number} tag - The `$tag` value that should be used for this
* if it doesn't have a `$tag` already
* @return {Object} - annotation with local (`$*`) fields set
*/
function initializeAnnot(annotation, tag) {
function initializeAnnotation(annotation, tag) {
let orphan = annotation.$orphan;
if (!annotation.id) {
// Currently the user ID, permissions and group of new annotations are
// initialized in the <annotation> component controller because the session
// state and focused group are not stored in the Redux store. Once they are,
// that initialization should be moved here.
// New annotations must be anchored
orphan = false;
}
......@@ -91,7 +95,7 @@ const update = {
const updated = [];
let nextTag = state.nextTag;
action.annotations.forEach(function(annot) {
action.annotations.forEach(annot => {
let existing;
if (annot.id) {
existing = findByID(state.annotations, annot.id);
......@@ -111,12 +115,12 @@ const update = {
updatedTags[existing.$tag] = true;
}
} else {
added.push(initializeAnnot(annot, 't' + nextTag));
added.push(initializeAnnotation(annot, 't' + nextTag));
++nextTag;
}
});
state.annotations.forEach(function(annot) {
state.annotations.forEach(annot => {
if (!updatedIDs[annot.id] && !updatedTags[annot.$tag]) {
unchanged.push(annot);
}
......@@ -218,29 +222,14 @@ function updateFlagStatus(id, isFlagged) {
};
}
/** Add annotations to the currently displayed set. */
function addAnnotations(annotations, now) {
now = now || new Date();
// Add dates to new annotations. These are ignored by the server but used
// when sorting unsaved annotation cards.
annotations = annotations.map(function(annot) {
if (annot.id) {
return annot;
}
return Object.assign(
{
// Date.prototype.toISOString returns a 0-offset (UTC) ISO8601
// datetime.
created: now.toISOString(),
updated: now.toISOString(),
},
annot
);
});
/**
* Add these `annotations` to the current collection of annotations in the store.
*
* @param {Object}[] annotations - Array of annotation objects to add.
*/
function addAnnotations(annotations) {
return function(dispatch, getState) {
const added = annotations.filter(function(annot) {
const added = annotations.filter(annot => {
return !findByID(getState().annotations.annotations, annot.id);
});
......@@ -250,6 +239,7 @@ function addAnnotations(annotations, now) {
currentAnnotationCount: getState().annotations.annotations.length,
});
// If we're not in the sidebar, we're done here.
if (!getState().viewer.isSidebar) {
return;
}
......@@ -330,19 +320,42 @@ function hideAnnotation(id) {
}
/**
* Create a new annotation
* Create a new annotation (as-yet unpersisted)
*
* This method has several responsibilities:
* 1. Set some default data attributes on the annotation
* 2. Remove any existing, empty drafts
* 3. Add the annotation to the current collection of annotations
* 4. Change focused tab to the applicable one for the new annotation's meta-type
* 5. Expand all of the new annotation's parents
*
* The method does 4 tasks:
* 1. Removes any existing empty drafts.
* 2. Creates a new annotation.
* 3. Changes the focused tab to match that of the newly created annotation.
* 4. Expands the collapsed state of all new annotation's parents.
*/
function createAnnotation(ann) {
return dispatch => {
function createAnnotation(ann, now = new Date()) {
return (dispatch, getState) => {
/**
* Extend the new, unsaved annotation object with defaults for some
* required data fields.
*
* Note: the `created` and `updated` values will be ignored and superseded
* by the service when the annotation is persisted, but they are used
* app-side for annotation card sorting until then.
*/
ann = Object.assign(
{
created: now.toISOString(),
group: getState().groups.focusedGroupId,
tags: [],
text: '',
updated: now.toISOString(),
user: getState().session.userid,
user_info: getState().session.user_info,
},
ann
);
// When a new annotation is created, remove any existing annotations
// that are empty.
dispatch(drafts.actions.deleteNewAndEmptyDrafts([ann]));
dispatch(addAnnotations([ann]));
// If the annotation is of type note or annotation, make sure
// the appropriate tab is selected. If it is of type reply, user
......
src/sidebar/store/test/index-test.js
View file @ 248458e4
......@@ -2,12 +2,10 @@ const immutable = require('seamless-immutable');
const storeFactory = require('../index');
const annotationFixtures = require('../../test/annotation-fixtures');
const metadata = require('../../util/annotation-metadata');
const uiConstants = require('../../ui-constants');
const defaultAnnotation = annotationFixtures.defaultAnnotation;
const newAnnotation = annotationFixtures.newAnnotation;
const oldPageNote = annotationFixtures.oldPageNote;
const fixtures = immutable({
pair: [
......@@ -123,190 +121,6 @@ describe('store', function() {
});
});
describe('#addAnnotations()', function() {
const ANCHOR_TIME_LIMIT = 1000;
let clock;
beforeEach(function() {
clock = sinon.useFakeTimers();
});
afterEach(function() {
clock.restore();
});
it('adds annotations not in the store', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
assert.match(store.getState().annotations.annotations, [
sinon.match(annot),
]);
});
it('does not change `selectedTab` state if annotations are already loaded', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
const page = oldPageNote();
store.addAnnotations([page]);
assert.equal(
store.getState().selection.selectedTab,
uiConstants.TAB_ANNOTATIONS
);
});
it('sets `selectedTab` to "note" if only page notes are present', function() {
const page = oldPageNote();
store.addAnnotations([page]);
assert.equal(
store.getState().selection.selectedTab,
uiConstants.TAB_NOTES
);
});
it('leaves `selectedTab` as "annotation" if annotations and/or page notes are present', function() {
const page = oldPageNote();
const annot = defaultAnnotation();
store.addAnnotations([annot, page]);
assert.equal(
store.getState().selection.selectedTab,
uiConstants.TAB_ANNOTATIONS
);
});
it('assigns a local tag to annotations', function() {
const annotA = Object.assign(defaultAnnotation(), { id: 'a1' });
const annotB = Object.assign(defaultAnnotation(), { id: 'a2' });
store.addAnnotations([annotA, annotB]);
const tags = store.getState().annotations.annotations.map(function(a) {
return a.$tag;
});
assert.deepEqual(tags, ['t1', 't2']);
});
it('updates annotations with matching IDs in the store', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
const update = Object.assign({}, defaultAnnotation(), { text: 'update' });
store.addAnnotations([update]);
const updatedAnnot = store.getState().annotations.annotations[0];
assert.equal(updatedAnnot.text, 'update');
});
it('updates annotations with matching tags in the store', function() {
const annot = newAnnotation();
annot.$tag = 'local-tag';
store.addAnnotations([annot]);
const saved = Object.assign({}, annot, { id: 'server-id' });
store.addAnnotations([saved]);
const annots = store.getState().annotations.annotations;
assert.equal(annots.length, 1);
assert.equal(annots[0].id, 'server-id');
});
// We add temporary created and updated timestamps to annotations to ensure
// that they sort correctly in the sidebar. These fields are ignored by the
// server.
it('adds created/updated timestamps to new annotations', function() {
const now = new Date();
const nowStr = now.toISOString();
store.addAnnotations([newAnnotation()], now);
const annot = store.getState().annotations.annotations[0];
assert.equal(annot.created, nowStr);
assert.equal(annot.updated, nowStr);
});
it('does not overwrite existing created/updated timestamps in new annotations', function() {
const now = new Date();
const annot = newAnnotation();
annot.created = '2000-01-01T01:02:03Z';
annot.updated = '2000-01-01T04:05:06Z';
store.addAnnotations([annot], now);
const result = store.getState().annotations.annotations[0];
assert.equal(result.created, annot.created);
assert.equal(result.updated, annot.updated);
});
it('preserves anchoring status of updated annotations', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
const update = Object.assign({}, defaultAnnotation(), { text: 'update' });
store.addAnnotations([update]);
const updatedAnnot = store.getState().annotations.annotations[0];
assert.isFalse(updatedAnnot.$orphan);
});
it('sets the timeout flag on annotations that fail to anchor within a time limit', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
clock.tick(ANCHOR_TIME_LIMIT);
assert.isTrue(store.getState().annotations.annotations[0].$anchorTimeout);
});
it('does not set the timeout flag on annotations that do anchor within a time limit', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
clock.tick(ANCHOR_TIME_LIMIT);
assert.isFalse(
store.getState().annotations.annotations[0].$anchorTimeout
);
});
it('does not attempt to modify orphan status if annotations are removed before anchoring timeout expires', function() {
const annot = defaultAnnotation();
store.addAnnotations([annot]);
store.updateAnchorStatus({ [tagForID(annot.id)]: 'anchored' });
store.removeAnnotations([annot]);
assert.doesNotThrow(function() {
clock.tick(ANCHOR_TIME_LIMIT);
});
});
it('does not expect annotations to anchor on the stream', function() {
const isOrphan = function() {
return !!metadata.isOrphan(store.getState().annotations.annotations[0]);
};
const annot = defaultAnnotation();
store.setAppIsSidebar(false);
store.addAnnotations([annot]);
clock.tick(ANCHOR_TIME_LIMIT);
assert.isFalse(isOrphan());
});
it('initializes the $orphan field for new annotations', function() {
store.addAnnotations([newAnnotation()]);
assert.isFalse(store.getState().annotations.annotations[0].$orphan);
});
it('adds multiple new annotations', function() {
store.addAnnotations([fixtures.newPair[0]]);
store.addAnnotations([fixtures.newPair[1]]);
assert.equal(store.getState().annotations.annotations.length, 2);
});
});
describe('#removeAnnotations()', function() {
it('removes annotations from the current state', function() {
const annot = defaultAnnotation();
......
src/sidebar/test/annotation-fixtures.js
View file @ 248458e4
......@@ -45,6 +45,8 @@ function publicAnnotation() {
/** Return an annotation domain model object for a new annotation
* (newly-created client-side, not yet saved to the server).
* Components will never see this data structure, as it will have been
* amended by store reducers.
*/
function newAnnotation() {
return {
......@@ -54,6 +56,7 @@ function newAnnotation() {
references: [],
text: 'Annotation text',
tags: ['tag_1', 'tag_2'],
user: 'acct:bill@localhost',
};
}
......@@ -89,6 +92,7 @@ function newHighlight() {
id: undefined,
$highlight: true,
target: [{ source: 'http://example.org' }],
user: 'acct:bill@localhost',
};
}
......
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