goodybag / cater-api-server

hacking this together as fast as possible but with an eye for re-organization
1 stars 0 forks source link

add option to suppress restaurant notifications at night #1737

Closed prestonp closed 9 years ago

prestonp commented 9 years ago

Some restaurants like Brick Oven do not want to be texted at night and can handle operations in the morning.

I wanted to add another flag to each contact like suppress_overnight and then have some of the notifications filter out the right contacts.

For example


// utility fns
function enabledSms(contact) { return !contact.disable_sms; }
function enabledOvernight(contact) { !return contact.suppress_overnight && isOvernight(); }

// notification registration
build: function( order, logger, options, callback ){
  var contacts = order.restaurant.contacts.filter(enabledSms).filter(enabledOvernight);
  ...
}
prestonp commented 9 years ago

@goodybag/cater RFR on this before I start implementing. Does this work, or is it too hacky to maintain all of these little flags like contact.notify, contact.disable_sms, contact.suppress_overnight ?

paulserraino commented 9 years ago

I think if order-notifications has it's own set of utils it would be fine and I kinda like the idea of having a set of composable functions.

prestonp commented 9 years ago

I think the best would be to create a separate module containing all of these predicate functions that return a boolean value. Maybe contact-predicates.js?

jrf0110 commented 9 years ago

Well, do they want the notification suppressed or deferred?

jrf0110 commented 9 years ago

If it's an issue deferral, then this may be better to put in the scheduling layer.

In the send order notification action, we could check the restaurant settings to see if they want their notifications deferred - if so, then we re-schedule the action for the correct deferred time 6:00am whatever

jrf0110 commented 9 years ago

and I say restaurant settings because this would likely be a restaurant-wide setting (I can imagine one manager getting a notification at night and another getting it in the morning and things being confusing for them)

prestonp commented 9 years ago

I could see one manager being more hands on and wanting all notifications at all times.

jrf0110 commented 9 years ago

Hrmm Can you post in Design about that decision?

Also, my question still stands - these are deferreds, right?

prestonp commented 9 years ago

Deferring the notifications in the scheduled task sounds good. The only problem I see is the the reminder to "take action after an hour has passed" could stack up and send a bunch in the morning

jrf0110 commented 9 years ago

yeaaaahh - perhaps we can split this up into two issues.

  1. Some sort of intelligent scheduler debouncer
  2. The deferral logic
prestonp commented 9 years ago

Not sure debouncing is the right solution because the reminders are scheduled in hourly intervals

prestonp commented 9 years ago

for the "take action after an hour has passed" notification