fix(memory/mongodb): $select as only property & force 'id' in '$select'#3081
fix(memory/mongodb): $select as only property & force 'id' in '$select'#3081
Conversation
|
This came up while porting feathers-graph-populate to dove (cc @marshallswain). This stops me from releasing dove versions of these two packages. |
|
I think that makes sense. It looks like the virtual property resolver test needs to be updated accordingly. I'm also wondering if this could be considered a breaking change 🤔 |
|
Sorry gents! Glad yall got it worked out. |
The test expects two properties (I guess
No, not at all. This was how every adapter was working in v4, even |
Quick fix for
@feathersjs/memory.First commit is the test which should work but fails without the second commit.
The problem was, that
select(...)only was called if there was$sort,$limitor$skipin the query.While being at it:
$selectalso returningid, also if not added explicitely.idautomatically, which is crucial for crud! https://github.com/feathersjs/feathers/blob/dove/packages/memory/test/index.test.ts#L193 I think this was introduced by recent changes from @DaddyWarbucksidautomatically, if it's not provided: https://github.com/feathersjs/feathers/blob/dove/packages/knex/src/adapter.ts#L128feathers-sequelizealso addsidI think that should be tested by
@feathersjs/adapter-tests. We have it in:.find + $selectnot included ❌ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/syntax.ts#L129.get + $select: included ✅ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/methods.ts#L32.create + $select: not included ❌ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/methods.ts#L620.patch + $select: not included ❌ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/methods.ts#L329.update + $select: not included ❌ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/methods.ts#L247.remove + $select: not included ❌ https://github.com/feathersjs/feathers/blob/dove/packages/adapter-tests/src/methods.ts#L97This PR adds explicit tests to
@feathersjs/adapter-testsand makes sure, that all tests pass for these linked tests.