Code review wanted: notification suite with @recipients, urgency levels, repeating, and presence-based message queuing

ipaterson · 2017-08-29 18:24:37 UTC

Are you sure that specific test is throwing the exception and not something else in the piston? It works for me.

bangali · 2017-08-29 18:27:01 UTC

from the trace it looks like it … may be i am missing something?

ipaterson · 2017-08-29 18:32:02 UTC

Hard to say, the trace just shows that the condition evaluated to false.

bangali · 2017-08-29 18:39:46 UTC

yeah. its a scratch piston, so going to skip posting the entire piston for now and look at it later.

thank you.

ipaterson · 2017-08-30 01:08:14 UTC

While discussing alternate notification services today I realized that with the current structure of these pistons it would be very difficult to add more properties for each recipient, like a device ID for Pushbullet. You would have to add a new array, populate it for each contact, create a variable to hold the argument value, add it to the argument list when executing the sender piston, then update the sender to make use of that argument.

Instead, contact info will be maintained in the sender piston with simple local variables for each of the datapoints, populated in a switch based on the recipient name. Adding a datapoint is as simple as creating the variable, setting the value for each recipient, then making use of it.

There are several merits to this change:

Less to edit when Smart notify is updated
“Public API” of the sender (now just level, recipient, message) is less subject to change
The sender piston can be executed directly by that API rather than going through Smart notify for simple notifications
Contact info does not need to be stored in variables, just set temporarily for the current execution

with a few concessions:

Recipients’ presence still needs to be tracked in Smart notify, I don’t think there is any way around this without global arrays
Sender piston (which some people will just rewrite from the ground up) is more complicated

ady624 · 2017-08-30 01:12:53 UTC

Maybe it would be easier to add a specialized task in webCoRE that implements the whole logic?

ipaterson · 2017-09-06 20:27:39 UTC

That would be very interesting! Let’s see once this is ironed out and posted in Examples what the response is and what requests come in. I am very near a size limit here, could squeak out a bit more by removing debugging but the debugging is necessary to make it testable.

The latest revision features:

much easier to write a custom sender since all the contact info is in local variables in the sender rather than arguments between two pistons
custom sender is usable independently since it just needs a recipient name and message
can deliver to recipients whose presence is not trackable (just can’t use :if-present et al. with them)
fewer arrays and less iteration thanks to help from @bangali over here
more thorough debug logging
separate piston that runs test cases
slightly less ugly regex due to (?s) flag

Smart notify piston

Basic sender piston using built-in webCoRE features

Tests

Some issues I had to work around:

arrayItem, count, etc barf when array values have commas in them, so I replaced commas in the repeating and presence messages with a unicode character in the private use subset, reverted when read out of the array
attempting to save with 31 chunks broke the entire dashboard temporarily, ISE’s everywhere

The next step for me is to switch over all my pistons to use it for some more thorough testing.

ipaterson · 2017-09-06 21:29:36 UTC

Earlier I had forgotten how I got the contactId values. I haven’t found a good way to do this but I’ve been doing the following:

Open browser’s Developer Tools from the page that shows your piston
Navigate to console
Paste c = $('ng-view').scope().instance.contacts; console.log(Object.keys(c).map(id => `${c[id].f} ${c[id].l}: ${id}`).join('\n')) and hit enter

The console then displays the names and IDs of your contacts. Any easier way to get these IDs or alternate expressions for sending a notification to a contact?

bangali · 2017-09-06 21:33:11 UTC

you dont have ST setup to support notification to contacts? once you have done that go to webcore settings and check the contacts you want webcore to be able to send notification to.

ipaterson · 2017-09-06 21:50:32 UTC

I do, the sender piston uses an expression to deliver to a contact by id rather than selecting the name from the dropdown.

bangali · 2017-09-06 23:12:58 UTC

hmm … looking at send notification to contact that seemed to support an expression for contact … that might suggest that there is some possibility here …

ipaterson · 2017-09-06 23:16:19 UTC

Yes the expression works fine, I am just curious whether there is an easier way for the end-user to get a contact into a variable in a way that can be used in this expression. Currently the variable needs to be set to the contact id formatted :0e657415ab1e4b0db90db24d12117ec2:, a value which can be found using the console snippet posted above.

bangali · 2017-09-06 23:55:08 UTC

understood. but, i was thinking about it slightly differently. if send notification to contact supports an expression for contact, was @ady624 expecting users to dig up the contact id via some steps like the one above or is there another way to specify the contact in that expression just as you can specify a device using [name of device]

ipaterson · 2017-09-07 00:01:54 UTC

Exactly, we’ve come back around to my original question of any easier way to get those ids or alternate expressions for sending a notification to a contact.

Oddly, the contactId as a string is no longer working for me - I keep getting “Invalid list of contacts” so I’m checking into that now.

ady624 · 2017-09-07 00:57:59 UTC

So I should add a contact type variable, I think? LOL

bangali · 2017-09-07 01:01:31 UTC

yeah, that would be nice. maybe just following the pattern of how devices can be used anywhere with []

ipaterson · 2017-09-07 01:01:55 UTC

That might be best, similar to how devices work. It looks like this could be worked around by handling strings here if ((operand.vt == 'contact') && (operand.c instanceof List)) { so that the contacts get loaded but frankly the raw id format is too obscure anyway.

Edit: not a good guess, that code wasn’t related

ipaterson · 2017-09-07 14:22:16 UTC

The following tweak works for me to support contact expressions as literal strings or variables though I’m sure it needs some safety and cleanup. It includes any ids specified in the expression so that the contacts do not also need to be selected.

if ((operand.vt == 'contact') && (operand.c instanceof List || operand.exp instanceof Map)) {
  def ids = []
  if (operand.c instanceof List) {
    ids += operand.c
  }
  if (operand.exp instanceof Map) {
    def val = evaluateExpression(rtData, operand.exp, 'string').v
    ids += val.tokenize(',')
  }
  for (c in ids.unique()) {
    rawContacts[c] = rtData.contacts[c]
  }
}

A bit of debug output showed me that this code runs when a piston is saved which is why nothing was happening when I tried to fuss with this bit of code earlier. Makes sense, didn’t get it last night

ipaterson · 2017-09-08 11:04:51 UTC

What do you think @ady624, is it worth making the Expression input work for notifying contacts or should I just avoid the example of sending to a contact for now? I’m not sure if the above code is the only place that needs to be modified but it has my live notifications to contacts working. Ultimately this is a low priority for me since this notification sender piston could be modified in a number of ways to work within the confines of only being able to select contacts from the dropdown.

ady624 · 2017-09-08 11:49:25 UTC

Right now I don’t think much, sorry… #irma