Iterable classes have incorrect inheritence
MooseV2 opened this issue · 1 comments
By definition, a ValuesIterable should yield dicts for each row in the queryset:
Iterable returned by QuerySet.values() that yields a dict for each row.
Similary, ValuesListIterable yields tuples, and FlatValuesListIterable yields single values.
Therefore, you should be able to do something like this:
if issubclass(queryset_type, django.db.models.query.ValuesIterable):
return [dict_to_json(row) for row in queryset]
if issubclass(queryset_type, django.db.models.query.ValuesListIterable):
return [tuple_to_json(row) for row in queryset]
if issubclass(queryset_type, django.db.models.query.FlatValuesListIterable):
return [value_to_json(value) for value in queryset]
This works correctly with base Django querysets and any managers that inherit from the default manager. However, with Django-modeltranslation, this fails because all of the iterable classes are incorrectly inheriting from ValuesIterable. If you are expecting to parse a dict with ValuesIterable but are given a str or list, things will break.
Minimal reproducible code:
from itertools import product
from modeltranslation.manager import (
FallbackValuesIterable,
FallbackValuesListIterable,
FallbackFlatValuesListIterable,
)
from django.db.models.query import (
ValuesIterable,
ValuesListIterable,
FlatValuesListIterable,
)
modeltrans_classes = [
FallbackValuesIterable,
FallbackValuesListIterable,
FallbackFlatValuesListIterable,
]
django_classes = [ValuesIterable, ValuesListIterable, FlatValuesListIterable]
for a, b in product(modeltrans_classes + django_classes, django_classes):
if a is b:
continue
subclass = issubclass(a, b)
print(f"[{'✅' if subclass else '❌'}] {a.__name__} is a subclass of {b.__name__}")
[✅] FallbackValuesIterable is a subclass of ValuesIterable
[❌] FallbackValuesIterable is a subclass of ValuesListIterable
[❌] FallbackValuesIterable is a subclass of FlatValuesListIterable
[✅] FallbackValuesListIterable is a subclass of ValuesIterable
[❌] FallbackValuesListIterable is a subclass of ValuesListIterable
[❌] FallbackValuesListIterable is a subclass of FlatValuesListIterable
[✅] FallbackFlatValuesListIterable is a subclass of ValuesIterable
[❌] FallbackFlatValuesListIterable is a subclass of ValuesListIterable
[❌] FallbackFlatValuesListIterable is a subclass of FlatValuesListIterable
[❌] ValuesIterable is a subclass of ValuesListIterable
[❌] ValuesIterable is a subclass of FlatValuesListIterable
[❌] ValuesListIterable is a subclass of ValuesIterable
[❌] ValuesListIterable is a subclass of FlatValuesListIterable
[❌] FlatValuesListIterable is a subclass of ValuesIterable
[❌] FlatValuesListIterable is a subclass of ValuesListIterable
As you can see, the above Django-modeltrans iterable classes should inherit from their respective Django class (ie FallbackFlatValuesList -> FlatValuesList), but they are not.
Hello, yeah it would be nice to fix it.
Please, create PR with changes and new test-cases and we can merge it after review.