pusher / django-pusherable

Real time notification when an object view is accessed via Pusher
BSD 3-Clause "New" or "Revised" License
55 stars 7 forks source link

Make the event callback configurable or consider changing the convention #3

Open leggetter opened 9 years ago

leggetter commented 9 years ago

One of the key benefits of the Pusher PubSub implementation is that it adds an event abstraction. Channels are generally used to identify the data (in this case an object instance) and the event is used to identify what's happening to that object instance (it's being viewed, edited, deleted etc.). We've got that more-or-less spot-on on the server, but we then funnel all events through a single pusherable_notify(event, data) function. This tends to lead to code such as:

function pusherable_notify(event, data) {
  switch(event) {
    case 'viewed':
      itemViewed(data);
      break;
    case 'update_pending':
      itemUpdatePending(data);
      break;
    // and so on...
} 

A way to avoid this boilerplate code in standard real-time messaging solutions has been built in to Pusher so I'm very keen for us to avoid bringing it back into django-pusherable.

django-pusherable currently forces all events to go through a single callback pusherable_notify(eventName, data). In doing this we lose the evented benefit.

We should consider:

  1. Make the convention function pusherable_notify_{event}(data){ ... }
  2. Keep the current ultra-simplified version, but make the function name configurable `{% pusherable_subscribe 'update' object 'myCallbackFunction' %}
  3. Something even nicer :)

For 1. it would be good to know if using underscore is a Django convention. Else it would probably be best to go with snake/camel case for the function name to match the general JS naming convention e.g. pusherableNotifyUpdate.

aaronbassett commented 9 years ago

I think option 2 is a good idea, if no-one is assigned by time I get back from DjangoCon I'll create a PR. Also the underscore is a Django Python convention, but I shouldn't be letting it bleed into my JS. I should be following the conventions of the language I'm working in, that was just an oversight on my part.