launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06906
[Merge] lp:~wallyworld/launchpad/audit-sharing-933815 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/audit-sharing-933815 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #933815 in Launchpad itself: "Cannot search and filter +sharing policies"
https://bugs.launchpad.net/launchpad/+bug/933815
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/audit-sharing-933815/+merge/99658
== Implementation ==
Add boiler plate to hook up the +audit-sharing view. The view currently just shows the same info as the normal +sharing view. Subsequent branches will populate the view with the additional content and add the searching/filtering.
== Tests ==
Add new test cases to test_pillar_sharing. Factor out common functionality since +sharing and +audit-sharing use many of the same tests.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/configure.zcml
lib/lp/registry/browser/pillar.py
lib/lp/registry/browser/tests/test_pillar_sharing.py
lib/lp/registry/templates/pillar-sharing.pt
--
https://code.launchpad.net/~wallyworld/launchpad/audit-sharing-933815/+merge/99658
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/audit-sharing-933815 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2012-03-21 14:30:16 +0000
+++ lib/lp/registry/browser/configure.zcml 2012-03-28 03:09:19 +0000
@@ -1431,7 +1431,13 @@
for="lp.registry.interfaces.product.IProduct"
permission="launchpad.Driver"
name="+sharing"
- class="lp.registry.browser.pillar.PillarSharingView"
+ class="lp.registry.browser.pillar.PillarSharingInformationView"
+ template="../templates/pillar-sharing.pt"/>
+ <browser:page
+ for="lp.registry.interfaces.product.IProduct"
+ permission="launchpad.Driver"
+ name="+audit-sharing"
+ class="lp.registry.browser.pillar.PillarAuditSharingView"
template="../templates/pillar-sharing.pt"/>
<browser:url
for="lp.registry.interfaces.pillar.IPillarPerson"
@@ -1813,7 +1819,13 @@
for="lp.registry.interfaces.distribution.IDistribution"
permission="launchpad.Driver"
name="+sharing"
- class="lp.registry.browser.pillar.PillarSharingView"
+ class="lp.registry.browser.pillar.PillarSharingInformationView"
+ template="../templates/pillar-sharing.pt"/>
+ <browser:page
+ for="lp.registry.interfaces.distribution.IDistribution"
+ permission="launchpad.Driver"
+ name="+audit-sharing"
+ class="lp.registry.browser.pillar.PillarAuditSharingView"
template="../templates/pillar-sharing.pt"/>
<browser:page
for="lp.registry.interfaces.distribution.IDistribution"
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2012-03-28 01:51:45 +0000
+++ lib/lp/registry/browser/pillar.py 2012-03-28 03:09:19 +0000
@@ -7,11 +7,12 @@
__all__ = [
'InvolvedMenu',
+ 'PillarAuditSharingView',
'PillarBugsMenu',
'PillarView',
'PillarNavigationMixin',
'PillarPersonSharingView',
- 'PillarSharingView',
+ 'PillarSharingInformationView',
]
@@ -252,10 +253,7 @@
return Link('+securitycontact', text, icon='edit')
-class PillarSharingView(LaunchpadView):
-
- page_title = "Sharing"
- label = "Sharing information"
+class BasePillarSharingView(LaunchpadView):
related_features = (
'disclosure.enhanced_sharing.enabled',
@@ -264,6 +262,14 @@
_batch_navigator = None
+ @property
+ def show_sharing_information_link(self):
+ return False
+
+ @property
+ def show_audit_sharing_link(self):
+ return False
+
def _getSharingService(self):
return getUtility(IService, 'sharing')
@@ -312,12 +318,8 @@
self._batch_navigator = self._getBatchNavigator(unbatchedSharees)
return self._batch_navigator
- def unbatched_sharees(self):
- """All the sharees for a pillar."""
- return self._getSharingService().getPillarSharees(self.context)
-
def initialize(self):
- super(PillarSharingView, self).initialize()
+ super(BasePillarSharingView, self).initialize()
enabled_readonly_flag = 'disclosure.enhanced_sharing.enabled'
enabled_writable_flag = (
'disclosure.enhanced_sharing.writable')
@@ -357,6 +359,34 @@
cache.objects.update(_getBatchInfo(batch_navigator.batch))
+class PillarSharingInformationView(BasePillarSharingView):
+
+ page_title = "Sharing Information"
+ label = "Sharing information"
+
+ @property
+ def show_audit_sharing_link(self):
+ return True
+
+ def unbatched_sharees(self):
+ """All the sharees for a pillar."""
+ return self._getSharingService().getPillarSharees(self.context)
+
+
+class PillarAuditSharingView(BasePillarSharingView):
+
+ page_title = "Audit Sharing"
+ label = "Audit sharing"
+
+ @property
+ def show_sharing_information_link(self):
+ return True
+
+ def unbatched_sharees(self):
+ """All the sharees for a pillar."""
+ return self._getSharingService().getPillarSharees(self.context)
+
+
class PillarPersonSharingView(LaunchpadView):
page_title = "Person or team"
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-28 01:51:45 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-03-28 03:09:19 +0000
@@ -134,11 +134,75 @@
login_person(self.driver)
-class PillarSharingViewTestMixin:
- """Test the PillarSharingView."""
+class BasePillarSharingViewTestMixin:
+ """Common pillar sharing view tests."""
layer = DatabaseFunctionalLayer
+ def test_init_without_feature_flag(self):
+ # We need a feature flag to enable the view.
+ self.assertRaises(
+ Unauthorized, create_initialized_view, self.pillar, self.view_name)
+
+ def test_picker_config(self):
+ # Test the config passed to the disclosure sharing picker.
+ with FeatureFixture(ENABLED_FLAG):
+ view = create_view(self.pillar, name=self.view_name)
+ picker_config = simplejson.loads(view.json_sharing_picker_config)
+ self.assertTrue('vocabulary_filters' in picker_config)
+ self.assertEqual(
+ 'Share with a user or team',
+ picker_config['header'])
+ self.assertEqual(
+ 'NewPillarSharee', picker_config['vocabulary'])
+
+ def test_view_range_factory(self):
+ # Test the view range factory is properly configured.
+ with FeatureFixture(ENABLED_FLAG):
+ view = create_initialized_view(self.pillar, name=self.view_name)
+ range_factory = view.sharees().batch.range_factory
+
+ def test_range_factory():
+ row = range_factory.resultset.get_plain_result_set()[0]
+ range_factory.getOrderValuesFor(row)
+
+ self.assertThat(
+ test_range_factory,
+ Not(Raises(MatchesException(StormRangeFactoryError))))
+
+ def test_view_query_count(self):
+ # Test the query count is within expected limit.
+ with FeatureFixture(ENABLED_FLAG):
+ view = create_view(self.pillar, name=self.view_name)
+ with StormStatementRecorder() as recorder:
+ view.initialize()
+ self.assertThat(recorder, HasQueryCount(LessThan(6)))
+
+ def test_view_write_enabled_without_feature_flag(self):
+ # Test that sharing_write_enabled is not set without the feature flag.
+ with FeatureFixture(ENABLED_FLAG):
+ login_person(self.owner)
+ view = create_initialized_view(self.pillar, name=self.view_name)
+ cache = IJSONRequestCache(view.request)
+ self.assertFalse(cache.objects.get('sharing_write_enabled'))
+
+ def test_view_write_enabled_with_feature_flag(self):
+ # Test that sharing_write_enabled is set when required.
+ with FeatureFixture(WRITE_FLAG):
+ view = create_initialized_view(self.pillar, name=self.view_name)
+ cache = IJSONRequestCache(view.request)
+ self.assertFalse(cache.objects.get('sharing_write_enabled'))
+ login_person(self.owner)
+ view = create_initialized_view(self.pillar, name=self.view_name)
+ cache = IJSONRequestCache(view.request)
+ self.assertTrue(cache.objects.get('sharing_write_enabled'))
+
+
+class PillarSharingInformationViewTestMixin(BasePillarSharingViewTestMixin):
+ """Test the PillarSharingInformationView."""
+
+ view_name = '+sharing'
+
def createSharees(self):
login_person(self.owner)
self.access_policy = self.factory.makeAccessPolicy(
@@ -161,16 +225,14 @@
for x in range(10):
makeGrants('name%s' % x)
- def test_init_without_feature_flag(self):
- # We need a feature flag to enable the view.
- self.assertRaises(
- Unauthorized, create_initialized_view, self.pillar, '+sharing')
-
def test_init_with_feature_flag(self):
# The view works with a feature flag.
with FeatureFixture(ENABLED_FLAG):
view = create_initialized_view(self.pillar, '+sharing')
- self.assertEqual('Sharing', view.page_title)
+ self.assertEqual('Sharing Information', view.page_title)
+ self.assertEqual('Sharing information', view.label)
+ self.assertFalse(view.show_sharing_information_link)
+ self.assertTrue(view.show_audit_sharing_link)
def test_sharing_menu_without_feature_flag(self):
url = canonical_url(self.pillar)
@@ -190,17 +252,15 @@
sharing_menu = soup.find('a', {'href': sharing_url})
self.assertIsNotNone(sharing_menu)
- def test_picker_config(self):
- # Test the config passed to the disclosure sharing picker.
+ def test_audit_sharing_link(self):
+ # The +audit-sharing link is rendered.
with FeatureFixture(ENABLED_FLAG):
- view = create_view(self.pillar, name='+sharing')
- picker_config = simplejson.loads(view.json_sharing_picker_config)
- self.assertTrue('vocabulary_filters' in picker_config)
- self.assertEqual(
- 'Share with a user or team',
- picker_config['header'])
- self.assertEqual(
- 'NewPillarSharee', picker_config['vocabulary'])
+ url = canonical_url(self.pillar, view_name='+sharing')
+ browser = setupBrowserForUser(user=self.driver)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ audit_sharing_link = soup.find('a', {'href': '+audit-sharing'})
+ self.assertIsNotNone(audit_sharing_link)
def test_view_data_model(self):
# Test that the json request cache contains the view data model.
@@ -228,71 +288,89 @@
self.assertContentEqual(
next_batch.range_memo, cache.objects.get('next')['memo'])
- def test_view_range_factory(self):
- # Test the view range factory is properly configured.
- with FeatureFixture(ENABLED_FLAG):
- view = create_initialized_view(self.pillar, name='+sharing')
- range_factory = view.sharees().batch.range_factory
-
- def test_range_factory():
- row = range_factory.resultset.get_plain_result_set()[0]
- range_factory.getOrderValuesFor(row)
-
- self.assertThat(
- test_range_factory,
- Not(Raises(MatchesException(StormRangeFactoryError))))
-
- def test_view_query_count(self):
- # Test the query count is within expected limit.
- with FeatureFixture(ENABLED_FLAG):
- view = create_view(self.pillar, name='+sharing')
- with StormStatementRecorder() as recorder:
- view.initialize()
- self.assertThat(recorder, HasQueryCount(LessThan(6)))
-
- def test_view_write_enabled_without_feature_flag(self):
- # Test that sharing_write_enabled is not set without the feature flag.
- with FeatureFixture(ENABLED_FLAG):
- login_person(self.owner)
- view = create_initialized_view(self.pillar, name='+sharing')
- cache = IJSONRequestCache(view.request)
- self.assertFalse(cache.objects.get('sharing_write_enabled'))
-
- def test_view_write_enabled_with_feature_flag(self):
- # Test that sharing_write_enabled is set when required.
- with FeatureFixture(WRITE_FLAG):
- view = create_initialized_view(self.pillar, name='+sharing')
- cache = IJSONRequestCache(view.request)
- self.assertFalse(cache.objects.get('sharing_write_enabled'))
- login_person(self.owner)
- view = create_initialized_view(self.pillar, name='+sharing')
- cache = IJSONRequestCache(view.request)
- self.assertTrue(cache.objects.get('sharing_write_enabled'))
-
-
-class TestProductSharingView(PillarSharingViewTestMixin,
- TestCaseWithFactory):
- """Test the PillarSharingView with products."""
-
- def setUp(self):
- super(TestProductSharingView, self).setUp()
- self.driver = self.factory.makePerson()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeProduct(
- owner=self.owner, driver=self.driver)
- self.createSharees()
- login_person(self.driver)
-
-
-class TestDistributionSharingView(PillarSharingViewTestMixin,
- TestCaseWithFactory):
- """Test the PillarSharingView with distributions."""
-
- def setUp(self):
- super(TestDistributionSharingView, self).setUp()
- self.driver = self.factory.makePerson()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeDistribution(
- owner=self.owner, driver=self.driver)
- self.createSharees()
+
+class TestProductSharingInformationView(
+ PillarSharingInformationViewTestMixin, TestCaseWithFactory):
+ """Test the PillarSharingInformationView with products."""
+
+ def setUp(self):
+ super(TestProductSharingInformationView, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.pillar = self.factory.makeProduct(
+ owner=self.owner, driver=self.driver)
+ self.createSharees()
+ login_person(self.driver)
+
+
+class TestDistributionSharingInformationView(
+ PillarSharingInformationViewTestMixin, TestCaseWithFactory):
+ """Test the PillarSharingInformationView with distributions."""
+
+ def setUp(self):
+ super(TestDistributionSharingInformationView, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.pillar = self.factory.makeDistribution(
+ owner=self.owner, driver=self.driver)
+ self.createSharees()
+ login_person(self.driver)
+
+
+class PillarAuditSharingViewTestMixin(BasePillarSharingViewTestMixin):
+ """Test the PillarAuditSharingView."""
+
+ view_name = '+audit-sharing'
+
+ def test_init_with_feature_flag(self):
+ # The view works with a feature flag.
+ with FeatureFixture(ENABLED_FLAG):
+ view = create_initialized_view(self.pillar, '+audit-sharing')
+ self.assertEqual('Audit Sharing', view.page_title)
+ self.assertEqual('Audit sharing', view.label)
+ self.assertTrue(view.show_sharing_information_link)
+ self.assertFalse(view.show_audit_sharing_link)
+
+ def test_sharing_information_link(self):
+ # The +sharing link is rendered.
+ with FeatureFixture(ENABLED_FLAG):
+ url = canonical_url(self.pillar, view_name='+audit-sharing')
+ browser = setupBrowserForUser(user=self.driver)
+ browser.open(url)
+ soup = BeautifulSoup(browser.contents)
+ sharing_info_link = soup.find('a', {'href': '+sharing'})
+ self.assertIsNotNone(sharing_info_link)
+
+ def test_view_data_model(self):
+ # Test that the json request cache contains the view data model.
+ with FeatureFixture(ENABLED_FLAG):
+ view = create_initialized_view(self.pillar, name='+audit-sharing')
+ cache = IJSONRequestCache(view.request)
+ self.assertIsNotNone(cache.objects.get('information_types'))
+ self.assertIsNotNone(cache.objects.get('sharing_permissions'))
+
+
+class TestProductAuditSharingView(
+ PillarAuditSharingViewTestMixin, TestCaseWithFactory):
+ """Test the PillarSharingInformationView with products."""
+
+ def setUp(self):
+ super(TestProductAuditSharingView, 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 TestDistributionAuditSharingView(
+ PillarAuditSharingViewTestMixin, TestCaseWithFactory):
+ """Test the PillarSharingInformationView with distributions."""
+
+ def setUp(self):
+ super(TestDistributionAuditSharingView, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.pillar = self.factory.makeDistribution(
+ owner=self.owner, driver=self.driver)
login_person(self.driver)
=== modified file 'lib/lp/registry/templates/pillar-sharing.pt'
--- lib/lp/registry/templates/pillar-sharing.pt 2012-03-22 09:00:36 +0000
+++ lib/lp/registry/templates/pillar-sharing.pt 2012-03-28 03:09:19 +0000
@@ -31,7 +31,12 @@
<ul class="horizontal">
<li><a id='add-sharee-link' class='sprite add js-action' href="#">Share
with someone</a></li>
- <li><a id="audit-link" class="sprite info" href='#'>Audit sharing</a></li>
+ <li tal:condition = "view/show_sharing_information_link">
+ <a id="sharing-link" class="sprite info" href='+sharing'>Sharing Information</a>
+ </li>
+ <li tal:condition = "view/show_audit_sharing_link">
+ <a id="audit-link" class="sprite info" href='+audit-sharing'>Audit sharing</a>
+ </li>
</ul>
<div tal:define="batch_navigator view/sharees">
