Starring/unstarring messages about chatexchange

[banksJeremy]

Being able to set whether a message is starred or not seems surprisingly difficult, as the only method is to toggle whether you've starred it, and I'm not sure how to figure out whether you have starred it.

The MessageStarred event seems to actually tell you the user doing the starring/unstarring, but it doesn't tell you whether the action is a starting or unstarring! It might be possible to infer it from the change in star count (this should probably be done with #51, not independently), but presumably that's not how the official client does it, so there's probably something I'm missing.

DEBUG:__main__:event: MessageStarred({u'user_id': 97938, u'event_type': 6, u'message_stars': 1, u'room_name': u'Charcoal Chatbot Sandbox', u'message_id': 15322579, u'content': u'@bot hello bot', u'parent_id': 15319377, u'room_id': 14219, u'time_stamp': 1399162767, u'user_name': u'bot', u'id': 28393710}, <chatexchange.wrapper.SEChatWrapper object at 0x102451cd0>)
DEBUG:__main__:event: MessageStarred({u'user_id': 97938, u'event_type': 6, u'room_name': u'Charcoal Chatbot Sandbox', u'message_id': 15322579, u'content': u'@bot hello bot', u'parent_id': 15319377, u'room_id': 14219, u'time_stamp': 1399162767, u'user_name': u'bot', u'id': 28393711}, <chatexchange.wrapper.SEChatWrapper object at 0x102451cd0>)
DEBUG:__main__:event: MessageStarred({u'user_id': 1251, u'event_type': 6, u'message_stars': 1, u'room_name': u'Charcoal Chatbot Sandbox', u'message_id': 15322686, u'content': u'[ <a href="https://github.com/Manishearth/ChatExchange/" title="This is a test message for ChatExchange using the nonce 78808c056a214c4c8e1b568efb05d11f." rel="nofollow">ChatExchange@localhost</a> ] This is a test message for ChatExchange.', u'room_id': 14219, u'time_stamp': 1399163413, u'user_name': u'Jeremy Banks', u'id': 28393714}, <chatexchange.wrapper.SEChatWrapper object at 0x102451cd0>)
DEBUG:__main__:event: MessageStarred({u'user_id': 1251, u'event_type': 6, u'room_name': u'Charcoal Chatbot Sandbox', u'message_id': 15322686, u'content': u'[ <a href="https://github.com/Manishearth/ChatExchange/" title="This is a test message for ChatExchange using the nonce 78808c056a214c4c8e1b568efb05d11f." rel="nofollow">ChatExchange@localhost</a> ] This is a test message for ChatExchange.', u'room_id': 14219, u'time_stamp': 1399163413, u'user_name': u'Jeremy Banks', u'id': 28393715}, <chatexchange.wrapper.SEChatWrapper object at 0x102451cd0>)

(The MessageStarred events with user_name "bot" actually are referring to the message_id of a message from Jeremy, and vice-versa. So even though per #53 which fields are present is just based on the message, what those fields refer to may depend on the message type.)

[Manishearth]

Yeah, starring is broken, you have to keep track of that independently. If you've noticed you can confuse the UI by starring a message and then unstarring it from the starboard or similar tricks.

[banksJeremy]

Well in that case, for the sake of being able to do something sane, I think it might be reasonable to add a Message().starred_by_me() method which scapes transcript/message/15322639 to find out whether we have starred a post, and to call this whenever we call our star(value=True) method to determine if we need to toggle or not.

(edit: this should be part of a method that scrapes the transcript for a any available information for a message, as you just suggested in #51)

[Manishearth]

More efficient to scrape the message history :)

This could be part of a larger lazy getter: if self._foo == None: fetchHistory(); return self._foo

-----Original Message-----
From: "Jeremy Banks" [email protected]
Sent: ‎5/‎4/‎2014 2:24 PM
To: "Manishearth/ChatExchange" [email protected]
Cc: "Manish Goregaokar" [email protected]
Subject: Re: [ChatExchange] Starring/unstarring messages (#50)

Well in that case, for the sake of being able to do something sane, I think it might be reasonable to add a Message().starred_by_me() method which scapes transcript/message/15322639 to find out whether we have starred a post, and to call this whenever we call our star(value=True) method to determine if we need to toggle or not.
—
Reply to this email directly or view it on GitHub.

[banksJeremy]

I added a docs/se-chat-api.md file which I'll use to document the behviour of this and other methods.

[banksJeremy]

@Manishearth, do you know if owner_stars can ever be greater than 1? If not, it might make more sense just to expose it as a boolean .pinned = True/False.

[Manishearth]

Nope, they can't be greater than one. Boolean makes sense.

If we're fetching the data from history, we might also want to store it as

• self.pin=None: Not pinned
• self.pin="": Pinned, but pinner unknown -- used when the Message object was loaded from transcript
• self.pin="Manishearth": When the pinner is known

But this is probably overkill. I don't think we need to go and expose every obscure bit of information through the API.

[banksJeremy]

Before merging and closing this, it would be nice to add some tests. However, it's not entirely obvious how that should be done.

test_specific_messages() could check that specific messages are or are not pinned or starred. However, anybody could go star a message and break our tests, and maybe pins expire? (Maybe they only expire from the star list, not the actual status on the post itself.)

We can't simply add this to test_se_message_echo because the bot can't star its own messages. It can pin its own messages, but that relies on having ownership of the room. It's one thing for the tests to require an account, but I'd rather not also require additional specific permissions.

Perhaps we could go ahead and add a test that pins and unpins messages, but just make it entirely distinct from test_se_message_echo, so the test failure is as localized as possible.

[Manishearth]

What can be done is to star the last unstarred message not owned by us, check the new star state, and then unstar it.

I don't find it too necessary to have tests for this, though, expecially if it gets complest.

[banksJeremy]

It turns out messages can have multiple pins (message_owner_stars), FWIW.

MessageStarred({u'message_owner_stars': 2, u'user_id': 1251, u'event_type': 6, u'message_stars': 2, u'room_name': u'Charcoal Chatbot Sandbox', u'message_id': 15493758, u'content': u'Hello, world!', u'room_id': 14219, u'time_stamp': 1399967198, u'user_name': u'Jeremy Banks', u'id': 28711044}, <chatexchange.wrapper.SEChatWrapper object at 0x1017ca490>)

Both users are listed in the message history: http://chat.stackexchange.com/messages/15493758/history

[Manishearth]

Now that is interesting :)