From 9dd47279e4323ed3900b3ba9763d5262a1f415eb Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Thu, 28 Nov 2024 17:04:27 +0000 Subject: [PATCH 01/13] Basic synchronous auto end measures functionality --- geo_areas/views.py | 2 +- .../workbaskets/auto_end_date_measures.jinja | 81 +++++++++++ workbaskets/jinja2/workbaskets/checks.jinja | 2 + .../confirm_auto_end_date_measures.jinja | 32 +++++ workbaskets/urls.py | 10 ++ workbaskets/views/ui.py | 130 ++++++++++++++++++ 6 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja create mode 100644 workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja diff --git a/geo_areas/views.py b/geo_areas/views.py index f29fb6baa..cd17b212e 100644 --- a/geo_areas/views.py +++ b/geo_areas/views.py @@ -141,7 +141,7 @@ def get_context_data(self, *args, **kwargs): class GeoAreaDetailMeasures(SortingMixin, WithPaginationListMixin, ListView): """Displays a paginated list of measures for a geo area as a simulated tab - on regulation detail view.""" + on geo area detail view.""" model = Measure template_name = "includes/geo_areas/tabs/measures.jinja" diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja new file mode 100644 index 000000000..75e928b8a --- /dev/null +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -0,0 +1,81 @@ +{% extends "layouts/layout.jinja" %} +{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %} +{% from "components/table/macro.njk" import govukTable %} +{% from "components/button/macro.njk" import govukButton %} + + +{% set page_title %} + Workbasket {{ workbasket.id if workbasket else request.user.current_workbasket.id }} - Auto end-date measures +{% endset %} + +{% block content %} +

{{ page_title }}

+ + {% set table_rows = [] %} + {% for measure in object_list %} + {% set measure_link %} + {{ measure.sid }} + {% endset %} + + {% set commodity_link %} + {{ measure.goods_nomenclature.item_id }} + {% endset %} + + {% set action %} + {% if measure.valid_between.lower > today %}To be deleted{% else %}To be end-dated{% endif %} + {% endset %} + + {{ table_rows.append([ + {"text": measure_link}, + {"html": commodity_link if measure.goods_nomenclature else "-"}, + {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.valid_between.upper) if measure.goods_nomenclature.valid_between.upper else "-" }, + {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, + {"text": action}, + ]) or "" }} + {% endfor %} + + {% set base_url = url('workbaskets:workbasket-ui-auto-end-date-measures', kwargs={"wb_pk": workbasket.id}) %} + + {% set commodity_code %} + {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }} + {% endset %} + + {% set start_date %} + {{ create_sortable_anchor(request, "start_date", "Measure start date", base_url) }} + {% endset %} + + {% if object_list %} +

The following measures' commodity codes have been end-dated in your workbasket. Click submit to end-date or delete these measures.

+ {{ govukTable({ + "head": [ + {"text": "Measure SID"}, + {"text": commodity_code}, + {"text": "Commodity end date"}, + {"text": start_date}, + {"text": "Action"}, + ], + "rows": table_rows + }) }} + +
+ + + {{ govukButton({ + 'text': "Submit", + 'preventDoubleClick': true, + "name": "action", + "value": "auto-end-date-measures", +}) }} +
+ + + + {% else %} +

There are no current measures related to commodity codes updated in this workbasket.

+ {% endif %} + {% include "includes/common/pagination.jinja" %} + + +{% endblock %} \ No newline at end of file diff --git a/workbaskets/jinja2/workbaskets/checks.jinja b/workbaskets/jinja2/workbaskets/checks.jinja index 8102703ca..01fdd3dda 100644 --- a/workbaskets/jinja2/workbaskets/checks.jinja +++ b/workbaskets/jinja2/workbaskets/checks.jinja @@ -32,6 +32,8 @@ + Auto end date measures + {# Intentionally not styling this as I know this page is getting a revamp #} {% endblock %} \ No newline at end of file diff --git a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja new file mode 100644 index 000000000..a2d391b54 --- /dev/null +++ b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja @@ -0,0 +1,32 @@ +{% extends "layouts/layout.jinja" %} + +{% from "components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "components/panel/macro.njk" import govukPanel %} +{% from "components/button/macro.njk" import govukButton %} + +{% set page_title = "Measures ended" %} + +{% block breadcrumb %} + {{ govukBreadcrumbs({ + "items": [ + {"text": "Home", "href": url("home")}, + {"text": "Workbasket " ~ request.user.current_workbasket.id }, + {"text": "Workbasket " ~ request.user.current_workbasket.id ~ " checks", "href": url("workbaskets:workbasket-checks")}, + {"text": "Workbasket " ~ request.user.current_workbasket.id ~ " auto end date measures"}, + {"text": page_title} + ] + }) }} +{% endblock %} + +{% block content %} +
+
+ {{ govukPanel({ + "titleText": page_title, + "text": "You have end dated or deleted measures linked to commodities end dated in workbasket " ~ request.user.current_workbasket.id, + "classes": "govuk-!-margin-bottom-7" + }) }} + Continue to workbasket +
+
+{% endblock %} diff --git a/workbaskets/urls.py b/workbaskets/urls.py index a4141d40c..e03a28dc4 100644 --- a/workbaskets/urls.py +++ b/workbaskets/urls.py @@ -211,6 +211,16 @@ ui_views.WorkBasketCommentDelete.as_view(), name="workbasket-ui-comment-delete", ), + path( + f"/auto-end-date-measures/", + ui_views.AutoEndDateMeasures.as_view(), + name="workbasket-ui-auto-end-date-measures", + ), + path( + f"/confirm-auto-end-date-measures/", + ui_views.AutoEndDateMeasuresConfirm.as_view(), + name="workbasket-ui-auto-end-date-measures-confirm", + ), ] urlpatterns = [ diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index f62fb86d1..70e61ab99 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -39,11 +39,14 @@ from additional_codes.models import AdditionalCode from certificates.models import Certificate from checks.models import TrackedModelCheck +from commodities.models.orm import GoodsNomenclature from common.filters import TamatoFilter from common.inspect_tap_tasks import TAPTasks from common.models import Transaction from common.models.transactions import TransactionPartition +from common.util import TaricDateRange from common.util import format_date_string +from common.validators import UpdateType from common.views import SortingMixin from common.views import WithPaginationListMixin from common.views import WithPaginationListView @@ -1779,3 +1782,130 @@ def status_tag_generator(self, task_status) -> dict: "text": task_status, "tag_class": "", } + + +class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): + model = Measure + paginate_by = 20 + template_name = "workbaskets/auto_end_date_measures.jinja" + sort_by_fields = ["start_date", "goods_nomenclature"] + custom_sorting = { + "start_date": "valid_between", + "goods_nomenclature": "goods_nomenclature__item_id", + } + + @cached_property + def workbasket(self): + return WorkBasket.objects.get(pk=self.kwargs["wb_pk"]) + + @property + def commodities(self): + """Return commodities in the current workbasket which have an end + date.""" + return ( + GoodsNomenclature.objects.current() + .filter( + transaction__workbasket=self.workbasket, + valid_between__upper_inf=False, + ) + .values_list("pk") + ) + + @cached_property + def measures(self): + # Should I only be filtering for measures without an end date? + return Measure.objects.current().filter( + goods_nomenclature__id__in=self.commodities, + ) + + def workbasket_transactions(self): + """Returns the current workbasket's transactions ordered by `order`, + while guarding against non-editing status on workbasket to minimise + chances of mishap.""" + return Transaction.objects.filter( + workbasket=self.workbasket, + workbasket__status=WorkflowStatus.EDITING, + ).order_by("order") + + def get_queryset(self): + ordering = self.get_ordering() + queryset = self.measures + if ordering: + if isinstance(ordering, str): + ordering = (ordering,) + queryset = queryset.order_by(*ordering) + return queryset + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["workbasket"] = self.workbasket + context["today"] = date.today() + return context + + def post(self, request, *args, **kwargs): + if request.POST.get("action", None) == "auto-end-date-measures": + self.end_measures() + + return redirect( + "workbaskets:workbasket-ui-auto-end-date-measures-confirm", + self.workbasket.pk, + ) + + @atomic + def end_measures(self): + """Iterate through measures on commodities, end-date those which have + already began and delete those which have not yet started.""" + # TODO: How do you check that the commodity code has actually been updated and it isn't a create or the same date as before + for ( + measure + ) in self.measures: # Does this need to be here? is this a likely occurence? + commodity = GoodsNomenclature.objects.all().get( + pk=measure.goods_nomenclature_id, + transaction__workbasket=self.workbasket, + ) + if measure.valid_between.lower > commodity.valid_between.upper: + continue + if measure.valid_between.lower > date.today(): + new_measure_version = measure.new_version( + workbasket=self.workbasket, + update_type=UpdateType.DELETE, + ) + else: + new_measure_version = measure.new_version( + workbasket=self.workbasket, + update_type=UpdateType.UPDATE, + valid_between=TaricDateRange( + measure.valid_between.lower, + commodity.valid_between.upper, + ), + ) + self.promote_measure_to_top(new_measure_version.transaction) + + def promote_measure_to_top(self, promoted_measure): + """Set the transaction order of `promoted_measure` to be first in the + workbasket, demoting the transactions that came before it.""" + # Is it better to bulk move all measure transactions at the end? + + top_transaction = self.workbasket_transactions().first() + + if ( + not promoted_measure + or not top_transaction + or promoted_measure == top_transaction + ): + return + + current_position = promoted_measure.order + top_position = top_transaction.order + self.workbasket_transactions().filter(order__lt=current_position).update( + order=F("order") + 1, + ) + + promoted_measure.order = top_position + promoted_measure.save(update_fields=["order"]) + + +class AutoEndDateMeasuresConfirm(DetailView): + template_name = "workbaskets/confirm_auto_end_date_measures.jinja" + model = WorkBasket + queryset = WorkBasket.objects.all() From 83d7101cf7aa90380f46105b1b054e180c9f60d0 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Fri, 29 Nov 2024 22:03:13 +0000 Subject: [PATCH 02/13] Make it a Celery task --- settings/common.py | 3 ++ workbaskets/tasks.py | 70 +++++++++++++++++++++++++++++++++++++++++ workbaskets/views/ui.py | 60 +++-------------------------------- 3 files changed, 78 insertions(+), 55 deletions(-) diff --git a/settings/common.py b/settings/common.py index 51ef14097..01fbd39a8 100644 --- a/settings/common.py +++ b/settings/common.py @@ -647,6 +647,9 @@ "workbaskets.tasks.check_workbasket": { "queue": "rule-check", }, + "workbaskets.tasks.call_end_measures": { + "queue": "standard", + }, "workbaskets.tasks.transition": { "queue": "standard", }, diff --git a/workbaskets/tasks.py b/workbaskets/tasks.py index 9482aee00..8a0dad962 100644 --- a/workbaskets/tasks.py +++ b/workbaskets/tasks.py @@ -1,12 +1,21 @@ +from datetime import date + from celery import group from celery import shared_task from celery.utils.log import get_task_logger +from django.db.models import F from django.db.transaction import atomic from checks.tasks import check_transaction from checks.tasks import check_transaction_sync +from commodities.models.orm import GoodsNomenclature from common.celery import app +from common.models.transactions import Transaction +from common.util import TaricDateRange +from common.validators import UpdateType +from measures.models.tracked_models import Measure from workbaskets.models import WorkBasket +from workbaskets.validators import WorkflowStatus # Celery logger adds the task id and status and outputs via the worker. logger = get_task_logger(__name__) @@ -64,3 +73,64 @@ def call_check_workbasket_sync(self, workbasket_id: int): workbasket: WorkBasket = WorkBasket.objects.get(pk=workbasket_id) workbasket.delete_checks() check_workbasket_sync(workbasket) + + +def promote_measure_to_top(promoted_measure, workbasket_transactions): + """Set the transaction order of `promoted_measure` to be first in the + workbasket, demoting the transactions that came before it.""" + + top_transaction = workbasket_transactions.first() + + if ( + not promoted_measure + or not top_transaction + or promoted_measure == top_transaction + ): + return + + current_position = promoted_measure.order + top_position = top_transaction.order + workbasket_transactions.filter(order__lt=current_position).update( + order=F("order") + 1, + ) + promoted_measure.order = top_position + promoted_measure.save(update_fields=["order"]) + + +def end_measures(measures, workbasket): + """Iterate through measures on commodities, end-date those which have + already began and delete those which have not yet started.""" + for measure in measures: + workbasket_transactions = Transaction.objects.filter( + workbasket=workbasket, + workbasket__status=WorkflowStatus.EDITING, + ).order_by("order") + + commodity = GoodsNomenclature.objects.all().get( + pk=measure.goods_nomenclature_id, + transaction__workbasket=workbasket, + ) + if measure.valid_between.lower > commodity.valid_between.upper: + continue + if measure.valid_between.lower > date.today(): + new_measure_version = measure.new_version( + workbasket=workbasket, + update_type=UpdateType.DELETE, + ) + else: + new_measure_version = measure.new_version( + workbasket=workbasket, + update_type=UpdateType.UPDATE, + valid_between=TaricDateRange( + measure.valid_between.lower, + commodity.valid_between.upper, + ), + ) + promote_measure_to_top(new_measure_version.transaction, workbasket_transactions) + + +@app.task +def call_end_measures(measure_pks, workbasket_pk): + workbasket = WorkBasket.objects.all().get(pk=workbasket_pk) + measures = Measure.objects.all().filter(pk__in=measure_pks) + end_measures(measures, workbasket) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 70e61ab99..40808cc0b 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -44,9 +44,7 @@ from common.inspect_tap_tasks import TAPTasks from common.models import Transaction from common.models.transactions import TransactionPartition -from common.util import TaricDateRange from common.util import format_date_string -from common.validators import UpdateType from common.views import SortingMixin from common.views import WithPaginationListMixin from common.views import WithPaginationListView @@ -75,6 +73,7 @@ from workbaskets.models import WorkBasket from workbaskets.session_store import SessionStore from workbaskets.tasks import call_check_workbasket_sync +from workbaskets.tasks import call_end_measures from workbaskets.validators import WorkflowStatus from workbaskets.views.decorators import require_current_workbasket from workbaskets.views.mixins import WithCurrentWorkBasket @@ -1794,7 +1793,7 @@ class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): "goods_nomenclature": "goods_nomenclature__item_id", } - @cached_property + @property def workbasket(self): return WorkBasket.objects.get(pk=self.kwargs["wb_pk"]) @@ -1811,7 +1810,7 @@ def commodities(self): .values_list("pk") ) - @cached_property + @property def measures(self): # Should I only be filtering for measures without an end date? return Measure.objects.current().filter( @@ -1845,7 +1844,6 @@ def get_context_data(self, **kwargs): def post(self, request, *args, **kwargs): if request.POST.get("action", None) == "auto-end-date-measures": self.end_measures() - return redirect( "workbaskets:workbasket-ui-auto-end-date-measures-confirm", self.workbasket.pk, @@ -1853,56 +1851,8 @@ def post(self, request, *args, **kwargs): @atomic def end_measures(self): - """Iterate through measures on commodities, end-date those which have - already began and delete those which have not yet started.""" - # TODO: How do you check that the commodity code has actually been updated and it isn't a create or the same date as before - for ( - measure - ) in self.measures: # Does this need to be here? is this a likely occurence? - commodity = GoodsNomenclature.objects.all().get( - pk=measure.goods_nomenclature_id, - transaction__workbasket=self.workbasket, - ) - if measure.valid_between.lower > commodity.valid_between.upper: - continue - if measure.valid_between.lower > date.today(): - new_measure_version = measure.new_version( - workbasket=self.workbasket, - update_type=UpdateType.DELETE, - ) - else: - new_measure_version = measure.new_version( - workbasket=self.workbasket, - update_type=UpdateType.UPDATE, - valid_between=TaricDateRange( - measure.valid_between.lower, - commodity.valid_between.upper, - ), - ) - self.promote_measure_to_top(new_measure_version.transaction) - - def promote_measure_to_top(self, promoted_measure): - """Set the transaction order of `promoted_measure` to be first in the - workbasket, demoting the transactions that came before it.""" - # Is it better to bulk move all measure transactions at the end? - - top_transaction = self.workbasket_transactions().first() - - if ( - not promoted_measure - or not top_transaction - or promoted_measure == top_transaction - ): - return - - current_position = promoted_measure.order - top_position = top_transaction.order - self.workbasket_transactions().filter(order__lt=current_position).update( - order=F("order") + 1, - ) - - promoted_measure.order = top_position - promoted_measure.save(update_fields=["order"]) + measure_pks = [measure.pk for measure in self.measures] + call_end_measures.apply_async((measure_pks, self.workbasket.pk)) class AutoEndDateMeasuresConfirm(DetailView): From 6321fa03d1edfb9050c4f66fdd77ebaf89159671 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Mon, 2 Dec 2024 16:37:11 +0000 Subject: [PATCH 03/13] Add tests --- .../workbaskets/auto_end_date_measures.jinja | 4 +- .../confirm_auto_end_date_measures.jinja | 7 +- workbaskets/tasks.py | 11 +- workbaskets/tests/test_views.py | 101 +++++++++++++++++- workbaskets/views/ui.py | 4 +- 5 files changed, 114 insertions(+), 13 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index 75e928b8a..51d9bb23d 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -28,7 +28,7 @@ {{ table_rows.append([ {"text": measure_link}, {"html": commodity_link if measure.goods_nomenclature else "-"}, - {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.valid_between.upper) if measure.goods_nomenclature.valid_between.upper else "-" }, + {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper) if measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper else "-" }, {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, {"text": action}, ]) or "" }} @@ -45,7 +45,7 @@ {% endset %} {% if object_list %} -

The following measures' commodity codes have been end-dated in your workbasket. Click submit to end-date or delete these measures.

+

The following measures are linked to commodity codes that have been end-dated in your workbasket. Click submit to automatically end-date or delete these measures.

{{ govukTable({ "head": [ {"text": "Measure SID"}, diff --git a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja index a2d391b54..2b003c344 100644 --- a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja @@ -4,7 +4,7 @@ {% from "components/panel/macro.njk" import govukPanel %} {% from "components/button/macro.njk" import govukButton %} -{% set page_title = "Measures ended" %} +{% set page_title = "Measures submitted to be ended" %} {% block breadcrumb %} {{ govukBreadcrumbs({ @@ -22,10 +22,11 @@
{{ govukPanel({ - "titleText": page_title, - "text": "You have end dated or deleted measures linked to commodities end dated in workbasket " ~ request.user.current_workbasket.id, + "titleText": "X measures will be ended", + "text": "You have successfully submitted X measures to be end-dated or deleted and they will be added to workbasket " ~ request.user.current_workbasket.id ~ " when complete.", "classes": "govuk-!-margin-bottom-7" }) }} +

Next steps

Continue to workbasket
diff --git a/workbaskets/tasks.py b/workbaskets/tasks.py index 8a0dad962..d67c5f01c 100644 --- a/workbaskets/tasks.py +++ b/workbaskets/tasks.py @@ -105,10 +105,13 @@ def end_measures(measures, workbasket): workbasket=workbasket, workbasket__status=WorkflowStatus.EDITING, ).order_by("order") - - commodity = GoodsNomenclature.objects.all().get( - pk=measure.goods_nomenclature_id, - transaction__workbasket=workbasket, + commodity = ( + GoodsNomenclature.objects.all() + .filter( + sid=measure.goods_nomenclature.sid, + transaction__workbasket=workbasket, + ) + .last() ) if measure.valid_between.lower > commodity.valid_between.upper: continue diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index 73e7d6635..d94cfa41f 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -17,6 +17,7 @@ from checks.tests.factories import TrackedModelCheckFactory from common.inspect_tap_tasks import CeleryTask from common.inspect_tap_tasks import TAPTasks +from common.models.trackedmodel import TrackedModel from common.models.utils import override_current_transaction from common.tests import factories from common.tests.util import date_post_data @@ -28,6 +29,7 @@ from tasks.models import Comment from tasks.models import UserAssignment from workbaskets import models +from workbaskets.tasks import call_end_measures from workbaskets.tasks import check_workbasket_sync from workbaskets.validators import WorkflowStatus from workbaskets.views import ui @@ -2608,7 +2610,9 @@ def test_current_tasks_is_called(valid_user_client): def test_remove_all_workbasket_changes_button_only_shown_to_superusers( - client, user_workbasket, superuser + client, + user_workbasket, + superuser, ): url = reverse( "workbaskets:workbasket-ui-changes", @@ -2626,7 +2630,8 @@ def test_remove_all_workbasket_changes_button_only_shown_to_superusers( def test_remove_all_workbasket_changes_button_not_shown_to_users_without_permision( - valid_user_client, user_workbasket + valid_user_client, + user_workbasket, ): url = reverse( "workbaskets:workbasket-ui-changes", @@ -2639,3 +2644,95 @@ def test_remove_all_workbasket_changes_button_not_shown_to_users_without_permisi remove_all_button = page.find("button", value="remove-all") assert not remove_all_button + + +@pytest.fixture +def commodity_with_measures(workbasket, date_ranges): + commodity = factories.GoodsNomenclatureFactory.create( + valid_between=date_ranges.no_end, + ) + measures = factories.MeasureFactory.create_batch(10, goods_nomenclature=commodity) + future_measures = factories.MeasureFactory.create_batch( + 2, + goods_nomenclature=commodity, + valid_between=date_ranges.future, + ) + return commodity + + +def test_auto_end_measures_renders( + valid_user_client, + user_workbasket, + commodity_with_measures, + date_ranges, +): + """Test that the list of measures to be ended renders correctly.""" + commodity_with_measures.new_version( + workbasket=user_workbasket, + valid_between=date_ranges.big, + ) + url = reverse( + "workbaskets:workbasket-ui-auto-end-date-measures", + kwargs={"wb_pk": user_workbasket.pk}, + ) + response = valid_user_client.get(url) + assert response.status_code == 200 + page = BeautifulSoup(str(response.content), "html.parser") + rows = page.find_all("tr", {"class": "govuk-table__row"}) + text = page.get_text() + assert text.count("To be end-dated") == 10 + assert text.count("To be deleted") == 2 + assert len(rows) == 13 + + +@patch("workbaskets.tasks.call_end_measures.apply_async") +def test_auto_end_measures_post( + call_check_end_measures, + valid_user_client, + commodity_with_measures, + user_workbasket, + date_ranges, +): + """Test that posting the auto end measures form results in the end_measures + Celery task being called and redirects to the confirmation page.""" + commodity_with_measures.new_version( + workbasket=user_workbasket, + valid_between=date_ranges.big, + ) + url = reverse( + "workbaskets:workbasket-ui-auto-end-date-measures", + kwargs={"wb_pk": user_workbasket.pk}, + ) + response = valid_user_client.post(url, {"action": "auto-end-date-measures"}) + confirmation_url = reverse( + "workbaskets:workbasket-ui-auto-end-date-measures-confirm", + kwargs={"pk": user_workbasket.pk}, + ) + assert response.status_code == 302 + assert response.url == confirmation_url + assert call_check_end_measures.called + + +def test_auto_end_measures(commodity_with_measures, user_workbasket, date_ranges): + """Test that the call_end_measures correctly ends measures and reorders them + in the workbasket.""" + new_commodity = commodity_with_measures.new_version( + workbasket=user_workbasket, + valid_between=date_ranges.big, + ) + measure_pks = [measure.pk for measure in commodity_with_measures.measures.all()] + call_end_measures(measure_pks, user_workbasket.pk) + measures = user_workbasket.measures + assert len(measures) == 12 + for measure in measures[:10]: + assert measure.valid_between.upper == new_commodity.valid_between.upper + update_types = [measure.update_type for measure in measures] + assert update_types.count(UpdateType.UPDATE) == 10 + assert update_types.count(UpdateType.DELETE) == 2 + first_12_items = ( + TrackedModel.objects.all() + .filter(transaction__workbasket=user_workbasket) + .order_by("transaction__order")[:12] + ) + for item in first_12_items: + assert isinstance(item, Measure) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 40808cc0b..4a1cc8c0a 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -1807,14 +1807,14 @@ def commodities(self): transaction__workbasket=self.workbasket, valid_between__upper_inf=False, ) - .values_list("pk") + .values_list("sid") ) @property def measures(self): # Should I only be filtering for measures without an end date? return Measure.objects.current().filter( - goods_nomenclature__id__in=self.commodities, + goods_nomenclature__sid__in=self.commodities, ) def workbasket_transactions(self): From 79e7d1a608b32c752006d3a4cc71034f35fdab75 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Tue, 3 Dec 2024 11:49:35 +0000 Subject: [PATCH 04/13] Make it part of current wb --- workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja | 2 +- workbaskets/jinja2/workbaskets/checks.jinja | 2 +- workbaskets/tests/test_views.py | 4 +--- workbaskets/urls.py | 2 +- workbaskets/views/ui.py | 5 +++-- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index 51d9bb23d..b91c624ca 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -34,7 +34,7 @@ ]) or "" }} {% endfor %} - {% set base_url = url('workbaskets:workbasket-ui-auto-end-date-measures', kwargs={"wb_pk": workbasket.id}) %} + {% set base_url = url('workbaskets:workbasket-ui-auto-end-date-measures') %} {% set commodity_code %} {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }} diff --git a/workbaskets/jinja2/workbaskets/checks.jinja b/workbaskets/jinja2/workbaskets/checks.jinja index 01fdd3dda..274fb4918 100644 --- a/workbaskets/jinja2/workbaskets/checks.jinja +++ b/workbaskets/jinja2/workbaskets/checks.jinja @@ -32,7 +32,7 @@ - Auto end date measures + Auto end date measures {# Intentionally not styling this as I know this page is getting a revamp #} diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index d94cfa41f..bd9d64813 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -2647,7 +2647,7 @@ def test_remove_all_workbasket_changes_button_not_shown_to_users_without_permisi @pytest.fixture -def commodity_with_measures(workbasket, date_ranges): +def commodity_with_measures(date_ranges): commodity = factories.GoodsNomenclatureFactory.create( valid_between=date_ranges.no_end, ) @@ -2673,7 +2673,6 @@ def test_auto_end_measures_renders( ) url = reverse( "workbaskets:workbasket-ui-auto-end-date-measures", - kwargs={"wb_pk": user_workbasket.pk}, ) response = valid_user_client.get(url) assert response.status_code == 200 @@ -2701,7 +2700,6 @@ def test_auto_end_measures_post( ) url = reverse( "workbaskets:workbasket-ui-auto-end-date-measures", - kwargs={"wb_pk": user_workbasket.pk}, ) response = valid_user_client.post(url, {"action": "auto-end-date-measures"}) confirmation_url = reverse( diff --git a/workbaskets/urls.py b/workbaskets/urls.py index e03a28dc4..8ac545a06 100644 --- a/workbaskets/urls.py +++ b/workbaskets/urls.py @@ -212,7 +212,7 @@ name="workbasket-ui-comment-delete", ), path( - f"/auto-end-date-measures/", + f"current/auto-end-date-measures/", ui_views.AutoEndDateMeasures.as_view(), name="workbasket-ui-auto-end-date-measures", ), diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 4a1cc8c0a..0e506f156 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -1783,6 +1783,7 @@ def status_tag_generator(self, task_status) -> dict: } +@method_decorator(require_current_workbasket, name="dispatch") class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): model = Measure paginate_by = 20 @@ -1794,8 +1795,8 @@ class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): } @property - def workbasket(self): - return WorkBasket.objects.get(pk=self.kwargs["wb_pk"]) + def workbasket(self) -> WorkBasket: + return WorkBasket.current(self.request) @property def commodities(self): From 6b032cd81150d881253c665daa5f9a623d79fa30 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Tue, 3 Dec 2024 12:34:12 +0000 Subject: [PATCH 05/13] Update confirmation page and move link --- workbaskets/jinja2/workbaskets/checks.jinja | 2 -- .../jinja2/workbaskets/confirm_auto_end_date_measures.jinja | 6 ++++-- workbaskets/jinja2/workbaskets/edit-workbasket.jinja | 1 + workbaskets/views/ui.py | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/checks.jinja b/workbaskets/jinja2/workbaskets/checks.jinja index 274fb4918..8102703ca 100644 --- a/workbaskets/jinja2/workbaskets/checks.jinja +++ b/workbaskets/jinja2/workbaskets/checks.jinja @@ -32,8 +32,6 @@ - Auto end date measures - {# Intentionally not styling this as I know this page is getting a revamp #} {% endblock %} \ No newline at end of file diff --git a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja index 2b003c344..bafec7554 100644 --- a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja @@ -18,12 +18,14 @@ }) }} {% endblock %} +{% set measure_count = request.session['count_ended_measures'] if request.session['count_ended_measures'] else '0'%} + {% block content %}
{{ govukPanel({ - "titleText": "X measures will be ended", - "text": "You have successfully submitted X measures to be end-dated or deleted and they will be added to workbasket " ~ request.user.current_workbasket.id ~ " when complete.", + "titleText": measure_count ~ " measure" ~ measure_count|pluralize ~ " will be ended", + "text": "You have successfully submitted " ~ measure_count ~ " measure" ~ measure_count|pluralize ~ " to be end-dated or deleted and added to workbasket " ~ request.user.current_workbasket.id ~ " when complete.", "classes": "govuk-!-margin-bottom-7" }) }}

Next steps

diff --git a/workbaskets/jinja2/workbaskets/edit-workbasket.jinja b/workbaskets/jinja2/workbaskets/edit-workbasket.jinja index b11760b13..d94b33a60 100644 --- a/workbaskets/jinja2/workbaskets/edit-workbasket.jinja +++ b/workbaskets/jinja2/workbaskets/edit-workbasket.jinja @@ -72,6 +72,7 @@ {{ workbasket_column("Measures", [ {"text": "Create a new measure", "url": url('measure-ui-create', kwargs={"step": "start"})}, {"text": "Find and edit measures", "url": url('measure-ui-search')}, + {"text": "Auto end date measures", "url": url('workbaskets:workbasket-ui-auto-end-date-measures')}, {"text": "Read the duty sentence reference", "url": url("duties")}, ]) }} diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 0e506f156..cff9a6385 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -1844,6 +1844,7 @@ def get_context_data(self, **kwargs): def post(self, request, *args, **kwargs): if request.POST.get("action", None) == "auto-end-date-measures": + self.request.session["count_ended_measures"] = len(self.measures) self.end_measures() return redirect( "workbaskets:workbasket-ui-auto-end-date-measures-confirm", From c55ed3b7a9e39ae16f8f4ef5bc49a39e279be595 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Thu, 5 Dec 2024 16:28:57 +0000 Subject: [PATCH 06/13] Move get measures to helper and make tests robust --- common/tests/util.py | 1 + .../workbaskets/auto_end_date_measures.jinja | 2 +- workbaskets/tasks.py | 7 +- workbaskets/tests/test_views.py | 109 ++++++++++++++---- workbaskets/util.py | 37 ++++++ workbaskets/views/ui.py | 20 +--- 6 files changed, 131 insertions(+), 45 deletions(-) diff --git a/common/tests/util.py b/common/tests/util.py index 28ede02a8..f0b3c6a83 100644 --- a/common/tests/util.py +++ b/common/tests/util.py @@ -706,6 +706,7 @@ class Dates: "normal_first_half": (relativedelta(), relativedelta(days=+14)), "starts_1_month_ago_to_delta": (relativedelta(months=-1), relativedelta()), "starts_delta_to_1_month_ahead": (relativedelta(), relativedelta(months=1)), + "starts_delta_to_2_months_ahead": (relativedelta(), relativedelta(months=2)), "starts_1_month_ago_to_1_month_ahead": ( relativedelta(months=-1), relativedelta(months=1), diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index b91c624ca..4ae4d6bab 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -28,7 +28,7 @@ {{ table_rows.append([ {"text": measure_link}, {"html": commodity_link if measure.goods_nomenclature else "-"}, - {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper) if measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper else "-" }, + {"text": "{:%d %b %Y}".format(measure.commodity_end_date) if measure.commodity_end_date else "-" }, {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, {"text": action}, ]) or "" }} diff --git a/workbaskets/tasks.py b/workbaskets/tasks.py index d67c5f01c..c449b823f 100644 --- a/workbaskets/tasks.py +++ b/workbaskets/tasks.py @@ -113,9 +113,10 @@ def end_measures(measures, workbasket): ) .last() ) - if measure.valid_between.lower > commodity.valid_between.upper: - continue - if measure.valid_between.lower > date.today(): + if measure.valid_between.lower > min( + date.today(), + commodity.valid_between.upper, + ): new_measure_version = measure.new_version( workbasket=workbasket, update_type=UpdateType.DELETE, diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index bd9d64813..21db541fa 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -18,6 +18,7 @@ from common.inspect_tap_tasks import CeleryTask from common.inspect_tap_tasks import TAPTasks from common.models.trackedmodel import TrackedModel +from common.models.transactions import Transaction from common.models.utils import override_current_transaction from common.tests import factories from common.tests.util import date_post_data @@ -31,6 +32,7 @@ from workbaskets import models from workbaskets.tasks import call_end_measures from workbaskets.tasks import check_workbasket_sync +from workbaskets.util import get_measures_to_end_date from workbaskets.validators import WorkflowStatus from workbaskets.views import ui @@ -2651,12 +2653,25 @@ def commodity_with_measures(date_ranges): commodity = factories.GoodsNomenclatureFactory.create( valid_between=date_ranges.no_end, ) - measures = factories.MeasureFactory.create_batch(10, goods_nomenclature=commodity) - future_measures = factories.MeasureFactory.create_batch( + factories.MeasureFactory.create_batch( + 5, + goods_nomenclature=commodity, + ) # Open ended measures should be end-dated + factories.MeasureFactory.create_batch( + 4, + goods_nomenclature=commodity, + valid_between=date_ranges.starts_delta_to_2_months_ahead, + ) # Measures ending after the comm code should have their end-date aligned + factories.MeasureFactory.create_batch( + 3, + goods_nomenclature=commodity, + valid_between=date_ranges.starts_with_normal, + ) # Measures ending before the comm code should be ignored + factories.MeasureFactory.create_batch( 2, goods_nomenclature=commodity, valid_between=date_ranges.future, - ) + ) # Measures that have not started yet should be deleted return commodity @@ -2669,7 +2684,7 @@ def test_auto_end_measures_renders( """Test that the list of measures to be ended renders correctly.""" commodity_with_measures.new_version( workbasket=user_workbasket, - valid_between=date_ranges.big, + valid_between=date_ranges.normal, ) url = reverse( "workbaskets:workbasket-ui-auto-end-date-measures", @@ -2679,9 +2694,9 @@ def test_auto_end_measures_renders( page = BeautifulSoup(str(response.content), "html.parser") rows = page.find_all("tr", {"class": "govuk-table__row"}) text = page.get_text() - assert text.count("To be end-dated") == 10 + assert text.count("To be end-dated") == 9 assert text.count("To be deleted") == 2 - assert len(rows) == 13 + assert len(rows) == 12 @patch("workbaskets.tasks.call_end_measures.apply_async") @@ -2716,21 +2731,69 @@ def test_auto_end_measures(commodity_with_measures, user_workbasket, date_ranges in the workbasket.""" new_commodity = commodity_with_measures.new_version( workbasket=user_workbasket, - valid_between=date_ranges.big, + valid_between=date_ranges.normal, + ) + + with override_current_transaction(Transaction.objects.last()): + measures_to_end = get_measures_to_end_date(user_workbasket) + assert len(measures_to_end) == 11 + measure_pks = [measure.pk for measure in measures_to_end] + call_end_measures(measure_pks, user_workbasket.pk) + ended_measures = user_workbasket.measures + for measure in ended_measures[:9]: + assert measure.valid_between.upper == new_commodity.valid_between.upper + update_types = [measure.update_type for measure in ended_measures] + assert update_types.count(UpdateType.UPDATE) == 9 + assert update_types.count(UpdateType.DELETE) == 2 + first_11_items = ( + TrackedModel.objects.all() + .filter(transaction__workbasket=user_workbasket) + .order_by("transaction__order")[:10] + ) + for item in first_11_items: + assert isinstance(item, Measure) + + +def test_get_measures_to_end_date(user_workbasket, date_ranges): + """Test that the utility function correctly gathers measures, not including + those which already have an end date before the commodity's.""" + commodity = factories.GoodsNomenclatureFactory.create( + valid_between=date_ranges.no_end, ) - measure_pks = [measure.pk for measure in commodity_with_measures.measures.all()] - call_end_measures(measure_pks, user_workbasket.pk) - measures = user_workbasket.measures - assert len(measures) == 12 - for measure in measures[:10]: - assert measure.valid_between.upper == new_commodity.valid_between.upper - update_types = [measure.update_type for measure in measures] - assert update_types.count(UpdateType.UPDATE) == 10 - assert update_types.count(UpdateType.DELETE) == 2 - first_12_items = ( - TrackedModel.objects.all() - .filter(transaction__workbasket=user_workbasket) - .order_by("transaction__order")[:12] - ) - for item in first_12_items: - assert isinstance(item, Measure) + + open_ended_measure = factories.MeasureFactory.create( + sid=11, + goods_nomenclature=commodity, + valid_between=date_ranges.no_end, + ) + measure_ended_before_commodity = factories.MeasureFactory.create( + sid=22, + goods_nomenclature=commodity, + valid_between=date_ranges.starts_with_normal, + ) + measure_ended_after_commodity_ends = factories.MeasureFactory.create( + sid=33, + goods_nomenclature=commodity, + valid_between=date_ranges.starts_delta_to_2_months_ahead, + ) + measure_to_start_after_commodity_ends = factories.MeasureFactory.create( + sid=44, + goods_nomenclature=commodity, + valid_between=date_ranges.future, + ) + + new_commodity = commodity.new_version( + workbasket=user_workbasket, + valid_between=date_ranges.normal, + ) + with override_current_transaction(Transaction.objects.last()): + measures = get_measures_to_end_date(user_workbasket) + assert all( + measure in measures + for measure in [ + open_ended_measure, + measure_ended_after_commodity_ends, + measure_to_start_after_commodity_ends, + ] + ) + assert measure_ended_before_commodity not in measures diff --git a/workbaskets/util.py b/workbaskets/util.py index adeee825a..14a85203f 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -2,6 +2,11 @@ from datetime import date from django.db import transaction +from django.db.models import Case +from django.db.models import DateField +from django.db.models import F +from django.db.models import Value +from django.db.models import When from parsec import ParseError from commodities.validators import ITEM_ID_REGEX @@ -180,3 +185,35 @@ def serialize_uploaded_data(data): serialized.append(row_data) return serialized + + +def get_measures_to_end_date(workbasket): + """Gets a list of measures on end-dated commodities along with the commodity + end-dates.""" + from commodities.models.orm import GoodsNomenclature + from measures.models.tracked_models import Measure + + end_dated_commodities = GoodsNomenclature.objects.current().filter( + transaction__workbasket=workbasket, + valid_between__upper_inf=False, + ) + commodity_dict = { + commodity.sid: commodity.valid_between for commodity in end_dated_commodities + } + measures_on_commodities = Measure.objects.current().filter( + goods_nomenclature__sid__in=commodity_dict.keys(), + ) + # TODO:get measures on declarable commodities + conditions = [ + When(goods_nomenclature__sid=commodity_sid, then=Value(commodity_end_date)) + for commodity_sid, commodity_end_date in commodity_dict.items() + ] + measures = measures_on_commodities.annotate( + commodity_valid_between=Case( + *conditions, + default=Value(None), + output_field=DateField(), + ), + ) + + return measures.exclude(valid_between__not_gt=F("commodity_valid_between")) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index cff9a6385..1276ade79 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -39,7 +39,6 @@ from additional_codes.models import AdditionalCode from certificates.models import Certificate from checks.models import TrackedModelCheck -from commodities.models.orm import GoodsNomenclature from common.filters import TamatoFilter from common.inspect_tap_tasks import TAPTasks from common.models import Transaction @@ -74,6 +73,7 @@ from workbaskets.session_store import SessionStore from workbaskets.tasks import call_check_workbasket_sync from workbaskets.tasks import call_end_measures +from workbaskets.util import get_measures_to_end_date from workbaskets.validators import WorkflowStatus from workbaskets.views.decorators import require_current_workbasket from workbaskets.views.mixins import WithCurrentWorkBasket @@ -1798,25 +1798,9 @@ class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): def workbasket(self) -> WorkBasket: return WorkBasket.current(self.request) - @property - def commodities(self): - """Return commodities in the current workbasket which have an end - date.""" - return ( - GoodsNomenclature.objects.current() - .filter( - transaction__workbasket=self.workbasket, - valid_between__upper_inf=False, - ) - .values_list("sid") - ) - @property def measures(self): - # Should I only be filtering for measures without an end date? - return Measure.objects.current().filter( - goods_nomenclature__sid__in=self.commodities, - ) + return get_measures_to_end_date(self.workbasket) def workbasket_transactions(self): """Returns the current workbasket's transactions ordered by `order`, From 61ce597b197c2afefa8f94ff5c88b3175228c8c8 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Thu, 5 Dec 2024 16:54:16 +0000 Subject: [PATCH 07/13] Update template --- .../jinja2/workbaskets/auto_end_date_measures.jinja | 8 ++++++-- workbaskets/views/ui.py | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index 4ae4d6bab..a6361e31d 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -28,7 +28,7 @@ {{ table_rows.append([ {"text": measure_link}, {"html": commodity_link if measure.goods_nomenclature else "-"}, - {"text": "{:%d %b %Y}".format(measure.commodity_end_date) if measure.commodity_end_date else "-" }, + {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper) if measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper else "-" }, {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, {"text": action}, ]) or "" }} @@ -44,11 +44,15 @@ {{ create_sortable_anchor(request, "start_date", "Measure start date", base_url) }} {% endset %} + {% set measure_sid %} + {{ create_sortable_anchor(request, "sid", "Measure SID", base_url) }} + {% endset %} + {% if object_list %}

The following measures are linked to commodity codes that have been end-dated in your workbasket. Click submit to automatically end-date or delete these measures.

{{ govukTable({ "head": [ - {"text": "Measure SID"}, + {"text": measure_sid}, {"text": commodity_code}, {"text": "Commodity end date"}, {"text": start_date}, diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 1276ade79..2a7d9d805 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -1788,10 +1788,11 @@ class AutoEndDateMeasures(SortingMixin, WithPaginationListMixin, ListView): model = Measure paginate_by = 20 template_name = "workbaskets/auto_end_date_measures.jinja" - sort_by_fields = ["start_date", "goods_nomenclature"] + sort_by_fields = ["start_date", "goods_nomenclature", "sid"] custom_sorting = { "start_date": "valid_between", "goods_nomenclature": "goods_nomenclature__item_id", + "sid": "sid", } @property From 235903de83768936e452369e5ac291677577882c Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Fri, 6 Dec 2024 12:18:16 +0000 Subject: [PATCH 08/13] Use effective end date on measures --- .../jinja2/workbaskets/auto_end_date_measures.jinja | 2 ++ workbaskets/util.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index a6361e31d..81cbf284f 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -30,6 +30,7 @@ {"html": commodity_link if measure.goods_nomenclature else "-"}, {"text": "{:%d %b %Y}".format(measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper) if measure.goods_nomenclature.version_at(workbasket.transactions.last()).valid_between.upper else "-" }, {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, + {"text": "{:%d %b %Y}".format(measure.effective_end_date) if measure.effective_end_date else '-' }, {"text": action}, ]) or "" }} {% endfor %} @@ -56,6 +57,7 @@ {"text": commodity_code}, {"text": "Commodity end date"}, {"text": start_date}, + {"text": 'Measure end date'}, {"text": "Action"}, ], "rows": table_rows diff --git a/workbaskets/util.py b/workbaskets/util.py index 14a85203f..0640572e2 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -200,8 +200,12 @@ def get_measures_to_end_date(workbasket): commodity_dict = { commodity.sid: commodity.valid_between for commodity in end_dated_commodities } - measures_on_commodities = Measure.objects.current().filter( - goods_nomenclature__sid__in=commodity_dict.keys(), + measures_on_commodities = ( + Measure.objects.current() + .with_effective_valid_between() + .filter( + goods_nomenclature__sid__in=commodity_dict.keys(), + ) ) # TODO:get measures on declarable commodities conditions = [ @@ -216,4 +220,6 @@ def get_measures_to_end_date(workbasket): ), ) - return measures.exclude(valid_between__not_gt=F("commodity_valid_between")) + return measures.exclude( + db_effective_valid_between__not_gt=F("commodity_valid_between"), + ) From 0f0fca304f0883360b9cdc778918458dd7e6a20d Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Wed, 11 Dec 2024 12:02:46 +0000 Subject: [PATCH 09/13] Move auto end date link as per design request --- .../workbaskets/confirm_auto_end_date_measures.jinja | 3 +-- workbaskets/jinja2/workbaskets/edit-workbasket.jinja | 1 - .../jinja2/workbaskets/summary-workbasket.jinja | 10 ++++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja index bafec7554..6586f99dc 100644 --- a/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/confirm_auto_end_date_measures.jinja @@ -10,8 +10,7 @@ {{ govukBreadcrumbs({ "items": [ {"text": "Home", "href": url("home")}, - {"text": "Workbasket " ~ request.user.current_workbasket.id }, - {"text": "Workbasket " ~ request.user.current_workbasket.id ~ " checks", "href": url("workbaskets:workbasket-checks")}, + {"text": "Workbasket " ~ request.user.current_workbasket.id, "href": url("workbaskets:edit-workbasket")}, {"text": "Workbasket " ~ request.user.current_workbasket.id ~ " auto end date measures"}, {"text": page_title} ] diff --git a/workbaskets/jinja2/workbaskets/edit-workbasket.jinja b/workbaskets/jinja2/workbaskets/edit-workbasket.jinja index d94b33a60..b11760b13 100644 --- a/workbaskets/jinja2/workbaskets/edit-workbasket.jinja +++ b/workbaskets/jinja2/workbaskets/edit-workbasket.jinja @@ -72,7 +72,6 @@ {{ workbasket_column("Measures", [ {"text": "Create a new measure", "url": url('measure-ui-create', kwargs={"step": "start"})}, {"text": "Find and edit measures", "url": url('measure-ui-search')}, - {"text": "Auto end date measures", "url": url('workbaskets:workbasket-ui-auto-end-date-measures')}, {"text": "Read the duty sentence reference", "url": url("duties")}, ]) }} diff --git a/workbaskets/jinja2/workbaskets/summary-workbasket.jinja b/workbaskets/jinja2/workbaskets/summary-workbasket.jinja index 0a7be4454..77f3aad2b 100644 --- a/workbaskets/jinja2/workbaskets/summary-workbasket.jinja +++ b/workbaskets/jinja2/workbaskets/summary-workbasket.jinja @@ -169,6 +169,16 @@ {% endif %}
{% endif %} +
+
Auto end-date measures
+
Automatically end-date measures on commodities which have been ended in this workbasket.
+
+ Auto end-date measures +
+
{% if comments and can_view_comment %} From 3152d5c1c9c9fbc6ec5256d1ab31dd613fcdc610 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Thu, 12 Dec 2024 12:07:48 +0000 Subject: [PATCH 10/13] Black --- workbaskets/tests/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index e0b59e167..e8fcb7516 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -2831,4 +2831,3 @@ def test_reordering_transactions_bug(valid_user_client, user_workbasket): pytest.fail( "IntegrityError - New trackedmodel cannot be created after reordering then deleting transactions.", ) - From 406c9bc7030110043ab61f41475efcbfa53f789e Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Thu, 12 Dec 2024 15:21:03 +0000 Subject: [PATCH 11/13] final touches --- workbaskets/tests/test_views.py | 2 ++ workbaskets/util.py | 5 ++--- workbaskets/views/ui.py | 20 +++++--------------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index e8fcb7516..1bf58d5ec 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -2651,6 +2651,8 @@ def test_remove_all_workbasket_changes_button_not_shown_to_users_without_permisi @pytest.fixture def commodity_with_measures(date_ranges): + """Fixture used for texting the automatic end-dating of measures on an end- + dated commodity code.""" commodity = factories.GoodsNomenclatureFactory.create( valid_between=date_ranges.no_end, ) diff --git a/workbaskets/util.py b/workbaskets/util.py index 0640572e2..513df12d6 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -207,10 +207,9 @@ def get_measures_to_end_date(workbasket): goods_nomenclature__sid__in=commodity_dict.keys(), ) ) - # TODO:get measures on declarable commodities conditions = [ - When(goods_nomenclature__sid=commodity_sid, then=Value(commodity_end_date)) - for commodity_sid, commodity_end_date in commodity_dict.items() + When(goods_nomenclature__sid=commodity_sid, then=Value(commodity_valid_between)) + for commodity_sid, commodity_valid_between in commodity_dict.items() ] measures = measures_on_commodities.annotate( commodity_valid_between=Case( diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 2a7d9d805..6053748da 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -1803,15 +1803,6 @@ def workbasket(self) -> WorkBasket: def measures(self): return get_measures_to_end_date(self.workbasket) - def workbasket_transactions(self): - """Returns the current workbasket's transactions ordered by `order`, - while guarding against non-editing status on workbasket to minimise - chances of mishap.""" - return Transaction.objects.filter( - workbasket=self.workbasket, - workbasket__status=WorkflowStatus.EDITING, - ).order_by("order") - def get_queryset(self): ordering = self.get_ordering() queryset = self.measures @@ -1829,12 +1820,12 @@ def get_context_data(self, **kwargs): def post(self, request, *args, **kwargs): if request.POST.get("action", None) == "auto-end-date-measures": - self.request.session["count_ended_measures"] = len(self.measures) self.end_measures() - return redirect( - "workbaskets:workbasket-ui-auto-end-date-measures-confirm", - self.workbasket.pk, - ) + self.request.session["count_ended_measures"] = len(self.measures) + return redirect( + "workbaskets:workbasket-ui-auto-end-date-measures-confirm", + self.workbasket.pk, + ) @atomic def end_measures(self): @@ -1845,4 +1836,3 @@ def end_measures(self): class AutoEndDateMeasuresConfirm(DetailView): template_name = "workbaskets/confirm_auto_end_date_measures.jinja" model = WorkBasket - queryset = WorkBasket.objects.all() From b64a9001a56858b83a0daa8125d180a62870ab23 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Tue, 17 Dec 2024 15:22:09 +0000 Subject: [PATCH 12/13] Responding to PR comments --- settings/common.py | 6 ++--- workbaskets/models.py | 48 +++++++++++++++++++++++++++++++++ workbaskets/tasks.py | 1 + workbaskets/tests/test_views.py | 5 ++-- workbaskets/util.py | 42 ----------------------------- workbaskets/views/ui.py | 4 +-- 6 files changed, 55 insertions(+), 51 deletions(-) diff --git a/settings/common.py b/settings/common.py index 01fbd39a8..041d45eba 100644 --- a/settings/common.py +++ b/settings/common.py @@ -647,9 +647,6 @@ "workbaskets.tasks.check_workbasket": { "queue": "rule-check", }, - "workbaskets.tasks.call_end_measures": { - "queue": "standard", - }, "workbaskets.tasks.transition": { "queue": "standard", }, @@ -674,6 +671,9 @@ "measures.tasks.bulk_edit_measures": { "queue": "bulk-create", }, + "workbaskets.tasks.call_end_measures": { + "queue": "bulk-create", + }, re.compile(r"(reference_documents)\.tasks\..*"): { "queue": "standard", }, diff --git a/workbaskets/models.py b/workbaskets/models.py index 674265df9..d7f58a8d8 100644 --- a/workbaskets/models.py +++ b/workbaskets/models.py @@ -14,9 +14,14 @@ from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ValidationError from django.db import models +from django.db.models import Case +from django.db.models import DateField +from django.db.models import F from django.db.models import Max from django.db.models import QuerySet from django.db.models import Subquery +from django.db.models import Value +from django.db.models import When from django_fsm import FSMField from django_fsm import transition @@ -687,6 +692,49 @@ def assigned_reviewers(self): user_ids = self.reviewer_assignments.values_list("user_id", flat=True) return User.objects.filter(id__in=user_ids) + def get_measures_to_end_date(self) -> QuerySet: + """ + Returns a queryset of measures on end-dated commodities in the + workbasket along with those commodities' end-dates. + + It filters out measures which have already ended. + """ + + from commodities.models.orm import GoodsNomenclature + + end_dated_commodities = GoodsNomenclature.objects.current().filter( + transaction__workbasket=self, + valid_between__upper_inf=False, + ) + commodity_dict = { + commodity.sid: commodity.valid_between + for commodity in end_dated_commodities + } + measures_on_commodities = ( + Measure.objects.current() + .with_effective_valid_between() + .filter( + goods_nomenclature__sid__in=commodity_dict.keys(), + ) + ) + conditions = [ + When( + goods_nomenclature__sid=commodity_sid, + then=Value(commodity_valid_between), + ) + for commodity_sid, commodity_valid_between in commodity_dict.items() + ] + measures = measures_on_commodities.annotate( + commodity_valid_between=Case( + *conditions, + output_field=DateField(), + ), + ) + + return measures.exclude( + db_effective_valid_between__not_gt=F("commodity_valid_between"), + ) + class Meta: verbose_name = "workbasket" verbose_name_plural = "workbaskets" diff --git a/workbaskets/tasks.py b/workbaskets/tasks.py index c449b823f..1a8a62bd7 100644 --- a/workbaskets/tasks.py +++ b/workbaskets/tasks.py @@ -97,6 +97,7 @@ def promote_measure_to_top(promoted_measure, workbasket_transactions): promoted_measure.save(update_fields=["order"]) +@atomic def end_measures(measures, workbasket): """Iterate through measures on commodities, end-date those which have already began and delete those which have not yet started.""" diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index 1bf58d5ec..fbb28313e 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -33,7 +33,6 @@ from workbaskets import models from workbaskets.tasks import call_end_measures from workbaskets.tasks import check_workbasket_sync -from workbaskets.util import get_measures_to_end_date from workbaskets.validators import WorkflowStatus from workbaskets.views import ui @@ -2738,7 +2737,7 @@ def test_auto_end_measures(commodity_with_measures, user_workbasket, date_ranges ) with override_current_transaction(Transaction.objects.last()): - measures_to_end = get_measures_to_end_date(user_workbasket) + measures_to_end = user_workbasket.get_measures_to_end_date() assert len(measures_to_end) == 11 measure_pks = [measure.pk for measure in measures_to_end] call_end_measures(measure_pks, user_workbasket.pk) @@ -2790,7 +2789,7 @@ def test_get_measures_to_end_date(user_workbasket, date_ranges): valid_between=date_ranges.normal, ) with override_current_transaction(Transaction.objects.last()): - measures = get_measures_to_end_date(user_workbasket) + measures = user_workbasket.get_measures_to_end_date() assert all( measure in measures for measure in [ diff --git a/workbaskets/util.py b/workbaskets/util.py index 513df12d6..adeee825a 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -2,11 +2,6 @@ from datetime import date from django.db import transaction -from django.db.models import Case -from django.db.models import DateField -from django.db.models import F -from django.db.models import Value -from django.db.models import When from parsec import ParseError from commodities.validators import ITEM_ID_REGEX @@ -185,40 +180,3 @@ def serialize_uploaded_data(data): serialized.append(row_data) return serialized - - -def get_measures_to_end_date(workbasket): - """Gets a list of measures on end-dated commodities along with the commodity - end-dates.""" - from commodities.models.orm import GoodsNomenclature - from measures.models.tracked_models import Measure - - end_dated_commodities = GoodsNomenclature.objects.current().filter( - transaction__workbasket=workbasket, - valid_between__upper_inf=False, - ) - commodity_dict = { - commodity.sid: commodity.valid_between for commodity in end_dated_commodities - } - measures_on_commodities = ( - Measure.objects.current() - .with_effective_valid_between() - .filter( - goods_nomenclature__sid__in=commodity_dict.keys(), - ) - ) - conditions = [ - When(goods_nomenclature__sid=commodity_sid, then=Value(commodity_valid_between)) - for commodity_sid, commodity_valid_between in commodity_dict.items() - ] - measures = measures_on_commodities.annotate( - commodity_valid_between=Case( - *conditions, - default=Value(None), - output_field=DateField(), - ), - ) - - return measures.exclude( - db_effective_valid_between__not_gt=F("commodity_valid_between"), - ) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 6053748da..b0373381e 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -73,7 +73,6 @@ from workbaskets.session_store import SessionStore from workbaskets.tasks import call_check_workbasket_sync from workbaskets.tasks import call_end_measures -from workbaskets.util import get_measures_to_end_date from workbaskets.validators import WorkflowStatus from workbaskets.views.decorators import require_current_workbasket from workbaskets.views.mixins import WithCurrentWorkBasket @@ -1801,7 +1800,7 @@ def workbasket(self) -> WorkBasket: @property def measures(self): - return get_measures_to_end_date(self.workbasket) + return self.workbasket.get_measures_to_end_date() def get_queryset(self): ordering = self.get_ordering() @@ -1827,7 +1826,6 @@ def post(self, request, *args, **kwargs): self.workbasket.pk, ) - @atomic def end_measures(self): measure_pks = [measure.pk for measure in self.measures] call_end_measures.apply_async((measure_pks, self.workbasket.pk)) From c21b6483c2e3ee5823a5a9cc40015036fc1cdc14 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie Date: Tue, 17 Dec 2024 15:51:59 +0000 Subject: [PATCH 13/13] Update message for no measures found --- workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja index 81cbf284f..8e6f24aa9 100644 --- a/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja +++ b/workbaskets/jinja2/workbaskets/auto_end_date_measures.jinja @@ -79,7 +79,8 @@ {% else %} -

There are no current measures related to commodity codes updated in this workbasket.

+

No measures to end-date have been found.

+

There are either no active measures related to commodity codes updated in this workbasket or the measures already have end-dates in line with their respective commodities.

{% endif %} {% include "includes/common/pagination.jinja" %}