Prevent error from being raised when user is not trying to update a

disabled env role

We were only checking to see if a role was disabled or deleted before
raising an error, so I added in a check to see if the user was trying to
update the env role before raising an error. The error should only be
raised if the role is disabled or deleted AND the user is trying to
assign a new role to the env role.

I also added in a disabled property to the EnvironmentRole model to make
things more readable.

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()

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 {

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)