launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-upload-file-links into lp:launchpad.

Commit message:
Make DistroSeries:+queue link to upload files via the webapp, to help dget users.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1663334 in Launchpad itself: "make queue files dget'able"
  https://bugs.launchpad.net/launchpad/+bug/1663334

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-upload-file-links/+merge/323708
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-upload-file-links into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py	2016-05-26 14:53:06 +0000
+++ lib/lp/soyuz/browser/queue.py	2017-05-06 09:36:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for package queue."""
@@ -27,7 +27,10 @@
     load_related,
     )
 from lp.services.job.model.job import Job
-from lp.services.librarian.browser import FileNavigationMixin
+from lp.services.librarian.browser import (
+    FileNavigationMixin,
+    ProxiedLibraryFileAlias,
+    )
 from lp.services.librarian.model import (
     LibraryFileAlias,
     LibraryFileContent,
@@ -588,7 +591,7 @@
         else:
             return structured(
                 '<a href="%s" title="Changes file for %s">%s</a>',
-                self.changesfile.http_url, self.displayname,
+                self.proxiedFile(self.changesfile).http_url, self.displayname,
                 self.displayname)
 
     @property
@@ -601,3 +604,7 @@
         return structured(
             """<div id="%s"> %s %s (%s)</div>""",
             iconlist_id, icon_string, link, self.displayarchs).escapedtext
+
+    def proxiedFile(self, libraryfile):
+        """Return a librarian file proxied in the context of this upload."""
+        return ProxiedLibraryFileAlias(libraryfile, self.context)

=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py	2015-10-21 09:37:08 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py	2017-05-06 09:36:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for QueueItemsView."""
@@ -17,6 +17,7 @@
 
 from lp.archiveuploader.tests import datadir
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.webapp.escaping import html_escape
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webapp.servers import LaunchpadTestRequest
@@ -588,7 +589,9 @@
         link = html.fromstring(
             html_escape(complete_upload.composeNameAndChangesLink()))
         self.assertEqual(
-            complete_upload.changesfile.http_url, link.get("href"))
+            ProxiedLibraryFileAlias(
+                complete_upload.changesfile, complete_upload.context).http_url,
+            link.get("href"))
 
     def test_composeNameAndChangesLink_escapes_nonlinked_display_name(self):
         filename = 'name"&name'

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt'
--- lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt	2016-01-26 15:47:37 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt	2017-05-06 09:36:26 +0000
@@ -80,11 +80,11 @@
 
     >>> print anon_browser.getLink("netapplet-1.0.0.tar.gz")
     <Link text='netapplet-1.0.0.tar.gz'
-	  url='http://.../1/netapplet-1.0.0.tar.gz'>
+	  url='http://.../+upload/7/+files/netapplet-1.0.0.tar.gz'>
 
     >>> print anon_browser.getLink("alsa-utils")
     <Link text='alsa-utils'
-	  url='http://.../1/netapplet-1.0.0.tar.gz'>
+	  url='http://.../+upload/4/+files/netapplet-1.0.0.tar.gz'>
 
 (This link for alsa-utils is pointing at the librarian URL for
 netapplet, because we have used its changes file for all the
@@ -227,16 +227,17 @@
     alsa-utils_1.0.9a-4ubuntu1.diff.gz
     diff from 1.0.9a-4 to 1.0.9a-4ubuntu1 (11 bytes)
 
-Each unexpired filename links to its respective librarian URL. Expired
-files have no link, so we just get None.
+Each unexpired filename links to its respective proxied librarian URL.
+Expired files have no link, so we just get None.
 
     >>> for row in filelist:
     ...     print row.find('a')
-    <a href="http://.../alsa-utils_1.0.9a-4ubuntu1.dsc";>
+    <a href="http://.../+upload/4/+files/alsa-utils_1.0.9a-4ubuntu1.dsc";>
       alsa-utils_1.0.9a-4ubuntu1.dsc
     </a>
     None
-    <a href="http://.../alsa-utils.diff.gz";>diff from 1.0.9a-4 to 1.0.9a-4ubuntu1</a>
+    <a href="http://.../alsa-utils.diff.gz";>diff from 1.0.9a-4 to
+    1.0.9a-4ubuntu1</a>
 
 On binary queue items we also present the stamp 'NEW' for files never
 published in the archive (it helps archive admins when reviewing

=== modified file 'lib/lp/soyuz/templates/distroseries-queue.pt'
--- lib/lp/soyuz/templates/distroseries-queue.pt	2013-05-30 00:32:12 +0000
+++ lib/lp/soyuz/templates/distroseries-queue.pt	2017-05-06 09:36:26 +0000
@@ -312,18 +312,22 @@
 <metal:macro define-macro="package-file">
   <tal:comment replace="nothing">
     This macro expects the following variables defined:
+    :packageupload: A PackageUpload record for which we display files.
     :libraryfilealias: A LibraryFileAlias to link to. If it is expired,
       no link will be created.
   </tal:comment>
-  <tal:unexpired tal:condition="libraryfilealias/content">
-    <a tal:attributes="href libraryfilealias/http_url">
-      <tal:filename replace="libraryfilealias/filename"/>
-    </a>
-    (<span tal:replace="libraryfilealias/content/filesize/fmt:bytes" />)
-  </tal:unexpired>
-  <tal:expired tal:condition="not:libraryfilealias/content">
-    <span tal:content="libraryfilealias/filename"/>
-  </tal:expired>
+  <tal:proxy
+      define="proxiedfile python:packageupload.proxiedFile(libraryfilealias)">
+    <tal:unexpired condition="proxiedfile/content">
+      <a tal:attributes="href proxiedfile/http_url">
+        <tal:filename replace="proxiedfile/filename"/>
+      </a>
+      (<span tal:replace="proxiedfile/content/filesize/fmt:bytes" />)
+    </tal:unexpired>
+    <tal:expired condition="not:proxiedfile/content">
+      <span tal:content="proxiedfile/filename"/>
+    </tal:expired>
+  </tal:proxy>
 </metal:macro>
 
 </metal:macros>