Commit 9333f013 authored by Robert Knight's avatar Robert Knight

Rename the module containing filter query parsing functions

The difference between `util/search-filter.ts` and `helpers/view-filter.ts` was
previously not obvious from the name. One contains code to parse queries entered
by the user, the other contains code to filter annotations against the parsed
queries. Rename the parsing module and its functions to make it more obvious
what they do.
parent 00d7b43d
...@@ -4,7 +4,7 @@ import { withServices } from '../service-context'; ...@@ -4,7 +4,7 @@ import { withServices } from '../service-context';
import type { APIService } from '../services/api'; import type { APIService } from '../services/api';
import type { ToastMessengerService } from '../services/toast-messenger'; import type { ToastMessengerService } from '../services/toast-messenger';
import { useSidebarStore } from '../store'; import { useSidebarStore } from '../store';
import * as searchFilter from '../util/search-filter'; import { parseHypothesisSearchQuery } from '../util/query-parser';
import ThreadList from './ThreadList'; import ThreadList from './ThreadList';
import { useRootThread } from './hooks/use-root-thread'; import { useRootThread } from './hooks/use-root-thread';
...@@ -34,7 +34,7 @@ function StreamView({ api, toastMessenger }: StreamViewProps) { ...@@ -34,7 +34,7 @@ function StreamView({ api, toastMessenger }: StreamViewProps) {
offset: 0, offset: 0,
limit: 20, limit: 20,
...searchFilter.toObject(query), ...parseHypothesisSearchQuery(query),
}; };
try { try {
store.annotationFetchStarted(); store.annotationFetchStarted();
......
...@@ -6,7 +6,7 @@ import StreamView, { $imports } from '../StreamView'; ...@@ -6,7 +6,7 @@ import StreamView, { $imports } from '../StreamView';
describe('StreamView', () => { describe('StreamView', () => {
let fakeApi; let fakeApi;
let fakeUseRootThread; let fakeUseRootThread;
let fakeSearchFilter; let fakeQueryParser;
let fakeStore; let fakeStore;
let fakeToastMessenger; let fakeToastMessenger;
...@@ -19,8 +19,8 @@ describe('StreamView', () => { ...@@ -19,8 +19,8 @@ describe('StreamView', () => {
children: [], children: [],
}); });
fakeSearchFilter = { fakeQueryParser = {
toObject: sinon.stub().returns({}), parseHypothesisSearchQuery: sinon.stub().returns({}),
}; };
fakeStore = { fakeStore = {
...@@ -40,7 +40,7 @@ describe('StreamView', () => { ...@@ -40,7 +40,7 @@ describe('StreamView', () => {
$imports.$mock({ $imports.$mock({
'./hooks/use-root-thread': { useRootThread: fakeUseRootThread }, './hooks/use-root-thread': { useRootThread: fakeUseRootThread },
'../store': { useSidebarStore: () => fakeStore }, '../store': { useSidebarStore: () => fakeStore },
'../util/search-filter': fakeSearchFilter, '../util/query-parser': fakeQueryParser,
}); });
}); });
...@@ -98,7 +98,7 @@ describe('StreamView', () => { ...@@ -98,7 +98,7 @@ describe('StreamView', () => {
]); ]);
// Assert that we use an empty string as query, when the `q` param is not set // Assert that we use an empty string as query, when the `q` param is not set
assert.calledWith(fakeSearchFilter.toObject, ''); assert.calledWith(fakeQueryParser.parseHypothesisSearchQuery, '');
}); });
it('displays an error if fetching annotations fails', async () => { it('displays an error if fetching annotations fails', async () => {
......
...@@ -18,7 +18,7 @@ const fixtures = immutable({ ...@@ -18,7 +18,7 @@ const fixtures = immutable({
describe('sidebar/helpers/thread-annotations', () => { describe('sidebar/helpers/thread-annotations', () => {
let fakeBuildThread; let fakeBuildThread;
let fakeFilterAnnotations; let fakeFilterAnnotations;
let fakeSearchFilter; let fakeQueryParser;
let fakeThreadState; let fakeThreadState;
beforeEach(() => { beforeEach(() => {
...@@ -38,13 +38,13 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -38,13 +38,13 @@ describe('sidebar/helpers/thread-annotations', () => {
fakeBuildThread = sinon.stub().returns(fixtures.emptyThread); fakeBuildThread = sinon.stub().returns(fixtures.emptyThread);
fakeFilterAnnotations = sinon.stub(); fakeFilterAnnotations = sinon.stub();
fakeSearchFilter = { fakeQueryParser = {
generateFacetedFilter: sinon.stub(), parseFilterQuery: sinon.stub(),
}; };
$imports.$mock({ $imports.$mock({
'./build-thread': { buildThread: fakeBuildThread }, './build-thread': { buildThread: fakeBuildThread },
'../util/search-filter': fakeSearchFilter, '../util/query-parser': fakeQueryParser,
'./view-filter': { filterAnnotations: fakeFilterAnnotations }, './view-filter': { filterAnnotations: fakeFilterAnnotations },
}); });
}); });
...@@ -187,9 +187,9 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -187,9 +187,9 @@ describe('sidebar/helpers/thread-annotations', () => {
const filterFn = fakeBuildThread.args[0][1].filterFn; const filterFn = fakeBuildThread.args[0][1].filterFn;
assert.isFunction(filterFn); assert.isFunction(filterFn);
assert.calledOnce(fakeSearchFilter.generateFacetedFilter); assert.calledOnce(fakeQueryParser.parseFilterQuery);
assert.calledWith( assert.calledWith(
fakeSearchFilter.generateFacetedFilter, fakeQueryParser.parseFilterQuery,
fakeThreadState.selection.filterQuery, fakeThreadState.selection.filterQuery,
fakeThreadState.selection.filters, fakeThreadState.selection.filters,
); );
...@@ -203,7 +203,7 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -203,7 +203,7 @@ describe('sidebar/helpers/thread-annotations', () => {
assert.isFunction(fakeBuildThread.args[0][1].filterFn); assert.isFunction(fakeBuildThread.args[0][1].filterFn);
assert.calledWith( assert.calledWith(
fakeSearchFilter.generateFacetedFilter, fakeQueryParser.parseFilterQuery,
sinon.match.any, sinon.match.any,
sinon.match({ user: 'somebody' }), sinon.match({ user: 'somebody' }),
); );
......
import type { Annotation } from '../../types/api'; import type { Annotation } from '../../types/api';
import { memoize } from '../util/memoize'; import { memoize } from '../util/memoize';
import { generateFacetedFilter } from '../util/search-filter'; import { parseFilterQuery } from '../util/query-parser';
import { buildThread } from './build-thread'; import { buildThread } from './build-thread';
import type { Thread, BuildThreadOptions } from './build-thread'; import type { Thread, BuildThreadOptions } from './build-thread';
import { shouldShowInTab } from './tabs'; import { shouldShowInTab } from './tabs';
...@@ -40,7 +40,7 @@ function buildRootThread(threadState: ThreadState): Thread { ...@@ -40,7 +40,7 @@ function buildRootThread(threadState: ThreadState): Thread {
!!selection.filterQuery || Object.keys(selection.filters).length > 0; !!selection.filterQuery || Object.keys(selection.filters).length > 0;
if (annotationsFiltered) { if (annotationsFiltered) {
const filters = generateFacetedFilter( const filters = parseFilterQuery(
selection.filterQuery || '', selection.filterQuery || '',
selection.filters, selection.filters,
); );
......
import { cfiInRange, stripCFIAssertions } from '../../shared/cfi'; import { cfiInRange, stripCFIAssertions } from '../../shared/cfi';
import type { Annotation } from '../../types/api'; import type { Annotation } from '../../types/api';
import { pageLabelInRange } from '../util/page-range'; import { pageLabelInRange } from '../util/page-range';
import type { Facet } from '../util/search-filter'; import type { Facet } from '../util/query-parser';
import * as unicodeUtils from '../util/unicode'; import * as unicodeUtils from '../util/unicode';
import { cfi as getCFI, quote, pageLabel } from './annotation-metadata'; import { cfi as getCFI, quote, pageLabel } from './annotation-metadata';
......
...@@ -20,7 +20,6 @@ import { createStoreModule, makeAction } from '../create-store'; ...@@ -20,7 +20,6 @@ import { createStoreModule, makeAction } from '../create-store';
* active (applied). * active (applied).
* - query: String query that is either typed in by the user or provided in * - query: String query that is either typed in by the user or provided in
* settings. A query string may contain supported facets. * settings. A query string may contain supported facets.
* (see `util/search-filter`)
*/ */
export type FilterOption = { export type FilterOption = {
......
...@@ -44,10 +44,10 @@ function splitTerm(term: string): [null | string, string] { ...@@ -44,10 +44,10 @@ function splitTerm(term: string): [null | string, string] {
* Remove a quote character from the beginning and end of the string, but * Remove a quote character from the beginning and end of the string, but
* only if they match. ie: * only if they match. ie:
* *
-* 'foo' -> foo * 'foo' -> foo
-* "bar" -> bar * "bar" -> bar
-* 'foo" -> 'foo" * 'foo" -> 'foo"
-* bar" -> bar" * bar" -> bar"
*/ */
function removeSurroundingQuotes(text: string) { function removeSurroundingQuotes(text: string) {
const start = text.slice(0, 1); const start = text.slice(0, 1);
...@@ -87,9 +87,12 @@ function tokenize(searchText: string): string[] { ...@@ -87,9 +87,12 @@ function tokenize(searchText: string): string[] {
} }
/** /**
* Parse a search query into a map of search field to term. * Parse a user-provided query ("term:value ...") into a key/value map that can
* be used when constructing queries to the Hypothesis search API.
*/ */
export function toObject(searchText: string): Record<string, string[]> { export function parseHypothesisSearchQuery(
searchText: string,
): Record<string, string[]> {
const obj = {} as Record<string, string[]>; const obj = {} as Record<string, string[]>;
const backendFilter = (field: string) => (field === 'tag' ? 'tags' : field); const backendFilter = (field: string) => (field === 'tag' ? 'tags' : field);
...@@ -123,17 +126,16 @@ export type FocusFilter = { ...@@ -123,17 +126,16 @@ export type FocusFilter = {
}; };
/** /**
* Parse a search query into a map of filters. * Parse a user-provided query into a map of filter field to term.
*
* Returns an object mapping facet names to Facet.
* *
* Terms that are not associated with a particular facet are stored in the "any" * Terms that are not associated with any particular field are stored under the
* facet. * "any" property.
* *
* @param searchText - Filter query to parse * @param searchText - Filter query to parse
* @param focusFilters - Additional filter terms to mix in * @param focusFilters - Additional query terms to merge with the results of
* parsing `searchText`.
*/ */
export function generateFacetedFilter( export function parseFilterQuery(
searchText: string, searchText: string,
focusFilters: FocusFilter = {}, focusFilters: FocusFilter = {},
): Record<string, Facet> { ): Record<string, Facet> {
......
import * as searchFilter from '../search-filter'; import { parseHypothesisSearchQuery, parseFilterQuery } from '../query-parser';
describe('sidebar/util/search-filter', () => { describe('sidebar/util/query-parser', () => {
function isEmptyFilter(filter) { function isEmptyFilter(filter) {
return Object.values(filter).every(value => value.length === 0); return Object.values(filter).every(value => value.length === 0);
} }
describe('toObject', () => { describe('parseHypothesisSearchQuery', () => {
it('puts a simple search string under the "any" filter', () => { it('puts a simple search string under the "any" filter', () => {
const query = 'foo'; const query = 'foo';
const result = searchFilter.toObject(query); const result = parseHypothesisSearchQuery(query);
assert.equal(result.any[0], query); assert.equal(result.any[0], query);
}); });
it('returns an empty filter if input query is empty', () => { it('returns an empty filter if input query is empty', () => {
// Verify `isEmptyFilter` returns false for non-empty query. // Verify `isEmptyFilter` returns false for non-empty query.
assert.isFalse(isEmptyFilter(searchFilter.toObject('some query'))); assert.isFalse(isEmptyFilter(parseHypothesisSearchQuery('some query')));
// Now check various queries which should produce empty filters // Now check various queries which should produce empty filters
for (let emptyQuery of ['', '""', "''", ' ']) { for (let emptyQuery of ['', '""', "''", ' ']) {
const result = searchFilter.toObject(emptyQuery); const result = parseHypothesisSearchQuery(emptyQuery);
assert.isTrue( assert.isTrue(
isEmptyFilter(result), isEmptyFilter(result),
`expected "${emptyQuery}" to produce empty filter`, `expected "${emptyQuery}" to produce empty filter`,
...@@ -28,7 +28,7 @@ describe('sidebar/util/search-filter', () => { ...@@ -28,7 +28,7 @@ describe('sidebar/util/search-filter', () => {
it('uses the filters as keys in the result object', () => { it('uses the filters as keys in the result object', () => {
const query = 'user:john text:foo quote:bar group:agroup other'; const query = 'user:john text:foo quote:bar group:agroup other';
const result = searchFilter.toObject(query); const result = parseHypothesisSearchQuery(query);
assert.equal(result.any[0], 'other'); assert.equal(result.any[0], 'other');
assert.equal(result.user[0], 'john'); assert.equal(result.user[0], 'john');
...@@ -40,7 +40,7 @@ describe('sidebar/util/search-filter', () => { ...@@ -40,7 +40,7 @@ describe('sidebar/util/search-filter', () => {
it('collects the same filters into a list', () => { it('collects the same filters into a list', () => {
const query = const query =
'user:john text:foo quote:bar other user:doe text:fuu text:fii'; 'user:john text:foo quote:bar other user:doe text:fuu text:fii';
const result = searchFilter.toObject(query); const result = parseHypothesisSearchQuery(query);
assert.equal(result.any[0], 'other'); assert.equal(result.any[0], 'other');
assert.equal(result.user[0], 'john'); assert.equal(result.user[0], 'john');
...@@ -53,14 +53,14 @@ describe('sidebar/util/search-filter', () => { ...@@ -53,14 +53,14 @@ describe('sidebar/util/search-filter', () => {
it('preserves data with semicolon characters', () => { it('preserves data with semicolon characters', () => {
const query = 'uri:http://test.uri'; const query = 'uri:http://test.uri';
const result = searchFilter.toObject(query); const result = parseHypothesisSearchQuery(query);
assert.equal(result.uri[0], 'http://test.uri'); assert.equal(result.uri[0], 'http://test.uri');
}); });
it('collects valid filters and puts invalid into the "any" category', () => { it('collects valid filters and puts invalid into the "any" category', () => {
const query = const query =
'uri:test foo:bar text:hey john:doe quote:according hi-fi a:bc'; 'uri:test foo:bar text:hey john:doe quote:according hi-fi a:bc';
const result = searchFilter.toObject(query); const result = parseHypothesisSearchQuery(query);
assert.isUndefined(result.foo); assert.isUndefined(result.foo);
assert.isUndefined(result.john); assert.isUndefined(result.john);
...@@ -75,14 +75,14 @@ describe('sidebar/util/search-filter', () => { ...@@ -75,14 +75,14 @@ describe('sidebar/util/search-filter', () => {
}); });
it('supports quoting terms', () => { it('supports quoting terms', () => {
const parsed = searchFilter.toObject('user:"Dan Whaley"'); const parsed = parseHypothesisSearchQuery('user:"Dan Whaley"');
assert.deepEqual(parsed, { assert.deepEqual(parsed, {
user: ['Dan Whaley'], user: ['Dan Whaley'],
}); });
}); });
it('assigns unquoted terms to "any" category', () => { it('assigns unquoted terms to "any" category', () => {
const parsed = searchFilter.toObject('user:Dan Whaley'); const parsed = parseHypothesisSearchQuery('user:Dan Whaley');
assert.deepEqual(parsed, { assert.deepEqual(parsed, {
any: ['Whaley'], any: ['Whaley'],
user: ['Dan'], user: ['Dan'],
...@@ -90,7 +90,7 @@ describe('sidebar/util/search-filter', () => { ...@@ -90,7 +90,7 @@ describe('sidebar/util/search-filter', () => {
}); });
}); });
describe('generateFacetedFilter', () => { describe('parseFilterQuery', () => {
[ [
// Empty queries. // Empty queries.
{ {
...@@ -181,8 +181,8 @@ describe('sidebar/util/search-filter', () => { ...@@ -181,8 +181,8 @@ describe('sidebar/util/search-filter', () => {
}, },
}, },
].forEach(({ query, expectedFilter }) => { ].forEach(({ query, expectedFilter }) => {
it('parses a search query', () => { it('parses a query', () => {
const filter = searchFilter.generateFacetedFilter(query); const filter = parseFilterQuery(query);
// Remove empty facets. // Remove empty facets.
Object.keys(filter).forEach(k => { Object.keys(filter).forEach(k => {
...@@ -231,7 +231,7 @@ describe('sidebar/util/search-filter', () => { ...@@ -231,7 +231,7 @@ describe('sidebar/util/search-filter', () => {
].forEach(({ timeExpr, expectedSecs }) => { ].forEach(({ timeExpr, expectedSecs }) => {
it('parses a "since:" query', () => { it('parses a "since:" query', () => {
const query = `since:${timeExpr}`; const query = `since:${timeExpr}`;
const filter = searchFilter.generateFacetedFilter(query); const filter = parseFilterQuery(query);
if (expectedSecs === null) { if (expectedSecs === null) {
assert.deepEqual(filter.since.terms, []); assert.deepEqual(filter.since.terms, []);
...@@ -241,8 +241,8 @@ describe('sidebar/util/search-filter', () => { ...@@ -241,8 +241,8 @@ describe('sidebar/util/search-filter', () => {
}); });
}); });
it('filters to a focused user', () => { it('adds additional filters to result', () => {
const filter = searchFilter.generateFacetedFilter('', { const filter = parseFilterQuery('', {
user: 'fakeusername', user: 'fakeusername',
}); });
// Remove empty facets. // Remove empty facets.
......
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