launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21454
[Merge] lp:~cjwatson/launchpad/sprint-permissions into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/sprint-permissions into lp:launchpad.
Commit message:
Set up model permissions on Sprint. Give registry experts permission to delete sprints.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/sprint-permissions/+merge/322282
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/sprint-permissions into lp:launchpad.
=== modified file 'lib/lp/blueprints/browser/configure.zcml'
--- lib/lp/blueprints/browser/configure.zcml 2017-04-08 16:58:52 +0000
+++ lib/lp/blueprints/browser/configure.zcml 2017-04-10 11:56:53 +0000
@@ -81,7 +81,7 @@
name="+delete"
for="lp.blueprints.interfaces.sprint.ISprint"
class="lp.blueprints.browser.sprint.SprintDeleteView"
- permission="launchpad.Edit"
+ permission="launchpad.Moderate"
facet="overview"
template="../templates/sprint-delete.pt"/>
<browser:page
=== modified file 'lib/lp/blueprints/browser/sprint.py'
--- lib/lp/blueprints/browser/sprint.py 2017-04-08 16:58:52 +0000
+++ lib/lp/blueprints/browser/sprint.py 2017-04-10 11:56:53 +0000
@@ -144,7 +144,7 @@
summary = 'Modify the imagery used to represent this meeting'
return Link('+branding', text, summary, icon='edit')
- @enabled_with_permission('launchpad.Edit')
+ @enabled_with_permission('launchpad.Moderate')
def delete(self):
return Link('+delete', 'Delete sprint', icon='trash-icon')
=== modified file 'lib/lp/blueprints/browser/tests/test_sprint.py'
--- lib/lp/blueprints/browser/tests/test_sprint.py 2017-04-08 16:58:52 +0000
+++ lib/lp/blueprints/browser/tests/test_sprint.py 2017-04-10 11:56:53 +0000
@@ -16,6 +16,7 @@
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
BrowserTestCase,
+ person_logged_in,
RequestTimelineCollector,
)
from lp.testing.layers import DatabaseFunctionalLayer
@@ -31,11 +32,12 @@
def test_query_count(self):
sprint = self.factory.makeSprint()
- for x in range(30):
- sprint.attend(
- self.factory.makePerson(),
- sprint.time_starts, sprint.time_ends, True)
- self.assertThat(sprint, BrowsesWithQueryLimit(18, sprint.owner))
+ with person_logged_in(sprint.owner):
+ for x in range(30):
+ sprint.attend(
+ self.factory.makePerson(),
+ sprint.time_starts, sprint.time_ends, True)
+ self.assertThat(sprint, BrowsesWithQueryLimit(21, sprint.owner))
def test_blueprint_listing_query_count(self):
"""Set a maximum number of queries for sprint blueprint lists."""
@@ -46,7 +48,7 @@
link.acceptBy(sprint.owner)
with RequestTimelineCollector() as recorder:
self.getViewBrowser(sprint)
- self.assertThat(recorder, HasQueryCount(Equals(28)))
+ self.assertThat(recorder, HasQueryCount(Equals(30)))
def test_proprietary_blueprint_listing_query_count(self):
"""Set a maximum number of queries for sprint blueprint lists."""
@@ -59,17 +61,27 @@
link.acceptBy(sprint.owner)
with RequestTimelineCollector() as recorder:
self.getViewBrowser(sprint)
- self.assertThat(recorder, HasQueryCount(Equals(20)))
+ self.assertThat(recorder, HasQueryCount(Equals(22)))
class TestSprintDeleteView(BrowserTestCase):
layer = DatabaseFunctionalLayer
+ def makePopulatedSprint(self):
+ sprint = self.factory.makeSprint()
+ with person_logged_in(sprint.owner):
+ sprint.attend(
+ self.factory.makePerson(),
+ sprint.time_starts, sprint.time_ends, True)
+ blueprint = self.factory.makeSpecification()
+ blueprint.linkSprint(sprint, blueprint.owner).acceptBy(sprint.owner)
+ return sprint
+
def test_unauthorized(self):
# A user without edit access cannot delete a sprint.
self.useFixture(FakeLogger())
- sprint = self.factory.makeSprint()
+ sprint = self.makePopulatedSprint()
sprint_url = canonical_url(sprint)
other_person = self.factory.makePerson()
browser = self.getViewBrowser(sprint, user=other_person)
@@ -78,19 +90,29 @@
Unauthorized, self.getUserBrowser, sprint_url + "/+delete",
user=other_person)
- def test_delete_sprint(self):
- # A sprint can be deleted, even if it has attendees and specifications.
+ def test_delete_sprint_owner(self):
+ # A sprint can be deleted by its owner, even if it has attendees and
+ # specifications.
self.useFixture(FakeLogger())
- sprint = self.factory.makeSprint()
+ sprint = self.makePopulatedSprint()
sprint_url = canonical_url(sprint)
owner_url = canonical_url(sprint.owner)
- sprint.attend(
- self.factory.makePerson(), sprint.time_starts, sprint.time_ends,
- True)
- blueprint = self.factory.makeSpecification()
- blueprint.linkSprint(sprint, blueprint.owner).acceptBy(sprint.owner)
browser = self.getViewBrowser(sprint, user=sprint.owner)
browser.getLink("Delete sprint").click()
browser.getControl("Delete sprint").click()
self.assertEqual(owner_url, browser.url)
self.assertRaises(NotFound, browser.open, sprint_url)
+
+ def test_delete_sprint_registry(self):
+ # A sprint can be deleted by a registry expert, even if it has
+ # attendees and specifications.
+ self.useFixture(FakeLogger())
+ sprint = self.makePopulatedSprint()
+ sprint_url = canonical_url(sprint)
+ owner_url = canonical_url(sprint.owner)
+ browser = self.getViewBrowser(
+ sprint, user=self.factory.makeRegistryExpert())
+ browser.getLink("Delete sprint").click()
+ browser.getControl("Delete sprint").click()
+ self.assertEqual(owner_url, browser.url)
+ self.assertRaises(NotFound, browser.open, sprint_url)
=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml 2015-09-25 10:07:24 +0000
+++ lib/lp/blueprints/configure.zcml 2017-04-10 11:56:53 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -33,10 +33,20 @@
<class
class="lp.blueprints.model.sprint.Sprint">
<allow
- interface="lp.blueprints.interfaces.sprint.ISprint"/>
+ interface="lp.blueprints.interfaces.sprint.ISprintPublic
+ lp.blueprints.interfaces.sprint.ISprintEditableAttributes"/>
<require
permission="launchpad.AnyPerson"
- set_schema="lp.blueprints.interfaces.sprint.ISprint"/>
+ interface="lp.blueprints.interfaces.sprint.ISprintAnyPerson"/>
+ <require
+ permission="launchpad.Edit"
+ set_schema="lp.blueprints.interfaces.sprint.ISprintEditableAttributes"/>
+ <require
+ permission="launchpad.Moderate"
+ interface="lp.blueprints.interfaces.sprint.ISprintModerate"/>
+ <require
+ permission="launchpad.Driver"
+ interface="lp.blueprints.interfaces.sprint.ISprintDriver"/>
</class>
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
=== modified file 'lib/lp/blueprints/doc/sprint-meeting-export.txt'
--- lib/lp/blueprints/doc/sprint-meeting-export.txt 2013-04-16 01:18:10 +0000
+++ lib/lp/blueprints/doc/sprint-meeting-export.txt 2017-04-10 11:56:53 +0000
@@ -82,9 +82,12 @@
>>> time_starts = datetime(2005, 10, 8, 7, 0, 0, tzinfo=timezone('UTC'))
>>> time_ends = datetime(2005, 11, 17, 20, 0, 0, tzinfo=timezone('UTC'))
+ >>> ignored = login_person(sampleperson)
>>> ubz.attend(sampleperson, time_starts, time_ends, True)
<...SprintAttendance ...>
+ >>> logout()
+ >>> login(ANONYMOUS)
>>> view = getMultiAdapter((ubz, request), name='+temp-meeting-export')
>>> view.initialize()
>>> sorted(person['name']
@@ -119,6 +122,7 @@
>>> link.sprint == ubz
True
+ >>> ignored = login_person(ubz.owner)
>>> ubz.acceptSpecificationLinks([link.id], mark)
0
=== modified file 'lib/lp/blueprints/interfaces/sprint.py'
--- lib/lp/blueprints/interfaces/sprint.py 2017-04-09 08:45:05 +0000
+++ lib/lp/blueprints/interfaces/sprint.py 2017-04-10 11:56:53 +0000
@@ -58,17 +58,69 @@
return getUtility(ISprintSet)[name]
-class ISprint(IHasOwner, IHasDrivers, IHasSpecifications, IHeadingContext):
- """A sprint, or conference, or meeting."""
+class ISprintPublic(IHasOwner, IHasDrivers, IHasSpecifications,
+ IHeadingContext):
+ """`ISprint` attributes that anyone can view."""
id = Int(title=_('The Sprint ID'))
+ displayname = Attribute('A pseudonym for the title.')
+ owner = PublicPersonChoice(
+ title=_('Owner'), required=True, readonly=True,
+ vocabulary='ValidPersonOrTeam')
+ datecreated = Datetime(
+ title=_('Date Created'), required=True, readonly=True)
+
+ # joins
+ attendees = Attribute('The set of attendees at this sprint.')
+ attendances = Attribute('The set of SprintAttendance records.')
+
+ def specificationLinks(status=None):
+ """Return the SprintSpecification records matching the filter,
+ quantity and sort given. The rules for filtering and sorting etc are
+ the same as those for IHasSpecifications.specifications()
+ """
+
+ def getSpecificationLink(id):
+ """Return the specification link for this sprint that has the given
+ ID. We use the naked ID because there is no unique name for a spec
+ outside of a single product or distro, and a sprint can cover
+ multiple products and distros.
+ """
+
+ def isDriver(user):
+ """Returns True if and only if the specified user
+ is a driver of this sprint.
+
+ A driver for a sprint is either the person in the
+ `driver` attribute, a person who is a member of a team
+ in the `driver` attribute, or an administrator.
+ """
+
+
+class ISprintAnyPerson(Interface):
+ """`ISprint` methods that any logged-in user can call."""
+
+ # subscription-related methods
+ def attend(person, time_starts, time_ends, is_physical):
+ """Record that this person will be attending the Sprint."""
+
+ def removeAttendance(person):
+ """Remove the person's attendance record."""
+
+
+class ISprintEditableAttributes(Interface):
+ """`ISprint` attributes that can be edited.
+
+ Anyone can view these attributes, but changing them requires
+ launchpad.Edit.
+ """
+
name = SprintNameField(
title=_('Name'), required=True, description=_('A unique name '
'for this sprint, or conference, or meeting. This will part of '
'the URL so pick something short. A single word is all you get.'),
constraint=name_validator)
- displayname = Attribute('A pseudonym for the title.')
title = TextLine(
title=_('Title'), required=True, description=_("Please provide "
"a title for this meeting. This will be shown in listings of "
@@ -115,9 +167,6 @@
"The content of this meeting's home page. Edit this and it "
"will be displayed for all the world to see. It is NOT a wiki "
"so you cannot undo changes."))
- owner = PublicPersonChoice(
- title=_('Owner'), required=True, readonly=True,
- vocabulary='ValidPersonOrTeam')
time_zone = Choice(
title=_('Timezone'), required=True, description=_('The time '
'zone in which this sprint, or conference, takes place. '),
@@ -126,28 +175,20 @@
title=_('Starting Date and Time'), required=True)
time_ends = Datetime(
title=_('Finishing Date and Time'), required=True)
- datecreated = Datetime(
- title=_('Date Created'), required=True, readonly=True)
is_physical = Bool(
title=_("Is the sprint being held in a physical location?"),
required=True, readonly=False, default=True)
- # joins
- attendees = Attribute('The set of attendees at this sprint.')
- attendances = Attribute('The set of SprintAttendance records.')
-
- def specificationLinks(status=None):
- """Return the SprintSpecification records matching the filter,
- quantity and sort given. The rules for filtering and sorting etc are
- the same as those for IHasSpecifications.specifications()
- """
-
- def getSpecificationLink(id):
- """Return the specification link for this sprint that has the given
- ID. We use the naked ID because there is no unique name for a spec
- outside of a single product or distro, and a sprint can cover
- multiple products and distros.
- """
+
+class ISprintModerate(Interface):
+ """`ISprint` methods that can be called by more than one community."""
+
+ def destroySelf():
+ """Remove this sprint."""
+
+
+class ISprintDriver(Interface):
+ """`ISprint` methods that require launchpad.Driver permission."""
def acceptSpecificationLinks(idlist, decider):
"""Accept the given sprintspec items, and return the number of
@@ -159,13 +200,6 @@
sprintspec items that remain proposed.
"""
- # subscription-related methods
- def attend(person, time_starts, time_ends, is_physical):
- """Record that this person will be attending the Sprint."""
-
- def removeAttendance(person):
- """Remove the person's attendance record."""
-
# bug linking
def linkSpecification(spec):
"""Link this sprint to the given specification."""
@@ -173,17 +207,10 @@
def unlinkSpecification(spec):
"""Remove this specification from the sprint spec list."""
- def isDriver(user):
- """Returns True if and only if the specified user
- is a driver of this sprint.
-
- A driver for a sprint is either the person in the
- `driver` attribute, a person who is memeber of a team
- in the `driver` attribute or an administrator.
- """
-
- def destroySelf():
- """Remove this sprint."""
+
+class ISprint(ISprintPublic, ISprintAnyPerson, ISprintEditableAttributes,
+ ISprintModerate, ISprintDriver):
+ """A sprint, or conference, or meeting."""
class IHasSprints(Interface):
=== modified file 'lib/lp/blueprints/model/tests/test_sprint.py'
--- lib/lp/blueprints/model/tests/test_sprint.py 2014-06-19 10:04:55 +0000
+++ lib/lp/blueprints/model/tests/test_sprint.py 2017-04-10 11:56:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit test for sprints."""
@@ -21,7 +21,10 @@
SpecificationSort,
)
from lp.registry.interfaces.accesspolicy import IAccessPolicySource
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
from lp.testing.layers import DatabaseFunctionalLayer
@@ -229,12 +232,13 @@
bob = self.factory.makePerson(name='zbob', displayname='Bob')
ced = self.factory.makePerson(name='xed', displayname='ced')
dave = self.factory.makePerson(name='wdave', displayname='Dave')
- sprint.attend(
- bob, sprint.time_starts, sprint.time_ends, True)
- sprint.attend(
- ced, sprint.time_starts, sprint.time_ends, True)
- sprint.attend(
- dave, sprint.time_starts, sprint.time_ends, True)
+ with person_logged_in(sprint.owner):
+ sprint.attend(
+ bob, sprint.time_starts, sprint.time_ends, True)
+ sprint.attend(
+ ced, sprint.time_starts, sprint.time_ends, True)
+ sprint.attend(
+ dave, sprint.time_starts, sprint.time_ends, True)
attendances = [bob.displayname, ced.displayname, dave.displayname]
people = [attendee.attendee.displayname for attendee in
sprint.attendances]
=== modified file 'lib/lp/blueprints/stories/blueprints/xx-creation.txt'
--- lib/lp/blueprints/stories/blueprints/xx-creation.txt 2011-09-02 04:04:46 +0000
+++ lib/lp/blueprints/stories/blueprints/xx-creation.txt 2017-04-10 11:56:53 +0000
@@ -584,6 +584,8 @@
>>> from lp.registry.interfaces.person import IPersonSet
>>> login('test@xxxxxxxxxxxxx')
>>> rome_sprint = factory.makeSprint(name='rome')
+ >>> logout()
+ >>> ignored = login_person(rome_sprint.owner)
>>> rome_sprint.time_ends = dt.datetime.now(UTC) + dt.timedelta(30)
>>> rome_sprint.time_starts = dt.datetime.now(UTC) + dt.timedelta(20)
>>> sample_person = getUtility(IPersonSet).getByName('name12')
=== modified file 'lib/lp/blueprints/stories/sprints/xx-sprints.txt'
--- lib/lp/blueprints/stories/sprints/xx-sprints.txt 2016-06-06 07:58:13 +0000
+++ lib/lp/blueprints/stories/sprints/xx-sprints.txt 2017-04-10 11:56:53 +0000
@@ -12,6 +12,8 @@
>>> from lp.registry.interfaces.person import IPersonSet
>>> login('test@xxxxxxxxxxxxx')
>>> rome_sprint = factory.makeSprint(name='rome', title='Rome')
+ >>> logout()
+ >>> ignored = login_person(rome_sprint.owner)
>>> rome_sprint.time_ends = dt.datetime.now(UTC) + dt.timedelta(30)
>>> rome_sprint.time_starts = dt.datetime.now(UTC) + dt.timedelta(20)
>>> sample_person = getUtility(IPersonSet).getByName('name12')
=== modified file 'lib/lp/blueprints/stories/standalone/sprint-links.txt'
--- lib/lp/blueprints/stories/standalone/sprint-links.txt 2011-05-29 21:23:37 +0000
+++ lib/lp/blueprints/stories/standalone/sprint-links.txt 2017-04-10 11:56:53 +0000
@@ -68,6 +68,8 @@
>>> from lp.registry.interfaces.person import IPersonSet
>>> login('test@xxxxxxxxxxxxx')
>>> rome_sprint = factory.makeSprint(name='rome')
+ >>> logout()
+ >>> ignored = login_person(rome_sprint.owner)
>>> rome_sprint.time_ends = dt.datetime.now(UTC) + dt.timedelta(30)
>>> rome_sprint.time_starts = dt.datetime.now(UTC) + dt.timedelta(20)
>>> sample_person = getUtility(IPersonSet).getByName('name12')
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2016-12-02 12:04:11 +0000
+++ lib/lp/security.py 2017-04-10 11:56:53 +0000
@@ -662,7 +662,7 @@
user.in_admin)
-class Sprint(AuthorizationBase):
+class ViewSprint(AuthorizationBase):
"""An attendee, owner, or driver of a sprint."""
permission = 'launchpad.View'
usedfor = ISprint
@@ -675,6 +675,21 @@
user.in_admin)
+class EditSprint(EditByOwnersOrAdmins):
+ usedfor = ISprint
+
+
+class ModerateSprint(ModerateByRegistryExpertsOrAdmins):
+ """The sprint owner, registry experts, and admins can moderate sprints."""
+ permission = 'launchpad.Moderate'
+ usedfor = ISprint
+
+ def checkAuthenticated(self, user):
+ return (
+ super(ModerateSprint, self).checkAuthenticated(user) or
+ user.isOwner(self.obj))
+
+
class EditSpecificationSubscription(AuthorizationBase):
"""The subscriber, and people related to the spec or the target of the
spec can determine who is essential."""
Follow ups
