Make user_id required and add post tests

- raise an exception in ApplicationRoles.get()
- permission_sets is not changed if bad data is sent

atst/domain/application_roles.py

@@ -1,6 +1,9 @@
+from sqlalchemy.orm.exc import NoResultFound
+
 from atst.database import db
 from atst.models import ApplicationRole, ApplicationRoleStatus
-from atst.domain.permission_sets import PermissionSets
+from .permission_sets import PermissionSets
+from .exceptions import NotFoundError
 
 
 class ApplicationRoles(object):
@@ -31,13 +34,16 @@ class ApplicationRoles(object):
 
     @classmethod
     def get(cls, user_id, application_id):
-        existing_app_role = (
+        try:
-            db.session.query(ApplicationRole)
+            app_role = (
-            .filter_by(user_id=user_id, application_id=application_id)
+                db.session.query(ApplicationRole)
-            .one_or_none()
+                .filter_by(user_id=user_id, application_id=application_id)
-        )
+                .one()
+            )
+        except NoResultFound:
+            raise NotFoundError("application_role")
 
-        return existing_app_role
+        return app_role
 
     @classmethod
     def update_permission_sets(cls, application_role, new_perm_sets_names):

atst/forms/team.py

@@ -1,5 +1,6 @@
 from flask_wtf import FlaskForm
 from wtforms.fields import FormField, FieldList, HiddenField, StringField
+from wtforms.validators import Required
 
 from .application_member import EnvironmentForm
 from .forms import BaseForm
@@ -43,7 +44,7 @@ class PermissionsForm(FlaskForm):
 
 
 class MemberForm(FlaskForm):
-    user_id = HiddenField()
+    user_id = HiddenField(validators=[Required()])
     user_name = StringField()
     environment_roles = FieldList(FormField(EnvironmentForm))
     permission_sets = FormField(PermissionsForm)

atst/routes/applications/team.py

@@ -104,7 +104,6 @@ def update_team(application_id):
     form = TeamForm(http_request.form)
 
     if form.validate():
-        # TODO check that all users coming through are app members
         for member in form.members:
             app_role = ApplicationRoles.get(member.data["user_id"], application.id)
             new_perms = [

tests/domain/test_application_roles.py

@@ -1,4 +1,7 @@
+import pytest
+
 from atst.domain.application_roles import ApplicationRoles
+from atst.domain.exceptions import NotFoundError
 from atst.domain.permission_sets import PermissionSets
 from atst.models import ApplicationRoleStatus
 
@@ -45,6 +48,14 @@ def test_get():
     assert app_role.user == user
 
 
+def test_get_handles_invalid_id():
+    user = UserFactory.create()
+    application = ApplicationFactory.create()
+
+    with pytest.raises(NotFoundError):
+        ApplicationRoles.get(user.id, application.id)
+
+
 def test_update_permission_sets():
     user = UserFactory.create()
     application = ApplicationFactory.create()

tests/routes/applications/test_team.py

@@ -1,6 +1,8 @@
 from flask import url_for
 
-from tests.factories import PortfolioFactory, ApplicationFactory, UserFactory
+from atst.domain.permission_sets import PermissionSets
+
+from tests.factories import *
 
 
 def test_application_team(client, user_session):
@@ -13,6 +15,79 @@ def test_application_team(client, user_session):
     assert response.status_code == 200
 
 
+def test_update_team(client, user_session):
+    application = ApplicationFactory.create()
+    owner = application.portfolio.owner
+    app_role = ApplicationRoleFactory.create(
+        application=application, permission_sets=[]
+    )
+    app_user = app_role.user
+    user_session(owner)
+    response = client.post(
+        url_for("applications.update_team", application_id=application.id),
+        data={
+            "members-0-user_id": app_user.id,
+            "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM,
+            "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS,
+            "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS,
+        },
+    )
+
+    assert response.status_code == 302
+    actual_perms_names = [perm.name for perm in app_role.permission_sets]
+    expected_perms_names = [
+        PermissionSets.VIEW_APPLICATION,
+        PermissionSets.EDIT_APPLICATION_TEAM,
+        PermissionSets.EDIT_APPLICATION_ENVIRONMENTS,
+        PermissionSets.DELETE_APPLICATION_ENVIRONMENTS,
+    ]
+    assert expected_perms_names == actual_perms_names
+
+
+def test_update_team_with_bad_permission_sets(client, user_session):
+    application = ApplicationFactory.create()
+    owner = application.portfolio.owner
+    app_role = ApplicationRoleFactory.create(
+        application=application, permission_sets=[]
+    )
+    app_user = app_role.user
+    permission_sets = app_user.permission_sets
+
+    user_session(owner)
+    response = client.post(
+        url_for("applications.update_team", application_id=application.id),
+        data={
+            "members-0-user_id": app_user.id,
+            "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM,
+            "members-0-permission_sets-perms_env_mgmt": "some random string",
+        },
+    )
+    assert app_user.permission_sets == permission_sets
+
+
+def test_update_team_with_non_app_user(client, user_session):
+    application = ApplicationFactory.create()
+    owner = application.portfolio.owner
+    app_role = ApplicationRoleFactory.create(
+        application=application, permission_sets=[]
+    )
+    non_app_user = UserFactory.create()
+    app_user = app_role.user
+
+    user_session(owner)
+    response = client.post(
+        url_for("applications.update_team", application_id=application.id),
+        data={
+            "members-0-user_id": non_app_user.id,
+            "members-0-permission_sets-perms_team_mgmt": PermissionSets.EDIT_APPLICATION_TEAM,
+            "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS,
+            "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS,
+        },
+    )
+
+    assert response.status_code == 404
+
+
 def test_create_member(client, user_session):
     user = UserFactory.create()
     application = ApplicationFactory.create(