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

Issue 38711002: tryton-env: marketing_email: WIP

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 3 months ago by ced
Modified:
3 weeks, 5 days ago
Reviewers:
pokoli, reviewbot, dave
Visibility:
Public.

Description

issue7835 COLLABORATOR=sergi@koolpi.com

Patch Set 1 #

Total comments: 5

Patch Set 2 : Make token required #

Patch Set 3 : Use tryton-env and implement message delivery #

Total comments: 18
Unified diffs Side-by-side diffs Delta from patch set Stats (+2188 lines, -1 line) Patch
M .hgsub View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A modules/marketing_email/.drone.yml View 1 2 1 chunk +51 lines, -0 lines 0 comments Download
A modules/marketing_email/COPYRIGHT View 1 2 1 chunk +15 lines, -0 lines 0 comments Download
A modules/marketing_email/INSTALL View 1 2 1 chunk +29 lines, -0 lines 0 comments Download
A modules/marketing_email/LICENSE View 1 2 1 chunk +674 lines, -0 lines 0 comments Download
A modules/marketing_email/MANIFEST.in View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
A modules/marketing_email/README View 1 2 1 chunk +35 lines, -0 lines 0 comments Download
A modules/marketing_email/__init__.py View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
A modules/marketing_email/doc/index.rst View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A modules/marketing_email/email_subscribe.html View 1 2 1 chunk +42 lines, -0 lines 1 comment Download
A modules/marketing_email/email_unsubscribe.html View 1 2 1 chunk +42 lines, -0 lines 1 comment Download
A modules/marketing_email/marketing.py View 1 2 1 chunk +493 lines, -0 lines 11 comments Download
A modules/marketing_email/marketing.xml View 1 2 1 chunk +284 lines, -0 lines 5 comments Download
A modules/marketing_email/setup.py View 1 2 1 chunk +125 lines, -0 lines 0 comments Download
A modules/marketing_email/tests/__init__.py View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
A modules/marketing_email/tests/test_marketing_email.py View 1 2 1 chunk +227 lines, -0 lines 0 comments Download
A modules/marketing_email/tox.ini View 1 2 1 chunk +15 lines, -0 lines 0 comments Download
A modules/marketing_email/tryton.cfg View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_event_form.xml View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_event_list.xml View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_form.xml View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_list.xml View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_list_form.xml View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_list_list.xml View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_message_form.xml View 1 2 1 chunk +27 lines, -0 lines 0 comments Download
A modules/marketing_email/view/email_message_list.xml View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M trytond/trytond/sendmail.py View 1 2 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 13
ced
1 year, 3 months ago (2017-09-19 17:23:10 UTC) #1
reviewbot
https://codereview.tryton.org/38711002/diff/1/setup.py#newcode93 setup.py:93: E501 line too long (85 > 79 characters) https://codereview.tryton.org/38711002/diff/1/tests/__init__.py#newcode5 tests/__init__.py:5: E501 line too ...
1 year, 3 months ago (2017-09-19 17:50:01 UTC) #2
pokoli
1 year, 2 months ago (2017-09-20 11:42:31 UTC) #3
pokoli
https://tryton-rietveld.appspot.com/38711002/diff/1/marketing.py File marketing.py (right): https://tryton-rietveld.appspot.com/38711002/diff/1/marketing.py#newcode135 marketing.py:135: for token in tokens: why not calling with a ...
1 year ago (2017-11-29 11:59:56 UTC) #4
ced
https://codereview.tryton.org/38711002/diff/1/marketing.py File marketing.py (right): https://codereview.tryton.org/38711002/diff/1/marketing.py#newcode135 marketing.py:135: for token in tokens: On 2017/11/29 11:59:56, pokoli wrote: ...
1 year ago (2017-11-29 14:12:20 UTC) #5
pokoli
https://codereview.tryton.org/38711002/diff/1/marketing.py File marketing.py (right): https://codereview.tryton.org/38711002/diff/1/marketing.py#newcode135 marketing.py:135: for token in tokens: On 2017/11/29 14:12:20, ced wrote: ...
1 year ago (2017-11-29 14:33:46 UTC) #6
ced
https://codereview.tryton.org/38711002/diff/1/marketing.py File marketing.py (right): https://codereview.tryton.org/38711002/diff/1/marketing.py#newcode135 marketing.py:135: for token in tokens: On 2017/11/29 14:33:46, pokoli wrote: ...
1 year ago (2017-11-29 15:13:06 UTC) #7
ced
Make token required
1 year ago (2017-11-30 14:17:21 UTC) #8
reviewbot
https://codereview.tryton.org/38711002/diff/20001/setup.py#newcode93 setup.py:93: E501 line too long (85 > 79 characters) https://codereview.tryton.org/38711002/diff/20001/tests/__init__.py#newcode5 tests/__init__.py:5: E501 line too ...
1 year ago (2017-11-30 14:37:18 UTC) #9
pokoli
Use tryton-env and implement message delivery
1 month, 2 weeks ago (2018-11-02 17:41:47 UTC) #10
reviewbot
patch is not applicable URL: https://codereview.tryton.org/38711002
1 month, 2 weeks ago (2018-11-02 17:57:09 UTC) #11
ced
https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/marketing.py File modules/marketing_email/marketing.py (right): https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/marketing.py#newcode296 modules/marketing_email/marketing.py:296: class Message(Workflow, ModelSQL, ModelView): I do not see the ...
3 weeks, 6 days ago (2018-11-21 15:05:13 UTC) #12
dave
3 weeks, 5 days ago (2018-11-22 11:28:02 UTC) #13
https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/ema...
File modules/marketing_email/email_subscribe.html (right):

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/ema...
modules/marketing_email/email_subscribe.html:4: <title>Subscribe Request</title>
This might be better called "Subscription Request".

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/ema...
File modules/marketing_email/email_unsubscribe.html (right):

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/ema...
modules/marketing_email/email_unsubscribe.html:4: <title>Unsubscribe
Request</title>
Perhaps "Request to Unsubscribe"?

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
File modules/marketing_email/marketing.py (right):

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:59: email = fields.Char("E-mail",
required=True)
Please consider adding "_address" to this field name or changing it to
"address", and possibly also change the model name.  I only ask because for me
it is slightly confusing and makes reading the code harder because in my
experience an "email message" is normally called an email, whereas an "email
address" is normally shortened to address.

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:74: "Email can subscribe only once to
list."),
What about "Email addresses can only appear once on each marketing list."?

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:152: # Make it slow to prevent brute force
attack
Would read slightly better with "attacks" instead of "attack".

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:184: # Make it slow to prevent brute force
attack
Same as above.

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:245: # and not subscribed requests.
What about:

        # Randomize processing time to prevent guessing whether the email
        # address is already subscribed to the list or not.

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.py:274: # and not subscribed requests.
What about:

        # Randomize processing time to prevent guessing whether the email 
        # address was subscribed to the list or not.

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
File modules/marketing_email/marketing.xml (right):

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.xml:14: <!-- Move to marketing module when it
will exists -->
Better as "Move to the marketing module when it exists".

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.xml:41: <field name="name">E-mails</field>
See above, so "E-mail Addresses"?

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.xml:120: <field name="name">Subscribe
Request</field>
"Subscription Request"?

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.xml:127: <field name="name">Unsubscribe
Request</field>
"Request to Unsubscribe"?

https://codereview.tryton.org/38711002/diff/40001/modules/marketing_email/mar...
modules/marketing_email/marketing.xml:272: <field name="name">Send Marketing
Email Messages</field>
Consider just: "Send Marketing Emails".
Sign in to reply to this message.

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