NilCoalescing / djangochannelsrestframework

A Rest-framework for websockets using Django channels-v4
https://djangochannelsrestframework.readthedocs.io/en/latest/
MIT License
603 stars 84 forks source link

Bugfix - Send notifications for a create event on a subscription if the model is using a custom pk #144

Closed littlemoses closed 2 years ago

littlemoses commented 2 years ago

Steps to reproduce:

What is (not) happening?

model_observer.py (note the "# <-- ..." remarks in the code)

    def post_init_receiver(self, instance: Model, **kwargs):

        if instance.pk is None:    # <-- the pk is only none here (created instance) if the pk is no "custom" field
            current_groups = set()
        else:
            current_groups = set(self.group_names_for_signal(instance=instance))

        self.get_observer_state(instance).current_groups = current_groups.  # <-- so the observer groups here are not empty but are set the group_names_for_signal

    ...

    def post_change_receiver(self, instance: Model, action: Action, **kwargs):
        """
        Triggers the old_binding to possibly send to its group.
        """

        old_group_names = self.get_observer_state(instance).current_groups  # <-- old_group_names was initialised with the "group_names_for_signal"

        if action == Action.DELETE:
            new_group_names = set()
        else:
            new_group_names = set(self.group_names_for_signal(instance=instance))  # <-- new_group_names will also be set to "group_names_for_signal"

        self.get_observer_state(instance).current_groups = new_group_names

        # if post delete, new_group_names should be []

        # Django DDP had used the ordering of DELETE, UPDATE then CREATE for good reasons.
        self.send_messages(
            instance, old_group_names - new_group_names, Action.DELETE, **kwargs
        )
        # the object has been updated so that its groups are not the same.
        self.send_messages(
            instance, old_group_names & new_group_names, Action.UPDATE, **kwargs
        )

        #
        self.send_messages(
            instance, new_group_names - old_group_names, Action.CREATE, **kwargs
        )  # <-- send_messages gets an empty set and therefore sends no notification
hishnash commented 2 years ago

This is a good find would you be able to add some unit tests for this?

littlemoses commented 2 years ago

I added a test for the broken behaviour

hishnash commented 2 years ago

Thankyou @littlemoses I will try to wrap up a new release in the next few days to get this out to pypi.