From e4bad109db66b6a35a7f71a75520ceddda7c038e Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 1 Nov 2018 13:19:08 -0400 Subject: [PATCH] more granular invitation status and a display status for workspace members --- .../e1081cf01780_adjust_invitation_status.py | 39 ++++++++++++++++ atst/domain/invitations.py | 4 +- atst/models/invitation.py | 20 +++++++-- atst/models/workspace_role.py | 20 ++++++--- tests/domain/test_invitations.py | 2 +- tests/models/test_workspace_user.py | 44 ++++++++++++++++--- tests/routes/test_workspaces.py | 6 ++- 7 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 alembic/versions/e1081cf01780_adjust_invitation_status.py diff --git a/alembic/versions/e1081cf01780_adjust_invitation_status.py b/alembic/versions/e1081cf01780_adjust_invitation_status.py new file mode 100644 index 00000000..a2c552cc --- /dev/null +++ b/alembic/versions/e1081cf01780_adjust_invitation_status.py @@ -0,0 +1,39 @@ +"""adjust invitation status + +Revision ID: e1081cf01780 +Revises: a9d8c6b6221c +Create Date: 2018-11-01 12:24:10.970963 + +""" +from alembic import op +import sqlalchemy as sa +from atst.models.invitation import Status +from enum import Enum + + +# revision identifiers, used by Alembic. +revision = 'e1081cf01780' +down_revision = 'a9d8c6b6221c' +branch_labels = None +depends_on = None + + +def upgrade(): + conn = op.get_bind() + constraints = ", ".join(["'{}'::character varying::text".format(s.name) for s in Status]) + conn.execute("ALTER TABLE invitations ALTER COLUMN status TYPE varchar(30)") + conn.execute("ALTER TABLE invitations DROP CONSTRAINT status") + conn.execute("ALTER TABLE invitations ADD CONSTRAINT status CHECK(status::text = ANY (ARRAY[{}]))".format(constraints)) + +class PreviousStatus(Enum): + ACCEPTED = "accepted" + REVOKED = "revoked" + PENDING = "pending" + REJECTED = "rejected" + +def downgrade(): + conn = op.get_bind() + constraints = ", ".join(["'{}'::character varying::text".format(s.name) for s in PreviousStatus]) + conn.execute("ALTER TABLE invitations ALTER COLUMN status TYPE varchar(8)") + conn.execute("ALTER TABLE invitations DROP CONSTRAINT status") + conn.execute("ALTER TABLE invitations ADD CONSTRAINT status CHECK(status::text = ANY (ARRAY[{}]))".format(constraints)) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 00797ddd..313f82f5 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -71,11 +71,11 @@ class Invitations(object): if invite.user.dod_id != user.dod_id: if invite.is_pending: - Invitations._update_status(invite, InvitationStatus.REJECTED) + Invitations._update_status(invite, InvitationStatus.REJECTED_WRONG_USER) raise WrongUserError(user, invite) elif invite.is_expired: - Invitations._update_status(invite, InvitationStatus.REJECTED) + Invitations._update_status(invite, InvitationStatus.REJECTED_EXPIRED) raise ExpiredError(invite) elif invite.is_accepted or invite.is_revoked or invite.is_rejected: diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 60f03b3c..ca3eba9c 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -4,7 +4,7 @@ import secrets from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum, TIMESTAMP, String from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import relationship +from sqlalchemy.orm import relationship, backref from atst.models import Base, types from atst.models.mixins.timestamps import TimestampsMixin @@ -15,7 +15,8 @@ class Status(Enum): ACCEPTED = "accepted" REVOKED = "revoked" PENDING = "pending" - REJECTED = "rejected" + REJECTED_WRONG_USER = "rejected_wrong_user" + REJECTED_EXPIRED = "rejected_expired" class Invitation(Base, TimestampsMixin, AuditableMixin): @@ -29,7 +30,10 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): workspace_role_id = Column( UUID(as_uuid=True), ForeignKey("workspace_roles.id"), index=True ) - workspace_role = relationship("WorkspaceRole", backref="invitations") + workspace_role = relationship( + "WorkspaceRole", + backref=backref("invitations", order_by="Invitation.time_created"), + ) inviter_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) inviter = relationship("User", backref="sent_invites", foreign_keys=[inviter_id]) @@ -59,7 +63,15 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): @property def is_rejected(self): - return self.status == Status.REJECTED + return self.status in [Status.REJECTED_WRONG_USER, Status.REJECTED_EXPIRED] + + @property + def is_rejected_expired(self): + return self.status == Status.REJECTED_EXPIRED + + @property + def is_rejected_wrong_user(self): + return self.status == Status.REJECTED_WRONG_USER @property def is_expired(self): diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 30b75837..50bc8144 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -36,18 +36,26 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): self.role.name, self.workspace.name, self.user_id, self.id ) + @property + def latest_invitation(self): + if self.invitations: + return self.invitations[-1] + @property def display_status(self): - import ipdb; ipdb.set_trace() if self.status == Status.ACTIVE: return "Active" - else: - if any(i.is_expired for i in self.invitations): - return "Invitation Expired" - elif any(i.is_rejected for i in self.invitations): - return "Invitation Rejected" + elif self.latest_invitation: + if self.latest_invitation.is_rejected_expired: + return "Invite expired" + elif self.latest_invitation.is_rejected_wrong_user: + return "Error on invite" + elif self.latest_invitation.is_expired: + return "Invite expired" else: return "Pending" + else: + return "Unknown errors" Index( diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index 059a8927..2ae38776 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -55,7 +55,7 @@ def test_accept_expired_invitation(): def test_accept_rejected_invite(): user = UserFactory.create() - invite = InvitationFactory.create(user_id=user.id, status=Status.REJECTED) + invite = InvitationFactory.create(user_id=user.id, status=Status.REJECTED_EXPIRED) with pytest.raises(InvitationError): Invitations.accept(user, invite.token) diff --git a/tests/models/test_workspace_user.py b/tests/models/test_workspace_user.py index b6256ed5..4d1ff7e1 100644 --- a/tests/models/test_workspace_user.py +++ b/tests/models/test_workspace_user.py @@ -1,9 +1,17 @@ +import datetime + from atst.domain.environments import Environments from atst.domain.workspaces import Workspaces from atst.domain.projects import Projects from atst.domain.workspace_users import WorkspaceUsers -from atst.models.invitation import Status -from tests.factories import RequestFactory, UserFactory, InvitationFactory, WorkspaceRoleFactory +from atst.models.workspace_role import Status +from atst.models.invitation import Status as InvitationStatus +from tests.factories import ( + RequestFactory, + UserFactory, + InvitationFactory, + WorkspaceRoleFactory, +) def test_has_no_environment_roles(): @@ -57,8 +65,34 @@ def test_role_displayname(): assert workspace_user.role_displayname == "Developer" -def test_status_when_member_has_pending_invitation(): +def test_status_when_member_is_active(): + workspace_role = WorkspaceRoleFactory.create(status=Status.ACTIVE) + assert workspace_role.display_status == "Active" + + +def test_status_when_invitation_has_been_rejected_for_expirations(): workspace_role = WorkspaceRoleFactory.create( - invitations=[InvitationFactory.create(status=Status.ACCEPTED)] + invitations=[InvitationFactory.create(status=InvitationStatus.REJECTED_EXPIRED)] ) - assert workspace_role.display_status == "Accepted" + assert workspace_role.display_status == "Invite expired" + + +def test_status_when_invitation_has_been_rejected_for_wrong_user(): + workspace_role = WorkspaceRoleFactory.create( + invitations=[ + InvitationFactory.create(status=InvitationStatus.REJECTED_WRONG_USER) + ] + ) + assert workspace_role.display_status == "Error on invite" + + +def test_status_when_invitation_is_expired(): + workspace_role = WorkspaceRoleFactory.create( + invitations=[ + InvitationFactory.create( + status=InvitationStatus.PENDING, + expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), + ) + ] + ) + assert workspace_role.display_status == "Invite expired" diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index be7097f2..1a2a9cb0 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -363,7 +363,9 @@ def test_member_accepts_invalid_invite(client, user_session): user=user, workspace=workspace, status=WorkspaceRoleStatus.PENDING ) invite = InvitationFactory.create( - user_id=user.id, workspace_role_id=ws_role.id, status=InvitationStatus.REJECTED + user_id=user.id, + workspace_role_id=ws_role.id, + status=InvitationStatus.REJECTED_WRONG_USER, ) user_session(user) response = client.get(url_for("workspaces.accept_invitation", token=invite.token)) @@ -411,7 +413,7 @@ def test_user_accepts_expired_invite(client, user_session): invite = InvitationFactory.create( user_id=user.id, workspace_role_id=ws_role.id, - status=InvitationStatus.REJECTED, + status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) user_session(user)