From c4ad7b43785f5bc2d17b0432cad6b13e6c8de6ef Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 22 Apr 2019 14:54:37 -0400 Subject: [PATCH 01/14] Make portfolio invitation specific to portfolio - add a base domain class - extract shared model code to mixin - rename invitation classes - invitation model relationship to portfolio_role name is now more generic "role" --- atst/domain/authz/decorator.py | 6 +- atst/domain/invitations.py | 52 +++++++------ atst/models/__init__.py | 8 +- atst/models/mixins/__init__.py | 1 + .../{invitation.py => mixins/invites.py} | 65 +++++----------- atst/models/portfolio_invitation.py | 41 ++++++++++ atst/models/user.py | 8 ++ atst/routes/portfolios/invitations.py | 8 +- atst/routes/task_orders/invitations.py | 8 +- atst/services/invitation.py | 4 +- tests/domain/test_invitations.py | 77 ++++++++++--------- tests/domain/test_portfolio_roles.py | 2 +- tests/factories.py | 13 ++-- ...ation.py => test_portfolio_invitations.py} | 16 ++-- tests/models/test_portfolio_role.py | 37 ++++----- tests/routes/portfolios/test_invitations.py | 47 ++++++----- tests/routes/portfolios/test_members.py | 2 +- tests/routes/task_orders/test_invitations.py | 22 +++--- tests/services/test_invitation.py | 2 +- tests/test_access.py | 12 +-- 20 files changed, 228 insertions(+), 203 deletions(-) rename atst/models/{invitation.py => mixins/invites.py} (55%) create mode 100644 atst/models/portfolio_invitation.py rename tests/models/{test_invitation.py => test_portfolio_invitations.py} (72%) diff --git a/atst/domain/authz/decorator.py b/atst/domain/authz/decorator.py index 8c02718a..5e0b876e 100644 --- a/atst/domain/authz/decorator.py +++ b/atst/domain/authz/decorator.py @@ -6,8 +6,8 @@ from . import user_can_access from atst.domain.portfolios import Portfolios from atst.domain.task_orders import TaskOrders from atst.domain.applications import Applications -from atst.domain.invitations import Invitations from atst.domain.environments import Environments +from atst.domain.invitations import PortfolioInvitations from atst.domain.exceptions import UnauthorizedError @@ -24,8 +24,8 @@ def check_access(permission, message, override, *args, **kwargs): access_args["portfolio"] = task_order.portfolio elif "token" in kwargs: - invite = Invitations._get(kwargs["token"]) - access_args["portfolio"] = invite.portfolio_role.portfolio + invite = PortfolioInvitations._get(kwargs["token"]) + access_args["portfolio"] = invite.role.portfolio elif "portfolio_id" in kwargs: access_args["portfolio"] = Portfolios.get( diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index c4d51248..cfedde11 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -2,7 +2,7 @@ import datetime from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.models.invitation import Invitation, Status as InvitationStatus +from atst.models import InvitationStatus, PortfolioInvitation from atst.domain.portfolio_roles import PortfolioRoles from .exceptions import NotFoundError @@ -38,27 +38,29 @@ class InvitationError(Exception): return "{} has a status of {}".format(self.invite.id, self.invite.status.value) -class Invitations(object): +class BaseInvitations(object): + model = None # number of minutes a given invitation is considered valid EXPIRATION_LIMIT_MINUTES = 360 @classmethod def _get(cls, token): try: - invite = db.session.query(Invitation).filter_by(token=token).one() + invite = db.session.query(cls.model).filter_by(token=token).one() except NoResultFound: raise NotFoundError("invite") return invite @classmethod - def create(cls, inviter, portfolio_role, email): - invite = Invitation( - portfolio_role=portfolio_role, + def create(cls, inviter, role, email): + # pylint: disable=not-callable + invite = cls.model( + role=role, inviter=inviter, - user=portfolio_role.user, + user=role.user, status=InvitationStatus.PENDING, - expiration_time=Invitations.current_expiration_time(), + expiration_time=cls.current_expiration_time(), email=email, ) db.session.add(invite) @@ -68,29 +70,29 @@ class Invitations(object): @classmethod def accept(cls, user, token): - invite = Invitations._get(token) + invite = cls._get(token) if invite.user.dod_id != user.dod_id: if invite.is_pending: - Invitations._update_status(invite, InvitationStatus.REJECTED_WRONG_USER) + cls._update_status(invite, InvitationStatus.REJECTED_WRONG_USER) raise WrongUserError(user, invite) elif invite.is_expired: - Invitations._update_status(invite, InvitationStatus.REJECTED_EXPIRED) + cls._update_status(invite, InvitationStatus.REJECTED_EXPIRED) raise ExpiredError(invite) elif invite.is_accepted or invite.is_revoked or invite.is_rejected: raise InvitationError(invite) elif invite.is_pending: # pragma: no branch - Invitations._update_status(invite, InvitationStatus.ACCEPTED) - PortfolioRoles.enable(invite.portfolio_role) + cls._update_status(invite, InvitationStatus.ACCEPTED) + PortfolioRoles.enable(invite.role) return invite @classmethod def current_expiration_time(cls): return datetime.datetime.now() + datetime.timedelta( - minutes=Invitations.EXPIRATION_LIMIT_MINUTES + minutes=cls.EXPIRATION_LIMIT_MINUTES ) @classmethod @@ -103,23 +105,25 @@ class Invitations(object): @classmethod def revoke(cls, token): - invite = Invitations._get(token) - return Invitations._update_status(invite, InvitationStatus.REVOKED) + invite = cls._get(token) + return cls._update_status(invite, InvitationStatus.REVOKED) @classmethod def lookup_by_portfolio_and_user(cls, portfolio, user): - portfolio_role = PortfolioRoles.get(portfolio.id, user.id) + role = PortfolioRoles.get(portfolio.id, user.id) - if portfolio_role.latest_invitation is None: + if role.latest_invitation is None: raise NotFoundError("invitation") - return portfolio_role.latest_invitation + return role.latest_invitation @classmethod def resend(cls, user, token): - previous_invitation = Invitations._get(token) - Invitations._update_status(previous_invitation, InvitationStatus.REVOKED) + previous_invitation = cls._get(token) + cls._update_status(previous_invitation, InvitationStatus.REVOKED) - return Invitations.create( - user, previous_invitation.portfolio_role, previous_invitation.email - ) + return cls.create(user, previous_invitation.role, previous_invitation.email) + + +class PortfolioInvitations(BaseInvitations): + model = PortfolioInvitation diff --git a/atst/models/__init__.py b/atst/models/__init__.py index 85130ceb..0369bc07 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -5,14 +5,16 @@ Base = declarative_base() from .permissions import Permissions from .permission_set import PermissionSet from .user import User -from .portfolio_role import PortfolioRole -from .application_role import ApplicationRole +from .portfolio_role import PortfolioRole, Status as PortfolioRoleStatus +from .application_role import ApplicationRole, Status as ApplicationRoleStatus from .environment_role import EnvironmentRole from .portfolio import Portfolio from .application import Application from .environment import Environment from .attachment import Attachment from .audit_event import AuditEvent -from .invitation import Invitation +from .portfolio_invitation import PortfolioInvitation from .task_order import TaskOrder from .dd_254 import DD254 + +from .mixins.invites import Status as InvitationStatus diff --git a/atst/models/mixins/__init__.py b/atst/models/mixins/__init__.py index 54c85d61..dd6d0faf 100644 --- a/atst/models/mixins/__init__.py +++ b/atst/models/mixins/__init__.py @@ -2,3 +2,4 @@ from .timestamps import TimestampsMixin from .auditable import AuditableMixin from .permissions import PermissionsMixin from .deletable import DeletableMixin +from .invites import InvitesMixin diff --git a/atst/models/invitation.py b/atst/models/mixins/invites.py similarity index 55% rename from atst/models/invitation.py rename to atst/models/mixins/invites.py index 62628d0a..39bedc08 100644 --- a/atst/models/invitation.py +++ b/atst/models/mixins/invites.py @@ -3,12 +3,11 @@ from enum import Enum import secrets from sqlalchemy import Column, ForeignKey, Enum as SQLAEnum, TIMESTAMP, String +from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import relationship, backref +from sqlalchemy.orm import relationship -from atst.models import Base, types -from atst.models.mixins.timestamps import TimestampsMixin -from atst.models.mixins.auditable import AuditableMixin +from atst.models import types class Status(Enum): @@ -19,24 +18,24 @@ class Status(Enum): REJECTED_EXPIRED = "rejected_expired" -class Invitation(Base, TimestampsMixin, AuditableMixin): - __tablename__ = "invitations" - +class InvitesMixin(object): id = types.Id() - user_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) - user = relationship("User", backref="invitations", foreign_keys=[user_id]) + @declared_attr + def user_id(cls): + return Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) - portfolio_role_id = Column( - UUID(as_uuid=True), ForeignKey("portfolio_roles.id"), index=True - ) - portfolio_role = relationship( - "PortfolioRole", - backref=backref("invitations", order_by="Invitation.time_created"), - ) + @declared_attr + def user(cls): + return relationship("User", foreign_keys=[cls.user_id]) - inviter_id = Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) - inviter = relationship("User", backref="sent_invites", foreign_keys=[inviter_id]) + @declared_attr + def inviter_id(cls): + return Column(UUID(as_uuid=True), ForeignKey("users.id"), index=True) + + @declared_attr + def inviter(cls): + return relationship("User", foreign_keys=[cls.inviter_id]) status = Column(SQLAEnum(Status, native_enum=False, default=Status.PENDING)) @@ -47,8 +46,9 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): email = Column(String, nullable=False) def __repr__(self): - return "".format( - self.user_id, self.portfolio_role_id, self.id, self.email + role_id = self.role.id if self.role else None + return "<{}(user='{}', role='{}', id='{}', email='{}')>".format( + self.__class__.__name__, self.user_id, role_id, self.id, self.email ) @property @@ -90,14 +90,9 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): Status.REVOKED, ] - @property - def portfolio(self): - if self.portfolio_role: # pragma: no branch - return self.portfolio_role.portfolio - @property def user_name(self): - return self.portfolio_role.user.full_name + return self.role.user.full_name @property def is_revokable(self): @@ -110,21 +105,3 @@ class Invitation(Base, TimestampsMixin, AuditableMixin): @property def user_dod_id(self): return self.user.dod_id if self.user is not None else None - - @property - def event_details(self): - return {"email": self.email, "dod_id": self.user_dod_id} - - @property - def history(self): - changes = self.get_changes() - change_set = {} - - if "status" in changes: - change_set["status"] = [s.name for s in changes["status"]] - - return change_set - - @property - def portfolio_id(self): - return self.portfolio_role.portfolio_id diff --git a/atst/models/portfolio_invitation.py b/atst/models/portfolio_invitation.py new file mode 100644 index 00000000..916e63ae --- /dev/null +++ b/atst/models/portfolio_invitation.py @@ -0,0 +1,41 @@ +from sqlalchemy import Column, ForeignKey +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import relationship, backref + +from atst.models import Base +from atst.models.mixins import TimestampsMixin, AuditableMixin, InvitesMixin + + +class PortfolioInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin): + __tablename__ = "invitations" + + portfolio_role_id = Column( + UUID(as_uuid=True), ForeignKey("portfolio_roles.id"), index=True + ) + role = relationship( + "PortfolioRole", + backref=backref("invitations", order_by="PortfolioInvitation.time_created"), + ) + + @property + def portfolio(self): + if self.role: # pragma: no branch + return self.role.portfolio + + @property + def portfolio_id(self): + return self.role.portfolio_id + + @property + def event_details(self): + return {"email": self.email, "dod_id": self.user_dod_id} + + @property + def history(self): + changes = self.get_changes() + change_set = {} + + if "status" in changes: + change_set["status"] = [s.name for s in changes["status"]] + + return change_set diff --git a/atst/models/user.py b/atst/models/user.py index 83f34326..0510d867 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -4,6 +4,7 @@ from sqlalchemy.dialects.postgresql import UUID from atst.models import Base, types, mixins from atst.models.permissions import Permissions +from atst.models.portfolio_invitation import PortfolioInvitation users_permission_sets = Table( @@ -31,6 +32,13 @@ class User( primaryjoin="and_(ApplicationRole.user_id==User.id, ApplicationRole.deleted==False)", ) + portfolio_invitations = relationship( + "PortfolioInvitation", foreign_keys=PortfolioInvitation.user_id + ) + sent_portfolio_invitations = relationship( + "PortfolioInvitation", foreign_keys=PortfolioInvitation.inviter_id + ) + email = Column(String) dod_id = Column(String, unique=True, nullable=False) first_name = Column(String) diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index e7fd6f92..a34fcee8 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -1,7 +1,7 @@ from flask import g, redirect, url_for, render_template from . import portfolios_bp -from atst.domain.invitations import Invitations +from atst.domain.invitations import PortfolioInvitations 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 @@ -19,7 +19,7 @@ def send_invite_email(owner_name, token, new_member_email): @portfolios_bp.route("/portfolios/invitations/", methods=["GET"]) def accept_invitation(token): - invite = Invitations.accept(g.current_user, token) + invite = PortfolioInvitations.accept(g.current_user, token) for task_order in invite.portfolio.task_orders: if g.current_user in task_order.officers: @@ -37,7 +37,7 @@ def accept_invitation(token): ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="revoke invitation") def revoke_invitation(portfolio_id, token): - Invitations.revoke(token) + PortfolioInvitations.revoke(token) return redirect( url_for( @@ -54,7 +54,7 @@ def revoke_invitation(portfolio_id, token): ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="resend invitation") def resend_invitation(portfolio_id, token): - invite = Invitations.resend(g.current_user, token) + invite = PortfolioInvitations.resend(g.current_user, token) send_invite_email(g.current_user.full_name, invite.token, invite.email) flash("resend_portfolio_invitation", user_name=invite.user_name) return redirect( diff --git a/atst/routes/task_orders/invitations.py b/atst/routes/task_orders/invitations.py index 024c0097..b89d6be2 100644 --- a/atst/routes/task_orders/invitations.py +++ b/atst/routes/task_orders/invitations.py @@ -7,7 +7,7 @@ from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.permissions import Permissions from atst.database import db from atst.domain.exceptions import NotFoundError, NoAccessError -from atst.domain.invitations import Invitations +from atst.domain.invitations import PortfolioInvitations from atst.domain.portfolios import Portfolios from atst.utils.localization import translate from atst.forms.officers import EditTaskOrderOfficersForm @@ -57,7 +57,7 @@ def resend_invite(task_order_id): if not officer: raise NotFoundError("officer") - invitation = Invitations.lookup_by_portfolio_and_user(portfolio, officer) + invitation = PortfolioInvitations.lookup_by_portfolio_and_user(portfolio, officer) if not invitation: raise NotFoundError("invitation") @@ -65,11 +65,11 @@ def resend_invite(task_order_id): if not invitation.can_resend: raise NoAccessError("invitation") - Invitations.revoke(token=invitation.token) + PortfolioInvitations.revoke(token=invitation.token) invite_service = InvitationService( g.current_user, - invitation.portfolio_role, + invitation.role, invitation.email, subject=invite_type_info["subject"], email_template=invite_type_info["template"], diff --git a/atst/services/invitation.py b/atst/services/invitation.py index b7c3c217..9d478776 100644 --- a/atst/services/invitation.py +++ b/atst/services/invitation.py @@ -1,6 +1,6 @@ from flask import render_template -from atst.domain.invitations import Invitations +from atst.domain.invitations import PortfolioInvitations from atst.queue import queue from atst.domain.task_orders import TaskOrders from atst.domain.portfolio_roles import PortfolioRoles @@ -68,7 +68,7 @@ class Invitation: return invite def _create_invite(self): - return Invitations.create(self.inviter, self.member, self.email) + return PortfolioInvitations.create(self.inviter, self.member, self.email) def _send_invite_email(self, token): body = render_template( diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index ef08c879..d914ba09 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -3,33 +3,32 @@ import pytest import re from atst.domain.invitations import ( - Invitations, + PortfolioInvitations, InvitationError, WrongUserError, ExpiredError, NotFoundError, ) -from atst.models.invitation import Status +from atst.domain.audit_log import AuditLog +from atst.models import InvitationStatus from tests.factories import ( PortfolioFactory, PortfolioRoleFactory, UserFactory, - InvitationFactory, + PortfolioInvitationFactory, ) -from atst.domain.audit_log import AuditLog - def test_create_invitation(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) assert invite.user == user - assert invite.portfolio_role == ws_role + assert invite.role == ws_role assert invite.inviter == portfolio.owner - assert invite.status == Status.PENDING + assert invite.status == InvitationStatus.PENDING assert re.match(r"^[\w\-_]+$", invite.token) @@ -37,9 +36,9 @@ def test_accept_invitation(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) assert invite.is_pending - accepted_invite = Invitations.accept(user, invite.token) + accepted_invite = PortfolioInvitations.accept(user, invite.token) assert accepted_invite.is_accepted @@ -47,16 +46,16 @@ def test_accept_expired_invitation(): user = UserFactory.create() portfolio = PortfolioFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - increment = Invitations.EXPIRATION_LIMIT_MINUTES + 1 + increment = PortfolioInvitations.EXPIRATION_LIMIT_MINUTES + 1 expiration_time = datetime.datetime.now() - datetime.timedelta(minutes=increment) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user=user, expiration_time=expiration_time, - status=Status.PENDING, - portfolio_role=ws_role, + status=InvitationStatus.PENDING, + role=ws_role, ) with pytest.raises(ExpiredError): - Invitations.accept(user, invite.token) + PortfolioInvitations.accept(user, invite.token) assert invite.is_rejected @@ -65,22 +64,22 @@ def test_accept_rejected_invite(): user = UserFactory.create() portfolio = PortfolioFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = InvitationFactory.create( - user=user, status=Status.REJECTED_EXPIRED, portfolio_role=ws_role + invite = PortfolioInvitationFactory.create( + user=user, status=InvitationStatus.REJECTED_EXPIRED, role=ws_role ) with pytest.raises(InvitationError): - Invitations.accept(user, invite.token) + PortfolioInvitations.accept(user, invite.token) def test_accept_revoked_invite(): user = UserFactory.create() portfolio = PortfolioFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = InvitationFactory.create( - user=user, status=Status.REVOKED, portfolio_role=ws_role + invite = PortfolioInvitationFactory.create( + user=user, status=InvitationStatus.REVOKED, role=ws_role ) with pytest.raises(InvitationError): - Invitations.accept(user, invite.token) + PortfolioInvitations.accept(user, invite.token) def test_wrong_user_accepts_invitation(): @@ -88,9 +87,9 @@ def test_wrong_user_accepts_invitation(): portfolio = PortfolioFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) wrong_user = UserFactory.create() - invite = InvitationFactory.create(user=user, portfolio_role=ws_role) + invite = PortfolioInvitationFactory.create(user=user, role=ws_role) with pytest.raises(WrongUserError): - Invitations.accept(wrong_user, invite.token) + PortfolioInvitations.accept(wrong_user, invite.token) def test_user_cannot_accept_invitation_accepted_by_wrong_user(): @@ -98,30 +97,30 @@ def test_user_cannot_accept_invitation_accepted_by_wrong_user(): portfolio = PortfolioFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) wrong_user = UserFactory.create() - invite = InvitationFactory.create(user=user, portfolio_role=ws_role) + invite = PortfolioInvitationFactory.create(user=user, role=ws_role) with pytest.raises(WrongUserError): - Invitations.accept(wrong_user, invite.token) + PortfolioInvitations.accept(wrong_user, invite.token) with pytest.raises(InvitationError): - Invitations.accept(user, invite.token) + PortfolioInvitations.accept(user, invite.token) def test_accept_invitation_twice(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) - Invitations.accept(user, invite.token) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) + PortfolioInvitations.accept(user, invite.token) with pytest.raises(InvitationError): - Invitations.accept(user, invite.token) + PortfolioInvitations.accept(user, invite.token) def test_revoke_invitation(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) assert invite.is_pending - Invitations.revoke(invite.token) + PortfolioInvitations.revoke(invite.token) assert invite.is_revoked @@ -129,8 +128,8 @@ def test_resend_invitation(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) - Invitations.resend(user, invite.token) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) + PortfolioInvitations.resend(user, invite.token) assert ws_role.invitations[0].is_revoked assert ws_role.invitations[1].is_pending @@ -139,8 +138,8 @@ def test_audit_event_for_accepted_invite(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) - invite = Invitations.accept(user, invite.token) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) + invite = PortfolioInvitations.accept(user, invite.token) accepted_event = AuditLog.get_by_resource(invite.id)[0] assert "email" in accepted_event.event_details @@ -151,9 +150,11 @@ def test_lookup_by_user_and_portfolio(): portfolio = PortfolioFactory.create() user = UserFactory.create() ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) - invite = Invitations.create(portfolio.owner, ws_role, user.email) + invite = PortfolioInvitations.create(portfolio.owner, ws_role, user.email) - assert Invitations.lookup_by_portfolio_and_user(portfolio, user) == invite + assert PortfolioInvitations.lookup_by_portfolio_and_user(portfolio, user) == invite with pytest.raises(NotFoundError): - Invitations.lookup_by_portfolio_and_user(portfolio, UserFactory.create()) + PortfolioInvitations.lookup_by_portfolio_and_user( + portfolio, UserFactory.create() + ) diff --git a/tests/domain/test_portfolio_roles.py b/tests/domain/test_portfolio_roles.py index 312e12ca..d98eff1b 100644 --- a/tests/domain/test_portfolio_roles.py +++ b/tests/domain/test_portfolio_roles.py @@ -7,7 +7,7 @@ from atst.models.portfolio_role import Status as PortfolioRoleStatus from tests.factories import ( PortfolioFactory, UserFactory, - InvitationFactory, + PortfolioInvitationFactory, PortfolioRoleFactory, ) diff --git a/tests/factories.py b/tests/factories.py index a9ac984e..89999c63 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -7,11 +7,8 @@ import datetime from atst.forms import data from atst.models import * -from atst.models.portfolio_role import Status as PortfolioRoleStatus -from atst.models.application_role import Status as ApplicationRoleStatus -from atst.models.invitation import Status as InvitationStatus -from atst.models.environment_role import CSPRole -from atst.domain.invitations import Invitations + +from atst.domain.invitations import PortfolioInvitations from atst.domain.permission_sets import PermissionSets from atst.domain.portfolio_roles import PortfolioRoles @@ -240,13 +237,13 @@ class EnvironmentRoleFactory(Base): user = factory.SubFactory(UserFactory) -class InvitationFactory(Base): +class PortfolioInvitationFactory(Base): class Meta: - model = Invitation + model = PortfolioInvitation email = factory.Faker("email") status = InvitationStatus.PENDING - expiration_time = Invitations.current_expiration_time() + expiration_time = PortfolioInvitations.current_expiration_time() class AttachmentFactory(Base): diff --git a/tests/models/test_invitation.py b/tests/models/test_portfolio_invitations.py similarity index 72% rename from tests/models/test_invitation.py rename to tests/models/test_portfolio_invitations.py index 0ecfff59..faf9365d 100644 --- a/tests/models/test_invitation.py +++ b/tests/models/test_portfolio_invitations.py @@ -1,11 +1,9 @@ -import pytest import datetime -from atst.models.invitation import Invitation, Status -from atst.models.portfolio_role import Status as PortfolioRoleStatus +from atst.models import InvitationStatus, PortfolioRoleStatus from tests.factories import ( - InvitationFactory, + PortfolioInvitationFactory, PortfolioFactory, UserFactory, PortfolioRoleFactory, @@ -18,9 +16,9 @@ def test_expired_invite_is_not_revokable(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( expiration_time=datetime.datetime.now() - datetime.timedelta(minutes=60), - portfolio_role=portfolio_role, + role=portfolio_role, ) assert not invite.is_revokable @@ -31,7 +29,7 @@ def test_unexpired_invite_is_revokable(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create(portfolio_role=portfolio_role) + invite = PortfolioInvitationFactory.create(role=portfolio_role) assert invite.is_revokable @@ -41,7 +39,7 @@ def test_invite_is_not_revokable_if_invite_is_not_pending(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( - portfolio_role=portfolio_role, status=Status.ACCEPTED + invite = PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.ACCEPTED ) assert not invite.is_revokable diff --git a/tests/models/test_portfolio_role.py b/tests/models/test_portfolio_role.py index 110df0eb..6438d2b1 100644 --- a/tests/models/test_portfolio_role.py +++ b/tests/models/test_portfolio_role.py @@ -6,14 +6,11 @@ from atst.domain.portfolios import Portfolios from atst.domain.portfolio_roles import PortfolioRoles from atst.domain.applications import Applications from atst.domain.permission_sets import PermissionSets -from atst.models.portfolio_role import Status -from atst.models.invitation import Status as InvitationStatus -from atst.models.audit_event import AuditEvent -from atst.models.portfolio_role import Status as PortfolioRoleStatus -from atst.models.environment_role import CSPRole +from atst.models import AuditEvent, InvitationStatus, PortfolioRoleStatus, CSPRole + from tests.factories import ( UserFactory, - InvitationFactory, + PortfolioInvitationFactory, PortfolioRoleFactory, EnvironmentFactory, EnvironmentRoleFactory, @@ -189,12 +186,12 @@ def test_has_environment_roles(): def test_status_when_member_is_active(): - portfolio_role = PortfolioRoleFactory.create(status=Status.ACTIVE) + portfolio_role = PortfolioRoleFactory.create(status=PortfolioRoleStatus.ACTIVE) assert portfolio_role.display_status == "Active" def test_status_when_member_is_disabled(): - portfolio_role = PortfolioRoleFactory.create(status=Status.DISABLED) + portfolio_role = PortfolioRoleFactory.create(status=PortfolioRoleStatus.DISABLED) assert portfolio_role.display_status == "Disabled" @@ -204,8 +201,8 @@ def test_status_when_invitation_has_been_rejected_for_expirations(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED ) assert portfolio_role.display_status == "Invite expired" @@ -216,8 +213,8 @@ def test_status_when_invitation_has_been_rejected_for_wrong_user(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, status=InvitationStatus.REJECTED_WRONG_USER + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.REJECTED_WRONG_USER ) assert portfolio_role.display_status == "Error on invite" @@ -228,8 +225,8 @@ def test_status_when_invitation_has_been_revoked(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, status=InvitationStatus.REVOKED + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.REVOKED ) assert portfolio_role.display_status == "Invite revoked" @@ -240,8 +237,8 @@ def test_status_when_invitation_is_expired(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.PENDING, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -254,8 +251,8 @@ def test_can_not_resend_invitation_if_active(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, status=InvitationStatus.ACCEPTED + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.ACCEPTED ) assert not portfolio_role.can_resend_invitation @@ -266,8 +263,8 @@ def test_can_resend_invitation_if_expired(): portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invitation = InvitationFactory.create( - portfolio_role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED + PortfolioInvitationFactory.create( + role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED ) assert portfolio_role.can_resend_invitation diff --git a/tests/routes/portfolios/test_invitations.py b/tests/routes/portfolios/test_invitations.py index eddae72e..78eb1e77 100644 --- a/tests/routes/portfolios/test_invitations.py +++ b/tests/routes/portfolios/test_invitations.py @@ -6,12 +6,11 @@ from tests.factories import ( UserFactory, PortfolioFactory, PortfolioRoleFactory, - InvitationFactory, + PortfolioInvitationFactory, TaskOrderFactory, ) from atst.domain.portfolios import Portfolios -from atst.models.portfolio_role import Status as PortfolioRoleStatus -from atst.models.invitation import Status as InvitationStatus +from atst.models import InvitationStatus, PortfolioRoleStatus from atst.domain.users import Users from atst.domain.permission_sets import PermissionSets @@ -22,7 +21,7 @@ def test_existing_member_accepts_valid_invite(client, user_session): ws_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create(user_id=user.id, portfolio_role=ws_role) + invite = PortfolioInvitationFactory.create(user_id=user.id, role=ws_role) # the user does not have access to the portfolio before accepting the invite assert len(Portfolios.for_user(user)) == 0 @@ -60,7 +59,7 @@ def test_new_member_accepts_valid_invite(monkeypatch, client, user_session): assert response.status_code == 302 user = Users.get_by_dod_id(user_info["dod_id"]) - token = user.invitations[0].token + token = user.portfolio_invitations[0].token monkeypatch.setattr( "atst.domain.auth.should_redirect_to_user_profile", lambda *args: False @@ -84,10 +83,8 @@ def test_member_accepts_invalid_invite(client, user_session): ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( - user_id=user.id, - portfolio_role=ws_role, - status=InvitationStatus.REJECTED_WRONG_USER, + invite = PortfolioInvitationFactory.create( + user_id=user.id, role=ws_role, status=InvitationStatus.REJECTED_WRONG_USER ) user_session(user) response = client.get(url_for("portfolios.accept_invitation", token=invite.token)) @@ -119,7 +116,7 @@ def test_user_accepts_invite_with_wrong_dod_id(client, user_session): ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create(user_id=user.id, portfolio_role=ws_role) + invite = PortfolioInvitationFactory.create(user_id=user.id, role=ws_role) user_session(different_user) response = client.get(url_for("portfolios.accept_invitation", token=invite.token)) @@ -132,9 +129,9 @@ def test_user_accepts_expired_invite(client, user_session): ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user_id=user.id, - portfolio_role=ws_role, + role=ws_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -150,9 +147,9 @@ def test_revoke_invitation(client, user_session): ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user_id=user.id, - portfolio_role=ws_role, + role=ws_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -176,9 +173,9 @@ def test_user_can_only_revoke_invites_in_their_portfolio(client, user_session): portfolio_role = PortfolioRoleFactory.create( user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user_id=user.id, - portfolio_role=portfolio_role, + role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -202,9 +199,9 @@ def test_user_can_only_resend_invites_in_their_portfolio(client, user_session, q portfolio_role = PortfolioRoleFactory.create( user=user, portfolio=other_portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user_id=user.id, - portfolio_role=portfolio_role, + role=portfolio_role, status=InvitationStatus.REJECTED_EXPIRED, expiration_time=datetime.datetime.now() - datetime.timedelta(seconds=1), ) @@ -227,8 +224,8 @@ def test_resend_invitation_sends_email(client, user_session, queue): ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( - user_id=user.id, portfolio_role=ws_role, status=InvitationStatus.PENDING + invite = PortfolioInvitationFactory.create( + user_id=user.id, role=ws_role, status=InvitationStatus.PENDING ) user_session(portfolio.owner) client.post( @@ -250,9 +247,9 @@ def test_existing_member_invite_resent_to_email_submitted_in_form( ws_role = PortfolioRoleFactory.create( user=user, portfolio=portfolio, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( user_id=user.id, - portfolio_role=ws_role, + role=ws_role, status=InvitationStatus.PENDING, email="example@example.com", ) @@ -290,7 +287,7 @@ def test_contracting_officer_accepts_invite(monkeypatch, client, user_session): # contracting officer accepts invitation user = Users.get_by_dod_id(user_info["dod_id"]) - token = user.invitations[0].token + token = user.portfolio_invitations[0].token monkeypatch.setattr( "atst.domain.auth.should_redirect_to_user_profile", lambda *args: False ) @@ -324,7 +321,7 @@ def test_cor_accepts_invite(monkeypatch, client, user_session): # contracting officer representative accepts invitation user = Users.get_by_dod_id(user_info["dod_id"]) - token = user.invitations[0].token + token = user.portfolio_invitations[0].token monkeypatch.setattr( "atst.domain.auth.should_redirect_to_user_profile", lambda *args: False ) @@ -358,7 +355,7 @@ def test_so_accepts_invite(monkeypatch, client, user_session): # security officer accepts invitation user = Users.get_by_dod_id(user_info["dod_id"]) - token = user.invitations[0].token + token = user.portfolio_invitations[0].token monkeypatch.setattr( "atst.domain.auth.should_redirect_to_user_profile", lambda *args: False ) diff --git a/tests/routes/portfolios/test_members.py b/tests/routes/portfolios/test_members.py index 818a62a5..b3f8c430 100644 --- a/tests/routes/portfolios/test_members.py +++ b/tests/routes/portfolios/test_members.py @@ -45,7 +45,7 @@ def test_create_member(client, user_session): assert response.status_code == 200 assert user.full_name in response.data.decode() assert user.has_portfolios - assert user.invitations + 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 diff --git a/tests/routes/task_orders/test_invitations.py b/tests/routes/task_orders/test_invitations.py index 795fe6f1..83978d8c 100644 --- a/tests/routes/task_orders/test_invitations.py +++ b/tests/routes/task_orders/test_invitations.py @@ -4,7 +4,7 @@ from flask import url_for import pytest from atst.domain.task_orders import TaskOrders -from atst.models.invitation import Status as InvitationStatus +from atst.models import InvitationStatus from atst.models.portfolio_role import Status as PortfolioStatus from atst.queue import queue @@ -13,7 +13,7 @@ from tests.factories import ( TaskOrderFactory, UserFactory, PortfolioRoleFactory, - InvitationFactory, + PortfolioInvitationFactory, ) @@ -79,7 +79,7 @@ def test_does_not_resend_officer_invitation(client, user_session): user_session(user) for i in range(2): client.post(url_for("task_orders.invite", task_order_id=task_order.id)) - assert len(contracting_officer.invitations) == 1 + assert len(contracting_officer.portfolio_invitations) == 1 def test_does_not_invite_if_task_order_incomplete(client, user_session, queue): @@ -272,9 +272,9 @@ class TestTaskOrderInvitations: cor_invite=True, ) portfolio_role = PortfolioRoleFactory.create(portfolio=self.portfolio, user=cor) - invitation = InvitationFactory.create( + PortfolioInvitationFactory.create( inviter=self.portfolio.owner, - portfolio_role=portfolio_role, + role=portfolio_role, user=cor, status=InvitationStatus.PENDING, ) @@ -369,9 +369,9 @@ def test_resend_invite_when_not_pending(app, client, user_session, portfolio, us portfolio=portfolio, user=user, status=PortfolioStatus.ACTIVE ) - original_invitation = InvitationFactory.create( + original_invitation = PortfolioInvitationFactory.create( inviter=user, - portfolio_role=portfolio_role, + role=portfolio_role, email=user.email, status=InvitationStatus.ACCEPTED, ) @@ -397,9 +397,9 @@ def test_resending_revoked_invite(app, client, user_session, portfolio, user): portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=user) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( inviter=user, - portfolio_role=portfolio_role, + role=portfolio_role, email=user.email, status=InvitationStatus.REVOKED, ) @@ -427,9 +427,9 @@ def test_resending_expired_invite(app, client, user_session, portfolio): portfolio=portfolio, contracting_officer=ko, ko_invite=True ) portfolio_role = PortfolioRoleFactory.create(portfolio=portfolio, user=ko) - invite = InvitationFactory.create( + invite = PortfolioInvitationFactory.create( inviter=portfolio.owner, - portfolio_role=portfolio_role, + role=portfolio_role, email=ko.email, expiration_time=datetime.now() - timedelta(days=1), ) diff --git a/tests/services/test_invitation.py b/tests/services/test_invitation.py index 2227b541..364cdbcc 100644 --- a/tests/services/test_invitation.py +++ b/tests/services/test_invitation.py @@ -10,5 +10,5 @@ def test_invite_member(queue): ws_member = PortfolioRoleFactory.create(user=new_member, portfolio=portfolio) invite_service = Invitation(inviter, ws_member, new_member.email) new_invitation = invite_service.invite() - assert new_invitation == new_member.invitations[0] + assert new_invitation == new_member.portfolio_invitations[0] assert len(queue.get_queue()) == 1 diff --git a/tests/test_access.py b/tests/test_access.py index 0b568c58..ded929e3 100644 --- a/tests/test_access.py +++ b/tests/test_access.py @@ -17,7 +17,7 @@ from tests.factories import ( ApplicationRoleFactory, EnvironmentFactory, EnvironmentRoleFactory, - InvitationFactory, + PortfolioInvitationFactory, PortfolioFactory, PortfolioRoleFactory, TaskOrderFactory, @@ -75,7 +75,9 @@ def test_all_protected_routes_have_access_control( monkeypatch.setattr("atst.domain.portfolios.Portfolios.get", lambda *a: None) monkeypatch.setattr("atst.domain.task_orders.TaskOrders.get", lambda *a: Mock()) monkeypatch.setattr("atst.domain.applications.Applications.get", lambda *a: Mock()) - monkeypatch.setattr("atst.domain.invitations.Invitations._get", lambda *a: Mock()) + monkeypatch.setattr( + "atst.domain.invitations.PortfolioInvitations._get", lambda *a: Mock() + ) monkeypatch.setattr( "atst.utils.context_processors.get_portfolio_from_context", lambda *a: None ) @@ -402,7 +404,7 @@ def test_portfolios_resend_invitation_access(post_url_assert_status): portfolio = PortfolioFactory.create(owner=owner) prr = PortfolioRoleFactory.create(user=invitee, portfolio=portfolio) - invite = InvitationFactory.create(user=UserFactory.create(), portfolio_role=prr) + invite = PortfolioInvitationFactory.create(user=UserFactory.create(), role=prr) url = url_for( "portfolios.resend_invitation", portfolio_id=portfolio.id, token=invite.token @@ -423,7 +425,7 @@ def test_task_orders_resend_invite_access(post_url_assert_status): portfolio = PortfolioFactory.create(owner=owner) task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=ko) prr = PortfolioRoleFactory.create(user=ko, portfolio=portfolio) - invite = InvitationFactory.create(user=UserFactory.create(), portfolio_role=prr) + PortfolioInvitationFactory.create(user=UserFactory.create(), role=prr) url = url_for( "task_orders.resend_invite", @@ -449,7 +451,7 @@ def test_portfolios_revoke_invitation_access(post_url_assert_status): prr = PortfolioRoleFactory.create( user=prt_member, portfolio=portfolio, status=PortfolioRoleStatus.ACTIVE ) - invite = InvitationFactory.create(user=prt_member, portfolio_role=prr) + invite = PortfolioInvitationFactory.create(user=prt_member, role=prr) url = url_for( "portfolios.revoke_invitation", portfolio_id=portfolio.id, From dd0b184bc235e2f6dd1beae4918040104164926d Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 23 Apr 2019 13:38:17 -0400 Subject: [PATCH 02/14] extract new member form into standalone form class --- atst/forms/member.py | 27 +++++++++++++++ atst/forms/portfolio_member.py | 33 +++++++------------ atst/routes/portfolios/members.py | 4 +-- .../admin/add_new_portfolio_member.html | 18 +++++----- tests/routes/portfolios/test_invitations.py | 13 +++++--- tests/routes/portfolios/test_members.py | 18 +++++----- 6 files changed, 67 insertions(+), 46 deletions(-) create mode 100644 atst/forms/member.py diff --git a/atst/forms/member.py b/atst/forms/member.py new file mode 100644 index 00000000..aca653e5 --- /dev/null +++ b/atst/forms/member.py @@ -0,0 +1,27 @@ +from flask_wtf import FlaskForm +from wtforms.fields.html5 import EmailField, TelField +from wtforms.validators import Required, Email, Length, Optional +from wtforms.fields import StringField + +from atst.forms.validators import IsNumber, PhoneNumber +from atst.utils.localization import translate + + +class NewForm(FlaskForm): + first_name = StringField( + label=translate("forms.new_member.first_name_label"), validators=[Required()] + ) + last_name = StringField( + label=translate("forms.new_member.last_name_label"), validators=[Required()] + ) + email = EmailField( + translate("forms.new_member.email_label"), validators=[Required(), Email()] + ) + phone_number = TelField( + translate("forms.new_member.phone_number_label"), + validators=[Optional(), PhoneNumber()], + ) + dod_id = StringField( + translate("forms.new_member.dod_id_label"), + validators=[Required(), Length(min=10), IsNumber()], + ) diff --git a/atst/forms/portfolio_member.py b/atst/forms/portfolio_member.py index 2e76efdd..2a59848f 100644 --- a/atst/forms/portfolio_member.py +++ b/atst/forms/portfolio_member.py @@ -1,10 +1,9 @@ -from wtforms.fields.html5 import EmailField, TelField -from wtforms.validators import Required, Email, Length, Optional +from wtforms.validators import Required from wtforms.fields import StringField, FormField, FieldList, HiddenField from atst.domain.permission_sets import PermissionSets from .forms import BaseForm -from atst.forms.validators import IsNumber, PhoneNumber +from .member import NewForm as BaseNewMemberForm from atst.forms.fields import SelectField from atst.utils.localization import translate @@ -62,24 +61,16 @@ class EditForm(PermissionsForm): pass -class NewForm(PermissionsForm): - first_name = StringField( - label=translate("forms.new_member.first_name_label"), validators=[Required()] - ) - last_name = StringField( - label=translate("forms.new_member.last_name_label"), validators=[Required()] - ) - email = EmailField( - translate("forms.new_member.email_label"), validators=[Required(), Email()] - ) - phone_number = TelField( - translate("forms.new_member.phone_number_label"), - validators=[Optional(), PhoneNumber()], - ) - dod_id = StringField( - translate("forms.new_member.dod_id_label"), - validators=[Required(), Length(min=10), IsNumber()], - ) +class NewForm(BaseForm): + user_data = FormField(BaseNewMemberForm) + permission_sets = FormField(PermissionsForm) + + @property + def update_data(self): + return { + "permission_sets": self.data.get("permission_sets").get("permission_sets"), + **self.data.get("user_data"), + } class AssignPPOCForm(PermissionsForm): diff --git a/atst/routes/portfolios/members.py b/atst/routes/portfolios/members.py index 573781b8..f25f0918 100644 --- a/atst/routes/portfolios/members.py +++ b/atst/routes/portfolios/members.py @@ -34,9 +34,9 @@ def create_member(portfolio_id): if form.validate(): try: - member = Portfolios.create_member(portfolio, form.data) + member = Portfolios.create_member(portfolio, form.update_data) invite_service = InvitationService( - g.current_user, member, form.data.get("email") + g.current_user, member, form.update_data.get("email") ) invite_service.invite() diff --git a/templates/fragments/admin/add_new_portfolio_member.html b/templates/fragments/admin/add_new_portfolio_member.html index a8c91a7a..2fd166e9 100644 --- a/templates/fragments/admin/add_new_portfolio_member.html +++ b/templates/fragments/admin/add_new_portfolio_member.html @@ -21,23 +21,23 @@
- {{ TextInput(member_form.first_name, validation='requiredField') }} + {{ TextInput(member_form.user_data.first_name, validation='requiredField') }}
- {{ TextInput(member_form.last_name, validation='requiredField') }} + {{ TextInput(member_form.user_data.last_name, validation='requiredField') }}
- {{ TextInput(member_form.email, validation='email') }} + {{ TextInput(member_form.user_data.email, validation='email') }}
- {{ TextInput(member_form.phone_number, validation='usPhone', optional=True) }} + {{ TextInput(member_form.user_data.phone_number, validation='usPhone', optional=True) }}
- {{ TextInput(member_form.dod_id, validation='dodId') }} + {{ TextInput(member_form.user_data.dod_id, validation='dodId') }}
@@ -61,10 +61,10 @@ {{ "portfolios.admin.permissions_info" | translate }}
- {{ SimpleOptionsInput(member_form.perms_app_mgmt) }} - {{ SimpleOptionsInput(member_form.perms_funding) }} - {{ SimpleOptionsInput(member_form.perms_reporting) }} - {{ SimpleOptionsInput(member_form.perms_portfolio_mgmt) }} + {{ SimpleOptionsInput(member_form.permission_sets.perms_app_mgmt) }} + {{ SimpleOptionsInput(member_form.permission_sets.perms_funding) }} + {{ SimpleOptionsInput(member_form.permission_sets.perms_reporting) }} + {{ SimpleOptionsInput(member_form.permission_sets.perms_portfolio_mgmt) }}
Date: Tue, 23 Apr 2019 09:28:56 -0400 Subject: [PATCH 03/14] add application_invitation table --- ...432c5287256d_add_application_invitation.py | 69 +++++++++++++++++++ atst/models/__init__.py | 1 + atst/models/application_invitation.py | 41 +++++++++++ atst/models/portfolio_invitation.py | 2 +- tests/factories.py | 9 +++ 5 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 alembic/versions/432c5287256d_add_application_invitation.py create mode 100644 atst/models/application_invitation.py diff --git a/alembic/versions/432c5287256d_add_application_invitation.py b/alembic/versions/432c5287256d_add_application_invitation.py new file mode 100644 index 00000000..fac3b326 --- /dev/null +++ b/alembic/versions/432c5287256d_add_application_invitation.py @@ -0,0 +1,69 @@ +"""add application_invitation + +Revision ID: 432c5287256d +Revises: 1880551a32e4 +Create Date: 2019-04-23 09:23:05.738680 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '432c5287256d' +down_revision = '1880551a32e4' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.rename_table('invitations', 'portfolio_invitations') + op.create_table('application_invitations', + sa.Column('time_created', sa.TIMESTAMP(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.Column('time_updated', sa.TIMESTAMP(timezone=True), server_default=sa.text('now()'), nullable=False), + sa.Column('id', postgresql.UUID(as_uuid=True), server_default=sa.text('uuid_generate_v4()'), nullable=False), + sa.Column('status', sa.Enum('ACCEPTED', 'REVOKED', 'PENDING', 'REJECTED_WRONG_USER', 'REJECTED_EXPIRED', name='status', native_enum=False), nullable=True), + sa.Column('expiration_time', sa.TIMESTAMP(timezone=True), nullable=True), + sa.Column('token', sa.String(), nullable=True), + sa.Column('email', sa.String(), nullable=False), + sa.Column('application_role_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('inviter_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.ForeignKeyConstraint(['application_role_id'], ['application_roles.id'], ), + sa.ForeignKeyConstraint(['inviter_id'], ['users.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_application_invitations_application_role_id'), 'application_invitations', ['application_role_id'], unique=False) + op.create_index(op.f('ix_application_invitations_inviter_id'), 'application_invitations', ['inviter_id'], unique=False) + op.create_index(op.f('ix_application_invitations_token'), 'application_invitations', ['token'], unique=False) + op.create_index(op.f('ix_application_invitations_user_id'), 'application_invitations', ['user_id'], unique=False) + op.drop_index('ix_invitations_inviter_id', table_name='invitations') + op.drop_index('ix_invitations_portfolio_role_id', table_name='invitations') + op.drop_index('ix_invitations_token', table_name='invitations') + op.drop_index('ix_invitations_user_id', table_name='invitations') + op.create_index(op.f('ix_portfolio_invitations_inviter_id'), 'portfolio_invitations', ['inviter_id'], unique=False) + op.create_index(op.f('ix_portfolio_invitations_portfolio_role_id'), 'portfolio_invitations', ['portfolio_role_id'], unique=False) + op.create_index(op.f('ix_portfolio_invitations_token'), 'portfolio_invitations', ['token'], unique=False) + op.create_index(op.f('ix_portfolio_invitations_user_id'), 'portfolio_invitations', ['user_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.rename_table('portfolio_invitations', 'invitations') + op.drop_index(op.f('ix_application_invitations_user_id'), table_name='application_invitations') + op.drop_index(op.f('ix_application_invitations_token'), table_name='application_invitations') + op.drop_index(op.f('ix_application_invitations_inviter_id'), table_name='application_invitations') + op.drop_index(op.f('ix_application_invitations_application_role_id'), table_name='application_invitations') + op.drop_table('application_invitations') + op.drop_index(op.f('ix_portfolio_invitations_user_id'), table_name='portfolio_invitations') + op.drop_index(op.f('ix_portfolio_invitations_token'), table_name='portfolio_invitations') + op.drop_index(op.f('ix_portfolio_invitations_portfolio_role_id'), table_name='portfolio_invitations') + op.drop_index(op.f('ix_portfolio_invitations_inviter_id'), table_name='portfolio_invitations') + op.create_index('ix_invitations_user_id', 'invitations', ['user_id'], unique=False) + op.create_index('ix_invitations_token', 'invitations', ['token'], unique=False) + op.create_index('ix_invitations_portfolio_role_id', 'invitations', ['portfolio_role_id'], unique=False) + op.create_index('ix_invitations_inviter_id', 'invitations', ['inviter_id'], unique=False) + # ### end Alembic commands ### diff --git a/atst/models/__init__.py b/atst/models/__init__.py index 0369bc07..7fe45e05 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -14,6 +14,7 @@ from .environment import Environment from .attachment import Attachment from .audit_event import AuditEvent from .portfolio_invitation import PortfolioInvitation +from .application_invitation import ApplicationInvitation from .task_order import TaskOrder from .dd_254 import DD254 diff --git a/atst/models/application_invitation.py b/atst/models/application_invitation.py new file mode 100644 index 00000000..9f1db550 --- /dev/null +++ b/atst/models/application_invitation.py @@ -0,0 +1,41 @@ +from sqlalchemy import Column, ForeignKey +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import relationship, backref + +from atst.models import Base +from atst.models.mixins import TimestampsMixin, AuditableMixin, InvitesMixin + + +class ApplicationInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin): + __tablename__ = "application_invitations" + + application_role_id = Column( + UUID(as_uuid=True), ForeignKey("application_roles.id"), index=True + ) + role = relationship( + "ApplicationRole", + backref=backref("invitations", order_by="ApplicationInvitation.time_created"), + ) + + @property + def application(self): + if self.role: # pragma: no branch + return self.role.application + + @property + def application_id(self): + return self.role.application_id + + @property + def event_details(self): + return {"email": self.email, "dod_id": self.user_dod_id} + + @property + def history(self): + changes = self.get_changes() + change_set = {} + + if "status" in changes: + change_set["status"] = [s.name for s in changes["status"]] + + return change_set diff --git a/atst/models/portfolio_invitation.py b/atst/models/portfolio_invitation.py index 916e63ae..1c1e43dd 100644 --- a/atst/models/portfolio_invitation.py +++ b/atst/models/portfolio_invitation.py @@ -7,7 +7,7 @@ from atst.models.mixins import TimestampsMixin, AuditableMixin, InvitesMixin class PortfolioInvitation(Base, TimestampsMixin, AuditableMixin, InvitesMixin): - __tablename__ = "invitations" + __tablename__ = "portfolio_invitations" portfolio_role_id = Column( UUID(as_uuid=True), ForeignKey("portfolio_roles.id"), index=True diff --git a/tests/factories.py b/tests/factories.py index 89999c63..8bdf9310 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -246,6 +246,15 @@ class PortfolioInvitationFactory(Base): expiration_time = PortfolioInvitations.current_expiration_time() +class ApplicationInvitationFactory(Base): + class Meta: + model = ApplicationInvitation + + email = factory.Faker("email") + status = InvitationStatus.PENDING + expiration_time = PortfolioInvitations.current_expiration_time() + + class AttachmentFactory(Base): class Meta: model = Attachment From ade77e6b91804a0e9dd3f8c8c101dea14730d602 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 23 Apr 2019 11:24:04 -0400 Subject: [PATCH 04/14] Route for adding new application member - domain method for creating a new application member - ApplicationInvitations domain class - nested form for adding a new user that holds user data, application permission sets, and environment roles - Invitation service can infer invitation type based on role it's given - new invitation email templates --- atst/domain/applications.py | 29 +++++++++++ atst/domain/invitations.py | 6 ++- atst/forms/application_member.py | 41 ++++++++++++++++ atst/forms/data.py | 1 + atst/models/__init__.py | 2 +- atst/models/user.py | 8 +++ atst/routes/applications/team.py | 49 ++++++++++++++++++- atst/routes/portfolios/invitations.py | 4 +- atst/services/invitation.py | 45 ++++++++++++----- atst/utils/flash.py | 7 +++ templates/emails/application/invitation.txt | 10 ++++ templates/emails/{invitation.txt => base.txt} | 5 +- templates/emails/portfolio/invitation.txt | 10 ++++ tests/domain/test_applications.py | 28 +++++++++++ tests/routes/applications/test_team.py | 42 +++++++++++++++- tests/services/test_invitation.py | 21 +++++++- translations.yaml | 1 + 17 files changed, 284 insertions(+), 25 deletions(-) create mode 100644 atst/forms/application_member.py create mode 100644 templates/emails/application/invitation.txt rename templates/emails/{invitation.txt => base.txt} (61%) create mode 100644 templates/emails/portfolio/invitation.txt diff --git a/atst/domain/applications.py b/atst/domain/applications.py index ba81461d..640242c0 100644 --- a/atst/domain/applications.py +++ b/atst/domain/applications.py @@ -2,8 +2,10 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db from . import BaseDomainClass +from atst.domain.application_roles import ApplicationRoles from atst.domain.environments import Environments from atst.domain.exceptions import NotFoundError +from atst.domain.users import Users from atst.models.application import Application from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole @@ -72,3 +74,30 @@ class Applications(BaseDomainClass): db.session.add(application) db.session.commit() + + @classmethod + def create_member( + cls, application, user_data, permission_sets=None, environment_roles_data=None + ): + permission_sets = [] if permission_sets is None else permission_sets + environment_roles_data = ( + [] if environment_roles_data is None else environment_roles_data + ) + + user = Users.get_or_create_by_dod_id( + user_data["dod_id"], + first_name=user_data["first_name"], + last_name=user_data["last_name"], + phone_number=user_data.get("phone_number"), + email=user_data["email"], + ) + + application_role = ApplicationRoles.create(user, application, permission_sets) + + for env_role_data in environment_roles_data: + role = env_role_data.get("role") + if role: + environment = Environments.get(env_role_data.get("environment_id")) + Environments.add_member(environment, user, env_role_data.get("role")) + + return application_role diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index cfedde11..209cd4d1 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -2,7 +2,7 @@ import datetime from sqlalchemy.orm.exc import NoResultFound from atst.database import db -from atst.models import InvitationStatus, PortfolioInvitation +from atst.models import ApplicationInvitation, InvitationStatus, PortfolioInvitation from atst.domain.portfolio_roles import PortfolioRoles from .exceptions import NotFoundError @@ -127,3 +127,7 @@ class BaseInvitations(object): class PortfolioInvitations(BaseInvitations): model = PortfolioInvitation + + +class ApplicationInvitations(BaseInvitations): + model = ApplicationInvitation diff --git a/atst/forms/application_member.py b/atst/forms/application_member.py new file mode 100644 index 00000000..03c85953 --- /dev/null +++ b/atst/forms/application_member.py @@ -0,0 +1,41 @@ +from wtforms.fields import FormField, FieldList, HiddenField, BooleanField + +from .forms import BaseForm +from .member import NewForm as BaseNewMemberForm +from .data import ENV_ROLES +from atst.forms.fields import SelectField +from atst.domain.permission_sets import PermissionSets + + +class EnvironmentForm(BaseForm): + environment_id = HiddenField() + environment_name = HiddenField() + role = SelectField(environment_name, choices=ENV_ROLES, default=None) + + +class PermissionsForm(BaseForm): + perms_env_mgmt = BooleanField(None, default=False) + perms_team_mgmt = BooleanField(None, default=False) + perms_del_env = BooleanField(None, default=False) + + @property + def data(self): + _data = super().data + perm_sets = [] + + if _data["perms_env_mgmt"]: + perm_sets.append(PermissionSets.EDIT_APPLICATION_ENVIRONMENTS) + + if _data["perms_team_mgmt"]: + perm_sets.append(PermissionSets.EDIT_APPLICATION_TEAM) + + if _data["perms_del_env"]: + perm_sets.append(PermissionSets.DELETE_APPLICATION_ENVIRONMENTS) + + return perm_sets + + +class NewForm(BaseForm): + user_data = FormField(BaseNewMemberForm) + permission_sets = FormField(PermissionsForm) + environment_roles = FieldList(FormField(EnvironmentForm)) diff --git a/atst/forms/data.py b/atst/forms/data.py index 23ab9c4a..515a70e9 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,3 +1,4 @@ +from atst.models import CSPRole from atst.utils.localization import translate, translate_duration from atst.models.environment_role import CSPRole diff --git a/atst/models/__init__.py b/atst/models/__init__.py index 7fe45e05..8a3cdbfa 100644 --- a/atst/models/__init__.py +++ b/atst/models/__init__.py @@ -7,7 +7,7 @@ from .permission_set import PermissionSet from .user import User from .portfolio_role import PortfolioRole, Status as PortfolioRoleStatus from .application_role import ApplicationRole, Status as ApplicationRoleStatus -from .environment_role import EnvironmentRole +from .environment_role import EnvironmentRole, CSPRole from .portfolio import Portfolio from .application import Application from .environment import Environment diff --git a/atst/models/user.py b/atst/models/user.py index 0510d867..d40501a4 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -5,6 +5,7 @@ from sqlalchemy.dialects.postgresql import UUID from atst.models import Base, types, mixins from atst.models.permissions import Permissions from atst.models.portfolio_invitation import PortfolioInvitation +from atst.models.application_invitation import ApplicationInvitation users_permission_sets = Table( @@ -39,6 +40,13 @@ class User( "PortfolioInvitation", foreign_keys=PortfolioInvitation.inviter_id ) + application_invitations = relationship( + "ApplicationInvitation", foreign_keys=ApplicationInvitation.user_id + ) + sent_application_invitations = relationship( + "ApplicationInvitation", foreign_keys=ApplicationInvitation.inviter_id + ) + email = Column(String) dod_id = Column(String, unique=True, nullable=False) first_name = Column(String) diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index c9e57296..7fb75f0e 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -1,12 +1,15 @@ -from flask import render_template +from flask import render_template, request as http_request, g, url_for, redirect from . import applications_bp from atst.domain.environments import Environments from atst.domain.applications import Applications from atst.domain.authz.decorator import user_can_access_decorator as user_can -from atst.models.permissions import Permissions from atst.domain.permission_sets import PermissionSets +from atst.forms.application_member import NewForm as NewMemberForm +from atst.models.permissions import Permissions +from atst.services.invitation import Invitation as InvitationService +from atst.utils.flash import formatted_flash as flash from atst.utils.localization import translate @@ -47,3 +50,45 @@ def team(application_id): application=application, environment_users=environment_users, ) + + +@applications_bp.route("/application//members/new", methods=["POST"]) +@user_can( + Permissions.CREATE_APPLICATION_MEMBER, message="create new application member" +) +def create_member(application_id): + application = Applications.get(application_id) + form = NewMemberForm(http_request.form) + + if form.validate(): + try: + member = Applications.create_member( + application, + form.user_data.data, + permission_sets=form.permission_sets.data, + environment_roles_data=form.environment_roles.data, + ) + + invite_service = InvitationService( + g.current_user, member, form.user_data.data.get("email") + ) + invite_service.invite() + + flash("new_portfolio_member", new_member=member) + + 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( + "applications.team", + application_id=application_id, + fragment="application-members", + _anchor="application-members", + ) + ) diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index a34fcee8..0e5901fe 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -9,7 +9,9 @@ from atst.models.permissions import Permissions def send_invite_email(owner_name, token, new_member_email): - body = render_template("emails/invitation.txt", owner=owner_name, token=token) + body = render_template( + "emails/portfolio/invitation.txt", owner=owner_name, token=token + ) queue.send_mail( [new_member_email], "{} has invited you to a JEDI Cloud Portfolio".format(owner_name), diff --git a/atst/services/invitation.py b/atst/services/invitation.py index 9d478776..877a3ab7 100644 --- a/atst/services/invitation.py +++ b/atst/services/invitation.py @@ -1,25 +1,26 @@ from flask import render_template -from atst.domain.invitations import PortfolioInvitations +from atst.domain.invitations import PortfolioInvitations, ApplicationInvitations from atst.queue import queue from atst.domain.task_orders import TaskOrders from atst.domain.portfolio_roles import PortfolioRoles +from atst.models import ApplicationRole, PortfolioRole OFFICER_INVITATIONS = { "ko_invite": { "role": "contracting_officer", "subject": "Review a task order", - "template": "emails/invitation.txt", + "template": "emails/portfolio/invitation.txt", }, "cor_invite": { "role": "contracting_officer_representative", "subject": "Help with a task order", - "template": "emails/invitation.txt", + "template": "emails/portfolio/invitation.txt", }, "so_invite": { "role": "security_officer", "subject": "Review security for a task order", - "template": "emails/invitation.txt", + "template": "emails/portfolio/invitation.txt", }, } @@ -47,20 +48,38 @@ def update_officer_invitations(user, task_order): class Invitation: - def __init__( - self, - inviter, - member, - email, - subject="{} has invited you to a JEDI cloud portfolio", - email_template="emails/invitation.txt", - ): + def __init__(self, inviter, member, email, subject="", email_template=None): self.inviter = inviter self.member = member self.email = email self.subject = subject self.email_template = email_template + if isinstance(member, PortfolioRole): + self.email_template = ( + "emails/portfolio/invitation.txt" + if self.email_template is None + else self.email_template + ) + self.subject = ( + "{} has invited you to a JEDI cloud portfolio" + if self.subject is None + else self.subject + ) + self.domain_class = PortfolioInvitations + elif isinstance(member, ApplicationRole): + self.email_template = ( + "emails/application/invitation.txt" + if self.email_template is None + else self.email_template + ) + self.subject = ( + "{} has invited you to a JEDI cloud application" + if self.subject is None + else self.subject + ) + self.domain_class = ApplicationInvitations + def invite(self): invite = self._create_invite() self._send_invite_email(invite.token) @@ -68,7 +87,7 @@ class Invitation: return invite def _create_invite(self): - return PortfolioInvitations.create(self.inviter, self.member, self.email) + return self.domain_class.create(self.inviter, self.member, self.email) def _send_invite_email(self, token): body = render_template( diff --git a/atst/utils/flash.py b/atst/utils/flash.py index da021ecc..9fff1be2 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -161,6 +161,13 @@ MESSAGES = { """, "category": "success", }, + "new_application_member": { + "title_template": translate("flash.success"), + "message_template": """ +

{{ "flash.new_application_member" | translate({ "user_name": new_member.user_name }) }}

+ """, + "category": "success", + }, } diff --git a/templates/emails/application/invitation.txt b/templates/emails/application/invitation.txt new file mode 100644 index 00000000..1d49b452 --- /dev/null +++ b/templates/emails/application/invitation.txt @@ -0,0 +1,10 @@ +{% extends "emails/base.txt" %} + +{% block content %} + +Join this JEDI Cloud Application +{{ owner }} has invited you to join a JEDI Cloud Application. Login now to view or use your JEDI Cloud resources. + +{# url_for("application.accept_invitation", token=token, _external=True) #} + +{% endblock %} diff --git a/templates/emails/invitation.txt b/templates/emails/base.txt similarity index 61% rename from templates/emails/invitation.txt rename to templates/emails/base.txt index f63e4db2..8a098d8e 100644 --- a/templates/emails/invitation.txt +++ b/templates/emails/base.txt @@ -1,7 +1,4 @@ -Join this JEDI Cloud Portfolio -{{ owner }} has invited you to join a JEDI Cloud Portfolio. Login now to view or use your JEDI Cloud resources. - -{{ url_for("portfolios.accept_invitation", token=token, _external=True) }} +{% block content %}{% endblock %} What is JEDI Cloud? JEDI Cloud is a DoD enterprise-wide solution for commercial cloud services. diff --git a/templates/emails/portfolio/invitation.txt b/templates/emails/portfolio/invitation.txt new file mode 100644 index 00000000..dd0f12df --- /dev/null +++ b/templates/emails/portfolio/invitation.txt @@ -0,0 +1,10 @@ +{% extends "emails/base.txt" %} + +{% block content %} + +Join this JEDI Cloud Portfolio +{{ owner }} has invited you to join a JEDI Cloud Portfolio. Login now to view or use your JEDI Cloud resources. + +{{ url_for("portfolios.accept_invitation", token=token, _external=True) }} + +{% endblock %} diff --git a/tests/domain/test_applications.py b/tests/domain/test_applications.py index b10c15f3..922b813c 100644 --- a/tests/domain/test_applications.py +++ b/tests/domain/test_applications.py @@ -1,7 +1,9 @@ import pytest from uuid import uuid4 +from atst.models import CSPRole from atst.domain.applications import Applications +from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import NotFoundError from tests.factories import ( @@ -100,3 +102,29 @@ def test_delete_application(session): # changes are flushed assert not session.dirty + + +def test_create_member(): + application = ApplicationFactory.create() + env1 = EnvironmentFactory.create(application=application) + env2 = EnvironmentFactory.create(application=application) + user_data = UserFactory.dictionary() + permission_set_names = [PermissionSets.EDIT_APPLICATION_TEAM] + + member_role = Applications.create_member( + application, + user_data, + permission_set_names, + environment_roles_data=[ + {"environment_id": env1.id, "role": CSPRole.BASIC_ACCESS.value}, + {"environment_id": env2.id, "role": None}, + ], + ) + + assert member_role.user.dod_id == user_data["dod_id"] + # view application AND edit application team + assert len(member_role.permission_sets) == 2 + + env_roles = member_role.user.environment_roles + assert len(env_roles) == 1 + assert env_roles[0].environment == env1 diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index d5a8e764..1c3e0a17 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -1,6 +1,6 @@ from flask import url_for -from tests.factories import PortfolioFactory, ApplicationFactory +from tests.factories import PortfolioFactory, ApplicationFactory, UserFactory def test_application_team(client, user_session): @@ -12,3 +12,43 @@ def test_application_team(client, user_session): response = client.get(url_for("applications.team", application_id=application.id)) assert response.status_code == 200 + + +def test_create_member(client, user_session): + user = UserFactory.create() + application = ApplicationFactory.create( + environments=[{"name": "Naboo"}, {"name": "Endor"}] + ) + env = application.environments[0] + + user_session(application.portfolio.owner) + + response = client.post( + url_for("applications.create_member", application_id=application.id), + data={ + "user_data-first_name": user.first_name, + "user_data-last_name": user.last_name, + "user_data-dod_id": user.dod_id, + "user_data-email": user.email, + "environment_roles-0-environment_id": env.id, + "environment_roles-0-environment_name": env.name, + "environment_roles-0-role": "Basic Access", + "permission_sets-perms_env_mgmt": True, + "permission_sets-perms_team_mgmt": True, + "permission_sets-perms_del_env": True, + }, + ) + + assert response.status_code == 302 + expected_url = url_for( + "applications.team", + application_id=application.id, + fragment="application-members", + _anchor="application-members", + _external=True, + ) + assert response.location == expected_url + assert len(user.application_roles) == 1 + assert user.application_roles[0].application == application + assert len(user.environment_roles) == 1 + assert user.environment_roles[0].environment == env diff --git a/tests/services/test_invitation.py b/tests/services/test_invitation.py index 364cdbcc..afa4ddde 100644 --- a/tests/services/test_invitation.py +++ b/tests/services/test_invitation.py @@ -1,9 +1,15 @@ -from tests.factories import UserFactory, PortfolioFactory, PortfolioRoleFactory +from tests.factories import ( + ApplicationFactory, + ApplicationRoleFactory, + UserFactory, + PortfolioFactory, + PortfolioRoleFactory, +) from atst.services.invitation import Invitation -def test_invite_member(queue): +def test_invite_portfolio_member(queue): inviter = UserFactory.create() new_member = UserFactory.create() portfolio = PortfolioFactory.create(owner=inviter) @@ -12,3 +18,14 @@ def test_invite_member(queue): new_invitation = invite_service.invite() assert new_invitation == new_member.portfolio_invitations[0] assert len(queue.get_queue()) == 1 + + +def test_invite_application_member(queue): + inviter = UserFactory.create() + new_member = UserFactory.create() + application = ApplicationFactory.create() + member = ApplicationRoleFactory.create(user=new_member, application=application) + invite_service = Invitation(inviter, member, new_member.email) + new_invitation = invite_service.invite() + assert new_invitation == new_member.application_invitations[0] + assert len(queue.get_queue()) == 1 diff --git a/translations.yaml b/translations.yaml index 43d406f5..7ec5a4e8 100644 --- a/translations.yaml +++ b/translations.yaml @@ -65,6 +65,7 @@ flash: next_steps: Review next steps below portfolio_home: Go to my portfolio home page success: Success! + new_application_member: 'You have successfully invited {user_name} to the team.' footer: about_link_text: Joint Enterprise Defense Infrastructure browser_support: JEDI Cloud supported on these web browsers From 9c84e3017205cbeb6983120989066afc8860732b Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 24 Apr 2019 13:43:38 -0400 Subject: [PATCH 05/14] frontend for adding new application member - updated styling - eliminated stray
tag in application team template --- atst/forms/application_member.py | 15 +- atst/forms/data.py | 3 + atst/routes/applications/team.py | 8 + js/components/forms/multi_step_modal_form.js | 2 + styles/components/_forms.scss | 16 ++ styles/sections/_application_edit.scss | 26 +++ templates/components/checkbox_input.html | 6 +- .../add_new_application_member.html | 104 +++++++++++ templates/portfolios/applications/team.html | 164 +++++++++--------- translations.yaml | 11 ++ 10 files changed, 266 insertions(+), 89 deletions(-) create mode 100644 templates/fragments/applications/add_new_application_member.html diff --git a/atst/forms/application_member.py b/atst/forms/application_member.py index 03c85953..4e54a17b 100644 --- a/atst/forms/application_member.py +++ b/atst/forms/application_member.py @@ -2,9 +2,10 @@ from wtforms.fields import FormField, FieldList, HiddenField, BooleanField from .forms import BaseForm from .member import NewForm as BaseNewMemberForm -from .data import ENV_ROLES +from .data import FORMATTED_ENV_ROLES as ENV_ROLES from atst.forms.fields import SelectField from atst.domain.permission_sets import PermissionSets +from atst.utils.localization import translate class EnvironmentForm(BaseForm): @@ -14,9 +15,15 @@ class EnvironmentForm(BaseForm): class PermissionsForm(BaseForm): - perms_env_mgmt = BooleanField(None, default=False) - perms_team_mgmt = BooleanField(None, default=False) - perms_del_env = BooleanField(None, default=False) + perms_env_mgmt = BooleanField( + translate("portfolios.applications.members.new.manage_envs"), default=False + ) + perms_team_mgmt = BooleanField( + translate("portfolios.applications.members.new.manage_team"), default=False + ) + perms_del_env = BooleanField( + translate("portfolios.applications.members.new.delete_envs"), default=False + ) @property def data(self): diff --git a/atst/forms/data.py b/atst/forms/data.py index 515a70e9..24e2cb7d 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -219,3 +219,6 @@ REQUIRED_DISTRIBUTIONS = [ ] ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [(None, "No access")] +FORMATTED_ENV_ROLES = [(role.value, "- {} -".format(role.value)) for role in CSPRole] + [ + (None, "- No Access -") +] diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 7fb75f0e..5bfbfaf5 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -6,6 +6,7 @@ from atst.domain.environments import Environments from atst.domain.applications import Applications from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.domain.permission_sets import PermissionSets +from atst.domain.exceptions import AlreadyExistsError from atst.forms.application_member import NewForm as NewMemberForm from atst.models.permissions import Permissions from atst.services.invitation import Invitation as InvitationService @@ -45,10 +46,17 @@ def team(application_id): ), } + env_roles = [ + {"environment_id": e.id, "environment_name": e.name} + for e in application.environments + ] + member_form = NewMemberForm(data={"environment_roles": env_roles}) + return render_template( "portfolios/applications/team.html", application=application, environment_users=environment_users, + member_form=member_form, ) diff --git a/js/components/forms/multi_step_modal_form.js b/js/components/forms/multi_step_modal_form.js index 5824e08c..ff50e373 100644 --- a/js/components/forms/multi_step_modal_form.js +++ b/js/components/forms/multi_step_modal_form.js @@ -1,6 +1,7 @@ import FormMixin from '../../mixins/form' import textinput from '../text_input' import optionsinput from '../options_input' +import checkboxinput from '../checkbox_input' import Selector from '../selector' import Modal from '../../mixins/modal' import toggler from '../toggler' @@ -16,6 +17,7 @@ export default { Selector, textinput, optionsinput, + checkboxinput, }, props: { diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index f78f2dc3..696a71ce 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -46,6 +46,14 @@ flex-basis: 16.66%; } + &.form-col--quarter { + flex-basis: 25%; + } + + &.form-col--three-quarters { + flex-basis: 75%; + } + .usa-input { margin-left: ($gap * 4); margin-right: ($gap * 4); @@ -171,3 +179,11 @@ } } +.input__inline-fields { + margin: 1rem 0 1rem 0; + + &> fieldset.usa-input__choices label { + display: inline; + font-weight: $font-normal; + } + } diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index c11c2f87..ab642442 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -59,3 +59,29 @@ } } } + +.environment-roles-new { + margin-top: 5*$gap; + margin-bottom: 8*$gap; + + .usa-input { + margin: 2rem 0 2rem 0; + + .usa-input__title-inline { + line-height: $hit-area; + } + + legend { + font-size: $lead-font-size; + padding: 0; + } + } + + .form-row { + margin: 0; + } +} + +.environment-roles-new__head { + font-weight: $font-bold; +} diff --git a/templates/components/checkbox_input.html b/templates/components/checkbox_input.html index 2f016517..998d825d 100644 --- a/templates/components/checkbox_input.html +++ b/templates/components/checkbox_input.html @@ -1,6 +1,6 @@ {% macro CheckboxInput( field, - label=field.label | striptags, + label=field.label, inline=False, classes="") -%} @@ -9,9 +9,7 @@
{{ field() }} - + {{ label | safe }} {% if field.description %} {{ field.description | safe }} diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html new file mode 100644 index 00000000..1f55efde --- /dev/null +++ b/templates/fragments/applications/add_new_application_member.html @@ -0,0 +1,104 @@ +{% from "components/icon.html" import Icon %} +{% from "components/text_input.html" import TextInput %} +{% from "components/checkbox_input.html" import CheckboxInput %} +{% from "components/multi_step_modal_form.html" import MultiStepModalForm %} + +{% set step_one %} +
+

Invite new portfolio member

+
+
+
+ {{ TextInput(member_form.user_data.first_name, validation='requiredField') }} +
+
+ {{ TextInput(member_form.user_data.last_name, validation='requiredField') }} +
+
+
+
+ {{ TextInput(member_form.user_data.email, validation='email') }} +
+
+ {{ TextInput(member_form.user_data.phone_number, validation='usPhone', optional=True) }} +
+
+
+
+ {{ TextInput(member_form.user_data.dod_id, validation='dodId') }} +
+
+
+
+
+ + Cancel +
+{% endset %} +{% set step_two %} +
+
+

{{ "portfolios.applications.members.new.assign_roles" | translate }}

+ + {{ Icon('info') }} + {{ "portfolios.applications.members.new.learn_more" | translate }} + +
+
+
+ + {{ "common.resource_names.environments" | translate }}: + +
+
+ + {{ "common.choose_role" | translate }}: + +
+
+ {% for environment_data in member_form.environment_roles %} +
+
+
+ +
+ {{ environment_data.environment_name.data }} +
+ +
+
+ {{ environment_data.role() }} +
+
+
+ {{ environment_data.environment_id() }} + {% endfor %} +
+

{{ "portfolios.applications.members.new.manage_perms" | translate({"application_name": application.name}) }}

+ {{ CheckboxInput(member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") }} + {{ CheckboxInput(member_form.permission_sets.perms_del_env, classes="input__inline-fields") }} + {{ CheckboxInput(member_form.permission_sets.perms_team_mgmt, classes="input__inline-fields") }} +
+
+ + Cancel +
+
+{% endset %} +{{ MultiStepModalForm( + 'add-app-mem', + member_form, + url_for("applications.create_member", application_id=application.id), + [step_one, step_two], + button_text=("portfolios.admin.add_new_member" | translate), + button_icon="plus-circle-solid", + ) }} diff --git a/templates/portfolios/applications/team.html b/templates/portfolios/applications/team.html index 2cb6f896..d8e70ffd 100644 --- a/templates/portfolios/applications/team.html +++ b/templates/portfolios/applications/team.html @@ -28,93 +28,95 @@ {% if g.matchesPath("application-members") %} {% include "fragments/flash.html" %} {% endif %} - -
-
-
-
- {{ "portfolios.applications.team_settings.section.title" | translate({ "application_name": application.name }) }} -
+
+
+
+
+ {{ "portfolios.applications.team_settings.section.title" | translate({ "application_name": application.name }) }}
- - {{ Icon('info') }} - {{ "portfolios.admin.settings_info" | translate }} -
-
- -
-
- - - {{ "portfolios.applications.team_settings.user" | translate }} - - - {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }} - - - {{ "portfolios.applications.team_settings.section.table.environment_management" | translate }} - - - {{ "portfolios.applications.team_settings.section.table.team_management" | translate }} - - - -
-
    - {% for member in application.members %} - {% set user = member.user %} - {% set user_info = environment_users[user.id] %} - {% set user_permissions = user_info["permissions"] %} - - - -
  • -
    - - {{ name }} - {{ user.full_name }} - {{ user_permissions["delete_access"] }} - {{ user_permissions["environment_management"] }} - {{ user_permissions["team_management"] }} - - - {% set open_html %} - {{ "common.show" | translate }} {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) - {% endset %} - - {% set close_html %} - {{ "common.hide" | translate }} {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) - {% endset %} - - {{ - ToggleButton( - open_html=open_html, - close_html=close_html, - section_name="environments" - ) - }} - -
    - {% call ToggleSection(section_name="environments") %} -
      - {% for environment in user_info["environments"] %} -
    • -
      - {{ environment.name }} -
      -
      • - {% endfor %} -
    - {% endcall %} -
    • -     - {% endfor %} -
+ + {{ Icon('info') }} + {{ "portfolios.admin.settings_info" | translate }} +
+
+ +
+
+ + + {{ "portfolios.applications.team_settings.user" | translate }} + + + {{ "portfolios.applications.team_settings.section.table.delete_access" | translate }} + + + {{ "portfolios.applications.team_settings.section.table.environment_management" | translate }} + + + {{ "portfolios.applications.team_settings.section.table.team_management" | translate }} + + + +
+
    + {% for member in application.members %} + {% set user = member.user %} + {% set user_info = environment_users[user.id] %} + {% set user_permissions = user_info["permissions"] %} + + + +
  • +
    + + {{ name }} + {{ user.full_name }} + {{ user_permissions["delete_access"] }} + {{ user_permissions["environment_management"] }} + {{ user_permissions["team_management"] }} + + + {% set open_html %} + {{ "common.show" | translate }} {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) + {% endset %} + + {% set close_html %} + {{ "common.hide" | translate }} {{ "portfolios.applications.team_settings.environments" | translate }} ({{ user_info['environments'] | length }}) + {% endset %} + + {{ + ToggleButton( + open_html=open_html, + close_html=close_html, + section_name="environments" + ) + }} + +
    + {% call ToggleSection(section_name="environments") %} +
      + {% for environment in user_info["environments"] %} +
    • +
      + {{ environment.name }} +
      +
      • + {% endfor %} +
    + {% endcall %} +
    • +     + {% endfor %} +
+
+ {% if user_can(permissions.CREATE_APPLICATION_MEMBER) %} + {% include "fragments/applications/add_new_application_member.html" %} + {% endif %}
diff --git a/translations.yaml b/translations.yaml index 7ec5a4e8..2f124d4b 100644 --- a/translations.yaml +++ b/translations.yaml @@ -35,6 +35,9 @@ common: save_and_continue: Save & continue show: Show sign: Sign + resource_names: + environments: Environments + choose_role: Choose a role components: modal: close: Close @@ -434,6 +437,14 @@ portfolios: user: User team_text: Team update_button_text: Save + members: + new: + assign_roles: Assign Member Environments and Roles + learn_more: Learn more about these roles + manage_perms: 'Manage permissions for {application_name}' + manage_envs: 'Allow member to add and rename environments within the application.' + delete_envs: 'Allow member to delete environments within the application.' + manage_team: 'Allow member to add, update and remove members from the application team.' index: empty: start_button: Start a new JEDI portfolio From 054d030e15b2f35a51330b5d768fd090c00661e6 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Apr 2019 09:56:37 -0400 Subject: [PATCH 06/14] Vue binding for environment role selections. The environment name will be grayed out until something besides the default "no access" is selected. Small changes to the application member subforms: - filter for "None" as a string - have nested forms inherit from FlaskForm; each nested form adds its own validation error flash otherwise if there are validation problems --- atst/forms/application_member.py | 12 +++++-- js/components/options_input.js | 1 + styles/sections/_application_edit.scss | 5 +++ .../add_new_application_member.html | 34 +++++++++++-------- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/atst/forms/application_member.py b/atst/forms/application_member.py index 4e54a17b..12e71873 100644 --- a/atst/forms/application_member.py +++ b/atst/forms/application_member.py @@ -1,3 +1,4 @@ +from flask_wtf import FlaskForm from wtforms.fields import FormField, FieldList, HiddenField, BooleanField from .forms import BaseForm @@ -8,13 +9,18 @@ from atst.domain.permission_sets import PermissionSets from atst.utils.localization import translate -class EnvironmentForm(BaseForm): +class EnvironmentForm(FlaskForm): environment_id = HiddenField() environment_name = HiddenField() - role = SelectField(environment_name, choices=ENV_ROLES, default=None) + role = SelectField( + environment_name, + choices=ENV_ROLES, + default=None, + filters=[lambda x: None if x == "None" else x], + ) -class PermissionsForm(BaseForm): +class PermissionsForm(FlaskForm): perms_env_mgmt = BooleanField( translate("portfolios.applications.members.new.manage_envs"), default=False ) diff --git a/js/components/options_input.js b/js/components/options_input.js index a2ad1838..fa4d8a57 100644 --- a/js/components/options_input.js +++ b/js/components/options_input.js @@ -18,6 +18,7 @@ export default { showError: showError, showValid: !showError && !!this.initialValue, validationError: this.initialErrors.join(' '), + value: this.initialValue, } }, diff --git a/styles/sections/_application_edit.scss b/styles/sections/_application_edit.scss index ab642442..3b106c1e 100644 --- a/styles/sections/_application_edit.scss +++ b/styles/sections/_application_edit.scss @@ -85,3 +85,8 @@ .environment-roles-new__head { font-weight: $font-bold; } + +.environment-name--gray { + font-weight: $font-normal; + color: $color-gray-medium; +} diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 1f55efde..cba84e70 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -5,7 +5,7 @@ {% set step_one %}
-

Invite new portfolio member

+

Invite new member

@@ -62,20 +62,24 @@
{% for environment_data in member_form.environment_roles %} -
-
-
- -
- {{ environment_data.environment_name.data }} -
- -
-
- {{ environment_data.role() }} -
-
-
+ +
+
+
+ +
+ {{ environment_data.environment_name.data }} +
+ +
+
+ {{ environment_data.role(**{"v-model": "value"}) }} +
+
+
+ {{ environment_data.environment_id() }} {% endfor %}
From 124970f9d6d61f4fe1956b8ebe2909d882d318fe Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Apr 2019 10:23:58 -0400 Subject: [PATCH 07/14] Frontend email validation should match wtforms Our long email regex for the frontend was stricter in some ways, but it allowed email addresses with a single-letter TLD ("frank@dod.m"), which the backend WTForms validator would reject. The two should be equivalent. Reference: https://bitbucket.org/simplecodes/wtforms/src/1939aec691af476960dfe16b4e17ba4fc070459b/wtforms/validators.py?at=default&fileviewer=file-view-default#validators.py-281 --- js/lib/input_validations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/lib/input_validations.js b/js/lib/input_validations.js index 00be8426..3a2fcd18 100644 --- a/js/lib/input_validations.js +++ b/js/lib/input_validations.js @@ -35,7 +35,7 @@ export default { }, email: { mask: emailMask, - match: /(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])/i, + match: /^.+@[^.].*\.[a-z]{2,10}$/, unmask: [], validationError: 'Please enter a valid e-mail address', }, From fd1d0b527a004965c22bac281a84fac2eab384ad Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Apr 2019 11:37:15 -0400 Subject: [PATCH 08/14] Delete environments permission disabled by default in new app member form It will be enabled when the user checks the "manage environments" permission. This updates the Jinja and Vue checkbox input components so that Vue can monitor the inpur state. --- js/components/checkbox_input.js | 11 +++++++ styles/components/_forms.scss | 6 +++- templates/components/checkbox_input.html | 30 ++++++++++++------- .../add_new_application_member.html | 25 ++++++++++++++-- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/js/components/checkbox_input.js b/js/components/checkbox_input.js index 6e646fa1..803041eb 100644 --- a/js/components/checkbox_input.js +++ b/js/components/checkbox_input.js @@ -3,8 +3,19 @@ import { emitEvent } from '../lib/emitters' export default { name: 'checkboxinput', + components: { + checkboxinput: this, + }, + props: { name: String, + initialChecked: Boolean, + }, + + data: function() { + return { + checked: this.initialChecked, + } }, methods: { diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index 696a71ce..5c30f1aa 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -182,8 +182,12 @@ .input__inline-fields { margin: 1rem 0 1rem 0; + &.input__inline-fields--indented { + margin-left: 4*$gap; + } + &> fieldset.usa-input__choices label { display: inline; font-weight: $font-normal; } - } +} diff --git a/templates/components/checkbox_input.html b/templates/components/checkbox_input.html index 998d825d..ac0906a2 100644 --- a/templates/components/checkbox_input.html +++ b/templates/components/checkbox_input.html @@ -3,19 +3,29 @@ label=field.label, inline=False, classes="") -%} - -
+ +
+
-
- - {{ field() }} - {{ label | safe }} +
+ + {{ field(**{"v-model": "checked"}) }} + {{ label | safe }} - {% if field.description %} + {% if field.description %} {{ field.description | safe }} - {% endif %} - -
+ {% endif %} + +
+
+ {% if caller %} + {{ caller() }} + {% endif %}
{%- endmacro %} diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index cba84e70..3f0eb559 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -84,9 +84,30 @@ {% endfor %}

{{ "portfolios.applications.members.new.manage_perms" | translate({"application_name": application.name}) }}

- {{ CheckboxInput(member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") }} - {{ CheckboxInput(member_form.permission_sets.perms_del_env, classes="input__inline-fields") }} {{ CheckboxInput(member_form.permission_sets.perms_team_mgmt, classes="input__inline-fields") }} + {% call CheckboxInput(member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") %} + {% set field=member_form.permission_sets.perms_del_env %} + +
+
+ + + {{ field.label | safe }} + +
+
+ + {% endcall %}
Date: Thu, 25 Apr 2019 13:21:21 -0400 Subject: [PATCH 09/14] fix form row rendering in chrome --- .../add_new_application_member.html | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 3f0eb559..42d393f9 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -66,16 +66,18 @@ v-bind:initial-value="'{{ environment_data.role.data | string }}'" >
-
-
- -
- {{ environment_data.environment_name.data }} -
- -
-
- {{ environment_data.role(**{"v-model": "value"}) }} +
+
+
+ +
+ {{ environment_data.environment_name.data }} +
+ +
+
+ {{ environment_data.role(**{"v-model": "value"}) }} +
From 0d5e0a3fa7557c8c289f05b242572477cba0a217 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Apr 2019 13:47:43 -0400 Subject: [PATCH 10/14] back button for new application member modal --- js/components/forms/multi_step_modal_form.js | 3 +++ styles/elements/_action_group.scss | 5 +++++ .../fragments/applications/add_new_application_member.html | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/js/components/forms/multi_step_modal_form.js b/js/components/forms/multi_step_modal_form.js index ff50e373..f0742336 100644 --- a/js/components/forms/multi_step_modal_form.js +++ b/js/components/forms/multi_step_modal_form.js @@ -47,6 +47,9 @@ export default { this.step += 1 } }, + previous: function() { + this.step -= 1 + }, goToStep: function(step) { if (this._checkIsValid()) { this.step = step diff --git a/styles/elements/_action_group.scss b/styles/elements/_action_group.scss index 2e1fb191..2d96dac8 100644 --- a/styles/elements/_action_group.scss +++ b/styles/elements/_action_group.scss @@ -21,4 +21,9 @@ &:last-child { margin-bottom: $gap * 3; } + + .action-group__action--left { + margin-left: 0; + margin-right: auto; + } } diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 42d393f9..85cf425e 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -118,6 +118,11 @@ form="add-app-mem" value='Invite member'> Cancel +
{% endset %} From b7a8cd4168f4557d55a7a385b0665b57994ea391 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 25 Apr 2019 14:12:06 -0400 Subject: [PATCH 11/14] Refinements to the new application member modal - remove extra padding from second modal screen - update modal styles to more closely match designs - ad `user_name` property to application_role model to fix flash message --- atst/models/application_role.py | 4 ++++ styles/components/_modal.scss | 4 +++- .../fragments/applications/add_new_application_member.html | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/atst/models/application_role.py b/atst/models/application_role.py index 3ae42845..fd606216 100644 --- a/atst/models/application_role.py +++ b/atst/models/application_role.py @@ -51,6 +51,10 @@ class ApplicationRole( "PermissionSet", secondary=application_roles_permission_sets ) + @property + def user_name(self): + return self.user.full_name + def __repr__(self): return "".format( self.application.name, self.user_id, self.id, self.permissions diff --git a/styles/components/_modal.scss b/styles/components/_modal.scss index b2c31798..2962a8f7 100644 --- a/styles/components/_modal.scss +++ b/styles/components/_modal.scss @@ -43,6 +43,7 @@ body { background-color: $color-white; padding: $gap * 2; width: 100%; + border-radius: 5px; overflow-y: auto; -ms-overflow-style: scrollbar; @@ -52,7 +53,7 @@ body { } @include media($medium-screen) { - padding: $gap * 4; + padding: $gap * 5; } h1, h2 { @@ -186,6 +187,7 @@ body { .form-row { margin-top: 0; + margin-bottom: 0; .form-col { .usa-input { diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 85cf425e..6e039726 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -41,7 +41,7 @@ {% endset %} {% set step_two %} -
+

{{ "portfolios.applications.members.new.assign_roles" | translate }}

From 47b07c2bb5d4ec0dfee8cd24f4be3f1909e037b8 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 29 Apr 2019 06:32:17 -0400 Subject: [PATCH 12/14] nested checkbox component --- js/components/checkbox_input.js | 5 +-- js/components/nested_checkbox_input.js | 31 +++++++++++++++++++ js/index.js | 2 ++ templates/components/checkbox_input.html | 2 +- .../add_new_application_member.html | 8 ++--- 5 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 js/components/nested_checkbox_input.js diff --git a/js/components/checkbox_input.js b/js/components/checkbox_input.js index 803041eb..db542fef 100644 --- a/js/components/checkbox_input.js +++ b/js/components/checkbox_input.js @@ -1,10 +1,11 @@ import { emitEvent } from '../lib/emitters' +import nestedcheckboxinput from './nested_checkbox_input' export default { name: 'checkboxinput', components: { - checkboxinput: this, + nestedcheckboxinput, }, props: { @@ -14,7 +15,7 @@ export default { data: function() { return { - checked: this.initialChecked, + isChecked: this.initialChecked, } }, diff --git a/js/components/nested_checkbox_input.js b/js/components/nested_checkbox_input.js new file mode 100644 index 00000000..ebe8553c --- /dev/null +++ b/js/components/nested_checkbox_input.js @@ -0,0 +1,31 @@ +import { emitEvent } from '../lib/emitters' + +export default { + name: 'nestedcheckboxinput', + + props: { + name: String, + isParentChecked: Boolean, + }, + + data: function() { + return { + isChecked: false, + } + }, + + updated: function() { + if (!this.isParentChecked) { + this.isChecked = false + } + }, + + methods: { + onInput: function(e) { + emitEvent('field-change', this, { + value: e.target.checked, + name: this.name, + }) + }, + }, +} diff --git a/js/index.js b/js/index.js index 44461fc2..5f5a886e 100644 --- a/js/index.js +++ b/js/index.js @@ -11,6 +11,7 @@ import optionsinput from './components/options_input' import multicheckboxinput from './components/multi_checkbox_input' import textinput from './components/text_input' import checkboxinput from './components/checkbox_input' +import nestedcheckboxinput from './components/nested_checkbox_input' import EditOfficerForm from './components/forms/edit_officer_form' import poc from './components/forms/poc' import oversight from './components/forms/oversight' @@ -74,6 +75,7 @@ const app = new Vue({ KoReview, BaseForm, DeleteConfirmation, + nestedcheckboxinput, }, mounted: function() { diff --git a/templates/components/checkbox_input.html b/templates/components/checkbox_input.html index ac0906a2..157c0409 100644 --- a/templates/components/checkbox_input.html +++ b/templates/components/checkbox_input.html @@ -14,7 +14,7 @@
- {{ field(**{"v-model": "checked"}) }} + {{ field(**{"v-model": "isChecked"}) }} {{ label | safe }} {% if field.description %} diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index 6e039726..d65d34e2 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -89,11 +89,11 @@ {{ CheckboxInput(member_form.permission_sets.perms_team_mgmt, classes="input__inline-fields") }} {% call CheckboxInput(member_form.permission_sets.perms_env_mgmt, classes="input__inline-fields") %} {% set field=member_form.permission_sets.perms_del_env %} -
@@ -102,8 +102,8 @@ id="permission_sets-perms_del_env" name="permission_sets-perms_del_env" type="checkbox" - v-model="checked" - v-bind:disabled="!$parent.checked"> + v-model="isChecked" + v-bind:disabled="!$parent.isChecked"> {{ field.label | safe }}
From 5db4d9bab3cebfee1877cb37e3479a65b1b5e495 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 29 Apr 2019 07:07:14 -0400 Subject: [PATCH 13/14] formatting, cleanup from rebase --- atst/forms/data.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/atst/forms/data.py b/atst/forms/data.py index 24e2cb7d..26f23547 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -1,6 +1,5 @@ from atst.models import CSPRole from atst.utils.localization import translate, translate_duration -from atst.models.environment_role import CSPRole SERVICE_BRANCHES = [ @@ -219,6 +218,6 @@ REQUIRED_DISTRIBUTIONS = [ ] ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [(None, "No access")] -FORMATTED_ENV_ROLES = [(role.value, "- {} -".format(role.value)) for role in CSPRole] + [ - (None, "- No Access -") -] +FORMATTED_ENV_ROLES = [ + (role.value, "- {} -".format(role.value)) for role in CSPRole +] + [(None, "- No Access -")] From 4f304d747e3993d105a3857416c14c278f5b858b Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 30 Apr 2019 14:49:53 -0400 Subject: [PATCH 14/14] Small tweaks for adding a new application member: - raise specific invitation type if invite not found in invitation domain classes - more terse assignments of defaults in invitation service, smh - terser margin expression for inline input fields - sass formatting - use translation for cancel link - oxford comma for app team management permission explanation - do not format environment roles with hyphens for role selection - generalize some additional methods in the invitation domain base class - use plain atst.models import path --- atst/domain/invitations.py | 14 +++++++++----- atst/forms/application_member.py | 2 +- atst/forms/data.py | 3 --- atst/routes/applications/team.py | 2 +- atst/routes/task_orders/invitations.py | 2 +- atst/services/invitation.py | 16 ++++------------ styles/components/_forms.scss | 4 ++-- .../applications/add_new_application_member.html | 4 ++-- tests/domain/test_invitations.py | 4 ++-- translations.yaml | 2 +- 10 files changed, 23 insertions(+), 30 deletions(-) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 209cd4d1..c38e88d8 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -4,6 +4,7 @@ from sqlalchemy.orm.exc import NoResultFound from atst.database import db from atst.models import ApplicationInvitation, InvitationStatus, PortfolioInvitation from atst.domain.portfolio_roles import PortfolioRoles +from atst.domain.application_roles import ApplicationRoles from .exceptions import NotFoundError @@ -40,6 +41,7 @@ class InvitationError(Exception): class BaseInvitations(object): model = None + role_domain_class = None # number of minutes a given invitation is considered valid EXPIRATION_LIMIT_MINUTES = 360 @@ -48,7 +50,7 @@ class BaseInvitations(object): try: invite = db.session.query(cls.model).filter_by(token=token).one() except NoResultFound: - raise NotFoundError("invite") + raise NotFoundError(cls.model.__tablename__) return invite @@ -86,7 +88,7 @@ class BaseInvitations(object): elif invite.is_pending: # pragma: no branch cls._update_status(invite, InvitationStatus.ACCEPTED) - PortfolioRoles.enable(invite.role) + cls.role_domain_class.enable(invite.role) return invite @classmethod @@ -109,11 +111,11 @@ class BaseInvitations(object): return cls._update_status(invite, InvitationStatus.REVOKED) @classmethod - def lookup_by_portfolio_and_user(cls, portfolio, user): - role = PortfolioRoles.get(portfolio.id, user.id) + def lookup_by_resource_and_user(cls, resource, user): + role = cls.role_domain_class.get(resource.id, user.id) if role.latest_invitation is None: - raise NotFoundError("invitation") + raise NotFoundError(cls.model.__tablename__) return role.latest_invitation @@ -127,7 +129,9 @@ class BaseInvitations(object): class PortfolioInvitations(BaseInvitations): model = PortfolioInvitation + role_domain_class = PortfolioRoles class ApplicationInvitations(BaseInvitations): model = ApplicationInvitation + role_domain_class = ApplicationRoles diff --git a/atst/forms/application_member.py b/atst/forms/application_member.py index 12e71873..1e13d896 100644 --- a/atst/forms/application_member.py +++ b/atst/forms/application_member.py @@ -3,7 +3,7 @@ from wtforms.fields import FormField, FieldList, HiddenField, BooleanField from .forms import BaseForm from .member import NewForm as BaseNewMemberForm -from .data import FORMATTED_ENV_ROLES as ENV_ROLES +from .data import ENV_ROLES from atst.forms.fields import SelectField from atst.domain.permission_sets import PermissionSets from atst.utils.localization import translate diff --git a/atst/forms/data.py b/atst/forms/data.py index 26f23547..2e0b689d 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -218,6 +218,3 @@ REQUIRED_DISTRIBUTIONS = [ ] ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [(None, "No access")] -FORMATTED_ENV_ROLES = [ - (role.value, "- {} -".format(role.value)) for role in CSPRole -] + [(None, "- No Access -")] diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index 5bfbfaf5..57994a3c 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -8,7 +8,7 @@ from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.domain.permission_sets import PermissionSets from atst.domain.exceptions import AlreadyExistsError from atst.forms.application_member import NewForm as NewMemberForm -from atst.models.permissions import Permissions +from atst.models import Permissions from atst.services.invitation import Invitation as InvitationService from atst.utils.flash import formatted_flash as flash from atst.utils.localization import translate diff --git a/atst/routes/task_orders/invitations.py b/atst/routes/task_orders/invitations.py index b89d6be2..8819859c 100644 --- a/atst/routes/task_orders/invitations.py +++ b/atst/routes/task_orders/invitations.py @@ -57,7 +57,7 @@ def resend_invite(task_order_id): if not officer: raise NotFoundError("officer") - invitation = PortfolioInvitations.lookup_by_portfolio_and_user(portfolio, officer) + invitation = PortfolioInvitations.lookup_by_resource_and_user(portfolio, officer) if not invitation: raise NotFoundError("invitation") diff --git a/atst/services/invitation.py b/atst/services/invitation.py index 877a3ab7..e9dec252 100644 --- a/atst/services/invitation.py +++ b/atst/services/invitation.py @@ -57,26 +57,18 @@ class Invitation: if isinstance(member, PortfolioRole): self.email_template = ( - "emails/portfolio/invitation.txt" - if self.email_template is None - else self.email_template + self.email_template or "emails/portfolio/invitation.txt" ) self.subject = ( - "{} has invited you to a JEDI cloud portfolio" - if self.subject is None - else self.subject + self.subject or "{} has invited you to a JEDI cloud portfolio" ) self.domain_class = PortfolioInvitations elif isinstance(member, ApplicationRole): self.email_template = ( - "emails/application/invitation.txt" - if self.email_template is None - else self.email_template + self.email_template or "emails/application/invitation.txt" ) self.subject = ( - "{} has invited you to a JEDI cloud application" - if self.subject is None - else self.subject + self.subject or "{} has invited you to a JEDI cloud application" ) self.domain_class = ApplicationInvitations diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index 5c30f1aa..3d91ca5e 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -180,10 +180,10 @@ } .input__inline-fields { - margin: 1rem 0 1rem 0; + margin: 1rem 0; &.input__inline-fields--indented { - margin-left: 4*$gap; + margin-left: $gap*4; } &> fieldset.usa-input__choices label { diff --git a/templates/fragments/applications/add_new_application_member.html b/templates/fragments/applications/add_new_application_member.html index d65d34e2..cbe18c9a 100644 --- a/templates/fragments/applications/add_new_application_member.html +++ b/templates/fragments/applications/add_new_application_member.html @@ -37,7 +37,7 @@ v-bind:disabled="invalid" class='action-group__action usa-button' value='Next'> - Cancel + {{ "common.cancel" | translate }}
{% endset %} {% set step_two %} @@ -117,7 +117,7 @@ class='action-group__action usa-button' form="add-app-mem" value='Invite member'> - Cancel + {{ "common.cancel" | translate }} add and rename environments within the application.' delete_envs: 'Allow member to delete environments within the application.' - manage_team: 'Allow member to add, update and remove members from the application team.' + manage_team: 'Allow member to add, update, and remove members from the application team.' index: empty: start_button: Start a new JEDI portfolio