Merge pull request #859 from dod-ccpo/new-invitation-flow

New portfolio invitation backend flow

alembic/versions/8467440c4ae6_add_user_data_fields_to_invitations.py

@@ -0,0 +1,54 @@
+"""add user data fields to invitations
+
+Revision ID: 8467440c4ae6
+Revises: 24700d113ea9
+Create Date: 2019-05-31 12:40:10.457529
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = '8467440c4ae6'
+down_revision = '24700d113ea9'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('application_invitations', sa.Column('dod_id', sa.String(), nullable=True))
+    op.add_column('application_invitations', sa.Column('first_name', sa.String(), nullable=True))
+    op.add_column('application_invitations', sa.Column('last_name', sa.String(), nullable=True))
+    op.add_column('application_invitations', sa.Column('phone_number', sa.String(), nullable=True))
+    op.alter_column('application_roles', 'user_id',
+               existing_type=postgresql.UUID(),
+               nullable=True)
+    op.add_column('portfolio_invitations', sa.Column('dod_id', sa.String(), nullable=True))
+    op.add_column('portfolio_invitations', sa.Column('first_name', sa.String(), nullable=True))
+    op.add_column('portfolio_invitations', sa.Column('last_name', sa.String(), nullable=True))
+    op.add_column('portfolio_invitations', sa.Column('phone_number', sa.String(), nullable=True))
+    op.alter_column('portfolio_roles', 'user_id',
+               existing_type=postgresql.UUID(),
+               nullable=True)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('portfolio_roles', 'user_id',
+               existing_type=postgresql.UUID(),
+               nullable=True)
+    op.drop_column('portfolio_invitations', 'phone_number')
+    op.drop_column('portfolio_invitations', 'last_name')
+    op.drop_column('portfolio_invitations', 'first_name')
+    op.drop_column('portfolio_invitations', 'dod_id')
+    op.alter_column('application_roles', 'user_id',
+               existing_type=postgresql.UUID(),
+               nullable=True)
+    op.drop_column('application_invitations', 'phone_number')
+    op.drop_column('application_invitations', 'last_name')
+    op.drop_column('application_invitations', 'first_name')
+    op.drop_column('application_invitations', 'dod_id')
+    # ### end Alembic commands ###

atst/domain/application_roles.py

@@ -28,8 +28,9 @@ class ApplicationRoles(object):
         return application_role
 
     @classmethod
-    def enable(cls, role):
+    def enable(cls, role, user):
         role.status = ApplicationRoleStatus.ACTIVE
+        role.user = user
 
         db.session.add(role)
         db.session.commit()

atst/domain/invitations.py

@@ -55,7 +55,7 @@ class BaseInvitations(object):
         return invite
 
     @classmethod
-    def create(cls, inviter, role, email):
+    def create(cls, inviter, role, member_data, commit=False):
         # pylint: disable=not-callable
         invite = cls.model(
             role=role,
@@ -63,9 +63,15 @@ class BaseInvitations(object):
             user=role.user,
             status=InvitationStatus.PENDING,
             expiration_time=cls.current_expiration_time(),
-            email=email,
+            email=member_data.get("email"),
+            dod_id=member_data.get("dod_id"),
+            first_name=member_data.get("first_name"),
+            phone_number=member_data.get("phone_number"),
+            last_name=member_data.get("last_name"),
         )
         db.session.add(invite)
+
+        if commit:
             db.session.commit()
 
         return invite
@@ -74,7 +80,7 @@ class BaseInvitations(object):
     def accept(cls, user, token):
         invite = cls._get(token)
 
-        if invite.user.dod_id != user.dod_id:
+        if invite.dod_id != user.dod_id:
             if invite.is_pending:
                 cls._update_status(invite, InvitationStatus.REJECTED_WRONG_USER)
             raise WrongUserError(user, invite)
@@ -88,7 +94,7 @@ class BaseInvitations(object):
 
         elif invite.is_pending:  # pragma: no branch
             cls._update_status(invite, InvitationStatus.ACCEPTED)
-            cls.role_domain_class.enable(invite.role)
+            cls.role_domain_class.enable(invite.role, user)
             return invite
 
     @classmethod
@@ -111,20 +117,22 @@ class BaseInvitations(object):
         return cls._update_status(invite, InvitationStatus.REVOKED)
 
     @classmethod
-    def lookup_by_resource_and_user(cls, resource, user):
+    def resend(cls, inviter, token):
-        role = cls.role_domain_class.get(resource.id, user.id)
-
-        if role.latest_invitation is None:
-            raise NotFoundError(cls.model.__tablename__)
-
-        return role.latest_invitation
-
-    @classmethod
-    def resend(cls, user, token):
         previous_invitation = cls._get(token)
         cls._update_status(previous_invitation, InvitationStatus.REVOKED)
 
-        return cls.create(user, previous_invitation.role, previous_invitation.email)
+        return cls.create(
+            inviter,
+            previous_invitation.role,
+            {
+                "email": previous_invitation.email,
+                "dod_id": previous_invitation.dod_id,
+                "first_name": previous_invitation.first_name,
+                "last_name": previous_invitation.last_name,
+                "phone_number": previous_invitation.last_name,
+            },
+            commit=True,
+        )
 
 
 class PortfolioInvitations(BaseInvitations):

atst/domain/portfolio_roles.py

@@ -163,8 +163,9 @@ class PortfolioRoles(object):
         return portfolio_role
 
     @classmethod
-    def enable(cls, portfolio_role):
+    def enable(cls, portfolio_role, user):
         portfolio_role.status = PortfolioRoleStatus.ACTIVE
+        portfolio_role.user = user
 
         db.session.add(portfolio_role)
         db.session.commit()

atst/domain/portfolios/portfolios.py

@@ -1,10 +1,9 @@
 from atst.domain.permission_sets import PermissionSets
 from atst.domain.authz import Authorization
-from atst.models.permissions import Permissions
-from atst.domain.users import Users
 from atst.domain.portfolio_roles import PortfolioRoles
+from atst.domain.invitations import PortfolioInvitations
 from atst.domain.environments import Environments
-from atst.models.portfolio_role import Status as PortfolioRoleStatus
+from atst.models import Permissions, PortfolioRole, PortfolioRoleStatus
 
 from .query import PortfoliosQuery
 from .scopes import ScopedPortfolio
@@ -49,25 +48,26 @@ class Portfolios(object):
             portfolios = PortfoliosQuery.get_for_user(user)
         return portfolios
 
-    @classmethod
-    def create_member(cls, portfolio, data):
-        new_user = Users.get_or_create_by_dod_id(
-            data["dod_id"],
-            first_name=data["first_name"],
-            last_name=data["last_name"],
-            email=data["email"],
-            provisional=True,
-        )
-        permission_sets = data.get("permission_sets", [])
-        return Portfolios.add_member(
-            portfolio, new_user, permission_sets=permission_sets
-        )
-
     @classmethod
     def add_member(cls, portfolio, member, permission_sets=None):
         portfolio_role = PortfolioRoles.add(member, portfolio.id, permission_sets)
         return portfolio_role
 
+    @classmethod
+    def invite(cls, portfolio, inviter, member_data):
+        permission_sets = PortfolioRoles._permission_sets_for_names(
+            member_data.get("permission_sets", [])
+        )
+        role = PortfolioRole(portfolio_id=portfolio.id, permission_sets=permission_sets)
+
+        invitation = PortfolioInvitations.create(
+            inviter=inviter, role=role, member_data=member_data
+        )
+
+        PortfoliosQuery.add_and_commit(role)
+
+        return invitation
+
     @classmethod
     def update_member(cls, member, permission_sets):
         return PortfolioRoles.update(member, permission_sets)

atst/models/application_role.py

@@ -42,7 +42,7 @@ class ApplicationRole(
     application = relationship("Application", back_populates="roles")
 
     user_id = Column(
-        UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False
+        UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=True
     )
 
     status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING)

atst/models/mixins/invites.py

@@ -45,6 +45,11 @@ class InvitesMixin(object):
 
     email = Column(String, nullable=False)
 
+    dod_id = Column(String)
+    first_name = Column(String)
+    last_name = Column(String)
+    phone_number = Column(String)
+
     def __repr__(self):
         role_id = self.role.id if self.role else None
         return "<{}(user='{}', role='{}', id='{}', email='{}')>".format(
@@ -92,7 +97,7 @@ class InvitesMixin(object):
 
     @property
     def user_name(self):
-        return self.role.user.full_name
+        return "{} {}".format(self.first_name, self.last_name)
 
     @property
     def is_revokable(self):

atst/models/portfolio_role.py

@@ -48,7 +48,7 @@ class PortfolioRole(
     portfolio = relationship("Portfolio", back_populates="roles")
 
     user_id = Column(
-        UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False
+        UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=True
     )
 
     status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING)
@@ -116,7 +116,14 @@ class PortfolioRole(
 
     @property
     def user_name(self):
+        if self.user:
             return self.user.full_name
+        else:
+            return self.latest_invitation.user_name
+
+    @property
+    def full_name(self):
+        return self.user_name
 
     @property
     def is_active(self):
@@ -128,10 +135,6 @@ class PortfolioRole(
             self.latest_invitation and self.latest_invitation.is_inactive
         )
 
-    @property
-    def full_name(self):
-        return self.user.full_name
-
     @property
     def application_id(self):
         return None

atst/routes/portfolios/__init__.py

@@ -4,7 +4,6 @@ from operator import attrgetter
 portfolios_bp = Blueprint("portfolios", __name__)
 
 from . import index
-from . import members
 from . import invitations
 from . import admin
 from atst.utils.context_processors import portfolio as portfolio_context_processor

atst/routes/portfolios/invitations.py

@@ -1,20 +1,23 @@
-from flask import g, redirect, url_for, render_template
+from flask import g, redirect, url_for, render_template, request as http_request
 
 from . import portfolios_bp
+from atst.domain.authz.decorator import user_can_access_decorator as user_can
+from atst.domain.exceptions import AlreadyExistsError
 from atst.domain.invitations import PortfolioInvitations
+from atst.domain.portfolios import Portfolios
+from atst.models import Permissions
 from atst.queue import queue
 from atst.utils.flash import formatted_flash as flash
-from atst.domain.authz.decorator import user_can_access_decorator as user_can
+import atst.forms.portfolio_member as member_forms
-from atst.models.permissions import Permissions
 
 
-def send_invite_email(owner_name, token, new_member_email):
+def send_portfolio_invitation(invitee_email, inviter_name, token):
     body = render_template(
-        "emails/portfolio/invitation.txt", owner=owner_name, token=token
+        "emails/portfolio/invitation.txt", owner=inviter_name, token=token
     )
     queue.send_mail(
-        [new_member_email],
+        [invitee_email],
-        "{} has invited you to a JEDI Cloud Portfolio".format(owner_name),
+        "{} has invited you to a JEDI cloud portfolio".format(inviter_name),
         body,
     )
 
@@ -23,12 +26,6 @@ def send_invite_email(owner_name, token, new_member_email):
 def accept_invitation(portfolio_token):
     invite = PortfolioInvitations.accept(g.current_user, portfolio_token)
 
-    for task_order in invite.portfolio.task_orders:
-        if g.current_user in task_order.officers:
-            return redirect(
-                url_for("task_orders.view_task_order", task_order_id=task_order.id)
-            )
-
     return redirect(
         url_for("applications.portfolio_applications", portfolio_id=invite.portfolio.id)
     )
@@ -57,7 +54,7 @@ def revoke_invitation(portfolio_id, portfolio_token):
 @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="resend invitation")
 def resend_invitation(portfolio_id, portfolio_token):
     invite = PortfolioInvitations.resend(g.current_user, portfolio_token)
-    send_invite_email(g.current_user.full_name, invite.token, invite.email)
+    send_portfolio_invitation(invite.email, g.current_user.full_name, invite.token)
     flash("resend_portfolio_invitation", user_name=invite.user_name)
     return redirect(
         url_for(
@@ -67,3 +64,38 @@ def resend_invitation(portfolio_id, portfolio_token):
             _anchor="portfolio-members",
         )
     )
+
+
+@portfolios_bp.route("/portfolios/<portfolio_id>/members/new", methods=["POST"])
+@user_can(Permissions.CREATE_PORTFOLIO_USERS, message="create new portfolio member")
+def invite_member(portfolio_id):
+    portfolio = Portfolios.get(g.current_user, portfolio_id)
+    form = member_forms.NewForm(http_request.form)
+
+    if form.validate():
+        try:
+            invite = Portfolios.invite(portfolio, g.current_user, form.update_data)
+            send_portfolio_invitation(
+                invite.email, g.current_user.full_name, invite.token
+            )
+
+            flash(
+                "new_portfolio_member", user_name=invite.user_name, portfolio=portfolio
+            )
+
+        except AlreadyExistsError:
+            return render_template(
+                "error.html", message="There was an error processing your request."
+            )
+    else:
+        pass
+        # TODO: flash error message
+
+    return redirect(
+        url_for(
+            "portfolios.admin",
+            portfolio_id=portfolio_id,
+            fragment="portfolio-members",
+            _anchor="portfolio-members",
+        )
+    )

atst/routes/portfolios/members.py

@@ -1,45 +0,0 @@
-from flask import render_template, request as http_request, g, redirect, url_for
-
-from . import portfolios_bp
-from atst.domain.exceptions import AlreadyExistsError
-from atst.domain.portfolios import Portfolios
-from atst.services.invitation import Invitation as InvitationService
-import atst.forms.portfolio_member as member_forms
-from atst.domain.authz.decorator import user_can_access_decorator as user_can
-from atst.models.permissions import Permissions
-
-from atst.utils.flash import formatted_flash as flash
-
-
-@portfolios_bp.route("/portfolios/<portfolio_id>/members/new", methods=["POST"])
-@user_can(Permissions.CREATE_PORTFOLIO_USERS, message="create new portfolio member")
-def create_member(portfolio_id):
-    portfolio = Portfolios.get(g.current_user, portfolio_id)
-    form = member_forms.NewForm(http_request.form)
-
-    if form.validate():
-        try:
-            member = Portfolios.create_member(portfolio, form.update_data)
-            invite_service = InvitationService(
-                g.current_user, member, form.update_data.get("email")
-            )
-            invite_service.invite()
-
-            flash("new_portfolio_member", new_member=member, portfolio=portfolio)
-
-        except AlreadyExistsError:
-            return render_template(
-                "error.html", message="There was an error processing your request."
-            )
-    else:
-        pass
-        # TODO: flash error message
-
-    return redirect(
-        url_for(
-            "portfolios.admin",
-            portfolio_id=portfolio_id,
-            fragment="portfolio-members",
-            _anchor="portfolio-members",
-        )
-    )

atst/services/invitation.py

@@ -79,7 +79,13 @@ class Invitation:
         return invite
 
     def _create_invite(self):
-        return self.domain_class.create(self.inviter, self.member, self.email)
+        user = self.member.user
+        return self.domain_class.create(
+            self.inviter,
+            self.member,
+            {"email": self.email, "dod_id": user.dod_id},
+            commit=True,
+        )
 
     def _send_invite_email(self, token):
         body = render_template(

atst/utils/flash.py

@@ -61,7 +61,7 @@ MESSAGES = {
     "new_portfolio_member": {
         "title_template": translate("flash.success"),
         "message_template": """
-          <p>{{ "flash.new_portfolio_member" | translate({ "user_name": new_member.user_name }) }}</p>
+          <p>{{ "flash.new_portfolio_member" | translate({ "user_name": user_name }) }}</p>
         """,
         "category": "success",
     },

script/seed_sample.py

@@ -152,9 +152,9 @@ def get_users():
 
 def add_members_to_portfolio(portfolio):
     for user_data in PORTFOLIO_USERS:
-        ws_role = Portfolios.create_member(portfolio, user_data)
+        invite = Portfolios.invite(portfolio, portfolio.owner, user_data)
-        db.session.refresh(ws_role)
+        user = Users.get_or_create_by_dod_id(user_data["dod_id"])
-        PortfolioRoles.enable(ws_role)
+        PortfolioRoles.enable(invite.role, user)
 
     db.session.commit()
 

templates/fragments/admin/add_new_portfolio_member.html

@@ -78,6 +78,6 @@
 {{ MultiStepModalForm(
     'add-port-mem',
     member_form,
-    url_for("portfolios.create_member", portfolio_id=portfolio.id),
+    url_for("portfolios.invite_member", portfolio_id=portfolio.id),
     [step_one, step_two],
   ) }}

tests/domain/test_application_roles.py

@@ -33,7 +33,7 @@ def test_enabled_application_role():
     )
     assert app_role.status == ApplicationRoleStatus.DISABLED
 
-    ApplicationRoles.enable(app_role)
+    ApplicationRoles.enable(app_role, app_role.user)
 
     assert app_role.status == ApplicationRoleStatus.ACTIVE
 

tests/domain/test_invitations.py

@@ -23,10 +23,11 @@ from tests.factories import (
 def test_create_invitation():
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    invite = PortfolioInvitations.create(
-    assert invite.user == user
+        portfolio.owner, role, user.to_dictionary(), commit=True
-    assert invite.role == ws_role
+    )
+    assert invite.role == role
     assert invite.inviter == portfolio.owner
     assert invite.status == InvitationStatus.PENDING
     assert re.match(r"^[\w\-_]+$", invite.token)
@@ -35,8 +36,10 @@ def test_create_invitation():
 def test_accept_invitation():
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    invite = PortfolioInvitations.create(
+        portfolio.owner, role, user.to_dictionary(), commit=True
+    )
     assert invite.is_pending
     accepted_invite = PortfolioInvitations.accept(user, invite.token)
     assert accepted_invite.is_accepted
@@ -45,14 +48,14 @@ def test_accept_invitation():
 def test_accept_expired_invitation():
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
     increment = PortfolioInvitations.EXPIRATION_LIMIT_MINUTES + 1
     expiration_time = datetime.datetime.now() - datetime.timedelta(minutes=increment)
     invite = PortfolioInvitationFactory.create(
-        user=user,
         expiration_time=expiration_time,
         status=InvitationStatus.PENDING,
-        role=ws_role,
+        role=role,
+        dod_id=user.dod_id,
     )
     with pytest.raises(ExpiredError):
         PortfolioInvitations.accept(user, invite.token)
@@ -63,9 +66,9 @@ def test_accept_expired_invitation():
 def test_accept_rejected_invite():
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
     invite = PortfolioInvitationFactory.create(
-        user=user, status=InvitationStatus.REJECTED_EXPIRED, role=ws_role
+        status=InvitationStatus.REJECTED_EXPIRED, role=role, dod_id=user.dod_id
     )
     with pytest.raises(InvitationError):
         PortfolioInvitations.accept(user, invite.token)
@@ -74,9 +77,9 @@ def test_accept_rejected_invite():
 def test_accept_revoked_invite():
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
     invite = PortfolioInvitationFactory.create(
-        user=user, status=InvitationStatus.REVOKED, role=ws_role
+        status=InvitationStatus.REVOKED, role=role, dod_id=user.dod_id
     )
     with pytest.raises(InvitationError):
         PortfolioInvitations.accept(user, invite.token)
@@ -85,9 +88,9 @@ def test_accept_revoked_invite():
 def test_wrong_user_accepts_invitation():
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
     wrong_user = UserFactory.create()
-    invite = PortfolioInvitationFactory.create(user=user, role=ws_role)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
     with pytest.raises(WrongUserError):
         PortfolioInvitations.accept(wrong_user, invite.token)
 
@@ -95,9 +98,9 @@ def test_wrong_user_accepts_invitation():
 def test_user_cannot_accept_invitation_accepted_by_wrong_user():
     user = UserFactory.create()
     portfolio = PortfolioFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
     wrong_user = UserFactory.create()
-    invite = PortfolioInvitationFactory.create(user=user, role=ws_role)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
     with pytest.raises(WrongUserError):
         PortfolioInvitations.accept(wrong_user, invite.token)
     with pytest.raises(InvitationError):
@@ -107,8 +110,8 @@ def test_user_cannot_accept_invitation_accepted_by_wrong_user():
 def test_accept_invitation_twice():
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
     PortfolioInvitations.accept(user, invite.token)
     with pytest.raises(InvitationError):
         PortfolioInvitations.accept(user, invite.token)
@@ -117,44 +120,31 @@ def test_accept_invitation_twice():
 def test_revoke_invitation():
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
     assert invite.is_pending
     PortfolioInvitations.revoke(invite.token)
     assert invite.is_revoked
 
 
-def test_resend_invitation():
+def test_resend_invitation(session):
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    first_invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
-    PortfolioInvitations.resend(user, invite.token)
+    assert first_invite.is_pending
-    assert ws_role.invitations[0].is_revoked
+    second_invite = PortfolioInvitations.resend(user, first_invite.token)
-    assert ws_role.invitations[1].is_pending
+    assert first_invite.is_revoked
+    assert second_invite.is_pending
 
 
 def test_audit_event_for_accepted_invite():
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user.dod_id)
     invite = PortfolioInvitations.accept(user, invite.token)
 
     accepted_event = AuditLog.get_by_resource(invite.id)[0]
     assert "email" in accepted_event.event_details
     assert "dod_id" in accepted_event.event_details
-
-
-def test_lookup_by_user_and_portfolio():
-    portfolio = PortfolioFactory.create()
-    user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio)
-    invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email)
-
-    assert PortfolioInvitations.lookup_by_resource_and_user(portfolio, user) == invite
-
-    with pytest.raises(NotFoundError):
-        PortfolioInvitations.lookup_by_resource_and_user(
-            portfolio, UserFactory.create()
-        )

tests/domain/test_portfolios.py

@@ -49,36 +49,6 @@ def test_portfolio_has_timestamps(portfolio):
     assert portfolio.time_created == portfolio.time_updated
 
 
-def test_can_create_portfolio_role(portfolio, portfolio_owner):
-    user_data = {
-        "first_name": "New",
-        "last_name": "User",
-        "email": "new.user@mail.com",
-        "portfolio_role": "developer",
-        "dod_id": "1234567890",
-    }
-
-    new_member = Portfolios.create_member(portfolio, user_data)
-    assert new_member.portfolio == portfolio
-    assert new_member.user.provisional
-
-
-def test_can_add_existing_user_to_portfolio(portfolio, portfolio_owner):
-    user = UserFactory.create()
-    user_data = {
-        "first_name": "New",
-        "last_name": "User",
-        "email": "new.user@mail.com",
-        "portfolio_role": "developer",
-        "dod_id": user.dod_id,
-    }
-
-    new_member = Portfolios.create_member(portfolio, user_data)
-    assert new_member.portfolio == portfolio
-    assert new_member.user.email == user.email
-    assert not new_member.user.provisional
-
-
 def test_update_portfolio_role_role(portfolio, portfolio_owner):
     user_data = {
         "first_name": "New",
@@ -238,3 +208,16 @@ def test_does_not_count_disabled_members(session):
     )
 
     assert portfolio.user_count == 3
+
+
+def test_invite():
+    portfolio = PortfolioFactory.create()
+    inviter = UserFactory.create()
+    member_data = UserFactory.dictionary()
+
+    invitation = Portfolios.invite(portfolio, inviter, member_data)
+
+    assert invitation.role
+    assert invitation.role.portfolio == portfolio
+    assert invitation.role.user is None
+    assert invitation.dod_id == member_data["dod_id"]

tests/models/test_portfolio_role.py

@@ -65,7 +65,10 @@ def test_has_portfolio_status_history(session):
     #  to commit after create()
     PortfolioRoleFactory._meta.sqlalchemy_session_persistence = "flush"
     portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=user)
-    PortfolioRoles.enable(portfolio_role)
+    portfolio_role.status = PortfolioRoleStatus.ACTIVE
+    session.add(portfolio_role)
+    session.commit()
+
     changed_events = (
         session.query(AuditEvent)
         .filter(

tests/routes/applications/test_invitations.py

@@ -8,7 +8,7 @@ def test_accept_application_invitation(client, user_session):
     application = ApplicationFactory.create()
     app_role = ApplicationRoleFactory.create(application=application, user=user)
     invite = ApplicationInvitationFactory.create(
-        role=app_role, user=user, inviter=application.portfolio.owner
+        role=app_role, inviter=application.portfolio.owner, dod_id=user.dod_id
     )
 
     user_session(user)
@@ -28,7 +28,7 @@ def test_accept_application_invitation_end_to_end(client, user_session):
     application = ApplicationFactory.create(name="Millenium Falcon")
     app_role = ApplicationRoleFactory.create(application=application, user=user)
     invite = ApplicationInvitationFactory.create(
-        role=app_role, user=user, inviter=application.portfolio.owner
+        role=app_role, dod_id=user.dod_id, inviter=application.portfolio.owner
     )
 
     user_session(user)

tests/routes/portfolios/test_invitations.py

@@ -1,27 +1,21 @@
-import pytest
 import datetime
 from flask import url_for
 
-from tests.factories import (
-    UserFactory,
-    PortfolioFactory,
-    PortfolioRoleFactory,
-    PortfolioInvitationFactory,
-    TaskOrderFactory,
-)
 from atst.domain.portfolios import Portfolios
 from atst.models import InvitationStatus, PortfolioRoleStatus
-from atst.domain.users import Users
 from atst.domain.permission_sets import PermissionSets
+from atst.queue import queue
+
+from tests.factories import *
 
 
 def test_existing_member_accepts_valid_invite(client, user_session):
     portfolio = PortfolioFactory.create()
     user = UserFactory.create()
-    ws_role = PortfolioRoleFactory.create(
+    role = PortfolioRoleFactory.create(
         portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING
     )
-    invite = PortfolioInvitationFactory.create(user_id=user.id, role=ws_role)
+    invite = PortfolioInvitationFactory.create(dod_id=user.dod_id, role=role)
 
     # the user does not have access to the portfolio before accepting the invite
     assert len(Portfolios.for_user(user)) == 0
@@ -46,32 +40,15 @@ def test_existing_member_accepts_valid_invite(client, user_session):
 def test_new_member_accepts_valid_invite(monkeypatch, client, user_session):
     portfolio = PortfolioFactory.create()
     user_info = UserFactory.dictionary()
-
+    role = PortfolioRoleFactory.create(portfolio=portfolio)
-    user_session(portfolio.owner)
+    invite = PortfolioInvitationFactory.create(role=role, dod_id=user_info["dod_id"])
-    response = client.post(
-        url_for("portfolios.create_member", portfolio_id=portfolio.id),
-        data={
-            "permission_sets-perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT,
-            "permission_sets-perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING,
-            "permission_sets-perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS,
-            "permission_sets-perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN,
-            "user_data-first_name": user_info["first_name"],
-            "user_data-last_name": user_info["last_name"],
-            "user_data-dod_id": user_info["dod_id"],
-            "user_data-email": user_info["email"],
-        },
-    )
-
-    assert response.status_code == 302
-    user = Users.get_by_dod_id(user_info["dod_id"])
-    token = user.portfolio_invitations[0].token
 
     monkeypatch.setattr(
         "atst.domain.auth.should_redirect_to_user_profile", lambda *args: False
     )
-    user_session(user)
+    user_session(UserFactory.create(dod_id=user_info["dod_id"]))
     response = client.get(
-        url_for("portfolios.accept_invitation", portfolio_token=token)
+        url_for("portfolios.accept_invitation", portfolio_token=invite.token)
     )
 
     # user is redirected to the portfolio view
@@ -81,7 +58,8 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session):
         in response.headers["Location"]
     )
     # the user has access to the portfolio
-    assert len(Portfolios.for_user(user)) == 1
+    assert role.user.dod_id == user_info["dod_id"]
+    assert len(role.user.portfolio_roles) == 1
 
 
 def test_member_accepts_invalid_invite(client, user_session):
@@ -108,7 +86,7 @@ def test_user_who_has_not_accepted_portfolio_invite_cannot_view(client, user_ses
     # create user in portfolio with invitation
     user_session(portfolio.owner)
     response = client.post(
-        url_for("portfolios.create_member", portfolio_id=portfolio.id),
+        url_for("portfolios.invite_member", portfolio_id=portfolio.id),
         data=user.to_dictionary(),
     )
 
@@ -279,3 +257,55 @@ def test_existing_member_invite_resent_to_email_submitted_in_form(
     assert user.email != "example@example.com"
     assert send_mail_job.func.__func__.__name__ == "_send_mail"
     assert send_mail_job.args[0] == ["example@example.com"]
+
+
+_DEFAULT_PERMS_FORM_DATA = {
+    "permission_sets-perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT,
+    "permission_sets-perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING,
+    "permission_sets-perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS,
+    "permission_sets-perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN,
+}
+
+
+def test_user_with_permission_has_add_member_link(client, user_session):
+    portfolio = PortfolioFactory.create()
+    user_session(portfolio.owner)
+    response = client.get(url_for("portfolios.admin", portfolio_id=portfolio.id))
+    assert response.status_code == 200
+    assert (
+        url_for("portfolios.invite_member", portfolio_id=portfolio.id).encode()
+        in response.data
+    )
+
+
+def test_invite_member(client, user_session, session):
+    user_data = UserFactory.dictionary()
+    portfolio = PortfolioFactory.create()
+    user_session(portfolio.owner)
+    queue_length = len(queue.get_queue())
+
+    response = client.post(
+        url_for("portfolios.invite_member", portfolio_id=portfolio.id),
+        data={
+            "user_data-dod_id": user_data.get("dod_id"),
+            "user_data-first_name": user_data.get("first_name"),
+            "user_data-last_name": user_data.get("last_name"),
+            "user_data-email": user_data.get("email"),
+            **_DEFAULT_PERMS_FORM_DATA,
+        },
+        follow_redirects=True,
+    )
+
+    assert response.status_code == 200
+    full_name = "{} {}".format(user_data.get("first_name"), user_data.get("last_name"))
+    assert full_name in response.data.decode()
+
+    invitation = (
+        session.query(PortfolioInvitation)
+        .filter_by(dod_id=user_data.get("dod_id"))
+        .one()
+    )
+    assert invitation.role.portfolio == portfolio
+
+    assert len(queue.get_queue()) == queue_length + 1
+    assert len(invitation.role.permission_sets) == 5

tests/routes/portfolios/test_members.py

@@ -1,51 +0,0 @@
-from flask import url_for
-
-from tests.factories import UserFactory, PortfolioFactory
-from atst.domain.permission_sets import PermissionSets
-from atst.queue import queue
-
-_DEFAULT_PERMS_FORM_DATA = {
-    "permission_sets-perms_app_mgmt": PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT,
-    "permission_sets-perms_funding": PermissionSets.VIEW_PORTFOLIO_FUNDING,
-    "permission_sets-perms_reporting": PermissionSets.VIEW_PORTFOLIO_REPORTS,
-    "permission_sets-perms_portfolio_mgmt": PermissionSets.VIEW_PORTFOLIO_ADMIN,
-}
-
-
-def test_user_with_permission_has_add_member_link(client, user_session):
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-    response = client.get(url_for("portfolios.admin", portfolio_id=portfolio.id))
-    assert response.status_code == 200
-    assert (
-        url_for("portfolios.create_member", portfolio_id=portfolio.id).encode()
-        in response.data
-    )
-
-
-def test_create_member(client, user_session):
-    user = UserFactory.create()
-    portfolio = PortfolioFactory.create()
-    user_session(portfolio.owner)
-    queue_length = len(queue.get_queue())
-
-    response = client.post(
-        url_for("portfolios.create_member", portfolio_id=portfolio.id),
-        data={
-            "user_data-dod_id": user.dod_id,
-            "user_data-first_name": "user_data-Wilbur",
-            "user_data-last_name": "user_data-Zuckerman",
-            "user_data-email": "user_data-some_pig@zuckermans.com",
-            "user_data-portfolio_role": "user_data-developer",
-            **_DEFAULT_PERMS_FORM_DATA,
-        },
-        follow_redirects=True,
-    )
-
-    assert response.status_code == 200
-    assert user.full_name in response.data.decode()
-    assert user.has_portfolios
-    assert user.portfolio_invitations
-    assert len(queue.get_queue()) == queue_length + 1
-    portfolio_role = user.portfolio_roles[0]
-    assert len(portfolio_role.permission_sets) == 5

tests/test_access.py

@@ -197,14 +197,14 @@ def test_applications_update_team_env_roles(post_url_assert_status):
     post_url_assert_status(rando, url, 404)
 
 
-# portfolios.create_member
+# portfolios.invite_member
-def test_portfolios_create_member_access(post_url_assert_status):
+def test_portfolios_invite_member_access(post_url_assert_status):
     ccpo = user_with(PermissionSets.EDIT_PORTFOLIO_ADMIN)
     owner = user_with()
     rando = user_with()
     portfolio = PortfolioFactory.create(owner=owner)
 
-    url = url_for("portfolios.create_member", portfolio_id=portfolio.id)
+    url = url_for("portfolios.invite_member", portfolio_id=portfolio.id)
     post_url_assert_status(ccpo, url, 302)
     post_url_assert_status(owner, url, 302)
     post_url_assert_status(rando, url, 404)