BezBartek / django-db-views

Creating automatic migrations for Views models witch working reverse and full command options like in normal makemigrations
MIT License
60 stars 12 forks source link

Turn view_definition into a class method? #5

Closed LECbg closed 4 years ago

LECbg commented 4 years ago

As far as I've been able to find, this is the only place where the class attribute view_definition is used:

https://github.com/BezBartek/django-db-views/blob/c815737fbb0bbb0a2bd65e10d290538759c76527/django_db_views/autodetector.py#L106

Would you consider turning this attribute into a method? I think this would allow to get some advantages from Django itself, since part of the query could be built by the ORM, just like with RunPython migrations. To explain myself better, consider the following example (using the one provided in the README):

@classmethod
def view_definition(cls):
    VirtualCard = apps.get_model('cards', 'VirtualCard')
    queryset = VirtualCard.objects.annotate(
        virtual_card_id=F('id'), total_discount=Sum(...), total_returns=Sum(...)
    )...values('virtual_card_id', 'total_discount', 'total_returns'...).annotate(
        id=Window(expression=RowNumber()))
    return str(queryset.query)

My use case includes the use of union(), which doesn't allow annotations afterwards, so I would have to call ROW_NUMBER manually. Despite this, this approach would still be worth it:

@classmethod
def view_definition(cls):
    ModelA = apps.get_model('myapp', 'ModelA')
    ModelB = apps.get_model('myapp', 'ModelB')
    some_complex_qs = ModelA.objects.filter(...)...values(...)
    another_complex_qs = ModelB.objects.filter(...)...values(...)
    queryset = some_complex_qs.union(another_complex_qs)
    return f"SELECT ROW_NUMBER() OVER () AS id, * FROM ({str(queryset.query)}) AS some_name"

I haven't tested this, so I don't if there's anything I'm missing that prevents from doing this. Any feedback would be appreciated.

BezBartek commented 4 years ago

Thank you for your proposal :). I will investigate and test your idea during the weekend, it's looking really good, and I will probably implement it

BezBartek commented 4 years ago

To keep compatibility with the old versions, the view definition is still an attribute, but now you can provide both, string and a function as a view definition. Providing a function allows you to do what you want. The function must return string.
Does that solution suit you? :)

LECbg commented 4 years ago

Yeah, of course. Thank you!