Merge pull request #386 from dod-ccpo/edit-user-#160268937

Edit user #160268937

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 ###

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)
 

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,29 @@ 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):
+        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)
+
+        db.session.add(user)
+        db.session.commit()
+
+        return user

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"),

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
@@ -9,42 +10,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=[Alphabet()]),
+    "last_name": StringField("Last Name", validators=[Alphabet()]),
+    "email": EmailField(
         "E-mail Address",
         description="Enter your preferred contact e-mail address",
-        validators=[Required(), Email()],
-    )
-
-    phone_number = TelField(
+        validators=[Email()],
+    ),
+    "phone_number": TelField(
         "Phone Number",
         description="Enter your 10-digit U.S. phone number",
-        validators=[Required(), PhoneNumber()],
-    )
-
-    service_branch = SelectField(
+        validators=[PhoneNumber()],
+    ),
+    "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"),
         ],
-        validators=[Required()],
-    )
-
-    designation = RadioField(
+    ),
+    "designation": RadioField(
         "Designation of Person",
         description="What is your designation within the DoD?",
         choices=[
@@ -52,19 +44,43 @@ class EditUserForm(ValidatedForm):
             ("civilian", "Civilian"),
             ("contractor", "Contractor"),
         ],
-        validators=[Required()],
-    )
-
-    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 <a class="icon-link" href="https://iatraining.disa.mil/eta/disa_cac2018/launchPage.htm" target="_blank">Information Assurance Cyber Awareness Challange</a> website.',
         validators=[
-            Required(),
             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",
+    ),
+}
+
+
+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 = 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
     )

atst/forms/new_request.py

@@ -1,17 +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, 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,58 +163,21 @@ class DetailsOfUseForm(ValidatedForm):
 
 
 class InformationAboutYouForm(ValidatedForm):
-    fname_request = StringField("First Name", validators=[InputRequired(), Alphabet()])
+    fname_request = inherit_field(USER_FIELDS["first_name"])
 
-    lname_request = StringField("Last Name", validators=[InputRequired(), Alphabet()])
+    lname_request = inherit_field(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"])
 
-    service_branch = SelectField(
-        "Service Branch or Agency",
-        description="Which service or organization do you belong to within the DoD?",
-        choices=SERVICE_BRANCHES,
-    )
+    service_branch = inherit_field(USER_FIELDS["service_branch"])
 
-    citizenship = RadioField(
-        description="What is your citizenship status?",
-        choices=[
-            ("United States", "United States"),
-            ("Foreign National", "Foreign National"),
-            ("Other", "Other"),
-        ],
-        validators=[InputRequired()],
-    )
+    citizenship = inherit_field(USER_FIELDS["citizenship"])
 
-    designation = RadioField(
-        "Designation of Person",
-        description="What is your designation within the DoD?",
-        choices=[
-            ("military", "Military"),
-            ("civilian", "Civilian"),
-            ("contractor", "Contractor"),
-        ],
-        validators=[InputRequired()],
-    )
+    designation = inherit_field(USER_FIELDS["designation"])
 
-    date_latest_training = DateField(
-        "Latest Information Assurance (IA) Training Completion Date",
-        description='To complete the training, you can find it in <a class="icon-link" href="https://iatraining.disa.mil/eta/disa_cac2018/launchPage.htm" target="_blank">Information Assurance Cyber Awareness Challange</a> 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"])
 
 
 class WorkspaceOwnerForm(ValidatedForm):

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):

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):
@@ -52,3 +57,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"]
+        }

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")

atst/routes/users.py

@@ -0,0 +1,25 @@
+from flask import Blueprint, render_template, g, request as http_request
+from atst.forms.edit_user import EditUserForm
+from atst.domain.users import Users
+
+
+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():
+    user = g.current_user
+    form = EditUserForm(http_request.form)
+    rerender_args = {"form": form, "user": user}
+    if form.validate():
+        Users.update(user, form.data)
+        rerender_args["updated"] = True
+
+    return render_template("user/edit.html", **rerender_args)

templates/base_public.html

@@ -26,7 +26,7 @@
       </a>
 
       {% if g.current_user %}
-        <a href="{{ url_for('atst.user') }}" class="topbar__link">
+        <a href="{{ url_for('users.user') }}" class="topbar__link">
           <span class="topbar__link-label">{{ g.current_user.first_name + " " + g.current_user.last_name }}</span>
           {{ Icon('avatar', classes='topbar__link-icon') }}
         </a>

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 action='{{ form_action }}'>
+<form method="POST" action='{{ form_action }}'>
+  {{ form.csrf_token }}
   <div class='panel'>
     <div class='panel__content'>
       <div class='form-row'>

templates/navigation/topbar.html

@@ -20,7 +20,7 @@
         </a>
       {% endif %}
 
-      <a href="{{ url_for('atst.user') }}" class="topbar__link">
+      <a href="{{ url_for('users.user') }}" class="topbar__link">
         <span class="topbar__link-label">{{ g.current_user.first_name + " " + g.current_user.last_name }}</span>
         {{ Icon('avatar', classes='topbar__link-icon') }}
       </a>

templates/user/edit.html

@@ -3,21 +3,29 @@
 
 {% block content %}
 <div class='col'>
-  {{ Alert('This form does not yet function',
-    message="<p>Functionality of this form is pending more engineering work. Engineers, please remove this alert when done.</p>",
-    level='warning'
+
+  {% if form.errors %}
+    {{ Alert('There were some errors',
+      message="<p>Please see below.</p>",
+      level='error'
     ) }}
+  {% endif %}
+
+  {% if updated %}
+    {{ Alert('User information updated.', level='success') }}
+  {% endif %}
 
   <div class='panel'>
     <div class='panel__heading'>
       <h1>
         <div class='h2'>{{ user.first_name }} {{ user.last_name }}</div>
+        <div class='h3'>DOD ID: {{ user.dod_id }}</div>
         <div class='subtitle h3'>Edit user details</div>
       </h1>
     </div>
   </div>
 
-  {% set form_action = url_for('atst.save_user') %}
+  {% set form_action = url_for('users.update_user') %}
   {% include "fragments/edit_user_form.html" %}
 
 </div>

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,34 @@ 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) as excinfo:
+        Users.update(new_user, {"dod_id": "1234567890"})
+
+    assert "dod_id" in str(excinfo.value)

tests/factories.py

@@ -48,6 +48,18 @@ 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):

tests/routes/test_users.py

@@ -0,0 +1,25 @@
+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.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"