Emails are sent "per recipient" instead of "per message"

Bug #1255611 reported by Martin
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Won't Fix
Undecided
Unassigned

Bug Description

 1) Steps to reproduce the issue you have observed

    A project issue with multiple followers exist. Add a message ("send message") to the project issue.

 2) The result you observed

    Multiple emails are sent, one per recipient (= followers minus the current user). Because my email server throttles the incoming emails, only half of the recipients actually got the message. There was no error message in the web user interface, however, and the message is shown as if it were sent to all followers.

 3) The result you expected

    Only one email should be sent out. Distributing the email to different addresses is a task of the MTA (email server), not the client.

 4) The platform you are using (and browser version, if relevant)

    Debian, MTA is a Microsoft product, AFAIK.

 5) The OpenERP version you are using

    7.0-20131126-00264

Revision history for this message
Martin (debacle) wrote :

There is also a code problem in addons/mail/mail_mail.py which is related to the bug:

The variable res is assigned to inside a loop. The value is checked after the loop, so only the last value is taken into account, previous values are silently overwritten:

283 email_list = []
284 if recipient_ids:
285 for partner in self.pool.get('res.partner').browse(...
286 email_list.append(self.send_get_email_dict(...
287 else:
288 email_list.append(self.send_get_email_dict(...
...
291 res = None
292 for email in email_list:
293 msg = ir_mail_server.build_email(
294 email_from = mail.email_from,
295 email_to = email.get('email_to'),
...
307 res = ir_mail_server.send_email(cr, uid, msg,
308 mail_server_id=mail.mail_server_id.id, context=context)
309 if res:
310 mail.write({'state': 'sent', 'message_id': res})
311 mail_sent = True
312 else:
313 mail.write({'state': 'exception'})
314 mail_sent = False

IMHO, the right fix would be to not use a loop in line 292 anyway, but sending one email per message, so res would assigned to only once in the first place.

Revision history for this message
Dave Burkholder (akxws32zf-dave-j0p9h616h) wrote :

Why is this a bug? To me, it's a feature because it enables personalized Mail Merge type emails. I don't want to be limited to CCing.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

As Dave said, this is indeed considered a feature :-) The fact that one or several emails are being sent is actually an implementation detail you should not care about, and is a consequence of the fact that each email is slightly personalized. The From/To headers are correctly set for each individual recipient (a single mail would force use to hide them completely or to incorrectly broadcast the emails of all followers, plus risk getting invalid "Reply all" effects).
And when the `portal` module is installed, this also permits adding a personalized link at the bottom of each email, letting the recipient either signup for a personal portal account using a unique invitation link connection to their existing "customer data", or directly open the corresponding record if they have previously signed up.

The fact that your mail server throttles emails is a configuration detail in your installation, and it's definitely recommended to fix that because OpenERP is not a true MTA/MUA: it expects a working SMTP server at all times. If you want that local SMTP server to be in charge of queuing the mails to comply with a maximum outgoing email rate, that's up to you.

Note that the outgoing messages are queued on the OpenERP side in the mail.mail model, but OpenERP does not currently distinguish a 4xx temporary failure from a 5xx permanent one, so it will mark them as permanently failed in all cases. There's a retry button if you go to the list of failed deliveries (See /Settings/Technical/Email/Emails), but that requires a manual action for each message, otherwise the queue runner will never retry them.

As a workaround for your particular situation you might want to add a scheduled job inside OpenERP to force the retry of all failed deliveries automatically.

Hope this helps,

Changed in openobject-addons:
status: New → Won't Fix
Revision history for this message
Martin (debacle) wrote :

Please check, whether the code is right or wrong. Even if you consider sending multiple emails per message a feature, overwriting the return value res in the loop and checking only the last return value afterwards looks like an error to me. Could you please explain, why this should be correct code?

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I would say the code is accidentally right, or involuntarily obfuscated. The `res` variable is used to hold the Message-Id header value of the outgoing messages, and it dates back to v6.1 when the Message-Id was not always forced upstream. There was also no `for` loop in the send() method.

Starting with 7.0 the Message-Id is generated upstream, and already set on the mail.message record before passing it to mail_mail.send(). The `for` loop can now spawn multiple RFC2822 mails for a single mail.message, and they all share the same Message-Id, so technically `res` holds a copy a that Message-Id as soon as at least one email is sent.
Furthermore, the same Message-Id value is used for all outgoing messages "instantiated" from a given `mail.message` record because technically they're the same message with a different SMTP envelope (plus sometimes a slightly custom footer).

This certainly deserves a refactoring (real variable names, no need to update mail.message_id after sending, obviously, etc.), but that is beyond the scope of this bug report I imagine. I'll try to land a quick refactoring in trunk for this later.

Revision history for this message
Martin (debacle) wrote :

Thanks for the explanation, this really helps in understanding the code!

Revision history for this message
Benoit Cousson (b-cousson) wrote :

If we assume that's a feature. How can we still enable Cc if we don't want individual emails to be sent? In my case I do want to send one email with every partners in Cc. It is doable?

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.