Bugfix various

Open jpjpjp opened this issue 7 years ago • 1 comments

Found a few places where this was going out of scope in .then handlers. Bug fix in logic for finding the name of the mentioned user in a message by email Some comments on how the handling of names in mentioned could potentially be more robust (probably should have been an issue instead).

Mar 24 '18 17:03 jpjpjp

Hey JP, Thanks for the PR and apologies on the late reply.

A few notes.

Aliasing "self" to "this" is not necessary as this is all using arrow function which do not behave like normal functions in the the internal "this" to the arrow function uses the external "this" outside the arrow function vs the non arrow function() {} which has it's own internal "this" and would require the alias for scoping alignment.
The change to person.emails[0] from person.email works, but is not necessary as person.email is already assigned further up in the trigger code.
The trigger.message.text property is the raw non normalized version of the text object minus the bot mention.
The trigger.message.normailzed property is the lowercase representation and is used to perform command parsing / NLP without the needed normalization to lower case for rules.
The trigger.message.array property, is an array of words in the message based on trigger.normalized
The trigger.message.words property, is an array of unique words in the message, and later will play a part in some basic NLP pre-processing that happens with conversations engine that is yet to be committed to the repo.

Here is what a trigger object looks like and includes examples of what is sent in the case of multiple mentions in addition to the bot and what file object looks like that may help clarify this.

This is the trigger object of a the simple "hello" example with the text being "@bot Hello world world @Nicholas @Nick" and with two files attached to the message.

{ 
  person: { 
    id: 'some_person_id',
     email: '[email protected]',
     username: 'person',
     domain: 'example.com',
     emails: [ '[email protected]' ],
     displayName: 'Some Person',
     nickName: 'Some',
     firstName: 'Some',
     lastName: 'Person',
     avatar: 'https://imageurl',
     orgId: 'some_org_id',
     created: '2016-10-17T19:24:15.163Z',
     status: 'unknown',
     type: 'person',
  },
  room: { 
    id: 'some_room_id',
    type: 'group',
  },
  message: { 
    id: 'some_message_id',
    text: 'BotTester Hello world world Nicholas Nick',
    html: '<html message>',
    files: [
      { name: 'test1.csv', ext: 'csv', type: 'text/csv', binary: Buffer [], base64: 'base64 string for file' },
      { name: 'test2.csv', ext: 'csv', type: 'text/csv', binary: Buffer [], base64: 'base64 string for file' },
    ],
    mentionedPeople: [ 'some_id_1', 'some_id_2', 'some_id_3' ], // some_id1 would be the bot mention
    created: '2018-06-04T13:24:33.795Z',
    normalized: 'hello world world nicholas nick ',
    array: [ 'hello', 'world', 'world', 'nicholas', 'nick' ],
    words: [ 'hello', 'world', 'nicholas', 'nick' ],
  },
  created: '2018-06-04T13:24:35.148Z', 
}

If there some bug that is fixed by this, not too sure what it is based on this PR. My bot testing works with this syntax and does not loose scope. Let me know the symptoms and/or example code that triggers it as there might be something related to the this property of any code that is wrapped around these functions and I can address it a bit differently with methods closer to code style and es6 syntax.

If there is some feedback on the message text, array, normalized, words, etc that would make more sense and apply more generic usability to shortcut the developer in parsing this himself from message.text let me know.

Thanks again,

Nick

Jun 04 '18 14:06 nmarus