From 68a1ffde8b774d1dde507e67443fd4420a9c5c2e Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 28 Feb 2019 11:18:09 -0500 Subject: [PATCH 01/18] Portfolio Name and Defense Component read only on TOs that are started from existing portfolios --- atst/routes/task_orders/new.py | 5 +++++ templates/portfolios/task_orders/index.html | 2 +- templates/task_orders/new/app_info.html | 15 +++++++++++---- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 77cb0c63..235e5384 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -237,6 +237,11 @@ def new(screen, task_order_id=None, portfolio_id=None): task_order_id=task_order_id, ) url_args["next"] = template_args["next"] + elif http_request.args.get("new_to_on_portfolio"): + portfolio = Portfolios.get(g.current_user, portfolio_id) + template_args["portfolio_name"] = portfolio.name + template_args["defense_component"] = portfolio.defense_component + template_args["new_to_on_portfolio"] = True template_args["action_url"] = url_for("task_orders.update", **url_args) diff --git a/templates/portfolios/task_orders/index.html b/templates/portfolios/task_orders/index.html index c26bd969..002111be 100644 --- a/templates/portfolios/task_orders/index.html +++ b/templates/portfolios/task_orders/index.html @@ -93,7 +93,7 @@
{% for task_order in pending_task_orders %} diff --git a/templates/task_orders/new/app_info.html b/templates/task_orders/new/app_info.html index ee8fe008..22204840 100644 --- a/templates/task_orders/new/app_info.html +++ b/templates/task_orders/new/app_info.html @@ -4,6 +4,7 @@ {% from "components/options_input.html" import OptionsInput %} {% from "components/date_input.html" import DateInput %} {% from "components/multi_checkbox_input.html" import MultiCheckboxInput %} +{% from "components/review_field.html" import ReviewField %} {% block heading %} {{ "task_orders.new.app_info.section_title"| translate }} @@ -14,11 +15,17 @@

{{ "task_orders.new.app_info.basic_info_title"| translate }}

-{{ TextInput(form.portfolio_name, placeholder="The name of your office or organization", validation="portfolioName") }} -{{ TextInput(form.scope, paragraph=True) }} -

{{ "task_orders.new.app_info.sample_scope" | translate | safe }}

+ +{% if new_to_on_portfolio %} + {{ ReviewField(heading="forms.portfolio.name_label" | translate, field=portfolio_name) }} +{% else %} + {{ TextInput(form.portfolio_name, placeholder="The name of your office or organization", validation="portfolioName") }} + {{ TextInput(form.scope, paragraph=True) }} +

{{ "task_orders.new.app_info.sample_scope" | translate | safe }}

+{% endif %} +
- {{ OptionsInput(form.defense_component) }} + {{ ReviewField(heading="forms.task_order.defense_component_label" | translate, field=defense_component) if new_to_on_portfolio else OptionsInput(form.defense_component) }}

From 1a19a163c4654564ed7dc6d68ce744bee282c64e Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 28 Feb 2019 11:33:53 -0500 Subject: [PATCH 02/18] Update test --- tests/routes/task_orders/test_new_task_order.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index a6908a21..1a3ba077 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -91,10 +91,8 @@ def test_create_new_task_order_for_portfolio(client, user_session): task_order_data = TaskOrderFactory.dictionary() app_info_data = slice_data_for_section(task_order_data, "app_info") - portfolio_name = "This is ignored for now" - app_info_data["portfolio_name"] = portfolio_name - defense_component = "Defense Health Agency" # this is also ignored - app_info_data["defense_component"] = defense_component + app_info_data["portfolio_name"] = portfolio.name + app_info_data["defense_component"] = portfolio.defense_component response = client.post( url_for("task_orders.update", screen=1, portfolio_id=portfolio.id), @@ -105,6 +103,8 @@ def test_create_new_task_order_for_portfolio(client, user_session): created_task_order_id = response.headers["Location"].split("/")[-1] created_task_order = TaskOrders.get(creator, created_task_order_id) + assert created_task_order.portfolio_name == portfolio.name + assert created_task_order.defense_component == portfolio.defense_component assert created_task_order.portfolio == portfolio From 943aa79aba51bb59eabaa7f2777c0dd9551e6199 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 1 Mar 2019 11:44:32 -0500 Subject: [PATCH 03/18] make if/else loop more readable --- templates/task_orders/new/app_info.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/templates/task_orders/new/app_info.html b/templates/task_orders/new/app_info.html index 22204840..19e1b361 100644 --- a/templates/task_orders/new/app_info.html +++ b/templates/task_orders/new/app_info.html @@ -25,7 +25,11 @@ {% endif %}
- {{ ReviewField(heading="forms.task_order.defense_component_label" | translate, field=defense_component) if new_to_on_portfolio else OptionsInput(form.defense_component) }} + {% if new_to_on_portfolio %} + {{ ReviewField(heading="forms.task_order.defense_component_label" | translate, field=defense_component) }} + {% else %} + {{ OptionsInput(form.defense_component) }} + {% endif %}

From e8a7131948988c40229c9d0310f1fd648e8a7d55 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 1 Mar 2019 13:39:11 -0500 Subject: [PATCH 04/18] Send portfolio to template instead of using query param --- atst/routes/task_orders/new.py | 25 +++++++++------------ templates/portfolios/task_orders/index.html | 2 +- templates/task_orders/new/app_info.html | 8 +++---- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 235e5384..5302fa00 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -206,20 +206,22 @@ def get_started(): @task_orders_bp.route("/task_orders/new//") @task_orders_bp.route("/portfolios//task_orders/new/") def new(screen, task_order_id=None, portfolio_id=None): + workflow = ShowTaskOrderWorkflow(g.current_user, screen, task_order_id) + template_args = { + "current": screen, + "task_order_id": task_order_id, + "screens": workflow.display_screens, + "form": workflow.form, + "complete": workflow.is_complete, + } + if task_order_id and screen is 4: task_order = TaskOrders.get(g.current_user, task_order_id) if not TaskOrders.all_sections_complete(task_order): flash("task_order_draft") - workflow = ShowTaskOrderWorkflow(g.current_user, screen, task_order_id) - template_args = { - "current": screen, - "task_order_id": task_order_id, - "portfolio_id": portfolio_id, - "screens": workflow.display_screens, - "form": workflow.form, - "complete": workflow.is_complete, - } + if portfolio_id: + template_args["portfolio"] = Portfolios.get(g.current_user, portfolio_id) url_args = {"screen": screen} if task_order_id: @@ -237,11 +239,6 @@ def new(screen, task_order_id=None, portfolio_id=None): task_order_id=task_order_id, ) url_args["next"] = template_args["next"] - elif http_request.args.get("new_to_on_portfolio"): - portfolio = Portfolios.get(g.current_user, portfolio_id) - template_args["portfolio_name"] = portfolio.name - template_args["defense_component"] = portfolio.defense_component - template_args["new_to_on_portfolio"] = True template_args["action_url"] = url_for("task_orders.update", **url_args) diff --git a/templates/portfolios/task_orders/index.html b/templates/portfolios/task_orders/index.html index 002111be..c26bd969 100644 --- a/templates/portfolios/task_orders/index.html +++ b/templates/portfolios/task_orders/index.html @@ -93,7 +93,7 @@
{% for task_order in pending_task_orders %} diff --git a/templates/task_orders/new/app_info.html b/templates/task_orders/new/app_info.html index 19e1b361..40462bb8 100644 --- a/templates/task_orders/new/app_info.html +++ b/templates/task_orders/new/app_info.html @@ -16,8 +16,8 @@

{{ "task_orders.new.app_info.basic_info_title"| translate }}

-{% if new_to_on_portfolio %} - {{ ReviewField(heading="forms.portfolio.name_label" | translate, field=portfolio_name) }} +{% if portfolio %} + {{ ReviewField(heading="forms.portfolio.name_label" | translate, field=portfolio.name) }} {% else %} {{ TextInput(form.portfolio_name, placeholder="The name of your office or organization", validation="portfolioName") }} {{ TextInput(form.scope, paragraph=True) }} @@ -25,8 +25,8 @@ {% endif %}
- {% if new_to_on_portfolio %} - {{ ReviewField(heading="forms.task_order.defense_component_label" | translate, field=defense_component) }} + {% if portfolio %} + {{ ReviewField(heading="forms.task_order.defense_component_label" | translate, field=portfolio.defense_component) }} {% else %} {{ OptionsInput(form.defense_component) }} {% endif %} From 3c21857c97c17e95cefb550fb5340ef6e80943c2 Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 4 Mar 2019 13:57:16 -0500 Subject: [PATCH 05/18] Show scope regardless of where TO is started from --- templates/task_orders/new/app_info.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/task_orders/new/app_info.html b/templates/task_orders/new/app_info.html index 40462bb8..f70be8ca 100644 --- a/templates/task_orders/new/app_info.html +++ b/templates/task_orders/new/app_info.html @@ -20,9 +20,9 @@ {{ ReviewField(heading="forms.portfolio.name_label" | translate, field=portfolio.name) }} {% else %} {{ TextInput(form.portfolio_name, placeholder="The name of your office or organization", validation="portfolioName") }} - {{ TextInput(form.scope, paragraph=True) }} -

{{ "task_orders.new.app_info.sample_scope" | translate | safe }}

{% endif %} +{{ TextInput(form.scope, paragraph=True) }} +

{{ "task_orders.new.app_info.sample_scope" | translate | safe }}

{% if portfolio %} From 85fcee005ed56f3b377297575ee0fdd11c1cf990 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 5 Mar 2019 11:23:46 -0500 Subject: [PATCH 06/18] Do not require portfolio name and defense component when building a new TO from a portfolio --- atst/forms/task_order.py | 35 ++++++++++++++++++---------------- atst/routes/task_orders/new.py | 8 ++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/atst/forms/task_order.py b/atst/forms/task_order.py index b70e4f99..cc24c14e 100644 --- a/atst/forms/task_order.py +++ b/atst/forms/task_order.py @@ -27,26 +27,11 @@ from .data import ( from atst.utils.localization import translate -class AppInfoForm(BaseForm): - portfolio_name = StringField( - translate("forms.task_order.portfolio_name_label"), - description=translate("forms.task_order.portfolio_name_description"), - validators=[ - Required(), - Length( - min=4, - max=100, - message=translate("forms.portfolio.name_length_validation_message"), - ), - ], - ) +class AppInfoWithExistingPortfolioForm(BaseForm): scope = TextAreaField( translate("forms.task_order.scope_label"), description=translate("forms.task_order.scope_description"), ) - defense_component = SelectField( - translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES - ) app_migration = RadioField( translate("forms.task_order.app_migration.label"), description=translate("forms.task_order.app_migration.description"), @@ -88,6 +73,24 @@ class AppInfoForm(BaseForm): ) +class AppInfoForm(AppInfoWithExistingPortfolioForm): + portfolio_name = StringField( + translate("forms.task_order.portfolio_name_label"), + description=translate("forms.task_order.portfolio_name_description"), + validators=[ + Required(), + Length( + min=4, + max=100, + message=translate("forms.portfolio.name_length_validation_message"), + ), + ], + ) + defense_component = SelectField( + translate("forms.task_order.defense_component_label"), choices=SERVICE_BRANCHES + ) + + class FundingForm(BaseForm): performance_length = SelectField( translate("forms.task_order.performance_length.label"), diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 5302fa00..48f59a13 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -92,6 +92,10 @@ class ShowTaskOrderWorkflow: return self._form + @form.setter + def form(self, value): + self._form = value + @property def template(self): return self._section["template"] @@ -137,6 +141,8 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @property def form(self): + if self.screen == 1 and self.portfolio_id: + return task_order_form.AppInfoWithExistingPortfolioForm() return self._form @property @@ -222,6 +228,8 @@ def new(screen, task_order_id=None, portfolio_id=None): if portfolio_id: template_args["portfolio"] = Portfolios.get(g.current_user, portfolio_id) + if screen == 1: + workflow.form = task_order_form.AppInfoWithExistingPortfolioForm() url_args = {"screen": screen} if task_order_id: From 16e3d5d36c551f4387dc25b7e1cdf21f10c968e9 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 6 Mar 2019 11:34:50 -0500 Subject: [PATCH 07/18] Remove duplicate test --- tests/routes/task_orders/test_new_task_order.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 1a3ba077..2b3117db 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -66,7 +66,6 @@ def test_create_new_task_order(client, user_session, pdf_upload): created_task_order = TaskOrders.get(creator, created_task_order_id) assert created_task_order.portfolio is not None assert created_task_order.portfolio.name == portfolio_name - assert created_task_order.portfolio is not None assert created_task_order.portfolio.defense_component == defense_component funding_data = slice_data_for_section(task_order_data, "funding") From 4a100f913fac330304436ca1272a1576a678208a Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 6 Mar 2019 14:26:40 -0500 Subject: [PATCH 08/18] Add num_task_orders property --- atst/models/portfolio.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 01e9e299..2d9a3aa0 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -36,6 +36,10 @@ class Portfolio(Base, mixins.TimestampsMixin, mixins.AuditableMixin): def user_count(self): return len(self.members) + @property + def num_task_orders(self): + return len(self.task_orders) + @property def members(self): return ( From be192081bca616d43a7024753ff0538d0fbbd914 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 6 Mar 2019 14:27:12 -0500 Subject: [PATCH 09/18] Add two workflow methods --- atst/routes/task_orders/new.py | 20 ++++++++ .../routes/task_orders/test_new_task_order.py | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 48f59a13..4c9d40e7 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -120,6 +120,23 @@ class ShowTaskOrderWorkflow: else: return False + def can_edit_pf_attributes(self, portfolio_id=None): + if self.task_order: + if ( + self.task_order.portfolio.num_task_orders > 1 + or not portfolio_id is None + ): + return False + elif portfolio_id: + if self.get_portfolio(portfolio_id).num_task_orders > 0: + return False + return True + + def get_portfolio(self, portfolio_id=None): + if self.task_order: + return self.task_order.portfolio + return Portfolios.get(self.user, portfolio_id) + class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): def __init__( @@ -221,6 +238,9 @@ def new(screen, task_order_id=None, portfolio_id=None): "complete": workflow.is_complete, } + if not workflow.can_edit_pf_attributes(portfolio_id): + template_args["portfolio"] = workflow.get_portfolio(portfolio_id=portfolio_id) + if task_order_id and screen is 4: task_order = TaskOrders.get(g.current_user, task_order_id) if not TaskOrders.all_sections_complete(task_order): diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 2b3117db..e600f7db 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -42,6 +42,54 @@ def serialize_dates(data): return data +def test_new_to_can_edit_pf_attributes_screen_1(): + portfolio = PortfolioFactory.create() + workflow = ShowTaskOrderWorkflow(user=portfolio.owner) + assert workflow.can_edit_pf_attributes(portfolio.id) + + +def test_new_to_can_edit_pf_attributes_on_return_to_screen_1(): + portfolio = PortfolioFactory.create() + workflow = ShowTaskOrderWorkflow(user=portfolio.owner) + assert workflow.can_edit_pf_attributes() + + +def test_to_on_pf_cannot_edit_pf_attributes(): + portfolio = PortfolioFactory.create() + pf_task_order = TaskOrderFactory(portfolio=portfolio) + + workflow = ShowTaskOrderWorkflow(user=portfolio.owner) + assert portfolio.num_task_orders == 1 + # case: TO is created from am existing portfolio + assert not workflow.can_edit_pf_attributes(portfolio.id) + # case: Portfolio is being created and user navigates back to app_info screen + assert workflow.can_edit_pf_attributes() + + second_task_order = TaskOrderFactory(portfolio=portfolio) + workflow = ShowTaskOrderWorkflow( + user=second_task_order.creator, task_order_id=second_task_order.id + ) + assert portfolio.num_task_orders > 1 + assert not workflow.can_edit_pf_attributes() + +def test_get_portfolio_when_task_order_exists(): + portfolio = PortfolioFactory.create() + task_order = TaskOrderFactory(portfolio=portfolio) + assert portfolio.num_task_orders > 0 + + workflow = ShowTaskOrderWorkflow( + user=task_order.creator, task_order_id=task_order.id + ) + assert portfolio == workflow.get_portfolio() + + +def test_get_portfolio_with_portfolio_id(): + user = UserFactory.create() + portfolio = PortfolioFactory.create(owner=user) + workflow = ShowTaskOrderWorkflow(user=portfolio.owner) + assert portfolio == workflow.get_portfolio(portfolio_id=portfolio.id) + + # TODO: this test will need to be more complicated when we add validation to # the forms def test_create_new_task_order(client, user_session, pdf_upload): From 93545d2dc98fcd15a9cb34d6ef181dc1f6d3989b Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 6 Mar 2019 16:26:57 -0500 Subject: [PATCH 10/18] Use new form --- atst/routes/task_orders/new.py | 18 ++++++++++-------- .../routes/task_orders/test_new_task_order.py | 13 +++++++------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 4c9d40e7..3d115745 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -122,10 +122,7 @@ class ShowTaskOrderWorkflow: def can_edit_pf_attributes(self, portfolio_id=None): if self.task_order: - if ( - self.task_order.portfolio.num_task_orders > 1 - or not portfolio_id is None - ): + if self.task_order.portfolio.num_task_orders > 1: return False elif portfolio_id: if self.get_portfolio(portfolio_id).num_task_orders > 0: @@ -237,6 +234,10 @@ def new(screen, task_order_id=None, portfolio_id=None): "form": workflow.form, "complete": workflow.is_complete, } + if not workflow.can_edit_pf_attributes(portfolio_id): + template_args["portfolio"] = workflow.get_portfolio( + user=g.current_user, portfolio_id=portfolio_id + ) if not workflow.can_edit_pf_attributes(portfolio_id): template_args["portfolio"] = workflow.get_portfolio(portfolio_id=portfolio_id) @@ -246,10 +247,11 @@ def new(screen, task_order_id=None, portfolio_id=None): if not TaskOrders.all_sections_complete(task_order): flash("task_order_draft") - if portfolio_id: - template_args["portfolio"] = Portfolios.get(g.current_user, portfolio_id) - if screen == 1: - workflow.form = task_order_form.AppInfoWithExistingPortfolioForm() + if screen == 1: + if portfolio_id or workflow.task_order.portfolio.num_task_orders > 1: + workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( + obj=workflow.task_order + ) url_args = {"screen": screen} if task_order_id: diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index e600f7db..6b8dbf37 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -48,10 +48,13 @@ def test_new_to_can_edit_pf_attributes_screen_1(): assert workflow.can_edit_pf_attributes(portfolio.id) -def test_new_to_can_edit_pf_attributes_on_return_to_screen_1(): +def test_new_pf_can_edit_pf_attributes_on_back_navigation(): portfolio = PortfolioFactory.create() - workflow = ShowTaskOrderWorkflow(user=portfolio.owner) - assert workflow.can_edit_pf_attributes() + pf_task_order = TaskOrderFactory(portfolio=portfolio) + pf_workflow = ShowTaskOrderWorkflow( + user=pf_task_order.creator, task_order_id=pf_task_order.id + ) + assert pf_workflow.can_edit_pf_attributes() def test_to_on_pf_cannot_edit_pf_attributes(): @@ -60,10 +63,7 @@ def test_to_on_pf_cannot_edit_pf_attributes(): workflow = ShowTaskOrderWorkflow(user=portfolio.owner) assert portfolio.num_task_orders == 1 - # case: TO is created from am existing portfolio assert not workflow.can_edit_pf_attributes(portfolio.id) - # case: Portfolio is being created and user navigates back to app_info screen - assert workflow.can_edit_pf_attributes() second_task_order = TaskOrderFactory(portfolio=portfolio) workflow = ShowTaskOrderWorkflow( @@ -72,6 +72,7 @@ def test_to_on_pf_cannot_edit_pf_attributes(): assert portfolio.num_task_orders > 1 assert not workflow.can_edit_pf_attributes() + def test_get_portfolio_when_task_order_exists(): portfolio = PortfolioFactory.create() task_order = TaskOrderFactory(portfolio=portfolio) From 7fb9a7572ce5c9ef74b4fa5e8af70611e976d3dc Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 7 Mar 2019 15:33:36 -0500 Subject: [PATCH 11/18] Preserve data on back navigation --- atst/routes/task_orders/new.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 3d115745..7086e997 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -155,8 +155,8 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @property def form(self): - if self.screen == 1 and self.portfolio_id: - return task_order_form.AppInfoWithExistingPortfolioForm() + if not self.can_edit_pf_attributes(self.portfolio_id) and self.screen == 1: + return task_order_form.AppInfoWithExistingPortfolioForm(self.form_data) return self._form @property @@ -234,25 +234,19 @@ def new(screen, task_order_id=None, portfolio_id=None): "form": workflow.form, "complete": workflow.is_complete, } - if not workflow.can_edit_pf_attributes(portfolio_id): - template_args["portfolio"] = workflow.get_portfolio( - user=g.current_user, portfolio_id=portfolio_id - ) if not workflow.can_edit_pf_attributes(portfolio_id): template_args["portfolio"] = workflow.get_portfolio(portfolio_id=portfolio_id) + if screen == 1: + workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( + obj=workflow.task_order + ) if task_order_id and screen is 4: task_order = TaskOrders.get(g.current_user, task_order_id) if not TaskOrders.all_sections_complete(task_order): flash("task_order_draft") - if screen == 1: - if portfolio_id or workflow.task_order.portfolio.num_task_orders > 1: - workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( - obj=workflow.task_order - ) - url_args = {"screen": screen} if task_order_id: url_args["task_order_id"] = task_order_id From acf70d1144d21d862d8d7c3c5da78c4599a93c72 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 7 Mar 2019 15:50:10 -0500 Subject: [PATCH 12/18] Rename method --- atst/routes/task_orders/new.py | 12 ++++++------ tests/routes/task_orders/test_new_task_order.py | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index 7086e997..e1f01fe8 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -120,14 +120,14 @@ class ShowTaskOrderWorkflow: else: return False - def can_edit_pf_attributes(self, portfolio_id=None): + def pf_attributes_read_only(self, portfolio_id=None): if self.task_order: if self.task_order.portfolio.num_task_orders > 1: - return False + return True elif portfolio_id: if self.get_portfolio(portfolio_id).num_task_orders > 0: - return False - return True + return True + return False def get_portfolio(self, portfolio_id=None): if self.task_order: @@ -155,7 +155,7 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @property def form(self): - if not self.can_edit_pf_attributes(self.portfolio_id) and self.screen == 1: + if self.pf_attributes_read_only(self.portfolio_id) and self.screen == 1: return task_order_form.AppInfoWithExistingPortfolioForm(self.form_data) return self._form @@ -235,7 +235,7 @@ def new(screen, task_order_id=None, portfolio_id=None): "complete": workflow.is_complete, } - if not workflow.can_edit_pf_attributes(portfolio_id): + if workflow.pf_attributes_read_only(portfolio_id): template_args["portfolio"] = workflow.get_portfolio(portfolio_id=portfolio_id) if screen == 1: workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 6b8dbf37..0cdbf432 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -45,7 +45,7 @@ def serialize_dates(data): def test_new_to_can_edit_pf_attributes_screen_1(): portfolio = PortfolioFactory.create() workflow = ShowTaskOrderWorkflow(user=portfolio.owner) - assert workflow.can_edit_pf_attributes(portfolio.id) + assert not workflow.pf_attributes_read_only(portfolio.id) def test_new_pf_can_edit_pf_attributes_on_back_navigation(): @@ -54,7 +54,7 @@ def test_new_pf_can_edit_pf_attributes_on_back_navigation(): pf_workflow = ShowTaskOrderWorkflow( user=pf_task_order.creator, task_order_id=pf_task_order.id ) - assert pf_workflow.can_edit_pf_attributes() + assert not pf_workflow.pf_attributes_read_only() def test_to_on_pf_cannot_edit_pf_attributes(): @@ -63,14 +63,14 @@ def test_to_on_pf_cannot_edit_pf_attributes(): workflow = ShowTaskOrderWorkflow(user=portfolio.owner) assert portfolio.num_task_orders == 1 - assert not workflow.can_edit_pf_attributes(portfolio.id) + assert workflow.pf_attributes_read_only(portfolio.id) second_task_order = TaskOrderFactory(portfolio=portfolio) - workflow = ShowTaskOrderWorkflow( - user=second_task_order.creator, task_order_id=second_task_order.id + second_workflow = ShowTaskOrderWorkflow( + user=portfolio.owner, task_order_id=second_task_order.id ) assert portfolio.num_task_orders > 1 - assert not workflow.can_edit_pf_attributes() + assert second_workflow.pf_attributes_read_only() def test_get_portfolio_when_task_order_exists(): From 970eb8aba30fea56a411e187267e220bd2d42a1b Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 8 Mar 2019 09:31:30 -0500 Subject: [PATCH 13/18] Make portfolio_id an init kwarg on ShowTaskOrderWorkflow class --- atst/routes/task_orders/new.py | 23 +++++++++++-------- .../routes/task_orders/test_new_task_order.py | 12 +++++----- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index e1f01fe8..e3b24ba4 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -46,10 +46,11 @@ TASK_ORDER_SECTIONS = [ class ShowTaskOrderWorkflow: - def __init__(self, user, screen=1, task_order_id=None): + def __init__(self, user, screen=1, task_order_id=None, portfolio_id=None): self.user = user self.screen = screen self.task_order_id = task_order_id + self.portfolio_id = portfolio_id self._section = TASK_ORDER_SECTIONS[screen - 1] self._task_order = None self._form = None @@ -120,19 +121,19 @@ class ShowTaskOrderWorkflow: else: return False - def pf_attributes_read_only(self, portfolio_id=None): + def pf_attributes_read_only(self): if self.task_order: if self.task_order.portfolio.num_task_orders > 1: return True - elif portfolio_id: - if self.get_portfolio(portfolio_id).num_task_orders > 0: + elif self.portfolio_id: + if self.get_portfolio().num_task_orders > 0: return True return False - def get_portfolio(self, portfolio_id=None): + def get_portfolio(self): if self.task_order: return self.task_order.portfolio - return Portfolios.get(self.user, portfolio_id) + return Portfolios.get(self.user, self.portfolio_id) class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @@ -155,7 +156,7 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @property def form(self): - if self.pf_attributes_read_only(self.portfolio_id) and self.screen == 1: + if self.pf_attributes_read_only() and self.screen == 1: return task_order_form.AppInfoWithExistingPortfolioForm(self.form_data) return self._form @@ -226,7 +227,9 @@ def get_started(): @task_orders_bp.route("/task_orders/new//") @task_orders_bp.route("/portfolios//task_orders/new/") def new(screen, task_order_id=None, portfolio_id=None): - workflow = ShowTaskOrderWorkflow(g.current_user, screen, task_order_id) + workflow = ShowTaskOrderWorkflow( + g.current_user, screen, task_order_id, portfolio_id + ) template_args = { "current": screen, "task_order_id": task_order_id, @@ -235,8 +238,8 @@ def new(screen, task_order_id=None, portfolio_id=None): "complete": workflow.is_complete, } - if workflow.pf_attributes_read_only(portfolio_id): - template_args["portfolio"] = workflow.get_portfolio(portfolio_id=portfolio_id) + if workflow.pf_attributes_read_only(): + template_args["portfolio"] = workflow.get_portfolio() if screen == 1: workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( obj=workflow.task_order diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 0cdbf432..4793be65 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -44,8 +44,8 @@ def serialize_dates(data): def test_new_to_can_edit_pf_attributes_screen_1(): portfolio = PortfolioFactory.create() - workflow = ShowTaskOrderWorkflow(user=portfolio.owner) - assert not workflow.pf_attributes_read_only(portfolio.id) + workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) + assert not workflow.pf_attributes_read_only() def test_new_pf_can_edit_pf_attributes_on_back_navigation(): @@ -61,9 +61,9 @@ def test_to_on_pf_cannot_edit_pf_attributes(): portfolio = PortfolioFactory.create() pf_task_order = TaskOrderFactory(portfolio=portfolio) - workflow = ShowTaskOrderWorkflow(user=portfolio.owner) + workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) assert portfolio.num_task_orders == 1 - assert workflow.pf_attributes_read_only(portfolio.id) + assert workflow.pf_attributes_read_only() second_task_order = TaskOrderFactory(portfolio=portfolio) second_workflow = ShowTaskOrderWorkflow( @@ -87,8 +87,8 @@ def test_get_portfolio_when_task_order_exists(): def test_get_portfolio_with_portfolio_id(): user = UserFactory.create() portfolio = PortfolioFactory.create(owner=user) - workflow = ShowTaskOrderWorkflow(user=portfolio.owner) - assert portfolio == workflow.get_portfolio(portfolio_id=portfolio.id) + workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) + assert portfolio == workflow.get_portfolio() # TODO: this test will need to be more complicated when we add validation to From 1dbdb35e32aaa1a959e1cdac2629af09d09f6757 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 8 Mar 2019 10:00:33 -0500 Subject: [PATCH 14/18] Fix redundant task order lookup --- atst/routes/task_orders/new.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index e3b24ba4..d885365d 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -246,8 +246,7 @@ def new(screen, task_order_id=None, portfolio_id=None): ) if task_order_id and screen is 4: - task_order = TaskOrders.get(g.current_user, task_order_id) - if not TaskOrders.all_sections_complete(task_order): + if not TaskOrders.all_sections_complete(workflow.task_order): flash("task_order_draft") url_args = {"screen": screen} From aa206192be083cb4de68800ca3d9c20c542b2273 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 8 Mar 2019 10:20:44 -0500 Subject: [PATCH 15/18] Delete unused branch of logic in pr_attributes_read_oly() --- atst/routes/task_orders/new.py | 3 +-- tests/routes/task_orders/test_new_task_order.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index d885365d..c153364c 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -126,8 +126,7 @@ class ShowTaskOrderWorkflow: if self.task_order.portfolio.num_task_orders > 1: return True elif self.portfolio_id: - if self.get_portfolio().num_task_orders > 0: - return True + return True return False def get_portfolio(self): diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 4793be65..5ed8bfc1 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -44,7 +44,7 @@ def serialize_dates(data): def test_new_to_can_edit_pf_attributes_screen_1(): portfolio = PortfolioFactory.create() - workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) + workflow = ShowTaskOrderWorkflow(user=portfolio.owner) assert not workflow.pf_attributes_read_only() From 24886fc482ccec909a4e34f8b0658f190f0d69ba Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 8 Mar 2019 10:37:59 -0500 Subject: [PATCH 16/18] Add portfolio to ShowTaskOrderWorkflow class --- atst/routes/task_orders/new.py | 13 ++++--- .../routes/task_orders/test_new_task_order.py | 38 ++++++++++--------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index c153364c..ee874ab4 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -62,6 +62,12 @@ class ShowTaskOrderWorkflow: return self._task_order + @property + def portfolio(self): + if self.task_order: + return self.task_order.portfolio + return Portfolios.get(self.user, self.portfolio_id) + @property def form(self): form_type = ( @@ -129,11 +135,6 @@ class ShowTaskOrderWorkflow: return True return False - def get_portfolio(self): - if self.task_order: - return self.task_order.portfolio - return Portfolios.get(self.user, self.portfolio_id) - class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): def __init__( @@ -238,7 +239,7 @@ def new(screen, task_order_id=None, portfolio_id=None): } if workflow.pf_attributes_read_only(): - template_args["portfolio"] = workflow.get_portfolio() + template_args["portfolio"] = workflow.portfolio if screen == 1: workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( obj=workflow.task_order diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index 5ed8bfc1..c99e3be4 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -9,6 +9,26 @@ from atst.utils.localization import translate from tests.factories import UserFactory, TaskOrderFactory, PortfolioFactory +class TestShowTaskOrderWorkflow: + def test_portfolio_when_task_order_exists(self): + portfolio = PortfolioFactory.create() + task_order = TaskOrderFactory(portfolio=portfolio) + assert portfolio.num_task_orders > 0 + + workflow = ShowTaskOrderWorkflow( + user=task_order.creator, task_order_id=task_order.id + ) + assert portfolio == workflow.portfolio + + def test_portfolio_with_portfolio_id(self): + user = UserFactory.create() + portfolio = PortfolioFactory.create(owner=user) + workflow = ShowTaskOrderWorkflow( + user=portfolio.owner, portfolio_id=portfolio.id + ) + assert portfolio == workflow.portfolio + + def test_new_task_order(client, user_session): creator = UserFactory.create() user_session() @@ -73,24 +93,6 @@ def test_to_on_pf_cannot_edit_pf_attributes(): assert second_workflow.pf_attributes_read_only() -def test_get_portfolio_when_task_order_exists(): - portfolio = PortfolioFactory.create() - task_order = TaskOrderFactory(portfolio=portfolio) - assert portfolio.num_task_orders > 0 - - workflow = ShowTaskOrderWorkflow( - user=task_order.creator, task_order_id=task_order.id - ) - assert portfolio == workflow.get_portfolio() - - -def test_get_portfolio_with_portfolio_id(): - user = UserFactory.create() - portfolio = PortfolioFactory.create(owner=user) - workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) - assert portfolio == workflow.get_portfolio() - - # TODO: this test will need to be more complicated when we add validation to # the forms def test_create_new_task_order(client, user_session, pdf_upload): From 1b329b96599c87687e70009da7e0954ca7a47be0 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 8 Mar 2019 10:54:00 -0500 Subject: [PATCH 17/18] Move form logic into ShowTaskOrderPortfolio class --- atst/routes/task_orders/new.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index ee874ab4..b0dfb586 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -97,6 +97,11 @@ class ShowTaskOrderWorkflow: else: self._form = self._section[form_type]() + if self.pf_attributes_read_only() and self.screen == 1: + self._form = task_order_form.AppInfoWithExistingPortfolioForm( + obj=self.task_order + ) + return self._form @form.setter @@ -238,17 +243,13 @@ def new(screen, task_order_id=None, portfolio_id=None): "complete": workflow.is_complete, } - if workflow.pf_attributes_read_only(): - template_args["portfolio"] = workflow.portfolio - if screen == 1: - workflow.form = task_order_form.AppInfoWithExistingPortfolioForm( - obj=workflow.task_order - ) - if task_order_id and screen is 4: if not TaskOrders.all_sections_complete(workflow.task_order): flash("task_order_draft") + if workflow.pf_attributes_read_only(): + template_args["portfolio"] = workflow.portfolio + url_args = {"screen": screen} if task_order_id: url_args["task_order_id"] = task_order_id From f57af648f81128bf9ee699a5f4731b01c26f8fab Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 8 Mar 2019 15:13:16 -0500 Subject: [PATCH 18/18] small refactors to new TO workflow classes --- atst/routes/task_orders/new.py | 63 +++++++++++-------- .../routes/task_orders/test_new_task_order.py | 8 +-- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/atst/routes/task_orders/new.py b/atst/routes/task_orders/new.py index b0dfb586..799fd024 100644 --- a/atst/routes/task_orders/new.py +++ b/atst/routes/task_orders/new.py @@ -50,9 +50,10 @@ class ShowTaskOrderWorkflow: self.user = user self.screen = screen self.task_order_id = task_order_id - self.portfolio_id = portfolio_id - self._section = TASK_ORDER_SECTIONS[screen - 1] self._task_order = None + self.portfolio_id = portfolio_id + self._portfolio = None + self._section = TASK_ORDER_SECTIONS[screen - 1] self._form = None @property @@ -64,9 +65,13 @@ class ShowTaskOrderWorkflow: @property def portfolio(self): - if self.task_order: - return self.task_order.portfolio - return Portfolios.get(self.user, self.portfolio_id) + if not self._portfolio: + if self.task_order: + self._portfolio = self.task_order.portfolio + elif self.portfolio_id: + self._portfolio = Portfolios.get(self.user, self.portfolio_id) + + return self._portfolio @property def form(self): @@ -97,17 +102,13 @@ class ShowTaskOrderWorkflow: else: self._form = self._section[form_type]() - if self.pf_attributes_read_only() and self.screen == 1: + if self.pf_attributes_read_only and self.screen == 1: self._form = task_order_form.AppInfoWithExistingPortfolioForm( obj=self.task_order ) return self._form - @form.setter - def form(self, value): - self._form = value - @property def template(self): return self._section["template"] @@ -126,19 +127,19 @@ class ShowTaskOrderWorkflow: @property def is_complete(self): - if self.task_order: - if TaskOrders.all_sections_complete(self.task_order): - return True + if self.task_order and TaskOrders.all_sections_complete(self.task_order): + return True else: return False + @property def pf_attributes_read_only(self): - if self.task_order: - if self.task_order.portfolio.num_task_orders > 1: - return True + if self.task_order and self.portfolio.num_task_orders > 1: + return True elif self.portfolio_id: return True - return False + else: + return False class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): @@ -152,17 +153,27 @@ class UpdateTaskOrderWorkflow(ShowTaskOrderWorkflow): self.portfolio_id = portfolio_id self._task_order = None self._section = TASK_ORDER_SECTIONS[screen - 1] - form_type = ( - "unclassified_form" - if "unclassified_form" in self._section and not app.config.get("CLASSIFIED") - else "form" - ) - self._form = self._section[form_type](self.form_data, obj=self.task_order) + self._form = None @property def form(self): - if self.pf_attributes_read_only() and self.screen == 1: - return task_order_form.AppInfoWithExistingPortfolioForm(self.form_data) + if not self._form: + form_type = ( + "unclassified_form" + if "unclassified_form" in self._section + and not app.config.get("CLASSIFIED") + else "form" + ) + + if self.pf_attributes_read_only and self.screen == 1: + self._form = task_order_form.AppInfoWithExistingPortfolioForm( + self.form_data + ) + else: + self._form = self._section[form_type]( + self.form_data, obj=self.task_order + ) + return self._form @property @@ -247,7 +258,7 @@ def new(screen, task_order_id=None, portfolio_id=None): if not TaskOrders.all_sections_complete(workflow.task_order): flash("task_order_draft") - if workflow.pf_attributes_read_only(): + if workflow.pf_attributes_read_only: template_args["portfolio"] = workflow.portfolio url_args = {"screen": screen} diff --git a/tests/routes/task_orders/test_new_task_order.py b/tests/routes/task_orders/test_new_task_order.py index c99e3be4..fe4ec09e 100644 --- a/tests/routes/task_orders/test_new_task_order.py +++ b/tests/routes/task_orders/test_new_task_order.py @@ -65,7 +65,7 @@ def serialize_dates(data): def test_new_to_can_edit_pf_attributes_screen_1(): portfolio = PortfolioFactory.create() workflow = ShowTaskOrderWorkflow(user=portfolio.owner) - assert not workflow.pf_attributes_read_only() + assert not workflow.pf_attributes_read_only def test_new_pf_can_edit_pf_attributes_on_back_navigation(): @@ -74,7 +74,7 @@ def test_new_pf_can_edit_pf_attributes_on_back_navigation(): pf_workflow = ShowTaskOrderWorkflow( user=pf_task_order.creator, task_order_id=pf_task_order.id ) - assert not pf_workflow.pf_attributes_read_only() + assert not pf_workflow.pf_attributes_read_only def test_to_on_pf_cannot_edit_pf_attributes(): @@ -83,14 +83,14 @@ def test_to_on_pf_cannot_edit_pf_attributes(): workflow = ShowTaskOrderWorkflow(user=portfolio.owner, portfolio_id=portfolio.id) assert portfolio.num_task_orders == 1 - assert workflow.pf_attributes_read_only() + assert workflow.pf_attributes_read_only second_task_order = TaskOrderFactory(portfolio=portfolio) second_workflow = ShowTaskOrderWorkflow( user=portfolio.owner, task_order_id=second_task_order.id ) assert portfolio.num_task_orders > 1 - assert second_workflow.pf_attributes_read_only() + assert second_workflow.pf_attributes_read_only # TODO: this test will need to be more complicated when we add validation to