launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/spec-notification-respect-sharing into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/spec-notification-respect-sharing/+merge/188988

Do not return people in ISpecification.notificationRecipientAddresses() that do not have permission to see the specification. Also drive-by a few whitespace issues and some spelling.
-- 
https://code.launchpad.net/~stevenk/launchpad/spec-notification-respect-sharing/+merge/188988
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/spec-notification-respect-sharing into lp:launchpad.

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2013-08-19 06:43:04 +0000
+++ lib/lp/blueprints/model/specification.py	2013-10-03 05:27:36 +0000
@@ -566,8 +566,13 @@
             person for person in related_people if person is not None]
         subscribers = [
             subscription.person for subscription in self.subscriptions]
+        notify_people = set(related_people + subscribers)
+        without_access = set(
+            getUtility(IService, 'sharing').getPeopleWithoutAccess(
+                self, notify_people))
+        notify_people -= without_access
         addresses = set()
-        for person in related_people + subscribers:
+        for person in notify_people:
             addresses.update(get_contact_email_addresses(person))
         return sorted(addresses)
 

=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py	2013-06-28 00:49:47 +0000
+++ lib/lp/blueprints/tests/test_specification.py	2013-10-03 05:27:36 +0000
@@ -49,7 +49,10 @@
     SharingPermission,
     SpecificationSharingPolicy,
     )
-from lp.registry.interfaces.accesspolicy import IAccessPolicySource
+from lp.registry.interfaces.accesspolicy import (
+    IAccessArtifactGrantSource,
+    IAccessPolicySource,
+    )
 from lp.security import (
     AdminSpecification,
     EditSpecificationByRelatedPeople,
@@ -303,8 +306,8 @@
                 user, specification, error_expected)
 
     def test_ordinary_user_write_access(self):
-        # Oridnary users can change the whiteborad of public specifications.
-        # They cannot change other attributes of public speicifcaitons and
+        # Ordinary users can change the whiteborad of public specifications.
+        # They cannot change other attributes of public specifications and
         # no attributes of private specifications.
         specification = self.factory.makeSpecification()
         removeSecurityProxy(specification.target)._ensurePolicies(
@@ -462,8 +465,7 @@
         with person_logged_in(owner):
             user = self.factory.makePerson()
             spec = self.factory.makeSpecification(
-                product=product,
-                information_type=InformationType.PROPRIETARY)
+                product=product, information_type=InformationType.PROPRIETARY)
             spec.subscribe(user, subscribed_by=owner)
             service = getUtility(IService, 'sharing')
             ignored, ignored, shared_specs = service.getVisibleArtifacts(
@@ -499,8 +501,7 @@
         with person_logged_in(owner):
             user = self.factory.makePerson()
             spec = self.factory.makeSpecification(
-                product=product,
-                information_type=InformationType.PROPRIETARY)
+                product=product, information_type=InformationType.PROPRIETARY)
             spec.subscribe(user, subscribed_by=owner)
             spec.unsubscribe(user, unsubscribed_by=owner)
             service = getUtility(IService, 'sharing')
@@ -508,6 +509,24 @@
                 user, specifications=[spec])
             self.assertEqual([], shared_specs)
 
+    def test_notificationRecipientAddresses_filters_based_on_sharing(self):
+        # notificationRecipientAddresses() won't return people who do not have
+        # access to the specification.
+        owner = self.factory.makePerson()
+        drafter = self.factory.makePerson()
+        spec = self.factory.makeSpecification(
+            owner=owner, information_type=InformationType.PROPRIETARY)
+        getUtility(IService, 'sharing').ensureAccessGrants(
+            [owner], owner, specifications=[spec], ignore_permissions=True)
+        with person_logged_in(owner):
+            spec.drafter = drafter
+            address = owner.preferredemail.email
+        # Remove the drafters access to the spec.
+        artifact = self.factory.makeAccessArtifact(concrete=spec)
+        getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+            [artifact], [drafter])
+        self.assertEqual([address], spec.notificationRecipientAddresses())
+
 
 class TestSpecificationSet(TestCaseWithFactory):