views: use id__in = get_patch_ids() to get the modified patch set

Currently, we generic_list processes patches in order of the patch_id:X
parameter, which is arbitrarily decided by the browser.

By using id__in, we get patches sorted by the default Patch model
ordering. This means that the (arbitrary) order of get_patch_ids()
doesn't affect the ordering of patches that we process, and bundles
are created with a reasonable default patch order.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

2 files changed, 63 insertions, 7 deletions

diff --git a/apps/patchwork/tests/bundles.py b/apps/patchwork/tests/bundles.py
index 659500e..53eee25 100644
--- a/apps/patchwork/tests/bundles.py
+++ b/apps/patchwork/tests/bundles.py
@@ -18,8 +18,10 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 import unittest
+import datetime
 from django.test import TestCase
 from django.test.client import Client
+from django.utils.http import urlencode
 from patchwork.models import Patch, Bundle, BundlePatch, Person
 from patchwork.tests.utils import defaults, create_user, find_in_context
 
@@ -334,6 +336,66 @@ class BundleAddFromPatchTest(BundleTestBase):
                 for i in [0, 1] ]
         self.failUnless(bps[0].order < bps[1].order)
 
+class BundleInitialOrderTest(BundleTestBase):
+    """When creating bundles from a patch list, ensure that the patches in the
+       bundle are ordered by date"""
+
+    def setUp(self):
+        super(BundleInitialOrderTest, self).setUp(5)
+
+        # put patches in an arbitrary order
+        idxs = [2, 4, 3, 1, 0]
+        self.patches = [ self.patches[i] for i in idxs ]
+
+        # set dates to be sequential
+        last_patch = self.patches[0]
+        for patch in self.patches[1:]:
+            patch.date = last_patch.date + datetime.timedelta(0, 1)
+            patch.save()
+            last_patch = patch
+
+    def _testOrder(self, ids, expected_order):
+        newbundlename = 'testbundle-new'
+
+        # need to define our querystring explicity to enforce ordering
+        params = {'form': 'patchlistform',
+                  'bundle_name': newbundlename,
+                  'action': 'Create',
+                  'project': defaults.project.id,
+        }
+
+        data = urlencode(params) + \
+               ''.join([ '&patch_id:%d=checked' % i for i in ids ])
+
+        response = self.client.post(
+                '/project/%s/list/' % defaults.project.linkname,
+                data = data,
+                content_type = 'application/x-www-form-urlencoded',
+                )
+
+        self.assertContains(response, 'Bundle %s created' % newbundlename)
+        self.assertContains(response, 'added to bundle %s' % newbundlename,
+            count = 5)
+
+        bundle = Bundle.objects.get(name = newbundlename)
+
+        # BundlePatches should be sorted by .order by default
+        bps = BundlePatch.objects.filter(bundle = bundle)
+
+        for (bp, p) in zip(bps, expected_order):
+            self.assertEqual(bp.patch.pk, p.pk)
+
+        bundle.delete()
+
+    def testBundleForwardOrder(self):
+        ids = map(lambda p: p.id, self.patches)
+        self._testOrder(ids, self.patches)
+
+    def testBundleReverseOrder(self):
+        ids = map(lambda p: p.id, self.patches)
+        ids.reverse()
+        self._testOrder(ids, self.patches)
+
 class BundleReorderTest(BundleTestBase):
     def setUp(self):
         super(BundleReorderTest, self).setUp(5)
diff --git a/apps/patchwork/views/__init__.py b/apps/patchwork/views/__init__.py
index 3f50380..fbe44f5 100644
--- a/apps/patchwork/views/__init__.py
+++ b/apps/patchwork/views/__init__.py
@@ -47,13 +47,7 @@ def generic_list(request, project, view,
         if request.POST.get('bundle_name', False):
             action = 'create'
 
-        ps = []
-        for patch_id in get_patch_ids(request.POST):
-            try:
-                patch = Patch.objects.get(id = patch_id)
-            except Patch.DoesNotExist:
-                pass
-            ps.append(patch)
+        ps = Patch.objects.filter(id__in = get_patch_ids(request.POST))
 
         (errors, form) = set_patches(request.user, project, action, \
                 request.POST, ps, context)