Merge lp:~cprov/uci-engine/webui-reviews-refix into lp:uci-engine

[Celso Providelo]

Re-fixing ticket review creation and updating and extending test coverage for vital features like that.

Re-fixing ticket review creation and updating and extending test coverage for vital features like that.

Despite of them being tastypie stock-features, it seems to be easy to get things messed up by adding changes to the top-level resource (ticket).

Also landing quite a few drive-by adjustments for using django-admin forms and enabling support for vivid in the TS (it was already exposed in the webui).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:883
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1672/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1672/rebuild

review: Approve (continuous-integration)

[Evan (ev)]

Looks good.

review: Approve

=== modified file 'ci-utils/ci_utils/__init__.py'
--- ci-utils/ci_utils/__init__.py	2014-10-30 17:30:49 +0000
+++ ci-utils/ci_utils/__init__.py	2014-11-05 05:32:38 +0000
@@ -37,6 +37,7 @@
     'precise',
     'trusty',
     'utopic',
+    'vivid',
 ]
 
 

=== modified file 'juju-deployer/configs/unit_config.yaml.tmpl'
--- juju-deployer/configs/unit_config.yaml.tmpl	2014-10-16 09:27:12 +0000
+++ juju-deployer/configs/unit_config.yaml.tmpl	2014-11-05 05:32:38 +0000
@@ -37,6 +37,7 @@
 private_ppas_only: $CI_PRIVATE_PPAS_ONLY
 
 image_map:
+  vivid: http://cloud-images.ubuntu.com/vivid/current/vivid-server-cloudimg-amd64-disk1.img
   utopic: http://cloud-images.ubuntu.com/utopic/current/utopic-server-cloudimg-amd64-disk1.img
   trusty: http://cloud-images.ubuntu.com/trusty/current/trusty-server-cloudimg-amd64-disk1.img
   precise: http://cloud-images.ubuntu.com/precise/current/precise-server-cloudimg-amd64-disk1.img

=== modified file 'ticket_system/ticket/api.py'
--- ticket_system/ticket/api.py	2014-10-30 04:57:56 +0000
+++ ticket_system/ticket/api.py	2014-11-05 05:32:38 +0000
@@ -136,7 +136,9 @@
             return bundle
 
         for subticket in subtickets:
-            if not isinstance(subticket, dict):
+            if isinstance(subticket, basestring):
+                continue
+            elif not isinstance(subticket, dict):
                 sp = subticket.data['sourcepackage']
             else:
                 sp = subticket['sourcepackage']
@@ -602,6 +604,7 @@
     class Meta:
         queryset = Review.objects.all()
         authorization = Authorization()
+        allowed_methods = ['get', 'post', 'patch', 'delete']
         filtering = {
             'ticket': ALL_WITH_RELATIONS,
             'review_type': ALL,

=== modified file 'ticket_system/ticket/models.py'
--- ticket_system/ticket/models.py	2014-11-04 09:05:06 +0000
+++ ticket_system/ticket/models.py	2014-11-05 05:32:38 +0000
@@ -99,7 +99,7 @@
         }, {
             'name': 'citrain',
             'steps': json.dumps([
-                {'name': 'silo_creation'},
+                {'name': 'silo_creating'},
                 {'name': 'silo_building'},
                 {'name': 'silo_testing'},
                 {'name': 'silo_publishing'}
@@ -124,7 +124,7 @@
     owner = models.EmailField(max_length=254)
     creator = models.EmailField(max_length=254, null=True, blank=True)
     title = models.CharField(max_length=4096)
-    description = models.TextField()
+    description = models.TextField(blank=True)
     bug_id = models.IntegerField(null=True, blank=True)
     current_workflow_step = models.IntegerField(
         choices=_choices(TicketWorkflowStep), max_length=4096,
@@ -288,7 +288,7 @@
         help_text='Branch revision number approved for this cycle.')
 
     source_package_upload = models.ForeignKey(
-        SourcePackageUpload, null=True,
+        SourcePackageUpload, null=True, blank=True,
         help_text='Current SPU generated by this merge proposal.')
 
     class Meta:
@@ -336,7 +336,8 @@
             ~models.Q(pk=self.ticket.pk),
             ~models.Q(status=TicketWorkflowStepStatus.FAILED),
             current_workflow_step__range=(
-                TicketWorkflowStep.QUEUED, TicketWorkflowStep.PKG_PUBLISHING),
+                TicketWorkflowStep.SILO_BUILDING,
+                TicketWorkflowStep.PKG_PUBLISHING),
         )
 
         return conflicting_tickets
@@ -405,4 +406,4 @@
 
     def __unicode__(self):
         return 'ticket-{} - {}: completed({})'.format(
-            self.ticket.id, self.review_type, self.completed)
+            self.ticket.uuid, self.review_type, self.completed)

=== modified file 'ticket_system/ticket/tests/test_write_api.py'
--- ticket_system/ticket/tests/test_write_api.py	2014-11-04 17:01:32 +0000
+++ ticket_system/ticket/tests/test_write_api.py	2014-11-05 05:32:38 +0000
@@ -135,25 +135,72 @@
             [unicode(spu) for spu in sub_bar.source_package_uploads.all()])
 
     def test_post_ticket_atomically_for_reviews(self):
+        # 'Reviews' can be created atomically with the related `Ticket`.
+
+        # We are injecting a review here to check if tastypie it behaving
+        # correctly and not updating existing reviews to accomplish the
+        # request.
+        other_ticket = create_ticket()
+        existing_review = other_ticket.review_set.create(
+            review_type='QA',
+            workflow_step=TicketWorkflowStep.IMAGE_BUILDING)
+
+        # Let's POST to create a new ticket with pre-defined reviews.
         params = self.post_ticket_data.copy()
         params.update({
             'title': 'Atomic Ticket with Review',
             'reviews': [{
                 'review_type': 'QA',
                 'workflow_step': 'Package validation',
+            }, {
+                'review_type': 'QA',
+                'workflow_step': 'Image building',
             }],
         })
         self.post(resource=self.resource, params=params)
 
         # 'Atomic Ticket with Review' was created.
-        [existing, ticket] = list(Ticket.objects.all())
+        [existing, other, ticket] = list(Ticket.objects.all())
+
         self.assertEqual('Atomic Ticket with Review', unicode(ticket))
-        # and already contain the specified `Review`
-        [review] = list(ticket.review_set.all())
-        self.assertEqual('QA', review.review_type)
-        self.assertEqual(
-            TicketWorkflowStep.PKG_VALIDATION.value, review.workflow_step)
-        self.assertFalse(review.completed)
+        # and already contain the specified `Review`s
+        [review_pkg, review_img] = list(ticket.review_set.all())
+        self.assertEqual('QA', review_pkg.review_type)
+        self.assertEqual(
+            TicketWorkflowStep.PKG_VALIDATION.value, review_pkg.workflow_step)
+        self.assertFalse(review_pkg.completed)
+        self.assertEqual('QA', review_img.review_type)
+        self.assertEqual(
+            TicketWorkflowStep.IMAGE_BUILDING.value, review_img.workflow_step)
+        self.assertFalse(review_img.completed)
+
+        # Note that the reviews created via POST are new, they are not
+        # mysteriously reused/updated by tastypie.
+        self.assertNotIn(existing_review.pk, [review_pkg.pk, review_img.pk])
+
+    def test_patch_ticket_reviews(self):
+        # `Ticket` PATCH can update `Review`s.
+        review = self.ticket.review_set.create(
+            review_type='QA',
+            workflow_step=TicketWorkflowStep.IMAGE_BUILDING)
+
+        params = self.post_ticket_data.copy()
+        params.update({
+            'title': 'Atomic Patched Ticket',
+            'reviews': [{
+                'id': review.id,
+                'completed': True,
+                'workflow_step': 'Image building',
+            }],
+        })
+        self.patch(resource=self.detail_url, params=params)
+
+        # The existing `Subticket` was updated and a new one was created.
+        [ticket] = list(Ticket.objects.all())
+        self.assertEqual('Atomic Patched Ticket', unicode(ticket))
+        [new_review] = list(ticket.review_set.all())
+        self.assertEqual(review.id, new_review.id)
+        self.assertTrue(new_review.completed)
 
     def test_post_ticket_atomically_reuses_workflows(self):
         # `Ticket` atomic creation re-uses existing `Workflows`s.

=== modified file 'webui/tickets/static/tickets/webui.js'
--- webui/tickets/static/tickets/webui.js	2014-11-04 13:32:04 +0000
+++ webui/tickets/static/tickets/webui.js	2014-11-05 05:32:38 +0000
@@ -54,9 +54,11 @@
             this.get('srcNode').all('button').on('click', function(e) {
                 var review = self.get('review');
                 var data = {
-                    reviews: [
-                        {'id': review.id, 'completed': true}
-                    ]
+                    reviews: [{
+                        'id': review.id,
+                        'workflow_step': review.workflow_step,
+                        'completed': true
+                    }]
                 };
                 // Issue a PATCH TS-request to update the review and
                 // update the widget accordingly.