Merge lp:~cprov/uci-engine/ts-reviews-ng into lp:uci-engine

[Celso Providelo]

Update TS model to support review 'status' instead of simple 'completed' flag.

Update TS model to support review 'status' instead of simple 'completed' flag.

Now review.status can hold PENDING (default), APPROVED and DISAPPROVED, similarly to LP MP votes.

The reviews in the webui will be broken for a short while, but a subsequent MP is adjusting it and adding tests.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Ursula Junque (ursinha)]

I have some comments below, regarding the use of the status you added:

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Ursula Junque (ursinha)]

As we discussed via IRC, if you are able to leave the review status easily editable before the ticket step changes, then you could go with the NEEDS_* options. I like the idea of having status similar to the MPs, so I won't say no to that, I was just trying to avoid confusion for the user whenever reviewing to a "transitional" status.

[Celso Providelo (cprov)]

We will be in a better position to decide if we are already in shape to add extra review status after working out the UI for them. No worries, let's go with it as it is and unblock the subsequent tasks.

[Ursula Junque (ursinha)]

review: Approve

[Ubuntu CI Bot (uci-bot)]

The attempt to merge lp:~cprov/uci-engine/ts-reviews-ng into lp:uci-engine failed. Below is the output from the failed tests.

Running cm...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
Updating source dependencies...
uploading webui-content.tgz to swift
Updating source dependencies...
2014-11-06 04:15:04 INFO juju.cmd supercommand.go:37 running jujud [1.20.11.1-precise-amd64 gc]
2014-11-06 04:15:04 DEBUG juju.agent agent.go:377 read agent config, format "1.18"
2014-11-06 04:15:04 INFO juju.jujud unit.go:78 unit agent unit-ci-airline-rabbit-0 start (1.20.11.1-precise-amd64 [gc])
2014-11-06 04:15:04 INFO juju.worker runner.go:260 start "api"
2014-11-06 04:15:04 INFO juju.state.api apiclient.go:242 dialing "wss://10.0.3.1:17070/"
2014-11-06 04:15:04 INFO juju.state.api apiclient.go:176 connection established to "wss://10.0.3.1:17070/"
2014-11-06 04:15:05 INFO juju.state.api apiclient.go:242 dialing "wss://10.0.3.1:17070/"
2014-11-06 04:15:05 INFO juju.state.api apiclient.go:176 connection established to "wss://10.0.3.1:17070/"
2014-11-06 04:15:05 INFO juju.state.api apiclient.go:242 dialing "wss://10.0.3.1:17070/"
2014-11-06 04:15:05 INFO juju.state.api apiclient.go:176 connection established to "wss://10.0.3.1:17070/"
2014-11-06 04:15:05 INFO juju.worker runner.go:260 start "upgrader"
2014-11-06 04:15:05 INFO juju.worker runner.go:260 start "logger"
2014-11-06 04:15:05 DEBUG juju.worker.logger logger.go:35 initial log config: "<root>=DEBUG"
2014-11-06 04:15:05 INFO juju.worker runner.go:260 start "uniter"
2014-11-06 04:15:05 DEBUG juju.worker.logger logger.go:60 logger setup
2014-11-06 04:15:05 INFO juju.worker runner.go:260 start "apiaddressupdater"
2014-11-06 04:15:05 INFO juju.worker runner.go:260 start "rsyslog"
2014-11-06 04:15:05 DEBUG juju.worker.rsyslog worker.go:75 starting rsyslog worker mode 1 for "unit-ci-airline-rabbit-0" "tarmac-local"
2014-11-06 04:15:05 DEBUG juju.worker.logger logger.go:45 reconfiguring logging from "<root>=DEBUG" to "<root>=WARNING;unit=DEBUG"
2014-11-06 04:15:10 INFO juju-log Installing python-jinja2 with options: ['--option=Dpkg::Options::=--force-confold']
2014-11-06 04:15:57 INFO install Reading package lists...
2014-11-06 04:15:58 INFO install Building dependency tree...
2014-11-06 04:15:58 INFO install Reading state information...
2014-11-06 04:15:59 INFO install The following extra packages will be installed:
2014-11-06 04:15:59 INFO install python-markupsafe
2014-11-06 04:15:59 INFO install Suggested packages:
2014-11-06 04:15:59 INFO install python-jinja2-doc
2014-11-06 04:15:59 INFO install The following NEW packages will be installed:
2014-11-06 04:15:59 INFO install python-jinja2 python-markupsafe
2014-11-06 04:15:59 INFO install 0 upgraded, 2 newly installed, 0 to remove and 3 not upgraded.
2014-11-06 04:15:59 INFO install Need to get 172 kB of archives.
2014-11-06 04:15:59 INFO install After this operation, 1124 kB of additional disk space will be used.
2014-11-06 04:15:59 INFO install Get:1 http://archive.ubuntu.com/ubuntu/ precise/main python-...

[lp:~cprov/uci-engine/ts-reviews-ng]

886. By Celso Providelo on 2014-11-06

Fixing Review representation and add a test.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

=== modified file 'ci-utils/ci_utils/ticket_states.py'
--- ci-utils/ci_utils/ticket_states.py	2014-10-23 01:21:04 +0000
+++ ci-utils/ci_utils/ticket_states.py	2014-11-06 04:57:04 +0000
@@ -80,6 +80,17 @@
     def __str__(self):
         return str(self.value)
 
+
+class ReviewStatus(DBEnumeratedType):
+
+    PENDING = Item(0, "Pending")
+    APPROVED = Item(10, "Approved")
+    DISAPPROVED = Item(100, "Disapproved")
+
+    def __str__(self):
+        return str(self.value)
+
+
 # we have workflow steps known to the lander (lander_service_wrapper). We
 # also have steps associated in the ticket-system. This provides a mapping
 # to translate between them

=== modified file 'ticket_system/ticket/api.py'
--- ticket_system/ticket/api.py	2014-11-05 05:23:20 +0000
+++ ticket_system/ticket/api.py	2014-11-06 04:57:04 +0000
@@ -45,6 +45,7 @@
     get_enum_value,
     MergeProposal,
     Review,
+    ReviewStatus,
     SubTicket,
     SourcePackageUpload,
     SubTicketArtifact,
@@ -583,6 +584,7 @@
 
 
 class ReviewResource(ModelResource):
+
     ticket = fields.ForeignKey(TicketResource, 'ticket')
 
     def dehydrate_workflow_step(self, bundle):
@@ -601,6 +603,21 @@
             bundle.data['workflow_step'] = value
         return bundle
 
+    def dehydrate_status(self, bundle):
+        return get_enum_title(bundle.data["status"], ReviewStatus)
+
+    def hydrate_status(self, bundle):
+        value = bundle.data.get('status')
+        if not value:
+            return bundle
+        try:
+            int(value)
+        except ValueError:
+            # user passed us "Approved"
+            value = get_enum_value(value, ReviewStatus)
+            bundle.data['status'] = value
+        return bundle
+
     class Meta:
         queryset = Review.objects.all()
         authorization = Authorization()
@@ -608,7 +625,7 @@
         filtering = {
             'ticket': ALL_WITH_RELATIONS,
             'review_type': ALL,
-            'completed': ALL,
+            'status': ALL,
         }
 
 

=== added file 'ticket_system/ticket/migrations/0019_auto__add_field_review_status__chg_field_review_review_type.py'
--- ticket_system/ticket/migrations/0019_auto__add_field_review_status__chg_field_review_review_type.py	1970-01-01 00:00:00 +0000
+++ ticket_system/ticket/migrations/0019_auto__add_field_review_status__chg_field_review_review_type.py	2014-11-06 04:57:04 +0000
@@ -0,0 +1,120 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'Review.status'
+        db.add_column(u'ticket_review', 'status',
+                      self.gf('django.db.models.fields.IntegerField')(default=0),
+                      keep_default=False)
+
+
+        # Changing field 'Review.review_type'
+        db.alter_column(u'ticket_review', 'review_type', self.gf('django.db.models.fields.CharField')(max_length=254))
+
+    def backwards(self, orm):
+        # Deleting field 'Review.status'
+        db.delete_column(u'ticket_review', 'status')
+
+
+        # Changing field 'Review.review_type'
+        db.alter_column(u'ticket_review', 'review_type', self.gf('django.db.models.fields.CharField')(max_length=4096))
+
+    models = {
+        u'project.sourcepackage': {
+            'Meta': {'object_name': 'SourcePackage', 'db_table': "'sourcepackage'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '4096'})
+        },
+        u'ticket.mergeproposal': {
+            'Meta': {'unique_together': "(('subticket', 'lp_url'),)", 'object_name': 'MergeProposal', 'db_table': "'mergeproposal'"},
+            'approved_revno': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lp_url': ('django.db.models.fields.CharField', [], {'max_length': '511'}),
+            'source_package_upload': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SourcePackageUpload']", 'null': 'True', 'blank': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'merge_proposals'", 'to': u"orm['ticket.SubTicket']"})
+        },
+        u'ticket.review': {
+            'Meta': {'object_name': 'Review'},
+            'completed': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'review_type': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'workflow_step': ('django.db.models.fields.IntegerField', [], {})
+        },
+        u'ticket.sourcepackageupload': {
+            'Meta': {'unique_together': "(('subticket', 'version'),)", 'object_name': 'SourcePackageUpload', 'db_table': "'sourcepackageupload'"},
+            'architecture': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'source_package_uploads'", 'to': u"orm['ticket.SubTicket']"}),
+            'version': ('django.db.models.fields.CharField', [], {'max_length': '254'})
+        },
+        u'ticket.subticket': {
+            'Meta': {'ordering': "['id']", 'unique_together': "(('ticket', 'sourcepackage'),)", 'object_name': 'SubTicket', 'db_table': "'subticket'"},
+            'assignee': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'sourcepackage': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['project.SourcePackage']"}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.subticketartifact': {
+            'Meta': {'object_name': 'SubTicketArtifact', 'db_table': "'subticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SubTicket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticket': {
+            'Meta': {'ordering': "['id']", 'object_name': 'Ticket', 'db_table': "'ticket'"},
+            'base_image': ('django.db.models.fields.CharField', [], {'default': "'http://cloud-images.ubuntu.com/utopic/current/utopic-server-cloudimg-amd64-disk1.img'", 'max_length': '4096'}),
+            'bug_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'build_ppa': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'creator': ('django.db.models.fields.EmailField', [], {'max_length': '254', 'null': 'True', 'blank': 'True'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lander_unit': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'master_ppa': ('django.db.models.fields.CharField', [], {'default': "'ppa:cprov/ci-master'", 'max_length': '4096'}),
+            'owner': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'private': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'series': ('django.db.models.fields.CharField', [], {'default': "'utopic'", 'max_length': '4096'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'default': "'37ff8af0-6543-11e4-b341-001c4229f9ac'", 'unique': 'True', 'max_length': '36'}),
+            'workflow': ('django.db.models.fields.related.ForeignKey', [], {'default': "'default'", 'to': u"orm['ticket.Workflow']"})
+        },
+        u'ticket.ticketartifact': {
+            'Meta': {'object_name': 'TicketArtifact', 'db_table': "'ticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticketcitrainoverlay': {
+            'Meta': {'object_name': 'TicketCiTrainOverlay', 'db_table': "'ticketcitrainoverlay'"},
+            'comments': ('django.db.models.fields.TextField', [], {}),
+            'job_url': ('django.db.models.fields.URLField', [], {'max_length': '200'}),
+            'landers': ('django.db.models.fields.TextField', [], {}),
+            'sync_request': ('django.db.models.fields.CharField', [], {'max_length': '256'}),
+            'test_notes': ('django.db.models.fields.TextField', [], {}),
+            'ticket': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'citrain_overlay'", 'unique': 'True', 'primary_key': 'True', 'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.workflow': {
+            'Meta': {'object_name': 'Workflow'},
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'primary_key': 'True'}),
+            'steps': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        }
+    }
+
+    complete_apps = ['ticket']
\ No newline at end of file

=== added file 'ticket_system/ticket/migrations/0020_fill_review_status.py'
--- ticket_system/ticket/migrations/0020_fill_review_status.py	1970-01-01 00:00:00 +0000
+++ ticket_system/ticket/migrations/0020_fill_review_status.py	2014-11-06 04:57:04 +0000
@@ -0,0 +1,119 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import DataMigration
+from django.db import models
+
+from ci_utils.ticket_states import ReviewStatus
+
+
+class Migration(DataMigration):
+
+    def forwards(self, orm):
+        "Completed reviews become APPROVED."
+        for r in orm['ticket.Review'].objects.all():
+            if r.completed:
+                r.status = ReviewStatus.APPROVED.value
+            r.save()
+
+    def backwards(self, orm):
+        "APPROVED reviews become 'completed'."
+        for r in orm['ticket.Review'].objects.all():
+            if r.status == ReviewStatus.APPROVED.value:
+                r.completed = True
+            r.save()
+
+    models = {
+        u'project.sourcepackage': {
+            'Meta': {'object_name': 'SourcePackage', 'db_table': "'sourcepackage'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '4096'})
+        },
+        u'ticket.mergeproposal': {
+            'Meta': {'unique_together': "(('subticket', 'lp_url'),)", 'object_name': 'MergeProposal', 'db_table': "'mergeproposal'"},
+            'approved_revno': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lp_url': ('django.db.models.fields.CharField', [], {'max_length': '511'}),
+            'source_package_upload': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SourcePackageUpload']", 'null': 'True', 'blank': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'merge_proposals'", 'to': u"orm['ticket.SubTicket']"})
+        },
+        u'ticket.review': {
+            'Meta': {'object_name': 'Review'},
+            'completed': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'review_type': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'workflow_step': ('django.db.models.fields.IntegerField', [], {})
+        },
+        u'ticket.sourcepackageupload': {
+            'Meta': {'unique_together': "(('subticket', 'version'),)", 'object_name': 'SourcePackageUpload', 'db_table': "'sourcepackageupload'"},
+            'architecture': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'source_package_uploads'", 'to': u"orm['ticket.SubTicket']"}),
+            'version': ('django.db.models.fields.CharField', [], {'max_length': '254'})
+        },
+        u'ticket.subticket': {
+            'Meta': {'ordering': "['id']", 'unique_together': "(('ticket', 'sourcepackage'),)", 'object_name': 'SubTicket', 'db_table': "'subticket'"},
+            'assignee': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'sourcepackage': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['project.SourcePackage']"}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.subticketartifact': {
+            'Meta': {'object_name': 'SubTicketArtifact', 'db_table': "'subticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SubTicket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticket': {
+            'Meta': {'ordering': "['id']", 'object_name': 'Ticket', 'db_table': "'ticket'"},
+            'base_image': ('django.db.models.fields.CharField', [], {'default': "'http://cloud-images.ubuntu.com/utopic/current/utopic-server-cloudimg-amd64-disk1.img'", 'max_length': '4096'}),
+            'bug_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'build_ppa': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'creator': ('django.db.models.fields.EmailField', [], {'max_length': '254', 'null': 'True', 'blank': 'True'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lander_unit': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'master_ppa': ('django.db.models.fields.CharField', [], {'default': "'ppa:cprov/ci-master'", 'max_length': '4096'}),
+            'owner': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'private': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'series': ('django.db.models.fields.CharField', [], {'default': "'utopic'", 'max_length': '4096'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'default': "'904d7686-6543-11e4-86b0-001c4229f9ac'", 'unique': 'True', 'max_length': '36'}),
+            'workflow': ('django.db.models.fields.related.ForeignKey', [], {'default': "'default'", 'to': u"orm['ticket.Workflow']"})
+        },
+        u'ticket.ticketartifact': {
+            'Meta': {'object_name': 'TicketArtifact', 'db_table': "'ticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticketcitrainoverlay': {
+            'Meta': {'object_name': 'TicketCiTrainOverlay', 'db_table': "'ticketcitrainoverlay'"},
+            'comments': ('django.db.models.fields.TextField', [], {}),
+            'job_url': ('django.db.models.fields.URLField', [], {'max_length': '200'}),
+            'landers': ('django.db.models.fields.TextField', [], {}),
+            'sync_request': ('django.db.models.fields.CharField', [], {'max_length': '256'}),
+            'test_notes': ('django.db.models.fields.TextField', [], {}),
+            'ticket': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'citrain_overlay'", 'unique': 'True', 'primary_key': 'True', 'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.workflow': {
+            'Meta': {'object_name': 'Workflow'},
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'primary_key': 'True'}),
+            'steps': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        }
+    }
+
+    complete_apps = ['ticket']
+    symmetrical = True

=== added file 'ticket_system/ticket/migrations/0021_auto__del_field_review_completed.py'
--- ticket_system/ticket/migrations/0021_auto__del_field_review_completed.py	1970-01-01 00:00:00 +0000
+++ ticket_system/ticket/migrations/0021_auto__del_field_review_completed.py	2014-11-06 04:57:04 +0000
@@ -0,0 +1,113 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Deleting field 'Review.completed'
+        db.delete_column(u'ticket_review', 'completed')
+
+
+    def backwards(self, orm):
+        # Adding field 'Review.completed'
+        db.add_column(u'ticket_review', 'completed',
+                      self.gf('django.db.models.fields.BooleanField')(default=False),
+                      keep_default=False)
+
+
+    models = {
+        u'project.sourcepackage': {
+            'Meta': {'object_name': 'SourcePackage', 'db_table': "'sourcepackage'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '4096'})
+        },
+        u'ticket.mergeproposal': {
+            'Meta': {'unique_together': "(('subticket', 'lp_url'),)", 'object_name': 'MergeProposal', 'db_table': "'mergeproposal'"},
+            'approved_revno': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lp_url': ('django.db.models.fields.CharField', [], {'max_length': '511'}),
+            'source_package_upload': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SourcePackageUpload']", 'null': 'True', 'blank': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'merge_proposals'", 'to': u"orm['ticket.SubTicket']"})
+        },
+        u'ticket.review': {
+            'Meta': {'object_name': 'Review'},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'review_type': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'workflow_step': ('django.db.models.fields.IntegerField', [], {})
+        },
+        u'ticket.sourcepackageupload': {
+            'Meta': {'unique_together': "(('subticket', 'version'),)", 'object_name': 'SourcePackageUpload', 'db_table': "'sourcepackageupload'"},
+            'architecture': ('django.db.models.fields.CharField', [], {'max_length': '254'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'source_package_uploads'", 'to': u"orm['ticket.SubTicket']"}),
+            'version': ('django.db.models.fields.CharField', [], {'max_length': '254'})
+        },
+        u'ticket.subticket': {
+            'Meta': {'ordering': "['id']", 'unique_together': "(('ticket', 'sourcepackage'),)", 'object_name': 'SubTicket', 'db_table': "'subticket'"},
+            'assignee': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'sourcepackage': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['project.SourcePackage']"}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.subticketartifact': {
+            'Meta': {'object_name': 'SubTicketArtifact', 'db_table': "'subticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'subticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.SubTicket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticket': {
+            'Meta': {'ordering': "['id']", 'object_name': 'Ticket', 'db_table': "'ticket'"},
+            'base_image': ('django.db.models.fields.CharField', [], {'default': "'http://cloud-images.ubuntu.com/utopic/current/utopic-server-cloudimg-amd64-disk1.img'", 'max_length': '4096'}),
+            'bug_id': ('django.db.models.fields.IntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'build_ppa': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'creator': ('django.db.models.fields.EmailField', [], {'max_length': '254', 'null': 'True', 'blank': 'True'}),
+            'current_workflow_step': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lander_unit': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'master_ppa': ('django.db.models.fields.CharField', [], {'default': "'ppa:cprov/ci-master'", 'max_length': '4096'}),
+            'owner': ('django.db.models.fields.EmailField', [], {'max_length': '254'}),
+            'private': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'series': ('django.db.models.fields.CharField', [], {'default': "'utopic'", 'max_length': '4096'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '4096'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'default': "'c434306e-6545-11e4-be5c-001c4229f9ac'", 'unique': 'True', 'max_length': '36'}),
+            'workflow': ('django.db.models.fields.related.ForeignKey', [], {'default': "'default'", 'to': u"orm['ticket.Workflow']"})
+        },
+        u'ticket.ticketartifact': {
+            'Meta': {'object_name': 'TicketArtifact', 'db_table': "'ticket_artifact'"},
+            u'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'reference': ('django.db.models.fields.CharField', [], {'max_length': '4096'}),
+            'ticket': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['ticket.Ticket']"}),
+            'type': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        },
+        u'ticket.ticketcitrainoverlay': {
+            'Meta': {'object_name': 'TicketCiTrainOverlay', 'db_table': "'ticketcitrainoverlay'"},
+            'comments': ('django.db.models.fields.TextField', [], {}),
+            'job_url': ('django.db.models.fields.URLField', [], {'max_length': '200'}),
+            'landers': ('django.db.models.fields.TextField', [], {}),
+            'sync_request': ('django.db.models.fields.CharField', [], {'max_length': '256'}),
+            'test_notes': ('django.db.models.fields.TextField', [], {}),
+            'ticket': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'citrain_overlay'", 'unique': 'True', 'primary_key': 'True', 'to': u"orm['ticket.Ticket']"})
+        },
+        u'ticket.workflow': {
+            'Meta': {'object_name': 'Workflow'},
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '4096', 'primary_key': 'True'}),
+            'steps': ('django.db.models.fields.CharField', [], {'max_length': '4096'})
+        }
+    }
+
+    complete_apps = ['ticket']
\ No newline at end of file

=== modified file 'ticket_system/ticket/models.py'
--- ticket_system/ticket/models.py	2014-11-05 05:23:20 +0000
+++ ticket_system/ticket/models.py	2014-11-06 04:57:04 +0000
@@ -29,6 +29,7 @@
 from ci_utils import unit_config
 
 from ci_utils.ticket_states import (
+    ReviewStatus,
     STEP_TO_WORKFLOW_MAPPING,
     SubTicketWorkflowStep,
     SubTicketWorkflowStepStatus,
@@ -202,7 +203,8 @@
     def _get_workflow_steps(self):
         '''build workflow steps injecting pauses where reviews are needed'''
         steps = json.loads(self.workflow.steps)
-        reviews = self.review_set.filter(completed=False)
+        reviews = self.review_set.exclude(
+            status=ReviewStatus.APPROVED.value)
         pauses = [x.workflow_step for x in reviews]
         for step in steps:
             step_val = STEP_TO_WORKFLOW_MAPPING.get(step['name'], -1)
@@ -390,20 +392,35 @@
 
 
 class Review(models.Model):
-    ticket = models.ForeignKey(Ticket)
-    workflow_step = models.IntegerField(choices=_choices(TicketWorkflowStep))
-    review_type = models.CharField(max_length=4096)
-    completed = models.BooleanField(default=False)
+
+    ticket = models.ForeignKey(
+        Ticket,
+        help_text='Context ticket of this review.')
+
+    workflow_step = models.IntegerField(
+        choices=_choices(TicketWorkflowStep),
+        help_text='Workflow step where the review is required.')
+
+    review_type = models.CharField(
+        max_length=254, null=False, blank=False,
+        help_text='What type of review is needed.')
+
+    status = models.IntegerField(
+        choices=_choices(ReviewStatus),
+        default=ReviewStatus.PENDING.value,
+        help_text='Current review status.')
 
     def save(self, *args, **kwargs):
+        """If all reviews are approved, resume the ticket processing."""
         super(Review, self).save(*args, **kwargs)
-        if self.ticket.review_set.filter(completed=False).count() == 0:
-            # all reviews are complete, if ticket is paused we should
-            # mark it "continue"
+        all_approved = not self.ticket.review_set.exclude(
+            status=ReviewStatus.APPROVED.value).exists()
+        if all_approved:
             if int(self.ticket.status) == int(TicketWorkflowStepStatus.PAUSED):
                 self.ticket.status = TicketWorkflowStepStatus.CONTINUE
                 self.ticket.save()
 
     def __unicode__(self):
-        return 'ticket-{} - {}: completed({})'.format(
-            self.ticket.uuid, self.review_type, self.completed)
+        return '{} - {}: {}'.format(
+            self.ticket.uuid, self.review_type,
+            get_enum_title(self.status, ReviewStatus))

=== modified file 'ticket_system/ticket/tests/test_models.py'
--- ticket_system/ticket/tests/test_models.py	2014-10-14 18:58:42 +0000
+++ ticket_system/ticket/tests/test_models.py	2014-11-06 04:57:04 +0000
@@ -24,6 +24,7 @@
 from ticket.models import (
     MergeProposal,
     Review,
+    ReviewStatus,
     SourcePackageUpload,
     SubTicket,
     SubTicketArtifact,
@@ -298,13 +299,33 @@
         self.assertEqual(0, send.call_count)
 
         # This will trigger the ticket object to be moved to CONTINUE
-        r.completed = True
+        r.status = ReviewStatus.APPROVED.value
         r.save()
         status = Ticket.objects.get(pk=self.ticket.id).status
         self.assertEqual(TicketWorkflowStepStatus.CONTINUE.value, status)
         self.assertEqual(1, send.call_count)
 
 
+class TestReview(TestCase):
+
+    def setUp(self):
+        _m = mock.patch('ticket.models.Ticket.create_container')
+        self.create_container = _m.start()
+        self.addCleanup(_m.stop)
+        self.ticket = create_ticket(owner='[e-mail]')
+
+    def test_title(self):
+        # Ensure review title include the ticket UUID, review type and
+        # its status.
+        review = Review.objects.create(
+            ticket=self.ticket, review_type='packaging',
+            workflow_step=TicketWorkflowStep.PKG_PUBLISHING)
+
+        self.assertEqual(
+            '{} - packaging: Pending'.format(self.ticket.uuid),
+            unicode(review))
+
+
 class WorkflowTestCase(TestCase):
     def test_unique_name(self):
         '''make sure the workflow name is unique'''

=== modified file 'ticket_system/ticket/tests/test_read_api.py'
--- ticket_system/ticket/tests/test_read_api.py	2014-11-04 09:05:06 +0000
+++ ticket_system/ticket/tests/test_read_api.py	2014-11-06 04:57:04 +0000
@@ -161,11 +161,11 @@
                         '/api/v1/subticket/{}/'.format(s.pk)
                         for s in self.ticket.subticket_set.all()],
                     u'reviews': [{
-                        u'completed': False,
                         u'id': self.review_1.pk,
                         u'resource_uri': unicode(
                             '/api/v1/review/{0}/'.format(self.review_1.pk)),
                         u'review_type': u'QA',
+                        u'status': u'Pending',
                         u'ticket': unicode(
                             '/api/v1/ticket/{}/'.format(self.ticket.uuid)),
                         u'workflow_step': u'Image testing'}],
@@ -235,11 +235,11 @@
                     '/api/v1/subticket/{}/'.format(s.pk)
                     for s in self.ticket.subticket_set.all()],
                 u'reviews': [{
-                    u'completed': False,
                     u'id': self.review_1.pk,
                     u'resource_uri': unicode(
                         '/api/v1/review/{0}/'.format(self.review_1.pk)),
                     u'review_type': u'QA',
+                    u'status': u'Pending',
                     u'ticket': unicode(
                         '/api/v1/ticket/{}/'.format(self.ticket.uuid)),
                     u'workflow_step': u'Image testing'}],
@@ -314,11 +314,11 @@
                     '/api/v1/subticket/{}/'.format(s.pk)
                     for s in self.ticket.subticket_set.all()],
                 u'reviews': [{
-                    u'completed': False,
                     u'id': self.review_1.pk,
                     u'resource_uri': unicode(
                         '/api/v1/review/{0}/'.format(self.review_1.pk)),
                     u'review_type': u'QA',
+                    u'status': u'Pending',
                     u'ticket': unicode(
                         '/api/v1/ticket/{}/'.format(self.ticket.uuid)),
                     u'workflow_step': u'Image testing'}],
@@ -439,11 +439,11 @@
                 u'/api/v1/subticket/{}/'.format(s.pk)
                 for s in self.ticket.subticket_set.all()],
             u'reviews': [{
-                u'completed': False,
                 u'id': self.review_1.pk,
                 u'resource_uri': unicode(
                     '/api/v1/review/{0}/'.format(self.review_1.pk)),
                 u'review_type': u'QA',
+                u'status': u'Pending',
                 u'ticket': unicode(
                     '/api/v1/ticket/{0}/'.format(self.ticket.uuid)),
                 u'workflow_step': u'Image testing'}],

=== modified file 'ticket_system/ticket/tests/test_write_api.py'
--- ticket_system/ticket/tests/test_write_api.py	2014-11-05 05:23:20 +0000
+++ ticket_system/ticket/tests/test_write_api.py	2014-11-06 04:57:04 +0000
@@ -30,6 +30,7 @@
 m.start()
 from ticket.models import (
     Review,
+    ReviewStatus,
     SourcePackageUpload,
     SubTicket,
     SubTicketWorkflowStep,
@@ -168,11 +169,11 @@
         self.assertEqual('QA', review_pkg.review_type)
         self.assertEqual(
             TicketWorkflowStep.PKG_VALIDATION.value, review_pkg.workflow_step)
-        self.assertFalse(review_pkg.completed)
+        self.assertEqual(ReviewStatus.PENDING.value, review_pkg.status)
         self.assertEqual('QA', review_img.review_type)
         self.assertEqual(
             TicketWorkflowStep.IMAGE_BUILDING.value, review_img.workflow_step)
-        self.assertFalse(review_img.completed)
+        self.assertFalse(ReviewStatus.PENDING.value, review_img.status)
 
         # Note that the reviews created via POST are new, they are not
         # mysteriously reused/updated by tastypie.
@@ -189,7 +190,7 @@
             'title': 'Atomic Patched Ticket',
             'reviews': [{
                 'id': review.id,
-                'completed': True,
+                'status': 'Approved',
                 'workflow_step': 'Image building',
             }],
         })
@@ -200,7 +201,7 @@
         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)
+        self.assertEqual(ReviewStatus.APPROVED.value, new_review.status)
 
     def test_post_ticket_atomically_reuses_workflows(self):
         # `Ticket` atomic creation re-uses existing `Workflows`s.
@@ -721,10 +722,12 @@
                                   workflow_step=0)
         uri = 'review/' + str(t.pk) + '/'
         params = self.get(uri)
-        self.assertFalse(params['completed'])
-        params['completed'] = True
+        self.assertEqual(ReviewStatus.PENDING.title, params['status'])
+        params['status'] = 'Approved'
         self.patch(uri, params)
-        self.assertTrue(Review.objects.get(pk=t.id).completed)
+        self.assertEqual(
+            ReviewStatus.APPROVED.value,
+            Review.objects.get(pk=t.id).status)
 
     def test_delete(self):
         t = Review.objects.create(ticket=self.ticket, review_type='a',
@@ -739,5 +742,5 @@
         resp = self.get('review/', {'ticket': self.ticket.pk})
         self.assertEqual(1, resp['meta']['total_count'])
 
-        resp = self.get('review/', {'completed': True})
+        resp = self.get('review/', {'status': '10'})
         self.assertEqual(0, resp['meta']['total_count'])