launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13308
lp:~wallyworld/launchpad/bugtask-index-milestone-queries-1064819 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/bugtask-index-milestone-queries-1064819 into lp:launchpad.
Commit message:
Rework milestone vocab and bugtask views to reduce query count on bugtask index page.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #1064819 in Launchpad itself: "bugtask index page claims to cache milestones but doesn't in practice"
https://bugs.launchpad.net/launchpad/+bug/1064819
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bugtask-index-milestone-queries-1064819/+merge/129095
== Implementation ==
This branch changes the bugtask index page so that it no longer executes 2-3 queries per milestone. Previously, an attempt was made to preload milestones using getBugTaskTargetMilestones() but then a MilestoneVocabulary was used which loaded each milestone one at a time.
A new vocab is introduced - BugTaskMilestoneVocabulary. This vocab implements ITokenizedVocabulary so it can be used in Launchpad forms but it is created using the preloaded milestones so it doesn't need to hit the db again. The existing bugtask support in the original MilestoneVocabulary was removed since I can't see where else it is used.
As a drive by, another optimisation was performed. The bugtask table itself and the links above and below the table are rendered using separate macros and TAL, using the same view BugTasksAndNominationsView. However, this view executes a bunch of queries in its initialize() which are not needed for both cases. So I introduced 2 separate view classes and put the methods from the old view on the correct new view.
The base query load is now about 10% less for a bugtask page with no milestones, and the same value also for a page with milestones.
== Tests ==
New tests for the new bugtask milestone vocab. Remove obsolete tests from the other milestone vocab.
Add new query count test for the bugtask page with milestones.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/configure.zcml
lib/lp/bugs/vocabularies.py
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/browser/configure.zcml
lib/lp/bugs/browser/tests/test_bugtask.py
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/model/bugtask.py
lib/lp/bugs/tests/test_vocabulary.py
lib/lp/registry/vocabularies.py
lib/lp/registry/tests/test_milestone_vocabularies.py
--
https://code.launchpad.net/~wallyworld/launchpad/bugtask-index-milestone-queries-1064819/+merge/129095
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2012-10-10 03:12:08 +0000
+++ lib/lp/bugs/browser/bugtask.py 2012-10-11 05:06:42 +0000
@@ -25,9 +25,10 @@
'BugTaskNavigation',
'BugTaskPrivacyAdapter',
'BugTaskRemoveQuestionView',
- 'BugTasksAndNominationsView',
'BugTaskSearchListingView',
'BugTaskSetNavigation',
+ 'BugTasksNominationsView',
+ 'BugTasksTableView',
'BugTaskTableRowView',
'BugTaskTextView',
'BugTaskView',
@@ -94,7 +95,6 @@
providedBy,
)
from zope.schema import Choice
-from zope.schema.interfaces import IContextSourceBinder
from zope.schema.vocabulary import (
getVocabularyRegistry,
SimpleVocabulary,
@@ -217,6 +217,7 @@
from lp.bugs.interfaces.cve import ICveSet
from lp.bugs.interfaces.malone import IMaloneApplication
from lp.bugs.model.bugtasksearch import orderby_expression
+from lp.bugs.vocabularies import BugTaskMilestoneVocabulary
from lp.code.interfaces.branchcollection import IAllBranches
from lp.layers import FeedsLayer
from lp.registry.interfaces.distribution import (
@@ -3421,42 +3422,154 @@
return re.sub(r"\W", "", bugtask.bugtargetdisplayname)
-class CachedMilestoneSourceFactory:
- """A factory for milestone vocabularies.
-
- When rendering a page with many bug tasks, this factory is useful,
- in order to avoid the number of db queries issues. For each bug task
- target, we cache the milestone vocabulary, so we don't have to
- create a new one for each target.
- """
-
- implements(IContextSourceBinder)
-
- def __init__(self):
- self.vocabularies = {}
- self.contexts = set()
-
- def __call__(self, context):
- assert context in self.contexts, ("context %r not added to "
- "self.contexts (%r)." % (context, self.contexts))
- self._load()
- target = MilestoneVocabulary.getMilestoneTarget(context)
- return self.vocabularies[target]
-
- def _load(self):
- """Load all the vocabularies, once only."""
- if self.vocabularies:
- return
- targets = set(
- map(MilestoneVocabulary.getMilestoneTarget, self.contexts))
- # TODO: instantiate for all targets at once.
- for target in targets:
- milestone_vocabulary = MilestoneVocabulary(target)
- self.vocabularies[target] = milestone_vocabulary
-
-
-class BugTasksAndNominationsView(LaunchpadView):
- """Browser class for rendering the bugtasks and nominations table."""
+class BugTasksNominationsView(LaunchpadView):
+ """Browser class for rendering the bug nominations portlet."""
+
+ def __init__(self, context, request):
+ """Ensure we always have a bug context."""
+ LaunchpadView.__init__(self, IBug(context), request)
+
+ def displayAlsoAffectsLinks(self):
+ """Return True if the Also Affects links should be displayed."""
+ # Hide the links when the bug is viewed in a CVE context
+ return self.request.getNearest(ICveSet) == (None, None)
+
+ @cachedproperty
+ def current_user_affected_status(self):
+ """Is the current user marked as affected by this bug?"""
+ return self.context.isUserAffected(self.user)
+
+ @property
+ def current_user_affected_js_status(self):
+ """A javascript literal indicating if the user is affected."""
+ affected = self.current_user_affected_status
+ if affected is None:
+ return 'null'
+ elif affected:
+ return 'true'
+ else:
+ return 'false'
+
+ @cachedproperty
+ def other_users_affected_count(self):
+ """The number of other users affected by this bug.
+ """
+ if getFeatureFlag('bugs.affected_count_includes_dupes.disabled'):
+ if self.current_user_affected_status:
+ return self.context.users_affected_count - 1
+ else:
+ return self.context.users_affected_count
+ else:
+ return self.context.other_users_affected_count_with_dupes
+
+ @cachedproperty
+ def total_users_affected_count(self):
+ """The number of affected users, typically across all users.
+
+ Counting across duplicates may be disabled at run time.
+ """
+ if getFeatureFlag('bugs.affected_count_includes_dupes.disabled'):
+ return self.context.users_affected_count
+ else:
+ return self.context.users_affected_count_with_dupes
+
+ @cachedproperty
+ def affected_statement(self):
+ """The default "this bug affects" statement to show.
+
+ The outputs of this method should be mirrored in
+ MeTooChoiceSource._getSourceNames() (Javascript).
+ """
+ me_affected = self.current_user_affected_status
+ other_affected = self.other_users_affected_count
+ if me_affected is None:
+ if other_affected == 1:
+ return "This bug affects 1 person. Does this bug affect you?"
+ elif other_affected > 1:
+ return (
+ "This bug affects %d people. Does this bug "
+ "affect you?" % other_affected)
+ else:
+ return "Does this bug affect you?"
+ elif me_affected is True:
+ if other_affected == 0:
+ return "This bug affects you"
+ elif other_affected == 1:
+ return "This bug affects you and 1 other person"
+ else:
+ return "This bug affects you and %d other people" % (
+ other_affected)
+ else:
+ if other_affected == 0:
+ return "This bug doesn't affect you"
+ elif other_affected == 1:
+ return "This bug affects 1 person, but not you"
+ elif other_affected > 1:
+ return "This bug affects %d people, but not you" % (
+ other_affected)
+
+ @cachedproperty
+ def anon_affected_statement(self):
+ """The "this bug affects" statement to show to anonymous users.
+
+ The outputs of this method should be mirrored in
+ MeTooChoiceSource._getSourceNames() (Javascript).
+ """
+ affected = self.total_users_affected_count
+ if affected == 1:
+ return "This bug affects 1 person"
+ elif affected > 1:
+ return "This bug affects %d people" % affected
+ else:
+ return None
+
+ def canAddProjectTask(self):
+ return can_add_project_task_to_bug(self.context)
+
+ def canAddPackageTask(self):
+ return can_add_package_task_to_bug(self.context)
+
+ @property
+ def current_bugtask(self):
+ """Return the current `IBugTask`.
+
+ 'current' is determined by simply looking in the ILaunchBag utility.
+ """
+ return getUtility(ILaunchBag).bugtask
+
+
+def can_add_project_task_to_bug(bug):
+ """Can a new bug task on a project be added to this bug?
+
+ If a bug has any bug tasks already, were it to be Proprietary or
+ Embargoed, it cannot be marked as also affecting any other
+ project, so return False.
+ """
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
+ return True
+ return len(bug.bugtasks) == 0
+
+
+def can_add_package_task_to_bug(bug):
+ """Can a new bug task on a src pkg be added to this bug?
+
+ If a bug has any existing bug tasks on a project, were it to
+ be Proprietary or Embargoed, then it cannot be marked as
+ affecting a package, so return False.
+
+ A task on a given package may still be illegal to add, but
+ this will be caught when bug.addTask() is attempted.
+ """
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
+ return True
+ for pillar in bug.affected_pillars:
+ if IProduct.providedBy(pillar):
+ return False
+ return True
+
+
+class BugTasksTableView(LaunchpadView):
+ """Browser class for rendering the bugtasks table."""
target_releases = None
@@ -3477,7 +3590,6 @@
search_params = BugTaskSearchParams(user=self.user, bug=self.context)
self.bugtasks = list(bugtask_set.search(search_params))
self.many_bugtasks = len(self.bugtasks) >= 10
- self.cached_milestone_source = CachedMilestoneSourceFactory()
self.user_is_subscribed = self.context.isSubscribed(self.user)
# If we have made it to here then the logged in user can see the
@@ -3489,13 +3601,6 @@
precache_permission_for_objects(
self.request, 'launchpad.LimitedView', authorised_people)
- # Pull all of the related milestones, if any, into the storm cache,
- # since they'll be needed for the vocabulary used in this view.
- if self.bugtasks:
- self.milestones = list(
- bugtask_set.getBugTaskTargetMilestones(self.bugtasks))
- else:
- self.milestones = []
distro_packages = defaultdict(list)
distro_series_packages = defaultdict(list)
for bugtask in self.bugtasks:
@@ -3521,6 +3626,19 @@
if ids:
list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(ids))
+ @cachedproperty
+ def caching_milestone_vocabulary(self):
+ return BugTaskMilestoneVocabulary(self.milestones)
+
+ @cachedproperty
+ def milestones(self):
+ if self.bugtasks:
+ bugtask_set = getUtility(IBugTaskSet)
+ return list(
+ bugtask_set.getBugTaskTargetMilestones(self.bugtasks))
+ else:
+ return []
+
def getTargetLinkTitle(self, target):
"""Return text to put as the title for the link to the target."""
if not (IDistributionSourcePackage.providedBy(target) or
@@ -3550,19 +3668,19 @@
The view's is_conjoined_slave and is_converted_to_question
attributes are set, as well as the edit view.
"""
- self.cached_milestone_source.contexts.add(context)
view = getMultiAdapter(
(context, self.request),
name='+bugtasks-and-nominations-table-row')
view.is_converted_to_question = is_converted_to_question
view.is_conjoined_slave = is_conjoined_slave
+
+ view.edit_view = getMultiAdapter(
+ (context, self.request), name='+edit-form')
+ view.milestone_source = self.caching_milestone_vocabulary
if IBugTask.providedBy(context):
view.target_link_title = self.getTargetLinkTitle(context.target)
-
- view.edit_view = getMultiAdapter(
- (context, self.request), name='+edit-form')
- view.milestone_source = self.cached_milestone_source
- view.edit_view.milestone_source = self.cached_milestone_source
+ view.edit_view.milestone_source = (
+ BugTaskMilestoneVocabulary(self.milestones, context))
view.edit_view.user_is_subscribed = self.user_is_subscribed
# Hint to optimize when there are many bugtasks.
view.many_bugtasks = self.many_bugtasks
@@ -3654,144 +3772,6 @@
return bugtask_and_nomination_views
- @property
- def current_bugtask(self):
- """Return the current `IBugTask`.
-
- 'current' is determined by simply looking in the ILaunchBag utility.
- """
- return getUtility(ILaunchBag).bugtask
-
- def displayAlsoAffectsLinks(self):
- """Return True if the Also Affects links should be displayed."""
- # Hide the links when the bug is viewed in a CVE context
- return self.request.getNearest(ICveSet) == (None, None)
-
- @cachedproperty
- def current_user_affected_status(self):
- """Is the current user marked as affected by this bug?"""
- return self.context.isUserAffected(self.user)
-
- @property
- def current_user_affected_js_status(self):
- """A javascript literal indicating if the user is affected."""
- affected = self.current_user_affected_status
- if affected is None:
- return 'null'
- elif affected:
- return 'true'
- else:
- return 'false'
-
- @cachedproperty
- def other_users_affected_count(self):
- """The number of other users affected by this bug.
- """
- if getFeatureFlag('bugs.affected_count_includes_dupes.disabled'):
- if self.current_user_affected_status:
- return self.context.users_affected_count - 1
- else:
- return self.context.users_affected_count
- else:
- return self.context.other_users_affected_count_with_dupes
-
- @cachedproperty
- def total_users_affected_count(self):
- """The number of affected users, typically across all users.
-
- Counting across duplicates may be disabled at run time.
- """
- if getFeatureFlag('bugs.affected_count_includes_dupes.disabled'):
- return self.context.users_affected_count
- else:
- return self.context.users_affected_count_with_dupes
-
- @cachedproperty
- def affected_statement(self):
- """The default "this bug affects" statement to show.
-
- The outputs of this method should be mirrored in
- MeTooChoiceSource._getSourceNames() (Javascript).
- """
- me_affected = self.current_user_affected_status
- other_affected = self.other_users_affected_count
- if me_affected is None:
- if other_affected == 1:
- return "This bug affects 1 person. Does this bug affect you?"
- elif other_affected > 1:
- return (
- "This bug affects %d people. Does this bug "
- "affect you?" % (other_affected))
- else:
- return "Does this bug affect you?"
- elif me_affected is True:
- if other_affected == 0:
- return "This bug affects you"
- elif other_affected == 1:
- return "This bug affects you and 1 other person"
- else:
- return "This bug affects you and %d other people" % (
- other_affected)
- else:
- if other_affected == 0:
- return "This bug doesn't affect you"
- elif other_affected == 1:
- return "This bug affects 1 person, but not you"
- elif other_affected > 1:
- return "This bug affects %d people, but not you" % (
- other_affected)
-
- @cachedproperty
- def anon_affected_statement(self):
- """The "this bug affects" statement to show to anonymous users.
-
- The outputs of this method should be mirrored in
- MeTooChoiceSource._getSourceNames() (Javascript).
- """
- affected = self.total_users_affected_count
- if affected == 1:
- return "This bug affects 1 person"
- elif affected > 1:
- return "This bug affects %d people" % affected
- else:
- return None
-
- def canAddProjectTask(self):
- return can_add_project_task_to_bug(self.context)
-
- def canAddPackageTask(self):
- return can_add_package_task_to_bug(self.context)
-
-
-def can_add_project_task_to_bug(bug):
- """Can a new bug task on a project be added to this bug?
-
- If a bug has any bug tasks already, were it to be Proprietary or
- Embargoed, it cannot be marked as also affecting any other
- project, so return False.
- """
- if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
- return True
- return len(bug.bugtasks) == 0
-
-
-def can_add_package_task_to_bug(bug):
- """Can a new bug task on a src pkg be added to this bug?
-
- If a bug has any existing bug tasks on a project, were it to
- be Proprietary or Embargoed, then it cannot be marked as
- affecting a package, so return False.
-
- A task on a given package may still be illegal to add, but
- this will be caught when bug.addTask() is attempted.
- """
- if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
- return True
- for pillar in bug.affected_pillars:
- if IProduct.providedBy(pillar):
- return False
- return True
-
class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin,
BugTaskPrivilegeMixin):
@@ -3954,7 +3934,7 @@
@cachedproperty
def _visible_milestones(self):
"""The visible milestones for this context."""
- return self.milestone_source(self.context).visible_milestones
+ return self.milestone_source.visible_milestones(self.context)
@property
def milestone_widget_items(self):
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2012-10-05 05:34:13 +0000
+++ lib/lp/bugs/browser/configure.zcml 2012-10-11 05:06:42 +0000
@@ -947,13 +947,13 @@
attribute="__call__"/>
<browser:page
for="lp.bugs.interfaces.bug.IBug"
- class="lp.bugs.browser.bugtask.BugTasksAndNominationsView"
+ class="lp.bugs.browser.bugtask.BugTasksNominationsView"
permission="launchpad.View"
name="+bugtasks-and-nominations-portal"
template="../templates/bugtasks-and-nominations-portal.pt"/>
<browser:page
for="lp.bugs.interfaces.bug.IBug"
- class="lp.bugs.browser.bugtask.BugTasksAndNominationsView"
+ class="lp.bugs.browser.bugtask.BugTasksTableView"
permission="launchpad.View"
name="+bugtasks-and-nominations-table"
template="../templates/bugtasks-and-nominations-table.pt"/>
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2012-10-10 02:45:36 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2012-10-11 05:06:42 +0000
@@ -39,7 +39,8 @@
BugListingBatchNavigator,
BugTaskEditView,
BugTaskListingItem,
- BugTasksAndNominationsView,
+ BugTasksNominationsView,
+ BugTasksTableView,
)
from lp.bugs.enums import BugNotificationLevel
from lp.bugs.feed.bug import PersonBugsFeed
@@ -133,14 +134,14 @@
self.addCleanup(recorder.unregister)
self.invalidate_caches(task)
self.getUserBrowser(url, person_no_teams)
- # This may seem large: it is; there is easily another 30% fat in
+ # This may seem large: it is; there is easily another 25% fat in
# there.
- # If this test is run in isolation, the query count is 94.
+ # If this test is run in isolation, the query count is 80.
# Other tests in this TestCase could cache the
# "SELECT id, product, project, distribution FROM PillarName ..."
# query by previously browsing the task url, in which case the
# query count is decreased by one.
- self.assertThat(recorder, HasQueryCount(LessThan(95)))
+ self.assertThat(recorder, HasQueryCount(LessThan(81)))
count_with_no_teams = recorder.count
# count with many teams
self.invalidate_caches(task)
@@ -156,7 +157,7 @@
def test_rendered_query_counts_constant_with_attachments(self):
with celebrity_logged_in('admin'):
browses_under_limit = BrowsesWithQueryLimit(
- 95, self.factory.makePerson())
+ 84, self.factory.makePerson())
# First test with a single attachment.
task = self.factory.makeBugTask()
@@ -250,7 +251,7 @@
# Render the view with one activity.
with celebrity_logged_in('admin'):
browses_under_limit = BrowsesWithQueryLimit(
- 93, self.factory.makePerson())
+ 82, self.factory.makePerson())
person = self.factory.makePerson()
add_activity("description", person)
@@ -264,6 +265,27 @@
self.assertThat(task, browses_under_limit)
+ def test_rendered_query_counts_constant_with_milestones(self):
+ # More queries are not used for extra milestones.
+ products = []
+ bug = self.factory.makeBug()
+
+ with celebrity_logged_in('admin'):
+ browses_under_limit = BrowsesWithQueryLimit(
+ 88, self.factory.makePerson())
+
+ self.assertThat(bug, browses_under_limit)
+
+ # Render the view with many milestones.
+ with celebrity_logged_in('admin'):
+ for _ in range(10):
+ product = self.factory.makeProduct()
+ products.append(product)
+ self.factory.makeBugTask(bug=bug, target=product)
+ self.factory.makeMilestone(product)
+
+ self.assertThat(bug, browses_under_limit)
+
def test_error_for_changing_target_with_invalid_status(self):
# If a user moves a bug task with a restricted status (say,
# Triaged) to a target where they do not have permission to set
@@ -361,20 +383,20 @@
self.assertIsNone(tag)
-class TestBugTasksAndNominationsView(TestCaseWithFactory):
+class TestBugTasksNominationsView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
def setUp(self):
- super(TestBugTasksAndNominationsView, self).setUp()
+ super(TestBugTasksNominationsView, self).setUp()
login(ADMIN_EMAIL)
self.bug = self.factory.makeBug()
- self.view = BugTasksAndNominationsView(
+ self.view = BugTasksNominationsView(
self.bug, LaunchpadTestRequest())
def refresh(self):
# The view caches, to see different scenarios, a refresh is needed.
- self.view = BugTasksAndNominationsView(
+ self.view = BugTasksNominationsView(
self.bug, LaunchpadTestRequest())
def test_current_user_affected_status(self):
@@ -401,24 +423,6 @@
self.failUnlessEqual(
'false', self.view.current_user_affected_js_status)
- def test_not_many_bugtasks(self):
- for count in range(10 - len(self.bug.bugtasks) - 1):
- self.factory.makeBugTask(bug=self.bug)
- self.view.initialize()
- self.failIf(self.view.many_bugtasks)
- row_view = self.view._getTableRowView(
- self.bug.default_bugtask, False, False)
- self.failIf(row_view.many_bugtasks)
-
- def test_many_bugtasks(self):
- for count in range(10 - len(self.bug.bugtasks)):
- self.factory.makeBugTask(bug=self.bug)
- self.view.initialize()
- self.failUnless(self.view.many_bugtasks)
- row_view = self.view._getTableRowView(
- self.bug.default_bugtask, False, False)
- self.failUnless(row_view.many_bugtasks)
-
def test_other_users_affected_count(self):
# The number of other users affected does not change when the
# logged-in user marked him or herself as affected or not.
@@ -635,6 +639,41 @@
self.failUnlessEqual(
"This bug affects 2 people", self.view.anon_affected_statement)
+
+class TestBugTasksTableView(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestBugTasksTableView, self).setUp()
+ login(ADMIN_EMAIL)
+ self.bug = self.factory.makeBug()
+ self.view = BugTasksTableView(
+ self.bug, LaunchpadTestRequest())
+
+ def refresh(self):
+ # The view caches, to see different scenarios, a refresh is needed.
+ self.view = BugTasksNominationsView(
+ self.bug, LaunchpadTestRequest())
+
+ def test_not_many_bugtasks(self):
+ for count in range(10 - len(self.bug.bugtasks) - 1):
+ self.factory.makeBugTask(bug=self.bug)
+ self.view.initialize()
+ self.failIf(self.view.many_bugtasks)
+ row_view = self.view._getTableRowView(
+ self.bug.default_bugtask, False, False)
+ self.failIf(row_view.many_bugtasks)
+
+ def test_many_bugtasks(self):
+ for count in range(10 - len(self.bug.bugtasks)):
+ self.factory.makeBugTask(bug=self.bug)
+ self.view.initialize()
+ self.failUnless(self.view.many_bugtasks)
+ row_view = self.view._getTableRowView(
+ self.bug.default_bugtask, False, False)
+ self.failUnless(row_view.many_bugtasks)
+
def test_getTargetLinkTitle_product(self):
# The target link title is always none for products.
target = self.factory.makeProduct()
@@ -753,7 +792,7 @@
request = LaunchpadTestRequest()
foo_bugtasks_and_nominations_view = getMultiAdapter(
- (foo_bug, request), name="+bugtasks-and-nominations-portal")
+ (foo_bug, request), name="+bugtasks-and-nominations-table")
foo_bugtasks_and_nominations_view.initialize()
task_and_nomination_views = (
@@ -777,7 +816,7 @@
request = LaunchpadTestRequest()
foo_bugtasks_and_nominations_view = getMultiAdapter(
- (foo_bug, request), name="+bugtasks-and-nominations-portal")
+ (foo_bug, request), name="+bugtasks-and-nominations-table")
foo_bugtasks_and_nominations_view.initialize()
task_and_nomination_views = (
@@ -823,7 +862,7 @@
any_person = self.factory.makePerson()
login_person(any_person, request)
foo_bugtasks_and_nominations_view = getMultiAdapter(
- (foo_bug, request), name="+bugtasks-and-nominations-portal")
+ (foo_bug, request), name="+bugtasks-and-nominations-table")
foo_bugtasks_and_nominations_view.initialize()
task_and_nomination_views = (
foo_bugtasks_and_nominations_view.getBugTaskAndNominationViews())
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2012-08-29 06:24:05 +0000
+++ lib/lp/bugs/configure.zcml 2012-10-11 05:06:42 +0000
@@ -1058,4 +1058,11 @@
>
<allow interface="zope.schema.interfaces.IVocabularyFactory"/>
</securedutility>
+ <securedutility
+ name="BugTaskMilestoneVocabulary"
+ component="lp.bugs.vocabularies.BugTaskMilestoneVocabulary"
+ provides="zope.schema.interfaces.IVocabularyFactory"
+ >
+ <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+ </securedutility>
</configure>
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2012-09-28 05:31:15 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2012-10-11 05:06:42 +0000
@@ -895,7 +895,7 @@
/*
* The results of _getSourceNames() should closely mirror the
- * results of BugTasksAndNominationsView.affected_statement and
+ * results of BugTasksNominationsView.affected_statement and
* anon_affected_statement.
*/
_getSourceNames: function(others_affected_count) {
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2012-09-28 06:25:44 +0000
+++ lib/lp/bugs/model/bugtask.py 2012-10-11 05:06:42 +0000
@@ -1944,6 +1944,7 @@
milestones = store.find(
Milestone,
+ Milestone.active == True,
Or(
Milestone.distributionID.is_in(distro_ids),
Milestone.distroseriesID.is_in(distro_series_ids),
=== modified file 'lib/lp/bugs/tests/test_vocabulary.py'
--- lib/lp/bugs/tests/test_vocabulary.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/tests/test_vocabulary.py 2012-10-11 05:06:42 +0000
@@ -5,7 +5,10 @@
__metaclass__ = type
-from lp.bugs.vocabularies import UsesBugsDistributionVocabulary
+from lp.bugs.vocabularies import (
+ BugTaskMilestoneVocabulary,
+ UsesBugsDistributionVocabulary,
+ )
from lp.testing import (
person_logged_in,
TestCaseWithFactory,
@@ -67,3 +70,54 @@
self.assertFalse(
thing in vocabulary,
"Vocabulary contains a non-distribution.")
+
+
+class TestBugTaskMilestoneVocabulary(TestCaseWithFactory):
+ """Test that the BugTaskMilestoneVocabulary behaves as expected."""
+ layer = DatabaseFunctionalLayer
+
+ def _assert_milestones(self, target, milestone):
+ bugtask = self.factory.makeBugTask(target=target)
+ vocabulary = BugTaskMilestoneVocabulary([milestone], bugtask)
+ self.assertEqual(
+ [term.title for term in vocabulary], [milestone.displayname])
+
+ def testUpstreamBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a upstraem bugtask."""
+ product = self.factory.makeProduct()
+ milestone = self.factory.makeMilestone(product=product)
+ self._assert_milestones(product, milestone)
+
+ def testProductseriesBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a productseries."""
+ series = self.factory.makeProductSeries()
+ milestone = self.factory.makeMilestone(productseries=series)
+ self._assert_milestones(series, milestone)
+
+ def testDistributionBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a distribution."""
+ distro = self.factory.makeDistribution()
+ milestone = self.factory.makeMilestone(distribution=distro)
+ self._assert_milestones(distro, milestone)
+
+ def testDistroseriesBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a distroseries."""
+ distroseries = self.factory.makeDistroSeries()
+ milestone = self.factory.makeMilestone(distroseries=distroseries)
+ self._assert_milestones(distroseries, milestone)
+
+ def testDistributionSourcePackageBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a distro source package."""
+ distro = self.factory.makeDistribution()
+ milestone = self.factory.makeMilestone(distribution=distro)
+ distro_sourcepackage = self.factory.makeDistributionSourcePackage(
+ distribution=distro)
+ self._assert_milestones(distro_sourcepackage, milestone)
+
+ def testSourcePackageBugTaskMilestoneVocabulary(self):
+ """Test of MilestoneVocabulary for a sourcepackage."""
+ distroseries = self.factory.makeDistroSeries()
+ milestone = self.factory.makeMilestone(distroseries=distroseries)
+ sourcepackage = self.factory.makeSourcePackage(
+ distroseries=distroseries)
+ self._assert_milestones(sourcepackage, milestone)
=== modified file 'lib/lp/bugs/vocabularies.py'
--- lib/lp/bugs/vocabularies.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/vocabularies.py 2012-10-11 05:06:42 +0000
@@ -9,6 +9,7 @@
'BugNominatableDistroSeriesVocabulary',
'BugNominatableProductSeriesVocabulary',
'BugNominatableSeriesVocabulary',
+ 'BugTaskMilestoneVocabulary',
'BugTrackerVocabulary',
'BugVocabulary',
'BugWatchVocabulary',
@@ -31,6 +32,7 @@
)
from zope.component import getUtility
from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
from zope.schema.interfaces import (
IVocabulary,
IVocabularyTokenized,
@@ -48,8 +50,15 @@
from lp.bugs.model.bugtracker import BugTracker
from lp.bugs.model.bugwatch import BugWatch
from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distributionsourcepackage import (
+ IDistributionSourcePackage,
+ )
+from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.product import IProduct
+from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.projectgroup import IProjectGroup
from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.productseries import ProductSeries
@@ -316,3 +325,78 @@
def _queryNominatableObjectByName(self, name):
"""See BugNominatableSeriesVocabularyBase."""
return self.distribution.getSeries(name)
+
+
+def milestone_matches_bugtask(milestone, bugtask):
+ """ Return True if the milestone can be set against this bugtask."""
+ bug_target = bugtask.target
+ naked_milestone = removeSecurityProxy(milestone)
+
+ if IProduct.providedBy(bug_target):
+ return bugtask.product.id == naked_milestone.productID
+ elif IProductSeries.providedBy(bug_target):
+ return bugtask.productseries.id == naked_milestone.productseriesID
+ elif (IDistribution.providedBy(bug_target) or
+ IDistributionSourcePackage.providedBy(bug_target)):
+ return bugtask.distribution.id == naked_milestone.distributionID
+ elif (IDistroSeries.providedBy(bug_target) or
+ ISourcePackage.providedBy(bug_target)):
+ return bugtask.distroseries.id == naked_milestone.distroseriesID
+ return False
+
+
+class BugTaskMilestoneVocabulary:
+ """Milestones for a set of bugtasks."""
+
+ implements(IVocabulary, IVocabularyTokenized)
+
+ def __init__(self, milestones, default_bugtask=None):
+ assert default_bugtask is None or IBugTask.providedBy(default_bugtask)
+ self.default_bugtask = default_bugtask
+ self.milestones = dict(
+ (milestone.id, milestone) for milestone in milestones)
+
+ def visible_milestones(self, bugtask):
+ return self._get_milestones(bugtask)
+
+ def _get_milestones(self, bugtask=None):
+ """All milestones for the specified bugtask."""
+ bugtask = bugtask or self.default_bugtask
+ if bugtask is None:
+ return []
+
+ milestones = [milestone
+ for milestone in self.milestones.values()
+ if milestone_matches_bugtask(milestone, bugtask)]
+
+ if (bugtask.milestone is not None and
+ bugtask.milestone not in milestones):
+ # Even if we inactivate a milestone, a bugtask might still be
+ # linked to it. Include such milestones in the vocabulary to
+ # ensure that the +editstatus page doesn't break.
+ milestones.append(bugtask.milestone)
+ return milestones
+
+ def getTermByToken(self, token):
+ """See `IVocabularyTokenized`."""
+ try:
+ return self.milestones[token]
+ except:
+ raise LookupError(token)
+
+ def __len__(self):
+ """See `IVocabulary`."""
+ return len(self._get_milestones())
+
+ def __iter__(self):
+ """See `IVocabulary`."""
+ return iter(
+ [self.toTerm(milestone) for milestone in self._get_milestones()])
+
+ def toTerm(self, obj):
+ """See `IVocabulary`."""
+ return SimpleTerm(obj, obj.id, obj.displayname)
+
+ def __contains__(self, obj):
+ """See `IVocabulary`."""
+ return obj in self._get_milestones()
=== modified file 'lib/lp/registry/tests/test_milestone_vocabularies.py'
--- lib/lp/registry/tests/test_milestone_vocabularies.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_milestone_vocabularies.py 2012-10-11 05:06:42 +0000
@@ -10,7 +10,6 @@
from zope.component import getUtility
from lp.blueprints.interfaces.specification import ISpecificationSet
-from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProductSet
@@ -20,12 +19,11 @@
login,
logout,
)
-from lp.testing.factory import LaunchpadObjectFactory
from lp.testing.layers import DatabaseFunctionalLayer
class TestMilestoneVocabulary(TestCase):
- """Test that the BranchVocabulary behaves as expected."""
+ """Test that the MilestoneVocabulary behaves as expected."""
layer = DatabaseFunctionalLayer
def setUp(self):
@@ -73,74 +71,6 @@
[term.title for term in vocabulary],
[u'Debian 3.1', u'Debian 3.1-rc1'])
- def testUpstreamBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a upstraem bugtask."""
- bugtask = getUtility(IBugTaskSet).get(2)
- firefox = getUtility(IProductSet).getByName('firefox')
- self.assertEqual(bugtask.product, firefox)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary], [u'Mozilla Firefox 1.0'])
-
- def testDistributionBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a distribution."""
- bugtask = getUtility(IBugTaskSet).get(4)
- debian = getUtility(IDistributionSet).getByName('debian')
- self.assertEqual(bugtask.distribution, debian)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary],
- [u'Debian 3.1', u'Debian 3.1-rc1'])
-
- def testDistroseriesBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a distroseries."""
- bugtask = getUtility(IBugTaskSet).get(18)
- debian = getUtility(IDistributionSet).getByName('debian')
- woody = debian.getSeries('woody')
- self.assertEqual(bugtask.distroseries, woody)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary],
- [u'Debian 3.1', u'Debian 3.1-rc1'])
-
- def testProductseriesBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a productseries."""
- bugtask = getUtility(IBugTaskSet).get(29)
- firefox = getUtility(IProductSet).getByName('firefox')
- series_1_0 = firefox.getSeries('1.0')
- self.assertEqual(bugtask.productseries, series_1_0)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary], [u'Mozilla Firefox 1.0'])
-
- def testDistributionsourcepackageBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a productseries."""
- factory = LaunchpadObjectFactory()
- debian = getUtility(IDistributionSet).getByName('debian')
- distro_sourcepackage = factory.makeDistributionSourcePackage(
- distribution=debian)
- factory.makeSourcePackagePublishingHistory(
- distroseries=debian.currentseries,
- sourcepackagename=distro_sourcepackage.sourcepackagename)
- bugtask = factory.makeBugTask(target=distro_sourcepackage)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary],
- [u'Debian 3.1', u'Debian 3.1-rc1'])
-
- def testSourcepackageBugTaskMilestoneVocabulary(self):
- """Test of MilestoneVocabulary for a productseries."""
- factory = LaunchpadObjectFactory()
- debian = getUtility(IDistributionSet).getByName('debian')
- woody = debian.getSeries('woody')
- sourcepackage = factory.makeSourcePackage(
- distroseries=woody)
- bugtask = factory.makeBugTask(target=sourcepackage)
- vocabulary = MilestoneVocabulary(bugtask)
- self.assertEqual(
- [term.title for term in vocabulary],
- [u'Debian 3.1', u'Debian 3.1-rc1'])
-
def testSpecificationMilestoneVocabulary(self):
"""Test of MilestoneVocabulary for a specification."""
spec = getUtility(ISpecificationSet).get(1)
=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py 2012-10-04 23:15:35 +0000
+++ lib/lp/registry/vocabularies.py 2012-10-11 05:06:42 +0000
@@ -96,7 +96,6 @@
)
from lp.blueprints.interfaces.specification import ISpecification
-from lp.bugs.interfaces.bugtask import IBugTask
from lp.code.interfaces.branch import IBranch
from lp.registry.enums import (
EXCLUSIVE_TEAM_POLICY,
@@ -1428,19 +1427,7 @@
@staticmethod
def getMilestoneTarget(milestone_context):
"""Return the milestone target."""
- if IBugTask.providedBy(milestone_context):
- bug_target = milestone_context.target
- if IProduct.providedBy(bug_target):
- target = milestone_context.product
- elif IProductSeries.providedBy(bug_target):
- target = milestone_context.productseries.product
- elif (IDistribution.providedBy(bug_target) or
- IDistributionSourcePackage.providedBy(bug_target)):
- target = milestone_context.distribution
- elif (IDistroSeries.providedBy(bug_target) or
- ISourcePackage.providedBy(bug_target)):
- target = milestone_context.distroseries
- elif IDistributionSourcePackage.providedBy(milestone_context):
+ if IDistributionSourcePackage.providedBy(milestone_context):
target = milestone_context.distribution
elif ISourcePackage.providedBy(milestone_context):
target = milestone_context.distroseries
@@ -1503,14 +1490,6 @@
else:
milestones = []
- if (IBugTask.providedBy(milestone_context) and
- milestone_context.milestone is not None and
- milestone_context.milestone not in milestones):
- # Even if we inactivate a milestone, a bugtask might still be
- # linked to it. Include such milestones in the vocabulary to
- # ensure that the +editstatus page doesn't break.
- milestones.append(milestone_context.milestone)
-
# Prefetch products and distributions for rendering
# milestones: optimization to reduce the number of queries.
product_ids = set(
Follow ups
