launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29279
[Merge] ~andrey-fedoseev/launchpad:bug-attachment-url into launchpad:master
Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad:bug-attachment-url into launchpad:master.
Commit message:
Add `url` to `BugAttachment`
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/430983
`libraryfile` field of `BugAttachment` is not optional, an attachment can either contain a file, or have a URL
Add `displayed_url` property to `BugAttachment` that contains either a proxied attachment file URL, or the URL associated with the attachment.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:bug-attachment-url into launchpad:master.
diff --git a/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css b/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
index e4f4ff3..37487d2 100644
--- a/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
+++ b/lib/canonical/launchpad/icing/css/components/sidebar_portlets.css
@@ -132,3 +132,9 @@
padding-right: 0.25em;
}
+/******************************************************************************
+ Attachments portlet
+*/
+.side .portlet .download-attachment {
+ word-break: break-word;
+ }
diff --git a/lib/lp/bugs/adapters/bugchange.py b/lib/lp/bugs/adapters/bugchange.py
index c7babdc..e97d6e8 100644
--- a/lib/lp/bugs/adapters/bugchange.py
+++ b/lib/lp/bugs/adapters/bugchange.py
@@ -63,7 +63,6 @@ from lp.bugs.interfaces.bugtask import (
IBugTask,
)
from lp.registry.interfaces.product import IProduct
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.webapp.publisher import canonical_url
# These are used lp.bugs.model.bugactivity.BugActivity.attribute to normalize
@@ -698,11 +697,6 @@ class BugTagsChange(AttributeChange):
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."""
@@ -712,13 +706,13 @@ class BugAttachmentChange(AttributeChange):
old_value = None
new_value = "%s %s" % (
self.new_value.title,
- download_url_of_bugattachment(self.new_value),
+ self.new_value.displayed_url,
)
else:
what_changed = ATTACHMENT_REMOVED
old_value = "%s %s" % (
self.old_value.title,
- download_url_of_bugattachment(self.old_value),
+ self.old_value.displayed_url,
)
new_value = None
@@ -737,7 +731,7 @@ class BugAttachmentChange(AttributeChange):
message = '** %s added: "%s"\n %s' % (
attachment_str,
self.new_value.title,
- download_url_of_bugattachment(self.new_value),
+ self.new_value.displayed_url,
)
else:
if self.old_value.is_patch:
@@ -747,7 +741,7 @@ class BugAttachmentChange(AttributeChange):
message = '** %s removed: "%s"\n %s' % (
attachment_str,
self.old_value.title,
- download_url_of_bugattachment(self.old_value),
+ self.old_value.displayed_url,
)
return {"text": message}
diff --git a/lib/lp/bugs/browser/bug.py b/lib/lp/bugs/browser/bug.py
index 3a675a8..717f39c 100644
--- a/lib/lp/bugs/browser/bug.py
+++ b/lib/lp/bugs/browser/bug.py
@@ -85,7 +85,6 @@ from lp.registry.interfaces.person import IPersonSet
from lp.services.compat import message_as_bytes
from lp.services.features import getFeatureFlag
from lp.services.fields import DuplicateBug
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.mail.mailwrapper import MailWrapper
from lp.services.propertycache import cachedproperty
from lp.services.searchbuilder import any, not_equals
@@ -534,17 +533,11 @@ class BugViewMixin:
"other": [],
}
for attachment in self.context.attachments_unpopulated:
- info = {
- "attachment": attachment,
- "file": ProxiedLibraryFileAlias(
- attachment.libraryfile, attachment
- ),
- }
if attachment.type == BugAttachmentType.PATCH:
key = attachment.type
else:
key = "other"
- result[key].append(info)
+ result[key].append(attachment)
return result
@property
@@ -636,12 +629,6 @@ class BugView(LaunchpadView, BugViewMixin):
return dupes
- def proxiedUrlForLibraryFile(self, attachment):
- """Return the proxied download URL for a Librarian file."""
- return ProxiedLibraryFileAlias(
- attachment.libraryfile, attachment
- ).http_url
-
class BugActivity(BugView):
@@ -1304,13 +1291,16 @@ class BugTextView(LaunchpadView):
def attachment_text(self, attachment):
"""Return a text representation of a bug attachment."""
- mime_type = normalize_mime_type.sub(
- " ", attachment.libraryfile.mimetype
- )
- download_url = ProxiedLibraryFileAlias(
- attachment.libraryfile, attachment
- ).http_url
- return "%s %s" % (download_url, mime_type)
+ if attachment.url:
+ if attachment.title != attachment.url:
+ return "{}: {}".format(attachment.title, attachment.url)
+ return attachment.url
+ elif attachment.libraryfile:
+ mime_type = normalize_mime_type.sub(
+ " ", attachment.libraryfile.mimetype
+ )
+ return "{} {}".format(attachment.displayed_url, mime_type)
+ raise AssertionError()
def comment_text(self):
"""Return a text representation of bug comments."""
diff --git a/lib/lp/bugs/browser/bugattachment.py b/lib/lp/bugs/browser/bugattachment.py
index 7d45e45..5b0aa05 100644
--- a/lib/lp/bugs/browser/bugattachment.py
+++ b/lib/lp/bugs/browser/bugattachment.py
@@ -25,10 +25,7 @@ from lp.bugs.interfaces.bugattachment import (
IBugAttachmentIsPatchConfirmationForm,
IBugAttachmentSet,
)
-from lp.services.librarian.browser import (
- FileNavigationMixin,
- ProxiedLibraryFileAlias,
-)
+from lp.services.librarian.browser import FileNavigationMixin
from lp.services.librarian.interfaces import ILibraryFileAliasWithParent
from lp.services.webapp import GetitemNavigation, Navigation, canonical_url
from lp.services.webapp.authorization import check_permission
@@ -115,15 +112,19 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
self.next_url = self.cancel_url = canonical_url(
ICanonicalUrlData(context).inside
)
+ if not context.libraryfile:
+ self.field_names = ["title", "patch"]
@property
def initial_values(self):
attachment = self.context
- return dict(
+ values = dict(
title=attachment.title,
patch=attachment.type == BugAttachmentType.PATCH,
- contenttype=attachment.libraryfile.mimetype,
)
+ if attachment.libraryfile:
+ values["contenttype"] = attachment.libraryfile.mimetype
+ return values
def canEditAttachment(self, action):
return check_permission("launchpad.Edit", self.context)
@@ -135,30 +136,38 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
else:
new_type = BugAttachmentType.UNSPECIFIED
if new_type != self.context.type:
- filename = self.context.libraryfile.filename
- file_content = self.context.libraryfile.read()
- # We expect that users set data['patch'] to True only for
- # real patch data, indicated by guessed_content_type ==
- # 'text/x-diff'. If there are inconsistencies, we don't
- # set the value automatically. Instead, we lead the user to
- # another form where we ask them if they are sure about their
- # choice of the patch flag.
- new_type_consistent_with_guessed_type = (
- self.attachmentTypeConsistentWithContentType(
- new_type == BugAttachmentType.PATCH, filename, file_content
+ if self.context.libraryfile:
+ filename = self.context.libraryfile.filename
+ file_content = self.context.libraryfile.read()
+ # We expect that users set data['patch'] to True only for
+ # real patch data, indicated by guessed_content_type ==
+ # 'text/x-diff'. If there are inconsistencies, we don't
+ # set the value automatically. Instead, we lead the user to
+ # another form where we ask them if they are sure about their
+ # choice of the patch flag.
+ new_type_consistent_with_guessed_type = (
+ self.attachmentTypeConsistentWithContentType(
+ new_type == BugAttachmentType.PATCH,
+ filename,
+ file_content,
+ )
)
- )
- if new_type_consistent_with_guessed_type:
- self.context.type = new_type
+ if new_type_consistent_with_guessed_type:
+ self.context.type = new_type
+ else:
+ self.next_url = self.nextUrlForInconsistentPatchFlags(
+ self.context
+ )
else:
- self.next_url = self.nextUrlForInconsistentPatchFlags(
- self.context
- )
+ self.context.type = new_type
if data["title"] != self.context.title:
self.context.title = data["title"]
- if self.context.libraryfile.mimetype != data["contenttype"]:
+ if (
+ self.context.libraryfile
+ and self.context.libraryfile.mimetype != data["contenttype"]
+ ):
lfa_with_parent = getMultiAdapter(
(self.context.libraryfile, self.context),
ILibraryFileAliasWithParent,
@@ -167,14 +176,11 @@ class BugAttachmentEditView(LaunchpadFormView, BugAttachmentContentCheck):
@action("Delete Attachment", name="delete", condition=canEditAttachment)
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=libraryfile_url,
+ url=self.context.displayed_url,
name=self.context.title,
)
)
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 6a32ff1..0f653f6 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -29,7 +29,6 @@ from lp.bugs.interfaces.bugmessage import IBugComment
from lp.services.comments.browser.comment import download_body
from lp.services.comments.browser.messagecomment import MessageComment
from lp.services.config import config
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.messages.interfaces.message import IMessage
from lp.services.propertycache import cachedproperty, get_property_cache
from lp.services.webapp import (
@@ -378,12 +377,6 @@ class BugCommentBoxViewMixin:
else:
return False
- def proxiedUrlOfLibraryFileAlias(self, attachment):
- """Return the proxied URL for the Librarian file of the attachment."""
- return ProxiedLibraryFileAlias(
- attachment.libraryfile, attachment
- ).http_url
-
class BugCommentBoxView(LaunchpadView, BugCommentBoxViewMixin):
"""Render a comment box with reply field collapsed."""
diff --git a/lib/lp/bugs/browser/bugmessage.py b/lib/lp/bugs/browser/bugmessage.py
index b0267a1..30cff9e 100644
--- a/lib/lp/bugs/browser/bugmessage.py
+++ b/lib/lp/bugs/browser/bugmessage.py
@@ -10,6 +10,7 @@ __all__ = [
from io import BytesIO
from zope.component import getUtility
+from zope.formlib.textwidgets import TextWidget
from zope.formlib.widget import CustomWidgetFactory
from zope.formlib.widgets import TextAreaWidget
@@ -29,6 +30,9 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
custom_widget_comment = CustomWidgetFactory(
TextAreaWidget, cssClass="comment-text"
)
+ custom_widget_attachment_url = CustomWidgetFactory(
+ TextWidget, displayWidth=44, displayMaxWidth=250
+ )
page_title = "Add a comment or attachment"
@@ -49,14 +53,19 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
def validate(self, data):
- # Ensure either a comment or filecontent was provide, but only
- # if no errors have already been noted.
+ # Ensure either a comment or filecontent or a URL was provided,
+ # but only if no errors have already been noted.
if len(self.errors) == 0:
comment = data.get("comment") or ""
filecontent = data.get("filecontent", None)
- if not comment.strip() and not filecontent:
+ attachment_url = data.get("attachment_url") or ""
+ if (
+ not comment.strip()
+ and not filecontent
+ and not attachment_url.strip()
+ ):
self.addError(
- "Either a comment or attachment " "must be provided."
+ "Either a comment or attachment must be provided."
)
@action("Post Comment", name="save")
@@ -73,8 +82,10 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
# Write proper FileUpload field and widget instead of this hack.
file_ = self.request.form.get(self.widgets["filecontent"].name)
+ attachment_url = data.get("attachment_url")
+
message = None
- if data["comment"] or file_:
+ if data["comment"] or file_ or attachment_url:
bugwatch_id = data.get("bugwatch_id")
if bugwatch_id is not None:
bugwatch = getUtility(IBugWatchSet).get(bugwatch_id)
@@ -87,7 +98,7 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
bugwatch=bugwatch,
)
- # A blank comment with only a subect line is always added
+ # A blank comment with only a subject line is always added
# when the user attaches a file, so show the add comment
# feedback message only when the user actually added a
# comment.
@@ -97,6 +108,9 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
)
self.next_url = canonical_url(self.context)
+
+ attachment_description = data.get("attachment_description")
+
if file_:
# Slashes in filenames cause problems, convert them to dashes
@@ -104,11 +118,8 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
filename = file_.filename.replace("/", "-")
# if no description was given use the converted filename
- file_description = None
- if "attachment_description" in data:
- file_description = data["attachment_description"]
- if not file_description:
- file_description = filename
+ if not attachment_description:
+ attachment_description = filename
# Process the attachment.
# If the patch flag is not consistent with the result of
@@ -132,7 +143,8 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
owner=self.user,
data=BytesIO(data["filecontent"]),
filename=filename,
- description=file_description,
+ url=None,
+ description=attachment_description,
comment=message,
is_patch=is_patch,
)
@@ -146,6 +158,21 @@ class BugMessageAddFormView(LaunchpadFormView, BugAttachmentContentCheck):
"Attachment %s added to bug." % filename
)
+ elif attachment_url:
+ is_patch = data["patch"]
+ bug.addAttachment(
+ owner=self.user,
+ data=None,
+ filename=None,
+ url=attachment_url,
+ description=attachment_description,
+ comment=message,
+ is_patch=is_patch,
+ )
+ self.request.response.addNotification(
+ "Attachment %s added to bug." % attachment_url
+ )
+
def shouldShowEmailMeWidget(self):
"""Should the subscribe checkbox be shown?"""
return not self.context.bug.isSubscribed(self.user)
diff --git a/lib/lp/bugs/browser/bugtarget.py b/lib/lp/bugs/browser/bugtarget.py
index e7b3dc6..43e084f 100644
--- a/lib/lp/bugs/browser/bugtarget.py
+++ b/lib/lp/bugs/browser/bugtarget.py
@@ -117,7 +117,6 @@ from lp.registry.vocabularies import ValidPersonOrTeamVocabulary
from lp.services.config import config
from lp.services.features import getFeatureFlag
from lp.services.job.interfaces.job import JobStatus
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.propertycache import cachedproperty
from lp.services.webapp import LaunchpadView, canonical_url, urlappend
from lp.services.webapp.authorization import check_permission
@@ -673,6 +672,7 @@ class FileBugViewBase(LaunchpadFormView):
owner=self.user,
data=BytesIO(data["filecontent"]),
filename=filename,
+ url=None,
description=file_description,
comment=attachment_comment,
is_patch=data["patch"],
@@ -686,6 +686,7 @@ class FileBugViewBase(LaunchpadFormView):
bug.linkAttachment(
owner=self.user,
file_alias=attachment["file_alias"],
+ url=None,
description=attachment["description"],
comment=attachment_comment,
send_notifications=False,
@@ -1472,10 +1473,6 @@ class BugsPatchesView(LaunchpadView):
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
-
class TargetSubscriptionView(LaunchpadView):
"""A view to show all a person's structural subscriptions to a target."""
diff --git a/lib/lp/bugs/browser/tests/bug-views.rst b/lib/lp/bugs/browser/tests/bug-views.rst
index 74a9fd7..008659a 100644
--- a/lib/lp/bugs/browser/tests/bug-views.rst
+++ b/lib/lp/bugs/browser/tests/bug-views.rst
@@ -334,22 +334,22 @@ librarian file of the attachment.
... )
>>> view = BugView(bug_seven, request)
>>> for attachment in view.regular_attachments:
- ... print(attachment["attachment"].title)
+ ... print(attachment.title)
...
attachment 1
attachment 2
>>> for patch in view.patches:
- ... print(patch["attachment"].title)
+ ... print(patch.title)
...
patch 1
patch 2
>>> for attachment in view.regular_attachments:
- ... print(attachment["file"].http_url)
+ ... print(attachment.displayed_url)
...
http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/a1
http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/a2
>>> for patch in view.patches:
- ... print(patch["file"].http_url)
+ ... print(patch.displayed_url)
...
http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/p1
http://bugs.launchpad.test/firefox/+bug/5/+attachment/.../+files/p2
diff --git a/lib/lp/bugs/browser/tests/test_bug_views.py b/lib/lp/bugs/browser/tests/test_bug_views.py
index 2264af4..787b257 100644
--- a/lib/lp/bugs/browser/tests/test_bug_views.py
+++ b/lib/lp/bugs/browser/tests/test_bug_views.py
@@ -779,6 +779,25 @@ class TestBugMessageAddFormView(TestCaseWithFactory):
)
self.assertEqual(0, len(view.errors))
+ def test_add_attachment_with_url(self):
+ bug = self.factory.makeBug()
+ form = {
+ "field.comment": " ",
+ "field.actions.save": "Post Comment",
+ "field.attachment_url": "https://launchpad.net";,
+ "field.attachment_description": "description",
+ "field.patch.used": "",
+ }
+ login_person(self.factory.makePerson())
+ view = create_initialized_view(
+ bug.default_bugtask, "+addcomment", form=form
+ )
+ self.assertEqual([], view.errors)
+ attachments = list(bug.attachments)
+ self.assertEqual(1, len(attachments))
+ self.assertEqual("description", attachments[0].title)
+ self.assertEqual("https://launchpad.net";, attachments[0].url)
+
class TestBugMarkAsDuplicateView(TestCaseWithFactory):
"""Tests for marking a bug as a duplicate."""
diff --git a/lib/lp/bugs/browser/tests/test_bugview.py b/lib/lp/bugs/browser/tests/test_bugview.py
index 04d2e84..1124544 100644
--- a/lib/lp/bugs/browser/tests/test_bugview.py
+++ b/lib/lp/bugs/browser/tests/test_bugview.py
@@ -35,10 +35,7 @@ class TestBugView(TestCaseWithFactory):
removeSecurityProxy(attachment.libraryfile).content = None
self.assertEqual(
["regular attachment"],
- [
- attachment["attachment"].title
- for attachment in self.view.regular_attachments
- ],
+ [attachment.title for attachment in self.view.regular_attachments],
)
def test_patches_dont_include_invalid_records(self):
@@ -55,10 +52,7 @@ class TestBugView(TestCaseWithFactory):
removeSecurityProxy(patch.libraryfile).content = None
self.assertEqual(
["patch"],
- [
- attachment["attachment"].title
- for attachment in self.view.patches
- ],
+ [attachment.title for attachment in self.view.patches],
)
diff --git a/lib/lp/bugs/doc/bug-export.rst b/lib/lp/bugs/doc/bug-export.rst
index 3f466dd..0be1dd9 100644
--- a/lib/lp/bugs/doc/bug-export.rst
+++ b/lib/lp/bugs/doc/bug-export.rst
@@ -146,6 +146,7 @@ the file when we later serialise the bug:
... io.BytesIO(b"Hello World"),
... "Added attachment",
... "hello.txt",
+ ... url=None,
... description='"Hello World" attachment',
... )
<lp.bugs.model.bugattachment.BugAttachment ...>
@@ -166,8 +167,8 @@ attachment contents encoded using base-64:
<text>Added attachment</text>
<attachment href="http://bugs.launchpad.test/bugs/4/.../+files/hello.txt";>
<type>UNSPECIFIED</type>
- <filename>hello.txt</filename>
<title>"Hello World" attachment</title>
+ <filename>hello.txt</filename>
<mimetype>text/plain</mimetype>
<contents>SGVsbG8gV29ybGQ=
</contents>
diff --git a/lib/lp/bugs/doc/bug.rst b/lib/lp/bugs/doc/bug.rst
index e28eff2..848028f 100644
--- a/lib/lp/bugs/doc/bug.rst
+++ b/lib/lp/bugs/doc/bug.rst
@@ -742,6 +742,7 @@ Adding an attachment.
>>> attachment = attachmentset.create(
... bug=firefox_bug,
... filealias=filealias,
+ ... url=None,
... title="Some info.",
... message=message,
... )
diff --git a/lib/lp/bugs/doc/bugattachments.rst b/lib/lp/bugs/doc/bugattachments.rst
index d2f99c8..58e16f1 100644
--- a/lib/lp/bugs/doc/bugattachments.rst
+++ b/lib/lp/bugs/doc/bugattachments.rst
@@ -53,6 +53,7 @@ ObjectCreatedEvent in order to trigger email notifications:
... owner=foobar,
... data=data,
... filename="foo.bar",
+ ... url=None,
... description="this fixes the bug",
... comment=message,
... is_patch=False,
@@ -77,6 +78,7 @@ passed in is often a file-like object, but can be bytes too.
... owner=foobar,
... data=data,
... filename="foo.baz",
+ ... url=None,
... description="this fixes the bug",
... comment="a string comment",
... is_patch=False,
@@ -92,6 +94,7 @@ If no description is given, the title is set to the filename.
>>> screenshot = bug_four.addAttachment(
... owner=foobar,
... data=data,
+ ... url=None,
... filename="screenshot.jpg",
... comment="a string comment",
... is_patch=False,
@@ -110,6 +113,7 @@ The content type is guessed based on the information provided.
... owner=foobar,
... data=data,
... filename="something.debdiff",
+ ... url=None,
... comment="something debdiffish",
... is_patch=False,
... )
@@ -503,6 +507,7 @@ It's also possible to delete attachments.
... owner=foobar,
... data=data,
... filename="foo.baz",
+ ... url=None,
... description="Attachment to be deleted",
... comment="a string comment",
... is_patch=False,
@@ -552,6 +557,7 @@ property returning True.
... owner=foobar,
... data=BytesIO(filecontent.getvalue()),
... filename="foo.baz",
+ ... url=None,
... description="A non-patch attachment",
... comment="a string comment",
... is_patch=False,
@@ -564,6 +570,7 @@ property returning True.
... owner=foobar,
... data=BytesIO(filecontent.getvalue()),
... filename="foo.baz",
+ ... url=None,
... description="A patch attachment",
... comment="a string comment",
... is_patch=True,
@@ -601,7 +608,10 @@ LibraryFileAlias.restricted and Bug.private. See also the section
>>> bug = factory.makeBug()
>>> bug.linkAttachment(
- ... owner=bug.owner, file_alias=file_alias, comment="Some attachment"
+ ... owner=bug.owner,
+ ... file_alias=file_alias,
+ ... url=None,
+ ... comment="Some attachment",
... )
<lp.bugs.model.bugattachment.BugAttachment ...>
@@ -631,6 +641,7 @@ meaningful description.
>>> bug.linkAttachment(
... owner=bug.owner,
... file_alias=file_alias,
+ ... url=None,
... comment="Some attachment",
... is_patch=True,
... description="An attachment of some sort",
@@ -688,6 +699,7 @@ its Librarian file is set.
... owner=private_bug_owner,
... data=b"secret",
... filename="baz.txt",
+ ... url=None,
... comment="Some attachment",
... )
>>> private_attachment.libraryfile.restricted
diff --git a/lib/lp/bugs/doc/bugcomment.rst b/lib/lp/bugs/doc/bugcomment.rst
index 14a8b71..5157e55 100644
--- a/lib/lp/bugs/doc/bugcomment.rst
+++ b/lib/lp/bugs/doc/bugcomment.rst
@@ -77,6 +77,7 @@ in activity_and_comments...
... data=BytesIO(b"whatever"),
... comment=bug_11.initial_message,
... filename="test.txt",
+ ... url=None,
... is_patch=False,
... content_type="text/plain",
... description="sample data",
@@ -235,6 +236,7 @@ attachments to a bug to see this in action:
... data=file_,
... description="Ho",
... filename="munchy",
+ ... url=None,
... comment="Hello there",
... )
>>> m6 = bug_three.newMessage(
@@ -436,6 +438,7 @@ not included in BugComment.patches.
... data=BytesIO(b"whatever"),
... comment=bug.initial_message,
... filename="file1",
+ ... url=None,
... is_patch=False,
... content_type="text/plain",
... description="sample data 1",
@@ -445,6 +448,7 @@ not included in BugComment.patches.
... data=BytesIO(b"whatever"),
... comment=bug.initial_message,
... filename="file2",
+ ... url=None,
... is_patch=False,
... content_type="text/plain",
... description="sample data 2",
@@ -454,6 +458,7 @@ not included in BugComment.patches.
... data=BytesIO(b"whatever"),
... comment=bug.initial_message,
... filename="patch1",
+ ... url=None,
... is_patch=True,
... content_type="text/plain",
... description="patch 1",
@@ -463,6 +468,7 @@ not included in BugComment.patches.
... data=BytesIO(b"whatever"),
... comment=bug.initial_message,
... filename="patch2",
+ ... url=None,
... is_patch=True,
... content_type="text/plain",
... description="patch 2",
diff --git a/lib/lp/bugs/interfaces/bug.py b/lib/lp/bugs/interfaces/bug.py
index 9697660..e02f2ff 100644
--- a/lib/lp/bugs/interfaces/bug.py
+++ b/lib/lp/bugs/interfaces/bug.py
@@ -43,6 +43,7 @@ from lazr.restful.interface import copy_field
from zope.component import getUtility
from zope.interface import Attribute, Interface
from zope.schema import (
+ URI,
Bool,
Bytes,
Choice,
@@ -877,11 +878,12 @@ class IBugAppend(Interface):
@call_with(owner=REQUEST_USER, from_api=True)
@operation_parameters(
- data=Bytes(constraint=attachment_size_constraint),
+ data=Bytes(constraint=attachment_size_constraint, required=False),
comment=Text(),
- filename=TextLine(),
+ filename=TextLine(required=False),
+ url=URI(required=False),
is_patch=Bool(),
- content_type=TextLine(),
+ content_type=TextLine(required=False),
description=Text(),
)
@export_factory_operation(IBugAttachment, [])
@@ -891,6 +893,7 @@ class IBugAppend(Interface):
data,
comment,
filename,
+ url,
is_patch=False,
content_type=None,
description=None,
@@ -903,6 +906,7 @@ class IBugAppend(Interface):
:description: A brief description of the attachment.
:comment: An IMessage or string.
:filename: A string.
+ :url: External URL of the attachment
:is_patch: A boolean.
"""
@@ -956,12 +960,13 @@ class IBugAppend(Interface):
"""
def linkAttachment(
- owner, file_alias, comment, is_patch=False, description=None
+ owner, file_alias, url, comment, is_patch=False, description=None
):
"""Link an `ILibraryFileAlias` to this bug.
:owner: An IPerson.
:file_alias: The `ILibraryFileAlias` to link to this bug.
+ :url: External URL of the attachment.
:description: A brief description of the attachment.
:comment: An IMessage or string.
:is_patch: A boolean.
diff --git a/lib/lp/bugs/interfaces/bugattachment.py b/lib/lp/bugs/interfaces/bugattachment.py
index 0ded472..b4b7599 100644
--- a/lib/lp/bugs/interfaces/bugattachment.py
+++ b/lib/lp/bugs/interfaces/bugattachment.py
@@ -22,7 +22,7 @@ from lazr.restful.declarations import (
)
from lazr.restful.fields import Reference
from zope.interface import Interface
-from zope.schema import Bool, Bytes, Choice, Int, TextLine
+from zope.schema import URI, Bool, Bytes, Choice, Int, TextLine
from lp import _
from lp.bugs.interfaces.hasbug import IHasBug
@@ -90,10 +90,13 @@ class IBugAttachmentView(IHasBug):
)
libraryfile = exported(
Bytes(
- title=_("The attachment content."), required=True, readonly=True
+ title=_("The attachment content."), required=False, readonly=True
),
exported_as="data",
)
+ url = exported(
+ URI(title=_("Attachment URL"), required=False, readonly=True)
+ )
_message_id = Int(title=_("Message ID"))
message = exported(
Reference(
@@ -109,6 +112,12 @@ class IBugAttachmentView(IHasBug):
description=_("Is this attachment a patch?"),
readonly=True,
)
+ displayed_url = URI(
+ title=_(
+ "Download URL of the files or the external URL of the attachment"
+ ),
+ readonly=True,
+ )
def getFileByName(filename):
"""Return the `ILibraryFileAlias` for the given file name.
@@ -176,6 +185,7 @@ class IBugAttachmentSet(Interface):
def create(
bug,
filealias,
+ url,
title,
message,
type=IBugAttachment["type"].default,
@@ -184,7 +194,8 @@ class IBugAttachmentSet(Interface):
"""Create a new attachment and return it.
:param bug: The `IBug` to which the new attachment belongs.
- :param filealias: The `IFilealias` containing the data.
+ :param filealias: The `IFilealias` containing the data (optional).
+ :param url: External URL of the attachment (optional).
:param message: The `IMessage` to which this attachment belongs.
:param type: The type of attachment. See `BugAttachmentType`.
:param send_notifications: If True, a notification is sent to
diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
index 7768b07..22c8d39 100644
--- a/lib/lp/bugs/interfaces/bugmessage.py
+++ b/lib/lp/bugs/interfaces/bugmessage.py
@@ -11,7 +11,7 @@ __all__ = [
]
from zope.interface import Attribute, Interface
-from zope.schema import Bool, Bytes, Int, Object, Text, TextLine
+from zope.schema import URI, Bool, Bytes, Int, Object, Text, TextLine
from lp.app.validators.attachment import attachment_size_constraint
from lp.bugs.interfaces.bug import IBug
@@ -103,6 +103,7 @@ class IBugMessageAddForm(Interface):
required=False,
constraint=attachment_size_constraint,
)
+ attachment_url = URI(title="URL", required=False)
patch = Bool(
title="This attachment contains a solution (patch) for this bug",
required=False,
diff --git a/lib/lp/bugs/mail/handler.py b/lib/lp/bugs/mail/handler.py
index 975af40..775b570 100644
--- a/lib/lp/bugs/mail/handler.py
+++ b/lib/lp/bugs/mail/handler.py
@@ -410,6 +410,7 @@ class MaloneHandler:
getUtility(IBugAttachmentSet).create(
bug=bug,
filealias=blob,
+ url=None,
attach_type=attach_type,
title=blob.filename,
message=message,
diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index 5b1465c..8e99ff0 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -1608,6 +1608,7 @@ class Bug(SQLBase, InformationTypeMixin):
data,
comment,
filename,
+ url,
is_patch=False,
content_type=None,
description=None,
@@ -1619,35 +1620,45 @@ class Bug(SQLBase, InformationTypeMixin):
# wrongly encoded.
if from_api:
data = get_raw_form_value_from_current_request(data, "data")
- if isinstance(data, bytes):
- filecontent = data
- else:
- filecontent = data.read()
- if is_patch:
- content_type = "text/plain"
- else:
- if content_type is None:
- content_type, encoding = guess_content_type(
- name=filename, body=filecontent
- )
+ if data:
+ if isinstance(data, bytes):
+ filecontent = data
+ else:
+ filecontent = data.read()
- filealias = getUtility(ILibraryFileAliasSet).create(
- name=filename,
- size=len(filecontent),
- file=BytesIO(filecontent),
- contentType=content_type,
- restricted=self.private,
- )
+ if is_patch:
+ content_type = "text/plain"
+ else:
+ if content_type is None:
+ content_type, encoding = guess_content_type(
+ name=filename, body=filecontent
+ )
+
+ file_alias = getUtility(ILibraryFileAliasSet).create(
+ name=filename,
+ size=len(filecontent),
+ file=BytesIO(filecontent),
+ contentType=content_type,
+ restricted=self.private,
+ )
+ else:
+ file_alias = None
return self.linkAttachment(
- owner, filealias, comment, is_patch, description
+ owner=owner,
+ file_alias=file_alias,
+ url=url,
+ comment=comment,
+ is_patch=is_patch,
+ description=description,
)
def linkAttachment(
self,
owner,
file_alias,
+ url,
comment,
is_patch=False,
description=None,
@@ -1672,8 +1683,10 @@ class Bug(SQLBase, InformationTypeMixin):
if description:
title = description
- else:
+ elif file_alias:
title = file_alias.filename
+ else:
+ title = url
if IMessage.providedBy(comment):
message = comment
@@ -1685,6 +1698,7 @@ class Bug(SQLBase, InformationTypeMixin):
return getUtility(IBugAttachmentSet).create(
bug=self,
filealias=file_alias,
+ url=url,
attach_type=attach_type,
title=title,
message=message,
@@ -2206,9 +2220,10 @@ class Bug(SQLBase, InformationTypeMixin):
# XXX: This should be a bulk update. RBC 20100827
# bug=https://bugs.launchpad.net/storm/+bug/625071
for attachment in self.attachments_unpopulated:
- attachment.libraryfile.restricted = (
- information_type in PRIVATE_INFORMATION_TYPES
- )
+ if attachment.libraryfile:
+ attachment.libraryfile.restricted = (
+ information_type in PRIVATE_INFORMATION_TYPES
+ )
self.information_type = information_type
self._reconcileAccess()
@@ -2558,16 +2573,34 @@ class Bug(SQLBase, InformationTypeMixin):
def _attachments_query(self):
"""Helper for the attachments* properties."""
- # bug attachments with no LibraryFileContent have been deleted - the
- # garbo_daily run will remove the LibraryFileAlias asynchronously.
- # See bug 542274 for more details.
+ # get bug attachments that have either a URL,
+ # or associated LibraryFileContent
+ # bug attachments with LibraryFileAlias and no LibraryFileContent have
+ # been deleted - the garbo_daily run will remove the LibraryFileAlias
+ # asynchronously. See bug 542274 for more details.
store = Store.of(self)
- return store.find(
- (BugAttachment, LibraryFileAlias, LibraryFileContent),
- BugAttachment.bug == self,
- BugAttachment.libraryfile == LibraryFileAlias.id,
- LibraryFileContent.id == LibraryFileAlias.contentID,
- ).order_by(BugAttachment.id)
+ return (
+ store.using(
+ BugAttachment,
+ LeftJoin(
+ LibraryFileAlias,
+ BugAttachment.libraryfile == LibraryFileAlias.id,
+ ),
+ LeftJoin(
+ LibraryFileContent,
+ LibraryFileContent.id == LibraryFileAlias.contentID,
+ ),
+ )
+ .find(
+ (BugAttachment, LibraryFileAlias, LibraryFileContent),
+ BugAttachment.bug == self,
+ Or(
+ BugAttachment.url != None,
+ LibraryFileAlias.contentID != None,
+ ),
+ )
+ .order_by(BugAttachment.id)
+ )
@property
def attachments(self):
diff --git a/lib/lp/bugs/model/bugattachment.py b/lib/lp/bugs/model/bugattachment.py
index 180c353..0a570d4 100644
--- a/lib/lp/bugs/model/bugattachment.py
+++ b/lib/lp/bugs/model/bugattachment.py
@@ -17,6 +17,7 @@ from lp.bugs.interfaces.bugattachment import (
from lp.services.database.enumcol import DBEnum
from lp.services.database.interfaces import IStore
from lp.services.database.stormbase import StormBase
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.propertycache import cachedproperty
@@ -36,8 +37,9 @@ class BugAttachment(StormBase):
default=IBugAttachment["type"].default,
)
title = Unicode(allow_none=False)
- libraryfile_id = Int(name="libraryfile", allow_none=False)
+ libraryfile_id = Int(name="libraryfile")
libraryfile = Reference(libraryfile_id, "LibraryFileAlias.id")
+ url = Unicode()
_message_id = Int(name="message", allow_none=False)
_message = Reference(_message_id, "Message.id")
@@ -46,6 +48,7 @@ class BugAttachment(StormBase):
bug,
title,
libraryfile,
+ url,
message,
type=IBugAttachment["type"].default,
):
@@ -53,6 +56,7 @@ class BugAttachment(StormBase):
self.bug = bug
self.title = title
self.libraryfile = libraryfile
+ self.url = url
self._message = message
self.type = type
@@ -81,15 +85,23 @@ class BugAttachment(StormBase):
# Delete the reference to the LibraryFileContent record right now,
# in order to avoid problems with not deleted files as described
# in bug 387188.
- self.libraryfile.content = None
+ if self.libraryfile:
+ self.libraryfile.content = None
Store.of(self).remove(self)
def getFileByName(self, filename):
"""See IBugAttachment."""
- if filename == self.libraryfile.filename:
+ if self.libraryfile and filename == self.libraryfile.filename:
return self.libraryfile
raise NotFoundError(filename)
+ @property
+ def displayed_url(self):
+ return (
+ self.url
+ or ProxiedLibraryFileAlias(self.libraryfile, self).http_url
+ )
+
@implementer(IBugAttachmentSet)
class BugAttachmentSet:
@@ -110,18 +122,26 @@ class BugAttachmentSet:
self,
bug,
filealias,
+ url,
title,
message,
attach_type=None,
send_notifications=False,
):
"""See `IBugAttachmentSet`."""
+ if not filealias and not url:
+ raise ValueError("Either filealias or url must be provided")
+
+ if filealias and url:
+ raise ValueError("Only one of filealias or url may be provided")
+
if attach_type is None:
# XXX kiko 2005-08-03 bug=1659: this should use DEFAULT.
attach_type = IBugAttachment["type"].default
attachment = BugAttachment(
bug=bug,
libraryfile=filealias,
+ url=url,
type=attach_type,
title=title,
message=message,
diff --git a/lib/lp/bugs/model/tests/test_bugtasksearch.py b/lib/lp/bugs/model/tests/test_bugtasksearch.py
index db52d71..05793e0 100644
--- a/lib/lp/bugs/model/tests/test_bugtasksearch.py
+++ b/lib/lp/bugs/model/tests/test_bugtasksearch.py
@@ -268,6 +268,7 @@ class OnceTests:
data=b"filedata",
comment="a comment",
filename="file1.txt",
+ url=None,
is_patch=False,
)
self.bugtasks[1].bug.addAttachment(
@@ -275,6 +276,7 @@ class OnceTests:
data=b"filedata",
comment="a comment",
filename="file1.txt",
+ url=None,
is_patch=True,
)
# We can search for bugs with non-patch attachments...
diff --git a/lib/lp/bugs/scripts/bugexport.py b/lib/lp/bugs/scripts/bugexport.py
index 3d3327a..c6c0c43 100644
--- a/lib/lp/bugs/scripts/bugexport.py
+++ b/lib/lp/bugs/scripts/bugexport.py
@@ -16,7 +16,6 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.bugs.browser.bugtask import get_comments_for_bugtask
from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
-from lp.services.librarian.browser import ProxiedLibraryFileAlias
BUGS_XMLNS = "https://launchpad.net/xmlns/2006/bugs";
@@ -85,27 +84,30 @@ def serialise_bugtask(bugtask):
attachment_node = ET.SubElement(
comment_node,
"attachment",
- href=ProxiedLibraryFileAlias(
- attachment.libraryfile, attachment
- ).http_url,
+ href=attachment.displayed_url,
)
attachment_node.text = attachment_node.tail = "\n"
addnode(attachment_node, "type", attachment.type.name)
- addnode(
- attachment_node, "filename", attachment.libraryfile.filename
- )
addnode(attachment_node, "title", attachment.title)
- addnode(
- attachment_node, "mimetype", attachment.libraryfile.mimetype
- )
- # Attach the attachment file contents, base 64 encoded.
- addnode(
- attachment_node,
- "contents",
- base64.encodebytes(attachment.libraryfile.read()).decode(
- "ASCII"
- ),
- )
+ if attachment.libraryfile:
+ addnode(
+ attachment_node,
+ "filename",
+ attachment.libraryfile.filename,
+ )
+ addnode(
+ attachment_node,
+ "mimetype",
+ attachment.libraryfile.mimetype,
+ )
+ # Attach the attachment file contents, base 64 encoded.
+ addnode(
+ attachment_node,
+ "contents",
+ base64.encodebytes(attachment.libraryfile.read()).decode(
+ "ASCII"
+ ),
+ )
return bug_node
diff --git a/lib/lp/bugs/scripts/bugimport.py b/lib/lp/bugs/scripts/bugimport.py
index 323c9b1..38306fb 100644
--- a/lib/lp/bugs/scripts/bugimport.py
+++ b/lib/lp/bugs/scripts/bugimport.py
@@ -445,6 +445,7 @@ class BugImporter:
getUtility(IBugAttachmentSet).create(
bug=bug,
filealias=filealias,
+ url=None,
attach_type=attach_type,
title=title,
message=message,
diff --git a/lib/lp/bugs/scripts/tests/test_bugnotification.py b/lib/lp/bugs/scripts/tests/test_bugnotification.py
index f9aa65d..80fee9c 100644
--- a/lib/lp/bugs/scripts/tests/test_bugnotification.py
+++ b/lib/lp/bugs/scripts/tests/test_bugnotification.py
@@ -1026,7 +1026,7 @@ class TestEmailNotificationsAttachments(
# another five minutes. Therefore, we send out separate
# change notifications.
return self.bug.addAttachment(
- self.person, b"content", "a comment", "stuff.txt"
+ self.person, b"content", "a comment", "stuff.txt", url=None
)
old = cachedproperty("old")(_attachment)
diff --git a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
index 090ca83..9df6720 100644
--- a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
+++ b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.rst
@@ -35,6 +35,7 @@ If we add an attachment to the bug report...
... data=BytesIO(b"whatever"),
... comment=bug_11.initial_message,
... filename="test.txt",
+ ... url=None,
... is_patch=False,
... content_type="text/plain",
... description="sample data",
@@ -74,6 +75,7 @@ Patch attachments are shown before non-patch attachments
... data=BytesIO(b"patchy patch patch"),
... comment=bug_11.initial_message,
... filename="patch.txt",
+ ... url=None,
... is_patch=True,
... content_type="text/plain",
... description="a patch",
diff --git a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
index 93e9cc6..bfa0b6e 100644
--- a/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
+++ b/lib/lp/bugs/stories/bugs/xx-bug-text-pages.rst
@@ -23,6 +23,7 @@ We'll start by adding some attachments to the bug:
... content,
... "comment for file a",
... "file_a.txt",
+ ... url=None,
... content_type="text/html",
... )
>>> content = BytesIO(b"do we need to")
@@ -31,6 +32,7 @@ We'll start by adding some attachments to the bug:
... content,
... "comment for file with space",
... "file with space.txt",
+ ... url=None,
... content_type='text/plain;\n name="file with space.txt"',
... )
>>> content = BytesIO(b"Yes we can!")
@@ -39,6 +41,7 @@ We'll start by adding some attachments to the bug:
... content,
... "comment for patch",
... "bug-patch.diff",
+ ... url=None,
... is_patch=True,
... content_type="text/plain",
... description="a patch",
diff --git a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
index 12f5e92..bed6dba 100644
--- a/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
+++ b/lib/lp/bugs/stories/bugtask-searches/xx-listing-basics.rst
@@ -311,6 +311,7 @@ Patches also appear as badges in bug listings.
... owner=foobar,
... data=BytesIO(b"file data"),
... filename="foo.bar",
+ ... url=None,
... description="this fixes the bug",
... comment=message,
... is_patch=True,
diff --git a/lib/lp/bugs/stories/webservice/xx-bug.rst b/lib/lp/bugs/stories/webservice/xx-bug.rst
index 80c2f24..8266254 100644
--- a/lib/lp/bugs/stories/webservice/xx-bug.rst
+++ b/lib/lp/bugs/stories/webservice/xx-bug.rst
@@ -1531,6 +1531,7 @@ An attachment can be added to the bug:
... "addAttachment",
... data=io.BytesIO(b"12345"),
... filename="numbers.txt",
+ ... url=None,
... content_type="foo/bar",
... comment="The numbers you asked for.",
... )
@@ -1558,6 +1559,7 @@ Now, bug 1 has one attachment:
self_link: 'http://.../bugs/1/+attachment/...'
title: 'numbers.txt'
type: 'Unspecified'
+ url: None
web_link: 'http://bugs.../bugs/1/+attachment/...'
---
@@ -1572,6 +1574,7 @@ The attachment can be fetched directly:
self_link: 'http://.../bugs/1/+attachment/...'
title: 'numbers.txt'
type: 'Unspecified'
+ url: None
web_link: 'http://bugs.../bugs/1/+attachment/...'
Fetching the data actually yields a redirect to the Librarian, which
diff --git a/lib/lp/bugs/templates/bug-attachment-macros.pt b/lib/lp/bugs/templates/bug-attachment-macros.pt
index a06b835..d91d0c3 100644
--- a/lib/lp/bugs/templates/bug-attachment-macros.pt
+++ b/lib/lp/bugs/templates/bug-attachment-macros.pt
@@ -15,6 +15,12 @@
<metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
</tal:filecontent>
+ <tal:url
+ tal:define="widget nocall:view/widgets/attachment_url|nothing"
+ tal:condition="widget">
+ <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
+ </tal:url>
+
<tal:patch
tal:define="widget nocall:view/widgets/patch|nothing"
tal:condition="widget">
diff --git a/lib/lp/bugs/templates/bug-portlet-attachments.pt b/lib/lp/bugs/templates/bug-portlet-attachments.pt
index 5a79a9f..552ede8 100644
--- a/lib/lp/bugs/templates/bug-portlet-attachments.pt
+++ b/lib/lp/bugs/templates/bug-portlet-attachments.pt
@@ -8,13 +8,13 @@
<ul>
<li class="download-attachment"
tal:repeat="attachment view/patches">
- <a tal:attributes="href attachment/file/http_url"
- tal:content="attachment/attachment/title"
+ <a tal:attributes="href attachment/displayed_url"
+ tal:content="attachment/title"
class="sprite haspatch-icon">
Attachment Title
</a>
<small>
- (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
+ (<a tal:attributes="href attachment/fmt:url">edit</a>)
</small>
</li>
</ul>
@@ -31,13 +31,13 @@
<ul>
<li class="download-attachment"
tal:repeat="attachment view/regular_attachments">
- <a tal:attributes="href attachment/file/http_url"
- tal:content="attachment/attachment/title"
+ <a tal:attributes="href attachment/displayed_url"
+ tal:content="attachment/title"
class="sprite download-icon">
Attachment Title
</a>
<small>
- (<a tal:attributes="href attachment/attachment/fmt:url">edit</a>)
+ (<a tal:attributes="href attachment/fmt:url">edit</a>)
</small>
</li>
</ul>
diff --git a/lib/lp/bugs/templates/bugcomment-box.pt b/lib/lp/bugs/templates/bugcomment-box.pt
index b25c482..2163a3d 100644
--- a/lib/lp/bugs/templates/bugcomment-box.pt
+++ b/lib/lp/bugs/templates/bugcomment-box.pt
@@ -113,24 +113,28 @@
<ul tal:condition="comment/bugattachments" style="margin-bottom: 1em">
<li tal:repeat="attachment comment/bugattachments"
class="download-attachment">
- <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
+ <a tal:attributes="href attachment/displayed_url"
tal:content="attachment/title"
class="sprite download-icon">foo.txt</a>
<a tal:attributes="href attachment/fmt:url" class="sprite edit action-icon">Edit</a>
+ <tal:block condition="attachment/libraryfile">
(<span
tal:replace="attachment/libraryfile/content/filesize/fmt:bytes" />,
<span tal:replace="attachment/libraryfile/mimetype" />)
+ </tal:block>
</li>
</ul>
<ul tal:condition="comment/patches" style="margin-bottom: 1em">
<li tal:repeat="attachment comment/patches" class="download-attachment">
- <a tal:attributes="href python: view.proxiedUrlOfLibraryFileAlias(attachment)"
+ <a tal:attributes="href attachment/displayed_url"
tal:content="attachment/title" class="sprite haspatch-icon">foo.txt</a>
<a tal:attributes="href attachment/fmt:url" class="sprite edit action-icon">Edit</a>
+ <tal:block condition="attachment/libraryfile">
(<span
tal:replace="attachment/libraryfile/content/filesize/fmt:bytes" />,
<span tal:replace="attachment/libraryfile/mimetype" />)
+ </tal:block>
</li>
</ul>
diff --git a/lib/lp/bugs/templates/bugtarget-patches.pt b/lib/lp/bugs/templates/bugtarget-patches.pt
index fca68c7..38c8aad 100644
--- a/lib/lp/bugs/templates/bugtarget-patches.pt
+++ b/lib/lp/bugs/templates/bugtarget-patches.pt
@@ -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 python:view.proxiedUrlForLibraryFile(patch)"
+ <a tal:attributes="href patch/displayed_url"
tal:content="age/fmt:approximateduration"></a>
<div class="popupTitle"
tal:attributes="id string:patch-popup-${repeat/patch_task/index};">
@@ -82,8 +82,8 @@
<strong>From:</strong>
<a tal:replace="structure submitter/fmt:link"></a><br/>
<strong>Link:</strong>
- <a tal:attributes="href python:view.proxiedUrlForLibraryFile(patch)"
- tal:content="patch/libraryfile/filename"></a></p>
+ <a tal:attributes="href patch/displayed_url"
+ tal:content="patch/libraryfile/filename | patch/url"></a></p>
<p tal:content="string:${patch/title}"></p>
</div>
<script type="text/javascript" tal:content="string:
diff --git a/lib/lp/bugs/tests/bugs-emailinterface.rst b/lib/lp/bugs/tests/bugs-emailinterface.rst
index 8e765d1..61b1ef7 100644
--- a/lib/lp/bugs/tests/bugs-emailinterface.rst
+++ b/lib/lp/bugs/tests/bugs-emailinterface.rst
@@ -657,7 +657,7 @@ otherwise permission to complete the operation will be denied.)
We will also add an attachment to the bug.
>>> bug_attachment = bug_four.addAttachment(
- ... bug_four.owner, b"Attachment", "No comment", "test.txt"
+ ... bug_four.owner, b"Attachment", "No comment", "test.txt", url=None
... )
>>> submit_commands(bug_four, "private yes")
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index b33159e..dbdedf5 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -1209,13 +1209,15 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
switch_dbuser("testadmin")
bug = self.factory.makeBug()
attachment = self.factory.makeBugAttachment(bug=bug)
+ # Attachments with URLs are never deleted.
+ self.factory.makeBugAttachment(bug=bug, url="https://launchpad.net";)
transaction.commit()
# Bug attachments that have a LibraryFileContent record are
# not deleted.
self.assertIsNot(attachment.libraryfile.content, None)
self.runDaily()
- self.assertEqual(bug.attachments.count(), 1)
+ self.assertEqual(bug.attachments.count(), 2)
# But once we delete the LfC record, the attachment is deleted
# in the next daily garbo run.
@@ -1224,7 +1226,7 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
transaction.commit()
self.runDaily()
switch_dbuser("testadmin")
- self.assertEqual(bug.attachments.count(), 0)
+ self.assertEqual(bug.attachments.count(), 1)
def test_TimeLimitedTokenPruner(self):
# Ensure there are no tokens
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index e70aef7..ce37ff0 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2565,6 +2565,7 @@ class LaunchpadObjectFactory(ObjectFactory):
filename=None,
content_type=None,
description=None,
+ url=None,
is_patch=_DEFAULT,
):
"""Create and return a new bug attachment.
@@ -2581,23 +2582,31 @@ class LaunchpadObjectFactory(ObjectFactory):
string will be used.
:param content_type: The MIME-type of this file.
:param description: The description of the attachment.
+ :param url: External URL of the attachment (a string or None)
:param is_patch: If true, this attachment is a patch.
:return: An `IBugAttachment`.
"""
+ if url:
+ if data or filename:
+ raise ValueError(
+ "Either `url` or `data` / `filename` " "can be provided."
+ )
+ else:
+ if data is None:
+ data = self.getUniqueBytes()
+ if filename is None:
+ filename = self.getUniqueString()
+
if bug is None:
bug = self.makeBug()
elif isinstance(bug, (int, str)):
bug = getUtility(IBugSet).getByNameOrID(str(bug))
if owner is None:
owner = self.makePerson()
- if data is None:
- data = self.getUniqueBytes()
if description is None:
description = self.getUniqueString()
if comment is None:
comment = self.getUniqueString()
- if filename is None:
- filename = self.getUniqueString()
# If the default value of is_patch when creating a new
# BugAttachment should ever change, we don't want to interfere
# with that. So, we only override it if our caller explicitly
@@ -2610,6 +2619,7 @@ class LaunchpadObjectFactory(ObjectFactory):
data,
comment,
filename,
+ url,
content_type=content_type,
description=description,
**other_params,
Follow ups
