Add support for Non-Null SQLAlchemyConnectionField #261
Conversation
graphene_sqlalchemy/fields.py
Outdated
| return rewrap(of_type._meta.connection) | ||
|
|
||
| @property | ||
| def connection_type(self): |
There was a problem hiding this comment.
The name is confusing since type already returns a connection. Also I'd rather not add another public property to this class since, as far as I can tell, it is only going to be used in model and get_resolver.
Can you instead call unwrap / get_nullable_type directly in those functions? This is in-line with graphene.relay.ConnectionField.
There was a problem hiding this comment.
You need to call get_nullable_type in model and get_resolver as they expect non-wrapped types. Can you add something like this to your tests to make sure this is caught?
class Query(ObjectType):
pets = SQLAlchemyConnectionField(Pet.connection, required=True)
Schema(query=Query)
|
@jnak Thanks for the great feedback on this PR. I've listed the changes in the last commit message. Anything else let me know. |
| @@ -17,6 +17,11 @@ class Meta: | |||
| interfaces = (Node,) | |||
|
|
|||
|
|
|||
| class PetConnection(Connection): | |||
There was a problem hiding this comment.
You don't need this. You can call Pet.connection directly below
graphene_sqlalchemy/fields.py
Outdated
| return rewrap(of_type._meta.connection) | ||
|
|
||
| @property | ||
| def connection_type(self): |
There was a problem hiding this comment.
You need to call get_nullable_type in model and get_resolver as they expect non-wrapped types. Can you add something like this to your tests to make sure this is caught?
class Query(ObjectType):
pets = SQLAlchemyConnectionField(Pet.connection, required=True)
Schema(query=Query)
Since SQLAlchemy 1.3.16 `orm.selectinload` will no longer `ORDER BY` the primary key of the parent entity.
|
No problem at all, thanks for taking the time. |
SQLAlchemyConnectionFielddoesn't handle Non-Null wrapped types. Currently, passingrequired=TruetoSQLAlchemyConnectionFieldraises an exception.