[8.0][IMP][mail_tracking] Email stars and generic webhook #77

antespi · 2016-07-13T16:45:41Z

Add a generic webhook for an easier inheritance
Add a smartbutton in res.partner form to see all tracking emails related to its email address
Add email score and email stars fields to res partner form and a default simple algorithm to calculate them

pedrobaeza · 2016-07-13T17:10:15Z

mail_tracking/models/mail_tracking_email.py

@@ -77,13 +84,81 @@ class MailTrackingEmail(models.Model):
        string="Tracking events", comodel_name='mail.tracking.event',
        inverse_name='tracking_email_id', readonly=True)

+    @api.model
+    def email_score(self, trackings):
+        """Default email score algorimth"""


Why this method here? And why not @api.multi in case of belonging here?

This is for reusing this in other models (like crm.lead) and inherit only in one place

OK, make it @api.multi then

rafaelbn · 2016-07-21T19:03:50Z

@antespi please review runbot and travis

hbrunn · 2016-07-25T11:00:49Z

mail_tracking/models/mail_tracking_email.py

+    @api.depends('recipient')
+    def _compute_recipient_address(self):
+        for email in self:
+            matches = re.search(r'<(.*@.*)>', email.recipient)


won't you be better off with https://github.com/OCA/OCB/blob/8.0/openerp/tools/mail.py#L606?

This match is not for checking email address, but for extrating it if recipient is in the form Name <[email protected]>

check your regex against This <hello@world>>

I think #77 (comment) should still work for you.

@hbrunn This only could happend if partner email is hello@world> because this form if built by Odoo automatically concatenating partner name and partner email.
This match is not for checking, just for extracting and easy matching by partner email afterwords

I know I'm late, just letting you know that still the suggested regexp should work for extracting the email as you want. 😉

hbrunn · 2016-07-25T11:04:27Z

it seems you didn't git add hooks.py

hbrunn · 2016-07-25T11:05:20Z

superseded by #78 it seems

antespi · 2016-07-25T12:08:55Z

This PR only affects to mail_tracking addon
#78 PR includes these commits, because it depends on this.

They are separated in order to review them separately

Please, open this PR again, in order to continues reviewing process

pedrobaeza · 2016-07-25T12:09:00Z

@hbrunn. They are different things. #78 includes also this one.

hbrunn · 2016-07-25T14:41:04Z

okay thanks, got the point. I really miss lauchpad's depending branch feature

rafaelbn · 2016-07-25T15:10:55Z

@antespi please review coveralls. This PR is ready to be merged 👍

yajo · 2016-08-05T10:07:57Z

Any chance to add some tests? 😇

antespi · 2016-08-05T19:05:10Z

All green 🐸 , rebase and squashed for merging.
Testing controllers using mock. @yajo, this trick is really awesome.

pedrobaeza · 2016-08-06T19:35:07Z

mail_tracking/models/ir_mail_server.py

@@ -79,7 +79,7 @@ def send_email(self, message, mail_server_id=None, smtp_server=None,
                smtp_server=smtp_server, smtp_port=smtp_port,
                smtp_user=smtp_user, smtp_password=smtp_password,
                smtp_encryption=smtp_encryption, smtp_debug=smtp_debug)
-        except Exception as e:
+        except Exception as e:  # pragma: no cover
            if tracking_email:
                tracking_email.smtp_error(self, smtp_server_used, e)
            raise


If you raise again the exception, isn't the previous DB operations rollbacked?

I have to raise the exception because i'm inheriting the method, so I have to propagate the result, even an exception

I don't known if the last operation is rollbacked. This is a cool test case.

I've added a new test case for testing SMTP error using mock. I think raising exception will not affect DB operations because is the standard behavior.

I took a look at your mocks, and I do not see a test case for an SMTP Exception. In order to test this case, you need to use mock_object.side_effect = SMTPError. I only see return_values being set in the mocks.

pedrobaeza · 2016-08-07T18:44:21Z

@antespi, we decided to not squash intermediate commits when fixing comments in review to ease reviewers to see what you have done new instead of reviewing again the full diff, and only squash on merge time or when all is clear. Please don't do it next time.

For me, this is 👍, but I'm going to wait to @lasley to give his approval before merging.

lasley · 2016-08-07T23:05:35Z

I admittedly die a bit each time I look at that except Exception, but it's in the core too so it's only right to replicate here.

I digress- LGTM 👍 thanks for the submission!

rafaelbn added this to the 8.0 milestone Jul 13, 2016

pedrobaeza reviewed Jul 13, 2016
View reviewed changes

antespi mentioned this pull request Jul 13, 2016

[8.0][ADD] mail_tracking_mass_mailing #78

Merged

rafaelbn added needs review work in progress and removed needs review labels Jul 13, 2016

hbrunn reviewed Jul 25, 2016
View reviewed changes

hbrunn closed this Jul 25, 2016

pedrobaeza reopened this Jul 25, 2016

pedrobaeza added needs review and removed work in progress labels Jul 25, 2016

rafaelbn assigned antespi Jul 27, 2016

antespi force-pushed the 8.0-mail_tracking_email_stars branch from 5013d12 to 2d5f855 Compare August 5, 2016 19:03

pedrobaeza reviewed Aug 6, 2016
View reviewed changes

antespi force-pushed the 8.0-mail_tracking_email_stars branch from 021eb44 to 5c646f5 Compare August 7, 2016 18:16

[IMP] Email stars and generic webhook

4621c8d

antespi force-pushed the 8.0-mail_tracking_email_stars branch from 5c646f5 to 4621c8d Compare August 7, 2016 18:17

pedrobaeza merged commit b8f2336 into OCA:8.0 Aug 7, 2016

yajo deleted the 8.0-mail_tracking_email_stars branch August 8, 2016 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[8.0][IMP][mail_tracking] Email stars and generic webhook #77

[8.0][IMP][mail_tracking] Email stars and generic webhook #77

antespi commented Jul 13, 2016 •

edited

Loading

pedrobaeza Jul 13, 2016

antespi Jul 13, 2016

pedrobaeza Jul 13, 2016

rafaelbn commented Jul 21, 2016

hbrunn Jul 25, 2016

antespi Jul 25, 2016

hbrunn Jul 25, 2016

yajo Aug 5, 2016

antespi Aug 5, 2016

yajo Aug 8, 2016

hbrunn commented Jul 25, 2016

hbrunn commented Jul 25, 2016

antespi commented Jul 25, 2016

pedrobaeza commented Jul 25, 2016

hbrunn commented Jul 25, 2016

rafaelbn commented Jul 25, 2016

yajo commented Aug 5, 2016

antespi commented Aug 5, 2016

pedrobaeza Aug 6, 2016

antespi Aug 7, 2016

antespi Aug 7, 2016 •

edited

Loading

antespi Aug 7, 2016

lasley Aug 7, 2016

pedrobaeza commented Aug 7, 2016

lasley commented Aug 7, 2016

[8.0][IMP][mail_tracking] Email stars and generic webhook #77

[8.0][IMP][mail_tracking] Email stars and generic webhook #77

Conversation

antespi commented Jul 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelbn commented Jul 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbrunn commented Jul 25, 2016

hbrunn commented Jul 25, 2016

antespi commented Jul 25, 2016

pedrobaeza commented Jul 25, 2016

hbrunn commented Jul 25, 2016

rafaelbn commented Jul 25, 2016

yajo commented Aug 5, 2016

antespi commented Aug 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antespi Aug 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Aug 7, 2016

lasley commented Aug 7, 2016

antespi commented Jul 13, 2016 •

edited

Loading

antespi Aug 7, 2016 •

edited

Loading