Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 31, 2022
Merged

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

merged 2 commits into from
May 31, 2022

Conversation

littlemoses
Copy link

Steps to reproduce:

  • Have a model with a custom primary key field
  • Subscribe to it with DCRF
  • Create a new instance of the model (using the admin interface or some other method)
  • Observe that you observe nothing - (having a subscription on a model with a "standard" pk field sends a create notification here)

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

   (model observer) if the performed action is
   a create
@hishnash
Copy link
Member

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

   (created "on the fly" anyways / not necessary / would give an error with the changed model)
 * Add a test for the "get correct subscription notification on instance creation" issue
@littlemoses
Copy link
Author

I added a test for the broken behaviour

@hishnash hishnash merged commit 95f3534 into NilCoalescing:master May 31, 2022
@hishnash
Copy link
Member

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

@littlemoses littlemoses deleted the model_custom_pk_no_create_notification_bugfix branch May 31, 2022 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants