launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bugsummary-v2-rebuild-ap into lp:launchpad with lp:launchpad/db-devel as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1017748 in Launchpad itself: "BugSummary assumes that subscription == visibility"
  https://bugs.launchpad.net/launchpad/+bug/1017748

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-rebuild-ap/+merge/117211

This branch teaches bugsummaryrebuild about BugSummary.access_policy, as implemented in the DB in https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-apg-db/+merge/116595, completing the BugSummary.access_policy work. Pretty mechanical.
-- 
https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-rebuild-ap/+merge/117211
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bugsummary-v2-rebuild-ap into lp:launchpad.

=== modified file 'lib/lp/bugs/scripts/bugsummaryrebuild.py'
--- lib/lp/bugs/scripts/bugsummaryrebuild.py	2012-07-24 07:24:58 +0000
+++ lib/lp/bugs/scripts/bugsummaryrebuild.py	2012-07-30 02:13:23 +0000
@@ -140,7 +140,8 @@
     return IStore(RawBugSummary).find(
         (RawBugSummary.status, RawBugSummary.milestone_id,
          RawBugSummary.importance, RawBugSummary.has_patch, RawBugSummary.tag,
-         RawBugSummary.viewed_by_id, RawBugSummary.count),
+         RawBugSummary.viewed_by_id, RawBugSummary.access_policy_id,
+         RawBugSummary.count),
         *get_bugsummary_constraint(target))
 
 
@@ -191,7 +192,8 @@
     key_cols = (
         RawBugSummary.status, RawBugSummary.milestone_id,
         RawBugSummary.importance, RawBugSummary.has_patch,
-        RawBugSummary.tag, RawBugSummary.viewed_by_id)
+        RawBugSummary.tag, RawBugSummary.viewed_by_id,
+        RawBugSummary.access_policy_id)
 
     # Postgres doesn't do bulk updates, so do a delete+add.
     for key, count in updated.iteritems():
@@ -253,7 +255,7 @@
              BugTaskFlat.status, BugTaskFlat.milestone_id,
              BugTaskFlat.importance,
              Alias(BugTaskFlat.latest_patch_uploaded != None, 'has_patch'),
-             BugTaskFlat.access_grants),
+             BugTaskFlat.access_grants, BugTaskFlat.access_policies),
             tables=[BugTaskFlat],
             where=And(
                 BugTaskFlat.duplicateof_id == None,
@@ -271,12 +273,14 @@
 
     # Prepare a union for all combination of privacy and taggedness.
     # It'll return a full set of
-    # (status, milestone, importance, has_patch, tag, viewed_by) rows.
+    # (status, milestone, importance, has_patch, tag, viewed_by, access_policy)
+    # rows.
     common_cols = (
         RelevantTask.status, RelevantTask.milestone_id,
         RelevantTask.importance, RelevantTask.has_patch)
     null_tag = Alias(Cast(None, 'text'), 'tag')
     null_viewed_by = Alias(Cast(None, 'integer'), 'viewed_by')
+    null_policy = Alias(Cast(None, 'integer'), 'access_policy')
 
     tag_join = Join(BugTag, BugTag.bugID == RelevantTask.bug_id)
 
@@ -288,19 +292,31 @@
     unions = Union(
         # Public, tagless
         Select(
-            common_cols + (null_tag, null_viewed_by),
+            common_cols + (null_tag, null_viewed_by, null_policy),
             tables=[RelevantTask], where=public_constraint),
         # Public, tagged
         Select(
-            common_cols + (BugTag.tag, null_viewed_by),
+            common_cols + (BugTag.tag, null_viewed_by, null_policy),
             tables=[RelevantTask, tag_join], where=public_constraint),
-        # Private, tagless
-        Select(
-            common_cols + (null_tag, Unnest(RelevantTask.access_grants)),
-            tables=[RelevantTask], where=private_constraint),
-        # Private, tagged
-        Select(
-            common_cols + (BugTag.tag, Unnest(RelevantTask.access_grants)),
+        # Private, access grant, tagless
+        Select(
+            common_cols +
+            (null_tag, Unnest(RelevantTask.access_grants), null_policy),
+            tables=[RelevantTask], where=private_constraint),
+        # Private, access grant, tagged
+        Select(
+            common_cols +
+            (BugTag.tag, Unnest(RelevantTask.access_grants), null_policy),
+            tables=[RelevantTask, tag_join], where=private_constraint),
+        # Private, access policy, tagless
+        Select(
+            common_cols +
+            (null_tag, null_viewed_by, Unnest(RelevantTask.access_policies)),
+            tables=[RelevantTask], where=private_constraint),
+        # Private, access policy, tagged
+        Select(
+            common_cols +
+            (BugTag.tag, null_viewed_by, Unnest(RelevantTask.access_policies)),
             tables=[RelevantTask, tag_join], where=private_constraint),
         all=True)
 
@@ -308,7 +324,8 @@
     proto_key_cols = (
         BugSummaryPrototype.status, BugSummaryPrototype.milestone_id,
         BugSummaryPrototype.importance, BugSummaryPrototype.has_patch,
-        BugSummaryPrototype.tag, BugSummaryPrototype.viewed_by_id)
+        BugSummaryPrototype.tag, BugSummaryPrototype.viewed_by_id,
+        BugSummaryPrototype.access_policy_id)
     origin = IStore(BugTaskFlat).with_(relevant_tasks).using(
         Alias(unions, 'bugsummary_prototype'))
     results = origin.find(proto_key_cols + (Count(),))

=== modified file 'lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py'
--- lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py	2012-07-12 01:28:17 +0000
+++ lib/lp/bugs/scripts/tests/test_bugsummaryrebuild.py	2012-07-30 02:13:23 +0000
@@ -26,6 +26,7 @@
     rebuild_bugsummary_for_target,
     )
 from lp.registry.enums import InformationType
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.services.database.lpstorm import IStore
 from lp.services.log.logger import BufferLogger
 from lp.testing import TestCaseWithFactory
@@ -107,22 +108,22 @@
         with dbuser('bugsummaryrebuild'):
             apply_bugsummary_changes(
                 product,
-                {(NEW, None, HIGH, False, None, None): 2,
-                (TRIAGED, None, LOW, False, None, None): 4},
+                {(NEW, None, HIGH, False, None, None, None): 2,
+                (TRIAGED, None, LOW, False, None, None, None): 4},
                 {}, [])
         self.assertContentEqual(
-            [(NEW, None, HIGH, False, None, None, 2),
-             (TRIAGED, None, LOW, False, None, None, 4)],
+            [(NEW, None, HIGH, False, None, None, None, 2),
+             (TRIAGED, None, LOW, False, None, None, None, 4)],
             get_bugsummary_rows(product))
 
         # Delete one, mutate the other.
         with dbuser('bugsummaryrebuild'):
             apply_bugsummary_changes(
                 product,
-                {}, {(NEW, None, HIGH, False, None, None): 3},
-                [(TRIAGED, None, LOW, False, None, None)])
+                {}, {(NEW, None, HIGH, False, None, None, None): 3},
+                [(TRIAGED, None, LOW, False, None, None, None)])
         self.assertContentEqual(
-            [(NEW, None, HIGH, False, None, None, 3)],
+            [(NEW, None, HIGH, False, None, None, None, 3)],
             get_bugsummary_rows(product))
 
     def test_rebuild_bugsummary_for_target(self):
@@ -172,7 +173,7 @@
         rollup_journal()
         new_rows = set(get_bugsummary_rows(product))
         self.assertContentEqual(
-            [(task.status, None, task.importance, False, None, None, 1)],
+            [(task.status, None, task.importance, False, None, None, None, 1)],
             new_rows - orig_rows)
 
 
@@ -186,7 +187,7 @@
         product = self.factory.makeProduct()
         bug = self.factory.makeBug(product=product).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, None, None, 1)],
+            [(bug.status, None, bug.importance, False, None, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_public_tagged(self):
@@ -196,35 +197,46 @@
         bug = self.factory.makeBug(
             product=product, tags=[u'foo', u'bar']).default_bugtask
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, None, None, 1),
-             (bug.status, None, bug.importance, False, u'foo', None, 1),
-             (bug.status, None, bug.importance, False, u'bar', None, 1),
+            [(bug.status, None, bug.importance, False, None, None, None, 1),
+             (bug.status, None, bug.importance, False, u'foo', None, None, 1),
+             (bug.status, None, bug.importance, False, u'bar', None, None, 1),
             ], calculate_bugsummary_rows(product))
 
     def test_private_untagged(self):
         # Private untagged bugs show up with tag = None, viewed_by =
-        # subscriber. There's no viewed_by = None row.
+        # subscriber; and tag = None, access_policy = ap. There's no
+        # viewed_by = None, access_policy = None row.
         product = self.factory.makeProduct()
-        owner = self.factory.makePerson()
+        o = self.factory.makePerson()
         bug = self.factory.makeBug(
-            product=product, owner=owner,
+            product=product, owner=o,
             information_type=InformationType.USERDATA).default_bugtask
+        [ap] = getUtility(IAccessPolicySource).find(
+            [(product, InformationType.USERDATA)])
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, None, owner.id, 1)],
+            [(bug.status, None, bug.importance, False, None, o.id, None, 1),
+             (bug.status, None, bug.importance, False, None, None, ap.id, 1)],
             calculate_bugsummary_rows(product))
 
     def test_private_tagged(self):
-        # Private tagged bugs show up with viewed_by = subscriber, with a
-        # row for each tag plus an untagged row.
+        # Private tagged bugs show up with viewed_by = subscriber and
+        # access_policy = ap rows, each with a row for each tag plus an
+        # untagged row.
         product = self.factory.makeProduct()
-        owner = self.factory.makePerson()
+        o = self.factory.makePerson()
         bug = self.factory.makeBug(
-            product=product, owner=owner, tags=[u'foo', u'bar'],
+            product=product, owner=o, tags=[u'foo', u'bar'],
             information_type=InformationType.USERDATA).default_bugtask
+        [ap] = getUtility(IAccessPolicySource).find(
+            [(product, InformationType.USERDATA)])
         self.assertContentEqual(
-            [(bug.status, None, bug.importance, False, None, owner.id, 1),
-             (bug.status, None, bug.importance, False, u'foo', owner.id, 1),
-             (bug.status, None, bug.importance, False, u'bar', owner.id, 1)],
+            [(bug.status, None, bug.importance, False, None, o.id, None, 1),
+             (bug.status, None, bug.importance, False, u'foo', o.id, None, 1),
+             (bug.status, None, bug.importance, False, u'bar', o.id, None, 1),
+             (bug.status, None, bug.importance, False, None, None, ap.id, 1),
+             (bug.status, None, bug.importance, False, u'foo', None, ap.id, 1),
+             (bug.status, None, bug.importance, False, u'bar', None, ap.id, 1),
+            ],
             calculate_bugsummary_rows(product))
 
     def test_aggregation(self):
@@ -236,8 +248,8 @@
         bug3 = self.factory.makeBug(
             product=product, status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, False, None, None, 2),
-             (bug3.status, None, bug3.importance, False, None, None, 1)],
+            [(bug1.status, None, bug1.importance, False, None, None, None, 2),
+             (bug3.status, None, bug3.importance, False, None, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_has_patch(self):
@@ -249,8 +261,8 @@
         bug2 = self.factory.makeBug(
             product=product, status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, True, None, None, 1),
-             (bug2.status, None, bug2.importance, False, None, None, 1)],
+            [(bug1.status, None, bug1.importance, True, None, None, None, 1),
+             (bug2.status, None, bug2.importance, False, None, None, None, 1)],
             calculate_bugsummary_rows(product))
 
     def test_milestone(self):
@@ -264,8 +276,10 @@
             product=product, milestone=mile2,
             status=BugTaskStatus.TRIAGED).default_bugtask
         self.assertContentEqual(
-            [(bug1.status, mile1.id, bug1.importance, False, None, None, 1),
-             (bug2.status, mile2.id, bug2.importance, False, None, None, 1)],
+            [(bug1.status, mile1.id, bug1.importance, False, None, None, None,
+              1),
+             (bug2.status, mile2.id, bug2.importance, False, None, None, None,
+              1)],
             calculate_bugsummary_rows(product))
 
     def test_distribution_includes_packages(self):
@@ -282,7 +296,7 @@
         # The DistributionSourcePackage task shows up in the
         # Distribution's rows.
         self.assertContentEqual(
-            [(bug1.status, None, bug1.importance, False, None, None, 1)],
+            [(bug1.status, None, bug1.importance, False, None, None, None, 1)],
             calculate_bugsummary_rows(dsp.distribution))
         self.assertContentEqual(
             calculate_bugsummary_rows(dsp.distribution),
@@ -290,7 +304,7 @@
 
         # The SourcePackage task shows up in the DistroSeries' rows.
         self.assertContentEqual(
-            [(bug2.status, None, bug2.importance, False, None, None, 1)],
+            [(bug2.status, None, bug2.importance, False, None, None, None, 1)],
             calculate_bugsummary_rows(sp.distroseries))
         self.assertContentEqual(
             calculate_bugsummary_rows(sp.distroseries),