launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06514
[Merge] lp:~rvb/maas/maas-remove-tabs2 into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-remove-tabs2 into lp:maas with lp:~huwshimi/maas/interface-elements as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-remove-tabs2/+merge/94801
This branch removes the JS tabs on the settings & prefs pages. Not that most of the code changed is a simple indentation change.
To do so I had to:
- remove the call to create YUI's tabs.
- reformat the HTML to remove the markup to create the tabs.
- removes the redirections to specific tabs (tab=?) in the view code.
- cleans up the mass.utils module and the related tests (the module is now empty but I've left it because I'm sure we will have other things that will fit in there soon).
Drive-by fix: I've also refactored the way the forms are displayed on those pages to use the new template: maasserver/form_field.html.
e.g.:
- {{ password_form.as_p }}
+ {% for field in password_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
--
https://code.launchpad.net/~rvb/maas/maas-remove-tabs2/+merge/94801
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-remove-tabs2 into lp:maas.
=== modified file 'src/maasserver/static/css/forms.css'
--- src/maasserver/static/css/forms.css 2012-02-22 07:03:13 +0000
+++ src/maasserver/static/css/forms.css 2012-02-28 06:53:18 +0000
@@ -44,6 +44,9 @@
li.error input[type="text"] {
background-color: #F2CFCE;
}
+a.button:hover {
+ text-decoration: none;
+ }
.yui3-button,
.button,
button,
=== modified file 'src/maasserver/static/js/tests/test_utils.html'
--- src/maasserver/static/js/tests/test_utils.html 2012-02-20 11:07:00 +0000
+++ src/maasserver/static/js/tests/test_utils.html 2012-02-28 06:53:18 +0000
@@ -15,16 +15,5 @@
<body>
<span id="suite">maas.utils.tests</span>
<div id="placeholder"></div>
- <script type="text/x-template" id="tabs_template"></script>
- <div id="tabs">
- <ul>
- <li><a href="#tab1">Tab1</a></li>
- <li><a href="#tab2">Tab2</a></li>
- </ul>
- <div>
- <div id="tab1">Tab1 content</div>
- <div id="tab2">Tab2 content</div>
- </div>
- </script>
</body>
</html>
=== modified file 'src/maasserver/static/js/tests/test_utils.js'
--- src/maasserver/static/js/tests/test_utils.js 2012-02-20 11:07:00 +0000
+++ src/maasserver/static/js/tests/test_utils.js 2012-02-28 06:53:18 +0000
@@ -10,53 +10,13 @@
var module = Y.maas.utils;
var suite = new Y.Test.Suite("maas.utils Tests");
-var tabs_template = Y.one('#tabs_template').getContent();
-
suite.add(new Y.maas.testing.TestCase({
- name: 'test-utils',
-
- createTabs: function() {
- // Create a tab with 2 tabs (valid indexes: 0 or 1).
- Y.one("body").append(Y.Node.create(tabs_template));
- var tabs = new Y.TabView({srcNode: '#tabs'});
- tabs.render();
- return tabs;
- },
-
- testgetTabIndex_index_empty: function() {
- var tabs = this.createTabs();
- var index = module.getTabIndex('', tabs);
- Y.Assert.isNull(index);
- },
-
- testgetTabIndex_index_invalid: function() {
- var tabs = this.createTabs();
- var index = module.getTabIndex('invalid!', tabs);
- Y.Assert.isNull(index);
- },
-
- testgetTabIndex_index_too_big: function() {
- var tabs = this.createTabs();
- var index = module.getTabIndex('2', tabs);
- Y.Assert.isNull(index);
- },
-
- testgetTabIndex_index_neg: function() {
- var tabs = this.createTabs();
- var index = module.getTabIndex('-1', tabs);
- Y.Assert.isNull(index);
- },
-
- testgetTabIndex_index_ok: function() {
- var tabs = this.createTabs();
- var index = module.getTabIndex('0', tabs);
- Y.Assert.areEqual(0, index);
- }
+ name: 'test-utils'
}));
namespace.suite = suite;
}, '0.1', {'requires': [
- 'tabview', 'node', 'test', 'maas.testing', 'maas.utils']}
+ 'node', 'test', 'maas.testing', 'maas.utils']}
);
=== modified file 'src/maasserver/static/js/utils.js'
--- src/maasserver/static/js/utils.js 2012-02-24 17:37:01 +0000
+++ src/maasserver/static/js/utils.js 2012-02-28 06:53:18 +0000
@@ -11,25 +11,5 @@
Y.log('loading mass.utils');
var module = Y.namespace('maas.utils');
-/**
- * Return a valid tab (integer) index if the string 'index' contains a valid
- * tab index for the given 'tabview'. Otherwise, return 'null'.
- *
- * @method getTabIndex
- */
-module.getTabIndex = function(index, tabview) {
- var tab_index = parseInt(index, 10);
- var valid_tab_index = (
- Y.Lang.isValue(tab_index) &&
- tab_index >= 0 &&
- tab_index < tabview.size());
- if (valid_tab_index) {
- return tab_index;
- }
- else {
- return null;
- }
-};
-
}, '0.1', {'requires': ['base']}
);
=== modified file 'src/maasserver/templates/maasserver/prefs.html'
--- src/maasserver/templates/maasserver/prefs.html 2012-02-20 11:02:48 +0000
+++ src/maasserver/templates/maasserver/prefs.html 2012-02-28 06:53:18 +0000
@@ -3,22 +3,12 @@
{% block nav-active-prefs %}active{% endblock %}
{% block title %}User preferences for {{ user.username }}{% endblock %}
{% block page-title %}User preferences for {{ user.username }}{% endblock %}
+{% block layout-modifiers %}sidebar{% endblock %}
{% block head %}
<script type="text/javascript">
<!--
- YUI().use('tabview', 'maas.prefs', 'maas.utils', function (Y) {
- var tabview = new Y.TabView({
- srcNode: '#prefs'
- });
- tabview.render();
- // Select the tab with index {{ tab }} (which is taken from the
- // query string and set by the Django view).
- var valid_tab_index = Y.maas.utils.getTabIndex('{{ tab }}', tabview);
- if (Y.Lang.isValue(valid_tab_index)) {
- tabview.selectChild(valid_tab_index);
- }
-
+ YUI().use('maas.prefs', function (Y) {
var profile_widget = new Y.maas.prefs.TokenWidget(
{'srcNode': Y.one('#api')});
profile_widget.render();
@@ -29,64 +19,65 @@
{% block content %}
<div id="prefs">
- <ul>
- <li><a href="#profile">Profile</a></li>
- <li><a href="#api">API access</a></li>
- <li><a href="#password">Change password</a></li>
- <li><a href="#keys">Manage keys</a></li>
- </ul>
- <div>
- <div id="profile">
- <form action="." method="post">
- {{ profile_form.as_p }}
- <input type="hidden" name="profile_submit" value="1" />
- <input type="submit" value="Save" />
- </form>
- </div>
- <div id="api">
- <p>
- An API token is the information you need to provide to a third
- party application for it to be able to access the MaaS server
- <a href="{% url 'api-doc' %}">API</a> on your behalf.
- </p>
- <p>
- You can create as many API tokens as you require.
- </p>
- <table>
- <thead>
- <tr>
- <th>API tokens</th>
- <th></th>
- </tr>
- </thead>
- <tbody>
- {% for token in user.get_profile.get_authorisation_tokens %}
- <tr class="bundle">
- <td id="{{ token.key }}">
- {{ token.consumer.key }}:{{ token.key }}:{{ token.secret }}</td>
- <td>
- <a href="#" class="delete-link">
- <img title="Delete token"
- src="{{ STATIC_URL }}img/delete.png" />
- </a>
- </td>
- </tr>
- {% endfor %}
- </tbody>
- </table>
- <p id="token_creation_placeholder" />
- </div>
- <div id="password">
- <form action="." method="post">
- {{ password_form.as_p }}
- <input type="hidden" name="password_submit" value="1" />
- <input type="submit" value="Change password" />
- </form>
- </div>
- <div id="keys">
- TODO manage keys.
- </div>
+ <div id="api">
+ <h2>MaaS keys</h2>
+ <p>
+ An API token is the information you need to provide to a third
+ party application for it to be able to access the MaaS server
+ <a href="{% url 'api-doc' %}">API</a> on your behalf.
+ </p>
+ <p>
+ You can create as many API tokens as you require.
+ </p>
+ <table class="list">
+ <thead>
+ <tr>
+ <th>API tokens</th>
+ <th></th>
+ </tr>
+ </thead>
+ <tbody>
+ {% for token in user.get_profile.get_authorisation_tokens %}
+ <tr class="bundle">
+ <td id="{{ token.key }}">
+ {{ token.consumer.key }}:{{ token.key }}:{{ token.secret }}</td>
+ <td>
+ <a href="#" class="delete-link">
+ <img title="Delete token"
+ src="{{ STATIC_URL }}img/delete.png" />
+ </a>
+ </td>
+ </tr>
+ {% endfor %}
+ </tbody>
+ </table>
+ <p id="token_creation_placeholder" />
+ </div>
+ <div id="profile">
+ <h2>User details</h2>
+ <form action="." method="post">
+ <ul>
+ {% for field in profile_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ </ul>
+ <input type="hidden" name="profile_submit" value="1" />
+ <input type="submit" class="button right" value="Save" />
+ <div class="clear" />
+ </form>
+ </div>
+ <div id="password">
+ <h2>Password</h2>
+ <form action="." method="post">
+ <ul>
+ {% for field in password_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
+ </ul>
+ <input type="hidden" name="password_submit" value="1" />
+ <input type="submit" class="button right" value="Change password" />
+ <div class="clear" />
+ </form>
</div>
</div>
-
{% endblock %}
=== modified file 'src/maasserver/templates/maasserver/settings.html'
--- src/maasserver/templates/maasserver/settings.html 2012-02-27 04:03:26 +0000
+++ src/maasserver/templates/maasserver/settings.html 2012-02-28 06:53:18 +0000
@@ -4,89 +4,66 @@
{% block title %}Settings{% endblock %}
{% block page-title %}Settings{% endblock %}
+{% block layout-modifiers %}sidebar{% endblock %}
{% block head %}
- <script type="text/javascript">
- <!--
- YUI().use('tabview', 'maas.utils', 'io-form', function (Y) {
- var tabview = new Y.TabView({
- srcNode: '#settings'
- });
- tabview.render();
- // Select the tab with index {{ tab }}.
- var valid_tab_index = Y.maas.utils.getTabIndex('{{ tab }}', tabview);
- if (Y.Lang.isValue(valid_tab_index)) {
- tabview.selectChild(valid_tab_index);
- }
-
- });
- // -->
- </script>
{% endblock %}
{% block content %}
<div id="settings">
- <ul>
- <li><a href="#users">Users</a></li>
- <li><a href="#maas">MaaS</a></li>
- </ul>
- <div>
- <div id="users">
- <h2>Users and keys</h2>
- <table class="list">
- <thead>
- <tr>
- <th>ID</th>
- <th>Number of nodes in use</th>
- <th>Last seen</th>
- <th>MaaS Admin</th>
- <th></th>
- </tr>
- </thead>
- <tbody>
+ <div id="users">
+ <h2>Users and Keys</h2>
+ <table class="list">
+ <thead>
+ <tr>
+ <th>ID</th>
+ <th>Number of nodes in use</th>
+ <th>Last seen</th>
+ <th>MaaS Admin</th>
+ <th></th>
+ </tr>
+ </thead>
+ <tbody>
{% for user_item in user_list %}
- <tr class="user" id="{{ user_item.username }}">
- <td>
- <a class="user"
- href="{% url 'accounts-view' user_item.username %}">
- {{ user_item.username }}
- </a>
- </td>
- <td>{{ user_item.node_set.count }}</td>
- <td>{{ user_item.last_login }}</td>
- <td>
- {% if user_item.is_superuser %}
- <img src="{{ STATIC_URL }}img/yes.png" />
- {% endif %}
- </td>
- <td>
- <a href="{% url 'accounts-edit' user_item.username %}"
- title="Edit user {{ user_item.username }}">
- <img src="{{ STATIC_URL }}img/edit.png" />
- </a>
- {% if user != user_item %}
- <a title="Delete user {{ user_item.username }}"
- class="delete-user"
- href="{% url 'accounts-del' user_item.username %}">
- <img src="{{ STATIC_URL }}img/delete.png" />
- </a>
- <form method="POST"
- action="{% url 'accounts-del' user_item.username %}">
- <input type="hidden" name="username"
- value="{{ user_item.username }}" />
- </form>
- {% endif %}
- </td>
- </tr>
+ <tr class="user" id="{{ user_item.username }}">
+ <td>
+ <a class="user"
+ href="{% url 'accounts-view' user_item.username %}">
+ {{ user_item.username }}
+ </a>
+ </td>
+ <td>{{ user_item.node_set.count }}</td>
+ <td>{{ user_item.last_login }}</td>
+ <td>
+ {% if user_item.is_superuser %}
+ <img src="{{ STATIC_URL }}img/yes.png" />
+ {% endif %}
+ </td>
+ <td>
+ <a href="{% url 'accounts-edit' user_item.username %}"
+ title="Edit user {{ user_item.username }}">
+ <img src="{{ STATIC_URL }}img/edit.png" />
+ </a>
+ {% if user != user_item %}
+ <a title="Delete user {{ user_item.username }}"
+ class="delete-user"
+ href="{% url 'accounts-del' user_item.username %}">
+ <img src="{{ STATIC_URL }}img/delete.png" />
+ </a>
+ <form method="POST"
+ action="{% url 'accounts-del' user_item.username %}">
+ <input type="hidden" name="username"
+ value="{{ user_item.username }}" />
+ </form>
+ {% endif %}
+ </td>
+ </tr>
{% endfor %}
- </tbody>
- </table>
- <a href="{% url 'accounts-add' %}">Add user</a>
- </div>
- <div id="maas">
- #TODO MaaS prefs
- </div>
+ </tbody>
+ </table>
+ <br />
+ <a class="button" href="{% url 'accounts-add' %}">
+ <button class="button right">Add user</button></a>
</div>
</div>
-
{% endblock %}
=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py 2012-02-16 16:34:05 +0000
+++ src/maasserver/tests/test_views.py 2012-02-28 06:53:18 +0000
@@ -26,8 +26,8 @@
class UserPrefsViewTest(LoggedInTestCase):
def test_prefs_GET_profile(self):
- # The preferences page (profile tab) displays a form with the
- # user's personal information.
+ # The preferences page displays a form with the user's personal
+ # information.
user = self.logged_in_user
user.first_name = 'Steve'
user.last_name = 'Bam'
@@ -44,12 +44,12 @@
doc.cssselect('input#id_profile-first_name')])
def test_prefs_GET_api(self):
- # The preferences page (api tab) displays the API access tokens.
+ # The preferences page displays the API access tokens.
user = self.logged_in_user
# Create a few tokens.
for i in xrange(3):
user.get_profile().create_authorisation_token()
- response = self.client.get('/account/prefs/?tab=1')
+ response = self.client.get('/account/prefs/')
doc = fromstring(response.content)
# The OAuth tokens are displayed.
for token in user.get_profile().get_authorisation_tokens():
@@ -107,9 +107,9 @@
class SettingsTest(AdminLoggedInTestCase):
def test_settings_list_users(self):
- # The settings page (users tab) displays a list of the users with
- # links to view, delete or edit each user.
- # Note that the link to delete the the logged-in user is not display.
+ # The settings page displays a list of the users with links to view,
+ # delete or edit each user. Note that the link to delete the the
+ # logged-in user is not display.
[factory.make_user() for i in range(3)]
users = UserProfile.objects.all_users()
response = self.client.get('/settings/')
=== modified file 'src/maasserver/views.py'
--- src/maasserver/views.py 2012-02-17 16:06:43 +0000
+++ src/maasserver/views.py 2012-02-28 06:53:18 +0000
@@ -68,28 +68,25 @@
def userprefsview(request):
user = request.user
- tab = request.GET.get('tab', 0)
# Process the profile update form.
if 'profile_submit' in request.POST:
- tab = 0
profile_form = ProfileForm(
request.POST, instance=user, prefix='profile')
if profile_form.is_valid():
messages.info(request, "Profile updated.")
profile_form.save()
- return HttpResponseRedirect('%s?tab=%d' % (reverse('prefs'), tab))
+ return HttpResponseRedirect(reverse('prefs'))
else:
profile_form = ProfileForm(instance=user, prefix='profile')
# Process the password change form.
if 'password_submit' in request.POST:
- tab = 2
password_form = PasswordForm(
data=request.POST, user=user, prefix='password')
if password_form.is_valid():
messages.info(request, "Password updated.")
password_form.save()
- return HttpResponseRedirect('%s?tab=%d' % (reverse('prefs'), tab))
+ return HttpResponseRedirect(reverse('prefs'))
else:
password_form = PasswordForm(user=user, prefix='password')
@@ -98,7 +95,6 @@
{
'profile_form': profile_form,
'password_form': password_form,
- 'tab': tab # Tab index to display.
},
context_instance=RequestContext(request))
@@ -168,12 +164,10 @@
def settings(request):
- tab = request.GET.get('tab', 0)
user_list = UserProfile.objects.all_users().order_by('username')
return render_to_response(
'maasserver/settings.html',
{
'user_list': user_list,
- 'tab': tab # Tab index to display.
},
context_instance=RequestContext(request))
