From 11c74b7be9cec9fa1d9179430235cc7783a16c9b Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Sat, 16 Sep 2023 10:07:33 +1200
Subject: [PATCH] Add code comments re the cleanup I didn't do

I want the code to be stable in 5.66 for a few releases, absent any bugs so
commenting the further cleanup for 'one day'
---
 CRM/Utils/Mail/EmailProcessor.php | 6 ++++++
 CRM/Utils/Mail/Incoming.php       | 8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/CRM/Utils/Mail/EmailProcessor.php b/CRM/Utils/Mail/EmailProcessor.php
index a4e4d31017..3df5cc4929 100644
--- a/CRM/Utils/Mail/EmailProcessor.php
+++ b/CRM/Utils/Mail/EmailProcessor.php
@@ -172,6 +172,9 @@ class CRM_Utils_Mail_EmailProcessor {
                 'target_contact_id' => $targetFields,
                 'assignee_contact_id' => $assigneeFields,
               ];
+              // @todo - if the function that gets/ creates emails were more sane it would return
+              // an array of ids rather than a mutli-value array from which the id can be
+              // extracted...
               foreach ($activityContacts as $activityContact => $activityKeys) {
                 $activityParams[$activityContact] = [];
                 foreach ($activityKeys as $activityKey) {
@@ -185,6 +188,9 @@ class CRM_Utils_Mail_EmailProcessor {
                 }
               }
             }
+            // @todo the IncomingMail class should have `getAttachments` - retrieving from
+            // the email & moving to the file system should be separate to formatting
+            // array for api
             $numAttachments = Civi::settings()->get('max_attachments_backend') ?? CRM_Core_BAO_File::DEFAULT_MAX_ATTACHMENTS_BACKEND;
             for ($i = 1; $i <= $numAttachments; $i++) {
               if (isset($mailParams["attachFile_$i"])) {
diff --git a/CRM/Utils/Mail/Incoming.php b/CRM/Utils/Mail/Incoming.php
index 81bec1dccb..fe21b726a8 100644
--- a/CRM/Utils/Mail/Incoming.php
+++ b/CRM/Utils/Mail/Incoming.php
@@ -308,11 +308,17 @@ class CRM_Utils_Mail_Incoming {
       $params[$field] = [];
       foreach ($value as $address) {
         $subParam = [];
+        // @todo - stop creating a complex array here - $params[$field][] is
+        // an array with name, email & id. The calling function only wants the
+        // id & does quite a bit of work to extract it....
         self::parseAddress($address, $subParam, $mail, $createContact);
         $params[$field][] = $subParam;
       }
     }
-    // format and move attachments to the civicrm area
+    // @todo mode this code to be a `getAttachments` function on the IncomingMail class
+    // which would get attachments & move the files to the civicrm area.
+    // The formatting to api array should go back to the calling function on
+    // EmailProcessor.
     if (!empty($attachments)) {
       $date = date('YmdHis');
       $config = CRM_Core_Config::singleton();
-- 
2.25.1