launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-830803 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #830803 in Launchpad itself: "BugTask target key attributes should not be directly writable"
  https://bugs.launchpad.net/launchpad/+bug/830803

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-830803/+merge/72378

Security declarations presently permit direct setting of BugTask.{distribution,distroseries,product,productseries,sourcepackagename}. But additional rules and restrictions apply to these, so all writes should go through transitionToTarget.

This branch makes those attributes read-only. Due to my 10 earlier branches, just one test needed fixing: test_bugsummary has reason to poke those attributes directly, so now uses removeSecurityProxy.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-830803/+merge/72378
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-830803 into lp:launchpad.

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-08-15 13:59:05 +0000
+++ lib/lp/bugs/configure.zcml	2011-08-22 06:28:25 +0000
@@ -269,17 +269,12 @@
                     age
                     bugwatch
                     datecreated
-                    distribution
-                    distroseries
                     importance
                     milestone
                     owner
-                    product
-                    productseries
                     related_tasks
                     setImportanceFromDebbugs
                     setStatusFromDebbugs
-                    sourcepackagename
                     statusdisplayhtml
                     statuselsewhere
                     statusexplanation

=== modified file 'lib/lp/bugs/model/tests/test_bugsummary.py'
--- lib/lp/bugs/model/tests/test_bugsummary.py	2011-08-03 11:00:11 +0000
+++ lib/lp/bugs/model/tests/test_bugsummary.py	2011-08-22 06:28:25 +0000
@@ -404,7 +404,7 @@
             self.getPublicCount(BugSummary.product == product_b),
             0)
 
-        bug_task.product = product_b
+        removeSecurityProxy(bug_task).product = product_b
 
         self.assertEqual(
             self.getPublicCount(BugSummary.product == product_a),
@@ -473,7 +473,7 @@
             self.getPublicCount(BugSummary.productseries == productseries_a),
             1)
 
-        series_task.productseries = productseries_b
+        removeSecurityProxy(series_task).productseries = productseries_b
 
         self.assertEqual(
             self.getPublicCount(BugSummary.product == product),
@@ -523,7 +523,7 @@
             self.getPublicCount(BugSummary.distribution == distribution_a),
             1)
 
-        bug_task.distribution = distribution_b
+        removeSecurityProxy(bug_task).distribution = distribution_b
 
         self.assertEqual(
             self.getPublicCount(BugSummary.distribution == distribution_a),
@@ -587,7 +587,7 @@
             self.getPublicCount(BugSummary.distroseries == series_b),
             0)
 
-        bug_task.distroseries = series_b
+        removeSecurityProxy(bug_task).distroseries = series_b
 
         self.assertEqual(
             self.getPublicCount(BugSummary.distribution == distribution),
@@ -667,7 +667,8 @@
                     == sourcepackage_b.sourcepackagename),
             0)
 
-        bug_task.sourcepackagename = sourcepackage_b.sourcepackagename
+        removeSecurityProxy(bug_task).sourcepackagename = (
+            sourcepackage_b.sourcepackagename)
 
         self.assertEqual(
             self.getPublicCount(
@@ -706,7 +707,7 @@
                     == sourcepackage.sourcepackagename),
             1)
 
-        bug_task.sourcepackagename = None
+        removeSecurityProxy(bug_task).sourcepackagename = None
 
         self.assertEqual(
             self.getPublicCount(
@@ -788,7 +789,7 @@
                 BugSummary.sourcepackagename == sourcepackagename_b),
             0)
 
-        bug_task.sourcepackagename = sourcepackagename_b
+        removeSecurityProxy(bug_task).sourcepackagename = sourcepackagename_b
 
         self.assertEqual(
             self.getPublicCount(
@@ -849,7 +850,7 @@
                 BugSummary.sourcepackagename == sourcepackagename),
             1)
 
-        bug_task.sourcepackagename = None
+        removeSecurityProxy(bug_task).sourcepackagename = None
 
         self.assertEqual(
             self.getPublicCount(