diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index d8960a3d..bade9e08 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -146,7 +146,10 @@ class Workspaces(object): @classmethod def can_revoke_access_for(cls, workspace, workspace_role): - return workspace_role.user != workspace.owner + return ( + workspace_role.user != workspace.owner + and workspace_role.status == WorkspaceRoleStatus.ACTIVE + ) @classmethod def revoke_access(cls, user, workspace_id, workspace_role_id): diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 1070ea4c..cc5f97c1 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -77,7 +77,18 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): @property def is_expired(self): - return datetime.datetime.now(self.expiration_time.tzinfo) > self.expiration_time + return ( + datetime.datetime.now(self.expiration_time.tzinfo) > self.expiration_time + and not self.status == Status.ACCEPTED + ) + + @property + def is_inactive(self): + return self.is_expired or self.status in [ + Status.REJECTED_WRONG_USER, + Status.REJECTED_EXPIRED, + Status.REVOKED, + ] @property def workspace(self): diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 692cafbb..580dc07f 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -117,6 +117,10 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def role_displayname(self): return self.role.display_name + @property + def is_active(self): + return self.status == Status.ACTIVE + @property def num_environment_roles(self): return ( @@ -147,8 +151,8 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def can_resend_invitation(self): - return self.latest_invitation and ( - self.latest_invitation.is_rejected or self.latest_invitation.is_expired + return not self.is_active and ( + self.latest_invitation and self.latest_invitation.is_inactive ) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index 63b408d9..38d20661 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -304,9 +304,11 @@ def test_can_create_workspaces_with_matching_names(): Workspaces.create(RequestFactory.create(), name=workspace_name) -def test_can_revoke_workspace_access(): +def test_able_to_revoke_workspace_access_for_active_member(): workspace = WorkspaceFactory.create() - workspace_role = WorkspaceRoleFactory.create(workspace=workspace) + workspace_role = WorkspaceRoleFactory.create( + workspace=workspace, status=WorkspaceRoleStatus.ACTIVE + ) Workspaces.revoke_access(workspace.owner, workspace.id, workspace_role.id) assert Workspaces.for_user(workspace_role.user) == [] @@ -314,13 +316,15 @@ def test_can_revoke_workspace_access(): def test_can_revoke_access(): workspace = WorkspaceFactory.create() owner_role = workspace.roles[0] - workspace_role = WorkspaceRoleFactory.create(workspace=workspace) + workspace_role = WorkspaceRoleFactory.create( + workspace=workspace, status=WorkspaceRoleStatus.ACTIVE + ) assert Workspaces.can_revoke_access_for(workspace, workspace_role) assert not Workspaces.can_revoke_access_for(workspace, owner_role) -def test_cant_revoke_owner_workspace_access(): +def test_unable_to_revoke_owner_workspace_access(): workspace = WorkspaceFactory.create() owner_workspace_role = workspace.roles[0] diff --git a/tests/routes/workspaces/test_members.py b/tests/routes/workspaces/test_members.py index 44a63879..c93e85c6 100644 --- a/tests/routes/workspaces/test_members.py +++ b/tests/routes/workspaces/test_members.py @@ -1,12 +1,39 @@ from flask import url_for -from tests.factories import UserFactory, WorkspaceFactory, WorkspaceRoleFactory +from tests.factories import ( + UserFactory, + WorkspaceFactory, + WorkspaceRoleFactory, + InvitationFactory, +) from atst.domain.workspaces import Workspaces from atst.domain.workspace_roles import WorkspaceRoles from atst.domain.projects import Projects from atst.domain.environments import Environments from atst.domain.environment_roles import EnvironmentRoles from atst.queue import queue +from atst.models.workspace_role import Status as WorkspaceRoleStatus +from atst.models.invitation import Status as InvitationStatus + + +def create_workspace_and_invite_user( + ws_role="developer", + ws_status=WorkspaceRoleStatus.PENDING, + invite_status=InvitationStatus.PENDING, +): + workspace = WorkspaceFactory.create() + if ws_role != "owner": + user = UserFactory.create() + member = WorkspaceRoleFactory.create( + user=user, workspace=workspace, status=ws_status + ) + InvitationFactory.create( + user=workspace.owner, + workspace_role=member, + email=member.user.email, + status=invite_status, + ) + return workspace def test_user_with_permission_has_add_member_link(client, user_session): @@ -174,10 +201,12 @@ def test_update_member_environment_role_with_no_data(client, user_session): assert EnvironmentRoles.get(user.id, env1_id).role == "developer" -def test_revoke_member_access(client, user_session): +def test_revoke_active_member_access(client, user_session): workspace = WorkspaceFactory.create() user = UserFactory.create() - member = WorkspaceRoles.add(user, workspace.id, "developer") + member = WorkspaceRoleFactory.create( + workspace=workspace, user=user, status=WorkspaceRoleStatus.ACTIVE + ) Projects.create( workspace.owner, workspace, @@ -195,22 +224,7 @@ def test_revoke_member_access(client, user_session): assert WorkspaceRoles.get_by_id(member.id).num_environment_roles == 0 -def test_shows_revoke_button(client, user_session): - workspace = WorkspaceFactory.create() - user = UserFactory.create() - member = WorkspaceRoleFactory.create(user=user, workspace=workspace) - user_session(workspace.owner) - response = client.get( - url_for( - "workspaces.view_member", - workspace_id=workspace.id, - member_id=member.user.id, - ) - ) - assert "Remove Workspace Access" in response.data.decode() - - -def test_does_not_show_revoke_button(client, user_session): +def test_does_not_show_any_buttons_if_owner(client, user_session): workspace = WorkspaceFactory.create() user_session(workspace.owner) response = client.get( @@ -221,3 +235,78 @@ def test_does_not_show_revoke_button(client, user_session): ) ) assert "Remove Workspace Access" not in response.data.decode() + assert "Resend Invitation" not in response.data.decode() + assert "Revoke Invitation" not in response.data.decode() + + +def test_only_shows_revoke_access_button_if_active(client, user_session): + workspace = create_workspace_and_invite_user( + ws_status=WorkspaceRoleStatus.ACTIVE, invite_status=InvitationStatus.ACCEPTED + ) + user_session(workspace.owner) + member = workspace.members[1] + response = client.get( + url_for( + "workspaces.view_member", + workspace_id=workspace.id, + member_id=member.user.id, + ) + ) + assert "Remove Workspace Access" in response.data.decode() + assert "Revoke Invitation" not in response.data.decode() + assert "Resend Invitation" not in response.data.decode() + + +def test_only_shows_revoke_invite_button_if_pending(client, user_session): + workspace = create_workspace_and_invite_user( + ws_status=WorkspaceRoleStatus.PENDING, invite_status=InvitationStatus.PENDING + ) + user_session(workspace.owner) + member = workspace.members[1] + response = client.get( + url_for( + "workspaces.view_member", + workspace_id=workspace.id, + member_id=member.user.id, + ) + ) + assert "Revoke Invitation" in response.data.decode() + assert "Remove Workspace Access" not in response.data.decode() + assert "Resend Invitation" not in response.data.decode() + + +def test_only_shows_resend_button_if_expired(client, user_session): + workspace = create_workspace_and_invite_user( + ws_status=WorkspaceRoleStatus.PENDING, + invite_status=InvitationStatus.REJECTED_EXPIRED, + ) + user_session(workspace.owner) + member = workspace.members[1] + response = client.get( + url_for( + "workspaces.view_member", + workspace_id=workspace.id, + member_id=member.user.id, + ) + ) + assert "Resend Invitation" in response.data.decode() + assert "Revoke Invitation" not in response.data.decode() + assert "Remove Workspace Access" not in response.data.decode() + + +def test_only_shows_resend_button_if_revoked(client, user_session): + workspace = create_workspace_and_invite_user( + ws_status=WorkspaceRoleStatus.PENDING, invite_status=InvitationStatus.REVOKED + ) + user_session(workspace.owner) + member = workspace.members[1] + response = client.get( + url_for( + "workspaces.view_member", + workspace_id=workspace.id, + member_id=member.user.id, + ) + ) + assert "Resend Invitation" in response.data.decode() + assert "Remove Workspace Access" not in response.data.decode() + assert "Revoke Invitation" not in response.data.decode()