isotests: massive overview refactor

Copy pasted code sucks. Fix it.

Also, ensure 500 errors aren't going to be popping up when people start
typing invalid URLs for fun, among a lot of other small fixes going in
here.

Signed-off-by: Dan McGee <dan@archlinux.org>

6 files changed, 110 insertions, 492 deletions

diff --git a/isotests/models.py b/isotests/models.py
index 1267fe85..de8dd13c 100644
--- a/isotests/models.py
+++ b/isotests/models.py
@@ -4,59 +4,43 @@ from django.db import models
 from django.db.models import Max
 
 class IsoOption(models.Model):
-    class Meta:
-        abstract = True
-
     name = models.CharField(max_length=200)
 
-    success_tests = None
-    failed_tests = None
-
     def __unicode__(self):
-        return str(self.name)
-
-    def get_success_test(self):
-        if not self.success_tests:
-            self.success_tests = self.test_set.filter(
-                    success=True).annotate(Max('iso__id'))
+        return self.name
 
-        if self.success_tests:
-            return self.success_tests[0].iso
-        return None
+    def get_test_result(self, success):
+        try:
+            return self.test_set.filter(success=success).select_related(
+                    'iso').latest('iso__id').iso
+        except Test.DoesNotExist:
+            return None
 
-    def get_failed_test(self):
-        if not self.failed_tests:
-            self.failed_tests = self.test_set.filter(
-                    success=False).annotate(Max('iso__id'))
+    def get_last_success(self):
+        return self.get_test_result(True)
 
-        if self.failed_tests:
-            return self.failed_tests[0].iso
-        return None
+    def get_last_failure(self):
+        return self.get_test_result(False)
 
-class RollbackOption(IsoOption):
     class Meta:
         abstract = True
 
-    success_rollback_tests = None
-    failed_rollback_tests = None
-
-    def get_rollback_success_test(self):
-        if not self.success_rollback_tests:
-            self.success_rollback_tests = self.rollback_test_set.filter(
-                    success=True).annotate(Max('iso__id'))
+class RollbackOption(IsoOption):
+    def get_rollback_test_result(self, success):
+        try:
+            return self.rollback_test_set.filter(success=success).select_related(
+                    'iso').latest('iso__id').iso
+        except Test.DoesNotExist:
+            return None
 
-        if self.success_rollback_tests:
-            return self.success_rollback_tests[0].iso
-        return None
+    def get_last_rollback_success(self):
+        return self.get_rollback_test_result(True)
 
-    def get_rollback_failed_test(self):
-        if not self.failed_rollback_tests:
-            self.failed_rollback_tests = self.rollback_test_set.filter(
-                    success=False).annotate(Max('iso__id'))
+    def get_last_rollback_failure(self):
+        return self.get_rollback_test_result(False)
 
-        if self.failed_rollback_tests:
-            return self.failed_rollback_tests[0].iso
-        return None
+    class Meta:
+        abstract = True
 
 class Iso(models.Model):
     name = models.CharField(max_length=255)
@@ -112,11 +96,11 @@ class Test(models.Model):
     clock_choice = models.ForeignKey(ClockChoice)
     filesystem = models.ForeignKey(Filesystem)
     modules = models.ManyToManyField(Module, null=True, blank=True)
+    bootloader = models.ForeignKey(Bootloader)
     rollback_filesystem = models.ForeignKey(Filesystem,
             related_name="rollback_test_set", null=True, blank=True)
     rollback_modules = models.ManyToManyField(Module,
             related_name="rollback_test_set", null=True, blank=True)
-    bootloader = models.ForeignKey(Bootloader)
 
     success = models.BooleanField()
     comments = models.TextField(null=True, blank=True)
diff --git a/isotests/urls.py b/isotests/urls.py
index 7f438368..12497cde 100644
--- a/isotests/urls.py
+++ b/isotests/urls.py
@@ -1,12 +1,11 @@
 from django.conf.urls.defaults import patterns
 
 urlpatterns = patterns('isotests.views',
-    (r'^$',                                              'view_results'),
-    (r'^add/$',                                          'add_result'),
-    (r'^thanks/$',                                       'thanks'),
-    (r'^results/$',                                      'view_results'),
-    (r'^results/(?P<option>[a-z0-9_]+)/(?P<value>.+)/$', 'view_results_for'),
-    (r'^results/(?P<isoid>.+)/$',                        'view_results_iso'),
+    (r'^$',                              'test_results_overview', {}, 'releng-test-overview'),
+    (r'^submit/$',                       'submit_test_result', {}, 'releng-test-submit'),
+    (r'^thanks/$',                       'submit_test_thanks', {}, 'releng-test-thanks'),
+    (r'^iso/(?P<iso_id>\d+)/$',          'test_results_iso', {}, 'releng-results-iso'),
+    (r'^(?P<option>.+)/(?P<value>\d+)/$','test_results_for', {}, 'releng-results-for'),
 )
 
 # vim: set ts=4 sw=4 et:
diff --git a/isotests/views.py b/isotests/views.py
index 50703bd6..8e66d7b4 100644
--- a/isotests/views.py
+++ b/isotests/views.py
@@ -1,6 +1,7 @@
 from django import forms
 from django.conf import settings
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, Http404
+from django.shortcuts import get_object_or_404
 from django.views.generic.simple import direct_to_template
 
 from .models import (Architecture, BootType, Bootloader, ClockChoice,
@@ -20,11 +21,11 @@ class TestForm(forms.ModelForm):
     install_type = standard_field(InstallType)
     source = standard_field(Source)
     clock_choice = standard_field(ClockChoice)
-    bootloader = standard_field(Bootloader)
     filesystem = standard_field(Filesystem,
             help_text="Check the installed system, including fstab.")
     modules = forms.ModelMultipleChoiceField(queryset=Module.objects.all(),
             help_text="", widget=forms.CheckboxSelectMultiple(), required=False)
+    bootloader = standard_field(Bootloader)
     rollback_filesystem = forms.ModelChoiceField(queryset=Filesystem.objects.all(),
             help_text="If you did a rollback followed by a new attempt to setup " \
             "your lockdevices/filesystems, select which option you took here.",
@@ -44,13 +45,13 @@ class TestForm(forms.ModelForm):
         fields = ("user_name", "user_email", "iso", "architecture",
                   "iso_type", "boot_type", "hardware_type",
                   "install_type", "source", "clock_choice", "filesystem",
-                  "modules", "rollback_filesystem", "rollback_modules",
-                  "bootloader", "success", "comments")
+                  "modules", "bootloader", "rollback_filesystem",
+                  "rollback_modules", "success", "comments")
         widgets = {
             "modules": forms.CheckboxSelectMultiple(),
         }
 
-def add_result(request):
+def submit_test_result(request):
     if request.POST:
         form = TestForm(request.POST)
         if form.is_valid() and request.POST['website'] == '':
@@ -64,53 +65,68 @@ def add_result(request):
     context = {'form': form}
     return direct_to_template(request, 'isotests/add.html', context)
 
-def view_results(request):
-    architecture_list = Architecture.objects.all()
-    iso_type_list = IsoType.objects.all()
-    boot_type_list = BootType.objects.all()
-    hardware_type_list = HardwareType.objects.all()
-    install_type_list = InstallType.objects.all()
-    source_list = Source.objects.all()
-    clock_choice_list = ClockChoice.objects.all()
-    module_list = Module.objects.all()
-    filesystem_list = Filesystem.objects.all()
-    bootloader_list = Bootloader.objects.all()
+def calculate_option_overview(model, is_rollback=False):
+    option = {
+        'option': model,
+        'name': model._meta.verbose_name,
+        'is_rollback': is_rollback,
+        'values': []
+    }
+    for value in model.objects.all():
+        data = { 'value': value }
+        if is_rollback:
+            data['success'] = value.get_last_rollback_success()
+            data['failure'] = value.get_last_rollback_failure()
+        else:
+            data['success'] = value.get_last_success()
+            data['failure'] = value.get_last_failure()
+        option['values'].append(data)
+
+    return option
+
+def test_results_overview(request):
+    # data structure produced:
+    # [ { option, name, is_rollback, values: [ { value, success, failure } ... ] } ... ]
+    all_options = []
+    models = [ Architecture, IsoType, BootType, HardwareType, InstallType,
+            Source, ClockChoice, Filesystem, Module, Bootloader ]
+    for model in models:
+        all_options.append(calculate_option_overview(model))
+    # now handle rollback options
+    for model in [ Filesystem, Module ]:
+        all_options.append(calculate_option_overview(model, True))
 
+    print all_options
     context = {
-            'architecture_list': architecture_list,
-            'iso_type_list': iso_type_list,
-            'boot_type_list': boot_type_list,
-            'hardware_type_list': hardware_type_list,
-            'install_type_list': install_type_list,
-            'source_list': source_list,
-            'clock_choices_list': clock_choice_list,
-            'filesystem_list': filesystem_list,
-            'module_list': module_list,
-            'bootloader_list': bootloader_list,
+            'options': all_options,
             'iso_url': settings.ISO_LIST_URL,
     }
     return direct_to_template(request, 'isotests/results.html', context)
 
-def view_results_iso(request, isoid):
-    iso = Iso.objects.get(pk=isoid)
-    test_list = Test.objects.filter(iso__pk=isoid)
+def test_results_iso(request, iso_id):
+    iso = get_object_or_404(Iso, pk=iso_id)
+    test_list = iso.test_set.all()
     context = {
         'iso_name': iso.name,
         'test_list': test_list
     }
     return direct_to_template(request, 'isotests/result_list.html', context)
 
-def view_results_for(request, option, value):
-    kwargs = {option: value}
-    test_list = Test.objects.filter(**kwargs).order_by("iso__name", "pk")
+def test_results_for(request, option, value):
+    if option not in Test._meta.get_all_field_names():
+        raise Http404
+    option_model = getattr(Test, option).field.rel.to
+    real_value = get_object_or_404(option_model, pk=value)
+    test_list = real_value.test_set.order_by("iso__name", "pk")
     context = {
         'option': option,
-        'value': value,
+        'value': real_value,
+        'value_id': value,
         'test_list': test_list
     }
     return direct_to_template(request, 'isotests/result_list.html', context)
 
-def thanks(request):
+def submit_test_thanks(request):
     return direct_to_template(request, "isotests/thanks.html", None)
 
 # vim: set ts=4 sw=4 et:
diff --git a/templates/isotests/result_list.html b/templates/isotests/result_list.html
index ac0475b6..29ad19ec 100644
--- a/templates/isotests/result_list.html
+++ b/templates/isotests/result_list.html
@@ -3,9 +3,7 @@
 {% block content %}
 <div class="box">
     <h2>Results for:
-        {% if option %}
-        {{ option }}: {{ value }}
-        {% endif %}
+        {% if option %}{{ option|title }}: {{ value }}{% endif %}
         {{ iso_name|default:"" }}
     </h2>
 
diff --git a/templates/isotests/result_section.html b/templates/isotests/result_section.html
new file mode 100644
index 00000000..52f03339
--- /dev/null
+++ b/templates/isotests/result_section.html
@@ -0,0 +1,26 @@
+<tr>
+    <td><h3>{% if option.is_rollback %}Rollback: {% endif %}{{ option.name|title }}</h3></td>
+</tr>
+{% for item in option.values %}
+<tr>
+    <td>
+        <a href="{% url releng-results-for option.name|lower item.value.pk %}">
+            {{ item.value.name|lower }}
+        </a>
+    </td>
+    <td>
+        {% if item.success %}
+        <a href="{% url releng-results-iso item.success.pk %}">
+            {{ item.success.name }}
+        </a>
+        {% else %}Never succeeded{% endif %}
+    </td>
+    <td>
+        {% if item.failure %}
+        <a href="{% url releng-results-iso item.failure.pk %}">
+            {{ item.failure.name }}
+        </a>
+        {% else %}Never failed{% endif %}
+    </td>
+</tr>
+{% endfor %}
diff --git a/templates/isotests/results.html b/templates/isotests/results.html
index d8835121..033fdbfe 100644
--- a/templates/isotests/results.html
+++ b/templates/isotests/results.html
@@ -9,7 +9,7 @@
     <p>This is a overview screen showing a test results matrix of release
     engineering produced ISOs. Various options and configurations are shown
     with last success and last failure results, if known. To help improve ISO
-    quality, you are encouraged to <a href="/isotests/add/">give feedback</a>
+    quality, you are encouraged to <a href="{% url releng-test-submit %}">give feedback</a>
     if you have tested and used any ISOs. Both successful and failed results
     are encouraged and welcome.</p>
 
@@ -17,414 +17,9 @@
     <a href="{{ iso_url }}">{{ iso_url }}</a>.</p>
 
     <table>
-        <tr>
-            <td>
-                <h3>Architecture</h3>
-            </td>
-        </tr>
-        {% if architecture_list %}
-        {% for architecture in architecture_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/architecture/{{ architecture.pk }}/">
-                    {{ architecture.name }}
-                </a>
-            </td>
-            <td>
-                {% if architecture.get_success_test %}
-                <a href="/isotests/results/{{ architecture.get_success_test.pk }}/">
-                    {{ architecture.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if architecture.get_failed_test %}
-                <a href="/isotests/results/{{ architecture.get_failed_test.pk }}/">
-                    {{ architecture.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
+        {% for option in options %}
+        {% include "isotests/result_section.html" %}
         {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Image Type</h3>
-            </td>
-        </tr>
-        {% if iso_type_list %}
-        {% for iso_type in iso_type_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/iso_type/{{ iso_type.pk }}/">
-                    {{ iso_type.name }}
-                </a>
-            </td>
-            <td>
-                {% if iso_type.get_success_test %}
-                <a href="/isotests/results/{{ iso_type.get_success_test.pk }}/">
-                    {{ iso_type.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if iso_type.get_failed_test %}
-                <a href="/isotests/results/{{ iso_type.get_failed_test.pk }}/">
-                    {{ iso_type.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Boot Type</h3>
-            </td>
-        </tr>
-        {% if boot_type_list %}
-        {% for boot_type in boot_type_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/boot_type/{{ boot_type.pk }}/">
-                    {{ boot_type.name }}
-                </a>
-            </td>
-            <td>
-                {% if boot_type.get_success_test %}
-                <a href="/isotests/results/{{ boot_type.get_success_test.pk }}/">
-                    {{ boot_type.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if boot_type.get_failed_test %}
-                <a href="/isotests/results/{{ boot_type.get_failed_test.pk }}/">
-                    {{ boot_type.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Hardware Type</h3>
-            </td>
-        </tr>
-        {% if hardware_type_list %}
-        {% for hardware_type in hardware_type_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/hardware_type/{{ hardware_type.pk }}/">
-                    {{ hardware_type.name }}
-                </a>
-            </td>
-            <td>
-                {% if hardware_type.get_success_test %}
-                <a href="/isotests/results/{{ hardware_type.get_success_test.pk }}/">
-                    {{ hardware_type.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if hardware_type.get_failed_test %}
-                <a href="/isotests/results/{{ hardware_type.get_failed_test.pk }}/">
-                    {{ hardware_type.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Install Type</h3>
-            </td>
-        </tr>
-        {% if install_type_list %}
-        {% for install_type in install_type_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/install_type/{{ install_type.pk }}/">
-                    {{ install_type.name }}
-                </a>
-            </td>
-            <td>
-                {% if install_type.get_success_test %}
-                <a href="/isotests/results/{{ install_type.get_success_test.pk }}/">
-                    {{ install_type.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if install_type.get_failed_test %}
-                <a href="/isotests/results/{{ install_type.get_failed_test.pk }}/">
-                    {{ install_type.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Source Selection</h3>
-            </td>
-        </tr>
-        {% if source_list %}
-        {% for source in source_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/source/{{ source.pk }}/">
-                    {{ source.name }}
-                </a>
-            </td>
-            <td>
-                {% if source.get_success_test %}
-                <a href="/isotests/results/{{ source.get_success_test.pk }}/">
-                    {{ source.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if source.get_failed_test %}
-                <a href="/isotests/results/{{ source.get_failed_test.pk }}/">
-                    {{ source.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Clock Choice</h3>
-            </td>
-        </tr>
-        {% if clock_choices_list %}
-        {% for clock_choice in clock_choices_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/clock_choice/{{ clock_choice.pk }}/">
-                    {{ clock_choice.name }}
-                </a>
-            </td>
-            <td>
-                {% if clock_choice.get_success_test %}
-                <a href="/isotests/results/{{ clock_choice.get_success_test.pk }}/">
-                    {{ clock_choice.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if clock_choice.get_failed_test %}
-                <a href="/isotests/results/{{ clock_choice.get_failed_test.pk }}/">
-                    {{ clock_choice.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Partitioning &amp; Filesystems</h3>
-            </td>
-        </tr>
-        {% if filesystem_list %}
-        {% for filesystem in filesystem_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/filesystem/{{ filesystem.pk }}/">
-                    {{ filesystem.name }}
-                </a>
-            </td>
-            <td>
-                {% if filesystem.get_success_test %}
-                <a href="/isotests/results/{{ filesystem.get_success_test.pk }}/">
-                    {{ filesystem.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if filesystem.get_failed_test %}
-                <a href="/isotests/results/{{ filesystem.get_failed_test.pk }}/">
-                    {{ filesystem.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Fancy Stuff</h3>
-            </td>
-        </tr>
-        {% if module_list %}
-        {% for module in module_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/modules/{{ module.pk }}/">
-                    {{ module.name }}
-                </a>
-            </td>
-            <td>
-                {% if module.get_success_test %}
-                <a href="/isotests/results/{{ module.get_success_test.pk }}/">
-                    {{ module.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if module.get_failed_test %}
-                <a href="/isotests/results/{{ module.get_failed_test.pk }}/">
-                    {{ module.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Rollback: Partitioning &amp; Filesystems</h3>
-            </td>
-        </tr>
-        {% if filesystem_list %}
-        {% for filesystem in filesystem_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/rollback_filesystem/{{ filesystem.pk }}/">
-                    {{ filesystem.name }}
-                </a>
-            </td>
-            <td>
-                {% if filesystem.get_rollback_success_test %}
-                <a href="/isotests/results/{{ filesystem.get_rollback_success_test.pk }}/">
-                    {{ filesystem.get_rollback_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if filesystem.get_rollback_failed_test %}
-                <a href="/isotests/results/{{ filesystem.get_rollback_failed_test.pk }}/">
-                    {{ filesystem.get_rollback_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Rollback: Fancy Stuff</h4>
-            </td>
-        </tr>
-        {% if module_list %}
-        {% for module in module_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/rollback_modules/{{ module.pk }}/">
-                    {{ module.name }}
-                </a>
-            </td>
-            <td>
-                {% if module.get_rollback_success_test %}
-                <a href="/isotests/results/{{ module.get_rollback_success_test.pk }}/">
-                    {{ module.get_rollback_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if module.get_rollback_failed_test %}
-                <a href="/isotests/results/{{ module.get_rollback_failed_test.pk }}/">
-                    {{ module.get_rollback_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
-        <tr>
-            <td>
-                <h3>Bootloader</h3>
-            </td>
-        </tr>
-        {% if bootloader_list %}
-        {% for bootloader in bootloader_list %}
-        <tr>
-            <td>
-                <a href="/isotests/results/bootloader/{{ bootloader.pk }}/">
-                    {{ bootloader.name }}
-                </a>
-            </td>
-            <td>
-                {% if bootloader.get_success_test %}
-                <a href="/isotests/results/{{ bootloader.get_success_test.pk }}/">
-                    {{ bootloader.get_success_test.name }}
-                </a>
-                {% else %}
-                Never succeeded
-                {% endif %}
-            </td>
-            <td>
-                {% if bootloader.get_failed_test %}
-                <a href="/isotests/results/{{ bootloader.get_failed_test.pk }}/">
-                    {{ bootloader.get_failed_test.name }}
-                </a>
-                {% else %}
-                Never failed
-                {% endif %}
-            </td>
-        </tr>
-        {% endfor %}
-        {% endif %}
     </table>
 </div>
 {% endblock %}