Skip to content

Commit

Permalink
[#93] Handle edited events when old message is not in cache
Browse files Browse the repository at this point in the history
Problem: At the moment, when we receive a "message edited" event, and if
the old message is not in our cache or it did not contain time
references, we ignore the event:

```
-- if not found or expired, just ignore this message 
-- it's too old or just didn't contain any time refs
```

However, it's important to handle these events even when a message is
not in the cache. For example:

1. A user types "let's meet at 7" and then amends "7" to "7am". The old
   message did not contain time refs, but the new one does, so we should
   handle that.
2. The server is restarted, we'll lose our cache. When the server is
   back up, we should still react when messages that were sent before
   the restart are edited.

Solution: If the old message's time refs are not found in the cache,
inspect the `previous_message` field from the event JSON, and parse its
time refs again.
  • Loading branch information
dcastro committed Aug 31, 2023
1 parent a37dd4c commit c78bb5b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 18 deletions.
2 changes: 1 addition & 1 deletion docs/implementation_details.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
The bot's main function is to capture time references in a message and convert them
to the receiver's time zone. This can be triggered by some events:
* A new message was posted in a channel where the bot is present;
* A message that has been previously converted was recently edited and it now contains some new time references;
* A message has been edited and it now contains some new time references;
* The user triggered an entrypoint from the message's context menu ``;
* The user DMed the bot.

Expand Down
40 changes: 23 additions & 17 deletions src/TzBot/ProcessEvents/Message.hs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ import TzBot.Slack.Fixtures qualified as Fixtures
import TzBot.TimeReference (TimeReference)
import TzBot.Util (whenT, withMaybe)

data MessageEventType = METMessage | METMessageEdited
data MessageEventType
= METMessage
| METMessageEdited
-- ^ A message has been edited.
Message
-- ^ The state of the message _before_ it was edited.
deriving stock (Eq)

-- We don't need to handle MDMessageBroadcast anyhow,
Expand All @@ -41,9 +46,9 @@ filterMessageTypeWithLog evt = case meMessageDetails evt of
MDMessage -> do
logInfo [int||Handling new message|]
pure $ Just METMessage
MDMessageEdited {} -> do
MDMessageEdited previousMsg -> do
logInfo [int||Message was edited|]
pure $ Just METMessageEdited
pure $ Just $ METMessageEdited previousMsg
MDMessageBroadcast -> do
logInfo [int||Incoming message is thread broadcast, ignoring|]
pure Nothing
Expand Down Expand Up @@ -96,7 +101,7 @@ processMessageEvent' evt mEventType sender timeRefs =
case meChannelType evt of
Just CTDirectChannel -> handleDirectMessage
_ -> case mEventType of
METMessageEdited -> handleMessageChanged
METMessageEdited previousMsg -> handleMessageChanged previousMsg
METMessage -> handleNewMessage

where
Expand Down Expand Up @@ -155,19 +160,21 @@ processMessageEvent' evt mEventType sender timeRefs =
}
sendEphemeralMessage req

handleMessageChanged :: BotM ()
handleMessageChanged = katipAddNamespaceText "edit" do
handleMessageChanged :: Message -> BotM ()
handleMessageChanged previousMsg = katipAddNamespaceText "edit" do
messageRefsCache <- asks bsMessageCache
mbMessageRefs <- Cache.lookup msgId messageRefsCache
-- if not found or expired, just ignore this message
-- it's too old or just didn't contain any time refs
whenJust mbMessageRefs $ \oldRefs -> do
let newRefsFound = not $ all (`elem` oldRefs) timeRefs
-- no new references found, ignoring
when newRefsFound $ withNonEmptyTimeRefs timeRefs \neTimeRefs -> do
Cache.insert msgId timeRefs messageRefsCache
permalink <- getMessagePermalinkCached channelId msgId
handleChannelMessageCommon (Just permalink) neTimeRefs
-- Fetch the time references from the old message (before it was edited).
-- If they're not found in the cache, parse the message.
oldRefs <- do
Cache.lookup msgId messageRefsCache >>= \case
Just cachedTimeRefs -> pure cachedTimeRefs
Nothing -> getTimeReferencesFromMessage previousMsg
let newRefsFound = not $ all (`elem` oldRefs) timeRefs
-- If no new references are found, we ignore the edited message.
when newRefsFound $ withNonEmptyTimeRefs timeRefs \neTimeRefs -> do
Cache.insert msgId timeRefs messageRefsCache
permalink <- getMessagePermalinkCached channelId msgId
handleChannelMessageCommon (Just permalink) neTimeRefs

handleNewMessage :: BotM ()
handleNewMessage = do
Expand Down Expand Up @@ -196,7 +203,6 @@ processMessageEvent' evt mEventType sender timeRefs =

handleDirectMessage :: BotM ()
handleDirectMessage =
when (mEventType /= METMessageEdited) $
withNonEmptyTimeRefs timeRefs $ \neTimeRefs -> do
-- According to
-- https://forums.slackcommunity.com/s/question/0D53a00008vsItQCAU
Expand Down

0 comments on commit c78bb5b

Please sign in to comment.