Skip to content

Commit 4a07157

Browse files
committed
Merge pull request zapier#18 from gvangool/pr7
Enhancement to support user agnostic Hook events. If you want a Hook to be triggered for all users, add '+' to built-in Hooks or pass user_override=False for custom_hook events.
2 parents 50f96ee + 84c7480 commit 4a07157

File tree

2 files changed

+32
-16
lines changed

2 files changed

+32
-16
lines changed

README.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ INSTALLED_APPS = (
7575
HOOK_EVENTS = {
7676
# 'any.event.name': 'App.Model.Action' (created/updated/deleted)
7777
'book.added': 'bookstore.Book.created',
78-
'book.changed': 'bookstore.Book.updated',
78+
'book.changed': 'bookstore.Book.updated+',
7979
'book.removed': 'bookstore.Book.deleted',
8080
# and custom events, no extra meta data needed
8181
'book.read': 'bookstore.Book.read',
@@ -87,6 +87,9 @@ HOOK_EVENTS = {
8787
class Book(models.Model):
8888
# NOTE: it is important to have a user property
8989
# as we use it to help find and trigger each Hook
90+
# which is specific to users. If you want a Hook to
91+
# be triggered for all users, add '+' to built-in Hooks
92+
# or pass user_override=False for custom_hook events
9093
user = models.ForeignKey('auth.User')
9194
# maybe user is off a related object, so try...
9295
# user = property(lambda self: self.intermediary.user)
@@ -348,9 +351,9 @@ HOOK_DELIVERER = 'path.to.tasks.deliver_hook_wrapper'
348351
### tasks.py ###
349352

350353
from celery.task import Task
351-
import requests
352354

353-
from django.utils import simplejson as json
355+
import json
356+
import requests
354357

355358

356359
class DeliverHook(Task):

rest_hooks/utils.py

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,33 @@ def find_and_fire_hook(event_name, instance, user_override=None):
3030
from django.contrib.auth.models import User
3131
from rest_hooks.models import Hook, HOOK_EVENTS
3232

33-
if user_override:
34-
user = user_override
35-
elif hasattr(instance, 'user'):
36-
user = instance.user
37-
elif isinstance(instance, User):
38-
user = instance
39-
else:
40-
raise Exception(
41-
'{} has no `user` property. REST Hooks needs this.'.format(repr(instance))
42-
)
43-
4433
if not event_name in HOOK_EVENTS.keys():
4534
raise Exception(
4635
'"{}" does not exist in `settings.HOOK_EVENTS`.'.format(event_name)
4736
)
4837

49-
hooks = Hook.objects.filter(user=user, event=event_name)
38+
filters = {'event': event_name}
39+
40+
# Ignore the user if the user_override is False
41+
if user_override is not False:
42+
if user_override:
43+
filters['user'] = user_override
44+
elif hasattr(instance, 'user'):
45+
filters['user'] = instance.user
46+
elif isinstance(instance, User):
47+
filters['user'] = instance
48+
else:
49+
raise Exception(
50+
'{} has no `user` property. REST Hooks needs this.'.format(repr(instance))
51+
)
52+
53+
# NOTE: This is probably up for discussion, but I think, in this
54+
# case, instead of raising an error, we should fire the hook for
55+
# all users/accounts it is subscribed to. That would be a genuine
56+
# usecase rather than erroring because no user is associated with
57+
# this event.
58+
59+
hooks = Hook.objects.filter(**filters)
5060
for hook in hooks:
5161
hook.deliver_hook(instance)
5262

@@ -65,8 +75,11 @@ def distill_model_event(instance, model, action, user_override=None):
6575
if auto:
6676
# break auto into App.Model, Action
6777
maybe_model, maybe_action = auto.rsplit('.', 1)
68-
if model == maybe_model and action == maybe_action:
78+
maybe_action = maybe_action.rsplit('+', 1)
79+
if model == maybe_model and action == maybe_action[0]:
6980
event_name = maybe_event_name
81+
if len(maybe_action) == 2:
82+
user_override = True
7083

7184
if event_name:
7285
find_and_fire_hook(event_name, instance, user_override=user_override)

0 commit comments

Comments
 (0)