TP2000-1537 Bulk create QuotaDefinitionPeriods #1320

CPrich905 · 2024-11-11T12:41:22Z

TP2000-1537 Bulk create QuotaDefinitionPeriods

Why

Currently QuotaDefinitionPeriods can only be created one by one
A Quota may cover a significant period of time and creating definition periods to cover the duration of the quota may require a significant amount of repetitions through this journey.

What

Following the figma design linked on the ticket and [here](Single and multiple journeys), this PR

utilises a Django form wizard to bulk create QuotaDefinitionPeriods, and
implements some design changes to the journey for creating a single QuotaDefinitionPeriod

There are four stages to the journey:
Beginning the journey from the Edit Workbasket View

Start, outlining the information the users will need to have available and collecting the QuotaOrderNumber the QuotaDefinitionPeriods are to be built with.

Enter the definition information. This collects the information required to build a QuotaDefinitionPeriod object, as well as the number of definition periods to be created, and the frequency with which they will be duplicated (Once, Annually, Twice-yearly, Quarterly, Custom)

Review page. Here, the user can review the details that for an individual QuotaDefinitionPeriod via a collapsible info box, or edit any that require changing via the Edit link. On submit and the QuotaDefinitionPeriods are created and the user is taken to a standard success page.

On success, the user is redirected to a standard success page with Next steps and Further actions

Design changes:

removing the accordion sections
updates to default and help text for various widgets
changes to the submit buttons/back links - these are now passed as args from the view, enabling us to reuse the QuotaDefinitionCreateForm for updating the bulk created definitions
reorganisation of the Actions panel on the Quotas page
adding help/info text to the journey for creating a single QuotaDefinitionPeriod
adding a custom widget that allows the use of a suffix on a Decimal input field

…fixing form links & next steps

mattjamc

Looks good! Just some small comments

mattjamc · 2024-12-30T13:05:36Z

quotas/jinja2/quota-definitions/bulk-create-start.jinja

+    </h1>
+    <p class="govuk-body">
+      You can create up to 20 quota definition periods for the same quota order number.
+      Subsequent periodswill be calculated from the first date range entered and the data will be duplicated across all definition periods.


Add a space

mattjamc · 2024-12-30T13:29:49Z

quotas/forms/wizards.py

        super().__init__(*args, **kwargs)
-        self.init_layout()
+        self.init_layout(self.request)


Does self.request get used in init_layout()?

mattjamc · 2024-12-30T13:31:05Z

quotas/forms/wizards.py

-
-        # Set these as the default values
-        self.fields["quota_critical"].initial = False
+        self.fields["end_date"].help_text = ""


Help text saying end date is optional is removed here but it is still optional as far as the code realises - a user not inputting an end date will need to be handled as the ValidityPeriodForm currently allows that

mattjamc · 2024-12-30T13:39:19Z

quotas/forms/wizards.py

+        cleaned_data = super().clean()
+        for field in self.required_fields:
+            if field not in cleaned_data:
+                raise ValidationError(f"A value for {field} must be provided")


Are these not handled by the fields having required=True? This just adds a second error message for the same thing e.g. both [Enter the number of definition periods to create] and [A value for instance_count must be provided]

quotas/jinja2/quota-definitions/bulk-create-review.jinja

mattjamc · 2024-12-30T14:48:54Z

quotas/serializers.py

+
+
+def serialize_definition_data(definition):
+    # Serializes data and returns a JSON dict that can be saved to session


Would these comments be better as docstrings if describing what the function does

mattjamc · 2024-12-30T15:02:55Z

quotas/jinja2/includes/quotas/actions.jinja

-            <li><a class="govuk-link" href="{{ url('quota_definition-ui-create', args=[object.sid]) }}">Create definition period</a></li>
+          <strong class="govuk-heading-xs">Create quota data</strong>
+            <li><a class="govuk-link" href="{{ url('quota_definition-ui-create', args=[object.sid]) }}?order_number={{object.order_number}}">Create definition period</a></li>
+            <li><a class="govuk-link" href="{{ url('quota_definition-ui-bulk-create', args=[object.sid]) }}">The bulk creator</a></li>


I like the name but I have a feeling it wouldn't pass the GDS naming conventions - is it clear what is being bulk created here? (Seems obvious to us)

mattjamc · 2024-12-30T15:06:35Z

quotas/forms/definitions.py

@@ -156,9 +156,14 @@ class Meta:
            "maximum_precision",
        ]

+    order_number = forms.ModelChoiceField(


What is the reason for adding this field in? What was happening before to get the order number?

I seem to be getting an error in this form that order number doesn't have a value when I submit but can't figure out why as you are assigning it an initial value

Fixed the error, but do double check it's working for you as well!
As discussed on Teams, there was an issue testing this due to inherited forms using transaction.atomic. This was fixed by passing in the quota_order_number in the query string and then extracting it here. The error you encountered was that, while the initial value was being set, the order_number widget was still showing as missing a value (this error doesn't occur on the bulk creator).
The fix for this was to check if an empty value for the order_number widget is in the form.errors when form.clean() is called, assign the value of cleaned_data['order_number'] as the quota_order_number and then remove the error.

mattjamc · 2024-12-30T15:22:47Z

quotas/jinja2/quota-definitions/create.jinja

+        </span>
+    </h1>
+  <p class="govuk-body">You can create one new quota definition period using this form. Alternatively, you can <a class="govuk-link" href="{{ url('quota_definition-ui-bulk-create') }}">create multiple quota definition periods</a> for a chose quota order number.</p>
+  {% call django_form() %}


mattjamc · 2024-12-30T15:24:50Z

quotas/jinja2/quota-definitions/create.jinja

 {% block breadcrumb %}
  {{ breadcrumbs(request, [
      {"text": "Find and edit quotas", "href": url("quota-ui-list")},
-      {"text": "Quota order number", "href": url("quota-ui-detail", args=[view.kwargs.sid]) ~ "#definitions"},
+      {"text": "Quota order number", "href": url("quota-ui-detail", args=[quota_sid]) ~ "#definitions"},


Appreciate it was already like that but I'm not sure #definitions is a real anchor tag for that page. Maybe #definition-details or none at all

CPrich905 and others added 29 commits November 11, 2024 11:59

formatting

62f11d8

Merge branch 'master' into TP2000-1537-bulk-create-definition-periods

811af31

notes

8139a2d

tests

f269c6c

moar tests

a2468e8

tests and tidying

667f909

wizard view test

abcbd18

working through changes from redesign

e0304cd

adding custom widget for decimal suffix

84b4dcc

working through copy updates to bulk and single create process

86cd34f

copy updates

f47ffea

resolving conflicts

413fb7e

post pair

d03864f

Fix test

b828488

fixing review styling

06c61c2

success page copy changes; resolving serializing unit qualifier bug; …

19fb387

…fixing form links & next steps

validation and requiring refactor

d3c07f8

reformat links and link_text

61105de

tests and refactors

9a2cdea

testing form.is_valid

79818c4

views.py tests

03a386f

Merge branch 'master' into TP2000-1537-bulk-create-definition-periods

0d8e749

wip - testing views get_form_kwargs

988f3ee

Merge branch 'master' into TP2000-1537-bulk-create-definition-periods

c81dacb

tests

c8b8674

tidying up

3c08652

tidying up and tests

9e1659f

Merge branch 'master' into TP2000-1537-bulk-create-definition-periods

8e1c336

Fix transaction error

1b0147a

CPrich905 marked this pull request as ready for review December 20, 2024 15:50

CPrich905 requested a review from a team as a code owner December 20, 2024 15:50

CPrich905 added 2 commits December 30, 2024 14:04

missing jinja files

eb1c7c7

removing extra template file

c1b65b0

mattjamc reviewed Dec 30, 2024

View reviewed changes

CPrich905 added 5 commits December 31, 2024 12:16

first round of CR changes

5e01055

first round of CR changes

87db359

fix for order_number hidden widget not populating

7913ba6

Merge branch 'master' into TP2000-1537-bulk-create-definition-periods

087ff53

fixing broken tests

0dec77a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TP2000-1537 Bulk create QuotaDefinitionPeriods #1320

TP2000-1537 Bulk create QuotaDefinitionPeriods #1320

CPrich905 commented Nov 11, 2024 •

edited

Loading

mattjamc left a comment

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

CPrich905 Dec 31, 2024

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024 •

edited

Loading

CPrich905 Jan 2, 2025

mattjamc Dec 30, 2024

mattjamc Dec 30, 2024

CPrich905 Dec 31, 2024



		def serialize_definition_data(definition):
		# Serializes data and returns a JSON dict that can be saved to session

TP2000-1537 Bulk create QuotaDefinitionPeriods #1320

Are you sure you want to change the base?

TP2000-1537 Bulk create QuotaDefinitionPeriods #1320

Conversation

CPrich905 commented Nov 11, 2024 • edited Loading

TP2000-1537 Bulk create QuotaDefinitionPeriods

Why

What

mattjamc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattjamc Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CPrich905 commented Nov 11, 2024 •

edited

Loading

mattjamc Dec 30, 2024 •

edited

Loading