Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(45)

Issue 39051002: notification_email: Add fallback users for the recipients (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 3 weeks ago by nicoe
Modified:
7 months, 1 week ago
Reviewers:
pokoli, rietveld-bot, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 15

Patch Set 2 : fix remarks #

Total comments: 4

Patch Set 3 : Fix remarks #

Total comments: 2

Patch Set 4 : add falback user in test only when required #

Unified diffs Side-by-side diffs Delta from patch set Stats (+130 lines, -33 lines) Patch
M CHANGELOG View 1 1 chunk +3 lines, -1 line 0 comments Download
M notification.py View 1 2 6 chunks +44 lines, -0 lines 0 comments Download
M tests/test_notification_email.py View 1 2 3 6 chunks +74 lines, -29 lines 0 comments Download
M view/email_form.xml View 1 2 1 chunk +9 lines, -3 lines 0 comments Download

Messages

Total messages: 21
nicoe
9 months, 3 weeks ago (2017-12-22 15:57:55 UTC) #1
reviewbot
https://codereview.tryton.org/39051002/diff/1/tests/test_notification_email.py#newcode251 tests/test_notification_email.py:251: E302 expected 2 blank lines, found 1 URL: https://codereview.tryton.org/39051002
9 months, 3 weeks ago (2017-12-22 16:21:10 UTC) #2
ced
Missing changelog entry. https://codereview.tryton.org/39051002/diff/1/notification.py File notification.py (right): https://codereview.tryton.org/39051002/diff/1/notification.py#newcode59 notification.py:59: fallback_recipients = fields.Many2One( Why is it ...
9 months, 3 weeks ago (2017-12-22 22:45:23 UTC) #3
nicoe
fix remarks
9 months, 3 weeks ago (2017-12-28 13:25:54 UTC) #4
reviewbot
flake8 OK URL: https://codereview.tryton.org/39051002
9 months, 3 weeks ago (2017-12-28 13:59:23 UTC) #5
nicoe
https://codereview.tryton.org/39051002/diff/1/notification.py File notification.py (right): https://codereview.tryton.org/39051002/diff/1/notification.py#newcode59 notification.py:59: fallback_recipients = fields.Many2One( On 2017/12/22 22:45:23, ced wrote: > ...
9 months, 3 weeks ago (2017-12-28 14:12:46 UTC) #6
ced
https://codereview.tryton.org/39051002/diff/1/notification.py File notification.py (right): https://codereview.tryton.org/39051002/diff/1/notification.py#newcode59 notification.py:59: fallback_recipients = fields.Many2One( On 2017/12/28 14:12:46, nicoe wrote: > ...
9 months, 3 weeks ago (2017-12-28 14:37:09 UTC) #7
ced
OK to keep plurals on fallback field names. https://codereview.tryton.org/39051002/diff/20001/notification.py File notification.py (right): https://codereview.tryton.org/39051002/diff/20001/notification.py#newcode85 notification.py:85: 'invisible': ...
9 months, 2 weeks ago (2018-01-02 11:53:16 UTC) #8
pokoli
I'm wondering if it's really required to have 3 fields for fallback. Won't be enought ...
9 months ago (2018-01-18 09:43:44 UTC) #9
ced
On 2018/01/18 09:43:44, pokoli wrote: > I'm wondering if it's really required to have 3 ...
9 months ago (2018-01-18 10:01:42 UTC) #10
pokoli
On 2018/01/18 10:01:42, ced wrote: > On 2018/01/18 09:43:44, pokoli wrote: > > I'm wondering ...
9 months ago (2018-01-18 10:10:24 UTC) #11
ced
On 2018/01/18 10:10:24, pokoli wrote: > On 2018/01/18 10:01:42, ced wrote: > > On 2018/01/18 ...
9 months ago (2018-01-18 10:42:01 UTC) #12
nicoe
Fix remarks
8 months, 1 week ago (2018-02-07 10:39:34 UTC) #13
nicoe
https://codereview.tryton.org/39051002/diff/20001/notification.py File notification.py (right): https://codereview.tryton.org/39051002/diff/20001/notification.py#newcode85 notification.py:85: 'invisible': ~Eval('recipients'), On 2018/01/02 11:53:16, ced wrote: > recipients_hidden ...
8 months, 1 week ago (2018-02-07 10:40:18 UTC) #14
reviewbot
flake8 OK URL: https://codereview.tryton.org/39051002
8 months, 1 week ago (2018-02-07 10:58:01 UTC) #15
ced
https://codereview.tryton.org/39051002/diff/40001/tests/test_notification_email.py File tests/test_notification_email.py (right): https://codereview.tryton.org/39051002/diff/40001/tests/test_notification_email.py#newcode66 tests/test_notification_email.py:66: fallback_user.save() It should be useful only for the test ...
8 months, 1 week ago (2018-02-07 12:35:15 UTC) #16
nicoe
add falback user in test only when required
8 months, 1 week ago (2018-02-07 13:58:43 UTC) #17
nicoe
https://codereview.tryton.org/39051002/diff/40001/tests/test_notification_email.py File tests/test_notification_email.py (right): https://codereview.tryton.org/39051002/diff/40001/tests/test_notification_email.py#newcode66 tests/test_notification_email.py:66: fallback_user.save() On 2018/02/07 12:35:15, ced wrote: > It should ...
8 months, 1 week ago (2018-02-07 13:58:54 UTC) #18
reviewbot
flake8 OK URL: https://codereview.tryton.org/39051002
8 months, 1 week ago (2018-02-07 14:23:08 UTC) #19
ced
LGTM
8 months, 1 week ago (2018-02-07 17:14:10 UTC) #20
rietveld-bot_tryton.org
7 months, 1 week ago (2018-03-10 16:37:12 UTC) #21
New changeset 0ceaecb72dba by Nicolas √Čvrard in branch 'default':
Add fallback users for the recipients
http://hg.tryton.org/modules/notification_email/rev/0ceaecb72dba
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 0147766