Commit 19fc6f7f authored by Robert Knight's avatar Robert Knight

Allow draft annotations to match filters

The internal function that filters annotations returned a list of matching IDs.
Unsaved annotations do not have IDs, so were excluded from the matches. Change
the function to return a list of matching annotations instead, lifting this
restriction.

No callers of the function had to change because currently they only test the
length of the returned array.

A caveat with this change is that it only matches the data stored in the
annotation object, not data stored in separate "draft" objects which are
combined when rendering unsaved annotations. The result is that a filter will
only match the fields of the annotation that are populated when the new
annotation initially appears in the sidebar. This for example includes the user
and page number, but not text added in the text field later.
parent 76040e76
...@@ -153,13 +153,11 @@ const fieldMatchers: Record<string, Matcher | Matcher<number>> = { ...@@ -153,13 +153,11 @@ const fieldMatchers: Record<string, Matcher | Matcher<number>> = {
/** /**
* Filter a set of annotations against a parsed query. * Filter a set of annotations against a parsed query.
*
* @return IDs of matching annotations.
*/ */
export function filterAnnotations( export function filterAnnotations(
annotations: Annotation[], annotations: Annotation[],
filters: Record<string, Facet>, filters: Record<string, Facet>,
): string[] { ): Annotation[] {
const makeTermFilter = <TermType>(field: string, term: TermType) => const makeTermFilter = <TermType>(field: string, term: TermType) =>
new TermFilter( new TermFilter(
term, term,
...@@ -191,9 +189,5 @@ export function filterAnnotations( ...@@ -191,9 +189,5 @@ export function filterAnnotations(
const rootFilter = new BooleanOpFilter('and', fieldFilters); const rootFilter = new BooleanOpFilter('and', fieldFilters);
return annotations return annotations.filter(ann => rootFilter.matches(ann));
.filter(ann => {
return ann.id && rootFilter.matches(ann);
})
.map(ann => ann.id!);
} }
...@@ -64,7 +64,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -64,7 +64,7 @@ describe('sidebar/helpers/filter-annotations', () => {
const result = filterAnnotations(annotations, filters); const result = filterAnnotations(annotations, filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, annotations.slice(0, 1));
}); });
it('requires at least one term to match for "or" operator', () => { it('requires at least one term to match for "or" operator', () => {
...@@ -142,7 +142,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -142,7 +142,7 @@ describe('sidebar/helpers/filter-annotations', () => {
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, [annotation]);
}); });
}); });
...@@ -171,7 +171,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -171,7 +171,7 @@ describe('sidebar/helpers/filter-annotations', () => {
]; ];
const result = filterAnnotations(anns, userQuery('john')); const result = filterAnnotations(anns, userQuery('john'));
assert.deepEqual(result, [anns[0].id, anns[2].id]); assert.deepEqual(result, [anns[0], anns[2]]);
}); });
it("matches user's display name if present", () => { it("matches user's display name if present", () => {
...@@ -190,7 +190,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -190,7 +190,7 @@ describe('sidebar/helpers/filter-annotations', () => {
]; ];
const result = filterAnnotations(anns, userQuery('james')); const result = filterAnnotations(anns, userQuery('james'));
assert.deepEqual(result, [anns[1].id, anns[2].id, anns[3].id]); assert.deepEqual(result, [anns[1], anns[2], anns[3]]);
}); });
it('matches username and display name independently', () => { it('matches username and display name independently', () => {
...@@ -219,7 +219,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -219,7 +219,7 @@ describe('sidebar/helpers/filter-annotations', () => {
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, [annotation]);
}); });
it('does not match if the annotation is older than the query', () => { it('does not match if the annotation is older than the query', () => {
...@@ -257,7 +257,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -257,7 +257,7 @@ describe('sidebar/helpers/filter-annotations', () => {
it('matches if annotation is in page range', () => { it('matches if annotation is in page range', () => {
const filters = { page: { terms: ['4-6'], operator: 'or' } }; const filters = { page: { terms: ['4-6'], operator: 'or' } };
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, [annotation]);
}); });
it('does not match if annotation is outside of page range', () => { it('does not match if annotation is outside of page range', () => {
...@@ -291,7 +291,7 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -291,7 +291,7 @@ describe('sidebar/helpers/filter-annotations', () => {
it('matches if annotation is in range', () => { it('matches if annotation is in range', () => {
const filters = { cfi: { terms: [range], operator: 'or' } }; const filters = { cfi: { terms: [range], operator: 'or' } };
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, [annotation]);
}); });
}); });
...@@ -331,13 +331,15 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -331,13 +331,15 @@ describe('sidebar/helpers/filter-annotations', () => {
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, [1]); assert.deepEqual(result, [annotation]);
}); });
it('ignores annotations (drafts) with no id', () => { it('can match annotations (drafts) with no id', () => {
const annotation = { const annotation = {
tags: ['foo'], tags: ['foo'],
target: [{}], target: [{}],
text: '',
url: 'https://example.com',
}; };
const filters = { const filters = {
any: { any: {
...@@ -348,6 +350,6 @@ describe('sidebar/helpers/filter-annotations', () => { ...@@ -348,6 +350,6 @@ describe('sidebar/helpers/filter-annotations', () => {
const result = filterAnnotations([annotation], filters); const result = filterAnnotations([annotation], filters);
assert.deepEqual(result, []); assert.deepEqual(result, [annotation]);
}); });
}); });
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