add orderBy to Entities.findManyPublic and useEntities(mode: public) - #553
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for ordering entities by property values in the public entity query APIs. The feature enables developers to sort results by any property field in ascending or descending order.
- Adds
orderByparameter toFindManyPublicParamsandQueryPublicParamstypes - Implements ordering logic with new GraphQL queries for
entitiesOrderedByProperty - Updates React hooks (
useEntities,useEntitiesPublic) to support the orderBy parameter
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/entity/find-many-public.ts | Adds orderBy parameter support, new GraphQL queries for ordered entity retrieval (Level 0-2), and validation logic to extract propertyId from schema |
| packages/hypergraph-react/src/internal/use-entities-public.tsx | Updates hook to extract and pass orderBy parameter to findManyPublic, includes it in query key for cache invalidation |
| packages/hypergraph-react/src/internal/types.ts | Adds orderBy type definition to QueryPublicParams |
| packages/hypergraph-react/src/hooks/use-entities.tsx | Updates useEntities hook to extract and forward orderBy parameter when mode is 'public' |
| apps/events/src/routes/podcasts.lazy.tsx | Demonstrates orderBy usage by sorting podcasts by dateFounded in ascending order |
| .changeset/calm-games-trade.md | Documents the feature addition for the patch release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+497
to
+501
| const propertyIdAnnotation = SchemaAST.getAnnotation<string>(Constants.PropertyIdSymbol)(propertyType); | ||
|
|
||
| if (Option.isNone(propertyIdAnnotation)) { | ||
| throw new Error(`Property "${String(orderBy.property)}" is missing a propertyId annotation`); | ||
| } |
There was a problem hiding this comment.
The orderBy logic should validate that the property being ordered by is not a relation field. Ordering by relation fields is not supported and would fail at the GraphQL layer. Consider adding a check using Utils.isRelation(propertyType) and throwing an error if it's a relation.
Comment on lines
+181
to
+341
| const entitiesOrderedByPropertyQueryDocumentLevel0 = gql` | ||
| query entitiesOrderedByProperty($spaceId: UUID!, $typeIds: [UUID!]!, $first: Int, $filter: EntityFilter!, $offset: Int, $propertyId: UUID!, $sortDirection: SortOrder!) { | ||
| entities: entitiesOrderedByProperty( | ||
| filter: { and: [{ | ||
| relations: {some: {typeId: {is: "8f151ba4-de20-4e3c-9cb4-99ddf96f48f1"}, toEntityId: {in: $typeIds}}}, | ||
| spaceIds: {in: [$spaceId]}, | ||
| }, $filter]} | ||
| first: $first | ||
| offset: $offset | ||
| propertyId: $propertyId | ||
| sortDirection: $sortDirection | ||
| ) { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const entitiesOrderedByPropertyQueryDocumentLevel1 = gql` | ||
| query entitiesOrderedByProperty($spaceId: UUID!, $typeIds: [UUID!]!, $relationTypeIdsLevel1: [UUID!]!, $first: Int, $filter: EntityFilter!, $offset: Int, $propertyId: UUID!, $sortDirection: SortOrder!) { | ||
| entities: entitiesOrderedByProperty( | ||
| first: $first | ||
| filter: { and: [{ | ||
| relations: {some: {typeId: {is: "8f151ba4-de20-4e3c-9cb4-99ddf96f48f1"}, toEntityId: {in: $typeIds}}}, | ||
| spaceIds: {in: [$spaceId]}, | ||
| }, $filter]} | ||
| offset: $offset | ||
| propertyId: $propertyId | ||
| sortDirection: $sortDirection | ||
| ) { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| relationsList( | ||
| filter: {spaceId: {is: $spaceId}, typeId:{ in: $relationTypeIdsLevel1}}, | ||
| ) { | ||
| id | ||
| entity { | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| toEntity { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| typeId | ||
| } | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const entitiesOrderedByPropertyQueryDocumentLevel2 = gql` | ||
| query entitiesOrderedByProperty($spaceId: UUID!, $typeIds: [UUID!]!, $relationTypeIdsLevel1: [UUID!]!, $relationTypeIdsLevel2: [UUID!]!, $first: Int, $filter: EntityFilter!, $offset: Int, $propertyId: UUID!, $sortDirection: SortOrder!) { | ||
| entities: entitiesOrderedByProperty( | ||
| first: $first | ||
| filter: { and: [{ | ||
| relations: {some: {typeId: {is: "8f151ba4-de20-4e3c-9cb4-99ddf96f48f1"}, toEntityId: {in: $typeIds}}}, | ||
| spaceIds: {in: [$spaceId]}, | ||
| }, $filter]} | ||
| offset: $offset | ||
| propertyId: $propertyId | ||
| sortDirection: $sortDirection | ||
| ) { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| relationsList( | ||
| filter: {spaceId: {is: $spaceId}, typeId:{ in: $relationTypeIdsLevel1}}, | ||
| ) { | ||
| id | ||
| entity { | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| toEntity { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| relationsList( | ||
| filter: {spaceId: {is: $spaceId}, typeId:{ in: $relationTypeIdsLevel2}}, | ||
| ) { | ||
| id | ||
| entity { | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| toEntity { | ||
| id | ||
| name | ||
| valuesList(filter: {spaceId: {is: $spaceId}}) { | ||
| propertyId | ||
| string | ||
| boolean | ||
| number | ||
| time | ||
| point | ||
| } | ||
| } | ||
| typeId | ||
| } | ||
| } | ||
| typeId | ||
| } | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
[nitpick] The three new entitiesOrderedByProperty GraphQL query documents (Level0, Level1, Level2) are essentially duplicates of the original queries with added sorting parameters. This creates significant code duplication (~160 lines) that will be difficult to maintain. Consider refactoring to use a query builder pattern or template string functions to generate these queries dynamically, reducing duplication and making future changes easier to maintain.
| const propertySignature = ast.propertySignatures.find((prop) => String(prop.name) === String(orderBy.property)); | ||
|
|
||
| if (!propertySignature) { | ||
| throw new Error(`Cannot order by unknown property "${String(orderBy.property)}"`); |
There was a problem hiding this comment.
The error message should provide more actionable guidance. Consider including available properties in the error message to help developers debug the issue, e.g., Cannot order by unknown property "${String(orderBy.property)}". Available properties: ${availableProps.join(', ')}.
Suggested change
| throw new Error(`Cannot order by unknown property "${String(orderBy.property)}"`); | |
| const availableProps = ast.propertySignatures.map((prop) => String(prop.name)).join(', '); | |
| throw new Error(`Cannot order by unknown property "${String(orderBy.property)}". Available properties: ${availableProps}`); |
Comment on lines
+6
to
+7
| add orderBy to Entities.findManyPublic and useEntities(mode: 'public') | ||
|
No newline at end of file |
There was a problem hiding this comment.
Remove trailing whitespace at the end of this line. Trailing whitespace is typically considered a linting issue and should be avoided.
Suggested change
| add orderBy to Entities.findManyPublic and useEntities(mode: 'public') | |
| add orderBy to Entities.findManyPublic and useEntities(mode: 'public') |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.