From a114af04b412d633d5005c9ddf7a76e94dc4d5a1 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 15:26:36 +0100 Subject: [PATCH 01/16] Add Async edit setting --- settings/common.py | 1 + 1 file changed, 1 insertion(+) diff --git a/settings/common.py b/settings/common.py index b3728ef59..7fd4957e3 100644 --- a/settings/common.py +++ b/settings/common.py @@ -917,3 +917,4 @@ # Asynchronous / background (bulk) object creation and editing config. MEASURES_ASYNC_CREATION = is_truthy(os.environ.get("MEASURES_ASYNC_CREATION", "true")) +MEASURES_ASYNC_EDIT = is_truthy(os.environ.get("MEASURES_ASYNC_EDIT", "true")) \ No newline at end of file From 2f5e5da26d37e19ad3976f70f2167a57e0546f63 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 15:36:10 +0100 Subject: [PATCH 02/16] Add serializable form mixin to all wizard forms --- measures/forms/wizard.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/measures/forms/wizard.py b/measures/forms/wizard.py index 2e68915ec..2f2a08d8d 100644 --- a/measures/forms/wizard.py +++ b/measures/forms/wizard.py @@ -781,7 +781,7 @@ def __init__(self, *args, **kwargs): ) -class MeasureStartDateForm(forms.Form): +class MeasureStartDateForm(forms.Form, SerializableFormMixin): start_date = DateInputFieldFixed( label="Start date", help_text="For example, 27 3 2008", @@ -820,7 +820,7 @@ def clean(self): return cleaned_data -class MeasureEndDateForm(forms.Form): +class MeasureEndDateForm(forms.Form, SerializableFormMixin): end_date = DateInputFieldFixed( label="End date", help_text="For example, 27 3 2008", @@ -862,7 +862,7 @@ def clean(self): return cleaned_data -class MeasureRegulationForm(forms.Form): +class MeasureRegulationForm(forms.Form, SerializableFormMixin): generating_regulation = AutoCompleteField( label="Regulation ID", help_text="Select the regulation which provides the legal basis for the measures.", @@ -889,7 +889,7 @@ def __init__(self, *args, **kwargs): ) -class MeasureDutiesForm(forms.Form): +class MeasureDutiesForm(forms.Form, SerializableFormMixin): duties = forms.CharField( label="Duties", help_text="Enter the duty that applies to the measures.", @@ -965,7 +965,7 @@ def __init__(self, *args, **kwargs): ) -class MeasureGeographicalAreaExclusionsFormSet(FormSet): +class MeasureGeographicalAreaExclusionsFormSet(FormSet, SerializableFormMixin): """Allows editing the geographical area exclusions of multiple measures in `MeasureEditWizard`.""" From 3f59a4f198574d0736f464bcb1d1dff91ec0d709 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 15:41:12 +0100 Subject: [PATCH 03/16] Add MeasuresBulkEditor model --- .../migrations/0017_measuresbulkeditor.py | 84 +++++++++++++++++++ measures/models/bulk_processing.py | 63 ++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 measures/migrations/0017_measuresbulkeditor.py diff --git a/measures/migrations/0017_measuresbulkeditor.py b/measures/migrations/0017_measuresbulkeditor.py new file mode 100644 index 000000000..e3adb412a --- /dev/null +++ b/measures/migrations/0017_measuresbulkeditor.py @@ -0,0 +1,84 @@ +# Generated by Django 4.2.14 on 2024-07-29 14:39 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_fsm +import measures.models.bulk_processing + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("workbaskets", "0008_datarow_dataupload"), + ("measures", "0016_measuresbulkcreator"), + ] + + operations = [ + migrations.CreateModel( + name="MeasuresBulkEditor", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "task_id", + models.CharField(blank=True, max_length=50, null=True, unique=True), + ), + ( + "processing_state", + django_fsm.FSMField( + choices=[ + ("AWAITING_PROCESSING", "Awaiting processing"), + ("CURRENTLY_PROCESSING", "Currently processing"), + ("SUCCESSFULLY_PROCESSED", "Successfully processed"), + ("FAILED_PROCESSING", "Failed processing"), + ("CANCELLED", "Cancelled"), + ], + db_index=True, + default="AWAITING_PROCESSING", + editable=False, + max_length=50, + protected=True, + ), + ), + ( + "successfully_processed_count", + models.PositiveIntegerField(default=0), + ), + ("form_data", models.JSONField()), + ("form_kwargs", models.JSONField()), + ("selected_measures", models.JSONField()), + ( + "user", + models.ForeignKey( + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "workbasket", + models.ForeignKey( + editable=False, + null=True, + on_delete=measures.models.bulk_processing.REVOKE_TASKS_AND_SET_NULL, + to="workbaskets.workbasket", + ), + ), + ], + options={ + "abstract": False, + }, + ), + ] diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index aef1d8763..1a3c70ff4 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -414,3 +414,66 @@ def _log_form_errors(self, form_class, form_or_formset) -> None: for form_errors in errors: for error_key, error_values in form_errors.items(): logger.error(f"{error_key}: {error_values}") + + +class MeasuresBulkEditorManager(models.Manager): + """Model Manager for MeasuresBulkEditor models.""" + + def create( + self, + form_data: Dict, + form_kwargs: Dict, + workbasket, + user, + selected_measures, + **kwargs, + ) -> "MeasuresBulkCreator": + """Create and save an instance of MeasuresBulkEditor.""" + + return super().create( + form_data=form_data, + form_kwargs=form_kwargs, + workbasket=workbasket, + user=user, + selected_measures=selected_measures, + **kwargs, + ) + + +class MeasuresBulkEditor(BulkProcessor): + """ + Model class used to bulk edit Measures instances from serialized form + data. + The stored form data is serialized and deserialized by Forms that subclass + SerializableFormMixin. + """ + + objects = MeasuresBulkEditorManager() + + form_data = models.JSONField() + """Dictionary of all Form.data, used to reconstruct bound Form instances as + if the form data had been sumbitted by the user within the measure wizard + process.""" + + form_kwargs = models.JSONField() + """Dictionary of all form init data, excluding a form's `data` param (which + is preserved via this class's `form_data` attribute).""" + + selected_measures = models.JSONField() + """List of all measures that have been selected for bulk editing.""" + + workbasket = models.ForeignKey( + "workbaskets.WorkBasket", + on_delete=REVOKE_TASKS_AND_SET_NULL, + null=True, + editable=False, + ) + """The workbasket with which created measures are associated.""" + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=SET_NULL, + null=True, + editable=False, + ) + """The user who submitted the task to create measures.""" \ No newline at end of file From 3e5190b79da57abc12973576006c97e2341ec8fe Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 17:16:52 +0100 Subject: [PATCH 04/16] Add shared functions to SerializableFormMixin --- common/forms.py | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/common/forms.py b/common/forms.py index 4189ffe0a..1c13c6977 100644 --- a/common/forms.py +++ b/common/forms.py @@ -892,3 +892,72 @@ def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: representation. """ return {} + + def get_data_form_list(self) -> dict: + """ + Returns a form list based on form_list, conditionally including only + those items as per condition_list and also appearing in data_form_list. + + The list is generated dynamically because conditions in condition_list + may be dynamic. + + Essentially, version of `WizardView.get_form_list()` filtering in only + those list items appearing in `data_form_list`. + """ + data_form_keys = [key for key, form in self.data_form_list] + return { + form_key: form_class + for form_key, form_class in self.get_form_list().items() + if form_key in data_form_keys + } + + def all_serializable_form_data(self) -> Dict: + """ + Returns serializable data for all wizard steps. + + This is a re-implementation of + MeasureCreateWizard.get_all_cleaned_data(), but using self.data after + is_valid() has been successfully run. + """ + + all_data = {} + + for form_key in self.get_data_form_list().keys(): + all_data[form_key] = self.serializable_form_data_for_step(form_key) + + return all_data + + def serializable_form_data_for_step(self, step) -> Dict: + """ + Returns serializable data for a wizard step. + + This is a re-implementation of WizardView.get_cleaned_data_for_step(), + returning the serializable version of data in place of the form's + regular cleaned_data. + """ + + form_obj = self.get_form( + step=step, + data=self.storage.get_step_data(step), + files=self.storage.get_step_files(step), + ) + + return form_obj.serializable_data(remove_key_prefix=step) + + def all_serializable_form_kwargs(self) -> Dict: + """Returns serializable kwargs for all wizard steps.""" + + all_kwargs = {} + + for form_key in self.get_data_form_list().keys(): + all_kwargs[form_key] = self.serializable_form_kwargs_for_step(form_key) + + return all_kwargs + + def serializable_form_kwargs_for_step(self, step) -> Dict: + """Returns serializable kwargs for a wizard step.""" + + form_kwargs = self.get_form_kwargs(step) + form_class = self.form_list[step] + + return form_class.serializable_init_kwargs(form_kwargs) \ No newline at end of file From 49276e5f46e3ace0cc45e8f3690990d87e41773b Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 17:18:12 +0100 Subject: [PATCH 05/16] Add MeasuresBulkEditor for exporting --- measures/models/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/measures/models/__init__.py b/measures/models/__init__.py index a4c641802..f49a0d12e 100644 --- a/measures/models/__init__.py +++ b/measures/models/__init__.py @@ -1,5 +1,6 @@ from measures.models.bulk_processing import BulkProcessor from measures.models.bulk_processing import MeasuresBulkCreator +from measures.models.bulk_processing import MeasuresBulkEditor from measures.models.bulk_processing import ProcessingState from measures.models.tracked_models import AdditionalCodeTypeMeasureType from measures.models.tracked_models import DutyExpression @@ -23,6 +24,7 @@ # - Classes exported from bulk_processing.py. "BulkProcessor", "MeasuresBulkCreator", + "MeasuresBulkEditor", "ProcessingState", # - Classes exported from tracked_model.py. "AdditionalCodeTypeMeasureType", From 77db17b87390e7d63d366f8ce1d6f1821a089080 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 17:18:56 +0100 Subject: [PATCH 06/16] split done function into sync and async --- measures/models/bulk_processing.py | 124 ++++++++++++++++++++++++++++- measures/views/wizard.py | 42 +++++++++- 2 files changed, 164 insertions(+), 2 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 1a3c70ff4..8a71854f2 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -476,4 +476,126 @@ class MeasuresBulkEditor(BulkProcessor): null=True, editable=False, ) - """The user who submitted the task to create measures.""" \ No newline at end of file + """The user who submitted the task to create measures.""" + + def schedule_task(self) -> AsyncResult: + """Implementation of base class method.""" + + from measures.tasks import bulk_edit_measures + + async_result = bulk_edit_measures.apply_async( + kwargs={ + "measures_bulk_editor_pk": self.pk, + }, + countdown=1, + ) + self.task_id = async_result.id + self.save() + + logger.info( + f"Measure bulk edit scheduled on task with ID {async_result.id}" + f"using MeasuresBulkEditor.pk={self.pk}.", + ) + + return async_result + + @atomic + def edit_measures(self) -> Iterable[Measure]: + logger.info("Bulk editing measures commencing") + logger.info(f"Form Data: {self.form_data}") + logger.info(f"Duties: {self.form_data['duties']['duties']}") + + form_data_keys = list(self.form_data.keys()) + new_data = {} + for key in form_data_keys: + match key: + case "start_date": + new_data.append( + {"new_start_date": TaricDateRange(datetime.date( + self.form_data["start_date"]["start_date_0"], + self.form_data["start_date"]["start_date_1"], + self.form_data["start_date"]["start_date_2"] + ) + } + ) + + # assign the simple values from the form data + # new_start_date = TaricDateRange(datetime.date(2020, 1, 6)) if self.form_data["start_date"]["start_date"] else None + # new_end_date = self.form_data["end_date"]["end_date"] if self.form_data["end_date"]["end_date"] else None + # new_duties = self.form_data["duties"]["duties"] if self.form_data["duties"]["duties"] else None + + # Assign the foreign keys from the form data + + + if self.form_data["quota_order_number"]["order_number"]: + new_quota_order_number = QuotaOrderNumber.objects.get(pk=self.form_data["quota_order_number"]["order_number"]) + else: + new_quota_order_number = None + + if self.form_data["regulation"]["generating_regulation"]: + new_generating_regulation = Regulation.objects.get(regulation_id=self.form_data["regulation"]["generating_regulation"]) + else: + new_generating_regulation = None + + if self.form_data["geographical_area_exclusions"]["geographical_area_exclusions"]: + new_exclusions = [] + for exclusion in self.form_data["geographical_area_exclusions"]["geographical_area_exclusions"]: + new_exclusions.append(GeographicalArea.objects.get(pk=exclusion)) + else: + new_exclusions = None + + + logger.info(f"new start date: {new_start_date}") + logger.info(f"new end date: {new_end_date}") + logger.info(f"new duties: {new_duties}") + logger.info(f"new order number: {new_quota_order_number}") + logger.info(f"new generating regulation: {new_generating_regulation}") + logger.info(f"new geo exclusions: {new_exclusions}") + # if selected_measures: + # for measure in selected_measures: + # new_measure = measure.new_version( + # workbasket=self.workbasket, + # update_type=UpdateType.UPDATE, + # valid_between=TaricDateRange( + # lower=( + # new_start_date + # if new_start_date + # else measure.valid_between.lower + # ), + # upper=( + # new_end_date + # if new_end_date is not False + # else measure.valid_between.upper + # ), + # ), + # order_number=( + # new_quota_order_number + # if new_quota_order_number + # else measure.order_number + # ), + # generating_regulation=( + # new_generating_regulation + # if new_generating_regulation + # else measure.generating_regulation + # ), + # ) + # self.update_measure_components( + # measure=new_measure, + # duties=new_duties, + # workbasket=self.workbasket, + # ) + # self.update_measure_condition_components( + # measure=new_measure, + # workbasket=self.workbasket, + # ) + # self.update_measure_excluded_geographical_areas( + # edited="geographical_area_exclusions" + # in cleaned_data.get("fields_to_edit", []), + # measure=new_measure, + # exclusions=new_exclusions, + # workbasket=self.workbasket, + # ) + # self.update_measure_footnote_associations( + # measure=new_measure, + # workbasket=self.workbasket, + # ) \ No newline at end of file diff --git a/measures/views/wizard.py b/measures/views/wizard.py index dfa156a2b..aa8955afa 100644 --- a/measures/views/wizard.py +++ b/measures/views/wizard.py @@ -2,6 +2,7 @@ from typing import Dict from typing import List +from common.forms import SerializableFormMixin from crispy_forms_gds.helper import FormHelper from django.conf import settings from django.contrib.auth.mixins import PermissionRequiredMixin @@ -41,6 +42,7 @@ class MeasureEditWizard( PermissionRequiredMixin, MeasureSelectionQuerysetMixin, NamedUrlSessionWizardView, + SerializableFormMixin, ): """ Multipart form wizard for editing multiple measures. @@ -51,7 +53,7 @@ class MeasureEditWizard( storage_name = "measures.wizard.MeasureEditSessionStorage" permission_required = ["common.change_trackedmodel"] - form_list = [ + data_form_list = [ (START, forms.MeasuresEditFieldsForm), (MeasureEditSteps.START_DATE, forms.MeasureStartDateForm), (MeasureEditSteps.END_DATE, forms.MeasureEndDateForm), @@ -63,6 +65,14 @@ class MeasureEditWizard( forms.MeasureGeographicalAreaExclusionsFormSet, ), ] + """Forms in this wizard's steps that collect user data.""" + + form_list = [ + (START, forms.MeasuresEditFieldsForm), + *data_form_list, + ] + """All Forms in this wizard's steps, including both those that collect user + data and those that don't.""" templates = { START: "measures/edit-multiple-start.jinja", @@ -94,6 +104,10 @@ class MeasureEditWizard( }, } + @property + def workbasket(self) -> WorkBasket: + return WorkBasket.current(self.request) + def get_template_names(self): return self.templates.get( self.steps.current, @@ -231,6 +245,32 @@ def update_measure_footnote_associations(self, measure, workbasket): ) def done(self, form_list, **kwargs): + if settings.MEASURES_ASYNC_EDIT: + return self.async_done(form_list, **kwargs) + else: + return self.sync_done(form_list, **kwargs) + + def async_done(self, form_list, **kwargs): + logger.info("Editing measures asynchronously.") + + serializable_data = self.all_serializable_form_data() + serializable_form_kwargs = self.all_serializable_form_kwargs() + + selected_measures = [] + for measure in self.get_queryset(): + selected_measures.append(measure.id) + + measures_bulk_editor = models.MeasuresBulkEditor.objects.create( + form_data=serializable_data, + form_kwargs=serializable_form_kwargs, + workbasket=self.workbasket, + user=self.request.user, + selected_measures=selected_measures, + ) + # self.session_store.clear() # TODO: Is this the best point to clear the session store? + measures_bulk_editor.schedule_task() + + def sync_done(self, form_list, **kwargs): cleaned_data = self.get_all_cleaned_data() selected_measures = self.get_queryset() workbasket = WorkBasket.current(self.request) From 0d2315f5f8d0123896288f55edf9c6f5e7f7aa91 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 29 Jul 2024 17:25:53 +0100 Subject: [PATCH 07/16] Add task - forgot to add it oops --- measures/tasks.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/measures/tasks.py b/measures/tasks.py index ed61c3f6d..8d8a391f1 100644 --- a/measures/tasks.py +++ b/measures/tasks.py @@ -2,6 +2,7 @@ from common.celery import app from measures.models import MeasuresBulkCreator +from measures.models import MeasuresBulkEditor logger = logging.getLogger(__name__) @@ -43,3 +44,35 @@ def bulk_create_measures(measures_bulk_creator_pk: int) -> None: f"succeeded but created no measures in " f"WorkBasket({measures_bulk_creator.workbasket.pk}).", ) + +@app.task +def bulk_edit_measures(measures_bulk_editor_pk: int) -> None: + """Bulk edit measures from serialized measures form data saved within an + instance of MeasuresBulkEditor.""" + + measures_bulk_editor = MeasuresBulkEditor.objects.get(pk=measures_bulk_editor_pk) + measures_bulk_editor.begin_processing() + measures_bulk_editor.save() + + try: + measures = measures_bulk_editor.edit_measures() + except Exception as e: + measures_bulk_editor.processing_failed() + measures_bulk_editor.save() + logger.error( + f"MeasuresBulkCreator({measures_bulk_editor.pk}) task failed " + f"attempting to edit measures in " + f"WorkBasket({measures_bulk_editor.workbasket.pk}).", + ) + raise e + + measures_bulk_editor.processing_succeeded() + measures_bulk_editor.successfully_processed_count = len(measures) + measures_bulk_editor.save() + + if measures: + logger.info( + f"MeasuresBulkEditoror({measures_bulk_editor.pk}) task " + f"succeeded in editing {len(measures)} Measures in " + f"WorkBasket({measures_bulk_editor.workbasket.pk}).", + ) \ No newline at end of file From 8f2acb124c0bf19d65f316154b137fb19622cd0f Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Tue, 30 Jul 2024 17:01:54 +0100 Subject: [PATCH 08/16] Wip - save progress --- measures/models/bulk_processing.py | 178 +++++++++++++++-------------- measures/views/wizard.py | 1 - settings/common.py | 3 + 3 files changed, 98 insertions(+), 84 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 8a71854f2..4af163052 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -1,5 +1,6 @@ import json import logging +import datetime from typing import Dict from typing import Iterable from typing import Tuple @@ -17,7 +18,12 @@ from common.celery import app from common.models.mixins import TimestampedMixin from common.models.utils import override_current_transaction +from common.util import TaricDateRange +from common.validators import UpdateType from measures.models.tracked_models import Measure +from quotas.models import QuotaOrderNumber +from regulations.models import Regulation +from geo_areas.models import GeographicalArea logger = logging.getLogger(__name__) @@ -503,9 +509,10 @@ def schedule_task(self) -> AsyncResult: def edit_measures(self) -> Iterable[Measure]: logger.info("Bulk editing measures commencing") logger.info(f"Form Data: {self.form_data}") - logger.info(f"Duties: {self.form_data['duties']['duties']}") form_data_keys = list(self.form_data.keys()) + logger.info(f"Keys: {form_data_keys}") + new_data = {} for key in form_data_keys: match key: @@ -515,87 +522,92 @@ def edit_measures(self) -> Iterable[Measure]: self.form_data["start_date"]["start_date_0"], self.form_data["start_date"]["start_date_1"], self.form_data["start_date"]["start_date_2"] - ) + )) } ) - - # assign the simple values from the form data - # new_start_date = TaricDateRange(datetime.date(2020, 1, 6)) if self.form_data["start_date"]["start_date"] else None - # new_end_date = self.form_data["end_date"]["end_date"] if self.form_data["end_date"]["end_date"] else None - # new_duties = self.form_data["duties"]["duties"] if self.form_data["duties"]["duties"] else None - - # Assign the foreign keys from the form data - - - if self.form_data["quota_order_number"]["order_number"]: - new_quota_order_number = QuotaOrderNumber.objects.get(pk=self.form_data["quota_order_number"]["order_number"]) - else: - new_quota_order_number = None - - if self.form_data["regulation"]["generating_regulation"]: - new_generating_regulation = Regulation.objects.get(regulation_id=self.form_data["regulation"]["generating_regulation"]) - else: - new_generating_regulation = None - - if self.form_data["geographical_area_exclusions"]["geographical_area_exclusions"]: - new_exclusions = [] - for exclusion in self.form_data["geographical_area_exclusions"]["geographical_area_exclusions"]: - new_exclusions.append(GeographicalArea.objects.get(pk=exclusion)) - else: - new_exclusions = None - - - logger.info(f"new start date: {new_start_date}") - logger.info(f"new end date: {new_end_date}") - logger.info(f"new duties: {new_duties}") - logger.info(f"new order number: {new_quota_order_number}") - logger.info(f"new generating regulation: {new_generating_regulation}") - logger.info(f"new geo exclusions: {new_exclusions}") - # if selected_measures: - # for measure in selected_measures: - # new_measure = measure.new_version( - # workbasket=self.workbasket, - # update_type=UpdateType.UPDATE, - # valid_between=TaricDateRange( - # lower=( - # new_start_date - # if new_start_date - # else measure.valid_between.lower - # ), - # upper=( - # new_end_date - # if new_end_date is not False - # else measure.valid_between.upper - # ), - # ), - # order_number=( - # new_quota_order_number - # if new_quota_order_number - # else measure.order_number - # ), - # generating_regulation=( - # new_generating_regulation - # if new_generating_regulation - # else measure.generating_regulation - # ), - # ) - # self.update_measure_components( - # measure=new_measure, - # duties=new_duties, - # workbasket=self.workbasket, - # ) - # self.update_measure_condition_components( - # measure=new_measure, - # workbasket=self.workbasket, - # ) - # self.update_measure_excluded_geographical_areas( - # edited="geographical_area_exclusions" - # in cleaned_data.get("fields_to_edit", []), - # measure=new_measure, - # exclusions=new_exclusions, - # workbasket=self.workbasket, - # ) - # self.update_measure_footnote_associations( - # measure=new_measure, - # workbasket=self.workbasket, - # ) \ No newline at end of file + case "end_date": + new_data.append( + {"new_end_date": TaricDateRange(datetime.date( + self.form_data["end_date"]["end_date_0"], + self.form_data["end_date"]["end_date_1"], + self.form_data["end_date"]["end_date_2"] + )) + } + ) + case "duties": + new_data.append( + {"new_duties": self.form_data["duties"]["duties"]} + ) + case "quota_order_number": + new_data.append( + {"new_order_number": QuotaOrderNumber.objects.get(pk=self.form_data["quota_order_number"]["order_number"])} + ) + case "regulation": + new_data.append( + {"new_generating_regulation": Regulation.objects.get(regulation_id=self.form_data["regulation"]["generating_regulation"])} + ) + # FIX THIS!!! + case "geographical_area_exclusions": + new_exclusions = [] + new_data.append( + {"new_geo_area_exclusions": self.form_data["duties"]["duties"]} + ) + # !!! + + logger.info(f"new start date: {new_data['new_start_date']}") + logger.info(f"new end date: {new_data['new_end_date']}") + logger.info(f"new duties: {new_data['new_duties']}") + logger.info(f"new order number: {new_data['new_quota_order_number']}") + logger.info(f"new generating regulation: {new_data['new_generating_regulation']}") + logger.info(f"new geo exclusions: {new_data['new_exclusions']}") + + deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) + + if self.selected_measures: + for measure in deserialized_selected_measures: + new_measure = measure.new_version( + workbasket=self.workbasket, + update_type=UpdateType.UPDATE, + valid_between=TaricDateRange( + lower=( + new_data['new_start_date'] + if new_data['new_start_date'] + else measure.valid_between.lower + ), + upper=( + new_data['new_end_date'] + if new_data['new_end_date'] is not False + else measure.valid_between.upper + ), + ), + order_number=( + new_data['new_quota_order_number'] + if new_data['new_quota_order_number'] + else measure.order_number + ), + generating_regulation=( + new_data['new_generating_regulation'] + if new_data['new_generating_regulation'] + else measure.generating_regulation + ), + ) + self.update_measure_components( + measure=new_measure, + duties=new_data['new_duties'], + workbasket=self.workbasket, + ) + self.update_measure_condition_components( + measure=new_data['new_measure'], + workbasket=self.workbasket, + ) + self.update_measure_excluded_geographical_areas( + edited="geographical_area_exclusions" + in cleaned_data.get("fields_to_edit", []), + measure=new_measure, + exclusions=new_exclusions, + workbasket=self.workbasket, + ) + self.update_measure_footnote_associations( + measure=new_measure, + workbasket=self.workbasket, + ) \ No newline at end of file diff --git a/measures/views/wizard.py b/measures/views/wizard.py index aa8955afa..de2a7017b 100644 --- a/measures/views/wizard.py +++ b/measures/views/wizard.py @@ -54,7 +54,6 @@ class MeasureEditWizard( permission_required = ["common.change_trackedmodel"] data_form_list = [ - (START, forms.MeasuresEditFieldsForm), (MeasureEditSteps.START_DATE, forms.MeasureStartDateForm), (MeasureEditSteps.END_DATE, forms.MeasureEndDateForm), (MeasureEditSteps.QUOTA_ORDER_NUMBER, forms.MeasureQuotaOrderNumberForm), diff --git a/settings/common.py b/settings/common.py index 7fd4957e3..f398ca328 100644 --- a/settings/common.py +++ b/settings/common.py @@ -663,6 +663,9 @@ "measures.tasks.bulk_create_measures": { "queue": "bulk-create", }, + "measures.tasks.bulk_edit_measures": { + "queue": "bulk-create", + }, } SQLITE_EXCLUDED_APPS = [ From 1fc57f4518212e45c04e4e50f21fa53c683d259f Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Fri, 9 Aug 2024 12:59:23 +0100 Subject: [PATCH 09/16] Add serialize and deserialize kwarg functions to each edit form --- measures/forms/wizard.py | 99 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/measures/forms/wizard.py b/measures/forms/wizard.py index 2f2a08d8d..9c00a3858 100644 --- a/measures/forms/wizard.py +++ b/measures/forms/wizard.py @@ -52,6 +52,9 @@ from . import MeasureFootnotesForm from . import MeasureGeoAreaInitialDataMixin +import logging +logger = logging.getLogger(__name__) + class MeasureConditionsWizardStepForm(MeasureConditionsFormMixin): # override methods that use form kwargs @@ -806,7 +809,6 @@ def __init__(self, *args, **kwargs): def clean(self): cleaned_data = super().clean() - if "start_date" in cleaned_data: start_date = cleaned_data["start_date"] for measure in self.selected_measures: @@ -818,6 +820,30 @@ def clean(self): ) return cleaned_data + + @classmethod + def serializable_init_kwargs(cls, kwargs: Dict) -> Dict: + selected_measures = kwargs.get("selected_measures") + selected_measures_pks = [] + for measure in selected_measures: + selected_measures_pks.append(measure.id) + + serializable_kwargs = { + "selected_measures": selected_measures_pks, + } + + return serializable_kwargs + + @classmethod + def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: + serialized_selected_measures_pks = form_kwargs.get("selected_measures") + deserialized_selected_measures = models.Measure.objects.filter(pk__in=serialized_selected_measures_pks) + + kwargs = { + "selected_measures": deserialized_selected_measures, + } + + return kwargs class MeasureEndDateForm(forms.Form, SerializableFormMixin): @@ -860,6 +886,30 @@ def clean(self): cleaned_data["end_date"] = None return cleaned_data + + @classmethod + def serializable_init_kwargs(cls, kwargs: Dict) -> Dict: + selected_measures = kwargs.get("selected_measures") + selected_measures_pks = [] + for measure in selected_measures: + selected_measures_pks.append(measure.id) + + serializable_kwargs = { + "selected_measures": selected_measures_pks, + } + + return serializable_kwargs + + @classmethod + def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: + serialized_selected_measures_pks = form_kwargs.get("selected_measures") + deserialized_selected_measures = models.Measure.objects.filter(pk__in=serialized_selected_measures_pks) + + kwargs = { + "selected_measures": deserialized_selected_measures, + } + + return kwargs class MeasureRegulationForm(forms.Form, SerializableFormMixin): @@ -887,6 +937,29 @@ def __init__(self, *args, **kwargs): data_prevent_double_click="true", ), ) + @classmethod + def serializable_init_kwargs(cls, kwargs: Dict) -> Dict: + selected_measures = kwargs.get("selected_measures") + selected_measures_pks = [] + for measure in selected_measures: + selected_measures_pks.append(measure.id) + + serializable_kwargs = { + "selected_measures": selected_measures_pks, + } + + return serializable_kwargs + + @classmethod + def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: + serialized_selected_measures_pks = form_kwargs.get("selected_measures") + deserialized_selected_measures = models.Measure.objects.filter(pk__in=serialized_selected_measures_pks) + + kwargs = { + "selected_measures": deserialized_selected_measures, + } + + return kwargs class MeasureDutiesForm(forms.Form, SerializableFormMixin): @@ -920,6 +993,30 @@ def __init__(self, *args, **kwargs): data_prevent_double_click="true", ), ) + + @classmethod + def serializable_init_kwargs(cls, kwargs: Dict) -> Dict: + selected_measures = kwargs.get("selected_measures") + selected_measures_pks = [] + for measure in selected_measures: + selected_measures_pks.append(measure.id) + + serializable_kwargs = { + "selected_measures": selected_measures_pks, + } + + return serializable_kwargs + + @classmethod + def deserialize_init_kwargs(cls, form_kwargs: Dict) -> Dict: + serialized_selected_measures_pks = form_kwargs.get("selected_measures") + deserialized_selected_measures = models.Measure.objects.filter(pk__in=serialized_selected_measures_pks) + + kwargs = { + "selected_measures": deserialized_selected_measures, + } + + return kwargs def clean(self): cleaned_data = super().clean() From 586cbcdf2b25f4dad329a3bbfd6ed0ee9921dc93 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Fri, 9 Aug 2024 15:56:52 +0100 Subject: [PATCH 10/16] flesh out the editing process - WIP --- measures/models/bulk_processing.py | 160 ++++++++++++++++------------- measures/views/wizard.py | 8 +- 2 files changed, 94 insertions(+), 74 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 4af163052..6bb5bd947 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -1,6 +1,5 @@ import json import logging -import datetime from typing import Dict from typing import Iterable from typing import Tuple @@ -21,9 +20,6 @@ from common.util import TaricDateRange from common.validators import UpdateType from measures.models.tracked_models import Measure -from quotas.models import QuotaOrderNumber -from regulations.models import Regulation -from geo_areas.models import GeographicalArea logger = logging.getLogger(__name__) @@ -422,6 +418,7 @@ def _log_form_errors(self, form_class, form_or_formset) -> None: logger.error(f"{error_key}: {error_values}") + class MeasuresBulkEditorManager(models.Manager): """Model Manager for MeasuresBulkEditor models.""" @@ -507,97 +504,56 @@ def schedule_task(self) -> AsyncResult: @atomic def edit_measures(self) -> Iterable[Measure]: - logger.info("Bulk editing measures commencing") - logger.info(f"Form Data: {self.form_data}") - - form_data_keys = list(self.form_data.keys()) - logger.info(f"Keys: {form_data_keys}") - - new_data = {} - for key in form_data_keys: - match key: - case "start_date": - new_data.append( - {"new_start_date": TaricDateRange(datetime.date( - self.form_data["start_date"]["start_date_0"], - self.form_data["start_date"]["start_date_1"], - self.form_data["start_date"]["start_date_2"] - )) - } - ) - case "end_date": - new_data.append( - {"new_end_date": TaricDateRange(datetime.date( - self.form_data["end_date"]["end_date_0"], - self.form_data["end_date"]["end_date_1"], - self.form_data["end_date"]["end_date_2"] - )) - } - ) - case "duties": - new_data.append( - {"new_duties": self.form_data["duties"]["duties"]} - ) - case "quota_order_number": - new_data.append( - {"new_order_number": QuotaOrderNumber.objects.get(pk=self.form_data["quota_order_number"]["order_number"])} - ) - case "regulation": - new_data.append( - {"new_generating_regulation": Regulation.objects.get(regulation_id=self.form_data["regulation"]["generating_regulation"])} - ) - # FIX THIS!!! - case "geographical_area_exclusions": - new_exclusions = [] - new_data.append( - {"new_geo_area_exclusions": self.form_data["duties"]["duties"]} - ) - # !!! - - logger.info(f"new start date: {new_data['new_start_date']}") - logger.info(f"new end date: {new_data['new_end_date']}") - logger.info(f"new duties: {new_data['new_duties']}") - logger.info(f"new order number: {new_data['new_quota_order_number']}") - logger.info(f"new generating regulation: {new_data['new_generating_regulation']}") - logger.info(f"new geo exclusions: {new_data['new_exclusions']}") + logger.info("INSIDE EDIT MEASURES TASK") + # Clean the forms to get the data back out of them + cleaned_data = self.get_forms_cleaned_data() + + logger.info(f"TASK - CLEANED DATA: {cleaned_data}") + logger.info(f"TASK - SELF.SELECTED MEASURES: {self.selected_measures}") deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) - - if self.selected_measures: + + new_exclusions = [ + e["excluded_area"] + for e in cleaned_data.get("formset-geographical_area_exclusions", []) + ] + + if deserialized_selected_measures: + logger.info("MADE IT TO IF SELECTED MEASURES!!!") for measure in deserialized_selected_measures: new_measure = measure.new_version( workbasket=self.workbasket, update_type=UpdateType.UPDATE, valid_between=TaricDateRange( lower=( - new_data['new_start_date'] - if new_data['new_start_date'] + cleaned_data['start_date'] + if cleaned_data['start_date'] else measure.valid_between.lower ), upper=( - new_data['new_end_date'] - if new_data['new_end_date'] is not False + cleaned_data['end_date'] + if cleaned_data['end_date'] is not False else measure.valid_between.upper ), ), order_number=( - new_data['new_quota_order_number'] - if new_data['new_quota_order_number'] + cleaned_data['order_number'] + if cleaned_data['order_number'] else measure.order_number ), generating_regulation=( - new_data['new_generating_regulation'] - if new_data['new_generating_regulation'] + cleaned_data['generating_regulation'] + if cleaned_data['generating_regulation'] else measure.generating_regulation ), ) self.update_measure_components( measure=new_measure, - duties=new_data['new_duties'], + duties=cleaned_data['duties'], workbasket=self.workbasket, ) self.update_measure_condition_components( - measure=new_data['new_measure'], + measure=new_measure, workbasket=self.workbasket, ) self.update_measure_excluded_geographical_areas( @@ -610,4 +566,68 @@ def edit_measures(self) -> Iterable[Measure]: self.update_measure_footnote_associations( measure=new_measure, workbasket=self.workbasket, - ) \ No newline at end of file + ) + + def get_forms_cleaned_data(self) -> Dict: + """ + Returns a merged dictionary of all Form cleaned_data. + + If a Form's data contains a `FormSet`, the key will be prefixed with + "formset-" and contain a list of the formset cleaned_data dictionaries. + + If form validation errors are encountered when constructing cleaned + data, then this function raises Django's `ValidationError` exception. + """ + all_cleaned_data = {} + + from measures.views import MeasureEditWizard + for form_key, form_class in MeasureEditWizard.data_form_list: + + if form_key not in self.form_data: + # Forms are conditionally included during step processing - see + # `MeasureEditWizard.show_step()` for details. + continue + + data = self.form_data[form_key] + + kwargs = form_class.deserialize_init_kwargs(self.form_kwargs[form_key]) + + form = form_class(data=data, **kwargs) + + if not form.is_valid(): + self._log_form_errors(form_class=form_class, form_or_formset=form) + raise ValidationError( + f"{form_class.__name__} has {len(form.errors)} errors.", + ) + + if isinstance(form.cleaned_data, (tuple, list)): + all_cleaned_data[f"formset-{form_key}"] = form.cleaned_data + else: + all_cleaned_data.update(form.cleaned_data) + + return all_cleaned_data + + def _log_form_errors(self, form_class, form_or_formset) -> None: + """Output errors associated with a Form or Formset instance, handling + output for each instance type in a uniform manner.""" + + logger.error( + f"MeasuresBulkEditor.edit_measures() - " + f"{form_class.__name__} has {len(form_or_formset.errors)} errors.", + ) + + # Form.errors is a dictionary of errors, but FormSet.errors is a + # list of dictionaries of Form.errors. Access their errors in + # a uniform manner. + errors = [] + + if isinstance(form_or_formset, BaseFormSet): + errors = [ + {"formset_errors": form_or_formset.non_form_errors()}, + ] + form_or_formset.errors + else: + errors = [form_or_formset.errors] + + for form_errors in errors: + for error_key, error_values in form_errors.items(): + logger.error(f"{error_key}: {error_values}") \ No newline at end of file diff --git a/measures/views/wizard.py b/measures/views/wizard.py index de2a7017b..c7fd68c6a 100644 --- a/measures/views/wizard.py +++ b/measures/views/wizard.py @@ -251,20 +251,20 @@ def done(self, form_list, **kwargs): def async_done(self, form_list, **kwargs): logger.info("Editing measures asynchronously.") - serializable_data = self.all_serializable_form_data() serializable_form_kwargs = self.all_serializable_form_kwargs() - selected_measures = [] + + db_selected_measures = [] for measure in self.get_queryset(): - selected_measures.append(measure.id) + db_selected_measures.append(measure.id) measures_bulk_editor = models.MeasuresBulkEditor.objects.create( form_data=serializable_data, form_kwargs=serializable_form_kwargs, workbasket=self.workbasket, user=self.request.user, - selected_measures=selected_measures, + selected_measures=db_selected_measures, ) # self.session_store.clear() # TODO: Is this the best point to clear the session store? measures_bulk_editor.schedule_task() From 1d838de7813359574d4288651887f87aac5e267f Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Fri, 9 Aug 2024 16:42:12 +0100 Subject: [PATCH 11/16] Move edit measure functions into utils so that it can be shared --- measures/models/bulk_processing.py | 14 ++-- measures/util.py | 110 ++++++++++++++++++++++++- measures/views/wizard.py | 124 ++++------------------------- 3 files changed, 133 insertions(+), 115 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 6bb5bd947..e127f1105 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -20,6 +20,10 @@ from common.util import TaricDateRange from common.validators import UpdateType from measures.models.tracked_models import Measure +from measures.util import update_measure_components +from measures.util import update_measure_condition_components +from measures.util import update_measure_excluded_geographical_areas +from measures.util import update_measure_footnote_associations logger = logging.getLogger(__name__) @@ -547,23 +551,23 @@ def edit_measures(self) -> Iterable[Measure]: else measure.generating_regulation ), ) - self.update_measure_components( + update_measure_components( measure=new_measure, duties=cleaned_data['duties'], workbasket=self.workbasket, ) - self.update_measure_condition_components( + update_measure_condition_components( measure=new_measure, workbasket=self.workbasket, ) - self.update_measure_excluded_geographical_areas( + update_measure_excluded_geographical_areas( edited="geographical_area_exclusions" in cleaned_data.get("fields_to_edit", []), measure=new_measure, exclusions=new_exclusions, workbasket=self.workbasket, ) - self.update_measure_footnote_associations( + update_measure_footnote_associations( measure=new_measure, workbasket=self.workbasket, ) @@ -606,7 +610,7 @@ def get_forms_cleaned_data(self) -> Dict: all_cleaned_data.update(form.cleaned_data) return all_cleaned_data - + def _log_form_errors(self, form_class, form_or_formset) -> None: """Output errors associated with a Form or Formset instance, handling output for each instance type in a uniform manner.""" diff --git a/measures/util.py b/measures/util.py index bf00efd6a..98264533a 100644 --- a/measures/util.py +++ b/measures/util.py @@ -2,11 +2,14 @@ from datetime import date from math import floor from typing import Type +from typing import List from common.models import TrackedModel from common.models.transactions import Transaction from common.validators import UpdateType -from measures.models import MeasureComponent +from geo_areas.models import GeographicalArea +from geo_areas.utils import get_all_members_of_geo_groups +from measures import models from workbaskets.models import WorkBasket @@ -31,7 +34,7 @@ def diff_components( start_date: date, workbasket: WorkBasket, transaction: Type[Transaction], - component_output: Type[TrackedModel] = MeasureComponent, + component_output: Type[TrackedModel] = models.MeasureComponent, reverse_attribute: str = "component_measure", ): """ @@ -91,3 +94,106 @@ def diff_components( update_type=UpdateType.DELETE, transaction=workbasket.new_transaction(), ) + + +def update_measure_components( + self, + measure: models.Measure, + duties: str, + workbasket: WorkBasket, + ): + """Updates the measure components associated to the measure.""" + diff_components( + instance=measure, + duty_sentence=duties if duties else measure.duty_sentence, + start_date=measure.valid_between.lower, + workbasket=workbasket, + transaction=workbasket.current_transaction, + ) + + +def update_measure_condition_components( + self, + measure: models.Measure, + workbasket: WorkBasket, +): + """Updates the measure condition components associated to the + measure.""" + conditions = measure.conditions.current() + for condition in conditions: + condition.new_version( + dependent_measure=measure, + workbasket=workbasket, + ) + + +def update_measure_excluded_geographical_areas( + self, + edited: bool, + measure: models.Measure, + exclusions: List[GeographicalArea], + workbasket: WorkBasket, +): + """Updates the excluded geographical areas associated to the measure.""" + existing_exclusions = measure.exclusions.current() + + # Update any exclusions to new measure version + if not edited: + for exclusion in existing_exclusions: + exclusion.new_version( + modified_measure=measure, + workbasket=workbasket, + ) + return + + new_excluded_areas = get_all_members_of_geo_groups( + validity=measure.valid_between, + geo_areas=exclusions, + ) + + for geo_area in new_excluded_areas: + existing_exclusion = existing_exclusions.filter( + excluded_geographical_area=geo_area, + ).first() + if existing_exclusion: + existing_exclusion.new_version( + modified_measure=measure, + workbasket=workbasket, + ) + else: + models.MeasureExcludedGeographicalArea.objects.create( + modified_measure=measure, + excluded_geographical_area=geo_area, + update_type=UpdateType.CREATE, + transaction=workbasket.new_transaction(), + ) + + removed_excluded_areas = { + e.excluded_geographical_area for e in existing_exclusions + }.difference(set(exclusions)) + + exclusions_to_remove = [ + existing_exclusions.get(excluded_geographical_area__id=geo_area.id) + for geo_area in removed_excluded_areas + ] + + for exclusion in exclusions_to_remove: + exclusion.new_version( + update_type=UpdateType.DELETE, + modified_measure=measure, + workbasket=workbasket, + ) + + +def update_measure_footnote_associations(self, measure, workbasket): + """Updates the footnotes associated to the measure.""" + footnote_associations = ( + models.FootnoteAssociationMeasure.objects.current().filter( + footnoted_measure__sid=measure.sid, + ) + ) + for fa in footnote_associations: + fa.new_version( + footnoted_measure=measure, + workbasket=workbasket, + ) diff --git a/measures/views/wizard.py b/measures/views/wizard.py index c7fd68c6a..2a47b220d 100644 --- a/measures/views/wizard.py +++ b/measures/views/wizard.py @@ -1,6 +1,5 @@ import logging from typing import Dict -from typing import List from common.forms import SerializableFormMixin from crispy_forms_gds.helper import FormHelper @@ -19,7 +18,6 @@ from geo_areas import constants from geo_areas.models import GeographicalArea from geo_areas.models import GeographicalMembership -from geo_areas.utils import get_all_members_of_geo_groups from geo_areas.validators import AreaCode from measures import forms from measures import models @@ -28,7 +26,10 @@ from measures.constants import START from measures.constants import MeasureEditSteps from measures.creators import MeasuresCreator -from measures.util import diff_components +from measures.util import update_measure_components +from measures.util import update_measure_condition_components +from measures.util import update_measure_excluded_geographical_areas +from measures.util import update_measure_footnote_associations from workbaskets.models import WorkBasket from workbaskets.views.decorators import require_current_workbasket @@ -144,105 +145,6 @@ def get_form_kwargs(self, step): return kwargs - def update_measure_components( - self, - measure: models.Measure, - duties: str, - workbasket: WorkBasket, - ): - """Updates the measure components associated to the measure.""" - diff_components( - instance=measure, - duty_sentence=duties if duties else measure.duty_sentence, - start_date=measure.valid_between.lower, - workbasket=workbasket, - transaction=workbasket.current_transaction, - ) - - def update_measure_condition_components( - self, - measure: models.Measure, - workbasket: WorkBasket, - ): - """Updates the measure condition components associated to the - measure.""" - conditions = measure.conditions.current() - for condition in conditions: - condition.new_version( - dependent_measure=measure, - workbasket=workbasket, - ) - - def update_measure_excluded_geographical_areas( - self, - edited: bool, - measure: models.Measure, - exclusions: List[GeographicalArea], - workbasket: WorkBasket, - ): - """Updates the excluded geographical areas associated to the measure.""" - existing_exclusions = measure.exclusions.current() - - # Update any exclusions to new measure version - if not edited: - for exclusion in existing_exclusions: - exclusion.new_version( - modified_measure=measure, - workbasket=workbasket, - ) - return - - new_excluded_areas = get_all_members_of_geo_groups( - validity=measure.valid_between, - geo_areas=exclusions, - ) - - for geo_area in new_excluded_areas: - existing_exclusion = existing_exclusions.filter( - excluded_geographical_area=geo_area, - ).first() - if existing_exclusion: - existing_exclusion.new_version( - modified_measure=measure, - workbasket=workbasket, - ) - else: - models.MeasureExcludedGeographicalArea.objects.create( - modified_measure=measure, - excluded_geographical_area=geo_area, - update_type=UpdateType.CREATE, - transaction=workbasket.new_transaction(), - ) - - removed_excluded_areas = { - e.excluded_geographical_area for e in existing_exclusions - }.difference(set(exclusions)) - - exclusions_to_remove = [ - existing_exclusions.get(excluded_geographical_area__id=geo_area.id) - for geo_area in removed_excluded_areas - ] - - for exclusion in exclusions_to_remove: - exclusion.new_version( - update_type=UpdateType.DELETE, - modified_measure=measure, - workbasket=workbasket, - ) - - def update_measure_footnote_associations(self, measure, workbasket): - """Updates the footnotes associated to the measure.""" - footnote_associations = ( - models.FootnoteAssociationMeasure.objects.current().filter( - footnoted_measure__sid=measure.sid, - ) - ) - for fa in footnote_associations: - fa.new_version( - footnoted_measure=measure, - workbasket=workbasket, - ) - def done(self, form_list, **kwargs): if settings.MEASURES_ASYNC_EDIT: return self.async_done(form_list, **kwargs) @@ -254,7 +156,6 @@ def async_done(self, form_list, **kwargs): serializable_data = self.all_serializable_form_data() serializable_form_kwargs = self.all_serializable_form_kwargs() - db_selected_measures = [] for measure in self.get_queryset(): db_selected_measures.append(measure.id) @@ -266,9 +167,16 @@ def async_done(self, form_list, **kwargs): user=self.request.user, selected_measures=db_selected_measures, ) - # self.session_store.clear() # TODO: Is this the best point to clear the session store? + self.session_store.clear() measures_bulk_editor.schedule_task() + return redirect( + reverse( + "workbaskets:workbasket-ui-review-measures", + kwargs={"pk": self.workbasket.pk}, + ), + ) + def sync_done(self, form_list, **kwargs): cleaned_data = self.get_all_cleaned_data() selected_measures = self.get_queryset() @@ -309,23 +217,23 @@ def sync_done(self, form_list, **kwargs): else measure.generating_regulation ), ) - self.update_measure_components( + update_measure_components( measure=new_measure, duties=new_duties, workbasket=workbasket, ) - self.update_measure_condition_components( + update_measure_condition_components( measure=new_measure, workbasket=workbasket, ) - self.update_measure_excluded_geographical_areas( + update_measure_excluded_geographical_areas( edited="geographical_area_exclusions" in cleaned_data.get("fields_to_edit", []), measure=new_measure, exclusions=new_exclusions, workbasket=workbasket, ) - self.update_measure_footnote_associations( + update_measure_footnote_associations( measure=new_measure, workbasket=workbasket, ) From 34828dc8fdf3e7167a6d030884d5e7dbaf7c35a7 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Tue, 13 Aug 2024 16:27:27 +0100 Subject: [PATCH 12/16] Move shared edit functions into util - gives circular import errors --- measures/models/bulk_processing.py | 10 ++++------ measures/tasks.py | 2 ++ measures/util.py | 26 ++++++++++++++------------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index e127f1105..ce9d8234f 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -508,13 +508,8 @@ def schedule_task(self) -> AsyncResult: @atomic def edit_measures(self) -> Iterable[Measure]: - logger.info("INSIDE EDIT MEASURES TASK") - # Clean the forms to get the data back out of them cleaned_data = self.get_forms_cleaned_data() - logger.info(f"TASK - CLEANED DATA: {cleaned_data}") - logger.info(f"TASK - SELF.SELECTED MEASURES: {self.selected_measures}") - deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) new_exclusions = [ @@ -523,7 +518,7 @@ def edit_measures(self) -> Iterable[Measure]: ] if deserialized_selected_measures: - logger.info("MADE IT TO IF SELECTED MEASURES!!!") + edited_measures = [] for measure in deserialized_selected_measures: new_measure = measure.new_version( workbasket=self.workbasket, @@ -572,6 +567,9 @@ def edit_measures(self) -> Iterable[Measure]: workbasket=self.workbasket, ) + edited_measures.append(new_measure.id) + return edited_measures + def get_forms_cleaned_data(self) -> Dict: """ Returns a merged dictionary of all Form cleaned_data. diff --git a/measures/tasks.py b/measures/tasks.py index 8d8a391f1..3ebe5e6d4 100644 --- a/measures/tasks.py +++ b/measures/tasks.py @@ -65,6 +65,8 @@ def bulk_edit_measures(measures_bulk_editor_pk: int) -> None: f"WorkBasket({measures_bulk_editor.workbasket.pk}).", ) raise e + + logger.info(f"MEASURES: {measures}") measures_bulk_editor.processing_succeeded() measures_bulk_editor.successfully_processed_count = len(measures) diff --git a/measures/util.py b/measures/util.py index 98264533a..a6be5fa8b 100644 --- a/measures/util.py +++ b/measures/util.py @@ -1,4 +1,6 @@ import decimal +from workbaskets import models as workbasket_models +from measures import models as measure_models from datetime import date from math import floor from typing import Type @@ -9,8 +11,8 @@ from common.validators import UpdateType from geo_areas.models import GeographicalArea from geo_areas.utils import get_all_members_of_geo_groups -from measures import models -from workbaskets.models import WorkBasket + + def convert_eur_to_gbp(amount: str, eur_gbp_conversion_rate: float) -> str: @@ -32,9 +34,9 @@ def diff_components( instance, duty_sentence: str, start_date: date, - workbasket: WorkBasket, + workbasket: workbasket_models.WorkBasket, transaction: Type[Transaction], - component_output: Type[TrackedModel] = models.MeasureComponent, + component_output: Type[TrackedModel] = measure_models.MeasureComponent, reverse_attribute: str = "component_measure", ): """ @@ -98,9 +100,9 @@ def diff_components( def update_measure_components( self, - measure: models.Measure, + measure: measure_models.Measure, duties: str, - workbasket: WorkBasket, + workbasket: workbasket_models.WorkBasket, ): """Updates the measure components associated to the measure.""" diff_components( @@ -114,8 +116,8 @@ def update_measure_components( def update_measure_condition_components( self, - measure: models.Measure, - workbasket: WorkBasket, + measure: measure_models.Measure, + workbasket: workbasket_models.WorkBasket, ): """Updates the measure condition components associated to the measure.""" @@ -130,9 +132,9 @@ def update_measure_condition_components( def update_measure_excluded_geographical_areas( self, edited: bool, - measure: models.Measure, + measure: measure_models.Measure, exclusions: List[GeographicalArea], - workbasket: WorkBasket, + workbasket: workbasket_models.WorkBasket, ): """Updates the excluded geographical areas associated to the measure.""" existing_exclusions = measure.exclusions.current() @@ -161,7 +163,7 @@ def update_measure_excluded_geographical_areas( workbasket=workbasket, ) else: - models.MeasureExcludedGeographicalArea.objects.create( + measure_models.MeasureExcludedGeographicalArea.objects.create( modified_measure=measure, excluded_geographical_area=geo_area, update_type=UpdateType.CREATE, @@ -188,7 +190,7 @@ def update_measure_excluded_geographical_areas( def update_measure_footnote_associations(self, measure, workbasket): """Updates the footnotes associated to the measure.""" footnote_associations = ( - models.FootnoteAssociationMeasure.objects.current().filter( + measure_models.FootnoteAssociationMeasure.objects.current().filter( footnoted_measure__sid=measure.sid, ) ) From 2949f384bed987c97d3aaffacf7b73d8aeece75d Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Tue, 27 Aug 2024 09:51:03 +0100 Subject: [PATCH 13/16] knot untangling - now gets up to editing a measure. Moves shared functions into a separate edit util --- measures/models/bulk_processing.py | 148 ++++++++++++++++------------- measures/util.py | 118 +---------------------- measures/utils/edit.py | 109 +++++++++++++++++++++ measures/views/wizard.py | 8 +- 4 files changed, 200 insertions(+), 183 deletions(-) create mode 100644 measures/utils/edit.py diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index ce9d8234f..6c19fdc9b 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -20,10 +20,10 @@ from common.util import TaricDateRange from common.validators import UpdateType from measures.models.tracked_models import Measure -from measures.util import update_measure_components -from measures.util import update_measure_condition_components -from measures.util import update_measure_excluded_geographical_areas -from measures.util import update_measure_footnote_associations +from measures.utils.edit import update_measure_components +from measures.utils.edit import update_measure_condition_components +from measures.utils.edit import update_measure_excluded_geographical_areas +from measures.utils.edit import update_measure_footnote_associations logger = logging.getLogger(__name__) @@ -508,67 +508,84 @@ def schedule_task(self) -> AsyncResult: @atomic def edit_measures(self) -> Iterable[Measure]: - cleaned_data = self.get_forms_cleaned_data() - - deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) - - new_exclusions = [ - e["excluded_area"] - for e in cleaned_data.get("formset-geographical_area_exclusions", []) - ] - - if deserialized_selected_measures: - edited_measures = [] - for measure in deserialized_selected_measures: - new_measure = measure.new_version( - workbasket=self.workbasket, - update_type=UpdateType.UPDATE, - valid_between=TaricDateRange( - lower=( - cleaned_data['start_date'] - if cleaned_data['start_date'] - else measure.valid_between.lower + logger.info("INSIDE EDIT MEASURES - BULK PROCESSING") + + with override_current_transaction( + transaction=self.workbasket.current_transaction, + ): + cleaned_data = self.get_forms_cleaned_data() + deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) + + new_exclusions = [ + e["excluded_area"] + for e in cleaned_data.get("formset-geographical_area_exclusions", []) + ] + + logger.info(f"CLEANED DATA: {cleaned_data}") + logger.info(f"DESERIALISED MEASURES: {deserialized_selected_measures}") + logger.info(f"NEW EXCLUSIONS: {new_exclusions}") + + if deserialized_selected_measures: + logger.info("INSIDE IF STATEMENT") + edited_measures = [] + logger.info(f" INITIAL EDITED MEASURES ARRAY: {edited_measures}") + for measure in deserialized_selected_measures: + logger.info("INSIDE FOR LOOP") + logger.info(f"MEASURE: {measure.__dict__}") + logger.info(f"CLEANED DATA: {cleaned_data}") + new_measure = measure.new_version( + workbasket=self.workbasket, + update_type=UpdateType.UPDATE, + valid_between=TaricDateRange( + lower=( + # Freaking out here even though it's an if?? not sure why + cleaned_data['start_date'] + if cleaned_data['start_date'] + else measure.valid_between.lower + ), + upper=( + cleaned_data['end_date'] + if cleaned_data['end_date'] is not False + else measure.valid_between.upper + ), ), - upper=( - cleaned_data['end_date'] - if cleaned_data['end_date'] is not False - else measure.valid_between.upper + order_number=( + cleaned_data['order_number'] + if cleaned_data['order_number'] + else measure.order_number ), - ), - order_number=( - cleaned_data['order_number'] - if cleaned_data['order_number'] - else measure.order_number - ), - generating_regulation=( - cleaned_data['generating_regulation'] - if cleaned_data['generating_regulation'] - else measure.generating_regulation - ), - ) - update_measure_components( - measure=new_measure, - duties=cleaned_data['duties'], - workbasket=self.workbasket, - ) - update_measure_condition_components( - measure=new_measure, - workbasket=self.workbasket, - ) - update_measure_excluded_geographical_areas( - edited="geographical_area_exclusions" - in cleaned_data.get("fields_to_edit", []), - measure=new_measure, - exclusions=new_exclusions, - workbasket=self.workbasket, - ) - update_measure_footnote_associations( - measure=new_measure, - workbasket=self.workbasket, - ) - - edited_measures.append(new_measure.id) - return edited_measures + generating_regulation=( + cleaned_data['generating_regulation'] + if cleaned_data['generating_regulation'] + else measure.generating_regulation + ), + ) + logger.info(f"NEW MEASURE: {new_measure}") + update_measure_components( + measure=new_measure, + duties=cleaned_data['duties'], + workbasket=self.workbasket, + ) + update_measure_condition_components( + measure=new_measure, + workbasket=self.workbasket, + ) + update_measure_excluded_geographical_areas( + edited="geographical_area_exclusions" + in cleaned_data.get("fields_to_edit", []), + measure=new_measure, + exclusions=new_exclusions, + workbasket=self.workbasket, + ) + update_measure_footnote_associations( + measure=new_measure, + workbasket=self.workbasket, + ) + logger.info(f"NEW MEASURE WITH FUNCTIONS RUN: {new_measure}") + + edited_measures.append(new_measure.id) + logger.info(f"EDITED MEASURES ARRAY ON CLOSE: {edited_measures}") + return edited_measures def get_forms_cleaned_data(self) -> Dict: """ @@ -591,7 +608,6 @@ def get_forms_cleaned_data(self) -> Dict: continue data = self.form_data[form_key] - kwargs = form_class.deserialize_init_kwargs(self.form_kwargs[form_key]) form = form_class(data=data, **kwargs) @@ -606,8 +622,8 @@ def get_forms_cleaned_data(self) -> Dict: all_cleaned_data[f"formset-{form_key}"] = form.cleaned_data else: all_cleaned_data.update(form.cleaned_data) - - return all_cleaned_data + + return all_cleaned_data def _log_form_errors(self, form_class, form_or_formset) -> None: """Output errors associated with a Form or Formset instance, handling diff --git a/measures/util.py b/measures/util.py index a6be5fa8b..7f56a65a3 100644 --- a/measures/util.py +++ b/measures/util.py @@ -1,18 +1,13 @@ import decimal -from workbaskets import models as workbasket_models -from measures import models as measure_models from datetime import date from math import floor from typing import Type -from typing import List from common.models import TrackedModel from common.models.transactions import Transaction from common.validators import UpdateType -from geo_areas.models import GeographicalArea -from geo_areas.utils import get_all_members_of_geo_groups - - +from measures import models as measure_models +from workbaskets import models as workbasket_models def convert_eur_to_gbp(amount: str, eur_gbp_conversion_rate: float) -> str: @@ -34,9 +29,9 @@ def diff_components( instance, duty_sentence: str, start_date: date, - workbasket: workbasket_models.WorkBasket, transaction: Type[Transaction], - component_output: Type[TrackedModel] = measure_models.MeasureComponent, + workbasket: Type[TrackedModel] = "workbasket_models.Workbasket", + component_output: Type[TrackedModel] = "measure_models.MeasureComponent", reverse_attribute: str = "component_measure", ): """ @@ -95,107 +90,4 @@ def diff_components( workbasket, update_type=UpdateType.DELETE, transaction=workbasket.new_transaction(), - ) - - -def update_measure_components( - self, - measure: measure_models.Measure, - duties: str, - workbasket: workbasket_models.WorkBasket, - ): - """Updates the measure components associated to the measure.""" - diff_components( - instance=measure, - duty_sentence=duties if duties else measure.duty_sentence, - start_date=measure.valid_between.lower, - workbasket=workbasket, - transaction=workbasket.current_transaction, - ) - - -def update_measure_condition_components( - self, - measure: measure_models.Measure, - workbasket: workbasket_models.WorkBasket, -): - """Updates the measure condition components associated to the - measure.""" - conditions = measure.conditions.current() - for condition in conditions: - condition.new_version( - dependent_measure=measure, - workbasket=workbasket, - ) - - -def update_measure_excluded_geographical_areas( - self, - edited: bool, - measure: measure_models.Measure, - exclusions: List[GeographicalArea], - workbasket: workbasket_models.WorkBasket, -): - """Updates the excluded geographical areas associated to the measure.""" - existing_exclusions = measure.exclusions.current() - - # Update any exclusions to new measure version - if not edited: - for exclusion in existing_exclusions: - exclusion.new_version( - modified_measure=measure, - workbasket=workbasket, - ) - return - - new_excluded_areas = get_all_members_of_geo_groups( - validity=measure.valid_between, - geo_areas=exclusions, - ) - - for geo_area in new_excluded_areas: - existing_exclusion = existing_exclusions.filter( - excluded_geographical_area=geo_area, - ).first() - if existing_exclusion: - existing_exclusion.new_version( - modified_measure=measure, - workbasket=workbasket, - ) - else: - measure_models.MeasureExcludedGeographicalArea.objects.create( - modified_measure=measure, - excluded_geographical_area=geo_area, - update_type=UpdateType.CREATE, - transaction=workbasket.new_transaction(), - ) - - removed_excluded_areas = { - e.excluded_geographical_area for e in existing_exclusions - }.difference(set(exclusions)) - - exclusions_to_remove = [ - existing_exclusions.get(excluded_geographical_area__id=geo_area.id) - for geo_area in removed_excluded_areas - ] - - for exclusion in exclusions_to_remove: - exclusion.new_version( - update_type=UpdateType.DELETE, - modified_measure=measure, - workbasket=workbasket, - ) - - -def update_measure_footnote_associations(self, measure, workbasket): - """Updates the footnotes associated to the measure.""" - footnote_associations = ( - measure_models.FootnoteAssociationMeasure.objects.current().filter( - footnoted_measure__sid=measure.sid, - ) - ) - for fa in footnote_associations: - fa.new_version( - footnoted_measure=measure, - workbasket=workbasket, - ) + ) \ No newline at end of file diff --git a/measures/utils/edit.py b/measures/utils/edit.py new file mode 100644 index 000000000..32f2a032c --- /dev/null +++ b/measures/utils/edit.py @@ -0,0 +1,109 @@ +from common.models import TrackedModel +from common.validators import UpdateType +from geo_areas.models import GeographicalArea +from geo_areas.utils import get_all_members_of_geo_groups +from measures import models as measure_models +from typing import List +from typing import Type +from measures.util import diff_components +from workbaskets import models as workbasket_models + + +def update_measure_components( + duties: str, + measure: Type[TrackedModel] = "measure_models.Measure", + workbasket: Type[TrackedModel] = "workbasket_models.WorkBasket", + ): + """Updates the measure components associated to the measure.""" + diff_components( + instance=measure, + duty_sentence=duties if duties else measure.duty_sentence, + start_date=measure.valid_between.lower, + workbasket=workbasket, + transaction=workbasket.current_transaction, + ) + + +def update_measure_condition_components( + workbasket: Type[TrackedModel] = "workbasket_models.WorkBasket", + measure: Type[TrackedModel] = "measure_models.Measure", +): + """Updates the measure condition components associated to the + measure.""" + conditions = measure.conditions.current() + for condition in conditions: + condition.new_version( + dependent_measure=measure, + workbasket=workbasket, + ) + + +def update_measure_excluded_geographical_areas( + edited: bool, + exclusions: List[GeographicalArea], + workbasket: Type[TrackedModel] = "workbasket_models.WorkBasket", + measure: Type[TrackedModel] = "measure_models.Measure", +): + """Updates the excluded geographical areas associated to the measure.""" + existing_exclusions = measure.exclusions.current() + + # Update any exclusions to new measure version + if not edited: + for exclusion in existing_exclusions: + exclusion.new_version( + modified_measure=measure, + workbasket=workbasket, + ) + return + + new_excluded_areas = get_all_members_of_geo_groups( + validity=measure.valid_between, + geo_areas=exclusions, + ) + + for geo_area in new_excluded_areas: + existing_exclusion = existing_exclusions.filter( + excluded_geographical_area=geo_area, + ).first() + if existing_exclusion: + existing_exclusion.new_version( + modified_measure=measure, + workbasket=workbasket, + ) + else: + measure_models.MeasureExcludedGeographicalArea.objects.create( + modified_measure=measure, + excluded_geographical_area=geo_area, + update_type=UpdateType.CREATE, + transaction=workbasket.new_transaction(), + ) + + removed_excluded_areas = { + e.excluded_geographical_area for e in existing_exclusions + }.difference(set(exclusions)) + + exclusions_to_remove = [ + existing_exclusions.get(excluded_geographical_area__id=geo_area.id) + for geo_area in removed_excluded_areas + ] + + for exclusion in exclusions_to_remove: + exclusion.new_version( + update_type=UpdateType.DELETE, + modified_measure=measure, + workbasket=workbasket, + ) + + +def update_measure_footnote_associations(measure, workbasket): + """Updates the footnotes associated to the measure.""" + footnote_associations = ( + measure_models.FootnoteAssociationMeasure.objects.current().filter( + footnoted_measure__sid=measure.sid, + ) + ) + for fa in footnote_associations: + fa.new_version( + footnoted_measure=measure, + workbasket=workbasket, + ) diff --git a/measures/views/wizard.py b/measures/views/wizard.py index 2a47b220d..0bb65eee4 100644 --- a/measures/views/wizard.py +++ b/measures/views/wizard.py @@ -26,10 +26,10 @@ from measures.constants import START from measures.constants import MeasureEditSteps from measures.creators import MeasuresCreator -from measures.util import update_measure_components -from measures.util import update_measure_condition_components -from measures.util import update_measure_excluded_geographical_areas -from measures.util import update_measure_footnote_associations +from measures.utils.edit import update_measure_components +from measures.utils.edit import update_measure_condition_components +from measures.utils.edit import update_measure_excluded_geographical_areas +from measures.utils.edit import update_measure_footnote_associations from workbaskets.models import WorkBasket from workbaskets.views.decorators import require_current_workbasket From 5957f81290799103e2b9740d4f89d689e129f94c Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Thu, 29 Aug 2024 14:50:16 +0100 Subject: [PATCH 14/16] Amend cleaned data variable assignment to fix if statements --- measures/models/bulk_processing.py | 41 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 6c19fdc9b..88ea03305 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -516,54 +516,52 @@ def edit_measures(self) -> Iterable[Measure]: cleaned_data = self.get_forms_cleaned_data() deserialized_selected_measures = Measure.objects.filter(pk__in=self.selected_measures) + new_start_date = cleaned_data.get("start_date", None) + new_end_date = cleaned_data.get("end_date", False) + new_quota_order_number = cleaned_data.get("order_number", None) + new_generating_regulation = cleaned_data.get("generating_regulation", None) + new_duties = cleaned_data.get("duties", None) new_exclusions = [ e["excluded_area"] for e in cleaned_data.get("formset-geographical_area_exclusions", []) ] - logger.info(f"CLEANED DATA: {cleaned_data}") - logger.info(f"DESERIALISED MEASURES: {deserialized_selected_measures}") - logger.info(f"NEW EXCLUSIONS: {new_exclusions}") - if deserialized_selected_measures: - logger.info("INSIDE IF STATEMENT") edited_measures = [] - logger.info(f" INITIAL EDITED MEASURES ARRAY: {edited_measures}") + for measure in deserialized_selected_measures: - logger.info("INSIDE FOR LOOP") - logger.info(f"MEASURE: {measure.__dict__}") - logger.info(f"CLEANED DATA: {cleaned_data}") new_measure = measure.new_version( workbasket=self.workbasket, update_type=UpdateType.UPDATE, valid_between=TaricDateRange( lower=( - # Freaking out here even though it's an if?? not sure why - cleaned_data['start_date'] - if cleaned_data['start_date'] + new_start_date + if new_start_date else measure.valid_between.lower ), upper=( - cleaned_data['end_date'] - if cleaned_data['end_date'] is not False + new_end_date + if new_end_date else measure.valid_between.upper ), ), order_number=( - cleaned_data['order_number'] - if cleaned_data['order_number'] + new_quota_order_number + if new_quota_order_number else measure.order_number ), generating_regulation=( - cleaned_data['generating_regulation'] - if cleaned_data['generating_regulation'] + new_generating_regulation + if new_generating_regulation else measure.generating_regulation ), ) - logger.info(f"NEW MEASURE: {new_measure}") + logger.info(f"NEW MEASURE: {new_measure.__dict__}") + logger.info("UPDATE FUNCTIONS STARTING") + logger.info(f"CLEANED DATA: {cleaned_data}") update_measure_components( measure=new_measure, - duties=cleaned_data['duties'], + duties=new_duties, workbasket=self.workbasket, ) update_measure_condition_components( @@ -623,7 +621,8 @@ def get_forms_cleaned_data(self) -> Dict: else: all_cleaned_data.update(form.cleaned_data) - return all_cleaned_data + logger.info(f"RESULT OF ALL CLEANED DATA: {all_cleaned_data}") + return all_cleaned_data def _log_form_errors(self, form_class, form_or_formset) -> None: """Output errors associated with a Form or Formset instance, handling From 00d536d86599bd1a5939967bde24963357e25c11 Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Fri, 30 Aug 2024 13:01:15 +0100 Subject: [PATCH 15/16] Wip - weird diff component issue --- measures/models/bulk_processing.py | 2 +- measures/util.py | 27 +++++++++++++++++++-------- measures/utils/edit.py | 10 ++++++++++ 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 88ea03305..2e02915ac 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -557,8 +557,8 @@ def edit_measures(self) -> Iterable[Measure]: ), ) logger.info(f"NEW MEASURE: {new_measure.__dict__}") - logger.info("UPDATE FUNCTIONS STARTING") logger.info(f"CLEANED DATA: {cleaned_data}") + logger.info("UPDATE FUNCTIONS STARTING") update_measure_components( measure=new_measure, duties=new_duties, diff --git a/measures/util.py b/measures/util.py index 7f56a65a3..9085a94c6 100644 --- a/measures/util.py +++ b/measures/util.py @@ -10,6 +10,9 @@ from workbaskets import models as workbasket_models +import logging +logger = logging.getLogger(__name__) + def convert_eur_to_gbp(amount: str, eur_gbp_conversion_rate: float) -> str: """Convert EUR amount to GBP and round down to nearest pence.""" converted_amount = ( @@ -47,18 +50,23 @@ def diff_components( of transactions and avoid business rule violations (e.g. ActionRequiresDuty). """ - from measures.parsers import DutySentenceParser + logger.info("DIFF COMPONENTS CALLED") + # from measures.parsers import DutySentenceParser + from measures.duty_sentence_parser import DutySentenceParser as LarkDutySentenceParser - parser = DutySentenceParser.create( + # I might be initialising this wrong. + parser = LarkDutySentenceParser( start_date, - component_output=component_output, ) - - new_components = parser.parse(duty_sentence) + logger.info(f"DIFF COMP DUTY SENTENCE: {duty_sentence}") + new_components = parser.transform(duty_sentence) old_components = instance.components.approved_up_to_transaction( workbasket.current_transaction, ) - new_by_id = {c.duty_expression.id: c for c in new_components} + logger.info(f"DIFF COMP NEW COMPONENTS: {new_components}") + logger.info(f"DIFF COMP OLD COMPONENTS: {old_components}") + + new_by_id = {c["duty_expression"].id: c for c in new_components} old_by_id = {c.duty_expression.id: c for c in old_components} all_ids = set(new_by_id.keys()) | set(old_by_id.keys()) update_transaction = transaction if transaction else None @@ -67,8 +75,11 @@ def diff_components( old = old_by_id.get(id) if new and old: # Component is having amount/unit changed – UPDATE it - new.update_type = UpdateType.UPDATE - new.version_group = old.version_group + logger.info(f"DIFF COMP NEW: {new}") + logger.info(f"DIFF COMP OLD: {old}") + new["update_type"] = UpdateType.UPDATE + new["version_group"] = old.version_group + setattr(new, reverse_attribute, instance) if not update_transaction: update_transaction = workbasket.new_transaction() diff --git a/measures/utils/edit.py b/measures/utils/edit.py index 32f2a032c..696b7a073 100644 --- a/measures/utils/edit.py +++ b/measures/utils/edit.py @@ -8,6 +8,8 @@ from measures.util import diff_components from workbaskets import models as workbasket_models +import logging +logger = logging.getLogger(__name__) def update_measure_components( duties: str, @@ -15,6 +17,10 @@ def update_measure_components( workbasket: Type[TrackedModel] = "workbasket_models.WorkBasket", ): """Updates the measure components associated to the measure.""" + + logger.info("UPDATE MEASURE COMPONENT CALLED") + logger.info(f"MEASURE DUTY SENTENCE: {measure.duty_sentence}") + logger.info(f"DUTIES : {duties}") diff_components( instance=measure, duty_sentence=duties if duties else measure.duty_sentence, @@ -30,6 +36,7 @@ def update_measure_condition_components( ): """Updates the measure condition components associated to the measure.""" + logger.info("UPDATE MEASURE CONDITION CALLED") conditions = measure.conditions.current() for condition in conditions: condition.new_version( @@ -45,6 +52,8 @@ def update_measure_excluded_geographical_areas( measure: Type[TrackedModel] = "measure_models.Measure", ): """Updates the excluded geographical areas associated to the measure.""" + + logger.info("UPDATE MEASURE EXCLUDED GEO AREAS CALLED") existing_exclusions = measure.exclusions.current() # Update any exclusions to new measure version @@ -97,6 +106,7 @@ def update_measure_excluded_geographical_areas( def update_measure_footnote_associations(measure, workbasket): """Updates the footnotes associated to the measure.""" + logger.info("UPDATE MEASURE FOOTNOTE ASSOSH CALLED") footnote_associations = ( measure_models.FootnoteAssociationMeasure.objects.current().filter( footnoted_measure__sid=measure.sid, From b20dcbd57b8346913031def5e2ae5b2e2a51f6ee Mon Sep 17 00:00:00 2001 From: Lauren Qurashi Date: Mon, 2 Sep 2024 15:24:27 +0100 Subject: [PATCH 16/16] WIP - for Paul to look at --- measures/models/bulk_processing.py | 4 +-- measures/parsers.py | 11 +++++-- measures/util.py | 49 +++++++++++++++++++----------- measures/utils/edit.py | 4 +-- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/measures/models/bulk_processing.py b/measures/models/bulk_processing.py index 2e02915ac..bef4ef427 100644 --- a/measures/models/bulk_processing.py +++ b/measures/models/bulk_processing.py @@ -556,9 +556,9 @@ def edit_measures(self) -> Iterable[Measure]: else measure.generating_regulation ), ) - logger.info(f"NEW MEASURE: {new_measure.__dict__}") - logger.info(f"CLEANED DATA: {cleaned_data}") logger.info("UPDATE FUNCTIONS STARTING") + logger.info(f"BE - NEW MEASURE: {new_measure.__dict__}") + logger.info(f"BE - NEW DUTIES: {new_duties}") update_measure_components( measure=new_measure, duties=new_duties, diff --git a/measures/parsers.py b/measures/parsers.py index e13a1bf11..bd112e85c 100644 --- a/measures/parsers.py +++ b/measures/parsers.py @@ -41,6 +41,11 @@ from measures.models import MonetaryUnit from measures.util import convert_eur_to_gbp + +import logging +logger = logging.getLogger(__name__) + + # Used to represent percentage or currency values. Amount = Decimal @@ -135,7 +140,7 @@ def __init__( duty_expressions: Iterable[DutyExpression], monetary_units: Iterable[MonetaryUnit], permitted_measurements: Iterable[Measurement], - component_output: Type[TrackedModel] = MeasureComponent, + component_output_type: Type[TrackedModel] = MeasureComponent, ): # Decimal numbers are a sequence of digits (without a left-trailing zero) # followed optionally by a decimal point and a number of digits (we have seen @@ -173,6 +178,8 @@ def component(duty_exp: DutyExpression) -> Parser: """Matches a string prefix and returns the associated type id, along with any parsed amounts and units according to their applicability, as a 4-tuple of (id, amount, monetary unit, measurement).""" + + logger.info("INSIDE COMPONENT") prefix = duty_exp.prefix has_amount = duty_exp.duty_amount_applicability_code has_measurement = duty_exp.measurement_unit_applicability_code @@ -211,7 +218,7 @@ def component(duty_exp: DutyExpression) -> Parser: and has_measurement != ApplicabilityCode.NOT_PERMITTED else component ).parsecmap( - lambda exp: component_output( + lambda exp: component_output_type( duty_expression=exp[0], duty_amount=exp[1], monetary_unit=exp[2], diff --git a/measures/util.py b/measures/util.py index 9085a94c6..da706d532 100644 --- a/measures/util.py +++ b/measures/util.py @@ -3,11 +3,12 @@ from math import floor from typing import Type + from common.models import TrackedModel from common.models.transactions import Transaction from common.validators import UpdateType -from measures import models as measure_models -from workbaskets import models as workbasket_models +# from measures import models as measure_models +# from workbaskets import models as workbasket_models import logging @@ -32,9 +33,9 @@ def diff_components( instance, duty_sentence: str, start_date: date, - transaction: Type[Transaction], - workbasket: Type[TrackedModel] = "workbasket_models.Workbasket", - component_output: Type[TrackedModel] = "measure_models.MeasureComponent", + transaction: Transaction, + workbasket: "workbasket_models.Workbasket", + component_output_type: Type = None, reverse_attribute: str = "component_measure", ): """ @@ -51,34 +52,46 @@ def diff_components( ActionRequiresDuty). """ logger.info("DIFF COMPONENTS CALLED") - # from measures.parsers import DutySentenceParser - from measures.duty_sentence_parser import DutySentenceParser as LarkDutySentenceParser + from measures.parsers import DutySentenceParser + from measures.models import MeasureComponent + # from measures.duty_sentence_parser import DutySentenceParser as LarkDutySentenceParser - # I might be initialising this wrong. - parser = LarkDutySentenceParser( + # Setting as a default parameter causes a circular import. To work round it, we set the default to none, + # Then reassign once we call the function + component_output_type = MeasureComponent if not component_output_type else component_output_type + parser = DutySentenceParser.create( start_date, + component_output=component_output_type, ) - logger.info(f"DIFF COMP DUTY SENTENCE: {duty_sentence}") - new_components = parser.transform(duty_sentence) + logger.info(f"DC - DUTY SENTENCE: {duty_sentence}") + logger.info(f"DC - DUTY SENTENCE TYPE: {type(duty_sentence)}") + new_components = parser.parse(duty_sentence) old_components = instance.components.approved_up_to_transaction( workbasket.current_transaction, ) - logger.info(f"DIFF COMP NEW COMPONENTS: {new_components}") - logger.info(f"DIFF COMP OLD COMPONENTS: {old_components}") + logger.info(f"DC - NEW COMPONENTS: {new_components}") + logger.info(f"DC - OLD COMPONENTS: {old_components}") - new_by_id = {c["duty_expression"].id: c for c in new_components} + new_by_id = {c.duty_expression.id: c for c in new_components} old_by_id = {c.duty_expression.id: c for c in old_components} + + logger.info(f"DC - NEW BY ID: {new_by_id}") + logger.info(f"DC - OLD BY ID: {old_by_id}") + all_ids = set(new_by_id.keys()) | set(old_by_id.keys()) + + logger.info(f"DC - ALL ID: {all_ids}") + update_transaction = transaction if transaction else None for id in all_ids: new = new_by_id.get(id) old = old_by_id.get(id) if new and old: # Component is having amount/unit changed – UPDATE it - logger.info(f"DIFF COMP NEW: {new}") - logger.info(f"DIFF COMP OLD: {old}") - new["update_type"] = UpdateType.UPDATE - new["version_group"] = old.version_group + logger.info(f"DC IF - NEW: {new}") + logger.info(f"DC IF - OLD: {old}") + new.update_type = UpdateType.UPDATE + new.version_group = old.version_group setattr(new, reverse_attribute, instance) if not update_transaction: diff --git a/measures/utils/edit.py b/measures/utils/edit.py index 696b7a073..5b13f67eb 100644 --- a/measures/utils/edit.py +++ b/measures/utils/edit.py @@ -19,8 +19,8 @@ def update_measure_components( """Updates the measure components associated to the measure.""" logger.info("UPDATE MEASURE COMPONENT CALLED") - logger.info(f"MEASURE DUTY SENTENCE: {measure.duty_sentence}") - logger.info(f"DUTIES : {duties}") + logger.info(f"UMC - MEASURE DUTY SENTENCE: {measure.duty_sentence}") + logger.info(f"UMC - DUTIES: {duties}") diff_components( instance=measure, duty_sentence=duties if duties else measure.duty_sentence,