Merge lp://staging/~bcsaller/juju-gui/service-click-actions-begone into lp://staging/~bcsaller/juju-gui/viewmodel-improvements

Reviewers: mp+145866_code.launchpad.net,

Message:
Please take a look.

Description:
Remove service click actions

This branch builds on the view model improvements to remove service
click
actions, simplify the calling convention of methods that lived there and

remove their dependencies on the service_click_actions closure.

https://code.launchpad.net/~bcsaller/juju-gui/service-click-actions-begone/+merge/145866

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7228070/

Affected files:
   A [revision details]
   M app/views/topology/service.js
   M test/test_application_notifications.js
   M test/test_environment_view.js
   M test/test_service_module.js
   M undocumented

Please take a look.

https://codereview.appspot.com/7228070/

Land as is.

See also some comments below.

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode94
app/views/topology/service.js:94: // with this module as self and a box
as its argument.
Why is it called "d" if it's a box?

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode195
app/views/topology/service.js:195: self.showService(box);
And here we have both a "d" and a "box". Confusing.

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode203
app/views/topology/service.js:203: destroyServiceClick: function(data,
self) {
Oh, so "d" means "data".

https://codereview.appspot.com/7228070/diff/3001/app/views/topology/service.js#newcode906
app/views/topology/service.js:906: showService: function(d) {
The parameter is intended to be called "box", according to the comment
block right above, and it's probably a good idea.

https://codereview.appspot.com/7228070/diff/3001/test/test_application_notifications.js
File test/test_application_notifications.js (right):

https://codereview.appspot.com/7228070/diff/3001/test/test_application_notifications.js#newcode240
test/test_application_notifications.js:240: // The _destroyCallback args
are: service, view, confirm button, event.
Is this comment still valid?

https://codereview.appspot.com/7228070/

Please take a look.

https://codereview.appspot.com/7228070/

Two things, one comment in code.

Also encountered: attempting to destroy a service leads to the following
in juju: http://pastebin.ubuntu.com/1597784/ The WS then disconnects and
reconnects, and the dialog is left hanging out in the GUI. Confirmed
that this works in my branch, which I've yet to merge with anything.

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js#newcode342
app/views/topology/service.js:342: box.inDrag = 2;
Line above is still using service_click_actions, throwing an exception
when dragging a service (if it's not caught, relation lines don't move
until annotations come back and things are redrawn).

https://codereview.appspot.com/7228070/

On 2013/02/01 16:41:54, matthew.scott wrote:
> Two things, one comment in code.

> Also encountered: attempting to destroy a service leads to the
following in
> juju: http://pastebin.ubuntu.com/1597784/ The WS then disconnects and
> reconnects, and the dialog is left hanging out in the GUI. Confirmed
that this
> works in my branch, which I've yet to merge with anything.

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js
> File app/views/topology/service.js (right):

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js#newcode342
> app/views/topology/service.js:342: box.inDrag = 2;
> Line above is still using service_click_actions, throwing an exception
when
> dragging a service (if it's not caught, relation lines don't move
until
> annotations come back and things are redrawn).

I wasn't able to reproduce the destroy service issue. I'll resubmit this
with the other fix from the poor merge.

https://codereview.appspot.com/7228070/

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7228070/diff/9001/app/views/topology/service.js#newcode342
app/views/topology/service.js:342: box.inDrag = 2;
On 2013/02/01 16:41:54, matthew.scott wrote:
> Line above is still using service_click_actions, throwing an exception
when
> dragging a service (if it's not caught, relation lines don't move
until
> annotations come back and things are redrawn).

ah, careless merge, thanks

https://codereview.appspot.com/7228070/

Please take a look.

https://codereview.appspot.com/7228070/

Land as is, thank you!

https://codereview.appspot.com/7228070/

*** Submitted:

Remove service click actions

This branch builds on the view model improvements to remove service
click
actions, simplify the calling convention of methods that lived there and

remove their dependencies on the service_click_actions closure.

R=teknico, matthew.scott
CC=
https://codereview.appspot.com/7228070

https://codereview.appspot.com/7228070/