-
Notifications
You must be signed in to change notification settings - Fork 0
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 where clause tests and relay node tests #53
Conversation
@@ -29,3 +29,13 @@ async def resolve_nodes(cls, *, info: Info, node_ids: Iterable[str], required: b | |||
gql_type: str = cls.__strawberry_definition__.name # type: ignore | |||
sql_model = getattr(db_module, gql_type) | |||
return await dataloader.resolve_nodes(sql_model, node_ids) | |||
|
|||
@classmethod | |||
async def resolve_node(cls, node_id: str, info: Info, required: bool = False) -> Any: |
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 is a resolver for the top level node
field required by relay -- it's (barely) documented in the Strawberry docs here:
https://strawberry.rocks/docs/guides/relay#convert-the-node-to-its-proper-type-when-resolving-the-connection
@@ -83,8 +83,17 @@ def resolver( | |||
info: Info, | |||
id: Annotated[strawberry.ID, argument(description="The ID of the object.")], | |||
): | |||
return id.resolve_type(info).resolve_node( | |||
id.node_id, | |||
type_resolvers = [] |
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.
We have our own node
data loader instead of using the one supplied by strawberry, since Strawberry makes weird assumptions about the format of the id
field that we don't adhere to. Strawberry assumes all id
fields are base64(f"{type}:{int}")
which we don't need because we're using globally-unique identifiers
), | ||
{"field_name": entity_field_name}, |
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.
Oops, this was a bug - if we have an entity with multiple File
fields, we weren't specifying which file to update the entity field id's with.
docker compose up -d | ||
sleep 5 | ||
sleep 5 # wait for the app to reload after having files updated. |
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.
I'm hoping 🤞 that this is enough - there's nothing slow happening between the first and second sleep, but we can add it back in or increase the time window if builds fail
@@ -55,6 +54,7 @@ clean: ## Remove all codegen'd artifacts. | |||
rm -rf cerbos | |||
rm -rf support | |||
rm -rf database | |||
rm -rf test_infra |
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.
Missed a cleanup step
entity_type = entity.__class__.__name__ | ||
node_id = f"{entity_type}:{entity.id}".encode("ascii") | ||
node_id_b64 = base64.b64encode(node_id).decode("utf-8") | ||
return node_id_b64 |
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 was code to support the old/lame strawberry ID generation, which we don't want anymore.
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.
🎉
No description provided.