Skip to content

Commit

Permalink
TP2000-885 Use quota origins and exclusions to populate geo data in c…
Browse files Browse the repository at this point in the history
…reate measure journey (#1032)

* Refactor MeasureGeographicalAreaForm

* Set up cleaned_data to use quota origins and exclusions

* Conditionally show geo areas form

* Update form tests

* Update view tests

* Add help text

* Sort cleaned_data in test

* Remove duplicate fixture

* Simplify dictionary get

* Use active origins only

* Fix if statement
  • Loading branch information
dalecannon authored Sep 19, 2023
1 parent e649a80 commit b19f556
Show file tree
Hide file tree
Showing 7 changed files with 237 additions and 65 deletions.
74 changes: 56 additions & 18 deletions measures/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@ class Meta:
help_text=(
"Search for a quota using its order number. "
"You can then select the correct quota from the dropdown list. "
"Selecting a quota will automatically populate the appropriate geographical areas on the next page."
),
queryset=QuotaOrderNumber.objects.all(),
required=False,
Expand Down Expand Up @@ -1034,6 +1035,10 @@ class MeasureGeographicalAreaForm(
error_messages={"required": "A Geographical area must be selected"},
)

@property
def erga_omnes_instance(self):
return GeographicalArea.objects.current().erga_omnes().get()

@property
def geo_area_field_name(self):
return f"{self.prefix}-geo_area"
Expand Down Expand Up @@ -1062,9 +1067,7 @@ def get_countries_initial(self):

return initial

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

def get_initial_data(self):
geographical_area_fields = {
constants.GeoAreaType.ERGA_OMNES: self.erga_omnes_instance,
constants.GeoAreaType.GROUP: self.data.get(
Expand All @@ -1088,9 +1091,20 @@ def __init__(self, *args, **kwargs):
countries_initial_data = self.get_countries_initial()
nested_forms_initial.update(geo_area_initial_data)
nested_forms_initial.update(countries_initial_data)
kwargs.pop("initial")
self.bind_nested_forms(*args, initial=nested_forms_initial, **kwargs)

return nested_forms_initial

def init_fields(self):
if (
self.order_number
and self.order_number.quotaordernumberorigin_set.current()
.as_at_today_and_beyond()
.exists()
):
self.fields["geo_area"].required = False
self.fields["geo_area"].disabled = True

def init_layout(self):
self.helper = FormHelper(self)
self.helper.label_size = Size.SMALL
self.helper.legend_size = Size.SMALL
Expand All @@ -1104,13 +1118,31 @@ def __init__(self, *args, **kwargs):
),
)

@property
def erga_omnes_instance(self):
return GeographicalArea.objects.current().erga_omnes().get()
def __init__(self, *args, **kwargs):
self.order_number = kwargs.pop("order_number", None)
super().__init__(*args, **kwargs)
self.init_fields()
nested_forms_initial = self.get_initial_data() if not self.order_number else {}
kwargs.pop("initial", None)
self.bind_nested_forms(*args, initial=nested_forms_initial, **kwargs)
self.init_layout()

def clean(self):
cleaned_data = super().clean()

# Use quota order number origins and exclusions to set cleaned_data
if self.order_number:
origins = self.order_number.quotaordernumberorigin_set.current()
cleaned_data["geo_areas_and_exclusions"] = [
{
"geo_area": origin.geographical_area,
"exclusions": list(origin.excluded_areas.current()),
}
for origin in origins
]
return cleaned_data

# Otherwise take geographical data from form
geo_area_choice = self.cleaned_data.get("geo_area")

geographical_area_fields = {
Expand All @@ -1121,29 +1153,35 @@ def clean(self):
if geo_area_choice:
if not self.formset_submitted:
if geo_area_choice == constants.GeoAreaType.ERGA_OMNES:
cleaned_data["geo_area_list"] = [self.erga_omnes_instance]
cleaned_data["geo_areas_and_exclusions"] = [
{"geo_area": self.erga_omnes_instance},
]

elif geo_area_choice == constants.GeoAreaType.GROUP:
data_key = constants.SUBFORM_PREFIX_MAPPING[geo_area_choice]
cleaned_data["geo_area_list"] = [cleaned_data[data_key]]
cleaned_data["geo_areas_and_exclusions"] = [
{"geo_area": cleaned_data[data_key]},
]

elif geo_area_choice == constants.GeoAreaType.COUNTRY:
field_name = geographical_area_fields[geo_area_choice]
data_key = constants.SUBFORM_PREFIX_MAPPING[geo_area_choice]
cleaned_data["geo_area_list"] = [
geo_area[field_name] for geo_area in cleaned_data[data_key]
cleaned_data["geo_areas_and_exclusions"] = [
{"geo_area": geo_area[field_name]}
for geo_area in cleaned_data[data_key]
]

exclusions = cleaned_data.get(
geo_area_exclusions = cleaned_data.get(
constants.EXCLUSIONS_FORMSET_PREFIX_MAPPING[geo_area_choice],
)
if exclusions:
cleaned_data["geo_area_exclusions"] = [
if geo_area_exclusions:
exclusions = [
exclusion[constants.FIELD_NAME_MAPPING[geo_area_choice]]
for exclusion in cleaned_data[
constants.EXCLUSIONS_FORMSET_PREFIX_MAPPING[geo_area_choice]
]
for exclusion in geo_area_exclusions
]
cleaned_data["geo_areas_and_exclusions"][0][
"exclusions"
] = exclusions

return cleaned_data

Expand Down
57 changes: 57 additions & 0 deletions measures/jinja2/measures/create-geo-areas-formset.jinja
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{% extends "measures/create-wizard-step.jinja" %}
{% from "components/details/macro.njk" import govukDetails %}
{% from "components/summary-list/macro.njk" import govukSummaryList %}

{% block breadcrumb %}
{% if request.path != "/" %}
{{ breadcrumbs(request, [{"text": "Review the geographical areas"}]) }}
{% endif %}
{% endblock %}

{% block page_title_heading %}Review the geographical areas{% endblock %}

{% block form %}
{% if order_number and origins_and_exclusions %}

{{ govukDetails({
"summaryText": "What can I do if the geographical areas are incorrect?",
"text": "The geographical areas are determined by the quota you entered on the previous page. If the geographical areas are incorrect, you will need to edit the quota. You will then need to re-enter the quota to reflect the changes."
}) }}

{% set row_data = [] %}
{% for data in origins_and_exclusions %}
{% set origin %}
{{ data["origin"].area_id }} - {{ data["origin"].get_description().description }}
{% endset %}

{{ row_data.append({
"key": {"text": "Geographical area"},
"value": {"text": origin},
"actions": {"items": []}
}) or "" }}

{% if data.get("exclusions") %}
{% set exclusions %}
{% for exclusion in data["exclusions"] %}
{{ exclusion.area_id }} - {{ exclusion.get_description().description }}{% if not loop.last %}, {% endif %}
{% endfor %}
{% endset %}

{{ row_data.append({
"key": {"text": "Geographical area exclusions"},
"value": {"text": exclusions if data["exclusions"] else "-"},
"actions": {"items": []}
}) or "" }}
{% endif %}
{% endfor %}

{{ govukSummaryList({
"rows": row_data,
})}}

{{ govukButton({"text": "Continue"}) }}

{% else %}
{{ crispy(form) }}
{% endif %}
{% endblock %}
21 changes: 10 additions & 11 deletions measures/jinja2/measures/create-review.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,18 @@
{% endcall %}

{% call(rows, data) review_step("geographical_area") %}
{% for item in data.geo_area_list %}
{{ rows.extend([
("Geographical area", item.get_description().description|safe),
]) }}
{% endfor %}
{% if data.geo_area_exclusions %}
{% for item in data.geo_area_exclusions %}
{% for geo_data in data.geo_areas_and_exclusions %}
{{ rows.extend([
("Excluded geographical area", item.get_description().description|safe),
("Geographical area", geo_data["geo_area"].get_description().description|safe),
]) }}
{% endfor %}
{% endif %}

{% if geo_data.get("exclusions") %}
{% for exclusion in geo_data["exclusions"] %}
{{ rows.extend([
("Excluded geographical area", exclusion.get_description().description|safe),
]) }}
{% endfor %}
{% endif %}
{% endfor %}
{% endcall %}

{% call(rows, data) review_step("commodities") %}
Expand Down
6 changes: 5 additions & 1 deletion measures/jinja2/measures/create-wizard-step.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds govuk-!-padding-right-9">
<span class="govuk-caption-l">{{ page_subtitle|default("") }}</span>
<h1 class="govuk-heading-xl">{{ page_title }}</h1>
<h1 class="govuk-heading-xl">
{% block page_title_heading %}
{{ page_title }}
{% endblock %}
</h1>

{% if step_metadata[wizard.steps.current].info %}
<p class="govuk-body">{{ step_metadata[wizard.steps.current].info }}</p>
Expand Down
62 changes: 55 additions & 7 deletions measures/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ def test_measure_forms_geo_area_valid_data_erga_omnes(erga_omnes):
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()
assert form.cleaned_data["geo_area_list"] == [erga_omnes]
assert (
form.cleaned_data["geo_areas_and_exclusions"][0]["geo_area"] == erga_omnes
)


def test_measure_forms_geo_area_valid_data_erga_omnes_exclusions(erga_omnes):
Expand Down Expand Up @@ -202,7 +204,10 @@ def test_measure_forms_geo_area_valid_data_erga_omnes_exclusions(erga_omnes):
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()
assert form.cleaned_data["geo_area_exclusions"] == [geo_area1, geo_area2]
assert form.cleaned_data["geo_areas_and_exclusions"][0]["exclusions"] == [
geo_area1,
geo_area2,
]


def test_measure_forms_geo_area_valid_data_erga_omnes_exclusions_delete(erga_omnes):
Expand Down Expand Up @@ -250,7 +255,9 @@ def test_measure_forms_geo_area_valid_data_geo_group_exclusions(erga_omnes):
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()
assert form.cleaned_data["geo_area_exclusions"] == [geo_area1]
assert form.cleaned_data["geo_areas_and_exclusions"][0]["exclusions"] == [
geo_area1,
]


def test_measure_forms_geo_area_valid_data_geo_group_exclusions_delete(erga_omnes):
Expand Down Expand Up @@ -312,8 +319,7 @@ def test_measure_forms_geo_area_valid_data_geo_group(erga_omnes):
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()
# https://uktrade.atlassian.net/browse/TP2000-437 500 error where object instead of a list of objects
assert form.cleaned_data["geo_area_list"] == [geo_group]
assert form.cleaned_data["geo_areas_and_exclusions"][0]["geo_area"] == geo_group


def test_measure_forms_geo_area_valid_data_countries_submit(erga_omnes):
Expand All @@ -336,9 +342,13 @@ def test_measure_forms_geo_area_valid_data_countries_submit(erga_omnes):
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()
assert form.cleaned_data["geo_area_list"] == [geo_area1, geo_area2]
cleaned_data = [
data["geo_area"] for data in form.cleaned_data["geo_areas_and_exclusions"]
]
assert cleaned_data == [geo_area1, geo_area2]
# specific country selection should have empty exclusions
assert not form.cleaned_data.get("geo_area_exclusions")
for data in form.cleaned_data["geo_areas_and_exclusions"]:
assert not data.get("exclusions")


def test_measure_forms_geo_area_valid_data_countries_delete(erga_omnes):
Expand Down Expand Up @@ -461,6 +471,44 @@ def test_measure_forms_geo_area_invalid_data_error_messages(data, error, erga_om
assert error in form.errors["geo_area"]


def test_measure_geographical_area_form_quota_order_number():
"""Tests that `MeasureGeographicalAreaForm` sets `cleaned_data` with
geographical data from quota order number if one is passed in `kwargs`."""
origin_1 = factories.QuotaOrderNumberOriginFactory.create()
origin_2 = factories.QuotaOrderNumberOriginFactory.create(
order_number=origin_1.order_number,
)
origin_1_exclusions = factories.QuotaOrderNumberOriginExclusionFactory.create_batch(
2,
origin=origin_1,
)
order_number = origin_1.order_number

with override_current_transaction(Transaction.objects.last()):
form = forms.MeasureGeographicalAreaForm(
data={},
initial={},
order_number=order_number,
prefix=GEO_AREA_FORM_PREFIX,
)
assert form.is_valid()

expected_cleaned_data = [
{
"geo_area": origin_1.geographical_area,
"exclusions": list(origin_1.excluded_areas.all()),
},
{
"geo_area": origin_2.geographical_area,
"exclusions": [],
},
]
assert sorted(
form.cleaned_data["geo_areas_and_exclusions"],
key=lambda d: sorted(d.items()),
) == sorted(expected_cleaned_data, key=lambda d: sorted(d.items()))


def test_measure_forms_details_invalid_data():
data = {
"measure_type": "foo",
Expand Down
Loading

0 comments on commit b19f556

Please sign in to comment.