Commit d4374884 authored by Robert Knight's avatar Robert Knight

Extract `quote` function from annotation / view-filter

Both `components/annotation` and `view-filter` modules had logic and
tests for extracting a quote from an annotation object. Extract these
into a shared `quote` function in `util/annotation-metadata`.

One small change is that `view-filter`'s implementation allowed
annotation objects with _no_ `target` property. These should never
occur. There is a known issue where creating a Page Note results in an
annotation with an _empty_ `target` property until the annotation is
saved (see [1]). That case, which is still handled, looks like a mistake,
but resolving it is outside the scope of this change.

[1] https://github.com/hypothesis/client/issues/1290
parent 96389f20
'use strict';
const annotationMetadata = require('../util/annotation-metadata');
const {
isNew,
isReply,
isPageNote,
quote,
} = require('../util/annotation-metadata');
const events = require('../events');
const { isThirdPartyUser } = require('../util/account-id');
const serviceConfig = require('../service-config');
const isNew = annotationMetadata.isNew;
const isReply = annotationMetadata.isReply;
const isPageNote = annotationMetadata.isPageNote;
/**
* Return a copy of `annotation` with changes made in the editor applied.
*/
......@@ -325,19 +326,7 @@ function AnnotationController(
/**
* Return the annotation's quote if it has one or `null` otherwise.
*/
this.quote = function() {
if (self.annotation.target.length === 0) {
return null;
}
const target = self.annotation.target[0];
if (!target.selector) {
return null;
}
const quoteSel = target.selector.find(function(sel) {
return sel.type === 'TextQuoteSelector';
});
return quoteSel ? quoteSel.exact : null;
};
this.quote = () => quote(self.annotation);
this.id = function() {
return self.annotation.id;
......
......@@ -685,41 +685,13 @@ describe('annotation', function() {
});
describe('#quote', function() {
it('returns `null` if the annotation has no quotes', function() {
const annotation = fixtures.defaultAnnotation();
annotation.target = [{}];
const controller = createDirective(annotation).controller;
assert.isNull(controller.quote());
});
it('returns `null` if the annotation has selectors but no quote selector', function() {
const annotation = fixtures.defaultAnnotation();
annotation.target = [
{
selector: [],
},
];
const controller = createDirective(annotation).controller;
assert.isNull(controller.quote());
});
it("returns the first quote's text if the annotation has quotes", function() {
const annotation = fixtures.defaultAnnotation();
annotation.target = [
{
selector: [
{
type: 'TextQuoteSelector',
exact: 'The text that the user selected',
},
],
},
it("returns the annotation's quote", () => {
const ann = fixtures.defaultAnnotation();
const controller = createDirective(ann).controller;
ann.target[0].selector = [
{ type: 'TextQuoteSelector', exact: 'test quote' },
];
const controller = createDirective(annotation).controller;
assert.equal(controller.quote(), 'The text that the user selected');
assert.equal(controller.quote(), 'test quote');
});
});
......
......@@ -260,40 +260,4 @@ describe('sidebar/services/view-filter', () => {
assert.deepEqual(result, []);
});
describe('malformed target object', () => {
it('should not fail on annotations without a target object', () => {
const annotation = {
id: 1,
text: 'foo',
// Missing target
};
const filters = {
any: {
terms: ['foo'],
operator: 'or',
},
};
viewFilter.filter([annotation], filters);
});
it('should not fail on annotations without a target object item', () => {
const annotation = {
id: 1,
text: 'foo',
target: [], // Missing target item
};
const filters = {
any: {
terms: ['foo'],
operator: 'or',
},
};
viewFilter.filter([annotation], filters);
});
});
});
'use strict';
const { quote } = require('../util/annotation-metadata');
// Prevent Babel inserting helper code after `@ngInject` comment below which
// breaks browserify-ngannotate.
let unused; // eslint-disable-line
......@@ -99,21 +101,7 @@ function viewFilter(unicode) {
const fieldMatchers = {
quote: {
autofalse: ann => (ann.references || []).length > 0,
value(annotation) {
if (!annotation.target || !annotation.target.length) {
// Sanity check that ignores any annotation without a target. We should
// never arrive at this place in the code, but its a safe guard against
// anything from the server that may be malformed.
return '';
}
const target = annotation.target[0];
const selectors = target.selector || [];
return selectors
.filter(s => s.type === 'TextQuoteSelector')
.map(s => s.exact)
.join('\n');
},
value: ann => quote(ann) || '',
match: (term, value) => value.indexOf(term) > -1,
},
since: {
......
......@@ -200,6 +200,23 @@ function flagCount(ann) {
return ann.moderation.flagCount;
}
/**
* Return the text quote that an annotation refers to.
*
* @return {string|null}
*/
function quote(ann) {
if (ann.target.length === 0) {
return null;
}
const target = ann.target[0];
if (!target.selector) {
return null;
}
const quoteSel = target.selector.find(s => s.type === 'TextQuoteSelector');
return quoteSel ? quoteSel.exact : null;
}
module.exports = {
documentMetadata: documentMetadata,
domainAndTitle: domainAndTitle,
......@@ -212,4 +229,5 @@ module.exports = {
isReply: isReply,
isWaitingToAnchor: isWaitingToAnchor,
location: location,
quote,
};
......@@ -372,4 +372,50 @@ describe('annotation-metadata', function() {
assert.equal(flagCount(ann), 10);
});
});
describe('quote', () => {
it('returns quote if annotation has a quote', () => {
const ann = {
target: [
{
source: 'https://publisher.org/article.pdf',
selector: [{ type: 'TextQuoteSelector', exact: 'expected quote' }],
},
],
};
assert.equal(annotationMetadata.quote(ann), 'expected quote');
});
// FIXME - This currently happens when creating a new Page Note. Annotations
// from the API should always have a target.
//
// See https://github.com/hypothesis/client/issues/1290.
it('returns `null` if annotation has an empty target array', () => {
const ann = { target: [] };
assert.equal(annotationMetadata.quote(ann), null);
});
it('returns `null` if annotation has no selectors', () => {
const ann = {
target: [
{
source: 'https://publisher.org/article.pdf',
},
],
};
assert.equal(annotationMetadata.quote(ann), null);
});
it('returns `null` if annotation has no text quote selector', () => {
const ann = {
target: [
{
source: 'https://publisher.org/article.pdf',
selector: [{ type: 'TextPositionSelector', start: 0, end: 100 }],
},
],
};
assert.equal(annotationMetadata.quote(ann), null);
});
});
});
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