Review: Approve code
Diff comments: > === modified file 'lib/lp/blueprints/configure.zcml' > --- lib/lp/blueprints/configure.zcml 2014-11-23 21:37:40 +0000 > +++ lib/lp/blueprints/configure.zcml 2015-03-02 00:02:56 +0000 > @@ -87,6 +87,14 @@ > > interface="lp.blueprints.interfaces.specificationworkitem.ISpecificationWorkItem"/> > </class> > > + <!-- SpecificationWorkItemSet --> > + > + <securedutility > + > class="lp.blueprints.model.specificationworkitem.SpecificationWorkItemSet" > + > provides="lp.blueprints.interfaces.specificationworkitem.ISpecificationWorkItemSet"> > + <allow > interface="lp.blueprints.interfaces.specificationworkitem.ISpecificationWorkItemSet"/> > + </securedutility> > + > <!-- SpecificationDependency --> > > <class > class="lp.blueprints.model.specificationdependency.SpecificationDependency"> > > === modified file 'lib/lp/blueprints/interfaces/specificationworkitem.py' > --- lib/lp/blueprints/interfaces/specificationworkitem.py 2012-04-05 > 13:05:04 +0000 > +++ lib/lp/blueprints/interfaces/specificationworkitem.py 2015-03-02 > 00:02:56 +0000 > @@ -7,6 +7,7 @@ > > __all__ = [ > 'ISpecificationWorkItem', > + 'ISpecificationWorkItemSet', > ] > > > @@ -80,3 +81,10 @@ > description=_( > "True or False depending on whether or not there is more " > "work required on this work item.")) > + > + > +class ISpecificationWorkItemSet(Interface): > + """SpecificationWorkItem's public attributes and methods.""" I think not. > + > + def unlinkMilestone(milestone): > + """Unlink the given milestone from all SpecificationWorkItems.""" > > === modified file 'lib/lp/blueprints/model/specificationworkitem.py' > --- lib/lp/blueprints/model/specificationworkitem.py 2012-11-16 20:13:32 > +0000 > +++ lib/lp/blueprints/model/specificationworkitem.py 2015-03-02 00:02:56 > +0000 > @@ -12,11 +12,13 @@ > Reference, > Unicode, > ) > +from storm.store import Store > from zope.interface import implements > > from lp.blueprints.enums import SpecificationWorkItemStatus > from lp.blueprints.interfaces.specificationworkitem import ( > ISpecificationWorkItem, > + ISpecificationWorkItemSet, > ) > from lp.registry.interfaces.person import validate_public_person > from lp.services.database.constants import DEFAULT > @@ -65,3 +67,12 @@ > def is_complete(self): > """See `ISpecificationWorkItem`.""" > return self.status == SpecificationWorkItemStatus.DONE > + > + > +class SpecificationWorkItemSet: > + implements(ISpecificationWorkItemSet) > + > + def unlinkMilestone(self, milestone): """See `ISpecificationWorkItemSet`.""" > + Store.of(milestone).find( > + SpecificationWorkItem, milestone_id=milestone.id).set( > + milestone_id=None) > > === modified file 'lib/lp/blueprints/model/tests/test_specification.py' > --- lib/lp/blueprints/model/tests/test_specification.py 2014-06-19 > 10:04:55 +0000 > +++ lib/lp/blueprints/model/tests/test_specification.py 2015-03-02 > 00:02:56 +0000 > @@ -24,6 +24,7 @@ > from lp.app.validators import LaunchpadValidationError > from lp.blueprints.interfaces.specification import ISpecification > from lp.blueprints.interfaces.specificationworkitem import ( > + ISpecificationWorkItemSet, > SpecificationWorkItemStatus, > ) > from lp.blueprints.model.specificationworkitem import SpecificationWorkItem > @@ -659,6 +660,24 @@ > title=wi.title, status=wi.status, assignee=wi.assignee, > milestone=wi.milestone, sequence=sequence) > > + def test_workitemspecificationset_can_unlink_milestones(self): > + milestone_a = self.factory.makeMilestone() > + milestone_b = self.factory.makeMilestone() > + work_item_1 = > self.factory.makeSpecificationWorkItem(milestone=milestone_a) > + work_item_2 = > self.factory.makeSpecificationWorkItem(milestone=milestone_a) > + work_item_3 = > self.factory.makeSpecificationWorkItem(milestone=milestone_b) > + > + > + self.assertEqual(work_item_1.milestone, milestone_a) > + self.assertEqual(work_item_2.milestone, milestone_a) > + self.assertEqual(work_item_3.milestone, milestone_b) > + > + getUtility(ISpecificationWorkItemSet).unlinkMilestone(milestone_a) > + > + self.assertIs(work_item_1.milestone, None) > + self.assertIs(work_item_2.milestone, None) > + self.assertEqual(work_item_3.milestone, milestone_b) Can you flip the assertion arguments? """ Help on method assertEqual in module testtools.testcase: assertEqual(self, expected, observed, message='') unbound testtools.testcase.TestCase method """ > + > > class TestSpecificationInformationType(TestCaseWithFactory): > > > === modified file 'lib/lp/registry/browser/__init__.py' > --- lib/lp/registry/browser/__init__.py 2014-01-30 15:04:06 +0000 > +++ lib/lp/registry/browser/__init__.py 2015-03-02 00:02:56 +0000 > @@ -30,7 +30,7 @@ > LaunchpadEditFormView, > ) > from lp.app.interfaces.launchpad import ILaunchpadCelebrities > -from lp.blueprints.model.specificationworkitem import SpecificationWorkItem > +from lp.blueprints.interfaces.specificationworkitem import > ISpecificationWorkItemSet > from lp.bugs.interfaces.bugtask import IBugTaskSet > from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams > from lp.registry.interfaces.productseries import IProductSeries > @@ -241,9 +241,7 @@ > else: > nb.milestone = None > > removeSecurityProxy(milestone.all_specifications).set(milestoneID=None) > - Store.of(milestone).find( > - SpecificationWorkItem, milestone_id=milestone.id).set( > - milestone_id=None) > + getUtility(ISpecificationWorkItemSet).unlinkMilestone(milestone) > self._deleteRelease(milestone.product_release) > milestone.destroySelf() > > -- https://code.launchpad.net/~thomir/launchpad/devel-fix-import-violations-specificationworkitem/+merge/251395 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp

