Update AllocationCreateView

[cecilialau6776]

This is one in a series of PRs intended to clean up Allocation views in an attempt to better utilize Django's ModelForms and generic views.

[simonLeary42]

I am assuming that these are equivalent:

diff --git a/tmp/before b/tmp/after
index 5cf0795e..1b38c590 100644
--- a/tmp/before
+++ b/tmp/after
@@ -1,27 +1,24 @@
-        # A resource is selected that requires an account name selection but user has no account names
+        # Ensure user has account name if ALLOCATION_ACCOUNT_ENABLED
         if (
             ALLOCATION_ACCOUNT_ENABLED
             and resource_obj.name in ALLOCATION_ACCOUNT_MAPPING
             and AllocationAttributeType.objects.filter(name=ALLOCATION_ACCOUNT_MAPPING[resource_obj.name]).exists()
             and not allocation_account
         ):
-            form.add_error(
-                None,
-                format_html(
-                    'You need to create an account name. Create it by clicking the link under the "Allocation account" field.'
-                ),
+            raise ValidationError(
+                'You need to create an account name. Create it by clicking the link under the "Allocation account" field.',
+                code="user_has_no_account_name",
             )
-            return self.form_invalid(form)

-        allocation_limit_objs = resource_obj.resourceattribute_set.filter(
-            resource_attribute_type__name="allocation_limit"
-        ).first()
-        if allocation_limit_objs:
-            allocation_limit = int(allocation_limit_objs.value)
+        # Ensure this allocaiton wouldn't exceed the limit
+        allocation_limit = resource_obj.get_attribute("allocation_limit", typed=True)
+        if allocation_limit:
             allocation_count = project_obj.allocation_set.filter(
                 resources=resource_obj,
                 status__name__in=["Active", "New", "Renewal Requested", "Paid", "Payment Pending", "Payment Requested"],
             ).count()
             if allocation_count >= allocation_limit:
-                form.add_error(None, format_html("Your project is at the allocation limit allowed for this resource."))
-                return self.form_invalid(form)
+                raise ValidationError(
+                    "Your project is at the allocation limit allowed for this resource.",
+                    code="reached_allocation_limit",
+                )

[cecilialau6776]

I believe those two are equivalent (or approximately equivalent), yes.

[simonLeary42]

LGTM, although my brain isn't quite big enough to keep track of all the noise and moved code, so I don't feel 100% confident.

[simonLeary42]

Is there a high-level motivation for moving logic out of AllocationCreateView.form_valid and into AllocationForm.clean?

[simonLeary42]

I've been reading https://docs.djangoproject.com/en/4.2/ref/forms/validation/ to try and wrap my head around all the different functions but it doesn't talk about form_valid

[cecilialau6776]

Is there a high-level motivation for moving logic out of AllocationCreateView.form_valid and into AllocationForm.clean?

Yes - AllocationForm.clean should have the logic that ensures form data/the form as a whole is valid, and modifies information to normalize it. In this case, it ensures that the user has an allocation account and that the new allocation wouldn't exceed the limit.

The allocation status is also set/modified here because the status is an attribute in the Allocation model, and it made sense to me to make changes that would allow for saving the ModelForm to set/create the attribute.
The users and resource form fields aren't changed/set in AllocationForm.clean since neither are attributes of Allocation (though it might be possible for resource to be, though having to add linked resources might be a pain in this case).

I've been reading https://docs.djangoproject.com/en/4.2/ref/forms/validation/ to try and wrap my head around all the different functions but it doesn't talk about form_valid

form_valid comes from FormMixin, which generic editing views (such as CreateView in this case) inherit from. It's called when the server receives a POST request to the view and the form is marked as valid.

To me, form_valid() seems like a reasonable place to put post/pre-save actions. In this case, adding users to the newly-created allocation and setting the related resource(s).

[simonLeary42]

Does AllocationForm.clean run only when a new allocation is created?

[cecilialau6776]

It's called when the server receives a POST request to the view, so yes.

[cecilialau6776]

Actually - I think it makes sense to override form.save() and move the logic currently in AllocationCreateView.form_valid() to form.save(). and form_valid() would just call send_allocation_admin_email() and allocation_new.send(). Thoughts?

[simonLeary42]

That sounds good but I don't really feel qualified to have an opinion on this.