-
Notifications
You must be signed in to change notification settings - Fork 54
Add LocalDate/LocalDateTime/OffsetDateTime/ZonedDateTime/Instant support for query and filter #158
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
Conversation
@SinaiNIN Great PR! Thank you for contributing. Seems like Travis is failing on the new test you have added because of missing Authors data in We also need more test coverage for new date/time datatypes added to predicate builder. It is best to cover them in a separate module similar to https://github.com/introproventures/graphql-jpa-query/blob/master/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/converter/GraphQLJpaConverterTests.java Let me know if you have any questions or comments. |
@SinaiNIN If you add new test record to Books and Authors, it will break a lot of existing unit tests. It would be best to amend existing records with dateOfBirth and releaseDate and add new tests to cover LocalDate time support... |
Codecov Report
@@ Coverage Diff @@
## master #158 +/- ##
============================================
- Coverage 69.56% 67.95% -1.61%
- Complexity 450 495 +45
============================================
Files 33 33
Lines 2343 2578 +235
Branches 343 431 +88
============================================
+ Hits 1630 1752 +122
- Misses 576 646 +70
- Partials 137 180 +43
Continue to review full report at Codecov.
|
@Test | ||
public void queryLocalDateWithEqualTest() { | ||
//given | ||
String query = "query{\n" + |
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.
@SinaiNIN Can you, please, remove \n
and \t
from queries?
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.
@igdianov Noted, Thanks
|
||
} | ||
|
||
@Test |
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.
@SinaiNIN Can you extract your tests into a separate a package with its own test class, data.sql and test model?
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.
Yes, I can. It would be better.
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.
Lgtm
Great thank b @SinaiNIN ❤️ |
For now, the current library version does not support query and filter when we tried to query with
where
condition onLocalDate/LocalDateTime/OffsetDate/ZonedDateTime/Instant
.The error while query is
The error on console log:
closes #157