Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix various #20

Open
wants to merge 3 commits into
base: v5-beta
Choose a base branch
from
Open

Bugfix various #20

wants to merge 3 commits into from

Conversation

jpjpjp
Copy link
Contributor

@jpjpjp jpjpjp commented Mar 24, 2018

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).

@nmarus nmarus self-assigned this Mar 26, 2018
@nmarus
Copy link
Member

nmarus commented Jun 4, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants