This is a followup of our discussion from July 18th about batching and sorting. I took a deeper dive into the batching feature and have a better understanding of it now. Basically the core issue was, that the batching resolver did not take care of running the initial query. Instead, the initial query had to be run "manually" in a custom "resolve_" field as indicated in the tests: https://github.com/graphql-python/graphene-sqlalchemy/blob/dfee3e9417cdb8a6ec67b5cd79ee203ce4f72ed7/graphene_sqlalchemy/tests/test_batching.py#L67 That of course bypassed the "get_query" function in the resolver and thus also made sorting "impossible". Let me know what you think.

https://github.com/graphql-python/graphene-sqlalchemy/blob/ac57fd4ba78a786f36aa0c948596a66766c33ec7/graphene_sqlalchemy/enums.py#L147

Changing this to field_name would fix the errors with sqa-13. It seems like column_prop of Reporter has the name set to something similar to %(4447102096 anon)s at the time of sort enum generation. This causes def sort_enum_for_object_type to generate a sort enum with field names not allowed by GQL (%(4447102096 anon)s_ASC & %(4447102096 anon)s_DESC), causing the test failure. Couldn't figure out why get_schema does not reproduce the same behavior although the same models were used.

I still need to investigate those failing tests

Codecov Report

Merging #355 (027ea0e) into master (bb7af4b) will increase coverage by 0.12%. The diff coverage is 96.38%.

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
+ Coverage   96.37%   96.50%   +0.12%     
==========================================
  Files           9        9              
  Lines         773      801      +28     
==========================================
+ Hits          745      773      +28     
  Misses         28       28              

Impacted Files	Coverage Δ
graphene_sqlalchemy/batching.py	95.45% <94.11%> (+1.70%)	:arrow_up:
graphene_sqlalchemy/fields.py	99.15% <97.87%> (+0.13%)	:arrow_up:
graphene_sqlalchemy/enums.py	97.80% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

The tests pass now, the remaining issue was that sqla1.2 is creating slighlty different select statments so my asserts where not working there. This is ready from my side now, please give it another look.

It would be great @sabard, if you could write the test for the potential issue you have in mind. If it fails I will look into it and work on a solution.

@PaulSchweizer @sabard are we ready to merge?