-
Notifications
You must be signed in to change notification settings - Fork 10
fix datetime and shedule #584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request updates the GraphQL schema field name from time to datetime and adds support for a new schedule field type. These changes align the codebase with an updated API schema that provides more semantically accurate field names and expands functionality to support schedule data.
Changes:
- Renamed
timefield todatetimeacross all GraphQL queries, type definitions, and tests - Added
schedulefield to GraphQL queries and type definitions - Introduced
ScheduleStringtype for creating schedule property schemas
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/test/entity/find-one-public.test.ts | Updated test helper to use datetime and schedule fields |
| packages/hypergraph/test/entity/find-many-public.test.ts | Updated test helper to use datetime and schedule fields |
| packages/hypergraph/src/utils/relation-query-helpers.ts | Updated GraphQL fragment to query datetime and schedule instead of time |
| packages/hypergraph/src/utils/convert-relations.ts | Updated ValueList type to use datetime and schedule fields |
| packages/hypergraph/src/utils/convert-property-value.ts | Updated property type and added handler for schedule property type |
| packages/hypergraph/src/type/type.ts | Added new ScheduleString schema type with proper annotations |
| packages/hypergraph/src/entity/search-many-public.ts | Updated GraphQL query to use datetime and schedule fields |
| packages/hypergraph/src/entity/find-one-public.ts | Updated GraphQL query to use datetime and schedule fields |
| packages/hypergraph/src/entity/find-many-public.ts | Updated GraphQL query and ValuesList type to use datetime and schedule fields |
| packages/hypergraph-react/test/prepare-publish.test.ts | Updated test data to use datetime field |
| packages/hypergraph-react/src/prepare-publish.ts | Updated GraphQL query, type definition, and date comparison to use datetime field |
| .changeset/bright-planes-unite.md | Added changeset documenting the field rename and new schedule type |
Comments suppressed due to low confidence (1)
packages/hypergraph-react/src/prepare-publish.ts:176
- The
preparePublishfunction is missing a handler for the new 'schedule' property type. While the GraphQL query and type definitions have been updated to include the schedule field, the property type handling logic (lines 153-176) does not have a case forpropertyType.value === 'schedule'. This means schedule properties will fall through to the default string handling case, which may not be the intended behavior. Add an explicit handler for the 'schedule' property type similar to how 'date' and 'point' are handled.
if (propertyType.value === 'boolean') {
const newValue = entity[prop.name] as boolean;
hasChanged = existingValueEntry?.boolean !== newValue;
typedValue = { property: propertyId.value, type: 'bool', value: newValue };
} else if (propertyType.value === 'date') {
const dateValue = entity[prop.name] as Date;
const newValue = dateValue.toISOString().split('T')[0];
hasChanged = existingValueEntry?.datetime !== newValue;
typedValue = { property: propertyId.value, type: 'date', value: newValue };
} else if (propertyType.value === 'point') {
const [lon, lat] = entity[prop.name] as [number, number];
const newValue = `${lon},${lat}`;
hasChanged = existingValueEntry?.point !== newValue;
typedValue = { property: propertyId.value, type: 'point', lon, lat };
} else if (propertyType.value === 'number') {
const newValue = entity[prop.name] as number;
hasChanged = existingValueEntry?.float !== newValue;
typedValue = { property: propertyId.value, type: 'float64', value: newValue };
} else {
// string (text)
const newValue = entity[prop.name] as string;
hasChanged = existingValueEntry?.text !== newValue;
typedValue = { property: propertyId.value, type: 'text', value: newValue };
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const ScheduleString = (propertyId: string) => { | ||
| return Schema.String.pipe(Schema.annotations({ [PropertyIdSymbol]: propertyId, [PropertyTypeSymbol]: 'schedule' })); | ||
| }; |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ScheduleString type lacks test coverage. While other property types like Date, Point, Number, Boolean, and String are tested in the test suite, there are no tests demonstrating how to use ScheduleString or verifying its behavior. Consider adding test cases that use the ScheduleString type in an entity schema, similar to how other property types are tested.
No description provided.