diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 2114ce21..a1056623 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -10,7 +10,6 @@ from atst.models import ( Portfolio, TaskOrder, CLIN, - EnvironmentRole, ) from atst.domain.environment_roles import EnvironmentRoles @@ -58,16 +57,11 @@ class Environments(object): @classmethod def update_env_role(cls, environment, application_role, new_role): env_role = EnvironmentRoles.get_for_update(application_role.id, environment.id) - if env_role and ( - env_role.status == EnvironmentRole.Status.DISABLED or env_role.deleted - ): + + if env_role and new_role and (env_role.disabled or env_role.deleted): raise DisabledError("environment_role", env_role.id) - if ( - env_role - and env_role.role != new_role - and env_role.status != EnvironmentRole.Status.DISABLED - ): + if env_role and env_role.role != new_role and not env_role.disabled: env_role.role = new_role db.session.add(env_role) elif not env_role and new_role: @@ -78,7 +72,7 @@ class Environments(object): ) db.session.add(env_role) - if env_role and not new_role: + if env_role and not new_role and not env_role.disabled: EnvironmentRoles.disable(env_role.id) db.session.commit() diff --git a/atst/models/environment_role.py b/atst/models/environment_role.py index 813d3d50..37d1c17c 100644 --- a/atst/models/environment_role.py +++ b/atst/models/environment_role.py @@ -66,6 +66,10 @@ class EnvironmentRole( def displayname(self): return self.role + @property + def disabled(self): + return self.status == EnvironmentRole.Status.DISABLED + @property def event_details(self): return { diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 16d227ed..22863ac6 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -14,7 +14,6 @@ from atst.forms.application import NameAndDescriptionForm, EditEnvironmentForm from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS from atst.forms.member import NewForm as MemberForm from atst.domain.authz.decorator import user_can_access_decorator as user_can -from atst.models.environment_role import EnvironmentRole from atst.models.permissions import Permissions from atst.domain.permission_sets import PermissionSets from atst.utils.flash import formatted_flash as flash @@ -92,7 +91,7 @@ def filter_env_roles_form_data(member, environments): if len(env_roles_set) == 1: (env_role,) = env_roles_set env_data["role"] = env_role.role - env_data["disabled"] = env_role.status == EnvironmentRole.Status.DISABLED + env_data["disabled"] = env_role.disabled env_roles_form_data.append(env_data) diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index 6ac96488..486da70a 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -88,7 +88,7 @@ def test_disable_completed(application_role, environment): environment_role = EnvironmentRoles.disable(environment_role.id) - assert environment_role.status == EnvironmentRole.Status.DISABLED + assert environment_role.disabled def test_get_for_update(application_role, environment): diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 55d54f44..3aa5f547 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -42,21 +42,26 @@ def test_update_env_role_no_access(): env_role.application_role.id, env_role.environment.id ) assert env_role.role is None - assert env_role.status == EnvironmentRole.Status.DISABLED + assert env_role.disabled def test_update_env_role_disabled_role(): env_role = EnvironmentRoleFactory.create(role=CSPRole.BASIC_ACCESS.value) Environments.update_env_role(env_role.environment, env_role.application_role, None) + # An exception should be raised when a new role is passed to Environments.update_env_role with pytest.raises(DisabledError): Environments.update_env_role( env_role.environment, env_role.application_role, CSPRole.TECHNICAL_READ.value, ) + assert env_role.role is None - assert env_role.status == EnvironmentRole.Status.DISABLED + assert env_role.disabled + + # An exception should not be raised when no new role is passed + Environments.update_env_role(env_role.environment, env_role.application_role, None) def test_get_excludes_deleted(): diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 45522edb..8eb0079b 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -563,7 +563,7 @@ def test_update_member(client, user_session, session): # check that the user has roles in the correct envs assert len(environment_roles) == 3 assert updated_role.role == CSPRole.TECHNICAL_READ.value - assert suspended_role.status == EnvironmentRole.Status.DISABLED + assert suspended_role.disabled def test_revoke_invite(client, user_session):