launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/sharee-picker-exclude-existing-933828 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933828 in Launchpad itself: "Sharee picker includes existing users to be selected"
  https://bugs.launchpad.net/launchpad/+bug/933828

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharee-picker-exclude-existing-933828/+merge/99464

== Implementation ==

Provide a new person/team vocabulary which only returns people not already a sharee of the pillar context. The filtering is done by the vocab's extra_clause method.

This functionality is used on the pillar sharing page. When "Select someone to share with" is clicked, the sharee picker will now not show anyone who is already a sharee.

== Tests ==

Add new vocab test case: TestNewPillarShareeVocabulary
Update PillarSharingViewTestMixin.test_picker_config

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/vocabularies.py
  lib/lp/registry/vocabularies.zcml
  lib/lp/registry/browser/pillar.py
  lib/lp/registry/browser/tests/test_pillar_sharing.py
  lib/lp/registry/tests/test_person_vocabularies.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/sharee-picker-exclude-existing-933828/+merge/99464
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharee-picker-exclude-existing-933828 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/browser/pillar.py	2012-03-27 04:09:19 +0000
@@ -285,7 +285,7 @@
     @property
     def sharing_picker_config(self):
         return dict(
-            vocabulary='ValidPillarOwner',
+            vocabulary='NewPillarSharee',
             vocabulary_filters=self.sharing_vocabulary_filters,
             header='Share with a user or team')
 

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-27 04:09:19 +0000
@@ -199,7 +199,7 @@
                 'Share with a user or team',
                 picker_config['header'])
             self.assertEqual(
-                'ValidPillarOwner', picker_config['vocabulary'])
+                'NewPillarSharee', picker_config['vocabulary'])
 
     def test_view_data_model(self):
         # Test that the json request cache contains the view data model.

=== modified file 'lib/lp/registry/tests/test_person_vocabularies.py'
--- lib/lp/registry/tests/test_person_vocabularies.py	2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_person_vocabularies.py	2012-03-27 04:09:19 +0000
@@ -359,3 +359,21 @@
     def test_supported_filters(self):
         # The vocab shouldn't support person or team filters.
         self.assertEqual([], self.getVocabulary(None).supportedFilters())
+
+
+class TestNewPillarShareeVocabulary(VocabularyTestBase,
+                                        TestCaseWithFactory):
+    """Test that the NewPillarShareeVocabulary behaves as expected."""
+
+    layer = LaunchpadZopelessLayer
+    vocabulary_name = 'NewPillarSharee'
+
+    def test_existing_grantees_excluded(self):
+        # Existing grantees should be excluded from the results.
+        product = self.factory.makeProduct()
+        person1 = self.factory.makePerson(name='sharee1')
+        person2 = self.factory.makePerson(name='sharee2')
+        policy = self.factory.makeAccessPolicy(pillar=product)
+        self.factory.makeAccessPolicyGrant(policy=policy, grantee=person1)
+        [newsharee] = self.searchVocabulary(product, 'sharee')
+        self.assertEqual(newsharee, person2)

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-03-05 18:32:53 +0000
+++ lib/lp/registry/vocabularies.py	2012-03-27 04:09:19 +0000
@@ -41,6 +41,7 @@
     'FilteredProductSeriesVocabulary',
     'KarmaCategoryVocabulary',
     'MilestoneVocabulary',
+    'NewPillarShareeVocabulary',
     'NonMergedPeopleAndTeamsVocabulary',
     'person_team_participations_vocabulary_factory',
     'PersonAccountToMergeVocabulary',
@@ -101,6 +102,7 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.blueprints.interfaces.specification import ISpecification
 from lp.bugs.interfaces.bugtask import IBugTask
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.distribution import (
     IDistribution,
     IDistributionSet,
@@ -1040,6 +1042,32 @@
         return obj in self._get_teams()
 
 
+class NewPillarShareeVocabulary(TeamVocabularyMixin,
+                                    ValidPersonOrTeamVocabulary):
+    """The set of people and teams with whom to share information.
+
+    A person or team is eligible for sharing with if they are not already an
+    existing sharee for the pillar.
+    """
+
+    def __init__(self, context):
+        assert IPillar.providedBy(context)
+        super(ValidPersonOrTeamVocabulary, self).__init__(context)
+        aps = getUtility(IAccessPolicySource)
+        access_policies = aps.findByPillar([self.context])
+        self.policy_ids = [policy.id for policy in access_policies]
+
+    @property
+    def extra_clause(self):
+        clause = SQL("""
+            Person.id NOT IN (
+                SELECT grantee FROM AccessPolicyGrantFlat
+                WHERE policy in %s
+                )
+            """ % sqlvalues(self.policy_ids))
+        return clause
+
+
 class ActiveMailingListVocabulary(FilteredVocabularyBase):
     """The set of all active mailing lists."""
 

=== modified file 'lib/lp/registry/vocabularies.zcml'
--- lib/lp/registry/vocabularies.zcml	2012-01-20 03:02:50 +0000
+++ lib/lp/registry/vocabularies.zcml	2012-03-27 04:09:19 +0000
@@ -430,6 +430,17 @@
     <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
   </class>
 
+  <securedutility
+    name="NewPillarSharee"
+    component="lp.registry.vocabularies.NewPillarShareeVocabulary"
+    provides="zope.schema.interfaces.IVocabularyFactory"
+    >
+    <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+  </securedutility>
+
+  <class class="lp.registry.vocabularies.NewPillarShareeVocabulary">
+    <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
+  </class>
 
   <securedutility
     name="DistributionOrProduct"