-
Notifications
You must be signed in to change notification settings - Fork 769
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
resolve subtypes of postgres RangeField to new RangeType #677
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
from .types import DjangoObjectType | ||
from .fields import DjangoConnectionField | ||
|
||
__version__ = "2.3.0" | ||
__version__ = "2.3.2" | ||
|
||
__all__ = ["__version__", "DjangoObjectType", "DjangoConnectionField"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
from graphene import ObjectType | ||
from graphene import ( | ||
Float, | ||
Int, | ||
DateTime, | ||
Date | ||
) | ||
|
||
class RangeResolver: | ||
def resolve_lower(parent, info): | ||
return parent.lower | ||
def resolve_upper(parent, info): | ||
return parent.upper | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the resolvers just pick attributes on the parent object that are the same name as the field then you don't need these resolvers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try |
||
|
||
class DateTimeRangeType(RangeResolver, ObjectType): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to http://initd.org/psycopg/docs/extras.html#adapt-range There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bounds is not accessible. you said missing is referring to mutation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jkimbo here is attributes:
I see original method either not implement it, so I didn't too. |
||
lower = DateTime() | ||
upper = DateTime() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these fields There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not quite, I think it could be optional. One problem is the original method which returns a tuple for (lower, upper), it is working for IntegerRange which is tested in test cases, but it is not working for DateTimeRange. |
||
|
||
class DateRangeType(RangeResolver, ObjectType): | ||
lower = Date() | ||
upper = Date() | ||
|
||
class IntRangeType(RangeResolver, ObjectType): | ||
lower = Int() | ||
upper = Int() | ||
|
||
class FloatRangeType(RangeResolver, ObjectType): | ||
lower = Float() | ||
upper = Float() |
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.
Can you revert this? The version number will be incremented when we create a new release.
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'll check. I didn't remember I changed version, could it be due to merge from some branch?