From e7a117be3376787bf07731aca6841362a4cf3bb1 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 15 Oct 2018 11:06:04 -0400 Subject: [PATCH 01/15] move user edit routes into their own module and blueprint --- atst/app.py | 2 ++ atst/models/user.py | 7 +++++++ atst/routes/__init__.py | 14 -------------- atst/routes/users.py | 17 +++++++++++++++++ templates/navigation/topbar.html | 2 +- templates/user/edit.html | 7 +------ 6 files changed, 28 insertions(+), 21 deletions(-) create mode 100644 atst/routes/users.py diff --git a/atst/app.py b/atst/app.py index 884d2bee..0cc2ca13 100644 --- a/atst/app.py +++ b/atst/app.py @@ -15,6 +15,7 @@ from atst.routes import bp from atst.routes.workspaces import bp as workspace_routes from atst.routes.requests import requests_bp from atst.routes.dev import bp as dev_routes +from atst.routes.users import bp as user_routes from atst.routes.errors import make_error_pages from atst.domain.authnid.crl import CRLCache from atst.domain.auth import apply_authentication @@ -57,6 +58,7 @@ def make_app(config): app.register_blueprint(bp) app.register_blueprint(workspace_routes) app.register_blueprint(requests_bp) + app.register_blueprint(user_routes) if ENV != "prod": app.register_blueprint(dev_routes) diff --git a/atst/models/user.py b/atst/models/user.py index 48570937..2c03b8a0 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -52,3 +52,10 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): self.has_workspaces, self.id, ) + + def to_dictionary(self): + return { + c.name: getattr(self, c.name) + for c in self.__table__.columns + if c.name not in ["id"] + } diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index c4b76de3..5051ad5a 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -10,7 +10,6 @@ from atst.domain.users import Users from atst.domain.authnid import AuthenticationContext from atst.domain.audit_log import AuditLog from atst.domain.auth import logout as _logout -from atst.forms.edit_user import EditUserForm bp = Blueprint("atst", __name__) @@ -119,19 +118,6 @@ def activity_history(): return render_template("audit_log.html", audit_events=audit_events) -@bp.route("/user") -def user(): - form = EditUserForm(request.form) - user = g.current_user - return render_template("user/edit.html", form=form, user=user) - - -@bp.route("/save_user") -def save_user(): - # no op - return redirect(url_for(".home")) - - @bp.route("/about") def about(): return render_template("about.html") diff --git a/atst/routes/users.py b/atst/routes/users.py new file mode 100644 index 00000000..2762421c --- /dev/null +++ b/atst/routes/users.py @@ -0,0 +1,17 @@ +from flask import Blueprint, render_template, g, redirect, session, url_for, request +from atst.forms.edit_user import EditUserForm + + +bp = Blueprint("users", __name__) + + +@bp.route("/user") +def user(): + user = g.current_user + form = EditUserForm(data=user.to_dictionary()) + return render_template("user/edit.html", form=form, user=user) + + +@bp.route("/user", methods=["POST"]) +def update_user(): + return redirect(url_for(".home")) diff --git a/templates/navigation/topbar.html b/templates/navigation/topbar.html index 6bcd0d0a..ceeefcfb 100644 --- a/templates/navigation/topbar.html +++ b/templates/navigation/topbar.html @@ -20,7 +20,7 @@ {% endif %} - + {{ g.current_user.first_name + " " + g.current_user.last_name }} {{ Icon('avatar', classes='topbar__link-icon') }} diff --git a/templates/user/edit.html b/templates/user/edit.html index 44cbc71f..9a26b115 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -1,12 +1,7 @@ {% extends "base.html" %} -{% from "components/alert.html" import Alert %} {% block content %}
- {{ Alert('This form does not yet function', - message="

Functionality of this form is pending more engineering work. Engineers, please remove this alert when done.

", - level='warning' - ) }}
@@ -17,7 +12,7 @@
- {% set form_action = url_for('atst.save_user') %} + {% set form_action = url_for('users.user') %} {% include "fragments/edit_user_form.html" %}
From 30318b68bb222419a0702b8eb09924100cb04970 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 15 Oct 2018 11:11:35 -0400 Subject: [PATCH 02/15] add additional user fields --- ...c99026ab9918_add_additional_user_fields.py | 36 +++++++++++++++++++ atst/models/user.py | 7 +++- 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/c99026ab9918_add_additional_user_fields.py diff --git a/alembic/versions/c99026ab9918_add_additional_user_fields.py b/alembic/versions/c99026ab9918_add_additional_user_fields.py new file mode 100644 index 00000000..f0dc6540 --- /dev/null +++ b/alembic/versions/c99026ab9918_add_additional_user_fields.py @@ -0,0 +1,36 @@ +"""add additional user fields + +Revision ID: c99026ab9918 +Revises: 903d7c66ff1d +Create Date: 2018-10-15 11:10:46.073745 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c99026ab9918' +down_revision = '903d7c66ff1d' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('citizenship', sa.String(), nullable=True)) + op.add_column('users', sa.Column('date_latest_training', sa.Date(), nullable=True)) + op.add_column('users', sa.Column('designation', sa.String(), nullable=True)) + op.add_column('users', sa.Column('phone_number', sa.String(), nullable=True)) + op.add_column('users', sa.Column('service_branch', sa.String(), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'service_branch') + op.drop_column('users', 'phone_number') + op.drop_column('users', 'designation') + op.drop_column('users', 'date_latest_training') + op.drop_column('users', 'citizenship') + # ### end Alembic commands ### diff --git a/atst/models/user.py b/atst/models/user.py index 2c03b8a0..bb89c51b 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -1,4 +1,4 @@ -from sqlalchemy import String, ForeignKey, Column +from sqlalchemy import String, ForeignKey, Column, Date from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import UUID @@ -20,6 +20,11 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): dod_id = Column(String, unique=True, nullable=False) first_name = Column(String) last_name = Column(String) + phone_number = Column(String) + service_branch = Column(String) + citizenship = Column(String) + designation = Column(String) + date_latest_training = Column(Date) @property def atat_permissions(self): From ec7504fb20722df1928f4cac2de2d5b0a12b48a9 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 15 Oct 2018 13:17:47 -0400 Subject: [PATCH 03/15] add update method to Users repo --- atst/domain/users.py | 28 ++++++++++++++++++++++++++-- templates/base_public.html | 2 +- tests/domain/test_users.py | 24 ++++++++++++++++++------ 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/atst/domain/users.py b/atst/domain/users.py index 3728c04f..c67c8cbd 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -5,7 +5,7 @@ from atst.database import db from atst.models import User from .roles import Roles -from .exceptions import NotFoundError, AlreadyExistsError +from .exceptions import NotFoundError, AlreadyExistsError, UnauthorizedError class Users(object): @@ -53,7 +53,7 @@ class Users(object): return user @classmethod - def update(cls, user_id, atat_role_name): + def update_role(cls, user_id, atat_role_name): user = Users.get(user_id) atat_role = Roles.get(atat_role_name) @@ -63,3 +63,27 @@ class Users(object): db.session.commit() return user + + _UPDATEABLE_ATTRS = { + "first_name", + "last_name", + "email", + "phone_number", + "service_branch", + "citizenship", + "designation", + "date_latest_training", + } + + @classmethod + def update(cls, user, user_delta): + if not set(user_delta.keys()).issubset(Users._UPDATEABLE_ATTRS): + raise UnauthorizedError(user, "update DOD ID") + + for key, value in user_delta.items(): + setattr(user, key, value) + + db.session.add(user) + db.session.commit() + + return user diff --git a/templates/base_public.html b/templates/base_public.html index 006b04e5..f444dff8 100644 --- a/templates/base_public.html +++ b/templates/base_public.html @@ -26,7 +26,7 @@ {% if g.current_user %} - + {{ g.current_user.first_name + " " + g.current_user.last_name }} {{ Icon('avatar', classes='topbar__link-icon') }} diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index 145e8f93..51cfaaac 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -2,7 +2,7 @@ import pytest from uuid import uuid4 from atst.domain.users import Users -from atst.domain.exceptions import NotFoundError, AlreadyExistsError +from atst.domain.exceptions import NotFoundError, AlreadyExistsError, UnauthorizedError DOD_ID = "my_dod_id" @@ -52,20 +52,32 @@ def test_get_user_by_dod_id(): assert user == new_user -def test_update_user(): +def test_update_role(): new_user = Users.create(DOD_ID, "developer") - updated_user = Users.update(new_user.id, "ccpo") + updated_user = Users.update_role(new_user.id, "ccpo") assert updated_user.atat_role.name == "ccpo" -def test_update_nonexistent_user(): +def test_update_role_with_nonexistent_user(): Users.create(DOD_ID, "developer") with pytest.raises(NotFoundError): - Users.update(uuid4(), "ccpo") + Users.update_role(uuid4(), "ccpo") def test_update_existing_user_with_nonexistent_role(): new_user = Users.create(DOD_ID, "developer") with pytest.raises(NotFoundError): - Users.update(new_user.id, "nonexistent") + Users.update_role(new_user.id, "nonexistent") + + +def test_update_user(): + new_user = Users.create(DOD_ID, "developer") + updated_user = Users.update(new_user, {"first_name": "Jabba"}) + assert updated_user.first_name == "Jabba" + + +def test_update_user_with_dod_id(): + new_user = Users.create(DOD_ID, "developer") + with pytest.raises(UnauthorizedError): + Users.update(new_user, {"dod_id": "1234567890"}) From f3c333212783528bd88fd85bbdeac22de8b9fae8 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 15 Oct 2018 15:30:15 -0400 Subject: [PATCH 04/15] user can update profile --- atst/routes/users.py | 12 ++++++++++-- templates/fragments/edit_user_form.html | 3 ++- tests/factories.py | 11 +++++++++++ tests/routes/test_users.py | 22 ++++++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 tests/routes/test_users.py diff --git a/atst/routes/users.py b/atst/routes/users.py index 2762421c..adb1246c 100644 --- a/atst/routes/users.py +++ b/atst/routes/users.py @@ -1,5 +1,6 @@ -from flask import Blueprint, render_template, g, redirect, session, url_for, request +from flask import Blueprint, render_template, g, redirect, url_for, request as http_request from atst.forms.edit_user import EditUserForm +from atst.domain.users import Users bp = Blueprint("users", __name__) @@ -14,4 +15,11 @@ def user(): @bp.route("/user", methods=["POST"]) def update_user(): - return redirect(url_for(".home")) + user = g.current_user + form = EditUserForm(http_request.form) + if form.validate(): + Users.update(user, form.data) + return redirect(url_for("atst.home")) + else: + return render_template("user/edit.html", form=form, user=user) + diff --git a/templates/fragments/edit_user_form.html b/templates/fragments/edit_user_form.html index ed0ec406..1e7f3fe2 100644 --- a/templates/fragments/edit_user_form.html +++ b/templates/fragments/edit_user_form.html @@ -2,7 +2,8 @@ {% from "components/options_input.html" import OptionsInput %} {% from "components/date_input.html" import DateInput %} -
+ + {{ form.csrf_token }}
diff --git a/tests/factories.py b/tests/factories.py index 0ab5f5fd..ae6f1a7e 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -48,6 +48,17 @@ class UserFactory(Base): last_name = factory.Faker("last_name") atat_role = factory.SubFactory(RoleFactory) dod_id = factory.LazyFunction(lambda: "".join(random.choices(string.digits, k=10))) + phone_number = factory.LazyFunction( + lambda: "".join(random.choices(string.digits, k=10)) + ) + service_branch = factory.LazyFunction( + lambda: random.choices([k for k, v in SERVICE_BRANCHES if k is not None])[0] + ) + citizenship = "United States" + designation = "military" + date_latest_training = factory.LazyFunction( + lambda: datetime.date.today() + datetime.timedelta(days=-(random.randrange(1,365))) + ) @classmethod def from_atat_role(cls, atat_role_name, **kwargs): diff --git a/tests/routes/test_users.py b/tests/routes/test_users.py new file mode 100644 index 00000000..deb2cc3a --- /dev/null +++ b/tests/routes/test_users.py @@ -0,0 +1,22 @@ +from flask import url_for + +from atst.domain.users import Users + +from tests.factories import UserFactory + +def test_user_can_view_profile(user_session, client): + user = UserFactory.create() + user_session(user) + response = client.get(url_for("users.user")) + assert user.email in response.data.decode() + + +def test_user_can_update_profile(user_session, client): + user = UserFactory.create() + user_session(user) + new_data = {**user.to_dictionary(), "first_name": "chad", "last_name": "vader"} + new_data["date_latest_training"] = new_data["date_latest_training"].strftime("%m/%d/%Y") + client.post(url_for("users.user"), data=new_data) + updated_user = Users.get_by_dod_id(user.dod_id) + assert updated_user.first_name == "chad" + assert updated_user.last_name == "vader" From ab422457973a66986b72575ac92bb4f55c89e18e Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 15 Oct 2018 16:21:40 -0400 Subject: [PATCH 05/15] display DOD ID on user update page --- atst/routes/users.py | 10 ++++++++-- templates/user/edit.html | 1 + tests/factories.py | 3 ++- tests/routes/test_users.py | 5 ++++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/atst/routes/users.py b/atst/routes/users.py index adb1246c..5129efeb 100644 --- a/atst/routes/users.py +++ b/atst/routes/users.py @@ -1,4 +1,11 @@ -from flask import Blueprint, render_template, g, redirect, url_for, request as http_request +from flask import ( + Blueprint, + render_template, + g, + redirect, + url_for, + request as http_request, +) from atst.forms.edit_user import EditUserForm from atst.domain.users import Users @@ -22,4 +29,3 @@ def update_user(): return redirect(url_for("atst.home")) else: return render_template("user/edit.html", form=form, user=user) - diff --git a/templates/user/edit.html b/templates/user/edit.html index 9a26b115..9fb84ac8 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -7,6 +7,7 @@

{{ user.first_name }} {{ user.last_name }}
+
DOD ID: {{ user.dod_id }}
Edit user details

diff --git a/tests/factories.py b/tests/factories.py index ae6f1a7e..b2338a06 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -57,7 +57,8 @@ class UserFactory(Base): citizenship = "United States" designation = "military" date_latest_training = factory.LazyFunction( - lambda: datetime.date.today() + datetime.timedelta(days=-(random.randrange(1,365))) + lambda: datetime.date.today() + + datetime.timedelta(days=-(random.randrange(1, 365))) ) @classmethod diff --git a/tests/routes/test_users.py b/tests/routes/test_users.py index deb2cc3a..d9615d5f 100644 --- a/tests/routes/test_users.py +++ b/tests/routes/test_users.py @@ -4,6 +4,7 @@ from atst.domain.users import Users from tests.factories import UserFactory + def test_user_can_view_profile(user_session, client): user = UserFactory.create() user_session(user) @@ -15,7 +16,9 @@ def test_user_can_update_profile(user_session, client): user = UserFactory.create() user_session(user) new_data = {**user.to_dictionary(), "first_name": "chad", "last_name": "vader"} - new_data["date_latest_training"] = new_data["date_latest_training"].strftime("%m/%d/%Y") + new_data["date_latest_training"] = new_data["date_latest_training"].strftime( + "%m/%d/%Y" + ) client.post(url_for("users.user"), data=new_data) updated_user = Users.get_by_dod_id(user.dod_id) assert updated_user.first_name == "chad" From 8af23fda369351f243250feb2807c8499cd98c00 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 09:37:59 -0400 Subject: [PATCH 06/15] make unpermitted attribute handling in Users.update more specific --- atst/domain/users.py | 6 ++++-- tests/domain/test_users.py | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/atst/domain/users.py b/atst/domain/users.py index c67c8cbd..7494f5f4 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -77,8 +77,10 @@ class Users(object): @classmethod def update(cls, user, user_delta): - if not set(user_delta.keys()).issubset(Users._UPDATEABLE_ATTRS): - raise UnauthorizedError(user, "update DOD ID") + delta_set = set(user_delta.keys()) + if not set(delta_set).issubset(Users._UPDATEABLE_ATTRS): + unpermitted = delta_set - Users._UPDATEABLE_ATTRS + raise UnauthorizedError(user, "update {}".format(", ".join(unpermitted))) for key, value in user_delta.items(): setattr(user, key, value) diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index 51cfaaac..de6c7fc8 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -79,5 +79,7 @@ def test_update_user(): def test_update_user_with_dod_id(): new_user = Users.create(DOD_ID, "developer") - with pytest.raises(UnauthorizedError): + with pytest.raises(UnauthorizedError) as excinfo: Users.update(new_user, {"dod_id": "1234567890"}) + + assert "dod_id" in str(excinfo.value) From 84d091fcf1f51123fbd593f05c8f1cbc927f6a30 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 13:21:41 -0400 Subject: [PATCH 07/15] only CAC fields should be required for editing user profile --- atst/forms/edit_user.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index 0bf8f211..87737091 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -25,7 +25,7 @@ class EditUserForm(ValidatedForm): phone_number = TelField( "Phone Number", description="Enter your 10-digit U.S. phone number", - validators=[Required(), PhoneNumber()], + validators=[PhoneNumber()], ) service_branch = SelectField( @@ -41,7 +41,6 @@ class EditUserForm(ValidatedForm): ("Foreign National", "Foreign National"), ("Other", "Other"), ], - validators=[Required()], ) designation = RadioField( @@ -52,14 +51,12 @@ class EditUserForm(ValidatedForm): ("civilian", "Civilian"), ("contractor", "Contractor"), ], - validators=[Required()], ) date_latest_training = DateField( "Latest Information Assurance (IA) Training Completion Date", description='To complete the training, you can find it in Information Assurance Cyber Awareness Challange website.', validators=[ - Required(), DateRange( lower_bound=pendulum.duration(years=1), upper_bound=pendulum.duration(days=0), From 65bc05d21430e0977a5410f74a89899f54c1edf9 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 13:30:22 -0400 Subject: [PATCH 08/15] display success alert when user updates profile info --- atst/forms/edit_user.py | 2 +- atst/routes/users.py | 16 +++++----------- templates/user/edit.html | 5 +++++ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index 87737091..8b527c3f 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -61,7 +61,7 @@ class EditUserForm(ValidatedForm): lower_bound=pendulum.duration(years=1), upper_bound=pendulum.duration(days=0), message="Must be a date within the last year.", - ), + ) ], format="%m/%d/%Y", ) diff --git a/atst/routes/users.py b/atst/routes/users.py index 5129efeb..e57584a7 100644 --- a/atst/routes/users.py +++ b/atst/routes/users.py @@ -1,11 +1,4 @@ -from flask import ( - Blueprint, - render_template, - g, - redirect, - url_for, - request as http_request, -) +from flask import Blueprint, render_template, g, request as http_request from atst.forms.edit_user import EditUserForm from atst.domain.users import Users @@ -24,8 +17,9 @@ def user(): def update_user(): user = g.current_user form = EditUserForm(http_request.form) + rerender_args = {"form": form, "user": user} if form.validate(): Users.update(user, form.data) - return redirect(url_for("atst.home")) - else: - return render_template("user/edit.html", form=form, user=user) + rerender_args["updated"] = True + + return render_template("user/edit.html", **rerender_args) diff --git a/templates/user/edit.html b/templates/user/edit.html index 9fb84ac8..77afdd62 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -1,8 +1,13 @@ {% extends "base.html" %} +{% from "components/alert.html" import Alert %} {% block content %}
+ {% if updated %} + {{ Alert('User information updated.', level='success') }} + {% endif %} +

From dab5fe83fc5f6b76b1dc181029b5a143fbba2e0a Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 15:14:23 -0400 Subject: [PATCH 09/15] create dictionary of base user form fields --- atst/forms/edit_user.py | 51 +++++++++++++++++--------------- atst/forms/new_request.py | 62 +++++++++++++-------------------------- 2 files changed, 47 insertions(+), 66 deletions(-) diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index 8b527c3f..017a9bbc 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -9,41 +9,33 @@ from .data import SERVICE_BRANCHES from .validators import Alphabet, DateRange, PhoneNumber - -class EditUserForm(ValidatedForm): - - first_name = StringField("First Name", validators=[Required(), Alphabet()]) - - last_name = StringField("Last Name", validators=[Required(), Alphabet()]) - - email = EmailField( +USER_FIELDS = { + "first_name": StringField("First Name", validators=[Required(), Alphabet()]), + "last_name": StringField("Last Name", validators=[Required(), Alphabet()]), + "email": EmailField( "E-mail Address", description="Enter your preferred contact e-mail address", validators=[Required(), Email()], - ) - - phone_number = TelField( + ), + "phone_number": TelField( "Phone Number", description="Enter your 10-digit U.S. phone number", validators=[PhoneNumber()], - ) - - service_branch = SelectField( + ), + "service_branch": SelectField( "Service Branch or Agency", description="Which service or organization do you belong to within the DoD?", choices=SERVICE_BRANCHES, - ) - - citizenship = RadioField( + ), + "citizenship": RadioField( description="What is your citizenship status?", choices=[ ("United States", "United States"), ("Foreign National", "Foreign National"), ("Other", "Other"), ], - ) - - designation = RadioField( + ), + "designation": RadioField( "Designation of Person", description="What is your designation within the DoD?", choices=[ @@ -51,9 +43,8 @@ class EditUserForm(ValidatedForm): ("civilian", "Civilian"), ("contractor", "Contractor"), ], - ) - - date_latest_training = DateField( + ), + "date_latest_training": DateField( "Latest Information Assurance (IA) Training Completion Date", description='To complete the training, you can find it in Information Assurance Cyber Awareness Challange website.', validators=[ @@ -64,4 +55,16 @@ class EditUserForm(ValidatedForm): ) ], format="%m/%d/%Y", - ) + ), +} + + +class EditUserForm(ValidatedForm): + first_name = USER_FIELDS["first_name"] + last_name = USER_FIELDS["last_name"] + email = USER_FIELDS["email"] + phone_number = USER_FIELDS["phone_number"] + service_branch = USER_FIELDS["service_branch"] + citizenship = USER_FIELDS["citizenship"] + designation = USER_FIELDS["designation"] + date_latest_training = USER_FIELDS["date_latest_training"] diff --git a/atst/forms/new_request.py b/atst/forms/new_request.py index 08d2fee4..92c04d78 100644 --- a/atst/forms/new_request.py +++ b/atst/forms/new_request.py @@ -5,6 +5,7 @@ from wtforms.validators import Email, Length, Optional, InputRequired, DataRequi from .fields import SelectField from .forms import ValidatedForm +from .edit_user import USER_FIELDS from .data import ( SERVICE_BRANCHES, ASSISTANCE_ORG_TYPES, @@ -161,58 +162,35 @@ class DetailsOfUseForm(ValidatedForm): ) -class InformationAboutYouForm(ValidatedForm): - fname_request = StringField("First Name", validators=[InputRequired(), Alphabet()]) +def inherit_field(unbound_field, append_validators=[]): + if "validators" in unbound_field.kwargs: + unbound_field.kwargs["validators"] += append_validators + return unbound_field.field_class(*unbound_field.args, **unbound_field.kwargs) - lname_request = StringField("Last Name", validators=[InputRequired(), Alphabet()]) + +class InformationAboutYouForm(ValidatedForm): + fname_request = USER_FIELDS["first_name"] + + lname_request = USER_FIELDS["last_name"] email_request = EmailField("E-mail Address", validators=[InputRequired(), Email()]) - phone_number = TelField( - "Phone Number", - description="Enter a 10-digit phone number", - validators=[InputRequired(), PhoneNumber()], + phone_number = inherit_field( + USER_FIELDS["phone_number"], append_validators=[InputRequired()] ) - service_branch = SelectField( - "Service Branch or Agency", - description="Which service or organization do you belong to within the DoD?", - choices=SERVICE_BRANCHES, + service_branch = USER_FIELDS["service_branch"] + + citizenship = inherit_field( + USER_FIELDS["citizenship"], append_validators=[InputRequired()] ) - citizenship = RadioField( - description="What is your citizenship status?", - choices=[ - ("United States", "United States"), - ("Foreign National", "Foreign National"), - ("Other", "Other"), - ], - validators=[InputRequired()], + designation = inherit_field( + USER_FIELDS["designation"], append_validators=[InputRequired()] ) - designation = RadioField( - "Designation of Person", - description="What is your designation within the DoD?", - choices=[ - ("military", "Military"), - ("civilian", "Civilian"), - ("contractor", "Contractor"), - ], - validators=[InputRequired()], - ) - - date_latest_training = DateField( - "Latest Information Assurance (IA) Training Completion Date", - description='To complete the training, you can find it in Information Assurance Cyber Awareness Challange website.', - validators=[ - InputRequired(), - DateRange( - lower_bound=pendulum.duration(years=1), - upper_bound=pendulum.duration(days=0), - message="Must be a date within the last year.", - ), - ], - format="%m/%d/%Y", + date_latest_training = inherit_field( + USER_FIELDS["date_latest_training"], append_validators=[InputRequired()] ) From a7ef93dad406681093743e97d96f3c95581ee097 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 16:31:50 -0400 Subject: [PATCH 10/15] date range validator should not try to evaluate empty dates --- atst/forms/validators.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atst/forms/validators.py b/atst/forms/validators.py index 9b094d1c..142d55d1 100644 --- a/atst/forms/validators.py +++ b/atst/forms/validators.py @@ -6,6 +6,9 @@ from datetime import datetime def DateRange(lower_bound=None, upper_bound=None, message=None): def _date_range(form, field): + if field.data is None: + return + now = pendulum.now().date() if isinstance(field.data, str): From d60b798af01f45a30e47a73e13984f53b00b94d8 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 16:32:49 -0400 Subject: [PATCH 11/15] fix optional and required validations in user forms --- atst/forms/edit_user.py | 40 +++++++++++++++++++++++++++------------ atst/forms/new_request.py | 34 ++++++++++----------------------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index 017a9bbc..a1b3763c 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -1,7 +1,8 @@ import pendulum +from copy import deepcopy from wtforms.fields.html5 import DateField, EmailField, TelField from wtforms.fields import RadioField, StringField -from wtforms.validators import Email, Required +from wtforms.validators import Email, Required, Optional from .fields import SelectField from .forms import ValidatedForm @@ -10,12 +11,12 @@ from .data import SERVICE_BRANCHES from .validators import Alphabet, DateRange, PhoneNumber USER_FIELDS = { - "first_name": StringField("First Name", validators=[Required(), Alphabet()]), - "last_name": StringField("Last Name", validators=[Required(), Alphabet()]), + "first_name": StringField("First Name", validators=[Alphabet()]), + "last_name": StringField("Last Name", validators=[Alphabet()]), "email": EmailField( "E-mail Address", description="Enter your preferred contact e-mail address", - validators=[Required(), Email()], + validators=[Email()], ), "phone_number": TelField( "Phone Number", @@ -59,12 +60,27 @@ USER_FIELDS = { } +def inherit_field(unbound_field, required=True): + kwargs = deepcopy(unbound_field.kwargs) + if not "validators" in kwargs: + kwargs["validators"] = [] + + if required: + kwargs["validators"].append(Required()) + else: + kwargs["validators"].append(Optional()) + + return unbound_field.field_class(*unbound_field.args, **kwargs) + + class EditUserForm(ValidatedForm): - first_name = USER_FIELDS["first_name"] - last_name = USER_FIELDS["last_name"] - email = USER_FIELDS["email"] - phone_number = USER_FIELDS["phone_number"] - service_branch = USER_FIELDS["service_branch"] - citizenship = USER_FIELDS["citizenship"] - designation = USER_FIELDS["designation"] - date_latest_training = USER_FIELDS["date_latest_training"] + first_name = inherit_field(USER_FIELDS["first_name"]) + last_name = inherit_field(USER_FIELDS["last_name"]) + email = inherit_field(USER_FIELDS["email"]) + phone_number = inherit_field(USER_FIELDS["phone_number"], required=False) + service_branch = inherit_field(USER_FIELDS["service_branch"], required=False) + citizenship = inherit_field(USER_FIELDS["citizenship"], required=False) + designation = inherit_field(USER_FIELDS["designation"], required=False) + date_latest_training = inherit_field( + USER_FIELDS["date_latest_training"], required=False + ) diff --git a/atst/forms/new_request.py b/atst/forms/new_request.py index 92c04d78..8c96ab4a 100644 --- a/atst/forms/new_request.py +++ b/atst/forms/new_request.py @@ -1,18 +1,18 @@ import pendulum -from wtforms.fields.html5 import DateField, EmailField, IntegerField, TelField +from wtforms.fields.html5 import DateField, EmailField, IntegerField from wtforms.fields import BooleanField, RadioField, StringField, TextAreaField from wtforms.validators import Email, Length, Optional, InputRequired, DataRequired from .fields import SelectField from .forms import ValidatedForm -from .edit_user import USER_FIELDS +from .edit_user import USER_FIELDS, inherit_field from .data import ( SERVICE_BRANCHES, ASSISTANCE_ORG_TYPES, DATA_TRANSFER_AMOUNTS, COMPLETION_DATE_RANGES, ) -from .validators import Alphabet, DateRange, PhoneNumber, IsNumber +from .validators import DateRange, IsNumber from atst.domain.requests import Requests @@ -162,36 +162,22 @@ class DetailsOfUseForm(ValidatedForm): ) -def inherit_field(unbound_field, append_validators=[]): - if "validators" in unbound_field.kwargs: - unbound_field.kwargs["validators"] += append_validators - return unbound_field.field_class(*unbound_field.args, **unbound_field.kwargs) - - class InformationAboutYouForm(ValidatedForm): - fname_request = USER_FIELDS["first_name"] + fname_request = inherit_field(USER_FIELDS["first_name"]) - lname_request = USER_FIELDS["last_name"] + lname_request = inherit_field(USER_FIELDS["last_name"]) email_request = EmailField("E-mail Address", validators=[InputRequired(), Email()]) - phone_number = inherit_field( - USER_FIELDS["phone_number"], append_validators=[InputRequired()] - ) + phone_number = inherit_field(USER_FIELDS["phone_number"]) - service_branch = USER_FIELDS["service_branch"] + service_branch = inherit_field(USER_FIELDS["service_branch"]) - citizenship = inherit_field( - USER_FIELDS["citizenship"], append_validators=[InputRequired()] - ) + citizenship = inherit_field(USER_FIELDS["citizenship"]) - designation = inherit_field( - USER_FIELDS["designation"], append_validators=[InputRequired()] - ) + designation = inherit_field(USER_FIELDS["designation"]) - date_latest_training = inherit_field( - USER_FIELDS["date_latest_training"], append_validators=[InputRequired()] - ) + date_latest_training = inherit_field(USER_FIELDS["date_latest_training"]) class WorkspaceOwnerForm(ValidatedForm): From f04f52c3d9b1d2e287c2fc716b659c49b586f5a2 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 16:34:45 -0400 Subject: [PATCH 12/15] add error alert for validation errors on user edit page --- templates/user/edit.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/templates/user/edit.html b/templates/user/edit.html index 77afdd62..8d87f1fc 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -4,6 +4,13 @@ {% block content %}
+ {% if form.errors %} + {{ Alert('There were some errors', + message="

Please see below.

", + level='error' + ) }} + {% endif %} + {% if updated %} {{ Alert('User information updated.', level='success') }} {% endif %} From d25ee2fe99676f22a3b4c3cff516bddd6b2759c8 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 16 Oct 2018 16:37:32 -0400 Subject: [PATCH 13/15] more explicit route name form user form action --- templates/user/edit.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/user/edit.html b/templates/user/edit.html index 8d87f1fc..6578c969 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -25,7 +25,7 @@

- {% set form_action = url_for('users.user') %} + {% set form_action = url_for('users.update_user') %} {% include "fragments/edit_user_form.html" %}
From 771bd27794a4c0a6e6e862dd7a22ca45049b5d1d Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 17 Oct 2018 10:47:30 -0400 Subject: [PATCH 14/15] use post endpoint for updating user in test --- tests/routes/test_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/routes/test_users.py b/tests/routes/test_users.py index d9615d5f..74fbdc44 100644 --- a/tests/routes/test_users.py +++ b/tests/routes/test_users.py @@ -19,7 +19,7 @@ def test_user_can_update_profile(user_session, client): new_data["date_latest_training"] = new_data["date_latest_training"].strftime( "%m/%d/%Y" ) - client.post(url_for("users.user"), data=new_data) + client.post(url_for("users.update_user"), data=new_data) updated_user = Users.get_by_dod_id(user.dod_id) assert updated_user.first_name == "chad" assert updated_user.last_name == "vader" From 92dc4f45c9c8f3644027d3e46654d606efbfee3e Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 17 Oct 2018 11:09:13 -0400 Subject: [PATCH 15/15] use empty string for null service branch selection --- atst/forms/data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/forms/data.py b/atst/forms/data.py index 06db4ed8..89ad9e8b 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,7 +1,7 @@ from atst.domain.roles import WORKSPACE_ROLES as WORKSPACE_ROLE_DEFINITIONS SERVICE_BRANCHES = [ - (None, "Select an option"), + ("", "Select an option"), ("Air Force, Department of the", "Air Force, Department of the"), ("Army and Air Force Exchange Service", "Army and Air Force Exchange Service"), ("Army, Department of the", "Army, Department of the"),