EMF support annotations, group by, and ordering #225
Conversation
WaVEV
force-pushed
the
EMF-support-group-by
branch
from
8c0e138 to
d7d40c3
Compare
WaVEV
changed the title
| @@ -104,6 +113,46 @@ def test_nested(self): | |||
| ) | |||
| self.assertCountEqual(Book.objects.filter(author__address__city="NYC"), [obj]) | |||
|
|
|||
| def truncate_ms(self, value): | |||
There was a problem hiding this comment.
Helper methods like this should typically be organized at the top of a class so they don't lost among test methods, however, I suppose we should make it a function in this file so we don't have to repeat the method on every class (the move can be a separate commit in this PR that precedes the fix).
| (Holder.objects.create(data=Data(integer=x)) for x in range(6)), | ||
| key=lambda x: x.data.integer, | ||
| ) | ||
| query = ( |
There was a problem hiding this comment.
I would rather use qs = since "query" usually refers to SQL or, in our case, MQL.
| """Truncate microsends to millisecond precision as supported by MongoDB.""" | ||
| return value.replace(microsecond=(value.microsecond // 1000) * 1000) | ||
|
|
||
| def test_ordering_by_embedded_field(self): |
There was a problem hiding this comment.
This is good to have, but it passes without the fix, so it should be a separate commit. (I think the fix is mostly about group by, not ordering without grouping.) I would sort these tests above test_nested since they also use the Holder model.
| key=lambda x: x.data.integer, | ||
| ) | ||
| query = ( | ||
| Holder.objects.annotate( |
There was a problem hiding this comment.
It would be helpful to add some comments about what this query is calculating since it's not straightforward.
| .annotate(max_auto_now=Max("data__auto_now")) | ||
| .order_by("data__integer") | ||
| ) | ||
| query_response = [{**e, "max_auto_now": self.truncate_ms(e["max_auto_now"])} for e in query] |
There was a problem hiding this comment.
Is truncate needed here? I'd expect MongoDB to return an already truncated result.
| self.assertSequenceEqual(query, expected) | ||
|
|
||
| def test_ordering_grouping_by_embedded_field(self): | ||
| expected = sorted( |
There was a problem hiding this comment.
I'd say expected_objs since there is still some modification in calculating the expected result.
| ], | ||
| ) | ||
|
|
||
| def test_ordering_grouping_by_sum(self): |
There was a problem hiding this comment.
Again, it's hard for me to reason about the expected result of query, so some explanatory comment would be useful. For example, the "cheat sheet" at https://docs.djangoproject.com/en/dev/topics/db/aggregation/ is easy to follow.
No description provided.