-
Notifications
You must be signed in to change notification settings - Fork 345
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
fix: subscriptions payloads _entity w/ relations fields #2626
base: main
Are you sure you want to change the base?
fix: subscriptions payloads _entity w/ relations fields #2626
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.
Im happy with this change. Can you please update the changelog too.
For getting relations working, i think the goal would be to take the id
from the payload and use that to run a graphql query and not use anything from the _entity
. I don't have much direction to give here, postgraphile is not trivial to work with. But the realtime feature might be a good reference as to how this could be implemented.
fa8f030
to
4a6ccd7
Compare
I've updated the changelog. I tried to get relations working too, from what I see in postgraphile documentation, I should be able to get it working with something like this in the resolver const [row] = await resolveInfo.graphile.selectGraphQLResultFromTable(
sql.fragment`${table.namespace.name}.${table.name}`,
(tableAlias, queryBuilder) => {
queryBuilder.where(
sql.fragment`${tableAlias}.id = ${sql.value(parent.id)}`
)
queryBuilder.limit(1);
}
);
return row; unfortunately i always end up with an error |
Ok I found the problem, context was missing from |
a431070
to
17e2381
Compare
I'm thinking of removeing these two lines: https://github.com/subquery/subql/blob/main/packages/node-core/src/db/sync-helper.ts#L197-L198 and use the |
I see no issue with that |
17e2381
to
48dc0c9
Compare
ready for a final review :) |
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.
This looks great, can you please update the changelog for node-core then this can be merged.
48dc0c9
to
cdb30b5
Compare
changelog uptated :) |
There is a test failing in |
Just updated the tests |
Description
With recent changes to subscriptions, the
_entity
field is now graphql-typed, but the said field is resolved with raw data from database, hence all camelCase named fields in the gql schema are not properly resolved. This PR solves this issue.Nonetheless all requested relations of the
_entity
are still not properly resolved... I'll try working on that now... if you have any advice regarding this, that'd be great :)Type of change
Checklist