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
=== modified file 'lib/canonical/launchpad/zcml/librarian.zcml'
--- lib/canonical/launchpad/zcml/librarian.zcml 2010-09-06 15:14:17 +0000
+++ lib/canonical/launchpad/zcml/librarian.zcml 2010-09-14 12:26:12 +0000
@@ -16,7 +16,7 @@
16 <allow interface="canonical.launchpad.interfaces.ILibraryFileAliasWithParent" />16 <allow interface="canonical.launchpad.interfaces.ILibraryFileAliasWithParent" />
17 <require17 <require
18 permission="launchpad.Edit"18 permission="launchpad.Edit"
19 set_attributes="restricted" />19 set_attributes="mimetype restricted" />
20 </class>20 </class>
2121
22 <class class="canonical.launchpad.database.LibraryFileContent">22 <class class="canonical.launchpad.database.LibraryFileContent">
2323
=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py 2010-09-06 07:44:21 +0000
+++ lib/lp/bugs/browser/bugattachment.py 2010-09-14 12:26:12 +0000
@@ -12,19 +12,21 @@
12 'BugAttachmentURL',12 'BugAttachmentURL',
13 ]13 ]
1414
15from cStringIO import StringIO15from zope.component import (
1616 getMultiAdapter,
17from zope.component import getUtility17 getUtility,
18 )
18from zope.contenttype import guess_content_type19from zope.contenttype import guess_content_type
19from zope.interface import implements20from zope.interface import implements
2021
21from canonical.launchpad.browser.librarian import (22from canonical.launchpad.browser.librarian import (
22 FileNavigationMixin,23 FileNavigationMixin,
23 ProxiedLibraryFileAlias,24 ProxiedLibraryFileAlias,
24 StreamOrRedirectLibraryFileAliasView,
25 SafeStreamOrRedirectLibraryFileAliasView,25 SafeStreamOrRedirectLibraryFileAliasView,
26 )26 )
27from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet27from canonical.launchpad.interfaces.librarian import (
28 ILibraryFileAliasWithParent,
29 )
28from canonical.launchpad.webapp import (30from canonical.launchpad.webapp import (
29 canonical_url,31 canonical_url,
30 custom_widget,32 custom_widget,
@@ -163,7 +165,10 @@
163 self.context.title = data['title']165 self.context.title = data['title']
164166
165 if self.context.libraryfile.mimetype != data['contenttype']:167 if self.context.libraryfile.mimetype != data['contenttype']:
166 self.updateContentType(data['contenttype'])168 lfa_with_parent = getMultiAdapter(
169 (self.context.libraryfile, self.context),
170 ILibraryFileAliasWithParent)
171 lfa_with_parent.mimetype = data['contenttype']
167172
168 @action('Delete Attachment', name='delete')173 @action('Delete Attachment', name='delete')
169 def delete_action(self, action, data):174 def delete_action(self, action, data):
@@ -174,20 +179,6 @@
174 url=libraryfile_url, name=self.context.title))179 url=libraryfile_url, name=self.context.title))
175 self.context.removeFromBug(user=self.user)180 self.context.removeFromBug(user=self.user)
176181
177 def updateContentType(self, new_content_type):
178 """Update the attachment content type."""
179 filealiasset = getUtility(ILibraryFileAliasSet)
180 old_filealias = self.context.libraryfile
181 # Download the file and upload it again with the new content
182 # type.
183 # XXX: Bjorn Tillenius 2005-06-30:
184 # It should be possible to simply create a new filealias
185 # with the same content as the old one.
186 old_content = old_filealias.read()
187 self.context.libraryfile = filealiasset.create(
188 name=old_filealias.filename, size=len(old_content),
189 file=StringIO(old_content), contentType=new_content_type)
190
191 @property182 @property
192 def label(self):183 def label(self):
193 return smartquote('Bug #%d - Edit attachment "%s"') % (184 return smartquote('Bug #%d - Edit attachment "%s"') % (
194185
=== added file 'lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py'
--- lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugattachment_edit_view.py 2010-09-14 12:26:12 +0000
@@ -0,0 +1,107 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4__metaclass__ = type
5
6import transaction
7from zope.security.interfaces import Unauthorized
8
9from canonical.testing import LaunchpadFunctionalLayer
10from lp.testing import (
11 login_person,
12 TestCaseWithFactory,
13 )
14from lp.testing.views import create_initialized_view
15
16
17class TestBugAttachmentEditView(TestCaseWithFactory):
18 """Tests of traversal to and access of files of bug attachments."""
19
20 layer = LaunchpadFunctionalLayer
21
22 CHANGE_FORM_DATA = {
23 'field.title': 'new description',
24 'field.patch': 'on',
25 'field.contenttype': 'application/whatever',
26 'field.actions.change': 'Change',
27 }
28
29 def setUp(self):
30 super(TestBugAttachmentEditView, self).setUp()
31 self.bug_owner = self.factory.makePerson()
32 login_person(self.bug_owner)
33 self.bug = self.factory.makeBug(owner=self.bug_owner)
34 self.bugattachment = self.factory.makeBugAttachment(
35 bug=self.bug, filename='foo.diff', data='file content',
36 description='attachment description', content_type='text/plain',
37 is_patch=False)
38 # The Librarian server should know about the new file before
39 # we start the tests.
40 transaction.commit()
41
42 def test_change_action_public_bug(self):
43 # Properties of attachments for public bugs can be
44 # changed by every user.
45 user = self.factory.makePerson()
46 login_person(user)
47 create_initialized_view(
48 self.bugattachment, name='+edit', form=self.CHANGE_FORM_DATA)
49 self.assertEqual('new description', self.bugattachment.title)
50 self.assertTrue(self.bugattachment.is_patch)
51 self.assertEqual(
52 'application/whatever', self.bugattachment.libraryfile.mimetype)
53
54 def test_change_action_private_bug(self):
55 # Subscribers of a private bug can edit attachments.
56 user = self.factory.makePerson()
57 self.bug.subscribe(user, self.bug_owner)
58 self.bug.setPrivate(True, self.bug_owner)
59 transaction.commit()
60 login_person(user)
61 create_initialized_view(
62 self.bugattachment, name='+edit', form=self.CHANGE_FORM_DATA)
63 self.assertEqual('new description', self.bugattachment.title)
64 self.assertTrue(self.bugattachment.is_patch)
65 self.assertEqual(
66 'application/whatever', self.bugattachment.libraryfile.mimetype)
67
68 def test_change_action_private_bug_unauthorized(self):
69 # Other users cannot edit attachments of private bugs.
70 user = self.factory.makePerson()
71 self.bug.setPrivate(True, self.bug_owner)
72 transaction.commit()
73 login_person(user)
74 self.assertRaises(
75 Unauthorized, create_initialized_view, self.bugattachment,
76 name='+edit', form=self.CHANGE_FORM_DATA)
77
78 DELETE_FORM_DATA = {
79 'field.actions.delete': 'Delete Attachment',
80 }
81
82 def test_delete_action_public_bug(self):
83 # Bug attachments can be removed from a bug.
84 user = self.factory.makePerson()
85 login_person(user)
86 create_initialized_view(
87 self.bugattachment, name='+edit', form=self.DELETE_FORM_DATA)
88 self.assertEqual(0, self.bug.attachments.count())
89
90 def test_delete_action_private_bug(self):
91 # Subscribers of a private bug can delete attachments.
92 user = self.factory.makePerson()
93 self.bug.subscribe(user, self.bug_owner)
94 self.bug.setPrivate(True, self.bug_owner)
95 login_person(user)
96 create_initialized_view(
97 self.bugattachment, name='+edit', form=self.DELETE_FORM_DATA)
98 self.assertEqual(0, self.bug.attachments.count())
99
100 def test_delete_action_private_bug_unautorized(self):
101 # Other users cannot delete private bug attachments.
102 user = self.factory.makePerson()
103 self.bug.setPrivate(True, self.bug_owner)
104 login_person(user)
105 self.assertRaises(
106 Unauthorized, create_initialized_view, self.bugattachment,
107 name='+edit', form=self.DELETE_FORM_DATA)

Subscribers

People subscribed via source and target branches

to status/vote changes: