From c78bb5b5d2128dbec8792ea7f7bb2d453b5c08a8 Mon Sep 17 00:00:00 2001
From: Diogo Castro <dc@diogocastro.com>
Date: Thu, 31 Aug 2023 15:46:13 +0100
Subject: [PATCH] [#93] Handle edited events when old message is not in cache

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.
---
 docs/implementation_details.md     |  2 +-
 src/TzBot/ProcessEvents/Message.hs | 40 +++++++++++++++++-------------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/docs/implementation_details.md b/docs/implementation_details.md
index 9a49ff9..bd621c0 100644
--- a/docs/implementation_details.md
+++ b/docs/implementation_details.md
@@ -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.
 
diff --git a/src/TzBot/ProcessEvents/Message.hs b/src/TzBot/ProcessEvents/Message.hs
index b8cc484..bbfa842 100644
--- a/src/TzBot/ProcessEvents/Message.hs
+++ b/src/TzBot/ProcessEvents/Message.hs
@@ -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,
@@ -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
@@ -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
@@ -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
@@ -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