Improve allocation request workflow (#294)

* add update option to unapproved allocation
* use signals for isilon allocation creation
* make automation form verification only necessary for approve action

coldfront/core/allocation/forms.py

@@ -164,11 +164,11 @@ class AllocationApprovalForm(forms.Form):
 
     sheetcheck = forms.BooleanField(
         label='I have ensured that enough space is available on this resource.',
-        required=True
+        required=False,
     )
     auto_create_opts = forms.ChoiceField(
         label='How will this allocation be created?',
-        required=True,
+        required=False,
         widget=forms.RadioSelect,
         choices=ALLOCATION_AUTOCREATE_OPTIONS,
     )
@@ -184,6 +184,29 @@ class AllocationApprovalForm(forms.Form):
         ),
     )
 
+    def clean(self):
+        cleaned_data = super().clean()
+        auto_create_opts = cleaned_data.get('auto_create_opts')
+        automation_specifications = cleaned_data.get('automation_specifications')
+        # if the action is 'approve', make auto_create_opts and sheetcheck mandatory
+        if not auto_create_opts:
+            self.add_error(
+                'auto_create_opts',
+                'You must select an option for how the allocation will be created.'
+            )
+        if auto_create_opts == '2':
+            if not automation_specifications:
+                self.add_error(
+                    'automation_specifications',
+                    'You must select at least one automation option if you choose to automatically create the allocation.'
+                )
+            if not cleaned_data.get('sheetcheck'):
+                self.add_error(
+                    'sheetcheck',
+                    'You must confirm that you have checked the space availability on this resource.'
+                )
+        return cleaned_data
+
 
 class AllocationResourceChoiceField(forms.ModelChoiceField):
     def label_from_instance(self, obj):

coldfront/core/allocation/management/commands/add_allocation_defaults.py

@@ -32,7 +32,7 @@ def handle(self, *args, **options):
             # 'Paid', 'Payment Pending', 'Payment Requested',
             # 'Payment Declined', 'Revoked', 'Renewal Requested', 'Unpaid',
         ):
-            choice_obj = AllocationStatusChoice.objects.get_or_create(name=choice)
+            choice_obj, created = AllocationStatusChoice.objects.get_or_create(name=choice)
             choice_obj.description = description
             choice_obj.save()
 

coldfront/core/allocation/signals.py

@@ -1,5 +1,8 @@
 import django.dispatch
 
+allocation_autocreate = django.dispatch.Signal()
+    #providing_args=["approval_form_data", "allocation_obj"]
+
 allocation_activate = django.dispatch.Signal()
     #providing_args=["allocation_pk"]
 allocation_disable = django.dispatch.Signal()

coldfront/core/allocation/templates/allocation/allocation_detail.html

@@ -310,9 +310,8 @@ <h3><i class="fas fa-list" aria-hidden="true"></i> Allocation Information</h3>
 
       {% if request.user.is_superuser %}
         <div class="float-right">
-          {% if not allocation.status.name in 'New,Renewal Requested' %}
-            <button type="submit" name="action" value="update" class="btn btn-primary"><i class="fas fa-sync" aria-hidden="true"></i> Update</button>
-          {% else %}
+          <button type="submit" name="action" value="update" class="btn btn-primary"><i class="fas fa-sync" aria-hidden="true"></i> Update</button>
+          {% if allocation.status.name in 'New,Renewal Requested,In Progress,On Hold' %}
             <button type="submit" name="action" value="approve" class="btn btn-success mr-1 confirm-activate">Approve</button>
             <button type="submit" name="action" value="deny" class="btn btn-danger mr-1 confirm-deny">Deny</button>
           {% endif %}

coldfront/core/allocation/views.py

@@ -58,6 +58,7 @@
                                               AllocationUserNote,
                                               AllocationUserStatusChoice)
 from coldfront.core.allocation.signals import (allocation_activate,
+                                               allocation_autocreate,
                                                allocation_activate_user,
                                                allocation_disable,
                                                allocation_remove_user,
@@ -77,9 +78,7 @@
 if 'django_q' in settings.INSTALLED_APPS:
     from django_q.tasks import Task
 if 'coldfront.plugins.isilon' in settings.INSTALLED_APPS:
-    from coldfront.plugins.isilon.utils import (
-        update_isilon_allocation_quota, create_isilon_allocation_quota
-    )
+    from coldfront.plugins.isilon.utils import update_isilon_allocation_quota
 
 ALLOCATION_ENABLE_ALLOCATION_RENEWAL = import_from_settings(
     'ALLOCATION_ENABLE_ALLOCATION_RENEWAL', True)
@@ -257,7 +256,7 @@ def get(self, request, *args, **kwargs):
         if not self.request.user.is_superuser:
             form.fields['is_locked'].disabled = True
             form.fields['is_changeable'].disabled = True
-        elif allocation_obj.status.name == 'New':
+        elif allocation_obj.status.name in PENDING_ALLOCATION_STATUSES:
             approval_form = AllocationApprovalForm()
             context['approval_form'] = approval_form
 
@@ -296,18 +295,24 @@ def post(self, request, *args, **kwargs):
 
         form_data = form.cleaned_data
         old_status = allocation_obj.status.name
-        approval_form = AllocationApprovalForm(request.POST)
-
-        if action in ['update', 'approve', 'deny']:
 
-            allocation_obj.end_date = form_data.get('end_date')
-            allocation_obj.start_date = form_data.get('start_date')
-            allocation_obj.description = form_data.get('description')
-            allocation_obj.is_locked = form_data.get('is_locked')
-            allocation_obj.is_changeable = form_data.get('is_changeable')
-            allocation_obj.status = form_data.get('status')
+        allocation_obj.end_date = form_data.get('end_date')
+        allocation_obj.start_date = form_data.get('start_date')
+        allocation_obj.description = form_data.get('description')
+        allocation_obj.is_locked = form_data.get('is_locked')
+        allocation_obj.is_changeable = form_data.get('is_changeable')
+        allocation_obj.status = form_data.get('status')
 
         if 'approve' in action:
+
+            approval_form = AllocationApprovalForm(request.POST)
+            if not approval_form.is_valid():
+                context = self.get_context_data()
+                context['form'] = form
+                context['allocation'] = allocation_obj
+                context['approval_form'] = approval_form
+                return render(request, self.template_name, context)
+
             err = None
             # ensure that Tier gets swapped out for storage volume
             resource = form_data.get('resource')
@@ -322,68 +327,30 @@ def post(self, request, *args, **kwargs):
                 messages.error(request, err)
                 return HttpResponseRedirect(reverse('allocation-detail', kwargs={'pk': pk}))
             if action == 'approve':
-                approval_form = AllocationApprovalForm(request.POST)
+                # ensure that sheetcheck and auto_create_opts are selected
                 autoapproval_choice = approval_form.data.get('auto_create_opts')
 
                 if autoapproval_choice == '2':
-                    resources_plugins = {
-                        'isilon': 'coldfront.plugins.isilon',
-                        # 'lfs': 'coldfront.plugins.lustre',
-                    }
-                    rtype = next((k for k in resources_plugins if k in alloc_change_obj.allocation.get_parent_resource.name), None)
-                    if not rtype:
-                        err = ('non-isilon resource interactions are not automated '
-                            'at this time. Please manually create the resource '
-                            'before approving this request.')
-                        messages.error(request, err)
-                        return HttpResponseRedirect(
-                            reverse('allocation-detail', kwargs={'pk': pk})
+                    error = None
+                    try:
+                        preactivation_responses = allocation_autocreate.send(
+                            sender=self.__class__,
+                            allocation_obj=allocation_obj,
+                            resource=resource,
+                            approval_form_data=approval_form.cleaned_data
                         )
+                        preactivation_replies = [p[1] for p in preactivation_responses if p[1]]
+                        if not preactivation_replies:
+                            error = ('this allocation\'s resource has no autocreate options '
+                                'at this time. Please manually create the resource '
+                                'before approving this request.')
+                    except Exception as e:
+                        error = f"An error was encountered during autocreation: {e} Please contact your administrator."
+                        logger.exception('A350: %s', e)
+                    if error:
+                        messages.error(request, error)
+                        return HttpResponseRedirect(reverse('allocation-detail', kwargs={'pk': pk}))
 
-                    if not approval_form.is_valid():
-                        messages.error(request, 'form is not valid.')
-                        context = self.get_context_data()
-                        context['form'] = form
-                        context['allocation'] = allocation_obj
-                        return render(request, self.template_name, context)
-
-                    approval_form_data = approval_form.cleaned_data
-                    automation_specifications = approval_form_data.get('automation_specifications')
-                    automation_kwargs = {k:True for k in automation_specifications}
-
-                    plugin = resources_plugins[rtype]
-                    if plugin in settings.INSTALLED_APPS:
-                        try:
-                            option_exceptions = create_isilon_allocation_quota(
-                                allocation_obj, resource, **automation_kwargs
-                            )
-                            if option_exceptions:
-                                err = f'some options failed to be created for new allocation {allocation_obj} ({allocation_obj.pk}): {option_exceptions}'
-                                logger.error(err)
-                                messages.error(request, f"{err}. Please contact Coldfront administration for further assistance.")
-                        except Exception as e:
-                            err = ("An error was encountered while auto-creating"
-                                " the allocation. Please contact Coldfront "
-                                f"administration and/or manually create the allocation: {e}")
-                            logger.error(err)
-                            ex_type, ex_value, ex_traceback = sys.exc_info()
-                            stack_trace = list()
-                            trace_back = traceback.extract_tb(ex_traceback)
-                            for trace in trace_back:
-                                stack_trace.append("File : %s , Line : %d, Func.Name : %s, Message : %s" % (trace[0], trace[1], trace[2], trace[3]))
-                            logger.error(stack_trace)
-                            messages.error(request, err)
-                            return HttpResponseRedirect(
-                                reverse('allocation-detail', kwargs={'pk': pk})
-                            )
-                    else:
-                        err = ("There is an issue with the configuration of "
-                            "Coldfront's auto-creation capabilities. Please contact Coldfront "
-                            "administration and/or manually create the allocation.")
-                        messages.error(request, err)
-                        return HttpResponseRedirect(
-                            reverse('allocation-detail', kwargs={'pk': pk})
-                        )
             if 'Tier ' in allocation_obj.get_resources_as_string:
                 # remove current resource from resources
                 allocation_obj.resources.clear()
@@ -394,6 +361,11 @@ def post(self, request, *args, **kwargs):
 
         elif action == 'deny':
             allocation_obj.status = AllocationStatusChoice.objects.get(name='Denied')
+        elif action == 'update':
+            if old_status in PENDING_ALLOCATION_STATUSES and allocation_obj.status.name not in PENDING_ALLOCATION_STATUSES:
+                err = "You can only use the 'update' option on new allocations to change their statuses to New, On Hold, or In Progress."
+                messages.error(request, err)
+                return HttpResponseRedirect(reverse('allocation-detail', kwargs={'pk': pk}))
 
         allocation_users = allocation_obj.allocationuser_set.exclude(
             status__name__in=['Removed', 'Error']
@@ -438,7 +410,7 @@ def post(self, request, *args, **kwargs):
                 )
                 return HttpResponseRedirect(reverse('allocation-request-list'))
 
-        elif old_status != allocation_obj.status.name in ['Denied', 'New', 'Revoked']:
+        elif old_status != allocation_obj.status.name in ['Denied', 'Revoked']+PENDING_ALLOCATION_STATUSES:
             allocation_obj.start_date = None
             allocation_obj.end_date = None
             allocation_obj.save()

coldfront/plugins/isilon/utils.py

@@ -2,9 +2,11 @@
 
 import isilon_sdk.v9_3_0 as isilon_api
 from isilon_sdk.v9_3_0.rest import ApiException
+from django.dispatch import receiver
 
 from coldfront.core.utils.common import import_from_settings
 from coldfront.core.allocation.models import AllocationAttributeType, AllocationAttribute
+from coldfront.core.allocation.signals import allocation_autocreate
 from coldfront.config.plugins.isilon import ISILON_AUTH_MODEL
 
 logger = logging.getLogger(__name__)
@@ -379,3 +381,30 @@ def update_coldfront_quota_and_usage(alloc, usage_attribute_type, value_list):
     usage.value = value_list[1]
     usage.save()
     return usage_attribute
+
+@receiver(allocation_autocreate)
+def activate_allocation(sender, **kwargs):
+
+    approval_form_data = kwargs['approval_form_data']
+    allocation_obj = kwargs['allocation_obj']
+    resource = kwargs['resource']
+
+    automation_specifications = approval_form_data.get('automation_specifications')
+    automation_kwargs = {k:True for k in automation_specifications}
+
+    if 'isilon' in resource.name:
+        try:
+            option_exceptions = create_isilon_allocation_quota(
+                allocation_obj, resource, **automation_kwargs
+            )
+            if option_exceptions:
+                err = f'some options failed to be created for new allocation {allocation_obj} ({allocation_obj.pk}): {option_exceptions}'
+                logger.error(err)
+                raise ValueError(err)
+        except Exception as e:
+            err = ("An error was encountered while auto-creating the "
+                "allocation. Please contact Coldfront administration "
+                f"and/or manually create the allocation: {e}")
+            logger.error(err)
+            raise ValueError(err)
+        return 'isilon'