launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19431
[Merge] lp:~wgrant/launchpad/buglinktarget-no-links into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/buglinktarget-no-links into lp:launchpad.
Commit message:
Factor IBugLinkTarget.bug_links away. The link objects are marked for death.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/buglinktarget-no-links/+merge/272356
Factor IBugLinkTarget.bug_links away. The link objects are marked for death.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/buglinktarget-no-links into lp:launchpad.
=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py 2015-07-21 09:04:01 +0000
+++ lib/lp/answers/browser/question.py 2015-09-25 08:57:18 +0000
@@ -1099,11 +1099,11 @@
@property
def original_bug(self):
"""Return the bug that the question was created from or None."""
- for buglink in self.context.bug_links:
- if (check_permission('launchpad.View', buglink.bug)
- and buglink.bug.owner == self.context.owner
- and buglink.bug.datecreated == self.context.datecreated):
- return buglink.bug
+ for bug in self.context.bugs:
+ if (check_permission('launchpad.View', bug)
+ and bug.owner == self.context.owner
+ and bug.datecreated == self.context.datecreated):
+ return bug
return None
=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml 2015-03-13 06:00:30 +0000
+++ lib/lp/answers/configure.zcml 2015-09-25 08:57:18 +0000
@@ -129,7 +129,7 @@
set_attributes="priority whiteboard"
/>
<!-- IBugLinkTarget -->
- <allow attributes="bugs bug_links" />
+ <allow attributes="bugs" />
<require
permission="launchpad.AnyPerson"
attributes="linkBug unlinkBug"
=== modified file 'lib/lp/answers/doc/expiration.txt'
--- lib/lp/answers/doc/expiration.txt 2011-12-30 06:14:56 +0000
+++ lib/lp/answers/doc/expiration.txt 2015-09-25 08:57:18 +0000
@@ -106,7 +106,7 @@
['Unknown', 'Invalid']
>>> bug_link_question = questionset.get(11)
>>> bug_link_question.linkBug(fixed_bug)
- <QuestionBug at ...>
+ True
# A question linked to an Invalid bug; it is expirable.
>>> invalid_bug = getUtility(IBugSet).get(10)
@@ -116,7 +116,7 @@
'Invalid'
>>> invalid_bug_question = questionset.get(12)
>>> invalid_bug_question.linkBug(invalid_bug)
- <QuestionBug at ...>
+ True
# Commit the current transaction because the script will run in
# another transaction and thus it won't see the changes done on this
=== modified file 'lib/lp/answers/doc/notifications.txt'
--- lib/lp/answers/doc/notifications.txt 2011-07-22 07:13:16 +0000
+++ lib/lp/answers/doc/notifications.txt 2015-09-25 08:57:18 +0000
@@ -197,7 +197,7 @@
... comment=ubuntu_question.description)
>>> bug = ubuntu_question.target.createBug(params)
>>> ubuntu_question.linkBug(bug)
- <QuestionBug...>
+ True
>>> notify(ObjectModifiedEvent(
... ubuntu_question, unmodified_question, ['bugs']))
@@ -224,7 +224,7 @@
>>> unmodified_question = Snapshot(ubuntu_question,
... providing=providedBy(ubuntu_question))
>>> ubuntu_question.unlinkBug(bug)
- <QuestionBug...>
+ True
>>> notify(ObjectModifiedEvent(
... ubuntu_question, unmodified_question, ['bugs']))
=== modified file 'lib/lp/answers/doc/question.txt'
--- lib/lp/answers/doc/question.txt 2015-08-26 12:29:01 +0000
+++ lib/lp/answers/doc/question.txt 2015-09-25 08:57:18 +0000
@@ -358,14 +358,14 @@
>>> bug7.isSubscribed(firefox_question.owner)
False
>>> firefox_question.linkBug(bug7)
- <QuestionBug...>
+ True
>>> bug7.isSubscribed(firefox_question.owner)
True
When the link is removed, the owner is unsubscribed.
>>> firefox_question.unlinkBug(bug7)
- <QuestionBug...>
+ True
>>> bug7.isSubscribed(firefox_question.owner)
False
=== modified file 'lib/lp/answers/doc/questiontarget.txt'
--- lib/lp/answers/doc/questiontarget.txt 2013-05-01 00:23:31 +0000
+++ lib/lp/answers/doc/questiontarget.txt 2015-09-25 08:57:18 +0000
@@ -559,8 +559,8 @@
>>> print question_message.text_contents
This is really a question.
- >>> for bug_link in target_question.bug_links:
- ... print bug_link.bug.title
+ >>> for bug in target_question.bugs:
+ ... print bug.title
Print is broken
>>> print target_question.messages[-1].text_contents
This is really a question.
=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py 2015-08-25 16:24:06 +0000
+++ lib/lp/answers/model/question.py 2015-09-25 08:57:18 +0000
@@ -210,8 +210,6 @@
subscribers = SQLRelatedJoin('Person',
joinColumn='question', otherColumn='person',
intermediateTable='QuestionSubscription', orderBy='name')
- bug_links = SQLMultipleJoin('QuestionBug',
- joinColumn='question', orderBy='id')
bugs = SQLRelatedJoin('Bug', joinColumn='question', otherColumn='bug',
intermediateTable='QuestionBug', orderBy='id')
messages = SQLMultipleJoin('QuestionMessage', joinColumn='question',
@@ -671,19 +669,23 @@
def unlinkBug(self, bug):
"""See `IBugLinkTarget`."""
- buglink = BugLinkTargetMixin.unlinkBug(self, bug)
- if buglink:
- # Additionnaly, unsubscribe the question's owner to the bug
+ unlinked = BugLinkTargetMixin.unlinkBug(self, bug)
+ if unlinked:
+ # Additionally, unsubscribe the question's owner from the bug
bug.unsubscribe(self.owner, self.owner)
- return buglink
-
- # Template methods for BugLinkTargetMixin.
- buglinkClass = QuestionBug
+ return unlinked
def createBugLink(self, bug):
"""See BugLinkTargetMixin."""
return QuestionBug(question=self, bug=bug)
+ def deleteBugLink(self, bug):
+ """See BugLinkTargetMixin."""
+ link = Store.of(self).find(QuestionBug, question=self, bug=bug).one()
+ if link is not None:
+ Store.of(link).remove(link)
+ return link
+
def setCommentVisibility(self, user, comment_number, visible):
"""See `IQuestion`."""
question_message = self.messages[comment_number]
=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml 2015-03-01 22:40:24 +0000
+++ lib/lp/blueprints/configure.zcml 2015-09-25 08:57:18 +0000
@@ -189,8 +189,7 @@
set_attributes="priority direction_approved"/>
<require
permission="launchpad.LimitedView"
- attributes="bugs
- bug_links"/>
+ attributes="bugs"/>
<require
permission="launchpad.AnyAllowedPerson"
attributes="linkBug
=== modified file 'lib/lp/blueprints/doc/specification.txt'
--- lib/lp/blueprints/doc/specification.txt 2014-06-17 06:13:08 +0000
+++ lib/lp/blueprints/doc/specification.txt 2015-09-25 08:57:18 +0000
@@ -132,7 +132,7 @@
>>> ubuspec.assignee = jdub
>>> ubuspec.drafter = jdub
>>> ubuspec.linkBug(getUtility(IBugSet).get(2))
- <...>
+ True
>>> delta = ubuspec.getDelta(unmodified_spec, jdub)
>>> delta.specification == ubuspec
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2015-07-08 16:05:11 +0000
+++ lib/lp/blueprints/model/specification.py 2015-09-25 08:57:18 +0000
@@ -239,8 +239,6 @@
sprints = SQLRelatedJoin('Sprint', orderBy='name',
joinColumn='specification', otherColumn='sprint',
intermediateTable='SprintSpecification')
- bug_links = SQLMultipleJoin(
- 'SpecificationBug', joinColumn='specification', orderBy='id')
bugs = SQLRelatedJoin('Bug',
joinColumn='specification', otherColumn='bug',
intermediateTable='SpecificationBug', orderBy='id')
@@ -793,13 +791,18 @@
return bool(self.subscription(person))
- # Template methods for BugLinkTargetMixin
- buglinkClass = SpecificationBug
-
def createBugLink(self, bug):
"""See BugLinkTargetMixin."""
return SpecificationBug(specification=self, bug=bug)
+ def deleteBugLink(self, bug):
+ """See BugLinkTargetMixin."""
+ link = Store.of(self).find(
+ SpecificationBug, specification=self, bug=bug).one()
+ if link is not None:
+ Store.of(link).remove(link)
+ return link
+
# sprint linking
def linkSprint(self, sprint, user):
"""See ISpecification."""
=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py 2015-02-16 13:01:34 +0000
+++ lib/lp/blueprints/tests/test_specification.py 2015-09-25 08:57:18 +0000
@@ -164,7 +164,7 @@
'id', 'information_type', 'private', 'userCanView')),
'launchpad.LimitedView': set((
'all_blocked', 'all_deps', 'approver', 'approverID',
- 'assignee', 'assigneeID', 'bug_links', 'bugs', 'completer',
+ 'assignee', 'assigneeID', 'bugs', 'completer',
'createDependency', 'date_completed', 'date_goal_decided',
'date_goal_proposed', 'date_started', 'datecreated',
'definition_status', 'dependencies', 'direction_approved',
=== modified file 'lib/lp/bugs/browser/buglinktarget.py'
--- lib/lp/bugs/browser/buglinktarget.py 2014-11-29 01:33:59 +0000
+++ lib/lp/bugs/browser/buglinktarget.py 2015-09-25 08:57:18 +0000
@@ -97,7 +97,7 @@
"""
# Do a regular search to get the bugtasks so that visibility is
# evaluated and eager loading is performed.
- bug_ids = map(attrgetter('bugID'), self.context.bug_links)
+ bug_ids = [bug.id for bug in self.context.bugs]
if not bug_ids:
return []
bugtask_set = getUtility(IBugTaskSet)
=== modified file 'lib/lp/bugs/interfaces/buglink.py'
--- lib/lp/bugs/interfaces/buglink.py 2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/interfaces/buglink.py 2015-09-25 08:57:18 +0000
@@ -27,7 +27,6 @@
)
from zope.schema import (
Choice,
- List,
Object,
Set,
)
@@ -66,24 +65,23 @@
CollectionField(title=_("Bugs related to this object."),
value_type=Reference(schema=IBug), readonly=True),
as_of="devel")
- bug_links = List(title=_("The links between bugs and this object."),
- value_type=Object(schema=IBugLink), readonly=True)
def linkBug(bug):
- """Link the object with this bug. If the object is already linked,
- return the old linker, otherwise return a new IBugLink object.
-
- If a new IBugLink is created by this method, a ObjectCreatedEvent
- should be sent.
+ """Link the object with this bug.
+
+ If a new IBugLink is created by this method, an ObjectCreatedEvent
+ is sent.
+
+ :return: True if a new link was created, False if it already existed.
"""
def unlinkBug(bug):
- """Remove any link between this object and the bug. If the bug wasn't
- linked to the target, returns None otherwise returns the IBugLink
- object which was removed.
-
- If an IBugLink is removed by this method, a ObjectDeletedEvent
- should be sent.
+ """Remove any link between this object and the bug.
+
+ If an IBugLink is removed by this method, an ObjectDeletedEvent
+ is sent.
+
+ :return: True if a link was deleted, False if it didn't exist.
"""
=== modified file 'lib/lp/bugs/model/buglinktarget.py'
--- lib/lp/bugs/model/buglinktarget.py 2013-01-07 02:40:55 +0000
+++ lib/lp/bugs/model/buglinktarget.py 2015-09-25 08:57:18 +0000
@@ -17,17 +17,16 @@
class BugLinkTargetMixin:
"""Mixin class for IBugLinkTarget implementation."""
- @property
- def buglinkClass(self):
- """Subclass should override this property to return the database
- class used for IBugLink."""
- raise NotImplementedError("missing buglinkClass() implementation")
-
def createBugLink(self, bug):
"""Subclass should override that method to create a BugLink instance.
"""
raise NotImplementedError("missing createBugLink() implementation")
+ def deleteBugLink(self, bug):
+ """Subclass should override that method to delete a BugLink instance.
+ """
+ raise NotImplementedError("missing deleteBugLink() implementation")
+
# IBugLinkTarget implementation
def linkBug(self, bug):
"""See IBugLinkTarget."""
@@ -40,12 +39,11 @@
if not check_permission('launchpad.View', bug):
raise Unauthorized(
"cannot link to a private bug you don't have access to")
- for buglink in self.bug_links:
- if buglink.bug.id == bug.id:
- return buglink
+ if bug in self.bugs:
+ return False
buglink = self.createBugLink(bug)
notify(ObjectCreatedEvent(buglink))
- return buglink
+ return True
def unlinkBug(self, bug):
"""See IBugLinkTarget."""
@@ -60,10 +58,8 @@
"cannot unlink a private bug you don't have access to")
# see if a relevant bug link exists, and if so, delete it
- for buglink in self.bug_links:
- if buglink.bug.id == bug.id:
- notify(ObjectDeletedEvent(buglink))
- self.buglinkClass.delete(buglink.id)
- # XXX: Bjorn Tillenius 2005-11-21: We shouldn't return the
- # object that we just deleted from the db.
- return buglink
+ buglink = self.deleteBugLink(bug)
+ if buglink is not None:
+ notify(ObjectDeletedEvent(buglink))
+ return True
+ return False
=== modified file 'lib/lp/bugs/model/cve.py'
--- lib/lp/bugs/model/cve.py 2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/model/cve.py 2015-09-25 08:57:18 +0000
@@ -85,13 +85,17 @@
assert ref.cve == self
CveReference.delete(ref.id)
- # Template methods for BugLinkTargetMixin
- buglinkClass = BugCve
-
def createBugLink(self, bug):
"""See BugLinkTargetMixin."""
return BugCve(cve=self, bug=bug)
+ def deleteBugLink(self, bug):
+ """See BugLinkTargetMixin."""
+ link = Store.of(self).find(BugCve, cve=self, bug=bug).one()
+ if link is not None:
+ Store.of(link).remove(link)
+ return link
+
@implementer(ICveSet)
class CveSet:
=== modified file 'lib/lp/bugs/tests/buglinktarget.txt'
--- lib/lp/bugs/tests/buglinktarget.txt 2011-12-30 07:38:46 +0000
+++ lib/lp/bugs/tests/buglinktarget.txt 2015-09-25 08:57:18 +0000
@@ -31,23 +31,17 @@
>>> bug1 = bugset.get(1)
The linkBug() method is used to link a bug to the target. It takes as
-parameter the bug which should be linked. The method should return the
-IBugLink that was created.
-
- >>> link1 = target.linkBug(bug1)
-
- >>> verifyObject(IBugLink, link1)
- True
- >>> link1.target == target
- True
- >>> link1.bug == bug1
+parameter the bug which should be linked. The method returns True if a
+new link was created.
+
+ >>> target.linkBug(bug1)
True
When the bug was already linked to the target, the existing link should
be used.
- >>> target.linkBug(bug1) == link1
- True
+ >>> target.linkBug(bug1)
+ False
When a IBugLink is created, one IObjectCreatedEvent for the created
should be fired by the method.
@@ -61,15 +55,16 @@
... lambda object, event: created_events.append(event))
>>> bug2 = bugset.get(2)
- >>> link2 = target.linkBug(bugset.get(2))
- >>> created_events[-1].object == link2
+ >>> target.linkBug(bugset.get(2))
+ True
+ >>> created_events[-1].object.bug == bug2
True
Of course, if no new IBugLink is created, no events should be fired:
>>> created_events = []
- >>> target.linkBug(bug2) == link2
- True
+ >>> target.linkBug(bug2)
+ False
>>> created_events
[]
@@ -104,14 +99,6 @@
>>> [bug.id for bug in target.bugs]
[1, 2, 6]
-== bug_links ==
-
-The IBugLink objects available on the target should be available in the
-bug_links attribute:
-
- >>> [link.bug.id for link in target.bug_links]
- [1, 2, 6]
-
== unlinkBug() ==
The unlinkBug() method is used to remove a link between a bug and
@@ -135,20 +122,20 @@
... IBugLink, IObjectDeletedEvent,
... lambda object, event: deleted_events.append(event))
- >>> target.unlinkBug(bug1) == link1
+ >>> target.unlinkBug(bug1)
True
- >>> deleted_events[-1].object == link1
+ >>> deleted_events[-1].object.bug == bug1
True
>>> [bug.id for bug in target.bugs]
[2, 6]
When the bug was not linked to the target, that method should return
-None (and not trigger any events):
+False (and not trigger any events):
>>> deleted_events = []
- >>> target.unlinkBug(bug1) is None
- True
+ >>> target.unlinkBug(bug1)
+ False
>>> deleted_events
[]
=== modified file 'lib/lp/bugs/tests/bugtarget-questiontarget.txt'
--- lib/lp/bugs/tests/bugtarget-questiontarget.txt 2013-11-30 00:03:25 +0000
+++ lib/lp/bugs/tests/bugtarget-questiontarget.txt 2015-09-25 08:57:18 +0000
@@ -164,7 +164,7 @@
>>> bug.title
u'Print is broken'
- >>> [bug_link.question.title for bug_link in question.bug_links]
+ >>> [bug.title for bug in question.bugs]
[u'Print is broken']
>>> [question.title for question in bug.questions]
Follow ups
