From 8420a783927f3f51dbc81465fd9178aa85600474 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 6 Aug 2019 14:47:36 -0400 Subject: [PATCH 01/16] Add CREATE_CCPO_USER permission, create context processor for ATAT so user_can and permissions can be used in the template, add placeholder button for adding new CCPO user --- atst/domain/permission_sets.py | 1 + atst/models/permissions.py | 1 + atst/routes/__init__.py | 2 ++ atst/utils/context_processors.py | 7 +++++ templates/ccpo/users.html | 51 +++++++++++++++++++------------- 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index ab55feac..727b34df 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -64,6 +64,7 @@ ATAT_PERMISSION_SETS = [ "description": "", "permissions": [ Permissions.VIEW_CCPO_USER, + Permissions.CREATE_CCPO_USER, Permissions.EDIT_CCPO_USER, Permissions.DELETE_CCPO_USER, ], diff --git a/atst/models/permissions.py b/atst/models/permissions.py index a7d735b8..f9e73046 100644 --- a/atst/models/permissions.py +++ b/atst/models/permissions.py @@ -2,6 +2,7 @@ class Permissions(object): # ccpo permissions VIEW_AUDIT_LOG = "view_audit_log" VIEW_CCPO_USER = "view_ccpo_user" + CREATE_CCPO_USER = "create_ccpo_user" EDIT_CCPO_USER = "edit_ccpo_user" DELETE_CCPO_USER = "delete_ccpo_user" diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 0729d878..7860eb74 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -24,10 +24,12 @@ from atst.domain.common import Paginator from atst.domain.portfolios import Portfolios from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.permissions import Permissions +from atst.utils.context_processors import atat as atat_context_processor from atst.utils.flash import formatted_flash as flash bp = Blueprint("atst", __name__) +bp.context_processor(atat_context_processor) @bp.route("/") diff --git a/atst/utils/context_processors.py b/atst/utils/context_processors.py index 1e8d619a..09423204 100644 --- a/atst/utils/context_processors.py +++ b/atst/utils/context_processors.py @@ -119,3 +119,10 @@ def portfolio(): "funding_end_date": funding_end_date, "funded": funded, } + + +def atat(): + return { + "permissions": Permissions, + "user_can": user_can_view, + } diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index f3d545de..7a9bd4bc 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -1,27 +1,36 @@ {% extends "base.html" %} +{% from "components/icon.html" import Icon %} + {% block content %} -
-
- CCPO Users -
- - - - - - - - - - {% for user in users %} +
+
+ CCPO Users +
+
NameEmailDoD ID
+ - - - + + + - {% endfor %} - -
{{ user.full_name }}{{ user.email }}{{ user.dod_id }}NameEmailDoD ID
-
+ + + {% for user in users %} + + {{ user.full_name }} + {{ user.email }} + {{ user.dod_id }} + + {% endfor %} + + + + + {% if user_can(permissions.CREATE_CCPO_USER) %} + + Add new CCPO user {{ Icon("plus") }} + + {% endif %} + {% endblock %} From 01ba73ef42d967592bdb870ba4eb9757a50da9ab Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 7 Aug 2019 14:53:30 -0400 Subject: [PATCH 02/16] Add in templates for ccpo user form and sketch out route functions --- atst/forms/ccpo_user.py | 13 ++++++++++++ atst/routes/__init__.py | 31 ++++++++++++++++++++++++++++ templates/ccpo/add_user.html | 27 ++++++++++++++++++++++++ templates/ccpo/confirm_user.html | 35 ++++++++++++++++++++++++++++++++ templates/ccpo/users.html | 2 +- 5 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 atst/forms/ccpo_user.py create mode 100644 templates/ccpo/add_user.html create mode 100644 templates/ccpo/confirm_user.html diff --git a/atst/forms/ccpo_user.py b/atst/forms/ccpo_user.py new file mode 100644 index 00000000..3ef23257 --- /dev/null +++ b/atst/forms/ccpo_user.py @@ -0,0 +1,13 @@ +from flask_wtf import FlaskForm +from wtforms.validators import Required, Length +from wtforms.fields import StringField + +from atst.forms.validators import IsNumber +from atst.utils.localization import translate + + +class CCPOUserForm(FlaskForm): + dod_id = StringField( + translate("forms.new_member.dod_id_label"), + validators=[Required(), Length(min=10), IsNumber()], + ) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 7860eb74..f0e05aec 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -21,8 +21,10 @@ from atst.domain.authnid import AuthenticationContext from atst.domain.audit_log import AuditLog from atst.domain.auth import logout as _logout from atst.domain.common import Paginator +from atst.domain.exceptions import NotFoundError from atst.domain.portfolios import Portfolios from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.forms.ccpo_user import CCPOUserForm from atst.models.permissions import Permissions from atst.utils.context_processors import atat as atat_context_processor from atst.utils.flash import formatted_flash as flash @@ -141,6 +143,35 @@ def ccpo_users(): return render_template("ccpo/users.html", users=users) +@bp.route("/ccpo-users/new") +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def add_new_ccpo_user(): + form = CCPOUserForm() + return render_template("ccpo/add_user.html", form=form) + + +@bp.route("/ccpo-users/new", methods=["POST"]) +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def submit_add_new_ccpo_user(): + try: + new_user = Users.get_by_dod_id(request.form['dod_id']) + form = CCPOUserForm(obj=new_user) + except NotFoundError: + new_user = None + form = CCPOUserForm() + + return render_template("ccpo/confirm_user.html", new_user=new_user, form=form) + + +@bp.route("/ccpo-users/confirm-new", methods=["POST"]) +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def confirm_new_ccpo_user(): + new_user = Users.get_by_dod_id(request.form['dod_id']) + # give new perms here + # flash w/ success message + return redirect(url_for("atst.ccpo_users")) + + @bp.route("/about") def about(): return render_template("about.html") diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html new file mode 100644 index 00000000..5647e144 --- /dev/null +++ b/templates/ccpo/add_user.html @@ -0,0 +1,27 @@ +{% extends "base.html" %} + +{% from "components/text_input.html" import TextInput %} + +{% block content %} + {% set ccpo_user_form = "add-ccpo-user-form" %} + +
+ {{ form.csrf_token }} +
+

Add new CCPO user

+
+
+
+ {{ TextInput(form.dod_id, validation='dodId') }} +
+
+
+ + {{ "common.cancel" | translate }} +
+
+{% endblock %} diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html new file mode 100644 index 00000000..eae94fec --- /dev/null +++ b/templates/ccpo/confirm_user.html @@ -0,0 +1,35 @@ +{% extends "base.html" %} + +{% from "components/alert.html" import Alert %} +{% from "components/text_input.html" import TextInput %} + +{% block content %} + {% set ccpo_user_form = "add-ccpo-user-form" %} + + {% call Alert('Confirm new CCPO user') %} + +
+ {{ form.csrf_token }} + +
+

+ Please confirm that the user details below match the user being given CCPO permissions. +

+

+ {{ new_user.full_name }} +

+

+ {{ new_user.email }} +

+
+
+ + {{ "common.cancel" | translate }} +
+
+ {% endcall %} +{% endblock %} diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 7a9bd4bc..df2b7324 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -28,7 +28,7 @@ {% if user_can(permissions.CREATE_CCPO_USER) %} - + Add new CCPO user {{ Icon("plus") }} {% endif %} From 811a498bf4c0285271cb20d31eb1785ad038b158 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 7 Aug 2019 16:25:10 -0400 Subject: [PATCH 03/16] Finish route for confirming CCPO user, add function to update user's ccpo perms, and add success flash message to template --- atst/domain/users.py | 12 ++++++++++++ atst/routes/__init__.py | 8 ++++---- atst/utils/context_processors.py | 5 +---- atst/utils/flash.py | 5 +++++ templates/ccpo/users.html | 3 +++ 5 files changed, 25 insertions(+), 8 deletions(-) diff --git a/atst/domain/users.py b/atst/domain/users.py index a75a1f28..8fa0ae91 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -87,6 +87,18 @@ class Users(object): return user + @classmethod + def update_ccpo_permissions(cls, user, add_perms=False): + if add_perms: + permission_sets = PermissionSets.get_all() + else: + permission_sets = [] + + user.permission_sets = permission_sets + db.session.add(user) + db.session.commit() + return user + @classmethod def update_last_login(cls, user): user.last_login = datetime.now() diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index f0e05aec..034d9956 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -154,7 +154,7 @@ def add_new_ccpo_user(): @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") def submit_add_new_ccpo_user(): try: - new_user = Users.get_by_dod_id(request.form['dod_id']) + new_user = Users.get_by_dod_id(request.form["dod_id"]) form = CCPOUserForm(obj=new_user) except NotFoundError: new_user = None @@ -166,9 +166,9 @@ def submit_add_new_ccpo_user(): @bp.route("/ccpo-users/confirm-new", methods=["POST"]) @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") def confirm_new_ccpo_user(): - new_user = Users.get_by_dod_id(request.form['dod_id']) - # give new perms here - # flash w/ success message + user = Users.get_by_dod_id(request.form["dod_id"]) + Users.update_ccpo_permissions(user, add_perms=True) + flash("ccpo_user_added", user_name=user.full_name) return redirect(url_for("atst.ccpo_users")) diff --git a/atst/utils/context_processors.py b/atst/utils/context_processors.py index 09423204..7d39b367 100644 --- a/atst/utils/context_processors.py +++ b/atst/utils/context_processors.py @@ -122,7 +122,4 @@ def portfolio(): def atat(): - return { - "permissions": Permissions, - "user_can": user_can_view, - } + return {"permissions": Permissions, "user_can": user_can_view} diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 0ea62028..e7fc72d7 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -30,6 +30,11 @@ MESSAGES = { "message_template": "You have successfully deleted {{ user_name }} from {{ application_name }}", "category": "success", }, + "ccpo_user_added": { + "title_template": translate("flash.success"), + "message_template": "You have successfully given {{ user_name }} CCPO permissions.", + "category": "success", + }, "environment_added": { "title_template": translate("flash.success"), "message_template": """ diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index df2b7324..7e29a1d6 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -7,6 +7,9 @@
CCPO Users
+ + {% include "fragments/flash.html" %} + From 9f54501df4a20dba53f2d02de1ff9c77c0cb258e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 7 Aug 2019 16:37:15 -0400 Subject: [PATCH 04/16] Add in alert for when user isn't found --- templates/ccpo/confirm_user.html | 61 +++++++++++++++++++------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index eae94fec..d9cb610f 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -4,32 +4,45 @@ {% from "components/text_input.html" import TextInput %} {% block content %} - {% set ccpo_user_form = "add-ccpo-user-form" %} + {% if new_user %} + {% set ccpo_user_form = "add-ccpo-user-form" %} - {% call Alert('Confirm new CCPO user') %} + {% call Alert('Confirm new CCPO user') %} -
- {{ form.csrf_token }} - + + {{ form.csrf_token }} + +
+

+ Please confirm that the user details below match the user being given CCPO permissions. +

+

+ {{ new_user.full_name }} +

+

+ {{ new_user.email }} +

+
+
+ + {{ "common.cancel" | translate }} +
+ + {% endcall %} + {% else %} + {% call Alert('User not found') %}
-

- Please confirm that the user details below match the user being given CCPO permissions. -

-

- {{ new_user.full_name }} -

-

- {{ new_user.email }} -

+ To add someone as a CCPO user, they must already have an ATAT account. +
+ + Return to list of CCPO users + +
-
- - {{ "common.cancel" | translate }} -
- - {% endcall %} + {% endcall %} + {% endif %} {% endblock %} From 6b15b41d560dda7a0fa972f4706c346a1f702ef9 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 10:00:20 -0400 Subject: [PATCH 05/16] Formatting and styling of form --- templates/ccpo/add_user.html | 28 +++++++++++++--------------- templates/ccpo/confirm_user.html | 17 ++++++----------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html index 5647e144..505ab20b 100644 --- a/templates/ccpo/add_user.html +++ b/templates/ccpo/add_user.html @@ -3,25 +3,23 @@ {% from "components/text_input.html" import TextInput %} {% block content %} - {% set ccpo_user_form = "add-ccpo-user-form" %} - -
+ {{ form.csrf_token }} -
-

Add new CCPO user

-
+

Add new CCPO user

-
+
{{ TextInput(form.dod_id, validation='dodId') }}
-
-
- - {{ "common.cancel" | translate }} +
+
+ + {{ "common.cancel" | translate }} +
+
{% endblock %} diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index d9cb610f..2012fc0f 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -5,11 +5,8 @@ {% block content %} {% if new_user %} - {% set ccpo_user_form = "add-ccpo-user-form" %} - {% call Alert('Confirm new CCPO user') %} - -
+ {{ form.csrf_token }}
@@ -35,13 +32,11 @@ {% endcall %} {% else %} {% call Alert('User not found') %} -
- To add someone as a CCPO user, they must already have an ATAT account. -
- - Return to list of CCPO users - -
+ To add someone as a CCPO user, they must already have an ATAT account. +
+ + Return to list of CCPO users +
{% endcall %} {% endif %} From 5b008e044a2c17364f1eab6cce36ceea1019551e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 10:33:27 -0400 Subject: [PATCH 06/16] Add tests --- tests/domain/test_users.py | 12 ++++++++++++ tests/test_access.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index fda69a12..08f75412 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -85,3 +85,15 @@ def test_get_ccpo_users(): assert ccpo_1 in ccpo_users assert ccpo_2 in ccpo_users assert rando not in ccpo_users + + +def test_update_ccpo_permissions(): + rando = UserFactory.create() + Users.update_ccpo_permissions(rando, add_perms=True) + + ccpo = UserFactory.create_ccpo() + Users.update_ccpo_permissions(ccpo) + + ccpo_users = Users.get_ccpo_users() + assert rando in ccpo_users + assert ccpo not in ccpo_users diff --git a/tests/test_access.py b/tests/test_access.py index 141fed18..29e3a796 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -130,6 +130,37 @@ def test_atst_ccpo_users_access(get_url_assert_status): get_url_assert_status(rando, url, 404) +# atst.add_new_ccpo_user +def test_atst_add_new_ccpo_user_access(get_url_assert_status): + ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) + rando = user_with() + + url = url_for("atst.add_new_ccpo_user") + get_url_assert_status(ccpo, url, 200) + get_url_assert_status(rando, url, 404) + + +# atst.submit_add_new_ccpo_user +def test_atst_submit_add_new_ccpo_user_access(post_url_assert_status): + ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) + rando = user_with() + + url = url_for("atst.submit_add_new_ccpo_user") + post_url_assert_status(ccpo, url, 200, data={"dod_id": "1234567890"}) + post_url_assert_status(rando, url, 404, data={"dod_id": "1234567890"}) + + +# atst.confirm_new_ccpo_user +def test_atst_confirm_new_ccpo_user_access(post_url_assert_status): + ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) + rando = user_with() + user = UserFactory.create() + + url = url_for("atst.confirm_new_ccpo_user") + post_url_assert_status(ccpo, url, 302, data={"dod_id": user.dod_id}) + post_url_assert_status(rando, url, 404, data={"dod_id": user.dod_id}) + + # applications.access_environment def test_applications_access_environment_access(get_url_assert_status): dev = UserFactory.create() From 1f8337485fb95e304aea4d491e9e15c29db0588f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 10:52:34 -0400 Subject: [PATCH 07/16] Move ccpo routes into their own file --- atst/app.py | 2 ++ atst/routes/__init__.py | 44 ----------------------- atst/routes/ccpo.py | 58 ++++++++++++++++++++++++++++++ templates/audit_log/audit_log.html | 2 +- templates/ccpo/add_user.html | 4 +-- templates/ccpo/confirm_user.html | 6 ++-- templates/ccpo/users.html | 2 +- tests/test_access.py | 28 +++++++-------- 8 files changed, 81 insertions(+), 65 deletions(-) create mode 100644 atst/routes/ccpo.py diff --git a/atst/app.py b/atst/app.py index 163a7bae..4f7bdbff 100644 --- a/atst/app.py +++ b/atst/app.py @@ -18,6 +18,7 @@ from atst.routes.applications import applications_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.routes.ccpo import bp as ccpo_routes from atst.domain.authnid.crl import CRLCache, NoOpCRLCache from atst.domain.auth import apply_authentication from atst.domain.authz import Authorization @@ -78,6 +79,7 @@ def make_app(config): app.register_blueprint(task_orders_bp) app.register_blueprint(applications_bp) app.register_blueprint(user_routes) + app.register_blueprint(ccpo_routes) if ENV != "prod": app.register_blueprint(dev_routes) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 034d9956..cb39a0bb 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -128,50 +128,6 @@ def logout(): return response -@bp.route("/activity-history") -@user_can(Permissions.VIEW_AUDIT_LOG, message="view activity log") -def activity_history(): - pagination_opts = Paginator.get_pagination_opts(request) - audit_events = AuditLog.get_all_events(pagination_opts) - return render_template("audit_log/audit_log.html", audit_events=audit_events) - - -@bp.route("/ccpo-users") -@user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") -def ccpo_users(): - users = Users.get_ccpo_users() - return render_template("ccpo/users.html", users=users) - - -@bp.route("/ccpo-users/new") -@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def add_new_ccpo_user(): - form = CCPOUserForm() - return render_template("ccpo/add_user.html", form=form) - - -@bp.route("/ccpo-users/new", methods=["POST"]) -@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def submit_add_new_ccpo_user(): - try: - new_user = Users.get_by_dod_id(request.form["dod_id"]) - form = CCPOUserForm(obj=new_user) - except NotFoundError: - new_user = None - form = CCPOUserForm() - - return render_template("ccpo/confirm_user.html", new_user=new_user, form=form) - - -@bp.route("/ccpo-users/confirm-new", methods=["POST"]) -@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def confirm_new_ccpo_user(): - user = Users.get_by_dod_id(request.form["dod_id"]) - Users.update_ccpo_permissions(user, add_perms=True) - flash("ccpo_user_added", user_name=user.full_name) - return redirect(url_for("atst.ccpo_users")) - - @bp.route("/about") def about(): return render_template("about.html") diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py new file mode 100644 index 00000000..f3e9d469 --- /dev/null +++ b/atst/routes/ccpo.py @@ -0,0 +1,58 @@ +from flask import Blueprint, render_template, redirect, url_for, request +from atst.domain.users import Users +from atst.domain.audit_log import AuditLog +from atst.domain.common import Paginator +from atst.domain.exceptions import NotFoundError +from atst.domain.authz.decorator import user_can_access_decorator as user_can +from atst.forms.ccpo_user import CCPOUserForm +from atst.models.permissions import Permissions +from atst.utils.context_processors import atat as atat_context_processor +from atst.utils.flash import formatted_flash as flash + + +bp = Blueprint("ccpo", __name__) +bp.context_processor(atat_context_processor) + + +@bp.route("/activity-history") +@user_can(Permissions.VIEW_AUDIT_LOG, message="view activity log") +def activity_history(): + pagination_opts = Paginator.get_pagination_opts(request) + audit_events = AuditLog.get_all_events(pagination_opts) + return render_template("audit_log/audit_log.html", audit_events=audit_events) + + +@bp.route("/ccpo-users") +@user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") +def ccpo_users(): + users = Users.get_ccpo_users() + return render_template("ccpo/users.html", users=users) + + +@bp.route("/ccpo-users/new") +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def add_new_ccpo_user(): + form = CCPOUserForm() + return render_template("ccpo/add_user.html", form=form) + + +@bp.route("/ccpo-users/new", methods=["POST"]) +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def submit_add_new_ccpo_user(): + try: + new_user = Users.get_by_dod_id(request.form["dod_id"]) + form = CCPOUserForm(obj=new_user) + except NotFoundError: + new_user = None + form = CCPOUserForm() + + return render_template("ccpo/confirm_user.html", new_user=new_user, form=form) + + +@bp.route("/ccpo-users/confirm-new", methods=["POST"]) +@user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") +def confirm_new_ccpo_user(): + user = Users.get_by_dod_id(request.form["dod_id"]) + Users.update_ccpo_permissions(user, add_perms=True) + flash("ccpo_user_added", user_name=user.full_name) + return redirect(url_for("ccpo.ccpo_users")) diff --git a/templates/audit_log/audit_log.html b/templates/audit_log/audit_log.html index 803d70ad..57a6656c 100644 --- a/templates/audit_log/audit_log.html +++ b/templates/audit_log/audit_log.html @@ -4,6 +4,6 @@ {% block content %}
{% include "fragments/audit_events_log.html" %} - {{ Pagination(audit_events, url_for('atst.activity_history'))}} + {{ Pagination(audit_events, url_for('ccpo.activity_history'))}}
{% endblock %} diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html index 505ab20b..81690e0f 100644 --- a/templates/ccpo/add_user.html +++ b/templates/ccpo/add_user.html @@ -3,7 +3,7 @@ {% from "components/text_input.html" import TextInput %} {% block content %} - + {{ form.csrf_token }}

Add new CCPO user

@@ -17,7 +17,7 @@ v-bind:disabled="invalid" class='action-group__action usa-button' value='Next'> - {{ "common.cancel" | translate }} + {{ "common.cancel" | translate }}
diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index 2012fc0f..a20cc3f9 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -6,7 +6,7 @@ {% block content %} {% if new_user %} {% call Alert('Confirm new CCPO user') %} - + {{ form.csrf_token }}
@@ -26,7 +26,7 @@ v-bind:disabled="invalid" class='action-group__action usa-button' value='Confirm and Add User'> - {{ "common.cancel" | translate }} + {{ "common.cancel" | translate }}
{% endcall %} @@ -34,7 +34,7 @@ {% call Alert('User not found') %} To add someone as a CCPO user, they must already have an ATAT account.
- + Return to list of CCPO users
diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 7e29a1d6..891ceb7a 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -31,7 +31,7 @@
{% if user_can(permissions.CREATE_CCPO_USER) %} - + Add new CCPO user {{ Icon("plus") }} {% endif %} diff --git a/tests/test_access.py b/tests/test_access.py index 29e3a796..b02cf489 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -110,53 +110,53 @@ def post_url_assert_status(client, user_session): return _get_url_assert_status -# atst.activity_history +# ccpo.activity_history def test_atst_activity_history_access(get_url_assert_status): ccpo = user_with(PermissionSets.VIEW_AUDIT_LOG) rando = user_with() - url = url_for("atst.activity_history") + url = url_for("ccpo.activity_history") get_url_assert_status(ccpo, url, 200) get_url_assert_status(rando, url, 404) -# atst.ccpo_users -def test_atst_ccpo_users_access(get_url_assert_status): +# ccpo.ccpo_users +def test_ccpo_ccpo_users_access(get_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("atst.ccpo_users") + url = url_for("ccpo.ccpo_users") get_url_assert_status(ccpo, url, 200) get_url_assert_status(rando, url, 404) -# atst.add_new_ccpo_user -def test_atst_add_new_ccpo_user_access(get_url_assert_status): +# ccpo.add_new_ccpo_user +def test_ccpo_add_new_ccpo_user_access(get_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("atst.add_new_ccpo_user") + url = url_for("ccpo.add_new_ccpo_user") get_url_assert_status(ccpo, url, 200) get_url_assert_status(rando, url, 404) -# atst.submit_add_new_ccpo_user -def test_atst_submit_add_new_ccpo_user_access(post_url_assert_status): +# ccpo.submit_add_new_ccpo_user +def test_ccpo_submit_add_new_ccpo_user_access(post_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("atst.submit_add_new_ccpo_user") + url = url_for("ccpo.submit_add_new_ccpo_user") post_url_assert_status(ccpo, url, 200, data={"dod_id": "1234567890"}) post_url_assert_status(rando, url, 404, data={"dod_id": "1234567890"}) -# atst.confirm_new_ccpo_user -def test_atst_confirm_new_ccpo_user_access(post_url_assert_status): +# ccpo.confirm_new_ccpo_user +def test_ccpo_confirm_new_ccpo_user_access(post_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() user = UserFactory.create() - url = url_for("atst.confirm_new_ccpo_user") + url = url_for("ccpo.confirm_new_ccpo_user") post_url_assert_status(ccpo, url, 302, data={"dod_id": user.dod_id}) post_url_assert_status(rando, url, 404, data={"dod_id": user.dod_id}) From a6eb89ffb8aa7f726dbdc55039f556f366dd0bbb Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 11:27:04 -0400 Subject: [PATCH 08/16] Add tests for route functions --- templates/ccpo/confirm_user.html | 2 +- tests/routes/test_ccpo.py | 54 ++++++++++++++++++++++++++++++++ translations.yaml | 2 ++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/routes/test_ccpo.py diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index a20cc3f9..150d25fd 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -31,7 +31,7 @@ {% endcall %} {% else %} - {% call Alert('User not found') %} + {% call Alert('forms.ccpo_user.user_not_found' | translate) %} To add someone as a CCPO user, they must already have an ATAT account.
diff --git a/tests/routes/test_ccpo.py b/tests/routes/test_ccpo.py new file mode 100644 index 00000000..8129408f --- /dev/null +++ b/tests/routes/test_ccpo.py @@ -0,0 +1,54 @@ +from flask import url_for + +from atst.utils.localization import translate + +from tests.factories import UserFactory + + +def test_ccpo_users(user_session, client): + ccpo = UserFactory.create_ccpo() + user_session(ccpo) + response = client.get(url_for("ccpo.ccpo_users")) + assert ccpo.email in response.data.decode() + + +def test_submit_add_new_ccpo_user(user_session, client): + ccpo = UserFactory.create_ccpo() + new_user = UserFactory.create() + random_dod_id = "1234567890" + user_session(ccpo) + + # give new_user CCPO permissions + response = client.post( + url_for("ccpo.submit_add_new_ccpo_user"), data={"dod_id": new_user.dod_id} + ) + assert new_user.email in response.data.decode() + + # give person with out ATAT account CCPO permissions + response = client.post( + url_for("ccpo.submit_add_new_ccpo_user"), data={"dod_id": random_dod_id} + ) + assert translate("forms.ccpo_user.user_not_found") in response.data.decode() + + +def test_confirm_new_ccpo_user(user_session, client): + ccpo = UserFactory.create_ccpo() + new_user = UserFactory.create() + random_dod_id = "1234567890" + user_session(ccpo) + + # give new_user CCPO permissions + response = client.post( + url_for("ccpo.confirm_new_ccpo_user"), + data={"dod_id": new_user.dod_id}, + follow_redirects=True, + ) + assert new_user.dod_id in response.data.decode() + + # give person with out ATAT account CCPO permissions + response = client.post( + url_for("ccpo.confirm_new_ccpo_user"), + data={"dod_id": random_dod_id}, + follow_redirects=True, + ) + assert random_dod_id not in response.data.decode() diff --git a/translations.yaml b/translations.yaml index fd5563c1..0a4f9dcf 100644 --- a/translations.yaml +++ b/translations.yaml @@ -103,6 +103,8 @@ forms: name_label: Name assign_ppoc: dod_id: 'Select new primary point of contact:' + ccpo_user: + user_not_found: "User not found" environments: name_label: Environment Name edit_user: From 1b86147ab610b08cd269c5feb659770b306672f3 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 8 Aug 2019 11:39:57 -0400 Subject: [PATCH 09/16] Move text into translations file --- templates/ccpo/add_user.html | 4 ++-- templates/ccpo/confirm_user.html | 18 +++++++++++------- templates/ccpo/users.html | 10 +++++----- tests/routes/test_ccpo.py | 2 +- translations.yaml | 17 +++++++++++++++-- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html index 81690e0f..15dfed75 100644 --- a/templates/ccpo/add_user.html +++ b/templates/ccpo/add_user.html @@ -5,7 +5,7 @@ {% block content %}
{{ form.csrf_token }} -

Add new CCPO user

+

{{ "ccpo.form.add_user_title" | translate }}

{{ TextInput(form.dod_id, validation='dodId') }} @@ -16,7 +16,7 @@ type='submit' v-bind:disabled="invalid" class='action-group__action usa-button' - value='Next'> + value='{{ "common.next" | translate }}'> {{ "common.cancel" | translate }}
diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index 150d25fd..5c54086d 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -5,13 +5,13 @@ {% block content %} {% if new_user %} - {% call Alert('Confirm new CCPO user') %} + {% call Alert('ccpo.form.confirm_user_title' | translate) %} {{ form.csrf_token }}

- Please confirm that the user details below match the user being given CCPO permissions. + {{ "ccpo.form.confirm_user_text" | translate }}

{{ new_user.full_name }} @@ -25,17 +25,21 @@ type='submit' v-bind:disabled="invalid" class='action-group__action usa-button' - value='Confirm and Add User'> - {{ "common.cancel" | translate }} + value='{{ "ccpo.form.confirm_button" | translate }}'> + + {{ "common.cancel" | translate }} +

{% endcall %} {% else %} - {% call Alert('forms.ccpo_user.user_not_found' | translate) %} - To add someone as a CCPO user, they must already have an ATAT account. + {% call Alert('ccpo.form.user_not_found_title' | translate) %} +

+ {{ "ccpo.form.user_not_found_text" | translate }} +

- Return to list of CCPO users + {{ "ccpo.form.return_link" | translate }}
{% endcall %} diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 891ceb7a..9e4c0fab 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -5,7 +5,7 @@ {% block content %}
- CCPO Users + {{ "ccpo.users_title" | translate }}
{% include "fragments/flash.html" %} @@ -13,9 +13,9 @@
- - - + + + @@ -32,7 +32,7 @@ {% if user_can(permissions.CREATE_CCPO_USER) %} - Add new CCPO user {{ Icon("plus") }} + {{ "ccpo.add_user" | translate }} {{ Icon("plus") }} {% endif %} diff --git a/tests/routes/test_ccpo.py b/tests/routes/test_ccpo.py index 8129408f..1e22e5b8 100644 --- a/tests/routes/test_ccpo.py +++ b/tests/routes/test_ccpo.py @@ -28,7 +28,7 @@ def test_submit_add_new_ccpo_user(user_session, client): response = client.post( url_for("ccpo.submit_add_new_ccpo_user"), data={"dod_id": random_dod_id} ) - assert translate("forms.ccpo_user.user_not_found") in response.data.decode() + assert translate("ccpo.form.user_not_found_title") in response.data.decode() def test_confirm_new_ccpo_user(user_session, client): diff --git a/translations.yaml b/translations.yaml index 0a4f9dcf..73b71951 100644 --- a/translations.yaml +++ b/translations.yaml @@ -26,6 +26,20 @@ home: applications_descrip: ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod reports_descrip: enim ad minim veniam, quis nostrud exercitation ullamco admin_descrip: aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat +ccpo: + users_title: CCPO Users + name_heading: Name + email_heading: Email + dod_id_heading: DoD ID + add_user: Add new CCPO user + form: + add_user_title: Add new CCPO user + confirm_user_title: Confirm new CCPO user + confirm_user_text: Please confirm that the user details below match the user being given CCPO permissions. + confirm_button: Confirm and Add User + return_link: Return to list of CCPO users + user_not_found_title: User not found + user_not_found_text: To add someone as a CCPO user, they must already have an ATAT account. common: cancel: Cancel close: Close @@ -36,6 +50,7 @@ common: delete_confirm: 'Please type the word {word} to confirm:' edit: Edit members: Members + next: Next 'yes': 'Yes' 'no': 'No' response_label: Response required @@ -103,8 +118,6 @@ forms: name_label: Name assign_ppoc: dod_id: 'Select new primary point of contact:' - ccpo_user: - user_not_found: "User not found" environments: name_label: Environment Name edit_user: From 0b2c7f595763a30a0c96b370a8adf68cbe47e039 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 09:40:19 -0400 Subject: [PATCH 10/16] Remove unused imports and context processor --- atst/routes/__init__.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index cb39a0bb..eaa447be 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -18,20 +18,11 @@ from werkzeug.exceptions import NotFound 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.domain.common import Paginator -from atst.domain.exceptions import NotFoundError -from atst.domain.portfolios import Portfolios -from atst.domain.authz.decorator import user_can_access_decorator as user_can -from atst.forms.ccpo_user import CCPOUserForm -from atst.models.permissions import Permissions -from atst.utils.context_processors import atat as atat_context_processor from atst.utils.flash import formatted_flash as flash bp = Blueprint("atst", __name__) -bp.context_processor(atat_context_processor) @bp.route("/") From feb24b8e16240cb68aca2ec56c2cd1ed28be5f08 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 09:51:47 -0400 Subject: [PATCH 11/16] Break class method for ccpo perms into two methods instead of switching on a kwarg to determine if perms are given or removed --- atst/domain/users.py | 14 ++++++++------ atst/routes/ccpo.py | 2 +- tests/domain/test_users.py | 14 ++++++++------ 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/atst/domain/users.py b/atst/domain/users.py index 8fa0ae91..03a8044e 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -88,13 +88,15 @@ class Users(object): return user @classmethod - def update_ccpo_permissions(cls, user, add_perms=False): - if add_perms: - permission_sets = PermissionSets.get_all() - else: - permission_sets = [] + def give_ccpo_perms(cls, user): + user.permission_sets = PermissionSets.get_all() + db.session.add(user) + db.session.commit() + return user - user.permission_sets = permission_sets + @classmethod + def revoke_ccpo_perms(cls, user): + user.permission_sets = [] db.session.add(user) db.session.commit() return user diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index f3e9d469..d7c0f048 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -53,6 +53,6 @@ def submit_add_new_ccpo_user(): @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") def confirm_new_ccpo_user(): user = Users.get_by_dod_id(request.form["dod_id"]) - Users.update_ccpo_permissions(user, add_perms=True) + Users.give_ccpo_perms(user) flash("ccpo_user_added", user_name=user.full_name) return redirect(url_for("ccpo.ccpo_users")) diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index 08f75412..b5a24058 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -87,13 +87,15 @@ def test_get_ccpo_users(): assert rando not in ccpo_users -def test_update_ccpo_permissions(): +def test_give_ccpo_perms(): rando = UserFactory.create() - Users.update_ccpo_permissions(rando, add_perms=True) - - ccpo = UserFactory.create_ccpo() - Users.update_ccpo_permissions(ccpo) - + Users.give_ccpo_perms(rando) ccpo_users = Users.get_ccpo_users() assert rando in ccpo_users + + +def test_revoke_ccpo_perms(): + ccpo = UserFactory.create_ccpo() + Users.revoke_ccpo_perms(ccpo) + ccpo_users = Users.get_ccpo_users() assert ccpo not in ccpo_users From 864e11ea114e430b6e011096690bd1e773959ebc Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 10:09:23 -0400 Subject: [PATCH 12/16] Remove redundant 'ccpo' from ccpo route function names --- atst/routes/ccpo.py | 10 +++++----- templates/ccpo/add_user.html | 4 ++-- templates/ccpo/confirm_user.html | 6 +++--- templates/ccpo/users.html | 2 +- tests/routes/test_ccpo.py | 14 +++++++------- tests/test_access.py | 24 ++++++++++++------------ 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index d7c0f048..90aaa03f 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -24,21 +24,21 @@ def activity_history(): @bp.route("/ccpo-users") @user_can(Permissions.VIEW_CCPO_USER, message="view ccpo users") -def ccpo_users(): +def users(): users = Users.get_ccpo_users() return render_template("ccpo/users.html", users=users) @bp.route("/ccpo-users/new") @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def add_new_ccpo_user(): +def add_new_user(): form = CCPOUserForm() return render_template("ccpo/add_user.html", form=form) @bp.route("/ccpo-users/new", methods=["POST"]) @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def submit_add_new_ccpo_user(): +def submit_new_user(): try: new_user = Users.get_by_dod_id(request.form["dod_id"]) form = CCPOUserForm(obj=new_user) @@ -51,8 +51,8 @@ def submit_add_new_ccpo_user(): @bp.route("/ccpo-users/confirm-new", methods=["POST"]) @user_can(Permissions.CREATE_CCPO_USER, message="create ccpo user") -def confirm_new_ccpo_user(): +def confirm_new_user(): user = Users.get_by_dod_id(request.form["dod_id"]) Users.give_ccpo_perms(user) flash("ccpo_user_added", user_name=user.full_name) - return redirect(url_for("ccpo.ccpo_users")) + return redirect(url_for("ccpo.users")) diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html index 15dfed75..b3b9b0f5 100644 --- a/templates/ccpo/add_user.html +++ b/templates/ccpo/add_user.html @@ -3,7 +3,7 @@ {% from "components/text_input.html" import TextInput %} {% block content %} -
+ {{ form.csrf_token }}

{{ "ccpo.form.add_user_title" | translate }}

@@ -17,7 +17,7 @@ v-bind:disabled="invalid" class='action-group__action usa-button' value='{{ "common.next" | translate }}'> - {{ "common.cancel" | translate }} + {{ "common.cancel" | translate }}
diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index 5c54086d..1304fa97 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -6,7 +6,7 @@ {% block content %} {% if new_user %} {% call Alert('ccpo.form.confirm_user_title' | translate) %} - + {{ form.csrf_token }}
@@ -26,7 +26,7 @@ v-bind:disabled="invalid" class='action-group__action usa-button' value='{{ "ccpo.form.confirm_button" | translate }}'> - + {{ "common.cancel" | translate }}
@@ -38,7 +38,7 @@ {{ "ccpo.form.user_not_found_text" | translate }}

- + {{ "ccpo.form.return_link" | translate }}
diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 9e4c0fab..3195fb36 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -31,7 +31,7 @@ {% if user_can(permissions.CREATE_CCPO_USER) %} - + {{ "ccpo.add_user" | translate }} {{ Icon("plus") }} {% endif %} diff --git a/tests/routes/test_ccpo.py b/tests/routes/test_ccpo.py index 1e22e5b8..9cbd6159 100644 --- a/tests/routes/test_ccpo.py +++ b/tests/routes/test_ccpo.py @@ -8,11 +8,11 @@ from tests.factories import UserFactory def test_ccpo_users(user_session, client): ccpo = UserFactory.create_ccpo() user_session(ccpo) - response = client.get(url_for("ccpo.ccpo_users")) + response = client.get(url_for("ccpo.users")) assert ccpo.email in response.data.decode() -def test_submit_add_new_ccpo_user(user_session, client): +def test_submit_new_user(user_session, client): ccpo = UserFactory.create_ccpo() new_user = UserFactory.create() random_dod_id = "1234567890" @@ -20,18 +20,18 @@ def test_submit_add_new_ccpo_user(user_session, client): # give new_user CCPO permissions response = client.post( - url_for("ccpo.submit_add_new_ccpo_user"), data={"dod_id": new_user.dod_id} + url_for("ccpo.submit_new_user"), data={"dod_id": new_user.dod_id} ) assert new_user.email in response.data.decode() # give person with out ATAT account CCPO permissions response = client.post( - url_for("ccpo.submit_add_new_ccpo_user"), data={"dod_id": random_dod_id} + url_for("ccpo.submit_new_user"), data={"dod_id": random_dod_id} ) assert translate("ccpo.form.user_not_found_title") in response.data.decode() -def test_confirm_new_ccpo_user(user_session, client): +def test_confirm_new_user(user_session, client): ccpo = UserFactory.create_ccpo() new_user = UserFactory.create() random_dod_id = "1234567890" @@ -39,7 +39,7 @@ def test_confirm_new_ccpo_user(user_session, client): # give new_user CCPO permissions response = client.post( - url_for("ccpo.confirm_new_ccpo_user"), + url_for("ccpo.confirm_new_user"), data={"dod_id": new_user.dod_id}, follow_redirects=True, ) @@ -47,7 +47,7 @@ def test_confirm_new_ccpo_user(user_session, client): # give person with out ATAT account CCPO permissions response = client.post( - url_for("ccpo.confirm_new_ccpo_user"), + url_for("ccpo.confirm_new_user"), data={"dod_id": random_dod_id}, follow_redirects=True, ) diff --git a/tests/test_access.py b/tests/test_access.py index b02cf489..43e842d2 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -120,43 +120,43 @@ def test_atst_activity_history_access(get_url_assert_status): get_url_assert_status(rando, url, 404) -# ccpo.ccpo_users -def test_ccpo_ccpo_users_access(get_url_assert_status): +# ccpo.users +def test_ccpo_users_access(get_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("ccpo.ccpo_users") + url = url_for("ccpo.users") get_url_assert_status(ccpo, url, 200) get_url_assert_status(rando, url, 404) -# ccpo.add_new_ccpo_user -def test_ccpo_add_new_ccpo_user_access(get_url_assert_status): +# ccpo.add_new_user +def test_ccpo_add_new_user_access(get_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("ccpo.add_new_ccpo_user") + url = url_for("ccpo.add_new_user") get_url_assert_status(ccpo, url, 200) get_url_assert_status(rando, url, 404) -# ccpo.submit_add_new_ccpo_user -def test_ccpo_submit_add_new_ccpo_user_access(post_url_assert_status): +# ccpo.submit_new_user +def test_ccpo_submit_new_user_access(post_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() - url = url_for("ccpo.submit_add_new_ccpo_user") + url = url_for("ccpo.submit_new_user") post_url_assert_status(ccpo, url, 200, data={"dod_id": "1234567890"}) post_url_assert_status(rando, url, 404, data={"dod_id": "1234567890"}) -# ccpo.confirm_new_ccpo_user -def test_ccpo_confirm_new_ccpo_user_access(post_url_assert_status): +# ccpo.confirm_new_user +def test_ccpo_confirm_new_user_access(post_url_assert_status): ccpo = user_with(PermissionSets.MANAGE_CCPO_USERS) rando = user_with() user = UserFactory.create() - url = url_for("ccpo.confirm_new_ccpo_user") + url = url_for("ccpo.confirm_new_user") post_url_assert_status(ccpo, url, 302, data={"dod_id": user.dod_id}) post_url_assert_status(rando, url, 404, data={"dod_id": user.dod_id}) From 1783cd494d0721911b20920a9e07b773c2b9a42b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 10:13:31 -0400 Subject: [PATCH 13/16] Add max length to dod id field in CCPO user form --- atst/forms/ccpo_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/forms/ccpo_user.py b/atst/forms/ccpo_user.py index 3ef23257..e9e07ec2 100644 --- a/atst/forms/ccpo_user.py +++ b/atst/forms/ccpo_user.py @@ -9,5 +9,5 @@ from atst.utils.localization import translate class CCPOUserForm(FlaskForm): dod_id = StringField( translate("forms.new_member.dod_id_label"), - validators=[Required(), Length(min=10), IsNumber()], + validators=[Required(), Length(min=10, max=10), IsNumber()], ) From 43855c549575f30e5682697692af34515580fa23 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 10:21:18 -0400 Subject: [PATCH 14/16] Remove alert from confirm ccpo user page --- templates/ccpo/confirm_user.html | 67 +++++++++++++++----------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index 1304fa97..d7f26c13 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -1,47 +1,44 @@ {% extends "base.html" %} -{% from "components/alert.html" import Alert %} {% from "components/text_input.html" import TextInput %} {% block content %} {% if new_user %} - {% call Alert('ccpo.form.confirm_user_title' | translate) %} - - {{ form.csrf_token }} - -
-

- {{ "ccpo.form.confirm_user_text" | translate }} -

-

- {{ new_user.full_name }} -

-

- {{ new_user.email }} -

-
-
- - - {{ "common.cancel" | translate }} - -
- - {% endcall %} - {% else %} - {% call Alert('ccpo.form.user_not_found_title' | translate) %} -

- {{ "ccpo.form.user_not_found_text" | translate }} -

+

{{ 'ccpo.form.confirm_user_title' | translate }}

+
+ {{ form.csrf_token }} + +
+

+ {{ "ccpo.form.confirm_user_text" | translate }} +

+

+ {{ new_user.full_name }} +

+

+ {{ new_user.email }} +

+
+ - {{ "ccpo.form.return_link" | translate }} + {{ "common.cancel" | translate }}
- {% endcall %} + + {% else %} +

{{ 'ccpo.form.user_not_found_title' | translate }}

+

+ {{ "ccpo.form.user_not_found_text" | translate }} +

+
+ + {{ "ccpo.form.return_link" | translate }} + +
{% endif %} {% endblock %} From fa83dfe6bd8397c6daa4664dadcda18b0055b18a Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 10:36:04 -0400 Subject: [PATCH 15/16] When user not found, redirect to users page and show flash message --- atst/routes/ccpo.py | 4 ++-- atst/utils/flash.py | 5 +++++ templates/ccpo/confirm_user.html | 10 ---------- tests/routes/test_ccpo.py | 4 ++-- tests/test_access.py | 2 +- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/atst/routes/ccpo.py b/atst/routes/ccpo.py index 90aaa03f..c2846aa1 100644 --- a/atst/routes/ccpo.py +++ b/atst/routes/ccpo.py @@ -43,8 +43,8 @@ def submit_new_user(): new_user = Users.get_by_dod_id(request.form["dod_id"]) form = CCPOUserForm(obj=new_user) except NotFoundError: - new_user = None - form = CCPOUserForm() + flash("ccpo_user_not_found") + return redirect(url_for("ccpo.users")) return render_template("ccpo/confirm_user.html", new_user=new_user, form=form) diff --git a/atst/utils/flash.py b/atst/utils/flash.py index e7fc72d7..e4f852ea 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -35,6 +35,11 @@ MESSAGES = { "message_template": "You have successfully given {{ user_name }} CCPO permissions.", "category": "success", }, + "ccpo_user_not_found": { + "title_template": translate("ccpo.form.user_not_found_title"), + "message_template": translate("ccpo.form.user_not_found_text"), + "category": "info", + }, "environment_added": { "title_template": translate("flash.success"), "message_template": """ diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index d7f26c13..4d054240 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -30,15 +30,5 @@ - {% else %} -

{{ 'ccpo.form.user_not_found_title' | translate }}

-

- {{ "ccpo.form.user_not_found_text" | translate }} -

-
- - {{ "ccpo.form.return_link" | translate }} - -
{% endif %} {% endblock %} diff --git a/tests/routes/test_ccpo.py b/tests/routes/test_ccpo.py index 9cbd6159..618eb9b9 100644 --- a/tests/routes/test_ccpo.py +++ b/tests/routes/test_ccpo.py @@ -24,11 +24,11 @@ def test_submit_new_user(user_session, client): ) assert new_user.email in response.data.decode() - # give person with out ATAT account CCPO permissions + # give person without ATAT account CCPO permissions response = client.post( url_for("ccpo.submit_new_user"), data={"dod_id": random_dod_id} ) - assert translate("ccpo.form.user_not_found_title") in response.data.decode() + assert url_for("ccpo.users") in response.location def test_confirm_new_user(user_session, client): diff --git a/tests/test_access.py b/tests/test_access.py index 43e842d2..f459ea06 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -146,7 +146,7 @@ def test_ccpo_submit_new_user_access(post_url_assert_status): rando = user_with() url = url_for("ccpo.submit_new_user") - post_url_assert_status(ccpo, url, 200, data={"dod_id": "1234567890"}) + post_url_assert_status(ccpo, url, 302, data={"dod_id": "1234567890"}) post_url_assert_status(rando, url, 404, data={"dod_id": "1234567890"}) From 14327e9a8b5b465e084cd6494d06b09c7527df9f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Aug 2019 10:47:25 -0400 Subject: [PATCH 16/16] Use common translations for table headings, remove unnecessary class --- templates/ccpo/users.html | 8 ++++---- translations.yaml | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index 3195fb36..bc77a83f 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -13,9 +13,9 @@
NameEmailDoD ID{{ "ccpo.name_heading" | translate }}{{ "ccpo.email_heading" | translate }}{{ "ccpo.dod_id_heading" | translate }}
- - - + + + @@ -31,7 +31,7 @@ {% if user_can(permissions.CREATE_CCPO_USER) %} - + {{ "ccpo.add_user" | translate }} {{ Icon("plus") }} {% endif %} diff --git a/translations.yaml b/translations.yaml index 73b71951..fd97ae8f 100644 --- a/translations.yaml +++ b/translations.yaml @@ -28,9 +28,6 @@ home: admin_descrip: aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat ccpo: users_title: CCPO Users - name_heading: Name - email_heading: Email - dod_id_heading: DoD ID add_user: Add new CCPO user form: add_user_title: Add new CCPO user @@ -48,8 +45,11 @@ common: delete: Delete deactivate: Deactivate delete_confirm: 'Please type the word {word} to confirm:' + dod_id: DoD ID edit: Edit + email: Email members: Members + name: Name next: Next 'yes': 'Yes' 'no': 'No' @@ -60,7 +60,6 @@ common: resource_names: environments: Environments choose_role: Choose a role - name: Name components: date_selector: day: Day
{{ "ccpo.name_heading" | translate }}{{ "ccpo.email_heading" | translate }}{{ "ccpo.dod_id_heading" | translate }}{{ "common.name" | translate }}{{ "common.email" | translate }}{{ "common.dod_id" | translate }}