Fix improper equality operator in sample

[sbohlen]

No description provided.

[msftclas]

@sbohlen,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

[v-ducvo]

I am rejecting this change because using "!=" operator does not make sense in this context. Because the message is sent to the current bot's address (i.e.: .address(message.address)), using the "!=" will send the "Goodbye" message to the current bot for every bot that's in the array. I don't think that is the intended behavior.

As it is written, the "Goodbye" message is only sent to the current bot if it is found in the ".membersRemoved" array. Otherwise, no "Goodbye" message is sent.

If we want to handle a case where multiple bots are removed from a conversation and a "Goodbye" message is sent to each, then we need to write a new sample code to illustrate this point.

As it stands, the current code sample is valid.

[sbohlen]

I think I must be misunderstanding the intent of the equality test (?)

The event this code responds to is raised whenever any participant in the conversation is added or removed (whether the bot or an actual user/human participant). My interpretation of the purpose of the if(...) tests in both the ADD and REMOVE code paths was to ensure that the message is only sent if the participant being added/removed is not the bot but is instead an actual user/person.

This seemed the intent of the equality tests in the earlier block of code that handles the ADD case:

if (message.membersAdded[i].id != botId) {
//...

By using the inverse test for the REMOVE event as in ...

if (message.membersRemoved[i].id === botId) {
//...

...my read of the behavior you will get is that the 'goodbye' message will only be sent if the bot is the one leaving the conversation. Am I wrong that the intent of this snippet was to show how to send the 'goodbye' to the user/person that leaves a conversation? And/or am I misreading how this code will behave?

I admit up front that I've not run it IRL, but am doing this entirely from just reading the code, but the way I read this you will end up with a case where the ADD will be sent to the user but the REMOVE will only be sent to the bot.

Thoughts/corrections to my understanding welcome here; entirely poss. I'm missing something in my interpretation .... !

[v-ducvo]

Hi Steve,

Could you point me to the page where you found this if(...) block for ADD case?

if (message.membersAdded[i].id != botId) {

I can't seem to find where that statement was used in this topic or in any of our content topics.

Like you, I inherited this content so I can't say for sure what the initial or actual intent is. You did bring up a good point about members being "user" or "bot". I interpret them as being the same but you seems to make a distinction. I will get with the engineers to see if I can understand this distinction better and see if it makes a difference in this case.

[sbohlen]

Apologies -- I'm confusing this sample code with different code I wrote in another project (that DOES also handle the ADD event). Sorry about that :P.

Discount my invalid reference :) but I think I still stand by my understanding: the check for == on remove will seemingly only send the message to the bot and never the user as I understand it. I'd also be interested to understand the response from the product team on this snippet.

[v-ducvo]

Hi Steve,

So, I did a quick sync with one of the engineer regarding the array content. SteveIckman told me that the array actually contains both user and bot who are added or removed from a conversation. But, not always. It all depends on how the channel chooses to handle these event updates.

That said, I can see where you might get the idea that the message should be sent to a "user" instead of a bot...the intro text to the sample code references "user" while the code sample is looking for the current "bot".

I understand this scenario better now. I will go in and update the content text to match with what the code is trying to do. I'd appreciate it if you could help review that content to make sure the information is presented in a clear and concise manner.

[sbohlen]

Sure, happy to assist, let me know when it's ready to review / comment. Cheers,

…

-Steve B.

On Sep 21, 2017 2:55 PM, "Duc Cash Vo" ***@***.***> wrote: Hi Steve, So, I did a quick sync with one of the engineer regarding the array content. SteveIckman told me that the array actually contains both user and bot who are added or removed from a conversation. But, not always. It all depends on how the channel chooses to handle these event updates. That said, I can see where you might get the idea that the message should be sent to a "user" instead of a bot...the intro text to the sample code references "user" while the code sample is looking for the current "bot". I understand this scenario better now. I will go in and update the content text to match with what the code is trying to do. I'd appreciate it if you could help review that content to make sure the information is presented in a clear and concise manner. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALX4FX9vaa1hgS_Ko9uMGzko-YGej3bks5skttYgaJpZM4PZ1IX> .

[RobStand]

Merging in to be edited and synced.