From ce9540d755ad70c311cbea423e07b2e1081f22ec Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 18 Feb 2020 15:51:48 -0500 Subject: [PATCH 01/17] Set shell for failing docker login command. On my last PR to fix this, I didn't notice that CircleCI was setting the shell to execute with `/bin/sh -eo pipefail`, which means that the commands will fail if any part of the command exits with 1, regardless of whether the result is being piped anywhere else. This updates the shell that that section of the config uses. https://circleci.com/docs/2.0/configuration-reference/#default-shell-options --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index c28b28a7..c1d2d810 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -96,6 +96,7 @@ commands: apk del --purge build - run: name: Login to Azure CLI + shell: /bin/sh -o pipefail command: | az login \ --service-principal \ From 237848c2c9d6006409e76c47faa6442ae9c82970 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Mon, 17 Feb 2020 16:12:46 -0500 Subject: [PATCH 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 07/17] 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 08/17] 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 09/17] 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 10/17] 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 11/17] 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 12/17] 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: From aa2d353260a409fed8df203a76adb203753bd74a Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 11 Feb 2020 15:13:17 -0500 Subject: [PATCH 13/17] Add query for finding new CLINs that have not been sent. Use fixtures in tests. --- atst/domain/task_orders.py | 10 +++++++ tests/domain/test_task_orders.py | 48 ++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 44d6b47e..7a538c2d 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -90,3 +90,13 @@ class TaskOrders(BaseDomainClass): ) .all() ) + + @classmethod + def get_clins_for_create_billing_instructions(cls): + return ( + db.session.query(CLIN) + .filter( + CLIN.last_sent_at.is_(None), CLIN.start_date < pendulum.now(tz="UTC") + ) + .all() + ) diff --git a/tests/domain/test_task_orders.py b/tests/domain/test_task_orders.py index 5999677a..4b56b030 100644 --- a/tests/domain/test_task_orders.py +++ b/tests/domain/test_task_orders.py @@ -9,6 +9,27 @@ from atst.models.task_order import TaskOrder, SORT_ORDERING, Status from tests.factories import TaskOrderFactory, CLINFactory, PortfolioFactory +@pytest.fixture +def new_task_order(): + return TaskOrderFactory.create(create_clins=[{}]) + + +@pytest.fixture +def updated_task_order(): + return TaskOrderFactory.create( + create_clins=[{"last_sent_at": pendulum.date(2020, 2, 1)}], + pdf_last_sent_at=pendulum.date(2020, 1, 1), + ) + + +@pytest.fixture +def sent_task_order(): + return TaskOrderFactory.create( + create_clins=[{"last_sent_at": pendulum.date(2020, 1, 1)}], + pdf_last_sent_at=pendulum.date(2020, 1, 1), + ) + + def test_create_adds_clins(): portfolio = PortfolioFactory.create() clins = [ @@ -181,19 +202,18 @@ def test_allows_alphanumeric_number(): assert TaskOrders.create(portfolio.id, number, [], None) -def test_get_for_send_task_order_files(): - new_to = TaskOrderFactory.create(create_clins=[{}]) - updated_to = TaskOrderFactory.create( - create_clins=[{"last_sent_at": pendulum.datetime(2020, 2, 1)}], - pdf_last_sent_at=pendulum.datetime(2020, 1, 1), - ) - sent_to = TaskOrderFactory.create( - create_clins=[{"last_sent_at": pendulum.datetime(2020, 1, 1)}], - pdf_last_sent_at=pendulum.datetime(2020, 1, 1), - ) - +def test_get_for_send_task_order_files( + new_task_order, updated_task_order, sent_task_order +): updated_and_new_task_orders = TaskOrders.get_for_send_task_order_files() assert len(updated_and_new_task_orders) == 2 - assert sent_to not in updated_and_new_task_orders - assert updated_to in updated_and_new_task_orders - assert new_to in updated_and_new_task_orders + assert sent_task_order not in updated_and_new_task_orders + assert updated_task_order in updated_and_new_task_orders + assert new_task_order in updated_and_new_task_orders + + +def test_get_clins_for_create_billing_instructions(new_task_order, sent_task_order): + new_clins = TaskOrders.get_clins_for_create_billing_instructions() + assert len(new_clins) == 1 + assert new_task_order.clins[0] in new_clins + assert sent_task_order.clins[0] not in new_clins From 6ef3265cb5f0e1ba76c51afef50e6c36f93c289f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 12 Feb 2020 11:20:21 -0500 Subject: [PATCH 14/17] Create celery task for create_billing_instruction --- atst/domain/csp/cloud/models.py | 8 +++++ atst/jobs.py | 29 +++++++++++++++++ atst/models/clin.py | 6 +++- tests/test_jobs.py | 58 ++++++++++++++++++++++++++++++++- 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 27f2c9c7..5bb056d7 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -220,6 +220,14 @@ class BillingInstructionCSPPayload(BaseCSPPayload): billing_account_name: str billing_profile_name: str + class Config: + fields = { + "initial_clin_amount": "obligated_amount", + "initial_clin_start_date": "start_date", + "initial_clin_end_date": "end_date", + "initial_clin_type": "number", + } + class BillingInstructionCSPResult(AliasModel): reported_clin_name: str diff --git a/atst/jobs.py b/atst/jobs.py index c125bcb5..e5805875 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -10,6 +10,7 @@ from atst.domain.csp.cloud import CloudProviderInterface from atst.domain.csp.cloud.exceptions import GeneralCSPException from atst.domain.csp.cloud.models import ( ApplicationCSPPayload, + BillingInstructionCSPPayload, EnvironmentCSPPayload, UserCSPPayload, UserRoleCSPPayload, @@ -317,3 +318,31 @@ def send_task_order_files(self): db.session.add(task_order) db.session.commit() + + +@celery.task(bind=True) +def create_billing_instruction(self): + clins = TaskOrders.get_clins_for_create_billing_instructions() + for clin in clins: + portfolio = clin.task_order.portfolio + clin_data = clin.to_dictionary() + portfolio_data = portfolio.to_dictionary() + + payload = BillingInstructionCSPPayload( + tenant_id=portfolio.csp_data.get("tenant_id"), + billing_account_name=portfolio.csp_data.get("billing_account_name"), + billing_profile_name=portfolio.csp_data.get("billing_profile_name"), + **clin_data, + **portfolio_data, + ) + + try: + app.csp.cloud.create_billing_instruction(payload) + except (AzureError) as err: + app.logger.exception(err) + continue + + clin.last_sent_at = pendulum.now(tz="UTC") + db.session.add(clin) + + db.session.commit() diff --git a/atst/models/clin.py b/atst/models/clin.py index accab107..440ee0a0 100644 --- a/atst/models/clin.py +++ b/atst/models/clin.py @@ -66,11 +66,15 @@ class CLIN(Base, mixins.TimestampsMixin): ) def to_dictionary(self): - return { + data = { c.name: getattr(self, c.name) for c in self.__table__.columns if c.name not in ["id"] } + data["start_date"] = str(data["start_date"]) + data["end_date"] = str(data["end_date"]) + + return data @property def is_active(self): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index ff681c7e..a7c6aced 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -6,7 +6,8 @@ from smtplib import SMTPException from azure.core.exceptions import AzureError from atst.domain.csp.cloud import MockCloudProvider -from atst.domain.csp.cloud.models import UserRoleCSPResult +from atst.domain.csp.cloud.models import BillingInstructionCSPPayload, UserRoleCSPResult +from atst.domain.portfolios import Portfolios from atst.models import ApplicationRoleStatus, Portfolio, FSMStates from atst.jobs import ( @@ -16,6 +17,7 @@ from atst.jobs import ( dispatch_create_user, dispatch_create_environment_role, dispatch_provision_portfolio, + create_billing_instruction, create_environment, do_create_user, do_provision_portfolio, @@ -489,3 +491,57 @@ class TestSendTaskOrderFiles: # Check that pdf_last_sent_at has not been updated assert not task_order.pdf_last_sent_at + + +class TestCreateBillingInstructions: + def test_update_clin_last_sent_at(self, session): + # create portfolio with one active clin + start_date = pendulum.now().subtract(days=1) + portfolio = PortfolioFactory.create( + csp_data={ + "tenant_id": str(uuid4()), + "billing_account_name": "fake", + "billing_profile_name": "fake", + }, + task_orders=[{"create_clins": [{"start_date": start_date}]}], + ) + unsent_clin = portfolio.task_orders[0].clins[0] + + assert not unsent_clin.last_sent_at + + # The session needs to be nested to prevent detached SQLAlchemy instance + session.begin_nested() + create_billing_instruction() + session.rollback() + + # check that last_sent_at has been updated + assert unsent_clin.last_sent_at + + def test_failure(self, monkeypatch, session): + def _create_billing_instruction(MockCloudProvider, object_name): + raise AzureError("something went wrong") + + monkeypatch.setattr( + "atst.domain.csp.cloud.MockCloudProvider.create_billing_instruction", + _create_billing_instruction, + ) + + # create portfolio with one active clin + start_date = pendulum.now().subtract(days=1) + portfolio = PortfolioFactory.create( + csp_data={ + "tenant_id": str(uuid4()), + "billing_account_name": "fake", + "billing_profile_name": "fake", + }, + task_orders=[{"create_clins": [{"start_date": start_date}]}], + ) + unsent_clin = portfolio.task_orders[0].clins[0] + + # The session needs to be nested to prevent detached SQLAlchemy instance + session.begin_nested() + create_billing_instruction() + session.rollback() + + # check that last_sent_at has not been updated + assert not unsent_clin.last_sent_at From 4dc5f2aa9130748b4b9fca0b46c86a91fe32c606 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 12 Feb 2020 12:05:16 -0500 Subject: [PATCH 15/17] Add beat task to queue --- atst/queue.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atst/queue.py b/atst/queue.py index 5ed2f136..dcd123d1 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -31,6 +31,10 @@ def update_celery(celery, app): "task": "atst.jobs.send_task_order_files", "schedule": 60, }, + "beat-create_billing_instruction": { + "task": "atst.jobs.create_billing_instruction", + "schedule": 60, + }, } class ContextTask(celery.Task): From 5c7dfc428ea3ea50438d3d75d93f5725f7395e11 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 15:30:19 -0500 Subject: [PATCH 16/17] Add new tests and refactor existing tests --- tests/test_jobs.py | 47 +++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index a7c6aced..8031d2aa 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -30,6 +30,7 @@ from atst.jobs import ( from tests.factories import ( ApplicationFactory, ApplicationRoleFactory, + CLINFactory, EnvironmentFactory, EnvironmentRoleFactory, PortfolioFactory, @@ -494,8 +495,8 @@ class TestSendTaskOrderFiles: class TestCreateBillingInstructions: - def test_update_clin_last_sent_at(self, session): - # create portfolio with one active clin + @pytest.fixture + def unsent_clin(self): start_date = pendulum.now().subtract(days=1) portfolio = PortfolioFactory.create( csp_data={ @@ -505,19 +506,20 @@ class TestCreateBillingInstructions: }, task_orders=[{"create_clins": [{"start_date": start_date}]}], ) - unsent_clin = portfolio.task_orders[0].clins[0] + return portfolio.task_orders[0].clins[0] + def test_update_clin_last_sent_at(self, session, unsent_clin): assert not unsent_clin.last_sent_at # The session needs to be nested to prevent detached SQLAlchemy instance session.begin_nested() create_billing_instruction() - session.rollback() # check that last_sent_at has been updated assert unsent_clin.last_sent_at + session.rollback() - def test_failure(self, monkeypatch, session): + def test_failure(self, monkeypatch, session, unsent_clin): def _create_billing_instruction(MockCloudProvider, object_name): raise AzureError("something went wrong") @@ -526,22 +528,41 @@ class TestCreateBillingInstructions: _create_billing_instruction, ) - # create portfolio with one active clin - start_date = pendulum.now().subtract(days=1) + # The session needs to be nested to prevent detached SQLAlchemy instance + session.begin_nested() + create_billing_instruction() + + # check that last_sent_at has not been updated + assert not unsent_clin.last_sent_at + session.rollback() + + def test_task_order_with_multiple_clins(self, session): + start_date = pendulum.now(tz="UTC").subtract(days=1) portfolio = PortfolioFactory.create( csp_data={ "tenant_id": str(uuid4()), "billing_account_name": "fake", "billing_profile_name": "fake", }, - task_orders=[{"create_clins": [{"start_date": start_date}]}], + task_orders=[ + { + "create_clins": [ + {"start_date": start_date, "last_sent_at": start_date} + ] + } + ], ) - unsent_clin = portfolio.task_orders[0].clins[0] + task_order = portfolio.task_orders[0] + sent_clin = task_order.clins[0] + + # Add new CLIN to the Task Order + new_clin = CLINFactory.create(task_order=task_order) + assert not new_clin.last_sent_at - # The session needs to be nested to prevent detached SQLAlchemy instance session.begin_nested() create_billing_instruction() - session.rollback() - # check that last_sent_at has not been updated - assert not unsent_clin.last_sent_at + # check that last_sent_at has been update for the new clin only + assert new_clin.last_sent_at + assert sent_clin.last_sent_at != new_clin.last_sent_at + session.rollback() From 0bda0c481e64f8050b78a37ee4862f70fc21e7ff Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 19 Feb 2020 11:10:20 -0500 Subject: [PATCH 17/17] Explicitly pass in kwargs instead of splatting clin and portfolio data --- atst/domain/csp/cloud/models.py | 8 -------- atst/jobs.py | 9 +++++---- atst/models/clin.py | 2 -- tests/test_jobs.py | 1 + 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 5bb056d7..27f2c9c7 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -220,14 +220,6 @@ class BillingInstructionCSPPayload(BaseCSPPayload): billing_account_name: str billing_profile_name: str - class Config: - fields = { - "initial_clin_amount": "obligated_amount", - "initial_clin_start_date": "start_date", - "initial_clin_end_date": "end_date", - "initial_clin_type": "number", - } - class BillingInstructionCSPResult(AliasModel): reported_clin_name: str diff --git a/atst/jobs.py b/atst/jobs.py index e5805875..855af5b2 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -325,15 +325,16 @@ def create_billing_instruction(self): clins = TaskOrders.get_clins_for_create_billing_instructions() for clin in clins: portfolio = clin.task_order.portfolio - clin_data = clin.to_dictionary() - portfolio_data = portfolio.to_dictionary() payload = BillingInstructionCSPPayload( tenant_id=portfolio.csp_data.get("tenant_id"), billing_account_name=portfolio.csp_data.get("billing_account_name"), billing_profile_name=portfolio.csp_data.get("billing_profile_name"), - **clin_data, - **portfolio_data, + initial_clin_amount=clin.obligated_amount, + initial_clin_start_date=str(clin.start_date), + initial_clin_end_date=str(clin.end_date), + initial_clin_type=clin.number, + initial_task_order_id=str(clin.task_order_id), ) try: diff --git a/atst/models/clin.py b/atst/models/clin.py index 440ee0a0..a96e907f 100644 --- a/atst/models/clin.py +++ b/atst/models/clin.py @@ -71,8 +71,6 @@ class CLIN(Base, mixins.TimestampsMixin): for c in self.__table__.columns if c.name not in ["id"] } - data["start_date"] = str(data["start_date"]) - data["end_date"] = str(data["end_date"]) return data diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 8031d2aa..eba01f9b 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -561,6 +561,7 @@ class TestCreateBillingInstructions: session.begin_nested() create_billing_instruction() + session.add(sent_clin) # check that last_sent_at has been update for the new clin only assert new_clin.last_sent_at