Enumerate sort.field with '*' #365
Conversation
src/wildcard.ts
Outdated
| @@ -279,6 +279,9 @@ export const DEFAULT_ENUM_INDEX: EnumIndex = { | |||
| 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
| @@ -375,6 +375,18 @@ export const FIELD_CONSTRAINTS: EncodingConstraintModel<FieldQuery>[] = [ | |||
| } | |||
| 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
| @@ -492,6 +492,25 @@ describe('constraints/encoding', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| 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? |
yhoonkim
force-pushed
the
yh/enum-sort-autoCount
branch
from
3c480e3
to
f47c927
Compare
|
@kanitw I rebased this branch but still tests fail. |
kanitw
force-pushed
the
yh/enum-sort-autoCount
branch
from
f47c927
to
a373f5c
Compare
I added
autoAddCountOnSortonconfigto enable to enumeratesort.fieldwith '*'.