launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/filter-milestone-vocabulary-for-projectgroups into lp:launchpad.

Commit message:
Updates project group milestone filtering to not display milestones from associated private products if the user shouldn't see them.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/filter-milestone-vocabulary-for-projectgroups/+merge/134667

Summary
=======
As it currently stands, when milestones are fetched for a projectgroup, the
milestones from all associated products are fetched, regardless of whether or
not they belong to a private product the current user cannot see.

Projectgroup must be updated to not show milestones the user can't see, using
our existing filtering clause.

Preimp
======
None.

Implementation
==============
The method `ProjectGroup`.`_getMilestoneCondition`, which creates the query to
fetch milestones, is updated to pull the current user from the launchbag and
create the privacy filtering clause from that. It then adds that clause to the
existing query.

Tests have also been updated for this case.

Tests
=====
bin/test -vvct TestMilestoneVocabulary

QA
==
Ensure the milestones listed for a project does not include anything from
associated private products.

LoC
===
Part of private projects.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/model/projectgroup.py
  lib/lp/registry/tests/test_milestone_vocabularies.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/filter-milestone-vocabulary-for-projectgroups/+merge/134667
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/filter-milestone-vocabulary-for-projectgroups into lp:launchpad.

=== modified file 'lib/lp/registry/model/projectgroup.py'
--- lib/lp/registry/model/projectgroup.py	2012-10-19 03:01:32 +0000
+++ lib/lp/registry/model/projectgroup.py	2012-11-16 14:13:37 +0000
@@ -407,8 +407,12 @@
 
     def _getMilestoneCondition(self):
         """See `HasMilestonesMixin`."""
-        return And(Milestone.productID == Product.id,
-                   Product.projectID == self.id)
+        user = getUtility(ILaunchBag).user
+        privacy_filter = ProductSet.getProductPrivacyFilter(user)
+        return And(
+            Milestone.productID == Product.id,
+            Product.projectID == self.id,
+            privacy_filter)
 
     def _getMilestones(self, user, only_active):
         """Return a list of milestones for this project group.

=== modified file 'lib/lp/registry/tests/test_milestone_vocabularies.py'
--- lib/lp/registry/tests/test_milestone_vocabularies.py	2012-10-11 04:57:59 +0000
+++ lib/lp/registry/tests/test_milestone_vocabularies.py	2012-11-16 14:13:37 +0000
@@ -5,10 +5,9 @@
 
 __metaclass__ = type
 
-from unittest import TestCase
-
 from zope.component import getUtility
 
+from lp.app.enums import InformationType
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
@@ -18,20 +17,47 @@
 from lp.testing import (
     login,
     logout,
+    person_logged_in,
+    TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
 
 
-class TestMilestoneVocabulary(TestCase):
+class TestMilestoneVocabulary(TestCaseWithFactory):
     """Test that the MilestoneVocabulary behaves as expected."""
+
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        super(TestMilestoneVocabulary, self).setUp()
         login('test@xxxxxxxxxxxxx')
 
     def tearDown(self):
+        super(TestMilestoneVocabulary, self).tearDown()
         logout()
 
+    def test_project_group_does_not_show_nonpublic_products(self):
+        # Milestones for a projectgroup should not include those on an
+        # associated private product.
+        owner = self.factory.makePerson()
+        group = self.factory.makeProject(owner=owner)
+        public = self.factory.makeProduct(project=group, owner=owner)
+        private = self.factory.makeProduct(project=group, owner=owner,
+            information_type=InformationType.PROPRIETARY)
+        with person_logged_in(owner):
+            m1 = self.factory.makeMilestone(name='public', product=public)
+            m2 = self.factory.makeMilestone(name='private', product=private)
+        vocabulary = MilestoneVocabulary(group)
+        expected = [m1.title]
+        listing = [term.title for term in vocabulary]
+        self.assertEqual(expected, listing)
+
+        with person_logged_in(owner):
+            vocabulary = MilestoneVocabulary(group)
+            expected = [m1.title, m2.title]
+            listing = [term.title for term in vocabulary]
+            self.assertEqual(expected, listing)
+
     def testProductMilestoneVocabulary(self):
         """Test of MilestoneVocabulary for a product."""
         firefox = getUtility(IProductSet).getByName('firefox')