launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06765
[Merge] lp:~jcsackett/launchpad/sharing-details into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/sharing-details into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/sharing-details/+merge/98223
Summary
=======
The managing sharing ui requires a page that shows the details of how a
particular person is being shared with by a pillar. This page anchors in the
infrastructure for that view. A subsequent branch
(lp:~jcsackett/launchpad/sharing-details-ui) adds the actual interaction
elements to the page.
Preimp
======
Spoke with Curtis Hovey about the necessary machinery and operation of
navigations and traversal for pillar person to allow the
+sharingdetails/{$person/name} url to function.
Implementation
==============
* A new view has been registered as the default view (+index) for pillar
person.
* A navigation mixin has added to Distribution and Product Navigation, adding
the traversal stepthrough for +sharingdetails. This mixin ensures that a
result (a PillarPerson) is only provided if the person exists, and there are
accesspolicyartifacts shared between the pillar and the person (i.e. there
is actually sharing going on, and information to show).
* A security adapter for pillarperson has been added to check
`launchpad.Driver`.
* A bunch of zcml has been added to register everything.
Tests
=====
bin/test -vvct test_pillar_sharing
QA
==
Using https://qastaging.launchpad.net/launchpad/+sharing, find or create a
user being shared with. Check the corresponding +sharingdetails link for that
person; an empty page with the right title and label should render.
Lint
====
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/configure.zcml
lib/lp/security.py
lib/lp/registry/browser/product.py
lib/lp/registry/templates/pillar-sharing-details.pt
lib/lp/registry/browser/pillar.py
lib/lp/registry/browser/tests/test_pillar_sharing.py
--
https://code.launchpad.net/~jcsackett/launchpad/sharing-details/+merge/98223
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/sharing-details into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2012-03-02 23:09:34 +0000
+++ lib/lp/registry/browser/configure.zcml 2012-03-19 16:18:49 +0000
@@ -1381,8 +1381,7 @@
name="+questions"/>
<browser:navigation
module="lp.registry.browser.product"
- classes="
- ProductNavigation"/>
+ classes="ProductNavigation"/>
<browser:url
for="lp.registry.interfaces.product.IProduct"
path_expression="name"
@@ -1428,6 +1427,20 @@
name="+sharing"
class="lp.registry.browser.pillar.PillarSharingView"
template="../templates/pillar-sharing.pt"/>
+ <browser:url
+ for="lp.registry.interfaces.pillar.IPillarPerson"
+ path_expression="string:+sharingdetails/${person/name}"
+ rootsite="mainsite"
+ attribute_to_parent="pillar"/>
+ <browser:page
+ for="lp.registry.interfaces.pillar.IPillarPerson"
+ permission="launchpad.Driver"
+ name="+index"
+ class="lp.registry.browser.pillar.PillarPersonSharingView"
+ template="../templates/pillar-sharing-details.pt"/>
+ <browser:defaultView
+ for="lp.registry.interfaces.pillar.IPillarPerson"
+ name="+index"/>
<browser:page
for="lp.registry.interfaces.product.IProduct"
permission="zope.Public"
=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py 2012-03-16 08:11:47 +0000
+++ lib/lp/registry/browser/distribution.py 2012-03-19 16:18:49 +0000
@@ -87,7 +87,10 @@
RegistryCollectionActionMenuBase,
)
from lp.registry.browser.objectreassignment import ObjectReassignmentView
-from lp.registry.browser.pillar import PillarBugsMenu
+from lp.registry.browser.pillar import (
+ PillarBugsMenu,
+ PillarNavigationMixin,
+ )
from lp.registry.interfaces.distribution import (
IDerivativeDistribution,
IDistribution,
@@ -135,7 +138,8 @@
class DistributionNavigation(
GetitemNavigation, BugTargetTraversalMixin, QuestionTargetTraversalMixin,
- FAQTargetNavigationMixin, StructuralSubscriptionTargetTraversalMixin):
+ FAQTargetNavigationMixin, StructuralSubscriptionTargetTraversalMixin,
+ PillarNavigationMixin):
usedfor = IDistribution
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2012-03-16 08:28:03 +0000
+++ lib/lp/registry/browser/pillar.py 2012-03-19 16:18:49 +0000
@@ -10,6 +10,8 @@
'PillarView',
'PillarBugsMenu',
'PillarSharingView',
+ 'PillarPersonSharingView',
+ 'PillarNavigationMixin',
]
@@ -39,20 +41,27 @@
from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionMenuMixin,
)
+from lp.registry.interfaces.accesspolicy import (
+ IAccessPolicyGrantFlatSource,
+ IAccessPolicySource,
+ )
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.pillar import IPillar
from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.registry.interfaces.person import IPersonSet
+from lp.registry.model.pillar import PillarPerson
from lp.services.propertycache import cachedproperty
from lp.services.features import getFeatureFlag
from lp.services.webapp.authorization import check_permission
-from lp.services.webapp.menu import (
+from lp.services.webapp import (
ApplicationMenu,
enabled_with_permission,
Link,
NavigationMenu,
+ stepthrough,
)
from lp.services.webapp.publisher import (
LaunchpadView,
@@ -60,6 +69,22 @@
)
+class PillarNavigationMixin:
+
+ @stepthrough('+sharingdetails')
+ def traverse_details(self, name):
+ """Traverse to the sharing details for a given person."""
+ person = getUtility(IPersonSet).getByName(name)
+ if person is None:
+ return None
+ policies = getUtility(IAccessPolicySource).findByPillar([self.context])
+ source = getUtility(IAccessPolicyGrantFlatSource)
+ artifacts = source.findArtifactsByGrantee(person, policies)
+ if artifacts.is_empty():
+ return None
+ return PillarPerson.create(self.context, person)
+
+
class IInvolved(Interface):
"""A marker interface for getting involved."""
@@ -281,3 +306,21 @@
cache.objects['information_types'] = self.information_types
cache.objects['sharing_permissions'] = self.sharing_permissions
cache.objects['sharee_data'] = self.sharee_data
+
+
+class PillarPersonSharingView(LaunchpadView):
+
+ page_title = "Person or team"
+ label = "Information shared with person or team"
+
+ def initialize(self):
+ enabled_flag = 'disclosure.enhanced_sharing.enabled'
+ enabled = bool(getFeatureFlag(enabled_flag))
+ if not enabled:
+ raise Unauthorized("This feature is not yet available.")
+
+ self.pillar = self.context.pillar
+ self.person = self.context.person
+
+ self.label = "Information shared with %s" % self.person.displayname
+ self.page_title = "%s" % self.person.displayname
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-03-16 08:11:47 +0000
+++ lib/lp/registry/browser/product.py 2012-03-19 16:18:49 +0000
@@ -149,6 +149,7 @@
)
from lp.registry.browser.pillar import (
PillarBugsMenu,
+ PillarNavigationMixin,
PillarView,
)
from lp.registry.browser.productseries import get_series_branch_error
@@ -211,7 +212,8 @@
class ProductNavigation(
Navigation, BugTargetTraversalMixin,
FAQTargetNavigationMixin, HasCustomLanguageCodesTraversalMixin,
- QuestionTargetTraversalMixin, StructuralSubscriptionTargetTraversalMixin):
+ QuestionTargetTraversalMixin, StructuralSubscriptionTargetTraversalMixin,
+ PillarNavigationMixin):
usedfor = IProduct
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-16 07:07:19 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-19 16:18:49 +0000
@@ -10,9 +10,11 @@
from BeautifulSoup import BeautifulSoup
from lazr.restful.interfaces import IJSONRequestCache
from zope.component import getUtility
+from zope.publisher.interfaces import NotFound
from zope.security.interfaces import Unauthorized
from lp.app.interfaces.services import IService
+from lp.registry.model.pillar import PillarPerson
from lp.services.features.testing import FeatureFixture
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
@@ -31,6 +33,94 @@
WRITE_FLAG = {'disclosure.enhanced_sharing.writable': 'true'}
+class PillarSharingDetailsMixin:
+ """Test the pillar sharing details view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def getPillarPerson(self, person=None, with_sharing=True):
+ if person is None:
+ person = self.factory.makePerson()
+ if with_sharing:
+ if self.pillar_type == 'product':
+ bug = self.factory.makeBug(product=self.pillar, private=True)
+ elif self.pillar_type == 'distribution':
+ bug = self.factory.makeBug(
+ distribution=self.pillar, private=True)
+ artifact = self.factory.makeAccessArtifact(concrete=bug)
+ policy = self.factory.makeAccessPolicy(pillar=self.pillar)
+ self.factory.makeAccessPolicyArtifact(
+ artifact=artifact, policy=policy)
+ self.factory.makeAccessArtifactGrant(
+ artifact=artifact, grantee=person, grantor=self.pillar.owner)
+
+ return PillarPerson(self.pillar, person)
+
+ def test_view_traverses_plus_sharingdetails(self):
+ # The traversed url in the app is pillar/+sharingdetails/person
+ with FeatureFixture(ENABLED_FLAG):
+ # We have to do some fun url hacking to force the traversal a user
+ # encounters.
+ pillarperson = self.getPillarPerson()
+ expected = pillarperson.person.displayname
+ url = 'http://launchpad.dev/%s/+sharingdetails/%s' % (
+ pillarperson.pillar.name, pillarperson.person.name)
+ browser = self.getUserBrowser(user=self.driver, url=url)
+ self.assertEqual(expected, browser.title)
+
+ def test_not_found_without_sharing(self):
+ # If there is no sharing between pillar and person, NotFound is the
+ # result.
+ with FeatureFixture(ENABLED_FLAG):
+ # We have to do some fun url hacking to force the traversal a user
+ # encounters.
+ pillarperson = self.getPillarPerson(with_sharing=False)
+ url = 'http://launchpad.dev/%s/+sharingdetails/%s' % (
+ pillarperson.pillar.name, pillarperson.person.name)
+ browser = self.getUserBrowser(user=self.driver)
+ self.assertRaises(NotFound, browser.open, url)
+
+ def test_init_without_feature_flag(self):
+ # We need a feature flag to enable the view.
+ pillarperson = self.getPillarPerson()
+ self.assertRaises(
+ Unauthorized, create_initialized_view, pillarperson, '+index')
+
+ def test_init_with_feature_flag(self):
+ # The view works with a feature flag.
+ with FeatureFixture(ENABLED_FLAG):
+ pillarperson = self.getPillarPerson()
+ view = create_initialized_view(pillarperson, '+index')
+ self.assertEqual(pillarperson.person.displayname, view.page_title)
+
+
+class TestProductSharingDetailsView(
+ TestCaseWithFactory, PillarSharingDetailsMixin):
+
+ pillar_type = 'product'
+
+ def setUp(self):
+ super(TestProductSharingDetailsView, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.pillar = self.factory.makeProduct(
+ owner=self.owner, driver=self.driver)
+ login_person(self.driver)
+
+
+class TestDistributionSharingDetailsView(
+ TestCaseWithFactory, PillarSharingDetailsMixin):
+
+ pillar_type = 'distribution'
+
+ def setUp(self):
+ super(TestDistributionSharingDetailsView, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.pillar = self.factory.makeProduct(
+ owner=self.owner, driver=self.driver)
+ login_person(self.driver)
+
class PillarSharingViewTestMixin:
"""Test the PillarSharingView."""
=== added file 'lib/lp/registry/templates/pillar-sharing-details.pt'
--- lib/lp/registry/templates/pillar-sharing-details.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/pillar-sharing-details.pt 2012-03-19 16:18:49 +0000
@@ -0,0 +1,14 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad"
+>
+
+<body>
+ <div metal:fill-slot="main">
+ </div>
+</body>
+</html>
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2012-02-15 00:57:40 +0000
+++ lib/lp/security.py 2012-03-19 16:18:49 +0000
@@ -124,7 +124,10 @@
ITeam,
PersonVisibility,
)
-from lp.registry.interfaces.pillar import IPillar
+from lp.registry.interfaces.pillar import (
+ IPillar,
+ IPillarPerson,
+ )
from lp.registry.interfaces.poll import (
IPoll,
IPollOption,
@@ -337,6 +340,17 @@
user.in_registry_experts)
+class PillarPersonSharingDriver(AuthorizationBase):
+ usedfor = IPillarPerson
+ permission = 'launchpad.Driver'
+
+ def checkAuthenticated(self, user):
+ """The Admins & Commercial Admins can see inactive pillars."""
+ return (user.in_admin or
+ user.isOwner(self.obj.pillar) or
+ user.isOneOfDrivers(self.obj.pillar))
+
+
class EditAccountBySelfOrAdmin(AuthorizationBase):
permission = 'launchpad.Edit'
usedfor = IAccount
Follow ups
