Merge lp:~wgrant/launchpad/bug-540819-fix-builder-list-icons into lp:launchpad

[William Grant]

Move the icon for package builds in builder listings back to the start of the text, where it is for the other build types.

This branch fixes the icon positioning on https://edge.launchpad.net/builders to match production's. It was broken a week or so ago during buildfarm generalisation changes.

Before: http://people.ubuntu.com/~wgrant/launchpad/bug-540819/before.png
After: http://people.ubuntu.com/~wgrant/launchpad/bug-540819/after.png

This brings the binary build rendering back into line with historical behaviour, except that it will always show the BUILDING icon. Previously it would occasionally show the SFULLYBUILT icon for a fraction of a second at the end of a build, which could probably have been called a bug anyway.

[Aaron Bentley (abentley)]

Has this had a preimplementation call? Is there no fmt:link formatter for these items, or is it somehow inappropriate?

[William Grant (wgrant)]

I discussed this a week or so ago with jtv and noodles, IIRC. Note that for Builds and SPRBs we already use fmt:link here -- but including the icon and other text in it is only appropriate in this particular context.

[William Grant (wgrant)]

20:55 < wgrant> jtv: The 'building' icon spacing on https://edge.launchpad.net/builders seems a bit broken. Is that related to your changes?
20:56 < jtv> wgrant: yes, it is. How appropriate it is depends a bit on your perspective: it's the build status, so I'm displaying it as part of the link to the build. It did simplify the code.
20:57 < jtv> wgrant: in fact this is the subject matter of a UI review that's currently somewhere in limbo.
20:57 < wgrant> jtv: The current spacing is definitely broken. I think the ordering is too, but that's up for debate.
20:58 < jtv> wgrant: I don't think I changed the ordering... did I?
20:58 < wgrant> It was '( ) Building _blah blah blah i386_'
20:58 < wgrant> It is now 'Building ( )_blah blah blah i386_'
20:59 < jtv> wgrant: oh, sorry, misunderstood you earlier. That's the part I was talking about all along.
20:59 < wgrant> jtv: Why does it make the code simpler?
20:59 < wgrant> Because it was looking at the build status to generate the icon?
21:00 < jtv> wgrant: and because it was easier to have a link formatter for builds.
21:01 < wgrant> jtv: Argh, where's the template for that?
21:01 * wgrant cannot find it.
21:01 < jtv> builder-index.pt
21:03 < wgrant> jtv: Looks like buildfarmbuildjob-current.pt
21:03 < jtv> ah yes, that's where the link happens now.
21:03 < jtv> That was another thing: we can't show that icon for a buildless job.
21:04 < jtv> So that's another way in which it belongs to the build, not the builder.
21:04 < wgrant> So, I think we should do what is done for the rest of the states.
21:04 < wgrant> And just use the currentlybuilding icon if there's a job assigned.
21:04 < wgrant> Regardless of state.
21:04 < jtv> That's fine by me.
21:06 < wgrant> jtv: Ah, so you already hardcode that icon form non-build BFJs?
21:07 < jtv> wgrant: I _think_ I did that in an earlier iteration but it didn't seem appropriate.
21:07 < wgrant> It appears to be in devel's buildfarmjob-current.pt
21:08 < jtv> wgrant: there was a lot of exploring to get it all in a sort of minimal working shape... I don't mind moving things around a bit now that we have a working situation.
21:09 < wgrant> Yep.

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py	2010-03-20 22:11:01 +0000
+++ lib/canonical/launchpad/webapp/tales.py	2010-03-30 08:05:49 +0000
@@ -1523,22 +1523,15 @@
         else:
             return ""
 
-    def icon(self, view_name):
-        if not check_permission('launchpad.View', self._context):
-            return '<img src="/@@/processing" alt="[build]" />'
-
-        return BuildImageDisplayAPI(self._context).icon()
-
     def link(self, view_name, rootsite=None):
-        icon = self.icon(view_name)
         build = self._context
         if not check_permission('launchpad.View', build):
-            return '%s private source' % icon
+            return 'private source'
 
         url = self.url(view_name=view_name, rootsite=rootsite)
         title = cgi.escape(build.title)
         archive = self._composeArchiveReference(build.archive)
-        return '<a href="%s">%s%s</a>%s' % (url, icon, title, archive)
+        return '<a href="%s">%s</a>%s' % (url, title, archive)
 
 
 class CodeImportMachineFormatterAPI(CustomizableFormatter):

=== modified file 'lib/lp/soyuz/templates/builder-index.pt'
--- lib/lp/soyuz/templates/builder-index.pt	2010-03-16 11:07:39 +0000
+++ lib/lp/soyuz/templates/builder-index.pt	2010-03-30 08:05:49 +0000
@@ -106,6 +106,7 @@
 
     <tal:job-header condition="job">
       <span class="sortkey" tal:content="job/id" />
+      <img src="/@@/processing" alt="[building]" />
       <tal:specific-job replace="structure job/specific_job/@@+current" />
     </tal:job-header>
 

=== modified file 'lib/lp/soyuz/templates/buildfarmbranchjob-current.pt'
--- lib/lp/soyuz/templates/buildfarmbranchjob-current.pt	2010-03-16 05:08:47 +0000
+++ lib/lp/soyuz/templates/buildfarmbranchjob-current.pt	2010-03-30 08:05:49 +0000
@@ -4,7 +4,6 @@
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
   omit-tag="">
 
-  <img src="/@@/processing" alt="[building]" />
   Working on
   <tal:jobtype replace="context/__class__/__name__" />
   for branch <tal:branch replace="structure context/branch/fmt:link" />.

=== modified file 'lib/lp/soyuz/templates/buildfarmjob-current.pt'
--- lib/lp/soyuz/templates/buildfarmjob-current.pt	2010-03-16 05:08:47 +0000
+++ lib/lp/soyuz/templates/buildfarmjob-current.pt	2010-03-30 08:05:49 +0000
@@ -4,7 +4,6 @@
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
   omit-tag="">
 
-  <img src="/@@/processing" alt="[building]" />
   Working on
   <tal:jobtype replace="context/__class__/__name__" />.
 </tal:root>