Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: sitescripts/formmail/web/formmail2.py

Issue 29374647: Issue 4814 - Adds csv log to formmail2 (Closed) Base URL: https://hg.adblockplus.org/sitescripts
Patch Set: Created Feb. 13, 2017, 12:54 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sitescripts/formmail/web/formmail2.py
===================================================================
--- a/sitescripts/formmail/web/formmail2.py
+++ b/sitescripts/formmail/web/formmail2.py
@@ -7,20 +7,21 @@
#
# Adblock Plus is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
-
+import os
import datetime
import collections
+from csv import DictWriter, DictReader
Sebastian Noack 2017/02/14 09:54:14 The imports are incorrectly grouped here. See my c
Jon Sonesen 2017/02/20 09:55:16 Done.
from sitescripts.utils import get_config, sendMail, encode_email_address
from sitescripts.web import registerUrlHandler, form_handler
def get_config_items():
config = get_config()
default_keys = set(config.defaults())
for name, value in config.items('formmail2'):
@@ -47,57 +48,103 @@
def make_error(spec, check_type, default_message):
if check_type in spec:
return spec[check_type].value
return default_message
+def formfield_error(parameters, log_path):
+ err_file = os.path.basename(log_path) + '_error'
+ err_path = os.path.join(os.path.dirname(log_path), err_file)
+ if os.path.isfile(err_path):
+ with open(err_path, 'a') as error_log:
+ writer = DictWriter(error_log, fieldnames=parameters.keys())
+ writer.writerow(parameters)
+ raise Exception(
Sebastian Noack 2017/02/14 09:54:14 We usually don't use hanging indentation. I'd pref
Jon Sonesen 2017/02/20 09:55:16 Yeah, it seems easier to follow as well.
+ 'Field names have changed, error log appended to ' + err_path)
+ with open(err_path, 'w') as error_log:
+ writer = DictWriter(error_log, fieldnames=parameters.keys())
+ writer.writeheader()
+ writer.writerow(parameters)
+ raise Exception(
+ 'Field names have changed, error log written to ' + err_path)
+
+
+def collect_formdata(params, path):
+ if os.path.isfile(path):
+ with open(path, 'ab+') as formlog:
+ formlog.seek(0)
Sebastian Noack 2017/02/14 09:54:14 This seek seems redundant. When I open a file with
Vasily Kuznetsov 2017/02/15 10:22:26 This is not true on MacOS: $ python Pytho
+ reader = DictReader(formlog)
+ if reader.fieldnames != params.keys():
+ formfield_error(params, path)
+ formlog.seek(1024, os.SEEK_END)
Sebastian Noack 2017/02/14 09:54:14 So you seek 1024 bytes after the end of the file.
Jon Sonesen 2017/02/20 09:55:16 This was a mistake, I was reading the last line (s
+ writer = DictWriter(formlog, fieldnames=params.keys())
+ writer.writerow(params)
+ return
+ with open(path, 'w') as new_formlog:
+ writer = DictWriter(new_formlog, fieldnames=params.keys())
+ writer.writeheader()
+ writer.writerow(params)
+ return
+
+
+def validate_fields(fields, params):
+ errors = []
+ for field, spec in fields.items():
+ if 'mandatory' in spec.value:
+ if field not in params.keys():
+ errors.append(make_error(spec, 'mandatory',
+ 'No {} entered'.format(field)))
+ if 'email' in spec.value and field in params.keys():
+ try:
+ params[field] = encode_email_address(params[field])
+ except ValueError:
+ errors.append(make_error(spec, 'email', 'Invalid email'))
+ return errors
+
+
def make_handler(name, config):
try:
+ log_path = config['csv_log'].value
+ except KeyError:
+ raise Exception('No log configured for form handler: ' + name)
+ try:
url = config['url'].value
except (KeyError, AttributeError):
raise Exception('No URL configured for form handler:' + name)
try:
template = config['template'].value
- except (KeyError, AttributeError):
- raise Exception('No template configured for form handler:' + name)
+ except KeyError:
+ template = None
try:
fields = config['fields']
for field, spec in fields.items():
spec.value = {s.strip() for s in spec.value.split(',')}
except KeyError:
raise Exception('No fields configured for form handler:' + name)
if len(fields) == 0:
raise Exception('No fields configured for form handler:' + name)
@form_handler
def handler(environ, start_response, params):
response_headers = [('Content-Type', 'text/plain; charset=utf-8')]
- errors = []
- for field, spec in fields.items():
- if 'mandatory' in spec.value:
- if field not in params.keys():
- errors.append(make_error(spec, 'mandatory',
- 'No {} entered'.format(field)))
- if 'email' in spec.value and field in params.keys():
- try:
- params[field] = encode_email_address(params[field])
- except ValueError:
- errors.append(make_error(spec, 'email', 'Invalid email'))
+ errors = validate_fields(fields, params)
if errors:
start_response('400 Bad Request', response_headers)
return '\n'.join(errors)
-
- template_args = {
- 'time': datetime.datetime.now(),
- 'fields': {field: params.get(field, '') for field in fields}
- }
- sendMail(template, template_args)
+ params['time'] = datetime.datetime.now()
+ if template is not None:
+ template_args = {
+ 'time': params['time'],
+ 'fields': {field: params.get(field, '') for field in fields}
+ }
+ sendMail(template, template_args)
+ collect_formdata(params, log_path)
start_response('200 OK', response_headers)
return ''
return url, handler
conf_dict = conf_parse(get_config_items())
for name, config in conf_dict.items():

Powered by Google App Engine
This is Rietveld