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: address comments, now encodes user input to utf8 Created Feb. 28, 2017, 4 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,19 +7,20 @@
#
# 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
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())
@@ -47,57 +48,108 @@
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('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):
Vasily Kuznetsov 2017/02/28 18:38:49 This function is basically adding the form data to
Jon Sonesen 2017/03/07 12:11:24 I agree
+ if os.path.isfile(path):
+ with open(path, 'ab+') as formlog:
+ formlog.seek(0)
+ reader = DictReader(formlog)
+ if reader.fieldnames != params.keys():
+ formfield_error(params, path)
+ formlog.seek(os.SEEK_END)
+ 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)
Vasily Kuznetsov 2017/02/28 18:38:49 It seems that the log is still mandatory here. Als
Jon Sonesen 2017/03/07 12:11:25 Ok, yeah I think I meant to change that to an attr
+ 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)
+ params = {field: params.get(field, '').encode('utf8')
+ for field in fields}
+ time = datetime.datetime.now()
+ if template is not None:
+ template_args = {
+ 'time': time,
+ 'fields': {field: params.get(field, '')
Vasily Kuznetsov 2017/02/28 18:38:49 This and the following lines could fit in one line
Jon Sonesen 2017/03/07 12:11:25 I usually feel it is easier on the eyes if the key
+ for field in fields}
- template_args = {
- 'time': datetime.datetime.now(),
- 'fields': {field: params.get(field, '') for field in fields}
- }
- sendMail(template, template_args)
+ }
+ sendMail(template, template_args)
+ params['time'] = time
+ ollect_formdata(params, log_path)
Vasily Kuznetsov 2017/02/28 18:38:49 The first 'c' seems to have disappeared somehow ;)
Jon Sonesen 2017/03/07 12:11:25 geeez XD sorry
start_response('200 OK', response_headers)
return ''
return url, handler
conf_dict = conf_parse(get_config_items())
for name, config in conf_dict.items():
« sitescripts/formmail/test/test_formmail2.py ('K') | « sitescripts/formmail/test/test_formmail2.py ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld