DEV: Clarify Email::Receiver existing incoming email behaviour

DEV: Clarify Email::Receiver existing incoming email behaviour

diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb
index 31aec68..0420963 100644
--- a/lib/email/receiver.rb
+++ b/lib/email/receiver.rb
@@ -67,9 +67,9 @@ module Email
       DistributedMutex.synchronize("process_email_#{id_hash}") do
         begin
 
-          # if we find an existing incoming email record with the
-          # exact same message id, be sure to update it with the correct IMAP
-          # metadata based on sync. this is so we do not double-create emails.
+          # If we find an existing incoming email record with the exact same
+          # message_id do not create a new IncomingEmail record to avoid double
+          # ups.
           @incoming_email = find_existing_and_update_imap
           return if @incoming_email
 
@@ -93,20 +93,23 @@ module Email
 
     def find_existing_and_update_imap
       incoming_email = IncomingEmail.find_by(message_id: @message_id)
+      return if !incoming_email
 
-      # if we are not doing this for IMAP purposes, then we do not want
-      # to double-process the same Message-ID
+      # If we are not doing this for IMAP purposes just return the record.
       if @opts[:imap_uid].blank?
         return incoming_email
       end
 
-      return if !incoming_email
-
-      # if the message_id matches the post id regexp then we
+      # If the message_id matches the post id regexp then we
       # generated the message_id not the imap server, e.g. in GroupSmtpEmail,
-      # so we want to just update the incoming email. Otherwise the
-      # incoming email is a completely new one from the IMAP server.
-      return if (@message_id =~ message_id_post_id_regexp).nil?
+      # so we want to update the incoming email because it will
+      # be missing IMAP details.
+      #
+      # Otherwise the incoming email is a completely new one from the IMAP
+      # server (e.g. a message_id generated by Gmail) and does not need to
+      # be updated, because message_ids from the IMAP server are not guaranteed
+      # to be unique.
+      return unless discourse_generated_message_id?
 
       incoming_email.update(
         imap_uid_validity: @opts[:imap_uid_validity],
@@ -960,6 +963,10 @@ module Email
       @host ||= Email::Sender.host_for(Discourse.base_url)
     end
 
+    def discourse_generated_message_id?
+      !(@message_id =~ message_id_post_id_regexp).nil?
+    end
+
     def message_id_post_id_regexp
       @message_id_post_id_regexp ||= Regexp.new "topic/\\d+/(\\d+)@#{Regexp.escape(host)}"
     end

GitHub sha: 7f55ed1a