Conversation
src/wildcard.ts
Outdated
| export function getDefaultEnumValues(prop: Property, schema: Schema, opt: QueryConfig): any[] { | ||
| if (prop === 'field' || (isEncodingNestedProp(prop) && prop.parent === 'sort' && prop.child === 'field')) { | ||
| // For field, by default enumerate all fields | ||
| if ((isEncodingNestedProp(prop) && prop.parent === 'sort' && prop.child === 'field') && opt.autoAddCountOnSort){ |
There was a problem hiding this comment.
The insertion here is a bit weird because it makes the following comment outdated:
For field, by default enumerate all fields
The name autoAddCountOnSort is a bit misleading since you are not really adding count here, you are adding '*'.
I'd say remove the misleading autoAddCountOnSort config and just keep this logic here (it seems generally useful thus shouldn't require a config), but add proper comment describing why you need to add '*' to sort.field (to support count).
src/constraint/encoding.ts
Outdated
| return true; | ||
| } | ||
| },{ | ||
| name: 'onlyCountWithAutoCountFieldOnSort', |
There was a problem hiding this comment.
This isn't related to autoCount.
onlyUseCountWithAsteriskSortField?
src/constraint/encoding.ts
Outdated
| allowWildcardForProperties: false, | ||
| strict: true, | ||
| satisfy: (fieldQ: FieldQuery, _: Schema, __: PropIndex<Wildcard<any>>, ___: QueryConfig) => { | ||
| if (fieldQ.sort && !!(fieldQ.sort as SortField).field) { |
There was a problem hiding this comment.
Don't hack using as SortField -- do proper type guard using isSortField in VL's sort.ts
test/constraint/encoding.test.ts
Outdated
| }); | ||
| }); | ||
|
|
||
| describe('onlyCountWithAutoCountFieldOnSort', () => { |
|
|
There are some conflicts, possibly because encoding.ts and its test file have been splitted. |
|
@kanitw |
|
@yhoonkim Can you rebase this when you have time? |
3c480e3 to
f47c927
Compare
|
@kanitw I rebased this branch but still tests fail. |
f47c927 to
a373f5c
Compare
I added
autoAddCountOnSortonconfigto enable to enumeratesort.fieldwith '*'.