Issue 2846 - Service for processing Microsoft Edge announcement subscription

sitescripts/submit_email/web/submit_email.py

Index: sitescripts/submit_email/web/submit_email.py
===================================================================
--- a/sitescripts/submit_email/web/submit_email.py
+++ b/sitescripts/submit_email/web/submit_email.py
@@ -49,8 +49,14 @@
   if lang:
     params.append(('lang', lang))
 
+  if data.get('product', '') == '':

[Oleksandr, 2015/08/13 04:12:07]

This "if" and the one below are here so that we would produce a non-ugly file
and template names. We might have stuff like edge_verification_template and
ios_verification_template, but I wasn't sure if we should be touching/renaming
the ios part.

[kzar, 2015/08/18 09:28:56]

Nit: `if data.get('product') is None`? (Or `if not data.get('product')`?)

[Sebastian Noack, 2015/08/18 16:35:03]

I think we should simply fallback to "adblockbrowser", adjusting the
configuration on the server, rather than having conditional logic here.

[Oleksandr, 2015/08/19 09:46:20]

Done.

+    email_template = 'verification_email_template'
+  else:
+    email_template = data.get('product', '') + '_verification_email_template'

[kzar, 2015/08/18 09:28:56]

We should probably assign data.get('product') to a variable above, and then
re-use it here.

[kzar, 2015/08/18 09:28:57]

Use string formatting like `"%s_verification_email_template" % product`?

[Sebastian Noack, 2015/08/18 16:35:03]

I think using the + operator when concatenating only two strings is fine. Format
strings neither make the code more readable nor faster in that case. Actually it
does the opposite. While it is the other way around when you concatenate more
strings.

[Oleksandr, 2015/08/19 09:46:20]

Done.

+    params.append(('product', data.get('product', '')))

[kzar, 2015/08/18 09:28:56]

Again, we should re-use the variable instead of performing `data.get('product',
'')`.

[Oleksandr, 2015/08/19 09:46:20]

Done.

+
   sendMail(
-    config.get('submit_email', 'verification_email_template'),
+    config.get('submit_email', email_template),
     {
       'recipient': email,
       'verification_url': '%s?%s' % (
@@ -79,7 +85,11 @@
       'Invalid signature in verification request.'
     )
 
-  filename = config.get('submit_email', 'filename')
+  if params.get('product', '') == '':

[kzar, 2015/08/18 09:28:57]

Same issues as above in this code.

+    filename = config.get('submit_email', 'filename')
+  else:
+    filename = config.get('submit_email', params.get('product', '') + '_filename')
+
   with open(filename, 'ab', 0) as file:
     fcntl.lockf(file, fcntl.LOCK_EX)
     try: