From 546e04555d17cf8ea0aed581a9812d78bd9d576f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 13 May 2019 11:29:24 -0400 Subject: [PATCH] Update role div height Move serialization into the route change member.role to member.role_name in form and route Return 400 for invalid form submission --- atst/domain/environments.py | 16 +------- atst/forms/app_settings.py | 7 ++-- atst/routes/applications/settings.py | 43 +++++++++++++++------- styles/sections/_application_edit.scss | 2 + tests/domain/test_environments.py | 42 ++++++--------------- tests/routes/applications/test_settings.py | 6 +-- 6 files changed, 51 insertions(+), 65 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index b641d584..515c7557 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -98,7 +98,7 @@ class Environments(object): environment = Environments.get(environment_id) for member in team_roles: - new_role = member["role"] + new_role = member["role_name"] user = Users.get(member["user_id"]) Environments.update_env_role( environment=environment, user=user, new_role=new_role @@ -115,25 +115,13 @@ class Environments(object): @classmethod def get_members_by_role(cls, env, role): - env_roles = ( + return ( db.session.query(EnvironmentRole) .filter(EnvironmentRole.environment_id == env.id) .filter(EnvironmentRole.role == role) .all() ) - members_list = [] - for env_role in env_roles: - members_list.append( - { - "user_id": str(env_role.user_id), - "user_name": env_role.user.full_name, - "role": role, - } - ) - - return members_list - @classmethod def revoke_access(cls, environment, target_user): EnvironmentRoles.delete(environment.id, target_user.id) diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index 094c1fb6..20e1aa25 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -8,14 +8,13 @@ from .data import ENV_ROLES class MemberForm(FlaskForm): user_id = HiddenField() user_name = StringField() - role = RadioField(choices=ENV_ROLES, default="no_access") + role_name = RadioField(choices=ENV_ROLES, default="no_access") @property def data(self): _data = super().data - for field in _data: - if field == "role" and _data[field] == "no_access": - _data[field] = None + if "role_name" in _data and _data["role_name"] == "no_access": + _data["role_name"] = None return _data diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index f7198c54..586e6d3d 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -27,11 +27,26 @@ def get_environments_obj_for_app(application): return environments_obj +def serialize_members(member_list, role): + serialized_list = [] + + for member in member_list: + serialized_list.append( + { + "user_id": str(member.user_id), + "user_name": member.user.full_name, + "role_name": role, + } + ) + + return serialized_list + + def sort_env_users_by_role(env): users_list = [] no_access_users = env.application.users - env.users no_access_list = [ - {"user_id": str(user.id), "user_name": user.full_name, "role": "no_access"} + {"user_id": str(user.id), "user_name": user.full_name, "role_name": "no_access"} for user in no_access_users ] users_list.append({"role": "no_access", "members": no_access_list}) @@ -40,7 +55,9 @@ def sort_env_users_by_role(env): users_list.append( { "role": role.value, - "members": Environments.get_members_by_role(env, role.value), + "members": serialize_members( + Environments.get_members_by_role(env, role.value), role.value + ), } ) @@ -196,18 +213,18 @@ def update_env_roles(environment_id): ) ) else: - # TODO: Create a better pattern to handle when a form doesn't validate - # if a user is submitting the data via the web page then they - # should never have any form validation errors - return render_template( - "portfolios/applications/settings.html", - application=application, - form=ApplicationForm( - name=application.name, description=application.description + return ( + render_template( + "portfolios/applications/settings.html", + application=application, + form=ApplicationForm( + name=application.name, description=application.description + ), + environments_obj=get_environments_obj_for_app(application=application), + active_toggler=environment.id, + active_toggler_section="edit", ), - environments_obj=get_environments_obj_for_app(application=application), - active_toggler=environment.id, - active_toggler_section="edit", + 400, ) diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index 75027e93..0795fc6c 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -54,6 +54,7 @@ white-space: nowrap; width: 20rem; position: relative; + height: 3.6rem; &.unassigned { border: solid 1px $color-gray-light; @@ -88,6 +89,7 @@ margin: $gap; padding: ($gap / 2) $gap; font-weight: $font-normal; + height: 3.6rem; } } } diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 8e633e3d..577c5b93 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -79,18 +79,18 @@ def test_update_env_roles_by_environment(): team_roles = [ { "user_id": env_role_1.user.id, - "name": env_role_1.user.full_name, - "role": CSPRole.BUSINESS_READ.value, + "user_name": env_role_1.user.full_name, + "role_name": CSPRole.BUSINESS_READ.value, }, { "user_id": env_role_2.user.id, - "name": env_role_2.user.full_name, - "role": CSPRole.NETWORK_ADMIN.value, + "user_name": env_role_2.user.full_name, + "role_name": CSPRole.NETWORK_ADMIN.value, }, { "user_id": env_role_3.user.id, - "name": env_role_3.user.full_name, - "role": None, + "user_name": env_role_3.user.full_name, + "role_name": None, }, ] @@ -155,32 +155,12 @@ def test_get_members_by_role(db): basic_access_members = Environments.get_members_by_role( environment, CSPRole.BASIC_ACCESS.value ) - assert basic_access_members == [ - { - "user_id": str(env_role_1.user_id), - "user_name": env_role_1.user.full_name, - "role": CSPRole.BASIC_ACCESS.value, - } - ] - assert { - "user_id": str(rando_env_role.user_id), - "user_name": rando_env_role.user.full_name, - "role": CSPRole.BASIC_ACCESS.value, - } not in basic_access_members - assert Environments.get_members_by_role( + technical_read_members = Environments.get_members_by_role( environment, CSPRole.TECHNICAL_READ.value - ) == [ - { - "user_id": str(env_role_2.user_id), - "user_name": env_role_2.user.full_name, - "role": CSPRole.TECHNICAL_READ.value, - }, - { - "user_id": str(env_role_3.user_id), - "user_name": env_role_3.user.full_name, - "role": CSPRole.TECHNICAL_READ.value, - }, - ] + ) + assert basic_access_members == [env_role_1] + assert rando_env_role not in basic_access_members + assert technical_read_members == [env_role_2, env_role_3] assert ( Environments.get_members_by_role(environment, CSPRole.BUSINESS_READ.value) == [] ) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 438dbb1d..ef28c3ed 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -171,7 +171,7 @@ def test_data_for_app_env_roles_form(app, client, user_session): { "user_id": str(app_role.user_id), "user_name": app_role.user.full_name, - "role": None, + "role_name": None, } ], }, @@ -181,7 +181,7 @@ def test_data_for_app_env_roles_form(app, client, user_session): { "user_id": str(env_role1.user_id), "user_name": env_role1.user.full_name, - "role": CSPRole.BASIC_ACCESS.value, + "role_name": CSPRole.BASIC_ACCESS.value, } ], }, @@ -191,7 +191,7 @@ def test_data_for_app_env_roles_form(app, client, user_session): { "user_id": str(env_role2.user_id), "user_name": env_role2.user.full_name, - "role": CSPRole.NETWORK_ADMIN.value, + "role_name": CSPRole.NETWORK_ADMIN.value, } ], },