From 3c154f445c1c1a3dca561341044b3819fb6da9e8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Sat, 17 Nov 2018 10:21:45 -0500 Subject: [PATCH 1/4] Send workspace invite email to email submitted in add new member form. --- atst/routes/workspaces.py | 2 +- tests/routes/test_workspaces.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index c216d5ee..d691f878 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -264,7 +264,7 @@ def create_member(workspace_id): new_member = Workspaces.create_member(user, workspace, form.data) invite = Invitations.create(user, new_member) send_invite_email( - g.current_user.full_name, invite.token, new_member.user.email + g.current_user.full_name, invite.token, form.data["email"] ) return redirect( diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 3737042e..1a91e723 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -1,5 +1,6 @@ import datetime from flask import url_for +import pytest from tests.factories import ( UserFactory, @@ -339,6 +340,29 @@ def test_existing_member_accepts_valid_invite(client, user_session): assert len(Workspaces.for_user(user)) == 1 +def test_existing_member_invite_sent_to_email_submitted_in_form( + client, user_session, queue +): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + member_form_data = { + "dod_id": user.dod_id, + "first_name": user.first_name, + "last_name": user.last_name, + "workspace_role": "developer", + "email": "example@example.com", + } + user_session(workspace.owner) + client.post( + url_for("workspaces.create_member", workspace_id=workspace.id), + data={**member_form_data}, + ) + + assert user.email != "example@example.com" + assert len(queue.get_queue().jobs[0].args[0]) == 1 + assert queue.get_queue().jobs[0].args[0][0] == "example@example.com" + + def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): workspace = WorkspaceFactory.create() user_info = UserFactory.dictionary() From 385878e1ab853199ed25cd160c7a4ce834e0c37e Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 19 Nov 2018 17:21:35 -0500 Subject: [PATCH 2/4] Resend workspace invitations to email submitted in new member form. --- .../02d11579a581_add_email_to_invite.py | 50 +++++++++++++++++++ atst/domain/invitations.py | 7 ++- atst/models/invitation.py | 12 ++--- atst/routes/workspaces.py | 8 ++- tests/domain/test_invitations.py | 10 ++-- tests/factories.py | 1 + tests/routes/test_workspaces.py | 28 +++++++++++ 7 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 alembic/versions/02d11579a581_add_email_to_invite.py diff --git a/alembic/versions/02d11579a581_add_email_to_invite.py b/alembic/versions/02d11579a581_add_email_to_invite.py new file mode 100644 index 00000000..3788c112 --- /dev/null +++ b/alembic/versions/02d11579a581_add_email_to_invite.py @@ -0,0 +1,50 @@ +"""Add email to invite + +Revision ID: 02d11579a581 +Revises: 4c0b8263d800 +Create Date: 2018-11-19 14:51:33.178358 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.sql import text + + +# revision identifiers, used by Alembic. +revision = '02d11579a581' +down_revision = '4c0b8263d800' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('invitations', sa.Column('email', sa.String())) + conn = op.get_bind() + # Add non-null value to email column + conn.execute( + text( + """ + insert into invitations (email) + select u.email + from invitations i + inner join workspace_roles wr on i.workspace_role_id = wr.id + inner join users u on wr.user_id = u.id + where i.email is null; + """ + ) + ) + conn.execute( + text( + """ + update invitations + set email = 'example@example.com' + where email is null; + """ + ) + ) + op.alter_column('invitations', 'email', nullable=False) + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('invitations', 'email') + # ### end Alembic commands ### diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 738525ab..5f69e039 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -54,13 +54,14 @@ class Invitations(object): return invite @classmethod - def create(cls, inviter, workspace_role): + def create(cls, inviter, workspace_role, email): invite = Invitation( workspace_role=workspace_role, inviter=inviter, user=workspace_role.user, status=InvitationStatus.PENDING, expiration_time=Invitations.current_expiration_time(), + email=email, ) db.session.add(invite) db.session.commit() @@ -120,4 +121,6 @@ class Invitations(object): previous_invitation = Invitations._get(token) Invitations._update_status(previous_invitation, InvitationStatus.REVOKED) - return Invitations.create(user, previous_invitation.workspace_role) + return Invitations.create( + user, previous_invitation.workspace_role, previous_invitation.email + ) diff --git a/atst/models/invitation.py b/atst/models/invitation.py index 3768849c..6097361a 100644 --- a/atst/models/invitation.py +++ b/atst/models/invitation.py @@ -42,11 +42,13 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): expiration_time = Column(TIMESTAMP(timezone=True)) - token = Column(String(), index=True, default=lambda: secrets.token_urlsafe()) + token = Column(String, index=True, default=lambda: secrets.token_urlsafe()) + + email = Column(String, nullable=False) def __repr__(self): - return "".format( - self.user_id, self.workspace_role_id, self.id + return "".format( + self.user_id, self.workspace_role_id, self.id, self.email ) @property @@ -82,10 +84,6 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): if self.workspace_role: return self.workspace_role.workspace - @property - def user_email(self): - return self.workspace_role.user.email - @property def user_name(self): return self.workspace_role.user.full_name diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index d691f878..c0a0cab6 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -262,10 +262,8 @@ def create_member(workspace_id): if form.validate(): try: new_member = Workspaces.create_member(user, workspace, form.data) - invite = Invitations.create(user, new_member) - send_invite_email( - g.current_user.full_name, invite.token, form.data["email"] - ) + invite = Invitations.create(user, new_member, form.data["email"]) + send_invite_email(g.current_user.full_name, invite.token, invite.email) return redirect( url_for( @@ -381,7 +379,7 @@ def revoke_invitation(workspace_id, token): @bp.route("/workspaces//invitations//resend", methods=["POST"]) def resend_invitation(workspace_id, token): invite = Invitations.resend(g.current_user, workspace_id, token) - send_invite_email(g.current_user.full_name, invite.token, invite.user_email) + send_invite_email(g.current_user.full_name, invite.token, invite.email) return redirect( url_for( "workspaces.workspace_members", diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index e63258e8..4f4a1492 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -22,7 +22,7 @@ def test_create_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.user == user assert invite.workspace_role == ws_role assert invite.inviter == workspace.owner @@ -34,7 +34,7 @@ def test_accept_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.is_pending accepted_invite = Invitations.accept(user, invite.token) assert accepted_invite.is_accepted @@ -89,7 +89,7 @@ def test_accept_invitation_twice(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) Invitations.accept(user, invite.token) with pytest.raises(InvitationError): Invitations.accept(user, invite.token) @@ -99,7 +99,7 @@ def test_revoke_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) assert invite.is_pending Invitations.revoke(invite.token) assert invite.is_revoked @@ -109,7 +109,7 @@ def test_resend_invitation(): workspace = WorkspaceFactory.create() user = UserFactory.create() ws_role = WorkspaceRoleFactory.create(user=user, workspace=workspace) - invite = Invitations.create(workspace.owner, ws_role) + invite = Invitations.create(workspace.owner, ws_role, user.email) Invitations.resend(workspace.owner, workspace.id, invite.token) assert ws_role.invitations[0].is_revoked assert ws_role.invitations[1].is_pending diff --git a/tests/factories.py b/tests/factories.py index 7c7bdfd8..96e9c22c 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -342,5 +342,6 @@ class InvitationFactory(Base): class Meta: model = Invitation + email = factory.Faker("email") status = InvitationStatus.PENDING expiration_time = Invitations.current_expiration_time() diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 1a91e723..85d30318 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -502,3 +502,31 @@ def test_resend_invitation_sends_email(client, user_session, queue): ) assert len(queue.get_queue()) == 1 + + +def test_existing_member_invite_resent_to_email_submitted_in_form( + client, user_session, queue +): + workspace = WorkspaceFactory.create() + user = UserFactory.create() + ws_role = WorkspaceRoleFactory.create( + user=user, workspace=workspace, status=WorkspaceRoleStatus.PENDING + ) + invite = InvitationFactory.create( + user_id=user.id, + workspace_role_id=ws_role.id, + status=InvitationStatus.PENDING, + email="example@example.com", + ) + user_session(workspace.owner) + client.post( + url_for( + "workspaces.resend_invitation", + workspace_id=workspace.id, + token=invite.token, + ) + ) + + assert user.email != "example@example.com" + assert len(queue.get_queue().jobs[0].args[0]) == 1 + assert queue.get_queue().jobs[0].args[0][0] == "example@example.com" From 8ea5e6a805599099af5d1f606cee925bddc83c7a Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 21 Nov 2018 15:13:53 -0500 Subject: [PATCH 3/4] Update test to make it clearer --- tests/routes/test_workspaces.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/routes/test_workspaces.py b/tests/routes/test_workspaces.py index 85d30318..c15ee2c5 100644 --- a/tests/routes/test_workspaces.py +++ b/tests/routes/test_workspaces.py @@ -527,6 +527,7 @@ def test_existing_member_invite_resent_to_email_submitted_in_form( ) ) + send_mail_job = queue.get_queue().jobs[0] assert user.email != "example@example.com" - assert len(queue.get_queue().jobs[0].args[0]) == 1 - assert queue.get_queue().jobs[0].args[0][0] == "example@example.com" + assert send_mail_job.func.__func__.__name__ == "_send_mail" + assert send_mail_job.args[0] == ["example@example.com"] From d0c6ea5dbdb3e8c5988e3e3176438ec82834a81f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 27 Nov 2018 17:03:31 -0500 Subject: [PATCH 4/4] Update migrations after rebase --- alembic/versions/02d11579a581_add_email_to_invite.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/02d11579a581_add_email_to_invite.py b/alembic/versions/02d11579a581_add_email_to_invite.py index 3788c112..cd21df51 100644 --- a/alembic/versions/02d11579a581_add_email_to_invite.py +++ b/alembic/versions/02d11579a581_add_email_to_invite.py @@ -1,7 +1,7 @@ """Add email to invite Revision ID: 02d11579a581 -Revises: 4c0b8263d800 +Revises: 4f46aecb337f Create Date: 2018-11-19 14:51:33.178358 """ @@ -12,7 +12,7 @@ from sqlalchemy.sql import text # revision identifiers, used by Alembic. revision = '02d11579a581' -down_revision = '4c0b8263d800' +down_revision = '4f46aecb337f' branch_labels = None depends_on = None