encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
27.83k stars 6.76k forks source link

DRF generic views not fire post_save signal #9394

Closed PunkFleet closed 1 month ago

PunkFleet commented 2 months ago

Checklist

In the new generic view, whether it's CreateAPIView or ListCreateAPIView, it will not trigger the Django Signal receiver Future updates should consider fixing it to work better with django!

I could achieve the goal by overriding perform_create, but geez, that's not a good way to do it.

FraCata00 commented 2 months ago

The CreateAPIView and ListCreateAPIView extends the CreateModelMixin that allow to perform creare, just save

class CreateModelMixin:
    """
    Create a model instance.
    """
    def create(self, request, *args, **kwargs):
        serializer = self.get_serializer(data=request.data)
        serializer.is_valid(raise_exception=True)
        self.perform_create(serializer)
        headers = self.get_success_headers(serializer.data)
        return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

    def perform_create(self, serializer):
        serializer.save()

see the CreateAPIView -> docs

So, CreateModelMixin in create function call the perform_create, and just call the save method of the serializer, if you override the save method, and maybe use a bulk option like bulk_create or bulk_update, not trigger the signals

From Django docs: image

PunkFleet commented 2 months ago

Thank you. @FraCata00 . I calling post_Save() with it

def perform_create(self, serializer):
        instance = super().perform_create(serializer)
        post_save.send(sender=type(instance), instance=instance, created=True)
        return instance

I'm just wondering if there isn't a more appropriate way to trigger this other than customizing the view and expanding CreateAPIView?

FraCata00 commented 2 months ago

Thank you. @FraCata00 . I calling post_Save() with it


def perform_create(self, serializer):

        instance = super().perform_create(serializer)

        post_save.send(sender=type(instance), instance=instance, created=True)

        return instance

I'm just wondering if there isn't a more appropriate way to trigger this other than customizing the view and expanding CreateAPIView?

But have you registered the signals in the ready function of AppConfig?

If you did, there's no need to call it in perform_create, trigger automatically

PunkFleet commented 2 months ago

Thank you. @FraCata00 . I calling post_Save() with it


def perform_create(self, serializer):

        instance = super().perform_create(serializer)

        post_save.send(sender=type(instance), instance=instance, created=True)

        return instance

I'm just wondering if there isn't a more appropriate way to trigger this other than customizing the view and expanding CreateAPIView?

But have you registered the signals in the ready function of AppConfig?

Yup, I have done the registration in app.py and this code works fine. I just don't think it's clean enough.

FraCata00 commented 2 months ago

Yeah, the best practice to call the signals, is write a custom signals like post_save or ever you want, just register it in AppConfig ready function inside the <application_module>.apps.py

Ever the instance call the .save method, the signals (if receiver is pre_save or post_save) is triggered There's no sense write custom perform_create only to trigger the signals

Can you put your apps.py and the signals?

PunkFleet commented 2 months ago

Yeah, the best practice to call the signals, is write a custom signals like post_save or ever you want, just register it in AppConfig ready function inside the <application_module>.apps.py

Ever the instance call the .save method, the signals (if receiver is pre_save or post_save) is triggered There's no sense write custom perform_create only to trigger the signals

Can you put your apps.py and the signals?

Yeah, there are mine apps.py and signals config: apps.py

from django.apps import AppConfig

class SettingsConfig(AppConfig):
    default_auto_field = 'django.db.models.BigAutoField'
    name = 'settings'

    def ready(self):
        import settings.signals

singals.py


@receiver(post_save, sender=Menu)
def sync_menu(sender, instance, created, **kwargs):
    if created:
        print('sync menu')
        sync_menu_to_front.delay(instance.id)
FraCata00 commented 2 months ago

this signals is from a project module or an application module?

from django.apps import AppConfig

class PortfolioConfig(AppConfig):
    default_auto_field = 'django.db.models.BigAutoField'
    name = 'settings'

    def ready(self):
        import settings.signals # --> the 'settings' is an application or project?

Yeah, the best practice to call the signals, is write a custom signals like post_save or ever you want, just register it in AppConfig ready function inside the <application_module>.apps.py Ever the instance call the .save method, the signals (if receiver is pre_save or post_save) is triggered There's no sense write custom perform_create only to trigger the signals Can you put your apps.py and the signals?

Yeah, there are mine apps.py and signals config: apps.py

from django.apps import AppConfig

class PortfolioConfig(AppConfig):
    default_auto_field = 'django.db.models.BigAutoField'
    name = 'settings'

    def ready(self):
        import settings.signals

singals.py


@receiver(post_save, sender=Menu)
def sync_menu(sender, instance, created, **kwargs):
    if created:
        print('sync menu')
        sync_menu_to_front.delay(instance.id)
PunkFleet commented 2 months ago

this signals is from a project module or an application module?

from django.apps import AppConfig

class PortfolioConfig(AppConfig):
    default_auto_field = 'django.db.models.BigAutoField'
    name = 'settings'

    def ready(self):
        import settings.signals # --> the 'settings' is an application or project?

Yeah, the best practice to call the signals, is write a custom signals like post_save or ever you want, just register it in AppConfig ready function inside the <application_module>.apps.py Ever the instance call the .save method, the signals (if receiver is pre_save or post_save) is triggered There's no sense write custom perform_create only to trigger the signals Can you put your apps.py and the signals?

Yeah, there are mine apps.py and signals config: apps.py

from django.apps import AppConfig

class PortfolioConfig(AppConfig):
    default_auto_field = 'django.db.models.BigAutoField'
    name = 'settings'

    def ready(self):
        import settings.signals

singals.py


@receiver(post_save, sender=Menu)
def sync_menu(sender, instance, created, **kwargs):
    if created:
        print('sync menu')
        sync_menu_to_front.delay(instance.id)

I'm sorry I'm in a drunken state. It's from application module, I changed I pasted the code for the settings module.

FraCata00 commented 2 months ago

However, the signals works fine right? If you create manually (maybe from django shell) a Menu object instance, the signals trigger correctly?

PunkFleet commented 2 months ago

Yup, It's working for now. It's just that for my case, I need to extend the perform_update method for every similar API for every application, and depending on the permissions and the functionality of the API, multiple APIs need to be customized. I'm guessing that scenarios with needs like mine should be relatively common. Is there a possibility to be able to add a method to perform_create


 from django.db.models.signals import post_save

 def perform_create(self, serializer):

        instance = super().perform_create(serializer)
        if(post_save):
                     post_save.send(sender=type(instance), instance=instance, created=True)

        return instance
FraCata00 commented 2 months ago

So, this isn't a best practice 😞 The perform_create, just call the .save method of the serializer and after that:

Can you put here the serializer code?