Sending Email Enhancements

[Kevin Seymour]

This patch will make two changes.

1. Remove the requirement to have a limit when sending outbound email (you can set it to false for no limit).
2. Add a new flag that will merge outbound emails to different recipients, that have the same contents, into one email with multiple recipients. These changes are centralized into the sending cron job procedure so they require no DDL changes or other changes to core parts of the application.

[Elan Ruusamäe (glen666)]

in what situation the $merge=true gets activated? and same for $merge=false?

[Kevin Seymour (kevin-seymour-gmail)]

The merge flag is set by modifying the cron PHP that sends email depending on what you want.

[Elan Ruusamäe (glen666)]

can you show sample?

[Kevin Seymour (kevin-seymour-gmail)]

Here is a diff of the original version and what we have now.

$ diff process_mail_queue.php.orig process_mail_queue.php
88,89c88
< $limit = 50;
< Mail_Queue::send('pending', $limit);
---
> Mail_Queue::send('pending', false, true);
92,93c91
< $limit = 50;
< Mail_Queue::send('error', $limit);
---
> Mail_Queue::send('error', false, false);

[Elan Ruusamäe (glen666)]

committed as revno: 4590 (e0fe90b)

[Elan Ruusamäe (glen666)]

does this change really work for you? as i do not see where it compares contents of the emails, i only see it groups by type, and in note case the content can be very different (different issue, two different notes to send)

this quite complicates the code so i'm thinking of revert it

14:56:00 mysql{3}> select maq_type,count(*) from eventum_mail_queue group by 1 order by 2 desc;
+--------------------+----------+
| maq_type | count(*) |
+--------------------+----------+
| | 761 |
| files | 51 |
| notes | 46 |
| new_issue | 46 |
| updated | 41 |
| auto_created_issue | 26 |
| other_email | 8 |
| closed | 5 |
| assignment | 3 |
| blocked_email | 1 |
+--------------------+----------+
10 rows in set (0.00 sec)

[Kevin Seymour (kevin-seymour-gmail)]

It does work. I'm using maq_type_id and not maq_type as in your query.
The maq_type_id column seems to be a sequencing number that groups sets of
queued emails together (although the name is a bit confusing).

Kevin

On Sun, Feb 7, 2016 at 7:57 AM, Elan Ruusamäe <email address hidden>
wrote:

> does this change really work for you? as i do not see where it compares
> contents of the emails, i only see it groups by type, and in note case
> the content can be very different (different issue, two different notes
> to send)
>
> this quite complicates the code so i'm thinking of revert it
>
> 14:56:00 mysql{3}> select maq_type,count(*) from eventum_mail_queue group
> by 1 order by 2 desc;
> +--------------------+----------+
> | maq_type | count(*) |
> +--------------------+----------+
> | | 761 |
> | files | 51 |
> | notes | 46 |
> | new_issue | 46 |
> | updated | 41 |
> | auto_created_issue | 26 |
> | other_email | 8 |
> | closed | 5 |
> | assignment | 3 |
> | blocked_email | 1 |
> +--------------------+----------+
> 10 rows in set (0.00 sec)
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1195856
>
> Title:
> Sending Email Enhancements
>
> Status in Eventum:
> Fix Released
>
> Bug description:
> This patch will make two changes.
>
> 1. Remove the requirement to have a limit when sending outbound email
> (you can set it to false for no limit).
> 2. Add a new flag that will merge outbound emails to different
> recipients, that have the same contents, into one email with multiple
> recipients. These changes are centralized into the sending cron job
> procedure so they require no DDL changes or other changes to core parts of
> the application.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/eventum/+bug/1195856/+subscriptions
>

[Elan Ruusamäe (glen666)]

well. it would work if there are no queue items added by same type and type_id since last mail queue successful run.

here's query from system where mail queue was never ran (test env):

17:04:52 mysql{6}> SELECT GROUP_CONCAT(maq_id) ids FROM eventum_mail_queue WHERE maq_status='pending' AND maq_type_id > 0 GROUP BY maq_type_id ORDER BY MIN(maq_id) ASC;
+-------+
| ids |
+-------+
| 17,37 |
| 36 |
| 38 |
| 39 |
+-------+
4 rows in set (0.00 sec)

17:04:39 mysql{5}> select md5(maq_body) from eventum_mail_queue where maq_id in (17,37);
+----------------------------------+
| md5(maq_body) |
+----------------------------------+
| e4ecfea92ec5e4315596ffeb1bdf8707 |
| 5eda30412eaba4658526c37cbb3d9261 |
+----------------------------------+
2 rows in set (0.00 sec)

imho to make it work properly should do the grouping by message-id header instead. message-id is supposed to be unique for each mail.

[Kevin Seymour (kevin-seymour-gmail)]

Is message-id the same for two recipients of the same email (different
records in eventum_main_queue)? As long as the value relates multiple
records of the same "email" together it works.

In Notification::notifyNewEmail we call Mail_Queue::add($to, $headers,
$fixed_body, 1, $issue_id, $type, $sender_usr_id, $sup_id);

The last parameter is $sup_id which ends up being maq_type_id which is a
unique ID of the email we are sending which is why the logic I have works.

The only other place we call Mail_Queue::add is in Mail_Helper:send which,
from what I can see, might not be called anymore?

Kevin

On Mon, Feb 8, 2016 at 10:07 AM, Elan Ruusamäe <email address hidden>
wrote:

> well. it would work if there are no queue items added by same type and
> type_id since last mail queue successful run.
>
> here's query from system where mail queue was never ran (test env):
>
> 17:04:52 mysql{6}> SELECT GROUP_CONCAT(maq_id) ids FROM eventum_mail_queue
> WHERE maq_status='pending' AND maq_type_id > 0 GROUP BY maq_type_id ORDER
> BY MIN(maq_id) ASC;
> +-------+
> | ids |
> +-------+
> | 17,37 |
> | 36 |
> | 38 |
> | 39 |
> +-------+
> 4 rows in set (0.00 sec)
>
>
> 17:04:39 mysql{5}> select md5(maq_body) from eventum_mail_queue where
> maq_id in (17,37);
> +----------------------------------+
> | md5(maq_body) |
> +----------------------------------+
> | e4ecfea92ec5e4315596ffeb1bdf8707 |
> | 5eda30412eaba4658526c37cbb3d9261 |
> +----------------------------------+
> 2 rows in set (0.00 sec)
>
> imho to make it work properly should do the grouping by message-id
> header instead. message-id is supposed to be unique for each mail.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1195856
>
> Title:
> Sending Email Enhancements
>
> Status in Eventum:
> Fix Released
>
> Bug description:
> This patch will make two changes.
>
> 1. Remove the requirement to have a limit when sending outbound email
> (you can set it to false for no limit).
> 2. Add a new flag that will merge outbound emails to different
> recipients, that have the same contents, into one email with multiple
> recipients. These changes are centralized into the sending cron job
> procedure so they require no DDL changes or other changes to core parts of
> the application.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/eventum/+bug/1195856/+subscriptions
>

[Elan Ruusamäe (glen666)]

Mail_Queue::add is still there, just different method signature therefore renamed the method to Mail_Queue::addMail.