From 130927ebddee975d0bde8cbb44917dc8ca18ffca Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 25 Oct 2022 14:27:16 +0100 Subject: [PATCH 01/13] Add start form and conditions --- measures/conditions.py | 53 ++++++++++++++++++++++++++++++++++++++++++ measures/constants.py | 12 ++++++++++ measures/forms.py | 51 ++++++++++++++++++++++++++++++++-------- pii-ner-exclude.txt | 1 + 4 files changed, 107 insertions(+), 10 deletions(-) create mode 100644 measures/conditions.py create mode 100644 measures/constants.py diff --git a/measures/conditions.py b/measures/conditions.py new file mode 100644 index 000000000..9566bd0f0 --- /dev/null +++ b/measures/conditions.py @@ -0,0 +1,53 @@ +from measures import constants + + +def show_step_measure_details(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.MEASURE_DETAILS in cleaned_data.get("fields_to_edit") + + +def show_step_regulation_id(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.REGULATION_ID in cleaned_data.get("fields_to_edit") + + +def show_step_quota_order_number(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.QUOTA_ORDER_NUMBER in cleaned_data.get("fields_to_edit") + + +def show_step_geographical_area(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.GEOGRAPHICAL_AREA in cleaned_data.get("fields_to_edit") + + +def show_step_commodities(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.COMMODITIES in cleaned_data.get("fields_to_edit") + + +def show_step_additional_code(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.ADDITIONAL_CODE in cleaned_data.get("fields_to_edit") + + +def show_step_conditions(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.CONDITIONS in cleaned_data.get("fields_to_edit") + + +def show_step_footnotes(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + return constants.FOOTNOTES in cleaned_data.get("fields_to_edit") + + +measure_edit_condition_dict = { + constants.MEASURE_DETAILS: show_step_measure_details, + constants.REGULATION_ID: show_step_regulation_id, + constants.QUOTA_ORDER_NUMBER: show_step_quota_order_number, + constants.GEOGRAPHICAL_AREA: show_step_geographical_area, + constants.COMMODITIES: show_step_commodities, + constants.ADDITIONAL_CODE: show_step_additional_code, + constants.CONDITIONS: show_step_conditions, + constants.FOOTNOTES: show_step_footnotes, +} diff --git a/measures/constants.py b/measures/constants.py new file mode 100644 index 000000000..0434328f3 --- /dev/null +++ b/measures/constants.py @@ -0,0 +1,12 @@ +START = "start" +VALIDITY_DATES = "validity_dates" +MEASURE_DETAILS = "measure_details" +REGULATION_ID = "regulation_id" +QUOTA_ORDER_NUMBER = "quota_order_number" +GEOGRAPHICAL_AREA = "geographical_area" +COMMODITIES = "commodities" +ADDITIONAL_CODE = "additional_code" +CONDITIONS = "conditions" +FOOTNOTES = "footnotes" +SUMMARY = "summary" +COMPLETE = "complete" diff --git a/measures/forms.py b/measures/forms.py index 03a95b570..20f64e211 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -30,6 +30,7 @@ from footnotes.models import Footnote from geo_areas.models import GeographicalArea from geo_areas.validators import AreaCode +from measures import constants from measures import models from measures.parsers import DutySentenceParser from measures.patterns import MeasureCreationPattern @@ -1041,16 +1042,9 @@ def clean(self): return cleaned_data -MeasureCommodityAndDutiesFormSet = formset_factory( - MeasureCommodityAndDutiesForm, - prefix="measure_commodities_duties_formset", - formset=FormSet, - min_num=1, - max_num=100, - extra=1, - validate_min=True, - validate_max=True, -) +class MeasureCommodityAndDutiesFormSet(FormSet): + form = MeasureCommodityAndDutiesForm + prefix = "measure_commodities_duties_formset" class MeasureFootnotesForm(forms.Form): @@ -1110,3 +1104,40 @@ class MeasureReviewForm(forms.Form): MeasureDeleteForm = delete_form_for(models.Measure) + + +class MeasuresEditStartForm(forms.Form): + choices = [ + (constants.MEASURE_DETAILS, "Validity dates and measure type"), + (constants.REGULATION_ID, "Regulation"), + (constants.QUOTA_ORDER_NUMBER, "Quotas"), + (constants.GEOGRAPHICAL_AREA, "Geographies"), + (constants.COMMODITIES, "Commodity code"), + (constants.ADDITIONAL_CODE, "Additional codes"), + (constants.CONDITIONS, "Conditions"), + (constants.FOOTNOTES, "Footnotes"), + ] + fields_to_edit = forms.MultipleChoiceField( + choices=choices, + widget=forms.CheckboxSelectMultiple, + label="", + help_text="Select all that apply", + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.helper = FormHelper(self) + self.helper.form_tag = False + self.helper.legend_size = Size.SMALL + self.helper.layout = Layout( + Fieldset( + "fields_to_edit", + ), + Submit( + "submit", + "Continue", + data_module="govuk-button", + data_prevent_double_click="true", + ), + ) diff --git a/pii-ner-exclude.txt b/pii-ner-exclude.txt index 51036bf68..c547f2a4b 100644 --- a/pii-ner-exclude.txt +++ b/pii-ner-exclude.txt @@ -21,6 +21,7 @@ AdditionalCode MeasurementUnit rotate(180deg model&d +Geographies sort_by(sort_by MonetaryUnit check_name&d From 10a944336f96dcd2f8084b95e92a94f415665a94 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 1 Nov 2022 15:40:32 +0000 Subject: [PATCH 02/13] Move SessionStore to common --- {workbaskets => common}/session_store.py | 0 workbaskets/views/ui.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename {workbaskets => common}/session_store.py (100%) diff --git a/workbaskets/session_store.py b/common/session_store.py similarity index 100% rename from workbaskets/session_store.py rename to common/session_store.py diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 6e8a6b770..e3593acdd 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -30,6 +30,7 @@ from common.filters import TamatoFilter from common.models import TrackedModel from common.pagination import build_pagination_list +from common.session_store import SessionStore from common.views import TamatoListView from common.views import WithPaginationListView from exporter.models import Upload @@ -39,7 +40,6 @@ from workbaskets import forms from workbaskets import tasks from workbaskets.models import WorkBasket -from workbaskets.session_store import SessionStore from workbaskets.tasks import call_check_workbasket_sync from workbaskets.validators import WorkflowStatus from workbaskets.views.decorators import require_current_workbasket From e03e4b69720ea3293717cc78b402d1dbc71de54a Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 1 Nov 2022 15:44:29 +0000 Subject: [PATCH 03/13] Separate duties step, make conditions not break on first step --- measures/conditions.py | 31 +++++++++++++++++++++++-------- measures/constants.py | 1 + 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/measures/conditions.py b/measures/conditions.py index 9566bd0f0..124dbb4a7 100644 --- a/measures/conditions.py +++ b/measures/conditions.py @@ -3,42 +3,56 @@ def show_step_measure_details(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.MEASURE_DETAILS in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.MEASURE_DETAILS in cleaned_data.get("fields_to_edit") def show_step_regulation_id(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.REGULATION_ID in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.REGULATION_ID in cleaned_data.get("fields_to_edit") def show_step_quota_order_number(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.QUOTA_ORDER_NUMBER in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.QUOTA_ORDER_NUMBER in cleaned_data.get("fields_to_edit") def show_step_geographical_area(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.GEOGRAPHICAL_AREA in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.GEOGRAPHICAL_AREA in cleaned_data.get("fields_to_edit") def show_step_commodities(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.COMMODITIES in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.COMMODITIES in cleaned_data.get("fields_to_edit") + + +def show_step_duties(wizard): + cleaned_data = wizard.get_cleaned_data_for_step(constants.START) + if cleaned_data: + return constants.DUTIES in cleaned_data.get("fields_to_edit") def show_step_additional_code(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.ADDITIONAL_CODE in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.ADDITIONAL_CODE in cleaned_data.get("fields_to_edit") def show_step_conditions(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.CONDITIONS in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.CONDITIONS in cleaned_data.get("fields_to_edit") def show_step_footnotes(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - return constants.FOOTNOTES in cleaned_data.get("fields_to_edit") + if cleaned_data: + return constants.FOOTNOTES in cleaned_data.get("fields_to_edit") measure_edit_condition_dict = { @@ -47,6 +61,7 @@ def show_step_footnotes(wizard): constants.QUOTA_ORDER_NUMBER: show_step_quota_order_number, constants.GEOGRAPHICAL_AREA: show_step_geographical_area, constants.COMMODITIES: show_step_commodities, + constants.DUTIES: show_step_duties, constants.ADDITIONAL_CODE: show_step_additional_code, constants.CONDITIONS: show_step_conditions, constants.FOOTNOTES: show_step_footnotes, diff --git a/measures/constants.py b/measures/constants.py index 0434328f3..623bbf78e 100644 --- a/measures/constants.py +++ b/measures/constants.py @@ -5,6 +5,7 @@ QUOTA_ORDER_NUMBER = "quota_order_number" GEOGRAPHICAL_AREA = "geographical_area" COMMODITIES = "commodities" +DUTIES = "duties" ADDITIONAL_CODE = "additional_code" CONDITIONS = "conditions" FOOTNOTES = "footnotes" From 11f3031c5869dbbaf3c7828217be4300ea5e55af Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 2 Nov 2022 11:46:43 +0000 Subject: [PATCH 04/13] Modify conditions form for multiple edits --- measures/forms.py | 120 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/measures/forms.py b/measures/forms.py index 20f64e211..b45b74b3a 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -443,10 +443,79 @@ def clean(self): return self.conditions_clean(cleaned_data, self.measure_start_date) +class MeasureConditionsEditWizardStepForm(MeasureConditionsFormMixin): + + # override methods that use form kwargs + def __init__(self, *args, **kwargs): + self.measure_start_dates = kwargs.pop("measure_start_dates") + super().__init__(*args, **kwargs) + + def clean_applicable_duty(self): + """ + Gets applicable_duty from cleaned data. + + We expect `measure_start_dates` to be passed in. Uses + `DutySentenceParser` to check that applicable_duty is a valid duty + string. + """ + applicable_duty = self.cleaned_data["applicable_duty"] + + if applicable_duty and self.measure_start_dates is not None: + for d in self.measure_start_dates: + validate_duties(applicable_duty, d) + + return applicable_duty + + def clean(self): + """ + We get the reference_price from cleaned_data and the measure_start_dates + from form kwargs. + + If reference_price is provided, we use DutySentenceParser with + measure_start_dates to check that we are dealing with a simple duty + (i.e. only one component). We then update cleaned_data with key-value + pairs created from this single, unsaved component. + """ + cleaned_data = super().clean() + + return self.conditions_clean(cleaned_data, self.measure_start_dates) + + def conditions_clean(self, cleaned_data, measure_start_dates): + price = cleaned_data.get("reference_price") + + if price and measure_start_dates is not None: + for start_date in self.measure_start_dates: + validate_duties(price, start_date) + + if price: + for start_date in self.measure_start_dates: + parser = DutySentenceParser.get(start_date) + components = parser.parse(price) + if len(components) > 1: + raise ValidationError( + "A MeasureCondition cannot be created with a compound reference price (e.g. 3.5% + 11 GBP / 100 kg)", + ) + cleaned_data["duty_amount"] = components[0].duty_amount + cleaned_data["monetary_unit"] = components[0].monetary_unit + cleaned_data["condition_measurement"] = components[0].component_measurement + + # The JS autocomplete does not allow for clearing unnecessary certificates + # In case of a user changing data, the information is cleared here. + condition_code = cleaned_data.get("condition_code") + if condition_code and not condition_code.accepts_certificate: + cleaned_data["required_certificate"] = None + + return cleaned_data + + class MeasureConditionsWizardStepFormSet(FormSet): form = MeasureConditionsWizardStepForm +class MeasureConditionsEditWizardStepFormSet(FormSet): + form = MeasureConditionsEditWizardStepForm + + class MeasureForm(ValidityPeriodForm, BindNestedFormMixin, forms.ModelForm): class Meta: model = models.Measure @@ -1042,6 +1111,56 @@ def clean(self): return cleaned_data +class MeasureDutiesForm(forms.Form): + duties = forms.CharField( + label="Duties", + help_text="The duty expression should be valid for the validity periods of all the measures you wish to edit", + ) + + def __init__(self, *args, **kwargs): + # remove measure_start_date from kwargs here because superclass will not be expecting it + self.measure_start_dates = kwargs.pop("measure_start_dates") + super().__init__(*args, **kwargs) + + self.helper = FormHelper(self) + self.helper.form_tag = False + self.helper.label_size = Size.SMALL + self.helper.layout = Layout( + Fieldset( + "duties", + HTML( + loader.render_to_string( + "components/duty_help.jinja", + context={"component": "measure"}, + ), + ), + ), + Submit( + "submit", + "Continue", + data_module="govuk-button", + data_prevent_double_click="true", + ), + ) + + def clean(self): + cleaned_data = super().clean() + duties = cleaned_data.get("duties", "") + for d in self.measure_start_dates: + validate_duties(duties, d) + + return cleaned_data + + +class MeasureCommodityForm(forms.Form): + commodity = AutoCompleteField( + label="Commodity code", + help_text="Select the 10-digit commodity code to which the measure applies.", + queryset=GoodsNomenclature.objects.all(), + attrs={"min_length": 3}, + ) + + class MeasureCommodityAndDutiesFormSet(FormSet): form = MeasureCommodityAndDutiesForm prefix = "measure_commodities_duties_formset" @@ -1113,6 +1232,7 @@ class MeasuresEditStartForm(forms.Form): (constants.QUOTA_ORDER_NUMBER, "Quotas"), (constants.GEOGRAPHICAL_AREA, "Geographies"), (constants.COMMODITIES, "Commodity code"), + (constants.DUTIES, "Duties"), (constants.ADDITIONAL_CODE, "Additional codes"), (constants.CONDITIONS, "Conditions"), (constants.FOOTNOTES, "Footnotes"), From fb14b637c0541858492ff769d9b0504a584fbdbf Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 2 Nov 2022 11:47:29 +0000 Subject: [PATCH 05/13] Add form wizard view and urls --- measures/urls.py | 19 +++ measures/views.py | 357 +++++++++++++++++++++++++++++++++----------- pii-ner-exclude.txt | 1 + 3 files changed, 288 insertions(+), 89 deletions(-) diff --git a/measures/urls.py b/measures/urls.py index 65333b9e1..a05dd9bbc 100644 --- a/measures/urls.py +++ b/measures/urls.py @@ -4,6 +4,7 @@ from common.paths import get_ui_paths from measures import views +from measures.conditions import measure_edit_condition_dict api_router = routers.DefaultRouter() api_router.register(r"measure_types", views.MeasureTypeViewSet, basename="measuretype") @@ -27,6 +28,24 @@ ), name="measure-ui-create", ), + path( + "edit/", + views.MeasuresEditWizard.as_view( + url_name="measure-ui-edit-multiple", + done_step_name="complete", + condition_dict=measure_edit_condition_dict, + ), + name="measure-ui-edit-multiple", + ), + path( + "edit//", + views.MeasuresEditWizard.as_view( + url_name="measure-ui-edit-multiple", + done_step_name="complete", + condition_dict=measure_edit_condition_dict, + ), + name="measure-ui-edit-multiple", + ), path( f"{detail}/edit-footnotes/", views.MeasureFootnotesUpdate.as_view(), diff --git a/measures/views.py b/measures/views.py index 499d0bb4a..6006332e7 100644 --- a/measures/views.py +++ b/measures/views.py @@ -4,6 +4,7 @@ from typing import Type from crispy_forms.helper import FormHelper +from django.contrib.auth.mixins import PermissionRequiredMixin from django.db.transaction import atomic from django.http import HttpRequest from django.http import HttpResponse @@ -11,16 +12,19 @@ from django.shortcuts import render from django.utils.decorators import method_decorator from django.views import View +from django.views.generic.edit import FormView from formtools.wizard.views import NamedUrlSessionWizardView from rest_framework import viewsets from rest_framework.reverse import reverse from common.models import TrackedModel from common.serializers import AutoCompleteSerializer +from common.session_store import SessionStore from common.validators import UpdateType from common.views import TamatoListView from common.views import TrackedModelDetailMixin from common.views import TrackedModelDetailView +from measures import constants from measures import forms from measures.filters import MeasureFilter from measures.filters import MeasureTypeFilterBackend @@ -31,10 +35,59 @@ from measures.pagination import MeasurePaginator from measures.parsers import DutySentenceParser from measures.patterns import MeasureCreationPattern +from workbaskets import forms as workbasket_forms from workbaskets.models import WorkBasket from workbaskets.views.decorators import require_current_workbasket from workbaskets.views.generic import DraftDeleteView from workbaskets.views.generic import DraftUpdateView +from workbaskets.views.mixins import WithCurrentWorkBasket + +STEP_METADATA = { + constants.MEASURE_DETAILS: { + "title": "Enter the basic data", + "link_text": "Measure details", + }, + constants.REGULATION_ID: { + "title": "Enter the Regulation ID", + "link_text": "Regulation ID", + }, + constants.QUOTA_ORDER_NUMBER: { + "title": "Enter the Quota Order Number", + "link_text": "Quota Order Number", + }, + constants.GEOGRAPHICAL_AREA: { + "title": "Select the geographical area", + "link_text": "Geographical areas", + "info": "The measure will only apply to imports from or exports to the selected area. You can specify exclusions.", + }, + constants.COMMODITIES: { + "title": "Select a commodity", + "link_text": "Commodity", + }, + constants.DUTIES: { + "title": "Enter the duties", + "link_text": "Duties", + }, + constants.ADDITIONAL_CODE: { + "title": "Assign an additional code", + "link_text": "Additional code", + }, + constants.CONDITIONS: { + "title": "Add any condition codes", + "info": ( + "This section is optional. If there are no condition " + "codes, select continue." + ), + "link_text": "Conditions", + }, + constants.FOOTNOTES: { + "title": "Add any footnotes", + "info": ( + "This section is optional. If there are no footnotes, " "select continue." + ), + "link_text": "Footnotes", + }, +} class MeasureTypeViewSet(viewsets.ReadOnlyModelViewSet): @@ -59,12 +112,182 @@ def get_queryset(self): return Measure.objects.approved_up_to_transaction(tx) -class MeasureList(MeasureMixin, TamatoListView): +class MeasureList(MeasureMixin, FormView, TamatoListView): """UI endpoint for viewing and filtering Measures.""" - paginator_class = MeasurePaginator template_name = "measures/list.jinja" filterset_class = MeasureFilter + form_class = workbasket_forms.SelectableObjectsForm + update_type = UpdateType.UPDATE + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + page = self.paginator.get_page(self.request.GET.get("page", 1)) + kwargs["objects"] = page.object_list + return kwargs + + @property + def paginator(self): + filterset_class = self.get_filterset_class() + self.filterset = self.get_filterset(filterset_class) + return MeasurePaginator(self.filterset.qs, per_page=10) + + def form_valid(self, form): + store = SessionStore( + self.request, + "MEASURE_SELECTIONS", + ) + # clear the store here before adding items + # in case there was a previous form in progress that was abandoned + store.clear() + object_pks = {k: True for k, v in form.cleaned_data_no_prefix.items() if v} + store.add_items(object_pks) + url = reverse("measure-ui-edit-multiple", kwargs={"step": "start"}) + return HttpResponseRedirect(url) + + +class MeasuresEditWizard( + WithCurrentWorkBasket, + PermissionRequiredMixin, + NamedUrlSessionWizardView, +): + + update_type = UpdateType.UPDATE + permission_required = "common.add_trackedmodel" + + step_metadata = { + constants.START: { + "title": "Select the elements you want to edit", + "link_text": "Start", + }, + constants.MEASURE_DETAILS: STEP_METADATA[constants.MEASURE_DETAILS], + constants.REGULATION_ID: STEP_METADATA[constants.REGULATION_ID], + constants.QUOTA_ORDER_NUMBER: STEP_METADATA[constants.QUOTA_ORDER_NUMBER], + constants.GEOGRAPHICAL_AREA: STEP_METADATA[constants.GEOGRAPHICAL_AREA], + constants.COMMODITIES: STEP_METADATA[constants.COMMODITIES], + constants.DUTIES: STEP_METADATA[constants.DUTIES], + constants.ADDITIONAL_CODE: STEP_METADATA[constants.ADDITIONAL_CODE], + constants.CONDITIONS: STEP_METADATA[constants.CONDITIONS], + constants.FOOTNOTES: STEP_METADATA[constants.FOOTNOTES], + constants.SUMMARY: { + "title": "Review your changes", + "link_text": "Summary", + }, + constants.COMPLETE: { + "title": "Finished", + "link_text": "Success", + }, + } + + form_list = [ + (constants.START, forms.MeasuresEditStartForm), + (constants.MEASURE_DETAILS, forms.MeasureDetailsForm), + (constants.REGULATION_ID, forms.MeasureRegulationIdForm), + (constants.QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), + (constants.GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), + (constants.COMMODITIES, forms.MeasureCommodityForm), + (constants.DUTIES, forms.MeasureDutiesForm), + (constants.ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), + (constants.CONDITIONS, forms.MeasureConditionsEditWizardStepFormSet), + (constants.FOOTNOTES, forms.MeasureFootnotesFormSet), + (constants.SUMMARY, forms.MeasureReviewForm), + ] + + templates = { + constants.START: "measures/edit-multiple-start.jinja", + constants.MEASURE_DETAILS: "measures/edit-wizard-step.jinja", + constants.REGULATION_ID: "measures/edit-wizard-step.jinja", + constants.QUOTA_ORDER_NUMBER: "measures/edit-wizard-step.jinja", + constants.GEOGRAPHICAL_AREA: "measures/edit-wizard-step.jinja", + constants.COMMODITIES: "measures/edit-wizard-step.jinja", + constants.DUTIES: "measures/edit-wizard-step.jinja", + constants.ADDITIONAL_CODE: "measures/edit-wizard-step.jinja", + constants.CONDITIONS: "measures/create-formset.jinja", + constants.FOOTNOTES: "measures/create-formset.jinja", + constants.SUMMARY: "measures/edit-review.jinja", + constants.COMPLETE: "measures/confirm-edit-multiple.jinja", + } + + def get_template_names(self): + return self.templates.get( + self.steps.current, + "measures/measure-wizard-step.jinja", + ) + + @property + def _session_store(self): + """Get the current user's SessionStore containing ids of the measures + that have been selected for editing.""" + + return SessionStore( + self.request, + "MEASURE_SELECTIONS", + ) + + @property + def measures(self): + return Measure.objects.filter(pk__in=self._session_store.data.keys()) + + def get_initial(self): + store = SessionStore( + self.request, + "MEASURE_SELECTIONS", + ) + return store.data.copy() + + def get_form_kwargs(self, step): + kwargs = {} + if step == constants.DUTIES: + # duty sentence validation requires the measure start date so pass it to form kwargs here + measure_start_dates = [ + measure.valid_between.lower for measure in self.measures + ] + # commodities/duties step is a formset which expects form_kwargs to pass kwargs to its child forms + kwargs["measure_start_dates"] = measure_start_dates + elif step == constants.CONDITIONS: + # duty sentence validation requires the measure start date so pass it to form kwargs here + measure_start_dates = [ + measure.valid_between.lower for measure in self.measures + ] + # commodities/duties step is a formset which expects form_kwargs to pass kwargs to its child forms + kwargs["form_kwargs"] = {"measure_start_dates": measure_start_dates} + + return kwargs + + def get_context_data(self, form, **kwargs): + context = super().get_context_data(form=form, **kwargs) + context["step_metadata"] = self.step_metadata + if form: + context["form"].is_bound = False + context["no_form_tags"] = FormHelper() + context["no_form_tags"].form_tag = False + context["measures"] = self.measures + return context + + def done(self, form_list, **kwargs): + cleaned_data = self.get_all_cleaned_data() + + model_fields = [f.name for f in Measure._meta.get_fields()] + form_changed_data = [f for f in cleaned_data if f in model_fields] + changed_data = {name: cleaned_data[name] for name in form_changed_data} + + edited_measures = [] + for measure in self.measures: + + measure.new_version( + workbasket=self.workbasket, + update_type=self.update_type, + **changed_data, + ) + edited_measures.append(measure) + + context = self.get_context_data( + form=None, + edited_measures=edited_measures, + **kwargs, + ) + + return render(self.request, "measures/confirm-edit-multiple.jinja", context) class MeasureDetail(MeasureMixin, TrackedModelDetailView): @@ -98,102 +321,56 @@ class MeasureCreateWizard( NamedUrlSessionWizardView, ): storage_name = "measures.wizard.MeasureCreateSessionStorage" - - START = "start" - MEASURE_DETAILS = "measure_details" - REGULATION_ID = "regulation_id" - QUOTA_ORDER_NUMBER = "quota_order_number" - GEOGRAPHICAL_AREA = "geographical_area" - COMMODITIES = "commodities" - ADDITIONAL_CODE = "additional_code" - CONDITIONS = "conditions" - FOOTNOTES = "footnotes" - SUMMARY = "summary" - COMPLETE = "complete" - - form_list = [ - (START, forms.MeasureCreateStartForm), - (MEASURE_DETAILS, forms.MeasureDetailsForm), - (REGULATION_ID, forms.MeasureRegulationIdForm), - (QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), - (GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), - (COMMODITIES, forms.MeasureCommodityAndDutiesFormSet), - (ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), - (CONDITIONS, forms.MeasureConditionsWizardStepFormSet), - (FOOTNOTES, forms.MeasureFootnotesFormSet), - (SUMMARY, forms.MeasureReviewForm), - ] - - templates = { - START: "measures/create-start.jinja", - MEASURE_DETAILS: "measures/create-wizard-step.jinja", - REGULATION_ID: "measures/create-wizard-step.jinja", - QUOTA_ORDER_NUMBER: "measures/create-wizard-step.jinja", - GEOGRAPHICAL_AREA: "measures/create-wizard-step.jinja", - COMMODITIES: "measures/create-formset.jinja", - ADDITIONAL_CODE: "measures/create-wizard-step.jinja", - CONDITIONS: "measures/create-formset.jinja", - FOOTNOTES: "measures/create-formset.jinja", - SUMMARY: "measures/create-review.jinja", - COMPLETE: "measures/confirm-create-multiple.jinja", - } - step_metadata = { - START: { + constants.START: { "title": "Create a new measure", "link_text": "Start", }, - MEASURE_DETAILS: { - "title": "Enter the basic data", - "link_text": "Measure details", - }, - REGULATION_ID: { - "title": "Enter the Regulation ID", - "link_text": "Regulation ID", - }, - QUOTA_ORDER_NUMBER: { - "title": "Enter the Quota Order Number", - "link_text": "Quota Order Number", - }, - GEOGRAPHICAL_AREA: { - "title": "Select the geographical area", - "link_text": "Geographical areas", - "info": "The measure will only apply to imports from or exports to the selected area. You can specify exclusions.", - }, - COMMODITIES: { - "title": "Select commodities and enter the duties", - "link_text": "Commodities and duties", - }, - ADDITIONAL_CODE: { - "title": "Assign an additional code", - "link_text": "Additional code", - }, - CONDITIONS: { - "title": "Add any condition codes", - "info": ( - "This section is optional. If there are no condition " - "codes, select continue." - ), - "link_text": "Conditions", - }, - FOOTNOTES: { - "title": "Add any footnotes", - "info": ( - "This section is optional. If there are no footnotes, " - "select continue." - ), - "link_text": "Footnotes", - }, - SUMMARY: { + constants.MEASURE_DETAILS: STEP_METADATA[constants.MEASURE_DETAILS], + constants.REGULATION_ID: STEP_METADATA[constants.REGULATION_ID], + constants.QUOTA_ORDER_NUMBER: STEP_METADATA[constants.QUOTA_ORDER_NUMBER], + constants.GEOGRAPHICAL_AREA: STEP_METADATA[constants.GEOGRAPHICAL_AREA], + constants.COMMODITIES: STEP_METADATA[constants.COMMODITIES], + constants.ADDITIONAL_CODE: STEP_METADATA[constants.ADDITIONAL_CODE], + constants.CONDITIONS: STEP_METADATA[constants.CONDITIONS], + constants.FOOTNOTES: STEP_METADATA[constants.FOOTNOTES], + constants.SUMMARY: { "title": "Review your measure", "link_text": "Summary", }, - COMPLETE: { + constants.COMPLETE: { "title": "Finished", "link_text": "Success", }, } + form_list = [ + (constants.START, forms.MeasureCreateStartForm), + (constants.MEASURE_DETAILS, forms.MeasureDetailsForm), + (constants.REGULATION_ID, forms.MeasureRegulationIdForm), + (constants.QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), + (constants.GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), + (constants.COMMODITIES, forms.MeasureCommodityAndDutiesFormSet), + (constants.ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), + (constants.CONDITIONS, forms.MeasureConditionsWizardStepFormSet), + (constants.FOOTNOTES, forms.MeasureFootnotesFormSet), + (constants.SUMMARY, forms.MeasureReviewForm), + ] + + templates = { + constants.START: "measures/create-start.jinja", + constants.MEASURE_DETAILS: "measures/create-wizard-step.jinja", + constants.REGULATION_ID: "measures/create-wizard-step.jinja", + constants.QUOTA_ORDER_NUMBER: "measures/create-wizard-step.jinja", + constants.GEOGRAPHICAL_AREA: "measures/create-wizard-step.jinja", + constants.COMMODITIES: "measures/create-formset.jinja", + constants.ADDITIONAL_CODE: "measures/create-wizard-step.jinja", + constants.CONDITIONS: "measures/create-formset.jinja", + constants.FOOTNOTES: "measures/create-formset.jinja", + constants.SUMMARY: "measures/create-review.jinja", + constants.COMPLETE: "measures/confirm-create-multiple.jinja", + } + @atomic def create_measures(self, data): """Returns a list of the created measures.""" @@ -284,9 +461,11 @@ def get_context_data(self, form, **kwargs): def get_form_kwargs(self, step): kwargs = {} - if step == self.COMMODITIES or step == self.CONDITIONS: + if step == constants.COMMODITIES or step == constants.CONDITIONS: # duty sentence validation requires the measure start date so pass it to form kwargs here - valid_between = self.get_cleaned_data_for_step(self.MEASURE_DETAILS).get( + valid_between = self.get_cleaned_data_for_step( + constants.MEASURE_DETAILS, + ).get( "valid_between", ) # commodities/duties step is a formset which expects form_kwargs to pass kwargs to its child forms @@ -315,7 +494,7 @@ def get_form(self, step=None, data=None, files=None): def get_template_names(self): return self.templates.get( self.steps.current, - "measures/create-wizard-step.jinja", + "measures/measure-wizard-step.jinja", ) diff --git a/pii-ner-exclude.txt b/pii-ner-exclude.txt index c547f2a4b..64ebd3abf 100644 --- a/pii-ner-exclude.txt +++ b/pii-ner-exclude.txt @@ -16,6 +16,7 @@ VersionGroup f"https://legislation.gov.uk/uksi/2021/{n UI FIXME +SessionStore Measure SID AdditionalCode MeasurementUnit From ad66ed2cc449f373ca860e0cb8e00a20c3c0aff0 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 2 Nov 2022 11:49:15 +0000 Subject: [PATCH 06/13] Update templates --- measures/jinja2/includes/measures/list.jinja | 83 ++++++--- .../measures/confirm-edit-multiple.jinja | 55 ++++++ .../jinja2/measures/create-wizard-step.jinja | 1 - .../jinja2/measures/edit-multiple-start.jinja | 25 +++ measures/jinja2/measures/edit-review.jinja | 164 ++++++++++++++++++ .../jinja2/measures/edit-wizard-step.jinja | 23 +++ pii-ner-exclude.txt | 1 + 7 files changed, 330 insertions(+), 22 deletions(-) create mode 100644 measures/jinja2/measures/confirm-edit-multiple.jinja create mode 100644 measures/jinja2/measures/edit-multiple-start.jinja create mode 100644 measures/jinja2/measures/edit-review.jinja create mode 100644 measures/jinja2/measures/edit-wizard-step.jinja diff --git a/measures/jinja2/includes/measures/list.jinja b/measures/jinja2/includes/measures/list.jinja index 10b48aadf..a5e8f7fa7 100644 --- a/measures/jinja2/includes/measures/list.jinja +++ b/measures/jinja2/includes/measures/list.jinja @@ -1,17 +1,49 @@ {% from 'macros/create_link.jinja' import create_link %} {% from "includes/measures/conditions.jinja" import conditions_list %} +{% macro checkbox_item(field) -%} + {% set id = field.id_for_label %} + {% set name = field.name %} + {% set checked = "checked" if field.value() else "" %} +
+
+
+
+ + +
+
+
+
+{%- endmacro %} + +{% set checkbox_check_all -%} +
+
+
+
+ + +
+
+
+
+{%- endset %} + {% set table_rows = [] %} -{% for measure in object_list %} + +{% for field in form %} + {% set checkbox = checkbox_item(field) %} + {% set measure = field.field.obj %} {% set measure_link -%} {{measure.sid}} {%- endset %} - {% set footnotes %} - {% for footnote in measure.footnotes.all() %} - {{ create_link(footnote.get_url(), footnote|string) }}{{ ", " if not loop.last else "" }} - {% endfor %} - {% endset %} + {% set object_description -%} + {{ measure.structure_description if measure.structure_description else "-" }} + {%- endset %} + {{ table_rows.append([ + {"html": checkbox}, {"html": measure_link}, {"text": measure.measure_type.sid ~ " - " ~ measure.measure_type.description}, {"text": measure.goods_nomenclature.item_id|wordwrap(2)|replace("\n", " ") if measure.goods_nomenclature else '-', "classes": "govuk-!-width-one-eighth"}, @@ -24,18 +56,27 @@ ]) or "" }} {% endfor %} -{{ govukTable({ - "head": [ - {"text": "ID"}, - {"text": "Type"}, - {"text": "Commodity code"}, - {"text": "Duties"}, - {"text": "Additional code"}, - {"text": "Geographical area"}, - {"text": "Quota"}, - {"text": "Footnote"}, - {"text": "Conditions"}, - ], - "rows": table_rows, - "classes": "govuk-table-m" -}) }} +
+ + + {% if object_list %} + + {% endif %} + + {{ govukTable({ + "head": [ + {"html": checkbox_check_all}, + {"text": "ID"}, + {"text": "Type"}, + {"text": "Commodity code"}, + {"text": "Duties"}, + {"text": "Additional code"}, + {"text": "Geographical area"}, + {"text": "Quota"}, + {"text": "Footnote"}, + {"text": "Conditions"}, + ], + "rows": table_rows, + "classes": "govuk-table-m" + }) }} +
\ No newline at end of file diff --git a/measures/jinja2/measures/confirm-edit-multiple.jinja b/measures/jinja2/measures/confirm-edit-multiple.jinja new file mode 100644 index 000000000..420b80094 --- /dev/null +++ b/measures/jinja2/measures/confirm-edit-multiple.jinja @@ -0,0 +1,55 @@ +{% extends "layouts/confirm.jinja" %} +{% from "components/breadcrumbs.jinja" import breadcrumbs %} + +{% set page_title = "Updated measures" %} + +{% macro panel_title(edited_measures) %} + {% if edited_measures|length > 1 %} + {{ edited_measures.0._meta.verbose_name_plural|title }} {% for m in edited_measures %}{% if not loop.last %}{{ m|string }}, {% else %}{{ m|string }} {% endif %}{% endfor %}have been updated + {% else %} + {{ edited_measures.0._meta.verbose_name|title }} {{ edited_measures.0|string }} has been updated + {% endif %} +{% endmacro %} + +{% macro panel_subtitle(edited_measures) %} + {% if edited_measures|length > 1 %} + These updated measures have been added to your tariff changes + {% else %} + This updated measure has been added to your tariff changes + {% endif %} +{% endmacro %} + +{% block breadcrumb %} + {{ breadcrumbs(request, [ + {"text": page_title }, + ]) + }} +{% endblock %} + +{% block content %} +
+
+ Confirmation +

{{ page_title }}

+ {{ govukPanel({ + "titleText": panel_title(edited_measures), + "text": panel_subtitle(edited_measures), + "classes": "govuk-!-margin-bottom-7" + }) }} +

Next steps

+

To complete your task you must publish your change.

+ {% if request.session.workbasket %} + {{ govukButton({ + "text": "Go to your tariff changes", + "href": url("workbaskets:edit-workbasket", args=[request.session.workbasket.id]), + "classes": "govuk-button--secondary" + }) }} + {% endif %} + +
+
+{% endblock %} diff --git a/measures/jinja2/measures/create-wizard-step.jinja b/measures/jinja2/measures/create-wizard-step.jinja index 9b17b6417..aa018c485 100644 --- a/measures/jinja2/measures/create-wizard-step.jinja +++ b/measures/jinja2/measures/create-wizard-step.jinja @@ -1,5 +1,4 @@ {% extends "layouts/form.jinja" %} -{% from "components/step-by-step-nav/macro.jinja" import step_by_step_nav %} {% set page_title = step_metadata[wizard.steps.current].title %} {% set page_subtitle = "Create a new measure" %} diff --git a/measures/jinja2/measures/edit-multiple-start.jinja b/measures/jinja2/measures/edit-multiple-start.jinja new file mode 100644 index 000000000..4296a4e8c --- /dev/null +++ b/measures/jinja2/measures/edit-multiple-start.jinja @@ -0,0 +1,25 @@ +{% extends "layouts/form.jinja" %} +{% from 'macros/create_link.jinja' import create_link %} + +{% set page_title = step_metadata[wizard.steps.current].title %} +{% set page_subtitle = "Edit measures" %} +{% set info = "Edit measures" %} + +{% block content %} +
+
+ {{ page_subtitle|default("") }} +

{{ page_title }}

+ +

Editing measures:

+
    + {% for measure in measures %}
  • {{ create_link(url("measure-ui-detail", kwargs={"sid": measure.sid}), measure.sid) }}
    • {% endfor %} +
+ + {% call django_form(action=view.get_step_url(wizard.steps.current)) %} + {{ wizard.management_form }} + {% block form %}{{ crispy(form) }}{% endblock %} + {% endcall %} +
+
+{% endblock %} diff --git a/measures/jinja2/measures/edit-review.jinja b/measures/jinja2/measures/edit-review.jinja new file mode 100644 index 000000000..6571e040d --- /dev/null +++ b/measures/jinja2/measures/edit-review.jinja @@ -0,0 +1,164 @@ +{% extends "measures/edit-wizard-step.jinja" %} + +{% from "components/summary-list/macro.njk" import govukSummaryList %} + +{% set page_title = step_metadata[wizard.steps.current].title %} +{% set page_subtitle = "Edit measures" %} +{% set info = "Edit measures" %} + +{% macro review_step(step) %} + {% set link_text = step_metadata[step].link_text %} +

{{ link_text }}

+ {% set rows = [] %} + {% set data = view.get_cleaned_data_for_step(step) %} + + {% set ignore %} + {{ caller(rows, data) }} + {% endset %} + + {% set row_data = [] %} + {% if rows %} + {% for label, value in rows %} + {{ row_data.append({ + "key": {"text": label}, + "value": {"text": value}, + "actions": { + "items": [ + { + "text": "Change", + "visuallyHiddenText": label|lower, + "href": view.get_step_url(step), + "attributes": {} + } + ] + } + }) or "" }} + {% endfor %} + {% else %} + {{ row_data.append({ + "key": {"text": link_text}, + "value": {"text": "None"}, + "actions": { + "items": [ + { + "text": "Change", + "visuallyHiddenText": link_text, + "href": view.get_step_url(step), + "attributes": {} + } + ] + } + }) or "" }} + {% endif %} + {{ govukSummaryList({"rows": row_data, "classes": "govuk-!-margin-bottom-9"}) }} +{% endmacro %} + +{% block form %} + {% set fields_to_edit = view.get_cleaned_data_for_step("start").fields_to_edit %} + {% if "measure_details" in fields_to_edit %} + {% call(rows, data) review_step("measure_details") %} + {{ rows.extend([ + ("Measure type", data["measure_type"] ~ " - " ~ data["measure_type"].description), + ("Measure start date", "{:%d/%m/%Y}".format(data["valid_between"].lower) if data["valid_between"] and data["valid_between"].lower else "-"), + ("Measure end date", "{:%d/%m/%Y}".format(data["valid_between"].upper) if data["valid_between"] and data["valid_between"].upper else "-"), + ]) }} + {% endcall %} + {% endif %} + + {% if "regulation_id" in fields_to_edit %} + {% call(rows, data) review_step("regulation_id") %} + {{ rows.extend([ + ("Regulation ID", data["generating_regulation"]), + ]) }} + {% endcall %} + {% endif %} + + {% if "quota_order_number" in fields_to_edit %} + {% call(rows, data) review_step("quota_order_number") %} + {{ rows.extend([ + ("Quota order number", data["order_number"]), + ]) }} + {% endcall %} + {% endif %} + + {% if "geographical_area" in fields_to_edit %} + {% call(rows, data) review_step("geographical_area") %} + {% for item in data.geo_area_list %} + {{ rows.extend([ + ("Geographical area", item.get_description().description|safe), + ]) }} + {% endfor %} + {% if data.geo_area_exclusions %} + {% for item in data.geo_area_exclusions %} + {{ rows.extend([ + ("Excluded geographical area", item.get_description().description|safe), + ]) }} + {% endfor %} + {% endif %} + + {% endcall %} + {% endif %} + + {% if "commodity" in fields_to_edit %} + {% call(rows, data) review_step("commodity") %} + {% for item in data %} + {{ rows.extend([ + ("Commodity code", item["commodity"] ~ " - " ~ item["commodity"].get_description().description|safe if item["commodity"] else "None"), + ]) }} + {% endfor %} + {% endcall %} + {% endif %} + + {% if "duties" in fields_to_edit %} + {% call(rows, data) review_step("duties") %} + {{ rows.extend([ + ("Duty", data["duties"]), + ]) }} + {% endcall %} + {% endif %} + + {% if "additional_code" in fields_to_edit %} + {% call(rows, data) review_step("additional_code") %} + {{ rows.extend([ + ("Additional code", data["additional_code"] ~ " - " ~ data["additional_code"].get_description().description|safe if data["additional_code"] else "None"), + ]) }} + {% endcall %} + {% endif %} + + {% if "conditions" in fields_to_edit %} + {% call(rows, data) review_step("conditions") %} + {# Conditions are optional, so cater for empty data. #} + {% if data %} + {% for item in data %} + {% if not item.DELETE %} + {{ rows.extend([ + ("Condition code", item.condition_code), + ("Duty amount", item.duty_amount), + ("Monetary unit", item.monetary_unit), + ("Condition measurement", item.condition_measurement), + ("Required certificate", item.required_certificate), + ("Action", item.action), + ("Applicable duty", item.applicable_duty), + ("Reference price", item.reference_price), + ]) }} + {% endif %} + {% endfor %} + {% endif %} + {% endcall %} + {% endif %} + + {% if "footnotes" in fields_to_edit %} + {% call(rows, data) review_step("footnotes") %} + {# Footnotes are optional, so cater for empty data. #} + {% if data %} + {% for item in data %} + {% if not item.DELETE %} + {{ rows.append(("Footnote", item.footnote ~ " - " ~ item.footnote.get_description().description|safe)) }} + {% endif %} + {% endfor %} + {% endif %} + {% endcall %} + {% endif %} + + {{ govukButton({"text": "Create", "preventDoubleClick": true,}) }} +{% endblock %} diff --git a/measures/jinja2/measures/edit-wizard-step.jinja b/measures/jinja2/measures/edit-wizard-step.jinja new file mode 100644 index 000000000..1b67e74e8 --- /dev/null +++ b/measures/jinja2/measures/edit-wizard-step.jinja @@ -0,0 +1,23 @@ +{% extends "layouts/form.jinja" %} + +{% set page_title = step_metadata[wizard.steps.current].title %} +{% set page_subtitle = "Edit measures" %} +{% set info = "Edit measures" %} + +{% block content %} +
+
+ {{ page_subtitle|default("") }} +

{{ page_title }}

+ + {% if step_metadata[wizard.steps.current].info %} +

{{ step_metadata[wizard.steps.current].info }}

+ {% endif %} + + {% call django_form(action=view.get_step_url(wizard.steps.current)) %} + {{ wizard.management_form }} + {% block form %}{{ crispy(form) }}{% endblock %} + {% endcall %} +
+
+{% endblock %} diff --git a/pii-ner-exclude.txt b/pii-ner-exclude.txt index 64ebd3abf..a741a251f 100644 --- a/pii-ner-exclude.txt +++ b/pii-ner-exclude.txt @@ -17,6 +17,7 @@ f"https://legislation.gov.uk/uksi/2021/{n UI FIXME SessionStore +item.duty_amount Measure SID AdditionalCode MeasurementUnit From 82bf1a1ee833694d4076d790e484534517d94ccb Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 2 Nov 2022 15:10:29 +0000 Subject: [PATCH 07/13] Rename, edit docstrings --- measures/forms.py | 34 ++++++++++++++-------------------- measures/views.py | 4 ++-- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/measures/forms.py b/measures/forms.py index b45b74b3a..240994900 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -408,7 +408,6 @@ class MeasureConditionsFormSet(FormSet): class MeasureConditionsWizardStepForm(MeasureConditionsFormMixin): - # override methods that use form kwargs def __init__(self, *args, **kwargs): self.measure_start_date = kwargs.pop("measure_start_date") super().__init__(*args, **kwargs) @@ -443,21 +442,19 @@ def clean(self): return self.conditions_clean(cleaned_data, self.measure_start_date) -class MeasureConditionsEditWizardStepForm(MeasureConditionsFormMixin): +class MeasureConditionsMultipleEditForm(MeasureConditionsFormMixin): + """ + Used in the MeasuresEditWizard. + + Like MeasureConditionsWizardStepForm but has validation for multiple + measures with different validity periods. + """ - # override methods that use form kwargs def __init__(self, *args, **kwargs): self.measure_start_dates = kwargs.pop("measure_start_dates") super().__init__(*args, **kwargs) def clean_applicable_duty(self): - """ - Gets applicable_duty from cleaned data. - - We expect `measure_start_dates` to be passed in. Uses - `DutySentenceParser` to check that applicable_duty is a valid duty - string. - """ applicable_duty = self.cleaned_data["applicable_duty"] if applicable_duty and self.measure_start_dates is not None: @@ -467,15 +464,6 @@ def clean_applicable_duty(self): return applicable_duty def clean(self): - """ - We get the reference_price from cleaned_data and the measure_start_dates - from form kwargs. - - If reference_price is provided, we use DutySentenceParser with - measure_start_dates to check that we are dealing with a simple duty - (i.e. only one component). We then update cleaned_data with key-value - pairs created from this single, unsaved component. - """ cleaned_data = super().clean() return self.conditions_clean(cleaned_data, self.measure_start_dates) @@ -1111,7 +1099,13 @@ def clean(self): return cleaned_data -class MeasureDutiesForm(forms.Form): +class MeasureDutiesMultipleEditForm(forms.Form): + """ + Used in the MeasuresEditWizard. + + Has validation for multiple measures with different validity periods. + """ + duties = forms.CharField( label="Duties", help_text="The duty expression should be valid for the validity periods of all the measures you wish to edit", diff --git a/measures/views.py b/measures/views.py index 6006332e7..f59f23399 100644 --- a/measures/views.py +++ b/measures/views.py @@ -186,9 +186,9 @@ class MeasuresEditWizard( (constants.QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), (constants.GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), (constants.COMMODITIES, forms.MeasureCommodityForm), - (constants.DUTIES, forms.MeasureDutiesForm), + (constants.DUTIES, forms.MeasureDutiesMultipleEditForm), (constants.ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), - (constants.CONDITIONS, forms.MeasureConditionsEditWizardStepFormSet), + (constants.CONDITIONS, forms.MeasureConditionsMultipleEditForm), (constants.FOOTNOTES, forms.MeasureFootnotesFormSet), (constants.SUMMARY, forms.MeasureReviewForm), ] From f08d948a5ef6548745fc3346e53a44bc26231aec Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Thu, 3 Nov 2022 11:07:53 +0000 Subject: [PATCH 08/13] Only allow editing of end date. Remove commodities step --- measures/conditions.py | 13 ++------ measures/constants.py | 1 + measures/forms.py | 37 +++++++++++++++------- measures/jinja2/measures/edit-review.jinja | 8 ++--- measures/views.py | 18 +++++++---- 5 files changed, 44 insertions(+), 33 deletions(-) diff --git a/measures/conditions.py b/measures/conditions.py index 124dbb4a7..45825f010 100644 --- a/measures/conditions.py +++ b/measures/conditions.py @@ -1,10 +1,10 @@ from measures import constants -def show_step_measure_details(wizard): +def show_step_end_dates(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) if cleaned_data: - return constants.MEASURE_DETAILS in cleaned_data.get("fields_to_edit") + return constants.END_DATES in cleaned_data.get("fields_to_edit") def show_step_regulation_id(wizard): @@ -25,12 +25,6 @@ def show_step_geographical_area(wizard): return constants.GEOGRAPHICAL_AREA in cleaned_data.get("fields_to_edit") -def show_step_commodities(wizard): - cleaned_data = wizard.get_cleaned_data_for_step(constants.START) - if cleaned_data: - return constants.COMMODITIES in cleaned_data.get("fields_to_edit") - - def show_step_duties(wizard): cleaned_data = wizard.get_cleaned_data_for_step(constants.START) if cleaned_data: @@ -56,11 +50,10 @@ def show_step_footnotes(wizard): measure_edit_condition_dict = { - constants.MEASURE_DETAILS: show_step_measure_details, + constants.END_DATES: show_step_end_dates, constants.REGULATION_ID: show_step_regulation_id, constants.QUOTA_ORDER_NUMBER: show_step_quota_order_number, constants.GEOGRAPHICAL_AREA: show_step_geographical_area, - constants.COMMODITIES: show_step_commodities, constants.DUTIES: show_step_duties, constants.ADDITIONAL_CODE: show_step_additional_code, constants.CONDITIONS: show_step_conditions, diff --git a/measures/constants.py b/measures/constants.py index 623bbf78e..363b6f15b 100644 --- a/measures/constants.py +++ b/measures/constants.py @@ -1,4 +1,5 @@ START = "start" +END_DATES = "end_dates" VALIDITY_DATES = "validity_dates" MEASURE_DETAILS = "measure_details" REGULATION_ID = "regulation_id" diff --git a/measures/forms.py b/measures/forms.py index 240994900..2fe996c16 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -20,6 +20,7 @@ from commodities.models import GoodsNomenclature from common.fields import AutoCompleteField from common.forms import BindNestedFormMixin +from common.forms import DateInputFieldFixed from common.forms import FormSet from common.forms import RadioNested from common.forms import ValidityPeriodForm @@ -245,6 +246,28 @@ class Meta: ] +class MeasureEndDateForm(forms.Form): + end_date = DateInputFieldFixed( + label="End date", + ) + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.helper = FormHelper(self) + self.helper.label_size = Size.SMALL + self.helper.legend_size = Size.SMALL + self.helper.layout = Layout( + "end_date", + Submit( + "submit", + "Continue", + data_module="govuk-button", + data_prevent_double_click="true", + ), + ) + + class MeasureConditionsFormMixin(forms.ModelForm): class Meta: model = models.MeasureCondition @@ -501,7 +524,7 @@ class MeasureConditionsWizardStepFormSet(FormSet): class MeasureConditionsEditWizardStepFormSet(FormSet): - form = MeasureConditionsEditWizardStepForm + form = MeasureConditionsMultipleEditForm class MeasureForm(ValidityPeriodForm, BindNestedFormMixin, forms.ModelForm): @@ -1146,15 +1169,6 @@ def clean(self): return cleaned_data -class MeasureCommodityForm(forms.Form): - commodity = AutoCompleteField( - label="Commodity code", - help_text="Select the 10-digit commodity code to which the measure applies.", - queryset=GoodsNomenclature.objects.all(), - attrs={"min_length": 3}, - ) - - class MeasureCommodityAndDutiesFormSet(FormSet): form = MeasureCommodityAndDutiesForm prefix = "measure_commodities_duties_formset" @@ -1221,11 +1235,10 @@ class MeasureReviewForm(forms.Form): class MeasuresEditStartForm(forms.Form): choices = [ - (constants.MEASURE_DETAILS, "Validity dates and measure type"), + (constants.END_DATES, "End dates"), (constants.REGULATION_ID, "Regulation"), (constants.QUOTA_ORDER_NUMBER, "Quotas"), (constants.GEOGRAPHICAL_AREA, "Geographies"), - (constants.COMMODITIES, "Commodity code"), (constants.DUTIES, "Duties"), (constants.ADDITIONAL_CODE, "Additional codes"), (constants.CONDITIONS, "Conditions"), diff --git a/measures/jinja2/measures/edit-review.jinja b/measures/jinja2/measures/edit-review.jinja index 6571e040d..da7b81c96 100644 --- a/measures/jinja2/measures/edit-review.jinja +++ b/measures/jinja2/measures/edit-review.jinja @@ -55,12 +55,10 @@ {% block form %} {% set fields_to_edit = view.get_cleaned_data_for_step("start").fields_to_edit %} - {% if "measure_details" in fields_to_edit %} - {% call(rows, data) review_step("measure_details") %} + {% if "end_dates" in fields_to_edit %} + {% call(rows, data) review_step("end_dates") %} {{ rows.extend([ - ("Measure type", data["measure_type"] ~ " - " ~ data["measure_type"].description), - ("Measure start date", "{:%d/%m/%Y}".format(data["valid_between"].lower) if data["valid_between"] and data["valid_between"].lower else "-"), - ("Measure end date", "{:%d/%m/%Y}".format(data["valid_between"].upper) if data["valid_between"] and data["valid_between"].upper else "-"), + ("Measure end date", "{:%d/%m/%Y}".format(data["end_date"]) if data["end_date"] else "-"), ]) }} {% endcall %} {% endif %} diff --git a/measures/views.py b/measures/views.py index f59f23399..61904d43c 100644 --- a/measures/views.py +++ b/measures/views.py @@ -20,6 +20,7 @@ from common.models import TrackedModel from common.serializers import AutoCompleteSerializer from common.session_store import SessionStore +from common.util import TaricDateRange from common.validators import UpdateType from common.views import TamatoListView from common.views import TrackedModelDetailMixin @@ -160,7 +161,7 @@ class MeasuresEditWizard( "title": "Select the elements you want to edit", "link_text": "Start", }, - constants.MEASURE_DETAILS: STEP_METADATA[constants.MEASURE_DETAILS], + constants.END_DATES: {"title": "Edit the end dates", "link_text": "End dates"}, constants.REGULATION_ID: STEP_METADATA[constants.REGULATION_ID], constants.QUOTA_ORDER_NUMBER: STEP_METADATA[constants.QUOTA_ORDER_NUMBER], constants.GEOGRAPHICAL_AREA: STEP_METADATA[constants.GEOGRAPHICAL_AREA], @@ -181,11 +182,10 @@ class MeasuresEditWizard( form_list = [ (constants.START, forms.MeasuresEditStartForm), - (constants.MEASURE_DETAILS, forms.MeasureDetailsForm), + (constants.END_DATES, forms.MeasureEndDateForm), (constants.REGULATION_ID, forms.MeasureRegulationIdForm), (constants.QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), (constants.GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), - (constants.COMMODITIES, forms.MeasureCommodityForm), (constants.DUTIES, forms.MeasureDutiesMultipleEditForm), (constants.ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), (constants.CONDITIONS, forms.MeasureConditionsMultipleEditForm), @@ -195,11 +195,10 @@ class MeasuresEditWizard( templates = { constants.START: "measures/edit-multiple-start.jinja", - constants.MEASURE_DETAILS: "measures/edit-wizard-step.jinja", + constants.END_DATES: "measures/edit-wizard-step.jinja", constants.REGULATION_ID: "measures/edit-wizard-step.jinja", constants.QUOTA_ORDER_NUMBER: "measures/edit-wizard-step.jinja", constants.GEOGRAPHICAL_AREA: "measures/edit-wizard-step.jinja", - constants.COMMODITIES: "measures/edit-wizard-step.jinja", constants.DUTIES: "measures/edit-wizard-step.jinja", constants.ADDITIONAL_CODE: "measures/edit-wizard-step.jinja", constants.CONDITIONS: "measures/create-formset.jinja", @@ -211,7 +210,7 @@ class MeasuresEditWizard( def get_template_names(self): return self.templates.get( self.steps.current, - "measures/measure-wizard-step.jinja", + "measures/edit-wizard-step.jinja", ) @property @@ -274,6 +273,13 @@ def done(self, form_list, **kwargs): edited_measures = [] for measure in self.measures: + if "end_date" in cleaned_data: + new_valid_between = TaricDateRange( + lower=measure.valid_between.lower, + upper=cleaned_data["end_date"], + ) + changed_data["valid_between"] = new_valid_between + measure.new_version( workbasket=self.workbasket, update_type=self.update_type, From d9886be9b5f2d5d0a5dd7776608c2426efe3f3f6 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Mon, 7 Nov 2022 16:50:05 +0000 Subject: [PATCH 09/13] Move update measure code into separate function --- measures/forms.py | 85 +--------------------------------- measures/helpers.py | 88 ++++++++++++++++++++++++++++++++++++ measures/tests/test_forms.py | 4 +- 3 files changed, 92 insertions(+), 85 deletions(-) create mode 100644 measures/helpers.py diff --git a/measures/forms.py b/measures/forms.py index 2fe996c16..9c77583f2 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -27,15 +27,13 @@ from common.forms import delete_form_for from common.forms import formset_factory from common.util import validity_range_contains_range -from common.validators import UpdateType from footnotes.models import Footnote from geo_areas.models import GeographicalArea from geo_areas.validators import AreaCode from measures import constants from measures import models +from measures.helpers import update_measure from measures.parsers import DutySentenceParser -from measures.patterns import MeasureCreationPattern -from measures.util import diff_components from measures.validators import validate_duties from quotas.models import QuotaOrderNumber from regulations.models import Regulation @@ -681,86 +679,7 @@ def save(self, commit=True): if commit: instance.save() - sid = instance.sid - - measure_creation_pattern = MeasureCreationPattern( - workbasket=WorkBasket.current(self.request), - base_date=instance.valid_between.lower, - defaults={ - "generating_regulation": self.cleaned_data["generating_regulation"], - }, - ) - - if self.cleaned_data.get("exclusions"): - for exclusion in self.cleaned_data.get("exclusions"): - pattern = ( - measure_creation_pattern.create_measure_excluded_geographical_areas( - instance, - exclusion, - ) - ) - [p for p in pattern] - - if ( - self.request.session[f"instance_duty_sentence_{self.instance.sid}"] - != self.cleaned_data["duty_sentence"] - ): - diff_components( - instance, - self.cleaned_data["duty_sentence"], - self.cleaned_data["valid_between"].lower, - WorkBasket.current(self.request), - # Creating components in the same transaction as the new version - # of the measure minimises number of transaction and groups the - # creation of measure and related objects in the same - # transaction. - instance.transaction, - models.MeasureComponent, - "component_measure", - ) - - footnote_pks = [ - dct["footnote"] - for dct in self.request.session.get(f"formset_initial_{sid}", []) - ] - footnote_pks.extend(self.request.session.get(f"instance_footnotes_{sid}", [])) - - self.request.session.pop(f"formset_initial_{sid}", None) - self.request.session.pop(f"instance_footnotes_{sid}", None) - - for pk in footnote_pks: - footnote = ( - Footnote.objects.filter(pk=pk) - .approved_up_to_transaction(instance.transaction) - .first() - ) - - existing_association = ( - models.FootnoteAssociationMeasure.objects.approved_up_to_transaction( - instance.transaction, - ) - .filter( - footnoted_measure__sid=instance.sid, - associated_footnote__footnote_id=footnote.footnote_id, - associated_footnote__footnote_type__footnote_type_id=footnote.footnote_type.footnote_type_id, - ) - .first() - ) - if existing_association: - existing_association.new_version( - workbasket=WorkBasket.current(self.request), - transaction=instance.transaction, - footnoted_measure=instance, - ) - else: - models.FootnoteAssociationMeasure.objects.create( - footnoted_measure=instance, - associated_footnote=footnote, - update_type=UpdateType.CREATE, - transaction=instance.transaction, - ) - - return instance + return update_measure(instance, self.request, self.cleaned_data) def is_valid(self) -> bool: """Check that measure conditions data is valid before calling super() on diff --git a/measures/helpers.py b/measures/helpers.py new file mode 100644 index 000000000..6554fe8f8 --- /dev/null +++ b/measures/helpers.py @@ -0,0 +1,88 @@ +from common.validators import UpdateType +from footnotes.models import Footnote +from measures.models import FootnoteAssociationMeasure +from measures.models import MeasureComponent +from measures.patterns import MeasureCreationPattern +from measures.util import diff_components +from workbaskets.models import WorkBasket + + +def update_measure(instance, request, cleaned_data): + sid = instance.sid + + measure_creation_pattern = MeasureCreationPattern( + workbasket=WorkBasket.current(request), + base_date=instance.valid_between.lower, + defaults={ + "generating_regulation": cleaned_data["generating_regulation"], + }, + ) + + if cleaned_data.get("exclusions"): + for exclusion in cleaned_data.get("exclusions"): + pattern = ( + measure_creation_pattern.create_measure_excluded_geographical_areas( + instance, + exclusion, + ) + ) + [p for p in pattern] + if ( + request.session[f"instance_duty_sentence_{instance.sid}"] + != cleaned_data["duty_sentence"] + ): + diff_components( + instance, + cleaned_data["duty_sentence"], + cleaned_data["valid_between"].lower, + WorkBasket.current(request), + # Creating components in the same transaction as the new version + # of the measure minimises number of transaction and groups the + # creation of measure and related objects in the same + # transaction. + instance.transaction, + MeasureComponent, + "component_measure", + ) + + footnote_pks = [ + dct["footnote"] for dct in request.session.get(f"formset_initial_{sid}", []) + ] + footnote_pks.extend(request.session.get(f"instance_footnotes_{sid}", [])) + + request.session.pop(f"formset_initial_{sid}", None) + request.session.pop(f"instance_footnotes_{sid}", None) + + for pk in footnote_pks: + footnote = ( + Footnote.objects.filter(pk=pk) + .approved_up_to_transaction(instance.transaction) + .first() + ) + + existing_association = ( + FootnoteAssociationMeasure.objects.approved_up_to_transaction( + instance.transaction, + ) + .filter( + footnoted_measure__sid=instance.sid, + associated_footnote__footnote_id=footnote.footnote_id, + associated_footnote__footnote_type__footnote_type_id=footnote.footnote_type.footnote_type_id, + ) + .first() + ) + if existing_association: + existing_association.new_version( + workbasket=WorkBasket.current(request), + transaction=instance.transaction, + footnoted_measure=instance, + ) + else: + FootnoteAssociationMeasure.objects.create( + footnoted_measure=instance, + associated_footnote=footnote, + update_type=UpdateType.CREATE, + transaction=instance.transaction, + ) + + return instance diff --git a/measures/tests/test_forms.py b/measures/tests/test_forms.py index 32a4082a5..fbb029f58 100644 --- a/measures/tests/test_forms.py +++ b/measures/tests/test_forms.py @@ -17,7 +17,7 @@ COUNTRY_REGION_FORM_PREFIX = "country_region" -@patch("measures.forms.diff_components") +@patch("measures.helpers.diff_components") def test_diff_components_not_called( diff_components, measure_form, @@ -29,7 +29,7 @@ def test_diff_components_not_called( assert diff_components.called == False -@patch("measures.forms.diff_components") +@patch("measures.helpers.diff_components") def test_diff_components_called(diff_components, measure_form, duty_sentence_parser): measure_form.data.update(duty_sentence="6.000%") with override_current_transaction(Transaction.objects.last()): From 250bed5f955766325bc71f13a4f6178bf9b0db8d Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 11 Nov 2022 11:01:34 +0000 Subject: [PATCH 10/13] Split out create_conditions --- measures/forms.py | 32 +++++++++- measures/helpers.py | 141 ++++++++++++++++++++++++++++++++++++-------- measures/views.py | 130 +++++++++++----------------------------- 3 files changed, 184 insertions(+), 119 deletions(-) diff --git a/measures/forms.py b/measures/forms.py index 9c77583f2..1c2d70a70 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -33,6 +33,7 @@ from measures import constants from measures import models from measures.helpers import update_measure +from measures.helpers import update_measure_footnotes from measures.parsers import DutySentenceParser from measures.validators import validate_duties from quotas.models import QuotaOrderNumber @@ -678,8 +679,37 @@ def save(self, commit=True): instance = super().save(commit=False) if commit: instance.save() + defaults = {"generating_regulation": self.cleaned_data["generating_regulation"]} - return update_measure(instance, self.request, self.cleaned_data) + footnote_pks = [ + dct["footnote"] + for dct in self.request.session.get(f"formset_initial_{instance.sid}", []) + ] + footnote_pks.extend( + self.request.session.get(f"instance_footnotes_{instance.sid}", []), + ) + + self.request.session.pop(f"formset_initial_{instance.sid}", None) + self.request.session.pop(f"instance_footnotes_{instance.sid}", None) + + tx = WorkBasket.get_current_transaction(self.request) + + new_measure = update_measure( + instance, + tx, + WorkBasket.current(self.request), + self.cleaned_data, + defaults, + ) + + update_measure_footnotes( + new_measure, + tx, + WorkBasket.current(self.request), + footnote_pks, + ) + + return new_measure def is_valid(self) -> bool: """Check that measure conditions data is valid before calling super() on diff --git a/measures/helpers.py b/measures/helpers.py index 6554fe8f8..c1a1534ca 100644 --- a/measures/helpers.py +++ b/measures/helpers.py @@ -2,20 +2,19 @@ from footnotes.models import Footnote from measures.models import FootnoteAssociationMeasure from measures.models import MeasureComponent +from measures.models import MeasureCondition +from measures.models import MeasureConditionComponent +from measures.parsers import DutySentenceParser from measures.patterns import MeasureCreationPattern from measures.util import diff_components -from workbaskets.models import WorkBasket -def update_measure(instance, request, cleaned_data): - sid = instance.sid +def update_measure(instance, transaction, current_workbasket, cleaned_data, defaults): measure_creation_pattern = MeasureCreationPattern( - workbasket=WorkBasket.current(request), + workbasket=current_workbasket, base_date=instance.valid_between.lower, - defaults={ - "generating_regulation": cleaned_data["generating_regulation"], - }, + defaults=defaults, ) if cleaned_data.get("exclusions"): @@ -27,42 +26,40 @@ def update_measure(instance, request, cleaned_data): ) ) [p for p in pattern] + if ( - request.session[f"instance_duty_sentence_{instance.sid}"] - != cleaned_data["duty_sentence"] + cleaned_data.get("duty_sentence") + and instance.duty_sentence != cleaned_data["duty_sentence"] ): diff_components( instance, cleaned_data["duty_sentence"], cleaned_data["valid_between"].lower, - WorkBasket.current(request), + current_workbasket, # Creating components in the same transaction as the new version # of the measure minimises number of transaction and groups the # creation of measure and related objects in the same # transaction. - instance.transaction, + transaction, MeasureComponent, "component_measure", ) - footnote_pks = [ - dct["footnote"] for dct in request.session.get(f"formset_initial_{sid}", []) - ] - footnote_pks.extend(request.session.get(f"instance_footnotes_{sid}", [])) + return instance.new_version(current_workbasket) + - request.session.pop(f"formset_initial_{sid}", None) - request.session.pop(f"instance_footnotes_{sid}", None) +def update_measure_footnotes(instance, transaction, current_workbasket, footnote_pks): for pk in footnote_pks: footnote = ( Footnote.objects.filter(pk=pk) - .approved_up_to_transaction(instance.transaction) + .approved_up_to_transaction(transaction) .first() ) existing_association = ( FootnoteAssociationMeasure.objects.approved_up_to_transaction( - instance.transaction, + transaction, ) .filter( footnoted_measure__sid=instance.sid, @@ -73,8 +70,8 @@ def update_measure(instance, request, cleaned_data): ) if existing_association: existing_association.new_version( - workbasket=WorkBasket.current(request), - transaction=instance.transaction, + workbasket=current_workbasket, + transaction=transaction, footnoted_measure=instance, ) else: @@ -82,7 +79,105 @@ def update_measure(instance, request, cleaned_data): footnoted_measure=instance, associated_footnote=footnote, update_type=UpdateType.CREATE, - transaction=instance.transaction, + transaction=transaction, ) - return instance + +def update_conditions(instance, transaction, current_workbasket, formset): + """ + Gets condition formset from context data, loops over these forms and + validates the data, checking for the condition_sid field in the data to + indicate whether an existing condition is being updated or a new one created + from scratch. + + Then deletes any existing conditions that are not being updated, + before calling the MeasureCreationPattern.create_condition_and_components with the appropriate parser and condition data. + """ + excluded_sids = [] + conditions_data = [] + existing_conditions = instance.conditions.approved_up_to_transaction( + transaction, + ) + + for f in formset.forms: + f.is_valid() + condition_data = f.cleaned_data + # If the form has changed and "condition_sid" is in the changed data, + # this means that the condition is preexisting and needs to updated + # so that its dependent_measure points to the latest version of measure + if f.has_changed() and "condition_sid" in f.changed_data: + excluded_sids.append(f.initial["condition_sid"]) + update_type = UpdateType.UPDATE + condition_data["version_group"] = existing_conditions.get( + sid=f.initial["condition_sid"], + ).version_group + condition_data["sid"] = f.initial["condition_sid"] + # If changed and condition_sid not in changed_data, then this is a newly created condition + elif f.has_changed() and "condition_sid" not in f.changed_data: + update_type = UpdateType.CREATE + + condition_data["update_type"] = update_type + conditions_data.append(condition_data) + + # Delete all existing conditions from the measure instance, except those that need to be updated + for condition in existing_conditions.exclude(sid__in=excluded_sids): + condition.new_version( + workbasket=current_workbasket, + update_type=UpdateType.DELETE, + transaction=transaction, + ) + + if conditions_data: + create_conditions(instance, transaction, current_workbasket, conditions_data) + + +def create_conditions(instance, transaction, current_workbasket, conditions_data): + + measure_creation_pattern = MeasureCreationPattern( + workbasket=current_workbasket, + base_date=instance.valid_between.lower, + ) + parser = DutySentenceParser.get( + instance.valid_between.lower, + component_output=MeasureConditionComponent, + ) + + # Loop over conditions_data, starting at 1 because component_sequence_number has to start at 1 + for component_sequence_number, condition_data in enumerate( + conditions_data, + start=1, + ): + # Create conditions and measure condition components, using instance as `dependent_measure` + + condition = MeasureCondition( + sid=condition_data.get("sid") + or measure_creation_pattern.measure_condition_sid_counter(), + component_sequence_number=component_sequence_number, + dependent_measure=instance, + update_type=condition_data.get("update_type") or UpdateType.CREATE, + transaction=transaction, + duty_amount=condition_data.get("duty_amount"), + condition_code=condition_data["condition_code"], + action=condition_data.get("action"), + required_certificate=condition_data.get("required_certificate"), + monetary_unit=condition_data.get("monetary_unit"), + condition_measurement=condition_data.get( + "condition_measurement", + ), + ) + if condition_data.get("version_group"): + condition.version_group = condition_data.get("version_group") + + condition.clean() + condition.save() + + if condition_data.get("applicable_duty"): + diff_components( + condition, + condition_data.get("applicable_duty"), + instance.valid_between.lower, + current_workbasket, + transaction, + MeasureConditionComponent, + "condition", + ) diff --git a/measures/views.py b/measures/views.py index 61904d43c..631ea4d31 100644 --- a/measures/views.py +++ b/measures/views.py @@ -9,6 +9,7 @@ from django.http import HttpRequest from django.http import HttpResponse from django.http import HttpResponseRedirect +from django.shortcuts import redirect from django.shortcuts import render from django.utils.decorators import method_decorator from django.views import View @@ -20,7 +21,6 @@ from common.models import TrackedModel from common.serializers import AutoCompleteSerializer from common.session_store import SessionStore -from common.util import TaricDateRange from common.validators import UpdateType from common.views import TamatoListView from common.views import TrackedModelDetailMixin @@ -29,6 +29,10 @@ from measures import forms from measures.filters import MeasureFilter from measures.filters import MeasureTypeFilterBackend +from measures.helpers import create_conditions +from measures.helpers import update_conditions +from measures.helpers import update_measure +from measures.helpers import update_measure_footnotes from measures.models import FootnoteAssociationMeasure from measures.models import Measure from measures.models import MeasureConditionComponent @@ -188,7 +192,7 @@ class MeasuresEditWizard( (constants.GEOGRAPHICAL_AREA, forms.MeasureGeographicalAreaForm), (constants.DUTIES, forms.MeasureDutiesMultipleEditForm), (constants.ADDITIONAL_CODE, forms.MeasureAdditionalCodeForm), - (constants.CONDITIONS, forms.MeasureConditionsMultipleEditForm), + (constants.CONDITIONS, forms.MeasureConditionsEditWizardStepFormSet), (constants.FOOTNOTES, forms.MeasureFootnotesFormSet), (constants.SUMMARY, forms.MeasureReviewForm), ] @@ -207,6 +211,11 @@ class MeasuresEditWizard( constants.COMPLETE: "measures/confirm-edit-multiple.jinja", } + def get(self, *args, **kwargs): + if not self.measures: + return redirect("measure-ui-list") + return super().get(*args, **kwargs) + def get_template_names(self): return self.templates.get( self.steps.current, @@ -265,26 +274,26 @@ def get_context_data(self, form, **kwargs): def done(self, form_list, **kwargs): cleaned_data = self.get_all_cleaned_data() - - model_fields = [f.name for f in Measure._meta.get_fields()] - form_changed_data = [f for f in cleaned_data if f in model_fields] - changed_data = {name: cleaned_data[name] for name in form_changed_data} + current_workbasket = WorkBasket.current(self.request) + tx = WorkBasket.get_current_transaction(self.request) edited_measures = [] + + footnote_pks = [ + item["footnote"].pk + for item in cleaned_data.get("formset-footnotes", []) + if not item.get("DELETE") + ] + conditions_data = cleaned_data.get("formset-conditions", []) + for condition in conditions_data: + condition["update_type"] = UpdateType.CREATE + for measure in self.measures: + defaults = {"generating_regulation": measure.generating_regulation} + update_measure(measure, tx, current_workbasket, cleaned_data, defaults) + create_conditions(measure, current_workbasket, conditions_data) + update_measure_footnotes(measure, tx, current_workbasket, footnote_pks) - if "end_date" in cleaned_data: - new_valid_between = TaricDateRange( - lower=measure.valid_between.lower, - upper=cleaned_data["end_date"], - ) - changed_data["valid_between"] = new_valid_between - - measure.new_version( - workbasket=self.workbasket, - update_type=self.update_type, - **changed_data, - ) edited_measures.append(measure) context = self.get_context_data( @@ -586,85 +595,16 @@ def get_context_data(self, **kwargs): context["conditions_formset"] = conditions_formset return context - def create_conditions(self, obj): - """ - Gets condition formset from context data, loops over these forms and - validates the data, checking for the condition_sid field in the data to - indicate whether an existing condition is being updated or a new one - created from scratch. - - Then deletes any existing conditions that are not being updated, - before calling the MeasureCreationPattern.create_condition_and_components with the appropriate parser and condition data. - """ - formset = self.get_context_data()["conditions_formset"] - excluded_sids = [] - conditions_data = [] - workbasket = WorkBasket.current(self.request) - existing_conditions = obj.conditions.approved_up_to_transaction( - workbasket.get_current_transaction(self.request), - ) - - for f in formset.forms: - f.is_valid() - condition_data = f.cleaned_data - # If the form has changed and "condition_sid" is in the changed data, - # this means that the condition is preexisting and needs to updated - # so that its dependent_measure points to the latest version of measure - if f.has_changed() and "condition_sid" in f.changed_data: - excluded_sids.append(f.initial["condition_sid"]) - update_type = UpdateType.UPDATE - condition_data["version_group"] = existing_conditions.get( - sid=f.initial["condition_sid"], - ).version_group - condition_data["sid"] = f.initial["condition_sid"] - # If changed and condition_sid not in changed_data, then this is a newly created condition - elif f.has_changed() and "condition_sid" not in f.changed_data: - update_type = UpdateType.CREATE - - condition_data["update_type"] = update_type - conditions_data.append(condition_data) - - workbasket = WorkBasket.current(self.request) - - # Delete all existing conditions from the measure instance, except those that need to be updated - for condition in existing_conditions.exclude(sid__in=excluded_sids): - condition.new_version( - workbasket=workbasket, - update_type=UpdateType.DELETE, - transaction=obj.transaction, - ) - - if conditions_data: - measure_creation_pattern = MeasureCreationPattern( - workbasket=workbasket, - base_date=obj.valid_between.lower, - ) - parser = DutySentenceParser.get( - obj.valid_between.lower, - component_output=MeasureConditionComponent, - ) - - # Loop over conditions_data, starting at 1 because component_sequence_number has to start at 1 - for component_sequence_number, condition_data in enumerate( - conditions_data, - start=1, - ): - # Create conditions and measure condition components, using instance as `dependent_measure` - measure_creation_pattern.create_condition_and_components( - condition_data, - component_sequence_number, - obj, - parser, - workbasket, - ) - def get_result_object(self, form): - obj = super().get_result_object(form) - form.instance = obj - self.create_conditions(obj) - form.save(commit=False) + instance = super().get_result_object(form) + form.instance = instance + current_workbasket = WorkBasket.current(self.request) + transaction = current_workbasket.get_current_transaction(self.request) + formset = self.get_context_data()["conditions_formset"] + new_measure = form.save(commit=False) + update_conditions(new_measure, transaction, current_workbasket, formset) - return obj + return instance class MeasureConfirmUpdate(MeasureMixin, TrackedModelDetailView): From ba787033bbc59406bd549268d106ff2c29068806 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 11 Nov 2022 11:02:54 +0000 Subject: [PATCH 11/13] Add test --- measures/tests/test_helpers.py | 51 ++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 measures/tests/test_helpers.py diff --git a/measures/tests/test_helpers.py b/measures/tests/test_helpers.py new file mode 100644 index 000000000..4c47a8a1c --- /dev/null +++ b/measures/tests/test_helpers.py @@ -0,0 +1,51 @@ +import pytest + +from common.tests import factories +from common.validators import UpdateType +from measures.helpers import create_conditions +from measures.helpers import update_measure + +pytestmark = pytest.mark.django_db + + +def test_create_conditions( + condition_codes, + certificates, + action_codes, + duty_sentence_parser, +): + measure = factories.MeasureFactory.create() + workbasket = factories.WorkBasketFactory.create() + transaction = workbasket.new_transaction() + conditions_data = [ + { + "condition_code": condition_codes["A"], + "duty_amount": 1.0, + "monetary_unit": None, + "condition_measurement": None, + "required_certificate": certificates["D017"], + "action": action_codes["01"], + "applicable_duty": "2%", + "condition_sid": "", + "reference_price": "1%", + "DELETE": False, + "update_type": UpdateType.CREATE, + }, + ] + + measure_data = {} + + assert not workbasket.measures.all() + + defaults = {"generating_regulation": measure.generating_regulation} + update_measure(measure, transaction, workbasket, measure_data, defaults) + create_conditions(measure, transaction, workbasket, conditions_data) + + assert workbasket.measures.count() == 1 + # measure, measure condition and measure component + assert workbasket.tracked_models.count() == 3 + assert workbasket.measures.first().transaction.workbasket == workbasket + + for mcondition in workbasket.measures.first().conditions.all(): + assert mcondition.transaction == transaction + assert mcondition.transaction.workbasket == workbasket From e22773da8f42196551bf79cd38aff1ca5288c07f Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 15 Nov 2022 14:45:13 +0000 Subject: [PATCH 12/13] Add and update tests --- measures/forms.py | 1 + measures/tests/test_views.py | 187 +++++++++++++++++++++++++++++++++-- measures/views.py | 31 +++++- 3 files changed, 206 insertions(+), 13 deletions(-) diff --git a/measures/forms.py b/measures/forms.py index 1c2d70a70..8e05d97fb 100644 --- a/measures/forms.py +++ b/measures/forms.py @@ -524,6 +524,7 @@ class MeasureConditionsWizardStepFormSet(FormSet): class MeasureConditionsEditWizardStepFormSet(FormSet): form = MeasureConditionsMultipleEditForm + prefix = "measure-conditions-formset" class MeasureForm(ValidityPeriodForm, BindNestedFormMixin, forms.ModelForm): diff --git a/measures/tests/test_views.py b/measures/tests/test_views.py index 64ffdf9aa..356ff95f7 100644 --- a/measures/tests/test_views.py +++ b/measures/tests/test_views.py @@ -21,6 +21,7 @@ from common.validators import UpdateType from common.views import TamatoListView from common.views import TrackedModelDetailMixin +from measures import constants from measures.business_rules import ME70 from measures.models import FootnoteAssociationMeasure from measures.models import Measure @@ -350,28 +351,54 @@ def test_measure_update_duty_sentence( post_data["update_type"] = 1 url = reverse("measure-ui-edit", args=(measure_form.instance.sid,)) client.force_login(valid_user) + + prev_transaction = Transaction.objects.last() + old_measure = Measure.objects.get(id=measure_form.data["id"]) + components = old_measure.components.approved_up_to_transaction( + prev_transaction, + ).filter( + component_measure__sid=measure_form.instance.sid, + ) + + prev_transaction_pks = [t.pk for t in list(Transaction.objects.all())] + + assert not components.exists() + response = client.post(url, data=post_data) assert response.status_code == 302 if update_data: - tx = Transaction.objects.last() - measure = Measure.objects.approved_up_to_transaction(tx).get( + last_tx = Transaction.objects.last() + measure = Measure.objects.approved_up_to_transaction(last_tx).get( sid=measure_form.instance.sid, ) - components = measure.components.approved_up_to_transaction(tx).filter( + components = measure.components.approved_up_to_transaction(last_tx).filter( component_measure__sid=measure_form.instance.sid, ) + # one transaction for the measure, one for its components + assert Transaction.objects.count() == len(prev_transaction_pks) + 2 + + new_transactions = Transaction.objects.all().exclude( + pk__in=prev_transaction_pks, + ) + + assert measure.transaction != old_measure.transaction + assert measure.transaction in new_transactions + assert components.first().transaction != old_measure.transaction + assert components.exists() assert components.count() == 1 assert components.first().duty_amount == 10.000 - assert components.first().transaction == measure.transaction + assert components.first().transaction in new_transactions # https://uktrade.atlassian.net/browse/TP2000-144 @patch("measures.forms.MeasureForm.save") +@patch("measures.views.update_conditions") def test_measure_form_save_called_on_measure_update( + update_conditions, save, client, valid_user, @@ -387,6 +414,7 @@ def test_measure_form_save_called_on_measure_update( client.force_login(valid_user) client.post(url, data=post_data) + update_conditions.assert_called() save.assert_called_with(commit=False) @@ -519,8 +547,8 @@ def test_measure_update_edit_conditions( sid=measure.sid, ) - # We expect one transaction for updating the measure and updating the condition, one for deleting a component and updating a component - assert Transaction.objects.count() == transaction_count + 2 + # We expect one transaction for updating the measure, one for updating the condition, one for deleting a component and updating a component + assert Transaction.objects.count() == transaction_count + 3 assert updated_measure.conditions.approved_up_to_transaction(tx).count() == 1 condition = updated_measure.conditions.approved_up_to_transaction(tx).first() @@ -610,8 +638,8 @@ def test_measure_update_remove_conditions( response = client.post(url, data=measure_edit_conditions_data) assert response.status_code == 302 - # We expect one transaction for the measure update and condition deletion - assert Transaction.objects.count() == transaction_count + 1 + # We expect two transactions for the measure update and condition deletion + assert Transaction.objects.count() == transaction_count + 2 tx = Transaction.objects.last() updated_measure = Measure.objects.approved_up_to_transaction(tx).get( @@ -1078,3 +1106,146 @@ def test_measure_form_creates_exclusions( assert not set( [e.excluded_geographical_area for e in measure.exclusions.all()], ).difference({excluded_country1, excluded_country2}) + + +@unittest.mock.patch("measures.parsers.DutySentenceParser") +def test_measure_update_form_wizard_finish( + mock_duty_sentence_parser, + valid_user_client, + workbasket, + additional_code, + regulation, + quota_order_number, + duty_sentence_parser, + erga_omnes, + measure_edit_conditions_data, +): + + mock_duty_sentence_parser.return_value = duty_sentence_parser + + measure1 = factories.MeasureFactory.create() + measure2 = factories.MeasureFactory.create() + footnote = factories.FootnoteFactory.create() + + session = valid_user_client.session + session.update( + { + "MEASURE_SELECTIONS": { + str(measure1.pk): True, + str(measure2.pk): True, + }, + }, + ) + session.update({"workbasket": {"id": workbasket.pk}}) + session.save() + + assert not workbasket.tracked_models.count() + + STEP_KEY = "measures_edit_wizard-current_step" + + wizard_data = [ + { + "data": { + STEP_KEY: constants.START, + "start-fields_to_edit": [ + constants.END_DATES, + constants.REGULATION_ID, + constants.QUOTA_ORDER_NUMBER, + constants.GEOGRAPHICAL_AREA, + constants.DUTIES, + constants.ADDITIONAL_CODE, + constants.CONDITIONS, + constants.FOOTNOTES, + ], + }, + "next_step": constants.END_DATES, + }, + { + "data": { + STEP_KEY: constants.END_DATES, + "end_dates-end_date_0": "01", + "end_dates-end_date_1": "01", + "end_dates-end_date_2": "2100", + }, + "next_step": constants.REGULATION_ID, + }, + { + "data": { + STEP_KEY: constants.REGULATION_ID, + "regulation_id-generating_regulation": regulation.pk, + }, + "next_step": constants.QUOTA_ORDER_NUMBER, + }, + { + "data": { + STEP_KEY: constants.QUOTA_ORDER_NUMBER, + "quota_order_number-order_number": quota_order_number.pk, + }, + "next_step": constants.GEOGRAPHICAL_AREA, + }, + { + "data": { + STEP_KEY: constants.GEOGRAPHICAL_AREA, + "geographical_area-geo_area": "ERGA_OMNES", + }, + "next_step": constants.DUTIES, + }, + { + "data": { + STEP_KEY: constants.DUTIES, + "duties-duties": "3.5%+++11+GBP+/+100+kg", + }, + "next_step": constants.ADDITIONAL_CODE, + }, + { + "data": { + STEP_KEY: constants.ADDITIONAL_CODE, + "additional_code-additional_code": additional_code.pk, + }, + "next_step": constants.CONDITIONS, + }, + { + "data": { + STEP_KEY: constants.CONDITIONS, + **measure_edit_conditions_data, + }, + "next_step": constants.FOOTNOTES, + }, + { + "data": { + STEP_KEY: constants.FOOTNOTES, + "form-0-footnote": footnote.pk, + }, + "next_step": constants.SUMMARY, + }, + { + "data": { + STEP_KEY: constants.SUMMARY, + }, + "next_step": constants.COMPLETE, + }, + ] + for step_data in wizard_data: + url = reverse( + "measure-ui-edit-multiple", + kwargs={"step": step_data["data"][STEP_KEY]}, + ) + response = valid_user_client.get(url) + assert response.status_code == 200 + + response = valid_user_client.post(url, step_data["data"]) + assert response.status_code == 302 + + assert response.url == reverse( + "measure-ui-edit-multiple", + kwargs={"step": step_data["next_step"]}, + ) + + complete_response = valid_user_client.get(response.url) + + # 2 measures edited with: + # 1 condition + # 1 footnote + assert workbasket.tracked_models.count() == 6 + + assert complete_response.status_code == 200 diff --git a/measures/views.py b/measures/views.py index 631ea4d31..ac577c47e 100644 --- a/measures/views.py +++ b/measures/views.py @@ -290,9 +290,25 @@ def done(self, form_list, **kwargs): for measure in self.measures: defaults = {"generating_regulation": measure.generating_regulation} - update_measure(measure, tx, current_workbasket, cleaned_data, defaults) - create_conditions(measure, current_workbasket, conditions_data) - update_measure_footnotes(measure, tx, current_workbasket, footnote_pks) + new_measure = update_measure( + measure, + tx, + current_workbasket, + cleaned_data, + defaults, + ) + create_conditions( + measure, + new_measure.transaction, + current_workbasket, + conditions_data, + ) + update_measure_footnotes( + new_measure, + new_measure.transaction, + current_workbasket, + footnote_pks, + ) edited_measures.append(measure) @@ -599,10 +615,15 @@ def get_result_object(self, form): instance = super().get_result_object(form) form.instance = instance current_workbasket = WorkBasket.current(self.request) - transaction = current_workbasket.get_current_transaction(self.request) + current_workbasket.get_current_transaction(self.request) formset = self.get_context_data()["conditions_formset"] new_measure = form.save(commit=False) - update_conditions(new_measure, transaction, current_workbasket, formset) + update_conditions( + instance, + new_measure.transaction, + current_workbasket, + formset, + ) return instance From 3e9faecaedd3d5ecf5ea80b23c964600a9f07199 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 15 Nov 2022 15:11:19 +0000 Subject: [PATCH 13/13] Remove unused method --- measures/views.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/measures/views.py b/measures/views.py index ac577c47e..9104df771 100644 --- a/measures/views.py +++ b/measures/views.py @@ -236,13 +236,6 @@ def _session_store(self): def measures(self): return Measure.objects.filter(pk__in=self._session_store.data.keys()) - def get_initial(self): - store = SessionStore( - self.request, - "MEASURE_SELECTIONS", - ) - return store.data.copy() - def get_form_kwargs(self, step): kwargs = {} if step == constants.DUTIES: