From d57b96cf05891e2f1a6d3e6fb9725509a80353c4 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Thu, 28 Feb 2019 10:00:18 -0500 Subject: [PATCH 01/12] wip --- atst/domain/invitations.py | 5 + atst/domain/task_orders.py | 13 ++ atst/routes/portfolios/task_orders.py | 92 +++++++- atst/services/invitation.py | 30 +-- atst/utils/flash.py | 5 + .../portfolios/task_orders/invitations.html | 123 ++++++----- tests/domain/test_invitations.py | 13 ++ tests/domain/test_task_orders.py | 27 ++- tests/routes/portfolios/test_task_orders.py | 208 ++++++++++++++---- translations.yaml | 5 + 10 files changed, 398 insertions(+), 123 deletions(-) diff --git a/atst/domain/invitations.py b/atst/domain/invitations.py index 24c11da3..06dab623 100644 --- a/atst/domain/invitations.py +++ b/atst/domain/invitations.py @@ -108,6 +108,11 @@ class Invitations(object): invite = Invitations._get(token) return Invitations._update_status(invite, InvitationStatus.REVOKED) + @classmethod + def lookup_by_portfolio_and_user(cls, portfolio, user): + portfolio_role = PortfolioRoles.get(portfolio.id, user.id) + return portfolio_role.latest_invitation + @classmethod def resend(cls, user, portfolio_id, token): portfolio = Portfolios.get(user, portfolio_id) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 63239679..473ca127 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -14,6 +14,10 @@ class TaskOrderError(Exception): pass +class InvalidOfficerError(Exception): + pass + + class TaskOrders(object): SECTIONS = { "app_info": [ @@ -145,6 +149,15 @@ class TaskOrders(object): "security_officer", ] + @classmethod + def remove_officer(cls, task_order, officer_type): + if officer_type in TaskOrders.OFFICERS: + setattr(task_order, officer_type, None) + db.session.add(task_order) + db.session.commit() + else: + raise (InvalidOfficerError) + @classmethod def add_officer(cls, user, task_order, officer_type, officer_data): Authorization.check_portfolio_permission( diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index ea0b3b95..554c60a4 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -4,15 +4,22 @@ from flask import g, redirect, render_template, url_for, request as http_request from . import portfolios_bp from atst.database import db -from atst.domain.task_orders import TaskOrders, DD254s -from atst.domain.exceptions import NotFoundError, NoAccessError -from atst.domain.portfolios import Portfolios from atst.domain.authz import Authorization +from atst.domain.exceptions import NotFoundError, NoAccessError +from atst.domain.invitations import Invitations +from atst.domain.portfolios import Portfolios +from atst.domain.task_orders import TaskOrders, DD254s +from atst.utils.localization import translate +from atst.forms.dd_254 import DD254Form +from atst.forms.ko_review import KOReviewForm from atst.forms.officers import EditTaskOrderOfficersForm from atst.models.task_order import Status as TaskOrderStatus -from atst.forms.ko_review import KOReviewForm -from atst.forms.dd_254 import DD254Form -from atst.services.invitation import update_officer_invitations +from atst.utils.flash import formatted_flash as flash +from atst.services.invitation import ( + update_officer_invitations, + OFFICER_INVITATIONS, + Invitation as InvitationService, +) @portfolios_bp.route("/portfolios//task_orders") @@ -96,6 +103,66 @@ def ko_review(portfolio_id, task_order_id): raise NoAccessError("task_order") +@portfolios_bp.route( + "/portfolios//task_order//resend_invite", + methods=["POST"], +) +def resend_invite(portfolio_id, task_order_id, form=None): + form_data = {**http_request.form} + invite_type = form_data["invite_type"][0] + + if invite_type not in dict.keys(OFFICER_INVITATIONS): + raise NotFoundError("invite_type") + + invite_type_info = OFFICER_INVITATIONS[invite_type] + + task_order = TaskOrders.get(g.current_user, task_order_id) + portfolio = Portfolios.get(g.current_user, portfolio_id) + + # + # TODO: Add in authorization check + # + + officer = getattr(task_order, invite_type_info["role"]) + + if not officer: + raise NotFoundError("officer") + + invitation = Invitations.lookup_by_portfolio_and_user(portfolio, officer) + + if not invitation: + raise NotFoundError("invitation") + + Invitations.resend(g.current_user, portfolio.id, invitation.token) + + invite_service = InvitationService( + g.current_user, + invitation.portfolio_role, + invitation.email, + subject=invite_type_info["subject"], + email_template=invite_type_info["template"], + ) + + invite_service.invite() + + flash( + "invitation_resent", + officer_type=translate( + "common.officer_helpers.underscore_to_friendly.{}".format( + invite_type_info["role"] + ) + ), + ) + + return redirect( + url_for( + "portfolios.task_order_invitations", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + ) + ) + + @portfolios_bp.route( "/portfolios//task_order//review", methods=["POST"] ) @@ -173,11 +240,14 @@ def edit_task_order_invitations(portfolio_id, task_order_id): ) ) else: - return render_template( - "portfolios/task_orders/invitations.html", - portfolio=portfolio, - task_order=task_order, - form=form, + return ( + render_template( + "portfolios/task_orders/invitations.html", + portfolio=portfolio, + task_order=task_order, + form=form, + ), + 400, ) diff --git a/atst/services/invitation.py b/atst/services/invitation.py index 1a18f4f8..b3bd94fb 100644 --- a/atst/services/invitation.py +++ b/atst/services/invitation.py @@ -5,43 +5,43 @@ from atst.queue import queue from atst.domain.task_orders import TaskOrders from atst.domain.portfolio_roles import PortfolioRoles -OFFICER_INVITATIONS = [ - { - "field": "ko_invite", +OFFICER_INVITATIONS = { + "ko_invite": { "role": "contracting_officer", "subject": "Review a task order", "template": "emails/invitation.txt", }, - { - "field": "cor_invite", + "cor_invite": { "role": "contracting_officer_representative", "subject": "Help with a task order", "template": "emails/invitation.txt", }, - { - "field": "so_invite", + "so_invite": { "role": "security_officer", "subject": "Review security for a task order", "template": "emails/invitation.txt", }, -] +} def update_officer_invitations(user, task_order): - for officer_type in OFFICER_INVITATIONS: - field = officer_type["field"] - if getattr(task_order, field) and not getattr(task_order, officer_type["role"]): - officer_data = task_order.officer_dictionary(officer_type["role"]) + for invite_type in dict.keys(OFFICER_INVITATIONS): + invite_opts = OFFICER_INVITATIONS[invite_type] + + if getattr(task_order, invite_type) and not getattr( + task_order, invite_opts["role"] + ): + officer_data = task_order.officer_dictionary(invite_opts["role"]) officer = TaskOrders.add_officer( - user, task_order, officer_type["role"], officer_data + user, task_order, invite_opts["role"], officer_data ) pf_officer_member = PortfolioRoles.get(task_order.portfolio.id, officer.id) invite_service = Invitation( user, pf_officer_member, officer_data["email"], - subject=officer_type["subject"], - email_template=officer_type["template"], + subject=invite_opts["subject"], + email_template=invite_opts["template"], ) invite_service.invite() diff --git a/atst/utils/flash.py b/atst/utils/flash.py index 89261795..b47dc1d2 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -2,6 +2,11 @@ from flask import flash, render_template_string from atst.utils.localization import translate MESSAGES = { + "invitation_resent": { + "title_template": "The {{ officer_type }} invite has been resent", + "message_template": "Invitation has been resent", + "category": "success", + }, "task_order_draft": { "title_template": translate("task_orders.form.draft_alert_title"), "message_template": """ diff --git a/templates/portfolios/task_orders/invitations.html b/templates/portfolios/task_orders/invitations.html index e79951b7..c0934c14 100644 --- a/templates/portfolios/task_orders/invitations.html +++ b/templates/portfolios/task_orders/invitations.html @@ -6,6 +6,7 @@ {% from "components/icon.html" import Icon %} {% from "components/text_input.html" import TextInput %} + {% macro Link(text, icon_name, onClick=None, url='#', classes='') %} {{ Icon(icon_name) }} @@ -14,51 +15,54 @@ {% endmacro %} {% macro EditOfficerInfo(form, officer_type, invited) -%} - + {% endmacro %} {% macro OfficerInfo(task_order, officer_type, form) %} @@ -66,10 +70,18 @@

{{ ("task_orders.invitations." + officer_type + ".title") | translate }}

{{ ("task_orders.invitations." + officer_type + ".description") | translate }}

+ {% set prefix = { "contracting_officer": "ko", "contracting_officer_representative": "cor", "security_officer": "so" }[officer_type] %} + +
+ {{ form.csrf_token }} + + +
+
- - {% set prefix = { "contracting_officer": "ko", "contracting_officer_representative": "cor", "security_officer": "so" }[officer_type] %} {% set first_name = task_order[prefix + "_first_name"] %} {% set last_name = task_order[prefix + "_last_name"] %} {% set email = task_order[prefix + "_email"] %} @@ -137,24 +149,21 @@ {% endmacro %} {% block portfolio_content %} +
{% include "fragments/flash.html" %} -
- {{ form.csrf_token }} - -
-
-

-
Edit Task Order
- Oversight -

-
- - {% for officer in ["contracting_officer", "contracting_officer_representative", "security_officer"] %} - {{ OfficerInfo(task_order, officer, form[officer]) }} - {% endfor %} +
+
+

+
Edit Task Order
+ Oversight +

- + + {% for officer in ["contracting_officer", "contracting_officer_representative", "security_officer"] %} + {{ OfficerInfo(task_order, officer, form[officer]) }} + {% endfor %} +
{% endblock %} diff --git a/tests/domain/test_invitations.py b/tests/domain/test_invitations.py index edcc8edd..7c0f0ff8 100644 --- a/tests/domain/test_invitations.py +++ b/tests/domain/test_invitations.py @@ -7,6 +7,7 @@ from atst.domain.invitations import ( InvitationError, WrongUserError, ExpiredError, + NotFoundError, ) from atst.models.invitation import Status @@ -144,3 +145,15 @@ def test_audit_event_for_accepted_invite(): accepted_event = AuditLog.get_by_resource(invite.id)[0] assert "email" in accepted_event.event_details assert "dod_id" in accepted_event.event_details + + +def test_lookup_by_user_and_portfolio(): + portfolio = PortfolioFactory.create() + user = UserFactory.create() + ws_role = PortfolioRoleFactory.create(user=user, portfolio=portfolio) + invite = Invitations.create(portfolio.owner, ws_role, user.email) + + assert Invitations.lookup_by_portfolio_and_user(portfolio, user) == invite + + with pytest.raises(NotFoundError): + Invitations.lookup_by_portfolio_and_user(portfolio, UserFactory.create()) diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index c7d2d371..36bac810 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -1,6 +1,11 @@ import pytest -from atst.domain.task_orders import TaskOrders, TaskOrderError, DD254s +from atst.domain.task_orders import ( + TaskOrders, + TaskOrderError, + InvalidOfficerError, + DD254s, +) from atst.domain.exceptions import UnauthorizedError from atst.models.attachment import Attachment @@ -127,6 +132,26 @@ def test_task_order_access(): ) +def test_remove_valid_officer(): + task_order = TaskOrderFactory.create() + owner = task_order.portfolio.owner + TaskOrders.add_officer( + owner, task_order, "contracting_officer", owner.to_dictionary() + ) + + assert task_order.contracting_officer == owner + + TaskOrders.remove_officer(task_order, "contracting_officer") + assert task_order.contracting_officer is None + + +def test_remove_invalid_officer(): + task_order = TaskOrderFactory.create() + + with pytest.raises(InvalidOfficerError): + TaskOrders.remove_officer(task_order, "invalid_officer_type") + + def test_dd254_complete(): finished = DD254Factory.create() unfinished = DD254Factory.create(certifying_official=None) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 0db25d16..940e0373 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -5,11 +5,14 @@ from datetime import timedelta, date from atst.domain.roles import Roles from atst.domain.task_orders import TaskOrders from atst.models.portfolio_role import Status as PortfolioStatus +from atst.models.invitation import Status as InvitationStatus from atst.utils.localization import translate from atst.queue import queue +from atst.domain.invitations import Invitations from tests.factories import ( PortfolioFactory, + InvitationFactory, PortfolioRoleFactory, TaskOrderFactory, UserFactory, @@ -20,9 +23,18 @@ from tests.factories import ( from tests.utils import captured_templates +@pytest.fixture +def portfolio(): + return PortfolioFactory.create() + + +@pytest.fixture +def user(): + return UserFactory.create() + + class TestPortfolioFunding: - def test_portfolio_with_no_task_orders(self, app, user_session): - portfolio = PortfolioFactory.create() + def test_portfolio_with_no_task_orders(self, app, user_session, portfolio): user_session(portfolio.owner) with captured_templates(app) as templates: @@ -38,8 +50,7 @@ class TestPortfolioFunding: assert context["active_task_orders"] == [] assert context["expired_task_orders"] == [] - def test_funded_portfolio(self, app, user_session): - portfolio = PortfolioFactory.create() + def test_funded_portfolio(self, app, user_session, portfolio): user_session(portfolio.owner) pending_to = TaskOrderFactory.create(portfolio=portfolio) @@ -71,8 +82,7 @@ class TestPortfolioFunding: assert context["funding_end_date"] is end_date assert context["total_balance"] == active_to1.budget + active_to2.budget - def test_expiring_and_funded_portfolio(self, app, user_session): - portfolio = PortfolioFactory.create() + def test_expiring_and_funded_portfolio(self, app, user_session, portfolio): user_session(portfolio.owner) expiring_to = TaskOrderFactory.create( @@ -98,8 +108,7 @@ class TestPortfolioFunding: assert context["funding_end_date"] is active_to.end_date assert context["funded"] == True - def test_expiring_and_unfunded_portfolio(self, app, user_session): - portfolio = PortfolioFactory.create() + def test_expiring_and_unfunded_portfolio(self, app, user_session, portfolio): user_session(portfolio.owner) expiring_to = TaskOrderFactory.create( @@ -154,6 +163,16 @@ class TestTaskOrderInvitations: assert updated_task_order.so_first_name == "Boba" assert updated_task_order.so_last_name == "Fett" assert len(queue.get_queue()) == queue_length + assert response.status_code == 302 + assert ( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=self.task_order.id, + _external=True, + ) + == response.headers["Location"] + ) def test_editing_with_complete_data(self, user_session, client): queue_length = len(queue.get_queue()) @@ -178,6 +197,16 @@ class TestTaskOrderInvitations: assert updated_task_order.ko_email == "luke@skywalker.mil" assert updated_task_order.ko_phone_number == "0123456789" assert len(queue.get_queue()) == queue_length + 1 + assert response.status_code == 302 + assert ( + url_for( + "portfolios.task_order_invitations", + portfolio_id=self.portfolio.id, + task_order_id=self.task_order.id, + _external=True, + ) + == response.headers["Location"] + ) def test_editing_with_invalid_data(self, user_session, client): queue_length = len(queue.get_queue()) @@ -196,19 +225,18 @@ class TestTaskOrderInvitations: updated_task_order = TaskOrders.get(self.portfolio.owner, self.task_order.id) assert updated_task_order.so_first_name != "Boba" assert len(queue.get_queue()) == queue_length + assert response.status_code == 400 -def test_ko_can_view_task_order(client, user_session): - portfolio = PortfolioFactory.create() - ko = UserFactory.create() +def test_ko_can_view_task_order(client, user_session, portfolio, user): PortfolioRoleFactory.create( - role=Roles.get("officer"), + role=Roles.get("owner"), portfolio=portfolio, - user=ko, + user=user, status=PortfolioStatus.ACTIVE, ) - task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=ko) - user_session(ko) + task_order = TaskOrderFactory.create(portfolio=portfolio, contracting_officer=user) + user_session(user) response = client.get( url_for( @@ -220,7 +248,7 @@ def test_ko_can_view_task_order(client, user_session): assert response.status_code == 200 assert translate("common.manage") in response.data.decode() - TaskOrders.update(ko, task_order, clin_01=None) + TaskOrders.update(user, task_order, clin_01=None) response = client.get( url_for( "portfolios.view_task_order", @@ -232,8 +260,7 @@ def test_ko_can_view_task_order(client, user_session): assert translate("common.manage") not in response.data.decode() -def test_can_view_task_order_invitations_when_complete(client, user_session): - portfolio = PortfolioFactory.create() +def test_can_view_task_order_invitations_when_complete(client, user_session, portfolio): user_session(portfolio.owner) task_order = TaskOrderFactory.create(portfolio=portfolio) response = client.get( @@ -246,8 +273,9 @@ def test_can_view_task_order_invitations_when_complete(client, user_session): assert response.status_code == 200 -def test_cant_view_task_order_invitations_when_not_complete(client, user_session): - portfolio = PortfolioFactory.create() +def test_cant_view_task_order_invitations_when_not_complete( + client, user_session, portfolio +): user_session(portfolio.owner) task_order = TaskOrderFactory.create(portfolio=portfolio, clin_01=None) response = client.get( @@ -324,8 +352,7 @@ def test_cor_cant_view_review_until_to_completed(client, user_session): assert response.status_code == 404 -def test_mo_redirected_to_build_page(client, user_session): - portfolio = PortfolioFactory.create() +def test_mo_redirected_to_build_page(client, user_session, portfolio): user_session(portfolio.owner) task_order = TaskOrderFactory.create(portfolio=portfolio) @@ -335,8 +362,7 @@ def test_mo_redirected_to_build_page(client, user_session): assert response.status_code == 200 -def test_cor_redirected_to_build_page(client, user_session): - portfolio = PortfolioFactory.create() +def test_cor_redirected_to_build_page(client, user_session, portfolio): cor = UserFactory.create() PortfolioRoleFactory.create( role=Roles.get("officer"), @@ -354,20 +380,18 @@ def test_cor_redirected_to_build_page(client, user_session): assert response.status_code == 200 -def test_submit_completed_ko_review_page_as_cor(client, user_session, pdf_upload): - portfolio = PortfolioFactory.create() - - cor = UserFactory.create() - +def test_submit_completed_ko_review_page_as_cor( + client, user_session, pdf_upload, portfolio, user +): PortfolioRoleFactory.create( role=Roles.get("officer"), portfolio=portfolio, - user=cor, + user=user, status=PortfolioStatus.ACTIVE, ) task_order = TaskOrderFactory.create( - portfolio=portfolio, contracting_officer_representative=cor + portfolio=portfolio, contracting_officer_representative=user ) form_data = { @@ -379,7 +403,7 @@ def test_submit_completed_ko_review_page_as_cor(client, user_session, pdf_upload "pdf": pdf_upload, } - user_session(cor) + user_session(user) response = client.post( url_for( @@ -399,9 +423,9 @@ def test_submit_completed_ko_review_page_as_cor(client, user_session, pdf_upload ) -def test_submit_completed_ko_review_page_as_ko(client, user_session, pdf_upload): - portfolio = PortfolioFactory.create() - +def test_submit_completed_ko_review_page_as_ko( + client, user_session, pdf_upload, portfolio +): ko = UserFactory.create() PortfolioRoleFactory.create( @@ -443,8 +467,7 @@ def test_submit_completed_ko_review_page_as_ko(client, user_session, pdf_upload) assert task_order.loas == loa_list -def test_so_review_page(app, client, user_session): - portfolio = PortfolioFactory.create() +def test_so_review_page(app, client, user_session, portfolio): so = UserFactory.create() PortfolioRoleFactory.create( role=Roles.get("officer"), @@ -482,8 +505,7 @@ def test_so_review_page(app, client, user_session): ) -def test_submit_so_review(app, client, user_session): - portfolio = PortfolioFactory.create() +def test_submit_so_review(app, client, user_session, portfolio): so = UserFactory.create() PortfolioRoleFactory.create( role=Roles.get("officer"), @@ -514,3 +536,111 @@ def test_submit_so_review(app, client, user_session): assert task_order.dd_254 assert task_order.dd_254.certifying_official == dd_254_data["certifying_official"] + + +def test_resend_invite_when_invalid_invite_officer( + app, client, user_session, portfolio, user +): + queue_length = len(queue.get_queue()) + + task_order = TaskOrderFactory.create( + portfolio=portfolio, contracting_officer=user, ko_invite=True + ) + + PortfolioRoleFactory.create( + role=Roles.get("owner"), + portfolio=portfolio, + user=user, + status=PortfolioStatus.ACTIVE, + ) + + user_session(user) + + response = client.post( + url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + _external=True, + ), + data={"invite_type": "invalid_invite_type"}, + ) + + assert response.status_code == 404 + assert len(queue.get_queue()) == queue_length + + +def test_resend_invite_when_officer_type_missing( + app, client, user_session, portfolio, user +): + queue_length = len(queue.get_queue()) + + task_order = TaskOrderFactory.create( + portfolio=portfolio, contracting_officer=None, ko_invite=True + ) + + PortfolioRoleFactory.create( + role=Roles.get("owner"), + portfolio=portfolio, + user=user, + status=PortfolioStatus.ACTIVE, + ) + + user_session(user) + + response = client.post( + url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + _external=True, + ), + data={"invite_type": "contracting_officer_invite"}, + ) + + assert response.status_code == 404 + assert len(queue.get_queue()) == queue_length + + +def test_resend_invite_when_ko(app, client, user_session, portfolio, user): + queue_length = len(queue.get_queue()) + + task_order = TaskOrderFactory.create( + portfolio=portfolio, contracting_officer=user, ko_invite=True + ) + + portfolio_role = PortfolioRoleFactory.create( + role=Roles.get("owner"), + portfolio=portfolio, + user=user, + status=PortfolioStatus.ACTIVE, + ) + + original_invitation = Invitations.create( + inviter=user, portfolio_role=portfolio_role, email=user.email + ) + + user_session(user) + + response = client.post( + url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + _external=True, + ), + data={"invite_type": "ko_invite"}, + ) + + assert original_invitation.status == InvitationStatus.REVOKED + assert response.status_code == 302 + assert ( + url_for( + "portfolios.task_order_invitations", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + _external=True, + ) + == response.headers["Location"] + ) + assert len(queue.get_queue()) == queue_length + 1 diff --git a/translations.yaml b/translations.yaml index dc4def8f..35e8f8fa 100644 --- a/translations.yaml +++ b/translations.yaml @@ -23,6 +23,11 @@ common: manage: manage save_and_continue: Save & Continue sign: Sign + officer_helpers: + underscore_to_friendly: + contracting_officer: Contracting Officer + security_officer: Security Officer + contracting_officer_representative: Contracting Officer Representative components: modal: close: Close From 5f241cf154ff5fb87c73eb7667e46b04fe0e8954 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Thu, 7 Mar 2019 10:57:35 -0500 Subject: [PATCH 02/12] Style the "Resend Invitation" button --- styles/components/_forms.scss | 3 +++ .../portfolios/task_orders/invitations.html | 23 +++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index f78f2dc3..fd0563d3 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -171,3 +171,6 @@ } } +.inline-form { + display: inline-block; +} diff --git a/templates/portfolios/task_orders/invitations.html b/templates/portfolios/task_orders/invitations.html index c0934c14..8718eb61 100644 --- a/templates/portfolios/task_orders/invitations.html +++ b/templates/portfolios/task_orders/invitations.html @@ -70,18 +70,9 @@

{{ ("task_orders.invitations." + officer_type + ".title") | translate }}

{{ ("task_orders.invitations." + officer_type + ".description") | translate }}

- {% set prefix = { "contracting_officer": "ko", "contracting_officer_representative": "cor", "security_officer": "so" }[officer_type] %} - -
- {{ form.csrf_token }} - - -
-
+ {% set prefix = { "contracting_officer": "ko", "contracting_officer_representative": "cor", "security_officer": "so" }[officer_type] %} {% set first_name = task_order[prefix + "_first_name"] %} {% set last_name = task_order[prefix + "_last_name"] %} {% set email = task_order[prefix + "_email"] %} @@ -89,7 +80,6 @@ {% set dod_id = task_order[prefix + "_dod_id"] %} {% set invited = False %} - {% if task_order[officer_type] %} {% set invited = True %}
@@ -106,7 +96,16 @@
{{ Link("Update", "edit", onClick="edit") }} - {{ Link("Resend Invitation", "avatar") }} + +
+ {{ form.csrf_token }} + + +
+ {{ Link("Remove", "trash", classes="remove") }}
{% elif first_name and last_name %} From a9199bc28d0aeb95c21fbcf3e6d8d3adfed880aa Mon Sep 17 00:00:00 2001 From: George Drummond Date: Thu, 7 Mar 2019 11:06:28 -0500 Subject: [PATCH 03/12] Remove method implemented but then not used --- atst/domain/task_orders.py | 13 ------------- tests/domain/test_task_orders.py | 27 +-------------------------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 473ca127..63239679 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -14,10 +14,6 @@ class TaskOrderError(Exception): pass -class InvalidOfficerError(Exception): - pass - - class TaskOrders(object): SECTIONS = { "app_info": [ @@ -149,15 +145,6 @@ class TaskOrders(object): "security_officer", ] - @classmethod - def remove_officer(cls, task_order, officer_type): - if officer_type in TaskOrders.OFFICERS: - setattr(task_order, officer_type, None) - db.session.add(task_order) - db.session.commit() - else: - raise (InvalidOfficerError) - @classmethod def add_officer(cls, user, task_order, officer_type, officer_data): Authorization.check_portfolio_permission( diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index 36bac810..c7d2d371 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -1,11 +1,6 @@ import pytest -from atst.domain.task_orders import ( - TaskOrders, - TaskOrderError, - InvalidOfficerError, - DD254s, -) +from atst.domain.task_orders import TaskOrders, TaskOrderError, DD254s from atst.domain.exceptions import UnauthorizedError from atst.models.attachment import Attachment @@ -132,26 +127,6 @@ def test_task_order_access(): ) -def test_remove_valid_officer(): - task_order = TaskOrderFactory.create() - owner = task_order.portfolio.owner - TaskOrders.add_officer( - owner, task_order, "contracting_officer", owner.to_dictionary() - ) - - assert task_order.contracting_officer == owner - - TaskOrders.remove_officer(task_order, "contracting_officer") - assert task_order.contracting_officer is None - - -def test_remove_invalid_officer(): - task_order = TaskOrderFactory.create() - - with pytest.raises(InvalidOfficerError): - TaskOrders.remove_officer(task_order, "invalid_officer_type") - - def test_dd254_complete(): finished = DD254Factory.create() unfinished = DD254Factory.create(certifying_official=None) From 679ec187a58d450965789d3d8603433a4a0c3d75 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Thu, 7 Mar 2019 11:19:03 -0500 Subject: [PATCH 04/12] You can only resend an invite for a pending invite --- atst/routes/portfolios/task_orders.py | 3 +- tests/routes/portfolios/test_task_orders.py | 37 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index 554c60a4..d88aaf67 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -14,6 +14,7 @@ from atst.forms.dd_254 import DD254Form from atst.forms.ko_review import KOReviewForm from atst.forms.officers import EditTaskOrderOfficersForm from atst.models.task_order import Status as TaskOrderStatus +from atst.models.invitation import Status as InvitationStatus from atst.utils.flash import formatted_flash as flash from atst.services.invitation import ( update_officer_invitations, @@ -130,7 +131,7 @@ def resend_invite(portfolio_id, task_order_id, form=None): invitation = Invitations.lookup_by_portfolio_and_user(portfolio, officer) - if not invitation: + if not invitation or (invitation.status is not InvitationStatus.PENDING): raise NotFoundError("invitation") Invitations.resend(g.current_user, portfolio.id, invitation.token) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 940e0373..0a7043e1 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -644,3 +644,40 @@ def test_resend_invite_when_ko(app, client, user_session, portfolio, user): == response.headers["Location"] ) assert len(queue.get_queue()) == queue_length + 1 + + +def test_resend_invite_when_not_pending(app, client, user_session, portfolio, user): + queue_length = len(queue.get_queue()) + + task_order = TaskOrderFactory.create( + portfolio=portfolio, contracting_officer=user, ko_invite=True + ) + + portfolio_role = PortfolioRoleFactory.create( + role=Roles.get("owner"), + portfolio=portfolio, + user=user, + status=PortfolioStatus.ACTIVE, + ) + + original_invitation = Invitations.create( + inviter=user, portfolio_role=portfolio_role, email=user.email + ) + + Invitations.accept(user=user, token=original_invitation.token) + + user_session(user) + + response = client.post( + url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + _external=True, + ), + data={"invite_type": "ko_invite"}, + ) + + assert original_invitation.status == InvitationStatus.ACCEPTED + assert response.status_code == 404 + assert len(queue.get_queue()) == queue_length From 064c3ab05b73c23718a95e8df50ac92948d0193b Mon Sep 17 00:00:00 2001 From: George Drummond Date: Thu, 7 Mar 2019 11:23:04 -0500 Subject: [PATCH 05/12] Add in authorization --- atst/routes/portfolios/task_orders.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index d88aaf67..4648bdea 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -120,9 +120,7 @@ def resend_invite(portfolio_id, task_order_id, form=None): task_order = TaskOrders.get(g.current_user, task_order_id) portfolio = Portfolios.get(g.current_user, portfolio_id) - # - # TODO: Add in authorization check - # + Authorization.check_is_ko(g.current_user, task_order) officer = getattr(task_order, invite_type_info["role"]) From d41622b842b46da104f1fabb2d90dc5bdc7149dc Mon Sep 17 00:00:00 2001 From: George Drummond Date: Tue, 12 Mar 2019 16:10:55 -0400 Subject: [PATCH 06/12] Don't need dict.keys --- atst/routes/portfolios/task_orders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index 4648bdea..b58d7960 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -112,7 +112,7 @@ def resend_invite(portfolio_id, task_order_id, form=None): form_data = {**http_request.form} invite_type = form_data["invite_type"][0] - if invite_type not in dict.keys(OFFICER_INVITATIONS): + if invite_type not in OFFICER_INVITATIONS: raise NotFoundError("invite_type") invite_type_info = OFFICER_INVITATIONS[invite_type] From d0ec4fb34d923cdd30b07027a95c9a216fd8a3ff Mon Sep 17 00:00:00 2001 From: George Drummond Date: Tue, 12 Mar 2019 16:39:14 -0400 Subject: [PATCH 07/12] Use invitation factory rather than domain class --- tests/routes/portfolios/test_task_orders.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index 0a7043e1..f90206af 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -660,12 +660,13 @@ def test_resend_invite_when_not_pending(app, client, user_session, portfolio, us status=PortfolioStatus.ACTIVE, ) - original_invitation = Invitations.create( - inviter=user, portfolio_role=portfolio_role, email=user.email + original_invitation = InvitationFactory.create( + inviter=user, + portfolio_role=portfolio_role, + email=user.email, + status=InvitationStatus.ACCEPTED, ) - Invitations.accept(user=user, token=original_invitation.token) - user_session(user) response = client.post( From 03bda7ec39749c436f38cc06482e63cc73ccab37 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 13 Mar 2019 09:45:57 -0400 Subject: [PATCH 08/12] Workspaces are now portfolios --- tests/models/test_invitation.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/models/test_invitation.py b/tests/models/test_invitation.py index 36aa54b2..0ecfff59 100644 --- a/tests/models/test_invitation.py +++ b/tests/models/test_invitation.py @@ -15,12 +15,12 @@ from tests.factories import ( def test_expired_invite_is_not_revokable(): portfolio = PortfolioFactory.create() user = UserFactory.create() - ws_role = PortfolioRoleFactory.create( + portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) invite = InvitationFactory.create( expiration_time=datetime.datetime.now() - datetime.timedelta(minutes=60), - portfolio_role=ws_role, + portfolio_role=portfolio_role, ) assert not invite.is_revokable @@ -28,18 +28,20 @@ def test_expired_invite_is_not_revokable(): def test_unexpired_invite_is_revokable(): portfolio = PortfolioFactory.create() user = UserFactory.create() - ws_role = PortfolioRoleFactory.create( + portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create(portfolio_role=ws_role) + invite = InvitationFactory.create(portfolio_role=portfolio_role) assert invite.is_revokable def test_invite_is_not_revokable_if_invite_is_not_pending(): portfolio = PortfolioFactory.create() user = UserFactory.create() - ws_role = PortfolioRoleFactory.create( + portfolio_role = PortfolioRoleFactory.create( portfolio=portfolio, user=user, status=PortfolioRoleStatus.PENDING ) - invite = InvitationFactory.create(portfolio_role=ws_role, status=Status.ACCEPTED) + invite = InvitationFactory.create( + portfolio_role=portfolio_role, status=Status.ACCEPTED + ) assert not invite.is_revokable From 8ca0e04402a675a28b43f4cf21d015b11968d6cd Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 13 Mar 2019 09:54:43 -0400 Subject: [PATCH 09/12] Take out Authorization.check_is_ko check --- atst/routes/portfolios/task_orders.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index b58d7960..7726fd6c 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -120,8 +120,6 @@ def resend_invite(portfolio_id, task_order_id, form=None): task_order = TaskOrders.get(g.current_user, task_order_id) portfolio = Portfolios.get(g.current_user, portfolio_id) - Authorization.check_is_ko(g.current_user, task_order) - officer = getattr(task_order, invite_type_info["role"]) if not officer: From c3cb46873e3faa01aa91c9d6b2b7c3f335da9f27 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 13 Mar 2019 11:33:55 -0400 Subject: [PATCH 10/12] Use ConfirmationPopover rather than custom form --- atst/routes/portfolios/task_orders.py | 3 +-- js/components/confirmation_popover.js | 8 ++++++- js/components/forms/edit_officer_form.js | 2 ++ styles/components/_forms.scss | 3 --- styles/sections/_task_order.scss | 4 ++++ templates/components/confirmation_button.html | 12 +++++++++- .../portfolios/task_orders/invitations.html | 23 ++++++++++++------- tests/routes/portfolios/test_task_orders.py | 4 ++-- 8 files changed, 42 insertions(+), 17 deletions(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index 7726fd6c..792d085a 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -109,8 +109,7 @@ def ko_review(portfolio_id, task_order_id): methods=["POST"], ) def resend_invite(portfolio_id, task_order_id, form=None): - form_data = {**http_request.form} - invite_type = form_data["invite_type"][0] + invite_type = http_request.args.get("invite_type") if invite_type not in OFFICER_INVITATIONS: raise NotFoundError("invite_type") diff --git a/js/components/confirmation_popover.js b/js/components/confirmation_popover.js index f58386ce..f771a516 100644 --- a/js/components/confirmation_popover.js +++ b/js/components/confirmation_popover.js @@ -4,10 +4,13 @@ export default { props: { action: String, btn_text: String, + btn_icon: String, + btn_class: String, cancel_btn_text: String, confirm_btn_text: String, confirm_msg: String, csrf_token: String, + name: String, }, template: ` @@ -26,7 +29,10 @@ export default {
- + `, } diff --git a/js/components/forms/edit_officer_form.js b/js/components/forms/edit_officer_form.js index 016187bc..dccf4327 100644 --- a/js/components/forms/edit_officer_form.js +++ b/js/components/forms/edit_officer_form.js @@ -1,6 +1,7 @@ import FormMixin from '../../mixins/form' import checkboxinput from '../checkbox_input' import textinput from '../text_input' +import ConfirmationPopover from '../confirmation_popover' export default { name: 'edit-officer-form', @@ -10,6 +11,7 @@ export default { components: { checkboxinput, textinput, + ConfirmationPopover, }, props: { diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index fd0563d3..f78f2dc3 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -171,6 +171,3 @@ } } -.inline-form { - display: inline-block; -} diff --git a/styles/sections/_task_order.scss b/styles/sections/_task_order.scss index 7a982261..a9d08b69 100644 --- a/styles/sections/_task_order.scss +++ b/styles/sections/_task_order.scss @@ -510,6 +510,10 @@ margin: 0 $gap; } + .v-popover { + display: inline-block; + } + .remove { color: $color-red; .icon { diff --git a/templates/components/confirmation_button.html b/templates/components/confirmation_button.html index 31db3355..dc74ba2c 100644 --- a/templates/components/confirmation_button.html +++ b/templates/components/confirmation_button.html @@ -1,6 +1,16 @@ -{% macro ConfirmationButton(btn_text, action, confirm_msg="Are you sure?", confirm_btn="Confirm", cancel_btn="Cancel") -%} +{% macro ConfirmationButton( + btn_text, + action, + btn_icon=None, + btn_class=None, + confirm_msg="Are you sure?", + confirm_btn="Confirm", + cancel_btn="Cancel") +-%}
{{ Link("Update", "edit", onClick="edit") }} + {% set invite_type = [prefix + "_invite"] %} -
- {{ form.csrf_token }} - - -
+ {{ + ConfirmationButton( + btn_text="Resend Invitation", + action=url_for( + "portfolios.resend_invite", + portfolio_id=portfolio.id, + task_order_id=task_order.id, + invite_type=invite_type, + ), + btn_icon=Icon('avatar'), + btn_class="icon-link", + ) + }} {{ Link("Remove", "trash", classes="remove") }}
diff --git a/tests/routes/portfolios/test_task_orders.py b/tests/routes/portfolios/test_task_orders.py index f90206af..ad93242a 100644 --- a/tests/routes/portfolios/test_task_orders.py +++ b/tests/routes/portfolios/test_task_orders.py @@ -627,9 +627,9 @@ def test_resend_invite_when_ko(app, client, user_session, portfolio, user): "portfolios.resend_invite", portfolio_id=portfolio.id, task_order_id=task_order.id, + invite_type="ko_invite", _external=True, - ), - data={"invite_type": "ko_invite"}, + ) ) assert original_invitation.status == InvitationStatus.REVOKED From faa212a04425b040ded99edfc3937ea79e12c495 Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 13 Mar 2019 13:41:13 -0400 Subject: [PATCH 11/12] Just revoke invite since service object handles creating new invite and sending it --- atst/routes/portfolios/task_orders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/portfolios/task_orders.py b/atst/routes/portfolios/task_orders.py index 792d085a..dd7856a7 100644 --- a/atst/routes/portfolios/task_orders.py +++ b/atst/routes/portfolios/task_orders.py @@ -129,7 +129,7 @@ def resend_invite(portfolio_id, task_order_id, form=None): if not invitation or (invitation.status is not InvitationStatus.PENDING): raise NotFoundError("invitation") - Invitations.resend(g.current_user, portfolio.id, invitation.token) + Invitations.revoke(token=invitation.token) invite_service = InvitationService( g.current_user, From 736079cf18295aee4f1093fe04e4b5b5bafb2beb Mon Sep 17 00:00:00 2001 From: George Drummond Date: Wed, 13 Mar 2019 13:45:27 -0400 Subject: [PATCH 12/12] Update js test snapshot --- .../__snapshots__/confirmation_popover.test.js.snap | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap index 2146b1aa..2e4be27b 100644 --- a/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap +++ b/js/components/__tests__/__snapshots__/confirmation_popover.test.js.snap @@ -1,3 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ConfirmationPopover matches snapshot 1`] = ` `; +exports[`ConfirmationPopover matches snapshot 1`] = ` + +`;