Launchpad itself

Merge lp:~adeuring/launchpad/bug-615763 into lp:launchpad/db-devel

  1. bug-615763
  2. Merge into db-devel
Proposed by Abel Deuring
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 9784
Proposed branch: lp:~adeuring/launchpad/bug-615763
Merge into: lp:launchpad/db-devel
Diff against target: 188 lines (+119/-21)
3 files modified
lib/canonical/launchpad/zcml/librarian.zcml (+1/-1)
lib/lp/bugs/browser/bugattachment.py (+11/-20)
lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py (+107/-0)
To merge this branch: bzr merge lp:~adeuring/launchpad/bug-615763
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+35394@code.launchpad.net

Description of the change

This branch makes changing the content type of a bug attachment a bit simpler and more efficient.

The content type is stored as an attribute of the LibraryFileAlias associtaed with the bugattachment. If a user changed the content type of a bug attachment, the old implementation of BugAttachmentEditView.change_action() downloaded the attachment's file content, created a new LibraryFileAlias instance having this content and finally associated the bug attachment with this new LFA.

This was necessary because LFA attributes are read-only, but since a few weeks we have a class LibraryFileAliasWithParent which allows to change the "restricted" flag. This branch makes another attribute of LFAWithParent, "mimetype", editable too. BugAttachmentEditView.change_action() now adapts the LFA to an LFAWithParent and changes the attribute "mimetype" there.

I also added a few unit tests for BugAttachmentEditView:

./bin/test -vvt test_bugattachment_edit_view

no lint

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/zcml/librarian.zcml'
2--- lib/canonical/launchpad/zcml/librarian.zcml 2010-09-06 15:14:17 +0000
3+++ lib/canonical/launchpad/zcml/librarian.zcml 2010-09-14 12:26:12 +0000
4@@ -16,7 +16,7 @@
5 <allow interface="canonical.launchpad.interfaces.ILibraryFileAliasWithParent" />
6 <require
7 permission="launchpad.Edit"
8- set_attributes="restricted" />
9+ set_attributes="mimetype restricted" />
10 </class>
11
12 <class class="canonical.launchpad.database.LibraryFileContent">
13
14=== modified file 'lib/lp/bugs/browser/bugattachment.py'
15--- lib/lp/bugs/browser/bugattachment.py 2010-09-06 07:44:21 +0000
16+++ lib/lp/bugs/browser/bugattachment.py 2010-09-14 12:26:12 +0000
17@@ -12,19 +12,21 @@
18 'BugAttachmentURL',
19 ]
20
21-from cStringIO import StringIO
22-
23-from zope.component import getUtility
24+from zope.component import (
25+ getMultiAdapter,
26+ getUtility,
27+ )
28 from zope.contenttype import guess_content_type
29 from zope.interface import implements
30
31 from canonical.launchpad.browser.librarian import (
32 FileNavigationMixin,
33 ProxiedLibraryFileAlias,
34- StreamOrRedirectLibraryFileAliasView,
35 SafeStreamOrRedirectLibraryFileAliasView,
36 )
37-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
38+from canonical.launchpad.interfaces.librarian import (
39+ ILibraryFileAliasWithParent,
40+ )
41 from canonical.launchpad.webapp import (
42 canonical_url,
43 custom_widget,
44@@ -163,7 +165,10 @@
45 self.context.title = data['title']
46
47 if self.context.libraryfile.mimetype != data['contenttype']:
48- self.updateContentType(data['contenttype'])
49+ lfa_with_parent = getMultiAdapter(
50+ (self.context.libraryfile, self.context),
51+ ILibraryFileAliasWithParent)
52+ lfa_with_parent.mimetype = data['contenttype']
53
54 @action('Delete Attachment', name='delete')
55 def delete_action(self, action, data):
56@@ -174,20 +179,6 @@
57 url=libraryfile_url, name=self.context.title))
58 self.context.removeFromBug(user=self.user)
59
60- def updateContentType(self, new_content_type):
61- """Update the attachment content type."""
62- filealiasset = getUtility(ILibraryFileAliasSet)
63- old_filealias = self.context.libraryfile
64- # Download the file and upload it again with the new content
65- # type.
66- # XXX: Bjorn Tillenius 2005-06-30:
67- # It should be possible to simply create a new filealias
68- # with the same content as the old one.
69- old_content = old_filealias.read()
70- self.context.libraryfile = filealiasset.create(
71- name=old_filealias.filename, size=len(old_content),
72- file=StringIO(old_content), contentType=new_content_type)
73-
74 @property
75 def label(self):
76 return smartquote('Bug #%d - Edit attachment "%s"') % (
77
78=== added file 'lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py'
79--- lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 1970-01-01 00:00:00 +0000
80+++ lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 2010-09-14 12:26:12 +0000
81@@ -0,0 +1,107 @@
82+# Copyright 2010 Canonical Ltd. This software is licensed under the
83+# GNU Affero General Public License version 3 (see the file LICENSE).
84+
85+__metaclass__ = type
86+
87+import transaction
88+from zope.security.interfaces import Unauthorized
89+
90+from canonical.testing import LaunchpadFunctionalLayer
91+from lp.testing import (
92+ login_person,
93+ TestCaseWithFactory,
94+ )
95+from lp.testing.views import create_initialized_view
96+
97+
98+class TestBugAttachmentEditView(TestCaseWithFactory):
99+ """Tests of traversal to and access of files of bug attachments."""
100+
101+ layer = LaunchpadFunctionalLayer
102+
103+ CHANGE_FORM_DATA = {
104+ 'field.title': 'new description',
105+ 'field.patch': 'on',
106+ 'field.contenttype': 'application/whatever',
107+ 'field.actions.change': 'Change',
108+ }
109+
110+ def setUp(self):
111+ super(TestBugAttachmentEditView, self).setUp()
112+ self.bug_owner = self.factory.makePerson()
113+ login_person(self.bug_owner)
114+ self.bug = self.factory.makeBug(owner=self.bug_owner)
115+ self.bugattachment = self.factory.makeBugAttachment(
116+ bug=self.bug, filename='foo.diff', data='file content',
117+ description='attachment description', content_type='text/plain',
118+ is_patch=False)
119+ # The Librarian server should know about the new file before
120+ # we start the tests.
121+ transaction.commit()
122+
123+ def test_change_action_public_bug(self):
124+ # Properties of attachments for public bugs can be
125+ # changed by every user.
126+ user = self.factory.makePerson()
127+ login_person(user)
128+ create_initialized_view(
129+ self.bugattachment, name='+edit', form=self.CHANGE_FORM_DATA)
130+ self.assertEqual('new description', self.bugattachment.title)
131+ self.assertTrue(self.bugattachment.is_patch)
132+ self.assertEqual(
133+ 'application/whatever', self.bugattachment.libraryfile.mimetype)
134+
135+ def test_change_action_private_bug(self):
136+ # Subscribers of a private bug can edit attachments.
137+ user = self.factory.makePerson()
138+ self.bug.subscribe(user, self.bug_owner)
139+ self.bug.setPrivate(True, self.bug_owner)
140+ transaction.commit()
141+ login_person(user)
142+ create_initialized_view(
143+ self.bugattachment, name='+edit', form=self.CHANGE_FORM_DATA)
144+ self.assertEqual('new description', self.bugattachment.title)
145+ self.assertTrue(self.bugattachment.is_patch)
146+ self.assertEqual(
147+ 'application/whatever', self.bugattachment.libraryfile.mimetype)
148+
149+ def test_change_action_private_bug_unauthorized(self):
150+ # Other users cannot edit attachments of private bugs.
151+ user = self.factory.makePerson()
152+ self.bug.setPrivate(True, self.bug_owner)
153+ transaction.commit()
154+ login_person(user)
155+ self.assertRaises(
156+ Unauthorized, create_initialized_view, self.bugattachment,
157+ name='+edit', form=self.CHANGE_FORM_DATA)
158+
159+ DELETE_FORM_DATA = {
160+ 'field.actions.delete': 'Delete Attachment',
161+ }
162+
163+ def test_delete_action_public_bug(self):
164+ # Bug attachments can be removed from a bug.
165+ user = self.factory.makePerson()
166+ login_person(user)
167+ create_initialized_view(
168+ self.bugattachment, name='+edit', form=self.DELETE_FORM_DATA)
169+ self.assertEqual(0, self.bug.attachments.count())
170+
171+ def test_delete_action_private_bug(self):
172+ # Subscribers of a private bug can delete attachments.
173+ user = self.factory.makePerson()
174+ self.bug.subscribe(user, self.bug_owner)
175+ self.bug.setPrivate(True, self.bug_owner)
176+ login_person(user)
177+ create_initialized_view(
178+ self.bugattachment, name='+edit', form=self.DELETE_FORM_DATA)
179+ self.assertEqual(0, self.bug.attachments.count())
180+
181+ def test_delete_action_private_bug_unautorized(self):
182+ # Other users cannot delete private bug attachments.
183+ user = self.factory.makePerson()
184+ self.bug.setPrivate(True, self.bug_owner)
185+ login_person(user)
186+ self.assertRaises(
187+ Unauthorized, create_initialized_view, self.bugattachment,
188+ name='+edit', form=self.DELETE_FORM_DATA)

Subscribers

People subscribed via source and target branches

to status/vote changes: