From 7a4335b8bc800afcaebd0074d04c2974e8795e7b Mon Sep 17 00:00:00 2001
From: rubenwardy <rw@rubenwardy.com>
Date: Tue, 15 Dec 2020 12:56:17 +0000
Subject: [PATCH] Improve form error messages

---
 app/blueprints/users/account.py |  7 ++---
 app/templates/macros/forms.html | 54 +++++++++++----------------------
 app/templates/users/claim.html  |  2 +-
 3 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/app/blueprints/users/account.py b/app/blueprints/users/account.py
index c82a592..9741b57 100644
--- a/app/blueprints/users/account.py
+++ b/app/blueprints/users/account.py
@@ -206,13 +206,13 @@ def forgot_password():
 class SetPasswordForm(FlaskForm):
 	email = StringField("Email", [Optional(), Email()])
 	password = PasswordField("New password", [InputRequired(), Length(8, 100)])
-	password2 = PasswordField("Verify password", [InputRequired(), Length(8, 100)])
+	password2 = PasswordField("Verify password", [InputRequired(), Length(8, 100), validators.EqualTo('password', message='Passwords must match')])
 	submit = SubmitField("Save")
 
 class ChangePasswordForm(FlaskForm):
 	old_password = PasswordField("Old password", [InputRequired(), Length(8, 100)])
 	password = PasswordField("New password", [InputRequired(), Length(8, 100)])
-	password2 = PasswordField("Verify password", [InputRequired(), Length(8, 100)])
+	password2 = PasswordField("Verify password", [InputRequired(), Length(8, 100), validators.EqualTo('password', message='Passwords must match')])
 	submit = SubmitField("Save")
 
 
@@ -252,9 +252,6 @@ def handle_set_password(form):
 def change_password():
 	form = ChangePasswordForm(request.form)
 
-	if current_user.email is None:
-		form.email.validators = [InputRequired(), Email()]
-
 	if form.validate_on_submit():
 		if check_password_hash(current_user.password, form.old_password.data):
 			ret = handle_set_password(form)
diff --git a/app/templates/macros/forms.html b/app/templates/macros/forms.html
index 9aaa55e..a58a2a3 100644
--- a/app/templates/macros/forms.html
+++ b/app/templates/macros/forms.html
@@ -1,20 +1,22 @@
+{% macro render_errors(field) %}
+	{% for e in field.errors %}
+		<p class="invalid-feedback" style="display: block;">{{ e }}</p>
+	{% endfor %}
+{% endmacro %}
+
 {% macro render_field(field, label=None, label_visible=true, right_url=None, right_label=None, fieldclass=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label and label != "" %}{% set label=field.label.text %}{% endif %}
 			{% if label %}<label for="{{ field.id }}">{{ label|safe }}</label>{% endif %}
 		{% endif %}
 		{{ field(class_=fieldclass or 'form-control', **kwargs) }}
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<p class="help-block">{{ e }}</p>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {%- endmacro %}
 
 {% macro render_field_prefix(field, label=None, prefix="@", label_visible=true, right_url=None, right_label=None, fieldclass=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label and label != "" %}{% set label=field.label.text %}{% endif %}
 			{% if label %}<label for="{{ field.id }}">{{ label|safe }}</label>{% endif %}
@@ -27,16 +29,12 @@
 			{{ field(class_=fieldclass or 'form-control', **kwargs) }}
 		</div>
 
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<p class="help-block">{{ e }}</p>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {%- endmacro %}
 
 {% macro render_field_prefix_button(field, label=None, prefix="@", label_visible=true, right_url=None, right_label=None, fieldclass=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label and label != "" %}{% set label=field.label.text %}{% endif %}
 			{% if label %}<label for="{{ field.id }}">{{ label|safe }}</label>{% endif %}
@@ -52,11 +50,7 @@
 			</a>
 		</div>
 
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<p class="help-block">{{ e }}</p>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {%- endmacro %}
 
@@ -101,7 +95,7 @@
 {% endmacro %}
 
 {% macro render_multiselect_field(field, label=None, label_visible=true, right_url=None, right_label=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label %}{% set label=field.label.text %}{% endif %}
 			<label for="{{ field.id }}">{{ label|safe }}</label>
@@ -112,16 +106,12 @@
 		</div>
 		<div class="invalid-remaining invalid-feedback"></div>
 		{{ field(class_='form-control', **kwargs) }}
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<div class="invalid-feedback">{{ e }}</div>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {% endmacro %}
 
 {% macro render_mpackage_field(field, label=None, label_visible=true, right_url=None, right_label=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label %}{% set label=field.label.text %}{% endif %}
 			<label for="{{ field.id }}">{{ label|safe }}</label>
@@ -132,16 +122,12 @@
 		</div>
 		{{ field(class_='form-control', **kwargs) }}
 		<div class="invalid-remaining invalid-feedback"></div>
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<p class="help-block">{{ e }}</p>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {% endmacro %}
 
 {% macro render_deps_field(field, label=None, label_visible=true, right_url=None, right_label=None) -%}
-	<div class="form-group {% if field.errors %}has-error{% endif %} {{ kwargs.pop('class_', '') }}">
+	<div class="form-group {% if field.errors %}has-danger{% endif %} {{ kwargs.pop('class_', '') }}">
 		{% if field.type != 'HiddenField' and label_visible %}
 			{% if not label %}{% set label=field.label.text %}{% endif %}
 			<label for="{{ field.id }}">{{ label|safe }}</label>
@@ -152,11 +138,7 @@
 		</div>
 		{{ field(class_='form-control', **kwargs) }}
 		<div class="invalid-remaining invalid-feedback"></div>
-		{% if field.errors %}
-			{% for e in field.errors %}
-				<p class="help-block">{{ e }}</p>
-			{% endfor %}
-		{% endif %}
+		{{ render_errors(field) }}
 	</div>
 {% endmacro %}
 
diff --git a/app/templates/users/claim.html b/app/templates/users/claim.html
index b1815c3..30bcb09 100644
--- a/app/templates/users/claim.html
+++ b/app/templates/users/claim.html
@@ -19,7 +19,7 @@ Creating an Account
 					Please log out to continue.
 				</p>
 				<p>
-					<a href="{{ url_for('user.logout', next=url_for('users.claim')) }}" class="btn">Logout</a>
+					<a href="{{ url_for('users.logout', next=url_for('users.claim')) }}" class="btn">Logout</a>
 				</p>
 			{% else %}
 				<p>