launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26143
[Merge] ~cjwatson/launchpad:tidy-bugsubscription-filter into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:tidy-bugsubscription-filter into launchpad:master.
Commit message:
Tidy up BugSubscriptionFilter creation
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/397168
`BugSubscriptionFilter` now has a constructor that takes a reasonable set of arguments, and the factory has a `makeBugSubscriptionFilter` method which ensures that new filters have a structural subscription. With this, it is possible to run the test suite with the `BugSubscriptionFilter.structuralsubscription` DB column marked as non-nullable.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:tidy-bugsubscription-filter into launchpad:master.
diff --git a/lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py b/lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py
index a893bce..fe6585e 100644
--- a/lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py
+++ b/lib/lp/bugs/adapters/tests/test_bugsubscriptionfilter.py
@@ -9,13 +9,9 @@ from lp.bugs.adapters.bugsubscriptionfilter import (
bugsubscriptionfilter_to_distribution,
bugsubscriptionfilter_to_product,
)
-from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.product import IProduct
-from lp.testing import (
- login_person,
- TestCaseWithFactory,
- )
+from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
@@ -23,17 +19,10 @@ class BugSubscriptionFilterTestCase(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def makeBugSubscriptionFilter(self, target):
- subscriber = self.factory.makePerson()
- login_person(subscriber)
- subscription = target.addBugSubscription(subscriber, subscriber)
- subscription_filter = BugSubscriptionFilter()
- subscription_filter.structural_subscription = subscription
- return subscription_filter
-
def test_bugsubscriptionfilter_to_product_with_product(self):
product = self.factory.makeProduct()
- subscription_filter = self.makeBugSubscriptionFilter(product)
+ subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=product)
self.assertEqual(
product, bugsubscriptionfilter_to_product(subscription_filter))
self.assertEqual(product, IProduct(subscription_filter))
@@ -41,14 +30,16 @@ class BugSubscriptionFilterTestCase(TestCaseWithFactory):
def test_bugsubscriptionfilter_to_product_with_productseries(self):
product = self.factory.makeProduct()
series = product.development_focus
- subscription_filter = self.makeBugSubscriptionFilter(series)
+ subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=series)
self.assertEqual(
product, bugsubscriptionfilter_to_product(subscription_filter))
self.assertEqual(product, IProduct(subscription_filter))
def test_bugsubscriptionfilter_to_distribution_with_distribution(self):
distribution = self.factory.makeDistribution()
- subscription_filter = self.makeBugSubscriptionFilter(distribution)
+ subscription_filter = self.factory.makeBugSubscriptionFilter(
+ distribution)
self.assertEqual(
distribution,
bugsubscriptionfilter_to_distribution(subscription_filter))
@@ -57,7 +48,8 @@ class BugSubscriptionFilterTestCase(TestCaseWithFactory):
def test_bugsubscriptionfilter_to_distroseries_with_distribution(self):
distribution = self.factory.makeDistribution()
series = self.factory.makeDistroSeries(distribution=distribution)
- subscription_filter = self.makeBugSubscriptionFilter(series)
+ subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=series)
self.assertEqual(
distribution,
bugsubscriptionfilter_to_distribution(subscription_filter))
diff --git a/lib/lp/bugs/model/bugsubscriptionfilter.py b/lib/lp/bugs/model/bugsubscriptionfilter.py
index 712a3c6..3e83aa3 100644
--- a/lib/lp/bugs/model/bugsubscriptionfilter.py
+++ b/lib/lp/bugs/model/bugsubscriptionfilter.py
@@ -81,6 +81,20 @@ class BugSubscriptionFilter(StormBase):
description = Unicode('description')
+ def __init__(self, structural_subscription,
+ bug_notification_level=BugNotificationLevel.COMMENTS,
+ find_all_tags=False,
+ include_any_tags=False, exclude_any_tags=False,
+ other_parameters=None, description=None):
+ super(BugSubscriptionFilter, self).__init__()
+ self.structural_subscription = structural_subscription
+ self.bug_notification_level = bug_notification_level
+ self.find_all_tags = find_all_tags
+ self.include_any_tags = include_any_tags
+ self.exclude_any_tags = exclude_any_tags
+ self.other_parameters = other_parameters
+ self.description = description
+
def _get_collection(self, cls, attribute):
kind = getattr(cls, attribute)
return frozenset(
diff --git a/lib/lp/bugs/model/structuralsubscription.py b/lib/lp/bugs/model/structuralsubscription.py
index c2834e3..59955d3 100644
--- a/lib/lp/bugs/model/structuralsubscription.py
+++ b/lib/lp/bugs/model/structuralsubscription.py
@@ -175,8 +175,7 @@ class StructuralSubscription(Storm):
def newBugFilter(self):
"""See `IStructuralSubscription`."""
- bug_filter = BugSubscriptionFilter()
- bug_filter.structural_subscription = self
+ bug_filter = BugSubscriptionFilter(structural_subscription=self)
# This flush is needed for the web service API.
IStore(StructuralSubscription).flush()
return bug_filter
diff --git a/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py b/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
index 6125dfc..57cfd6d 100644
--- a/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
+++ b/lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
@@ -49,15 +49,14 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_basics(self):
"""Test the basic operation of `BugSubscriptionFilter` objects."""
# Create.
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
- bug_subscription_filter.bug_notification_level = (
- BugNotificationLevel.METADATA)
- bug_subscription_filter.find_all_tags = True
- bug_subscription_filter.include_any_tags = True
- bug_subscription_filter.exclude_any_tags = True
- bug_subscription_filter.other_parameters = u"foo"
- bug_subscription_filter.description = u"bar"
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription,
+ bug_notification_level=BugNotificationLevel.METADATA,
+ find_all_tags=True,
+ include_any_tags=True,
+ exclude_any_tags=True,
+ other_parameters=u"foo",
+ description=u"bar")
# Flush and reload.
IStore(bug_subscription_filter).flush()
IStore(bug_subscription_filter).reload(bug_subscription_filter)
@@ -80,15 +79,16 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_description(self):
"""Test the description property."""
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.description = u"foo"
self.assertEqual(u"foo", bug_subscription_filter.description)
def test_defaults(self):
"""Test the default values of `BugSubscriptionFilter` objects."""
# Create.
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
# Check.
self.assertEqual(
BugNotificationLevel.COMMENTS,
@@ -106,8 +106,8 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
deleted.
"""
# This is a second filter for the subscription.
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
bug_subscription_filter.importances = [BugTaskImportance.LOW]
bug_subscription_filter.statuses = [BugTaskStatus.NEW]
bug_subscription_filter.tags = [u"foo"]
@@ -157,12 +157,14 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_statuses(self):
# The statuses property is a frozenset of the statuses that are
# filtered upon.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(frozenset(), bug_subscription_filter.statuses)
def test_statuses_set(self):
# Assigning any iterable to statuses updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.statuses = [
BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE]
self.assertEqual(
@@ -176,25 +178,29 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_statuses_set_all(self):
# Setting all statuses is normalized into setting no statuses.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.statuses = list(BugTaskStatus.items)
self.assertEqual(frozenset(), bug_subscription_filter.statuses)
def test_statuses_set_empty(self):
# Assigning an empty iterable to statuses updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.statuses = []
self.assertEqual(frozenset(), bug_subscription_filter.statuses)
def test_importances(self):
# The importances property is a frozenset of the importances that are
# filtered upon.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(frozenset(), bug_subscription_filter.importances)
def test_importances_set(self):
# Assigning any iterable to importances updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.importances = [
BugTaskImportance.HIGH, BugTaskImportance.LOW]
self.assertEqual(
@@ -209,26 +215,30 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_importances_set_all(self):
# Setting all importances is normalized into setting no importances.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.importances = list(BugTaskImportance.items)
self.assertEqual(frozenset(), bug_subscription_filter.importances)
def test_importances_set_empty(self):
# Assigning an empty iterable to importances updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.importances = []
self.assertEqual(frozenset(), bug_subscription_filter.importances)
def test_information_types(self):
# The information_types property is a frozenset of the
# information_types that are filtered upon.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(
frozenset(), bug_subscription_filter.information_types)
def test_information_types_set(self):
# Assigning any iterable to information_types updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.information_types = [
InformationType.PRIVATESECURITY, InformationType.USERDATA]
self.assertEqual(
@@ -245,7 +255,8 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_information_types_set_all(self):
# Setting all information_types is normalized into setting no
# information_types.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.information_types = list(
InformationType.items)
self.assertEqual(
@@ -254,19 +265,22 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_information_types_set_empty(self):
# Assigning an empty iterable to information_types updates the
# database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.information_types = []
self.assertEqual(
frozenset(), bug_subscription_filter.information_types)
def test_tags(self):
# The tags property is a frozenset of the tags that are filtered upon.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(frozenset(), bug_subscription_filter.tags)
def test_tags_set(self):
# Assigning any iterable to tags updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.tags = [u"foo", u"-bar"]
self.assertEqual(
frozenset((u"foo", u"-bar")),
@@ -279,14 +293,16 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_tags_set_empty(self):
# Assigning an empty iterable to tags updates the database.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
bug_subscription_filter.tags = []
self.assertEqual(frozenset(), bug_subscription_filter.tags)
def test_tags_set_wildcard(self):
# Setting one or more wildcard tags may update include_any_tags or
# exclude_any_tags.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(frozenset(), bug_subscription_filter.tags)
self.assertFalse(bug_subscription_filter.include_any_tags)
self.assertFalse(bug_subscription_filter.exclude_any_tags)
@@ -315,7 +331,8 @@ class TestBugSubscriptionFilter(TestCaseWithFactory):
def test_tags_with_any_and_all(self):
# If the tags are bundled in a c.l.searchbuilder.any or .all, the
# find_any_tags attribute will also be updated.
- bug_subscription_filter = BugSubscriptionFilter()
+ bug_subscription_filter = self.factory.makeBugSubscriptionFilter(
+ target=self.target)
self.assertEqual(frozenset(), bug_subscription_filter.tags)
self.assertFalse(bug_subscription_filter.find_all_tags)
@@ -348,8 +365,8 @@ class TestBugSubscriptionFilterPermissions(TestCaseWithFactory):
def test_read_to_all(self):
"""`BugSubscriptionFilter`s can be read by anyone."""
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
bug_subscription_filter = ProxyFactory(bug_subscription_filter)
with person_logged_in(self.subscriber):
bug_subscription_filter.find_all_tags
@@ -360,8 +377,8 @@ class TestBugSubscriptionFilterPermissions(TestCaseWithFactory):
def test_write_to_subscribers(self):
"""`BugSubscriptionFilter`s can only be modifed by subscribers."""
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
bug_subscription_filter = ProxyFactory(bug_subscription_filter)
# The subscriber can edit the filter.
with person_logged_in(self.subscriber):
@@ -379,8 +396,8 @@ class TestBugSubscriptionFilterPermissions(TestCaseWithFactory):
def test_delete_by_subscribers(self):
"""`BugSubscriptionFilter`s can only be deleted by subscribers."""
- bug_subscription_filter = BugSubscriptionFilter()
- bug_subscription_filter.structural_subscription = self.subscription
+ bug_subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
bug_subscription_filter = ProxyFactory(bug_subscription_filter)
# Anonymous users are denied rights to delete the filter.
with anonymous_logged_in():
@@ -406,8 +423,8 @@ class TestBugSubscriptionFilterImportance(TestCaseWithFactory):
login_person(self.subscriber)
self.subscription = self.target.addBugSubscription(
self.subscriber, self.subscriber)
- self.subscription_filter = BugSubscriptionFilter()
- self.subscription_filter.structural_subscription = self.subscription
+ self.subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
def test_basics(self):
"""Test the basics of `BugSubscriptionFilterImportance` objects."""
@@ -438,8 +455,8 @@ class TestBugSubscriptionFilterStatus(TestCaseWithFactory):
login_person(self.subscriber)
self.subscription = self.target.addBugSubscription(
self.subscriber, self.subscriber)
- self.subscription_filter = BugSubscriptionFilter()
- self.subscription_filter.structural_subscription = self.subscription
+ self.subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
def test_basics(self):
"""Test the basics of `BugSubscriptionFilterStatus` objects."""
@@ -469,8 +486,8 @@ class TestBugSubscriptionFilterTag(TestCaseWithFactory):
login_person(self.subscriber)
self.subscription = self.target.addBugSubscription(
self.subscriber, self.subscriber)
- self.subscription_filter = BugSubscriptionFilter()
- self.subscription_filter.structural_subscription = self.subscription
+ self.subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
def test_basics(self):
"""Test the basics of `BugSubscriptionFilterTag` objects."""
@@ -517,8 +534,8 @@ class TestBugSubscriptionFilterInformationType(TestCaseWithFactory):
login_person(self.subscriber)
self.subscription = self.target.addBugSubscription(
self.subscriber, self.subscriber)
- self.subscription_filter = BugSubscriptionFilter()
- self.subscription_filter.structural_subscription = self.subscription
+ self.subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
def test_basics(self):
# Test the basics of `BugSubscriptionFilterInformationType` objects.
diff --git a/lib/lp/bugs/tests/test_structuralsubscription.py b/lib/lp/bugs/tests/test_structuralsubscription.py
index 6ca025b..8678532 100644
--- a/lib/lp/bugs/tests/test_structuralsubscription.py
+++ b/lib/lp/bugs/tests/test_structuralsubscription.py
@@ -105,8 +105,8 @@ class TestStructuralSubscription(TestCaseWithFactory):
def test_bug_filters(self):
# The bug_filters attribute returns the BugSubscriptionFilter records
# associated with this subscription.
- subscription_filter = BugSubscriptionFilter()
- subscription_filter.structural_subscription = self.subscription
+ subscription_filter = BugSubscriptionFilter(
+ structural_subscription=self.subscription)
self.assertContentEqual(
[subscription_filter, self.original_filter],
list(self.subscription.bug_filters))
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 0bd8f83..0047f80 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2161,6 +2161,25 @@ class BareLaunchpadObjectFactory(ObjectFactory):
owner, data, comment, filename, content_type=content_type,
description=description, **other_params)
+ def makeBugSubscriptionFilter(self, target=None, subscriber=None,
+ subscribed_by=None):
+ """Create and return a new bug subscription filter.
+
+ :param target: An `IStructuralSubscriptionTarget`. Defaults to a
+ new `Product`.
+ :param subscriber: An `IPerson`. Defaults to a new `Person`.
+ :param subscribed_by: An `IPerson`. Defaults to `subscriber`.
+ :return: An `IBugSubscriptionFilter`.
+ """
+ if target is None:
+ target = self.makeProduct()
+ if subscriber is None:
+ subscriber = self.makePerson()
+ if subscribed_by is None:
+ subscribed_by = subscriber
+ return removeSecurityProxy(target).addBugSubscriptionFilter(
+ subscriber, subscribed_by)
+
def makeSignedMessage(self, msgid=None, body=None, subject=None,
attachment_contents=None, force_transfer_encoding=False,
email_address=None, signing_context=None, to_address=None):
Follow ups
