launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04305
[Merge] lp:~gary/launchpad/bug777874 into lp:launchpad
Gary Poster has proposed merging lp:~gary/launchpad/bug777874 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~gary/launchpad/bug777874/+merge/68406
This branch fixes bug 777874 by enabling autoconfirmation of new bugtasks that affect more than one person. It is feature-flagged by project and distribution, though the intent is to roll it out to all Launchpad users.
The discussion in the bug, particularly in comment 11 (https://bugs.launchpad.net/launchpad/+bug/777874/comments/11) is a good background to what I'm doing. The only difference is that I ended up not using events. The call sites were very constrained, and generic modified events would have been more difficult that what I actually needed. gmb was fine with this on a follow-up conversation.
The Python is well tested.
The JavaScript was within a completely untested module, and I did not try to add tests. Moreover, this is the kind of use case that calls out for a client-side MVC pattern. We just don't have it yet.
Tests can be run with
./bin/test -vv lp.bugs.model.tests.test_bug TestBugAutoConfirmation
and
./bin/test -vv lp.bugs.model.tests.test_bugtask TestAutoConfirmBugTasks
I get layer teardown errors when I do this, but it has nothing to do with my branch's changes to my knowledge. I think it is something to do with running tests from a single layer, but I didn't dig into it past a cursory investigation.
File "/home/gary/launchpad/lp-branches/bug777874/lib/canonical/testing/layers.py", line 363, in tearDown
cls.fixture.cleanUp()
AttributeError: 'NoneType' object has no attribute 'cleanUp'
Lint is happy.
I am a bit paranoid that privacy issues will bite this code somehow (e.g., the current user does not have privileges to change the status of a bug and things fall over when they try to set a dupe or say "me too"), but I can't think of a way: the calls I make are either internal to an object, and so not wrapped with a security proxy; or they are publicly allowed. I'd appreciate your consideration of this issue in the review though.
QA/testing:
I'll define two scenarios, then describe what to do with them.
Scenario 1:
A. Go to or make a bug that has a bugtask in a NEW status and only one person affected (but at least one--launchpad.dev has some that are don't affect anyone to start with, like its bug 1) who is not you.
B. Mark the bug as affecting you.
Scenario 2:
A. Go to or make a bug that has a bugtask in a NEW status and only one person affected (but at least one--launchpad.dev has some that are don't affect anyone to start with, like its bug 1) who is not you.
B. Go to another bug and mark it as a duplicate of the first bug.
C. Return to the first bug.
Scenario 3:
A. Go to or make a bug that has a bugtask in a NEW status and that *only affects you*.
B. Go to another bug and mark it as a duplicate of the first bug.
C. Return to the first bug.
First, verify that, without feature flags set, there is no autoconfirm for all three scenarios.
Now set feature flags to turn on the behavior for ubuntu and launchpad. Locally, that means that you go to https://launchpad.dev/+feature-rules . You want these rules.
bugs.autoconfirm.enabled_distribution_names default 1 ubuntu
bugs.autoconfirm.enabled_product_names default 1 launchpad
Now, go through scenarios 1 and 2 for ubuntu and/or launchpad-related bugs and make sure that bugtasks change; and go through scenarios 1 and 2 for other projects and/or distributions and make sure they do not change. Finally, if you want, go through scenario 3 for ubuntu/launchpad and make sure that bugtasks do *not* change.
For extra credit, change the flags to match everything:
bugs.autoconfirm.enabled_distribution_names default 1 *
bugs.autoconfirm.enabled_product_names default 1 *
Now make sure that scenarios 1 and 2 make a change, and 3 does not.
Writing this, it strikes me that distribution names and product names share a namespace, so I didn't need to separate them. I'm not inclined to change anything, but I could if you wanted. It would make the code that we plan to delete anyway a bit simpler.
Thank you!
Gary
--
https://code.launchpad.net/~gary/launchpad/bug777874/+merge/68406
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug777874 into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-06-16 13:50:58 +0000
+++ lib/lp/bugs/configure.zcml 2011-07-19 15:24:29 +0000
@@ -206,6 +206,7 @@
canTransitionToStatus
isSubscribed
getPackageComponent
+ maybeConfirm
userCanEditImportance
userCanEditMilestone
userCanSetAnyAssignee
@@ -748,6 +749,7 @@
getDirectSubscriptions
getIndirectSubscribers
is_complete
+ maybeConfirmBugtasks
official_tags
who_made_private
date_made_private
@@ -755,6 +757,7 @@
personIsDirectSubscriber
personIsAlsoNotifiedSubscriber
personIsSubscribedToDuplicate
+ shouldConfirmBugtasks
heat
heat_last_updated
has_patches
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-07-18 05:32:40 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-07-19 15:24:29 +0000
@@ -658,11 +658,35 @@
Y.fire('lp:branch-linked', bug_branch_node);
}
+var status_choice_data = [];
+
+var update_maybe_confirmed_status = function() {
+ // This would be better done via client-side MVC for the pertinent
+ // bugtasks, but we don't have that yet.
+ Y.Array.each(
+ status_choice_data,
+ function(rowdata) {
+ if (rowdata.widget.get('value') === 'New') {
+ lp_client.get(
+ rowdata.config.bugtask_path,
+ // We will silently fail.
+ // This is not critical functionality.
+ {on: {success: function(bugtask) {
+ var status = bugtask.get('status');
+ if (status !== rowdata.widget.get('value')) {
+ rowdata.widget.set('value', status);
+ rowdata.widget.fire('save');
+ }
+ }}});
+ }
+ }
+ );
+};
/**
* Set up a bug task table row.
*
- * Called once, on load, to initialize the page.
+ * Called once per row, on load, to initialize the page.
*
* @method setup_bugtasks_row
*/
@@ -740,6 +764,8 @@
patch: 'status',
resource: conf.bugtask_path}});
status_choice_edit.render();
+ status_choice_data.push(
+ {widget: status_choice_edit, config: conf});
}
if (conf.user_can_edit_importance) {
var importance_choice_edit = new Y.ChoiceSource({
@@ -1086,6 +1112,11 @@
widget._uiClearWaiting();
MeTooChoiceSource.superclass._saveData.call(
widget, value);
+ if (value && widget.get('others_affected_count') > 0) {
+ // If we increased the affected count to 2 or more,
+ // maybe update the statuses of our bugtasks.
+ update_maybe_confirmed_status();
+ }
},
failure: this.error_handler.getFailureHandler()
},
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-06-21 08:36:42 +0000
+++ lib/lp/bugs/model/bug.py 2011-07-19 15:24:29 +0000
@@ -60,7 +60,6 @@
Select,
SQL,
Sum,
- Union,
)
from storm.info import ClassAlias
from storm.locals import (
@@ -445,20 +444,15 @@
@property
def user_ids_affected_with_dupes(self):
"""Return all IDs of Persons affected by this bug and its dupes.
- The return value is a Storm expression. Running a query with
- this expression returns a result that may contain the same ID
- multiple times, for example if that person is affected via
- more than one duplicate."""
- return Union(
- Select(Person.id,
- And(BugAffectsPerson.person == Person.id,
- BugAffectsPerson.affected,
- BugAffectsPerson.bug == self)),
- Select(Person.id,
- And(BugAffectsPerson.person == Person.id,
- BugAffectsPerson.bug == Bug.id,
- BugAffectsPerson.affected,
- Bug.duplicateof == self.id)))
+ The return value is a Storm expression."""
+ return Select(
+ Person.id,
+ And(BugAffectsPerson.person == Person.id,
+ BugAffectsPerson.affected,
+ Or(BugAffectsPerson.bug == self,
+ And(BugAffectsPerson.bug == Bug.id,
+ Bug.duplicateof == self.id))),
+ distinct=True)
@property
def users_affected_with_dupes(self):
@@ -1780,6 +1774,20 @@
store.flush()
store.invalidate(self)
+ def shouldConfirmBugtasks(self):
+ """Should we try to confirm this bug's bugtasks?
+ The answer is yes if more than one user is affected."""
+ # == 2 would probably be sufficient once we have all legacy bug tasks
+ # confirmed. For now, this is a compromise: we don't need a migration
+ # step, but we will make some unnecessary comparisons.
+ return self.users_affected_count_with_dupes > 1
+
+ def maybeConfirmBugtasks(self):
+ """Maybe try to confirm our new bugtasks."""
+ if self.shouldConfirmBugtasks():
+ for bugtask in self.bugtasks:
+ bugtask.maybeConfirm()
+
def markUserAffected(self, user, affected=True):
"""See `IBug`."""
bap = self._getAffectedUser(user)
@@ -1796,6 +1804,9 @@
if dupe._getAffectedUser(user) is not None:
dupe.markUserAffected(user, affected)
+ if affected:
+ self.maybeConfirmBugtasks()
+
self.updateHeat()
def _markAsDuplicate(self, duplicate_of):
@@ -1836,6 +1847,9 @@
# to 0 (since it's a duplicate, it shouldn't have any heat
# at all).
self.setHeat(0, affected_targets=affected_targets)
+ # Maybe confirm bug tasks, now that more people might be affected
+ # by this bug.
+ duplicate_of.maybeConfirmBugtasks()
else:
# Otherwise, recalculate this bug's heat, since it will be 0
# from having been a duplicate. We also update the bug that
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2011-07-19 10:19:59 +0000
+++ lib/lp/bugs/model/bugtask.py 2011-07-19 15:24:29 +0000
@@ -24,8 +24,11 @@
import datetime
from itertools import chain
from operator import attrgetter
+import re
from lazr.enum import BaseItem
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
import pytz
from sqlobject import (
ForeignKey,
@@ -53,9 +56,11 @@
Store,
)
from zope.component import getUtility
+from zope.event import notify
from zope.interface import (
alsoProvides,
implements,
+ providedBy,
)
from zope.security.proxy import (
isinstance as zope_isinstance,
@@ -263,7 +268,7 @@
raise IllegalRelatedBugTasksParams(
('Cannot search for related tasks to \'%s\', at least one '
'of these parameter has to be empty: %s'
- %(context.name, ", ".join(relevant_fields))))
+ % (context.name, ", ".join(relevant_fields))))
return search_params
@@ -814,6 +819,51 @@
raise ValueError('Unknown debbugs severity "%s".' % severity)
return self.importance
+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+ _parse_launchpad_names = re.compile(r"[a-z0-9][a-z0-9\+\.\-]+").findall
+
+ def _checkAutoconfirmFeatureFlag(self):
+ """Does a feature flag enable automatic switching of our bugtasks?"""
+ # This method should be ripped out if we determine that we like
+ # this behavior for all projects.
+ # This is a bit of a feature flag hack, but has been discussed as
+ # a reasonable way to deploy this quickly.
+ pillar = self.pillar
+ if IDistribution.providedBy(pillar):
+ flag_name = 'bugs.autoconfirm.enabled_distribution_names'
+ else:
+ assert IProduct.providedBy(pillar), 'unexpected pillar'
+ flag_name = 'bugs.autoconfirm.enabled_product_names'
+ enabled = features.getFeatureFlag(flag_name)
+ if enabled is None:
+ return False
+ if (enabled.strip() != '*' and
+ pillar.name not in self._parse_launchpad_names(enabled)):
+ # We are not generically enabled ('*') and our pillar's name
+ # is not explicitly enabled.
+ return False
+ return True
+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+
+ def maybeConfirm(self):
+ """Maybe confirm this bugtask.
+ Only call this if the bug._shouldConfirmBugtasks().
+ This adds the further constraint that the bugtask needs to be NEW,
+ and not imported from an external bug tracker.
+ """
+ if (self.status == BugTaskStatus.NEW
+ and self.bugwatch is None
+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+ and self._checkAutoconfirmFeatureFlag()
+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+ ):
+ user = getUtility(ILaunchpadCelebrities).janitor
+ bugtask_before_modification = Snapshot(
+ self, providing=providedBy(self))
+ self.transitionToStatus(BugTaskStatus.CONFIRMED, user)
+ notify(ObjectModifiedEvent(
+ self, bugtask_before_modification, ['status'], user=user))
+
def canTransitionToStatus(self, new_status, user):
"""See `IBugTask`."""
celebrities = getUtility(ILaunchpadCelebrities)
@@ -1079,6 +1129,12 @@
if self.target != target_before_change:
target_before_change.recalculateBugHeatCache()
self.target.recalculateBugHeatCache()
+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+ # We also should see if we ought to auto-transition to the
+ # CONFIRMED status.
+ if self.bug.shouldConfirmBugtasks():
+ self.maybeConfirm()
+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
def updateTargetNameCache(self, newtarget=None):
"""See `IBugTask`."""
@@ -1791,7 +1847,6 @@
"BugTaskSearchParam.exclude_conjoined cannot be True if "
"BugTaskSearchParam.milestone is not set")
-
if params.project:
# Prevent circular import problems.
from lp.registry.model.product import Product
@@ -1895,7 +1950,6 @@
% sqlvalues(params.structural_subscriber))
has_duplicate_results = True
-
# Remove bugtasks from deactivated products, if necessary.
# We don't have to do this if
# 1) We're searching on bugtasks for a specific product
@@ -2531,7 +2585,8 @@
from lp.bugs.model.bugsummary import BugSummary
conditions = []
# Open bug statuses
- conditions.append(BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES))
+ conditions.append(
+ BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES))
# BugSummary does not include duplicates so no need to exclude.
context_conditions = []
for context in contexts:
@@ -2564,15 +2619,17 @@
"teams AS ("
"SELECT team from TeamParticipation WHERE person=?)",
(user.id,)))
- # Note that because admins can see every bug regardless of subscription
- # they will see rather inflated counts. Admins get to deal.
+ # Note that because admins can see every bug regardless of
+ # subscription they will see rather inflated counts. Admins get to
+ # deal.
if user is None:
conditions.append(BugSummary.viewed_by_id == None)
elif not user.inTeam(admin_team):
conditions.append(
Or(
BugSummary.viewed_by_id == None,
- BugSummary.viewed_by_id.is_in(SQL("SELECT team FROM teams"))
+ BugSummary.viewed_by_id.is_in(
+ SQL("SELECT team FROM teams"))
))
sum_count = Sum(BugSummary.count)
resultset = store.find(group_on + (sum_count,), *conditions)
@@ -3189,7 +3246,7 @@
distro_series_ids = set()
product_ids = set()
product_series_ids = set()
-
+
# Gather all the ids that might have milestones to preload for the
# for the milestone vocabulary
for task in bugtasks:
@@ -3199,11 +3256,11 @@
product_ids.add(task.productID)
product_series_ids.add(task.productseriesID)
- distro_ids.discard(None)
- distro_series_ids.discard(None)
- product_ids.discard(None)
- product_series_ids.discard(None)
-
+ distro_ids.discard(None)
+ distro_series_ids.discard(None)
+ product_ids.discard(None)
+ product_series_ids.discard(None)
+
milestones = store.find(
Milestone,
Or(
@@ -3221,7 +3278,5 @@
Product, Product.id.is_in(product_ids)))
list(store.find(
ProductSeries, ProductSeries.id.is_in(product_series_ids)))
-
+
return milestones
-
-
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2011-06-16 13:50:58 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2011-07-19 15:24:29 +0000
@@ -5,11 +5,14 @@
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.bugs.enum import BugNotificationLevel
+from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.bugs.model.bug import BugSubscriptionInfo
from lp.registry.interfaces.person import PersonVisibility
from lp.testing import (
+ feature_flags,
login_person,
person_logged_in,
+ set_feature_flag,
TestCaseWithFactory,
)
@@ -313,3 +316,101 @@
bug.subscribe(team, person)
bug.setPrivate(True, person)
self.assertFalse(bug.personIsDirectSubscriber(person))
+
+
+class TestBugAutoConfirmation(TestCaseWithFactory):
+ """Tests for auto confirming bugs"""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_shouldConfirmBugtasks_initial_False(self):
+ # After a bug is created, only one person is affected, and we should
+ # not try to confirm bug tasks.
+ bug = self.factory.makeBug()
+ self.assertFalse(bug.shouldConfirmBugtasks())
+
+ def test_shouldConfirmBugtasks_after_another_positively_affected(self):
+ # We should confirm bug tasks if the number of affected users is
+ # more than one.
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ bug.markUserAffected(person)
+ self.assertTrue(bug.shouldConfirmBugtasks())
+
+ def test_shouldConfirmBugtasks_after_another_persons_dupe(self):
+ # We should confirm bug tasks if someone else files a dupe.
+ bug = self.factory.makeBug()
+ duplicate_bug = self.factory.makeBug()
+ with person_logged_in(duplicate_bug.owner):
+ duplicate_bug.markAsDuplicate(bug)
+ self.assertTrue(bug.shouldConfirmBugtasks())
+
+ def test_shouldConfirmBugtasks_after_same_persons_dupe_False(self):
+ # We should not confirm bug tasks if same person files a dupe.
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ duplicate_bug = self.factory.makeBug(owner=bug.owner)
+ duplicate_bug.markAsDuplicate(bug)
+ self.assertFalse(bug.shouldConfirmBugtasks())
+
+ def test_shouldConfirmBugtasks_honors_negatively_affected(self):
+ # We should confirm bug tasks if the number of affected users is
+ # more than one.
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ bug.markUserAffected(bug.owner, False)
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ bug.markUserAffected(person)
+ self.assertFalse(bug.shouldConfirmBugtasks())
+
+ def test_markUserAffected_autoconfirms(self):
+ # markUserAffected will auto confirm if appropriate.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ bug.markUserAffected(person)
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug.bugtasks[0].status)
+
+ def test_markUserAffected_does_not_autoconfirm_wrongly(self):
+ # markUserAffected will not auto confirm if incorrect.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ with person_logged_in(bug.owner):
+ bug.markUserAffected(bug.owner, False)
+ with person_logged_in(person):
+ bug.markUserAffected(person)
+ self.assertEqual(BugTaskStatus.NEW, bug.bugtasks[0].status)
+
+ def test_markAsDuplicate_autoconfirms(self):
+ # markAsDuplicate will auto confirm if appropriate.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug = self.factory.makeBug()
+ duplicate_bug = self.factory.makeBug()
+ with person_logged_in(duplicate_bug.owner):
+ duplicate_bug.markAsDuplicate(bug)
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug.bugtasks[0].status)
+
+ def test_markAsDuplicate_does_not_autoconfirm_wrongly(self):
+ # markAsDuplicate will not auto confirm if incorrect.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug = self.factory.makeBug()
+ with person_logged_in(bug.owner):
+ duplicate_bug = self.factory.makeBug(owner=bug.owner)
+ duplicate_bug.markAsDuplicate(bug)
+ self.assertEqual(BugTaskStatus.NEW, bug.bugtasks[0].status)
=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py 2011-07-04 03:10:41 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py 2011-07-19 15:24:29 +0000
@@ -10,6 +10,7 @@
from testtools.matchers import Equals
from zope.component import getUtility
from zope.interface import providedBy
+from zope.security.proxy import removeSecurityProxy
from canonical.database.sqlbase import flush_database_updates
from canonical.launchpad.searchbuilder import (
@@ -52,11 +53,14 @@
from lp.registry.interfaces.projectgroup import IProjectGroupSet
from lp.testing import (
ANONYMOUS,
+ EventRecorder,
+ feature_flags,
login,
login_person,
logout,
normalize_whitespace,
person_logged_in,
+ set_feature_flag,
StormStatementRecorder,
TestCase,
TestCaseWithFactory,
@@ -1445,3 +1449,205 @@
self.generic_task.sourcepackagename = source_package_name
self.assertEqual(
source_package_name, self.series_task.sourcepackagename)
+
+# START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+# When feature flag code is removed, delete these tests (up to "# END
+# TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.")
+
+
+class TestAutoConfirmBugTasksFlagForProduct(TestCaseWithFactory):
+ """Tests for auto-confirming bug tasks."""
+ # Tests for _checkAutoconfirmFeatureFlag.
+
+ layer = DatabaseFunctionalLayer
+
+ def makeTarget(self):
+ return self.factory.makeProduct()
+
+ flag = u'bugs.autoconfirm.enabled_product_names'
+ alt_flag = u'bugs.autoconfirm.enabled_distribution_names'
+
+ def test_False(self):
+ # With no feature flags turned on, we do not auto-confirm.
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ self.assertFalse(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+ def test_flag_False(self):
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ with feature_flags():
+ set_feature_flag(self.flag, u' ')
+ self.assertFalse(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+ def test_explicit_flag(self):
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ with feature_flags():
+ set_feature_flag(self.flag, bug_task.pillar.name)
+ self.assertTrue(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+ def test_explicit_flag_of_many(self):
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ with feature_flags():
+ set_feature_flag(
+ self.flag, u' foo bar ' + bug_task.pillar.name + ' baz ')
+ self.assertTrue(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+ def test_match_all_flag(self):
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ with feature_flags():
+ set_feature_flag(self.flag, u'*')
+ self.assertTrue(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+ def test_alt_flag_does_not_affect(self):
+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
+ with feature_flags():
+ set_feature_flag(self.alt_flag, bug_task.pillar.name)
+ self.assertFalse(
+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
+
+
+class TestAutoConfirmBugTasksFlagForProductSeries(
+ TestAutoConfirmBugTasksFlagForProduct):
+ """Tests for auto-confirming bug tasks."""
+
+ def makeTarget(self):
+ return self.factory.makeProductSeries()
+
+
+class TestAutoConfirmBugTasksFlagForDistribution(
+ TestAutoConfirmBugTasksFlagForProduct):
+ """Tests for auto-confirming bug tasks."""
+
+ flag = TestAutoConfirmBugTasksFlagForProduct.alt_flag
+ alt_flag = TestAutoConfirmBugTasksFlagForProduct.flag
+
+ def makeTarget(self):
+ return self.factory.makeDistribution()
+
+
+class TestAutoConfirmBugTasksFlagForDistributionSeries(
+ TestAutoConfirmBugTasksFlagForDistribution):
+ """Tests for auto-confirming bug tasks."""
+
+ def makeTarget(self):
+ return self.factory.makeDistroSeries()
+
+
+class TestAutoConfirmBugTasksFlagForDistributionSourcePackage(
+ TestAutoConfirmBugTasksFlagForDistribution):
+ """Tests for auto-confirming bug tasks."""
+
+ def makeTarget(self):
+ return self.factory.makeDistributionSourcePackage()
+
+
+class TestAutoConfirmBugTasksTransitionToTarget(TestCaseWithFactory):
+ """Tests for auto-confirming bug tasks."""
+ # Tests for making sure that switching a task from one project that
+ # does not auto-confirm to another that does performs the auto-confirm
+ # correctly, if appropriate. This is only necessary for as long as a
+ # project may not participate in auto-confirm.
+
+ layer = DatabaseFunctionalLayer
+
+ def test_no_transitionToTarget(self):
+ # We can change the target. If the normal bug conditions do not
+ # hold, there will be no transition.
+ person = self.factory.makePerson()
+ autoconfirm_product = self.factory.makeProduct(owner=person)
+ no_autoconfirm_product = self.factory.makeProduct(owner=person)
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names',
+ autoconfirm_product.name)
+ bug_task = self.factory.makeBugTask(
+ target=no_autoconfirm_product, owner=person)
+ with person_logged_in(person):
+ bug_task.maybeConfirm()
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+ bug_task.transitionToTarget(autoconfirm_product)
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+
+ def test_transitionToTarget(self):
+ # If the conditions *do* hold, though, we will auto-confirm.
+ person = self.factory.makePerson()
+ another_person = self.factory.makePerson()
+ autoconfirm_product = self.factory.makeProduct(owner=person)
+ no_autoconfirm_product = self.factory.makeProduct(owner=person)
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names',
+ autoconfirm_product.name)
+ bug_task = self.factory.makeBugTask(
+ target=no_autoconfirm_product, owner=person)
+ with person_logged_in(another_person):
+ bug_task.bug.markUserAffected(another_person)
+ with person_logged_in(person):
+ bug_task.maybeConfirm()
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+ bug_task.transitionToTarget(autoconfirm_product)
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
+# END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
+
+
+class TestAutoConfirmBugTasks(TestCaseWithFactory):
+ """Tests for auto-confirming bug tasks."""
+ # Tests for maybeConfirm
+
+ layer = DatabaseFunctionalLayer
+
+ def test_auto_confirm(self):
+ # A typical new bugtask auto-confirms.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug_task = self.factory.makeBugTask()
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+ with EventRecorder() as recorder:
+ bug_task.maybeConfirm()
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
+ self.assertEqual(1, len(recorder.events))
+ event = recorder.events[0]
+ self.assertEqual(getUtility(ILaunchpadCelebrities).janitor,
+ event.user)
+ self.assertEqual(['status'], event.edited_fields)
+ self.assertEqual(BugTaskStatus.NEW,
+ event.object_before_modification.status)
+ self.assertEqual(bug_task, event.object)
+
+ def test_do_not_confirm_bugwatch_tasks(self):
+ # A bugwatch bugtask does not auto-confirm.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ product = self.factory.makeProduct()
+ with person_logged_in(product.owner):
+ bug = self.factory.makeBug(
+ product=product, owner=product.owner)
+ bug_task = bug.getBugTask(product)
+ watch = self.factory.makeBugWatch(bug=bug)
+ bug_task.bugwatch = watch
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+ with EventRecorder() as recorder:
+ bug_task.maybeConfirm()
+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
+ self.assertEqual(0, len(recorder.events))
+
+ def test_only_confirm_new_tasks(self):
+ # A non-new bugtask does not auto-confirm.
+ # When feature flag code is removed, remove the next two lines and
+ # dedent the rest.
+ with feature_flags():
+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
+ bug_task = self.factory.makeBugTask()
+ removeSecurityProxy(bug_task).transitionToStatus(
+ BugTaskStatus.CONFIRMED, bug_task.bug.owner)
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
+ with EventRecorder() as recorder:
+ bug_task.maybeConfirm()
+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
+ self.assertEqual(0, len(recorder.events))
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2011-06-16 13:50:58 +0000
+++ lib/lp/services/features/flags.py 2011-07-19 15:24:29 +0000
@@ -26,6 +26,8 @@
'The flag value is set to the given floating point number.'),
('int',
"An integer."),
+ ('space delimited',
+ 'Space-delimited strings.')
])
# Data for generating web-visible feature flag documentation.
@@ -106,6 +108,18 @@
'boolean',
('Enables ranking by pillar affiliation in the person picker.'),
''),
+ ('bugs.autoconfirm.enabled_distribution_names',
+ 'space delimited',
+ ('Enables auto-confirming bugtasks for distributions (and their '
+ 'series and packages). Use the default domain. Specify a single '
+ 'asterisk ("*") to enable for all distributions.'),
+ 'None are enabled'),
+ ('bugs.autoconfirm.enabled_product_names',
+ 'space delimited',
+ ('Enables auto-confirming bugtasks for products (and their '
+ 'series). Use the default domain. Specify a single '
+ 'asterisk ("*") to enable for all products.'),
+ 'None are enabled'),
])
# The set of all flag names that are documented.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-07-15 11:34:12 +0000
+++ lib/lp/testing/factory.py 2011-07-19 15:24:29 +0000
@@ -1677,7 +1677,8 @@
bug = getUtility(IBugSet).createBug(create_bug_params)
if bug_watch_url is not None:
# fromText() creates a bug watch associated with the bug.
- getUtility(IBugWatchSet).fromText(bug_watch_url, bug, owner)
+ with person_logged_in(owner):
+ getUtility(IBugWatchSet).fromText(bug_watch_url, bug, owner)
bugtask = bug.default_bugtask
if date_closed is not None:
bugtask.transitionToStatus(
Follow ups
