Skip to content

Commit

Permalink
Fix: bug in range filter; add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
erhathaway committed Mar 13, 2020
1 parent ef4f053 commit c73c08d
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 36 deletions.
22 changes: 0 additions & 22 deletions __tests__/integration/filters/boolean_filter/set_filter.test.ts

This file was deleted.

51 changes: 51 additions & 0 deletions __tests__/integration/filters/set_filter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import {setUp} from '../utils';
import waitForExpect from 'wait-for-expect';

describe('Filters', () => {
describe('setFilter', () => {
it('calls client search with filter that was set', async () => {
const {manager, client} = setUp();
await manager.getFieldNamesAndTypes();

manager.filters.boolean.setFilter('boolean_field', {state: true});

expect(client.search).toHaveBeenCalledWith({
_source: {},
aggs: {},
query: {bool: {should: [{term: {boolean_field: true}}]}},
size: 10,
sort: ['_score', '_doc'],
track_scores: true
});
});

it('adds existing filters to the search request when set', async () => {
const {manager, client} = setUp();

await manager.getFieldNamesAndTypes();

manager.filters.boolean.setFilter('boolean_field', {state: true});
manager.filters.range.setFilter('double_field', {greaterThan: 0, lessThanEqual: 10});

await waitForExpect(() => {
expect(manager._sideEffectQueue.length).toEqual(0);
});

expect(client.search).toHaveBeenCalledWith({
query: {
bool: {
should: [
{term: {boolean_field: true}},
{range: {double_field: {gt: 0, lte: 10}}}
]
}
},
aggs: {},
_source: {},
size: 0,
track_scores: false,
sort: ['_score', '_doc']
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {setUp} from '../../utils';
import {setUp} from '../utils';

describe('Suggestion', () => {
describe('Prefix', () => {
Expand Down
2 changes: 1 addition & 1 deletion __tests__/integration/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const fakeResponse = <Source extends object = object>(params?: Partial<ES
// tslint:disable-next-line
return {
took: (params && params.took) || 20,
timed_out: (params && params.timed_out) || true,
timed_out: (params && params.timed_out) || false,
_shards: (params && params._shards) || {total: 5, successful: 4, skipped: 1, failed: 0},
hits: (params && params.hits) || {total: 200, max_score: 1, hits: [fakeResponseHit()]},
aggregations: (params && params.aggregations) || {}
Expand Down
8 changes: 7 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@social-native/snpkg-client-elasticsearch",
"version": "3.6.1",
"version": "3.6.2",
"description": "",
"main": "dist/index.cjs.js",
"module": "dist/index.es.js",
Expand All @@ -19,6 +19,7 @@
"prettier": "./node_modules/.bin/prettier \"src/**/*\" --write",
"lint": "tslint -t stylish --project \"tsconfig.json\"",
"test": "jest --runInBand",
"test:clear-cache": "jest --clearCache",
"test:watch": "npm run test -- --watchAll --runInBand",
"type-check:watch": "npm run type-check -- --watch",
"type-check": "tsc --noEmit",
Expand Down Expand Up @@ -93,6 +94,7 @@
"tslint-eslint-rules": "^5.4.0",
"tslint-immutable": "^5.5.2",
"typescript": "^3.8.3",
"victory": "^34.0.0"
"victory": "^34.0.0",
"wait-for-expect": "^3.0.2"
}
}
5 changes: 5 additions & 0 deletions src/filters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,11 @@ class BaseFilter<Fields extends string, Config extends BaseFilterConfig, Filter
* Sets a filter for a field.
*/
public setFilter(field: Fields, filter: Filter): void {
if (this.fieldConfigs[field] === undefined) {
throw new Error(
`${field} filter field doesnt exist. Either add it explicitly (example for range filters: https://github.com/social-native/snpkg-client-elasticsearch#instantiate-a-manager-with-specific-config-options-for-a-range-filter) or run an introspection query (via manager.getFieldNamesAndTypes())`
);
}
runInAction(() => {
set(this.fieldFilters, {
[field]: filter
Expand Down
3 changes: 2 additions & 1 deletion src/filters/range_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ const convertLesserRanges = (filter: RangeFieldFilter) => {
if (isLessThanFilter(filter)) {
return {lt: filter.lessThan};
} else if (isLessThanEqualFilter(filter)) {
return {gt: filter.lessThanEqual};
return {lte: filter.lessThanEqual};
} else {
return undefined;
}
Expand Down Expand Up @@ -319,6 +319,7 @@ class RangeFilterClass<RangeFields extends string> extends BaseFilter<
if (!this.fieldFilters) {
return request;
}

// tslint:disable-next-line
return objKeys(this.fieldConfigs).reduce((acc, rangeFieldName) => {
if (!this.fieldFilters) {
Expand Down
39 changes: 31 additions & 8 deletions src/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,14 @@ class Manager<
runInAction(() => {
if (response && response.hits && response.hits.hits) {
this.results = response.hits.hits;
} else {
this.results = [];
}
});
} else {
runInAction(() => {
this.results = [];
});
}
runInAction(() => {
this.rawESResponse = response;
Expand Down Expand Up @@ -1038,7 +1044,9 @@ class Manager<
this._extractSuggestionStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // No cursor change b/c only dealing with filters
Expand All @@ -1062,7 +1070,9 @@ class Manager<
this._extractSuggestionStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // No cursor change b/c only dealing with filters
Expand All @@ -1080,7 +1090,9 @@ class Manager<
this._extractUnfilteredAggsStateFromResponse(formattedResponse);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} finally {
Expand All @@ -1096,6 +1108,7 @@ class Manager<
effectRequest,
BLANK_ES_REQUEST
);

const response = await this.client.search(removeEmptyArrays(request));

// Save the results
Expand All @@ -1105,7 +1118,9 @@ class Manager<
this._extractFilteredAggsStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} finally {
Expand Down Expand Up @@ -1159,7 +1174,9 @@ class Manager<
this._extractFilteredAggsStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // No cursor change b/c only dealing with filters
Expand All @@ -1183,7 +1200,9 @@ class Manager<
this._extractUnfilteredAggsStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // No cursor change b/c only dealing with filters
Expand All @@ -1198,7 +1217,9 @@ class Manager<
this._extractUnfilteredAggsStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // no cursor change b/c that should already be handled by the initial request in the batch
Expand All @@ -1213,7 +1234,9 @@ class Manager<
this._extractFilteredAggsStateFromResponse(response);

// Timeout used as the debounce time.
await Timeout.set(this.queryThrottleInMS);
if (this.queryThrottleInMS > 0) {
await Timeout.set(this.queryThrottleInMS);
}
} catch (e) {
throw e;
} // no cursor change b/c that should already be handled by the initial request in the batch
Expand Down
6 changes: 6 additions & 0 deletions src/suggestions/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ class BaseSuggestion<Fields extends string, Config extends BaseSuggestionConfig>
}

public setSearch = (field: Fields, searchTerm: string) => {
if (this.fieldConfigs[field] === undefined) {
throw new Error(
`${field} search field doesnt exist. Either add it explicitly (example for range filters: https://github.com/social-native/snpkg-client-elasticsearch#instantiate-a-manager-with-specific-config-options-for-a-range-filter) or run an introspection query (via manager.getFieldNamesAndTypes())`
);
}

runInAction(() => {
set(this.fieldSearches, {
[field]: searchTerm
Expand Down

0 comments on commit c73c08d

Please sign in to comment.