Merge lp:~adeuring/launchpad/bug-39674-change-remaining-lfa-http_url into lp:launchpad/db-devel

[Abel Deuring]

This branch changes the URLs of LFAs of bug attachments shown in
some web pages and notification emails from LFA.http_url to
ProxiedLibraryFileAlias.http_url and fixes some related issues.

lib/canonical/launchpad/browser/librarian.py:

The URLs created by ProxiedLibraryFileAlias.http_url were not properly
escaped; non-ASCII-characters in file names caused exceptions, for example
in lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt

I copied the pattern to generate URLs from LFA.http_url resp.
FileDownloadClient.getURLForAlias().

lib/canonical/launchpad/webapp/launchbag.py:

Some tests failed because canonical_url(bug_attachment) wants to access
launchbag.bugtask, but this property is not defined for a freshly created
launchbag.

lib/canonical/librarian/client.py:

The function quote() is now also used in ProxiedLibraryFileAlias.http_url,
so it should appear in __all__.

lib/lp/bugs/adapters/bugchange.py:

The data generated by class BugAttachmentChange when an attachment
is created or deleted now contains URLs from ProxiedLFAs.

lib/lp/bugs/browser/bug.py:

class BugView has a new method proxiedUrlForLibraryFile(). This method
is called from at least one page template. (Sorry, can't remember
which template is it -- this branch contains too many small changes...)

BugTextView.attachment_text() uses proxied LFA URLs.

lib/lp/bugs/browser/bugattachment.py:

BugAttachmentEditView.delete_action shows proxied LFA URLs in a notification.

lib/lp/bugs/browser/bugtarget.py:

A new helper method to get proxied LFA URLs in class BugsPatchesView,
similar to that ini class BugView.

lib/lp/bugs/model/bugattachment.py:

When notifications about a new bug attachment are created,
canonical_url(new_attachment) is called; this function needs the
bug attachment ID, and this ID is not available if the DB record
is not yet created. Hence the Store.of(attachment).flush() call.

The remaining changes are in tests which are affected by the changed
URLs. The test lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt
now looks quite ugly in line 197ff: The download URLs now start with
regular bug URLs, i.e., we can access the LFAs via

http://bugs.launchpad.dev/bugs/1/+attachment/1/+files/file_a.txt

as well as

http://bugs.launchpad.dev/firefox/+bug/1/+attachment/1/+files/file_a.txt

Which URL is created, depends on the existence of a bugtask in LaunchBag.

This is a bit silly for the file content, but I simply did not have enough
time to fix it, and I'd like to really make bug attachments private
for private bugs in the next release...

tests: many. All those which I touched...

[Deryck Hodge (deryck)]

The changes generally look good, Abel. Thanks for all the work you've been doing to get this ready!

I have two minor requests:

1) If there are bugs for these related issues that are fixed, can you link the branch to them so they get marked fixed when this lands?

2) Can you add a comment in code explaining why the flush is required?

Consider this r=me with these minor changes.

Cheers,
deryck

review: Approve (code)

=== modified file 'lib/canonical/launchpad/browser/librarian.py'
--- lib/canonical/launchpad/browser/librarian.py	2010-08-03 13:37:55 +0000
+++ lib/canonical/launchpad/browser/librarian.py	2010-08-04 15:48:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser file for LibraryFileAlias."""
@@ -32,6 +32,7 @@
     IWebBrowserOriginatingRequest)
 from canonical.launchpad.webapp.url import urlappend
 from canonical.lazr.utils import get_current_browser_request
+from canonical.librarian.client import quote
 from canonical.librarian.interfaces import LibrarianServerError
 from canonical.librarian.utils import filechunks, guess_librarian_encoding
 
@@ -220,5 +221,6 @@
 
         parent_url = canonical_url(self.parent, request=request)
         traversal_url = urlappend(parent_url, '+files')
-        url = urlappend(traversal_url, self.context.filename)
+        url = urlappend(
+            traversal_url, quote(self.context.filename.encode('utf-8')))
         return url
 
=== modified file 'lib/canonical/launchpad/webapp/launchbag.py'
--- lib/canonical/launchpad/webapp/launchbag.py	2010-07-30 00:06:00 +0000
+++ lib/canonical/launchpad/webapp/launchbag.py	2010-08-04 15:48:48 +0000
@@ -136,7 +136,7 @@
 
     @property
     def bugtask(self):
-        return self._store.bugtask
+        return getattr(self._store, "bugtask", None)
 
     @property
     def time_zone(self):
 
=== modified file 'lib/canonical/librarian/client.py'
--- lib/canonical/librarian/client.py	2010-03-25 20:03:55 +0000
+++ lib/canonical/librarian/client.py	2010-08-04 15:48:48 +0000
@@ -8,6 +8,7 @@
     'FileUploadClient',
     'LibrarianClient',
     'RestrictedLibrarianClient',
+    'quote',
     ]
 
 
 
=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py	2010-06-09 16:24:23 +0000
+++ lib/lp/bugs/adapters/bugchange.py	2010-08-04 15:48:48 +0000
@@ -38,6 +38,7 @@
 from lp.bugs.interfaces.bugchange import IBugChange
 from lp.bugs.interfaces.bugtask import IBugTask
 from lp.registry.interfaces.product import IProduct
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.webapp.publisher import canonical_url
 
 
@@ -527,6 +528,12 @@
         return {'text': "\n".join(messages)}
 
 
+def download_url_of_bugattachment(attachment):
+    """Return the URL of the ProxiedLibraryFileAlias for the attachment."""
+    return ProxiedLibraryFileAlias(
+        attachment.libraryfile, attachment).http_url
+
+
 class BugAttachmentChange(AttributeChange):
     """Used to represent a change to an `IBug`'s attachments."""
 
@@ -535,12 +542,14 @@
             what_changed = "attachment added"
             old_value = None
             new_value = "%s %s" % (
-                self.new_value.title, self.new_value.libraryfile.http_url)
+                self.new_value.title,
+                download_url_of_bugattachment(self.new_value))
         else:
             what_changed = "attachment removed"
             attachment = self.new_value
             old_value = "%s %s" % (
-                self.old_value.title, self.old_value.libraryfile.http_url)
+                self.old_value.title,
+                download_url_of_bugattachment(self.old_value))
             new_value = None
 
         return {
@@ -557,7 +566,7 @@
                 attachment_str = 'Attachment'
             message = '** %s added: "%s"\n   %s' % (
                 attachment_str, self.new_value.title,
-                self.new_value.libraryfile.http_url)
+                download_url_of_bugattachment(self.new_value))
         else:
             if self.old_value.is_patch:
                 attachment_str = 'Patch'
@@ -565,7 +574,7 @@
                 attachment_str = 'Attachment'
             message = '** %s removed: "%s"\n   %s' % (
                 attachment_str, self.old_value.title,
-                self.old_value.libraryfile.http_url)
+                download_url_of_bugattachment(self.old_value))
 
         return {'text': message}
 
 
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2010-08-03 08:49:19 +0000
+++ lib/lp/bugs/browser/bug.py	2010-08-04 15:48:48 +0000
@@ -557,6 +557,11 @@
 
         return dupes
 
+    def proxiedUrlForLibraryFile(self, attachment):
+        """Return the proxied download URL for a Librarian file."""
+        return ProxiedLibraryFileAlias(
+            attachment.libraryfile, attachment).http_url
+
 
 class BugWithoutContextView:
     """View that redirects to the new bug page.
@@ -870,7 +875,9 @@
         """Return a text representation of a bug attachment."""
         mime_type = normalize_mime_type.sub(
             ' ', attachment.libraryfile.mimetype)
-        return "%s %s" % (attachment.libraryfile.http_url, mime_type)
+        download_url = ProxiedLibraryFileAlias(
+            attachment.libraryfile, attachment).http_url
+        return "%s %s" % (download_url, mime_type)
 
     def comment_text(self):
         """Return a text representation of bug comments."""
 
=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py	2010-08-03 13:37:55 +0000
+++ lib/lp/bugs/browser/bugattachment.py	2010-08-04 15:48:48 +0000
@@ -20,7 +20,7 @@
 from zope.contenttype import guess_content_type
 
 from canonical.launchpad.browser.librarian import (
-    FileNavigationMixin, StreamOrRedirectLibraryFileAliasView)
+    FileNavigationMixin, ProxiedLibraryFileAlias, StreamOrRedirectLibraryFileAliasView)
 from canonical.launchpad.webapp import (
     canonical_url, custom_widget, GetitemNavigation, Navigation)
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
@@ -156,9 +156,11 @@
 
     @action('Delete Attachment', name='delete')
     def delete_action(self, action, data):
+        libraryfile_url = ProxiedLibraryFileAlias(
+            self.context.libraryfile, self.context).http_url
         self.request.response.addInfoNotification(structured(
             'Attachment "<a href="%(url)s">%(name)s</a>" has been deleted.',
-            url=self.context.libraryfile.http_url, name=self.context.title))
+            url=libraryfile_url, name=self.context.title))
         self.context.removeFromBug(user=self.user)
 
     def updateContentType(self, new_content_type):
 
=== modified file 'lib/lp/bugs/browser/bugtarget.py'
--- lib/lp/bugs/browser/bugtarget.py	2010-08-02 02:13:52 +0000
+++ lib/lp/bugs/browser/bugtarget.py	2010-08-04 15:48:48 +0000
@@ -57,6 +57,7 @@
 from canonical.launchpad import _
 from canonical.launchpad.browser.feeds import (
     BugFeedLink, BugTargetLatestBugsFeedLink, FeedsMixin)
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
 from lp.bugs.interfaces.bugtarget import (
     IBugTarget, IOfficialBugTagTargetPublic, IOfficialBugTagTargetRestricted)
@@ -1469,3 +1470,7 @@
         """Return a timedelta object for the age of a patch attachment."""
         now = datetime.now(timezone('UTC'))
         return now - patch.message.datecreated
+
+    def proxiedUrlForLibraryFile(self, patch):
+        """Return the proxied download URL for a Librarian file."""
+        return ProxiedLibraryFileAlias(patch.libraryfile, patch).http_url
 
=== modified file 'lib/lp/bugs/doc/bug-change.txt'
--- lib/lp/bugs/doc/bug-change.txt	2010-06-16 08:27:19 +0000
+++ lib/lp/bugs/doc/bug-change.txt	2010-08-04 15:48:48 +0000
@@ -479,13 +479,14 @@
     ...     old_value=None, new_value=attachment)
 
     >>> print pretty(attachment_change.getBugActivity())
-    {'newvalue': u'sample-attachment http://...',
+    {'newvalue':
+         u'sample-attachment http://bugs.launchpad.dev/bugs/...+files/...',
      'oldvalue': None,
      'whatchanged': 'attachment added'}
 
     >>> print attachment_change.getBugNotification()['text']
     ** Attachment added: "sample-attachment"
-       http...
+    http://bugs.launchpad.dev/bugs/.../+attachment/1/+files/...
 
 Or remove one.
 
@@ -496,12 +497,13 @@
 
     >>> print pretty(attachment_change.getBugActivity())
     {'newvalue': None,
-     'oldvalue': u'sample-attachment http://...',
+     'oldvalue':
+         u'sample-attachment http://bugs.launchpad.dev/bugs/...+files/...',
      'whatchanged': 'attachment removed'}
 
     >>> print attachment_change.getBugNotification()['text']
     ** Attachment removed: "sample-attachment"
-       http...
+    http://bugs.launchpad.dev/bugs/.../+attachment/1/+files/...
 
 
 == BugTaskAttributeChange ==
 
=== modified file 'lib/lp/bugs/doc/bug-export.txt'
--- lib/lp/bugs/doc/bug-export.txt	2009-09-04 10:43:39 +0000
+++ lib/lp/bugs/doc/bug-export.txt	2010-08-04 15:48:48 +0000
@@ -141,7 +141,7 @@
   <sender name="name12">Sample Person</sender>
   <date>...</date>
   <text>Added attachment</text>
-  <attachment href="http://localhost:58000/.../hello.txt">
+  <attachment href="http://bugs.launchpad.dev/bugs/1/.../+files/hello.txt">
   <type>UNSPECIFIED</type>
   <filename>hello.txt</filename>
   <title>"Hello World" attachment</title>
 
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt	2010-07-28 22:33:59 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt	2010-08-04 15:48:48 +0000
@@ -343,18 +343,9 @@
 
 Adding an attachment will generate a notification that looks as follows:
 
-    >>> class MockLibraryFile:
-    ...     def __init__(self, url):
-    ...         self.http_url = url
-    >>> class MockAttachment:
-    ...     def __init__(self, title, libraryfile, is_patch=False):
-    ...         self.title = title
-    ...         self.libraryfile = libraryfile
-    ...         self.is_patch = is_patch
-    >>> attachment = MockAttachment(
-    ...     title="A screenshot of the problem",
-    ...     libraryfile=MockLibraryFile('http://foo.com/screenshot.png'))
-
+    >>> attachment = factory.makeBugAttachment(
+    ...     description="A screenshot of the problem",
+    ...     filename='screenshot.png')
     >>> bug_delta = BugDelta(
     ...     bug=edited_bug,
     ...     bugurl="http://www.example.com/bugs/2",
@@ -365,7 +356,7 @@
     ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
     ...     print "-----------------------------"
     ** Attachment added: "A screenshot of the problem"
-       http://foo.com/screenshot.png
+       http://bugs.launchpad.dev/bugs/.../+attachment/1/+files/screenshot.png
     -----------------------------
 
 Removing an attachment generates a notification, too.
@@ -380,16 +371,15 @@
     ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
     ...     print "-----------------------------"
     ** Attachment removed: "A screenshot of the problem"
-       http://foo.com/screenshot.png
+       http://bugs.launchpad.dev/bugs/.../+attachment/1/+files/screenshot.png
     -----------------------------
 
 Adding an attachment and marking it as a patch generates a different
 notification.
 
-    >>> attachment = MockAttachment(
-    ...     title="A new icon for the application",
-    ...     libraryfile=MockLibraryFile('http://foo.com/new-icon.png'),
-    ...     is_patch=True)
+    >>> attachment = factory.makeBugAttachment(
+    ...     description="A new icon for the application",
+    ...     filename='new-icon.png', is_patch=True)
     >>> bug_delta = BugDelta(
     ...     bug=edited_bug,
     ...     bugurl="http://www.example.com/bugs/2",
@@ -400,7 +390,7 @@
     ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
     ...     print "-----------------------------"
     ** Patch added: "A new icon for the application"
-       http://foo.com/new-icon.png
+       http://bugs.launchpad.dev/bugs/.../+attachment/2/+files/new-icon.png
     -----------------------------
 
 Removing a patch also generates a different notification.
@@ -415,7 +405,7 @@
     ...     print notification['text'] #doctest: -NORMALIZE_WHITESPACE
     ...     print "-----------------------------"
     ** Patch removed: "A new icon for the application"
-       http://foo.com/new-icon.png
+       http://bugs.launchpad.dev/bugs/.../+attachment/2/+files/new-icon.png
     -----------------------------
 
 
 
=== modified file 'lib/lp/bugs/model/bugattachment.py'
--- lib/lp/bugs/model/bugattachment.py	2010-08-03 08:49:19 +0000
+++ lib/lp/bugs/model/bugattachment.py	2010-08-04 15:48:48 +0000
@@ -12,6 +12,7 @@
 from lazr.lifecycle.event import ObjectCreatedEvent, ObjectDeletedEvent
 
 from sqlobject import ForeignKey, StringCol, SQLObjectNotFound
+from storm.store import Store
 
 from canonical.database.enumcol import EnumCol
 from canonical.database.sqlbase import SQLBase
@@ -92,6 +93,10 @@
         attachment = BugAttachment(
             bug=bug, libraryfile=filealias, type=attach_type, title=title,
             message=message)
+        # canonial_url(attachment) (called by notification subscribers
+        # to generate the download URL of the attachments) blows up if
+        # attachment.id is not (yet) set.
+        Store.of(attachment).flush()
         if send_notifications:
             notify(ObjectCreatedEvent(attachment, user=message.owner))
         return attachment
 
=== modified file 'lib/lp/bugs/scripts/bugexport.py'
--- lib/lp/bugs/scripts/bugexport.py	2009-09-04 10:43:39 +0000
+++ lib/lp/bugs/scripts/bugexport.py	2010-08-04 15:48:48 +0000
@@ -15,6 +15,7 @@
 
 from zope.component import getUtility
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from lp.bugs.interfaces.bugtask import BugTaskSearchParams, IBugTaskSet
 from lp.bugs.browser.bugtask import get_comments_for_bugtask
 
@@ -80,7 +81,8 @@
         for attachment in comment.bugattachments:
             attachment_node = ET.SubElement(
                 comment_node, 'attachment',
-                href=attachment.libraryfile.http_url)
+                href=ProxiedLibraryFileAlias(
+                    attachment.libraryfile, attachment).http_url)
             attachment_node.text = attachment_node.tail = '\n'
             addnode(attachment_node, 'type', attachment.type.name)
             addnode(attachment_node, 'filename',
 
=== modified file 'lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt'
--- lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt	2010-07-29 11:20:47 +0000
+++ lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt	2010-08-04 15:48:48 +0000
@@ -23,8 +23,11 @@
     >>> from zope.component import getUtility
     >>> from canonical.launchpad.ftests import login, logout, syncUpdate
     >>> from canonical.launchpad.interfaces import IBugSet
+    >>> from canonical.launchpad.webapp.interfaces import ILaunchBag
     >>> login("[e-mail]")
-    >>> bug_11 =  getUtility(IBugSet).get(11)
+    >>> bug_11 = getUtility(IBugSet).get(11)
+    >>> launchbag = getUtility(ILaunchBag)
+    >>> launchbag.clear()
     >>> attachment = bug_11.addAttachment(
     ...     owner=None, data=StringIO.StringIO('whatever'),
     ...     comment=bug_11.initial_message, filename='test.txt',
@@ -55,6 +58,7 @@
 == Patch attachments are shown before non-patch attachments ==
 
     >>> login("[e-mail]")
+    >>> launchbag.clear()
     >>> patch_attachment = bug_11.addAttachment(
     ...     owner=None, data=StringIO.StringIO('patchy patch patch'),
     ...     comment=bug_11.initial_message, filename='patch.txt',
 
=== modified file 'lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt'
--- lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt	2010-07-29 11:20:47 +0000
+++ lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt	2010-08-04 15:48:48 +0000
@@ -51,7 +51,9 @@
     'http://.../+bug/2'
     >>> for message in find_tags_by_class(user_browser.contents, 'message'):
     ...     print message.renderContents()
-    Attachment "<a href="...">Great deal</a>" has been deleted.
+    Attachment
+    "<a href="http://bugs.launchpad.dev/...+files/foo.txt">Great deal</a>"
+    has been deleted.
 
     >>> print find_portlet(user_browser.contents, 'Bug attachments')
     None
 
=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt	2010-06-25 13:34:37 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt	2010-08-04 15:48:48 +0000
@@ -70,8 +70,9 @@
     duplicate-of: 
     duplicates: 
     attachments:
-        http://.../file_a.txt text/html
-        http://.../file%20with%20space.txt text/plain; name="file with space.txt"
+       http://bugs.launchpad.dev/.../+files/file_a.txt text/html
+       http://bugs.launchpad.dev/.../+files/file%20with%20space.txt
+         text/plain; name="file with space.txt"
     patches:
         http://.../bug-patch.diff text/plain
     tags: 
@@ -149,7 +150,7 @@
     >>> attachments_text = text_bug[text_bug.find('attachments:'):]
     >>> attachment_2 = attachments_text.split('\n')[2]
     >>> attachment_2
-    ' http://localhost:58000/.../file%20with%20space.txt text/plain;
+    ' http://bugs.launchpad.dev/.../file%20with%20space.txt text/plain;
     name="file with space.txt"'
 
 
@@ -196,11 +197,29 @@
     >>> separator_bug = separator_regex.findall(text_bug)[0]
     >>> separator_bug_task = separator_regex.findall(text_bug_task)[0]
 
-Now we can show that the individual sections are identical for each report:
+Now we can show that the individual sections are identical for each report.
+The only differences are the download URLs of bug attachments:
 
-    >>> text_bug.split(separator_bug) == text_bug_task.split(separator_bug_task)
+    >>> text_bug_chunks = text_bug.split(separator_bug)
+    >>> text_bug_task_chunks = text_bug_task.split(separator_bug_task)
+    >>> len(text_bug_chunks) == len(text_bug_task_chunks)
     True
 
+    >>> for chunk_no in range(len(text_bug_task_chunks)):
+    ...     if text_bug_task_chunks[chunk_no] != text_bug_chunks[chunk_no]:
+    ...         bug_task_lines = text_bug_task_chunks[chunk_no].split('\n')
+    ...         bug_lines = text_bug_chunks[chunk_no].split('\n')
+    ...         assert(len(bug_task_lines) == len(bug_lines))
+    ...         for line_no in range(len(bug_task_lines)):
+    ...             if bug_lines[line_no] != bug_task_lines[line_no]:
+    ...                 print bug_lines[line_no]
+    ...                 print bug_task_lines[line_no]
+    http://bugs.launchpad.dev/bugs/1/+attachment/1/+files/file_a.txt text/html
+    http://bugs.launchpad.dev/firefox/+bug/.../+files/file_a.txt text/html
+    http://bugs.launchpad.dev/bugs/1/.../+files/file%20with%20space.txt...
+    http://bugs.launchpad.dev/firefox/+bug/.../+files/file%20with%20space.txt...
+    http://bugs.launchpad.dev/bugs/1/.../+files/bug-patch.diff text/plain
+    http://bugs.launchpad.dev/firefox/+bug/.../+files/bug-patch.diff text/plain
 
 == Duplicate Bugs ==
 
 
=== modified file 'lib/lp/bugs/templates/bugtarget-patches.pt'
--- lib/lp/bugs/templates/bugtarget-patches.pt	2010-02-24 06:17:33 +0000
+++ lib/lp/bugs/templates/bugtarget-patches.pt	2010-08-04 15:48:48 +0000
@@ -74,7 +74,7 @@
                  tal:define="patch patch_task/bug/latest_patch;
                              age python:view.patchAge(patch)"
                  tal:attributes="id string:patch-cell-${repeat/patch_task/index}">
-                   <a tal:attributes="href patch/libraryfile/http_url"
+                   <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
                       tal:content="age/fmt:approximateduration/use-digits"></a>
                <div class="popupTitle"
                     tal:attributes="id string:patch-popup-${repeat/patch_task/index};">
@@ -82,7 +82,7 @@
                     <strong>From:</strong>
                     <a tal:replace="structure submitter/fmt:link"></a><br/>
                     <strong>Link:</strong>
-                    <a tal:attributes="href patch/libraryfile/http_url"
+                    <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
                        tal:content="patch/libraryfile/filename"></a></p>
                  <p tal:content="string:${patch/title}"></p>
                </div>
 
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py	2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py	2010-08-04 15:48:48 +0000
@@ -12,6 +12,7 @@
 from lazr.lifecycle.event import ObjectCreatedEvent, ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
 
+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
 from canonical.launchpad.database import BugNotification
 from canonical.launchpad.ftests import login
 from lp.bugs.interfaces.bug import IBug
@@ -686,13 +687,17 @@
             'whatchanged': 'attachment added',
             'oldvalue': None,
             'newvalue': '%s %s' % (
-                attachment.title, attachment.libraryfile.http_url),
+                attachment.title,
+                ProxiedLibraryFileAlias(
+                    attachment.libraryfile, attachment).http_url),
             }
 
         attachment_added_notification = {
             'person': self.user,
             'text': '** Attachment added: "%s"\n   %s' % (
-                attachment.title, attachment.libraryfile.http_url),
+                attachment.title,
+                ProxiedLibraryFileAlias(
+                    attachment.libraryfile, attachment).http_url),
             }
 
         self.assertRecordedChange(
@@ -705,7 +710,9 @@
         attachment = self.factory.makeBugAttachment(
             bug=self.bug, owner=self.user)
         self.saveOldChanges()
-        download_url = attachment.libraryfile.http_url
+        download_url = ProxiedLibraryFileAlias(
+            attachment.libraryfile, attachment).http_url
+
         attachment.removeFromBug(user=self.user)
 
         attachment_removed_activity = {