From ce562d4ec1489b0e14ca7e118713e85145dc442d Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 9 Aug 2023 19:17:17 +0100 Subject: [PATCH 01/17] Add workbasket comparison prototype --- workbaskets/forms.py | 25 ++++ .../includes/workbaskets/navigation.jinja | 3 + workbaskets/jinja2/workbaskets/compare.jinja | 101 ++++++++++++++ .../migrations/0008_datarow_dataupload.py | 80 +++++++++++ workbaskets/models.py | 35 +++++ workbaskets/urls.py | 5 + workbaskets/util.py | 125 ++++++++++++++++++ workbaskets/views/mixins.py | 3 + workbaskets/views/ui.py | 113 ++++++++++------ 9 files changed, 449 insertions(+), 41 deletions(-) create mode 100644 workbaskets/jinja2/workbaskets/compare.jinja create mode 100644 workbaskets/migrations/0008_datarow_dataupload.py diff --git a/workbaskets/forms.py b/workbaskets/forms.py index df31474d8..0419a87c3 100644 --- a/workbaskets/forms.py +++ b/workbaskets/forms.py @@ -9,6 +9,7 @@ from common.validators import SymbolValidator from workbaskets import models from workbaskets import validators +from workbaskets.util import serialize_uploaded_data class WorkbasketCreateForm(forms.ModelForm): @@ -150,3 +151,27 @@ def cleaned_data_no_prefix(self): SelectableObjectsForm.object_id_from_field_name(key): value for key, value in self.cleaned_data.items() } + + +class WorkbasketCompareForm(forms.Form): + data = forms.CharField(widget=forms.Textarea(), validators=[SymbolValidator]) + + def clean(self): + serialized = serialize_uploaded_data(self.cleaned_data["data"]) + return {"data": serialized, "raw_data": self.cleaned_data["data"]} + + 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( + Field.textarea("data", rows=10), + Submit( + "submit", + "Compare", + data_module="govuk-button--secondary", + data_prevent_double_click="true", + ), + ) diff --git a/workbaskets/jinja2/includes/workbaskets/navigation.jinja b/workbaskets/jinja2/includes/workbaskets/navigation.jinja index 67935a359..0deb803e8 100644 --- a/workbaskets/jinja2/includes/workbaskets/navigation.jinja +++ b/workbaskets/jinja2/includes/workbaskets/navigation.jinja @@ -16,6 +16,9 @@
  • Review goods
    • +
  • + Compare data +
    • {% endmacro %} \ No newline at end of file diff --git a/workbaskets/jinja2/workbaskets/compare.jinja b/workbaskets/jinja2/workbaskets/compare.jinja new file mode 100644 index 000000000..7c6c69789 --- /dev/null +++ b/workbaskets/jinja2/workbaskets/compare.jinja @@ -0,0 +1,101 @@ +{% extends "layouts/form.jinja" %} + +{% from 'macros/create_link.jinja' import create_link %} +{% from 'macros/footnotes_display.jinja' import footnotes_display %} +{% from "includes/measures/conditions.jinja" import conditions_list %} +{% from "components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "components/button/macro.njk" import govukButton %} +{% from "includes/workbaskets/navigation.jinja" import navigation %} +{% from "components/table/macro.njk" import govukTable %} + +{% set page_title %} + Workbasket {{ workbasket.id if workbasket else request.session.workbasket.id }} - Summary +{% endset %} + +{% set change_workbasket_details_link = url("workbaskets:workbasket-ui-update", kwargs={"pk": workbasket.pk}) %} + +{% block breadcrumb %} + {{ govukBreadcrumbs({ + "items": [ + {"text": "Home", "href": url("home")}, + {"text": "Edit an existing workbasket", "href": url("workbaskets:workbasket-ui-list")}, + {"text": "Workbasket " ~ request.session.workbasket.id ~ " - Summary" } + ]}) + }} +{% endblock %} + +{% block content %} +

    + {{ page_title }} +

    + + {{ navigation(request, "compare") }} + + {% call django_form() %} + {{ crispy(form) }} + {% endcall %} + +

    Previously uploaded data

    + + {% set table_rows = [] %} + {% for row in data_upload.serialized %} + {{ table_rows.append([ + {"text": row.commodity}, + {"text": row.duty }, + {"text": "{:%d %b %Y}".format(row.valid_between.lower) }, + {"text": "{:%d %b %Y}".format(row.valid_between.upper) if row.valid_between.upper else "—" }, + ]) or "" }} + {% endfor %} + + {{ govukTable({ + "head": [ + {"text": "Commodity code"}, + {"text": "Duties"}, + {"text": "Start date"}, + {"text": "End date"}, + ], + "rows": table_rows, + "classes": "govuk-table-m" + }) }} + +

    {{ matching_measures|length }} matching measures found

    + + {% set table_rows = [] %} + {% for measure in matching_measures %} + {% set measure_link -%} + {{measure.sid}} + {%- endset %} + {{ table_rows.append([ + {"html": measure_link}, + {"text": measure.measure_type.sid ~ " - " ~ measure.measure_type.description}, + {"text": create_link(url("commodity-ui-detail", kwargs={"sid": measure.goods_nomenclature.sid}), measure.goods_nomenclature.item_id) if measure.goods_nomenclature else '-', "classes": "govuk-!-width-one-eighth"}, + {"text": measure.duty_sentence if measure.duty_sentence else '-'}, + {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, + {"text": "{:%d %b %Y}".format(measure.effective_end_date) if measure.effective_end_date else "-" }, + {"html": create_link(url("additional_code-ui-detail", kwargs={"sid": measure.additional_code.sid}), measure.additional_code.type.sid ~ measure.additional_code.code) if measure.additional_code else '-'}, + {"html": create_link(url("geo_area-ui-detail", kwargs={"sid": measure.geographical_area.sid}), measure.geographical_area.area_id ~ " - " ~ measure.geographical_area.get_description().description) if measure.geographical_area else '-'}, + {"text": create_link(measure.order_number.get_url(), measure.order_number.order_number) if measure.order_number else '-'}, + {"text": footnotes_display(measure.footnoteassociationmeasure_set.current())}, + {"text": conditions_list(measure) if measure.conditions.current() else "-"}, + ]) or "" }} + {% endfor %} + + {{ govukTable({ + "head": [ + {"text": "Measure SID"}, + {"text": "Measure type"}, + {"text": "Commodity code"}, + {"text": "Duties"}, + {"text": "Start date"}, + {"text": "End date"}, + {"text": "Additional code"}, + {"text": "Geographical area"}, + {"text": "Quota"}, + {"text": "Footnote"}, + {"text": "Conditions"}, + ], + "rows": table_rows, + "classes": "govuk-table-m" + }) }} + +{% endblock %} diff --git a/workbaskets/migrations/0008_datarow_dataupload.py b/workbaskets/migrations/0008_datarow_dataupload.py new file mode 100644 index 000000000..9c98153d3 --- /dev/null +++ b/workbaskets/migrations/0008_datarow_dataupload.py @@ -0,0 +1,80 @@ +# Generated by Django 3.2.20 on 2023-08-09 12:39 + +import django.db.models.deletion +from django.db import migrations +from django.db import models + +import common.fields + + +class Migration(migrations.Migration): + dependencies = [ + ("commodities", "0012_TOPS_745_description_date_fix"), + ("workbaskets", "0007_alter_workbasket_options"), + ] + + operations = [ + migrations.CreateModel( + name="DataUpload", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("raw_data", models.TextField()), + ( + "workbasket", + models.ForeignKey( + editable=False, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="workbaskets.workbasket", + ), + ), + ], + ), + migrations.CreateModel( + name="DataRow", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("valid_between", common.fields.TaricDateRangeField(db_index=True)), + ( + "duty_sentence", + models.CharField(blank=True, max_length=255, null=True), + ), + ( + "commodity", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.PROTECT, + to="commodities.goodsnomenclature", + ), + ), + ( + "data_upload", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="rows", + to="workbaskets.dataupload", + ), + ), + ], + options={ + "abstract": False, + }, + ), + ] diff --git a/workbaskets/models.py b/workbaskets/models.py index b3ae6a103..038419a09 100644 --- a/workbaskets/models.py +++ b/workbaskets/models.py @@ -19,6 +19,7 @@ from checks.models import TrackedModelCheck from checks.models import TransactionCheck from common.models.mixins import TimestampedMixin +from common.models.mixins.validity import ValidityMixin from common.models.tracked_qs import TrackedModelQuerySet from common.models.trackedmodel import TrackedModel from common.models.transactions import Transaction @@ -26,6 +27,7 @@ from common.models.transactions import TransactionQueryset from measures.models import Measure from measures.querysets import MeasuresQuerySet +from workbaskets.util import serialize_uploaded_data from workbaskets.validators import WorkflowStatus logger = logging.getLogger(__name__) @@ -625,3 +627,36 @@ def unchecked_or_errored_transactions(self): class Meta: verbose_name = "workbasket" verbose_name_plural = "workbaskets" + + +class DataUpload(models.Model): + raw_data = models.TextField() + workbasket = models.ForeignKey( + WorkBasket, + on_delete=models.CASCADE, + editable=False, + null=True, + ) + + @property + def serialized(self): + return serialize_uploaded_data(self.raw_data) + + +class DataRow(ValidityMixin, models.Model): + data_upload = models.ForeignKey( + DataUpload, + on_delete=models.CASCADE, + null=True, + related_name="rows", + ) + commodity = models.ForeignKey( + "commodities.GoodsNomenclature", + on_delete=models.PROTECT, + null=True, + ) + duty_sentence = models.CharField( + max_length=255, + null=True, + blank=True, + ) diff --git a/workbaskets/urls.py b/workbaskets/urls.py index 1c3fd1fed..472294a9b 100644 --- a/workbaskets/urls.py +++ b/workbaskets/urls.py @@ -81,6 +81,11 @@ ui_views.WorkBasketDeleteChangesDone.as_view(), name="workbasket-ui-delete-changes-done", ), + path( + f"compare/", + ui_views.WorkBasketCompare.as_view(), + name="workbasket-ui-compare", + ), path( f"/changes/", ui_views.WorkBasketChanges.as_view(), diff --git a/workbaskets/util.py b/workbaskets/util.py index 6f2f2f279..6a4f1120e 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -1,5 +1,12 @@ +import re +from datetime import date + +from django.core.exceptions import ValidationError from django.db import transaction +from parsec import ParseError +from commodities.validators import ITEM_ID_REGEX +from common.util import TaricDateRange from importer.nursery import get_nursery @@ -41,3 +48,121 @@ def delete_workbasket(workbasket): itself.""" clear_workbasket(workbasket) workbasket.delete() + + +DATE_REGEX = ( + r"([0-9]{2}(\/|-)[0-9]{2}(\/|-)[0-9]{4})|([0-9]{4}(\/|-)[0-9]{2}(\/|-)[0-9]{2})" +) + + +class TableRow: + def __init__(self, commodity=None, valid_between=None, duty=None): + self.commodity = commodity + self.valid_between = valid_between + self.duty = duty + + +def serialize_uploaded_data(data): + serialized = [] + rows = data.strip().split("\n") + table = [row.strip().split("\t") for row in rows] + from commodities.models.orm import GoodsNomenclature + + for row in table: + row_data = TableRow() + dates = [] + for cell in row: + # look for comm code + matches = re.compile(ITEM_ID_REGEX).match(cell) + if matches: + commodity = ( + GoodsNomenclature.objects.latest_approved() + .filter(item_id=cell) + .first() + ) + row_data.commodity = commodity + continue + # look for dates + matches = re.compile(DATE_REGEX).match(cell) + if matches: + dates.append(cell) + continue + + # look for duty sentence + # if it didn't match comm code or date it's probably a duty + row_data.duty = cell + continue + + if len(dates) == 2: + # should only have 2 dates - start and end + if "/" in dates[0]: + delimiter = "/" + elif "-" in dates[0]: + delimiter = "-" + parsed_dates = [] + for date_string in dates: + components = [ + int(component) for component in date_string.split(delimiter) + ] + + # year first + if components[0] > 1000: + parsed_dates.append( + date(components[0], components[1], components[2]), + ) + # year last + elif components[-1] > 1000: + parsed_dates.append( + date(components[2], components[1], components[0]), + ) + + if parsed_dates[0] > parsed_dates[1]: + row_data.valid_between = TaricDateRange( + parsed_dates[1], + parsed_dates[0], + ) + elif parsed_dates[1] > parsed_dates[0]: + row_data.valid_between = TaricDateRange( + parsed_dates[0], + parsed_dates[1], + ) + + # if only one date this must be the start date + elif len(dates) == 1: + start_date = dates[0] + + if "/" in start_date: + delimiter = "/" + elif "-" in start_date: + delimiter = "-" + + components = [int(component) for component in start_date.split(delimiter)] + + # year first + if components[0] > 1000: + row_data.valid_between = TaricDateRange( + date(components[0], components[1], components[2]), + ) + # year last + elif components[-1] > 1000: + row_data.valid_between = TaricDateRange( + date(components[2], components[1], components[0]), + ) + + from measures.parsers import DutySentenceParser + + duty_sentence_parser = DutySentenceParser.create( + row_data.valid_between.lower, + ) + + if row_data.duty: + try: + duty_sentence_parser.parse(row_data.duty) + except ParseError as error: + error_index = int(error.loc().split(":", 1)[1]) + raise ValidationError( + f'"{row_data.duty[error_index:]}" is an invalid duty expression', + ) + + serialized.append(row_data) + return serialized diff --git a/workbaskets/views/mixins.py b/workbaskets/views/mixins.py index 83a626352..089d76db9 100644 --- a/workbaskets/views/mixins.py +++ b/workbaskets/views/mixins.py @@ -16,3 +16,6 @@ def get_queryset(self): transaction = current.transactions.last() return qs.approved_up_to_transaction(transaction) + + def get_context_data(self, *args, **kwargs): + return super().get_context_data(workbasket=self.workbasket, *args, **kwargs) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 9f67951ca..3d84c3459 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -39,6 +39,8 @@ from measures.filters import MeasureFilter from measures.models import Measure from workbaskets import forms +from workbaskets.models import DataRow +from workbaskets.models import DataUpload from workbaskets.models import WorkBasket from workbaskets.session_store import SessionStore from workbaskets.tasks import call_check_workbasket_sync @@ -369,6 +371,7 @@ class CurrentWorkBasket(FormView): "remove-all": "workbaskets:workbasket-ui-delete-changes", "page-prev": "workbaskets:current-workbasket", "page-next": "workbaskets:current-workbasket", + "compare-data": "workbaskets:current-workbasket", } @property @@ -409,20 +412,6 @@ def _append_url_page_param(self, url, form_action): page_number = page.next_page_number() return f"{url}?page={page_number}" - def get(self, request, *args, **kwargs): - """Service GET requests by displaying the page and form.""" - return self.render_to_response(self.get_context_data()) - - def post(self, request, *args, **kwargs): - """Manage POST requests, which can either be requests to change the - paged form data while preserving the user's form changes, or finally - submit the form data for processing.""" - form = self.get_form() - if form.is_valid(): - return self.form_valid(form) - else: - return self.form_invalid(form) - @atomic def run_business_rules(self): """Remove old checks, start new checks via a Celery task and save the @@ -442,42 +431,19 @@ def run_business_rules(self): def get_success_url(self): form_action = self.request.POST.get("form-action") - if form_action == "remove-selected": - return reverse( - self.action_success_url_names[form_action], - ) - elif form_action == "remove-all": - return reverse( - self.action_success_url_names[form_action], - ) - elif form_action in ("page-prev", "page-next"): - return self._append_url_page_param( - reverse( - self.action_success_url_names[form_action], - ), - form_action, - ) - elif form_action == "run-business-rules": + if form_action == "run-business-rules": self.run_business_rules() - return self._append_url_page_param( - reverse( - self.action_success_url_names[form_action], - ), - form_action, - ) - elif form_action == "submit-for-packaging": - return reverse( - self.action_success_url_names[form_action], - ) elif form_action == "terminate-rule-check": self.workbasket.terminate_rule_check() + try: return self._append_url_page_param( reverse( self.action_success_url_names[form_action], ), form_action, ) - return reverse("home") + except KeyError: + return reverse("home") def get_initial(self): store = SessionStore( @@ -716,3 +682,68 @@ def get_context_data(self, **kwargs): context_data = super().get_context_data(**kwargs) context_data["deleted_pk"] = self.kwargs["deleted_pk"] return context_data + + +class WorkBasketCompare(WithCurrentWorkBasket, FormView): + success_url = reverse_lazy("workbaskets:workbasket-ui-compare") + template_name = "workbaskets/compare.jinja" + form_class = forms.WorkbasketCompareForm + + @property + def workbasket_measures(self): + return self.workbasket.measures.all() + + @property + def data_upload(self): + try: + return DataUpload.objects.get(workbasket=self.workbasket) + except DataUpload.DoesNotExist: + return None + + def form_valid(self, form): + try: + existing = DataUpload.objects.get(workbasket=self.workbasket) + existing.raw_data = form.cleaned_data["raw_data"] + existing.rows.all().delete() + for row in form.cleaned_data["data"]: + DataRow.objects.create( + valid_between=row.valid_between, + duty_sentence=row.duty, + commodity=row.commodity, + data_upload=existing, + ) + existing.save() + except DataUpload.DoesNotExist: + data_upload = DataUpload.objects.create( + raw_data=form.cleaned_data["data"], + workbasket=self.workbasket, + ) + for row in form.cleaned_data["data"]: + DataRow.objects.create( + valid_between=row.valid_between, + duty_sentence=row.duty, + commodity=row.commodity, + data_upload=data_upload, + ) + return super().form_valid(form) + + @property + def matching_measures(self): + measures = [] + for row in self.data_upload.rows.all(): + matches = self.workbasket_measures.filter( + valid_between=row.valid_between, + goods_nomenclature=row.commodity, + ) + measures += matches + # TODO: match duty sentence + + return measures + + def get_context_data(self, *args, **kwargs): + return super().get_context_data( + data_upload=self.data_upload, + matching_measures=self.matching_measures, + *args, + **kwargs, + ) From 234760cfd91d3ef375caf3b6e6d5e4eea822d144 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Thu, 10 Aug 2023 10:05:31 +0100 Subject: [PATCH 02/17] Hide sections if data does not exist --- workbaskets/jinja2/workbaskets/compare.jinja | 130 ++++++++++--------- workbaskets/views/ui.py | 15 ++- 2 files changed, 76 insertions(+), 69 deletions(-) diff --git a/workbaskets/jinja2/workbaskets/compare.jinja b/workbaskets/jinja2/workbaskets/compare.jinja index 7c6c69789..edcbe87bc 100644 --- a/workbaskets/jinja2/workbaskets/compare.jinja +++ b/workbaskets/jinja2/workbaskets/compare.jinja @@ -35,67 +35,73 @@ {{ crispy(form) }} {% endcall %} -

    Previously uploaded data

    - - {% set table_rows = [] %} - {% for row in data_upload.serialized %} - {{ table_rows.append([ - {"text": row.commodity}, - {"text": row.duty }, - {"text": "{:%d %b %Y}".format(row.valid_between.lower) }, - {"text": "{:%d %b %Y}".format(row.valid_between.upper) if row.valid_between.upper else "—" }, - ]) or "" }} - {% endfor %} - - {{ govukTable({ - "head": [ - {"text": "Commodity code"}, - {"text": "Duties"}, - {"text": "Start date"}, - {"text": "End date"}, - ], - "rows": table_rows, - "classes": "govuk-table-m" - }) }} - -

    {{ matching_measures|length }} matching measures found

    - - {% set table_rows = [] %} - {% for measure in matching_measures %} - {% set measure_link -%} - {{measure.sid}} - {%- endset %} - {{ table_rows.append([ - {"html": measure_link}, - {"text": measure.measure_type.sid ~ " - " ~ measure.measure_type.description}, - {"text": create_link(url("commodity-ui-detail", kwargs={"sid": measure.goods_nomenclature.sid}), measure.goods_nomenclature.item_id) if measure.goods_nomenclature else '-', "classes": "govuk-!-width-one-eighth"}, - {"text": measure.duty_sentence if measure.duty_sentence else '-'}, - {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, - {"text": "{:%d %b %Y}".format(measure.effective_end_date) if measure.effective_end_date else "-" }, - {"html": create_link(url("additional_code-ui-detail", kwargs={"sid": measure.additional_code.sid}), measure.additional_code.type.sid ~ measure.additional_code.code) if measure.additional_code else '-'}, - {"html": create_link(url("geo_area-ui-detail", kwargs={"sid": measure.geographical_area.sid}), measure.geographical_area.area_id ~ " - " ~ measure.geographical_area.get_description().description) if measure.geographical_area else '-'}, - {"text": create_link(measure.order_number.get_url(), measure.order_number.order_number) if measure.order_number else '-'}, - {"text": footnotes_display(measure.footnoteassociationmeasure_set.current())}, - {"text": conditions_list(measure) if measure.conditions.current() else "-"}, - ]) or "" }} - {% endfor %} - - {{ govukTable({ - "head": [ - {"text": "Measure SID"}, - {"text": "Measure type"}, - {"text": "Commodity code"}, - {"text": "Duties"}, - {"text": "Start date"}, - {"text": "End date"}, - {"text": "Additional code"}, - {"text": "Geographical area"}, - {"text": "Quota"}, - {"text": "Footnote"}, - {"text": "Conditions"}, - ], - "rows": table_rows, - "classes": "govuk-table-m" - }) }} + {% if data_upload %} +

    Previously uploaded data

    + + {% set table_rows = [] %} + {% for row in data_upload.serialized %} + {{ table_rows.append([ + {"text": row.commodity}, + {"text": row.duty }, + {"text": "{:%d %b %Y}".format(row.valid_between.lower) }, + {"text": "{:%d %b %Y}".format(row.valid_between.upper) if row.valid_between.upper else "—" }, + ]) or "" }} + {% endfor %} + + {{ govukTable({ + "head": [ + {"text": "Commodity code"}, + {"text": "Duties"}, + {"text": "Start date"}, + {"text": "End date"}, + ], + "rows": table_rows, + "classes": "govuk-table-m" + }) }} + + {% endif %} + + {% if matching_measures %} +

    {{ matching_measures|length }} matching measures found

    + + {% set table_rows = [] %} + {% for measure in matching_measures %} + {% set measure_link -%} + {{measure.sid}} + {%- endset %} + {{ table_rows.append([ + {"html": measure_link}, + {"text": measure.measure_type.sid ~ " - " ~ measure.measure_type.description}, + {"text": create_link(url("commodity-ui-detail", kwargs={"sid": measure.goods_nomenclature.sid}), measure.goods_nomenclature.item_id) if measure.goods_nomenclature else '-', "classes": "govuk-!-width-one-eighth"}, + {"text": measure.duty_sentence if measure.duty_sentence else '-'}, + {"text": "{:%d %b %Y}".format(measure.valid_between.lower) }, + {"text": "{:%d %b %Y}".format(measure.effective_end_date) if measure.effective_end_date else "-" }, + {"html": create_link(url("additional_code-ui-detail", kwargs={"sid": measure.additional_code.sid}), measure.additional_code.type.sid ~ measure.additional_code.code) if measure.additional_code else '-'}, + {"html": create_link(url("geo_area-ui-detail", kwargs={"sid": measure.geographical_area.sid}), measure.geographical_area.area_id ~ " - " ~ measure.geographical_area.get_description().description) if measure.geographical_area else '-'}, + {"text": create_link(measure.order_number.get_url(), measure.order_number.order_number) if measure.order_number else '-'}, + {"text": footnotes_display(measure.footnoteassociationmeasure_set.current())}, + {"text": conditions_list(measure) if measure.conditions.current() else "-"}, + ]) or "" }} + {% endfor %} + + {{ govukTable({ + "head": [ + {"text": "Measure SID"}, + {"text": "Measure type"}, + {"text": "Commodity code"}, + {"text": "Duties"}, + {"text": "Start date"}, + {"text": "End date"}, + {"text": "Additional code"}, + {"text": "Geographical area"}, + {"text": "Quota"}, + {"text": "Footnote"}, + {"text": "Conditions"}, + ], + "rows": table_rows, + "classes": "govuk-table-m" + }) }} + + {% endif %} {% endblock %} diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 3d84c3459..f5abffd12 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -730,13 +730,14 @@ def form_valid(self, form): @property def matching_measures(self): measures = [] - for row in self.data_upload.rows.all(): - matches = self.workbasket_measures.filter( - valid_between=row.valid_between, - goods_nomenclature=row.commodity, - ) - measures += matches - # TODO: match duty sentence + if self.data_upload: + for row in self.data_upload.rows.all(): + matches = self.workbasket_measures.filter( + valid_between=row.valid_between, + goods_nomenclature=row.commodity, + ) + measures += matches + # TODO: match duty sentence return measures From cee0235ed097c96ccd078f6c1eaea878d7e386ff Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Thu, 10 Aug 2023 12:13:12 +0100 Subject: [PATCH 03/17] Find duty sentence matches --- workbaskets/views/ui.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index f5abffd12..07943d9c8 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -730,6 +730,7 @@ def form_valid(self, form): @property def matching_measures(self): measures = [] + duty_matches = [] if self.data_upload: for row in self.data_upload.rows.all(): matches = self.workbasket_measures.filter( @@ -737,9 +738,15 @@ def matching_measures(self): goods_nomenclature=row.commodity, ) measures += matches - # TODO: match duty sentence + for row in self.data_upload.rows.all(): + matches = [ + measure + for measure in measures + if measure.duty_sentence == row.duty_sentence + ] + duty_matches += matches - return measures + return duty_matches def get_context_data(self, *args, **kwargs): return super().get_context_data( From 33b118a79f50201c88b59a5c69e8c49a34553fd4 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Thu, 10 Aug 2023 12:25:55 +0100 Subject: [PATCH 04/17] Content tweak --- workbaskets/jinja2/workbaskets/compare.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workbaskets/jinja2/workbaskets/compare.jinja b/workbaskets/jinja2/workbaskets/compare.jinja index edcbe87bc..33845ec35 100644 --- a/workbaskets/jinja2/workbaskets/compare.jinja +++ b/workbaskets/jinja2/workbaskets/compare.jinja @@ -62,7 +62,7 @@ {% endif %} {% if matching_measures %} -

    {{ matching_measures|length }} matching measures found

    +

    {{ matching_measures|length }} matching measure{{ matching_measures|pluralize }} found

    {% set table_rows = [] %} {% for measure in matching_measures %} From feee9baa7e2670f370df2c769879697211be57e3 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Thu, 10 Aug 2023 12:26:12 +0100 Subject: [PATCH 05/17] Tweak duty sentence filtering --- workbaskets/views/ui.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 07943d9c8..63ca5fb11 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -730,23 +730,19 @@ def form_valid(self, form): @property def matching_measures(self): measures = [] - duty_matches = [] if self.data_upload: for row in self.data_upload.rows.all(): matches = self.workbasket_measures.filter( valid_between=row.valid_between, goods_nomenclature=row.commodity, ) - measures += matches - for row in self.data_upload.rows.all(): - matches = [ + duty_matches = [ measure - for measure in measures + for measure in matches if measure.duty_sentence == row.duty_sentence ] - duty_matches += matches - - return duty_matches + measures += duty_matches + return measures def get_context_data(self, *args, **kwargs): return super().get_context_data( From 7018bf70e95af240f11ab4e8c13fa7ff6252b7be Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 1 Sep 2023 20:21:44 +0100 Subject: [PATCH 06/17] Fix transaction error --- workbaskets/views/mixins.py | 3 --- workbaskets/views/ui.py | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/workbaskets/views/mixins.py b/workbaskets/views/mixins.py index 089d76db9..83a626352 100644 --- a/workbaskets/views/mixins.py +++ b/workbaskets/views/mixins.py @@ -16,6 +16,3 @@ def get_queryset(self): transaction = current.transactions.last() return qs.approved_up_to_transaction(transaction) - - def get_context_data(self, *args, **kwargs): - return super().get_context_data(workbasket=self.workbasket, *args, **kwargs) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 63ca5fb11..aa0a48f8b 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -746,6 +746,7 @@ def matching_measures(self): def get_context_data(self, *args, **kwargs): return super().get_context_data( + workbasket=self.workbasket, data_upload=self.data_upload, matching_measures=self.matching_measures, *args, From 93065dc43e644b332dff3815ce1fca4a8c70694b Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 1 Sep 2023 20:22:09 +0100 Subject: [PATCH 07/17] Refactor --- workbaskets/util.py | 200 ++++++++++++++++++++++++-------------------- 1 file changed, 110 insertions(+), 90 deletions(-) diff --git a/workbaskets/util.py b/workbaskets/util.py index 6a4f1120e..8b9da2332 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -1,7 +1,6 @@ import re from datetime import date -from django.core.exceptions import ValidationError from django.db import transaction from parsec import ParseError @@ -50,6 +49,10 @@ def delete_workbasket(workbasket): workbasket.delete() +# permits both date formats +# YYYY-MM-DD +# DD-MM-YYYY +# and either - or / separator DATE_REGEX = ( r"([0-9]{2}(\/|-)[0-9]{2}(\/|-)[0-9]{4})|([0-9]{4}(\/|-)[0-9]{2}(\/|-)[0-9]{2})" ) @@ -61,108 +64,125 @@ def __init__(self, commodity=None, valid_between=None, duty=None): self.valid_between = valid_between self.duty = duty + @property + def all_none(self): + return not all([self.commodity, self.valid_between, self.duty]) + + +def find_comm_code(cell, row_data): + from commodities.models.orm import GoodsNomenclature + + matches = re.compile(ITEM_ID_REGEX).match(cell) + if matches: + commodity = ( + GoodsNomenclature.objects.latest_approved().filter(item_id=cell).first() + ) + row_data.commodity = commodity + return True + return False + + +def find_dates(cell, dates): + matches = re.compile(DATE_REGEX).match(cell) + if matches: + dates.append(cell) + return True + return False + + +def find_duty_sentence(cell, row_data): + from measures.parsers import DutySentenceParser + + # because we may not know the measure validity period, take today's date instead + # we only need to look for something that looks like a duty sentence, not necessarily a valid one + duty_sentence_parser = DutySentenceParser.create( + date.today(), + ) + duty_sentence = cell.replace(" ", "") + try: + duty_sentence_parser.parse(duty_sentence) + row_data.duty = cell + return True + except ParseError: + return False + + +def get_delimiter(date_string): + if "/" in date_string: + delimiter = "/" + elif "-" in date_string: + delimiter = "-" + return delimiter + + +def parse_dates(dates): + parsed_dates = [] + for date_string in dates: + delimiter = get_delimiter(date_string) + components = [int(component) for component in date_string.split(delimiter)] + + # year first + if components[0] > 1000: + parsed_dates.append( + date(components[0], components[1], components[2]), + ) + # year last + elif components[-1] > 1000: + parsed_dates.append( + date(components[2], components[1], components[0]), + ) + return parsed_dates + + +def assign_validity(dates, row_data): + parsed_dates = parse_dates(dates) + + # assume start date for an open-ended measure + if len(parsed_dates) == 1: + row_data.valid_between = TaricDateRange( + parsed_dates[0], + None, + ) + elif len(parsed_dates) == 2: + if parsed_dates[0] > parsed_dates[1]: + row_data.valid_between = TaricDateRange( + parsed_dates[1], + parsed_dates[0], + ) + elif parsed_dates[1] > parsed_dates[0]: + row_data.valid_between = TaricDateRange( + parsed_dates[0], + parsed_dates[1], + ) + def serialize_uploaded_data(data): serialized = [] rows = data.strip().split("\n") table = [row.strip().split("\t") for row in rows] - from commodities.models.orm import GoodsNomenclature for row in table: row_data = TableRow() dates = [] for cell in row: - # look for comm code - matches = re.compile(ITEM_ID_REGEX).match(cell) - if matches: - commodity = ( - GoodsNomenclature.objects.latest_approved() - .filter(item_id=cell) - .first() - ) - row_data.commodity = commodity + if not cell: continue - # look for dates - matches = re.compile(DATE_REGEX).match(cell) - if matches: - dates.append(cell) + + commodity = find_comm_code(cell, row_data) + if commodity: continue - # look for duty sentence - # if it didn't match comm code or date it's probably a duty - row_data.duty = cell - continue - - if len(dates) == 2: - # should only have 2 dates - start and end - if "/" in dates[0]: - delimiter = "/" - elif "-" in dates[0]: - delimiter = "-" - parsed_dates = [] - for date_string in dates: - components = [ - int(component) for component in date_string.split(delimiter) - ] - - # year first - if components[0] > 1000: - parsed_dates.append( - date(components[0], components[1], components[2]), - ) - # year last - elif components[-1] > 1000: - parsed_dates.append( - date(components[2], components[1], components[0]), - ) - - if parsed_dates[0] > parsed_dates[1]: - row_data.valid_between = TaricDateRange( - parsed_dates[1], - parsed_dates[0], - ) - elif parsed_dates[1] > parsed_dates[0]: - row_data.valid_between = TaricDateRange( - parsed_dates[0], - parsed_dates[1], - ) - - # if only one date this must be the start date - elif len(dates) == 1: - start_date = dates[0] - - if "/" in start_date: - delimiter = "/" - elif "-" in start_date: - delimiter = "-" - - components = [int(component) for component in start_date.split(delimiter)] - - # year first - if components[0] > 1000: - row_data.valid_between = TaricDateRange( - date(components[0], components[1], components[2]), - ) - # year last - elif components[-1] > 1000: - row_data.valid_between = TaricDateRange( - date(components[2], components[1], components[0]), - ) - - from measures.parsers import DutySentenceParser - - duty_sentence_parser = DutySentenceParser.create( - row_data.valid_between.lower, - ) + found_dates = find_dates(cell, dates) + if found_dates: + continue + + duty = find_duty_sentence(cell, row_data) + if duty: + continue + + assign_validity(dates, row_data) - if row_data.duty: - try: - duty_sentence_parser.parse(row_data.duty) - except ParseError as error: - error_index = int(error.loc().split(":", 1)[1]) - raise ValidationError( - f'"{row_data.duty[error_index:]}" is an invalid duty expression', - ) + if not row_data.all_none: + serialized.append(row_data) - serialized.append(row_data) return serialized From a031f7cf09b5250da4e5c446db2372ab4f1c91a9 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Fri, 1 Sep 2023 21:15:47 +0100 Subject: [PATCH 08/17] Add tests --- workbaskets/tests/test_util.py | 42 ++++++++++++++++++++++++++++++++++ workbaskets/util.py | 17 +++++++------- 2 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 workbaskets/tests/test_util.py diff --git a/workbaskets/tests/test_util.py b/workbaskets/tests/test_util.py new file mode 100644 index 000000000..37331bcff --- /dev/null +++ b/workbaskets/tests/test_util.py @@ -0,0 +1,42 @@ +import pytest + +from common.tests import factories +from workbaskets.util import TableRow +from workbaskets.util import find_comm_code +from workbaskets.util import find_date +from workbaskets.util import get_delimiter + + +@pytest.mark.parametrize( + "date_string,exp", + [ + ("30-12-2020", "-"), + ("30/12/2020", "/"), + ("30+12+2020", None), + ], +) +def test_get_date_delimiter(date_string, exp): + delimiter = get_delimiter(date_string) + assert delimiter == exp + + +@pytest.mark.django_db +def test_get_comm_code(): + created_commodity = factories.GoodsNomenclatureFactory() + row_data = TableRow() + find_comm_code(created_commodity.item_id, row_data) + assert row_data.commodity == created_commodity + + +@pytest.mark.parametrize( + "cell,exp", + [ + ("12-05-2020", "12-05-2020"), + ("12230984", None), + ("04-04", None), + ("-----", None), + ], +) +def test_find_dates(cell, exp): + found_date = find_date(cell) + assert found_date == exp diff --git a/workbaskets/util.py b/workbaskets/util.py index 8b9da2332..dfbee8d46 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -82,12 +82,10 @@ def find_comm_code(cell, row_data): return False -def find_dates(cell, dates): +def find_date(cell): matches = re.compile(DATE_REGEX).match(cell) if matches: - dates.append(cell) - return True - return False + return cell def find_duty_sentence(cell, row_data): @@ -109,10 +107,10 @@ def find_duty_sentence(cell, row_data): def get_delimiter(date_string): if "/" in date_string: - delimiter = "/" + return "/" elif "-" in date_string: - delimiter = "-" - return delimiter + return "-" + return None def parse_dates(dates): @@ -172,8 +170,9 @@ def serialize_uploaded_data(data): if commodity: continue - found_dates = find_dates(cell, dates) - if found_dates: + found_date = find_date(cell) + if found_date: + dates.append(found_date) continue duty = find_duty_sentence(cell, row_data) From 8b578950b1e10ea462cfa307fc06a9ffccbeff23 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Mon, 4 Sep 2023 14:28:20 +0100 Subject: [PATCH 09/17] Finish tests for utils --- workbaskets/tests/test_util.py | 55 ++++++++++++++++++++++++++++++++++ workbaskets/util.py | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/workbaskets/tests/test_util.py b/workbaskets/tests/test_util.py index 37331bcff..e37f102b3 100644 --- a/workbaskets/tests/test_util.py +++ b/workbaskets/tests/test_util.py @@ -1,10 +1,16 @@ +from datetime import date + import pytest from common.tests import factories +from common.util import TaricDateRange from workbaskets.util import TableRow +from workbaskets.util import assign_validity from workbaskets.util import find_comm_code from workbaskets.util import find_date from workbaskets.util import get_delimiter +from workbaskets.util import parse_dates +from workbaskets.util import serialize_uploaded_data @pytest.mark.parametrize( @@ -40,3 +46,52 @@ def test_get_comm_code(): def test_find_dates(cell, exp): found_date = find_date(cell) assert found_date == exp + + +@pytest.mark.parametrize( + "dates", + [ + (["01-01-2020", "01-01-2000"]), + (["2020-01-01", "2000-01-01"]), + (["2020-01-01", "01-01-2000"]), + ], +) +def test_parse_dates(dates): + assert set(parse_dates(dates)) == {date(2020, 1, 1), date(2000, 1, 1)} + + +@pytest.mark.parametrize( + "dates", + [ + (["01-01-2020", "01-01-2000"]), + (["01-01-2000", "01-01-2020"]), + ], +) +def test_assign_validity(dates): + row_data = TableRow() + assign_validity(dates, row_data) + assert row_data.valid_between == TaricDateRange(date(2000, 1, 1), date(2020, 1, 1)) + + +@pytest.mark.django_db +def test_serialize_uploaded_data(): + commodity1 = factories.GoodsNomenclatureFactory() + commodity2 = factories.GoodsNomenclatureFactory() + + # format of copypasted data from excel + # each cell is separated by a \t tab character + # at the moment we only match measures against comm code, duty, and start/end dates + raw_data = ( + f"{commodity1.item_id}\t0.000% + 2.000 GBP / 100 kg\t20/05/2021\t31/08/2024\n" + f"{commodity2.item_id}\t0.000%\t\t31/08/2024\n" + "3945875\tfoo bar\t438573\t\n" # line with nonsense data + ) + serialized = serialize_uploaded_data(raw_data) + assert len(serialized) == 2 + assert serialized[0].valid_between == TaricDateRange( + date(2021, 5, 20), + date(2024, 8, 31), + ) + assert serialized[0].commodity == commodity1 + assert serialized[1].valid_between == TaricDateRange(date(2024, 8, 31), None) + assert serialized[1].commodity == commodity2 diff --git a/workbaskets/util.py b/workbaskets/util.py index dfbee8d46..1b409f310 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -66,7 +66,7 @@ def __init__(self, commodity=None, valid_between=None, duty=None): @property def all_none(self): - return not all([self.commodity, self.valid_between, self.duty]) + return not any([self.commodity, self.valid_between, self.duty]) def find_comm_code(cell, row_data): From 01d97e6cb7cc6677daec3f31eec87cc5905d7197 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Mon, 4 Sep 2023 14:46:18 +0100 Subject: [PATCH 10/17] Add form tests --- workbaskets/forms.py | 5 +-- workbaskets/tests/test_forms.py | 56 ++++++++++++++++++++++++++------- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/workbaskets/forms.py b/workbaskets/forms.py index 0419a87c3..c2da58336 100644 --- a/workbaskets/forms.py +++ b/workbaskets/forms.py @@ -157,8 +157,9 @@ class WorkbasketCompareForm(forms.Form): data = forms.CharField(widget=forms.Textarea(), validators=[SymbolValidator]) def clean(self): - serialized = serialize_uploaded_data(self.cleaned_data["data"]) - return {"data": serialized, "raw_data": self.cleaned_data["data"]} + if self.cleaned_data: + serialized = serialize_uploaded_data(self.cleaned_data["data"]) + return {"data": serialized, "raw_data": self.cleaned_data["data"]} def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/workbaskets/tests/test_forms.py b/workbaskets/tests/test_forms.py index 6906198a0..64dadc4df 100644 --- a/workbaskets/tests/test_forms.py +++ b/workbaskets/tests/test_forms.py @@ -1,9 +1,7 @@ import pytest from common.tests import factories -from workbaskets.forms import SelectableObjectField -from workbaskets.forms import SelectableObjectsForm -from workbaskets.forms import WorkbasketCreateForm +from workbaskets import forms from workbaskets.validators import tops_jira_number_validator pytestmark = pytest.mark.django_db @@ -12,7 +10,7 @@ def test_workbasket_create_form_valid_data(): """Test that WorkbasketCreateForm is valid when required fields in data.""" data = {"title": "123", "reason": "testing testing"} - form = WorkbasketCreateForm(data=data) + form = forms.WorkbasketCreateForm(data=data) assert form.is_valid() @@ -21,17 +19,17 @@ def test_workbasket_create_form_invalid_data(): """Test that WorkbasketCreateForm is not valid when required fields not in data.""" - form = WorkbasketCreateForm(data={}) + form = forms.WorkbasketCreateForm(data={}) assert not form.is_valid() assert "This field is required." in form.errors["title"] assert "This field is required." in form.errors["reason"] - form = WorkbasketCreateForm(data={"title": "abc", "reason": "test"}) + form = forms.WorkbasketCreateForm(data={"title": "abc", "reason": "test"}) assert not form.is_valid() assert tops_jira_number_validator.message in form.errors["title"] factories.WorkBasketFactory(title="123321") - form = WorkbasketCreateForm(data={"title": "123321", "reason": "test"}) + form = forms.WorkbasketCreateForm(data={"title": "123321", "reason": "test"}) assert not form.is_valid() assert "Workbasket with this Title already exists." in form.errors["title"] @@ -41,7 +39,7 @@ def test_selectable_objects_form(): each item passed to it, and the class methods and properties provide the expected data for the item.""" measures = factories.MeasureFactory.create_batch(5) - form = SelectableObjectsForm(initial={}, prefix=None, objects=measures) + form = forms.SelectableObjectsForm(initial={}, prefix=None, objects=measures) # grab a list of the ids tacked on to the end of the field name, as this # should be the pk of the measure. form_fields_keys = [str.partition("_")[2] for str in form.fields.keys()] @@ -55,18 +53,23 @@ def test_selectable_objects_form(): for item in measures: # Check that the field name class method returns the right name. assert ( - SelectableObjectsForm.field_name_for_object(measures[measures.index(item)]) + forms.SelectableObjectsForm.field_name_for_object( + measures[measures.index(item)], + ) == f"selectableobject_{item.pk}" ) # Check that the id class method returns the right id. assert ( - SelectableObjectsForm.object_id_from_field_name( + forms.SelectableObjectsForm.object_id_from_field_name( list(form.fields.keys())[measures.index(item)], ) == measure_pks[measures.index(item)] ) # Check that the fields are of SelectableObjectField type - assert type(form.fields[f"selectableobject_{item.pk}"]) is SelectableObjectField + assert ( + type(form.fields[f"selectableobject_{item.pk}"]) + is forms.SelectableObjectField + ) # Check that the cleaned_data_no_prefix property returns a list of the data with no prefixes. form.cleaned_data = { @@ -87,4 +90,33 @@ def test_selectable_objects_form(): } # Check that the fields are of SelectableObjectField type for item in measures: - assert type(form.fields[f"selectableobject_{item.pk}"]) is SelectableObjectField + assert ( + type(form.fields[f"selectableobject_{item.pk}"]) + is forms.SelectableObjectField + ) + + +def test_workbasket_compare_form_valid(): + data = { + "data": ( + "2909500090\t0.000% + 2.000 GBP / 100 kg\t20/05/2021\t31/08/2024\n" + "2909500090\t0.000%\t\t31/08/2024\n" + "3945875\tfoo bar\t438573\t\n" # line with nonsense data + ), + } + form = forms.WorkbasketCompareForm(data=data) + assert form.is_valid() + assert form.cleaned_data + + +def test_workbasket_compare_form_invalid(): + data = { + "data": ( + "2909500090\t0.000% + 2.000 GBP / 100 kg\t20/05/2021\t31/08/2024\n" + "290950<>0090\t0.000%\t\t31/08/2024\n" + "3945875\tfoo bar\t438573\t\n" # line with nonsense data + ), + } + form = forms.WorkbasketCompareForm(data=data) + assert not form.is_valid() + assert "Only symbols" in form.errors["data"][0] From 7dcf4ff46969d964062ab0ba190749dc973a6c33 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Mon, 4 Sep 2023 16:40:17 +0100 Subject: [PATCH 11/17] Test workbasket compare form submit --- common/tests/factories.py | 22 ++++++++++++++ workbaskets/tests/test_views.py | 53 ++++++++++++++++++++++++--------- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/common/tests/factories.py b/common/tests/factories.py index 278cf4b3d..b7d7c6950 100644 --- a/common/tests/factories.py +++ b/common/tests/factories.py @@ -1404,3 +1404,25 @@ class CrownDependenciesPublishingOperationalStatusFactory( class Meta: model = "publishing.CrownDependenciesPublishingOperationalStatus" + + +class DataUploadFactory(factory.django.DjangoModelFactory): + """Creates a DataUpload instance.""" + + class Meta: + model = "workbaskets.DataUpload" + + raw_data = factory.Faker("text", max_nb_chars=500) + workbasket = factory.SubFactory(WorkBasketFactory) + + +class DataRowFactory(factory.django.DjangoModelFactory): + """Creates a DataRow instance.""" + + class Meta: + model = "workbaskets.DataRow" + + data_upload = factory.SubFactory(DataUploadFactory) + commodity = factory.SubFactory(GoodsNomenclatureFactory) + duty_sentence = factory.Faker("text", max_nb_chars=24) + valid_between = date_ranges("no_end") diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index 6a8559f80..1f21fd6dd 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -12,9 +12,6 @@ from checks.tests.factories import TrackedModelCheckFactory from common.models.utils import override_current_transaction from common.tests import factories -from common.tests.factories import GeographicalAreaFactory -from common.tests.factories import GoodsNomenclatureFactory -from common.tests.factories import MeasureFactory from exporter.tasks import upload_workbaskets from measures.models import Measure from workbaskets import models @@ -165,7 +162,7 @@ def test_review_workbasket_displays_objects_in_current_workbasket( selection form of the review workbasket page.""" with session_workbasket.new_transaction(): - GoodsNomenclatureFactory.create() + factories.GoodsNomenclatureFactory.create() response = valid_user_client.get( reverse( @@ -189,7 +186,7 @@ def test_review_workbasket_displays_rule_violation_summary( detailing the number of tracked model changes and business rule violations, dated to the most recent `TrackedModelCheck`.""" with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) check = TrackedModelCheckFactory.create( transaction_check__transaction=transaction, model=good, @@ -555,7 +552,7 @@ def test_run_business_rules(check_workbasket, valid_user_client, session_workbas assert not session_workbasket.rule_check_task_id with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) check = TrackedModelCheckFactory.create( transaction_check__transaction=transaction, model=good, @@ -630,9 +627,9 @@ def test_submit_for_packaging(valid_user_client, session_workbasket): """Test that a GET request to the submit-for-packaging endpoint returns a 302, redirecting to the create packaged workbasket page.""" with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) - measure = MeasureFactory.create(transaction=transaction) - geo_area = GeographicalAreaFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) + measure = factories.MeasureFactory.create(transaction=transaction) + geo_area = factories.GeographicalAreaFactory.create(transaction=transaction) objects = [good, measure, geo_area] for obj in objects: TrackedModelCheckFactory.create( @@ -689,7 +686,7 @@ def test_workbasket_violations(valid_user_client, session_workbasket): "workbaskets:workbasket-ui-violations", ) with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) check = TrackedModelCheckFactory.create( transaction_check__transaction=transaction, model=good, @@ -721,7 +718,7 @@ def test_workbasket_violations(valid_user_client, session_workbasket): def test_violation_detail_page(valid_user_client, session_workbasket): with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) check = TrackedModelCheckFactory.create( transaction_check__transaction=transaction, model=good, @@ -864,9 +861,9 @@ def test_violation_detail_page_non_superuser_override_violation( @pytest.fixture def setup(session_workbasket, valid_user_client): with session_workbasket.new_transaction() as transaction: - good = GoodsNomenclatureFactory.create(transaction=transaction) - measure = MeasureFactory.create(transaction=transaction) - geo_area = GeographicalAreaFactory.create(transaction=transaction) + good = factories.GoodsNomenclatureFactory.create(transaction=transaction) + measure = factories.MeasureFactory.create(transaction=transaction) + geo_area = factories.GeographicalAreaFactory.create(transaction=transaction) regulation = factories.RegulationFactory.create(transaction=transaction) additional_code = factories.AdditionalCodeFactory.create( transaction=transaction, @@ -1193,3 +1190,31 @@ def test_application_access_after_workbasket_delete( # workbasket deletion. assert response.status_code == 200 assert not page.select("header nav a.workbasket-link") + + +def test_workbasket_compare_200(valid_user_client, session_workbasket): + url = reverse("workbaskets:workbasket-ui-compare") + response = valid_user_client.get(url) + assert response.status_code == 200 + + +def test_workbasket_compare_prev_uploaded(valid_user_client, session_workbasket): + factories.GoodsNomenclatureFactory() + factories.GoodsNomenclatureFactory() + data_upload = factories.DataUploadFactory(workbasket=session_workbasket) + url = reverse("workbaskets:workbasket-ui-compare") + response = valid_user_client.get(url) + assert "Previously uploaded data" in response.content.decode(response.charset) + + +def test_workbasket_compare_form_submit_302(valid_user_client, session_workbasket): + url = reverse("workbaskets:workbasket-ui-compare") + data = { + "data": ( + "0000000001\t1.000%\t20/05/2021\t31/08/2024\n" + "0000000002\t2.000%\t20/05/2021\t31/08/2024\n" + ), + } + response = valid_user_client.post(url, data) + assert response.status_code == 302 + assert response.url == url From 53eba61caa98e46a48a2daf4a8b70c27de7f0e1d Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 5 Sep 2023 12:43:02 +0100 Subject: [PATCH 12/17] Move duty sentence parser and related fixtures into top level conftest file --- conftest.py | 273 ++++++++++++++++++++++++++++++++++++ measures/tests/conftest.py | 275 ------------------------------------- 2 files changed, 273 insertions(+), 275 deletions(-) diff --git a/conftest.py b/conftest.py index 0f69c106b..354490825 100644 --- a/conftest.py +++ b/conftest.py @@ -8,6 +8,7 @@ from typing import Dict from typing import Optional from typing import Sequence +from typing import Tuple from typing import Type from unittest.mock import patch @@ -53,10 +54,18 @@ from common.tests.util import make_duplicate_record from common.tests.util import make_non_duplicate_record from common.tests.util import raises_if +from common.validators import ApplicabilityCode from common.validators import UpdateType from importer.models import ImportBatchStatus from importer.nursery import get_nursery from importer.taric import process_taric_xml_stream +from measures.models import DutyExpression +from measures.models import MeasureConditionComponent +from measures.models import Measurement +from measures.models import MeasurementUnit +from measures.models import MeasurementUnitQualifier +from measures.models import MonetaryUnit +from measures.parsers import DutySentenceParser from workbaskets.models import WorkBasket from workbaskets.models import get_partition_scheme from workbaskets.validators import WorkflowStatus @@ -1547,3 +1556,267 @@ def empty_goods_import_batch(): status=ImportBatchStatus.SUCCEEDED, workbasket_id=archived_workbasket.id, ) + + +@pytest.fixture +def duty_sentence_parser( + duty_expressions: Dict[int, DutyExpression], + monetary_units: Dict[str, MonetaryUnit], + measurements: Dict[Tuple[str, Optional[str]], Measurement], +) -> DutySentenceParser: + return DutySentenceParser( + duty_expressions.values(), + monetary_units.values(), + measurements.values(), + ) + + +@pytest.fixture +def percent_or_amount() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=1, + prefix="", + duty_amount_applicability_code=ApplicabilityCode.MANDATORY, + measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, + monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, + ) + + +@pytest.fixture +def plus_percent_or_amount() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=4, + prefix="+", + duty_amount_applicability_code=ApplicabilityCode.MANDATORY, + measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, + monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, + ) + + +@pytest.fixture +def plus_agri_component() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=12, + prefix="+ AC", + duty_amount_applicability_code=ApplicabilityCode.NOT_PERMITTED, + measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, + monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, + ) + + +@pytest.fixture +def plus_amount_only() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=20, + prefix="+", + duty_amount_applicability_code=ApplicabilityCode.MANDATORY, + measurement_unit_applicability_code=ApplicabilityCode.MANDATORY, + monetary_unit_applicability_code=ApplicabilityCode.MANDATORY, + ) + + +@pytest.fixture +def nothing() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=37, + prefix="NIHIL", + duty_amount_applicability_code=ApplicabilityCode.NOT_PERMITTED, + measurement_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, + monetary_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, + ) + + +@pytest.fixture +def supplementary_unit() -> DutyExpression: + return factories.DutyExpressionFactory( + sid=99, + prefix="", + duty_amount_applicability_code=ApplicabilityCode.PERMITTED, + measurement_unit_applicability_code=ApplicabilityCode.MANDATORY, + monetary_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, + ) + + +@pytest.fixture +def duty_expressions( + percent_or_amount: DutyExpression, + plus_percent_or_amount: DutyExpression, + plus_agri_component: DutyExpression, + plus_amount_only: DutyExpression, + supplementary_unit: DutyExpression, + nothing: DutyExpression, +) -> Dict[int, DutyExpression]: + return { + d.sid: d + for d in [ + percent_or_amount, + plus_percent_or_amount, + plus_agri_component, + plus_amount_only, + supplementary_unit, + nothing, + ] + } + + +@pytest.fixture +def monetary_units() -> Dict[str, MonetaryUnit]: + return { + m.code: m + for m in [ + factories.MonetaryUnitFactory(code="EUR"), + factories.MonetaryUnitFactory(code="GBP"), + factories.MonetaryUnitFactory(code="XEM"), + ] + } + + +@pytest.fixture +def measurement_units() -> Sequence[MeasurementUnit]: + return [ + factories.MeasurementUnitFactory(code="KGM", abbreviation="kg"), + factories.MeasurementUnitFactory(code="DTN", abbreviation="100 kg"), + factories.MeasurementUnitFactory(code="MIL", abbreviation="1,000 p/st"), + ] + + +@pytest.fixture +def unit_qualifiers() -> Sequence[MeasurementUnitQualifier]: + return [ + factories.MeasurementUnitQualifierFactory(code="Z", abbreviation="lactic."), + ] + + +@pytest.fixture +def measurements( + measurement_units, + unit_qualifiers, +) -> Dict[Tuple[str, Optional[str]], Measurement]: + measurements = [ + *[ + factories.MeasurementFactory( + measurement_unit=m, + measurement_unit_qualifier=None, + ) + for m in measurement_units + ], + factories.MeasurementFactory( + measurement_unit=measurement_units[1], + measurement_unit_qualifier=unit_qualifiers[0], + ), + ] + return { + ( + m.measurement_unit.code, + m.measurement_unit_qualifier.code if m.measurement_unit_qualifier else None, + ): m + for m in measurements + } + + +@pytest.fixture +def condition_duty_sentence_parser( + duty_expressions: Dict[int, DutyExpression], + monetary_units: Dict[str, MonetaryUnit], + measurements: Dict[Tuple[str, Optional[str]], Measurement], +) -> DutySentenceParser: + return DutySentenceParser( + duty_expressions.values(), + monetary_units.values(), + measurements.values(), + MeasureConditionComponent, + ) + + +@pytest.fixture +def get_component_data(duty_expressions, monetary_units, measurements) -> Callable: + def getter( + duty_expression_id, + amount, + monetary_unit_code, + measurement_codes, + ) -> Dict: + return { + "duty_expression": duty_expressions.get(duty_expression_id), + "duty_amount": amount, + "monetary_unit": monetary_units.get(monetary_unit_code), + "component_measurement": measurements.get(measurement_codes), + } + + return getter + + +@pytest.fixture( + params=( + ("4.000%", [(1, 4.0, None, None)]), + ("1.230 EUR / kg", [(1, 1.23, "EUR", ("KGM", None))]), + ("0.300 XEM / 100 kg / lactic.", [(1, 0.3, "XEM", ("DTN", "Z"))]), + ( + "12.900% + 20.000 EUR / kg", + [(1, 12.9, None, None), (4, 20.0, "EUR", ("KGM", None))], + ), + ("kg", [(99, None, None, ("KGM", None))]), + ("100 kg", [(99, None, None, ("DTN", None))]), + ("1.000 EUR", [(1, 1.0, "EUR", None)]), + ("0.000% + AC", [(1, 0.0, None, None), (12, None, None, None)]), + ), + ids=[ + "simple_ad_valorem", + "simple_specific_duty", + "unit_with_qualifier", + "multi_component_expression", + "supplementary_unit", + "supplementary_unit_with_numbers", + "monetary_unit_without_measurement", + "non_amount_expression", + ], +) +def reversible_duty_sentence_data(request, get_component_data): + """Duty sentence test cases that are syntactically correct and are also + formatted correctly.""" + expected, component_data = request.param + return expected, [get_component_data(*args) for args in component_data] + + +@pytest.fixture( + params=( + ("20.0 EUR/100kg", [(1, 20.0, "EUR", ("DTN", None))]), + ("1.0 EUR/1000 p/st", [(1, 1.0, "EUR", ("MIL", None))]), + ), + ids=[ + "parses_without_spaces", + "parses_without_commas", + ], +) +def irreversible_duty_sentence_data(request, get_component_data): + """Duty sentence test cases that are syntactically correct but are not in + the canonical rendering format with spaces and commas in the correct + places.""" + expected, component_data = request.param + return expected, [get_component_data(*args) for args in component_data] + + +@pytest.fixture( + params=( + ( + ( + "0.000% + AC", + [(1, 0.0, None, None), (12, None, None, None)], + ), + ( + "12.900% + 20.000 EUR / kg", + [(1, 12.9, None, None), (4, 20.0, "EUR", ("KGM", None))], + ), + ), + ), +) +def duty_sentence_x_2_data(request, get_component_data): + """Duty sentence test cases that can be used to create a history of + components.""" + history = [] + for version in request.param: + expected, component_data = version + history.append( + (expected, [get_component_data(*args) for args in component_data]), + ) + return history diff --git a/measures/tests/conftest.py b/measures/tests/conftest.py index 257fd023f..b94e9bb01 100644 --- a/measures/tests/conftest.py +++ b/measures/tests/conftest.py @@ -1,9 +1,5 @@ import datetime -from typing import Callable from typing import Dict -from typing import Optional -from typing import Sequence -from typing import Tuple import pytest import requests @@ -19,16 +15,9 @@ from geo_areas.validators import AreaCode from measures.forms import MEASURE_CONDITIONS_FORMSET_PREFIX from measures.forms import MeasureForm -from measures.models import DutyExpression from measures.models import Measure from measures.models import MeasureAction from measures.models import MeasureConditionCode -from measures.models import MeasureConditionComponent -from measures.models import Measurement -from measures.models import MeasurementUnit -from measures.models import MeasurementUnitQualifier -from measures.models import MonetaryUnit -from measures.parsers import DutySentenceParser @pytest.fixture @@ -168,94 +157,6 @@ def existing_measure(request, existing_goods_nomenclature): return factories.MeasureWithQuotaFactory.create(**data, **now), overlaps_normal -@pytest.fixture -def percent_or_amount() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=1, - prefix="", - duty_amount_applicability_code=ApplicabilityCode.MANDATORY, - measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, - monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, - ) - - -@pytest.fixture -def plus_percent_or_amount() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=4, - prefix="+", - duty_amount_applicability_code=ApplicabilityCode.MANDATORY, - measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, - monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, - ) - - -@pytest.fixture -def plus_agri_component() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=12, - prefix="+ AC", - duty_amount_applicability_code=ApplicabilityCode.NOT_PERMITTED, - measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, - monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, - ) - - -@pytest.fixture -def plus_amount_only() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=20, - prefix="+", - duty_amount_applicability_code=ApplicabilityCode.MANDATORY, - measurement_unit_applicability_code=ApplicabilityCode.MANDATORY, - monetary_unit_applicability_code=ApplicabilityCode.MANDATORY, - ) - - -@pytest.fixture -def nothing() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=37, - prefix="NIHIL", - duty_amount_applicability_code=ApplicabilityCode.NOT_PERMITTED, - measurement_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, - monetary_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, - ) - - -@pytest.fixture -def supplementary_unit() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=99, - prefix="", - duty_amount_applicability_code=ApplicabilityCode.PERMITTED, - measurement_unit_applicability_code=ApplicabilityCode.MANDATORY, - monetary_unit_applicability_code=ApplicabilityCode.NOT_PERMITTED, - ) - - -@pytest.fixture -def duty_expressions( - percent_or_amount: DutyExpression, - plus_percent_or_amount: DutyExpression, - plus_agri_component: DutyExpression, - plus_amount_only: DutyExpression, - supplementary_unit: DutyExpression, - nothing: DutyExpression, -) -> Dict[int, DutyExpression]: - return { - d.sid: d - for d in [ - percent_or_amount, - plus_percent_or_amount, - plus_agri_component, - plus_amount_only, - supplementary_unit, - nothing, - ] - } - - @pytest.fixture def condition_codes() -> Dict[str, MeasureConditionCode]: return { @@ -292,182 +193,6 @@ def certificates(): } -@pytest.fixture -def monetary_units() -> Dict[str, MonetaryUnit]: - return { - m.code: m - for m in [ - factories.MonetaryUnitFactory(code="EUR"), - factories.MonetaryUnitFactory(code="GBP"), - factories.MonetaryUnitFactory(code="XEM"), - ] - } - - -@pytest.fixture -def measurement_units() -> Sequence[MeasurementUnit]: - return [ - factories.MeasurementUnitFactory(code="KGM", abbreviation="kg"), - factories.MeasurementUnitFactory(code="DTN", abbreviation="100 kg"), - factories.MeasurementUnitFactory(code="MIL", abbreviation="1,000 p/st"), - ] - - -@pytest.fixture -def unit_qualifiers() -> Sequence[MeasurementUnitQualifier]: - return [ - factories.MeasurementUnitQualifierFactory(code="Z", abbreviation="lactic."), - ] - - -@pytest.fixture -def measurements( - measurement_units, - unit_qualifiers, -) -> Dict[Tuple[str, Optional[str]], Measurement]: - measurements = [ - *[ - factories.MeasurementFactory( - measurement_unit=m, - measurement_unit_qualifier=None, - ) - for m in measurement_units - ], - factories.MeasurementFactory( - measurement_unit=measurement_units[1], - measurement_unit_qualifier=unit_qualifiers[0], - ), - ] - return { - ( - m.measurement_unit.code, - m.measurement_unit_qualifier.code if m.measurement_unit_qualifier else None, - ): m - for m in measurements - } - - -@pytest.fixture -def duty_sentence_parser( - duty_expressions: Dict[int, DutyExpression], - monetary_units: Dict[str, MonetaryUnit], - measurements: Dict[Tuple[str, Optional[str]], Measurement], -) -> DutySentenceParser: - return DutySentenceParser( - duty_expressions.values(), - monetary_units.values(), - measurements.values(), - ) - - -@pytest.fixture -def condition_duty_sentence_parser( - duty_expressions: Dict[int, DutyExpression], - monetary_units: Dict[str, MonetaryUnit], - measurements: Dict[Tuple[str, Optional[str]], Measurement], -) -> DutySentenceParser: - return DutySentenceParser( - duty_expressions.values(), - monetary_units.values(), - measurements.values(), - MeasureConditionComponent, - ) - - -@pytest.fixture -def get_component_data(duty_expressions, monetary_units, measurements) -> Callable: - def getter( - duty_expression_id, - amount, - monetary_unit_code, - measurement_codes, - ) -> Dict: - return { - "duty_expression": duty_expressions.get(duty_expression_id), - "duty_amount": amount, - "monetary_unit": monetary_units.get(monetary_unit_code), - "component_measurement": measurements.get(measurement_codes), - } - - return getter - - -@pytest.fixture( - params=( - ("4.000%", [(1, 4.0, None, None)]), - ("1.230 EUR / kg", [(1, 1.23, "EUR", ("KGM", None))]), - ("0.300 XEM / 100 kg / lactic.", [(1, 0.3, "XEM", ("DTN", "Z"))]), - ( - "12.900% + 20.000 EUR / kg", - [(1, 12.9, None, None), (4, 20.0, "EUR", ("KGM", None))], - ), - ("kg", [(99, None, None, ("KGM", None))]), - ("100 kg", [(99, None, None, ("DTN", None))]), - ("1.000 EUR", [(1, 1.0, "EUR", None)]), - ("0.000% + AC", [(1, 0.0, None, None), (12, None, None, None)]), - ), - ids=[ - "simple_ad_valorem", - "simple_specific_duty", - "unit_with_qualifier", - "multi_component_expression", - "supplementary_unit", - "supplementary_unit_with_numbers", - "monetary_unit_without_measurement", - "non_amount_expression", - ], -) -def reversible_duty_sentence_data(request, get_component_data): - """Duty sentence test cases that are syntactically correct and are also - formatted correctly.""" - expected, component_data = request.param - return expected, [get_component_data(*args) for args in component_data] - - -@pytest.fixture( - params=( - ("20.0 EUR/100kg", [(1, 20.0, "EUR", ("DTN", None))]), - ("1.0 EUR/1000 p/st", [(1, 1.0, "EUR", ("MIL", None))]), - ), - ids=[ - "parses_without_spaces", - "parses_without_commas", - ], -) -def irreversible_duty_sentence_data(request, get_component_data): - """Duty sentence test cases that are syntactically correct but are not in - the canonical rendering format with spaces and commas in the correct - places.""" - expected, component_data = request.param - return expected, [get_component_data(*args) for args in component_data] - - -@pytest.fixture( - params=( - ( - ( - "0.000% + AC", - [(1, 0.0, None, None), (12, None, None, None)], - ), - ( - "12.900% + 20.000 EUR / kg", - [(1, 12.9, None, None), (4, 20.0, "EUR", ("KGM", None))], - ), - ), - ), -) -def duty_sentence_x_2_data(request, get_component_data): - """Duty sentence test cases that can be used to create a history of - components.""" - history = [] - for version in request.param: - expected, component_data = version - history.append( - (expected, [get_component_data(*args) for args in component_data]), - ) - return history - - @pytest.fixture def erga_omnes(): return factories.GeographicalAreaFactory.create( From 0ab0544ff8daad8d21a9d7ce9848c53bf833e66e Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 5 Sep 2023 16:54:01 +0100 Subject: [PATCH 13/17] Remove print statement --- common/tariffs_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/common/tariffs_api.py b/common/tariffs_api.py index 1ebd6b2aa..55568e0e3 100644 --- a/common/tariffs_api.py +++ b/common/tariffs_api.py @@ -41,7 +41,6 @@ def parse_response(response): def get_commodity_data(id): url = f"{Endpoints.COMMODITIES.value}{id}" - print(url) return parse_response(requests.get(url)) From 3cca3ad4af362f3ff95c18b099882744fcc88e77 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 6 Sep 2023 13:08:00 +0100 Subject: [PATCH 14/17] Standardise naming --- workbaskets/util.py | 12 ++++++------ workbaskets/views/ui.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/workbaskets/util.py b/workbaskets/util.py index 1b409f310..8d4c65b10 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -59,14 +59,14 @@ def delete_workbasket(workbasket): class TableRow: - def __init__(self, commodity=None, valid_between=None, duty=None): + def __init__(self, commodity=None, valid_between=None, duty_sentence=None): self.commodity = commodity self.valid_between = valid_between - self.duty = duty + self.duty_sentence = duty_sentence @property def all_none(self): - return not any([self.commodity, self.valid_between, self.duty]) + return not any([self.commodity, self.valid_between, self.duty_sentence]) def find_comm_code(cell, row_data): @@ -99,7 +99,7 @@ def find_duty_sentence(cell, row_data): duty_sentence = cell.replace(" ", "") try: duty_sentence_parser.parse(duty_sentence) - row_data.duty = cell + row_data.duty_sentence = cell return True except ParseError: return False @@ -175,8 +175,8 @@ def serialize_uploaded_data(data): dates.append(found_date) continue - duty = find_duty_sentence(cell, row_data) - if duty: + duty_sentence = find_duty_sentence(cell, row_data) + if duty_sentence: continue assign_validity(dates, row_data) diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index aa0a48f8b..22e902dff 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -708,20 +708,20 @@ def form_valid(self, form): for row in form.cleaned_data["data"]: DataRow.objects.create( valid_between=row.valid_between, - duty_sentence=row.duty, + duty_sentence=row.duty_sentence, commodity=row.commodity, data_upload=existing, ) existing.save() except DataUpload.DoesNotExist: data_upload = DataUpload.objects.create( - raw_data=form.cleaned_data["data"], + raw_data=form.cleaned_data["raw_data"], workbasket=self.workbasket, ) for row in form.cleaned_data["data"]: DataRow.objects.create( valid_between=row.valid_between, - duty_sentence=row.duty, + duty_sentence=row.duty_sentence, commodity=row.commodity, data_upload=data_upload, ) From e6768b00280f01214226a3233762ac13d5ad7131 Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Wed, 6 Sep 2023 13:08:29 +0100 Subject: [PATCH 15/17] Add tests --- workbaskets/tests/test_views.py | 69 ++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index 1f21fd6dd..174e14c35 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -1201,12 +1201,29 @@ def test_workbasket_compare_200(valid_user_client, session_workbasket): def test_workbasket_compare_prev_uploaded(valid_user_client, session_workbasket): factories.GoodsNomenclatureFactory() factories.GoodsNomenclatureFactory() - data_upload = factories.DataUploadFactory(workbasket=session_workbasket) + factories.DataUploadFactory(workbasket=session_workbasket) url = reverse("workbaskets:workbasket-ui-compare") response = valid_user_client.get(url) assert "Previously uploaded data" in response.content.decode(response.charset) +def test_workbasket_update_prev_uploaded(valid_user_client, session_workbasket): + factories.GoodsNomenclatureFactory() + factories.GoodsNomenclatureFactory() + data_upload = factories.DataUploadFactory(workbasket=session_workbasket) + url = reverse("workbaskets:workbasket-ui-compare") + data = { + "data": ( + "0000000001\t1.000%\t20/05/2021\t31/08/2024\n" + "0000000002\t2.000%\t20/05/2021\t31/08/2024" + ), + } + response = valid_user_client.post(url, data) + assert response.status_code == 302 + data_upload.refresh_from_db() + assert data_upload.raw_data == data["data"] + + def test_workbasket_compare_form_submit_302(valid_user_client, session_workbasket): url = reverse("workbaskets:workbasket-ui-compare") data = { @@ -1218,3 +1235,53 @@ def test_workbasket_compare_form_submit_302(valid_user_client, session_workbaske response = valid_user_client.post(url, data) assert response.status_code == 302 assert response.url == url + + +def test_workbasket_compare_found_measures( + valid_user_client, + session_workbasket, + date_ranges, + duty_sentence_parser, + percent_or_amount, +): + commodity = factories.GoodsNomenclatureFactory() + + with session_workbasket.new_transaction(): + measure = factories.MeasureFactory( + valid_between=date_ranges.normal, + goods_nomenclature=commodity, + ) + duty_string = "4.000%" + # create measure components equivalent to a duty sentence of "4.000%" + factories.MeasureComponentFactory.create( + component_measure=measure, + duty_expression=percent_or_amount, + duty_amount=4.0, + monetary_unit=None, + component_measurement=None, + ) + + url = reverse("workbaskets:workbasket-ui-compare") + data = { + "data": ( + # this first line should match the measure in the workbasket + f"{commodity.item_id}\t{duty_string}\t{date_ranges.normal.lower.isoformat()}\t{date_ranges.normal.upper.isoformat()}\n" + "0000000002\t2.000%\t20/05/2021\t31/08/2024\n" + ), + } + response = valid_user_client.post(url, data) + assert response.status_code == 302 + assert response.url == url + + # view the uploaded data + response2 = valid_user_client.get(response.url) + assert response2.status_code == 200 + decoded = response2.content.decode(response2.charset) + soup = BeautifulSoup(decoded, "html.parser") + assert "1 matching measure found" in soup.select("h2")[1].text + + # previously uploaded data + assert len(soup.select("tbody")[0].select("tr")) == 2 + + # measure found + assert len(soup.select("tbody")[1].select("tr")) == 1 From e23c625109d80be5710dcc217e637de1bb9c32cb Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 12 Sep 2023 14:51:59 +0100 Subject: [PATCH 16/17] Content changes, fix incorrect attribute name --- workbaskets/forms.py | 6 +++++- workbaskets/jinja2/includes/workbaskets/navigation.jinja | 2 +- workbaskets/jinja2/workbaskets/compare.jinja | 6 +++--- workbaskets/tests/test_views.py | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/workbaskets/forms.py b/workbaskets/forms.py index c2da58336..eaafb9ab7 100644 --- a/workbaskets/forms.py +++ b/workbaskets/forms.py @@ -154,7 +154,11 @@ def cleaned_data_no_prefix(self): class WorkbasketCompareForm(forms.Form): - data = forms.CharField(widget=forms.Textarea(), validators=[SymbolValidator]) + data = forms.CharField( + label="Compare worksheet data against the measures in this workbasket", + widget=forms.Textarea(), + validators=[SymbolValidator], + ) def clean(self): if self.cleaned_data: diff --git a/workbaskets/jinja2/includes/workbaskets/navigation.jinja b/workbaskets/jinja2/includes/workbaskets/navigation.jinja index 0deb803e8..1817982a9 100644 --- a/workbaskets/jinja2/includes/workbaskets/navigation.jinja +++ b/workbaskets/jinja2/includes/workbaskets/navigation.jinja @@ -17,7 +17,7 @@ Review goods
  • - Compare data + Worksheet check
    • diff --git a/workbaskets/jinja2/workbaskets/compare.jinja b/workbaskets/jinja2/workbaskets/compare.jinja index 33845ec35..683b964f6 100644 --- a/workbaskets/jinja2/workbaskets/compare.jinja +++ b/workbaskets/jinja2/workbaskets/compare.jinja @@ -36,13 +36,13 @@ {% endcall %} {% if data_upload %} -

    Previously uploaded data

    +

    Worksheet data

    {% set table_rows = [] %} {% for row in data_upload.serialized %} {{ table_rows.append([ - {"text": row.commodity}, - {"text": row.duty }, + {"text": row.commodity if row.commodity else "—"}, + {"text": row.duty_sentence if row.duty_sentence else "—" }, {"text": "{:%d %b %Y}".format(row.valid_between.lower) }, {"text": "{:%d %b %Y}".format(row.valid_between.upper) if row.valid_between.upper else "—" }, ]) or "" }} diff --git a/workbaskets/tests/test_views.py b/workbaskets/tests/test_views.py index 174e14c35..5ecfc3fcb 100644 --- a/workbaskets/tests/test_views.py +++ b/workbaskets/tests/test_views.py @@ -1204,7 +1204,7 @@ def test_workbasket_compare_prev_uploaded(valid_user_client, session_workbasket) factories.DataUploadFactory(workbasket=session_workbasket) url = reverse("workbaskets:workbasket-ui-compare") response = valid_user_client.get(url) - assert "Previously uploaded data" in response.content.decode(response.charset) + assert "Worksheet data" in response.content.decode(response.charset) def test_workbasket_update_prev_uploaded(valid_user_client, session_workbasket): From 15cc05b0d993a026d0a4555341edf2921df1bd8c Mon Sep 17 00:00:00 2001 From: Edie Pearce Date: Tue, 12 Sep 2023 16:47:27 +0100 Subject: [PATCH 17/17] Use only item_id for commodity --- workbaskets/migrations/0008_datarow_dataupload.py | 12 ++---------- workbaskets/models.py | 6 +++--- workbaskets/tests/test_util.py | 6 +++--- workbaskets/util.py | 7 +------ workbaskets/views/ui.py | 2 +- 5 files changed, 10 insertions(+), 23 deletions(-) diff --git a/workbaskets/migrations/0008_datarow_dataupload.py b/workbaskets/migrations/0008_datarow_dataupload.py index 9c98153d3..02d74ee3b 100644 --- a/workbaskets/migrations/0008_datarow_dataupload.py +++ b/workbaskets/migrations/0008_datarow_dataupload.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.20 on 2023-08-09 12:39 +# Generated by Django 3.2.20 on 2023-09-12 15:37 import django.db.models.deletion from django.db import migrations @@ -9,7 +9,6 @@ class Migration(migrations.Migration): dependencies = [ - ("commodities", "0012_TOPS_745_description_date_fix"), ("workbaskets", "0007_alter_workbasket_options"), ] @@ -51,18 +50,11 @@ class Migration(migrations.Migration): ), ), ("valid_between", common.fields.TaricDateRangeField(db_index=True)), + ("commodity", models.CharField(blank=True, max_length=255, null=True)), ( "duty_sentence", models.CharField(blank=True, max_length=255, null=True), ), - ( - "commodity", - models.ForeignKey( - null=True, - on_delete=django.db.models.deletion.PROTECT, - to="commodities.goodsnomenclature", - ), - ), ( "data_upload", models.ForeignKey( diff --git a/workbaskets/models.py b/workbaskets/models.py index 038419a09..aa6372f2f 100644 --- a/workbaskets/models.py +++ b/workbaskets/models.py @@ -650,10 +650,10 @@ class DataRow(ValidityMixin, models.Model): null=True, related_name="rows", ) - commodity = models.ForeignKey( - "commodities.GoodsNomenclature", - on_delete=models.PROTECT, + commodity = models.CharField( + max_length=255, null=True, + blank=True, ) duty_sentence = models.CharField( max_length=255, diff --git a/workbaskets/tests/test_util.py b/workbaskets/tests/test_util.py index e37f102b3..04a629446 100644 --- a/workbaskets/tests/test_util.py +++ b/workbaskets/tests/test_util.py @@ -31,7 +31,7 @@ def test_get_comm_code(): created_commodity = factories.GoodsNomenclatureFactory() row_data = TableRow() find_comm_code(created_commodity.item_id, row_data) - assert row_data.commodity == created_commodity + assert row_data.commodity == created_commodity.item_id @pytest.mark.parametrize( @@ -92,6 +92,6 @@ def test_serialize_uploaded_data(): date(2021, 5, 20), date(2024, 8, 31), ) - assert serialized[0].commodity == commodity1 + assert serialized[0].commodity == commodity1.item_id assert serialized[1].valid_between == TaricDateRange(date(2024, 8, 31), None) - assert serialized[1].commodity == commodity2 + assert serialized[1].commodity == commodity2.item_id diff --git a/workbaskets/util.py b/workbaskets/util.py index 8d4c65b10..adeee825a 100644 --- a/workbaskets/util.py +++ b/workbaskets/util.py @@ -70,14 +70,9 @@ def all_none(self): def find_comm_code(cell, row_data): - from commodities.models.orm import GoodsNomenclature - matches = re.compile(ITEM_ID_REGEX).match(cell) if matches: - commodity = ( - GoodsNomenclature.objects.latest_approved().filter(item_id=cell).first() - ) - row_data.commodity = commodity + row_data.commodity = cell return True return False diff --git a/workbaskets/views/ui.py b/workbaskets/views/ui.py index 22e902dff..66c1d3729 100644 --- a/workbaskets/views/ui.py +++ b/workbaskets/views/ui.py @@ -734,7 +734,7 @@ def matching_measures(self): for row in self.data_upload.rows.all(): matches = self.workbasket_measures.filter( valid_between=row.valid_between, - goods_nomenclature=row.commodity, + goods_nomenclature__item_id=row.commodity, ) duty_matches = [ measure