Merge lp:~wallyworld/launchpad/delete-bugtask-ui-ajax-878909 into lp:launchpad
- delete-bugtask-ui-ajax-878909
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Ian Booth |
Approved revision: | no longer in the source branch. |
Merged at revision: | 14255 |
Proposed branch: | lp:~wallyworld/launchpad/delete-bugtask-ui-ajax-878909 |
Merge into: | lp:launchpad |
Prerequisite: | lp:~wallyworld/launchpad/delete-bugtask-ui-878909 |
Diff against target: |
1400 lines (+850/-106) 17 files modified
lib/lp/app/widgets/templates/form-picker-macros.pt (+7/-2) lib/lp/bugs/browser/bugtask.py (+85/-14) lib/lp/bugs/browser/configure.zcml (+6/-0) lib/lp/bugs/browser/tests/bug-views.txt (+14/-3) lib/lp/bugs/browser/tests/test_bug_views.py (+1/-1) lib/lp/bugs/browser/tests/test_bugtask.py (+117/-5) lib/lp/bugs/javascript/bugtask_index.js (+243/-9) lib/lp/bugs/javascript/subscribers.js (+6/-1) lib/lp/bugs/javascript/tests/test_bugtask_delete.html (+81/-0) lib/lp/bugs/javascript/tests/test_bugtask_delete.js (+223/-0) lib/lp/bugs/javascript/tests/test_subscribers.js (+9/-2) lib/lp/bugs/stories/bugs/xx-bug-index.txt (+7/-7) lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt (+6/-2) lib/lp/bugs/templates/bugtask-index.pt (+2/-5) lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt (+15/-33) lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt (+1/-22) lib/lp/bugs/templates/bugtasks-and-nominations-table.pt (+27/-0) |
To merge this branch: | bzr merge lp:~wallyworld/launchpad/delete-bugtask-ui-ajax-878909 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Review via email:
|
Commit message
[r=rvb][bug=878909] Add ajax support for deleting bug tasks.
Description of the change
== Implementation ==
Requires feature flag: disclosure.
1. The core ajax implementation is relatively straightforward:
- capture the click on the delete link
- perform a POST with url = <bug task url>/+delete
- the standard zope BugTaskDeleteView form performs the delete as for a HTML form submit
- the form detects an XHR request being used and renders and returns the HTML for the new bug tasks table
- the javascript caller replaces the old bugtasks table with the new HTML received from the XHR call
Some small refactoring was required to extract the tales required to render just the bug tasks table. The existing view called +bugtasks-
So first issue: the new bugtasks table is duly rendered but none of the javascript widgets are wired up. This is because some of the javascript is included as part of the tales used to render the picker widgets, while other javascript is only executed once on page load.
2. Extract bugtask row javascript from tales
The lp.bug.bugtak_index module already contains a method (setup_bugtask_row) for setting up a bug task row (wiring up importance/status widgets, adding expander etc). So the javascript from the bugtask-
An onload handler was added to execute a new setup_bugtask_
When the new bug tasks table is rendered after the XHR delete call, the same setup_bugtask_
3. Bug found
There were conflicting implementations of userCanEditImpo
bugtask.
while the other left out the second bugwatch condition. This caused the edit icon to be incorrectly rendered for remote bug tasks. I've fixed it.
4. Wire the pickers
There's pickers for assignee and also project/source package selection in the expandable form for each bug task. The javascript to wire these up lives in the form-picker-
5. Confirmation dialog
A confirmation dialog was added to guard against accidental deletion.
6. Problem - deleting the highlighted bug task
The page used to display list of bug tasks for a given bug is rendered as the bug index view as well as the index view for each individual bug task url. The bug task corresponding to the current url is highlighted and:
i) the client cache self_link and web_link values are for the highlighted bugtask
ii) the XHR links for duplicate marking, privacy setting etc are all relative to this url
When the current bug task is deleted, the table is correctly re-rendered and the bug's new default bug tasks is now the highlighted one. But the urls referred to above in the cache and links are now invalid.
One existing case was fixed - the subscribers portlet setup was changed to use the bug's url rather than the bug task.
The other cases need fixing though.
Options:
i) update the URLs using javascript code when the bug task table is re-rendered
ii) server side zope publisher redirect when invoked with the old bug task urls
iii) when highlighted bug task is deleted, in that case simply redirect to the bugtask url of the new default task. this causes a new page load but the urls will be correct
Option (iii) is the one that can be best implemented. It has its own problem though. The the server does a redirect during an XHR call, the redirect is followed and the rendered contents are returned to the Javascript caller with status 200. The caller interprets this data as if the original call completed without the redirect. So the new bugtask page is rendered over the top of the bugtask table. Not what we want.
To solve the problem, the view returns a JSON dict containing the new bugtask URL when a redirection is required. The caller then does the redirect itself. The existing ReturnToReferre
== Demo and QA ==
http://
== Tests ==
1. Renamed +bugtasks-
Add extra test to bug-views.txt and update existing tests to use the new view name.
2. New setup for bugs.javascript
Update test_subscribers.js
3. New DeleteBugTaskView behaviour
Add new tests to TestDeleteBugTa
- test_ajax_
- test_ajax_
4. New Javascript bugtask delete functionality
Add new yui test: bugs.javascript
5. Under the covers picker changes etc
Reply on yuixhr tests ported from windmill tests (when done) since the windmill tests covered the rendering and wiring up of the pickers.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
640: Move 'var' declarations to the top of the function.
640: Stopping. (46% scanned).
-1: JSLINT had a fatal error.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ian Booth (wallyworld) wrote : | # |
Hi Raphaël
Thanks for the great review!
>
> [0]
>
> 1249 lines. Please have mercy on us poor reviewers ;). I know Javascript is verbose and you've done a lot of refactoring but still, I'm sure you would have had a volunteer reviewer for this branch sooner if you had split it into 2 or 3. Also, the problem with enormous branches is that the odds of getting a conflict goes way up.
>
Yeah, the core work was < 800 lines but then the yui tests pushed it
over. Normally I would go and split it but the core work would still
have produced a large diff.
>> 3. Bug found
>> There were conflicting implementations of userCanEditImpo
>> bugtask.
>
> For instance you could have fixed this in another pipe (I don't know if you already use the bzr pipeline plugin but in case you don't, please check it out: http://
>
In fact, this mp itself is the 2nd in a pipeline so I know how to use
such things :-P
The first branch adds the non-ajax ui for deleting bugtasks.
This mp would have been broken without the bug fix; if I were to have
introduced a new pipe before this one, it would only have saved a few lines.
> [2]
>
> 312 + def check_bugtask_
> 313 + self.assertIn(
>
> Don't you think it would be easier to read if you moved this method outside of the test method?
>
I like the use of an inner method here. The code is not relevant outside
the test method it is in and nicely packages the common checks together
to eliminate duplication.
> [3]
>
> 361 + self.assertEqual(
> 362 + view.request.
> 363 + 'application/json')
>
> 'application/json' should be the first argument, otherwise a failure will be difficult to interpret.
>
Of course. Typo. Thanks for spotting that.
> [4]
>
> 334 + bug = self.factory.
> 335 + bugtask = self.factory.
> 336 + target_name = bugtask.
> 337 + bugtask_url = canonical_
>
> And
>
> 370 + bug = self.factory.
> 371 + bugtask = self.factory.
> 372 + target_name = bugtask.
> 373 + default_bugtask_url = canonical_url(
> 374 + bug.default_
>
> Would you mind refactoring that code into a common setup method?
>
Sure. I didn't bother because it was only a few lines but I'll add the
method.
> [5]
>
> 517 + if( bugtask_
>
> small typos: if( bugtask_
>
> Same here:
>
> 532 + if( link.hasClass(
> 537 + if( Y.Lang.
> 629 + if( content_type === 'application/json' ) {
>
Old habits. I prefer the above formatting and used it for the past 15
years so am still adjusting to the requirement to put the braces in the
"wrong" place :-/
> [6]
>
> 591 + var delete_te...
Preview Diff
1 | === modified file 'lib/lp/app/widgets/templates/form-picker-macros.pt' |
2 | --- lib/lp/app/widgets/templates/form-picker-macros.pt 2011-08-18 08:00:28 +0000 |
3 | +++ lib/lp/app/widgets/templates/form-picker-macros.pt 2011-11-04 14:06:00 +0000 |
4 | @@ -40,9 +40,11 @@ |
5 | var vocabulary = config.vocabulary_name; |
6 | var vocabulary_filters = config.vocabulary_filters; |
7 | var input_element = config.input_element; |
8 | - Y.on('domready', function(e) { |
9 | + var show_widget_id = '${view/show_widget_id}'; |
10 | + var namespace = Y.namespace('lp.app.picker.connect'); |
11 | + namespace[show_widget_id] = function() { |
12 | // Sort out the Choose... link. |
13 | - var show_widget_node = Y.one('#${view/show_widget_id}'); |
14 | + var show_widget_node = Y.one('#'+show_widget_id); |
15 | |
16 | show_widget_node.set('innerHTML', 'Choose…'); |
17 | show_widget_node.addClass('js-action'); |
18 | @@ -56,6 +58,9 @@ |
19 | picker.show(); |
20 | e.preventDefault(); |
21 | }); |
22 | + }; |
23 | + Y.on('domready', function(e) { |
24 | + namespace[show_widget_id](); |
25 | }); |
26 | }); |
27 | "/> |
28 | |
29 | === modified file 'lib/lp/bugs/browser/bugtask.py' |
30 | --- lib/lp/bugs/browser/bugtask.py 2011-11-04 14:05:57 +0000 |
31 | +++ lib/lp/bugs/browser/bugtask.py 2011-11-04 14:06:00 +0000 |
32 | @@ -119,6 +119,7 @@ |
33 | isinstance as zope_isinstance, |
34 | removeSecurityProxy, |
35 | ) |
36 | +from zope.traversing.browser import absoluteURL |
37 | from zope.traversing.interfaces import IPathAdapter |
38 | |
39 | from canonical.config import config |
40 | @@ -678,11 +679,20 @@ |
41 | cancel_url = canonical_url(self.context) |
42 | return cancel_url |
43 | |
44 | + @cachedproperty |
45 | + def api_request(self): |
46 | + return IWebServiceClientRequest(self.request) |
47 | + |
48 | def initialize(self): |
49 | """Set up the needed widgets.""" |
50 | bug = self.context.bug |
51 | cache = IJSONRequestCache(self.request) |
52 | cache.objects['bug'] = bug |
53 | + subscribers_url_data = { |
54 | + 'web_link': canonical_url(bug, rootsite='bugs'), |
55 | + 'self_link': absoluteURL(bug, self.api_request), |
56 | + } |
57 | + cache.objects['subscribers_portlet_url_data'] = subscribers_url_data |
58 | cache.objects['total_comments_and_activity'] = ( |
59 | self.total_comments + self.total_activity) |
60 | cache.objects['initial_comment_batch_offset'] = ( |
61 | @@ -1771,13 +1781,42 @@ |
62 | label = 'Remove bug task' |
63 | page_title = label |
64 | |
65 | + @property |
66 | + def next_url(self): |
67 | + """Return the next URL to call when this call completes.""" |
68 | + if not self.request.is_ajax: |
69 | + return super(BugTaskDeletionView, self).next_url |
70 | + return None |
71 | + |
72 | @action('Delete', name='delete_bugtask') |
73 | def delete_bugtask_action(self, action, data): |
74 | bugtask = self.context |
75 | + bug = bugtask.bug |
76 | + deleted_bugtask_url = canonical_url(self.context, rootsite='bugs') |
77 | message = ("This bug no longer affects %s." |
78 | - % bugtask.target.bugtargetdisplayname) |
79 | + % bugtask.bugtargetdisplayname) |
80 | bugtask.delete() |
81 | self.request.response.addNotification(message) |
82 | + if self.request.is_ajax: |
83 | + launchbag = getUtility(ILaunchBag) |
84 | + launchbag.add(bug.default_bugtask) |
85 | + # If we are deleting the current highlighted bugtask via ajax, |
86 | + # we must force a redirect to the new default bugtask to ensure |
87 | + # all URLs and other client cache content is correctly refreshed. |
88 | + # We can't do the redirect here since the XHR caller won't see it |
89 | + # so we return the URL to go to and let the caller do it. |
90 | + if self._return_url == deleted_bugtask_url: |
91 | + next_url = canonical_url( |
92 | + bug.default_bugtask, rootsite='bugs') |
93 | + self.request.response.setHeader('Content-type', |
94 | + 'application/json') |
95 | + return dumps(dict(bugtask_url=next_url)) |
96 | + # No redirect required so return the new bugtask table HTML. |
97 | + view = getMultiAdapter( |
98 | + (bug, self.request), |
99 | + name='+bugtasks-and-nominations-table') |
100 | + view.initialize() |
101 | + return view.render() |
102 | |
103 | |
104 | class BugTaskListingView(LaunchpadView): |
105 | @@ -3681,6 +3720,10 @@ |
106 | super(BugTaskTableRowView, self).__init__(context, request) |
107 | self.milestone_source = MilestoneVocabulary |
108 | |
109 | + @cachedproperty |
110 | + def api_request(self): |
111 | + return IWebServiceClientRequest(self.request) |
112 | + |
113 | def initialize(self): |
114 | super(BugTaskTableRowView, self).initialize() |
115 | link = canonical_url(self.context) |
116 | @@ -3710,15 +3753,23 @@ |
117 | target_link_title=self.target_link_title, |
118 | user_can_delete=self.user_can_delete_bugtask, |
119 | delete_link=delete_link, |
120 | - user_can_edit_importance=self.context.userCanEditImportance( |
121 | - self.user), |
122 | + user_can_edit_importance=self.user_can_edit_importance, |
123 | importance_css_class='importance' + self.context.importance.name, |
124 | importance_title=self.context.importance.title, |
125 | # We always look up all milestones, so there's no harm |
126 | # using len on the list here and avoid the COUNT query. |
127 | target_has_milestones=len(self._visible_milestones) > 0, |
128 | + user_can_edit_status=self.user_can_edit_status, |
129 | ) |
130 | |
131 | + if not self.many_bugtasks: |
132 | + cache = IJSONRequestCache(self.request) |
133 | + bugtask_data = cache.objects.get('bugtask_data', None) |
134 | + if bugtask_data is None: |
135 | + bugtask_data = dict() |
136 | + cache.objects['bugtask_data'] = bugtask_data |
137 | + bugtask_data[bugtask_id] = self.bugtask_config() |
138 | + |
139 | def canSeeTaskDetails(self): |
140 | """Whether someone can see a task's status details. |
141 | |
142 | @@ -3821,7 +3872,7 @@ |
143 | items = vocabulary_to_choice_edit_items( |
144 | self._visible_milestones, |
145 | value_fn=lambda item: canonical_url( |
146 | - item, request=IWebServiceClientRequest(self.request))) |
147 | + item, request=self.api_request)) |
148 | items.append({ |
149 | "name": "Remove milestone", |
150 | "disabled": False, |
151 | @@ -3835,13 +3886,29 @@ |
152 | """Return the canonical url for the bugtask.""" |
153 | return canonical_url(self.context) |
154 | |
155 | - @property |
156 | + @cachedproperty |
157 | def user_can_edit_importance(self): |
158 | """Can the user edit the Importance field? |
159 | |
160 | If yes, return True, otherwise return False. |
161 | """ |
162 | - return self.context.userCanEditImportance(self.user) |
163 | + bugtask = self.context |
164 | + return (self.user_can_edit_status |
165 | + and bugtask.userCanEditImportance(self.user)) |
166 | + |
167 | + @cachedproperty |
168 | + def user_can_edit_status(self): |
169 | + """Can the user edit the Status field? |
170 | + |
171 | + If yes, return True, otherwise return False. |
172 | + """ |
173 | + bugtask = self.context |
174 | + edit_allowed = bugtask.target_uses_malone or bugtask.bugwatch |
175 | + if bugtask.bugwatch: |
176 | + bugtracker = bugtask.bugwatch.bugtracker |
177 | + edit_allowed = ( |
178 | + bugtracker.bugtrackertype == BugTrackerType.EMAILADDRESS) |
179 | + return edit_allowed |
180 | |
181 | @property |
182 | def user_can_edit_assignee(self): |
183 | @@ -3883,8 +3950,8 @@ |
184 | else: |
185 | return '' |
186 | |
187 | - def js_config(self): |
188 | - """Configuration for the JS widgets on the row, JSON-serialized.""" |
189 | + def bugtask_config(self): |
190 | + """Configuration for the bugtask JS widgets on the row.""" |
191 | assignee_vocabulary, assignee_vocabulary_filters = ( |
192 | get_assignee_vocabulary_info(self.context)) |
193 | # If we have no filters or just the ALL filter, then no filtering |
194 | @@ -3906,16 +3973,21 @@ |
195 | not self.context.userCanSetAnyAssignee(user) and |
196 | (user is None or user.teams_participated_in.count() == 0)) |
197 | cx = self.context |
198 | - return dumps(dict( |
199 | + return dict( |
200 | row_id=self.data['row_id'], |
201 | + form_row_id=self.data['form_row_id'], |
202 | bugtask_path='/'.join([''] + self.data['link'].split('/')[3:]), |
203 | prefix=get_prefix(cx), |
204 | + targetname=cx.bugtargetdisplayname, |
205 | + bug_title=cx.bug.title, |
206 | assignee_value=cx.assignee and cx.assignee.name, |
207 | assignee_is_team=cx.assignee and cx.assignee.is_team, |
208 | assignee_vocabulary=assignee_vocabulary, |
209 | assignee_vocabulary_filters=filter_details, |
210 | hide_assignee_team_selection=hide_assignee_team_selection, |
211 | user_can_unassign=cx.userCanUnassign(user), |
212 | + user_can_delete=self.user_can_delete_bugtask, |
213 | + delete_link=self.data['delete_link'], |
214 | target_is_product=IProduct.providedBy(cx.target), |
215 | status_widget_items=self.status_widget_items, |
216 | status_value=cx.status.title, |
217 | @@ -3925,14 +3997,13 @@ |
218 | milestone_value=( |
219 | canonical_url( |
220 | cx.milestone, |
221 | - request=IWebServiceClientRequest(self.request)) |
222 | + request=self.api_request) |
223 | if cx.milestone else None), |
224 | user_can_edit_assignee=self.user_can_edit_assignee, |
225 | user_can_edit_milestone=self.user_can_edit_milestone, |
226 | - user_can_edit_status=not cx.bugwatch, |
227 | - user_can_edit_importance=( |
228 | - self.user_can_edit_importance and not cx.bugwatch) |
229 | - )) |
230 | + user_can_edit_status=self.user_can_edit_status, |
231 | + user_can_edit_importance=self.user_can_edit_importance, |
232 | + ) |
233 | |
234 | |
235 | class BugsBugTaskSearchListingView(BugTaskSearchListingView): |
236 | |
237 | === modified file 'lib/lp/bugs/browser/configure.zcml' |
238 | --- lib/lp/bugs/browser/configure.zcml 2011-11-04 14:05:57 +0000 |
239 | +++ lib/lp/bugs/browser/configure.zcml 2011-11-04 14:06:00 +0000 |
240 | @@ -1029,6 +1029,12 @@ |
241 | for="lp.bugs.interfaces.bug.IBug" |
242 | class="lp.bugs.browser.bugtask.BugTasksAndNominationsView" |
243 | permission="launchpad.View" |
244 | + name="+bugtasks-and-nominations-portal" |
245 | + template="../templates/bugtasks-and-nominations-portal.pt"/> |
246 | + <browser:page |
247 | + for="lp.bugs.interfaces.bug.IBug" |
248 | + class="lp.bugs.browser.bugtask.BugTasksAndNominationsView" |
249 | + permission="launchpad.View" |
250 | name="+bugtasks-and-nominations-table" |
251 | template="../templates/bugtasks-and-nominations-table.pt"/> |
252 | <browser:page |
253 | |
254 | === modified file 'lib/lp/bugs/browser/tests/bug-views.txt' |
255 | --- lib/lp/bugs/browser/tests/bug-views.txt 2011-08-01 05:25:59 +0000 |
256 | +++ lib/lp/bugs/browser/tests/bug-views.txt 2011-11-04 14:06:00 +0000 |
257 | @@ -432,9 +432,20 @@ |
258 | BugTasks and Nominations Table |
259 | ------------------------------ |
260 | |
261 | -A table is rendered at the top of the bug page which shows both bugtasks |
262 | -and nominations. This table is rendered with the |
263 | -+bugtasks-and-nomination-table view. |
264 | +Content is rendered at the top of the bug page which shows both bugtasks |
265 | +and nominations and various links like "Does this bug affect you" and |
266 | +"Also Affects Project" etc. This content is rendered with the |
267 | ++bugtasks-and-nominations-portal view. |
268 | + |
269 | + >>> request = LaunchpadTestRequest() |
270 | + |
271 | + >>> bugtasks_and_nominations_view = getMultiAdapter( |
272 | + ... (bug_one_bugtask.bug, request), |
273 | + ... name="+bugtasks-and-nominations-portal") |
274 | + >>> bugtasks_and_nominations_view.initialize() |
275 | + |
276 | +The bugtasks and nominations table itself is rendered with the |
277 | ++bugtasks-and-nominations-table view. |
278 | |
279 | >>> request = LaunchpadTestRequest() |
280 | |
281 | |
282 | === modified file 'lib/lp/bugs/browser/tests/test_bug_views.py' |
283 | --- lib/lp/bugs/browser/tests/test_bug_views.py 2011-10-25 02:12:44 +0000 |
284 | +++ lib/lp/bugs/browser/tests/test_bug_views.py 2011-11-04 14:06:00 +0000 |
285 | @@ -105,7 +105,7 @@ |
286 | email_address = "mark@example.com" |
287 | browser = self.getBrowserForBugWithEmail( |
288 | email_address, no_login=False) |
289 | - self.assertEqual(6, browser.contents.count(email_address)) |
290 | + self.assertEqual(7, browser.contents.count(email_address)) |
291 | |
292 | def test_anonymous_sees_not_email_address(self): |
293 | """The anonymous user cannot see the email address on the page.""" |
294 | |
295 | === modified file 'lib/lp/bugs/browser/tests/test_bugtask.py' |
296 | --- lib/lp/bugs/browser/tests/test_bugtask.py 2011-11-04 14:05:57 +0000 |
297 | +++ lib/lp/bugs/browser/tests/test_bugtask.py 2011-11-04 14:06:00 +0000 |
298 | @@ -6,6 +6,7 @@ |
299 | from contextlib import contextmanager |
300 | from datetime import datetime |
301 | import re |
302 | +import simplejson |
303 | import urllib |
304 | |
305 | from lazr.lifecycle.event import ObjectModifiedEvent |
306 | @@ -118,7 +119,7 @@ |
307 | self.getUserBrowser(url, person_no_teams) |
308 | # This may seem large: it is; there is easily another 30% fat in |
309 | # there. |
310 | - self.assertThat(recorder, HasQueryCount(LessThan(76))) |
311 | + self.assertThat(recorder, HasQueryCount(LessThan(84))) |
312 | count_with_no_teams = recorder.count |
313 | # count with many teams |
314 | self.invalidate_caches(task) |
315 | @@ -134,7 +135,7 @@ |
316 | def test_rendered_query_counts_constant_with_attachments(self): |
317 | with celebrity_logged_in('admin'): |
318 | browses_under_limit = BrowsesWithQueryLimit( |
319 | - 82, self.factory.makePerson()) |
320 | + 86, self.factory.makePerson()) |
321 | |
322 | # First test with a single attachment. |
323 | task = self.factory.makeBugTask() |
324 | @@ -569,7 +570,7 @@ |
325 | |
326 | request = LaunchpadTestRequest() |
327 | foo_bugtasks_and_nominations_view = getMultiAdapter( |
328 | - (foo_bug, request), name="+bugtasks-and-nominations-table") |
329 | + (foo_bug, request), name="+bugtasks-and-nominations-portal") |
330 | foo_bugtasks_and_nominations_view.initialize() |
331 | |
332 | task_and_nomination_views = ( |
333 | @@ -593,7 +594,7 @@ |
334 | |
335 | request = LaunchpadTestRequest() |
336 | foo_bugtasks_and_nominations_view = getMultiAdapter( |
337 | - (foo_bug, request), name="+bugtasks-and-nominations-table") |
338 | + (foo_bug, request), name="+bugtasks-and-nominations-portal") |
339 | foo_bugtasks_and_nominations_view.initialize() |
340 | |
341 | task_and_nomination_views = ( |
342 | @@ -689,6 +690,37 @@ |
343 | 'bugtask-delete-task%d' % bug.default_bugtask.id) |
344 | self.assertIsNone(delete_icon) |
345 | |
346 | + def test_client_cache_contents(self): |
347 | + """ Test that the client cache contains the expected data. |
348 | + |
349 | + The cache data is used by the Javascript to enable the delete |
350 | + links to work as expected. |
351 | + """ |
352 | + bug = self.factory.makeBug() |
353 | + bugtask_owner = self.factory.makePerson() |
354 | + bugtask = self.factory.makeBugTask(bug=bug, owner=bugtask_owner) |
355 | + with FeatureFixture(DELETE_BUGTASK_ENABLED): |
356 | + login_person(bugtask.owner) |
357 | + getUtility(ILaunchBag).add(bug.default_bugtask) |
358 | + view = create_initialized_view( |
359 | + bug, name='+bugtasks-and-nominations-table', |
360 | + principal=bugtask.owner) |
361 | + view.render() |
362 | + cache = IJSONRequestCache(view.request) |
363 | + all_bugtask_data = cache.objects['bugtask_data'] |
364 | + |
365 | + def check_bugtask_data(bugtask, can_delete): |
366 | + self.assertIn(bugtask.id, all_bugtask_data) |
367 | + bugtask_data = all_bugtask_data[bugtask.id] |
368 | + self.assertEqual( |
369 | + 'task%d' % bugtask.id, bugtask_data['form_row_id']) |
370 | + self.assertEqual( |
371 | + 'tasksummary%d' % bugtask.id, bugtask_data['row_id']) |
372 | + self.assertEqual(can_delete, bugtask_data['user_can_delete']) |
373 | + |
374 | + check_bugtask_data(bug.default_bugtask, False) |
375 | + check_bugtask_data(bugtask, True) |
376 | + |
377 | |
378 | class TestBugTaskDeleteView(TestCaseWithFactory): |
379 | """Test the bug task delete form.""" |
380 | @@ -734,6 +766,86 @@ |
381 | expected = 'This bug no longer affects %s.' % target_name |
382 | self.assertEqual(expected, notifications[0].message) |
383 | |
384 | + def _create_bugtask_to_delete(self): |
385 | + bug = self.factory.makeBug() |
386 | + bugtask = self.factory.makeBugTask(bug=bug) |
387 | + target_name = bugtask.bugtargetdisplayname |
388 | + bugtask_url = canonical_url(bugtask, rootsite='bugs') |
389 | + return bug, bugtask, target_name, bugtask_url |
390 | + |
391 | + def test_ajax_delete_current_bugtask(self): |
392 | + # Test that deleting the current bugtask returns a JSON dict |
393 | + # containing the URL of the bug's default task to redirect to. |
394 | + bug, bugtask, target_name, bugtask_url = ( |
395 | + self._create_bugtask_to_delete()) |
396 | + with FeatureFixture(DELETE_BUGTASK_ENABLED): |
397 | + login_person(bugtask.owner) |
398 | + # Set up the request so that we correctly simulate an XHR call |
399 | + # from the URL of the bugtask we are deleting. |
400 | + server_url = canonical_url( |
401 | + getUtility(ILaunchpadRoot), rootsite='bugs') |
402 | + extra = { |
403 | + 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest', |
404 | + 'HTTP_REFERER': bugtask_url, |
405 | + } |
406 | + form = { |
407 | + 'field.actions.delete_bugtask': 'Delete' |
408 | + } |
409 | + view = create_initialized_view( |
410 | + bugtask, name='+delete', server_url=server_url, form=form, |
411 | + principal=bugtask.owner, **extra) |
412 | + result_data = simplejson.loads(view.render()) |
413 | + self.assertEqual([bug.default_bugtask], bug.bugtasks) |
414 | + notifications = simplejson.loads( |
415 | + view.request.response.getHeader('X-Lazr-Notifications')) |
416 | + self.assertEqual(1, len(notifications)) |
417 | + expected = 'This bug no longer affects %s.' % target_name |
418 | + self.assertEqual(expected, notifications[0][1]) |
419 | + self.assertEqual( |
420 | + 'application/json', |
421 | + view.request.response.getHeader('content-type')) |
422 | + expected_url = canonical_url(bug.default_bugtask, rootsite='bugs') |
423 | + self.assertEqual(dict(bugtask_url=expected_url), result_data) |
424 | + |
425 | + def test_ajax_delete_non_current_bugtask(self): |
426 | + # Test that deleting the non-current bugtask returns the new bugtasks |
427 | + # table as HTML. |
428 | + bug, bugtask, target_name, bugtask_url = ( |
429 | + self._create_bugtask_to_delete()) |
430 | + default_bugtask_url = canonical_url( |
431 | + bug.default_bugtask, rootsite='bugs') |
432 | + with FeatureFixture(DELETE_BUGTASK_ENABLED): |
433 | + login_person(bugtask.owner) |
434 | + # Set up the request so that we correctly simulate an XHR call |
435 | + # from the URL of the default bugtask, not the one we are |
436 | + # deleting. |
437 | + server_url = canonical_url( |
438 | + getUtility(ILaunchpadRoot), rootsite='bugs') |
439 | + extra = { |
440 | + 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest', |
441 | + 'HTTP_REFERER': default_bugtask_url, |
442 | + } |
443 | + form = { |
444 | + 'field.actions.delete_bugtask': 'Delete' |
445 | + } |
446 | + view = create_initialized_view( |
447 | + bugtask, name='+delete', server_url=server_url, form=form, |
448 | + principal=bugtask.owner, **extra) |
449 | + result_html = view.render() |
450 | + self.assertEqual([bug.default_bugtask], bug.bugtasks) |
451 | + notifications = view.request.response.notifications |
452 | + self.assertEqual(1, len(notifications)) |
453 | + expected = 'This bug no longer affects %s.' % target_name |
454 | + self.assertEqual(expected, notifications[0].message) |
455 | + self.assertEqual( |
456 | + view.request.response.getHeader('content-type'), 'text/html') |
457 | + table = find_tag_by_id(result_html, 'affected-software') |
458 | + self.assertIsNotNone(table) |
459 | + [row] = table.tbody.findAll('tr', {'class': 'highlight'}) |
460 | + target_link = row.find('a', {'class': 'sprite product'}) |
461 | + self.assertIn( |
462 | + bug.default_bugtask.bugtargetdisplayname, target_link) |
463 | + |
464 | |
465 | class TestBugTasksAndNominationsViewAlsoAffects(TestCaseWithFactory): |
466 | """ Tests the boolean methods on the view used to indicate whether the |
467 | @@ -752,7 +864,7 @@ |
468 | def _createView(self, bug): |
469 | request = LaunchpadTestRequest() |
470 | bugtasks_and_nominations_view = getMultiAdapter( |
471 | - (bug, request), name="+bugtasks-and-nominations-table") |
472 | + (bug, request), name="+bugtasks-and-nominations-portal") |
473 | return bugtasks_and_nominations_view |
474 | |
475 | def test_project_bug_cannot_affect_something_else(self): |
476 | |
477 | === modified file 'lib/lp/bugs/javascript/bugtask_index.js' |
478 | --- lib/lp/bugs/javascript/bugtask_index.js 2011-10-27 01:11:39 +0000 |
479 | +++ lib/lp/bugs/javascript/bugtask_index.js 2011-11-04 14:06:00 +0000 |
480 | @@ -11,6 +11,9 @@ |
481 | |
482 | var namespace = Y.namespace('lp.bugs.bugtask_index'); |
483 | |
484 | +// Override for testing |
485 | +namespace.ANIM_DURATION = 1; |
486 | + |
487 | // lazr.FormOverlay objects. |
488 | var duplicate_form_overlay; |
489 | var privacy_form_overlay; |
490 | @@ -246,7 +249,10 @@ |
491 | dupe_span.one('a').set('href', update_dupe_url); |
492 | hide_comment_on_duplicate_warning(); |
493 | } |
494 | - Y.lp.anim.green_flash({node: dupe_span}).run(); |
495 | + Y.lp.anim.green_flash({ |
496 | + node: dupe_span, |
497 | + duration: namespace.ANIM_DURATION |
498 | + }).run(); |
499 | // ensure the new link is hooked up correctly: |
500 | dupe_span.one('a').on( |
501 | 'click', function(e){ |
502 | @@ -355,7 +361,10 @@ |
503 | privacy_link.setStyle('display', 'inline'); |
504 | }; |
505 | error_handler.showError = function (error_msg) { |
506 | - Y.lp.anim.red_flash({node: privacy_div}).run(); |
507 | + Y.lp.anim.red_flash({ |
508 | + node: privacy_div, |
509 | + duration: namespace.ANIM_DURATION |
510 | + }).run(); |
511 | privacy_form_overlay.showError(error_msg); |
512 | privacy_form_overlay.show(); |
513 | }; |
514 | @@ -438,7 +447,10 @@ |
515 | } |
516 | Y.lp.client.display_notifications( |
517 | response.getResponseHeader('X-Lazr-Notifications')); |
518 | - Y.lp.anim.green_flash({node: privacy_div}).run(); |
519 | + Y.lp.anim.green_flash({ |
520 | + node: privacy_div, |
521 | + duration: namespace.ANIM_DURATION |
522 | + }).run(); |
523 | }, |
524 | failure: error_handler.getFailureHandler() |
525 | } |
526 | @@ -578,9 +590,15 @@ |
527 | |
528 | var bug_branch_container = Y.one('#bug-branches-container'); |
529 | bug_branch_container.appendChild(bug_branch_list); |
530 | - anim = Y.lp.anim.green_flash({node: bug_branch_list}); |
531 | + anim = Y.lp.anim.green_flash({ |
532 | + node: bug_branch_list, |
533 | + duration: namespace.ANIM_DURATION |
534 | + }); |
535 | } else { |
536 | - anim = Y.lp.anim.green_flash({node: bug_branch_node}); |
537 | + anim = Y.lp.anim.green_flash({ |
538 | + node: bug_branch_node, |
539 | + duration: namespace.ANIM_DURATION |
540 | + }); |
541 | } |
542 | |
543 | var existing_bug_branch_node = bug_branch_list.one( |
544 | @@ -591,7 +609,10 @@ |
545 | bug_branch_list.appendChild(bug_branch_node); |
546 | } else { |
547 | // If the bug branch exists already, flash it. |
548 | - anim = Y.lp.anim.green_flash({node: existing_bug_branch_node}); |
549 | + anim = Y.lp.anim.green_flash({ |
550 | + node: existing_bug_branch_node, |
551 | + duration: namespace.ANIM_DURATION |
552 | + }); |
553 | } |
554 | anim.run(); |
555 | // Fire of the generic branch linked event. |
556 | @@ -624,6 +645,196 @@ |
557 | }; |
558 | |
559 | /** |
560 | + * Set up the bug task table. |
561 | + * |
562 | + * Called once on load, to initialize the page, and also when the contents of |
563 | + * the bug task table is replaced after an XHR call. |
564 | + * |
565 | + * @method setup_bugtask_table |
566 | + */ |
567 | +namespace.setup_bugtask_table = function() { |
568 | + var bugtask_data = LP.cache.bugtask_data; |
569 | + if (!Y.Lang.isValue(bugtask_data)) { |
570 | + return; |
571 | + } |
572 | + var picker_connect = Y.namespace('lp.app.picker.connect'); |
573 | + var process_link = function(link) { |
574 | + // The link may already have been processed. |
575 | + if (link.hasClass('js-action')) { |
576 | + return; |
577 | + } |
578 | + var func_name = link.get('id'); |
579 | + var connect_func = picker_connect[func_name]; |
580 | + if (Y.Lang.isFunction(connect_func)) { |
581 | + connect_func(); |
582 | + } |
583 | + }; |
584 | + var id; |
585 | + for (id in bugtask_data) { |
586 | + if (bugtask_data.hasOwnProperty(id)) { |
587 | + var conf = bugtask_data[id]; |
588 | + // We need to wire the target and assignee pickers in the |
589 | + // expandable bugtask edit form. This setup_bugtask_table() method |
590 | + // is called when the page loads as well as after replacing the |
591 | + // table. On page load, the pickers are wired by javascript |
592 | + // embedded in the picker tales so we need to ensure we handle |
593 | + // this case. |
594 | + var tr = Y.one('#' + conf.form_row_id); |
595 | + if (tr === null) { |
596 | + //The row has been deleted. |
597 | + continue; |
598 | + } |
599 | + tr.all('a').each(process_link); |
600 | + // Now wire up the javascript widgets in the table row. |
601 | + namespace.setup_bugtask_row(conf); |
602 | + } |
603 | + } |
604 | +}; |
605 | + |
606 | +/** |
607 | + * Show a spinner next to the delete icon. |
608 | + * |
609 | + * @method _showDeleteSpinner |
610 | + */ |
611 | +namespace._showDeleteSpinner = function(delete_link) { |
612 | + var spinner_node = Y.Node.create( |
613 | + '<img class="spinner" src="/@@/spinner" alt="Deleting..." />'); |
614 | + delete_link.insertBefore(spinner_node, delete_link); |
615 | + delete_link.addClass('unseen'); |
616 | +}; |
617 | + |
618 | +/** |
619 | + * Hide the delete spinner. |
620 | + * |
621 | + * @method _hideDeleteSpinner |
622 | + */ |
623 | +namespace._hideDeleteSpinner = function(delete_link) { |
624 | + delete_link.removeClass('unseen'); |
625 | + var spinner = delete_link.get('parentNode').one('.spinner'); |
626 | + if (spinner !== null) { |
627 | + spinner.remove(); |
628 | + } |
629 | +}; |
630 | + |
631 | +/** |
632 | + * Replace the currect bugtask table with a new one, ensuring all Javascript |
633 | + * widgets are correctly wired up. |
634 | + * |
635 | + * @method _render_bugtask_table |
636 | + */ |
637 | +namespace._render_bugtask_table = function(new_table) { |
638 | + var bugtask_table = Y.one('#affected-software'); |
639 | + bugtask_table.replace(new_table); |
640 | + namespace.setup_bugtask_table(); |
641 | +}; |
642 | + |
643 | +/** |
644 | + * Prompt the user to confirm the deletion of the selected bugtask. |
645 | + * widgets are correctly wired up. |
646 | + * |
647 | + * @method _confirm_bugtask_delete |
648 | + */ |
649 | +namespace._confirm_bugtask_delete = function(delete_link, conf) { |
650 | + var delete_text_template = [ |
651 | + '<p class="large-warning" style="padding:2px 2px 0 36px;">', |
652 | + ' You are about to mark bug "{bug}"<br>as no longer affecting', |
653 | + ' {target}.<br><br>', |
654 | + ' <strong>Please confirm you really want to do this.</strong>', |
655 | + '</p>' |
656 | + ].join(''); |
657 | + var delete_text = Y.Lang.substitute(delete_text_template, |
658 | + {bug: conf.bug_title, target: conf.targetname}); |
659 | + var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({ |
660 | + submit_fn: function() { |
661 | + namespace.delete_bugtask(delete_link, conf); |
662 | + }, |
663 | + form_content: delete_text, |
664 | + headerContent: '<h2>Confirm bugtask deletion</h2>' |
665 | + }); |
666 | + co.show(); |
667 | +}; |
668 | + |
669 | +/** |
670 | + * Redirect to a new URL. We need to break this out to allow testing. |
671 | + * |
672 | + * @method _redirect |
673 | + */ |
674 | +namespace._redirect = function(url) { |
675 | + window.location.replace(url); |
676 | +}; |
677 | + |
678 | +/** |
679 | + * Process the result of the XHR request to delete a bugtask. |
680 | + * |
681 | + * @method _process_bugtask_delete_response |
682 | + */ |
683 | +namespace._process_bugtask_delete_response = function(response, row_id) { |
684 | + // If the result is json, then we need to perform a redirect to a new |
685 | + // bugtask URL. This happens when the current bugtask is deleted and we |
686 | + // need to ensure all link URLS are correctly reset. |
687 | + var content_type = response.getResponseHeader('Content-type'); |
688 | + if (content_type === 'application/json') { |
689 | + Y.lp.client.display_notifications( |
690 | + response.getResponseHeader('X-Lazr-Notifications')); |
691 | + var redirect = Y.JSON.parse(response.responseText); |
692 | + Y.lp.anim.red_flash({ |
693 | + node: '#' + row_id, |
694 | + duration: namespace.ANIM_DURATION |
695 | + }).run(); |
696 | + namespace._redirect(redirect.bugtask_url); |
697 | + return; |
698 | + } |
699 | + // We have received HTML, so we replace the current bugtask table with a |
700 | + // new one. |
701 | + var anim = Y.lp.anim.red_flash({ |
702 | + node: '#' + row_id, |
703 | + duration: namespace.ANIM_DURATION |
704 | + }); |
705 | + anim.on('end', function() { |
706 | + namespace._render_bugtask_table(response.responseText); |
707 | + Y.lp.client.display_notifications( |
708 | + response.getResponseHeader('X-Lazr-Notifications')); |
709 | + }); |
710 | + anim.run(); |
711 | +}; |
712 | + |
713 | +/** |
714 | + * Delete the bugtask defined by the delete_link using an XHR call. |
715 | + * |
716 | + * @method delete_bugtask |
717 | + */ |
718 | +namespace.delete_bugtask = function (delete_link, conf) { |
719 | + Y.lp.client.remove_notifications(); |
720 | + var error_handler = new Y.lp.client.ErrorHandler(); |
721 | + error_handler.showError = Y.bind(function (error_msg) { |
722 | + namespace._hideDeleteSpinner(delete_link); |
723 | + Y.lp.app.errors.display_error(undefined, error_msg); |
724 | + }, this); |
725 | + |
726 | + var submit_url = delete_link.get('href'); |
727 | + var qs = Y.lp.client.append_qs( |
728 | + '', 'field.actions.delete_bugtask', 'Delete'); |
729 | + var y_config = { |
730 | + method: "POST", |
731 | + headers: {'Accept': 'application/json; application/xhtml'}, |
732 | + on: { |
733 | + start: |
734 | + Y.bind(namespace._showDeleteSpinner, namespace, delete_link), |
735 | + failure: |
736 | + error_handler.getFailureHandler(), |
737 | + success: |
738 | + function(id, response) { |
739 | + namespace._process_bugtask_delete_response( |
740 | + response, conf.row_id); |
741 | + } |
742 | + }, |
743 | + data: qs |
744 | + }; |
745 | + var io_provider = Y.lp.client.get_configured_io_provider(conf); |
746 | + io_provider.io(submit_url, y_config); |
747 | +}; |
748 | + |
749 | +/** |
750 | * Set up a bug task table row. |
751 | * |
752 | * Called once per row, on load, to initialize the page. |
753 | @@ -641,6 +852,15 @@ |
754 | var importance_content = tr.one('.importance-content'); |
755 | var assignee_content = Y.one('#assignee-picker-' + conf.row_id); |
756 | var milestone_content = tr.one('.milestone-content'); |
757 | + var delete_link = tr.one('.bugtask-delete'); |
758 | + |
759 | + if (Y.Lang.isValue(LP.links.me) && Y.Lang.isValue(delete_link) |
760 | + && conf.user_can_delete) { |
761 | + delete_link.on('click', function (e) { |
762 | + e.preventDefault(); |
763 | + namespace._confirm_bugtask_delete(delete_link, conf); |
764 | + }); |
765 | + } |
766 | |
767 | if (status_content === null) { |
768 | // Not all table rows have status widgets. If this is one of those |
769 | @@ -894,6 +1114,19 @@ |
770 | } |
771 | assignee_picker.render(); |
772 | } |
773 | + |
774 | + // Set-up the expander on the bug task summary row. |
775 | + var icon_node = Y.one('tr#' + conf.row_id + ' a.bugtask-expander'); |
776 | + var row_node = Y.one('tr#' + conf.form_row_id); |
777 | + if (Y.Lang.isValue(row_node)) { |
778 | + // When no row is present, this is bug task on a project with |
779 | + // multiple per-series tasks, so we do not need to set |
780 | + // the expander for the descriptive parent project task. |
781 | + var content_node = row_node.one('td form'); |
782 | + var expander = new Y.lp.app.widgets.expander.Expander( |
783 | + icon_node, row_node, { animate_node: content_node }); |
784 | + expander.setUp(); |
785 | + } |
786 | }; |
787 | |
788 | /** |
789 | @@ -1122,7 +1355,8 @@ |
790 | comments_container.appendChild(new_comments_node); |
791 | if (Y.Lang.isValue(Y.lp.anim)) { |
792 | var success_anim = Y.lp.anim.green_flash( |
793 | - {node: new_comments_node}); |
794 | + {node: new_comments_node, |
795 | + duration: namespace.ANIM_DURATION}); |
796 | success_anim.run(); |
797 | } |
798 | batch_url_div = Y.one('#next-batch-url'); |
799 | @@ -1209,6 +1443,6 @@ |
800 | "lazr.formoverlay", "lp.anim", "lazr.base", |
801 | "lazr.overlay", "lazr.choiceedit", "lp.app.picker", |
802 | "lp.bugs.bugtask_index.portlets.subscription", |
803 | - "lp.client", "escape", |
804 | + "lp.app.widgets.expander", "lp.client", "escape", |
805 | "lp.client.plugins", "lp.app.errors", |
806 | - "lp.app.privacy"]}); |
807 | + "lp.app.privacy", "lp.app.confirmationoverlay"]}); |
808 | |
809 | === modified file 'lib/lp/bugs/javascript/subscribers.js' |
810 | --- lib/lp/bugs/javascript/subscribers.js 2011-07-25 04:32:49 +0000 |
811 | +++ lib/lp/bugs/javascript/subscribers.js 2011-11-04 14:06:00 +0000 |
812 | @@ -41,9 +41,14 @@ |
813 | * a relative URI to load subscribers' details from. |
814 | */ |
815 | function createBugSubscribersLoader(config) { |
816 | + var url_data = LP.cache.subscribers_portlet_url_data; |
817 | + if (!Y.Lang.isValue(url_data)) { |
818 | + url_data = { self_link: LP.cache.context.bug_link, |
819 | + web_link: LP.cache.context.web_link }; |
820 | + } |
821 | config.subscriber_levels = subscriber_levels; |
822 | config.subscriber_level_order = subscriber_level_order; |
823 | - config.context = config.bug; |
824 | + config.context = url_data; |
825 | config.subscribe_someone_else_level = 'Discussion'; |
826 | config.default_subscriber_level = 'Maybe'; |
827 | var module = Y.lp.app.subscribers.subscribers_list; |
828 | |
829 | === added file 'lib/lp/bugs/javascript/tests/test_bugtask_delete.html' |
830 | --- lib/lp/bugs/javascript/tests/test_bugtask_delete.html 1970-01-01 00:00:00 +0000 |
831 | +++ lib/lp/bugs/javascript/tests/test_bugtask_delete.html 2011-11-04 14:06:00 +0000 |
832 | @@ -0,0 +1,81 @@ |
833 | +<html> |
834 | + <head> |
835 | + <title>Bug task deletion</title> |
836 | + |
837 | + <!-- YUI and test setup --> |
838 | + <script type="text/javascript" |
839 | + src="../../../../canonical/launchpad/icing/yui/yui/yui.js"> |
840 | + </script> |
841 | + <link rel="stylesheet" href="../../../app/javascript/testing/test.css" /> |
842 | + <script type="text/javascript" |
843 | + src="../../../app/javascript/testing/testrunner.js"></script> |
844 | + |
845 | + <script type="text/javascript" src="../../../app/javascript/client.js"></script> |
846 | + <script type="text/javascript" src="../../../app/javascript/errors.js"></script> |
847 | + <script type="text/javascript" src="../../../app/javascript/lp.js"></script> |
848 | + |
849 | + <!-- Other dependencies --> |
850 | + <script type="text/javascript" src="../../../app/javascript/testing/mockio.js"></script> |
851 | + <script type="text/javascript" |
852 | + src="../../../contrib/javascript/mustache.js"></script> |
853 | + <script type="text/javascript" src="../../../app/javascript/activator/activator.js"></script> |
854 | + <script type="text/javascript" src="../../../app/javascript/anim/anim.js"></script> |
855 | + <script type="text/javascript" src="../../../app/javascript/confirmationoverlay/confirmationoverlay.js"></script> |
856 | + <script type="text/javascript" src="../../../app/javascript/choiceedit/choiceedit.js"></script> |
857 | + <script type="text/javascript" src="../../../app/javascript/effects/effects.js"></script> |
858 | + <script type="text/javascript" src="../../../app/javascript/expander.js"></script> |
859 | + <script type="text/javascript" src="../../../app/javascript/extras/extras.js"></script> |
860 | + <script type="text/javascript" src="../../../app/javascript/formoverlay/formoverlay.js"></script> |
861 | + <script type="text/javascript" src="../../../app/javascript/inlineedit/editor.js"></script> |
862 | + <script type="text/javascript" src="../../../app/javascript/lazr/lazr.js"></script> |
863 | + <script type="text/javascript" src="../../../app/javascript/overlay/overlay.js"></script> |
864 | + <script type="text/javascript" src="../../../app/javascript/picker/picker.js"></script> |
865 | + <script type="text/javascript" src="../../../app/javascript/picker/picker_patcher.js"></script> |
866 | + <script type="text/javascript" src="../../../app/javascript/picker/person_picker.js"></script> |
867 | + <script type="text/javascript" src="../../../app/javascript/privacy.js"></script> |
868 | + <script type="text/javascript" src="../bug_subscription_portlet.js"></script> |
869 | + |
870 | + <!-- The module under test --> |
871 | + <script type="text/javascript" |
872 | + src="../bugtask_index.js"></script> |
873 | + |
874 | + <!-- The test suite --> |
875 | + <script type="text/javascript" |
876 | + src="test_bugtask_delete.js"></script> |
877 | + |
878 | + <!-- Pretty up the sample html --> |
879 | + <style type="text/css"> |
880 | + div#sample {margin:15px; width:200px; border:1px solid #999; padding:10px;} |
881 | + </style> |
882 | + </head> |
883 | + <body class="yui3-skin-sam"> |
884 | + <div id="fixture"></div> |
885 | + <div id="request-notifications"></div> |
886 | + <script type="text/x-template" id="form-template"> |
887 | + <table id="affected-software"> |
888 | + <tbody> |
889 | + <tr id="tasksummary49"> |
890 | + <td> |
891 | + <div> |
892 | + <a class="sprite product" href="#">Product</a> |
893 | + <button class="lazr-btn yui3-activator-act"> |
894 | + Edit |
895 | + </button> |
896 | + <a id="bugtask-delete-task49" href="http://foo" |
897 | + class="sprite remove bugtask-delete"></a> |
898 | + </div> |
899 | + </td> |
900 | + <td> |
901 | + <table> |
902 | + <tr id="task49"><td> |
903 | + <a id="show-widget-product" class="sprite product" |
904 | + href="#">Product</a> |
905 | + </td></tr> |
906 | + </table> |
907 | + </td> |
908 | + </tr> |
909 | + </tbody> |
910 | + </table> |
911 | + </script> |
912 | + </body> |
913 | +</html> |
914 | |
915 | === added file 'lib/lp/bugs/javascript/tests/test_bugtask_delete.js' |
916 | --- lib/lp/bugs/javascript/tests/test_bugtask_delete.js 1970-01-01 00:00:00 +0000 |
917 | +++ lib/lp/bugs/javascript/tests/test_bugtask_delete.js 2011-11-04 14:06:00 +0000 |
918 | @@ -0,0 +1,223 @@ |
919 | +YUI().use('lp.testing.runner', 'lp.testing.mockio', 'base', 'test', 'console', |
920 | + 'node', 'node-event-simulate', 'lp.bugs.bugtask_index', |
921 | + function(Y) { |
922 | + |
923 | +var suite = new Y.Test.Suite("Bugtask deletion Tests"); |
924 | +var module = Y.lp.bugs.bugtask_index; |
925 | + |
926 | + |
927 | +suite.add(new Y.Test.Case({ |
928 | + name: 'Bugtask delete', |
929 | + |
930 | + setUp: function() { |
931 | + module.ANIM_DURATION = 0; |
932 | + this.link_conf = { |
933 | + row_id: 'tasksummary49', |
934 | + form_row_id: 'tasksummary49', |
935 | + user_can_delete: true |
936 | + }; |
937 | + window.LP = { |
938 | + links: {me : "/~user"}, |
939 | + cache: { |
940 | + bugtask_data: {49: this.link_conf} |
941 | + } |
942 | + }; |
943 | + this.fixture = Y.one('#fixture'); |
944 | + var bugtasks_table = Y.Node.create( |
945 | + Y.one('#form-template').getContent()); |
946 | + this.fixture.appendChild(bugtasks_table); |
947 | + this.delete_link = bugtasks_table.one('#bugtask-delete-task49'); |
948 | + }, |
949 | + |
950 | + tearDown: function() { |
951 | + if (this.fixture !== null) { |
952 | + this.fixture.empty(); |
953 | + } |
954 | + Y.one('#request-notifications').empty(); |
955 | + delete this.fixture; |
956 | + delete window.LP; |
957 | + }, |
958 | + |
959 | + test_show_spinner: function() { |
960 | + // Test the delete progress spinner is shown. |
961 | + module._showDeleteSpinner(this.delete_link); |
962 | + Y.Assert.isNotNull(this.fixture.one('.spinner')); |
963 | + Y.Assert.isTrue(this.delete_link.hasClass('unseen')); |
964 | + }, |
965 | + |
966 | + test_hide_spinner: function() { |
967 | + // Test the delete progress spinner is hidden. |
968 | + module._showDeleteSpinner(this.delete_link); |
969 | + module._hideDeleteSpinner(this.delete_link); |
970 | + Y.Assert.isNull(this.fixture.one('.spinner')); |
971 | + Y.Assert.isFalse(this.delete_link.hasClass('unseen')); |
972 | + }, |
973 | + |
974 | + _test_delete_confirmation: function(click_ok) { |
975 | + // Test the delete confirmation dialog when delete is clicked. |
976 | + var orig_delete_bugtask = module.delete_bugtask; |
977 | + |
978 | + var delete_called = false; |
979 | + var self = this; |
980 | + module.delete_bugtask = function(delete_link, conf) { |
981 | + Y.Assert.areEqual(self.delete_link, delete_link); |
982 | + Y.Assert.areEqual(self.link_conf, conf); |
983 | + delete_called = true; |
984 | + }; |
985 | + module.setup_bugtask_table(); |
986 | + this.delete_link.simulate('click'); |
987 | + var co = Y.one('.yui3-overlay.yui3-lp-app-confirmationoverlay'); |
988 | + var actions = co.one('.yui3-lazr-formoverlay-actions'); |
989 | + var btn_style; |
990 | + if (click_ok) { |
991 | + btn_style = '.ok-btn'; |
992 | + } else { |
993 | + btn_style = '.cancel-btn'; |
994 | + } |
995 | + var button = actions.one(btn_style); |
996 | + button.simulate('click'); |
997 | + Y.Assert.areEqual(click_ok, delete_called); |
998 | + Y.Assert.isTrue( |
999 | + co.hasClass('yui3-lp-app-confirmationoverlay-hidden')); |
1000 | + module.delete_bugtask = orig_delete_bugtask; |
1001 | + }, |
1002 | + |
1003 | + test_delete_confirmation_ok: function() { |
1004 | + // Test the delete confirmation dialog Ok functionality. |
1005 | + this._test_delete_confirmation(true); |
1006 | + }, |
1007 | + |
1008 | + test_delete_confirmation_cancel: function() { |
1009 | + // Test the delete confirmation dialog Cancel functionality. |
1010 | + this._test_delete_confirmation(false); |
1011 | + }, |
1012 | + |
1013 | + test_setup_bugtask_table: function() { |
1014 | + // Test that the bugtask table is wired up, the pickers and the |
1015 | + // delete links etc. |
1016 | + var namespace = Y.namespace('lp.app.picker.connect'); |
1017 | + var connect_picker_called = false; |
1018 | + namespace['show-widget-product'] = function() { |
1019 | + connect_picker_called = true; |
1020 | + }; |
1021 | + var orig_confirm_bugtask_delete = module._confirm_bugtask_delete; |
1022 | + var self = this; |
1023 | + var confirm_delete_called = false; |
1024 | + module._confirm_bugtask_delete = function(delete_link, conf) { |
1025 | + Y.Assert.areEqual(self.delete_link, delete_link); |
1026 | + Y.Assert.areEqual(self.link_conf, conf); |
1027 | + confirm_delete_called = true; |
1028 | + }; |
1029 | + module.setup_bugtask_table(); |
1030 | + this.delete_link.simulate('click'); |
1031 | + Y.Assert.isTrue(connect_picker_called); |
1032 | + Y.Assert.isTrue(confirm_delete_called); |
1033 | + module._confirm_bugtask_delete = orig_confirm_bugtask_delete; |
1034 | + }, |
1035 | + |
1036 | + test_render_bugtask_table: function() { |
1037 | + // Test that a new bug task table is rendered and setup. |
1038 | + var orig_setup_bugtask_table = module.setup_bugtask_table; |
1039 | + var setup_called = false; |
1040 | + module.setup_bugtask_table = function() { |
1041 | + setup_called = true; |
1042 | + }; |
1043 | + var test_table = |
1044 | + '<table id="affected-software">'+ |
1045 | + '<tr><td>foo</td></tr></table>'; |
1046 | + module._render_bugtask_table(test_table); |
1047 | + Y.Assert.isTrue(setup_called); |
1048 | + Y.Assert.areEqual( |
1049 | + '<tbody><tr><td>foo</td></tr></tbody>', |
1050 | + this.fixture.one('table#affected-software').getContent()); |
1051 | + module.setup_bugtask_table = orig_setup_bugtask_table; |
1052 | + }, |
1053 | + |
1054 | + test_process_bugtask_delete_redirect_response: function() { |
1055 | + // Test the processing of a XHR delete result which is to |
1056 | + // redirect the browser to a new URL. |
1057 | + var orig_redirect = module._redirect; |
1058 | + var redirect_called = false; |
1059 | + module._redirect = function(url) { |
1060 | + Y.Assert.areEqual('http://foo', url); |
1061 | + redirect_called = true; |
1062 | + }; |
1063 | + var response = new Y.lp.testing.mockio.MockHttpResponse({ |
1064 | + responseText: '{"bugtask_url": "http://foo"}', |
1065 | + responseHeaders: {'Content-type': 'application/json'}}); |
1066 | + module._process_bugtask_delete_response( |
1067 | + response, this.link_conf.row_id); |
1068 | + this.wait(function() { |
1069 | + // Wait for the animation to complete. |
1070 | + Y.Assert.isTrue(redirect_called); |
1071 | + }, 50); |
1072 | + module._redirect = orig_redirect; |
1073 | + }, |
1074 | + |
1075 | + test_process_bugtask_delete_new_table_response: function() { |
1076 | + // Test the processing of a XHR delete result which is to |
1077 | + // replace the current bugtasks table. |
1078 | + var orig_render_bugtask_table = module._render_bugtask_table; |
1079 | + var render_table_called = false; |
1080 | + module._render_bugtask_table = function(new_table) { |
1081 | + Y.Assert.areEqual('<table>Foo</table>', new_table); |
1082 | + render_table_called = true; |
1083 | + }; |
1084 | + var notifications = '[ [20, "Delete Success"] ]'; |
1085 | + var response = new Y.lp.testing.mockio.MockHttpResponse({ |
1086 | + responseText: '<table>Foo</table>', |
1087 | + responseHeaders: { |
1088 | + 'Content-type': 'text/html', |
1089 | + 'X-Lazr-Notifications': notifications}}); |
1090 | + module._process_bugtask_delete_response( |
1091 | + response, this.link_conf.row_id); |
1092 | + this.wait(function() { |
1093 | + // Wait for the animation to complete. |
1094 | + Y.Assert.isTrue(render_table_called); |
1095 | + var node = Y.one('div#request-notifications ' + |
1096 | + 'div.informational.message'); |
1097 | + Y.Assert.areEqual('Delete Success', node.getContent()); |
1098 | + }, 50); |
1099 | + module._render_bugtask_table = orig_render_bugtask_table; |
1100 | + }, |
1101 | + |
1102 | + test_delete_bugtask: function() { |
1103 | + // Test that when delete_bugtask is called, the expected XHR call |
1104 | + // is made. |
1105 | + var orig_delete_repsonse = |
1106 | + module._process_bugtask_delete_response; |
1107 | + |
1108 | + var delete_response_called = false; |
1109 | + var self = this; |
1110 | + module._process_bugtask_delete_response = function(response, id) { |
1111 | + Y.Assert.areEqual('<p>Foo</p>', response.responseText); |
1112 | + Y.Assert.areEqual(self.link_conf.row_id, id); |
1113 | + delete_response_called = true; |
1114 | + }; |
1115 | + |
1116 | + var mockio = new Y.lp.testing.mockio.MockIo(); |
1117 | + var conf = Y.merge(this.link_conf, {io_provider: mockio}); |
1118 | + module.delete_bugtask(this.delete_link, conf); |
1119 | + mockio.success({ |
1120 | + responseText: '<p>Foo</p>', |
1121 | + responseHeaders: {'Content-Type': 'text/html'}}); |
1122 | + // Check the parameters passed to the io call. |
1123 | + Y.Assert.areEqual( |
1124 | + this.delete_link.get('href'), |
1125 | + mockio.last_request.url); |
1126 | + Y.Assert.areEqual( |
1127 | + 'POST', mockio.last_request.config.method); |
1128 | + Y.Assert.areEqual( |
1129 | + 'application/json; application/xhtml', |
1130 | + mockio.last_request.config.headers.Accept); |
1131 | + Y.Assert.areEqual( |
1132 | + 'field.actions.delete_bugtask=Delete', |
1133 | + mockio.last_request.config.data); |
1134 | + Y.Assert.isTrue(delete_response_called); |
1135 | + |
1136 | + module._process_bugtask_delete_response = orig_delete_repsonse; |
1137 | + } |
1138 | +})); |
1139 | + |
1140 | +Y.lp.testing.Runner.run(suite); |
1141 | +}); |
1142 | |
1143 | === modified file 'lib/lp/bugs/javascript/tests/test_subscribers.js' |
1144 | --- lib/lp/bugs/javascript/tests/test_subscribers.js 2011-07-25 04:32:49 +0000 |
1145 | +++ lib/lp/bugs/javascript/tests/test_subscribers.js 2011-11-04 14:06:00 +0000 |
1146 | @@ -12,19 +12,26 @@ |
1147 | setUp: function() { |
1148 | this.root = Y.Node.create('<div />'); |
1149 | Y.one('body').appendChild(this.root); |
1150 | + window.LP = { |
1151 | + cache: { |
1152 | + context: { |
1153 | + bug_link: '/bug/1', |
1154 | + web_link: '/base' |
1155 | + } |
1156 | + } |
1157 | + }; |
1158 | }, |
1159 | |
1160 | tearDown: function() { |
1161 | this.root.remove(); |
1162 | + delete window.LP; |
1163 | }, |
1164 | |
1165 | setUpLoader: function() { |
1166 | this.root.appendChild( |
1167 | Y.Node.create('<div />').addClass('container')); |
1168 | - var bug = { web_link: '/base', self_link: '/bug/1'}; |
1169 | return new module.createBugSubscribersLoader({ |
1170 | container_box: '.container', |
1171 | - bug: bug, |
1172 | subscribers_details_view: '/+bug-portlet-subscribers-details'}); |
1173 | }, |
1174 | |
1175 | |
1176 | === modified file 'lib/lp/bugs/stories/bugs/xx-bug-index.txt' |
1177 | --- lib/lp/bugs/stories/bugs/xx-bug-index.txt 2011-07-01 15:12:32 +0000 |
1178 | +++ lib/lp/bugs/stories/bugs/xx-bug-index.txt 2011-11-04 14:06:00 +0000 |
1179 | @@ -147,11 +147,11 @@ |
1180 | >>> bug_url = canonical_url(bug) |
1181 | >>> logout() |
1182 | |
1183 | -On the bug page, for every bug task there's one coresponding init script. |
1184 | - |
1185 | +On the bug page, for every bug task there's one expander. |
1186 | + |
1187 | >>> browser.open(bug_url) |
1188 | >>> print len(find_tags_by_class( |
1189 | - ... browser.contents, 'bugtasks-table-row-init-script')) |
1190 | + ... browser.contents, 'bugtask-expander')) |
1191 | 1 |
1192 | |
1193 | We add four more tasks. |
1194 | @@ -161,14 +161,14 @@ |
1195 | ... _ = bug.addTask(bug.owner, factory.makeProduct()) |
1196 | >>> logout() |
1197 | |
1198 | -And the init script appears five times. |
1199 | +And the expander appears five times. |
1200 | |
1201 | >>> browser.open(bug_url) |
1202 | >>> print len(find_tags_by_class( |
1203 | - ... browser.contents, 'bugtasks-table-row-init-script')) |
1204 | + ... browser.contents, 'bugtask-expander')) |
1205 | 5 |
1206 | |
1207 | -But on pages with more than ten bug tasks, we don't include the init scripts |
1208 | +But on pages with more than ten bug tasks, we don't include the expander |
1209 | at all. |
1210 | |
1211 | >>> login('foo.bar@canonical.com') |
1212 | @@ -178,6 +178,6 @@ |
1213 | |
1214 | >>> browser.open(bug_url) |
1215 | >>> print len(find_tags_by_class( |
1216 | - ... browser.contents, 'bugtasks-table-row-init-script')) |
1217 | + ... browser.contents, 'bugtask-expander')) |
1218 | 0 |
1219 | |
1220 | |
1221 | === modified file 'lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt' |
1222 | --- lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt 2011-07-26 06:22:41 +0000 |
1223 | +++ lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt 2011-11-04 14:06:00 +0000 |
1224 | @@ -26,9 +26,13 @@ |
1225 | http://bugs.launchpad.dev/firefox/+bug/1/+editstatus |
1226 | |
1227 | >>> print admin_browser.getLink('Confirmed').url |
1228 | -http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/1/+editstatus |
1229 | +Traceback (most recent call last): |
1230 | +... |
1231 | +LinkNotFoundError |
1232 | >>> print admin_browser.getLink('Low', index=1).url |
1233 | -http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/1/+editstatus |
1234 | +Traceback (most recent call last): |
1235 | +... |
1236 | +LinkNotFoundError |
1237 | |
1238 | >>> print admin_browser.getLink('New', index=1).url |
1239 | http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/1/+editstatus |
1240 | |
1241 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' |
1242 | --- lib/lp/bugs/templates/bugtask-index.pt 2011-10-06 18:51:36 +0000 |
1243 | +++ lib/lp/bugs/templates/bugtask-index.pt 2011-11-04 14:06:00 +0000 |
1244 | @@ -20,13 +20,11 @@ |
1245 | Y.lp.code.branchmergeproposal.diff.connect_diff_links(); |
1246 | }, window); |
1247 | Y.on('domready', function() { |
1248 | - var bug = { self_link: LP.cache.context.bug_link, |
1249 | - web_link: LP.cache.context.web_link }; |
1250 | + Y.lp.bugs.bugtask_index.setup_bugtask_table(); |
1251 | LP.cache.comment_context = LP.cache.bug; |
1252 | Y.lp.comments.hide.setup_hide_controls(); |
1253 | var sl = new Y.lp.bugs.subscribers.createBugSubscribersLoader({ |
1254 | container_box: '#other-bug-subscribers', |
1255 | - bug: bug, |
1256 | subscribers_details_view: |
1257 | '/+bug-portlet-subscribers-details', |
1258 | subscribe_someone_else_link: '.menu-link-addsubscriber' |
1259 | @@ -138,8 +136,7 @@ |
1260 | <tal:heat replace="structure view/bug_heat_html" /> |
1261 | </div> |
1262 | |
1263 | - <div tal:replace="structure context/bug/@@+bugtasks-and-nominations-table" /> |
1264 | - |
1265 | + <div tal:replace="structure context/bug/@@+bugtasks-and-nominations-portal" /> |
1266 | <div id="maincontentsub"> |
1267 | <div><!-- id="nonportlets"> --> |
1268 | <div class="top-portlet"> |
1269 | |
1270 | === modified file 'lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt' |
1271 | --- lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2011-11-04 14:05:57 +0000 |
1272 | +++ lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2011-11-04 14:06:00 +0000 |
1273 | @@ -62,15 +62,21 @@ |
1274 | <div class="status-content" |
1275 | style="width: 100%; float: left" |
1276 | tal:define="status context/status"> |
1277 | - <a href="+editstatus" |
1278 | - tal:attributes="class string:value status${status/name}; |
1279 | - href data/edit_link" |
1280 | - style="float: left" |
1281 | - tal:content="status/title" /> |
1282 | - <a href="+editstatus" style="margin-left: 3px" |
1283 | - tal:attributes="href data/edit_link"> |
1284 | - <img class="editicon" src="/@@/edit" /> |
1285 | - </a> |
1286 | + <span tal:condition="not: data/user_can_edit_status" |
1287 | + style="float: left" |
1288 | + tal:attributes="class string:value status${status/name};" |
1289 | + tal:content="status/title"/> |
1290 | + <tal:edit-status tal:condition="data/user_can_edit_status"> |
1291 | + <a href="+editstatus" |
1292 | + tal:attributes="class string:value status${status/name}; |
1293 | + href data/edit_link" |
1294 | + style="float: left" |
1295 | + tal:content="status/title" /> |
1296 | + <a href="+editstatus" style="margin-left: 3px" |
1297 | + tal:attributes="href data/edit_link"> |
1298 | + <img class="editicon" src="/@@/edit" /> |
1299 | + </a> |
1300 | + </tal:edit-status> |
1301 | </div> |
1302 | </td> |
1303 | |
1304 | @@ -177,30 +183,6 @@ |
1305 | |
1306 | </tal:not-conjoined-task> |
1307 | </tr> |
1308 | - <script type="text/javascript" |
1309 | - class="bugtasks-table-row-init-script" |
1310 | - tal:condition="not:view/many_bugtasks" |
1311 | - tal:content="string: |
1312 | - LPS.use('event', 'lp.bugs.bugtask_index', 'lp.app.widgets.expander', |
1313 | - function(Y) { |
1314 | - Y.on('domready', function() { |
1315 | - Y.lp.bugs.bugtask_index.setup_bugtask_row(${view/js_config}); |
1316 | - |
1317 | - // Set-up the expander on the bug task summary row. |
1318 | - var icon_node = Y.one('tr#${data/row_id} a.bugtask-expander'); |
1319 | - var row_node = Y.one('tr#${data/form_row_id}'); |
1320 | - if (Y.Lang.isValue(row_node)) { |
1321 | - // When no row is present, this is bug task on a project with |
1322 | - // multiple per-series tasks, so we do not need to set |
1323 | - // the expander for the descriptive parent project task. |
1324 | - var content_node = row_node.one('td form'); |
1325 | - var expander = new Y.lp.app.widgets.expander.Expander( |
1326 | - icon_node, row_node, { animate_node: content_node }); |
1327 | - expander.setUp(); |
1328 | - } |
1329 | - }); |
1330 | - }); |
1331 | - "/> |
1332 | |
1333 | <tal:form condition="view/displayEditForm"> |
1334 | <tr |
1335 | |
1336 | === renamed file 'lib/lp/bugs/templates/bugtasks-and-nominations-table.pt' => 'lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt' |
1337 | --- lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2011-10-26 03:58:48 +0000 |
1338 | +++ lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt 2011-11-04 14:06:00 +0000 |
1339 | @@ -59,28 +59,7 @@ |
1340 | </tal:not-editable> |
1341 | </tal:affects-me-too> |
1342 | |
1343 | -<table |
1344 | - id="affected-software" |
1345 | - tal:attributes="class python: context.duplicateof and 'duplicate listing' or 'listing'" |
1346 | -> |
1347 | - <thead> |
1348 | - <tr> |
1349 | - <th colspan="2">Affects</th> |
1350 | - <th>Status</th> |
1351 | - <th>Importance</th> |
1352 | - <th>Assigned to</th> |
1353 | - <th>Milestone</th> |
1354 | - </tr> |
1355 | - </thead> |
1356 | - |
1357 | - <tbody> |
1358 | - <tal:bugtask-or-nomination |
1359 | - repeat="task_or_nom_view view/getBugTaskAndNominationViews"> |
1360 | - <tal:block replace="structure task_or_nom_view" /> |
1361 | - </tal:bugtask-or-nomination> |
1362 | - </tbody> |
1363 | - |
1364 | -</table> |
1365 | +<tal:bugtask_table replace="structure context/@@+bugtasks-and-nominations-table" /> |
1366 | |
1367 | <div class="actions" |
1368 | tal:define="current_bugtask view/current_bugtask" |
1369 | |
1370 | === added file 'lib/lp/bugs/templates/bugtasks-and-nominations-table.pt' |
1371 | --- lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 1970-01-01 00:00:00 +0000 |
1372 | +++ lib/lp/bugs/templates/bugtasks-and-nominations-table.pt 2011-11-04 14:06:00 +0000 |
1373 | @@ -0,0 +1,27 @@ |
1374 | +<tal:root |
1375 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
1376 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
1377 | + omit-tag=""> |
1378 | + <table |
1379 | + id="affected-software" |
1380 | + tal:attributes="class python: context.duplicateof and 'duplicate listing' or 'listing'" |
1381 | + > |
1382 | + <thead> |
1383 | + <tr> |
1384 | + <th colspan="2">Affects</th> |
1385 | + <th>Status</th> |
1386 | + <th>Importance</th> |
1387 | + <th>Assigned to</th> |
1388 | + <th>Milestone</th> |
1389 | + </tr> |
1390 | + </thead> |
1391 | + |
1392 | + <tbody> |
1393 | + <tal:bugtask-or-nomination |
1394 | + repeat="task_or_nom_view view/getBugTaskAndNominationViews"> |
1395 | + <tal:block replace="structure task_or_nom_view" /> |
1396 | + </tal:bugtask-or-nomination> |
1397 | + </tbody> |
1398 | + |
1399 | + </table> |
1400 | +</tal:root> |
Hi Ian,
Excellent work. Thanks for the detailed MP and all the comments you put in the code, I really appreciate it.
A few remarks/ suggestions. I suppose you're quite eager to land this so please take the nitpicks (especially the ones about indentation) with a grain of salt ;).
[0]
1249 lines. Please have mercy on us poor reviewers ;). I know Javascript is verbose and you've done a lot of refactoring but still, I'm sure you would have had a volunteer reviewer for this branch sooner if you had split it into 2 or 3. Also, the problem with enormous branches is that the odds of getting a conflict goes way up.
> 3. Bug found rtance( ) found. One was essentially: userCanEditImpo rtance( self.user) and not bugtask.bugwatch
> There were conflicting implementations of userCanEditImpo
> bugtask.
For instance you could have fixed this in another pipe (I don't know if you already use the bzr pipeline plugin but in case you don't, please check it out: http:// wiki.bazaar. canonical. com/BzrPipeline).
[1]
294 + """ Test that the client cache contains the expected data.
295 +
296 + The cache data is used by the Javascript to enable the delete
297 + links to work as expected.
298 + """
Weird indentation for the second """.
[2]
312 + def check_bugtask_ data(bugtask, can_delete): bugtask. id, all_bugtask_data)
313 + self.assertIn(
Don't you think it would be easier to read if you moved this method outside of the test method?
[3]
361 + self.assertEqual( response. getHeader( 'content- type'),
362 + view.request.
363 + 'application/json')
'application/json' should be the first argument, otherwise a failure will be difficult to interpret.
[4]
334 + bug = self.factory. makeBug( ) makeBugTask( bug=bug) bugtargetdispla yname url(bugtask, rootsite='bugs')
335 + bugtask = self.factory.
336 + target_name = bugtask.
337 + bugtask_url = canonical_
And
370 + bug = self.factory. makeBug( ) makeBugTask( bug=bug) bugtargetdispla yname bugtask, rootsite='bugs')
371 + bugtask = self.factory.
372 + target_name = bugtask.
373 + default_bugtask_url = canonical_url(
374 + bug.default_
Would you mind refactoring that code into a common setup method?
[5]
517 + if( bugtask_ data.hasOwnProp erty(id) ) {
small typos: if( bugtask_ data.hasOwnProp erty(id) ) { → if (bugtask_ data.hasOwnProp erty(id) ) {
Same here:
532 + if( link.hasClass( 'js-action' ) ) { isFunction( connect_ func)) {
537 + if( Y.Lang.
629 + if( content_type === 'application/json' ) {
[6]
591 + var delete_text = [ large-warning" style="padding:2px 2px 0 36px;">', br><strong> Please confirm you really want to do this.',
592 + '<p class="
593 + 'You are about to mark bug "',
594 + conf.bug_title,
595 + '"<br>as no longer affecting ',
596 + conf.targetname,
597 + '.<br><
598 + '</strong></p>'
599 + ].join('');
Well, I do that all the times so I won't blame you but you will admit this is not very readable and someone editing this code might easily introduce a bug…
a) mu...