[damned-lies] Validate new category in branch edit form

[Claude Paroz <claudep src gnome org>]

commit 54da2e453089dcd468ab45378a577c6dbad92ba9
Author: Claude Paroz <claude 2xlibre net>
Date:   Mon Feb 23 23:00:40 2015 +0100

    Validate new category in branch edit form

 stats/forms.py       |   17 ++++++++++++++++-
 stats/tests/tests.py |   23 +++++++++++++++++++++--
 stats/views.py       |   38 +++++++++++++++++++-------------------
 urls.py              |    3 ++-
 4 files changed, 58 insertions(+), 23 deletions(-)
---
diff --git a/stats/forms.py b/stats/forms.py
index 713ab19..df5c720 100644
--- a/stats/forms.py
+++ b/stats/forms.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 from django import forms
-from stats.models import CATEGORY_CHOICES, Release
+from stats.models import CATEGORY_CHOICES, Branch, Category, Release
 
 class ReleaseField(forms.ModelChoiceField):
     def __init__(self, *args, **kwargs):
@@ -12,6 +12,7 @@ class ReleaseField(forms.ModelChoiceField):
 class ModuleBranchForm(forms.Form):
     def __init__(self, module, *args, **kwargs):
         super(ModuleBranchForm, self).__init__(*args, **kwargs)
+        self.module = module
         self.branch_fields = []
         default_cat_name = None
         for branch in module.get_branches(reverse=True):
@@ -41,3 +42,17 @@ class ModuleBranchForm(forms.Form):
     def get_branches(self):
         for rel_field, cat_field in self.branch_fields:
             yield (self[rel_field], self[cat_field])
+
+    def clean(self):
+        cleaned_data = super(ModuleBranchForm, self).clean()
+        if cleaned_data['new_branch']:
+            try:
+                branch = Branch.objects.get(module=self.module, name=cleaned_data['new_branch'])
+            except Branch.DoesNotExist:
+                # The branch will be created later in the view
+                return cleaned_data
+            rel = Release.objects.get(pk=cleaned_data['new_branch_release'].pk)
+            cat = Category(release=rel, branch=branch, name=cleaned_data['new_branch_category'])
+            cat.full_clean()
+            cleaned_data['new_category'] = cat
+        return cleaned_data
diff --git a/stats/tests/tests.py b/stats/tests/tests.py
index d25d724..6a9dce4 100644
--- a/stats/tests/tests.py
+++ b/stats/tests/tests.py
@@ -23,11 +23,12 @@ import tarfile
 from datetime import date
 from functools import wraps
 
-from django.test import TestCase
+from django.conf import settings
+from django.contrib.auth.models import User
 from django.core import mail
 from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
-from django.conf import settings
+from django.test import TestCase
 
 from stats.models import Module, Domain, Branch, Release, Statistics, FakeLangStatistics
 from stats import utils
@@ -149,6 +150,24 @@ class ModuleTestCase(TestCase):
                         module = self.mod)
         self.assertRaises(ValidationError, branch.clean)
 
+    def test_edit_branches_form(self):
+        coord = User.objects.get(username='coord')
+        coord.set_password('coord')
+        coord.save()
+        self.client.login(username='coord', password='coord')
+        response = self.client.get(reverse('module_edit_branches', args=[self.mod]), follow=True)
+        self.assertContains(response, "you're not allowed to edit")
+        # Test now with appropriate permissions
+        coord.is_superuser = True
+        coord.save()
+        # Duplicate data
+        response = self.client.post(reverse('module_edit_branches', args=[self.mod]), {
+            '1': '1', '1_cat': 'desktop',
+            '2': '2', '2_cat': 'desktop',
+            'new_branch': 'master', 'new_branch_release': '2', 'new_branch_category': 'desktop'}
+        )
+        self.assertContains(response, "the form is not valid")
+
     def test_branch_sorting(self):
         b1 = Branch(name='a-branch', module=self.mod)
         b1.save(update_statistics=False)
diff --git a/stats/views.py b/stats/views.py
index 0b42dff..1b658eb 100644
--- a/stats/views.py
+++ b/stats/views.py
@@ -83,7 +83,8 @@ def module_edit_branches(request, module_name):
     mod = get_object_or_404(Module, name=module_name)
     if not mod.can_edit_branches(request.user):
         messages.error(request, "Sorry, you're not allowed to edit this module's branches")
-        return module(request, module_name)
+        return HttpResponseRedirect(mod.get_absolute_url())
+
     if request.method == 'POST':
         form = ModuleBranchForm(mod, request.POST)
         if form.is_valid():
@@ -115,25 +116,24 @@ def module_edit_branches(request, module_name):
                     updated = True
             # New branch (or new category)
             if form.cleaned_data['new_branch']:
-                branch_name = form.cleaned_data['new_branch']
-                try:
-                    branch = Branch.objects.get(module=mod, name=branch_name)
-                except Branch.DoesNotExist:
-                    # It is probably a new branch
-                    try:
-                        branch = Branch(module=mod, name=branch_name)
-                        branch.save()
-                        messages.success(request, "The new branch %s has been added" % branch_name)
-                        updated = True
-                        # Send message to gnome-i18n?
-                    except Exception, e:
-                        messages.error(request, "Error adding the branch '%s': %s" % (branch_name, str(e)))
-                        branch = None
-                if branch and form.cleaned_data['new_branch_release']:
-                    rel = Release.objects.get(pk=form.cleaned_data['new_branch_release'].pk)
-                    cat = Category(release=rel, branch=branch, name=form.cleaned_data['new_branch_category'])
-                    cat.save()
+                if 'new_category' in form.cleaned_data:
+                    form.cleaned_data['new_category'].save()
                     updated = True
+                else:
+                    branch_name = form.cleaned_data['new_branch']
+                    try:
+                        branch = Branch.objects.get(module=mod, name=branch_name)
+                    except Branch.DoesNotExist:
+                        # It is probably a new branch
+                        try:
+                            branch = Branch(module=mod, name=branch_name)
+                            branch.save()
+                            messages.success(request, "The new branch %s has been added" % branch_name)
+                            updated = True
+                            # Send message to gnome-i18n?
+                        except Exception, e:
+                            messages.error(request, "Error adding the branch '%s': %s" % (branch_name, 
str(e)))
+                            branch = None
             else:
                 messages.success(request, "Branches updated")
             if updated:
diff --git a/urls.py b/urls.py
index a541845..88b0bd9 100644
--- a/urls.py
+++ b/urls.py
@@ -75,7 +75,8 @@ urlpatterns += patterns('stats.views',
         name = 'module'),
     url(
         regex = r'^module/(?P<module_name>[\w\-\+]+)/edit/branches/$',
-        view = 'module_edit_branches'),
+        view = 'module_edit_branches',
+        name = 'module_edit_branches'),
     url(
         regex = r'^module/(?P<module_name>[\w\-\+]+)/branch/(?P<branch_name>[\w\-\.]+)/$',
         view = 'module_branch'),