Skip to content

Commit

Permalink
Remove dependency on lxml to render error messages
Browse files Browse the repository at this point in the history
We have FormEncode-Jinja2 which includes error message display in
its formfill extension. We can therefore wrap all existing forms
in formfill statements that include the 'with' control to include
form_errors:

    {% formfill form_fill with form_errors %}
        <form>...</form>
    {% endformfill %}

Because of that we do not have to preprocess error messages and
can remove that legacy code.
  • Loading branch information
Tryggvi Bjorgvinsson committed Nov 24, 2014
1 parent 673567d commit 8b1cc50
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 49 deletions.
12 changes: 7 additions & 5 deletions openspending/ui/alttemplates/account/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,27 @@ <h3>{% trans %}Login{% endtrans %}</h3>
</div>
<div class="span6">
<h3>{% trans %}Create an account{% endtrans %}</h3>
{% formfill form_fill with form_errors %}
<form class="basic register form-horizontal" id="register" action="{{ h.url_for(controller='account', action='register') }}" method="POST">
<fieldset>
<div class="control-group">
<label for="name">{% trans %}Login{% endtrans %}</label>
<div class="controls">
<input name="name"{% if form_fill and 'name' in form_fill %} value="{{form_fill.name}}"{% endif %}>
<input name="name">
</div>
</div>
<div class="control-group">
<label for="fullname">{% trans %}Fullname{% endtrans %}</label>
<div class="controls">
<input name="fullname"{% if form_fill and 'fullname' in form_fill %} value="{{form_fill.fullname}}"{% endif %}>
<input name="fullname">
</div>
</div>
<div class="control-group form-inline">
<label for="email">{% trans %}Email{% endtrans %}</label>
<div class="controls">
<input name="email"{% if form_fill and 'email' in form_fill %} value="{{form_fill.email}}"{% endif %}>
<input name="email">
<label class="checkbox">
<input name="public_email" type="checkbox"{% if form_fill and 'public_email' in form_fill %} checked="checked"{% endif %}> {% trans %}Visible to other users{% endtrans %}
<input name="public_email" type="checkbox"> {% trans %}Visible to other users{% endtrans %}
</label>
</div>
</div>
Expand Down Expand Up @@ -81,7 +82,7 @@ <h3>{% trans %}Create an account{% endtrans %}</h3>
{% endif %}
{% if c.config.get('openspending.subscribe_developer') %}
<label class="checkbox">
<input type="checkbox" name="subscribe_developer"{% if form_fill and 'subscribe_developer' in form_fill %} checked="checked"{% endif %}>
<input type="checkbox" name="subscribe_developer">
{% trans %}Sign me up for the OpenSpending developer mailing list{% endtrans %}
</label>
{% endif %}
Expand All @@ -93,6 +94,7 @@ <h3>{% trans %}Create an account{% endtrans %}</h3>
<input value="{% trans %}Create new account{% endtrans %}" class="btn btn-primary" type="submit" />
</div>
</form>
{% endformfill %}
</div>
</div>

Expand Down
12 changes: 7 additions & 5 deletions openspending/ui/alttemplates/account/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,29 @@ <h2 class="page-header">{% trans %}Account Settings{% endtrans %} <small>{{ c.ac

<div class="row">
<div class="offset2 span10">
{% formfill form_fill with form_errors %}
<form class="basic settings form-horizontal" id="settings" action="/settings" method="POST">
<div class="control-group">
<label for="fullname">{% trans %}Fullname{% endtrans %}</label>
<div class="controls">
<input name="fullname"{% if 'fullname' in form_fill %} value="{{form_fill.fullname}}"{% endif %}>
<input name="fullname">
</div>
</div>
<div class="control-group form-inline">
<label for="email">{% trans %}Email{% endtrans %}</label>
<div class="controls">
<input name="email"{% if 'email' in form_fill %} value="{{form_fill.email}}"{% endif %}>
<input name="email">
<label class="checkbox">
<input name="public_email" type="checkbox"{% if 'public_email' in form_fill %} checked="checked"{% endif %}> {% trans %}Visible to users{% endtrans %}
<input name="public_email" type="checkbox"> {% trans %}Visible to users{% endtrans %}
</label>
</div>
</div>
<div class="control-group form-inline">
<label for="email">Twitter</label>
<div class="controls">
<input name="twitter"{% if 'twitter' in form_fill %} value="{{form_fill.twitter}}"{% endif %}>
<input name="twitter">
<label class="checkbox">
<input name="public_twitter" type="checkbox"{% if 'public_twitter' in form_fill %} checked="checked"{% endif %}> {% trans %}Visible to users{% endtrans %}
<input name="public_twitter" type="checkbox"> {% trans %}Visible to users{% endtrans %}
</label>
</div>
</div>
Expand Down Expand Up @@ -64,6 +65,7 @@ <h2 class="page-header">{% trans %}Account Settings{% endtrans %} <small>{{ c.ac
<input value="{% trans %}Save{% endtrans %}" type="submit" class="btn btn-primary" />
</div>
</form>
{% endformfill %}
</div>
</div>

Expand Down
2 changes: 2 additions & 0 deletions openspending/ui/alttemplates/account/trigger_reset.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ <h2 class="page-header">{% trans %}Reset your password{% endtrans %}</h2>
<p>
{% trans %}Please enter the email address you used to register your account{% endtrans %}:
</p>
{% formfill form_fill with form_errors %}
<form action="{{ h.url_for(controller='account', action='trigger_reset') }}" method="POST" class="form-inline">
<input type="text" class="input-large span5" name="email" placeholder="{% trans %}Email{% endtrans %}" />
<button type="submit" class="btn">{% trans %}Request a reset{% endtrans %}</button>
</form>
{% endformfill %}
</div>
</div>

Expand Down
2 changes: 2 additions & 0 deletions openspending/ui/alttemplates/badge/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ <h2 class="page-header">{% trans %}Badges in OpenSpending{% endtrans %}</h2>
<p>{% trans %}These badges can be awarded to specific datasets by administrators of OpenSpending. The badges indicate that an official, either from <a href="http://okfn.org" title="Open Knowledge Foundation">The Open Knowledge Foundation</a> (OKF) or from another entity approved by the OKF, has carefully examined the dataset and decided to award it that particular badge.{% endtrans %}</p>

{% if c.account.admin %}
{% formfill form_fill with form_errors %}
<form action="{{h.url_for(controller='badge', action='create')}}" method="post" enctype="multipart/form-data">
<fieldset>
<legend>{% trans %}Create a new badge{% endtrans %}</legend>
Expand All @@ -50,6 +51,7 @@ <h2 class="page-header">{% trans %}Badges in OpenSpending{% endtrans %}</h2>
</div>
</fieldset>
</form>
{% endformfill %}
{% endif %}
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions openspending/ui/alttemplates/dataset/new.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ <h2 class="page-header">{% trans %}Import a dataset{% endtrans %}</h2>
</div>
</div>
<div class="span8 dataset-info">
{% formfill form_fill with form_errors %}
<form class="basic form-horizontal" id="login" action="/datasets" method="POST">
<input name="ckan_uri" type="hidden" />
<fieldset>
Expand Down Expand Up @@ -112,6 +113,7 @@ <h2 class="page-header">{% trans %}Import a dataset{% endtrans %}</h2>
<input value="{% trans %}Next Step{% endtrans %}" class="btn btn-success" type="submit" />
</div>
</form>
{% endformfill %}
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions openspending/ui/alttemplates/editor/dimensions.html
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,14 @@ <h3>{% trans %}Unmapped source data columns{% endtrans %}</h3>
</div>
{% endif %}

{% if c.errors|length %}
{% if form_errors|length %}
<div class="alert block-message alert-error">
<p>
{% trans %}The dimensions mapping could not be saved as it
contains some errors{% endtrans %}:
</p>
<ul>
{% for field, error in c.errors.items() -%}
{% for field, error in form_errors.items() -%}
<li><strong>{{ field }}:</strong> {{ error }}</li>
{%- endfor %}
</ul>
Expand Down
2 changes: 2 additions & 0 deletions openspending/ui/alttemplates/editor/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ <h3>{% trans %}Dataset Actions{% endtrans %}</h3>
<!-- Create source modal -->
<div id="new-source" class="modal hide fade"
ckan-uri="{{ c.dataset.ckan_uri }}">
{% formfill form_fill with form_errors %}
<form class="basic" action="{{ h.url_for(controller='source', action='create',
dataset=c.dataset.name) }}" method="POST">
<div class="modal-header">
Expand All @@ -136,6 +137,7 @@ <h3>{% trans %}Create a new data source{% endtrans %}</h3>
<input value="{% trans %}Create{% endtrans %}" class="btn btn-success" type="submit" />
</div>
</form>
{% endformfill %}
</div>
</div>
{% endblock %}
4 changes: 2 additions & 2 deletions openspending/ui/alttemplates/editor/team.html
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@
manage this dataset.{% endtrans %}
</div>
<div class="span8">
{% if c.errors %}
{% if form_errors %}
<div class="alert block-message alert-error">
<ul>{% for field, error in c.errors.items() %}
<ul>{% for field, error in form_errors.items() %}
<li><strong>{{ field }}:</strong> {{ error }}</li>
{% endfor %}</ul>
</div>
Expand Down
2 changes: 1 addition & 1 deletion openspending/ui/alttemplates/editor/templates.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
</div>
</div>
<div class="span8">
{% formfill form_fill %}
{% formfill form_fill with form_errors %}
<form class="basic form-horizontal"
action="{{ h.url_for(controller='editor',
action='templates_update', dataset=c.dataset.name) }}"
Expand Down
6 changes: 3 additions & 3 deletions openspending/ui/alttemplates/editor/views.html
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,19 @@ <h3>{% trans %}Available Dimensions{% endtrans %}</h3>
{% endfor %}</ul>
</div>
<div class="span8">
{% formfill form_fill%}
{% formfill form_fill %}
<form class="basic" id="form"
action="{{ h.url_for(controller='editor',
action='views_update', dataset=c.dataset.name) }}"
method="POST">
<fieldset>
{% if c.errors|length %}
{% if form_errors|length %}
<div class="alert block-message alert-error">
<p>
{% trans %}The views could not be saved as they contain some
errors{% endtrans %}:
</p>
<ul>{% for field, error in c.errors.items() %}
<ul>{% for field, error in form_errors.items() %}
<li><strong>{{ field }}:</strong> {{ error }}</li>
{% endfor %}</ul>
</div>
Expand Down
38 changes: 8 additions & 30 deletions openspending/ui/alttemplates/templating.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
from jinja2.environment import Environment
import formencode_jinja2

import lxml.html
from lxml.html import builder as E

# Set the directory where this file is as the template root directory
template_rootdir = os.path.abspath(os.path.dirname(__file__))
Expand Down Expand Up @@ -55,31 +53,6 @@ def section_active(section):
}[v]) for k, v in tmp.iteritems()])


def postprocess_forms(s, form_errors):
def tag_errors(tag, root):
for i in root.cssselect(tag):
name = i.attrib.get('name', None)
value = form_errors.get(name, None)
if value is not None:
p = E.P(value)
p.set('class', 'help-block error')
i.addnext(p)

def input_errors(root):
return tag_errors('input', root)

def select_errors(root):
return tag_errors('select', root)

def textarea_errors(root):
return tag_errors('textarea', root)

root = lxml.html.fromstring(s)
processors = [input_errors, select_errors, textarea_errors]
[process(root) for process in processors]
return lxml.html.tostring(root, doctype=root.getroottree().docinfo.doctype)


def render(path, **kwargs):
"""Render a template with jinja2
Expand Down Expand Up @@ -118,8 +91,13 @@ def render(path, **kwargs):
"c": c,
"g": app_globals,
"can": can,
"show_rss": hasattr(c, 'show_rss') and c.show_rss or None
"show_rss": hasattr(c, 'show_rss') and c.show_rss or None,
}
params.update(kwargs)
form_errors = params.get('form_errors', {})
return postprocess_forms(template.render(params), form_errors)
if 'form_fill' in params:
if params['form_fill'] is not None and len(params['form_fill']) > 0:
params['form_fill'] = dict(params['form_fill'])
else:
params['form_fill'] = {}

return template.render(params)
7 changes: 6 additions & 1 deletion openspending/ui/public/static/style/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,14 @@ a:hover, a:active {
margin-bottom: 0.5em;
}

form .error {
form .error, form .error-message {
color: #B94A48;
}
form .error-message {
display: inline-block;
padding-top: 5px;
margin-bottom: 0px;
}

.modal.big-modal {
width: 784px;
Expand Down

0 comments on commit 8b1cc50

Please sign in to comment.