Merge lp:~caio1982/capomastro/depstatus-1421337 into lp:capomastro

[Caio Begotti]

Improve how direct dependencies builds have their status shown in the detail view.

Address the problems of https://bugs.launchpad.net/capomastro/+bug/1421337 which basically have to do with how we display recently queued dependencies when someone requests a direct build of them (not via a project).

The original bug ticket has a screenshot showing the before and after these changes.

Some tests got changed because of the new default status for dependencies and because, well, we're not using the notifications bubbles texts in the tests anymore.

I have only tested these changes locally, not on Wendigo because PS4.5 is a bit overloaded now.

[Daniel Manrique (roadmr)]

Looks good to me, the "building" status is quite clear and less likely to get lost than the bubble.

review: Approve

=== modified file 'capomastro/site/templatetags/capomastro_bootstrap.py'
--- capomastro/site/templatetags/capomastro_bootstrap.py	2014-03-21 10:45:10 +0000
+++ capomastro/site/templatetags/capomastro_bootstrap.py	2015-07-02 19:45:24 +0000
@@ -13,7 +13,13 @@
     known_statuses = {
         "SUCCESS": "success",
         "FAILURE": "danger",
-        "ABORTED": "info"
+        "ABORTED": "warning",
+
+        # not actual jenkins statuses but
+        # these are useful to provide meaninful
+        # information for users in the views
+        "QUEUED": "info",
+        "BUILDING": "info"
     }
     return known_statuses.get(value, "")
 

=== modified file 'capomastro/site/templatetags/tests/test_capomastro_bootstrap.py'
--- capomastro/site/templatetags/tests/test_capomastro_bootstrap.py	2014-03-20 16:17:58 +0000
+++ capomastro/site/templatetags/tests/test_capomastro_bootstrap.py	2015-07-02 19:45:24 +0000
@@ -15,8 +15,9 @@
         "ABORTED" etc to a class suitable for Bootstrap tables.
         """
         for status, class_ in [
-                ("SUCCESS", "success"), ("ABORTED", "info"),
-                ("FAILURE", "danger"), ("", "")]:
+                ("SUCCESS", "success"), ("ABORTED", "warning"),
+                ("FAILURE", "danger"), ("BUILDING", "info"),
+                ("QUEUED", "info"), ("", "")]:
             self.assertEqual(class_, build_status_to_class(status))
 
     def test_build_status_to_class_in_template(self):

=== modified file 'jenkins/models.py'
--- jenkins/models.py	2014-08-07 14:40:40 +0000
+++ jenkins/models.py	2015-07-02 19:45:24 +0000
@@ -77,7 +77,8 @@
     duration = models.IntegerField(null=True)
     url = models.CharField(max_length=255)
     phase = models.CharField(max_length=25)  # FINALIZED, STARTED, COMPLETED
-    status = models.CharField(max_length=255)
+    # the default value here is only used a build had just been requested
+    status = models.CharField(max_length=255, default='QUEUED')
     console_log = models.TextField(blank=True, null=True, editable=False)
     parameters = fields.JSONField(blank=True, null=True, editable=False)
     created_at = models.DateTimeField(auto_now_add=True)

=== modified file 'jenkins/tests/test_views.py'
--- jenkins/tests/test_views.py	2014-11-29 13:10:09 +0000
+++ jenkins/tests/test_views.py	2015-07-02 19:45:24 +0000
@@ -102,7 +102,7 @@
 
         self.assertEqual(1, Build.objects.count())
         build = Build.objects.get(job=self.job, number=11)
-        self.assertEqual("", build.status)
+        self.assertEqual("QUEUED", build.status)
         self.assertEqual(Build.STARTED, build.phase)
 
     def test_handle_started_notification_with_build_id(self):

=== modified file 'projects/tests/test_views.py'
--- projects/tests/test_views.py	2015-05-06 16:49:56 +0000
+++ projects/tests/test_views.py	2015-07-02 19:45:24 +0000
@@ -544,7 +544,8 @@
 
         self.assertEqual(dependency, response.context["dependency"])
         self.assertEqual([project], list(response.context["projects"]))
-        self.assertNotContains(response, "Dependency currently building")
+        self.assertNotContains(response, "BUILDING")
+        self.assertNotContains(response, "QUEUED")
 
         self.assertEqual('', response.context["archiver"])
         archive = ArchiveFactory.create(policy="cdimage", default=True)
@@ -553,15 +554,15 @@
 
     def test_dependency_detail_with_currently_building(self):
         """
-        If the Dependency is currently building, we should get an info message
-        with this in the page.
+        If the Dependency is currently building, we should have a
+        proper status being shown in the build list of the page.
         """
         dependency = DependencyFactory.create()
         BuildFactory.create(job=dependency.job, status="STARTED")
         dependency_url = reverse("dependency_detail",
                                  kwargs={"pk": dependency.pk})
         response = self.app.get(dependency_url, user=self.user)
-        self.assertContains(response, "Dependency currently building")
+        self.assertContains(response, "BUILDING")
 
     def test_dependency_build(self):
         """

=== modified file 'projects/views.py'
--- projects/views.py	2015-05-06 16:49:56 +0000
+++ projects/views.py	2015-07-02 19:45:24 +0000
@@ -378,13 +378,17 @@
             context=context, page=self.request.GET.get('page'))
         context["projects"] = Project.objects.filter(
             dependencies=context["dependency"])
-        if context["dependency"].is_building:
-            messages.add_message(
-                self.request, messages.INFO,
-                "Dependency currently building")
-
         dependency = get_object_or_404(Dependency, pk=self.object.pk)
         build_parameters = dependency.get_build_parameters()
+
+        if dependency.is_building:
+            job = Build.objects.filter(job=dependency.job).first()
+            job.status = u'BUILDING'
+            # i was a bit unsure whether i should pass all the args again
+            # and simply call save() or not, but i decided to only update one
+            # field and keep it safe
+            job.save(update_fields=['status'])
+
         context["default_build_parameters"] = [x for x in
                   parse_parameters_from_job(self.object.job.jobtype.config_xml)
                   if x["name"] != "BUILD_ID" and x["name"] not in build_parameters]
@@ -404,6 +408,7 @@
         messages.add_message(
             self.request, messages.INFO,
             "Build for '%s' queued." % dependency.name)
+
         url = reverse("dependency_detail", kwargs={"pk": dependency.pk})
         return HttpResponseRedirect(url)
 
@@ -487,7 +492,7 @@
         return response
 
     def get_success_url(self):
-        return reverse("home")
+        return reverse("dependency_list")
 
     def get_context_data(self, **kwargs):
         """