From 237848c2c9d6006409e76c47faa6442ae9c82970 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Mon, 17 Feb 2020 16:12:46 -0500 Subject: [PATCH 01/11] Azure cloud method to get a url to the calculator --- .secrets.baseline | 4 ++-- README.md | 4 ++++ atst/domain/csp/cloud/azure_cloud_provider.py | 15 +++++++++++++- config/base.ini | 4 ++++ tests/domain/cloud/test_azure_csp.py | 20 +++++++++++++++++++ tests/mock_azure.py | 3 +++ 6 files changed, 47 insertions(+), 3 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 45f10336..d2fd7baf 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2020-02-12T18:51:01Z", + "generated_at": "2020-02-17T20:49:33Z", "plugins_used": [ { "base64_limit": 4.5, @@ -82,7 +82,7 @@ "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", "is_secret": false, "is_verified": false, - "line_number": 44, + "line_number": 48, "type": "Secret Keyword" } ], diff --git a/README.md b/README.md index accecd38..2982ff33 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,10 @@ To generate coverage reports for the Javascript tests: - `ASSETS_URL`: URL to host which serves static assets (such as a CDN). - `AZURE_ACCOUNT_NAME`: The name for the Azure blob storage account +- `AZURE_CALC_CLIENT_ID`: The client id used to generate a token for the Azure pricing calculator +- `AZURE_CALC_RESOURCE`: The resource URL used to generate a token for the Azure pricing calculator +- `AZURE_CALC_SECRET`: The secret key used to generate a token for the Azure pricing calculator +- `AZURE_CALC_URL`: The redirect URL for the Azure pricing calculator - `AZURE_LOGIN_URL`: The URL used to login for an Azure instance. - `AZURE_STORAGE_KEY`: A valid secret key for the Azure blob storage account - `AZURE_TO_BUCKET_NAME`: The Azure blob storage container name for task order uploads diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 679d5cbd..39c2e83a 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -1738,7 +1738,6 @@ class AzureCloudProvider(CloudProviderInterface): cost_mgmt_url = ( f"/providers/Microsoft.CostManagement/query?api-version=2019-11-01" ) - try: result = self.sdk.requests.post( f"{self.sdk.cloud.endpoints.resource_manager}{payload.invoice_section_id}{cost_mgmt_url}", @@ -1770,3 +1769,17 @@ class AzureCloudProvider(CloudProviderInterface): result.status_code, f"azure application error getting reporting data. {str(exc)}", ) + + def _get_calculator_creds(self): + authority = f"{self.sdk.cloud.endpoints.active_directory}/{self.tenant_id}" + context = self.sdk.adal.AuthenticationContext(authority=authority) + response = context.acquire_token_with_client_credentials( + self.config.get("AZURE_CALC_RESOURCE"), + self.config.get("AZURE_CALC_CLIENT_ID"), + self.config.get("AZURE_CALC_SECRET"), + ) + return response.get("accessToken") + + def get_calculator_url(self): + calc_access_token = self._get_calculator_creds() + return f"{self.config.get('AZURE_CALC_URL')}?access_token={calc_access_token}" diff --git a/config/base.ini b/config/base.ini index 3aa9ad86..a587c74f 100644 --- a/config/base.ini +++ b/config/base.ini @@ -3,6 +3,10 @@ ASSETS_URL AZURE_AADP_QTY=5 AZURE_ACCOUNT_NAME AZURE_CLIENT_ID +AZURE_CALC_CLIENT_ID +AZURE_CALC_RESOURCE="http://azurecom.onmicrosoft.com/acom-prod/" +AZURE_CALC_SECRET +AZURE_CALC_URL="https://azure.microsoft.com/en-us/pricing/calculator/" AZURE_GRAPH_RESOURCE="https://graph.microsoft.com/" AZURE_LOGIN_URL="https://portal.azure.com/" AZURE_POLICY_LOCATION=policies diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index b6a71501..5deddc63 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -1523,3 +1523,23 @@ def test_update_tenant_creds(mock_azure: AzureCloudProvider): assert updated_secret == KeyVaultCredentials( **{**existing_secrets, **MOCK_CREDS} ) + + +def test_get_calculator_creds(mock_azure: AzureCloudProvider): + mock_azure.sdk.adal.AuthenticationContext.return_value.acquire_token_with_client_credentials.return_value = { + "accessToken": "TOKEN" + } + assert mock_azure._get_calculator_creds() == "TOKEN" + + +def test_get_calculator_url(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_calculator_creds", + wraps=mock_azure._get_calculator_creds, + ) as _get_calculator_creds: + _get_calculator_creds.return_value = "TOKEN" + assert ( + mock_azure.get_calculator_url() + == f"{mock_azure.config.get('AZURE_CALC_URL')}?access_token=TOKEN" + ) diff --git a/tests/mock_azure.py b/tests/mock_azure.py index 2ca2a547..e89f6094 100644 --- a/tests/mock_azure.py +++ b/tests/mock_azure.py @@ -4,6 +4,9 @@ from unittest.mock import Mock from atst.domain.csp.cloud import AzureCloudProvider AZURE_CONFIG = { + "AZURE_CALC_CLIENT_ID": "MOCK", + "AZURE_CALC_SECRET": "MOCK", # pragma: allowlist secret + "AZURE_CALC_RESOURCE": "http://calc", "AZURE_CLIENT_ID": "MOCK", "AZURE_SECRET_KEY": "MOCK", "AZURE_TENANT_ID": "MOCK", From 207c34e5361982852a336a49c69b161f5a8a690f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 13 Feb 2020 11:17:09 -0500 Subject: [PATCH 02/11] Send email to user when App Role is created --- atst/jobs.py | 12 ++++++++++++ tests/test_jobs.py | 36 +++++++++++++++++++++++++++++++++++- translations.yaml | 5 ++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/atst/jobs.py b/atst/jobs.py index 77a8c2f6..55ff6027 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -111,6 +111,18 @@ def do_create_user(csp: CloudProviderInterface, application_role_ids=None): db.session.add(app_role) db.session.commit() + username = payload.user_principal_name + send_mail( + recipients=[user.email], + subject=translate("email.app_role_created.subject"), + body=translate( + "email.app_role_created.body", + {"url": app.config.get("AZURE_LOGIN_URL"), "username": username}, + ), + ) + app.logger.info( + f"Application role created notification email sent. User id: {user.id}" + ) def do_create_environment(csp: CloudProviderInterface, environment_id=None): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 9df83264..cb9ea0ff 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -153,12 +153,46 @@ def test_create_user_job(session, csp, app): cloud_id=None, ) + session.begin_nested() do_create_user(csp, [app_role.id]) - session.refresh(app_role) + session.rollback() assert app_role.cloud_id +def test_create_user_sends_email(monkeypatch, csp): + mock = Mock() + monkeypatch.setattr("atst.jobs.send_mail", mock) + + portfolio = PortfolioFactory.create( + csp_data={ + "tenant_id": str(uuid4()), + "domain_name": "rebelalliance.onmicrosoft.com", + } + ) + application_1 = ApplicationFactory.create(portfolio=portfolio, cloud_id="321") + application_2 = ApplicationFactory.create(portfolio=portfolio, cloud_id="123") + + user = UserFactory.create() + + app_role_1 = ApplicationRoleFactory.create( + user=user, + application=application_1, + status=ApplicationRoleStatus.ACTIVE, + cloud_id=None, + ) + + app_role_2 = ApplicationRoleFactory.create( + user=user, + application=application_2, + status=ApplicationRoleStatus.ACTIVE, + cloud_id=None, + ) + + do_create_user(csp, [app_role_1.id, app_role_2.id]) + assert mock.call_count == 1 + + def test_dispatch_create_environment(session, monkeypatch): # Given that I have a portfolio with an active CLIN and two environments, # one of which is deleted diff --git a/translations.yaml b/translations.yaml index 4bb463cc..9c77b396 100644 --- a/translations.yaml +++ b/translations.yaml @@ -83,7 +83,10 @@ errors: not_found_sub: This page does not exist. email: application_invite: "{inviter_name} has invited you to a JEDI cloud application" - portfolio_invite: "{inviter_name} has invited you to a JEDI cloud portfolio" + app_role_created: + subject: Application Role Created + body: "Your application role has been created.\nVisit {url}, and use your username, {username}, to log in." + portfolio_invite: "{inviter_name} has invited you to a JEDI cloud portfolio." portfolio_ready: subject: Portfolio Provisioned body: "Your portfolio has been provisioned.\nVisit {password_reset_address}, and use your username, {username}, to create a password." From 0deca6afcb3d79d679d420546318daef45f0f6e8 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Thu, 13 Feb 2020 11:28:15 -0500 Subject: [PATCH 03/11] Refactor tests --- tests/test_jobs.py | 95 +++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index cb9ea0ff..25a0b4c8 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -135,62 +135,63 @@ def test_create_application_job_is_idempotent(csp): csp.create_application.assert_not_called() -def test_create_user_job(session, csp, app): - portfolio = PortfolioFactory.create( - csp_data={ - "tenant_id": str(uuid4()), - "domain_name": f"rebelalliance.{app.config.get('OFFICE_365_DOMAIN')}", - } - ) - application = ApplicationFactory.create(portfolio=portfolio, cloud_id="321") - user = UserFactory.create( - first_name="Han", last_name="Solo", email="han@example.com" - ) - app_role = ApplicationRoleFactory.create( - application=application, - user=user, - status=ApplicationRoleStatus.ACTIVE, - cloud_id=None, - ) +class TestCreateUserJob: + @pytest.fixture + def portfolio(self, app): + return PortfolioFactory.create( + csp_data={ + "tenant_id": str(uuid4()), + "domain_name": f"rebelalliance.{app.config.get('OFFICE_365_DOMAIN')}", + } + ) - session.begin_nested() - do_create_user(csp, [app_role.id]) - session.rollback() + @pytest.fixture + def app_1(self, portfolio): + return ApplicationFactory.create(portfolio=portfolio, cloud_id="321") - assert app_role.cloud_id + @pytest.fixture + def app_2(self, portfolio): + return ApplicationFactory.create(portfolio=portfolio, cloud_id="123") + @pytest.fixture + def user(self): + return UserFactory.create( + first_name="Han", last_name="Solo", email="han@example.com" + ) -def test_create_user_sends_email(monkeypatch, csp): - mock = Mock() - monkeypatch.setattr("atst.jobs.send_mail", mock) + @pytest.fixture + def app_role_1(self, app_1, user): + return ApplicationRoleFactory.create( + application=app_1, + user=user, + status=ApplicationRoleStatus.ACTIVE, + cloud_id=None, + ) - portfolio = PortfolioFactory.create( - csp_data={ - "tenant_id": str(uuid4()), - "domain_name": "rebelalliance.onmicrosoft.com", - } - ) - application_1 = ApplicationFactory.create(portfolio=portfolio, cloud_id="321") - application_2 = ApplicationFactory.create(portfolio=portfolio, cloud_id="123") + @pytest.fixture + def app_role_2(self, app_2, user): + return ApplicationRoleFactory.create( + application=app_2, + user=user, + status=ApplicationRoleStatus.ACTIVE, + cloud_id=None, + ) - user = UserFactory.create() + def test_create_user_job(self, session, csp, app_role_1): + assert not app_role_1.cloud_id - app_role_1 = ApplicationRoleFactory.create( - user=user, - application=application_1, - status=ApplicationRoleStatus.ACTIVE, - cloud_id=None, - ) + session.begin_nested() + do_create_user(csp, [app_role_1.id]) + session.rollback() - app_role_2 = ApplicationRoleFactory.create( - user=user, - application=application_2, - status=ApplicationRoleStatus.ACTIVE, - cloud_id=None, - ) + assert app_role_1.cloud_id - do_create_user(csp, [app_role_1.id, app_role_2.id]) - assert mock.call_count == 1 + def test_create_user_sends_email(self, monkeypatch, csp, app_role_1, app_role_2): + mock = Mock() + monkeypatch.setattr("atst.jobs.send_mail", mock) + + do_create_user(csp, [app_role_1.id, app_role_2.id]) + assert mock.call_count == 1 def test_dispatch_create_environment(session, monkeypatch): From 266509413655f5e8563a029c98b9c93f49086570 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 17 Feb 2020 11:05:43 -0500 Subject: [PATCH 04/11] Apply max width to all input elements. Update form class with better name and add form-conrainer--narrow class. --- styles/components/_forms.scss | 13 +++++++++---- styles/core/_util.scss | 4 ---- styles/core/_variables.scss | 1 + styles/elements/_inputs.scss | 5 ++--- templates/applications/new/step_1.html | 2 +- templates/applications/new/step_2.html | 2 +- templates/applications/settings.html | 2 +- templates/portfolios/admin.html | 2 +- templates/portfolios/new/step_1.html | 2 +- templates/task_orders/builder_base.html | 2 +- 10 files changed, 18 insertions(+), 17 deletions(-) diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index f11af8f8..255622f8 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -22,8 +22,9 @@ } .usa-input { - input { - max-width: none; + input, + textarea { + max-width: $max-input-width; } } } @@ -204,6 +205,10 @@ } } -.form-container__half { - max-width: 46rem; +.form-container { + margin-bottom: $action-footer-height + $large-spacing; + + &--narrow { + max-width: $max-input-width; + } } diff --git a/styles/core/_util.scss b/styles/core/_util.scss index 7fd8e152..0790a121 100644 --- a/styles/core/_util.scss +++ b/styles/core/_util.scss @@ -98,7 +98,3 @@ hr { .usa-section { padding: 0; } - -.form { - margin-bottom: $action-footer-height + $large-spacing; -} diff --git a/styles/core/_variables.scss b/styles/core/_variables.scss index 09b838f3..f98a3663 100644 --- a/styles/core/_variables.scss +++ b/styles/core/_variables.scss @@ -21,6 +21,7 @@ $home-pg-icon-width: 6rem; $large-spacing: 4rem; $max-page-width: $max-panel-width + $sidenav-expanded-width + $large-spacing; $action-footer-height: 6rem; +$max-input-width: 46rem; /* * USWDS Variables diff --git a/styles/elements/_inputs.scss b/styles/elements/_inputs.scss index c424b723..c5a02e59 100644 --- a/styles/elements/_inputs.scss +++ b/styles/elements/_inputs.scss @@ -58,7 +58,7 @@ .usa-input { margin: ($gap * 2) 0; - max-width: 75rem; + max-width: $max-input-width; &-label-helper { font-size: $small-font-size; @@ -111,8 +111,7 @@ @include h5; font-weight: normal; - - @include line-max; + max-width: $max-input-width; .icon-link { padding: 0 ($gap / 2); diff --git a/templates/applications/new/step_1.html b/templates/applications/new/step_1.html index 39656257..688aa5a3 100644 --- a/templates/applications/new/step_1.html +++ b/templates/applications/new/step_1.html @@ -22,7 +22,7 @@ {% include "fragments/flash.html" %} -
+ {{ form.csrf_token }}
diff --git a/templates/applications/new/step_2.html b/templates/applications/new/step_2.html index fe07b44d..5fa6be34 100644 --- a/templates/applications/new/step_2.html +++ b/templates/applications/new/step_2.html @@ -21,7 +21,7 @@

- +
{{ 'portfolios.applications.environments_heading' | translate }}
diff --git a/templates/applications/settings.html b/templates/applications/settings.html index 1ec7be37..9fc42a17 100644 --- a/templates/applications/settings.html +++ b/templates/applications/settings.html @@ -20,7 +20,7 @@ {% if user_can(permissions.EDIT_APPLICATION) %} - + {{ application_form.csrf_token }} {{ TextInput(application_form.name, validation="applicationName", optional=False) }} {{ TextInput(application_form.description, validation="defaultTextAreaField", paragraph=True, optional=True, showOptional=False) }} diff --git a/templates/portfolios/admin.html b/templates/portfolios/admin.html index 40721c0d..6e0e795d 100644 --- a/templates/portfolios/admin.html +++ b/templates/portfolios/admin.html @@ -13,7 +13,7 @@
{% include "fragments/flash.html" %} -
+

Portfolio name and component

{% if user_can(permissions.EDIT_PORTFOLIO_NAME) %} diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html index d7d4f522..911f19db 100644 --- a/templates/portfolios/new/step_1.html +++ b/templates/portfolios/new/step_1.html @@ -19,7 +19,7 @@ {{ StickyCTA(text="portfolios.new.cta_step_1"|translate, context=("portfolios.new.sticky_header_context"|translate({"step": "1"}) )) }}
- + {{ form.csrf_token }}
diff --git a/templates/task_orders/builder_base.html b/templates/task_orders/builder_base.html index 41c253f6..7ee402fb 100644 --- a/templates/task_orders/builder_base.html +++ b/templates/task_orders/builder_base.html @@ -28,7 +28,7 @@ {% include "fragments/flash.html" %} -
+
{% block to_builder_form_field %}{% endblock %}
Date: Mon, 17 Feb 2020 11:49:32 -0500 Subject: [PATCH 05/11] Fix inputs in member modal forms --- styles/components/_member_form.scss | 15 ++++----------- styles/elements/_inputs.scss | 11 ++++------- templates/applications/fragments/members.html | 5 +++-- templates/components/multi_step_modal_form.html | 4 ++-- .../portfolios/fragments/portfolio_members.html | 5 +++-- 5 files changed, 16 insertions(+), 24 deletions(-) diff --git a/styles/components/_member_form.scss b/styles/components/_member_form.scss index 5cbeaf87..3644fce2 100644 --- a/styles/components/_member_form.scss +++ b/styles/components/_member_form.scss @@ -5,17 +5,11 @@ margin-left: 0; } - .input__inline-fields { - text-align: left; - - .usa-input__choices label { - font-weight: $font-bold; - } - } - .input__inline-fields { padding: $gap * 2; border: 1px solid $color-gray-lighter; + text-align: left; + max-width: 100%; &.checked { border: 1px solid $color-blue; @@ -33,7 +27,7 @@ .user-info { .usa-input { - width: 45rem; + max-width: $max-input-width; input, label, @@ -53,8 +47,7 @@ } } -#modal--add-app-mem, -#modal--add-portfolio-manager { +.form-content--member-form { .modal__body { min-width: 75rem; } diff --git a/styles/elements/_inputs.scss b/styles/elements/_inputs.scss index c5a02e59..1a1d1252 100644 --- a/styles/elements/_inputs.scss +++ b/styles/elements/_inputs.scss @@ -371,19 +371,15 @@ select { .phone-input { display: flex; flex-direction: row; + justify-content: flex-start; &__phone { margin-right: $gap * 4; + flex-grow: 1; .usa-input { margin: 0; - input, - label, - .usa-input__message { - max-width: 20rem; - } - .icon-validation { left: 20rem; } @@ -391,7 +387,8 @@ select { } &__extension { - margin-left: $gap * 4; + margin-right: $gap * 4; + flex-grow: 0; .usa-input { margin: 0; diff --git a/templates/applications/fragments/members.html b/templates/applications/fragments/members.html index f60fcba8..5fbecd35 100644 --- a/templates/applications/fragments/members.html +++ b/templates/applications/fragments/members.html @@ -36,7 +36,7 @@ {% set invite_expired = member.role_status == 'invite_expired' %} {%- if user_can(permissions.EDIT_APPLICATION_MEMBER) %} {% set modal_name = "edit_member-{}".format(loop.index) %} - {% call Modal(modal_name, classes="form-content--app-mem") %} + {% call Modal(modal_name, classes="form-content--member-form") %}

{{ Icon('avatar') }} {{ "portfolios.applications.members.form.edit_access_header" | translate({ "user": member.user_name }) }}

@@ -56,7 +56,7 @@ {%- if invite_pending or invite_expired %} {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} - {% call Modal(resend_invite_modal, classes="form-content--app-mem") %} + {% call Modal(resend_invite_modal, classes="form-content--member-form") %}

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

@@ -183,6 +183,7 @@ modal=new_member_modal_name, ) ], + classes="form-content--member-form", ) }} {% endif %}
diff --git a/templates/components/multi_step_modal_form.html b/templates/components/multi_step_modal_form.html index a7f71edf..d3c9b520 100644 --- a/templates/components/multi_step_modal_form.html +++ b/templates/components/multi_step_modal_form.html @@ -25,10 +25,10 @@
{% endmacro %} -{% macro MultiStepModalForm(name, form, form_action, steps, dismissable=False) -%} +{% macro MultiStepModalForm(name, form, form_action, steps, dismissable=False, classes="") -%} {% set step_count = steps|length %} - {% call Modal(name=name, dismissable=dismissable) %} + {% call Modal(name=name, dismissable=dismissable, classes=classes) %} {{ form.csrf_token }}
diff --git a/templates/portfolios/fragments/portfolio_members.html b/templates/portfolios/fragments/portfolio_members.html index 91e99d24..6e9c4be2 100644 --- a/templates/portfolios/fragments/portfolio_members.html +++ b/templates/portfolios/fragments/portfolio_members.html @@ -14,7 +14,7 @@ {% set invite_expired = member.status == 'invite_expired' %} {% set modal_name = "edit_member-{}".format(loop.index) %} - {% call Modal(modal_name, classes="form-content--app-mem") %} + {% call Modal(modal_name, classes="form-content--member-form") %}

{{ Icon('avatar') }} {{ "portfolios.applications.members.form.edit_access_header" | translate({ "user": member.user_name }) }}

@@ -34,7 +34,7 @@ {% if invite_pending or invite_expired -%} {% set resend_invite_modal = "resend_invite-{}".format(member.role_id) %} - {% call Modal(resend_invite_modal, classes="form-content--app-mem") %} + {% call Modal(resend_invite_modal, classes="form-content--member-form") %}

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

@@ -182,6 +182,7 @@ modal=new_manager_modal, ) ], + classes="form-content--member-form", ) }} {% endif %}
From 151d3f7454dbc0b9f2ef6693b090a72fbf49b9b4 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 17 Feb 2020 11:52:22 -0500 Subject: [PATCH 06/11] Fix input width in TO form --- styles/sections/_task_order.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/styles/sections/_task_order.scss b/styles/sections/_task_order.scss index 79f391e0..1edc5b66 100644 --- a/styles/sections/_task_order.scss +++ b/styles/sections/_task_order.scss @@ -140,6 +140,10 @@ &__confirmation { margin-left: $gap * 8; } + + .usa-input { + max-width: 100%; + } } .task-order__modal-cancel { From 437ed6d9b92cea57156dc816000b284a7a9af755 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 17 Feb 2020 12:11:48 -0500 Subject: [PATCH 07/11] Update portfolio styling and move DoD component help text --- atst/forms/portfolio.py | 1 + styles/components/_forms.scss | 4 ---- styles/components/_portfolio_layout.scss | 12 ++++++++++++ styles/elements/_inputs.scss | 4 ++++ templates/portfolios/new/step_1.html | 7 ++++--- translations.yaml | 8 +++----- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/atst/forms/portfolio.py b/atst/forms/portfolio.py index 7c4e6644..74f5330d 100644 --- a/atst/forms/portfolio.py +++ b/atst/forms/portfolio.py @@ -33,6 +33,7 @@ class PortfolioForm(BaseForm): class PortfolioCreationForm(PortfolioForm): defense_component = SelectMultipleField( translate("forms.portfolio.defense_component.title"), + description=translate("forms.portfolio.defense_component.help_text"), choices=SERVICE_BRANCHES, widget=ListWidget(prefix_label=False), option_widget=CheckboxInput(), diff --git a/styles/components/_forms.scss b/styles/components/_forms.scss index 255622f8..6921eb01 100644 --- a/styles/components/_forms.scss +++ b/styles/components/_forms.scss @@ -2,10 +2,6 @@ .form-row { margin: ($gap * 4) 0; - &--bordered { - border-bottom: $color-gray-lighter 1px solid; - } - .form-col { flex-grow: 1; diff --git a/styles/components/_portfolio_layout.scss b/styles/components/_portfolio_layout.scss index e9af6d5e..8aae2ca5 100644 --- a/styles/components/_portfolio_layout.scss +++ b/styles/components/_portfolio_layout.scss @@ -519,3 +519,15 @@ } } } + +#portfolio-create { + .usa-input__choices { + .usa-input__title { + font-weight: $font-bold; + } + + label { + font-size: $base-font-size; + } + } +} diff --git a/styles/elements/_inputs.scss b/styles/elements/_inputs.scss index 1a1d1252..6bd3acbb 100644 --- a/styles/elements/_inputs.scss +++ b/styles/elements/_inputs.scss @@ -179,6 +179,10 @@ left: -3rem; } } + + .usa-input__title { + margin-bottom: $gap; + } } select { diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html index 911f19db..0173d031 100644 --- a/templates/portfolios/new/step_1.html +++ b/templates/portfolios/new/step_1.html @@ -21,22 +21,23 @@
{{ form.csrf_token }} -
+
{{ TextInput(form.name, validation="portfolioName", optional=False, classes="form-col") }} {{"forms.portfolio.name.help_text" | translate | safe }}
-
+
+
{{ TextInput(form.description, validation="defaultTextAreaField", paragraph=True) }} {{"forms.portfolio.description.help_text" | translate | safe }}
+
{{ MultiCheckboxInput(form.defense_component, optional=False) }} - {{ "forms.portfolio.defense_component.help_text" | translate | safe }}
+

Naming can be difficult. Choose a name that is descriptive enough for users to identify the Portfolio. You may consider naming based on your organization.

@@ -285,7 +285,7 @@ forms: description: label: Portfolio Description help_text: | -
+

Add a brief one to two sentence description of your Portfolio. Consider this your statement of work.

@@ -310,11 +310,9 @@ forms: title: Select DoD component(s) funding your Portfolio validation_message: You must select at least one defense component. help_text: | -

Select the DOD component(s) that will fund all Applications within this Portfolio. In JEDI, multiple DoD organizations can fund the same Portfolio.
- Select all that apply.
-

+ Select all that apply. attachment: object_name: length_error: Object name may be no longer than 40 characters. From 411f0477e9f8339d1a2e83fb6ce56a97558d6b8f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 10:35:43 -0500 Subject: [PATCH 08/11] Schedule celery task for dispatch_send_task_order_files --- atst/queue.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atst/queue.py b/atst/queue.py index 10bcb350..392fc8e2 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -27,6 +27,10 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment_role", "schedule": 60, }, + "beat-dispatch_send_task_order_files": { + "task": "atst.jobs.dispatch_send_task_order_files", + "schedule": 60, + }, } class ContextTask(celery.Task): From b8bf106a96fa000223407d9ee659883d04bc6b12 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 10:56:35 -0500 Subject: [PATCH 09/11] Move dispatch_send_task_order_files tests into a test class --- tests/test_jobs.py | 127 ++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 66 deletions(-) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 25a0b4c8..dd285ca4 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -415,82 +415,77 @@ def test_create_environment_role(): assert env_role.cloud_id == "a-cloud-id" -# TODO: Refactor the tests related to dispatch_send_task_order_files() into a class -# and separate the success test into two tests -def test_dispatch_send_task_order_files(monkeypatch, app): - mock = Mock() - monkeypatch.setattr("atst.jobs.send_mail", mock) +class TestDispatchSendTaskOrderFiles: + @pytest.fixture(scope="function") + def send_mail(self, monkeypatch): + mock = Mock() + monkeypatch.setattr("atst.jobs.send_mail", mock) + return mock - def _download_task_order(MockFileService, object_name): - return {"name": object_name} + @pytest.fixture(scope="function") + def download_task_order(self, monkeypatch): + def _download_task_order(MockFileService, object_name): + return {"name": object_name} - monkeypatch.setattr( - "atst.domain.csp.files.MockFileService.download_task_order", - _download_task_order, - ) + monkeypatch.setattr( + "atst.domain.csp.files.MockFileService.download_task_order", + _download_task_order, + ) - # Create 3 new Task Orders - for i in range(3): - TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + def test_sends_multiple_emails(self, send_mail, download_task_order): + # Create 3 Task Orders + for i in range(3): + TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + dispatch_send_task_order_files.run() - # Check that send_with_attachment was called once for each task order - assert mock.call_count == 3 - mock.reset_mock() + # Check that send_with_attachment was called once for each task order + assert send_mail.call_count == 3 - # Create new TO - task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - assert not task_order.pdf_last_sent_at + def test_kwargs(self, send_mail, download_task_order, app): + task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + dispatch_send_task_order_files.run() - dispatch_send_task_order_files.run() + # Check that send_with_attachment was called with correct kwargs + send_mail.assert_called_once_with( + recipients=[app.config.get("MICROSOFT_TASK_ORDER_EMAIL_ADDRESS")], + subject=translate( + "email.task_order_sent.subject", {"to_number": task_order.number} + ), + body=translate( + "email.task_order_sent.body", {"to_number": task_order.number} + ), + attachments=[ + { + "name": task_order.pdf.object_name, + "maintype": "application", + "subtype": "pdf", + } + ], + ) + assert task_order.pdf_last_sent_at - # Check that send_with_attachment was called with correct kwargs - mock.assert_called_once_with( - recipients=[app.config.get("MICROSOFT_TASK_ORDER_EMAIL_ADDRESS")], - subject=translate( - "email.task_order_sent.subject", {"to_number": task_order.number} - ), - body=translate("email.task_order_sent.body", {"to_number": task_order.number}), - attachments=[ - { - "name": task_order.pdf.object_name, - "maintype": "application", - "subtype": "pdf", - } - ], - ) + def test_send_failure(self, monkeypatch): + def _raise_smtp_exception(**kwargs): + raise SMTPException - assert task_order.pdf_last_sent_at + monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) + task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + dispatch_send_task_order_files.run() + # Check that pdf_last_sent_at has not been updated + assert not task_order.pdf_last_sent_at -def test_dispatch_send_task_order_files_send_failure(monkeypatch): - def _raise_smtp_exception(**kwargs): - raise SMTPException + def test_download_failure(self, send_mail, monkeypatch): + def _download_task_order(MockFileService, object_name): + raise AzureError("something went wrong") - monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) + monkeypatch.setattr( + "atst.domain.csp.files.MockFileService.download_task_order", + _download_task_order, + ) + task_order = TaskOrderFactory.create(create_clins=[{"number": "0002"}]) + dispatch_send_task_order_files.run() - task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() - - # Check that pdf_last_sent_at has not been updated - assert not task_order.pdf_last_sent_at - - -def test_dispatch_send_task_order_files_download_failure(monkeypatch): - mock = Mock() - monkeypatch.setattr("atst.jobs.send_mail", mock) - - def _download_task_order(MockFileService, object_name): - raise AzureError("something went wrong") - - monkeypatch.setattr( - "atst.domain.csp.files.MockFileService.download_task_order", - _download_task_order, - ) - - task_order = TaskOrderFactory.create(create_clins=[{"number": "0002"}]) - dispatch_send_task_order_files.run() - - # Check that pdf_last_sent_at has not been updated - assert not task_order.pdf_last_sent_at + # Check that pdf_last_sent_at has not been updated + assert not task_order.pdf_last_sent_at From 1d8bb2811d7c0c5050bd41f5efcaa87f939f6073 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 11:55:22 -0500 Subject: [PATCH 10/11] Update name of function --- atst/jobs.py | 4 ++-- atst/queue.py | 4 ++-- tests/test_jobs.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/atst/jobs.py b/atst/jobs.py index 55ff6027..c125bcb5 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -292,7 +292,7 @@ def dispatch_create_atat_admin_user(self): @celery.task(bind=True) -def dispatch_send_task_order_files(self): +def send_task_order_files(self): task_orders = TaskOrders.get_for_send_task_order_files() recipients = [app.config.get("MICROSOFT_TASK_ORDER_EMAIL_ADDRESS")] @@ -313,7 +313,7 @@ def dispatch_send_task_order_files(self): app.logger.exception(err) continue - task_order.pdf_last_sent_at = pendulum.now() + task_order.pdf_last_sent_at = pendulum.now(tz="UTC") db.session.add(task_order) db.session.commit() diff --git a/atst/queue.py b/atst/queue.py index 392fc8e2..5ed2f136 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -27,8 +27,8 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment_role", "schedule": 60, }, - "beat-dispatch_send_task_order_files": { - "task": "atst.jobs.dispatch_send_task_order_files", + "beat-send_task_order_files": { + "task": "atst.jobs.send_task_order_files", "schedule": 60, }, } diff --git a/tests/test_jobs.py b/tests/test_jobs.py index dd285ca4..ff681c7e 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -16,7 +16,6 @@ from atst.jobs import ( dispatch_create_user, dispatch_create_environment_role, dispatch_provision_portfolio, - dispatch_send_task_order_files, create_environment, do_create_user, do_provision_portfolio, @@ -24,6 +23,7 @@ from atst.jobs import ( do_create_environment_role, do_create_application, send_PPOC_email, + send_task_order_files, ) from tests.factories import ( ApplicationFactory, @@ -415,7 +415,7 @@ def test_create_environment_role(): assert env_role.cloud_id == "a-cloud-id" -class TestDispatchSendTaskOrderFiles: +class TestSendTaskOrderFiles: @pytest.fixture(scope="function") def send_mail(self, monkeypatch): mock = Mock() @@ -437,14 +437,14 @@ class TestDispatchSendTaskOrderFiles: for i in range(3): TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that send_with_attachment was called once for each task order assert send_mail.call_count == 3 def test_kwargs(self, send_mail, download_task_order, app): task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that send_with_attachment was called with correct kwargs send_mail.assert_called_once_with( @@ -471,7 +471,7 @@ class TestDispatchSendTaskOrderFiles: monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that pdf_last_sent_at has not been updated assert not task_order.pdf_last_sent_at @@ -485,7 +485,7 @@ class TestDispatchSendTaskOrderFiles: _download_task_order, ) task_order = TaskOrderFactory.create(create_clins=[{"number": "0002"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that pdf_last_sent_at has not been updated assert not task_order.pdf_last_sent_at From a3e9400a0ddd6e364ab3132f834d885ed78e4540 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 19 Feb 2020 10:26:38 -0500 Subject: [PATCH 11/11] Tweak what "total portfolio value" represents On the reports page, "total portfolio value" should represent the sum of obligated funds for active task orders. --- atst/routes/portfolios/index.py | 4 +--- translations.yaml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index 6472fca1..02805480 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -50,9 +50,7 @@ def reports(portfolio_id): "portfolios/reports/index.html", portfolio=portfolio, # wrapped in str() because this sum returns a Decimal object - total_portfolio_value=str( - portfolio.total_obligated_funds + portfolio.upcoming_obligated_funds - ), + total_portfolio_value=str(portfolio.total_obligated_funds), current_obligated_funds=current_obligated_funds, expired_task_orders=Reports.expired_task_orders(portfolio), retrieved=pendulum.now(), # mocked datetime of reporting data retrival diff --git a/translations.yaml b/translations.yaml index 7e6da323..bc4e1479 100644 --- a/translations.yaml +++ b/translations.yaml @@ -516,7 +516,7 @@ portfolios: estimate_warning: Reports displayed in JEDI are estimates and not a system of record. To manage your costs, go to Azure by selecting the Login to Azure button above. total_value: header: Total Portfolio Value - tooltip: Total portfolio value is all obligated funds for current and upcoming task orders in this portfolio. + tooltip: Total portfolio value is all obligated funds for active task orders in this portfolio. task_orders: add_new_button: Add New Task Order review: