Merge lp:~gary/launchpad/bug777874 into lp:launchpad

Proposed by Gary Poster
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 13470
Proposed branch: lp:~gary/launchpad/bug777874
Merge into: lp:launchpad
Diff against target: 756 lines (+458/-33)
8 files modified
lib/lp/bugs/configure.zcml (+3/-0)
lib/lp/bugs/javascript/bugtask_index.js (+32/-1)
lib/lp/bugs/model/bug.py (+29/-15)
lib/lp/bugs/model/bugtask.py (+71/-16)
lib/lp/bugs/model/tests/test_bug.py (+101/-0)
lib/lp/bugs/model/tests/test_bugtask.py (+206/-0)
lib/lp/services/features/flags.py (+14/-0)
lib/lp/testing/factory.py (+2/-1)
To merge this branch: bzr merge lp:~gary/launchpad/bug777874
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+68406@code.launchpad.net

Commit message

[r=gmb][bug=777874] enable autocompletion of "New" bugtasks that affect more than one person. Feature-flagged by project/distro.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

One other comment: I changed user_ids_affected_with_dupes because I thought I needed to. It turned out I was wrong, but the change still seemed good to me. If you disagree, I'm happy to reinstate the old code.

Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Robert Collins (lifeless) wrote :

Two comments:
 - changing the user affects union to an or may throw out bug search
query plans; you'll need to test this with an eye towards timeouts.
[OTOH it may have been written that way in the first place and never
altered; I suspect its deliberately a union [but I haven't annotated
to be sure]].

 - in the client side js, the limit of checking whether the task was
new seems unnecessary - isn't it reasonable to just cross check
regardless?

Revision history for this message
Gary Poster (gary) wrote :

As Robert said on IRC, reverting the union change will make qa easier. I'll do it.

For the js, since we don't do this generically (yet) I thought it would be surprising to update other tasks in response to saying "me too" because people might confuse coincidence with consequence. I'd be fine if we always kept statuses up-to-date with long-poll later, for instance; then we would not need this "something happened" code path at all. For now, though, I stand by my call there. Robert said on IRC he saw my point, and was fine with me proceeding as planned.

Revision history for this message
Gary Poster (gary) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/configure.zcml'
2--- lib/lp/bugs/configure.zcml 2011-06-16 13:50:58 +0000
3+++ lib/lp/bugs/configure.zcml 2011-07-19 15:24:29 +0000
4@@ -206,6 +206,7 @@
5 canTransitionToStatus
6 isSubscribed
7 getPackageComponent
8+ maybeConfirm
9 userCanEditImportance
10 userCanEditMilestone
11 userCanSetAnyAssignee
12@@ -748,6 +749,7 @@
13 getDirectSubscriptions
14 getIndirectSubscribers
15 is_complete
16+ maybeConfirmBugtasks
17 official_tags
18 who_made_private
19 date_made_private
20@@ -755,6 +757,7 @@
21 personIsDirectSubscriber
22 personIsAlsoNotifiedSubscriber
23 personIsSubscribedToDuplicate
24+ shouldConfirmBugtasks
25 heat
26 heat_last_updated
27 has_patches
28
29=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
30--- lib/lp/bugs/javascript/bugtask_index.js 2011-07-18 05:32:40 +0000
31+++ lib/lp/bugs/javascript/bugtask_index.js 2011-07-19 15:24:29 +0000
32@@ -658,11 +658,35 @@
33 Y.fire('lp:branch-linked', bug_branch_node);
34 }
35
36+var status_choice_data = [];
37+
38+var update_maybe_confirmed_status = function() {
39+ // This would be better done via client-side MVC for the pertinent
40+ // bugtasks, but we don't have that yet.
41+ Y.Array.each(
42+ status_choice_data,
43+ function(rowdata) {
44+ if (rowdata.widget.get('value') === 'New') {
45+ lp_client.get(
46+ rowdata.config.bugtask_path,
47+ // We will silently fail.
48+ // This is not critical functionality.
49+ {on: {success: function(bugtask) {
50+ var status = bugtask.get('status');
51+ if (status !== rowdata.widget.get('value')) {
52+ rowdata.widget.set('value', status);
53+ rowdata.widget.fire('save');
54+ }
55+ }}});
56+ }
57+ }
58+ );
59+};
60
61 /**
62 * Set up a bug task table row.
63 *
64- * Called once, on load, to initialize the page.
65+ * Called once per row, on load, to initialize the page.
66 *
67 * @method setup_bugtasks_row
68 */
69@@ -740,6 +764,8 @@
70 patch: 'status',
71 resource: conf.bugtask_path}});
72 status_choice_edit.render();
73+ status_choice_data.push(
74+ {widget: status_choice_edit, config: conf});
75 }
76 if (conf.user_can_edit_importance) {
77 var importance_choice_edit = new Y.ChoiceSource({
78@@ -1086,6 +1112,11 @@
79 widget._uiClearWaiting();
80 MeTooChoiceSource.superclass._saveData.call(
81 widget, value);
82+ if (value && widget.get('others_affected_count') > 0) {
83+ // If we increased the affected count to 2 or more,
84+ // maybe update the statuses of our bugtasks.
85+ update_maybe_confirmed_status();
86+ }
87 },
88 failure: this.error_handler.getFailureHandler()
89 },
90
91=== modified file 'lib/lp/bugs/model/bug.py'
92--- lib/lp/bugs/model/bug.py 2011-06-21 08:36:42 +0000
93+++ lib/lp/bugs/model/bug.py 2011-07-19 15:24:29 +0000
94@@ -60,7 +60,6 @@
95 Select,
96 SQL,
97 Sum,
98- Union,
99 )
100 from storm.info import ClassAlias
101 from storm.locals import (
102@@ -445,20 +444,15 @@
103 @property
104 def user_ids_affected_with_dupes(self):
105 """Return all IDs of Persons affected by this bug and its dupes.
106- The return value is a Storm expression. Running a query with
107- this expression returns a result that may contain the same ID
108- multiple times, for example if that person is affected via
109- more than one duplicate."""
110- return Union(
111- Select(Person.id,
112- And(BugAffectsPerson.person == Person.id,
113- BugAffectsPerson.affected,
114- BugAffectsPerson.bug == self)),
115- Select(Person.id,
116- And(BugAffectsPerson.person == Person.id,
117- BugAffectsPerson.bug == Bug.id,
118- BugAffectsPerson.affected,
119- Bug.duplicateof == self.id)))
120+ The return value is a Storm expression."""
121+ return Select(
122+ Person.id,
123+ And(BugAffectsPerson.person == Person.id,
124+ BugAffectsPerson.affected,
125+ Or(BugAffectsPerson.bug == self,
126+ And(BugAffectsPerson.bug == Bug.id,
127+ Bug.duplicateof == self.id))),
128+ distinct=True)
129
130 @property
131 def users_affected_with_dupes(self):
132@@ -1780,6 +1774,20 @@
133 store.flush()
134 store.invalidate(self)
135
136+ def shouldConfirmBugtasks(self):
137+ """Should we try to confirm this bug's bugtasks?
138+ The answer is yes if more than one user is affected."""
139+ # == 2 would probably be sufficient once we have all legacy bug tasks
140+ # confirmed. For now, this is a compromise: we don't need a migration
141+ # step, but we will make some unnecessary comparisons.
142+ return self.users_affected_count_with_dupes > 1
143+
144+ def maybeConfirmBugtasks(self):
145+ """Maybe try to confirm our new bugtasks."""
146+ if self.shouldConfirmBugtasks():
147+ for bugtask in self.bugtasks:
148+ bugtask.maybeConfirm()
149+
150 def markUserAffected(self, user, affected=True):
151 """See `IBug`."""
152 bap = self._getAffectedUser(user)
153@@ -1796,6 +1804,9 @@
154 if dupe._getAffectedUser(user) is not None:
155 dupe.markUserAffected(user, affected)
156
157+ if affected:
158+ self.maybeConfirmBugtasks()
159+
160 self.updateHeat()
161
162 def _markAsDuplicate(self, duplicate_of):
163@@ -1836,6 +1847,9 @@
164 # to 0 (since it's a duplicate, it shouldn't have any heat
165 # at all).
166 self.setHeat(0, affected_targets=affected_targets)
167+ # Maybe confirm bug tasks, now that more people might be affected
168+ # by this bug.
169+ duplicate_of.maybeConfirmBugtasks()
170 else:
171 # Otherwise, recalculate this bug's heat, since it will be 0
172 # from having been a duplicate. We also update the bug that
173
174=== modified file 'lib/lp/bugs/model/bugtask.py'
175--- lib/lp/bugs/model/bugtask.py 2011-07-19 10:19:59 +0000
176+++ lib/lp/bugs/model/bugtask.py 2011-07-19 15:24:29 +0000
177@@ -24,8 +24,11 @@
178 import datetime
179 from itertools import chain
180 from operator import attrgetter
181+import re
182
183 from lazr.enum import BaseItem
184+from lazr.lifecycle.event import ObjectModifiedEvent
185+from lazr.lifecycle.snapshot import Snapshot
186 import pytz
187 from sqlobject import (
188 ForeignKey,
189@@ -53,9 +56,11 @@
190 Store,
191 )
192 from zope.component import getUtility
193+from zope.event import notify
194 from zope.interface import (
195 alsoProvides,
196 implements,
197+ providedBy,
198 )
199 from zope.security.proxy import (
200 isinstance as zope_isinstance,
201@@ -263,7 +268,7 @@
202 raise IllegalRelatedBugTasksParams(
203 ('Cannot search for related tasks to \'%s\', at least one '
204 'of these parameter has to be empty: %s'
205- %(context.name, ", ".join(relevant_fields))))
206+ % (context.name, ", ".join(relevant_fields))))
207 return search_params
208
209
210@@ -814,6 +819,51 @@
211 raise ValueError('Unknown debbugs severity "%s".' % severity)
212 return self.importance
213
214+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
215+ _parse_launchpad_names = re.compile(r"[a-z0-9][a-z0-9\+\.\-]+").findall
216+
217+ def _checkAutoconfirmFeatureFlag(self):
218+ """Does a feature flag enable automatic switching of our bugtasks?"""
219+ # This method should be ripped out if we determine that we like
220+ # this behavior for all projects.
221+ # This is a bit of a feature flag hack, but has been discussed as
222+ # a reasonable way to deploy this quickly.
223+ pillar = self.pillar
224+ if IDistribution.providedBy(pillar):
225+ flag_name = 'bugs.autoconfirm.enabled_distribution_names'
226+ else:
227+ assert IProduct.providedBy(pillar), 'unexpected pillar'
228+ flag_name = 'bugs.autoconfirm.enabled_product_names'
229+ enabled = features.getFeatureFlag(flag_name)
230+ if enabled is None:
231+ return False
232+ if (enabled.strip() != '*' and
233+ pillar.name not in self._parse_launchpad_names(enabled)):
234+ # We are not generically enabled ('*') and our pillar's name
235+ # is not explicitly enabled.
236+ return False
237+ return True
238+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
239+
240+ def maybeConfirm(self):
241+ """Maybe confirm this bugtask.
242+ Only call this if the bug._shouldConfirmBugtasks().
243+ This adds the further constraint that the bugtask needs to be NEW,
244+ and not imported from an external bug tracker.
245+ """
246+ if (self.status == BugTaskStatus.NEW
247+ and self.bugwatch is None
248+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
249+ and self._checkAutoconfirmFeatureFlag()
250+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
251+ ):
252+ user = getUtility(ILaunchpadCelebrities).janitor
253+ bugtask_before_modification = Snapshot(
254+ self, providing=providedBy(self))
255+ self.transitionToStatus(BugTaskStatus.CONFIRMED, user)
256+ notify(ObjectModifiedEvent(
257+ self, bugtask_before_modification, ['status'], user=user))
258+
259 def canTransitionToStatus(self, new_status, user):
260 """See `IBugTask`."""
261 celebrities = getUtility(ILaunchpadCelebrities)
262@@ -1079,6 +1129,12 @@
263 if self.target != target_before_change:
264 target_before_change.recalculateBugHeatCache()
265 self.target.recalculateBugHeatCache()
266+ # START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
267+ # We also should see if we ought to auto-transition to the
268+ # CONFIRMED status.
269+ if self.bug.shouldConfirmBugtasks():
270+ self.maybeConfirm()
271+ # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
272
273 def updateTargetNameCache(self, newtarget=None):
274 """See `IBugTask`."""
275@@ -1791,7 +1847,6 @@
276 "BugTaskSearchParam.exclude_conjoined cannot be True if "
277 "BugTaskSearchParam.milestone is not set")
278
279-
280 if params.project:
281 # Prevent circular import problems.
282 from lp.registry.model.product import Product
283@@ -1895,7 +1950,6 @@
284 % sqlvalues(params.structural_subscriber))
285 has_duplicate_results = True
286
287-
288 # Remove bugtasks from deactivated products, if necessary.
289 # We don't have to do this if
290 # 1) We're searching on bugtasks for a specific product
291@@ -2531,7 +2585,8 @@
292 from lp.bugs.model.bugsummary import BugSummary
293 conditions = []
294 # Open bug statuses
295- conditions.append(BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES))
296+ conditions.append(
297+ BugSummary.status.is_in(UNRESOLVED_BUGTASK_STATUSES))
298 # BugSummary does not include duplicates so no need to exclude.
299 context_conditions = []
300 for context in contexts:
301@@ -2564,15 +2619,17 @@
302 "teams AS ("
303 "SELECT team from TeamParticipation WHERE person=?)",
304 (user.id,)))
305- # Note that because admins can see every bug regardless of subscription
306- # they will see rather inflated counts. Admins get to deal.
307+ # Note that because admins can see every bug regardless of
308+ # subscription they will see rather inflated counts. Admins get to
309+ # deal.
310 if user is None:
311 conditions.append(BugSummary.viewed_by_id == None)
312 elif not user.inTeam(admin_team):
313 conditions.append(
314 Or(
315 BugSummary.viewed_by_id == None,
316- BugSummary.viewed_by_id.is_in(SQL("SELECT team FROM teams"))
317+ BugSummary.viewed_by_id.is_in(
318+ SQL("SELECT team FROM teams"))
319 ))
320 sum_count = Sum(BugSummary.count)
321 resultset = store.find(group_on + (sum_count,), *conditions)
322@@ -3189,7 +3246,7 @@
323 distro_series_ids = set()
324 product_ids = set()
325 product_series_ids = set()
326-
327+
328 # Gather all the ids that might have milestones to preload for the
329 # for the milestone vocabulary
330 for task in bugtasks:
331@@ -3199,11 +3256,11 @@
332 product_ids.add(task.productID)
333 product_series_ids.add(task.productseriesID)
334
335- distro_ids.discard(None)
336- distro_series_ids.discard(None)
337- product_ids.discard(None)
338- product_series_ids.discard(None)
339-
340+ distro_ids.discard(None)
341+ distro_series_ids.discard(None)
342+ product_ids.discard(None)
343+ product_series_ids.discard(None)
344+
345 milestones = store.find(
346 Milestone,
347 Or(
348@@ -3221,7 +3278,5 @@
349 Product, Product.id.is_in(product_ids)))
350 list(store.find(
351 ProductSeries, ProductSeries.id.is_in(product_series_ids)))
352-
353+
354 return milestones
355-
356-
357
358=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
359--- lib/lp/bugs/model/tests/test_bug.py 2011-06-16 13:50:58 +0000
360+++ lib/lp/bugs/model/tests/test_bug.py 2011-07-19 15:24:29 +0000
361@@ -5,11 +5,14 @@
362
363 from canonical.testing.layers import DatabaseFunctionalLayer
364 from lp.bugs.enum import BugNotificationLevel
365+from lp.bugs.interfaces.bugtask import BugTaskStatus
366 from lp.bugs.model.bug import BugSubscriptionInfo
367 from lp.registry.interfaces.person import PersonVisibility
368 from lp.testing import (
369+ feature_flags,
370 login_person,
371 person_logged_in,
372+ set_feature_flag,
373 TestCaseWithFactory,
374 )
375
376@@ -313,3 +316,101 @@
377 bug.subscribe(team, person)
378 bug.setPrivate(True, person)
379 self.assertFalse(bug.personIsDirectSubscriber(person))
380+
381+
382+class TestBugAutoConfirmation(TestCaseWithFactory):
383+ """Tests for auto confirming bugs"""
384+
385+ layer = DatabaseFunctionalLayer
386+
387+ def test_shouldConfirmBugtasks_initial_False(self):
388+ # After a bug is created, only one person is affected, and we should
389+ # not try to confirm bug tasks.
390+ bug = self.factory.makeBug()
391+ self.assertFalse(bug.shouldConfirmBugtasks())
392+
393+ def test_shouldConfirmBugtasks_after_another_positively_affected(self):
394+ # We should confirm bug tasks if the number of affected users is
395+ # more than one.
396+ bug = self.factory.makeBug()
397+ person = self.factory.makePerson()
398+ with person_logged_in(person):
399+ bug.markUserAffected(person)
400+ self.assertTrue(bug.shouldConfirmBugtasks())
401+
402+ def test_shouldConfirmBugtasks_after_another_persons_dupe(self):
403+ # We should confirm bug tasks if someone else files a dupe.
404+ bug = self.factory.makeBug()
405+ duplicate_bug = self.factory.makeBug()
406+ with person_logged_in(duplicate_bug.owner):
407+ duplicate_bug.markAsDuplicate(bug)
408+ self.assertTrue(bug.shouldConfirmBugtasks())
409+
410+ def test_shouldConfirmBugtasks_after_same_persons_dupe_False(self):
411+ # We should not confirm bug tasks if same person files a dupe.
412+ bug = self.factory.makeBug()
413+ with person_logged_in(bug.owner):
414+ duplicate_bug = self.factory.makeBug(owner=bug.owner)
415+ duplicate_bug.markAsDuplicate(bug)
416+ self.assertFalse(bug.shouldConfirmBugtasks())
417+
418+ def test_shouldConfirmBugtasks_honors_negatively_affected(self):
419+ # We should confirm bug tasks if the number of affected users is
420+ # more than one.
421+ bug = self.factory.makeBug()
422+ with person_logged_in(bug.owner):
423+ bug.markUserAffected(bug.owner, False)
424+ person = self.factory.makePerson()
425+ with person_logged_in(person):
426+ bug.markUserAffected(person)
427+ self.assertFalse(bug.shouldConfirmBugtasks())
428+
429+ def test_markUserAffected_autoconfirms(self):
430+ # markUserAffected will auto confirm if appropriate.
431+ # When feature flag code is removed, remove the next two lines and
432+ # dedent the rest.
433+ with feature_flags():
434+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
435+ bug = self.factory.makeBug()
436+ person = self.factory.makePerson()
437+ with person_logged_in(person):
438+ bug.markUserAffected(person)
439+ self.assertEqual(BugTaskStatus.CONFIRMED, bug.bugtasks[0].status)
440+
441+ def test_markUserAffected_does_not_autoconfirm_wrongly(self):
442+ # markUserAffected will not auto confirm if incorrect.
443+ # When feature flag code is removed, remove the next two lines and
444+ # dedent the rest.
445+ with feature_flags():
446+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
447+ bug = self.factory.makeBug()
448+ person = self.factory.makePerson()
449+ with person_logged_in(bug.owner):
450+ bug.markUserAffected(bug.owner, False)
451+ with person_logged_in(person):
452+ bug.markUserAffected(person)
453+ self.assertEqual(BugTaskStatus.NEW, bug.bugtasks[0].status)
454+
455+ def test_markAsDuplicate_autoconfirms(self):
456+ # markAsDuplicate will auto confirm if appropriate.
457+ # When feature flag code is removed, remove the next two lines and
458+ # dedent the rest.
459+ with feature_flags():
460+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
461+ bug = self.factory.makeBug()
462+ duplicate_bug = self.factory.makeBug()
463+ with person_logged_in(duplicate_bug.owner):
464+ duplicate_bug.markAsDuplicate(bug)
465+ self.assertEqual(BugTaskStatus.CONFIRMED, bug.bugtasks[0].status)
466+
467+ def test_markAsDuplicate_does_not_autoconfirm_wrongly(self):
468+ # markAsDuplicate will not auto confirm if incorrect.
469+ # When feature flag code is removed, remove the next two lines and
470+ # dedent the rest.
471+ with feature_flags():
472+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
473+ bug = self.factory.makeBug()
474+ with person_logged_in(bug.owner):
475+ duplicate_bug = self.factory.makeBug(owner=bug.owner)
476+ duplicate_bug.markAsDuplicate(bug)
477+ self.assertEqual(BugTaskStatus.NEW, bug.bugtasks[0].status)
478
479=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
480--- lib/lp/bugs/model/tests/test_bugtask.py 2011-07-04 03:10:41 +0000
481+++ lib/lp/bugs/model/tests/test_bugtask.py 2011-07-19 15:24:29 +0000
482@@ -10,6 +10,7 @@
483 from testtools.matchers import Equals
484 from zope.component import getUtility
485 from zope.interface import providedBy
486+from zope.security.proxy import removeSecurityProxy
487
488 from canonical.database.sqlbase import flush_database_updates
489 from canonical.launchpad.searchbuilder import (
490@@ -52,11 +53,14 @@
491 from lp.registry.interfaces.projectgroup import IProjectGroupSet
492 from lp.testing import (
493 ANONYMOUS,
494+ EventRecorder,
495+ feature_flags,
496 login,
497 login_person,
498 logout,
499 normalize_whitespace,
500 person_logged_in,
501+ set_feature_flag,
502 StormStatementRecorder,
503 TestCase,
504 TestCaseWithFactory,
505@@ -1445,3 +1449,205 @@
506 self.generic_task.sourcepackagename = source_package_name
507 self.assertEqual(
508 source_package_name, self.series_task.sourcepackagename)
509+
510+# START TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
511+# When feature flag code is removed, delete these tests (up to "# END
512+# TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.")
513+
514+
515+class TestAutoConfirmBugTasksFlagForProduct(TestCaseWithFactory):
516+ """Tests for auto-confirming bug tasks."""
517+ # Tests for _checkAutoconfirmFeatureFlag.
518+
519+ layer = DatabaseFunctionalLayer
520+
521+ def makeTarget(self):
522+ return self.factory.makeProduct()
523+
524+ flag = u'bugs.autoconfirm.enabled_product_names'
525+ alt_flag = u'bugs.autoconfirm.enabled_distribution_names'
526+
527+ def test_False(self):
528+ # With no feature flags turned on, we do not auto-confirm.
529+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
530+ self.assertFalse(
531+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
532+
533+ def test_flag_False(self):
534+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
535+ with feature_flags():
536+ set_feature_flag(self.flag, u' ')
537+ self.assertFalse(
538+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
539+
540+ def test_explicit_flag(self):
541+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
542+ with feature_flags():
543+ set_feature_flag(self.flag, bug_task.pillar.name)
544+ self.assertTrue(
545+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
546+
547+ def test_explicit_flag_of_many(self):
548+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
549+ with feature_flags():
550+ set_feature_flag(
551+ self.flag, u' foo bar ' + bug_task.pillar.name + ' baz ')
552+ self.assertTrue(
553+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
554+
555+ def test_match_all_flag(self):
556+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
557+ with feature_flags():
558+ set_feature_flag(self.flag, u'*')
559+ self.assertTrue(
560+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
561+
562+ def test_alt_flag_does_not_affect(self):
563+ bug_task = self.factory.makeBugTask(target=self.makeTarget())
564+ with feature_flags():
565+ set_feature_flag(self.alt_flag, bug_task.pillar.name)
566+ self.assertFalse(
567+ removeSecurityProxy(bug_task)._checkAutoconfirmFeatureFlag())
568+
569+
570+class TestAutoConfirmBugTasksFlagForProductSeries(
571+ TestAutoConfirmBugTasksFlagForProduct):
572+ """Tests for auto-confirming bug tasks."""
573+
574+ def makeTarget(self):
575+ return self.factory.makeProductSeries()
576+
577+
578+class TestAutoConfirmBugTasksFlagForDistribution(
579+ TestAutoConfirmBugTasksFlagForProduct):
580+ """Tests for auto-confirming bug tasks."""
581+
582+ flag = TestAutoConfirmBugTasksFlagForProduct.alt_flag
583+ alt_flag = TestAutoConfirmBugTasksFlagForProduct.flag
584+
585+ def makeTarget(self):
586+ return self.factory.makeDistribution()
587+
588+
589+class TestAutoConfirmBugTasksFlagForDistributionSeries(
590+ TestAutoConfirmBugTasksFlagForDistribution):
591+ """Tests for auto-confirming bug tasks."""
592+
593+ def makeTarget(self):
594+ return self.factory.makeDistroSeries()
595+
596+
597+class TestAutoConfirmBugTasksFlagForDistributionSourcePackage(
598+ TestAutoConfirmBugTasksFlagForDistribution):
599+ """Tests for auto-confirming bug tasks."""
600+
601+ def makeTarget(self):
602+ return self.factory.makeDistributionSourcePackage()
603+
604+
605+class TestAutoConfirmBugTasksTransitionToTarget(TestCaseWithFactory):
606+ """Tests for auto-confirming bug tasks."""
607+ # Tests for making sure that switching a task from one project that
608+ # does not auto-confirm to another that does performs the auto-confirm
609+ # correctly, if appropriate. This is only necessary for as long as a
610+ # project may not participate in auto-confirm.
611+
612+ layer = DatabaseFunctionalLayer
613+
614+ def test_no_transitionToTarget(self):
615+ # We can change the target. If the normal bug conditions do not
616+ # hold, there will be no transition.
617+ person = self.factory.makePerson()
618+ autoconfirm_product = self.factory.makeProduct(owner=person)
619+ no_autoconfirm_product = self.factory.makeProduct(owner=person)
620+ with feature_flags():
621+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names',
622+ autoconfirm_product.name)
623+ bug_task = self.factory.makeBugTask(
624+ target=no_autoconfirm_product, owner=person)
625+ with person_logged_in(person):
626+ bug_task.maybeConfirm()
627+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
628+ bug_task.transitionToTarget(autoconfirm_product)
629+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
630+
631+ def test_transitionToTarget(self):
632+ # If the conditions *do* hold, though, we will auto-confirm.
633+ person = self.factory.makePerson()
634+ another_person = self.factory.makePerson()
635+ autoconfirm_product = self.factory.makeProduct(owner=person)
636+ no_autoconfirm_product = self.factory.makeProduct(owner=person)
637+ with feature_flags():
638+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names',
639+ autoconfirm_product.name)
640+ bug_task = self.factory.makeBugTask(
641+ target=no_autoconfirm_product, owner=person)
642+ with person_logged_in(another_person):
643+ bug_task.bug.markUserAffected(another_person)
644+ with person_logged_in(person):
645+ bug_task.maybeConfirm()
646+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
647+ bug_task.transitionToTarget(autoconfirm_product)
648+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
649+# END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
650+
651+
652+class TestAutoConfirmBugTasks(TestCaseWithFactory):
653+ """Tests for auto-confirming bug tasks."""
654+ # Tests for maybeConfirm
655+
656+ layer = DatabaseFunctionalLayer
657+
658+ def test_auto_confirm(self):
659+ # A typical new bugtask auto-confirms.
660+ # When feature flag code is removed, remove the next two lines and
661+ # dedent the rest.
662+ with feature_flags():
663+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
664+ bug_task = self.factory.makeBugTask()
665+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
666+ with EventRecorder() as recorder:
667+ bug_task.maybeConfirm()
668+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
669+ self.assertEqual(1, len(recorder.events))
670+ event = recorder.events[0]
671+ self.assertEqual(getUtility(ILaunchpadCelebrities).janitor,
672+ event.user)
673+ self.assertEqual(['status'], event.edited_fields)
674+ self.assertEqual(BugTaskStatus.NEW,
675+ event.object_before_modification.status)
676+ self.assertEqual(bug_task, event.object)
677+
678+ def test_do_not_confirm_bugwatch_tasks(self):
679+ # A bugwatch bugtask does not auto-confirm.
680+ # When feature flag code is removed, remove the next two lines and
681+ # dedent the rest.
682+ with feature_flags():
683+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
684+ product = self.factory.makeProduct()
685+ with person_logged_in(product.owner):
686+ bug = self.factory.makeBug(
687+ product=product, owner=product.owner)
688+ bug_task = bug.getBugTask(product)
689+ watch = self.factory.makeBugWatch(bug=bug)
690+ bug_task.bugwatch = watch
691+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
692+ with EventRecorder() as recorder:
693+ bug_task.maybeConfirm()
694+ self.assertEqual(BugTaskStatus.NEW, bug_task.status)
695+ self.assertEqual(0, len(recorder.events))
696+
697+ def test_only_confirm_new_tasks(self):
698+ # A non-new bugtask does not auto-confirm.
699+ # When feature flag code is removed, remove the next two lines and
700+ # dedent the rest.
701+ with feature_flags():
702+ set_feature_flag(u'bugs.autoconfirm.enabled_product_names', u'*')
703+ bug_task = self.factory.makeBugTask()
704+ removeSecurityProxy(bug_task).transitionToStatus(
705+ BugTaskStatus.CONFIRMED, bug_task.bug.owner)
706+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
707+ with EventRecorder() as recorder:
708+ bug_task.maybeConfirm()
709+ self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
710+ self.assertEqual(0, len(recorder.events))
711
712=== modified file 'lib/lp/services/features/flags.py'
713--- lib/lp/services/features/flags.py 2011-06-16 13:50:58 +0000
714+++ lib/lp/services/features/flags.py 2011-07-19 15:24:29 +0000
715@@ -26,6 +26,8 @@
716 'The flag value is set to the given floating point number.'),
717 ('int',
718 "An integer."),
719+ ('space delimited',
720+ 'Space-delimited strings.')
721 ])
722
723 # Data for generating web-visible feature flag documentation.
724@@ -106,6 +108,18 @@
725 'boolean',
726 ('Enables ranking by pillar affiliation in the person picker.'),
727 ''),
728+ ('bugs.autoconfirm.enabled_distribution_names',
729+ 'space delimited',
730+ ('Enables auto-confirming bugtasks for distributions (and their '
731+ 'series and packages). Use the default domain. Specify a single '
732+ 'asterisk ("*") to enable for all distributions.'),
733+ 'None are enabled'),
734+ ('bugs.autoconfirm.enabled_product_names',
735+ 'space delimited',
736+ ('Enables auto-confirming bugtasks for products (and their '
737+ 'series). Use the default domain. Specify a single '
738+ 'asterisk ("*") to enable for all products.'),
739+ 'None are enabled'),
740 ])
741
742 # The set of all flag names that are documented.
743
744=== modified file 'lib/lp/testing/factory.py'
745--- lib/lp/testing/factory.py 2011-07-15 11:34:12 +0000
746+++ lib/lp/testing/factory.py 2011-07-19 15:24:29 +0000
747@@ -1677,7 +1677,8 @@
748 bug = getUtility(IBugSet).createBug(create_bug_params)
749 if bug_watch_url is not None:
750 # fromText() creates a bug watch associated with the bug.
751- getUtility(IBugWatchSet).fromText(bug_watch_url, bug, owner)
752+ with person_logged_in(owner):
753+ getUtility(IBugWatchSet).fromText(bug_watch_url, bug, owner)
754 bugtask = bug.default_bugtask
755 if date_closed is not None:
756 bugtask.transitionToStatus(