Merge lp:~therp-nl/server-env-tools/7.0-add_email_template_conditional_attachment into lp:~server-env-tools-core-editors/server-env-tools/7.0

Proposed by Stefan Rijnhart (Opener)
Status: Work in progress
Proposed branch: lp:~therp-nl/server-env-tools/7.0-add_email_template_conditional_attachment
Merge into: lp:~server-env-tools-core-editors/server-env-tools/7.0
Diff against target: 409 lines (+345/-0)
11 files modified
email_template_conditional_attachment/__init__.py (+1/-0)
email_template_conditional_attachment/__openerp__.py (+58/-0)
email_template_conditional_attachment/i18n/email_template_conditional_attachment.pot (+52/-0)
email_template_conditional_attachment/i18n/nl.po (+64/-0)
email_template_conditional_attachment/model/__init__.py (+3/-0)
email_template_conditional_attachment/model/email_template.py (+51/-0)
email_template_conditional_attachment/model/email_template_conditional_attachment.py (+35/-0)
email_template_conditional_attachment/model/ir_attachment.py (+30/-0)
email_template_conditional_attachment/security/ir.model.access.csv (+3/-0)
email_template_conditional_attachment/view/email_template.xml (+22/-0)
email_template_conditional_attachment/view/ir_attachment.xml (+26/-0)
To merge this branch: bzr merge lp:~therp-nl/server-env-tools/7.0-add_email_template_conditional_attachment
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) code review Needs Fixing
Nicolas Bessi - Camptocamp (community) no test, code review Needs Fixing
Graeme Gellatly code review, no test Abstain
Review via email: mp+202740@code.launchpad.net

Description of the change

See the description in __openerp__.py

To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

I suppose that you mean 'object' instead of 'invoice' in the module description in the part "...as a regular domain expression against the invoice model."

Regards.

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Pedro, thanks for having a look! This part of the text is a real world example on invoices, so I do mean 'invoice model' here. Of course the functionality is generic so the expression is always against the model of the template.

Revision history for this message
Graeme Gellatly (gdgellatly) wrote :

Hi I reviewed code only

All looks good and useful. Only comments for improvement are

Line 215, I know it is extremely unlikely/impossible in current modules but for futureproofing consider

if res_id and res:

rather than just if res_id.

From a usability standpoint it would be useful to have the reverse view. i.e. If you have an attachment you wish to attach to multiple templates being able to specify in the attachment rather than going in to each email template. I think of the use case of say a new product launch or an important change which needs to go with many emails. But that is new feature rather than complaint.

review: Abstain (code review, no test)
63. By Stefan Rijnhart (Opener)

[ADD] Allow configuration from the attachment itself
 Thanks to Graeme Gellatly for this suggestion

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Graeme,

thank you for the review!

I checked generate_email() in email_template/email_template.py and like you say, it is impossible not to have a return value that is not an email dictionary so I prefer to leave it like this to reflect that. If this changes in the future we can adapt then.

As for your usability suggestion, I just committed this as I think this is a very welcome improvement. Thank you for bringing this up!

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

And the above should say: *impossible to have a return value that is not an email dictionary*

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

Some neat picks:
PEP8 in __openerp__.py
Some trailing white space here and there.

Here I'm not sure of this expression:

                domain = (
                    ['&', ('id', '=', res_id)] +
                    literal_eval(attachment.domain)

Why do you use ast.literal_eval instead of openerp safe_eval ?

review: Needs Fixing (no test, code review)
64. By Stefan Rijnhart (Opener)

[FIX] Pep8

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Nicolas, thanks for the review! I fixed up __openerp__.py.

Using ast.literal_eval here because its functionality is a subset of safe_eval and it is standard python functionality. So it should be safer than safe_eval in this case. Can you live with that?

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

I'd also prefer OpenERP's API, so +1 for safe_eval.

But more picking on this line: The '&' makes your code crash for an empty attachment domain ('[]') and adds no benefit I can see. Do I overlook some benefit?

Generally, I think it's safer to use osv.expression.normalize and osv.expression.combine where applicable.

Further: What purpose does conditional_attachment_ids on ir.attachment serve?

review: Needs Fixing (code review)
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

In this case I prefere safe eval it is the standard way to do it and should be more portable.

Regards

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

Any news about safe eval ?

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks for the comments. Setting to work in progress as I'm currently thinking about whether or not to reset the m2m relationship when attachments are copied as this currently leads to duplicate attachments on the template (which is useful when you are configuring multiple attachments on the same template but rather disturbing when you are not.

Unmerged revisions

64. By Stefan Rijnhart (Opener)

[FIX] Pep8

63. By Stefan Rijnhart (Opener)

[ADD] Allow configuration from the attachment itself
 Thanks to Graeme Gellatly for this suggestion

62. By Stefan Rijnhart (Opener)

[IMP] Module description

61. By Stefan Rijnhart (Opener)

[IMP] Manifest

60. By Stefan Rijnhart (Opener)

[FIX] XML indentation
[ADD] Model access rules
[IMP] Translation
[IMP] Docstring

59. By Stefan Rijnhart (Opener)

[IMP] Module description

58. By Stefan Rijnhart (Opener)

[IMP] Module description

57. By Stefan Rijnhart (Opener)

[ADD] Module for conditional attachments on email templates

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'email_template_conditional_attachment'
2=== added file 'email_template_conditional_attachment/__init__.py'
3--- email_template_conditional_attachment/__init__.py 1970-01-01 00:00:00 +0000
4+++ email_template_conditional_attachment/__init__.py 2014-01-27 10:03:19 +0000
5@@ -0,0 +1,1 @@
6+from . import model
7
8=== added file 'email_template_conditional_attachment/__openerp__.py'
9--- email_template_conditional_attachment/__openerp__.py 1970-01-01 00:00:00 +0000
10+++ email_template_conditional_attachment/__openerp__.py 2014-01-27 10:03:19 +0000
11@@ -0,0 +1,58 @@
12+# -*- coding: utf-8 -*-
13+##############################################################################
14+#
15+# OpenERP, Open Source Management Solution
16+# This module copyright (C) 2014 Therp BV (<http://therp.nl>).
17+#
18+# This program is free software: you can redistribute it and/or modify
19+# it under the terms of the GNU Affero General Public License as
20+# published by the Free Software Foundation, either version 3 of the
21+# License, or (at your option) any later version.
22+#
23+# This program is distributed in the hope that it will be useful,
24+# but WITHOUT ANY WARRANTY; without even the implied warranty of
25+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
26+# GNU Affero General Public License for more details.
27+#
28+# You should have received a copy of the GNU Affero General Public License
29+# along with this program. If not, see <http://www.gnu.org/licenses/>.
30+#
31+##############################################################################
32+{
33+ 'name': 'Conditional attachments on email templates',
34+ 'version': '0.1',
35+ 'author': 'Therp BV',
36+ 'category': 'Email',
37+ 'website': 'http://therp.nl',
38+ 'depends': [
39+ 'email_template',
40+ ],
41+ 'data': [
42+ 'view/email_template.xml',
43+ 'view/ir_attachment.xml',
44+ 'security/ir.model.access.csv',
45+ ],
46+ 'description': """
47+This module allows you to configure a set of static attachments on an email
48+template that appear on generated emails conditionally. Conditions are
49+specified in the technical *domain* notation that is widely used in OpenERP.
50+
51+For instance, if you want
52+a specific attachment to only be sent out with invoices to Dutch speaking
53+customers of a specific company, you can encode this on the template for
54+outgoing invoices as a regular domain expression against the invoice model:
55+
56+ [('company_id', '=', 1), ('partner_id.lang', '=', 'nl_NL')]
57+
58+Generated emails will then only contain the attachment if the associated
59+invoice meets these requirements.
60+
61+You can configure the conditional attachments on the *Advanced* tab of the
62+email template form view.
63+
64+Again, only static attachments (so no dynamically generated reports) are
65+currently supported. An example of usage is attaching a document containing
66+general terms and conditions in various languages which can vary per company
67+or customer region.
68+""",
69+}
70
71=== added directory 'email_template_conditional_attachment/i18n'
72=== added file 'email_template_conditional_attachment/i18n/email_template_conditional_attachment.pot'
73--- email_template_conditional_attachment/i18n/email_template_conditional_attachment.pot 1970-01-01 00:00:00 +0000
74+++ email_template_conditional_attachment/i18n/email_template_conditional_attachment.pot 2014-01-27 10:03:19 +0000
75@@ -0,0 +1,52 @@
76+# Translation of OpenERP Server.
77+# This file contains the translation of the following modules:
78+# * email_template_conditional_attachment
79+#
80+msgid ""
81+msgstr ""
82+
83+#. module: email_template_conditional_attachment
84+#: field:ir.attachment,conditional_attachment_ids:0
85+msgid "Conditionally attach to email templates"
86+msgstr ""
87+
88+#. module: email_template_conditional_attachment
89+#: field:email.template.conditional.attachment,domain:0
90+msgid "Domain"
91+msgstr ""
92+
93+#. module: email_template_conditional_attachment
94+#: field:email.template.conditional.attachment,template_id:0
95+msgid "Email template"
96+msgstr ""
97+
98+#. module: email_template_conditional_attachment
99+#: model:ir.model,name:email_template_conditional_attachment.model_email_template
100+msgid "Email Templates"
101+msgstr ""
102+
103+#. module: email_template_conditional_attachment
104+#: model:ir.model,name:email_template_conditional_attachment.model_email_template_conditional_attachment
105+msgid "email.template.conditional.attachment"
106+msgstr ""
107+
108+#. module: email_template_conditional_attachment
109+#: model:ir.model,name:email_template_conditional_attachment.model_ir_attachment
110+msgid "ir.attachment"
111+msgstr ""
112+
113+#. module: email_template_conditional_attachment
114+#: field:email.template.conditional.attachment,attachment_id:0
115+msgid "Attachment"
116+msgstr ""
117+
118+#. module: email_template_conditional_attachment
119+#: view:email.template:0 field:email.template,conditional_attachment_ids:0
120+#: view:ir.attachment:0
121+msgid "Conditional attachments"
122+msgstr ""
123+
124+#. module: email_template_conditional_attachment
125+#: view:ir.attachment:0
126+msgid "Email templates to conditionally attach to"
127+msgstr ""
128
129=== added file 'email_template_conditional_attachment/i18n/nl.po'
130--- email_template_conditional_attachment/i18n/nl.po 1970-01-01 00:00:00 +0000
131+++ email_template_conditional_attachment/i18n/nl.po 2014-01-27 10:03:19 +0000
132@@ -0,0 +1,64 @@
133+# Translation of OpenERP Server.
134+# This file contains the translation of the following modules:
135+# * email_template_conditional_attachment
136+#
137+msgid ""
138+msgstr ""
139+"Project-Id-Version: OpenERP Server 7.0\n"
140+"Report-Msgid-Bugs-To: \n"
141+"POT-Creation-Date: 2014-01-24 14:04+0000\n"
142+"PO-Revision-Date: 2014-01-24 14:04+0000\n"
143+"Last-Translator: <>\n"
144+"Language-Team: \n"
145+"MIME-Version: 1.0\n"
146+"Content-Type: text/plain; charset=UTF-8\n"
147+"Content-Transfer-Encoding: \n"
148+"Plural-Forms: \n"
149+
150+#. module: email_template_conditional_attachment
151+#: field:ir.attachment,conditional_attachment_ids:0
152+msgid "Conditionally attach to email templates"
153+msgstr "Voorwaardelijke bijlage bij emailsjabloon"
154+
155+#. module: email_template_conditional_attachment
156+#: field:email.template.conditional.attachment,domain:0
157+msgid "Domain"
158+msgstr "Domein"
159+
160+#. module: email_template_conditional_attachment
161+#: field:email.template.conditional.attachment,template_id:0
162+msgid "Email template"
163+msgstr "Emailsjabloon"
164+
165+#. module: email_template_conditional_attachment
166+#: model:ir.model,name:email_template_conditional_attachment.model_email_template
167+msgid "Email Templates"
168+msgstr "Email-sjablonen"
169+
170+#. module: email_template_conditional_attachment
171+#: model:ir.model,name:email_template_conditional_attachment.model_email_template_conditional_attachment
172+msgid "email.template.conditional.attachment"
173+msgstr "email.template.conditional.attachment"
174+
175+#. module: email_template_conditional_attachment
176+#: model:ir.model,name:email_template_conditional_attachment.model_ir_attachment
177+msgid "ir.attachment"
178+msgstr "ir.attachment"
179+
180+#. module: email_template_conditional_attachment
181+#: field:email.template.conditional.attachment,attachment_id:0
182+msgid "Attachment"
183+msgstr "Bijlage"
184+
185+#. module: email_template_conditional_attachment
186+#: view:email.template:0
187+#: field:email.template,conditional_attachment_ids:0
188+#: view:ir.attachment:0
189+msgid "Conditional attachments"
190+msgstr "Conditionele bijlagen"
191+
192+#. module: email_template_conditional_attachment
193+#: view:ir.attachment:0
194+msgid "Email templates to conditionally attach to"
195+msgstr "Voorwaardelijke bijlage bij emailsjablonen"
196+
197
198=== added directory 'email_template_conditional_attachment/model'
199=== added file 'email_template_conditional_attachment/model/__init__.py'
200--- email_template_conditional_attachment/model/__init__.py 1970-01-01 00:00:00 +0000
201+++ email_template_conditional_attachment/model/__init__.py 2014-01-27 10:03:19 +0000
202@@ -0,0 +1,3 @@
203+from . import email_template_conditional_attachment
204+from . import email_template
205+from . import ir_attachment
206
207=== added file 'email_template_conditional_attachment/model/email_template.py'
208--- email_template_conditional_attachment/model/email_template.py 1970-01-01 00:00:00 +0000
209+++ email_template_conditional_attachment/model/email_template.py 2014-01-27 10:03:19 +0000
210@@ -0,0 +1,51 @@
211+# -*- coding: utf-8 -*-
212+##############################################################################
213+#
214+# OpenERP, Open Source Management Solution
215+# This module copyright (C) 2014 Therp BV (<http://therp.nl>).
216+#
217+# This program is free software: you can redistribute it and/or modify
218+# it under the terms of the GNU Affero General Public License as
219+# published by the Free Software Foundation, either version 3 of the
220+# License, or (at your option) any later version.
221+#
222+# This program is distributed in the hope that it will be useful,
223+# but WITHOUT ANY WARRANTY; without even the implied warranty of
224+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
225+# GNU Affero General Public License for more details.
226+#
227+# You should have received a copy of the GNU Affero General Public License
228+# along with this program. If not, see <http://www.gnu.org/licenses/>.
229+#
230+##############################################################################
231+from ast import literal_eval
232+from openerp.osv import orm, fields
233+
234+
235+class EmailTemplate(orm.Model):
236+ _inherit = 'email.template'
237+ _columns = {
238+ 'conditional_attachment_ids': fields.one2many(
239+ 'email.template.conditional.attachment',
240+ 'template_id', 'Conditional attachments'),
241+ }
242+
243+ def generate_email(self, cr, uid, template_id, res_id, context=None):
244+ """
245+ Go through the conditional attachments and select those
246+ of which the domain matches the resource.
247+ """
248+ res = super(EmailTemplate, self).generate_email(
249+ cr, uid, template_id, res_id, context=context)
250+ if res_id:
251+ template = self.browse(cr, uid, template_id, context=context)
252+ for attachment in template.conditional_attachment_ids:
253+ domain = (
254+ ['&', ('id', '=', res_id)] +
255+ literal_eval(attachment.domain))
256+ if self.pool.get(template.model).search(
257+ cr, uid, domain, context=context):
258+ res['attachment_ids'].append(
259+ attachment.attachment_id.id)
260+ return res
261+
262
263=== added file 'email_template_conditional_attachment/model/email_template_conditional_attachment.py'
264--- email_template_conditional_attachment/model/email_template_conditional_attachment.py 1970-01-01 00:00:00 +0000
265+++ email_template_conditional_attachment/model/email_template_conditional_attachment.py 2014-01-27 10:03:19 +0000
266@@ -0,0 +1,35 @@
267+# -*- coding: utf-8 -*-
268+##############################################################################
269+#
270+# OpenERP, Open Source Management Solution
271+# This module copyright (C) 2014 Therp BV (<http://therp.nl>).
272+#
273+# This program is free software: you can redistribute it and/or modify
274+# it under the terms of the GNU Affero General Public License as
275+# published by the Free Software Foundation, either version 3 of the
276+# License, or (at your option) any later version.
277+#
278+# This program is distributed in the hope that it will be useful,
279+# but WITHOUT ANY WARRANTY; without even the implied warranty of
280+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
281+# GNU Affero General Public License for more details.
282+#
283+# You should have received a copy of the GNU Affero General Public License
284+# along with this program. If not, see <http://www.gnu.org/licenses/>.
285+#
286+##############################################################################
287+from openerp.osv import orm, fields
288+
289+
290+class EmailTemplateConditionalAttachment(orm.Model):
291+ _name = 'email.template.conditional.attachment'
292+
293+ _columns = {
294+ 'template_id': fields.many2one(
295+ 'email.template', 'Email template', required=True,
296+ ondelete='CASCADE'),
297+ 'attachment_id': fields.many2one(
298+ 'ir.attachment', 'Attachment', required=True),
299+ 'domain': fields.char(
300+ size=256, string='Domain', required=True),
301+ }
302
303=== added file 'email_template_conditional_attachment/model/ir_attachment.py'
304--- email_template_conditional_attachment/model/ir_attachment.py 1970-01-01 00:00:00 +0000
305+++ email_template_conditional_attachment/model/ir_attachment.py 2014-01-27 10:03:19 +0000
306@@ -0,0 +1,30 @@
307+# -*- coding: utf-8 -*-
308+##############################################################################
309+#
310+# OpenERP, Open Source Management Solution
311+# This module copyright (C) 2014 Therp BV (<http://therp.nl>).
312+#
313+# This program is free software: you can redistribute it and/or modify
314+# it under the terms of the GNU Affero General Public License as
315+# published by the Free Software Foundation, either version 3 of the
316+# License, or (at your option) any later version.
317+#
318+# This program is distributed in the hope that it will be useful,
319+# but WITHOUT ANY WARRANTY; without even the implied warranty of
320+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
321+# GNU Affero General Public License for more details.
322+#
323+# You should have received a copy of the GNU Affero General Public License
324+# along with this program. If not, see <http://www.gnu.org/licenses/>.
325+#
326+##############################################################################
327+from openerp.osv import orm, fields
328+
329+
330+class IrAttachment(orm.Model):
331+ _inherit = 'ir.attachment'
332+ _columns = {
333+ 'conditional_attachment_ids': fields.one2many(
334+ 'email.template.conditional.attachment',
335+ 'attachment_id', 'Conditionally attach to email templates'),
336+ }
337
338=== added directory 'email_template_conditional_attachment/security'
339=== added file 'email_template_conditional_attachment/security/ir.model.access.csv'
340--- email_template_conditional_attachment/security/ir.model.access.csv 1970-01-01 00:00:00 +0000
341+++ email_template_conditional_attachment/security/ir.model.access.csv 2014-01-27 10:03:19 +0000
342@@ -0,0 +1,3 @@
343+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
344+access_email_template_conditional_attachment,email.template,model_email_template_conditional_attachment,,1,1,1,0
345+access_email_template_conditional_attachment_system,email.template system,model_email_template_conditional_attachment,base.group_system,1,1,1,1
346
347=== added directory 'email_template_conditional_attachment/static'
348=== added directory 'email_template_conditional_attachment/static/src'
349=== added directory 'email_template_conditional_attachment/static/src/img'
350=== added file 'email_template_conditional_attachment/static/src/img/icon.png'
351Binary files email_template_conditional_attachment/static/src/img/icon.png 1970-01-01 00:00:00 +0000 and email_template_conditional_attachment/static/src/img/icon.png 2014-01-27 10:03:19 +0000 differ
352=== added directory 'email_template_conditional_attachment/view'
353=== added file 'email_template_conditional_attachment/view/email_template.xml'
354--- email_template_conditional_attachment/view/email_template.xml 1970-01-01 00:00:00 +0000
355+++ email_template_conditional_attachment/view/email_template.xml 2014-01-27 10:03:19 +0000
356@@ -0,0 +1,22 @@
357+<?xml version="1.0" encoding="utf-8"?>
358+<openerp>
359+ <data>
360+
361+ <record model="ir.ui.view" id="email_template_form">
362+ <field name="name">Add list of conditional attachments to email template form</field>
363+ <field name="model">email.template</field>
364+ <field name="inherit_id" ref="email_template.email_template_form" />
365+ <field name="arch" type="xml">
366+ <field name="attachment_ids" position="after">
367+ <field name="conditional_attachment_ids" colspan="4">
368+ <tree string="Conditional attachments" editable="bottom">
369+ <field name="attachment_id" />
370+ <field name="domain" />
371+ </tree>
372+ </field>
373+ </field>
374+ </field>
375+ </record>
376+
377+ </data>
378+</openerp>
379
380=== added file 'email_template_conditional_attachment/view/ir_attachment.xml'
381--- email_template_conditional_attachment/view/ir_attachment.xml 1970-01-01 00:00:00 +0000
382+++ email_template_conditional_attachment/view/ir_attachment.xml 2014-01-27 10:03:19 +0000
383@@ -0,0 +1,26 @@
384+<?xml version="1.0" encoding="utf-8"?>
385+<openerp>
386+ <data>
387+
388+ <record id="view_attachment_form" model="ir.ui.view">
389+ <field name="name">Conditional attachment onto email template</field>
390+ <field name="model">ir.attachment</field>
391+ <field name="inherit_id" ref="base.view_attachment_form" />
392+ <field name="arch" type="xml">
393+ <group name="description_group" position="after">
394+ <group name="conditionally_attach"
395+ string ="Email templates to conditionally attach to"
396+ />
397+ <field name="conditional_attachment_ids"
398+ nolabel="1" colspan="4">
399+ <tree string="Conditional attachments" editable="bottom">
400+ <field name="template_id" />
401+ <field name="domain" />
402+ </tree>
403+ </field>
404+ </group>
405+ </field>
406+ </record>
407+
408+ </data>
409+</openerp>