launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/stop-using-legacy-bug-access into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/stop-using-legacy-bug-access/+merge/114094

This branch removes the feature flag guarding revocation of access grants when unsubscribing a person from a bug. This is a prereq for removing the triggers that these bits of the app replace.

It's been tested for a few weeks on qastaging and coexists with the triggers, so we can turn it on by default now.
-- 
https://code.launchpad.net/~wgrant/launchpad/stop-using-legacy-bug-access/+merge/114094
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/stop-using-legacy-bug-access into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-07-09 22:53:34 +0000
+++ database/schema/security.cfg	2012-07-10 04:34:24 +0000
@@ -1684,7 +1684,7 @@
 [processmail]
 groups=script
 public.accessartifact                   = SELECT, INSERT, DELETE
-public.accessartifactgrant              = SELECT, INSERT
+public.accessartifactgrant              = SELECT, INSERT, DELETE
 public.accesspolicy                     = SELECT
 public.accesspolicyartifact             = SELECT, INSERT, DELETE
 public.accesspolicygrant                = SELECT

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-07-03 08:04:35 +0000
+++ lib/lp/bugs/model/bug.py	2012-07-10 04:34:24 +0000
@@ -897,13 +897,11 @@
                 self.updateHeat()
                 del get_property_cache(self)._known_viewers
 
-                # Revoke access to bug if feature flag is on.
-                flag = 'disclosure.legacy_subscription_visibility.enabled'
-                if bool(getFeatureFlag(flag)):
-                    artifacts_to_delete = getUtility(
-                        IAccessArtifactSource).find([self])
-                    getUtility(IAccessArtifactGrantSource).revokeByArtifact(
-                        artifacts_to_delete, [person])
+                # Revoke access to bug
+                artifacts_to_delete = getUtility(
+                    IAccessArtifactSource).find([self])
+                getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+                    artifacts_to_delete, [person])
                 return
 
     def unsubscribeFromDupes(self, person, unsubscribed_by):

=== modified file 'lib/lp/bugs/tests/test_bugvisibility.py'
--- lib/lp/bugs/tests/test_bugvisibility.py	2012-06-08 02:16:58 +0000
+++ lib/lp/bugs/tests/test_bugvisibility.py	2012-07-10 04:34:24 +0000
@@ -16,9 +16,7 @@
     )
 
 
-LEGACY_VISIBILITY_FLAG = {
-    u"disclosure.enhanced_sharing.writable": "true",
-    u"disclosure.legacy_subscription_visibility.enabled": u"true"}
+LEGACY_VISIBILITY_FLAG = {u"disclosure.enhanced_sharing.writable": "true"}
 
 
 class TestPublicBugVisibility(TestCaseWithFactory):
@@ -114,19 +112,6 @@
                     self.bug.unsubscribe(user, self.owner)
             self.assertFalse(self.bug.userCanView(user))
 
-    def test_privateBugUnsubscribeRetainsVisibility(self):
-        # A person unsubscribed from a private bug can still see it if the
-        # feature flag to enable legacy subscription visibility is not set.
-        # This test disables the current temporary database triggers which
-        # mirror subscription status into visibility.
-        user = self.factory.makePerson()
-        with celebrity_logged_in('admin'):
-            self.bug.subscribe(user, self.owner)
-            self.assertTrue(self.bug.userCanView(user))
-            with self.disable_trigger_fixture:
-                self.bug.unsubscribe(user, self.owner)
-        self.assertTrue(self.bug.userCanView(user))
-
     def test_subscribeGrantsVisibilityWithTriggersRemoved(self):
         # When a user is subscribed to a bug, they are granted access. In this
         # test, the database triggers are removed and so model code is used.

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-06-26 10:19:37 +0000
+++ lib/lp/services/features/flags.py	2012-07-10 04:34:24 +0000
@@ -280,13 +280,6 @@
      '',
      '',
      ''),
-    ('disclosure.legacy_subscription_visibility.enabled',
-     'boolean',
-     ('If true, the legacy behaviour of unsubscribing from a bug or branch'
-      'revokes access.'),
-     '',
-     '',
-     ''),
     ('disclosure.enhanced_sharing.writable',
      'boolean',
      ('If true, will allow the use of the new sharing view and apis used '