launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:remove-bogus-bug-subscription-filter-test into launchpad:master.

Commit message:
Remove bogus test for BugSubscriptionFilter without subscription

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/397103

BugSubscriptionFilter.structural_subscription is marked as allow_none=False, so it doesn't really make sense to test its behaviour without a related subscription.  In fact there are no rows where this column is NULL on any of our instances, so we ought to make the column formally NOT NULL in the database.  Removing this test is the first step towards being able to do that.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-bogus-bug-subscription-filter-test into launchpad:master.

diff --git a/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py b/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
index 5b9bdc8..6125dfc 100644
--- a/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
+++ b/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
@@ -394,25 +394,6 @@ class TestBugSubscriptionFilterPermissions(TestCaseWithFactory):
         with person_logged_in(self.subscriber):
             bug_subscription_filter.delete()
 
-    def test_write_to_any_user_when_no_subscription(self):
-        """
-        `BugSubscriptionFilter`s can be modifed by any logged-in user when
-        there is no related subscription.
-        """
-        bug_subscription_filter = BugSubscriptionFilter()
-        bug_subscription_filter = ProxyFactory(bug_subscription_filter)
-        # The subscriber can edit the filter.
-        with person_logged_in(self.subscriber):
-            bug_subscription_filter.find_all_tags = True
-        # Any other person can edit the filter.
-        with person_logged_in(self.factory.makePerson()):
-            bug_subscription_filter.find_all_tags = True
-        # Anonymous users are denied rights to edit the filter.
-        with anonymous_logged_in():
-            self.assertRaises(
-                Unauthorized, setattr, bug_subscription_filter,
-                "find_all_tags", True)
-
 
 class TestBugSubscriptionFilterImportance(TestCaseWithFactory):
 
diff --git a/lib/lp/bugs/security.py b/lib/lp/bugs/security.py
index da8bd30..40668d5 100644
--- a/lib/lp/bugs/security.py
+++ b/lib/lp/bugs/security.py
@@ -322,6 +322,4 @@ class EditBugSubscriptionFilter(AuthorizationBase):
     usedfor = IBugSubscriptionFilter
 
     def checkAuthenticated(self, user):
-        return (
-            self.obj.structural_subscription is None or
-            user.inTeam(self.obj.structural_subscription.subscriber))
+        return user.inTeam(self.obj.structural_subscription.subscriber)