[INTERNAL] Refactor AlertRuleForm and AlertRuleRegister #214

- Rename AlertRuleRegister to distinguish our future RecordingRule
- Refactor AlertRuleRegister to split up form_valid from form_import

promgen/forms.py

@@ -110,7 +110,7 @@ class Meta:
         }
 
 
-class RuleForm(forms.ModelForm):
+class AlertRuleForm(forms.ModelForm):
     class Meta:
         model = models.Rule
         exclude = ['parent', 'content_type', 'object_id']

promgen/mixins.py

@@ -19,16 +19,29 @@ def set_object(self, content_type, object_id):
         self.object_id = object_id
 
 
-class ContentFormMixin:
+class RuleFormMixin:
+    # When creating a single object, we want to use the
+    # default form class and delagate to form_valid but
+    # when we are importing multiple objects, we delegate
+    # a form_import class to handle processing
     def post(self, request, content_type, object_id):
-        form = self.get_form()
-        # Set an instance of our service here so that we can pass it
-        # along for promtool to render
-        form.instance.set_object(content_type, object_id)
-        if form.is_valid():
-            return self.form_valid(form)
-        else:
-            return self.form_invalid(form)
+        single = self.get_form(self.form_class)
+        # Set an instance of our content_object here so that we can
+        # pass it along for promtool to render
+        single.instance.set_object(content_type, object_id)
+        if single.is_valid():
+            return self.form_valid(single)
+
+        importer = self.get_form(self.form_import_class)
+        if importer.is_valid():
+            ct = ContentType.objects.get_by_natural_key(
+                "promgen", content_type
+            ).model_class()
+            content_object = ct.objects.get(pk=object_id)
+
+            return self.form_import(importer, content_object)
+
+        return self.form_invalid(single)
 
 
 class PromgenPermissionMixin(PermissionRequiredMixin):

promgen/tests/test_routes.py

@@ -120,11 +120,11 @@ def test_failed_permission(self):
         # Test for redirect
         for request in [{"viewname": "rule-new", "args": ("site", 1)}]:
             response = self.client.get(reverse(**request))
-            self.assertRoute(response, views.RuleRegister, 302)
+            self.assertRoute(response, views.AlertRuleRegister, 302)
             self.assertTrue(response.url.startswith("/login"))
 
     def test_other_routes(self):
         self.add_user_permissions("promgen.add_rule", "promgen.change_site")
         for request in [{"viewname": "rule-new", "args": ("site", 1)}]:
             response = self.client.get(reverse(**request))
-            self.assertRoute(response, views.RuleRegister, 200)
+            self.assertRoute(response, views.AlertRuleRegister, 200)

promgen/urls.py

@@ -85,7 +85,7 @@
     path('rule/<int:pk>/test', csrf_exempt(views.RuleTest.as_view()), name='rule-test'),
     path('rule/<int:pk>/duplicate', views.RulesCopy.as_view(), name='rule-overwrite'),
 
-    path('<content_type>/<object_id>/rule', views.RuleRegister.as_view(), name='rule-new'),
+    path('<content_type>/<object_id>/rule', views.AlertRuleRegister.as_view(), name='rule-new'),
 
     path("audit", views.AuditList.as_view(), name="audit-list"),
     path("site", views.SiteDetail.as_view(), name="site-detail"),

promgen/views.py

@@ -757,7 +757,7 @@ def get_permission_required(self):
         "content_object", "overrides", "overrides__content_object"
     )
     template_name = "promgen/rule_update.html"
-    form_class = forms.RuleForm
+    form_class = forms.AlertRuleForm
 
     def get_context_data(self, **kwargs):
         context = super(RuleUpdate, self).get_context_data(**kwargs)
@@ -814,51 +814,41 @@ def post(self, request, *args, **kwargs):
         return self.form_valid(form)
 
 
-class RuleRegister(mixins.PromgenPermissionMixin, FormView):
+class AlertRuleRegister(mixins.PromgenPermissionMixin, mixins.RuleFormMixin, FormView):
     model = models.Rule
-    template_name = 'promgen/rule_register.html'
-    form_class = forms.RuleForm
+    template_name = "promgen/rule_register.html"
+    form_class = forms.AlertRuleForm
+    form_import_class = forms.ImportRuleForm
 
     def get_permission_required(self):
         # In the case of rules, we want to make sure the user has permission
         # to add the rule itself, but also permission to change the linked object
-        yield 'promgen.add_rule'
-        yield 'promgen.change_' + self.kwargs['content_type']
+        yield "promgen.add_rule"
+        yield "promgen.change_" + self.kwargs["content_type"]
 
     def get_context_data(self, **kwargs):
-        context = super(RuleRegister, self).get_context_data(**kwargs)
+        context = super().get_context_data(**kwargs)
         # Set a dummy rule, so that our header/breadcrumbs render correctly
-        context['rule'] = models.Rule()
-        context['rule'].pk = 0
-        context['rule'].set_object(self.kwargs['content_type'], self.kwargs['object_id'])
-        context['macro'] = macro.EXCLUSION_MACRO
+        context["rule"] = models.Rule()
+        context["rule"].pk = 0
+        context["rule"].set_object(
+            self.kwargs["content_type"], self.kwargs["object_id"]
+        )
+        context["macro"] = macro.EXCLUSION_MACRO
         return context
 
-    def post(self, request, content_type, object_id):
-        form = self.get_form()
-        # Set an instance of our service here so that we can pass it
-        # along for promtool to render
-        form.instance.set_object(content_type, object_id)
-        if form.is_valid():
-            form.instance.save()
-            form.instance.add_label(form.instance.content_type.model, form.instance.content_object.name)
-
-            return HttpResponseRedirect(form.instance.get_absolute_url())
-
-        if 'rules' not in request.POST:
-            return self.form_invalid(form)
-
-        importform = forms.ImportRuleForm(request.POST)
-        ct = ContentType.objects.get_by_natural_key('promgen', content_type).model_class()
-        obj = ct.objects.get(pk=object_id)
-
-        if importform.is_valid():
-            data = importform.clean()
-            counters = prometheus.import_rules_v2(data['rules'], obj)
-            messages.info(request, 'Imported %s' % counters)
-            return HttpResponseRedirect(obj.get_absolute_url())
+    def form_valid(self, form):
+        form.instance.save()
+        form.instance.add_label(
+            form.instance.content_type.model, form.instance.content_object.name
+        )
+        return HttpResponseRedirect(form.instance.get_absolute_url())
 
-        return self.form_invalid(form)
+    def form_import(self, form, content_object):
+        data = form.clean()
+        counters = prometheus.import_rules_v2(data["rules"], content_object)
+        messages.info(self.request, "Imported %s" % counters)
+        return HttpResponseRedirect(content_object.get_absolute_url())
 
 
 class ServiceRegister(LoginRequiredMixin, CreateView):