progval / Limnoria

A robust, full-featured, and user/programmer-friendly Python IRC bot, with many existing plugins.
https://docs.limnoria.net/
Other
622 stars 174 forks source link

Seen: mention online status #1523

Closed MMzF7 closed 1 year ago

MMzF7 commented 1 year ago

Limnoria replying to seen command or "any" (seen plugin command) if the nick is currently online as "Nick is online right now, last spoken x hr x minutes ago $nick message here". This modification will be very helpful.

matiasw commented 1 year ago

I quickly put this together: https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

I tested it, and it worked in the situations I could think of, but maybe I missed some corner cases..?

If it looks good, I will make a PR.

progval commented 1 year ago

Looks good. Add tests and send a PR, thanks!

matiasw commented 1 year ago

My code was missing the case where a named user is still in the channel/not. Have a look: https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

So, let's review the possible cases I need to test against:

  1. bot is triggered from channel with @seen nick
  2. bot is triggered in private with seen #channel nick
  3. bot is triggered from channel with @seen user nick
  4. bot is triggered in private with @seen user #channel nick

And, for all of these, see if the bot correctly reports that the user is still in the channel when they are, and doesn't when they're not. Make sense? Missing anything?

One thing that I was thinking about was the @seen last, either in channel or in private. In this case, we do not have the nick of the speaker of the last line immediately available; should we dig it out and report on whether they're still in the channel?

As a practical matter, which is a better structure: modify the existing test cases like testSeen to handle testing the in-channel status, or adding entirely new test cases?

progval commented 1 year ago

You don't need to test both in-channel and in private.

And, for all of these, see if the bot correctly reports that the user is still in the channel when they are, and doesn't when they're not. Make sense? Missing anything?

There's also the case of them being in channel but not saying anything.

One thing that I was thinking about was the @seen last, either in channel or in private. In this case, we do not have the nick of the speaker of the last line immediately available

Why?

matiasw commented 1 year ago

There's also the case of them being in channel but not saying anything.

Ah, right. I added code to handle that case: https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

One thing that I was thinking about was the @seen last, either in channel or in private. In this case, we do not have the nick of the speaker of the last line immediately available

Why?

Because it's not passed to the last method: https://github.com/matiasw/Limnoria/blob/c9305b43ffc164ae4623bc65760877e8d0d3cb48/plugins/Seen/plugin.py#L298 We could extract it from the said db entry, but unless I'm mistaken, it's not "directly available" (note that the msg.nick is the nick of the user calling the plugin, not the sought-after nick).

progval commented 1 year ago

We could extract it from the said db entry

sounds good

matiasw commented 1 year ago

Ok, that case is now handled: https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

Could you more seasoned devs give me some pointers for writing the tests..?

progval commented 1 year ago

https://docs.limnoria.net/develop/advanced_plugin_testing.html

matiasw commented 1 year ago

Yes, I have read that, but I'm still wavering on where & how to start... sorry for my noobishness, I can of course just beat my head against the wall until the blood spells out a clue, but I'd rather hear it in a friendly tone.

matiasw commented 1 year ago

This plugin doesn't handle multiple networks with a channel of the same name gracefully. Steps to reproduce:

  1. join #channel on network1
  2. join #channel on network2
  3. channel@network1 has some nick currently in the channel

  4. issue @seen nick on #channel@network1
  5. the bot says "I have not seen nick", even though they are in the channel.

I will look into it later... this is enough for today.

progval commented 1 year ago

This plugin doesn't handle multiple networks with a channel of the same name gracefully

That's fine, there are a few places like this where Limnoria has this issue

matiasw commented 1 year ago

I added the first test: https://github.com/progval/Limnoria/commit/1484a7c19d9c7dc11d7ac9ec241389ce570a3937

...but, I need help writing the rest of the cases. Let's start with the one where the user is not in the channel anymore: how do I test to make sure that "is in the channel now" is not in the reply to seen? I mean, I think I can use assertRegexp with ^(?:(?!is in the channel now).)*$ (I think?), but how to feed the dummy data of the user joining and parting to the bot?

progval commented 1 year ago

think I can use assertRegexp with ^(?:(?!is in the channel now).)*$

assertNotRegexp

how to feed the dummy data of the user joining and parting to the bot?

self.irc.feedMsg(ircmsgs.join(self.channel, ..., prefix=...))

matiasw commented 1 year ago

Ok, there's one new test case at https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

I ran into a case that is unhandled in the current Seen: when a user has joined, said nothing, and parted, the bot replies to seen nick with "I have not seen nick". As I see it, it would be better if it said nick was last seen leaving #channel with the part message <part message>. I think I need to add something to the bot's doPart, but it is not immediately clear to me what..?

progval commented 1 year ago

when a user has joined, said nothing, and parted, the bot replies to seen nick with "I have not seen nick". As I see it, it would be better if it said nick was last seen leaving #channel with the part message .

the Seen plugin provides two commands: @seen and @any (aka. @seen any). The latter should show the part message.

matiasw commented 1 year ago

Ok, in that case, the tests should now be comprehensive at https://github.com/progval/Limnoria/compare/master...matiasw:Limnoria:seen-now

I've created a PR: https://github.com/progval/Limnoria/pull/1559