Skip to content

Upgrade deps #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daffl merged 2 commits into from
Jul 13, 2021
Merged

Upgrade deps #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46f52f3
Upgrade all dependencies to the latest versions
KidkArolis Dec 17, 2020
753e6c2
Include the latest adapter tests and fix patch() behaviour
KidkArolis Dec 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { _ } = require('@feathersjs/commons');
const { AdapterService } = require('@feathersjs/adapter-commons');
const isPlainObject = require('is-plain-object');
const { isPlainObject } = require('is-plain-object');
const errors = require('@feathersjs/errors');

const errorHandler = require('./error-handler');
Expand Down Expand Up @@ -102,7 +102,8 @@ class Service extends AdapterService {
return query[method].call(query, column, value);
}

return operator === '=' ? query.where(column, value)
return operator === '='
? query.where(column, value)
: query.where(column, operator, value);
});

Expand Down Expand Up @@ -251,11 +252,12 @@ class Service extends AdapterService {
[`${this.table}.${this.id}`]: { $in: idList }
};
const q = this.knexify(this.db(params), query);
const originalQuerySubset = params.query && params.query.$select ? { $select: params.query.$select } : {};
const findParams = Object.assign({}, params, {
query: Object.assign({}, params.query, query)
query: Object.assign(originalQuerySubset, query)

@KidkArolis KidkArolis Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, the adapter test .patch multi query changed fails, because the original query is { age: 10 }, but we patched age to 2 in that test, which results in not fetching any result.

At first, I wanted to introduce returning arg to q.update, like it's done in create/insert case, but that doesn't work for sqlite. So I've modified the code to only apply the $select from the original query for filtering the result set, but keep all other query bits out, so that we always reliably return patched objects (we don't want to do any filtering).

This does seem a bit of a backwards incompatible change, especially if someone was relying on this behaviour before (e.g. expecting nothing to be returned due to filtering, or using some other special query param that I haven't taken into account). But this is what was needed (or something like it) to pass the adapter tests.

});

return q.update(data).then(() => {
return q.update(data).then((rows) => {
return this._findOrGet(null, findParams).then(items => {
if (id !== null) {
if (items.length === 1) {
Expand Down
Loading