Merge pull request #1188 from dod-ccpo/new-app-form-step-3-bug

Edit member perms in new app form bugfix

atst/routes/applications/new.py

@@ -11,6 +11,7 @@ from atst.routes.applications.settings import (
     get_members_data,
     get_new_member_form,
     handle_create_member,
+    handle_update_member,
 )
 
 
@@ -156,10 +157,16 @@ def view_new_application_step_3(application_id):
 
 
 @applications_bp.route("/applications/<application_id>/new/step_3", methods=["POST"])
+@applications_bp.route(
+    "/applications/<application_id>/new/step_3/member/<application_role_id>",
+    methods=["POST"],
+)
 @user_can(Permissions.CREATE_APPLICATION, message="view create new application form")
-def update_new_application_step_3(application_id):
+def update_new_application_step_3(application_id, application_role_id=None):
-
+    if application_role_id:
-    handle_create_member(application_id, http_request.form)
+        handle_update_member(application_id, application_role_id, http_request.form)
+    else:
+        handle_create_member(application_id, http_request.form)
 
     return redirect(
         url_for(

atst/routes/applications/settings.py

@@ -210,6 +210,30 @@ def handle_create_member(application_id, form_data):
         # TODO: flash error message
 
 
+def handle_update_member(application_id, application_role_id, form_data):
+    app_role = ApplicationRoles.get_by_id(application_role_id)
+    application = Applications.get(application_id)
+    existing_env_roles_data = filter_env_roles_form_data(
+        app_role, application.environments
+    )
+    form = UpdateMemberForm(
+        formdata=form_data, environment_roles=existing_env_roles_data
+    )
+
+    if form.validate():
+        ApplicationRoles.update_permission_sets(app_role, form.data["permission_sets"])
+
+        for env_role in form.environment_roles:
+            environment = Environments.get(env_role.environment_id.data)
+            new_role = None if env_role.disabled.data else env_role.data["role"]
+            Environments.update_env_role(environment, app_role, new_role)
+
+        flash("application_member_updated", user_name=app_role.user_name)
+    else:
+        pass
+        # TODO: flash error message
+
+
 @applications_bp.route("/applications/<application_id>/settings")
 @user_can(Permissions.VIEW_APPLICATION, message="view application edit form")
 def settings(application_id):
@@ -382,27 +406,8 @@ def remove_member(application_id, application_role_id):
 )
 @user_can(Permissions.EDIT_APPLICATION_MEMBER, message="update application member")
 def update_member(application_id, application_role_id):
-    app_role = ApplicationRoles.get_by_id(application_role_id)
-    application = Applications.get(application_id)
-    existing_env_roles_data = filter_env_roles_form_data(
-        app_role, application.environments
-    )
-    form = UpdateMemberForm(
-        formdata=http_request.form, environment_roles=existing_env_roles_data
-    )
 
-    if form.validate():
+    handle_update_member(application_id, application_role_id, http_request.form)
-        ApplicationRoles.update_permission_sets(app_role, form.data["permission_sets"])
-
-        for env_role in form.environment_roles:
-            environment = Environments.get(env_role.environment_id.data)
-            new_role = None if env_role.disabled.data else env_role.data["role"]
-            Environments.update_env_role(environment, app_role, new_role)
-
-        flash("application_member_updated", user_name=app_role.user_name)
-    else:
-        pass
-        # TODO: flash error message
 
     return redirect(
         url_for(

templates/applications/fragments/members.html

@@ -12,7 +12,8 @@
   application,
   members,
   new_member_form,
-  action) %}
+  action_new,
+  action_update) %}
 
   <h3  id="application-members">
     {{ 'portfolios.applications.settings.team_members' | translate }}
@@ -40,7 +41,7 @@
             <hr>
           </div>
           <base-form inline-template>
-            <form id='{{ modal_name }}' method="POST" action="{{ url_for('applications.update_member', application_id=application.id, application_role_id=member.role_id) }}">
+            <form id='{{ modal_name }}' method="POST" action="{{ url_for(action_update, application_id=application.id, application_role_id=member.role_id,) }}">
               {{ member.form.csrf_token }}
               {{ member_fields.PermsFields(form=member.form, member_role_id=member.role_id) }}
               <div class="action-group">
@@ -181,7 +182,7 @@
     {{ MultiStepModalForm(
         name=new_member_modal_name,
         form=new_member_form,
-        form_action=url_for(action, application_id=application.id),
+        form_action=url_for(action_new, application_id=application.id),
         steps=[
           member_steps.MemberStepOne(new_member_form),
           member_steps.MemberStepTwo(new_member_form, application)

templates/applications/new/step_3.html

@@ -21,7 +21,8 @@
     application,
     members,
     new_member_form,
-    "applications.update_new_application_step_3") }}
+    action_new="applications.update_new_application_step_3",
+    action_update="applications.update_new_application_step_3") }}
 
 
   <span class="action-group-footer">

templates/applications/settings.html

@@ -51,7 +51,8 @@
     application,
     members,
     new_member_form,
-    "applications.create_member") }}
+    action_new="applications.create_member",
+    action_update="applications.update_member") }}
 
   {{ EnvironmentManagementTemplate(
     application,

tests/conftest.py

@@ -5,6 +5,7 @@ import alembic.command
 from logging.config import dictConfig
 from werkzeug.datastructures import FileStorage
 from collections import OrderedDict
+from unittest.mock import Mock
 
 from atst.app import make_app, make_config
 from atst.database import db as _db

tests/routes/applications/test_new.py

@@ -1,6 +1,11 @@
 from flask import url_for
 
-from tests.factories import PortfolioFactory, ApplicationFactory, UserFactory
+from tests.factories import (
+    PortfolioFactory,
+    ApplicationFactory,
+    UserFactory,
+    ApplicationRoleFactory,
+)
 from unittest.mock import Mock
 from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS
 from atst.models.application_invitation import ApplicationInvitation
@@ -109,7 +114,7 @@ def test_get_members(client, session, user_session):
     assert response.status_code == 200
 
 
-def test_post_member(monkeypatch, client, user_session, session):
+def test_post_new_member(monkeypatch, client, user_session, session):
     job_mock = Mock()
     monkeypatch.setattr("atst.jobs.send_mail.delay", job_mock)
     user = UserFactory.create()
@@ -121,7 +126,9 @@ def test_post_member(monkeypatch, client, user_session, session):
     user_session(application.portfolio.owner)
 
     response = client.post(
-        url_for("applications.create_member", application_id=application.id),
+        url_for(
+            "applications.update_new_application_step_3", application_id=application.id
+        ),
         data={
             "user_data-first_name": user.first_name,
             "user_data-last_name": user.last_name,
@@ -141,10 +148,8 @@ def test_post_member(monkeypatch, client, user_session, session):
 
     assert response.status_code == 302
     expected_url = url_for(
-        "applications.settings",
+        "applications.view_new_application_step_3",
         application_id=application.id,
-        fragment="application-members",
-        _anchor="application-members",
         _external=True,
     )
     assert response.location == expected_url
@@ -159,3 +164,43 @@ def test_post_member(monkeypatch, client, user_session, session):
     assert invitation.role.application == application
 
     assert job_mock.called
+
+
+def test_post_update_member(client, user_session):
+    user = UserFactory.create()
+    application = ApplicationFactory.create(
+        environments=[{"name": "Naboo"}, {"name": "Endor"}]
+    )
+    (env, env_1) = application.environments
+    app_role = ApplicationRoleFactory(application=application)
+
+    user_session(application.portfolio.owner)
+    response = client.post(
+        url_for(
+            "applications.update_new_application_step_3",
+            application_id=application.id,
+            application_role_id=app_role.id,
+        ),
+        data={
+            "environment_roles-0-environment_id": env.id,
+            "environment_roles-0-role": "Basic Access",
+            "environment_roles-0-environment_name": env.name,
+            "environment_roles-1-environment_id": env_1.id,
+            "environment_roles-1-role": NO_ACCESS,
+            "environment_roles-1-environment_name": env_1.name,
+            "perms_env_mgmt": True,
+            "perms_team_mgmt": True,
+            "perms_del_env": True,
+        },
+    )
+
+    assert response.status_code == 302
+    expected_url = url_for(
+        "applications.view_new_application_step_3",
+        application_id=application.id,
+        _external=True,
+    )
+    assert response.location == expected_url
+    assert len(application.roles) == 1
+    assert len(app_role.environment_roles) == 1
+    assert app_role.environment_roles[0].environment == env

tests/routes/applications/test_settings.py

@@ -2,6 +2,8 @@ import uuid
 from flask import url_for, get_flashed_messages
 from unittest.mock import Mock
 import datetime
+from werkzeug.datastructures import ImmutableMultiDict
+import pytest
 
 from tests.factories import *
 
@@ -20,6 +22,8 @@ from atst.routes.applications.settings import (
     filter_env_roles_form_data,
     filter_env_roles_data,
     get_environments_obj_for_app,
+    handle_create_member,
+    handle_update_member,
 )
 
 from tests.utils import captured_templates
@@ -658,3 +662,89 @@ def test_filter_env_roles_data():
     # ensure that the environment roles and environments are associated with the
     # same application.
     assert [env["environment_name"] for env in env_role_data] == ["a", "b", "c"]
+
+
+@pytest.fixture
+def set_g(monkeypatch):
+    _g = Mock()
+    monkeypatch.setattr("atst.app.g", _g)
+    monkeypatch.setattr("atst.routes.applications.settings.g", _g)
+
+    def _set_g(attr, val):
+        setattr(_g, attr, val)
+
+    yield _set_g
+
+
+def test_handle_create_member(monkeypatch, set_g, session):
+    user = UserFactory.create()
+    application = ApplicationFactory.create(
+        environments=[{"name": "Naboo"}, {"name": "Endor"}]
+    )
+    (env, env_1) = application.environments
+
+    job_mock = Mock()
+    monkeypatch.setattr("atst.jobs.send_mail.delay", job_mock)
+    set_g("current_user", application.portfolio.owner)
+    set_g("portfolio", application.portfolio)
+    set_g("application", application)
+
+    form_data = ImmutableMultiDict(
+        {
+            "user_data-first_name": user.first_name,
+            "user_data-last_name": user.last_name,
+            "user_data-dod_id": user.dod_id,
+            "user_data-email": user.email,
+            "environment_roles-0-environment_id": env.id,
+            "environment_roles-0-role": "Basic Access",
+            "environment_roles-0-environment_name": env.name,
+            "environment_roles-1-environment_id": env_1.id,
+            "environment_roles-1-role": NO_ACCESS,
+            "environment_roles-1-environment_name": env_1.name,
+            "perms_env_mgmt": True,
+            "perms_team_mgmt": True,
+            "perms_del_env": True,
+        }
+    )
+    handle_create_member(application.id, form_data)
+
+    assert len(application.roles) == 1
+    environment_roles = application.roles[0].environment_roles
+    assert len(environment_roles) == 1
+    assert environment_roles[0].environment == env
+    invitation = (
+        session.query(ApplicationInvitation).filter_by(dod_id=user.dod_id).one()
+    )
+    assert invitation.role.application == application
+    assert job_mock.called
+
+
+def test_handle_update_member(set_g):
+    user = UserFactory.create()
+    application = ApplicationFactory.create(
+        environments=[{"name": "Naboo"}, {"name": "Endor"}]
+    )
+    (env, env_1) = application.environments
+    app_role = ApplicationRoleFactory(application=application)
+    set_g("current_user", application.portfolio.owner)
+    set_g("portfolio", application.portfolio)
+    set_g("application", application)
+
+    form_data = ImmutableMultiDict(
+        {
+            "environment_roles-0-environment_id": env.id,
+            "environment_roles-0-role": "Basic Access",
+            "environment_roles-0-environment_name": env.name,
+            "environment_roles-1-environment_id": env_1.id,
+            "environment_roles-1-role": NO_ACCESS,
+            "environment_roles-1-environment_name": env_1.name,
+            "perms_env_mgmt": True,
+            "perms_team_mgmt": True,
+            "perms_del_env": True,
+        }
+    )
+    handle_update_member(application.id, app_role.id, form_data)
+
+    assert len(application.roles) == 1
+    assert len(app_role.environment_roles) == 1
+    assert app_role.environment_roles[0].environment == env