launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02835
[Merge] lp:~wallyworld/launchpad/remove-decoratedbug into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/remove-decoratedbug into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#719901 Cannot view branch with revisions fixing private bugs
https://bugs.launchpad.net/bugs/719901
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/remove-decoratedbug/+merge/52003
This branch is the first of 3. The bug which started it all said "Cannot view branch with revisions fixing private bugs". Fixing the root cause is to ensure that private bugs are not exposed via APIs on branch merge proposals.
The core work is done here: remove the now obsolete DecoratedBug class and the linked_bugs property on DecoratedBranch and add a new getRelatedBugTasks() API on branch merge proposal. The subsequent branches expose the new API on the webservice and do some more work to remove the merge proposal related_bugs property.
== Implementation ==
The basis of the approach is to use the recently added Branch.getRelatedBugTasks() API instead of DecoratedBranch.linked_bugs and DecoratedBug.
A new getRelatedBugTasks() API is added to IBranchMergeProposal. Some refactoring is done to use the new APIs. Some common functionality from the Branch.getRelatedBugTasks() was moved to a new filter_one_task_per_bug() helper method.
== Demo and QA ==
Create a merge proposal for a branch which is linked to at least one private bug. The private bug should not show on the merge proposal index page for an unauthorised user.
Merge the source branch of the merge proposal into another branch. The target branch's index page should show the recent revisions with the merge proposal but the private bug should not show for unauthorised users.
== Tests ==
Some new tests were added:
- browser.tests.test_branchmergeproposal:
test_linked_bugs_excludes_private_bugs()
- model.tests.test_branchcollection:
test_some_revisions_with_private_bugs()
- model.tests.test_branchmergeproposal:
test_related_bugtasks_includes_source_bugtasks()
test_related_bugtasks_excludes_target_bugs()
test_related_bugtasks_excludes_mutual_bugs()
test_related_bugtasks_excludes_private_bugs()
Existing test_branchmergeproposal, test_branchcollection, test_branch tests were also run.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/configure.zcml
lib/lp/code/browser/branch.py
lib/lp/code/browser/branchmergeproposal.py
lib/lp/code/browser/decorations.py
lib/lp/code/browser/tests/test_branch.py
lib/lp/code/browser/tests/test_branchmergeproposal.py
lib/lp/code/interfaces/branchcollection.py
lib/lp/code/interfaces/branchmergeproposal.py
lib/lp/code/model/branch.py
lib/lp/code/model/branchcollection.py
lib/lp/code/model/branchmergeproposal.py
lib/lp/code/model/tests/test_branchcollection.py
lib/lp/code/model/tests/test_branchmergeproposal.py
lib/lp/code/templates/branch-macros.pt
./lib/lp/code/interfaces/branchcollection.py
50: E301 expected 1 blank line, found 2
./lib/lp/code/model/branch.py
317: Line exceeds 78 characters.
739: Line exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/remove-decoratedbug/+merge/52003
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/remove-decoratedbug into lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2011-02-27 19:45:44 +0000
+++ lib/lp/code/browser/branch.py 2011-03-03 12:24:42 +0000
@@ -613,7 +613,7 @@
def revision_info(self):
collection = getUtility(IAllBranches).visibleByUser(self.user)
return collection.getExtendedRevisionDetails(
- self.context.latest_revisions)
+ self.user, self.context.latest_revisions)
@cachedproperty
def latest_code_import_results(self):
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2011-02-27 20:49:43 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2011-03-03 12:24:42 +0000
@@ -94,10 +94,7 @@
from lp.app.browser.tales import DateTimeFormatterAPI
from lp.code.adapters.branch import BranchMergeProposalDelta
from lp.code.browser.codereviewcomment import CodeReviewDisplayComment
-from lp.code.browser.decorations import (
- DecoratedBranch,
- DecoratedBug,
- )
+from lp.code.browser.decorations import DecoratedBranch
from lp.code.enums import (
BranchMergeProposalStatus,
BranchType,
@@ -681,18 +678,12 @@
"""Return whether or not the merge proposal has a linked bug or spec.
"""
branch = self.context.source_branch
- return branch.linked_bugs or branch.spec_links
-
- @cachedproperty
- def linked_bugs(self):
- """Return DecoratedBugs linked to the source branch."""
- return [DecoratedBug(bug, self.context.source_branch)
- for bug in self.context.related_bugs]
+ return self.linked_bugtasks or branch.spec_links
@cachedproperty
def linked_bugtasks(self):
- """Return DecoratedBugs linked to the source branch."""
- return [bug.bugtask for bug in self.linked_bugs]
+ """Return BugTasks linked to the source branch."""
+ return self.context.source_branch.getLinkedBugTasks(self.user)
@property
def edit_description_link_class(self):
=== modified file 'lib/lp/code/browser/decorations.py'
--- lib/lp/code/browser/decorations.py 2011-02-25 09:35:49 +0000
+++ lib/lp/code/browser/decorations.py 2011-03-03 12:24:42 +0000
@@ -6,16 +6,10 @@
__metaclass__ = type
__all__ = [
'DecoratedBranch',
- 'DecoratedBug',
]
-
-from collections import defaultdict
-
from lazr.delegates import delegates
-from canonical.launchpad.webapp.authorization import check_permission
-from lp.bugs.interfaces.bug import IBug
from lp.code.interfaces.branch import (
BzrIdentityMixin,
IBranch,
@@ -23,64 +17,6 @@
from lp.services.propertycache import cachedproperty
-class DecoratedBug:
- """Provide some cached attributes to a normal bug.
-
- We provide cached properties where sensible, and override default bug
- behaviour where the cached properties can be used to avoid extra database
- queries.
- """
- delegates(IBug, 'bug')
-
- def __init__(self, bug, branch, tasks=None):
- self.bug = bug
- self.branch = branch
- if tasks is None:
- tasks = self.bug.bugtasks
- self.tasks = tasks
-
- @property
- def bugtasks(self):
- """This needs to be a property rather than an attribute.
-
- If we try to assign to self.bugtasks, the lazr.delegates things we are
- trying to assign to the property of the bug.
- """
- return self.tasks
-
- @property
- def default_bugtask(self):
- """Use the first bugtask.
-
- Use the cached tasks as calling default_bugtask on the bug object
- causes a DB query.
- """
- return self.bugtasks[0]
-
- def getBugTask(self, target):
- """Get the bugtask for a specific target.
-
- This method is overridden rather than letting it fall through to the
- underlying bug as the underlying implementation gets the bugtasks from
- self, which would in that case be the normal bug model object, which
- would then hit the database to get the tasks.
- """
- # Copied from Bug.getBugTarget to avoid importing.
- for bugtask in self.bugtasks:
- if bugtask.target == target:
- return bugtask
- return None
-
- @property
- def bugtask(self):
- """Return the bugtask for the branch project, or the default bugtask.
-
- This method defers the identitication of the appropriate task to the
- branch target.
- """
- return self.branch.target.getBugTask(self)
-
-
class DecoratedBranch(BzrIdentityMixin):
"""Wrap a number of the branch accessors to cache results.
@@ -91,25 +27,6 @@
def __init__(self, branch):
self.branch = branch
- @cachedproperty
- def linked_bugs(self):
- """Override the default behaviour of the branch object.
-
- The default behaviour is just to get the bugs. We want to display the
- tasks however, and to avoid the extra database queries to get the
- tasks, we get them all at once, and provide decorated bugs (that have
- their tasks cached).
- """
- # To whomever it may concern, this function should be pushed down to
- # the model, and the related visibility checks made part of the query.
- # Alternatively it may be unused at this stage.
- bugs = defaultdict(list)
- for bug in self.branch.linked_bugs:
- bugs[bug.id].extend(bug.bugtasks)
- return [DecoratedBug(bug, self.branch, tasks)
- for bug, tasks in bugs.iteritems()
- if check_permission('launchpad.View', bug)]
-
@property
def displayname(self):
"""Override the default model property.
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2011-02-27 21:14:53 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2011-03-03 12:24:42 +0000
@@ -508,7 +508,8 @@
for x in range(0, 2):
bug = self.factory.makeBug()
mp.source_branch.linkBug(bug, branch.owner)
- linked_bug_urls.append(canonical_url(bug, rootsite='bugs'))
+ linked_bug_urls.append(
+ canonical_url(bug.default_bugtask, rootsite='bugs'))
bug_text = "Bug #%s: %s" % (bug.id, bug.title)
linked_bug_text.append(bug_text)
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2010-12-22 00:26:20 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2011-03-03 12:24:42 +0000
@@ -726,7 +726,18 @@
self.bmp.source_branch.linkBug(bug, self.bmp.registrant)
self.bmp.target_branch.linkBug(bug, self.bmp.registrant)
view = create_initialized_view(self.bmp, '+index')
- self.assertEqual([], view.linked_bugs)
+ self.assertEqual([], view.linked_bugtasks)
+
+ def test_linked_bugs_excludes_private_bugs(self):
+ """List bugs that are linked to the source only."""
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ private_bug = self.factory.makeBug(owner=person, private=True)
+ self.bmp.source_branch.linkBug(bug, self.bmp.registrant)
+ with person_logged_in(person):
+ self.bmp.source_branch.linkBug(private_bug, self.bmp.registrant)
+ view = create_initialized_view(self.bmp, '+index')
+ self.assertEqual([bug.default_bugtask], view.linked_bugtasks)
def makeRevisionGroups(self):
review_date = datetime(2009, 9, 10, tzinfo=pytz.UTC)
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2011-03-02 13:22:31 +0000
+++ lib/lp/code/configure.zcml 2011-03-03 12:24:42 +0000
@@ -248,6 +248,7 @@
votes
all_comments
related_bugs
+ getRelatedBugTasks
revision_end_date
isMergable
getComment
=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py 2011-03-03 01:13:47 +0000
+++ lib/lp/code/interfaces/branchcollection.py 2011-03-03 12:24:42 +0000
@@ -102,12 +102,16 @@
are returned.
"""
- def getExtendedRevisionDetails(revisions):
+ def getExtendedRevisionDetails(user, revisions):
"""Return information about the specified revisions on a branch.
For each revision, see if the revision resulted from merging in a
merge proposal, and if so package up the merge proposal and any linked
- bugs on the merge proposal's source branch.
+ bug tasks on the merge proposal's source branch.
+
+ :param user: The user who is making the request. Only bug tasks
+ visible to this user are returned.
+ :param revisions: The revisions we want details for.
"""
def getTeamsWithBranches(person):
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2011-02-04 16:13:13 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2011-03-03 12:24:42 +0000
@@ -296,6 +296,9 @@
def getComment(id):
"""Return the CodeReviewComment with the specified ID."""
+ def getRelatedBugTasks(user):
+ """Return the Bug tasks related to this merge proposal."""
+
def getRevisionsSinceReviewStart():
"""Return all the revisions added since the review began.
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2011-03-03 01:13:47 +0000
+++ lib/lp/code/model/branch.py 2011-03-03 12:24:42 +0000
@@ -7,6 +7,7 @@
__all__ = [
'Branch',
'BranchSet',
+ 'filter_one_task_per_bug',
]
from datetime import datetime
@@ -308,31 +309,14 @@
'Bug', joinColumn='branch', otherColumn='bug',
intermediateTable='BugBranch', orderBy='id')
- def getLinkedBugTasks(self, user, status_filter):
+ def getLinkedBugTasks(self, user, status_filter=None):
"""See `IBranch`."""
params = BugTaskSearchParams(user=user, linked_branches=self.id,
status=status_filter)
tasks = shortlist(getUtility(IBugTaskSet).search(params), 1000)
# Post process to discard irrelevant tasks: we only return one task per
# bug, and cannot easily express this in sql (yet).
- order = {}
- bugtarget = self.target.context
- # First pass calculates the order and selects the bugtasks that match
- # our target.
- # Second pass selects the earliest bugtask where the bug has no task on
- # our target.
- for task in tasks:
- if task.bug not in order:
- order[task.bug] = [len(order) + 1, None]
- if task.target == bugtarget:
- order[task.bug][1] = task
- for task in tasks:
- if order[task.bug][1] is None:
- order[task.bug][1] = task
- # Now we pull out the tasks
- result = order.values()
- result.sort()
- return [task for pos, task in result]
+ return filter_one_task_per_bug(self, tasks)
def linkBug(self, bug, registrant):
"""See `IBranch`."""
@@ -1404,3 +1388,28 @@
"""
update_trigger_modified_fields(branch)
send_branch_modified_notifications(branch, event)
+
+
+def filter_one_task_per_bug(branch, tasks):
+ """Given bug tasks for a branch, discard irrelevant ones.
+
+ Cannot easily be expressed in SQL yet, so we need this helper method.
+ """
+ order = {}
+ bugtarget = branch.target.context
+ # First pass calculates the order and selects the bugtasks that match
+ # our target.
+ # Second pass selects the earliest bugtask where the bug has no task on
+ # our target.
+ for task in tasks:
+ if task.bug not in order:
+ order[task.bug] = [len(order) + 1, None]
+ if task.target == bugtarget:
+ order[task.bug][1] = task
+ for task in tasks:
+ if order[task.bug][1] is None:
+ order[task.bug][1] = task
+ # Now we pull out the tasks
+ result = order.values()
+ result.sort()
+ return [task for pos, task in result]
=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py 2011-03-03 03:04:30 +0000
+++ lib/lp/code/model/branchcollection.py 2011-03-03 12:24:42 +0000
@@ -26,15 +26,21 @@
from canonical.launchpad.components.decoratedresultset import (
DecoratedResultSet,
)
+from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.launchpad.webapp.interfaces import (
DEFAULT_FLAVOR,
IStoreSelector,
MAIN_STORE,
)
+from canonical.launchpad.searchbuilder import any
from canonical.launchpad.webapp.vocabulary import CountableIterator
from canonical.lazr.utils import safe_hasattr
-from lp.bugs.model.bug import Bug
+from lp.bugs.interfaces.bugtask import (
+ IBugTaskSet,
+ BugTaskSearchParams,
+ )
from lp.bugs.model.bugbranch import BugBranch
+from lp.bugs.model.bugtask import BugTask
from lp.code.interfaces.branch import user_has_special_branch_access
from lp.code.interfaces.branchcollection import (
IBranchCollection,
@@ -46,7 +52,10 @@
from lp.code.enums import BranchMergeProposalStatus
from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
-from lp.code.model.branch import Branch
+from lp.code.model.branch import (
+ Branch,
+ filter_one_task_per_bug,
+ )
from lp.code.model.branchmergeproposal import BranchMergeProposal
from lp.code.model.branchsubscription import BranchSubscription
from lp.code.model.codereviewcomment import CodeReviewComment
@@ -158,6 +167,7 @@
resultset = self.store.using(*tables).find(Branch, *expressions)
if not eager_load:
return resultset
+
def do_eager_load(rows):
branch_ids = set(branch.id for branch in rows)
if not branch_ids:
@@ -254,23 +264,24 @@
proposals.order_by(Desc(CodeReviewComment.vote))
return proposals
- def getExtendedRevisionDetails(self, revisions):
+ def getExtendedRevisionDetails(self, user, revisions):
"""See `IBranchCollection`."""
if not revisions:
return []
branch = revisions[0].branch
- def make_rev_info(branch_revision, merge_proposal_revs, linked_bugs):
+ def make_rev_info(
+ branch_revision, merge_proposal_revs, linked_bugtasks):
rev_info = {
'revision': branch_revision,
- 'linked_bugs': None,
+ 'linked_bugtasks': None,
'merge_proposal': None,
}
merge_proposal = merge_proposal_revs.get(branch_revision.sequence)
rev_info['merge_proposal'] = merge_proposal
if merge_proposal is not None:
- rev_info['linked_bugs'] = linked_bugs.get(
+ rev_info['linked_bugtasks'] = linked_bugtasks.get(
merge_proposal.source_branch.id)
return rev_info
@@ -281,19 +292,40 @@
merge_proposal_revs = dict(
[(mp.merged_revno, mp) for mp in merge_proposals])
source_branch_ids = [mp.source_branch.id for mp in merge_proposals]
-
- filter = [
- BugBranch.bug == Bug.id,
- BugBranch.branchID.is_in(
- source_branch_ids),
- ]
- bugs = self.store.find((Bug, BugBranch), filter)
- linked_bugs = defaultdict(list)
- for (bug, bugbranch) in bugs:
- linked_bugs[bugbranch.branchID].append(bug)
+ linked_bugtasks = defaultdict(list)
+
+ if source_branch_ids:
+ # We get the bugtasks for our merge proposal branches
+
+ # First, the bug ids
+ params = BugTaskSearchParams(
+ user=user, status=None,
+ linked_branches=any(*source_branch_ids))
+ bug_ids = getUtility(IBugTaskSet).searchBugIds(params)
+
+ # Then the bug tasks and branches
+ store = IStore(BugBranch)
+ rs = store.using(
+ BugBranch,
+ Join(BugTask, BugTask.bugID == BugBranch.bugID),
+ ).find(
+ (BugTask, BugBranch),
+ BugBranch.bugID.is_in(bug_ids),
+ BugBranch.branchID.is_in(source_branch_ids)
+ )
+
+ # Build up a collection of bugtasks for each branch
+ bugtasks_for_branch = defaultdict(list)
+ for bugtask, bugbranch in rs:
+ bugtasks_for_branch[bugbranch.branch].append(bugtask)
+
+ # Now filter those down to one bugtask per branch
+ for branch, tasks in bugtasks_for_branch.iteritems():
+ linked_bugtasks[branch.id].extend(
+ filter_one_task_per_bug(branch, tasks))
return [make_rev_info(
- rev, merge_proposal_revs, linked_bugs)
+ rev, merge_proposal_revs, linked_bugtasks)
for rev in revisions]
def getTeamsWithBranches(self, person):
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2011-01-27 00:31:44 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2011-03-03 12:24:42 +0000
@@ -242,8 +242,22 @@
Implies that these bugs would be fixed, in the target, by the merge.
"""
- return (bug for bug in self.source_branch.linked_bugs
- if bug not in self.target_branch.linked_bugs)
+ from operator import attrgetter
+ bugtasks = self.getRelatedBugTasks(self.registrant)
+ bugs = set()
+ for bugtask in bugtasks:
+ bugs.add(bugtask.bug)
+ return sorted(bugs, attrgetter('id'))
+
+ def getRelatedBugTasks(self, user):
+ """Bug tasks which are linked to the source but not the target.
+
+ Implies that these would be fixed, in the target, by the merge.
+ """
+ source_tasks = self.source_branch.getLinkedBugTasks(user)
+ target_tasks = self.target_branch.getLinkedBugTasks(user)
+ return (bugtask
+ for bugtask in source_tasks if bugtask not in target_tasks)
@property
def address(self):
=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py 2011-01-27 14:02:38 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py 2011-03-03 12:24:42 +0000
@@ -613,17 +613,14 @@
self.all_branches = getUtility(IAllBranches)
def test_empty_revisions(self):
- rev_details = self.all_branches.getExtendedRevisionDetails([])
- self.assertEqual([], rev_details)
- rev_details = self.all_branches.getExtendedRevisionDetails(None)
- self.assertEqual([], rev_details)
-
- def test_some_revisions(self):
- branch = self.factory.makeBranch()
- merge_proposals = [
- self.factory.makeBranchMergeProposal(target_branch=branch)
- for x in range(0, 2)]
-
+ person = self.factory.makePerson()
+ rev_details = self.all_branches.getExtendedRevisionDetails(person, [])
+ self.assertEqual([], rev_details)
+ rev_details = self.all_branches.getExtendedRevisionDetails(
+ person, None)
+ self.assertEqual([], rev_details)
+
+ def _makeBranchRevisions(self, merge_proposals, branch):
expected_rev_details = []
with person_logged_in(branch.owner):
self.factory.makeRevisionsForBranch(branch, 3)
@@ -632,7 +629,7 @@
branch_revision = branch_revisions[x]
rev_info = {
'revision': branch_revision,
- 'linked_bugs': None,
+ 'linked_bugtasks': None,
'merge_proposal': None,
}
if x < len(merge_proposals):
@@ -640,20 +637,64 @@
branch_revision.sequence)
rev_info['merge_proposal'] = merge_proposals[x]
expected_rev_details.append(rev_info)
+ return expected_rev_details, branch_revisions
+
+ def test_some_revisions_with_no_bugs(self):
+ branch = self.factory.makeBranch()
+ merge_proposals = [
+ self.factory.makeBranchMergeProposal(target_branch=branch)
+ for x in range(0, 2)]
+
+ expected_rev_details, branch_revisions = (
+ self._makeBranchRevisions(merge_proposals, branch))
result = self.all_branches.getExtendedRevisionDetails(
- branch_revisions)
+ branch.owner, branch_revisions)
self.assertEqual(sorted(expected_rev_details), sorted(result))
- linked_bugs = []
+ def test_some_revisions_with_bugs(self):
+ branch = self.factory.makeBranch()
+ merge_proposals = [
+ self.factory.makeBranchMergeProposal(target_branch=branch)
+ for x in range(0, 2)]
+
+ expected_rev_details, branch_revisions = (
+ self._makeBranchRevisions(merge_proposals, branch))
+
+ linked_bugtasks = []
with person_logged_in(branch.owner):
for x in range(0, 2):
bug = self.factory.makeBug()
merge_proposals[0].source_branch.linkBug(bug, branch.owner)
- linked_bugs.append(bug)
- expected_rev_details[0]['linked_bugs'] = linked_bugs
- result = self.all_branches.getExtendedRevisionDetails(
- branch_revisions)
+ linked_bugtasks.append(bug.default_bugtask)
+ expected_rev_details[0]['linked_bugtasks'] = linked_bugtasks
+ result = self.all_branches.getExtendedRevisionDetails(
+ branch.owner, branch_revisions)
+ self.assertEqual(sorted(expected_rev_details), sorted(result))
+
+ def test_some_revisions_with_private_bugs(self):
+ branch = self.factory.makeBranch()
+ merge_proposals = [
+ self.factory.makeBranchMergeProposal(target_branch=branch)
+ for x in range(0, 2)]
+
+ expected_rev_details, branch_revisions = (
+ self._makeBranchRevisions(merge_proposals, branch))
+
+ linked_bugtasks = []
+ with person_logged_in(branch.owner):
+ for x in range(0, 4):
+ private = x%2
+ bug = self.factory.makeBug(
+ owner=branch.owner, private=private)
+ merge_proposals[0].source_branch.linkBug(bug, branch.owner)
+ if not private:
+ linked_bugtasks.append(bug.default_bugtask)
+ expected_rev_details[0]['linked_bugtasks'] = linked_bugtasks
+
+ person = self.factory.makePerson()
+ result = self.all_branches.getExtendedRevisionDetails(
+ person, branch_revisions)
self.assertEqual(sorted(expected_rev_details), sorted(result))
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py 2011-01-27 00:31:44 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py 2011-03-03 12:24:42 +0000
@@ -3,8 +3,6 @@
# pylint: disable-msg=F0401
-from __future__ import with_statement
-
"""Tests for BranchMergeProposals."""
__metaclass__ = type
@@ -28,10 +26,7 @@
from zope.security.proxy import removeSecurityProxy
from canonical.database.constants import UTC_NOW
-from canonical.launchpad.ftests import (
- import_secret_test_key,
- syncUpdate,
- )
+from canonical.launchpad.ftests import import_secret_test_key
from canonical.launchpad.interfaces.launchpad import IPrivacy
from canonical.launchpad.interfaces.message import IMessageJob
from canonical.launchpad.webapp import canonical_url
@@ -83,7 +78,6 @@
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProductSet
from lp.testing import (
- ANONYMOUS,
login,
login_person,
person_logged_in,
@@ -1272,6 +1266,46 @@
bmp.target_branch.linkBug(bug, bmp.registrant)
self.assertEqual([], list(bmp.related_bugs))
+ def test_related_bugtasks_includes_source_bugtasks(self):
+ """related_bugtasks includes bugtasks linked to the source branch."""
+ bmp = self.factory.makeBranchMergeProposal()
+ source_branch = bmp.source_branch
+ bug = self.factory.makeBug()
+ source_branch.linkBug(bug, bmp.registrant)
+ self.assertEqual(
+ bug.bugtasks, list(bmp.getRelatedBugTasks(self.user)))
+
+ def test_related_bugtasks_excludes_target_bugs(self):
+ """related_bugtasks ignores bugs linked to the source branch."""
+ bmp = self.factory.makeBranchMergeProposal()
+ bug = self.factory.makeBug()
+ bmp.target_branch.linkBug(bug, bmp.registrant)
+ self.assertEqual([], list(bmp.getRelatedBugTasks(self.user)))
+
+ def test_related_bugtasks_excludes_mutual_bugs(self):
+ """related_bugtasks ignores bugs linked to both branches."""
+ bmp = self.factory.makeBranchMergeProposal()
+ bug = self.factory.makeBug()
+ bmp.source_branch.linkBug(bug, bmp.registrant)
+ bmp.target_branch.linkBug(bug, bmp.registrant)
+ self.assertEqual([], list(bmp.getRelatedBugTasks(self.user)))
+
+ def test_related_bugtasks_excludes_private_bugs(self):
+ """related_bugtasks ignores private bugs for non-authorised users."""
+ bmp = self.factory.makeBranchMergeProposal()
+ bug = self.factory.makeBug()
+ bmp.source_branch.linkBug(bug, bmp.registrant)
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ private_bug = self.factory.makeBug(private=True, owner=person)
+ bmp.source_branch.linkBug(private_bug, person)
+ self.assertEqual(
+ bug.bugtasks, list(bmp.getRelatedBugTasks(self.user)))
+ all_bugtasks = list(bug.bugtasks)
+ all_bugtasks.extend(private_bug.bugtasks)
+ self.assertEqual(
+ all_bugtasks, list(bmp.getRelatedBugTasks(person)))
+
class TestNotifyModified(TestCaseWithFactory):
=== modified file 'lib/lp/code/templates/branch-macros.pt'
--- lib/lp/code/templates/branch-macros.pt 2011-02-25 05:12:46 +0000
+++ lib/lp/code/templates/branch-macros.pt 2011-03-03 12:24:42 +0000
@@ -179,7 +179,7 @@
revisions - the revisions to list.
or
revision_info - extended revision information (revision,
- merge_proposal, linked_bugs) to list.
+ merge_proposal, linked_bugtasks) to list.
</tal:comment>
<style type="text/css">
@@ -214,7 +214,7 @@
rev_no - the branch revision to be displayed.
or
rev_info - a dict of the branch revision information (revision,
- merge_proposal, linked_bugs) to be displayed.
+ merge_proposal, linked_bugtasks) to be displayed.
codebrowse - the branch's codebrowse root.
It is expected that this macro is called from within a definition
@@ -263,12 +263,12 @@
Merged branch <a tal:attributes="href merge_proposal/fmt:url"
tal:content="merge_proposal/source_branch/displayname">source branch</a>
<tal:linkedbugs
- define="linked_bugs python:rev_info['linked_bugs']"
- condition="linked_bugs">with linked bugs
+ define="linked_bugtasks python:rev_info['linked_bugtasks']"
+ condition="linked_bugtasks">
<dl>
- <tal:bug repeat="bug linked_bugs">
+ <tal:bug repeat="bugtask linked_bugtasks">
<dd class="subordinate revision-comment">
- <tal:bug-link replace="structure bug/fmt:link"/>
+ <tal:bug-link replace="structure bugtask/fmt:link"/>
</dd>
</tal:bug>
</dl>
