Ines Almeida has proposed merging
~ines-almeida/launchpad:invert-bug-webhook-feature-flag into launchpad:master.
Commit message:
Invert bug webhooks feature flag to be enabled by default
For feature flags that we intend to keep enabled (and particularly for the unit
tests to run with the newly added feature) this change is inverting this
feature flag so that the feature is enabled by default. To disable the feature,
one just needs to set the "bugs.webhooks.disabled" feature flag
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/445657
I haven't run the full test suite in this (I ran enough to feel confident to MP
it), so we might discover a couple extra places that are missing permissions
when all the full test suite runs after merge.
IMO, there might still be value in setting feature flags globally for testing,
and I opened a ticket to potentially address that in the future
(https://warthogs.atlassian.net/browse/LP-1272). For this particular case, I
think this is reasonable.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~ines-almeida/launchpad:invert-bug-webhook-feature-flag into launchpad:master.
diff --git a/lib/lp/bugs/interfaces/bugtarget.py b/lib/lp/bugs/interfaces/bugtarget.py
index 540079f..3a090e6 100644
--- a/lib/lp/bugs/interfaces/bugtarget.py
+++ b/lib/lp/bugs/interfaces/bugtarget.py
@@ -15,7 +15,7 @@ __all__ = [
"ISeriesBugTarget",
"BUG_POLICY_ALLOWED_TYPES",
"BUG_POLICY_DEFAULT_TYPES",
- "BUG_WEBHOOKS_FEATURE_FLAG",
+ "DISABLE_BUG_WEBHOOKS_FEATURE_FLAG",
]
@@ -157,7 +157,7 @@ BUG_POLICY_DEFAULT_TYPES = {
}
-BUG_WEBHOOKS_FEATURE_FLAG = "bugs.webhooks.enabled"
+DISABLE_BUG_WEBHOOKS_FEATURE_FLAG = "bugs.webhooks.disabled"
@exported_as_webservice_entry(as_of="beta")
diff --git a/lib/lp/bugs/subscribers/webhooks.py b/lib/lp/bugs/subscribers/webhooks.py
index 4ea257f..8d1b56f 100644
--- a/lib/lp/bugs/subscribers/webhooks.py
+++ b/lib/lp/bugs/subscribers/webhooks.py
@@ -10,7 +10,7 @@ from zope.component import getUtility
from lp.bugs.interfaces.bug import IBug
from lp.bugs.interfaces.bugmessage import IBugMessage
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtarget import DISABLE_BUG_WEBHOOKS_FEATURE_FLAG
from lp.bugs.interfaces.bugtask import IBugTask
from lp.bugs.subscribers.bugactivity import what_changed
from lp.services.database.sqlbase import block_implicit_flushes
@@ -27,7 +27,7 @@ def _trigger_bugtask_webhook(
previous_state: Union[IBug, IBugTask] = None,
):
"""Triggers 'bug' event for a specific bugtask"""
- if not getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG):
+ if not getFeatureFlag(DISABLE_BUG_WEBHOOKS_FEATURE_FLAG):
return
if IWebhookTarget.providedBy(bugtask.target):
@@ -38,7 +38,7 @@ def _trigger_bugtask_webhook(
@block_implicit_flushes
def _trigger_bug_comment_webhook(action: str, bug_comment: IBugMessage):
"""Triggers 'bug:comment' events for each bug target for that comment"""
- if not getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG):
+ if not getFeatureFlag(DISABLE_BUG_WEBHOOKS_FEATURE_FLAG):
return
bugtasks = bug_comment.bug.bugtasks
diff --git a/lib/lp/bugs/tests/test_subscribers.py b/lib/lp/bugs/tests/test_subscribers.py
index 7bddc92..938002d 100644
--- a/lib/lp/bugs/tests/test_subscribers.py
+++ b/lib/lp/bugs/tests/test_subscribers.py
@@ -9,7 +9,7 @@ from testtools.matchers import Equals, MatchesDict, MatchesStructure
from zope.event import notify
from lp.bugs.interfaces.bug import IBug
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtarget import DISABLE_BUG_WEBHOOKS_FEATURE_FLAG
from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.bugs.subscribers.bugactivity import what_changed
from lp.services.features.testing import FeatureFixture
@@ -63,7 +63,7 @@ class TestBugWebhooksTriggered(TestCaseWithFactory):
self.useFixture(
FeatureFixture(
{
- BUG_WEBHOOKS_FEATURE_FLAG: "on",
+ DISABLE_BUG_WEBHOOKS_FEATURE_FLAG: "on",
}
)
)
diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index b990f6f..716da3d 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -83,7 +83,7 @@ from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionTargetTraversalMixin,
expose_structural_subscription_data_to_js,
)
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtarget import DISABLE_BUG_WEBHOOKS_FEATURE_FLAG
from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
from lp.registry.browser import RegistryEditFormView, add_subscribe_link
@@ -440,7 +440,7 @@ class DistributionNavigationMenu(NavigationMenu, DistributionLinksMixin):
"+webhooks",
"Manage webhooks",
icon="edit",
- enabled=bool(getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG)),
+ enabled=not (getFeatureFlag(DISABLE_BUG_WEBHOOKS_FEATURE_FLAG)),
)
diff --git a/lib/lp/registry/browser/distributionsourcepackage.py b/lib/lp/registry/browser/distributionsourcepackage.py
index e0d960a..f7df149 100644
--- a/lib/lp/registry/browser/distributionsourcepackage.py
+++ b/lib/lp/registry/browser/distributionsourcepackage.py
@@ -42,7 +42,7 @@ from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionTargetTraversalMixin,
expose_structural_subscription_data_to_js,
)
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtarget import DISABLE_BUG_WEBHOOKS_FEATURE_FLAG
from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
@@ -179,7 +179,7 @@ class DistributionSourcePackageLinksMixin:
"+webhooks",
"Manage webhooks",
icon="edit",
- enabled=bool(getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG)),
+ enabled=not (getFeatureFlag(DISABLE_BUG_WEBHOOKS_FEATURE_FLAG)),
)
diff --git a/lib/lp/registry/browser/product.py b/lib/lp/registry/browser/product.py
index 907d7f1..a75d7c0 100644
--- a/lib/lp/registry/browser/product.py
+++ b/lib/lp/registry/browser/product.py
@@ -111,7 +111,7 @@ from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionTargetTraversalMixin,
expose_structural_subscription_data_to_js,
)
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtarget import DISABLE_BUG_WEBHOOKS_FEATURE_FLAG
from lp.bugs.interfaces.bugtask import RESOLVED_BUGTASK_STATUSES
from lp.charms.browser.hascharmrecipes import HasCharmRecipesMenuMixin
from lp.code.browser.branchref import BranchRef
@@ -521,7 +521,7 @@ class ProductEditLinksMixin(StructuralSubscriptionMenuMixin):
"+webhooks",
"Manage webhooks",
icon="edit",
- enabled=bool(getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG)),
+ enabled=not (getFeatureFlag(DISABLE_BUG_WEBHOOKS_FEATURE_FLAG)),
)
diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index 5d42d9d..4ed46ff 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -296,9 +296,10 @@ flag_info = sorted(
"",
),
(
- "bugs.webhooks.enabled",
+ "bugs.webhooks.disabled",
"boolean",
- "If true, allow adding webhooks to bug updates and comments",
+ "If true, disable webhooks from being triggered for bug updates "
+ "and comments.",
"",
"",
"",
diff --git a/lib/lp/services/webhooks/tests/test_browser.py b/lib/lp/services/webhooks/tests/test_browser.py
index 4392228..7a73803 100644
--- a/lib/lp/services/webhooks/tests/test_browser.py
+++ b/lib/lp/services/webhooks/tests/test_browser.py
@@ -10,7 +10,6 @@ import transaction
from testtools.matchers import MatchesAll, MatchesStructure, Not
from zope.component import getUtility
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
from lp.charms.interfaces.charmrecipe import (
CHARM_RECIPE_ALLOW_CREATE,
CHARM_RECIPE_WEBHOOKS_FEATURE_FLAG,
@@ -192,14 +191,12 @@ class BugUpdateTestHelpersBase:
class ProductTestHelpers(BugUpdateTestHelpersBase):
def makeTarget(self):
- self.useFixture(FeatureFixture({BUG_WEBHOOKS_FEATURE_FLAG: "on"}))
owner = self.factory.makePerson()
return self.factory.makeProduct(owner=owner)
class DistributionTestHelpers(BugUpdateTestHelpersBase):
def makeTarget(self):
- self.useFixture(FeatureFixture({BUG_WEBHOOKS_FEATURE_FLAG: "on"}))
owner = self.factory.makePerson()
return self.factory.makeDistribution(owner=owner)
@@ -209,7 +206,6 @@ class DistributionSourcePackageTestHelpers(BugUpdateTestHelpersBase):
return self.target.distribution.owner
def makeTarget(self):
- self.useFixture(FeatureFixture({BUG_WEBHOOKS_FEATURE_FLAG: "on"}))
return self.factory.makeDistributionSourcePackage()
diff --git a/lib/lp/soyuz/stories/distribution/xx-distribution-packages.rst b/lib/lp/soyuz/stories/distribution/xx-distribution-packages.rst
index 72471e9..ab342f1 100644
--- a/lib/lp/soyuz/stories/distribution/xx-distribution-packages.rst
+++ b/lib/lp/soyuz/stories/distribution/xx-distribution-packages.rst
@@ -333,6 +333,7 @@ menu, and a "Subscribers" portlet.
Subscribe to bug mail
Edit bug mail
Configure bug tracker
+ Manage webhooks
>>> print(
... extract_text(find_tag_by_id(user_browser.contents, "involvement"))
diff --git a/lib/lp/soyuz/tests/test_packagecopyjob.py b/lib/lp/soyuz/tests/test_packagecopyjob.py
index f4d2e53..ddebebd 100644
--- a/lib/lp/soyuz/tests/test_packagecopyjob.py
+++ b/lib/lp/soyuz/tests/test_packagecopyjob.py
@@ -18,7 +18,6 @@ from zope.component import getUtility
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
-from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
@@ -1664,16 +1663,6 @@ class PlainPackageCopyJobTests(TestCaseWithFactory, LocalTestHelper):
def test_copying_closes_bugs_with_webhooks(self):
# Copying a package into a primary archive closes any bugs mentioned
# in its recent changelog, including triggering their webhooks.
-
- # Set bug webhooks feature flag on for the purpose of this test
- self.useFixture(
- FeatureFixture(
- {
- BUG_WEBHOOKS_FEATURE_FLAG: "on",
- }
- )
- )
-
target_archive = self.factory.makeArchive(
self.distroseries.distribution, purpose=ArchivePurpose.PRIMARY
)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
