From 1b0ef4d19ed110778975130723cfd11ddf5591d4 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 20 Sep 2024 15:01:33 +0100 Subject: [PATCH 01/12] TP2000-1352: Add react origins form to quota create page (#1297) * Add react origins form to quota create page to fix workbasket transaction error * Update test * Add extra errors to create form * Pass initial data to origins form --- common/forms.py | 32 +++ common/jinja2/layouts/create.jinja | 10 +- .../js/components/QuotaOriginFormset/index.js | 5 +- .../tests/__snapshots__/index.test.js.snap | 233 ++++++++++++++++++ .../QuotaOriginFormset/tests/index.test.js | 3 +- quotas/forms.py | 189 +++++++------- .../quotas/quota-create-origins.jinja | 30 +++ quotas/tests/test_forms.py | 140 ++++++++++- quotas/tests/test_views.py | 221 ++++++++++++++++- quotas/views.py | 65 +++++ 10 files changed, 819 insertions(+), 109 deletions(-) create mode 100644 quotas/jinja2/includes/quotas/quota-create-origins.jinja diff --git a/common/forms.py b/common/forms.py index 6c5de5117..af31c8386 100644 --- a/common/forms.py +++ b/common/forms.py @@ -14,6 +14,7 @@ from crispy_forms_gds.layout import Submit from django import forms from django.contrib.postgres.forms.ranges import DateRangeField +from django.core.exceptions import NON_FIELD_ERRORS from django.core.exceptions import ValidationError from django.forms.renderers import get_default_renderer from django.forms.utils import ErrorList @@ -892,3 +893,34 @@ def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: representation. """ return {} + + +class ExtraErrorFormMixin: + def add_extra_error(self, field, error): + """ + A modification of Django's add_error method that allows us to add data + to self._errors under custom keys that are not field names or + NON_FIELD_ERRORS. + + Used to pass errors to the React form. + """ + if not isinstance(error, ValidationError): + error = ValidationError(error) + + if hasattr(error, "error_dict"): + if field is not None: + raise TypeError( + "The argument `field` must be `None` when the `error` " + "argument contains errors for multiple fields.", + ) + else: + error = error.error_dict + else: + error = {field or NON_FIELD_ERRORS: error.error_list} + + for field, error_list in error.items(): + if field not in self.errors: + self._errors[field] = self.error_class() + self._errors[field].extend(error_list) + if field in self.cleaned_data: + del self.cleaned_data[field] diff --git a/common/jinja2/layouts/create.jinja b/common/jinja2/layouts/create.jinja index 725925119..7220f2739 100644 --- a/common/jinja2/layouts/create.jinja +++ b/common/jinja2/layouts/create.jinja @@ -1,11 +1,7 @@ {% extends 'layouts/form.jinja' %} {% block form %} -
-
- {% call django_form() %} - {{ crispy(form) }} - {% endcall %} -
-
+ {% call django_form() %} + {{ crispy(form) }} + {% endcall %} {% endblock %} diff --git a/common/static/common/js/components/QuotaOriginFormset/index.js b/common/static/common/js/components/QuotaOriginFormset/index.js index f08b771d3..d0c9ab28a 100644 --- a/common/static/common/js/components/QuotaOriginFormset/index.js +++ b/common/static/common/js/components/QuotaOriginFormset/index.js @@ -13,7 +13,6 @@ function QuotaOriginFormset({ groupsWithMembers, errors, }) { - const [origins, setOrigins] = useState([...data]); const emptyOrigin = { id: "", pk: "", @@ -26,6 +25,10 @@ function QuotaOriginFormset({ end_date_1: "", end_date_2: "", }; + if (data.length == 0) { + data.push(emptyOrigin); + } + const [origins, setOrigins] = useState([...data]); const addEmptyOrigin = (e) => { e.preventDefault(); diff --git a/common/static/common/js/components/QuotaOriginFormset/tests/__snapshots__/index.test.js.snap b/common/static/common/js/components/QuotaOriginFormset/tests/__snapshots__/index.test.js.snap index 0c5c5abb3..3bb048dcf 100644 --- a/common/static/common/js/components/QuotaOriginFormset/tests/__snapshots__/index.test.js.snap +++ b/common/static/common/js/components/QuotaOriginFormset/tests/__snapshots__/index.test.js.snap @@ -4,6 +4,239 @@ exports[`QuotaOriginFormset renders empty formset when no initial data 1`] = `
+
+

+ Origin + 1 +

+ +
+
+ + + Start date + + +
+ + + +
+
+
+
+
+ + + End date + + + + Leave empty if a quota order number origin is needed for an unlimited time + +
+ + + +
+
+
+
+ + Geographical area + + +
+
+
diff --git a/quotas/jinja2/includes/quotas/tabs/blocking_periods.jinja b/quotas/jinja2/includes/quotas/tabs/blocking_periods.jinja index 542ebaa28..e65adc223 100644 --- a/quotas/jinja2/includes/quotas/tabs/blocking_periods.jinja +++ b/quotas/jinja2/includes/quotas/tabs/blocking_periods.jinja @@ -1,6 +1,6 @@
-

Details

+

Blocking periods

{% if blocking_period %} {{ govukSummaryList({ "rows": [ diff --git a/quotas/jinja2/includes/quotas/tabs/core_data.jinja b/quotas/jinja2/includes/quotas/tabs/core_data.jinja index d5209de73..f88e22ddf 100644 --- a/quotas/jinja2/includes/quotas/tabs/core_data.jinja +++ b/quotas/jinja2/includes/quotas/tabs/core_data.jinja @@ -8,11 +8,11 @@
-

Details

+

Order number details

{{ govukSummaryList({ "rows": [ { - "key": {"text": "Order Number"}, + "key": {"text": "Order number"}, "value": {"text": object.order_number}, "actions": {"items": []} }, diff --git a/quotas/jinja2/includes/quotas/tabs/definition_details.jinja b/quotas/jinja2/includes/quotas/tabs/definition_details.jinja index 628036b57..1cc10892a 100644 --- a/quotas/jinja2/includes/quotas/tabs/definition_details.jinja +++ b/quotas/jinja2/includes/quotas/tabs/definition_details.jinja @@ -1,6 +1,6 @@
-

Details

+

Definition details

{% if current_definition %} {{ govukSummaryList({ "rows": [ diff --git a/quotas/jinja2/includes/quotas/tabs/measures.jinja b/quotas/jinja2/includes/quotas/tabs/measures.jinja index 7798c56df..761e6a562 100644 --- a/quotas/jinja2/includes/quotas/tabs/measures.jinja +++ b/quotas/jinja2/includes/quotas/tabs/measures.jinja @@ -24,7 +24,7 @@
-

Details

+

Measures

{% if measures %}

Showing currently active measures.

{{ govukTable({ diff --git a/quotas/jinja2/includes/quotas/tabs/sub_quotas.jinja b/quotas/jinja2/includes/quotas/tabs/sub_quotas.jinja index a03d4147e..f8514ccba 100644 --- a/quotas/jinja2/includes/quotas/tabs/sub_quotas.jinja +++ b/quotas/jinja2/includes/quotas/tabs/sub_quotas.jinja @@ -1,33 +1,69 @@ {% set table_rows = [] %} -{% for quota_association in quota_associations %} +{% for object in sub_quota_associations %} {% set sub_quota_link %} - {{ quota_association.sub_quota.order_number }} + href="{{ url('quota-ui-detail', args=[object.sub_quota.order_number.sid]) }}"> + {{ object.sub_quota.order_number }} {% endset %} + {% set actions_html %} + Edit +
+ Delete + {% endset %} {{ table_rows.append([ {"html": sub_quota_link}, - {"html": quota_association.sub_quota_relation_type}, - {"text": quota_association.coefficient}, + {"html": object.get_sub_quota_relation_type_display()}, + {"text": object.coefficient}, + {"text": actions_html } + ]) or "" }} +{% endfor %} + +{% for object in main_quota_associations %} + {% set main_quota_link %} + + {{ object.main_quota.order_number }} + + {% endset %} + {% set actions_html %} + Edit +
+ Delete + {% endset %} + + {{ table_rows.append([ + {"html": main_quota_link}, + {"html": object.get_sub_quota_relation_type_display()}, + {"text": object.coefficient}, + {"text": actions_html } ]) or "" }} {% endfor %}
-

Details

- {% if quota_associations %} - {{ govukTable({ - "head": [ - {"text": "Sub-quota order number"}, - {"text": "Relation type"}, +

Quota associations

+ {% if sub_quota_associations or main_quota_associations %} + {% if main_quota_associations %} +

Main quota

+ {% elif sub_quota_associations %} +

Sub-quotas

+ {% endif %} + {{ + govukTable({ + "head": [ + {"text": "Order number"}, + {"text": "Relationship type"}, {"text": "Co-efficient"}, - ], - "rows": table_rows - }) }} - {% else %} -

No active sub-quotas.

- {% endif %} + {"text": "Actions"} + ], + "rows": table_rows + }) + }} + {% else %} +

No active main or sub-quotas.

+ {% endif %} +
{% include "includes/quotas/actions.jinja"%}
diff --git a/quotas/jinja2/includes/quotas/tabs/suspension_periods.jinja b/quotas/jinja2/includes/quotas/tabs/suspension_periods.jinja index 960fc4e3f..722db386e 100644 --- a/quotas/jinja2/includes/quotas/tabs/suspension_periods.jinja +++ b/quotas/jinja2/includes/quotas/tabs/suspension_periods.jinja @@ -1,6 +1,6 @@
-

Details

+

Suspension periods

{% if suspension_period %} {{ govukSummaryList({ "rows": [ diff --git a/quotas/jinja2/quota-definitions/sub-quota-definitions-updates.jinja b/quotas/jinja2/quota-definitions/sub-quota-definitions-updates.jinja index 8d6974575..303aba2f4 100644 --- a/quotas/jinja2/quota-definitions/sub-quota-definitions-updates.jinja +++ b/quotas/jinja2/quota-definitions/sub-quota-definitions-updates.jinja @@ -3,9 +3,10 @@ {% from "components/table/macro.njk" import govukTable %} -{% set page_title = "Update sub-quota definition and association" %} +{% set page_title = "Edit sub-quota definition and association" %} {% block content %} +

{{ page_title }}

Main quota definition details

{% set main_definition = view.get_main_definition() %} {% set table_rows = [] %} diff --git a/quotas/jinja2/quotas/definitions.jinja b/quotas/jinja2/quotas/definitions.jinja index 7fdf121a9..614da2ec4 100644 --- a/quotas/jinja2/quotas/definitions.jinja +++ b/quotas/jinja2/quotas/definitions.jinja @@ -10,6 +10,12 @@ {% set page_title = "Quota ID: " ~ quota.order_number ~ " - Data" %} {% set find_edit_url = url("quota-ui-list") %} +{% if sub_quotas %} + {% set page_subtitle = "Main quota" %} +{% elif main_quotas %} + {% set page_subtitle = "Sub-quota" %} +{% endif %} + {% set links = [ { "text": "Quota definition periods", @@ -17,9 +23,9 @@ "selected": quota_type == None }, { - "text": "Sub-quotas", - "href": url('quota_definition-ui-list-filter', kwargs={"sid": quota.sid, "quota_type": "sub_quotas"}), - "selected": quota_type == "sub_quotas", + "text": "Quota associations", + "href": url('quota_definition-ui-list-filter', kwargs={"sid": quota.sid, "quota_type": "quota_associations"}), + "selected": quota_type == "quota_associations", }, { "text": "Blocking periods", @@ -43,9 +49,16 @@ {% endblock %} {% block content %} - +{% if page_subtitle %} + + {% block page_subtitle %} + {{ page_subtitle }} + {% endblock %} + +{% endif %}

{{ page_title }}

+ {{ fake_tabs(links) }}
{% set base_url = url('quota_definition-ui-list', args=[quota.sid] ) %} @@ -54,7 +67,7 @@ {% include "quotas/tables/blocking_periods.jinja" %} {% elif quota_type == "suspension_periods" %} {% include "quotas/tables/suspension_periods.jinja" %} - {% elif quota_type == "sub_quotas" %} + {% elif quota_type == "quota_associations" %} {% include "quotas/tables/sub_quotas.jinja" %} {% else %} {% include "quotas/tables/definitions.jinja" %} diff --git a/quotas/jinja2/quotas/detail.jinja b/quotas/jinja2/quotas/detail.jinja index 77d02fa2d..a3f18e522 100644 --- a/quotas/jinja2/quotas/detail.jinja +++ b/quotas/jinja2/quotas/detail.jinja @@ -2,9 +2,15 @@ {% from "components/summary-list/macro.njk" import govukSummaryList %} {% from "components/table/macro.njk" import govukTable %} -{% set page_title = "Quota ID: " ~ object.order_number %} +{% set page_title = "Quota order number: " ~ object.order_number %} {% set find_edit_url = url("quota-ui-list") %} +{% if sub_quota_associations %} + {% set title_caption = "Main quota" %} +{% elif main_quota_associations %} + {% set title_caption = "Sub-quota" %} +{% endif %} + {% set core_data_tab_html %}{% include "includes/quotas/tabs/core_data.jinja" %}{% endset %} {% set definition_details_html %}{% include "includes/quotas/tabs/definition_details.jinja" %}{% endset %} {% set sub_quotas_html %}{% include "includes/quotas/tabs/sub_quotas.jinja" %}{% endset %} @@ -30,8 +36,8 @@ } }, { - "label": "Sub-quotas", - "id": "sub-quotas", + "label": "Quota associations", + "id": "quotas-associations", "panel": { "html": sub_quotas_html } diff --git a/quotas/jinja2/quotas/tables/definitions.jinja b/quotas/jinja2/quotas/tables/definitions.jinja index 27105d196..494a25dca 100644 --- a/quotas/jinja2/quotas/tables/definitions.jinja +++ b/quotas/jinja2/quotas/tables/definitions.jinja @@ -65,7 +65,7 @@ {% endfor %} {% set sid %} - {{ create_sortable_anchor(request, "sid", "Quota definition sid", base_url) }} + {{ create_sortable_anchor(request, "sid", "Quota definition SID", base_url) }} {% endset %} {% set start_date %} diff --git a/quotas/jinja2/quotas/tables/sub_quotas.jinja b/quotas/jinja2/quotas/tables/sub_quotas.jinja index 430089fa7..acc8b0701 100644 --- a/quotas/jinja2/quotas/tables/sub_quotas.jinja +++ b/quotas/jinja2/quotas/tables/sub_quotas.jinja @@ -1,8 +1,8 @@ -

Sub-quotas

- +

Quota associations

{% set table_rows = [] %} {% if sub_quotas %} +

Sub-quotas

{% for object in sub_quotas %} {% set definition_link -%} {{ object.sub_quota.sid }} @@ -11,9 +11,9 @@ {{ object.sub_quota.order_number.order_number }} {% endset %} {% set actions_html %} - Edit association + Edit
- Delete association + Delete {% endset %} {{ table_rows.append([ @@ -30,7 +30,43 @@ {{ govukTable({ "head": [ {"text": "Quota definition SID"}, - {"text": "Sub-quota order number"}, + {"text": "Order number"}, + {"text": "Start date"}, + {"text": "End date"}, + {"text": "Relationship type"}, + {"text": "Coefficient"}, + {"text": "Actions"}, + ], + "rows": table_rows + }) }} +{% elif main_quotas %} +

Main quotas

+ {% for object in main_quotas %} + {% set definition_link -%} + {{ object.main_quota.sid }} + {% endset %} + {% set main_quota_link -%} + {{ object.main_quota.order_number.order_number }} + {% endset %} + {% set actions_html %} + Edit +
+ Delete + {% endset %} + {{ table_rows.append([ + {"text": definition_link }, + {"text": main_quota_link }, + {"text": "{:%d %b %Y}".format(object.main_quota.version_at(request.user.current_workbasket.transactions.last()).valid_between.lower) }, + {"text": "{:%d %b %Y}".format(object.main_quota.version_at(request.user.current_workbasket.transactions.last()).valid_between.upper) if object.main_quota.valid_between.upper else "-"}, + {"text": object.get_sub_quota_relation_type_display() }, + {"text": object.coefficient }, + {"text": actions_html }, + ]) or "" }} + {% endfor %} + {{ govukTable({ + "head": [ + {"text": "Quota definition SID"}, + {"text": "Order number"}, {"text": "Start date"}, {"text": "End date"}, {"text": "Relationship type"}, @@ -40,5 +76,5 @@ "rows": table_rows }) }} {% else %} -

There are no sub-quotas for this quota order number.

+

There are no main or sub-quotas for this quota order number.

{% endif %} \ No newline at end of file diff --git a/quotas/tests/test_views.py b/quotas/tests/test_views.py index eb960c1a5..ca366c957 100644 --- a/quotas/tests/test_views.py +++ b/quotas/tests/test_views.py @@ -680,6 +680,7 @@ def test_quota_detail_sub_quota_tab( valid_user_client, date_ranges, mock_quota_api_no_data, + session_request_with_workbasket, ): quota_order_number = factories.QuotaOrderNumberFactory() current_definition = factories.QuotaDefinitionFactory.create( @@ -2010,7 +2011,7 @@ def test_quota_definition_view(client_with_current_workbasket): response = client_with_current_workbasket.get( reverse( "quota_definition-ui-list-filter", - kwargs={"sid": main_quota.sid, "quota_type": "sub_quotas"}, + kwargs={"sid": main_quota.sid, "quota_type": "quota_associations"}, ), ) assert response.status_code == 200 diff --git a/quotas/views.py b/quotas/views.py index af6bf09e3..d373a8a3f 100644 --- a/quotas/views.py +++ b/quotas/views.py @@ -240,12 +240,14 @@ def get_context_data(self, *args, **kwargs): f"{URLs.BASE_URL.value}quota_search?order_number={self.object.order_number}" ) - context[ - "quota_associations" - ] = QuotaAssociation.objects.latest_approved().filter( + context["sub_quota_associations"] = QuotaAssociation.objects.current().filter( main_quota=current_definition, ) + context["main_quota_associations"] = QuotaAssociation.objects.current().filter( + sub_quota=current_definition, + ) + context["blocking_period"] = ( QuotaBlocking.objects.filter(quota_definition=current_definition) .as_at_and_beyond(date.today()) @@ -314,6 +316,13 @@ def sub_quotas(self): .order_by("sub_quota__sid") ) + @property + def main_quotas(self): + main_quotas = QuotaAssociation.objects.current().filter( + sub_quota__order_number=self.quota, + ) + return main_quotas + @cached_property def quota_data(self): if not self.kwargs.get("quota_type"): @@ -332,6 +341,7 @@ def get_context_data(self, *args, **kwargs): blocking_periods=self.blocking_periods, suspension_periods=self.suspension_periods, sub_quotas=self.sub_quotas, + main_quotas=self.main_quotas, *args, **kwargs, ) From eb9a3100a0e9b481fe6efa846ce461e855ab663c Mon Sep 17 00:00:00 2001 From: Matthew McKenzie <97194636+mattjamc@users.noreply.github.com> Date: Mon, 21 Oct 2024 12:39:54 +0100 Subject: [PATCH 10/12] TP2000-1477 Edit capability for suspension periods (#1306) * Ability to edit suspension periods * Add ability to delete a suspension period * Form tests * View tests * Fix failing tests * Fix and reorder cancel button * Redesign confirm pages * PR review comments * test changes following PR review * Update get_success_url to make kwarg clearer --- common/forms.py | 19 +++-- common/jinja2/common/delete.jinja | 6 -- quotas/forms.py | 65 +++++++++++++++ .../quota-suspensions/confirm-delete.jinja | 49 ++++++++++++ .../quota-suspensions/confirm-update.jinja | 25 ++++++ quotas/jinja2/quota-suspensions/delete.jinja | 13 +++ quotas/jinja2/quota-suspensions/edit.jinja | 14 ++++ .../quotas/tables/suspension_periods.jinja | 9 ++- quotas/models.py | 18 +++++ quotas/tests/test_forms.py | 52 +++++++++++- quotas/tests/test_views.py | 79 ++++++++++++++++++- quotas/urls.py | 30 +++++++ quotas/views.py | 77 +++++++++++++++++- 13 files changed, 439 insertions(+), 17 deletions(-) create mode 100644 quotas/jinja2/quota-suspensions/confirm-delete.jinja create mode 100644 quotas/jinja2/quota-suspensions/confirm-update.jinja create mode 100644 quotas/jinja2/quota-suspensions/delete.jinja create mode 100644 quotas/jinja2/quota-suspensions/edit.jinja diff --git a/common/forms.py b/common/forms.py index af31c8386..2da520ca7 100644 --- a/common/forms.py +++ b/common/forms.py @@ -7,6 +7,7 @@ from crispy_forms_gds.fields import DateInputField from crispy_forms_gds.helper import FormHelper +from crispy_forms_gds.layout import HTML from crispy_forms_gds.layout import Div from crispy_forms_gds.layout import Field from crispy_forms_gds.layout import Layout @@ -458,12 +459,18 @@ def __init__(self, *args, **kwargs): self.helper.label_size = Size.SMALL self.helper.legend_size = Size.SMALL self.helper.layout = Layout( - Submit( - "submit", - "Delete", - css_class="govuk-button--warning", - data_module="govuk-button", - data_prevent_double_click="true", + Div( + Submit( + "submit", + "Delete", + css_class="govuk-button--warning", + data_module="govuk-button", + data_prevent_double_click="true", + ), + HTML( + f"Cancel", + ), + css_class="govuk-button-group", ), ) diff --git a/common/jinja2/common/delete.jinja b/common/jinja2/common/delete.jinja index e4baf83fe..cc8761c44 100644 --- a/common/jinja2/common/delete.jinja +++ b/common/jinja2/common/delete.jinja @@ -22,10 +22,4 @@ {% call django_form(action=object.get_url("delete")) %} {{ crispy(form) }} {% endcall %} - - {{ govukButton({ - "text": "Cancel", - "href": object.get_url(), - "classes": "govuk-button--secondary" - }) }} {% endblock %} diff --git a/quotas/forms.py b/quotas/forms.py index 0d30029ad..0550b5240 100644 --- a/quotas/forms.py +++ b/quotas/forms.py @@ -999,6 +999,71 @@ def create_blocking_period(self, workbasket): ) +class QuotaSuspensionUpdateForm(ValidityPeriodForm, forms.ModelForm): + + description = forms.CharField( + label="Description", + widget=forms.Textarea(), + required=False, + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.init_layout() + + def init_layout(self): + cancel_url = reverse_lazy( + "quota-ui-detail", + kwargs={"sid": self.instance.quota_definition.order_number.sid}, + ) + self.helper = FormHelper(self) + self.helper.label_size = Size.SMALL + self.helper.legend_size = Size.SMALL + self.helper.layout = Layout( + "start_date", + "end_date", + Field.textarea("description", rows=5), + Div( + Submit( + "submit", + "Save", + css_class="govuk-button--primary", + data_module="govuk-button", + data_prevent_double_click="true", + ), + HTML( + f"Cancel", + ), + css_class="govuk-button-group", + ), + ) + + def clean(self): + cleaned_data = super().clean() + definition_period = self.instance.quota_definition.valid_between + validity_period = cleaned_data.get("valid_between") + if ( + definition_period + and validity_period + and not validity_range_contains_range(definition_period, validity_period) + ): + raise ValidationError( + f"The start and end date must sit within the selected quota definition's start and end date ({definition_period.lower} - {definition_period.upper})", + ) + + return cleaned_data + + class Meta: + model = models.QuotaSuspension + fields = [ + "valid_between", + "description", + ] + + +QuotaSuspensionDeleteForm = delete_form_for(models.QuotaSuspension) + + class DuplicateQuotaDefinitionPeriodStartForm(forms.Form): pass diff --git a/quotas/jinja2/quota-suspensions/confirm-delete.jinja b/quotas/jinja2/quota-suspensions/confirm-delete.jinja new file mode 100644 index 000000000..4646fea93 --- /dev/null +++ b/quotas/jinja2/quota-suspensions/confirm-delete.jinja @@ -0,0 +1,49 @@ +{% extends "layouts/confirm.jinja" %} +{% from "components/breadcrumbs.jinja" import breadcrumbs %} + +{% set page_title = "Quota suspension deleted" %} + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": "Find and edit quotas", "href": url("quota-ui-list")}, + {"text": "Quota " ~ object.quota_definition.order_number|string, "href": object.quota_definition.order_number.get_url()}, + {"text": "Quota ID: " ~ object.quota_definition.order_number ~ " - Data", "href": url("quota_definition-ui-list", args=[object.quota_definition.order_number.sid])}, + {"text": "Suspension periods", "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"})}, + {"text": page_title} + ]) + }} +{% endblock %} + +{% block panel %} + {{ govukPanel({ + "titleText": "Quota suspension period " ~ object.sid ~ " has been deleted", + "text": "This change has been added to your workbasket", + "classes": "govuk-!-margin-bottom-7" + }) }} +{% endblock %} + +{% block main_button %} +{{ govukButton({ + "text": "View other suspension periods for this quota order number", + "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"}), + "classes": "govuk-button--secondary" +}) }} +{% endblock%} + + +{% block button_group %} + {{ govukButton({ + "text": "View workbasket summary", + "href": url("workbaskets:current-workbasket"), + "classes": "govuk-button" + }) }} + {{ govukButton({ + "text": "View quota order number: " ~ object.quota_definition.order_number|string, + "href": object.quota_definition.order_number.get_url(), + "classes": "govuk-button--secondary" + }) }} +{% endblock %} + +{% block actions %} +
  • Find and edit quotas
  • +{% endblock %} \ No newline at end of file diff --git a/quotas/jinja2/quota-suspensions/confirm-update.jinja b/quotas/jinja2/quota-suspensions/confirm-update.jinja new file mode 100644 index 000000000..83fe99be2 --- /dev/null +++ b/quotas/jinja2/quota-suspensions/confirm-update.jinja @@ -0,0 +1,25 @@ +{% extends "common/confirm_update.jinja" %} +{% from "components/breadcrumbs.jinja" import breadcrumbs %} + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": "Find and edit quotas", "href": url("quota-ui-list")}, + {"text": object.quota_definition.order_number._meta.verbose_name|capitalize ~ ": " ~ object.quota_definition.order_number|string, "href": object.quota_definition.order_number.get_url()}, + {"text": "Quota ID: " ~ object.quota_definition.order_number.order_number ~ " - Data", "href": url("quota_definition-ui-list", args=[object.quota_definition.order_number.sid])}, + {"text": "Suspension periods", "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"})}, + {"text": page_title} + ]) + }} +{% endblock %} + +{% block main_button %} +{{ govukButton({ + "text": "View other suspension periods for this quota order number", + "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"}), + "classes": "govuk-button--secondary" +}) }} +{% endblock%} + +{% block actions %} +
  • Find and edit quotas
  • +{% endblock %} \ No newline at end of file diff --git a/quotas/jinja2/quota-suspensions/delete.jinja b/quotas/jinja2/quota-suspensions/delete.jinja new file mode 100644 index 000000000..35d59c2ff --- /dev/null +++ b/quotas/jinja2/quota-suspensions/delete.jinja @@ -0,0 +1,13 @@ +{% extends "common/delete.jinja" %} + + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": "Find and edit quotas", "href": url("quota-ui-list")}, + {"text": object.quota_definition.order_number._meta.verbose_name|capitalize ~ " " ~ object.quota_definition.order_number|string, "href": object.quota_definition.order_number.get_url()}, + {"text": object.quota_definition.order_number._meta.verbose_name|capitalize ~ " " ~ object.quota_definition.order_number|string ~ ": Data" , "href": url('quota_definition-ui-list', args=[object.quota_definition.order_number.sid])}, + {"text": "Suspension periods", "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"})}, + {"text": page_title} + ]) + }} +{% endblock %} \ No newline at end of file diff --git a/quotas/jinja2/quota-suspensions/edit.jinja b/quotas/jinja2/quota-suspensions/edit.jinja new file mode 100644 index 000000000..83030483d --- /dev/null +++ b/quotas/jinja2/quota-suspensions/edit.jinja @@ -0,0 +1,14 @@ +{% extends "common/edit.jinja" %} + +{% set page_title = "Edit quota suspension details" %} + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": "Find and edit quotas", "href": url("quota-ui-list")}, + {"text": object.quota_definition.order_number._meta.verbose_name|capitalize ~ " " ~ object.quota_definition.order_number|string, "href": object.quota_definition.order_number.get_url()}, + {"text": object.quota_definition.order_number._meta.verbose_name|capitalize ~ " " ~ object.quota_definition.order_number|string ~ ": Data" , "href": url('quota_definition-ui-list', args=[object.quota_definition.order_number.sid])}, + {"text": "Suspension periods", "href": url('quota_definition-ui-list-filter', kwargs={'sid': object.quota_definition.order_number.sid, 'quota_type': "suspension_periods"})}, + {"text": page_title} + ]) + }} +{% endblock %} \ No newline at end of file diff --git a/quotas/jinja2/quotas/tables/suspension_periods.jinja b/quotas/jinja2/quotas/tables/suspension_periods.jinja index 2e2e81924..8d72277f3 100644 --- a/quotas/jinja2/quotas/tables/suspension_periods.jinja +++ b/quotas/jinja2/quotas/tables/suspension_periods.jinja @@ -1,4 +1,3 @@ -

    Suspension periods

    {% set table_rows = [] %} @@ -7,21 +6,27 @@ {% for object in suspension_periods %} {% set definition_link -%} {{ object.quota_definition.sid }} + {% endset %} + {% set actions %} + Edit
    + Delete {% endset %} {{ table_rows.append([ {"text": definition_link }, {"text": "{:%d %b %Y}".format(object.valid_between.lower) }, {"text": "{:%d %b %Y}".format(object.valid_between.upper) if object.valid_between.upper else "-"}, {"text": object.description }, + {"html": actions}, ]) or "" }} {% endfor %} {{ govukTable({ "head": [ - {"text": "Quota definition sid"}, + {"text": "Quota definition SID"}, {"text": "Start date"}, {"text": "End date"}, {"text": "Description", "classes": "govuk-!-width-one-half"}, + {"text": "Actions",}, ], "rows": table_rows }) }} diff --git a/quotas/models.py b/quotas/models.py index 402a6654a..9a6232f33 100644 --- a/quotas/models.py +++ b/quotas/models.py @@ -480,6 +480,24 @@ class QuotaSuspension(TrackedModel, ValidityMixin): business_rules = (business_rules.QSP2, UniqueIdentifyingFields, UpdateValidity) + def get_url(self, action: str = "detail") -> Optional[str]: + """Overrides the parent get_url as there is no detail view for + QuotaSuspensions for it to default to.""" + url = super().get_url(action=action) + if not url: + url = reverse( + "quota_definition-ui-list-filter", + kwargs={ + "sid": self.quota_definition.order_number.sid, + "quota_type": "suspension_periods", + }, + ) + + return url + + def __str__(self): + return f"{self.sid}" + class QuotaBlocking(TrackedModel, ValidityMixin): """Defines a blocking period for a (sub-)quota.""" diff --git a/quotas/tests/test_forms.py b/quotas/tests/test_forms.py index 6a2811b94..5ecf1d449 100644 --- a/quotas/tests/test_forms.py +++ b/quotas/tests/test_forms.py @@ -949,7 +949,7 @@ def test_quota_association_edit_form_valid(): def test_quota_association_edit_form_invalid(): - "Test that the quota association edit form is invalid when data is passed in." + "Test that the quota association edit form is invalid when invalid data is passed in." association = factories.QuotaAssociationFactory.create( coefficient=1.67, sub_quota_relation_type="EQ", @@ -968,3 +968,53 @@ def test_quota_association_edit_form_invalid(): ) assert "Enter a number." in form.errors["coefficient"] assert not form.is_valid() + + +def test_quota_suspension_update_form_valid(date_ranges): + "Test that the quota suspension update form is valid when correct data is passed in" + suspension = factories.QuotaSuspensionFactory.create( + valid_between=date_ranges.normal, + ) + current_validity = suspension.valid_between + data = { + "start_date_0": current_validity.lower.day + 1, + "start_date_1": current_validity.lower.month, + "start_date_2": current_validity.lower.year, + "end_date_0": "", + "end_date_1": "", + "end_date_2": "", + "description": "New description", + } + with override_current_transaction(Transaction.objects.last()): + form = forms.QuotaSuspensionUpdateForm( + data=data, + instance=suspension, + ) + assert form.is_valid() + + +def test_quota_suspension_update_form_invalid(date_ranges): + "Test that the quota suspension update form is invalid when the validity period does not span the validity of the definition" + definition = factories.QuotaDefinitionFactory.create( + valid_between=date_ranges.normal, + ) + suspension = factories.QuotaSuspensionFactory.create( + quota_definition=definition, + valid_between=date_ranges.normal, + ) + data = { + "start_date_0": date_ranges.earlier.lower.day, + "start_date_1": date_ranges.earlier.lower.month, + "start_date_2": date_ranges.earlier.lower.year, + } + + with override_current_transaction(Transaction.objects.last()): + form = forms.QuotaSuspensionUpdateForm( + data=data, + instance=suspension, + ) + assert not form.is_valid() + assert ( + f"The start and end date must sit within the selected quota definition's start and end date ({definition.valid_between.lower} - {definition.valid_between.upper})" + in form.errors["__all__"] + ) diff --git a/quotas/tests/test_views.py b/quotas/tests/test_views.py index ca366c957..70c285a35 100644 --- a/quotas/tests/test_views.py +++ b/quotas/tests/test_views.py @@ -2040,7 +2040,7 @@ def test_quota_definition_view(client_with_current_workbasket): ) assert response.status_code == 200 soup = BeautifulSoup(response.content.decode(response.charset), "html.parser") - description_cell_text = soup.select("tbody tr:first-child td:last-child")[0].text + description_cell_text = soup.select("tbody tr:first-child td")[-2].text assert description_cell_text == suspension.description @@ -2480,3 +2480,80 @@ def test_delete_quota_association(client_with_current_workbasket): h1.text.strip() == f"Quota association between {main_quota.sid} and {sub_quota.sid} has been deleted" ) + + +def test_quota_suspension_edit(client_with_current_workbasket): + """Test the QuotaSuspensionUpdate view including the + QuotaSuspensionUpdateMixin.""" + suspension = factories.QuotaSuspensionFactory.create() + current_validity = suspension.valid_between + data = { + "start_date_0": current_validity.lower.day, + "start_date_1": current_validity.lower.month, + "start_date_2": current_validity.lower.year, + "end_date_0": current_validity.upper.day, + "end_date_1": current_validity.upper.month, + "end_date_2": current_validity.upper.year, + "description": "New description", + } + + url = reverse("quota_suspension-ui-edit", kwargs={"sid": suspension.sid}) + + response = client_with_current_workbasket.post(url, data=data) + assert response.status_code == 302 + assert response.url == reverse( + "quota_suspension-ui-confirm-update", + kwargs={"sid": suspension.sid}, + ) + + updated_suspension = models.QuotaSuspension.objects.approved_up_to_transaction( + Transaction.objects.last(), + ).get(sid=suspension.sid) + assert updated_suspension.description == "New description" + assert updated_suspension.update_type == UpdateType.UPDATE + + confirm_response = client_with_current_workbasket.get(response.url) + + soup = BeautifulSoup( + confirm_response.content.decode(response.charset), + "html.parser", + ) + div = soup.select("div .govuk-panel__body")[0] + + assert f"Quota suspension: {suspension.sid} has been updated" in div.text.strip() + + +def test_quota_suspension_edit_update(client_with_current_workbasket): + """Test that posting the edit update form edits the existing quota + suspension update object rather than creating a new one.""" + suspension = factories.QuotaSuspensionFactory.create() + data = { + "start_date_0": suspension.valid_between.lower.day, + "start_date_1": suspension.valid_between.lower.month, + "start_date_2": suspension.valid_between.lower.year, + "end_date_0": suspension.valid_between.upper.day, + "end_date_1": suspension.valid_between.upper.month, + "end_date_2": suspension.valid_between.upper.year, + "description": "New description", + } + + edit_url = reverse("quota_suspension-ui-edit", kwargs={"sid": suspension.sid}) + client_with_current_workbasket.post(edit_url, data=data) + edit_update_url = reverse( + "quota_suspension-ui-edit-update", + kwargs={"sid": suspension.sid}, + ) + response = client_with_current_workbasket.post(edit_update_url, data=data) + assert response.status_code == 302 + versions = models.QuotaSuspension.objects.all().filter(sid=suspension.sid) + assert len(versions) == 2 + assert versions.first().update_type == UpdateType.CREATE + assert versions.last().update_type == UpdateType.UPDATE + + +@pytest.mark.parametrize( + "factory", + (factories.QuotaSuspensionFactory,), +) +def test_quota_suspension_delete_form(factory, use_delete_form): + use_delete_form(factory()) diff --git a/quotas/urls.py b/quotas/urls.py index bfa967752..899f43134 100644 --- a/quotas/urls.py +++ b/quotas/urls.py @@ -185,6 +185,36 @@ views.QuotaBlockingConfirmCreate.as_view(), name="quota_blocking-ui-confirm-create", ), + path( + f"quotas/suspension-periods//edit/", + views.QuotaSuspensionUpdate.as_view(), + name="quota_suspension-ui-edit", + ), + path( + f"quotas/suspension-periods//edit-create/", + views.QuotaSuspensionEditCreate.as_view(), + name="quota_suspension-ui-edit-create", + ), + path( + f"quotas/suspension-periods//edit-update/", + views.QuotaSuspensionEditUpdate.as_view(), + name="quota_suspension-ui-edit-update", + ), + path( + f"quotas/suspension-periods//confirm-update/", + views.QuotaSuspensionConfirmUpdate.as_view(), + name="quota_suspension-ui-confirm-update", + ), + path( + f"quotas/suspension-periods//delete/", + views.QuotaSuspensionDelete.as_view(), + name="quota_suspension-ui-delete", + ), + path( + f"quotas/suspension-periods//confirm-delete/", + views.QuotaSuspensionConfirmDelete.as_view(), + name="quota_suspension-ui-confirm-delete", + ), path( f"quota_definitions/quota-association//delete/", views.QuotaAssociationDelete.as_view(), diff --git a/quotas/views.py b/quotas/views.py index d373a8a3f..254913c9e 100644 --- a/quotas/views.py +++ b/quotas/views.py @@ -306,7 +306,11 @@ def blocking_periods(self): @property def suspension_periods(self): - return QuotaSuspension.objects.filter(quota_definition__order_number=self.quota) + return ( + QuotaSuspension.objects.current() + .filter(quota_definition__order_number=self.quota) + .order_by("quota_definition__sid") + ) @property def sub_quotas(self): @@ -1167,6 +1171,77 @@ def get_context_data(self, *args, **kwargs): return context +class QuotaSuspensionUpdateMixin(TrackedModelDetailMixin): + model = QuotaSuspension + template_name = "quota-suspensions/edit.jinja" + form_class = forms.QuotaSuspensionUpdateForm + permission_required = ["common.change_trackedmodel"] + + def get_success_url(self): + return reverse( + "quota_suspension-ui-confirm-update", + kwargs={"sid": self.object.sid}, + ) + + +class QuotaSuspensionUpdate( + QuotaSuspensionUpdateMixin, + CreateTaricUpdateView, +): + pass + + +class QuotaSuspensionEditCreate( + QuotaSuspensionUpdateMixin, + EditTaricView, +): + pass + + +class QuotaSuspensionEditUpdate( + QuotaSuspensionUpdateMixin, + EditTaricView, +): + pass + + +class QuotaSuspensionConfirmUpdate(TrackedModelDetailView): + model = models.QuotaSuspension + template_name = "quota-suspensions/confirm-update.jinja" + + +class QuotaSuspensionDelete(TrackedModelDetailMixin, CreateTaricDeleteView): + form_class = forms.QuotaSuspensionDeleteForm + model = models.QuotaSuspension + template_name = "quota-suspensions/delete.jinja" + + def get_success_url(self): + return reverse( + "quota_suspension-ui-confirm-delete", + kwargs={"sid": self.object.sid}, + ) + + +class QuotaSuspensionConfirmDelete(TrackedModelDetailView): + model = QuotaSuspension + template_name = "quota-suspensions/confirm-delete.jinja" + + @property + def deleted_suspension(self): + return QuotaSuspension.objects.filter(sid=self.kwargs["sid"]).last() + + def get_queryset(self): + """ + Returns a queryset with one single version of the suspension in + question. + + Done this way so the sid can be rendered on the confirm delete page and + generic tests don't fail which try to load the page without having + deleted anything. + """ + return QuotaSuspension.objects.filter(pk=self.deleted_suspension) + + class SubQuotaDefinitionAssociationMixin: template_name = "quota-definitions/sub-quota-definitions-updates.jinja" form_class = forms.SubQuotaDefinitionAssociationUpdateForm From 4d5fc904b5e76d1de3703874b66d669ccc18cca4 Mon Sep 17 00:00:00 2001 From: Matthew McKenzie <97194636+mattjamc@users.noreply.github.com> Date: Mon, 21 Oct 2024 13:50:16 +0100 Subject: [PATCH 11/12] TP2000-1477 Edit capability for suspension periods pt2 (#1309) * Ability to edit suspension periods * Add ability to delete a suspension period * Form tests * View tests * Fix failing tests * Fix and reorder cancel button * Redesign confirm pages * PR review comments * test changes following PR review * Update get_success_url to make kwarg clearer * Get rid of +1 to prevent flaky test --- quotas/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quotas/tests/test_forms.py b/quotas/tests/test_forms.py index 5ecf1d449..5f56e0e47 100644 --- a/quotas/tests/test_forms.py +++ b/quotas/tests/test_forms.py @@ -977,7 +977,7 @@ def test_quota_suspension_update_form_valid(date_ranges): ) current_validity = suspension.valid_between data = { - "start_date_0": current_validity.lower.day + 1, + "start_date_0": current_validity.lower.day, "start_date_1": current_validity.lower.month, "start_date_2": current_validity.lower.year, "end_date_0": "", From 40ab16a821dfe5963b15ba87904a43c57c22e65e Mon Sep 17 00:00:00 2001 From: Lawrence <34475808+acodeninja@users.noreply.github.com> Date: Thu, 24 Oct 2024 20:11:07 +0100 Subject: [PATCH 12/12] fix: run collect static at build time (#1311) --- .copilot/image_build_run.sh | 1 + .profile | 20 -------------------- scripts/web-worker-entrypoint.sh | 2 ++ 3 files changed, 3 insertions(+), 20 deletions(-) delete mode 100755 .profile diff --git a/.copilot/image_build_run.sh b/.copilot/image_build_run.sh index 7a42ecf3b..a8eaefa02 100755 --- a/.copilot/image_build_run.sh +++ b/.copilot/image_build_run.sh @@ -3,3 +3,4 @@ set -e # Add commands below to run inside the container after all the other buildpacks have been applied +python manage.py collectstatic --noinput --clear diff --git a/.profile b/.profile deleted file mode 100755 index ee12fb823..000000000 --- a/.profile +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bash -# custom initialisation tasks -# ref - https://docs.cloudfoundry.org/devguide/deploy-apps/deploy-app.html - -echo "---- RUNNING release tasks (.profile) ------" - -if [[ "$MAINTENANCE_MODE" != "True" && "$MAINTENANCE_MODE" != "true" ]] ; then - echo "---- Apply Migrations ------" - python manage.py migrate -else - echo "---- Skip Applying Migrations (Maintenance Mode) ------" -fi - -echo "---- Collect Static Files ------" -OUTPUT=$(python manage.py collectstatic --noinput --clear) -mkdir -p ~/logs -echo ${OUTPUT} > ~/logs/collectstatic.txt -echo ${OUTPUT##*$'\n'} -echo "NOTE: the full output of collectstatic has been saved to ~/logs/collectstatic.txt" -echo "---- FINISHED release tasks (.profile) ------" diff --git a/scripts/web-worker-entrypoint.sh b/scripts/web-worker-entrypoint.sh index 6a7c0882d..23ec23b2c 100755 --- a/scripts/web-worker-entrypoint.sh +++ b/scripts/web-worker-entrypoint.sh @@ -1,5 +1,7 @@ #!/bin/sh -e +python manage.py migrate + WORKER_COMMAND="gunicorn wsgi --bind 0.0.0.0:${PORT} --timeout 1000 --worker-class=gevent --worker-connections=1000 --workers 9" echo "Starting worker using the command: ${WORKER_COMMAND}"