Issue 2846 - Service for processing Microsoft Edge announcement subscription

sitescripts/submit_email/web/submit_email.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
@@ skipping 31 matching lines @@
       start_response, 400,
       'Please enter a valid email address.'
     )
 
   config = get_config()
   params = [('email', email), ('signature', sign(config, email))]
   lang = data.get('lang')
   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' % (
         urljoin(wsgiref.util.application_uri(environ), VERIFICATION_PATH),
         urlencode(params)
       )
     }
   )
 
   return send_simple_response(
     start_response, 200,
     'A confirmation email has been sent. Please check '
     'your email and click the confirmation link.'
   )
 
 @url_handler(VERIFICATION_PATH)
 def verify_email(environ, start_response):
   config = get_config()
   params = dict(parse_qsl(environ.get('QUERY_STRING', '')))
 
   email = params.get('email', '')
   signature = params.get('signature', '')
   if sign(config, email) != signature:
     return send_simple_response(
       start_response, 403,
       '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:
       print >>file, email
     finally:
       fcntl.lockf(file, fcntl.LOCK_UN)
 
   location = config.get('submit_email', 'successful_verification_redirect_location')
   location = location.format(lang=quote(params.get('lang') or 'en', ''))
   start_response('303 See Other', [('Location', location)])
   return []