Do not attempt login if ws is not connected

Do not attempt login if ws is not connected

Based on Brad's branch [1], this one moves the
connection check from the environment to
the application. If the environment is not
connected the application suspends the 
authentication process waiting for the 
connection to be established. Avoiding
the execution of check_user_credentials if
the ws is not ready also fixes a test problem 
for which we added a work-around.

The env.login now checks for credentials,
as suggested by Kapil.

Also added tests.

[1] https://codereview.appspot.com/7130051/

https://code.launchpad.net/~frankban/juju-gui/1099909/+merge/143840

(do not edit description out of merge proposal)

[frankban, 2013-01-18 10:17:46]

Please take a look.

[bac, 2013-01-18 11:56:37]

Thanks for picking up this branch in my absence and improving it greatly.  It
looks good to land except for one trivial.

https://codereview.appspot.com/7129055/diff/1/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/7129055/diff/1/test/test_app.js#newcode200
test/test_app.js:200: 
Nice, clear tests.

https://codereview.appspot.com/7129055/diff/1/test/test_login.js
File test/test_login.js (right):

https://codereview.appspot.com/7129055/diff/1/test/test_login.js#newcode23
test/test_login.js:23: // env.set('connected', true);
I'm not sure why this commented out code is here.  We should probably remove it.

[frankban, 2013-01-18 13:42:10]

Please take a look.

https://codereview.appspot.com/7129055/diff/1/test/test_login.js
File test/test_login.js (right):

https://codereview.appspot.com/7129055/diff/1/test/test_login.js#newcode23
test/test_login.js:23: // env.set('connected', true);
On 2013/01/18 11:56:38, bac wrote:
> I'm not sure why this commented out code is here.  We should probably remove
it.

Done.

[benji, 2013-01-18 14:29:21]

This looks good.  Feel free to land after responding to my comments.

https://codereview.appspot.com/7129055/diff/5001/app/store/env.js
File app/store/env.js (right):

https://codereview.appspot.com/7129055/diff/5001/app/store/env.js#newcode303
app/store/env.js:303: }
It would be nicer if this never happened, but I suspect this code wouldn't be
here if it were easy to arrange that.

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js#newcode190
test/test_app.js:190: });
Shouldn't "done" be used here, since the assert is in an asynchronous event
handler?

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js#newcode197
test/test_app.js:197: assert.equal('login', conn.last_message().op);
This looks like another case for "done".

[frankban, 2013-01-18 14:45:21]

Hi Brad and Benji,

thanks for the reviews.

https://codereview.appspot.com/7129055/diff/5001/app/store/env.js
File app/store/env.js (right):

https://codereview.appspot.com/7129055/diff/5001/app/store/env.js#newcode303
app/store/env.js:303: }
On 2013/01/18 14:29:22, benji wrote:
> It would be nicer if this never happened, but I suspect this code wouldn't be
> here if it were easy to arrange that.

Actually this should never happen in the app. This check is here just to log
that something went really wrong, or some error in how the login API is used. In
that case, we are able to avoid sending an incomplete request to the backend.

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js#newcode190
test/test_app.js:190: });
On 2013/01/18 14:29:22, benji wrote:
> Shouldn't "done" be used here, since the assert is in an asynchronous event
> handler?
You are right, fixed.

https://codereview.appspot.com/7129055/diff/5001/test/test_app.js#newcode197
test/test_app.js:197: assert.equal('login', conn.last_message().op);
On 2013/01/18 14:29:22, benji wrote:
> This looks like another case for "done".

Done.

[frankban, 2013-01-18 14:48:23]

*** Submitted:

Do not attempt login if ws is not connected

Based on Brad's branch [1], this one moves the
connection check from the environment to
the application. If the environment is not
connected the application suspends the 
authentication process waiting for the 
connection to be established. Avoiding
the execution of check_user_credentials if
the ws is not ready also fixes a test problem 
for which we added a work-around.

The env.login now checks for credentials,
as suggested by Kapil.

Also added tests.

[1] https://codereview.appspot.com/7130051/

R=bac, benji
CC=
https://codereview.appspot.com/7129055