launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01183
[Merge] lp:~lifeless/launchpad/cp into lp:~launchpad-pqm/launchpad/production-devel
Robert Collins has proposed merging lp:~lifeless/launchpad/cp into lp:~launchpad-pqm/launchpad/production-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
More fixes from stable.
--
https://code.launchpad.net/~lifeless/launchpad/cp/+merge/36406
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/cp into lp:~launchpad-pqm/launchpad/production-devel.
=== modified file '.bzrignore'
--- .bzrignore 2010-09-05 20:29:40 +0000
+++ .bzrignore 2010-09-23 02:21:04 +0000
@@ -78,3 +78,5 @@
lib/canonical/buildd/debian/*
lib/canonical/buildd/launchpad-files/*
*.pt.py
+.project
+.pydevproject
=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
--- lib/canonical/launchpad/browser/launchpad.py 2010-08-24 10:45:57 +0000
+++ lib/canonical/launchpad/browser/launchpad.py 2010-09-23 02:21:04 +0000
@@ -514,19 +514,39 @@
'foo' can be the unique name of the branch, or any of the aliases for
the branch.
+ If 'foo' resolves to an ICanHasLinkedBranch instance but the linked
+ branch is not yet set, redirect back to the referring page with a
+ suitable notification message.
+ If 'foo' is completely invalid, redirect back to the referring page
+ with a suitable error message.
"""
+
+ # The default url to go to will be back to the referring page (in
+ # the case that there is an error resolving the branch url)
+ url = self.request.getHeader('referer')
path = '/'.join(self.request.stepstogo)
try:
- branch_data = getUtility(IBranchLookup).getByLPPath(path)
- except (CannotHaveLinkedBranch, NoLinkedBranch, InvalidNamespace,
- InvalidProductName):
- raise NotFoundError
- branch, trailing = branch_data
- if branch is None:
- raise NotFoundError
- url = canonical_url(branch)
- if trailing is not None:
- url = urlappend(url, trailing)
+ # first check for a valid branch url
+ try:
+ branch_data = getUtility(IBranchLookup).getByLPPath(path)
+ branch, trailing = branch_data
+ url = canonical_url(branch)
+ if trailing is not None:
+ url = urlappend(url, trailing)
+
+ except (NoLinkedBranch):
+ # a valid ICanHasLinkedBranch target exists but there's no
+ # branch or it's not visible
+ self.request.response.addNotification(
+ "The target %s does not have a linked branch." % path)
+
+ except (CannotHaveLinkedBranch, InvalidNamespace,
+ InvalidProductName, NotFoundError) as e:
+ error_msg = str(e)
+ if error_msg == '':
+ error_msg = "Invalid branch lp:%s." % path
+ self.request.response.addErrorNotification(error_msg)
+
return self.redirectSubTree(url)
@stepto('+builds')
=== modified file 'lib/canonical/launchpad/browser/tests/test_launchpad.py'
--- lib/canonical/launchpad/browser/tests/test_launchpad.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/browser/tests/test_launchpad.py 2010-09-23 02:21:04 +0000
@@ -1,7 +1,7 @@
# Copyright 2009 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Tests for traversal from the root branch object.."""
+"""Tests for traversal from the root branch object."""
__metaclass__ = type
@@ -13,17 +13,21 @@
from canonical.launchpad.browser.launchpad import LaunchpadRootNavigation
from canonical.launchpad.interfaces.account import AccountStatus
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.interfaces import BrowserNotificationLevel
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.launchpad.webapp.url import urlappend
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.app.errors import GoneError
+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
from lp.registry.interfaces.person import (
IPersonSet,
PersonVisibility,
)
from lp.testing import (
login_person,
+ person_logged_in,
TestCaseWithFactory,
)
from lp.testing.views import create_view
@@ -31,6 +35,48 @@
class TraversalMixin:
+ def _validateNotificationContext(
+ self, request, notification=None,
+ level=BrowserNotificationLevel.INFO):
+ """Check the browser notifications associated with the request.
+
+ Ensure that the notification instances attached to the request match
+ the expected values for text and type.
+
+ :param notification: The exact notification text to validate. If None
+ then we don't care what the notification text is, so long as there
+ is some.
+ : param level: the required notification level
+ """
+
+ notifications = request.notifications
+ if notification is None:
+ self.assertEquals(len(notifications), 0)
+ return
+ self.assertEqual(len(notifications), 1)
+ self.assertEquals(notifications[0].level, level)
+ self.assertEqual(notification, notifications[0].message)
+
+ def assertDisplaysNotification(
+ self, path, notification=None,
+ level=BrowserNotificationLevel.INFO):
+ """Assert that an invalid path redirects back to referrer.
+
+ The request object is expected to have a notification message to
+ display to the user to explain the reason for the error.
+
+ :param path: The path to check
+ :param notification: The exact notification text to validate. If None
+ then we don't care what the notification text is, so long as there
+ is some.
+ : param level: the required notification level
+ """
+
+ redirection = self.traverse(path)
+ self.assertIs(redirection.target, None)
+ self._validateNotificationContext(
+ redirection.request, notification, level)
+
def assertNotFound(self, path):
self.assertRaises(NotFound, self.traverse, path)
@@ -58,8 +104,8 @@
class TestBranchTraversal(TestCaseWithFactory, TraversalMixin):
"""Branches are traversed to from IPersons. Test we can reach them.
- This class tests the `PersonNavigation` class to see that we can traverse
- to branches from such objects.
+ This class tests the `LaunchpadRootNavigation` class to see that we can
+ traverse to branches from URLs of the form +branch/xxxx.
"""
layer = DatabaseFunctionalLayer
@@ -75,26 +121,107 @@
def test_no_such_unique_name(self):
# Traversing to /+branch/<unique_name> where 'unique_name' is for a
- # branch that doesn't exist will generate a 404.
+ # branch that doesn't exist will display an error message.
branch = self.factory.makeAnyBranch()
- self.assertNotFound(branch.unique_name + 'wibble')
+ bad_name = branch.unique_name + 'wibble'
+ requiredMessage = "No such branch: '%s'." % (branch.name+"wibble")
+ self.assertDisplaysNotification(
+ bad_name, requiredMessage,
+ BrowserNotificationLevel.ERROR)
+
+ def test_private_branch(self):
+ # If an attempt is made to access a private branch, display an error.
+ branch = self.factory.makeProductBranch()
+ branch_unique_name = branch.unique_name
+ naked_product = removeSecurityProxy(branch.product)
+ ICanHasLinkedBranch(naked_product).setBranch(branch)
+ removeSecurityProxy(branch).private = True
+
+ any_user = self.factory.makePerson()
+ login_person(any_user)
+ requiredMessage = "No such branch: '%s'." % branch_unique_name
+ self.assertDisplaysNotification(
+ branch_unique_name,
+ requiredMessage,
+ BrowserNotificationLevel.ERROR)
def test_product_alias(self):
# Traversing to /+branch/<product> redirects to the page for the
# branch that is the development focus branch for that product.
branch = self.factory.makeProductBranch()
- product = removeSecurityProxy(branch.product)
- product.development_focus.branch = branch
- self.assertRedirects(product.name, canonical_url(branch))
+ naked_product = removeSecurityProxy(branch.product)
+ ICanHasLinkedBranch(naked_product).setBranch(branch)
+ self.assertRedirects(naked_product.name, canonical_url(branch))
+
+ def test_private_branch_for_product(self):
+ # If the development focus of a product is private, display a
+ # message telling the user there is no linked branch.
+ branch = self.factory.makeProductBranch()
+ naked_product = removeSecurityProxy(branch.product)
+ ICanHasLinkedBranch(naked_product).setBranch(branch)
+ removeSecurityProxy(branch).private = True
+
+ any_user = self.factory.makePerson()
+ login_person(any_user)
+ requiredMessage = (u"The target %s does not have a linked branch."
+ % naked_product.name)
+ self.assertDisplaysNotification(
+ naked_product.name,
+ requiredMessage,
+ BrowserNotificationLevel.NOTICE)
def test_nonexistent_product(self):
- # Traversing to /+branch/<no-such-product> generates a 404.
- self.assertNotFound('non-existent')
+ # Traversing to /+branch/<no-such-product> displays an error message.
+ non_existent = 'non-existent'
+ requiredMessage = u"No such product: '%s'." % non_existent
+ self.assertDisplaysNotification(
+ non_existent, requiredMessage,
+ BrowserNotificationLevel.ERROR)
def test_product_without_dev_focus(self):
- # Traversing to a product without a development focus generates a 404.
+ # Traversing to a product without a development focus displays a
+ # user message on the same page.
product = self.factory.makeProduct()
- self.assertNotFound(product.name)
+ requiredMessage = (u"The target %s does not have a linked branch."
+ % product.name)
+ self.assertDisplaysNotification(
+ product.name,
+ requiredMessage,
+ BrowserNotificationLevel.NOTICE)
+
+ def test_distro_package_alias(self):
+ # Traversing to /+branch/<distro>/<sourcepackage package> redirects
+ # to the page for the branch that is the development focus branch
+ # for that package.
+ sourcepackage = self.factory.makeSourcePackage()
+ branch = self.factory.makePackageBranch(sourcepackage=sourcepackage)
+ distro_package = sourcepackage.distribution_sourcepackage
+ ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches
+ registrant = ubuntu_branches.teamowner
+ target = ICanHasLinkedBranch(distro_package)
+ with person_logged_in(registrant):
+ target.setBranch(branch, registrant)
+ self.assertRedirects("%s" % target.bzr_path, canonical_url(branch))
+
+ def test_private_branch_for_distro_package(self):
+ # If the development focus of a distro package is private, display a
+ # message telling the user there is no linked branch.
+ sourcepackage = self.factory.makeSourcePackage()
+ branch = self.factory.makePackageBranch(
+ sourcepackage=sourcepackage, private=True)
+ distro_package = sourcepackage.distribution_sourcepackage
+ ubuntu_branches = getUtility(ILaunchpadCelebrities).ubuntu_branches
+ registrant = ubuntu_branches.teamowner
+ with person_logged_in(registrant):
+ ICanHasLinkedBranch(distro_package).setBranch(branch, registrant)
+
+ any_user = self.factory.makePerson()
+ login_person(any_user)
+ path = ICanHasLinkedBranch(distro_package).bzr_path
+ requiredMessage = (u"The target %s does not have a linked branch."
+ % path)
+ self.assertDisplaysNotification(
+ path, requiredMessage, BrowserNotificationLevel.NOTICE)
def test_trailing_path_redirect(self):
# If there are any trailing path segments after the branch identifier,
@@ -106,33 +233,63 @@
def test_product_series_redirect(self):
# Traversing to /+branch/<product>/<series> redirects to the branch
# for that series, if there is one.
- branch = self.factory.makeProductBranch()
- product = branch.product
- series = self.factory.makeProductSeries(product=product)
- removeSecurityProxy(series).branch = branch
+ branch = self.factory.makeBranch()
+ series = self.factory.makeProductSeries(branch=branch)
self.assertRedirects(
- '%s/%s' % (product.name, series.name), canonical_url(branch))
+ ICanHasLinkedBranch(series).bzr_path, canonical_url(branch))
def test_nonexistent_product_series(self):
- # /+branch/<product>/<series> generates a 404 if there is no such
- # series.
+ # /+branch/<product>/<series> displays an error message if there is
+ # no such series.
product = self.factory.makeProduct()
- self.assertNotFound('%s/nonexistent' % product.name)
+ non_existent = 'nonexistent'
+ requiredMessage = u"No such product series: '%s'." % non_existent
+ self.assertDisplaysNotification(
+ '%s/%s' % (product.name, non_existent),
+ requiredMessage,
+ BrowserNotificationLevel.ERROR)
def test_no_branch_for_series(self):
- # If there's no branch for a product series, generate a 404.
+ # If there's no branch for a product series, display a
+ # message telling the user there is no linked branch.
series = self.factory.makeProductSeries()
- self.assertNotFound('%s/%s' % (series.product.name, series.name))
+ path = ICanHasLinkedBranch(series).bzr_path
+ requiredMessage = ("The target %s does not have a linked branch."
+ % path)
+ self.assertDisplaysNotification(
+ path, requiredMessage, BrowserNotificationLevel.NOTICE)
+
+ def test_private_branch_for_series(self):
+ # If the development focus of a product series is private, display a
+ # message telling the user there is no linked branch.
+ branch = self.factory.makeBranch(private=True)
+ series = self.factory.makeProductSeries(branch=branch)
+
+ any_user = self.factory.makePerson()
+ login_person(any_user)
+ path = ICanHasLinkedBranch(series).bzr_path
+ requiredMessage = (u"The target %s does not have a linked branch."
+ % path)
+ self.assertDisplaysNotification(
+ path, requiredMessage, BrowserNotificationLevel.NOTICE)
def test_too_short_branch_name(self):
- # 404 if the thing following +branch is a unique name that's too short
- # to be a real unique name.
+ # error notification if the thing following +branch is a unique name
+ # that's too short to be a real unique name.
owner = self.factory.makePerson()
- self.assertNotFound('~%s' % owner.name)
+ requiredMessage = (u"Cannot understand namespace name: '%s'"
+ % owner.name)
+ self.assertDisplaysNotification(
+ '~%s' % owner.name,
+ requiredMessage,
+ BrowserNotificationLevel.ERROR)
def test_invalid_product_name(self):
- # 404 if the thing following +branch has an invalid product name.
- self.assertNotFound('a')
+ # error notification if the thing following +branch has an invalid
+ # product name.
+ self.assertDisplaysNotification(
+ 'a', u"Invalid name for product: a.",
+ BrowserNotificationLevel.ERROR)
class TestPersonTraversal(TestCaseWithFactory, TraversalMixin):
=== modified file 'lib/canonical/launchpad/database/message.py'
--- lib/canonical/launchpad/database/message.py 2010-08-29 21:24:47 +0000
+++ lib/canonical/launchpad/database/message.py 2010-09-23 02:21:04 +0000
@@ -82,6 +82,7 @@
from lp.app.errors import NotFoundError
from lp.registry.interfaces.person import validate_public_person
from lp.services.job.model.job import Job
+from lp.services.propertycache import cachedproperty
# this is a hard limit on the size of email we will be willing to store in
# the database.
@@ -158,10 +159,14 @@
"""See IMessage."""
return self.owner
- @property
+ @cachedproperty
def text_contents(self):
"""See IMessage."""
- bits = [unicode(chunk) for chunk in self if chunk.content]
+ return Message.chunks_text(self.chunks)
+
+ @classmethod
+ def chunks_text(cls, chunks):
+ bits = [unicode(chunk) for chunk in chunks if chunk.content]
return '\n\n'.join(bits)
# XXX flacoste 2006-09-08: Bogus attribute only present so that
=== modified file 'lib/canonical/launchpad/interfaces/emailaddress.py'
--- lib/canonical/launchpad/interfaces/emailaddress.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/interfaces/emailaddress.py 2010-09-23 02:21:04 +0000
@@ -115,7 +115,7 @@
def destroySelf():
"""Destroy this email address and any associated subscriptions.
-
+
:raises UndeletableEmailAddress: When the email address is a person's
preferred one or a hosted mailing list's address.
"""
@@ -153,4 +153,3 @@
Return None if there is no such email address.
"""
-
=== modified file 'lib/canonical/launchpad/interfaces/validation.py'
--- lib/canonical/launchpad/interfaces/validation.py 2010-08-22 18:31:30 +0000
+++ lib/canonical/launchpad/interfaces/validation.py 2010-09-23 02:21:04 +0000
@@ -19,7 +19,7 @@
'validate_new_distrotask',
'valid_upstreamtask',
'valid_password',
- 'validate_date_interval'
+ 'validate_date_interval',
]
from cgi import escape
@@ -31,10 +31,16 @@
from canonical.launchpad import _
from canonical.launchpad.interfaces.launchpad import ILaunchBag
+from canonical.launchpad.interfaces.account import IAccount
+from canonical.launchpad.interfaces.emailaddress import (
+ IEmailAddress,
+ IEmailAddressSet,
+ )
from canonical.launchpad.validators import LaunchpadValidationError
from canonical.launchpad.validators.cve import valid_cve
from canonical.launchpad.validators.email import valid_email
from canonical.launchpad.validators.url import valid_absolute_url
+from canonical.launchpad.webapp import canonical_url
from canonical.launchpad.webapp.menu import structured
from lp.app.errors import NotFoundError
@@ -124,6 +130,7 @@
scheme (for instance, http:// for a web URL), and ensure the
URL uses either http, https or ftp.""")))
+
def valid_branch_url(branch_url):
"""Returns True if web_ref is a valid download URL, or raises a
LaunchpadValidationError.
@@ -188,6 +195,28 @@
"${email} isn't a valid email address.",
mapping={'email': email}))
+def _check_email_availability(email):
+ email_address = getUtility(IEmailAddressSet).getByEmail(email)
+ if email_address is not None:
+ # The email already exists; determine what has it.
+ if email_address.person is not None:
+ person = email_address.person
+ message = _('${email} is already registered in Launchpad and is '
+ 'associated with <a href="${url}">${person}</a>.',
+ mapping={'email': escape(email),
+ 'url': canonical_url(person),
+ 'person': escape(person.displayname)})
+ elif email_address.account is not None:
+ account = email_address.account
+ message = _('${email} is already registered in Launchpad and is '
+ 'associated with the ${account} account.',
+ mapping={'email': escape(email),
+ 'account': escape(account.displayname)})
+ else:
+ message = _('${email} is already registered in Launchpad.',
+ mapping={'email': escape(email)})
+ raise LaunchpadValidationError(structured(message))
+
def validate_new_team_email(email):
"""Check that the given email is valid and not registered to
@@ -195,16 +224,9 @@
"""
from canonical.launchpad.webapp import canonical_url
from canonical.launchpad.interfaces import IEmailAddressSet
+
_validate_email(email)
- email_address = getUtility(IEmailAddressSet).getByEmail(email)
- if email_address is not None:
- person = email_address.person
- message = _('${email} is already registered in Launchpad and is '
- 'associated with <a href="${url}">${team}</a>.',
- mapping={'email': escape(email),
- 'url': canonical_url(person),
- 'team': escape(person.displayname)})
- raise LaunchpadValidationError(structured(message))
+ _check_email_availability(email)
return True
=== modified file 'lib/lp/blueprints/browser/specificationtarget.py'
--- lib/lp/blueprints/browser/specificationtarget.py 2010-08-24 10:45:57 +0000
+++ lib/lp/blueprints/browser/specificationtarget.py 2010-09-23 02:21:04 +0000
@@ -326,6 +326,12 @@
return self.context.specifications(filter=filter)
@cachedproperty
+ def specs_batched(self):
+ navigator = BatchNavigator(self.specs, self.request, size=500)
+ navigator.setHeadings('specification', 'specifications')
+ return navigator
+
+ @cachedproperty
def spec_count(self):
return self.specs.count()
=== modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
--- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-08-20 20:31:18 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-23 02:21:04 +0000
@@ -3,8 +3,8 @@
__metaclass__ = type
-import unittest
-
+from canonical.launchpad.webapp.batching import BatchNavigator
+from canonical.launchpad.testing.pages import find_tag_by_id
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.blueprints.interfaces.specificationtarget import (
IHasSpecifications,
@@ -15,7 +15,10 @@
login_person,
TestCaseWithFactory,
)
-from lp.testing.views import create_view
+from lp.testing.views import (
+ create_view,
+ create_initialized_view,
+ )
class TestRegisterABlueprintButtonView(TestCaseWithFactory):
@@ -86,12 +89,43 @@
self.assertFalse(
'<div id="involvement" class="portlet involvement">' in view())
-
-def test_suite():
- suite = unittest.TestSuite()
- suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
- return suite
-
-
-if __name__ == '__main__':
- unittest.TextTestRunner().run(test_suite())
+ def test_specs_batch(self):
+ # Some pages turn up in very large contexts and patch. E.g.
+ # Distro:+assignments which uses SpecificationAssignmentsView, a
+ # subclass.
+ person = self.factory.makePerson()
+ view = create_initialized_view(person, name='+assignments')
+ self.assertIsInstance(view.specs_batched, BatchNavigator)
+
+ def test_batch_headings(self):
+ person = self.factory.makePerson()
+ view = create_initialized_view(person, name='+assignments')
+ navigator = view.specs_batched
+ self.assertEqual('specification', navigator._singular_heading)
+ self.assertEqual('specifications', navigator._plural_heading)
+
+ def test_batch_size(self):
+ # Because +assignments is meant to provide an overview, we default to
+ # 500 as the default batch size.
+ person = self.factory.makePerson()
+ view = create_initialized_view(person, name='+assignments')
+ navigator = view.specs_batched
+ self.assertEqual(500, view.specs_batched.default_size)
+
+
+class TestAssignments(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_assignments_are_batched(self):
+ product = self.factory.makeProduct()
+ spec1 = self.factory.makeSpecification(product=product)
+ spec2 = self.factory.makeSpecification(product=product)
+ view = create_initialized_view(product, name='+assignments',
+ query_string="batch=1")
+ content = view.render()
+ self.assertEqual('next',
+ find_tag_by_id(content, 'upper-batch-nav-batchnav-next')['class'])
+ self.assertEqual('next',
+ find_tag_by_id(content, 'lower-batch-nav-batchnav-next')['class'])
+
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2010-08-31 19:38:30 +0000
+++ lib/lp/blueprints/model/specification.py 2010-09-23 02:21:04 +0000
@@ -713,50 +713,39 @@
"""
# Circular import.
from lp.registry.model.person import Person
- assignee = ClassAlias(Person, "assignee")
- approver = ClassAlias(Person, "approver")
- drafter = ClassAlias(Person, "drafter")
- origin = [Specification,
- LeftJoin(assignee, Specification.assignee==assignee.id),
- LeftJoin(approver, Specification.approver==approver.id),
- LeftJoin(drafter, Specification.drafter==drafter.id),
- ]
- columns = [Specification, assignee, approver, drafter]
- for alias in (assignee, approver, drafter):
- validity_info = Person._validity_queries(person_table=alias)
+ def cache_people(rows):
+ # Find the people we need:
+ person_ids = set()
+ for spec in rows:
+ person_ids.add(spec.assigneeID)
+ person_ids.add(spec.approverID)
+ person_ids.add(spec.drafterID)
+ person_ids.discard(None)
+ if not person_ids:
+ return
+ # Query those people
+ origin = [Person]
+ columns = [Person]
+ validity_info = Person._validity_queries()
origin.extend(validity_info["joins"])
columns.extend(validity_info["tables"])
- # We only need one decorators list: all the decorators will be
- # bound the same, and having a single list lets us more easily call
- # the right one.
decorators = validity_info["decorators"]
- columns = tuple(columns)
- results = Store.of(self).using(*origin).find(
- columns,
+ personset = Store.of(self).using(*origin).find(
+ tuple(columns),
+ Person.id.is_in(person_ids),
+ )
+ for row in personset:
+ person = row[0]
+ index = 1
+ for decorator in decorators:
+ column = row[index]
+ index += 1
+ decorator(person, column)
+ results = Store.of(self).find(
+ Specification,
SQL(query),
)
- def cache_person(person, row, start_index):
- """apply caching decorators to person."""
- index = start_index
- for decorator in decorators:
- column = row[index]
- index += 1
- decorator(person, column)
- return index
- def cache_validity(row):
- # Assignee
- person = row[1]
- # After drafter
- index = 4
- index = cache_person(person, row, index)
- # approver
- person = row[2]
- index = cache_person(person, row, index)
- # drafter
- person = row[3]
- index = cache_person(person, row, index)
- return row[0]
- return DecoratedResultSet(results, cache_validity)
+ return DecoratedResultSet(results, pre_iter_hook=cache_people)
@property
def valid_specifications(self):
=== modified file 'lib/lp/blueprints/templates/specificationtarget-assignments.pt'
--- lib/lp/blueprints/templates/specificationtarget-assignments.pt 2009-09-22 15:02:41 +0000
+++ lib/lp/blueprints/templates/specificationtarget-assignments.pt 2010-09-23 02:21:04 +0000
@@ -31,6 +31,7 @@
sign off on the specification.
</p>
+ <tal:navigation replace="structure view/specs_batched/@@+navigation-links-upper" />
<table class="listing sortable" id="work">
<thead>
<tr>
@@ -43,8 +44,8 @@
<th>Approver</th>
</tr>
</thead>
- <tbody class="lesser">
- <tr tal:repeat="spec view/specs">
+ <tbody class="lesser" tal:define="specs view/specs_batched/currentBatch">
+ <tr tal:repeat="spec specs">
<td>
<span class="sortkey" tal:content="spec/priority/sortkey" />
<span tal:content="spec/priority/title"
@@ -91,6 +92,7 @@
</tr>
</tbody>
</table>
+ <tal:navigation replace="structure view/specs_batched/@@+navigation-links-lower" />
</div>
</div>
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2010-09-09 17:02:33 +0000
+++ lib/lp/bugs/model/bug.py 2010-09-23 02:21:04 +0000
@@ -58,6 +58,7 @@
SQLRaw,
Union,
)
+from storm.info import ClassAlias
from storm.store import (
EmptyResultSet,
Store,
@@ -426,21 +427,101 @@
@property
def indexed_messages(self):
"""See `IMessageTarget`."""
+ return self._indexed_messages(include_content=True)
+
+ def _indexed_messages(self, include_content=False, include_parents=True):
+ """Get the bugs messages, indexed.
+
+ :param include_content: If True retrieve the content for the messages
+ too.
+ :param include_parents: If True retrieve the object for parent messages
+ too. If False the parent attribute will be *forced* to None to
+ reduce database lookups.
+ """
+ # Make all messages be 'in' the main bugtask.
inside = self.default_bugtask
- messages = list(self.messages)
- message_set = set(messages)
-
- indexed_messages = []
- for index, message in enumerate(messages):
- if message.parent not in message_set:
- parent = None
+ store = Store.of(self)
+ message_by_id = {}
+ if include_parents:
+ def to_messages(rows):
+ return [row[0] for row in rows]
+ else:
+ def to_messages(rows):
+ return rows
+ def eager_load_owners(messages):
+ # Because we may have multiple owners, we spend less time in storm
+ # with very large bugs by not joining and instead querying a second
+ # time. If this starts to show high db time, we can left outer join
+ # instead.
+ owner_ids = set(message.ownerID for message in messages)
+ owner_ids.discard(None)
+ if not owner_ids:
+ return
+ list(store.find(Person, Person.id.is_in(owner_ids)))
+ def eager_load_content(messages):
+ # To avoid the complexity of having multiple rows per
+ # message, or joining in the database (though perhaps in
+ # future we should do that), we do a single separate query
+ # for the message content.
+ message_ids = set(message.id for message in messages)
+ chunks = store.find(
+ MessageChunk, MessageChunk.messageID.is_in(message_ids))
+ chunks.order_by(MessageChunk.id)
+ chunk_map = {}
+ for chunk in chunks:
+ message_chunks = chunk_map.setdefault(chunk.messageID, [])
+ message_chunks.append(chunk)
+ for message in messages:
+ if message.id not in chunk_map:
+ continue
+ cache = IPropertyCache(message)
+ cache.text_contents = Message.chunks_text(
+ chunk_map[message.id])
+ def eager_load(rows, slice_info):
+ messages = to_messages(rows)
+ eager_load_owners(messages)
+ if include_content:
+ eager_load_content(messages)
+ def index_message(row, index):
+ # convert row to an IndexedMessage
+ if include_parents:
+ message, parent = row
+ if parent is not None:
+ # If there is an IndexedMessage available as parent, use
+ # that to reduce on-demand parent lookups.
+ parent = message_by_id.get(parent.id, parent)
else:
- parent = message.parent
-
- indexed_message = IndexedMessage(message, inside, index, parent)
- indexed_messages.append(indexed_message)
-
- return indexed_messages
+ message = row
+ parent = None # parent attribute is not going to be accessed.
+ result = IndexedMessage(message, inside, index, parent)
+ # This message may be the parent for another: stash it to permit
+ # use.
+ message_by_id[message.id] = result
+ return result
+ # There is possibly some nicer way to do this in storm, but
+ # this is a lot easier to figure out.
+ if include_parents:
+ ParentMessage = ClassAlias(Message, name="parent_message")
+ tables = SQL("""
+Message left outer join
+message as parent_message on (
+ message.parent=parent_message.id and
+ parent_message.id in (
+ select bugmessage.message from bugmessage where bugmessage.bug=%s)),
+BugMessage""" % sqlvalues(self.id))
+ results = store.using(tables).find(
+ (Message, ParentMessage),
+ BugMessage.bugID == self.id,
+ BugMessage.messageID == Message.id,
+ )
+ else:
+ results = store.find(Message,
+ BugMessage.bugID == self.id,
+ BugMessage.messageID == Message.id,
+ )
+ results.order_by(Message.datecreated, Message.id)
+ return DecoratedResultSet(results, index_message,
+ pre_iter_hook=eager_load, slice_info=True)
@property
def displayname(self):
@@ -1785,7 +1866,7 @@
when attachments is evaluated, not when the resultset is processed).
"""
message_to_indexed = {}
- for message in self.indexed_messages:
+ for message in self._indexed_messages(include_parents=False):
message_to_indexed[message.id] = message
def set_indexed_message(row):
attachment = row[0]
=== modified file 'lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt'
--- lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt 2010-05-21 04:38:42 +0000
+++ lib/lp/bugs/stories/structural-subscriptions/xx-bug-subscriptions.txt 2010-09-23 02:21:04 +0000
@@ -194,3 +194,33 @@
>>> anon_browser.open('http://launchpad.dev/bzr')
>>> print anon_browser.title
Bazaar Version Control System in Launchpad
+
+== Distribution with a bug supervisor ==
+
+If a distribution has a bug supervisor only that team or members of it can
+subscribe to all of the distribution's bugs.
+
+First, check the page content for a distribution without a bug supervisor.
+
+ >>> browser.open('http://bugs.launchpad.dev/ubuntu/+subscribe')
+ >>> text_contents = extract_text(find_main_content(browser.contents))
+ >>> "You can choose to receive an e-mail every time" in text_contents
+ True
+
+Set a bug supervisor for Ubuntu.
+
+ >>> from zope.security.proxy import removeSecurityProxy
+ >>> login('foo.bar@xxxxxxxxxxxxx')
+ >>> ubuntu = removeSecurityProxy(getUtility(IDistributionSet).get(1))
+ >>> guadamen = getUtility(IPersonSet).getByName('guadamen')
+ >>> ubuntu.bug_supervisor = guadamen
+ >>> flush_database_updates()
+ >>> logout()
+
+Second, check that the page content for a distribution with a bug supervisor
+contains a message about not being able to subscribe.
+
+ >>> browser.open('http://bugs.launchpad.dev/ubuntu/+subscribe')
+ >>> text_contents = extract_text(find_main_content(browser.contents))
+ >>> "You are unable to subscribe to bug reports about" in text_contents
+ True
=== modified file 'lib/lp/bugs/tests/test_bugs_webservice.py'
--- lib/lp/bugs/tests/test_bugs_webservice.py 2010-09-02 19:30:03 +0000
+++ lib/lp/bugs/tests/test_bugs_webservice.py 2010-09-23 02:21:04 +0000
@@ -143,7 +143,7 @@
rendered_comment, self.expected_comment_html)
-class TestBugAttachments(TestCaseWithFactory):
+class TestBugScaling(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
@@ -165,12 +165,12 @@
collector = QueryCollector()
collector.register()
self.addCleanup(collector.unregister)
- url = '/bugs/%d/attachments' % self.bug.id
+ url = '/bugs/%d/attachments?ws.size=75' % self.bug.id
#First request
store.flush()
store.reset()
response = webservice.get(url)
- self.assertThat(collector, HasQueryCount(LessThan(24)))
+ self.assertThat(collector, HasQueryCount(LessThan(21)))
with_2_count = collector.count
self.failUnlessEqual(response.status, 200)
login(USER_EMAIL)
@@ -181,7 +181,44 @@
store.flush()
store.reset()
response = webservice.get(url)
- self.assertThat(collector, HasQueryCount(Equals(with_2_count+1)))
+ self.assertThat(collector, HasQueryCount(Equals(with_2_count)))
+
+ def test_messages_query_counts_constant(self):
+ # XXX Robert Collins 2010-09-15 bug=619017
+ # This test may be thrown off by the reference bug. To get around the
+ # problem, flush and reset are called on the bug storm cache before
+ # each call to the webservice. When lp's storm is updated to release
+ # the committed fix for this bug, please see about updating this test.
+ login(USER_EMAIL)
+ bug = self.factory.makeBug()
+ store = Store.of(bug)
+ self.factory.makeBugComment(bug)
+ self.factory.makeBugComment(bug)
+ self.factory.makeBugComment(bug)
+ person = self.factory.makePerson()
+ webservice = LaunchpadWebServiceCaller(
+ 'launchpad-library', 'salgado-change-anything')
+ collector = QueryCollector()
+ collector.register()
+ self.addCleanup(collector.unregister)
+ url = '/bugs/%d/messages?ws.size=75' % bug.id
+ #First request
+ store.flush()
+ store.reset()
+ response = webservice.get(url)
+ self.assertThat(collector, HasQueryCount(LessThan(21)))
+ with_2_count = collector.count
+ self.failUnlessEqual(response.status, 200)
+ login(USER_EMAIL)
+ for i in range(50):
+ self.factory.makeBugComment(bug)
+ self.factory.makeBugAttachment(bug)
+ logout()
+ #Second request
+ store.flush()
+ store.reset()
+ response = webservice.get(url)
+ self.assertThat(collector, HasQueryCount(Equals(with_2_count)))
class TestBugMessages(TestCaseWithFactory):
=== modified file 'lib/lp/bugs/tests/test_searchtasks_webservice.py'
--- lib/lp/bugs/tests/test_searchtasks_webservice.py 2010-08-20 21:48:35 +0000
+++ lib/lp/bugs/tests/test_searchtasks_webservice.py 2010-09-23 02:21:04 +0000
@@ -5,8 +5,6 @@
__metaclass__ = type
-from unittest import TestLoader
-
from canonical.launchpad.ftests import login
from lp.testing import TestCaseWithFactory
from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
@@ -37,7 +35,3 @@
response = self.webservice.named_get('/mebuntu/inkanyamba',
'searchTasks', api_version='devel').jsonBody()
self.assertEqual(response['total_size'], 1)
-
-
-def test_suite():
- return TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposals.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-05-27 02:19:27 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposals.txt 2010-09-23 02:21:04 +0000
@@ -543,7 +543,6 @@
>>> nopriv_browser.open(bmp_url)
>>> print_bugs_and_specs(nopriv_browser)
- Linked bug reports and blueprints
Bug #...: Bug for linking Undecided New
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt 2010-07-15 13:57:39 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt 2010-09-23 02:21:04 +0000
@@ -46,6 +46,10 @@
#proposal-summary td {
padding-left: 0.5em;
}
+ .related-bugs-list {
+ padding-left: 20px;
+ padding-bottom: 10px;
+ }
/* A page-specific fix for inline text are editing to line up box. */
#edit-description .yui-ieditor-input { top: 0; }
#edit-commit_message .yui-ieditor-input { top: 0; }
@@ -138,6 +142,18 @@
</tal:has-description>
</div>
+ <div class="yui-g" tal:condition="view/has_bug_or_spec">
+ <div id="related-bugs-and-blueprints" class="related-bugs-list"
+ tal:define="branch context/source_branch">
+ <div class="actions">
+ <metal:bug-branch-links use-macro="branch/@@+macros/bug-branch-links"/>
+ </div>
+ <div class="actions">
+ <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/>
+ </div>
+ </div>
+ </div>
+
<div class="yui-g">
<tal:not-logged-in condition="not: view/user">
<div align="center" id="add-comment-login-first">
@@ -176,36 +192,19 @@
</tal:logged-in>
<div class="yui-g">
- <div class="yui-u first">
- <div id="source-revisions"
- tal:condition="not: context/queue_status/enumvalue:MERGED">
-
- <tal:history-available condition="context/source_branch/revision_count"
- define="branch context/source_branch;
- revisions view/unlanded_revisions">
- <h2>Unmerged revisions</h2>
- <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
- </tal:history-available>
-
- <tal:remote-branch condition="context/source_branch/branch_type/enumvalue:REMOTE">
- <h2>Unmerged revisions</h2>
- <p>Recent revisions are not available due to the source branch being remote.</p>
- </tal:remote-branch>
- </div>
- </div>
-
- <div class="yui-u" tal:condition="view/has_bug_or_spec">
- <div id="related-bugs-and-blueprints"
- tal:define="branch context/source_branch">
- <h2>Linked bug reports and blueprints</h2>
- <div class="actions">
-
- <metal:bug-branch-links use-macro="branch/@@+macros/bug-branch-links"/>
- </div>
- <div class="actions">
- <metal:spec-branch-links use-macro="branch/@@+macros/spec-branch-links"/>
- </div>
- </div>
+ <div id="source-revisions"
+ tal:condition="not: context/queue_status/enumvalue:MERGED">
+
+ <tal:history-available condition="context/source_branch/revision_count"
+ define="branch context/source_branch;
+ revisions view/unlanded_revisions">
+ <h2>Unmerged revisions</h2>
+ <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
+ </tal:history-available>
+ <tal:remote-branch condition="context/source_branch/branch_type/enumvalue:REMOTE">
+ <h2>Unmerged revisions</h2>
+ <p>Recent revisions are not available due to the source branch being remote.</p>
+ </tal:remote-branch>
</div>
</div>
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-09-19 21:23:58 +0000
+++ lib/lp/registry/browser/person.py 2010-09-23 02:21:04 +0000
@@ -4835,6 +4835,11 @@
"'%s' doesn't seem to be a valid email address." % newemail)
return self.errors
+ # XXX j.c.sackett 2010-09-15 bug=628247, 576757 There is a validation
+ # system set up for this that is almost identical in
+ # canonical.launchpad.interfaces.validation, called
+ # _check_email_available or similar. It would be really nice if we
+ # could merge that code somehow with this.
email = getUtility(IEmailAddressSet).getByEmail(newemail)
person = self.context
if email is not None:
@@ -4846,20 +4851,32 @@
"detected it as being yours. If it was detected by our "
"system, it's probably shown on this page and is waiting "
"to be confirmed as yours." % newemail)
- else:
+ elif email.person is not None:
owner = email.person
owner_name = urllib.quote(owner.name)
merge_url = (
'%s/+requestmerge?field.dupe_person=%s'
% (canonical_url(getUtility(IPersonSet)), owner_name))
- self.addError(
- structured(
- "The email address '%s' is already registered to "
- '<a href="%s">%s</a>. If you think that is a '
- 'duplicated account, you can <a href="%s">merge it'
- "</a> into your account.",
- newemail, canonical_url(owner), owner.displayname,
- merge_url))
+ self.addError(structured(
+ "The email address '%s' is already registered to "
+ '<a href="%s">%s</a>. If you think that is a '
+ 'duplicated account, you can <a href="%s">merge it'
+ "</a> into your account.",
+ newemail,
+ canonical_url(owner),
+ owner.displayname,
+ merge_url))
+ elif email.account is not None:
+ account = email.account
+ self.addError(structured(
+ "The email address '%s' is already registered to an "
+ "account, %s.",
+ newemail,
+ account.displayname))
+ else:
+ self.addError(structured(
+ "The email address '%s' is already registered.",
+ newemail))
return self.errors
@action(_("Add"), name="add_email", validator=validate_action_add_email)
=== modified file 'lib/lp/registry/browser/structuralsubscription.py'
--- lib/lp/registry/browser/structuralsubscription.py 2010-08-24 10:45:57 +0000
+++ lib/lp/registry/browser/structuralsubscription.py 2010-09-23 02:21:04 +0000
@@ -172,6 +172,10 @@
"""Return True, if the current user is subscribed."""
return self.isSubscribed(self.user)
+ def userCanAlter(self):
+ if self.context.userCanAlterBugSubscription(self.user, self.user):
+ return True
+
@action(u'Save these changes', name='save')
def save_action(self, action, data):
"""Process the subscriptions submitted by the user."""
=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py 2010-09-02 19:28:35 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py 2010-09-23 02:21:04 +0000
@@ -5,13 +5,16 @@
import transaction
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from canonical.config import config
from canonical.launchpad.ftests import (
ANONYMOUS,
login,
)
+from canonical.launchpad.interfaces.authtoken import LoginTokenType
from canonical.launchpad.interfaces.account import AccountStatus
+from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.testing import (
@@ -29,7 +32,10 @@
)
from lp.registry.interfaces.karma import IKarmaCacheManager
-from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.person import (
+ PersonVisibility,
+ IPersonSet,
+ )
from lp.registry.interfaces.teammembership import (
ITeamMembershipSet,
TeamMembershipStatus,
@@ -207,7 +213,8 @@
def setUp(self):
TestCaseWithFactory.setUp(self)
- self.person = self.factory.makePerson()
+ self.valid_email_address = self.factory.getUniqueEmailAddress()
+ self.person = self.factory.makePerson(email=self.valid_email_address)
login_person(self.person)
self.ppa = self.factory.makeArchive(owner=self.person)
self.view = PersonEditView(
@@ -256,6 +263,94 @@
self.view.initialize()
self.assertFalse(self.view.form_fields['name'].for_display)
+ def test_add_email_good_data(self):
+ email_address = self.factory.getUniqueEmailAddress()
+ form = {
+ 'field.VALIDATED_SELECTED': self.valid_email_address,
+ 'field.VALIDATED_SELECTED-empty-marker': 1,
+ 'field.actions.add_email': 'Add',
+ 'field.newemail': email_address,
+ }
+ view = create_initialized_view(self.person, "+editemails", form=form)
+
+ # If everything worked, there should now be a login token to validate
+ # this email address for this user.
+ token = getUtility(ILoginTokenSet).searchByEmailRequesterAndType(
+ email_address,
+ self.person,
+ LoginTokenType.VALIDATEEMAIL)
+ self.assertTrue(token is not None)
+
+ def test_add_email_address_taken(self):
+ email_address = self.factory.getUniqueEmailAddress()
+ account = self.factory.makeAccount(
+ displayname='deadaccount',
+ email=email_address,
+ status=AccountStatus.NOACCOUNT)
+ form = {
+ 'field.VALIDATED_SELECTED': self.valid_email_address,
+ 'field.VALIDATED_SELECTED-empty-marker': 1,
+ 'field.actions.add_email': 'Add',
+ 'field.newemail': email_address,
+ }
+ view = create_initialized_view(self.person, "+editemails", form=form)
+ error_msg = view.errors[0]
+ expected_msg = ("The email address '%s' is already registered to an "
+ "account, deadaccount." % email_address)
+ self.assertEqual(expected_msg, error_msg)
+
+
+class TestTeamCreationView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestTeamCreationView, self).setUp()
+ person = self.factory.makePerson()
+ login_person(person)
+
+ def test_team_creation_good_data(self):
+ form = {
+ 'field.actions.create': 'Create Team',
+ 'field.contactemail': 'contactemail@xxxxxxxxxxx',
+ 'field.displayname': 'liberty-land',
+ 'field.name': 'libertyland',
+ 'field.renewal_policy': 'NONE',
+ 'field.renewal_policy-empty-marker': 1,
+ 'field.subscriptionpolicy': 'RESTRICTED',
+ 'field.subscriptionpolicy-empty-marker': 1,
+ }
+ person_set = getUtility(IPersonSet)
+ view = create_initialized_view(
+ person_set, '+newteam', form=form)
+ team = person_set.getByName('libertyland')
+ self.assertTrue(team is not None)
+ self.assertEqual('libertyland', team.name)
+
+ def test_validate_email_catches_taken_emails(self):
+ email_address = self.factory.getUniqueEmailAddress()
+ account = self.factory.makeAccount(
+ displayname='libertylandaccount',
+ email=email_address,
+ status=AccountStatus.NOACCOUNT)
+ form = {
+ 'field.actions.create': 'Create Team',
+ 'field.contactemail': email_address,
+ 'field.displayname': 'liberty-land',
+ 'field.name': 'libertyland',
+ 'field.renewal_policy': 'NONE',
+ 'field.renewal_policy-empty-marker': 1,
+ 'field.subscriptionpolicy': 'RESTRICTED',
+ 'field.subscriptionpolicy-empty-marker': 1,
+ }
+ person_set = getUtility(IPersonSet)
+ view = create_initialized_view(person_set, '+newteam', form=form)
+ expected_msg = ('%s is already registered in Launchpad and is '
+ 'associated with the libertylandaccount '
+ 'account.' % email_address)
+ error_msg = view.errors[0].errors[0]
+ self.assertEqual(expected_msg, error_msg)
+
class TestPersonParticipationView(TestCaseWithFactory):
@@ -602,6 +697,7 @@
self.assertEqual(
'This account is already deactivated.', view.errors[0])
+
class TestTeamInvitationView(TestCaseWithFactory):
"""Tests for TeamInvitationView."""
=== modified file 'lib/lp/registry/templates/structural-subscriptions-manage.pt'
--- lib/lp/registry/templates/structural-subscriptions-manage.pt 2009-12-05 18:37:28 +0000
+++ lib/lp/registry/templates/structural-subscriptions-manage.pt 2010-09-23 02:21:04 +0000
@@ -8,6 +8,20 @@
>
<body>
<div metal:fill-slot="main">
+ <tal:no_permissions condition="not: view/userCanAlter|nothing">
+ <p>
+ You are unable to subscribe to bug reports about <span
+ tal:replace="context/title">this item</span> as it generates
+ a high amount of bug activity which results in more e-mails than
+ most users can handle.
+ </p>
+ <p>
+ If you really want to subscribe to <span
+ tal:replace="context/title">this item</span> bug mail than
+ please contact its bug supervisor.
+ </p>
+ </tal:no_permissions>
+ <tal:has_permissions condition="view/userCanAlter|nothing">
<p>
You can choose to receive an e-mail every time someone reports or
changes a public bug associated with
@@ -19,6 +33,7 @@
time.
</p>
<div metal:use-macro="context/@@launchpad_form/form" />
+ </tal:has_permissions>
</div>
<div metal:fill-slot="side">
<div tal:replace="structure context/@@+portlet-malone-bugmail-filtering-faq"/>
=== modified file 'lib/lp/services/database/prejoin.py'
--- lib/lp/services/database/prejoin.py 2010-08-20 20:31:18 +0000
+++ lib/lp/services/database/prejoin.py 2010-09-23 02:21:04 +0000
@@ -1,7 +1,11 @@
# Copyright 2009 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Prejoin for Storm."""
+"""Prejoin for Storm.
+
+XXX bug=632150 This is duplicated with DecoratedResultSet. please use that.
+RBC 20100907.
+"""
__metaclass__ = type
__all__ = ['prejoin']
=== modified file 'lib/lp/soyuz/scripts/ftpmaster.py'
--- lib/lp/soyuz/scripts/ftpmaster.py 2010-09-16 21:13:12 +0000
+++ lib/lp/soyuz/scripts/ftpmaster.py 2010-09-23 02:21:04 +0000
@@ -75,6 +75,7 @@
Mostly used to describe errors in the initialisation of this object.
"""
+
class TagFileNotFound(Exception):
"""Raised when an archive tag file could not be found."""
@@ -126,6 +127,7 @@
def architectures(self):
return dict([(a.architecturetag, a)
for a in self.distroseries.architectures])
+
@property
def components(self):
return dict([(c.name, c) for c in self.distroseries.components])
@@ -223,7 +225,6 @@
for architecture in self.architectures:
self.buildArchNBS(component, architecture)
-
def buildArchNBS(self, component, architecture):
"""Build NBS per architecture.
@@ -257,7 +258,7 @@
source = m.group(1)
version = m.group(2)
- if not self.bin_pkgs.has_key(package):
+ if package not in self.bin_pkgs:
self.nbs.setdefault(source, {})
self.nbs[source].setdefault(package, {})
self.nbs[source][package][version] = ""
@@ -265,7 +266,7 @@
if architecture != "all":
self.arch_any.setdefault(package, "0")
if apt_pkg.VersionCompare(
- version,self.arch_any[package]) < 1:
+ version, self.arch_any[package]) < 1:
self.arch_any[package] = version
finally:
# close fd and remove temporary file used to store uncompressed
@@ -273,7 +274,6 @@
temp_fd.close()
os.unlink(temp_filename)
-
def buildASBA(self):
"""Build the group of 'all superseded by any' binaries."""
self.logger.debug("Building all superseded by any list (ASBA):")
@@ -281,7 +281,6 @@
for architecture in self.architectures:
self.buildArchASBA(component, architecture)
-
def buildArchASBA(self, component, architecture):
"""Build ASBA per architecture.
@@ -315,7 +314,7 @@
version = m.group(2)
if architecture == "all":
- if (self.arch_any.has_key(package) and
+ if (package in self.arch_any and
apt_pkg.VersionCompare(
version, self.arch_any[package]) > -1):
self.asba.setdefault(source, {})
@@ -483,6 +482,7 @@
Currently used for auxiliary storage in PubSourceChecker.
"""
+
def __init__(self, name, version, arch, component, section, priority):
self.name = name
self.version = version
@@ -521,6 +521,7 @@
return "\n".join(report)
+
class PubBinaryDetails:
"""Store the component, section and priority of binary packages and, for
each binary package the most frequent component, section and priority.
@@ -537,6 +538,7 @@
- correct_sections: same as correct_components, but for sections
- correct_priorities: same as correct_components, but for priorities
"""
+
def __init__(self):
self.components = {}
self.sections = {}
@@ -591,6 +593,7 @@
Receive the source publication data and its binaries and perform
a group of heuristic consistency checks.
"""
+
def __init__(self, name, version, component, section, urgency):
self.name = name
self.version = version
@@ -655,7 +658,8 @@
def renderReport(self):
"""Render a formatted report for the publication group.
- Return None if no issue was annotated or an formatted string including:
+ Return None if no issue was annotated or an formatted string
+ including:
SourceName_Version Component/Section/Urgency | # bin
<BINREPORTS>
@@ -719,7 +723,7 @@
ftype = filenameToContentType(filename)
try:
- alias_id = getUtility(ILibrarianClient).addFile(
+ alias_id = getUtility(ILibrarianClient).addFile(
filename, flen, fd, contentType=ftype)
except UploadFailed, info:
raise ChrootManagerError("Librarian upload failed: %s" % info)
@@ -845,7 +849,8 @@
origin: a dictionary similar to 'files' but where the values
contain information for download files to be synchronized
logger: a logger
- downloader: a callable that fetchs URLs, 'downloader(url, destination)'
+ downloader: a callable that fetchs URLs,
+ 'downloader(url, destination)'
todistro: target distribution object
"""
self.files = files
=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py 2010-09-17 00:53:33 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py 2010-09-23 02:21:04 +0000
@@ -7,6 +7,7 @@
__all__ = [
'close_bugs',
'close_bugs_for_queue_item',
+ 'close_bugs_for_sourcepackagerelease',
'close_bugs_for_sourcepublication',
'get_bugs_from_changes_file',
'ProcessAccepted',
@@ -16,6 +17,7 @@
import sys
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.webapp.errorlog import (
@@ -165,6 +167,14 @@
janitor = getUtility(ILaunchpadCelebrities).janitor
for bug in bugs_to_close:
+ # We need to remove the security proxy here because the bug
+ # might be private and if this code is called via someone using
+ # the +queue page they will get an OOPS. Ideally, we should
+ # migrate this code to the Job system though, but that's a lot
+ # of work. If you don't do that and you're changing stuff in
+ # here, BE CAREFUL with the unproxied bug object and look at
+ # what you're doing with it that might violate security.
+ bug = removeSecurityProxy(bug)
edited_task = bug.setStatus(
target=source_release.sourcepackage,
status=BugTaskStatus.FIXRELEASED,
=== modified file 'lib/lp/soyuz/scripts/tests/test_queue.py'
--- lib/lp/soyuz/scripts/tests/test_queue.py 2010-08-27 12:21:25 +0000
+++ lib/lp/soyuz/scripts/tests/test_queue.py 2010-09-23 02:21:04 +0000
@@ -3,6 +3,8 @@
"""queue tool base class tests."""
+from __future__ import with_statement
+
__metaclass__ = type
__all__ = [
'upload_bar_source',
@@ -12,6 +14,7 @@
import hashlib
import os
import shutil
+from StringIO import StringIO
import tempfile
from unittest import (
TestCase,
@@ -19,6 +22,7 @@
)
from zope.component import getUtility
+from zope.security.interfaces import ForbiddenAttribute
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
@@ -33,7 +37,10 @@
fillLibrarianFile,
)
from canonical.librarian.utils import filechunks
-from canonical.testing import LaunchpadZopelessLayer
+from canonical.testing import (
+ DatabaseFunctionalLayer,
+ LaunchpadZopelessLayer,
+ )
from lp.archiveuploader.nascentupload import NascentUpload
from lp.archiveuploader.tests import (
datadir,
@@ -42,7 +49,10 @@
mock_logger_quiet,
)
from lp.bugs.interfaces.bug import IBugSet
-from lp.bugs.interfaces.bugtask import IBugTaskSet
+from lp.bugs.interfaces.bugtask import (
+ BugTaskStatus,
+ IBugTaskSet,
+ )
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -56,6 +66,9 @@
from lp.soyuz.interfaces.archive import (
IArchiveSet,
)
+from lp.soyuz.scripts.processaccepted import (
+ close_bugs_for_sourcepackagerelease,
+ )
from lp.soyuz.interfaces.queue import (
IPackageUploadSet,
)
@@ -64,6 +77,11 @@
CommandRunnerError,
name_queue_map,
)
+from lp.testing import (
+ celebrity_logged_in,
+ person_logged_in,
+ TestCaseWithFactory,
+ )
class TestQueueBase(TestCase):
@@ -926,6 +944,36 @@
'override binary pmount', component_name='partner')
+class TestQueuePageClosingBugs(TestCaseWithFactory):
+ # The distroseries +queue page can close bug when accepting
+ # packages. Unit tests for that belong here.
+
+ layer = DatabaseFunctionalLayer
+
+ def test_close_bugs_for_sourcepackagerelease_with_private_bug(self):
+ # lp.soyuz.scripts.processaccepted.close_bugs_for_sourcepackagerelease
+ # should work with private bugs where the person using the queue
+ # page doesn't have access to it.
+ changes_file_template = "Format: 1.7\nLaunchpad-bugs-fixed: %s\n"
+ # changelog_entry is required for an assertion inside the function
+ # we're testing.
+ spr = self.factory.makeSourcePackageRelease(changelog_entry="blah")
+ archive_admin = self.factory.makePerson()
+ bug = self.factory.makeBug(private=True)
+ bug_task = self.factory.makeBugTask(target=spr.sourcepackage, bug=bug)
+ changes = StringIO(changes_file_template % bug.id)
+
+ with person_logged_in(archive_admin):
+ # The archive admin user can't normally see this bug.
+ self.assertRaises(ForbiddenAttribute, bug, 'status')
+ # But the bug closure should work.
+ close_bugs_for_sourcepackagerelease(spr, changes)
+
+ # Verify it was closed.
+ with celebrity_logged_in("admin"):
+ self.assertEqual(bug_task.status, BugTaskStatus.FIXRELEASED)
+
+
class TestQueueToolInJail(TestQueueBase):
layer = LaunchpadZopelessLayer
dbuser = config.uploadqueue.dbuser
=== modified file 'lib/lp/soyuz/scripts/tests/test_sync_source.py'
--- lib/lp/soyuz/scripts/tests/test_sync_source.py 2010-09-16 21:13:12 +0000
+++ lib/lp/soyuz/scripts/tests/test_sync_source.py 2010-09-23 02:21:04 +0000
@@ -124,7 +124,7 @@
on disk.
"""
files = {
- 'foo': {'md5sum': 'dd21ab16f950f7ac4f9c78ef1498eee1', 'size': 15}
+ 'foo': {'md5sum': 'dd21ab16f950f7ac4f9c78ef1498eee1', 'size': 15},
}
origin = {}
sync_source = self._getSyncSource(files, origin)
@@ -138,7 +138,7 @@
def testCheckDownloadedFilesWrongMD5(self):
"""Expect SyncSourceError to be raised due the wrong MD5."""
files = {
- 'foo': {'md5sum': 'duhhhhh', 'size': 15}
+ 'foo': {'md5sum': 'duhhhhh', 'size': 15},
}
origin = {}
sync_source = self._getSyncSource(files, origin)
@@ -154,7 +154,7 @@
def testCheckDownloadedFilesWrongSize(self):
"""Expect SyncSourceError to be raised due the wrong size."""
files = {
- 'foo': {'md5sum': 'dd21ab16f950f7ac4f9c78ef1498eee1', 'size': 10}
+ 'foo': {'md5sum': 'dd21ab16f950f7ac4f9c78ef1498eee1', 'size': 10},
}
origin = {}
sync_source = self._getSyncSource(files, origin)
@@ -209,7 +209,8 @@
def testFetchLibrarianFilesOK(self):
"""Probe fetchLibrarianFiles.
- Seek on files published from librarian and download matching filenames.
+ Seek on files published from librarian and download matching
+ filenames.
"""
files = {
'netapplet_1.0.0.orig.tar.gz': {},
@@ -359,7 +360,7 @@
os.unlink(expected_changesfile)
def testSyncSourceRunV3(self):
- """Try a simple sync-source.py run with a version 3 source format
+ """Try a simple sync-source.py run with a version 3 source format
package.
It will run in a special tree prepared to cope with sync-source
@@ -386,7 +387,7 @@
self.assertEqual(
err.splitlines(),
['W: Could not find blacklist file on '
- '/srv/launchpad.net/dak/sync-blacklist.txt',
+ '/srv/launchpad.net/dak/sync-blacklist.txt',
'INFO - <sample1_1.0.orig-component3.tar.gz: cached>',
'INFO - <sample1_1.0-1.dsc: cached>',
'INFO - <sample1_1.0-1.debian.tar.gz: cached>',
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-09-10 03:20:48 +0000
+++ lib/lp/testing/factory.py 2010-09-23 02:21:04 +0000
@@ -881,7 +881,7 @@
return product
def makeProductSeries(self, product=None, name=None, owner=None,
- summary=None, date_created=None):
+ summary=None, date_created=None, branch=None):
"""Create and return a new ProductSeries."""
if product is None:
product = self.makeProduct()
@@ -895,7 +895,7 @@
# so we remove the security proxy before creating the series.
naked_product = removeSecurityProxy(product)
series = naked_product.newSeries(
- owner=owner, name=name, summary=summary)
+ owner=owner, name=name, summary=summary, branch=branch)
if date_created is not None:
series.datecreated = date_created
return ProxyFactory(series)
@@ -2537,6 +2537,7 @@
source_package_recipe_build=None,
dscsigningkey=None,
user_defined_fields=None,
+ changelog_entry=None,
homepage=None):
"""Make a `SourcePackageRelease`."""
if distroseries is None:
@@ -2594,7 +2595,7 @@
build_conflicts_indep=build_conflicts_indep,
architecturehintlist=architecturehintlist,
changelog=None,
- changelog_entry=None,
+ changelog_entry=changelog_entry,
dsc=None,
copyright=self.getUniqueString(),
dscsigningkey=dscsigningkey,
=== modified file 'scripts/ftpmaster-tools/sync-source.py'
--- scripts/ftpmaster-tools/sync-source.py 2010-09-16 21:13:12 +0000
+++ scripts/ftpmaster-tools/sync-source.py 2010-09-23 02:21:04 +0000
@@ -18,15 +18,7 @@
import apt_pkg
import commands
-try:
- from debian.deb822 import Dsc
-except ImportError:
- # In older versions of python-debian the main package was named
- # debian_bundle
- # XXX: Remove this when an up to date version of python-debian lands in
- # the PPA or Ubuntu. Maverick will be the first release that has an
- # up to date version of python-debian.
- from debian_bundle.deb822 import Dsc
+from debian.deb822 import Dsc
import errno
import optparse
@@ -92,7 +84,7 @@
return md5sum
-def reject (str, prefix="Rejected: "):
+def reject(str, prefix="Rejected: "):
global reject_message
if str:
reject_message += prefix + str + "\n"
@@ -338,69 +330,6 @@
source, source_component, binary, current_version,
current_component)
-def split_gpg_and_payload(sequence):
- """Return a (gpg_pre, payload, gpg_post) tuple
-
- Each element of the returned tuple is a list of lines (with trailing
- whitespace stripped).
- """
- # XXX JRV 20100211: Copied from deb822.py in python-debian. When
- # Launchpad switches to Lucid this copy should be removed.
- # bug=520508
-
- gpg_pre_lines = []
- lines = []
- gpg_post_lines = []
- state = 'SAFE'
- gpgre = re.compile(r'^-----(?P<action>BEGIN|END) PGP (?P<what>[^-]+)-----$')
- blank_line = re.compile('^$')
- first_line = True
-
- for line in sequence:
- line = line.strip('\r\n')
-
- # skip initial blank lines, if any
- if first_line:
- if blank_line.match(line):
- continue
- else:
- first_line = False
-
- m = gpgre.match(line)
-
- if not m:
- if state == 'SAFE':
- if not blank_line.match(line):
- lines.append(line)
- else:
- if not gpg_pre_lines:
- # There's no gpg signature, so we should stop at
- # this blank line
- break
- elif state == 'SIGNED MESSAGE':
- if blank_line.match(line):
- state = 'SAFE'
- else:
- gpg_pre_lines.append(line)
- elif state == 'SIGNATURE':
- gpg_post_lines.append(line)
- else:
- if m.group('action') == 'BEGIN':
- state = m.group('what')
- elif m.group('action') == 'END':
- gpg_post_lines.append(line)
- break
- if not blank_line.match(line):
- if not lines:
- gpg_pre_lines.append(line)
- else:
- gpg_post_lines.append(line)
-
- if len(lines):
- return (gpg_pre_lines, lines, gpg_post_lines)
- else:
- raise EOFError('only blank lines found in input')
-
def import_dsc(dsc_filename, suite, previous_version, signing_rules,
files_from_librarian, requested_by, origin, current_sources,
@@ -410,10 +339,7 @@
if signing_rules.startswith("must be signed"):
dsc_file.seek(0)
- # XXX JRV 20100211: When Launchpad starts depending on Lucid,
- # use dsc.split_gpg_and_payload() instead.
- # bug=520508
- (gpg_pre, payload, gpg_post) = split_gpg_and_payload(dsc_file)
+ (gpg_pre, payload, gpg_post) = Dsc.split_gpg_and_payload(dsc_file)
if gpg_pre == [] and gpg_post == []:
dak_utils.fubar("signature required for %s but not present"
% dsc_filename)
Follow ups
