Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MIG] mail_tracking_mass_mailing: Migration to 10.0 #174

Merged
merged 5 commits into from
Jul 18, 2017
Merged

[MIG] mail_tracking_mass_mailing: Migration to 10.0 #174

merged 5 commits into from
Jul 18, 2017

Conversation

chienandalu
Copy link
Member

Mail tracking for mass mailing

Links mail statistics objects with mail tracking objects.

cc @Tecnativa

@chienandalu chienandalu force-pushed the 10.0-mig-mail_tracking_mass_mailing branch from 4e0181a to 9c82fc3 Compare June 22, 2017 11:24
@yajo yajo added this to the 10.0 milestone Jun 22, 2017
@pedrobaeza
Copy link
Member

Check error on tests.

(odoo.addons.mail_tracking_mass_mailing.tests.test_mass_mailing.TestMassMailing)
2017-06-22 11:50:01,988 5613 ERROR openerp_test odoo.addons.mail.models.mail_mail: failed sending mail (id: 43) due to Mock test error
Traceback (most recent call last):
File "/home/travis/odoo-10.0/addons/mail/models/mail_mail.py", line 278, in send
res = IrMailServer.send_email(msg, mail_server_id=mail.mail_server_id.id)
File "/home/travis/build/OCA/social/mail_tracking/models/ir_mail_server.py", line 85, in send_email
smtp_encryption=smtp_encryption, smtp_debug=smtp_debug)
File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/mock.py", line 955, in __call__
return _mock_self._mock_call(*args, **kwargs)
File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/mock.py", line 1010, in _mock_call
raise effect
Warning: Mock test error
2017-06-22 11:50:02,057 5613 ERROR openerp_test odoo.addons.mail.models.mail_mail: failed sending mail (id: 43) due to Mock test error
Traceback (most recent call last):
File "/home/travis/odoo-10.0/addons/mail/models/mail_mail.py", line 278, in send
res = IrMailServer.send_email(msg, mail_server_id=mail.mail_server_id.id)
File "/home/travis/build/OCA/social/mail_tracking/models/ir_mail_server.py", line 85, in send_email
smtp_encryption=smtp_encryption, smtp_debug=smtp_debug)
File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/mock.py", line 955, in __call__
return _mock_self._mock_call(*args, **kwargs)
File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/mock.py", line 1010, in _mock_call
raise effect
Warning: Mock test error

@chienandalu
Copy link
Member Author

@pedrobaeza that error comes generated from the mock function inside here https://github.com/Tecnativa/social/blob/9c82fc3b218be0b8fc144f1dacc8a279676102d1/mail_tracking_mass_mailing/tests/test_mass_mailing.py#L37
I tried to contain it into an assertRaise but I couldn't 😕

The same mock error is raised in mail_tracking tests

@yajo
Copy link
Member

yajo commented Jun 23, 2017

Seems like that test you link tries to disable mail sending in the test, but after all that's done by default in Odoo. Maybe the mock can be simply dropped there.

@chienandalu
Copy link
Member Author

@yajo I was doing some quick tests with your suggestions and indeed Odoo mail sending is dummy on tests. Wich is a problem here because we need to raise such errors 😕

@yajo
Copy link
Member

yajo commented Jun 29, 2017

I see what is happening.

Travis reads the test output and fails if it detects any error log, which is happening in your build because it's the expected behaviour you are testing.

So, to get it green again, you will have to mute the logger. Following this suggestion you can disable a certain logger under setUp and re-enable it under tearDown. That should do the trick.

@chienandalu
Copy link
Member Author

@yajo That was it 😃

@pedrobaeza
Copy link
Member

I think you should be more specific and put it before and after the line it generates the error, or you will mute other possible errors.

@chienandalu
Copy link
Member Author

@pedrobaeza Yes, that's better. Done.

@pedrobaeza
Copy link
Member

But you haven't changed the restore to immediately after...

@chienandalu chienandalu force-pushed the 10.0-mig-mail_tracking_mass_mailing branch from ca34e9e to 7dfa753 Compare June 29, 2017 09:54
@chienandalu
Copy link
Member Author

@pedrobaeza Quite right... 😅 Amended

@yajo
Copy link
Member

yajo commented Jun 29, 2017

Now travis is ❌

@pedrobaeza
Copy link
Member

This is because mail_tracking needs the same kind of patch.

@ecino ecino mentioned this pull request Jun 30, 2017
Closed
@ecino
Copy link
Contributor

ecino commented Jul 14, 2017

@chienandalu Could you fix it as well in mail_tracking?

@chienandalu
Copy link
Member Author

@ecino Here you are: #181 😉

@chienandalu chienandalu force-pushed the 10.0-mig-mail_tracking_mass_mailing branch from 7dfa753 to 5c8c0b1 Compare July 17, 2017 13:38
@chienandalu
Copy link
Member Author

All green now 🙂

pedrobaeza
pedrobaeza reviewed Jul 17, 2017
View reviewed changes
mail_tracking_mass_mailing/tests/test_mass_mailing.py Outdated
@@ -34,8 +36,9 @@ def setUp(self, *args, **kwargs):
})

def test_smtp_error(self):
logging.disable(logging.CRITICAL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use mute_logger also?

@pedrobaeza pedrobaeza mentioned this pull request Jul 17, 2017
Closed
21 tasks
@pedrobaeza
Copy link
Member

@chienandalu please squash your last 2 commits and I'll merge.

antespi and others added 4 commits July 18, 2017 10:44
@antespi @chienandalu
mail_tracking_mass_mailing (OCA#78)
6760266 
[ADD] mail_tracking_mass_mailing

==============================
Mail tracking for mass mailing
==============================

Links mail statistics objects with mail tracking objects.

Installation
============

This addon will be automatically installed when 'mail_tracking' and
'mass_mailing' are both installed

Usage
=====

From mail statistic object, you can see:
- Email tracking state
- Email related tracking object
- Email related tracking events

From mass mailing contact, you can see:
- Email score, in order to clean up your lists from bad score emails

As a bonus feature, you have a new checkbox 'Avoid resend' in mass mailing,
in order to not send twice the same email to the same recipient. This is very
useful when you want to resend the mass mailing after changing selection
recipients. Notice that recipient selection could be a domain over a model, so
result ids could change over the time. With this flag you can send
the same email several times but only once to each recipient.
@oca-transbot @chienandalu
OCA Transbot updated translations from Transifex
0494447
@cubells @chienandalu
mail_tracking_mass_mailing module
a348250
@oca-transbot @chienandalu
OCA Transbot updated translations from Transifex
4192c94
@chienandalu chienandalu force-pushed the 10.0-mig-mail_tracking_mass_mailing branch from 8919239 to b5b9200 Compare July 18, 2017 08:44
@chienandalu
Copy link
Member Author

@pedrobaeza Squashed

@pedrobaeza pedrobaeza merged commit a739efd into OCA:10.0 Jul 18, 2017
@pedrobaeza pedrobaeza deleted the 10.0-mig-mail_tracking_mass_mailing branch July 18, 2017 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yajo yajo yajo approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@chienandalu @pedrobaeza @yajo @ecino @antespi @oca-transbot @cubells