Failed to exclude properties by setting false in fields filter via url

[agnes512]

Description

From the docs https://loopback.io/doc/en/lb3/Fields-filter.html ( it's the same for LB4), filter fields can be set to true/false in a query to include/exclude model properties. fields works with API Explorer and Node. But it couldn't set properties to false (exclude) via url.

Current Behavior

For example, model Order has three properties: id, name, and price. To exclude the name property from the result, as the docs suggested, the query can be written as ?filter[fields][name]=false. But it returns:

[{name: 'order 1'}, {name: 'order 2'}..]

i.e it includes only the name property instead. I checked the filter sent to the controller, it shows that the filter is being converted to

{fields: { name: 'false'}}  // string

while with API Explorer, the filter is converted to

{fields: { name: false}}  // boolean

The wrong type might cause the problem.

In order to make it work with urls, the query needs to be set as ?filter[fields][id]=true&filter[fields][price]=true

The issue might relate to #2208

Expected Behavior

fields should be able to exclude properties by setting properties to false via url just like what API Explorer/Node does.

Acceptance

• check where clause if it has the same issue ( see this).
• fix it so that API Explorer/Node/url all have the same behavior
• add tests to check the filter that is sent to the controllers
• fix docs in LB4/Working-with-data/fields-filter page
• Based on the code, it seems accept arrays {fields: ['foo', 'bar']}. Check if array works for fields. Add proper normalization and error checking if needed. (explained in comment Failed to exclude properties by setting false in fields filter via url #4992 (comment))

[dhmlau]

@agnes512, could you please add acceptance criteria? Thanks.

[agnes512]

can start at https://github.com/strongloop/loopback-next/blob/master/packages/rest/src/coercion/coerce-parameter.ts#L172

[agnes512]

Note: stringified JSON works well e.g ?filter[fields][name]=false works

[InvictusMB]

@agnes512 @raymondfeng
I see some more inconsistencies in handling filter.fields in general. Do we expect fields to be {[k: string]: boolean} objects in future or arrays should also be universally acceptable?

https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/repository/src/query.ts#L508-L523

The code above, for example, accepts both arrays and objects as arguments but misbehaves if this.filter.fields is array. Therefore if I set scope: {fields: ['foo', 'bar']} in a model decorator, all inclusions of that model will have broken filters as a result.

Should there be a normalization step somewhere?

[agnes512]

I checked {fields: ['foo', 'bar']} on my end. It won't let me have such filter because of some type constraints.

But I didn't know it accepts arrays. I thought
{ fields: {propertyName: <true|false>, propertyName: <true|false>, ... } } was the only form that it took. This should be checked throughly. I will add it to the criteria. Thanks!

[InvictusMB]

I think my confusion stems from loopback-datasource-juggler.
If I make a request via REST API and it goes directly into the juggler layer it works with arrays and does not report any violations.
If a filter is a part of include clause and gets amended inside @loopback/repository it breaks silently and that filter reaches the juggler mangled.

[jannyHou]

fields: ['foo', 'bar'] is not allowed with bad request error. The reason is, type Fields<T> only allows object, so that type Filter<T> can only have fields as object. Array is now accepted. See:

https://github.com/strongloop/loopback-next/blob/ccea25fc382457f9436adfc0d8f6ce3a2d029c5e/packages/repository/src/query.ts#L196

and

https://github.com/strongloop/loopback-next/blob/ccea25fc382457f9436adfc0d8f6ce3a2d029c5e/packages/repository/src/query.ts#L162

But as @InvictusMB pointed out, the where builder allows array. And LB3 supports array syntax as well.

https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/repository/src/query.ts#L508-L523

I am creating a new story to allow array in type Fields<T>. We can move the further discussion there.

[jannyHou]

Created new story for the array fields: #5857

[jannyHou]

Closed as PRs landed.