From 27cf1783e31a8222c417a8838f87436beb368042 Mon Sep 17 00:00:00 2001
From: richard-dds <richard-ctr@friends.dds.mil>
Date: Mon, 26 Nov 2018 10:58:05 -0500
Subject: [PATCH] Don't show DISABLED workspace members

---
 atst/domain/workspaces/workspaces.py |  5 ++++-
 atst/models/workspace.py             | 15 ++++++++++-----
 tests/domain/test_workspaces.py      | 11 +++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py
index 69bfd640..9cd795bb 100644
--- a/atst/domain/workspaces/workspaces.py
+++ b/atst/domain/workspaces/workspaces.py
@@ -144,7 +144,10 @@ class Workspaces(object):
     def revoke_access(cls, user, workspace_id, workspace_role_id):
         workspace = WorkspacesQuery.get(workspace_id)
         Authorization.check_workspace_permission(
-            user, workspace, Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, "revoke workspace access"
+            user,
+            workspace,
+            Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE,
+            "revoke workspace access",
         )
         workspace_role = WorkspaceRoles.get_by_id(workspace_role_id)
         workspace_role.status = WorkspaceRoleStatus.DISABLED
diff --git a/atst/models/workspace.py b/atst/models/workspace.py
index 18df639a..1a29a6da 100644
--- a/atst/models/workspace.py
+++ b/atst/models/workspace.py
@@ -2,16 +2,16 @@ from sqlalchemy import Column, ForeignKey, String
 from sqlalchemy.orm import relationship
 from itertools import chain
 
-from atst.models import Base
-from atst.models.types import Id
-from atst.models import mixins
+from atst.models import Base, mixins, types
+from atst.models.workspace_role import WorkspaceRole, Status as WorkspaceRoleStatus
 from atst.utils import first_or_none
+from atst.database import db
 
 
 class Workspace(Base, mixins.TimestampsMixin, mixins.AuditableMixin):
     __tablename__ = "workspaces"
 
-    id = Id()
+    id = types.Id()
     name = Column(String)
     request_id = Column(ForeignKey("requests.id"), nullable=False)
     projects = relationship("Project", back_populates="workspace")
@@ -39,7 +39,12 @@ class Workspace(Base, mixins.TimestampsMixin, mixins.AuditableMixin):
 
     @property
     def members(self):
-        return self.roles
+        return (
+            db.session.query(WorkspaceRole)
+            .filter(WorkspaceRole.workspace_id == self.id)
+            .filter(WorkspaceRole.status != WorkspaceRoleStatus.DISABLED)
+            .all()
+        )
 
     @property
     def displayname(self):
diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py
index f077eb08..b41a0789 100644
--- a/tests/domain/test_workspaces.py
+++ b/tests/domain/test_workspaces.py
@@ -309,3 +309,14 @@ def test_can_remove_workspace_access():
     workspace_role = WorkspaceRoleFactory.create(workspace=workspace)
     Workspaces.revoke_access(workspace.owner, workspace.id, workspace_role.id)
     assert Workspaces.for_user(workspace_role.user) == []
+
+
+def test_disabled_members_dont_show_up(session):
+    workspace = WorkspaceFactory.create()
+    WorkspaceRoleFactory.create(workspace=workspace, status=WorkspaceRoleStatus.ACTIVE)
+    WorkspaceRoleFactory.create(
+        workspace=workspace, status=WorkspaceRoleStatus.DISABLED
+    )
+
+    # should only return workspace owner and ACTIVE member
+    assert len(workspace.members) == 2