Quota definition create link fix (#1190)

* Fix quota definition create link not being generated correctly

* Fix create url not being generated

* Update url names

* Fix test error

quotas/forms.py

@@ -78,7 +78,10 @@ def __init__(self, *args, **kwargs):
         self.fields["quota_type"].initial = quota_type_initial
         self.helper = FormHelper()
 
-        clear_url = reverse_lazy("quota-definitions", kwargs={"sid": object_sid})
+        clear_url = reverse_lazy(
+            "quota_definition-ui-list",
+            kwargs={"sid": object_sid},
+        )
 
         self.helper.layout = Layout(
             Field.radios("quota_type", label_size=Size.SMALL),

quotas/jinja2/includes/quotas/actions.jinja

@@ -2,7 +2,7 @@
   <div class="app-related-items" role="complementary">
       <h2 class="govuk-heading-s" id="subsection-title">Actions</h2>
       <ul class="govuk-list govuk-!-font-size-16">
-            <li><a class="govuk-link" href="{{ url('quota-definitions', args=[object.sid]) }}">View definition periods</a></li>
+            <li><a class="govuk-link" href="{{ url('quota_definition-ui-list', args=[object.sid]) }}">View definition periods</a></li>
             <li><a class="govuk-link" href="{{ url('quota_definition-ui-create', args=[object.sid]) }}">Create definition period</a></li>
             <br>
             <li><a class="govuk-link" href="{{ measures_url }}">View all measures</a></li>

quotas/jinja2/quota-definitions/confirm-delete.jinja

@@ -7,7 +7,7 @@
   {{ breadcrumbs(request, [
       {"text": "Find and edit quotas", "href": url("quota-ui-list")},
       {"text": object._meta.verbose_name|capitalize ~ ": " ~ object|string, "href": object.get_url()},
-      {"text": "Quota ID: " ~ object.order_number ~ " - Data", "href": url("quota-definitions", args=[object.sid])},
+      {"text": "Quota ID: " ~ object.order_number ~ " - Data", "href": url("quota_definition-ui-list", args=[object.sid])},
       {"text": page_title}
     ])
   }}
@@ -30,7 +30,7 @@
 {% block main_button %}
 {{ govukButton({
   "text": "View other definition periods for this quota order number",
-  "href": url('quota-definitions', kwargs={'sid': object.sid}),
+  "href": url('quota_definition-ui-list', kwargs={'sid': object.sid}),
   "classes": "govuk-button--primary"
 }) }}
 {% endblock%}

quotas/jinja2/quota-definitions/confirm-update.jinja

@@ -7,7 +7,7 @@
   {{ breadcrumbs(request, [
       {"text": "Find and edit quotas", "href": url("quota-ui-list")},
       {"text": object.order_number._meta.verbose_name|capitalize ~ ": " ~ object.order_number|string, "href": object.order_number.get_url()},
-      {"text": "Quota ID: " ~ object.order_number.order_number ~ " - Data", "href": url("quota-definitions", args=[object.order_number.sid])},
+      {"text": "Quota ID: " ~ object.order_number.order_number ~ " - Data", "href": url("quota_definition-ui-list", args=[object.order_number.sid])},
       {"text": page_title}
     ])
   }}
@@ -16,7 +16,7 @@
 {% block main_button %}
 {{ govukButton({
   "text": "View other definition periods for this quota order number",
-  "href": url('quota-definitions', kwargs={'sid': object.order_number.sid}),
+  "href": url('quota_definition-ui-list', kwargs={'sid': object.order_number.sid}),
   "classes": "govuk-button--primary"
 }) }}
 {% endblock%}

quotas/jinja2/quota-definitions/delete.jinja

@@ -4,7 +4,7 @@
   {{ breadcrumbs(request, [
       {"text": "Find and edit quotas", "href": url("quota-ui-list")},
       {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string, "href": object.order_number.get_url()},
-      {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string ~ ": Data" , "href": url('quota-definitions', args=[object.order_number.sid])},
+      {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string ~ ": Data" , "href": url('quota_definition-ui-list', args=[object.order_number.sid])},
       {"text": page_title}
     ])
   }}

quotas/jinja2/quota-definitions/edit.jinja

@@ -4,7 +4,7 @@
   {{ breadcrumbs(request, [
       {"text": "Find and edit quotas", "href": url("quota-ui-list")},
       {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string, "href": object.order_number.get_url()},
-      {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string ~ ": Data" , "href": url('quota-definitions', args=[object.order_number.sid])},
+      {"text": object.order_number._meta.verbose_name|capitalize ~ " " ~ object.order_number|string ~ ": Data" , "href": url('quota_definition-ui-list', args=[object.order_number.sid])},
       {"text": page_title}
     ])
   }}

quotas/jinja2/quotas/definitions.jinja

@@ -33,7 +33,7 @@
   }) }}
 
 <div class="quota-definitions">
-  {% set base_url = url('quota-definitions', args=[quota.sid] ) %}
+  {% set base_url = url('quota_definition-ui-list', args=[quota.sid] ) %}
 
   {% if quota_type == "blocking_periods" %}
     {% include "quotas/tables/blocking_periods.jinja" %}

quotas/models.py

@@ -367,36 +367,33 @@ def get_url(self, action: str = "detail") -> Optional[str]:
             "detail"), eg: "list" or "edit" :rtype Optional[str]: The generated
             URL
         """
-        kwargs = {}
-        if action not in ["list", "create"]:
-            kwargs = self.get_identifying_fields()
-        try:
-            if (
-                action == "edit"
-                and self.transaction.workbasket.status == WorkflowStatus.EDITING
-            ):
-                # Edits in WorkBaskets that are in EDITING state get real
-                # changes via DB updates, not newly created UPDATE instances.
-                if self.update_type == UpdateType.CREATE:
-                    action += "-create"
-                elif self.update_type == UpdateType.UPDATE:
-                    action += "-update"
-
-            if action == "detail":
-                url = reverse(
-                    f"{self.get_url_pattern_name_prefix()}-ui-{action}",
-                    kwargs={"sid": getattr(self, self.url_relation_field).sid},
-                )
-            else:
-                url = reverse(
-                    f"{self.slugify_model_name()}-ui-{action}",
-                    kwargs=kwargs,
-                )
-            if action not in ["detail", "list"]:
-                return url
+        kwargs = self.get_identifying_fields()
+        if (
+            action == "edit"
+            and self.transaction.workbasket.status == WorkflowStatus.EDITING
+        ):
+            # Edits in WorkBaskets that are in EDITING state get real
+            # changes via DB updates, not newly created UPDATE instances.
+            if self.update_type == UpdateType.CREATE:
+                action += "-create"
+            elif self.update_type == UpdateType.UPDATE:
+                action += "-update"
+
+        if action == "detail":
+            url = reverse(
+                f"{self.get_url_pattern_name_prefix()}-ui-{action}",
+                kwargs={"sid": getattr(self, self.url_relation_field).sid},
+            )
             return f"{url}{self.url_suffix}"
-        except NoReverseMatch:
-            return None
+
+        if action in ["create", "list"]:
+            url = reverse(
+                f"{self.slugify_model_name()}-ui-{action}",
+                kwargs={"sid": getattr(self, self.url_relation_field).sid},
+            )
+        else:
+            url = reverse(f"{self.slugify_model_name()}-ui-{action}", kwargs=kwargs)
+        return url
 
     @classmethod
     def slugify_model_name(cls):

quotas/tests/test_models.py

@@ -127,3 +127,25 @@ def test_quota_order_number_autocomplete_label(date_ranges):
     autocomplete = AutoCompleteSerializer()
     autocomplete_label = autocomplete.to_representation(order_number).get("label")
     assert order_number.autocomplete_label == autocomplete_label
+
+
+@pytest.mark.parametrize(
+    "url_name,exp_path",
+    [
+        ("create", "/quotas/123456789/quota_definitions/create/"),
+        ("list", "/quotas/123456789/quota_definitions/"),
+        ("confirm-create", "/quota_definitions/987654321/confirm-create/"),
+        ("confirm-delete", "/quota_definitions/987654321/confirm-delete/"),
+        ("edit", "/quota_definitions/987654321/edit/"),
+        ("edit-update", "/quota_definitions/987654321/edit-update/"),
+        ("confirm-update", "/quota_definitions/987654321/confirm-update/"),
+        ("delete", "/quota_definitions/987654321/delete/"),
+    ],
+)
+def test_quota_definition_urls(url_name, exp_path):
+    quota = factories.QuotaOrderNumberFactory.create(sid=123456789)
+    definition = factories.QuotaDefinitionFactory.create(
+        order_number=quota,
+        sid=987654321,
+    )
+    assert definition.get_url(url_name) == exp_path

quotas/tests/test_views.py

@@ -387,7 +387,7 @@ def test_quota_event_api_list_view(valid_user_client):
 def test_quota_definitions_list_200(valid_user_client, quota_order_number):
     factories.QuotaDefinitionFactory.create_batch(5, order_number=quota_order_number)
 
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(url)
 
@@ -398,7 +398,7 @@ def test_quota_definitions_list_no_quota_data(valid_user_client, quota_order_num
     factories.QuotaDefinitionFactory.create_batch(5, order_number=quota_order_number)
 
     url = (
-        reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+        reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
         + "?quota_type=sub_quotas"
     )
 
@@ -417,7 +417,7 @@ def test_quota_definitions_list_sids(valid_user_client, quota_order_number):
         order_number=quota_order_number,
     )
 
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(url)
 
@@ -435,7 +435,7 @@ def test_quota_definitions_list_sids(valid_user_client, quota_order_number):
 def test_quota_definitions_list_title(valid_user_client, quota_order_number):
     factories.QuotaDefinitionFactory.create_batch(5, order_number=quota_order_number)
 
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(url)
 
@@ -470,7 +470,7 @@ def test_quota_definitions_list_current_versions(
     with override_current_transaction(approved_transaction):
         assert quota_order_number.definitions.current().count() == 1
 
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(url)
 
@@ -530,7 +530,7 @@ def test_quota_definitions_list_edit_delete(
         valid_between=date_ranges.later,
     )
 
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(url)
 
@@ -568,7 +568,7 @@ def test_quota_definitions_list_sort_by_start_date(
         order_number=quota_order_number,
         valid_between=date_ranges.later,
     )
-    url = reverse("quota-definitions", kwargs={"sid": quota_order_number.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": quota_order_number.sid})
 
     response = valid_user_client.get(f"{url}?sort_by=valid_between&ordered=asc")
     assert response.status_code == 200
@@ -707,7 +707,7 @@ def test_current_quota_order_number_returned(
         order_number=current_version,
         valid_between=date_ranges.normal,
     )
-    url = reverse("quota-definitions", kwargs={"sid": current_version.sid})
+    url = reverse("quota_definition-ui-list", kwargs={"sid": current_version.sid})
     response = valid_user_client.get(url)
 
     assert response.status_code == 200

quotas/urls.py

@@ -55,9 +55,9 @@
         name="quota-ui-confirm-create",
     ),
     path(
-        f"quotas/<sid>/quota-definitions/",
+        f"quotas/<sid>/quota_definitions/",
         views.QuotaDefinitionList.as_view(),
-        name="quota-definitions",
+        name="quota_definition-ui-list",
     ),
     path(
         f"quotas/<sid>/quota_definitions/create/",
@@ -70,7 +70,7 @@
         name="quota_definition-ui-confirm-create",
     ),
     path(
-        f"quotas/<sid>/quota_definitions/confirm-delete/",
+        f"quota_definitions/<sid>/confirm-delete/",
         views.QuotaDefinitionConfirmDelete.as_view(),
         name="quota_definition-ui-confirm-delete",
     ),

quotas/views.py

@@ -263,7 +263,10 @@ def get_context_data(self, *args, **kwargs):
             blocking_periods=self.blocking_periods,
             suspension_periods=self.suspension_periods,
             sub_quotas=self.sub_quotas,
-            form_url=reverse("quota-definitions", kwargs={"sid": self.quota.sid}),
+            form_url=reverse(
+                "quota_definition-ui-list",
+                kwargs={"sid": self.quota.sid},
+            ),
             *args,
             **kwargs,
         )