Merge lp:~bac/launchpad/bug-720147 into lp:launchpad
- bug-720147
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Brad Crittenden |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12967 |
Proposed branch: | lp:~bac/launchpad/bug-720147 |
Merge into: | lp:launchpad |
Diff against target: |
432 lines (+220/-32) 5 files modified
lib/lp/bugs/browser/bugtarget.py (+10/-10) lib/lp/bugs/interfaces/bug.py (+5/-1) lib/lp/bugs/model/bug.py (+14/-4) lib/lp/bugs/scripts/tests/test_bugnotification.py (+65/-13) lib/lp/bugs/tests/test_bug.py (+126/-4) |
To merge this branch: | bzr merge lp:~bac/launchpad/bug-720147 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+59677@code.launchpad.net |
Commit message
[r=benji][bug=720147] Correctly handle filtering for bugs created without the default status.
Description of the change
= Summary =
Bugs created with a status other than NEW were sending notifications for
subscriptions with other filters.
== Proposed fix ==
The bug notification was kicked off before the status change was
effected resulting in improper notices being sent.
== Pre-implementation notes ==
This work was done by Graham and Gary. I'm merely shepherding it
through review and landing in their absence.
I did, however, clean up some lint.
== Implementation details ==
As above.
== Tests ==
bin/test -vvm lp.bugs -t test_bugnotific
== Demo and Q/A ==
Create a structural subscription with filter with NEW status. Create a
bug with initial status of TRIAGE. Ensure notification is not sent.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Preview Diff
1 | === modified file 'lib/lp/bugs/browser/bugtarget.py' | |||
2 | --- lib/lp/bugs/browser/bugtarget.py 2011-04-15 02:56:03 +0000 | |||
3 | +++ lib/lp/bugs/browser/bugtarget.py 2011-05-02 17:11:01 +0000 | |||
4 | @@ -556,20 +556,20 @@ | |||
5 | 556 | 556 | ||
6 | 557 | if extra_data.private: | 557 | if extra_data.private: |
7 | 558 | params.private = extra_data.private | 558 | params.private = extra_data.private |
13 | 559 | 559 | if 'status' in data: | |
14 | 560 | self.added_bug = bug = context.createBug(params) | 560 | params.status = data['status'] |
10 | 561 | |||
11 | 562 | # Apply any extra options given by a bug supervisor. | ||
12 | 563 | bugtask = self.added_bug.default_bugtask | ||
15 | 564 | if 'assignee' in data: | 561 | if 'assignee' in data: |
17 | 565 | bugtask.transitionToAssignee(data['assignee']) | 562 | params.assignee = data['assignee'] |
18 | 566 | if 'status' in data: | 563 | if 'status' in data: |
20 | 567 | bugtask.transitionToStatus(data['status'], self.user) | 564 | params.status = data['status'] |
21 | 568 | if 'importance' in data: | 565 | if 'importance' in data: |
23 | 569 | bugtask.transitionToImportance(data['importance'], self.user) | 566 | params.importance = data['importance'] |
24 | 570 | if 'milestone' in data: | 567 | if 'milestone' in data: |
27 | 571 | bugtask.milestone = data['milestone'] | 568 | params.milestone = data['milestone'] |
28 | 572 | 569 | ||
29 | 570 | self.added_bug = bug = context.createBug(params) | ||
30 | 571 | |||
31 | 572 | # Apply any extra options given by a bug supervisor. | ||
32 | 573 | for comment in extra_data.comments: | 573 | for comment in extra_data.comments: |
33 | 574 | bug.newMessage(self.user, bug.followup_subject(), comment) | 574 | bug.newMessage(self.user, bug.followup_subject(), comment) |
34 | 575 | notifications.append( | 575 | notifications.append( |
35 | 576 | 576 | ||
36 | === modified file 'lib/lp/bugs/interfaces/bug.py' | |||
37 | --- lib/lp/bugs/interfaces/bug.py 2011-04-29 11:20:01 +0000 | |||
38 | +++ lib/lp/bugs/interfaces/bug.py 2011-05-02 17:11:01 +0000 | |||
39 | @@ -97,7 +97,8 @@ | |||
40 | 97 | def __init__(self, owner, title, comment=None, description=None, msg=None, | 97 | def __init__(self, owner, title, comment=None, description=None, msg=None, |
41 | 98 | status=None, datecreated=None, security_related=False, | 98 | status=None, datecreated=None, security_related=False, |
42 | 99 | private=False, subscribers=(), binarypackagename=None, | 99 | private=False, subscribers=(), binarypackagename=None, |
44 | 100 | tags=None, subscribe_owner=True, filed_by=None): | 100 | tags=None, subscribe_owner=True, filed_by=None, |
45 | 101 | importance=None, milestone=None, assignee=None): | ||
46 | 101 | self.owner = owner | 102 | self.owner = owner |
47 | 102 | self.title = title | 103 | self.title = title |
48 | 103 | self.comment = comment | 104 | self.comment = comment |
49 | @@ -115,6 +116,9 @@ | |||
50 | 115 | self.tags = tags | 116 | self.tags = tags |
51 | 116 | self.subscribe_owner = subscribe_owner | 117 | self.subscribe_owner = subscribe_owner |
52 | 117 | self.filed_by = filed_by | 118 | self.filed_by = filed_by |
53 | 119 | self.importance = importance | ||
54 | 120 | self.milestone = milestone | ||
55 | 121 | self.assignee = assignee | ||
56 | 118 | 122 | ||
57 | 119 | def setBugTarget(self, product=None, distribution=None, | 123 | def setBugTarget(self, product=None, distribution=None, |
58 | 120 | sourcepackagename=None): | 124 | sourcepackagename=None): |
59 | 121 | 125 | ||
60 | === modified file 'lib/lp/bugs/model/bug.py' | |||
61 | --- lib/lp/bugs/model/bug.py 2011-05-02 01:27:43 +0000 | |||
62 | +++ lib/lp/bugs/model/bug.py 2011-05-02 17:11:01 +0000 | |||
63 | @@ -222,7 +222,8 @@ | |||
64 | 222 | "datecreated", "security_related", "private", | 222 | "datecreated", "security_related", "private", |
65 | 223 | "distribution", "sourcepackagename", "binarypackagename", | 223 | "distribution", "sourcepackagename", "binarypackagename", |
66 | 224 | "product", "status", "subscribers", "tags", | 224 | "product", "status", "subscribers", "tags", |
68 | 225 | "subscribe_owner", "filed_by"]) | 225 | "subscribe_owner", "filed_by", "importance", |
69 | 226 | "milestone", "assignee"]) | ||
70 | 226 | 227 | ||
71 | 227 | 228 | ||
72 | 228 | class BugTag(SQLBase): | 229 | class BugTag(SQLBase): |
73 | @@ -2442,15 +2443,24 @@ | |||
74 | 2442 | # Create the task on a product if one was passed. | 2443 | # Create the task on a product if one was passed. |
75 | 2443 | if params.product: | 2444 | if params.product: |
76 | 2444 | getUtility(IBugTaskSet).createTask( | 2445 | getUtility(IBugTaskSet).createTask( |
79 | 2445 | bug=bug, product=params.product, owner=params.owner, | 2446 | bug=bug, product=params.product, owner=params.owner) |
78 | 2446 | status=params.status) | ||
80 | 2447 | 2447 | ||
81 | 2448 | # Create the task on a source package name if one was passed. | 2448 | # Create the task on a source package name if one was passed. |
82 | 2449 | if params.distribution: | 2449 | if params.distribution: |
83 | 2450 | getUtility(IBugTaskSet).createTask( | 2450 | getUtility(IBugTaskSet).createTask( |
84 | 2451 | bug=bug, distribution=params.distribution, | 2451 | bug=bug, distribution=params.distribution, |
85 | 2452 | sourcepackagename=params.sourcepackagename, | 2452 | sourcepackagename=params.sourcepackagename, |
87 | 2453 | owner=params.owner, status=params.status) | 2453 | owner=params.owner) |
88 | 2454 | |||
89 | 2455 | bug_task = bug.default_bugtask | ||
90 | 2456 | if params.assignee: | ||
91 | 2457 | bug_task.transitionToAssignee(params.assignee) | ||
92 | 2458 | if params.status: | ||
93 | 2459 | bug_task.transitionToStatus(params.status, params.owner) | ||
94 | 2460 | if params.importance: | ||
95 | 2461 | bug_task.transitionToImportance(params.importance, params.owner) | ||
96 | 2462 | if params.milestone: | ||
97 | 2463 | bug_task.transitionToMilestone(params.milestone, params.owner) | ||
98 | 2454 | 2464 | ||
99 | 2455 | # Tell everyone. | 2465 | # Tell everyone. |
100 | 2456 | notify(event) | 2466 | notify(event) |
101 | 2457 | 2467 | ||
102 | === modified file 'lib/lp/bugs/scripts/tests/test_bugnotification.py' | |||
103 | --- lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-04-05 22:34:35 +0000 | |||
104 | +++ lib/lp/bugs/scripts/tests/test_bugnotification.py 2011-05-02 17:11:01 +0000 | |||
105 | @@ -21,6 +21,7 @@ | |||
106 | 21 | ) | 21 | ) |
107 | 22 | from canonical.launchpad.ftests import login | 22 | from canonical.launchpad.ftests import login |
108 | 23 | from canonical.launchpad.helpers import get_contact_email_addresses | 23 | from canonical.launchpad.helpers import get_contact_email_addresses |
109 | 24 | from canonical.launchpad.interfaces.lpstorm import IStore | ||
110 | 24 | from canonical.launchpad.interfaces.message import IMessageSet | 25 | from canonical.launchpad.interfaces.message import IMessageSet |
111 | 25 | from canonical.testing.layers import LaunchpadZopelessLayer | 26 | from canonical.testing.layers import LaunchpadZopelessLayer |
112 | 26 | from lp.bugs.adapters.bugchange import ( | 27 | from lp.bugs.adapters.bugchange import ( |
113 | @@ -38,16 +39,20 @@ | |||
114 | 38 | CveUnlinkedFromBug, | 39 | CveUnlinkedFromBug, |
115 | 39 | ) | 40 | ) |
116 | 40 | from lp.bugs.interfaces.bug import ( | 41 | from lp.bugs.interfaces.bug import ( |
117 | 42 | CreateBugParams, | ||
118 | 41 | IBug, | 43 | IBug, |
119 | 42 | IBugSet, | 44 | IBugSet, |
120 | 43 | ) | 45 | ) |
121 | 44 | from lp.bugs.interfaces.bugnotification import IBugNotificationSet | 46 | from lp.bugs.interfaces.bugnotification import IBugNotificationSet |
123 | 45 | from lp.bugs.interfaces.bugtask import BugTaskStatus | 47 | from lp.bugs.interfaces.bugtask import ( |
124 | 48 | BugTaskImportance, | ||
125 | 49 | BugTaskStatus, | ||
126 | 50 | ) | ||
127 | 46 | from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients | 51 | from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients |
128 | 47 | from lp.bugs.model.bugnotification import ( | 52 | from lp.bugs.model.bugnotification import ( |
129 | 48 | BugNotification, | 53 | BugNotification, |
130 | 49 | BugNotificationFilter, | 54 | BugNotificationFilter, |
132 | 50 | # BugNotificationRecipient, | 55 | BugNotificationRecipient, |
133 | 51 | ) | 56 | ) |
134 | 52 | from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilterMute | 57 | from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilterMute |
135 | 53 | from lp.bugs.model.bugtask import BugTask | 58 | from lp.bugs.model.bugtask import BugTask |
136 | @@ -1021,7 +1026,7 @@ | |||
137 | 1021 | def test_header_single(self): | 1026 | def test_header_single(self): |
138 | 1022 | # A single filter with a description makes all emails | 1027 | # A single filter with a description makes all emails |
139 | 1023 | # include that particular filter description in a header. | 1028 | # include that particular filter description in a header. |
141 | 1024 | bug_filter = self.addFilter(u"Test filter") | 1029 | self.addFilter(u"Test filter") |
142 | 1025 | 1030 | ||
143 | 1026 | self.assertContentEqual([u"Test filter"], | 1031 | self.assertContentEqual([u"Test filter"], |
144 | 1027 | self.getSubscriptionEmailHeaders()) | 1032 | self.getSubscriptionEmailHeaders()) |
145 | @@ -1029,8 +1034,8 @@ | |||
146 | 1029 | def test_header_multiple(self): | 1034 | def test_header_multiple(self): |
147 | 1030 | # Multiple filters with a description make all emails | 1035 | # Multiple filters with a description make all emails |
148 | 1031 | # include all filter descriptions in the header. | 1036 | # include all filter descriptions in the header. |
151 | 1032 | bug_filter = self.addFilter(u"First filter") | 1037 | self.addFilter(u"First filter") |
152 | 1033 | bug_filter = self.addFilter(u"Second filter") | 1038 | self.addFilter(u"Second filter") |
153 | 1034 | 1039 | ||
154 | 1035 | self.assertContentEqual([u"First filter", u"Second filter"], | 1040 | self.assertContentEqual([u"First filter", u"Second filter"], |
155 | 1036 | self.getSubscriptionEmailHeaders()) | 1041 | self.getSubscriptionEmailHeaders()) |
156 | @@ -1042,10 +1047,10 @@ | |||
157 | 1042 | other_person = self.factory.makePerson() | 1047 | other_person = self.factory.makePerson() |
158 | 1043 | other_subscription = self.bug.default_bugtask.target.addSubscription( | 1048 | other_subscription = self.bug.default_bugtask.target.addSubscription( |
159 | 1044 | other_person, other_person) | 1049 | other_person, other_person) |
161 | 1045 | bug_filter = self.addFilter(u"Someone's filter", other_subscription) | 1050 | self.addFilter(u"Someone's filter", other_subscription) |
162 | 1046 | self.addNotificationRecipient(self.notification, other_person) | 1051 | self.addNotificationRecipient(self.notification, other_person) |
163 | 1047 | 1052 | ||
165 | 1048 | this_filter = self.addFilter(u"Test filter") | 1053 | self.addFilter(u"Test filter") |
166 | 1049 | 1054 | ||
167 | 1050 | the_subscriber = self.subscription.subscriber | 1055 | the_subscriber = self.subscription.subscriber |
168 | 1051 | self.assertEquals( | 1056 | self.assertEquals( |
169 | @@ -1062,7 +1067,7 @@ | |||
170 | 1062 | def test_body_single(self): | 1067 | def test_body_single(self): |
171 | 1063 | # A single filter with a description makes all emails | 1068 | # A single filter with a description makes all emails |
172 | 1064 | # include that particular filter description in the body. | 1069 | # include that particular filter description in the body. |
174 | 1065 | bug_filter = self.addFilter(u"Test filter") | 1070 | self.addFilter(u"Test filter") |
175 | 1066 | 1071 | ||
176 | 1067 | self.assertContentEqual([u"Matching subscriptions: Test filter"], | 1072 | self.assertContentEqual([u"Matching subscriptions: Test filter"], |
177 | 1068 | self.getSubscriptionEmailBody()) | 1073 | self.getSubscriptionEmailBody()) |
178 | @@ -1070,15 +1075,15 @@ | |||
179 | 1070 | def test_body_multiple(self): | 1075 | def test_body_multiple(self): |
180 | 1071 | # Multiple filters with description make all emails | 1076 | # Multiple filters with description make all emails |
181 | 1072 | # include them in the email body. | 1077 | # include them in the email body. |
184 | 1073 | bug_filter = self.addFilter(u"First filter") | 1078 | self.addFilter(u"First filter") |
185 | 1074 | bug_filter = self.addFilter(u"Second filter") | 1079 | self.addFilter(u"Second filter") |
186 | 1075 | 1080 | ||
187 | 1076 | self.assertContentEqual( | 1081 | self.assertContentEqual( |
188 | 1077 | [u"Matching subscriptions: First filter, Second filter"], | 1082 | [u"Matching subscriptions: First filter, Second filter"], |
189 | 1078 | self.getSubscriptionEmailBody()) | 1083 | self.getSubscriptionEmailBody()) |
190 | 1079 | 1084 | ||
191 | 1080 | def test_muted(self): | 1085 | def test_muted(self): |
193 | 1081 | bug_filter = self.addFilter(u"Test filter") | 1086 | self.addFilter(u"Test filter") |
194 | 1082 | BugSubscriptionFilterMute( | 1087 | BugSubscriptionFilterMute( |
195 | 1083 | person=self.subscription.subscriber, | 1088 | person=self.subscription.subscriber, |
196 | 1084 | filter=self.notification.bug_filters.one()) | 1089 | filter=self.notification.bug_filters.one()) |
197 | @@ -1089,11 +1094,58 @@ | |||
198 | 1089 | def test_header_multiple_one_muted(self): | 1094 | def test_header_multiple_one_muted(self): |
199 | 1090 | # Multiple filters with a description make all emails | 1095 | # Multiple filters with a description make all emails |
200 | 1091 | # include all filter descriptions in the header. | 1096 | # include all filter descriptions in the header. |
203 | 1092 | bug_filter = self.addFilter(u"First filter") | 1097 | self.addFilter(u"First filter") |
204 | 1093 | bug_filter = self.addFilter(u"Second filter") | 1098 | self.addFilter(u"Second filter") |
205 | 1094 | BugSubscriptionFilterMute( | 1099 | BugSubscriptionFilterMute( |
206 | 1095 | person=self.subscription.subscriber, | 1100 | person=self.subscription.subscriber, |
207 | 1096 | filter=self.notification.bug_filters[0]) | 1101 | filter=self.notification.bug_filters[0]) |
208 | 1097 | 1102 | ||
209 | 1098 | self.assertContentEqual([u"Second filter"], | 1103 | self.assertContentEqual([u"Second filter"], |
210 | 1099 | self.getSubscriptionEmailHeaders()) | 1104 | self.getSubscriptionEmailHeaders()) |
211 | 1105 | |||
212 | 1106 | |||
213 | 1107 | class TestEmailNotificationsWithFiltersWhenBugCreated(TestCaseWithFactory): | ||
214 | 1108 | # See bug 720147. | ||
215 | 1109 | |||
216 | 1110 | layer = LaunchpadZopelessLayer | ||
217 | 1111 | |||
218 | 1112 | def setUp(self): | ||
219 | 1113 | super(TestEmailNotificationsWithFiltersWhenBugCreated, self).setUp() | ||
220 | 1114 | self.subscriber = self.factory.makePerson() | ||
221 | 1115 | self.submitter = self.factory.makePerson() | ||
222 | 1116 | self.product = self.factory.makeProduct( | ||
223 | 1117 | bug_supervisor=self.submitter) | ||
224 | 1118 | self.subscription = self.product.addSubscription( | ||
225 | 1119 | self.subscriber, self.subscriber) | ||
226 | 1120 | self.filter = self.subscription.bug_filters[0] | ||
227 | 1121 | self.filter.description = u'Needs triage' | ||
228 | 1122 | self.filter.statuses = [BugTaskStatus.NEW, BugTaskStatus.INCOMPLETE] | ||
229 | 1123 | |||
230 | 1124 | def test_filters_match_when_bug_is_created(self): | ||
231 | 1125 | message = u"this is an unfiltered comment" | ||
232 | 1126 | params = CreateBugParams( | ||
233 | 1127 | title=u"crashes all the time", | ||
234 | 1128 | comment=message, owner=self.submitter, | ||
235 | 1129 | status=BugTaskStatus.NEW) | ||
236 | 1130 | bug = self.product.createBug(params) | ||
237 | 1131 | notification = IStore(BugNotification).find( | ||
238 | 1132 | BugNotification, | ||
239 | 1133 | BugNotification.id==BugNotificationRecipient.bug_notificationID, | ||
240 | 1134 | BugNotificationRecipient.personID == self.subscriber.id, | ||
241 | 1135 | BugNotification.bug == bug).one() | ||
242 | 1136 | self.assertEqual(notification.message.text_contents, message) | ||
243 | 1137 | |||
244 | 1138 | def test_filters_do_not_match_when_bug_is_created(self): | ||
245 | 1139 | message = u"this is a filtered comment" | ||
246 | 1140 | params = CreateBugParams( | ||
247 | 1141 | title=u"crashes all the time", | ||
248 | 1142 | comment=message, owner=self.submitter, | ||
249 | 1143 | status=BugTaskStatus.TRIAGED, | ||
250 | 1144 | importance=BugTaskImportance.HIGH) | ||
251 | 1145 | bug = self.product.createBug(params) | ||
252 | 1146 | notifications = IStore(BugNotification).find( | ||
253 | 1147 | BugNotification, | ||
254 | 1148 | BugNotification.id==BugNotificationRecipient.bug_notificationID, | ||
255 | 1149 | BugNotificationRecipient.personID == self.subscriber.id, | ||
256 | 1150 | BugNotification.bug == bug) | ||
257 | 1151 | self.assertTrue(notifications.is_empty()) | ||
258 | 1100 | 1152 | ||
259 | === modified file 'lib/lp/bugs/tests/test_bug.py' | |||
260 | --- lib/lp/bugs/tests/test_bug.py 2011-04-13 15:23:54 +0000 | |||
261 | +++ lib/lp/bugs/tests/test_bug.py 2011-05-02 17:11:01 +0000 | |||
262 | @@ -1,4 +1,4 @@ | |||
264 | 1 | # Copyright 2011 Canonical Ltd. This software is licensed under the | 1 | |
265 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
266 | 3 | 3 | ||
267 | 4 | """Tests for lp.bugs.model.Bug.""" | 4 | """Tests for lp.bugs.model.Bug.""" |
268 | @@ -6,11 +6,23 @@ | |||
269 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
270 | 7 | 7 | ||
271 | 8 | from lazr.lifecycle.snapshot import Snapshot | 8 | from lazr.lifecycle.snapshot import Snapshot |
272 | 9 | from zope.component import getUtility | ||
273 | 9 | from zope.interface import providedBy | 10 | from zope.interface import providedBy |
274 | 10 | 11 | ||
275 | 11 | from canonical.testing.layers import DatabaseFunctionalLayer | 12 | from canonical.testing.layers import DatabaseFunctionalLayer |
276 | 12 | 13 | ||
277 | 13 | from lp.bugs.enum import BugNotificationLevel | 14 | from lp.bugs.enum import BugNotificationLevel |
278 | 15 | from lp.bugs.interfaces.bug import( | ||
279 | 16 | CreateBugParams, | ||
280 | 17 | IBugSet, | ||
281 | 18 | ) | ||
282 | 19 | from lp.bugs.interfaces.bugtask import ( | ||
283 | 20 | BugTaskImportance, | ||
284 | 21 | BugTaskStatus, | ||
285 | 22 | UserCannotEditBugTaskAssignee, | ||
286 | 23 | UserCannotEditBugTaskImportance, | ||
287 | 24 | UserCannotEditBugTaskMilestone, | ||
288 | 25 | ) | ||
289 | 14 | from lp.testing import ( | 26 | from lp.testing import ( |
290 | 15 | person_logged_in, | 27 | person_logged_in, |
291 | 16 | StormStatementRecorder, | 28 | StormStatementRecorder, |
292 | @@ -30,7 +42,7 @@ | |||
293 | 30 | # Bug.isMuted() will return True if the passed to it has a | 42 | # Bug.isMuted() will return True if the passed to it has a |
294 | 31 | # BugSubscription with a BugNotificationLevel of NOTHING. | 43 | # BugSubscription with a BugNotificationLevel of NOTHING. |
295 | 32 | with person_logged_in(self.person): | 44 | with person_logged_in(self.person): |
297 | 33 | subscription = self.bug.subscribe( | 45 | self.bug.subscribe( |
298 | 34 | self.person, self.person, level=BugNotificationLevel.NOTHING) | 46 | self.person, self.person, level=BugNotificationLevel.NOTHING) |
299 | 35 | self.assertEqual(True, self.bug.isMuted(self.person)) | 47 | self.assertEqual(True, self.bug.isMuted(self.person)) |
300 | 36 | 48 | ||
301 | @@ -38,7 +50,7 @@ | |||
302 | 38 | # Bug.isMuted() will return False if the user has a subscription | 50 | # Bug.isMuted() will return False if the user has a subscription |
303 | 39 | # with BugNotificationLevel that's not NOTHING. | 51 | # with BugNotificationLevel that's not NOTHING. |
304 | 40 | with person_logged_in(self.person): | 52 | with person_logged_in(self.person): |
306 | 41 | subscription = self.bug.subscribe( | 53 | self.bug.subscribe( |
307 | 42 | self.person, self.person, level=BugNotificationLevel.METADATA) | 54 | self.person, self.person, level=BugNotificationLevel.METADATA) |
308 | 43 | self.assertEqual(False, self.bug.isMuted(self.person)) | 55 | self.assertEqual(False, self.bug.isMuted(self.person)) |
309 | 44 | 56 | ||
310 | @@ -122,7 +134,7 @@ | |||
311 | 122 | # optimizations that might trigger the problem again. | 134 | # optimizations that might trigger the problem again. |
312 | 123 | with person_logged_in(self.person): | 135 | with person_logged_in(self.person): |
313 | 124 | with StormStatementRecorder() as recorder: | 136 | with StormStatementRecorder() as recorder: |
315 | 125 | snapshot = Snapshot(self.bug, providing=providedBy(self.bug)) | 137 | Snapshot(self.bug, providing=providedBy(self.bug)) |
316 | 126 | sql_statements = recorder.statements | 138 | sql_statements = recorder.statements |
317 | 127 | # This uses "self" as a marker to show that the attribute does not | 139 | # This uses "self" as a marker to show that the attribute does not |
318 | 128 | # exist. We do not use hasattr because it eats exceptions. | 140 | # exist. We do not use hasattr because it eats exceptions. |
319 | @@ -139,3 +151,113 @@ | |||
320 | 139 | [token for token in sql_tokens | 151 | [token for token in sql_tokens |
321 | 140 | if token.startswith('message')], | 152 | if token.startswith('message')], |
322 | 141 | []) | 153 | []) |
323 | 154 | |||
324 | 155 | |||
325 | 156 | class TestBugCreation(TestCaseWithFactory): | ||
326 | 157 | """Tests for bug creation.""" | ||
327 | 158 | |||
328 | 159 | layer = DatabaseFunctionalLayer | ||
329 | 160 | |||
330 | 161 | def test_CreateBugParams_accepts_importance(self): | ||
331 | 162 | # The importance of the initial bug task can be set using | ||
332 | 163 | # CreateBugParams | ||
333 | 164 | owner = self.factory.makePerson() | ||
334 | 165 | target = self.factory.makeProduct(owner=owner) | ||
335 | 166 | with person_logged_in(owner): | ||
336 | 167 | params = CreateBugParams( | ||
337 | 168 | owner=owner, title="A bug", comment="Nothing important.", | ||
338 | 169 | importance=BugTaskImportance.HIGH) | ||
339 | 170 | params.setBugTarget(product=target) | ||
340 | 171 | bug = getUtility(IBugSet).createBug(params) | ||
341 | 172 | self.assertEqual( | ||
342 | 173 | bug.default_bugtask.importance, params.importance) | ||
343 | 174 | |||
344 | 175 | def test_CreateBugParams_accepts_assignee(self): | ||
345 | 176 | # The assignee of the initial bug task can be set using | ||
346 | 177 | # CreateBugParams | ||
347 | 178 | owner = self.factory.makePerson() | ||
348 | 179 | target = self.factory.makeProduct(owner=owner) | ||
349 | 180 | with person_logged_in(owner): | ||
350 | 181 | params = CreateBugParams( | ||
351 | 182 | owner=owner, title="A bug", comment="Nothing important.", | ||
352 | 183 | assignee=owner) | ||
353 | 184 | params.setBugTarget(product=target) | ||
354 | 185 | bug = getUtility(IBugSet).createBug(params) | ||
355 | 186 | self.assertEqual( | ||
356 | 187 | bug.default_bugtask.assignee, params.assignee) | ||
357 | 188 | |||
358 | 189 | def test_CreateBugParams_accepts_milestone(self): | ||
359 | 190 | # The milestone of the initial bug task can be set using | ||
360 | 191 | # CreateBugParams | ||
361 | 192 | owner = self.factory.makePerson() | ||
362 | 193 | target = self.factory.makeProduct(owner=owner) | ||
363 | 194 | with person_logged_in(owner): | ||
364 | 195 | params = CreateBugParams( | ||
365 | 196 | owner=owner, title="A bug", comment="Nothing important.", | ||
366 | 197 | milestone=self.factory.makeMilestone(product=target)) | ||
367 | 198 | params.setBugTarget(product=target) | ||
368 | 199 | bug = getUtility(IBugSet).createBug(params) | ||
369 | 200 | self.assertEqual( | ||
370 | 201 | bug.default_bugtask.milestone, params.milestone) | ||
371 | 202 | |||
372 | 203 | def test_CreateBugParams_accepts_status(self): | ||
373 | 204 | # The status of the initial bug task can be set using | ||
374 | 205 | # CreateBugParams | ||
375 | 206 | owner = self.factory.makePerson() | ||
376 | 207 | target = self.factory.makeProduct(owner=owner) | ||
377 | 208 | with person_logged_in(owner): | ||
378 | 209 | params = CreateBugParams( | ||
379 | 210 | owner=owner, title="A bug", comment="Nothing important.", | ||
380 | 211 | status=BugTaskStatus.TRIAGED) | ||
381 | 212 | params.setBugTarget(product=target) | ||
382 | 213 | bug = getUtility(IBugSet).createBug(params) | ||
383 | 214 | self.assertEqual( | ||
384 | 215 | bug.default_bugtask.status, params.status) | ||
385 | 216 | |||
386 | 217 | def test_CreateBugParams_rejects_not_allowed_importance_changes(self): | ||
387 | 218 | # createBug() will reject any importance value passed by users | ||
388 | 219 | # who don't have the right to set the importance. | ||
389 | 220 | person = self.factory.makePerson() | ||
390 | 221 | target = self.factory.makeProduct() | ||
391 | 222 | with person_logged_in(person): | ||
392 | 223 | params = CreateBugParams( | ||
393 | 224 | owner=person, title="A bug", comment="Nothing important.", | ||
394 | 225 | importance=BugTaskImportance.HIGH) | ||
395 | 226 | params.setBugTarget(product=target) | ||
396 | 227 | self.assertRaises( | ||
397 | 228 | UserCannotEditBugTaskImportance, | ||
398 | 229 | getUtility(IBugSet).createBug, params) | ||
399 | 230 | |||
400 | 231 | def test_CreateBugParams_rejects_not_allowed_assignee_changes(self): | ||
401 | 232 | # createBug() will reject any importance value passed by users | ||
402 | 233 | # who don't have the right to set the assignee. | ||
403 | 234 | person = self.factory.makePerson() | ||
404 | 235 | person_2 = self.factory.makePerson() | ||
405 | 236 | target = self.factory.makeProduct() | ||
406 | 237 | # Setting the target's bug supervisor means that | ||
407 | 238 | # canTransitionToAssignee() will return False for `person` if | ||
408 | 239 | # another Person is passed as `assignee`. | ||
409 | 240 | with person_logged_in(target.owner): | ||
410 | 241 | target.setBugSupervisor(target.owner, target.owner) | ||
411 | 242 | with person_logged_in(person): | ||
412 | 243 | params = CreateBugParams( | ||
413 | 244 | owner=person, title="A bug", comment="Nothing important.", | ||
414 | 245 | assignee=person_2) | ||
415 | 246 | params.setBugTarget(product=target) | ||
416 | 247 | self.assertRaises( | ||
417 | 248 | UserCannotEditBugTaskAssignee, | ||
418 | 249 | getUtility(IBugSet).createBug, params) | ||
419 | 250 | |||
420 | 251 | def test_CreateBugParams_rejects_not_allowed_milestone_changes(self): | ||
421 | 252 | # createBug() will reject any importance value passed by users | ||
422 | 253 | # who don't have the right to set the milestone. | ||
423 | 254 | person = self.factory.makePerson() | ||
424 | 255 | target = self.factory.makeProduct() | ||
425 | 256 | with person_logged_in(person): | ||
426 | 257 | params = CreateBugParams( | ||
427 | 258 | owner=person, title="A bug", comment="Nothing important.", | ||
428 | 259 | milestone=self.factory.makeMilestone(product=target)) | ||
429 | 260 | params.setBugTarget(product=target) | ||
430 | 261 | self.assertRaises( | ||
431 | 262 | UserCannotEditBugTaskMilestone, | ||
432 | 263 | getUtility(IBugSet).createBug, params) |
This branch looks good.
The only thought I had was that the part starting on line 13 of the diff pastebin. ubuntu. com/602393/ (untested).
is a bit repetitive and I had to look very closely to see if it was
exactly the same pattern for each line or if one or more were subtly
different. However, I'm not sure my version is any better:
http://