Extending subscription support #467
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
==========================================
- Coverage 87.89% 85.18% -2.71%
==========================================
Files 37 42 +5
Lines 3164 3267 +103
==========================================
+ Hits 2781 2783 +2
- Misses 383 484 +101 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I've used similar solutions in some projects, and I like the way you organized this.
Left some suggestions. Let me know if I can help you with anything here (either ping me here or on discord)
cc @patrick91 what do you think of this?
| def refresh_subscription_receiver(instance): | ||
| group = get_group(instance) | ||
| msg = get_msg(instance) | ||
|
|
||
| channel_layer = channels.layers.get_channel_layer() | ||
| async_to_sync(channel_layer.group_send)(group=group, message=msg) |
There was a problem hiding this comment.
Maybe this should be async and the caller itself should use async_to_sync?
There was a problem hiding this comment.
I'll need to play with it. On that makes sense, since we're looking at async first.
| if user.is_anonymous: | ||
| raise ValidationError("Anynomous users are not allowed to subscribe.") |
There was a problem hiding this comment.
Hrm, why not?
I mean, which user is allowed to subscribe should be defined by the application. Maybe it is an application which allows that?
What do you think about adding a way to check for permissions using the model_subscriber function? We can even try to use the permissioning system we already have for this (it might need some adjustments)
There was a problem hiding this comment.
This is from the OneSila project, most endpoints are not available unless logged in. I left it here as a reminder to investigate if we can leverage the existing extensions somehow. Would prefer to stay away from a second set of permissions.
Having said that, I also still want to reshuffle a few things in order to ensure the Publisher class is easy to extend and override for custom behaviour on the Subscription class.
As promised on #454 I'm hereby publishing my subscription implementation for OneSila. .
What does it do:
What it doesn't do:
This PR is intended as first step towards comprehensive subscriptions support for models out of the box. The reason this wasn't published sooner is because this PR should be stamped as experimental.
Types of Changes
Issues Fixed or Closed by This PR
Todos:
Want to help?