Skip to content

Commit f65d5c1

Browse files
baiirunbaiirun
committed
fix: harden orderBy dataType behavior and coverage
Make ordered queries include dataType only when it is resolvable, and avoid forcing a nullable dataType argument into every orderBy query. This keeps SDK behavior compatible with API fallback while preventing ambiguous null argument paths. Also add request-level tests for findManyPublic orderBy payload wiring and move mapping checks into a focused orderBy test suite.
1 parent d465b01 commit f65d5c1

4 files changed

Lines changed: 123 additions & 59 deletions

File tree

‎packages/hypergraph/src/entity/find-many-public.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export type FindManyPublicParams<
3535
const buildEntitiesQuery = (
3636
relationInfoLevel1: RelationTypeIdInfo[],
3737
useOrderBy: boolean,
38+
includeOrderByDataType: boolean,
3839
spaceSelection: SpaceSelection,
3940
includeSpaceIds: boolean,
4041
) => {
@@ -50,7 +51,7 @@ const buildEntitiesQuery = (
5051
: undefined,
5152
'$typeIds: [UUID!]!',
5253
useOrderBy ? '$propertyId: UUID!' : undefined,
53-
useOrderBy ? '$dataType: String' : undefined,
54+
useOrderBy && includeOrderByDataType ? '$dataType: String' : undefined,
5455
useOrderBy ? '$sortDirection: SortOrder!' : undefined,
5556
'$first: Int',
5657
'$filter: EntityFilter!',
@@ -69,7 +70,8 @@ const buildEntitiesQuery = (
6970
// entitiesOrderedByProperty doesn't support the native typeIds filter yet,
7071
// so we fall back to the relation-based filter for orderBy queries
7172
if (useOrderBy) {
72-
const orderByArgs = 'propertyId: $propertyId\n dataType: $dataType\n sortDirection: $sortDirection\n ';
73+
const orderByDataTypeArg = includeOrderByDataType ? 'dataType: $dataType\n ' : '';
74+
const orderByArgs = `propertyId: $propertyId\n ${orderByDataTypeArg}sortDirection: $sortDirection\n `;
7375
const entitySpaceFilter =
7476
spaceSelection.mode === 'single'
7577
? 'spaceIds: {in: [$spaceId]},'
@@ -316,7 +318,13 @@ export const findManyPublic = async <
316318
const spaceSelection = normalizeSpaceSelection(space, spaces);
317319

318320
// Build the query dynamically with aliases for each relation type ID
319-
const queryDocument = buildEntitiesQuery(relationTypeIds, Boolean(orderBy), spaceSelection, includeSpaceIds);
321+
const queryDocument = buildEntitiesQuery(
322+
relationTypeIds,
323+
Boolean(orderBy),
324+
Boolean(orderByDataType),
325+
spaceSelection,
326+
includeSpaceIds,
327+
);
320328

321329
const filterParams = filter ? Utils.translateFilterToGraphql(filter, type) : {};
322330

‎packages/hypergraph/src/utils/convert-property-value.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ const ORDER_BY_DATA_TYPE_BY_PROPERTY_TYPE: Record<string, OrderByDataType | unde
1111
date: 'datetime',
1212
point: 'point',
1313
schedule: 'schedule',
14-
relation: undefined,
1514
};
1615

1716
export const getOrderByDataType = (type: SchemaAST.AST): OrderByDataType | undefined => {
1817
const propertyType = SchemaAST.getAnnotation<string>(Constants.PropertyTypeSymbol)(type);
1918
if (Option.isSome(propertyType)) {
20-
return ORDER_BY_DATA_TYPE_BY_PROPERTY_TYPE[propertyType.value];
19+
const mappedType = ORDER_BY_DATA_TYPE_BY_PROPERTY_TYPE[propertyType.value];
20+
if (mappedType) {
21+
return mappedType;
22+
}
2123
}
2224

2325
if (SchemaAST.isStringKeyword(type)) {

‎packages/hypergraph/test/entity/find-many-public-orderby.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { Id } from '@geoprotocol/geo-sdk';
2+
import { beforeEach, describe, expect, it, vi } from 'vitest';
3+
import { findManyPublic } from '../../src/entity/find-many-public.js';
4+
import * as Entity from '../../src/entity/index.js';
5+
import * as Type from '../../src/type/type.js';
6+
import { getOrderByDataType } from '../../src/utils/convert-property-value.js';
7+
8+
const mockRequest = vi.hoisted(() => vi.fn());
9+
10+
vi.mock('graphql-request', () => ({
11+
request: mockRequest,
12+
}));
13+
14+
const TITLE_PROPERTY_ID = Id('79c1a9510074401087d07501ef9d7b3d');
15+
const SCORE_PROPERTY_ID = Id('0f0f62df02194f16983ad2ae5fc43ee5');
16+
const CHILDREN_RELATION_PROPERTY_ID = Id('ca7c7167250249c490b084c147f9b12b');
17+
const CHILD_NAME_PROPERTY_ID = Id('25584af039414ab986f7a603305b19bb');
18+
19+
const Child = Entity.Schema(
20+
{
21+
name: Type.String,
22+
},
23+
{
24+
types: [Id('3c2ae3aa4ec141e3bc4c1fe7a5e07bc1')],
25+
properties: {
26+
name: CHILD_NAME_PROPERTY_ID,
27+
},
28+
},
29+
);
30+
31+
const Parent = Entity.Schema(
32+
{
33+
title: Type.String,
34+
score: Type.Number,
35+
children: Type.Relation(Child),
36+
},
37+
{
38+
types: [Id('af571d8c06d44add8cfa4c6b50412254')],
39+
properties: {
40+
title: TITLE_PROPERTY_ID,
41+
score: SCORE_PROPERTY_ID,
42+
children: CHILDREN_RELATION_PROPERTY_ID,
43+
},
44+
},
45+
);
46+
47+
describe('findManyPublic orderBy', () => {
48+
beforeEach(() => {
49+
mockRequest.mockReset();
50+
mockRequest.mockResolvedValue({ entities: [] });
51+
});
52+
53+
it('passes inferred dataType for sortable fields', async () => {
54+
await findManyPublic(Parent, {
55+
space: 'space-1',
56+
orderBy: {
57+
property: 'score',
58+
direction: 'desc',
59+
},
60+
logInvalidResults: false,
61+
});
62+
63+
expect(mockRequest).toHaveBeenCalledTimes(1);
64+
const [, queryDocument, queryVariables] = mockRequest.mock.calls[0];
65+
66+
expect(queryDocument as string).toContain('$dataType: String');
67+
expect(queryDocument as string).toContain('dataType: $dataType');
68+
expect(queryVariables).toMatchObject({
69+
propertyId: SCORE_PROPERTY_ID,
70+
dataType: 'float',
71+
sortDirection: 'DESC',
72+
});
73+
});
74+
75+
it('omits dataType for unresolved orderBy field types', async () => {
76+
await findManyPublic(Parent, {
77+
space: 'space-1',
78+
orderBy: {
79+
property: 'children',
80+
direction: 'asc',
81+
},
82+
logInvalidResults: false,
83+
});
84+
85+
expect(mockRequest).toHaveBeenCalledTimes(1);
86+
const [, queryDocument, queryVariables] = mockRequest.mock.calls[0];
87+
88+
expect(queryDocument as string).not.toContain('$dataType: String');
89+
expect(queryDocument as string).not.toContain('dataType: $dataType');
90+
expect(queryVariables).toMatchObject({
91+
propertyId: CHILDREN_RELATION_PROPERTY_ID,
92+
sortDirection: 'ASC',
93+
});
94+
expect((queryVariables as Record<string, unknown>).dataType).toBeUndefined();
95+
});
96+
});
97+
98+
describe('getOrderByDataType', () => {
99+
it('maps schema builder outputs to GraphQL order dataType values', () => {
100+
expect(getOrderByDataType(Type.String('prop').ast)).toBe('text');
101+
expect(getOrderByDataType(Type.Number('prop').ast)).toBe('float');
102+
expect(getOrderByDataType(Type.Boolean('prop').ast)).toBe('boolean');
103+
expect(getOrderByDataType(Type.Date('prop').ast)).toBe('datetime');
104+
expect(getOrderByDataType(Type.Point('prop').ast)).toBe('point');
105+
expect(getOrderByDataType(Type.ScheduleString('prop').ast)).toBe('schedule');
106+
expect(getOrderByDataType(Type.Relation(Child)('prop').ast)).toBeUndefined();
107+
});
108+
});

‎packages/hypergraph/test/entity/find-many-public.test.ts‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,12 @@ import { describe, expect, it } from 'vitest';
33
import { parseResult } from '../../src/entity/find-many-public.js';
44
import * as Entity from '../../src/entity/index.js';
55
import * as Type from '../../src/type/type.js';
6-
import { getOrderByDataType } from '../../src/utils/convert-property-value.js';
76
import { getRelationTypeIds } from '../../src/utils/get-relation-type-ids.js';
87
import { getRelationAlias } from '../../src/utils/relation-query-helpers.js';
98

109
const TITLE_PROPERTY_ID = Id('79c1a9510074401087d07501ef9d7b3d');
1110
const CHILDREN_RELATION_PROPERTY_ID = Id('ca7c7167250249c490b084c147f9b12b');
1211
const CHILD_NAME_PROPERTY_ID = Id('25584af039414ab986f7a603305b19bb');
13-
const SCORE_PROPERTY_ID = Id('0f0f62df02194f16983ad2ae5fc43ee5');
14-
const IS_ACTIVE_PROPERTY_ID = Id('774f4b5dbfaf4af5925ef4c7ef2ebd76');
15-
const PUBLISHED_AT_PROPERTY_ID = Id('2ece4d97ea964a269f3fee0d0f00de53');
16-
const LOCATION_PROPERTY_ID = Id('2df8bd4f7bc34aafaa8db20e3ad41657');
17-
const CADENCE_PROPERTY_ID = Id('0f7952a1f8474b4286d0ef7e6ef8dbb2');
1812

1913
const Child = Entity.Schema(
2014
{
@@ -42,44 +36,6 @@ const Parent = Entity.Schema(
4236
},
4337
);
4438

45-
const OrderableParent = Entity.Schema(
46-
{
47-
title: Type.String,
48-
score: Type.Number,
49-
isActive: Type.Boolean,
50-
publishedAt: Type.Date,
51-
location: Type.Point,
52-
cadence: Type.ScheduleString,
53-
children: Type.Relation(Child),
54-
},
55-
{
56-
types: [Id('af571d8c06d44add8cfa4c6b50412254')],
57-
properties: {
58-
title: TITLE_PROPERTY_ID,
59-
score: SCORE_PROPERTY_ID,
60-
isActive: IS_ACTIVE_PROPERTY_ID,
61-
publishedAt: PUBLISHED_AT_PROPERTY_ID,
62-
location: LOCATION_PROPERTY_ID,
63-
cadence: CADENCE_PROPERTY_ID,
64-
children: CHILDREN_RELATION_PROPERTY_ID,
65-
},
66-
},
67-
);
68-
69-
const getPropertyTypeAst = (property: string) => {
70-
const ast = OrderableParent.ast;
71-
if (!('propertySignatures' in ast)) {
72-
throw new Error('Expected schema AST to be a TypeLiteral');
73-
}
74-
75-
const signature = ast.propertySignatures.find((prop) => String(prop.name) === property);
76-
if (!signature) {
77-
throw new Error(`Property ${property} not found in schema`);
78-
}
79-
80-
return signature.type;
81-
};
82-
8339
const buildValueEntry = (
8440
propertyId: string,
8541
value: Partial<{
@@ -101,16 +57,6 @@ const buildValueEntry = (
10157
});
10258

10359
describe('findManyPublic parseResult', () => {
104-
it('maps schema property types to orderBy data types', () => {
105-
expect(getOrderByDataType(getPropertyTypeAst('title'))).toBe('text');
106-
expect(getOrderByDataType(getPropertyTypeAst('score'))).toBe('float');
107-
expect(getOrderByDataType(getPropertyTypeAst('isActive'))).toBe('boolean');
108-
expect(getOrderByDataType(getPropertyTypeAst('publishedAt'))).toBe('datetime');
109-
expect(getOrderByDataType(getPropertyTypeAst('location'))).toBe('point');
110-
expect(getOrderByDataType(getPropertyTypeAst('cadence'))).toBe('schedule');
111-
expect(getOrderByDataType(getPropertyTypeAst('children'))).toBeUndefined();
112-
});
113-
11460
it('collects invalidEntities when decoding fails', () => {
11561
const queryData = {
11662
entities: [

0 commit comments

Comments
 (0)