Merge lp:~frankban/juju-quickstart/env-manage-edit into lp:juju-quickstart

[Francesco Banconi]

Implement the environment create/edit view.

Add a cli/forms.py module including helpers
to create and handle Urwid forms.

Allow for creating a new environment from the
env_index view.

Implement the env_edit view, used for creating
or updating environments.

Fix a corner case in envs.set_env_data: delete
the env_db default key only if the environments
has been modified.

Tests: `make check`.

QA: start the demo app
(`make` and `./cli-app-demo.py).
Use it to edit existing environments and
to create new ones (ec2 and local).
Check form errors are correctly handled.
Exit the demo app with either ^X or
selecting an environment to use. The
demo app should show changes on exit,
and notify whether a new db has been saved
and/or an environment has been selected.

https://codereview.appspot.com/44750044/

[Francesco Banconi (frankban)]

Reviewers: mp+200305_code.launchpad.net,

Message:
Please take a look.

Description:
Implement the environment create/edit view.

Add a cli/forms.py module including helpers
to create and handle Urwid forms.

Allow for creating a new environment from the
env_index view.

Implement the env_edit view, used for creating
or updating environments.

Fix a corner case in envs.set_env_data: delete
the env_db default key only if the environments
has been modified.

Tests: `make check`.

QA: start the demo app
(`make` and `./cli-app-demo.py).
Use it to edit existing environments and
to create new ones (ec2 and local).
Check form errors are correctly handled.
Exit the demo app with either ^X or
selecting an environment to use. The
demo app should show changes on exit,
and notify whether a new db has been saved
and/or an environment has been selected.

https://code.launchpad.net/~frankban/juju-quickstart/env-manage-edit/+merge/200305

(do not edit description out of merge proposal)

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

Affected files (+948, -20 lines):
   A [revision details]
   A quickstart/cli/forms.py
   M quickstart/cli/ui.py
   M quickstart/cli/views.py
   M quickstart/models/envs.py
   A quickstart/tests/cli/test_forms.py
   M quickstart/tests/cli/test_views.py
   M quickstart/tests/models/test_envs.py

[Francesco Banconi (frankban)]

Sorry for the long diff, tests for views are a little verbose.

https://codereview.appspot.com/44750044/

[Gary Poster (gary)]

QA comments, pre-review:

- Looking very cool.
- I like the fact that I can right click on links to launch them. That
may well be a terminal feature rather than a quickstart or urwid
feature, but it all fits together nicely.
- Trivial, but I continue to be dissatisfied with the footer formatting.
  I don't like the fact that the text in the legend line wraps. For
instance, when creating, I want "field errors" to be on the same line as
the dot it describes. Possible resolutions include the following: (a)
we don't show "juju-quickstart 0.5.0" anywhere; (b) we show it in the
right of the header; or (c) we show it in its own line in the footer.
- It would be nice if you could tab between fields. I assume this is an
Urwid limitation, in which case never mind. Maybe we could clarify in
the footer that arrow keys move between fields?
- It was not clear to me that clicking on "automatically generate" would
generate the admin secret. Could we maybe change that text to "Click
here to automatically generate this value"?
- For the default series, you say that you may leave the series empty.
It would be nice if you explained the semantics of that in the help
text. Am I right that the semantics are that the series of the host
computer is used?
- Should we default the default series value to precise?
- Maybe a bad idea: we could make the "precise," "quantal," and other
default series text clickable. When you click on them, they fill in the
field with the given value. Same could be done with ec2 region.
- Maybe a better idea: Does Urwid support the idea of a combo box,
somehow? Would be nice. :-)
- I wish the two "create" options in the main menu looked more
different. Any bright UX ideas? :-) Right now the whitespace
separating the environments from the creation options don't clarify
their difference sufficiently for my eye.
- Relatedly, the header instructions in the main menu are currently
"Select the Juju environment you want to use." That no longer seems to
describe reality. "Select the Juju environment you want to use, edit,
view, or remove." That's wordy and leaves out the creation options.
"Select an existing Juju environment or create a new one." Maybe that's
good enough? The user can see what to do with the environment once they
select it.
- I know I suggested linking to
https://juju.ubuntu.com/docs/config-aws.html for help, but it is
confusing since the first half of the page is not pertinent to
quickstart. The good stuff begins with "You can retrieve these values
easily from your AWS Management Console at...." My ideas so far include
the following: (a) confer with Nick to see if he has any ideas on
whether we can add an anchor in the page to the part we want, or if we
can have a separate page with just the part we want; (b) include the
information for users actually in the help text for access key ("You can
retrieve these values easily from your AWS Management Console at
http://console.aws.amazon.com. Click on your name in the top-right and
then the "Security Credentials" link from the drop down menu. Under the
"Access Keys" heading click the "Create New Root Key" button. You will
be prompted to "Download Key File" which by def...

[Benji York (benji)]

Looks great.

LGTM-ly, yrs
Benji

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py
File quickstart/cli/forms.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode208
quickstart/cli/forms.py:208: return ui.create_controls(*controls)
This is so small it is almost not worth mentioning, but I thought you
might like it:

return ui.create_controls(
     *(ui.MenuButton(caption, callback) for caption, callback in
actions))

Or if we want to go full-on functional:

return ui.create_controls(*map(ui.MenuButton, actions))

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/ui.py
File quickstart/cli/ui.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/ui.py#newcode49
quickstart/cli/ui.py:49: ('optional status', 'light magenta', 'light
gray'),
Unfortunately the non-256 color terminal colors are not standardized so
I suggest testing on multiple terminals (I can test on xterm, gnome
terminal, and terminator if you want).

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py#newcode347
quickstart/cli/views.py:347: def save(env_data, get_new_env_data):
I like using functions defined in functions like this, but it means that
they are hard to test (or require functional tests instead of unit
tests). Consider making them module-level functions so they can be
tested directly.

https://codereview.appspot.com/44750044/diff/1/quickstart/tests/cli/test_forms.py
File quickstart/tests/cli/test_forms.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/tests/cli/test_forms.py#newcode1
quickstart/tests/cli/test_forms.py:1: # This file is part of the Juju
Quickstart Plugin, which lets users set up a
These tests look really nice. The running comments in each test really
increase understanding of what is going on.

https://codereview.appspot.com/44750044/

[Gary Poster (gary)]

LGTM! Thank you!

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py
File quickstart/cli/forms.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode44
quickstart/cli/forms.py:44: def create_string_widget(field, value,
error):
Nice behavior/approach for the widget generation. I like it.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode82
quickstart/cli/forms.py:82: if generate_method is not None:
For readability, I wonder if we ought to stick most of this block in a
separate function.

if generate_method is not None:
     widgets.append(
         create_generate_widget(generate_method, edit_widget))

...or similar?

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode98
quickstart/cli/forms.py:98: # edit widget when a choice is clicked.
Hah, ok, cool, that's what I mentioned in my qa. :-)

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode172
quickstart/cli/forms.py:172: urwid.Text(('error', 'please correct the
errors below')),
Nice to have: a count. ("Please correct the 2 errors below")

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode179
quickstart/cli/forms.py:179: # Boolean values as represented as
checkboxes.
s/as/are/

Boolean values are represented a checkboxes

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode195
quickstart/cli/forms.py:195: return dict((key, value_getter()) for key,
value_getter in form.items())
Nice :-)

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py#newcode168
quickstart/cli/views.py:168: for position, env_data in
enumerate(environments):
Just as a way to make the main function smaller, what would you think of
breaking this block out into its own function?

...eh, nevermind. I tried to do it and you need default_found and
focus_position and errors_found, so it doesn't seem like there's much
point.

The function is very clear and linear but I was hoping to make it
shorter by making some nice internal verbs. Maybe you have another
idea? Could be for later, or never. :-)

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py#newcode230
quickstart/cli/views.py:230: def use(env_data):
I wonder if the env_detail function would look less forbidding at first
glance if these internal functions were defined outside (_use,
_set_default, etc.) and then hooked up as partials. I think the verbs
are clear enough that it would read well.

A counter-argument is that you are defining everything locally, so you
can read what is going on inline, and it similar to what people are
accustomed to doing with OO. If env_detail were in its own file as is,
would it look super easy, because it would look similar to a class with
methods? I don't know.

What do you think?

https://codereview.appspot.com/44750044/diff/1/quickstart/tests/cli/test_forms.py
File quickstart/tests/cli/test_forms.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/tests/cli/test_forms.py#newcode1...

[lp:~frankban/juju-quickstart/env-manage-edit]

55. By Francesco Banconi on 2014-01-02

Form changes as per review.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py
File quickstart/cli/forms.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode82
quickstart/cli/forms.py:82: if generate_method is not None:
On 2014/01/02 15:19:28, gary.poster wrote:
> For readability, I wonder if we ought to stick most of this block in a
separate
> function.

> if generate_method is not None:
> widgets.append(
> create_generate_widget(generate_method, edit_widget))

> ...or similar?

Done.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode172
quickstart/cli/forms.py:172: urwid.Text(('error', 'please correct the
errors below')),
On 2014/01/02 15:19:28, gary.poster wrote:
> Nice to have: a count. ("Please correct the 2 errors below")

Done.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode179
quickstart/cli/forms.py:179: # Boolean values as represented as
checkboxes.
On 2014/01/02 15:19:28, gary.poster wrote:
> s/as/are/

> Boolean values are represented a checkboxes

Done.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/forms.py#newcode208
quickstart/cli/forms.py:208: return ui.create_controls(*controls)
On 2014/01/02 15:05:32, benji1 wrote:
> This is so small it is almost not worth mentioning, but I thought you
might like
> it:

> return ui.create_controls(
> *(ui.MenuButton(caption, callback) for caption, callback in
actions))

> Or if we want to go full-on functional:

> return ui.create_controls(*map(ui.MenuButton, actions))

Done.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/ui.py
File quickstart/cli/ui.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/ui.py#newcode49
quickstart/cli/ui.py:49: ('optional status', 'light magenta', 'light
gray'),
On 2014/01/02 15:05:32, benji1 wrote:
> Unfortunately the non-256 color terminal colors are not standardized
so I
> suggest testing on multiple terminals (I can test on xterm, gnome
terminal, and
> terminator if you want).

My understanding is that these color combinations can be safely used
cross-terminal because the colors are handled by Urwid itself:
http://excess.org/urwid/docs/reference/constants.html?highlight=colors#foreground-and-background-colors
But I can be wrong, and maybe it is worth testing it.

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py#newcode168
quickstart/cli/views.py:168: for position, env_data in
enumerate(environments):
On 2014/01/02 15:19:28, gary.poster wrote:
> Just as a way to make the main function smaller, what would you think
of
> breaking this block out into its own function?

> ...eh, nevermind. I tried to do it and you need default_found and
> focus_position and errors_found, so it doesn't seem like there's much
point.

> The function is very clear and linear but I was hoping to make it
shorter by
> making some nice internal verbs. Maybe you have another idea? Could
be for
> later, or never. :-)

Not sure, a quick solution might be splitting the f...

[Francesco Banconi (frankban)]

Now working on the QA comments.
In the meanwhile, thank you for the great reviews!

https://codereview.appspot.com/44750044/

[Gary Poster (gary)]

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/44750044/diff/1/quickstart/cli/views.py#newcode230
quickstart/cli/views.py:230: def use(env_data):
On 2014/01/02 17:51:04, frankban wrote:
> On 2014/01/02 15:19:28, gary.poster wrote:
> > I wonder if the env_detail function would look less forbidding at
first glance
> > if these internal functions were defined outside (_use,
_set_default, etc.)
> and
> > then hooked up as partials. I think the verbs are clear enough that
it would
> > read well.
> >
> > A counter-argument is that you are defining everything locally, so
you can
> read
> > what is going on inline, and it similar to what people are
accustomed to doing
> > with OO. If env_detail were in its own file as is, would it look
super easy,
> > because it would look similar to a class with methods? I don't
know.
> >
> > What do you think?

> This is a really good question. I like the possibility to use closures
so that
> you can share the scope and group code logic, but, as you mentioned,
this can be
> surprising at first sight. If you add to this the good point Benji
made below
> (it is easier to directly unit test separate functions), then we have
two good
> arguments to make this change. Since it requires a bit of code
reorganization
> I'd be inclined to tackle this separately, how does it sound?

Sounds great to me. Thank you.

https://codereview.appspot.com/44750044/

[lp:~frankban/juju-quickstart/env-manage-edit]

56. By Francesco Banconi on 2014-01-03

Footer fixes.

57. By Francesco Banconi on 2014-01-03

Footer fixes.

58. By Francesco Banconi on 2014-01-03

Changes as per QA.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/44750044/

[Francesco Banconi (frankban)]

On 2014/01/02 14:24:57, gary.poster wrote:
> QA comments, pre-review:

> - Looking very cool.
> - I like the fact that I can right click on links to launch them.
That may well
> be a terminal feature rather than a quickstart or urwid feature, but
it all fits
> together nicely.

I agree, that's cool.

> - Trivial, but I continue to be dissatisfied with the footer
formatting. I
> don't like the fact that the text in the legend line wraps. For
instance, when
> creating, I want "field errors" to be on the same line as the dot it
describes.
> Possible resolutions include the following: (a) we don't show
"juju-quickstart
> 0.5.0" anywhere; (b) we show it in the right of the header; or (c) we
show it in
> its own line in the footer.

Removed the quickstart brand and put the notifications widget in its own
line
(above the footer). This way I see the status in a single line even when
the terminal size is 80x24.

> - It would be nice if you could tab between fields. I assume this is
an Urwid
> limitation, in which case never mind. Maybe we could clarify in the
footer that
> arrow keys move between fields?

I'd also like tab navigation to be implemented as part of the UX
improvement
card(s). I think it should not be too difficult to implement that in
Urwid.
In the meanwhile, added navigation help to the status as you suggested.

> - It was not clear to me that clicking on "automatically generate"
would
> generate the admin secret. Could we maybe change that text to "Click
here to
> automatically generate this value"?

Good idea, done.

> - For the default series, you say that you may leave the series empty.
  It would
> be nice if you explained the semantics of that in the help text. Am I
right
> that the semantics are that the series of the host computer is used?
> - Should we default the default series value to precise?

You are right. While we usually let juju itself handle the default
values
where not set, for the default series quickstart should force an
explicit
"precise" when the value is not specified. This means the default series
field must overwrite the normalize method, and that should be easy
enough.
I'll do this in another card, perhaps the same as the one below
(choices).

> - Maybe a bad idea: we could make the "precise," "quantal," and other
default
> series text clickable. When you click on them, they fill in the field
with the
> given value. Same could be done with ec2 region.
> - Maybe a better idea: Does Urwid support the idea of a combo box,
somehow?
> Would be nice. :-)

AFAIK Urwid does not have a combo widget out of the box, but we can try
to
implement it on top of the other base widgets. The first solution sounds
good
to me anyway, and as you noticed I already have an XXX for it, and I
will
create a card as well.

> - I wish the two "create" options in the main menu looked more
different. Any
> bright UX ideas? :-) Right now the whitespace separating the
environments from
> the creation options don't clarify their difference sufficiently for
my eye.

Updated the index view so that the environment creation options are more
separated. Now there is a text indicating how to use the creation links.
Does it look better?

> ...

[Gary Poster (gary)]

On 2014/01/03 11:42:10, frankban wrote:
> On 2014/01/02 14:24:57, gary.poster wrote:
> > QA comments, pre-review:
> >
> > - Looking very cool.
> > - I like the fact that I can right click on links to launch them.
That may
> well
> > be a terminal feature rather than a quickstart or urwid feature, but
it all
> fits
> > together nicely.

> I agree, that's cool.

> > - Trivial, but I continue to be dissatisfied with the footer
formatting. I
> > don't like the fact that the text in the legend line wraps. For
instance,
> when
> > creating, I want "field errors" to be on the same line as the dot it
> describes.
> > Possible resolutions include the following: (a) we don't show
"juju-quickstart
> > 0.5.0" anywhere; (b) we show it in the right of the header; or (c)
we show it
> in
> > its own line in the footer.

> Removed the quickstart brand and put the notifications widget in its
own line
> (above the footer). This way I see the status in a single line even
when
> the terminal size is 80x24.

Much better, thanks!

> > - It would be nice if you could tab between fields. I assume this
is an Urwid
> > limitation, in which case never mind. Maybe we could clarify in the
footer
> that
> > arrow keys move between fields?

> I'd also like tab navigation to be implemented as part of the UX
improvement
> card(s). I think it should not be too difficult to implement that in
Urwid.
> In the meanwhile, added navigation help to the status as you
suggested.

Yes, looks good.

> > - It was not clear to me that clicking on "automatically generate"
would
> > generate the admin secret. Could we maybe change that text to
"Click here to
> > automatically generate this value"?

> Good idea, done.

> > - For the default series, you say that you may leave the series
empty. It
> would
> > be nice if you explained the semantics of that in the help text. Am
I right
> > that the semantics are that the series of the host computer is used?
> > - Should we default the default series value to precise?

> You are right. While we usually let juju itself handle the default
values
> where not set, for the default series quickstart should force an
explicit
> "precise" when the value is not specified. This means the default
series
> field must overwrite the normalize method, and that should be easy
enough.
> I'll do this in another card, perhaps the same as the one below
(choices).

Cool

> > - Maybe a bad idea: we could make the "precise," "quantal," and
other default
> > series text clickable. When you click on them, they fill in the
field with
> the
> > given value. Same could be done with ec2 region.
> > - Maybe a better idea: Does Urwid support the idea of a combo box,
somehow?
> > Would be nice. :-)

> AFAIK Urwid does not have a combo widget out of the box, but we can
try to
> implement it on top of the other base widgets. The first solution
sounds good
> to me anyway, and as you noticed I already have an XXX for it, and I
will
> create a card as well.

Great. I think the links will be fine (instead of trying to implement a
combobox) as long as our text clearly indicates the functionality (e.g.
"Click one to autofill the field with these standard options: precise,
quant...

[Francesco Banconi (frankban)]

*** Submitted:

Implement the environment create/edit view.

Add a cli/forms.py module including helpers
to create and handle Urwid forms.

Allow for creating a new environment from the
env_index view.

Implement the env_edit view, used for creating
or updating environments.

Fix a corner case in envs.set_env_data: delete
the env_db default key only if the environments
has been modified.

Tests: `make check`.

QA: start the demo app
(`make` and `./cli-app-demo.py).
Use it to edit existing environments and
to create new ones (ec2 and local).
Check form errors are correctly handled.
Exit the demo app with either ^X or
selecting an environment to use. The
demo app should show changes on exit,
and notify whether a new db has been saved
and/or an environment has been selected.

R=gary.poster, benji1
CC=
https://codereview.appspot.com/44750044

https://codereview.appspot.com/44750044/

=== modified file 'quickstart/app.py'
--- quickstart/app.py	2013-12-19 03:29:39 +0000
+++ quickstart/app.py	2014-01-03 11:38:58 +0000
@@ -232,8 +232,10 @@
 
     Do not bootstrap the environment if already bootstrapped.
 
-    Return True without errors if the environment is already bootstrapped.
-    Return False otherwise. Only return when the bootstrap node is ready.
+    Return a tuple (already_bootstrapped, series) in which:
+        - already_bootstrapped indicates whether the environment was already
+          bootstrapped;
+        - series is the bootstrap node Ubuntu series.
 
     The is_local argument indicates whether the environment is configured to
     use the local provider. If so, sudo privileges are requested in order to

=== modified file 'quickstart/cli/base.py'
--- quickstart/cli/base.py	2013-12-19 11:10:46 +0000
+++ quickstart/cli/base.py	2014-01-03 11:38:58 +0000
@@ -29,7 +29,6 @@
 
 import urwid
 
-from quickstart import get_version
 from quickstart.cli import ui
 
 
@@ -129,13 +128,13 @@
 
     # Set up the application footer.
     # The CTRL-x shortcut is automatically set up by views.show().
-    brand_message = 'juju-quickstart v{} - ^X exit '.format(get_version())
-    brand = urwid.Text(brand_message)
+    message = urwid.Text('', align='center')
+    base_status = urwid.Text('^X exit ')
     status = urwid.Text('')
-    message = urwid.Text('', align='right')
+    status_columns = urwid.Columns([('pack', base_status), status])
     footer_line = urwid.Divider('\N{UPPER ONE EIGHTH BLOCK}')
-    status_columns = urwid.Columns([('pack', brand), status, message])
     footer = urwid.Pile([
+        message,
         urwid.Divider(),
         urwid.AttrMap(ui.padding(status_columns), 'footer'),
         urwid.AttrMap(footer_line, 'line footer'),

=== added file 'quickstart/cli/forms.py'
--- quickstart/cli/forms.py	1970-01-01 00:00:00 +0000
+++ quickstart/cli/forms.py	2014-01-03 11:38:58 +0000
@@ -0,0 +1,215 @@
+# This file is part of the Juju Quickstart Plugin, which lets users set up a
+# Juju environment in very few steps (https://launchpad.net/juju-quickstart).
+# Copyright (C) 2014 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Juju Quickstart CLI forms management.
+
+This module contains a collection of functions which help creating and
+manipulating forms in Urwid.
+"""
+
+from __future__ import unicode_literals
+
+import functools
+
+import urwid
+
+from quickstart.cli import ui
+
+
+# Define the value used in boolean widgets to specify they allow mixed state.
+MIXED = 'mixed'
+
+
+def _generate(generate_callable, edit_widget):
+    """Update the widget contents using the given generate_callable.
+
+    The passed callable function takes no arguments and returns a string.
+    """
+    edit_widget.set_edit_text(generate_callable())
+
+
+def _create_generate_widget(generate_callable, edit_widget):
+    """Create and return a button widget used to generate values for a field.
+
+    Receives the generate callable and the target edit widget.
+    """
+    generate_callback = ui.thunk(_generate, generate_callable, edit_widget)
+    generate_button = ui.MenuButton(
+        ('highlight', 'Click here to automatically generate'),
+        generate_callback)
+    return urwid.Columns([
+        ('pack', generate_button),
+        urwid.Text(' this value'),
+    ])
+
+
+def create_string_widget(field, value, error):
+    """Create a string widget and return a tuple (widget, value_getter).
+
+    Receives a Field instance (see quickstart.models.fields), the field value,
+    and an error string (or None if the field has no errors).
+
+    In the returned tuple, widget is a Urwid widget suitable for editing string
+    values, and value_getter is a callable returning the value currently stored
+    in the widget. The value_getter callable must be called without arguments.
+    """
+    if value is None:
+        # Unset values are converted to empty strings.
+        value = ''
+    elif not isinstance(value, unicode):
+        # We do not expect byte strings, and all other values are converted to
+        # unicode strings.
+        value = unicode(value)
+    caption_class = 'highlight' if field.required else 'optional'
+    caption = []
+    widgets = []
+    if error:
+        caption.append(('error', '\N{BULLET} '))
+        # Display the error message above the edit widget.
+        widgets.append(urwid.Text(('error', error)))
+    caption.append((caption_class, '{}: '.format(field.label)))
+    edit_widget = urwid.Edit(edit_text=value)
+    widget = urwid.Columns([('pack', urwid.Text(caption)), edit_widget])
+    if field.readonly:
+        # Disable the widget if the field is not editable.
+        widgets.append(urwid.WidgetDisable(widget))
+    else:
+        widgets.append(urwid.AttrMap(widget, 'edit'))
+    if field.help:
+        # Display the field help message below the edit widget.
+        widgets.append(urwid.Text(field.help))
+    if not field.readonly:
+        # Can the value be automatically generated?
+        generate_callable = getattr(field, 'generate', None)
+        if generate_callable is not None:
+            widgets.append(
+                _create_generate_widget(generate_callable, edit_widget))
+    # If the value must be in a range of choices, display the possible choices
+    # as part of the help message.
+    choices = getattr(field, 'choices', None)
+    if choices is not None:
+        # XXX frankban 2014-01-02: make the choices clickable and update the
+        # edit widget when a choice is clicked.
+        text = 'possible values are {}'.format(', '.join(choices))
+        if not field.required:
+            text = '{}, but this field can also be left empty'.format(text)
+        widgets.append(urwid.Text(text))
+    if field.default is not None:
+        widgets.append(
+            urwid.Text('default if not set: {}'.format(field.default)))
+    widgets.append(urwid.Divider())
+    return urwid.Pile(widgets), edit_widget.get_edit_text
+
+
+def create_bool_widget(field, value, error):
+    """Create a boolean widget and return a tuple (widget, value_getter).
+
+    Receives a Field instance (see quickstart.models.fields), the field value,
+    and an error string (or None if the field has no errors).
+
+    In the returned tuple, widget is a Urwid widget suitable for editing
+    boolean values (a checkbox), and value_getter is a callable returning the
+    value currently stored in the widget. The value_getter callable receives no
+    arguments.
+    """
+    if value is None:
+        # Unset values are converted to a more convenient value for the
+        # checkbox (a boolean or a mixed state if allowed by the field).
+        value = MIXED if field.allow_mixed else bool(field.default)
+    label = ('highlight', field.label)
+    widget = urwid.CheckBox(label, state=value, has_mixed=field.allow_mixed)
+    widgets = []
+    if error:
+        # Display the error message above the checkbox.
+        widgets.append(urwid.Text(('error', error)))
+    if field.readonly:
+        # Disable the widget if the field is not editable.
+        widgets.append(urwid.WidgetDisable(widget))
+    else:
+        widgets.append(widget)
+    if field.help:
+        # Display the field help message below the checkbox.
+        widgets.append(urwid.Text(field.help))
+    widgets.append(urwid.Divider())
+
+    def get_state():
+        # Reconvert mixed value to None value.
+        state = widget.get_state()
+        return None if state == MIXED else state
+
+    return urwid.Pile(widgets), get_state
+
+
+def create_form(field_value_pairs, errors):
+    """Create and return the form widgets for each field/value pair.
+
+    Return a tuple (widgets, values_getter) in which:
+        - widgets is a list if Urwid objects that can be used to build view
+          contents (e.g. by wrapping them in a urwid.ListBox);
+        - values_getter is a function returning a dictionary mapping field
+          names to values. By calling the values_getter function it is always
+          possible to retrieve the current new field values, even if they have
+          been changed by the user.
+
+    The field_value_pairs argument is a list of (field, value) tuples where
+    field is a Field instance (see quickstart.models.fields) and value is
+    the corresponding field's value.
+
+    The errors argument is a dictionary mapping field names to error messages.
+    Passing an empty dictionary means the form has no errors.
+    """
+    form = {}
+    widgets = []
+    if errors:
+        # Inform the user that the form has errors that need to be fixed.
+        num_errors = len(errors)
+        msg = 'error' if num_errors == 1 else '{} errors'.format(num_errors)
+        widgets.extend([
+            urwid.Text(('error', 'please correct the {} below'.format(msg))),
+            urwid.Divider(),
+        ])
+    # Build a widget and populate the form for each field/value pair.
+    for field, value in field_value_pairs:
+        error = errors.get(field.name)
+        if field.field_type == 'bool':
+            # Boolean values are represented as checkboxes.
+            widget_factory = create_bool_widget
+        else:
+            # All the other field types are displayed as strings.
+            widget_factory = create_string_widget
+        widget, value_getter = widget_factory(field, value, error)
+        widgets.append(widget)
+        form[field.name] = value_getter
+    return widgets, functools.partial(_get_data, form)
+
+
+def _get_data(form):
+    """Return a dictionary mapping the given form field names to values.
+
+    This is done just calling all the value getters in the form.
+    """
+    return dict((key, value_getter()) for key, value_getter in form.items())
+
+
+def create_actions(actions):
+    """Return the control widgets based on the given actions.
+
+    The actions arguments is as a sequence of (caption, callback) tuples. Those
+    pairs are used to generate the clickable controls (MenuButton instances)
+    used to manipulate the form.
+    """
+    return ui.create_controls(
+        *(ui.MenuButton(caption, callback) for caption, callback in actions))

=== modified file 'quickstart/cli/ui.py'
--- quickstart/cli/ui.py	2013-12-20 10:30:37 +0000
+++ quickstart/cli/ui.py	2014-01-03 11:38:58 +0000
@@ -45,6 +45,8 @@
     ('highlight', 'white', 'black'),
     ('line header', 'dark gray', 'dark magenta'),
     ('line footer', 'light gray', 'light gray'),
+    ('optional', 'light magenta', 'black'),
+    ('optional status', 'light magenta', 'light gray'),
     ('selected', 'white', 'dark blue'),
 ]
 # Map attributes to new attributes to apply when the widget is selected.
@@ -52,6 +54,7 @@
     None: 'selected',
     'control alert': 'error selected',
     'error': 'error selected',
+    'highlight': 'selected',
 }
 # Define a default padding for the Urwid application.
 padding = functools.partial(urwid.Padding, left=2, right=2)

=== modified file 'quickstart/cli/views.py'
--- quickstart/cli/views.py	2013-12-19 17:40:36 +0000
+++ quickstart/cli/views.py	2014-01-03 11:38:58 +0000
@@ -104,6 +104,7 @@
 
 from quickstart.cli import (
     base,
+    forms,
     ui,
 )
 from quickstart.models import envs
@@ -132,6 +133,8 @@
     """Show the Juju environments list.
 
     The env_detail view is displayed when the user clicks on an environment.
+    From here it is also possible to switch to the edit view in order to create
+    a new environment.
 
     Receives:
         - env_type_db: the environments meta information;
@@ -145,15 +148,16 @@
     app.set_return_value_on_exit((env_db, None))
     detail_view = functools.partial(
         env_detail, app, env_type_db, env_db, save_callable)
+    edit_view = functools.partial(
+        env_edit, app, env_type_db, env_db, save_callable)
     # Alphabetically sort the existing environments.
     environments = sorted([
         envs.get_env_data(env_db, env_name)
         for env_name in env_db['environments']
     ], key=operator.itemgetter('name'))
     if environments:
-        title = 'Select the Juju environment you want to use'
+        title = 'Select an existing Juju environment or create a new one'
     else:
-        # XXX frankban 2013-12-18: implement the env creation view.
         title = 'No Juju environments already set up: please create one'
     app.set_title(title)
     app.set_status('')
@@ -178,10 +182,20 @@
         env_short_description = envs.get_env_short_description(env_data)
         text = [bullet, ' {}'.format(env_short_description)]
         widgets.append(ui.MenuButton(text, ui.thunk(detail_view, env_data)))
-    widgets.append(urwid.Divider())
-    contents = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
-    if focus_position is not None:
-        contents.set_focus(focus_position)
+    # Add the buttons used to create new environments.
+    widgets.extend([
+        urwid.Divider(),
+        urwid.Text((
+            'highlight', 'Use the links below to create new environments')),
+        urwid.Divider(),
+    ])
+    widgets.extend([
+        ui.MenuButton(
+            ['\N{BULLET} new ', ('highlight', env_type), ' environment'],
+            ui.thunk(edit_view, {'type': env_type})
+        )
+        for env_type in envs.get_supported_env_types(env_type_db)
+    ])
     # Set up the application status messages.
     status = []
     if default_found:
@@ -190,6 +204,10 @@
         status.extend([('error status', ' \N{BULLET}'), ' has errors '])
     if status:
         app.set_status(status)
+    # Set up the application contents.
+    contents = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
+    if focus_position is not None:
+        contents.set_focus(focus_position)
     app.set_contents(contents)
 
 
@@ -202,7 +220,7 @@
     Receives:
         - env_type_db: the environments meta information;
         - env_db: the environments database;
-        - save_callable: a function called to save a new environment database.
+        - save_callable: a function called to save a new environment database;
         - env_data: the environment data.
     """
     env_db = copy.deepcopy(env_db)
@@ -212,6 +230,8 @@
     app.set_return_value_on_exit((env_db, None))
     index_view = functools.partial(
         env_index, app, env_type_db, env_db, save_callable)
+    edit_view = functools.partial(
+        env_edit, app, env_type_db, env_db, save_callable, env_data)
 
     def use(env_data):
         # Quit the interactive session returning the (possibly modified)
@@ -231,8 +251,10 @@
     def remove(env_data):
         # The environment deletion is confirmed: remove the environment from
         # the database, save the new env_db and return to the index view.
-        envs.remove_env(env_db, env_data['name'])
+        env_name = env_data['name']
+        envs.remove_env(env_db, env_name)
         save_callable(env_db)
+        app.set_message('{} successfully removed'.format(env_name))
         index_view()
 
     def confirm_removal(env_data):
@@ -272,10 +294,113 @@
         controls.append(
             ui.MenuButton('set default', ui.thunk(set_default, env_data)))
     controls.extend([
-        # XXX frankban 2013-12-18: implement the env modification view.
+        ui.MenuButton('edit', ui.thunk(edit_view)),
         ui.MenuButton(
             ('control alert', 'remove'), ui.thunk(confirm_removal, env_data)),
     ])
     widgets.append(ui.create_controls(*controls))
     listbox = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
     app.set_contents(listbox)
+
+
+def env_edit(app, env_type_db, env_db, save_callable, env_data):
+    """Create or modify a Juju environment.
+
+    This view displays an edit form allowing for environment
+    creation/modification. Saving the form redirects to the environment detail
+    view if the values are valid.
+
+    Receives:
+        - env_type_db: the environments meta information;
+        - env_db: the environments database;
+        - save_callable: a function called to save a new environment database;
+        - env_data: the environment data.
+
+    The last value (env_data) indicates whether this view is used to create a
+    new environment or to change an existing one. In the former case, env_data
+    only includes the "type" key. If instead the environment already exists,
+    env_data includes the "name" key and all the other environment info.
+    """
+    env_db = copy.deepcopy(env_db)
+    # All the environment views return a tuple (new_env_db, env_data).
+    # Set the env_data to None in the case the user quits the application
+    # without selecting an environment to use.
+    app.set_return_value_on_exit((env_db, None))
+    env_metadata = envs.get_env_metadata(env_type_db, env_data)
+    index_view = functools.partial(
+        env_index, app, env_type_db, env_db, save_callable)
+    detail_view = functools.partial(
+        env_detail, app, env_type_db, env_db, save_callable)
+    if 'name' in env_data:
+        exists = True
+        title = 'Edit the {} environment'
+        # Retrieve all the errors for the existing environment.
+        initial_errors = envs.validate(env_metadata, env_data)
+    else:
+        exists = False
+        title = 'Create a new {} environment'
+        # The environment does not exist: avoid bothering the user with errors
+        # before the form is submitted.
+        initial_errors = {}
+    app.set_title(title.format(env_data['type']))
+    app.set_status([
+        ' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW} navigate ',
+        ('optional status', ' \N{LOWER SEVEN EIGHTHS BLOCK}'),
+        ' optional field ',
+        ('error status', ' \N{BULLET}'),
+        ' field errors ',
+    ])
+
+    def save(env_data, get_new_env_data):
+        # Create a new environment or save changes for an existing one.
+        # The new values are saved only if the new env_data is valid, in which
+        # case also redirect to the environment detail view.
+        new_env_data = get_new_env_data()
+        # Validate the new env_data.
+        errors = envs.validate(env_metadata, new_env_data)
+        new_name = new_env_data['name']
+        initial_name = env_data.get('name')
+        if (new_name != initial_name) and new_name in env_db['environments']:
+            errors['name'] = 'an environment with this name already exists'
+        # If errors are found, re-render the form passing the errors. This way
+        # the errors are displayed as part of the form and the user is given
+        # the opportunity to fix the invalid values.
+        if errors:
+            return render_form(new_env_data, errors)
+        # Without errors, normalize the new values, update the env_db and save
+        # the resulting environments database.
+        env_data = envs.normalize(env_metadata, new_env_data)
+        envs.set_env_data(env_db, initial_name, env_data)
+        save_callable(env_db)
+        verb = 'modified' if exists else 'created'
+        app.set_message('{} successfully {}'.format(new_name, verb))
+        return detail_view(env_data)
+
+    def cancel(env_data):
+        # Dismiss any changes and return to the index or detail view.
+        return detail_view(env_data) if exists else index_view()
+
+    def render_form(data, errors):
+        # Render the environment edit form.
+        widgets = [
+            urwid.Text(env_metadata['description']),
+            urwid.Divider(),
+        ]
+        field_value_pairs = envs.map_fields_to_env_data(env_metadata, data)
+        # Retrieve the form widgets and the data getter function. The latter
+        # can be used to retrieve the field name/value pairs included in the
+        # displayed form, including user's changes (see quickstart.cli.forms).
+        form_widgets, get_new_env_data = forms.create_form(
+            field_value_pairs, errors)
+        widgets.extend(form_widgets)
+        actions = (
+            ('save', ui.thunk(save, env_data, get_new_env_data)),
+            ('cancel', ui.thunk(cancel, env_data)),
+            ('restore', ui.thunk(render_form, env_data, initial_errors)),
+        )
+        widgets.append(forms.create_actions(actions))
+        contents = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
+        app.set_contents(contents)
+
+    # Render the initial form.
+    render_form(env_data, initial_errors)

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2013-12-06 18:28:40 +0000
+++ quickstart/manage.py	2014-01-03 11:38:58 +0000
@@ -303,10 +303,11 @@
     env = app.connect(api_url, options.admin_secret)
 
     # It is not possible to deploy on the bootstrap node if we are using the
-    # local provider.
-    to_new_machine = (is_local or
-                      bsn_series != settings.JUJU_GUI_PREFERRED_SERIES)
-    machine = None if to_new_machine else '0'
+    # local provider, or if the bootstrap node series is not compatible with
+    # the Juju GUI charm.
+    machine = '0'
+    if is_local or (bsn_series != settings.JUJU_GUI_PREFERRED_SERIES):
+        machine = None
     unit_name = app.deploy_gui(
         env, settings.JUJU_GUI_SERVICE_NAME, machine,
         charm_url=options.charm_url, check_preexisting=already_bootstrapped)

=== modified file 'quickstart/models/envs.py'
--- quickstart/models/envs.py	2013-12-19 16:48:38 +0000
+++ quickstart/models/envs.py	2014-01-03 11:38:58 +0000
@@ -292,7 +292,7 @@
     environments[new_name] = new_env_data
     if is_default:
         env_db['default'] = new_name
-    elif env_db.get('default') == env_name:
+    elif (env_db.get('default') == env_name) and (env_name is not None):
         del env_db['default']
 
 
@@ -363,10 +363,21 @@
                 default_series_field,
                 fields.PasswordField(
                     'access-key', label='access key', required=True,
-                    help='the access key to use to authenticate to AWS'),
+                    help='The access key to use to authenticate to AWS. '
+                         'You can retrieve these values easily from your AWS '
+                         'Management Console (http://console.aws.amazon.com). '
+                         'Click on your name in the top-right and then the '
+                         '"Security Credentials" link from the drop down '
+                         'menu. Under the "Access Keys" heading click the '
+                         '"Create New Root Key" button. You will be prompted '
+                         'to "Download Key File" which by default is named '
+                         'rootkey.csv. Open this file to get the access-key '
+                         'and secret-key for the environments.yaml '
+                         'configuration file.'),
                 fields.PasswordField(
                     'secret-key', label='secret key', required=True,
-                    help='the secret key to use to authenticate to AWS'),
+                    help='The secret key to use to authenticate to AWS. '
+                         'See the access key help above.'),
                 fields.AutoGeneratedStringField(
                     'control-bucket', label='control bucket', required=True,
                     help='the globally unique S3 bucket name'),

=== modified file 'quickstart/tests/cli/test_base.py'
--- quickstart/tests/cli/test_base.py	2013-12-19 10:59:57 +0000
+++ quickstart/tests/cli/test_base.py	2014-01-03 11:38:58 +0000
@@ -82,25 +82,25 @@
         # The contents widget is the body's original widget.
         return body.original_widget
 
-    def _get_footer_columns(self, loop):
+    def _get_footer(self, loop):
         # The frame is the main overlay's top widget.
         frame = loop.widget.top_w
         # Retrieve the footer.
-        footer = frame.contents['footer'][0]
-        # Return the columns widget.
-        return footer.contents[1][0].base_widget
+        return frame.contents['footer'][0]
 
     def get_status_widget(self, loop):
         """Return the status widget given the application main loop."""
-        columns = self._get_footer_columns(loop)
-        # The status widget is the second one (brand, status, message).
+        footer = self._get_footer(loop)
+        # The status columns is the third widget (message, divider, status).
+        columns = footer.contents[2][0].base_widget
+        # The status widget is the second one (base status, status).
         return columns.contents[1][0]
 
     def get_message_widget(self, loop):
         """Return the message widget given the application main loop."""
-        columns = self._get_footer_columns(loop)
-        # The message widget is the third one (brand, status, message).
-        return columns.contents[2][0]
+        footer = self._get_footer(loop)
+        # The message widget is the first one (message, divider, status).
+        return footer.contents[0][0].base_widget
 
     def test_loop(self):
         # The returned loop is an instance of the base customized loop.

=== added file 'quickstart/tests/cli/test_forms.py'
--- quickstart/tests/cli/test_forms.py	1970-01-01 00:00:00 +0000
+++ quickstart/tests/cli/test_forms.py	2014-01-03 11:38:58 +0000
@@ -0,0 +1,345 @@
+# This file is part of the Juju Quickstart Plugin, which lets users set up a
+# Juju environment in very few steps (https://launchpad.net/juju-quickstart).
+# Copyright (C) 2014 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for the Juju Quickstart CLI forms management."""
+
+from __future__ import unicode_literals
+
+import unittest
+
+import mock
+import urwid
+
+from quickstart.cli import (
+    forms,
+    ui,
+)
+from quickstart.models import fields
+from quickstart.tests.cli import helpers as cli_helpers
+
+
+class TestGenerate(unittest.TestCase):
+
+    def test_generation(self):
+        # The text of the given widget is set to the return value of the given
+        # callable.
+        generate_callable = lambda: 'generated'
+        mock_edit_widget = mock.Mock()
+        forms._generate(generate_callable, mock_edit_widget)
+        mock_edit_widget.set_edit_text.assert_called_once_with('generated')
+
+
+class TestCreateGenerateWidget(unittest.TestCase):
+
+    def test_widget_creation(self):
+        # The generate widget is properly created and returned.
+        generate_callable = lambda: 'generated'
+        mock_edit_widget = mock.Mock()
+        widget = forms._create_generate_widget(
+            generate_callable, mock_edit_widget)
+        # The generate button is the first widget in the urwid.Columns.
+        button = widget.contents[0][0]
+        cli_helpers.emit(button)
+        mock_edit_widget.set_edit_text.assert_called_once_with('generated')
+
+
+class TestCreateStringWidget(unittest.TestCase):
+
+    def inspect_widget(self, widget, field):
+        """Return a list of sub-widgets composing the given string widget.
+
+        The sub-widgets are:
+            - the wrapper edit widget;
+            - the base edit widget;
+            - the caption text widget;
+            - the help widget (or None if not present);
+            - the error text widget (or None if not present);
+            - the generate text widget (or None if not present);
+            - the choices text widget (or None if not present);
+            - the default text widget (or None if not present).
+        """
+        help = error = generate = choices = default = None
+        # Retrieve the Pile contents ignoring the last Divider widget.
+        contents = list(widget.contents)[:-1]
+        first_widget = contents.pop(0)[0]
+        if isinstance(first_widget, urwid.Text):
+            # This is the error message.
+            error = first_widget
+            wrapper = contents.pop(0)[0]
+        else:
+            # The widget has no errors.
+            wrapper = first_widget
+        caption_attrs, edit_attrs = wrapper.base_widget.contents
+        if field.help:
+            help = contents.pop(0)[0]
+        if hasattr(field, 'generate'):
+            generate = contents.pop(0)[0]
+        if hasattr(field, 'choices'):
+            choices = contents.pop(0)[0]
+        if field.default is not None:
+            default = contents.pop(0)[0]
+        return (
+            wrapper, edit_attrs[0], caption_attrs[0],
+            help, error, generate, choices, default,
+        )
+
+    def test_widget_structure(self):
+        # The widget includes all the information about a field.
+        field = fields.StringField(
+            'first-name', label='first name', help='your first name',
+            default='Jean')
+        widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
+        wrapper, edit, caption, help, error, generate, choices, default = (
+            self.inspect_widget(widget, field))
+        # Since the field is not read-only, the widget is properly enabled.
+        self.assertNotIsInstance(wrapper, urwid.WidgetDisable)
+        # The edit widget is set to the given value.
+        self.assertEqual('Luc', edit.get_edit_text())
+        # The caption and help are properly set.
+        self.assertEqual('\N{BULLET} first name: ', caption.text)
+        self.assertEqual('your first name', help.text)
+        # The error is displayed.
+        self.assertEqual('invalid name', error.text)
+        # The field is not able to generate a value, and there are no choices.
+        self.assertIsNone(generate)
+        self.assertIsNone(choices)
+        # The default value is properly displayed.
+        self.assertEqual('default if not set: Jean', default.text)
+
+    def test_value_getter(self):
+        # The returned value getter function returns the current widget value.
+        field = fields.StringField('first-name')
+        widget, value_getter = forms.create_string_widget(field, 'Luc', None)
+        edit = self.inspect_widget(widget, field)[1]
+        self.assertEqual('Luc', value_getter())
+        # The value getter is lazy and always returns the current value.
+        edit.set_edit_text('Jean-Luc')
+        self.assertEqual('Jean-Luc', value_getter())
+
+    def test_unset_value(self):
+        # The initial value is set to an empty string if the field is unset.
+        field = fields.StringField('first-name')
+        _, value_getter = forms.create_string_widget(field, None, None)
+        self.assertEqual('', value_getter())
+
+    def test_value_not_a_string(self):
+        # Non-string values are converted to unicode strings.
+        field = fields.StringField('first-name')
+        _, value_getter = forms.create_string_widget(field, 42, None)
+        self.assertEqual('42', value_getter())
+
+    def test_readonly_field(self):
+        # The widget is disabled if the field is read-only.
+        field = fields.StringField('first-name', readonly=True)
+        widget, _ = forms.create_string_widget(field, 'Jean-Luc', None)
+        wrapper = self.inspect_widget(widget, field)[0]
+        self.assertIsInstance(wrapper, urwid.WidgetDisable)
+
+    def test_autogenerated_field(self):
+        # The widgets allows for automatically generating field values.
+        field = fields.AutoGeneratedStringField('password')
+        with mock.patch.object(field, 'generate', lambda: 'auto-generated!'):
+            widget, value_getter = forms.create_string_widget(field, '', None)
+        generate = self.inspect_widget(widget, field)[5]
+        # The generate button is the first widget in the urwid.Columns.
+        generate_button = generate.contents[0][0]
+        # Click the generate button, and ensure the value has been generated.
+        cli_helpers.emit(generate_button)
+        self.assertEqual('auto-generated!', value_getter())
+
+    def test_choices(self):
+        # Possible choices are properly displayed below the edit widget.
+        field = fields.ChoiceField('captain', choices=('Kirk', 'Picard'))
+        widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
+        choices = self.inspect_widget(widget, field)[6]
+        self.assertEqual(
+            'possible values are Kirk, Picard, '
+            'but this field can also be left empty',
+            choices.text)
+
+    def test_choices_required_field(self):
+        # Possible choices are properly displayed below the edit widget for
+        # required fields.
+        field = fields.ChoiceField(
+            'captain', required=True, choices=('Janeway', 'Sisko'))
+        widget, _ = forms.create_string_widget(field, 'Luc', 'invalid name')
+        choices = self.inspect_widget(widget, field)[6]
+        self.assertEqual('possible values are Janeway, Sisko', choices.text)
+
+
+class TestCreateBoolWidget(unittest.TestCase):
+
+    def inspect_widget(self, widget, field):
+        """Return a list of sub-widgets composing the given boolean widget.
+
+        The sub-widgets are:
+            - the checkbox widget;
+            - the help widget (or None if not present);
+            - the error text widget (or None if not present);
+        """
+        help = error = None
+        # Retrieve the Pile contents ignoring the last Divider widget.
+        contents = list(widget.contents)[:-1]
+        first_widget = contents.pop(0)[0]
+        if isinstance(first_widget, urwid.Text):
+            # This is the error message.
+            error = first_widget
+            checkbox = contents.pop(0)[0]
+        else:
+            # The widget has no errors.
+            checkbox = first_widget
+        if field.help:
+            help = contents.pop(0)[0]
+        return checkbox, help, error
+
+    def test_widget_structure(self):
+        # The widget includes all the information about a field.
+        field = fields.BoolField(
+            'enabled', label='is enabled', help='something is enabled')
+        widget, _ = forms.create_bool_widget(field, False, 'bad wolf')
+        checkbox, help, error = self.inspect_widget(widget, field)
+        # Since the field is not read-only, the widget is properly enabled.
+        self.assertNotIsInstance(checkbox, urwid.WidgetDisable)
+        # The checkbox widget is set to the given value.
+        self.assertFalse(checkbox.get_state())
+        # The help message is properly displayed.
+        self.assertEqual('something is enabled', help.text)
+        # The error is displayed.
+        self.assertEqual('bad wolf', error.text)
+
+    def test_value_getter(self):
+        # The returned value getter function returns the current widget value.
+        field = fields.BoolField('enabled')
+        widget, value_getter = forms.create_bool_widget(field, True, None)
+        checkbox = self.inspect_widget(widget, field)[0]
+        self.assertTrue(value_getter())
+        # The value getter is lazy and always returns the current value.
+        checkbox.set_state(False)
+        self.assertFalse(value_getter())
+
+    def test_allow_mixed(self):
+        # The checkbox has three possible states if allow_mixed is True.
+        field = fields.BoolField('enabled', allow_mixed=True)
+        widget, value_getter = forms.create_bool_widget(field, None, None)
+        checkbox = self.inspect_widget(widget, field)[0]
+        self.assertTrue(checkbox.has_mixed)
+        self.assertEqual(forms.MIXED, checkbox.get_state())
+        # A mixed value is converted to None when the value is retrieved.
+        self.assertIsNone(value_getter())
+
+    def test_mixed_not_allowed(self):
+        # The checkbox can only be in a True/False state if the field's
+        # allow_mixed is set to False.
+        field = fields.BoolField('enabled', allow_mixed=False, default=True)
+        widget, value_getter = forms.create_bool_widget(field, None, None)
+        checkbox = self.inspect_widget(widget, field)[0]
+        self.assertFalse(checkbox.has_mixed)
+        # The default value is used if the input value is unset and mixed state
+        # is not allowed.
+        self.assertTrue(checkbox.get_state())
+        # The retrieved value reflects the checkbox internal state.
+        self.assertTrue(value_getter())
+
+    def test_readonly_field(self):
+        # The widget is disabled if the field is read-only.
+        field = fields.BoolField('enabled', readonly=True)
+        widget, _ = forms.create_bool_widget(field, False, None)
+        checkbox = self.inspect_widget(widget, field)[0]
+        self.assertIsInstance(checkbox, urwid.WidgetDisable)
+
+
+class TestCreateForm(unittest.TestCase):
+
+    field_value_pairs = (
+        (fields.StringField('first-name'), 'Jean-Luc'),
+        (fields.BoolField('enabled'), True),
+    )
+
+    def test_form_creation(self):
+        # The create_form factory correctly creates and returns the form
+        # widgets for each field/value pair provided.
+        widgets, _ = forms.create_form(self.field_value_pairs, {})
+        self.assertEqual(2, len(widgets))
+        string_widget, bool_widget = widgets
+        caption = string_widget.contents[0][0].base_widget.contents[0][0].text
+        self.assertEqual('first-name: ', caption)
+        checkbox_label = bool_widget.contents[0][0].label
+        self.assertEqual('enabled', checkbox_label)
+
+    def test_values_getter(self):
+        # The returned values getter function returns the current form values.
+        widgets, values_getter = forms.create_form(self.field_value_pairs, {})
+        self.assertEqual(
+            {'enabled': True, 'first-name': u'Jean-Luc'}, values_getter())
+        # The values getter is lazy and always returns the current values.
+        bool_widget = widgets[1]
+        checkbox = bool_widget.contents[0][0]
+        checkbox.set_state(False)
+        self.assertEqual(
+            {'enabled': False, 'first-name': u'Jean-Luc'}, values_getter())
+
+    def test_error_message(self):
+        # The user is asked to correct the form errors.
+        errors = {'first-name': 'invalid name'}
+        widgets, _ = forms.create_form(self.field_value_pairs, errors)
+        message = widgets[0]
+        self.assertEqual('please correct the error below', message.text)
+
+    def test_multiple_error_messages(self):
+        # The user is asked to correct multiple form errors.
+        errors = {'first-name': 'invalid name', 'enabled': 'invalid value'}
+        widgets, _ = forms.create_form(self.field_value_pairs, errors)
+        message = widgets[0]
+        self.assertEqual('please correct the 2 errors below', message.text)
+
+    def test_errros(self):
+        # Widgets are created passing the corresponding errors.
+        errors = {'first-name': 'invalid name'}
+        widgets, _ = forms.create_form(self.field_value_pairs, errors)
+        # The first widget is the error message, the second is a divider.
+        string_widget = widgets[2]
+        error = string_widget.contents[0][0]
+        self.assertEqual('invalid name', error.text)
+
+
+class TestCreateActions(unittest.TestCase):
+
+    def setUp(self):
+        # Set up actions.
+        self.clicked = []
+        actions = [
+            ('load', ui.thunk(self.clicked.append, 'load clicked')),
+            ('save', ui.thunk(self.clicked.append, 'save clicked')),
+        ]
+        # Create the controls and retrieve the menu buttons.
+        controls = forms.create_actions(actions)
+        columns = controls.contents[1][0].base_widget
+        self.buttons = [widget.base_widget for widget, _ in columns.contents]
+
+    def test_resulting_controls(self):
+        # A menu button is created for each action.
+        self.assertEqual(2, len(self.buttons))
+        load_button, save_button = self.buttons
+        self.assertEqual('load', cli_helpers.get_button_caption(load_button))
+        self.assertEqual('save', cli_helpers.get_button_caption(save_button))
+
+    def test_callbacks(self):
+        # The action callbacks are properly attached to each menu button.
+        load_button, save_button = self.buttons
+        cli_helpers.emit(load_button)
+        self.assertEqual(['load clicked'], self.clicked)
+        cli_helpers.emit(save_button)
+        self.assertEqual(['load clicked', 'save clicked'], self.clicked)

=== modified file 'quickstart/tests/cli/test_views.py'
--- quickstart/tests/cli/test_views.py	2013-12-20 10:30:37 +0000
+++ quickstart/tests/cli/test_views.py	2014-01-03 11:38:58 +0000
@@ -26,6 +26,7 @@
 
 from quickstart.cli import (
     base,
+    forms,
     ui,
     views,
 )
@@ -96,7 +97,7 @@
         Use the filter_function argument to filter the returned list.
         """
         contents = self.app.get_contents()
-        return filter(filter_function, list(contents.body))
+        return filter(filter_function, list(contents.base_widget.body))
 
     def get_control_buttons(self):
         """Return the list of buttons included in a control box.
@@ -138,7 +139,7 @@
         env_db = helpers.make_env_db()
         views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
         self.assertEqual(
-            'Select the Juju environment you want to use',
+            'Select an existing Juju environment or create a new one',
             self.app.get_title())
 
     def test_view_title_no_environments(self):
@@ -149,17 +150,28 @@
             'No Juju environments already set up: please create one',
             self.app.get_title())
 
+    def test_view_contents(self):
+        # The view displays a list of the environments in env_db, and buttons
+        # to create new environments.
+        env_db = helpers.make_env_db()
+        views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
+        buttons = self.get_widgets_in_contents(
+            filter_function=self.is_a(ui.MenuButton))
+        # A button is created for each existing environment (see details) and
+        # for each environment type supported by quickstart (create).
+        env_types = envs.get_supported_env_types(self.env_type_db)
+        expected_buttons_number = len(env_db['environments']) + len(env_types)
+        self.assertEqual(expected_buttons_number, len(buttons))
+
     @mock.patch('quickstart.cli.views.env_detail')
-    def test_view_contents(self, mock_env_detail):
-        # The view displays a list of the environments in env_db.
+    def test_environment_clicked(self, mock_env_detail):
+        # The environment detail view is called when clicking an environment.
         env_db = helpers.make_env_db()
         views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # The environments are listed in alphabetical order.
         environments = sorted(env_db['environments'])
-        # A button is created for each environment.
-        self.assertEqual(len(environments), len(buttons))
         for env_name, button in zip(environments, buttons):
             env_data = envs.get_env_data(env_db, env_name)
             # The caption includes the environment description.
@@ -176,6 +188,30 @@
             # loop cycle.
             mock_env_detail.reset_mock()
 
+    @mock.patch('quickstart.cli.views.env_edit')
+    def test_create_new_environment_clicked(self, mock_env_edit):
+        # The environment edit view is called when clicking to create a new
+        # environment.
+        env_db = helpers.make_env_db()
+        views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
+        buttons = self.get_widgets_in_contents(
+            filter_function=self.is_a(ui.MenuButton))
+        env_types = envs.get_supported_env_types(self.env_type_db)
+        for env_type, button in zip(env_types, buttons[-len(env_types):]):
+            # The caption includes the environment type.
+            expected_caption = 'new {} environment'.format(env_type)
+            caption = cli_helpers.get_button_caption(button)
+            self.assertIn(expected_caption, caption)
+            # When the button is clicked, the edit view is called passing the
+            # corresponding environment data.
+            cli_helpers.emit(button)
+            mock_env_edit.assert_called_once_with(
+                self.app, self.env_type_db, env_db, self.save_callable,
+                {'type': env_type})
+            # Reset the mock so that it does not include any calls on the next
+            # loop cycle.
+            mock_env_edit.reset_mock()
+
     def test_selected_environment(self):
         # The default environment is already selected in the list.
         env_db = helpers.make_env_db(default='lxc')
@@ -261,28 +297,30 @@
             self.assertEqual(expected_text, widget.text)
 
     def test_view_buttons(self):
-        # The following buttons are displayed: "back", "use", "set default" and
-        # "remove".
+        # The following buttons are displayed: "back", "use", "set default",
+        # "edit" and "remove".
         self.call_view(env_name='ec2-west')
         buttons = self.get_control_buttons()
         captions = map(cli_helpers.get_button_caption, buttons)
-        self.assertEqual(['back', 'use', 'set default', 'remove'], captions)
+        self.assertEqual(
+            ['back', 'use', 'set default', 'edit', 'remove'], captions)
 
     def test_view_buttons_default(self):
         # If the environment is the default one, the "set default" button is
-        # not displayed. The buttons we expect are "back", "use" and "remove".
+        # not displayed. The buttons we expect are "back", "use", "edit" and
+        # "remove".
         self.call_view(env_name='lxc')
         buttons = self.get_control_buttons()
         captions = map(cli_helpers.get_button_caption, buttons)
-        self.assertEqual(['back', 'use', 'remove'], captions)
+        self.assertEqual(['back', 'use', 'edit', 'remove'], captions)
 
     def test_view_buttons_error(self):
         # If the environment is not valid, the "use" button is not displayed.
-        # The buttons we expect are "back", "set default" and "remove".
+        # The buttons we expect are "back", "set default", "edit" and "remove".
         self.call_view(env_name='local-with-errors')
         buttons = self.get_control_buttons()
         captions = map(cli_helpers.get_button_caption, buttons)
-        self.assertEqual(['back', 'set default', 'remove'], captions)
+        self.assertEqual(['back', 'set default', 'edit', 'remove'], captions)
 
     @mock.patch('quickstart.cli.views.env_index')
     def test_back_button(self, mock_env_index):
@@ -323,6 +361,17 @@
         # The new env_db has been saved.
         self.save_callable.assert_called_once_with(new_env_db)
 
+    @mock.patch('quickstart.cli.views.env_edit')
+    def test_edit_button(self, mock_env_edit):
+        # The edit view is called if the "edit" button is clicked.
+        self.call_view(env_name='ec2-west')
+        # The "edit" button is the fourth one.
+        edit_button = self.get_control_buttons()[3]
+        cli_helpers.emit(edit_button)
+        mock_env_edit.assert_called_once_with(
+            self.app, self.env_type_db, self.env_db, self.save_callable,
+            self.env_data)
+
     def test_remove_button(self):
         # A confirmation dialog is displayed if the "remove" button is clicked.
         self.call_view(env_name='ec2-west')
@@ -392,3 +441,213 @@
         self.call_view(env_name='lxc')
         status = self.app.get_status()
         self.assertEqual('', status)
+
+
+class TestEnvEdit(EnvViewTestsMixin, unittest.TestCase):
+
+    env_db = helpers.make_env_db(default='lxc')
+
+    def call_view(self, env_name='lxc', env_type=None):
+        """Call the view passing the env_data corresponding to env_name.
+
+        If env_type provider, the view is a creation form, env_name is ignored
+        and a new env_data is passed to the view.
+        """
+        if env_type is None:
+            self.env_data = envs.get_env_data(self.env_db, env_name)
+        else:
+            self.env_data = {'type': env_type}
+        return views.env_edit(
+            self.app, self.env_type_db, self.env_db, self.save_callable,
+            self.env_data)
+
+    def get_form_contents(self):
+        """Return the form contents included in the app page.
+
+        The contents are returned as a sequence of (caption, value) tuples.
+        """
+        pile_widgets = self.get_widgets_in_contents(
+            filter_function=self.is_a(urwid.Pile))
+        form_contents = []
+        for pile_widget in pile_widgets:
+            base_widget = pile_widget.contents[0][0].base_widget
+            if isinstance(base_widget, urwid.CheckBox):
+                # This is a boolean widget.
+                form_contents.append((
+                    base_widget.label, base_widget.get_state()))
+            elif hasattr(base_widget, 'contents'):
+                # This is a string widget.
+                caption = base_widget.contents[0][0].text
+                value = base_widget.contents[1][0].get_edit_text()
+                form_contents.append((caption, value))
+        return form_contents
+
+    def patch_create_form(self, changes=None):
+        """Patch the forms.create_form function.
+
+        The create_form function returns the form widgets and a callable
+        returning the new env data. Make the latter return the current
+        self.env_data instead, optionally updated using the given changes.
+        """
+        original_create_form = forms.create_form
+        testcase = self
+
+        class MockCreateForm(object):
+
+            call_count = 0
+            errors = None
+            new_env_data = None
+
+            def __call__(self, field_value_pairs, errors):
+                self.call_count += 1
+                self.errors = errors
+                self.new_env_data = testcase.env_data.copy()
+                if changes is not None:
+                    self.new_env_data.update(changes)
+                form_widgets, _ = original_create_form(
+                    field_value_pairs, errors)
+                return form_widgets, lambda: self.new_env_data
+
+            @property
+            def called(self):
+                return bool(self.call_count)
+
+        return mock.patch('quickstart.cli.forms.create_form', MockCreateForm())
+
+    def test_view_default_return_value_on_exit(self):
+        # The view configures the app so that the return value on user exit is
+        # a tuple including a copy of the given env_db and None, the latter
+        # meaning no environment has been selected (for now).
+        self.call_view()
+        new_env_db, env_data = self.get_on_exit_return_value(self.loop)
+        self.assertEqual(self.env_db, new_env_db)
+        self.assertIsNot(self.env_db, new_env_db)
+        self.assertIsNone(env_data)
+
+    def test_creation_view_title(self):
+        # The application title is correctly set up when the view is used to
+        # create a new environment.
+        self.call_view(env_type='ec2')
+        self.assertEqual('Create a new ec2 environment', self.app.get_title())
+
+    def test_modification_view_title(self):
+        # The application title is correctly set up when the view is used to
+        # change an existing environment.
+        self.call_view(env_name='lxc')
+        self.assertEqual('Edit the local environment', self.app.get_title())
+
+    def test_view_contents_description(self):
+        # The view displays the provider description.
+        self.call_view()
+        text_widgets = self.get_widgets_in_contents(
+            filter_function=self.is_a(urwid.Text))
+        description = text_widgets[0]
+        expected_description = self.env_type_db['local']['description']
+        self.assertEqual(expected_description, description.text)
+
+    def test_view_contents_form(self):
+        # The view displays a form containing all the environment fields.
+        self.call_view()
+        expected_form_contents = [
+            ('provider type: ', 'local'),
+            ('environment name: ', 'lxc'),
+            ('admin secret: ', 'bones'),
+            ('default series: ', 'raring'),
+            ('root dir: ', ''),
+            ('storage port: ', '8888'),
+            ('shared storage port: ', ''),
+            ('network bridge: ', ''),
+            ('default', True),
+        ]
+        self.assertEqual(expected_form_contents, self.get_form_contents())
+
+    def test_view_buttons(self):
+        # The following buttons are displayed: "save", "cancel" and "restore".
+        self.call_view(env_name='ec2-west')
+        buttons = self.get_control_buttons()
+        captions = map(cli_helpers.get_button_caption, buttons)
+        self.assertEqual(['save', 'cancel', 'restore'], captions)
+
+    @mock.patch('quickstart.cli.views.env_detail')
+    def test_save_button(self, mock_env_detail):
+        # The new data is saved if the user clicks the save button.
+        # Subsequently the user is redirected to the env detail view.
+        changes = {'admin-secret': 'Secret!'}
+        with self.patch_create_form(changes=changes) as mock_create_form:
+            self.call_view(env_name='ec2-west')
+        self.assertTrue(mock_create_form.called)
+        # The "save" button is the first one.
+        save_button = self.get_control_buttons()[0]
+        cli_helpers.emit(save_button)
+        # At this point the new env data should be normalized and saved into
+        # the environments database.
+        env_metadata = envs.get_env_metadata(self.env_type_db, self.env_data)
+        new_env_data = envs.normalize(
+            env_metadata, mock_create_form.new_env_data)
+        envs.set_env_data(self.env_db, 'ec2-west', new_env_data)
+        # The new data has been correctly saved.
+        self.save_callable.assert_called_once_with(self.env_db)
+        # A message notifies the environment has been saved.
+        self.assertEqual(
+            'ec2-west successfully modified', self.app.get_message())
+        # The application displays the environment detail view.
+        mock_env_detail.assert_called_once_with(
+            self.app, self.env_type_db, self.env_db, self.save_callable,
+            new_env_data)
+
+    def test_save_invalid_form_data(self):
+        # Errors are displayed if the user tries to save invalid data.
+        changes = {'admin-secret': ''}
+        with self.patch_create_form(changes=changes) as mock_create_form:
+            self.call_view(env_name='ec2-west')
+            self.assertTrue(mock_create_form.called)
+            # The "save" button is the first one.
+            save_button = self.get_control_buttons()[0]
+            cli_helpers.emit(save_button)
+        # In case of errors, the save callable is not called.
+        self.assertFalse(self.save_callable.called)
+        # The form has been re-rendered passing the errors.
+        self.assertEqual(2, mock_create_form.call_count)
+        self.assertEqual(
+            {'admin-secret': 'a value is required for the admin secret field'},
+            mock_create_form.errors)
+
+    def test_save_invalid_new_name(self):
+        # It is not allowed to save an environment with an already used name.
+        changes = {'name': 'lxc'}
+        with self.patch_create_form(changes=changes) as mock_create_form:
+            self.call_view(env_name='ec2-west')
+            self.assertTrue(mock_create_form.called)
+            # The "save" button is the first one.
+            save_button = self.get_control_buttons()[0]
+            cli_helpers.emit(save_button)
+        # In case of errors, the save callable is not called.
+        self.assertFalse(self.save_callable.called)
+        # The form has been re-rendered passing the errors.
+        self.assertEqual(2, mock_create_form.call_count)
+        self.assertEqual(
+            {'name': 'an environment with this name already exists'},
+            mock_create_form.errors)
+
+    @mock.patch('quickstart.cli.views.env_index')
+    def test_creation_view_cancel_button(self, mock_env_index):
+        # The index view is called if the "cancel" button is clicked while
+        # creating a new environment.
+        self.call_view(env_type='ec2')
+        # The "cancel" button is the second one.
+        cancel_button = self.get_control_buttons()[1]
+        cli_helpers.emit(cancel_button)
+        mock_env_index.assert_called_once_with(
+            self.app, self.env_type_db, self.env_db, self.save_callable)
+
+    @mock.patch('quickstart.cli.views.env_detail')
+    def test_modification_view_cancel_button(self, mock_env_detail):
+        # The index view is called if the "cancel" button is clicked while
+        # creating a new environment.
+        self.call_view(env_name='ec2-west')
+        # The "cancel" button is the second one.
+        cancel_button = self.get_control_buttons()[1]
+        cli_helpers.emit(cancel_button)
+        mock_env_detail.assert_called_once_with(
+            self.app, self.env_type_db, self.env_db, self.save_callable,
+            self.env_data)

=== modified file 'quickstart/tests/models/test_envs.py'
--- quickstart/tests/models/test_envs.py	2013-12-19 17:00:36 +0000
+++ quickstart/tests/models/test_envs.py	2014-01-03 11:38:58 +0000
@@ -386,6 +386,20 @@
             self.env_db['environments']['new-one'])
         self.assertEqual('new-one', self.env_db['default'])
 
+    def test_new_environment_with_no_default(self):
+        # A new environment is properly added in an env_db with no default.
+        env_data = {
+            'default-series': 'edgy',
+            'is-default': False,
+            'name': 'new-one',
+        }
+        del self.env_db['default']
+        envs.set_env_data(self.env_db, None, env_data)
+        self.assertEqual(
+            {'default-series': 'edgy'},
+            self.env_db['environments']['new-one'])
+        self.assertNotIn('default', self.env_db)
+
     def test_existing_environment_updated(self):
         # An existing environment is properly updated.
         env_data = {