diff --git a/atst/domain/auth.py b/atst/domain/auth.py index a60a503a..10db8273 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -7,6 +7,7 @@ UNPROTECTED_ROUTES = [ "atst.root", "dev.login_dev", "atst.login_redirect", + "atst.logout", "atst.unauthorized", "atst.helpdocs", "static", @@ -21,10 +22,26 @@ def apply_authentication(app): user = get_current_user() if user: g.current_user = user + if should_redirect_to_user_profile(request, user): + return redirect(url_for("users.user", next=request.path)) elif not _unprotected_route(request): return redirect(url_for("atst.root", next=request.path)) +def should_redirect_to_user_profile(request, user): + has_complete_profile = user.profile_complete + is_unprotected_route = _unprotected_route(request) + is_requesting_user_endpoint = request.endpoint in [ + "users.user", + "users.update_user", + ] + + if has_complete_profile or is_unprotected_route or is_requesting_user_endpoint: + return False + + return True + + def get_current_user(): user_id = session.get("user_id") if user_id: diff --git a/atst/forms/edit_user.py b/atst/forms/edit_user.py index 651c3c15..3c337e0b 100644 --- a/atst/forms/edit_user.py +++ b/atst/forms/edit_user.py @@ -2,14 +2,16 @@ 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, Optional +from wtforms.validators import Email, DataRequired, Optional from .fields import SelectField from .forms import ValidatedForm from .data import SERVICE_BRANCHES +from atst.models.user import User from .validators import Name, DateRange, PhoneNumber + USER_FIELDS = { "first_name": StringField("First Name", validators=[Name()]), "last_name": StringField("Last Name", validators=[Name()]), @@ -66,21 +68,25 @@ def inherit_field(unbound_field, required=True): kwargs["validators"] = [] if required: - kwargs["validators"].append(Required()) + kwargs["validators"].append(DataRequired()) else: kwargs["validators"].append(Optional()) return unbound_field.field_class(*unbound_field.args, **kwargs) +def inherit_user_field(field_name): + required = field_name in User.REQUIRED_FIELDS + return inherit_field(USER_FIELDS[field_name], required=required) + + 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 - ) + + first_name = inherit_user_field("first_name") + last_name = inherit_user_field("last_name") + email = inherit_user_field("email") + phone_number = inherit_user_field("phone_number") + service_branch = inherit_user_field("service_branch") + citizenship = inherit_user_field("citizenship") + designation = inherit_user_field("designation") + date_latest_training = inherit_user_field("date_latest_training") diff --git a/atst/forms/new_request.py b/atst/forms/new_request.py index 8c96ab4a..c1b3e14d 100644 --- a/atst/forms/new_request.py +++ b/atst/forms/new_request.py @@ -164,19 +164,12 @@ class DetailsOfUseForm(ValidatedForm): class InformationAboutYouForm(ValidatedForm): fname_request = inherit_field(USER_FIELDS["first_name"]) - lname_request = inherit_field(USER_FIELDS["last_name"]) - - email_request = EmailField("E-mail Address", validators=[InputRequired(), Email()]) - + email_request = inherit_field(USER_FIELDS["email"]) phone_number = inherit_field(USER_FIELDS["phone_number"]) - service_branch = inherit_field(USER_FIELDS["service_branch"]) - citizenship = inherit_field(USER_FIELDS["citizenship"]) - designation = inherit_field(USER_FIELDS["designation"]) - date_latest_training = inherit_field(USER_FIELDS["date_latest_training"]) diff --git a/atst/models/user.py b/atst/models/user.py index 5f8765dd..14d8480f 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -28,6 +28,27 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): provisional = Column(Boolean) + REQUIRED_FIELDS = [ + "email", + "dod_id", + "first_name", + "last_name", + "phone_number", + "service_branch", + "citizenship", + "designation", + "date_latest_training", + ] + + @property + def profile_complete(self): + return all( + [ + getattr(self, field_name) is not None + for field_name in self.REQUIRED_FIELDS + ] + ) + @property def atat_permissions(self): return self.atat_role.permissions diff --git a/atst/routes/users.py b/atst/routes/users.py index e57584a7..f658b823 100644 --- a/atst/routes/users.py +++ b/atst/routes/users.py @@ -1,4 +1,4 @@ -from flask import Blueprint, render_template, g, request as http_request +from flask import Blueprint, render_template, g, request as http_request, redirect from atst.forms.edit_user import EditUserForm from atst.domain.users import Users @@ -10,16 +10,21 @@ bp = Blueprint("users", __name__) def user(): user = g.current_user form = EditUserForm(data=user.to_dictionary()) - return render_template("user/edit.html", form=form, user=user) + return render_template( + "user/edit.html", next=http_request.args.get("next"), 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} + next_url = http_request.args.get("next") + rerender_args = {"form": form, "user": user, "next": next_url} if form.validate(): Users.update(user, form.data) rerender_args["updated"] = True + if next_url: + return redirect(next_url) return render_template("user/edit.html", **rerender_args) diff --git a/templates/user/edit.html b/templates/user/edit.html index 6578c969..bd247a2f 100644 --- a/templates/user/edit.html +++ b/templates/user/edit.html @@ -4,6 +4,13 @@ {% block content %}
+ {% if next is not none %} + {{ Alert('You must complete your profile', + message='

Before continuing, you must complete your profile

', + level='info' + ) }} + {% endif %} + {% if form.errors %} {{ Alert('There were some errors', message="

Please see below.

", @@ -25,7 +32,7 @@
- {% set form_action = url_for('users.update_user') %} + {% set form_action = url_for('users.update_user', next=next) %} {% include "fragments/edit_user_form.html" %} diff --git a/tests/forms/test_edit_user.py b/tests/forms/test_edit_user.py new file mode 100644 index 00000000..2522f264 --- /dev/null +++ b/tests/forms/test_edit_user.py @@ -0,0 +1,27 @@ +import pytest +from werkzeug.datastructures import ImmutableMultiDict + +from atst.forms.edit_user import EditUserForm + +from tests.factories import UserFactory + + +def test_edit_user_form_requires_all_fields(): + user = UserFactory.create() + user_data = user.to_dictionary() + del user_data["date_latest_training"] + form_data = ImmutableMultiDict(user_data) + form = EditUserForm(form_data) + assert not form.validate() + assert form.errors == {"date_latest_training": ["This field is required."]} + + +def test_edit_user_form_valid_with_all_fields(): + user = UserFactory.create() + user_data = user.to_dictionary() + user_data["date_latest_training"] = user_data["date_latest_training"].strftime( + "%m/%d/%Y" + ) + form_data = ImmutableMultiDict(user_data) + form = EditUserForm(form_data) + assert form.validate() diff --git a/tests/models/test_user.py b/tests/models/test_user.py new file mode 100644 index 00000000..6d5e1d13 --- /dev/null +++ b/tests/models/test_user.py @@ -0,0 +1,17 @@ +import pytest + +from atst.models.user import User + +from tests.factories import UserFactory + + +def test_profile_complete_with_all_info(): + user = UserFactory.create() + assert user.profile_complete + + +@pytest.mark.parametrize("missing_field", User.REQUIRED_FIELDS) +def test_profile_complete_with_missing_info(missing_field): + user = UserFactory.create() + setattr(user, missing_field, None) + assert not user.profile_complete diff --git a/tests/routes/test_auth.py b/tests/routes/test_auth.py new file mode 100644 index 00000000..aef34fbd --- /dev/null +++ b/tests/routes/test_auth.py @@ -0,0 +1,45 @@ +from flask import url_for +from urllib.parse import quote + +from tests.factories import UserFactory + + +def test_request_page_with_complete_profile(client, user_session): + user = UserFactory.create() + user_session(user) + response = client.get("/requests", follow_redirects=False) + assert response.status_code == 200 + + +def test_redirect_when_profile_missing_fields(client, user_session): + user = UserFactory.create(date_latest_training=None) + user_session(user) + requested_url = "/requests" + response = client.get(requested_url, follow_redirects=False) + assert response.status_code == 302 + assert "/user?next={}".format(quote(requested_url, safe="")) in response.location + + +def test_unprotected_route_with_incomplete_profile(client, user_session): + user = UserFactory.create(date_latest_training=None) + user_session(user) + response = client.get("/about", follow_redirects=False) + assert response.status_code == 200 + + +def test_completing_user_profile(client, user_session): + user = UserFactory.create(phone_number=None) + user_session(user) + response = client.get("/requests", follow_redirects=True) + assert b"You must complete your profile" in response.data + + updated_data = {**user.to_dictionary(), "phone_number": "5558675309"} + updated_data["date_latest_training"] = updated_data[ + "date_latest_training" + ].strftime("%m/%d/%Y") + response = client.post(url_for("users.update_user"), data=updated_data) + assert response.status_code == 200 + + response = client.get("/requests", follow_redirects=False) + assert response.status_code == 200 + assert b"You must complete your profile" not in response.data diff --git a/tests/routes/test_users.py b/tests/routes/test_users.py index 74fbdc44..50a723a1 100644 --- a/tests/routes/test_users.py +++ b/tests/routes/test_users.py @@ -23,3 +23,21 @@ def test_user_can_update_profile(user_session, client): updated_user = Users.get_by_dod_id(user.dod_id) assert updated_user.first_name == "chad" assert updated_user.last_name == "vader" + + +def test_user_is_redirected_when_updating_profile(user_session, client): + user = UserFactory.create() + user_session(user) + next_url = "/requests" + + user_data = user.to_dictionary() + user_data["date_latest_training"] = user_data["date_latest_training"].strftime( + "%m/%d/%Y" + ) + response = client.post( + url_for("users.update_user", next=next_url), + data=user_data, + follow_redirects=False, + ) + assert response.status_code == 302 + assert response.location.endswith(next_url)