Merge lp:~wallyworld/launchpad/delete-bugtask-ui-ajax-878909 into lp:launchpad

Proposed by Ian Booth
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
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+80779@code.launchpad.net

Commit message

[r=rvb][bug=878909] Add ajax support for deleting bug tasks.

Description of the change

== Implementation ==

Requires feature flag: disclosure.delete_bugtask.enabled

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-and-nominations-table was renamed to +bugtasks-and-nominations-portal since it renders Affects Me Too and Also Affacts links as well as the table. The +bugtasks-and-nominations-table view renders just the bugtasks table. The tales used for the portal references the newly created table view, allowing the tales to be reused.

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-tasks-nominations-and-table-row.pt tales was moved into this existing method. The BugTaskTableRowView view had a js_config method which provided data for the javascript embedded in the tales. This was renamed to bugtask_config and was re-purposed to stuff data into the client cache, allowing the moved javascript to get at the data it needs.

An onload handler was added to execute a new setup_bugtask_table() method. This uses the client cache data and the newly refactored javascript to wire up the bug tasks table.

When the new bug tasks table is rendered after the XHR delete call, the same setup_bugtask_table() is called to do the wiring.

3. Bug found

There were conflicting implementations of userCanEditImportance() found. One was essentially:

bugtask.userCanEditImportance(self.user) and not bugtask.bugwatch

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-macro.pt tales. So it's not feasible to rip this out and reuse as per item #2 above since this picker stuff is generic infrastructure and things would break. So I chose the following solution. The javascript function to do the picker wiring is registered in a new yui namespace "lp.app.picker.connect". The function is still run as per the current picker infrastructure but the function is also available to run later as and when required. So when the html for the new table is rendered, the picker widgets embedded in the table are re-wired using the previously registered functions.

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 ReturnToReferrerMixin class was used to determine the referring URL and hence whether the bugtask being deleted is the current one and hence whether redirection is required. There is a slight pause when the redirect happens but I'm not sure I can do much about that.

== Demo and QA ==

http://people.canonical.com/~ianb/delete_bugtask.ogv

== Tests ==

1. Renamed +bugtasks-and-nominations-table view

Add extra test to bug-views.txt and update existing tests to use the new view name.

2. New setup for bugs.javascript.subscribers.js

Update test_subscribers.js

3. New DeleteBugTaskView behaviour

Add new tests to TestDeleteBugTaskView to check the ajax behaviour:
- test_ajax_delete_non_current_bugtask
- test_ajax_delete_current_bugtask

4. New Javascript bugtask delete functionality

Add new yui test: bugs.javascript.tests.test_bugtask_delete.js

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/app/widgets/templates/form-picker-macros.pt
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/browser/configure.zcml
  lib/lp/bugs/browser/tests/bug-views.txt
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/bugs/javascript/bugtask_index.js
  lib/lp/bugs/javascript/subscribers.js
  lib/lp/bugs/javascript/tests/test_bugtask_delete.html
  lib/lp/bugs/javascript/tests/test_bugtask_delete.js
  lib/lp/bugs/javascript/tests/test_subscribers.js
  lib/lp/bugs/templates/bugtask-index.pt
  lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt
  lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt
  lib/lp/bugs/templates/bugtasks-and-nominations-table.pt

./lib/lp/bugs/javascript/bugtask_index.js
     640: Move 'var' declarations to the top of the function.
     640: Stopping. (46% scanned).
      -1: JSLINT had a fatal error.

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :
Download full text (7.5 KiB)

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
> There were conflicting implementations of userCanEditImportance() found. One was essentially:
> bugtask.userCanEditImportance(self.user) and not bugtask.bugwatch

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):
313 + self.assertIn(bugtask.id, all_bugtask_data)

Don't you think it would be easier to read if you moved this method outside of the test method?

[3]

361 + self.assertEqual(
362 + view.request.response.getHeader('content-type'),
363 + 'application/json')

'application/json' should be the first argument, otherwise a failure will be difficult to interpret.

[4]

334 + bug = self.factory.makeBug()
335 + bugtask = self.factory.makeBugTask(bug=bug)
336 + target_name = bugtask.bugtargetdisplayname
337 + bugtask_url = canonical_url(bugtask, rootsite='bugs')

And

370 + bug = self.factory.makeBug()
371 + bugtask = self.factory.makeBugTask(bug=bug)
372 + target_name = bugtask.bugtargetdisplayname
373 + default_bugtask_url = canonical_url(
374 + bug.default_bugtask, rootsite='bugs')

Would you mind refactoring that code into a common setup method?

[5]

517 + if( bugtask_data.hasOwnProperty(id) ) {

small typos: if( bugtask_data.hasOwnProperty(id) ) { → if (bugtask_data.hasOwnProperty(id)) {

Same here:

532 + if( link.hasClass('js-action') ) {
537 + if( Y.Lang.isFunction(connect_func)) {
629 + if( content_type === 'application/json' ) {

[6]

591 + var delete_text = [
592 + '<p class="large-warning" style="padding:2px 2px 0 36px;">',
593 + 'You are about to mark bug "',
594 + conf.bug_title,
595 + '"<br>as no longer affecting ',
596 + conf.targetname,
597 + '.<br><br><strong>Please confirm you really want to do this.',
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...

Read more...

review: Approve
Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (6.0 KiB)

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 userCanEditImportance() found. One was essentially:
>> bugtask.userCanEditImportance(self.user) and not bugtask.bugwatch
>
> 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).
>

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_data(bugtask, can_delete):
> 313 + self.assertIn(bugtask.id, all_bugtask_data)
>
> 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.response.getHeader('content-type'),
> 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.makeBug()
> 335 + bugtask = self.factory.makeBugTask(bug=bug)
> 336 + target_name = bugtask.bugtargetdisplayname
> 337 + bugtask_url = canonical_url(bugtask, rootsite='bugs')
>
> And
>
> 370 + bug = self.factory.makeBug()
> 371 + bugtask = self.factory.makeBugTask(bug=bug)
> 372 + target_name = bugtask.bugtargetdisplayname
> 373 + default_bugtask_url = canonical_url(
> 374 + bug.default_bugtask, rootsite='bugs')
>
> 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_data.hasOwnProperty(id) ) {
>
> small typos: if( bugtask_data.hasOwnProperty(id) ) { → if (bugtask_data.hasOwnProperty(id)) {
>
> Same here:
>
> 532 + if( link.hasClass('js-action') ) {
> 537 + if( Y.Lang.isFunction(connect_func)) {
> 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...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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&hellip;');
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>