Merge lp:~openerp-dev/openobject-addons/trunk-signature-jsh into lp:openobject-addons

Proposed by Jaysinh Shukla(OpenERP)
Status: Needs review
Proposed branch: lp:~openerp-dev/openobject-addons/trunk-signature-jsh
Merge into: lp:openobject-addons
Diff against target: 101 lines (+10/-12)
5 files modified
crm_partner_assign/crm_partner_assign_data.xml (+1/-1)
hr_recruitment/hr_recruitment_data.xml (+2/-2)
mail/mail_followers.py (+2/-3)
mail/tests/test_mail_features.py (+3/-4)
portal/portal_demo.xml (+2/-2)
To merge this branch: bzr merge lp:~openerp-dev/openobject-addons/trunk-signature-jsh
Reviewer Review Type Date Requested Status
Thibault Delavallée (OpenERP) (community) technical Needs Fixing
Review via email: mp+215863@code.launchpad.net

Description of the change

Modified 'mail' module to accept HTML tags as signature. The change is because of modification in 'signature' field of 'res.users'. Previously it was 'text' now 'html'. Modified test condition in which text of body is striped downed and compared to striped downed signature. For detaild explaination of test case kindly observe the following example.

Previous test case ('signature' is 'text') :
  Is 'By Admin' In html2plaintext('Mail body' + ' By Admin') ? <- No broke.

Now After change in signature field ('signature' is 'html) :
  Is '<b>By Admin</b>' In html2plaintext('Mail body' + ' <b>By Admin</b>') ? <- Broke the condition. Because html2plaintext will remove the '<b>' tag.

To not break the test condition after changing 'signature' field observ following example :
  Is html2plaintext('<b>By Admin</b>') In ('Mail body' + ' <b>By Admin</b>') ? <- YES !

To post a comment you must log in.
Revision history for this message
Thibault Delavallée (OpenERP) (tde-openerp) wrote :

Hello,

You should grep signature and all addons and see if your change has an impact.

Notably, I would like you to check the following things :
- account_followup/report/account_followup_print.py: uses user signature -> how does it behave with html tags and things like that ?
- hr_evaluation/hr_evaluation.py: user signature seems to be used, so same question
- crm_partner_assign/crm_partner_assign_data.xml: user signature is used in a pre, it wlil give some strange result

Best regards,

review: Needs Fixing (technical)
9336. By Amit Vora(OpenERP)

[MRG] merge with main branch

9337. By Amit Vora(OpenERP)

[IMP] use safe for signature field

9338. By Amit Vora(OpenERP)

[MRG] merge with main branch

9339. By Amit Vora(OpenERP)

[IMP] improve code

9340. By Mehul Mehta(OpenERP)

[Merge] Merged with lp:openobject-addons

9341. By Mehul Mehta(OpenERP)

[IMP] improved a signature in portal_demo file

9342. By Mehul Mehta(OpenERP)

minor changes apply

9343. By Sunil Sharma(OpenERP)

[mrg]:lp:openobject-addons

9344. By Amit Vora(OpenERP)

[MRG] merge with main branch

Unmerged revisions

9344. By Amit Vora(OpenERP)

[MRG] merge with main branch

9343. By Sunil Sharma(OpenERP)

[mrg]:lp:openobject-addons

9342. By Mehul Mehta(OpenERP)

minor changes apply

9341. By Mehul Mehta(OpenERP)

[IMP] improved a signature in portal_demo file

9340. By Mehul Mehta(OpenERP)

[Merge] Merged with lp:openobject-addons

9339. By Amit Vora(OpenERP)

[IMP] improve code

9338. By Amit Vora(OpenERP)

[MRG] merge with main branch

9337. By Amit Vora(OpenERP)

[IMP] use safe for signature field

9336. By Amit Vora(OpenERP)

[MRG] merge with main branch

9335. By Jaysinh Shukla(OpenERP)

[IMP]:mail Unnecessary comments removed. Improved mail test case.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'crm_partner_assign/crm_partner_assign_data.xml'
--- crm_partner_assign/crm_partner_assign_data.xml 2013-11-22 19:44:15 +0000
+++ crm_partner_assign/crm_partner_assign_data.xml 2014-05-16 09:33:42 +0000
@@ -48,7 +48,7 @@
48<p>Thanks,</p>48<p>Thanks,</p>
4949
50<pre>50<pre>
51${ctx['partner_id'].user_id and ctx['partner_id'].user_id.signature or ''}51${ctx['partner_id'].user_id and ctx['partner_id'].user_id.signature | safe or ''}
52</pre>52</pre>
53% if not ctx['partner_id'].user_id:53% if not ctx['partner_id'].user_id:
54PS: It looks like you do not have an account manager assigned to you, please contact us.54PS: It looks like you do not have an account manager assigned to you, please contact us.
5555
=== modified file 'hr_recruitment/hr_recruitment_data.xml'
--- hr_recruitment/hr_recruitment_data.xml 2014-02-12 09:56:06 +0000
+++ hr_recruitment/hr_recruitment_data.xml 2014-05-16 09:33:42 +0000
@@ -33,7 +33,7 @@
33 <p>If you want more details, feel free to contact us by phone.</p>33 <p>If you want more details, feel free to contact us by phone.</p>
34 <p>Kind regards,</p>34 <p>Kind regards,</p>
35 <br/>35 <br/>
36 ${object.user_id and object.user_id.signature or ''}]]></field>36 ${object.user_id and object.user_id.signature | safe or ''}]]></field>
37 </record>37 </record>
3838
39 <record id="applicant_interest" model="email.template">39 <record id="applicant_interest" model="email.template">
@@ -51,7 +51,7 @@
51 If I do not answer, please let me a message with some schedules to call you back.</p>51 If I do not answer, please let me a message with some schedules to call you back.</p>
52 <p>Kind regards,</p>52 <p>Kind regards,</p>
53 <br/>53 <br/>
54 ${object.user_id.signature}]]></field>54 ${object.user_id.signature | safe}]]></field>
55 </record>55 </record>
5656
57 <!-- HR Recruitment Source -->57 <!-- HR Recruitment Source -->
5858
=== modified file 'mail/mail_followers.py'
--- mail/mail_followers.py 2014-04-23 12:53:19 +0000
+++ mail/mail_followers.py 2014-05-16 09:33:42 +0000
@@ -119,11 +119,10 @@
119 # add user signature119 # add user signature
120 user = self.pool.get("res.users").browse(cr, SUPERUSER_ID, [user_id], context=context)[0]120 user = self.pool.get("res.users").browse(cr, SUPERUSER_ID, [user_id], context=context)[0]
121 if user.signature:121 if user.signature:
122 signature = plaintext2html(user.signature)122 signature = user.signature
123 else:123 else:
124 signature = "--<br />%s" % user.name124 signature = "--<br />%s" % user.name
125 footer = tools.append_content_to_html(footer, signature, plaintext=False, container_tag='p')125 footer = tools.append_content_to_html(footer, signature, plaintext=False)
126
127 # add company signature126 # add company signature
128 if user.company_id.website:127 if user.company_id.website:
129 website_url = ('http://%s' % user.company_id.website) if not user.company_id.website.lower().startswith(('http:', 'https:')) \128 website_url = ('http://%s' % user.company_id.website) if not user.company_id.website.lower().startswith(('http:', 'https:')) \
130129
=== modified file 'mail/tests/test_mail_features.py'
--- mail/tests/test_mail_features.py 2014-04-17 09:41:33 +0000
+++ mail/tests/test_mail_features.py 2014-05-16 09:33:42 +0000
@@ -22,10 +22,9 @@
22from openerp.addons.mail.mail_mail import mail_mail22from openerp.addons.mail.mail_mail import mail_mail
23from openerp.addons.mail.mail_thread import mail_thread23from openerp.addons.mail.mail_thread import mail_thread
24from openerp.addons.mail.tests.common import TestMail24from openerp.addons.mail.tests.common import TestMail
25from openerp.tools import mute_logger, email_split25from openerp.tools import mute_logger, email_split, html2plaintext
26from openerp.tools.mail import html_sanitize26from openerp.tools.mail import html_sanitize
2727
28
29class test_mail(TestMail):28class test_mail(TestMail):
3029
31 def test_000_alias_setup(self):30 def test_000_alias_setup(self):
@@ -474,7 +473,7 @@
474 'message_post: notification email body alternative should contain the body')473 'message_post: notification email body alternative should contain the body')
475 self.assertNotIn('<p>', sent_email['body_alternative'],474 self.assertNotIn('<p>', sent_email['body_alternative'],
476 'message_post: notification email body alternative still contains html')475 'message_post: notification email body alternative still contains html')
477 self.assertIn(user_raoul.signature, sent_email['body_alternative'],476 self.assertIn(html2plaintext(user_raoul.signature), sent_email['body_alternative'],
478 'message_post: notification email body alternative should contain the sender signature')477 'message_post: notification email body alternative should contain the sender signature')
479 self.assertFalse(sent_email['references'],478 self.assertFalse(sent_email['references'],
480 'message_post: references should be False when sending a message that is not a reply')479 'message_post: references should be False when sending a message that is not a reply')
@@ -545,7 +544,7 @@
545 'message_post: notification email body alternative should contain the body')544 'message_post: notification email body alternative should contain the body')
546 self.assertNotIn('<p>', sent_email['body_alternative'],545 self.assertNotIn('<p>', sent_email['body_alternative'],
547 'message_post: notification email body alternative still contains html')546 'message_post: notification email body alternative still contains html')
548 self.assertIn(user_raoul.signature, sent_email['body_alternative'],547 self.assertIn(html2plaintext(user_raoul.signature), sent_email['body_alternative'],
549 'message_post: notification email body alternative should contain the sender signature')548 'message_post: notification email body alternative should contain the sender signature')
550 self.assertIn(msg_message_id, sent_email['references'],549 self.assertIn(msg_message_id, sent_email['references'],
551 'message_post: notification email references lacks parent message message_id')550 'message_post: notification email references lacks parent message message_id')
552551
=== modified file 'portal/portal_demo.xml'
--- portal/portal_demo.xml 2014-04-09 10:16:04 +0000
+++ portal/portal_demo.xml 2014-05-16 09:33:42 +0000
@@ -18,8 +18,8 @@
18 <field name="partner_id" ref="partner_demo_portal"/>18 <field name="partner_id" ref="partner_demo_portal"/>
19 <field name="login">portal</field>19 <field name="login">portal</field>
20 <field name="password">portal</field>20 <field name="password">portal</field>
21 <field name="signature">--21 <field name="signature"><![CDATA[<span>--<br/>
22Mr Demo Portal</field>22Mr Demo Portal</span>]]></field>
23 <!-- Avoid auto-including this user in any default group -->23 <!-- Avoid auto-including this user in any default group -->
24 <field name="groups_id" eval="[(5,)]"/>24 <field name="groups_id" eval="[(5,)]"/>
25 </record>25 </record>

Subscribers

People subscribed via source and target branches

to all changes: