Check for Empty List before returning to user when removing items

[Muir-Fionn]

If a user removes the last item in their list, the bot currently returns

Got it. I have removed (item) from your list. Your list is: .

Would be better if it returned:

Got it. I have removed (item) from your list. Your list is empty.

Best solution might be to build a list string using a ternary operator before returning the message. listString = 'Your list is' + (user.list.length > 0 ? ': ' + user.list.join(', ') : ' empty') + '.'

[Muir-Fionn]

Just realized this feature seems to be a result of the merge last night and is actually from the set of code in @the-thief sent me last night in slack. Maybe something to consider, but not a priority issue.

[jacques-navarro]

I'll have a look at it tonight.

[jacques-navarro]

Ok, here's my change so far.

var listString = 'Your list is' + (user.list.length > 0 ? ': ' + user.list.join(', ') : ' empty') + '.';
          controller.storage.users.save(user, function(err, id) {
            bot.reply(message, 'Got it. I have removed ' + item + ' from your list.\n' + listString);

That seems to do the job of removing items and letting the user know when the last item is removed.

One problem I see is that items that are not on list cause the bot to say the item is removed.

add pizza, bananas

Got it. I have added pizza, bananas to your list. Your list is: pizza, bananas.

remove pizzza

Got it. I have removed pizzza from your list. Your list is: pizza, bananas.

remove pizza

Got it. I have removed pizza from your list. Your list is: bananas.

remove bananas

Got it. I have removed bananas from your list. Your list is empty.

remove bananas

Got it. I have removed bananas from your list. Your list is empty.

[jacques-navarro]

Ok. I made a bit of progress.

There's a small bug that prints the messages out of order after an item is removed.

Your list is: pizza, bread.

remove bread

Your list is: pizza Ok. I have removed bread from your list.

I'm going to commit this and push up to your repo in a new branch so you can have access to it.

[jacques-navarro]

We have a new branch called "remove_items". We should only make changes on this branch that are related to removing items from the list. We can merge it back to into the Master branch once we have the basic functionality of removing items working.

This will reduce the chance of introducing conflicts in our merges.

[Muir-Fionn]

The messages print out of order because you are sending two different replies. I'm going to update the code so that it saves as a string instead and then add the string to a single reply at the end of the code. This should fix the order issue.

[jacques-navarro]

How come it doesn't print of out order every time though?

[Muir-Fionn]

Has to do with how it's communicating with the Slack server. Whichever reply hits the Slack server first is the one that shows first.

[jacques-navarro]

OK. I thought I was going crazy. I thought it had something to do with which item in the list was deleted but it looked completely random.

How did you figure that out?

[Muir-Fionn]

I had the same issue when I had the replies split in my add function. Also, similar things happen in almost any networking situation. Think about how sometimes using text messaging a long message might arrive out of order on the receiving end.

[Muir-Fionn]

I'm also going to move the "Your list is..." reply inside the else statement to prevent an error at user.list.length when user.list does not exist

[jacques-navarro]

That makes sense. I thought you saw it in documentation or something.

Ok. I thought I had done that. My brain got really tired last night.

On Tue, Mar 14, 2017 at 4:49 PM, Amber notifications@github.com wrote:

I'm also going to move the "Your list is..." reply inside the else statement to prevent an error at user.list.length when user.list does not exist

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Muir-Fionn/Slackbot-Using-Botkit/issues/3#issuecomment-286555098, or mute the thread https://github.com/notifications/unsubscribe-auth/AEN2bRnWeeNlWPcfGxL-Dkd6f2DJ9kPlks5rlv1MgaJpZM4Mbkxc .

[Muir-Fionn]

No problem. Just tested in the general challenge. Going to try to commit now

[jacques-navarro]

Ok. I'm almost thinking we should add some code in the bot to do automatic testing. We would require two bots and would probably be better if the test bot is online.

I'm thinking the test bot sends a message to the development bot to add items and then listens for the response and responds if the expected response was received or not. Hopefully there's a way to time things out if it doesn't get a response.

It sounded simpler in my head until I got to the time out part. We could test things like adding and removing items, clearing the list.

We don't have to do this but we could try it down the road.

What do you think?

On Tue, Mar 14, 2017 at 4:56 PM, Amber notifications@github.com wrote:

No problem. Just tested in the general challenge. Going to try to commit now

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Muir-Fionn/Slackbot-Using-Botkit/issues/3#issuecomment-286557436, or mute the thread https://github.com/notifications/unsubscribe-auth/AEN2bUYzb426T5ep7ENIV61iqugF9j2Cks5rlv8KgaJpZM4Mbkxc .

[Muir-Fionn]

It sounds interesting, but I have no experience writing a test suite. My guess would be to use a setTimeout or setDelay function to check for the response in a predefined time period. Also, maybe a better option after we move to hosting on a persistent server.

[jacques-navarro]

​Something for the future like I said.

bot1: @bot2 add pizza, bread

bot2: ​Got it. I have added pizza, bread to your list. Your list is: pizza, bread

bot1 then listens to the response and checks if bot2 says: "Got it. I have added pizza, bread to your list. Your list is: pizza, bread" and responds: "Ok, that's what I was expecting" or "That's not what I wanted to hear" if he doesn't hear the right thing or hears anything at all. Not hearing anything at all would be the tricky part.

On Tue, Mar 14, 2017 at 5:13 PM, Amber notifications@github.com wrote:

It sounds interesting, but I have no experience writing a test suite. My guess would be to use a setTimeout or setDelay function to check for the response in a predefined time period. Also, maybe a better option after we move to hosting on a persistent server.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Muir-Fionn/Slackbot-Using-Botkit/issues/3#issuecomment-286562410, or mute the thread https://github.com/notifications/unsubscribe-auth/AEN2bV3_yVz-JN6rGSAw8FoLn9zTctfsks5rlwLfgaJpZM4Mbkxc .

[Muir-Fionn]

We'll look into that. I think that botkit studio might also have a built-in testing suite if we decide to host that way. It might at least be a good starting place.