Skip to content

Commit 73bfcc4

Browse files
authored
fix: Or filters with warnings (#1075)
* or filters interface * implementation of new filter * Remove duplicate code * Parse with type guard * Remove the newFilter variable * Add filter and enable chaining * test add filter * Add another unit test * Add system-test stubs * Add system-tests for the OR filter * Move things around * Added a unit test * Add a unit test for OR filters * Just use filter method * new warning test * Revert "new warning test" This reverts commit 37400a6. * Now removes deprecation warning properly * Add a test for new warning * Add setAncestor Adds a new setAncestor method for ensuring only one ancestor is set for the query at a time. This will avoid errors that result because of setting multiple ancestors. Also deprecate hasAncestor because it will lead to warnings like this. Add parser logic to use the value provided in setAncestor for query sent to backend. * Basic unit tests for setAncestor Added tests for the query proto, one to make sure the query structure is right when setting ancestor once and one to make sure the query structure is right when setting ancestor twice. Also added a unit test to make sure that ancestor is set the right way internally when using setAncestor. * change expected result for OR query This code change adjusts the expected result for running an OR query. Old result used to correspond with AND, but now corresponds to OR. * Fix a test by not requiring the done callback A test is timing out because we are waiting for done to be called. This fix does not require done to be called. * Revert "Fix a test by not requiring the done callback" This reverts commit 1159b37. * Revert "Basic unit tests for setAncestor" This reverts commit 86841d6. * Revert "Add setAncestor" This reverts commit e84582f. * Separate filters and new filters internally This commit is done to avoid a breaking typescript change which could have the potential to affect some users who read `filters` on a query as its type had been changed to be more flexible, but is now back to what it was. * Move AND/OR into their own separate function AND and OR should not be static functions of the filter class because then the user has to type Filter.AND instead of AND for example. * Eliminate unused imports Artifacts of having imports laying around and moving functionality between files * Revert "Add a test for new warning" This reverts commit bc15f32. * Revert "Now removes deprecation warning properly" This reverts commit db02a50. * Revert "Revert "Now removes deprecation warning properly"" This reverts commit 8e96bf5. * Revert "Revert "Add a test for new warning"" This reverts commit 1ddeea6. * Change the warning message to include examples Change the warning message to include examples as per the nit mentioned in the PR.
1 parent 8fc58c0 commit 73bfcc4

File tree

3 files changed

+19
-3
lines changed

3 files changed

+19
-3
lines changed

src/query.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ class Query {
218218
this.entityFilters.push(propertyOrFilter);
219219
return this;
220220
} else {
221+
process.emitWarning(
222+
'Providing Filter objects like Composite Filter or Property Filter is recommended when using .filter'
223+
);
221224
let operator = operatorOrValue as Operator;
222225
if (arguments.length === 2) {
223226
value = operatorOrValue as {};

test/entity.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,10 +770,12 @@ describe('entity', () => {
770770
"' property is outside of bounds of a JavaScript Number.\n" +
771771
"Use 'Datastore.int(<integer_value_as_string>)' to preserve accuracy during the upload.";
772772

773-
process.on('warning', warning => {
773+
const onWarning = (warning: {message: unknown}) => {
774774
assert.strictEqual(warning.message, expectedWarning);
775+
process.removeListener('warning', onWarning);
775776
done();
776-
});
777+
};
778+
process.on('warning', onWarning);
777779
entity.encodeValue(largeIntValue, property);
778780
});
779781

test/query.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,18 @@ describe('Query', () => {
165165
assert.strictEqual(filter.val, 'Stephen');
166166
});
167167
});
168-
168+
it('should issue a warning when a Filter instance is not provided', done => {
169+
const onWarning = (warning: {message: unknown}) => {
170+
assert.strictEqual(
171+
warning.message,
172+
'Providing Filter objects like Composite Filter or Property Filter is recommended when using .filter'
173+
);
174+
process.removeListener('warning', onWarning);
175+
done();
176+
};
177+
process.on('warning', onWarning);
178+
new Query(['kind1']).filter('name', 'Stephen');
179+
});
169180
describe('filter with Filter class', () => {
170181
it('should support filter with Filter', () => {
171182
const now = new Date();

0 commit comments

Comments
 (0)