launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14152
[Merge] lp:~rvb/maas/manage-mirrors into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/manage-mirrors into lp:maas.
Commit message:
Add page to manage (delete or add) archives.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/manage-mirrors/+merge/133867
This branch changes the "Add custom archives" page into a "Manage custom archives pages": instead of simply featuring a form to add new archives, the page now also contains the list of the built-in and the custom archives along with a link to delete custom archives.
= Notes =
Screenshot: http://people.canonical.com/~rvb/settings-manage-archives.png
This was pre-imp'ed with Jeroen and we decided it would be good to forbid the deletion of the default (built-in) archives.
In case you wonder, the deleted test in src/maasserver/tests/test_views_settings.py has simply been moved to src/maasserver/tests/test_views_settings_manage_archives.py.
--
https://code.launchpad.net/~rvb/maas/manage-mirrors/+merge/133867
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/manage-mirrors into lp:maas.
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-11-09 17:09:07 +0000
+++ src/maasserver/forms.py 2012-11-12 08:54:21 +0000
@@ -691,15 +691,24 @@
class AddArchiveForm(ConfigForm):
archive_name = HostnameFormField(label="Archive name")
+ def clean_archive_name(self):
+ """Check that the archive does not already exist."""
+ archive_name = self.cleaned_data['archive_name']
+ archive_choice = [archive_name, archive_name]
+ archive_choices = Config.objects.get_config('archive_choices')
+ if archive_choice in archive_choices:
+ raise forms.ValidationError("Archive already exists.")
+ return archive_name
+
def save(self):
"""Save the archive name in the Config table.
This implementation of `save` does not support the `commit` argument.
"""
archive_name = self.cleaned_data.get('archive_name')
- archives = Config.objects.get_config('archive_choices')
- archives.append([archive_name, archive_name])
- Config.objects.set_config('archive_choices', archives)
+ archive_choices = Config.objects.get_config('archive_choices')
+ archive_choices.append([archive_name, archive_name])
+ Config.objects.set_config('archive_choices', archive_choices)
class NodeGroupInterfaceForm(ModelForm):
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/models/config.py 2012-11-12 08:54:21 +0000
@@ -34,6 +34,23 @@
from provisioningserver.enum import POWER_TYPE
+# Built-in archives that can't be removed.
+builtin_archive_choices = [
+ [
+ 'http://archive.ubuntu.com/ubuntu',
+ 'http://archive.ubuntu.com/ubuntu',
+ ],
+ [
+ 'http://ports.ubuntu.com/ubuntu-ports',
+ 'http://ports.ubuntu.com/ubuntu-ports',
+ ],
+ [
+ 'https://maas.ubuntu.com/images',
+ 'https://maas.ubuntu.com/images',
+ ],
+]
+
+
def get_default_config():
return {
## settings default values.
@@ -45,22 +62,7 @@
'fallback_master_archive': False,
'keep_mirror_list_uptodate': False,
'fetch_new_releases': False,
- 'archive_choices': (
- [
- [
- 'http://archive.ubuntu.com/ubuntu',
- 'http://archive.ubuntu.com/ubuntu',
- ],
- [
- 'http://ports.ubuntu.com/ubuntu-ports',
- 'http://ports.ubuntu.com/ubuntu-ports',
- ],
- [
- 'https://maas.ubuntu.com/images',
- 'https://maas.ubuntu.com/images',
- ],
- ]
- ),
+ 'archive_choices': builtin_archive_choices,
'main_archive': 'http://archive.ubuntu.com/ubuntu',
'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports',
'cloud_images_archive': 'https://maas.ubuntu.com/images',
=== modified file 'src/maasserver/templates/maasserver/settings.html'
--- src/maasserver/templates/maasserver/settings.html 2012-11-09 09:11:51 +0000
+++ src/maasserver/templates/maasserver/settings.html 2012-11-12 08:54:21 +0000
@@ -108,10 +108,10 @@
{% include "maasserver/form_field.html" %}
{% endwith %}
<li>
- <a href="{% url "settings-add-archive" %}">
+ <a href="{% url "settings-manage-archives" %}">
<img src="{{ STATIC_URL }}img/inline_add.png"
alt="Add" class="icon" />
- Add custom archive
+ Manage custom archives
</a>
</li>
</ul>
=== added file 'src/maasserver/templates/maasserver/settings_archive_delete.html'
--- src/maasserver/templates/maasserver/settings_archive_delete.html 1970-01-01 00:00:00 +0000
+++ src/maasserver/templates/maasserver/settings_archive_delete.html 2012-11-12 08:54:21 +0000
@@ -0,0 +1,23 @@
+{% extends "maasserver/base.html" %}
+
+{% block nav-active-settings %}active{% endblock %}
+{% block title %}Delete Archive{% endblock %}
+{% block page-title %}Delete Archive{% endblock %}
+
+{% block content %}
+ <div class="block auto-width">
+ <h2>
+ Are you sure you want to delete the archive "{{ archive }}"?
+ </h2>
+ <p>This action is permanent and can not be undone.</p>
+ <p>
+ <form action="{% url 'settings-delete-archive'%}?archive={{archive|urlencode:""}}"
+ id='delete-archive'
+ method="post">{% csrf_token %}
+ <input type="submit" value="Delete archive" class="right" />
+ <a href="{% url 'settings-manage-archives' %}">Cancel</a>
+ </form>
+ </p>
+ </div>
+{% endblock %}
+
=== renamed file 'src/maasserver/templates/maasserver/settings_add_archive.html' => 'src/maasserver/templates/maasserver/settings_manage_archives.html'
--- src/maasserver/templates/maasserver/settings_add_archive.html 2012-06-28 12:05:09 +0000
+++ src/maasserver/templates/maasserver/settings_manage_archives.html 2012-11-12 08:54:21 +0000
@@ -1,10 +1,37 @@
{% extends "maasserver/base.html" %}
{% block nav-active-settings %}active{% endblock %}
-{% block title %}Add archive{% endblock %}
-{% block page-title %}Add archive{% endblock %}
+{% block title %}Manage archives{% endblock %}
+{% block page-title %}Manage archives{% endblock %}
{% block content %}
+ <div class="block size7 first">
+ <h2>Manage archives</h2>
+ <h3>Built-in archives</h2>
+ <table class="list" id="builtin-archive-list">
+ {% for archive, _ in builtin_archive_choices %}
+ <tr class="{% cycle 'even' 'odd' %}">
+ <td class="archive">{{ archive }}</td>
+ </tr>
+ {% endfor %}
+ </table>
+ <h3>Custom archives</h3>
+ <table class="list" id="custom-archive-list">
+ {% for archive, _ in custom_archive_choices %}
+ <tr class="{% cycle 'even' 'odd' %}">
+ <td class="archive">{{ archive }}</td>
+ <td>
+ <a title="Delete archive {{ archive }}"
+ href="{% url 'settings-delete-archive'%}?archive={{archive|urlencode:""}}">
+ <img src="{{ STATIC_URL }}img/delete.png" alt="delete" />
+ </a>
+ </td>
+ </tr>
+ {% empty %}
+ No custom archive.
+ {% endfor %}
+ </table>
+ <h2>Add custom archive</h2>
<form action="." method="post" class="block auto-width">
{% csrf_token %}
<ul>
@@ -15,4 +42,5 @@
<input type="submit" class="right" value="Add archive" />
<a class="link-button" href="{% url 'settings' %}">Cancel</a>
</form>
+ </div>
{% endblock %}
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-11-09 17:07:11 +0000
+++ src/maasserver/tests/test_forms.py 2012-11-12 08:54:21 +0000
@@ -30,6 +30,7 @@
NODEGROUPINTERFACE_MANAGEMENT,
)
from maasserver.forms import (
+ AddArchiveForm,
AdminNodeForm,
AdminNodeWithMACAddressesForm,
ConfigForm,
@@ -992,3 +993,14 @@
self.assertTrue(form.is_valid())
form.save()
self.assertEqual(data['name'], reload_object(nodegroup).name)
+
+
+class TestAddArchiveForm(TestCase):
+
+ def test_refuses_duplicate_archives(self):
+ archive_choices = Config.objects.get_config('archive_choices')
+ existing_archive = archive_choices[0][0]
+ form = AddArchiveForm(data={'archive_name': existing_archive})
+ self.assertFalse(form.is_valid())
+ self.assertEqual(
+ "Archive already exists.", form._errors['archive_name'][0])
=== modified file 'src/maasserver/tests/test_views_settings.py'
--- src/maasserver/tests/test_views_settings.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/tests/test_views_settings.py 2012-11-12 08:54:21 +0000
@@ -171,19 +171,6 @@
Config.objects.get_config('default_distro_series'),
))
- def test_settings_add_archive_POST(self):
- choices = Config.objects.get_config('archive_choices')
- response = self.client.post(
- '/settings/archives/add/',
- data={'archive_name': 'http://my.hostname.com'}
- )
- new_choices = Config.objects.get_config('archive_choices')
-
- self.assertEqual(httplib.FOUND, response.status_code, response.content)
- self.assertItemsEqual(
- choices + [['http://my.hostname.com', 'http://my.hostname.com']],
- new_choices)
-
def test_settings_kernelopts_POST(self):
new_kernel_opts = "--new='arg' --flag=1 other"
response = self.client.post(
=== added file 'src/maasserver/tests/test_views_settings_manage_archives.py'
--- src/maasserver/tests/test_views_settings_manage_archives.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_views_settings_manage_archives.py 2012-11-12 08:54:21 +0000
@@ -0,0 +1,143 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test maasserver settings views."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+import httplib
+from urllib import urlencode
+
+from django.core.urlresolvers import reverse
+from lxml.html import fromstring
+from maasserver.models import Config
+from maasserver.models.config import builtin_archive_choices
+from maasserver.testing import extract_redirect
+from maasserver.testing.testcase import AdminLoggedInTestCase
+
+
+def setup_custom_archives():
+ custom_archives = [
+ 'http://my.hostname.com/ubuntu',
+ 'https://example.com/custom/archive'
+ ]
+ archive_choices = Config.objects.get_config('archive_choices')
+ for custom_archive in custom_archives:
+ archive_choices.append([custom_archive, custom_archive])
+ Config.objects.set_config('archive_choices', archive_choices)
+ return custom_archives
+
+
+class SettingsManageArchiveTest(AdminLoggedInTestCase):
+
+ def test_page_displays_built_in_archives(self):
+ archives = [choice[0] for choice in builtin_archive_choices]
+ response = self.client.get(reverse('settings-manage-archives'))
+ document = fromstring(response.content)
+ archive_list = [
+ arch_row.text_content()
+ for arch_row in document.xpath(
+ "//table[@id='builtin-archive-list']//td[@class='archive']")]
+ self.assertItemsEqual(archives, archive_list)
+
+ def test_page_displays_custom_archives(self):
+ custom_archives = setup_custom_archives()
+ response = self.client.get(reverse('settings-manage-archives'))
+ document = fromstring(response.content)
+ archive_list = [
+ arch_row.text_content()
+ for arch_row in document.xpath(
+ "//table[@id='custom-archive-list']//td[@class='archive']")]
+ self.assertItemsEqual(custom_archives, archive_list)
+
+ def test_page_displays_links_to_delete_custom_archives(self):
+ custom_archives = setup_custom_archives()
+ response = self.client.get(reverse('settings-manage-archives'))
+ document = fromstring(response.content)
+ link_list = [
+ link for link in document.xpath(
+ "//table[@id='custom-archive-list']//td/a/@href")]
+ delete_links = [
+ '%s?%s' % (
+ reverse('settings-delete-archive'),
+ urlencode({'archive': archive}))
+ for archive in custom_archives
+ ]
+ self.assertItemsEqual(delete_links, link_list)
+
+ def test_page_adds_archive_POST(self):
+ choices = Config.objects.get_config('archive_choices')
+ response = self.client.post(
+ reverse('settings-manage-archives'),
+ data={'archive_name': 'http://my.hostname.com'}
+ )
+ new_choices = Config.objects.get_config('archive_choices')
+
+ self.assertEqual(httplib.FOUND, response.status_code, response.content)
+ self.assertItemsEqual(
+ choices + [['http://my.hostname.com', 'http://my.hostname.com']],
+ new_choices)
+
+
+class SettingsDeleteArchiveTest(AdminLoggedInTestCase):
+
+ def test_page_delete_archive_POST(self):
+ custom_archives = setup_custom_archives()
+ archive_choices = Config.objects.get_config('archive_choices')
+ archive_to_delete = custom_archives.pop()
+ delete_link = '%s?%s' % (
+ reverse('settings-delete-archive'),
+ urlencode({'archive': archive_to_delete}))
+ response = self.client.post(delete_link)
+ self.assertEqual(httplib.FOUND, response.status_code, response.content)
+ archive_choices.remove([archive_to_delete, archive_to_delete]),
+ self.assertItemsEqual(
+ archive_choices,
+ Config.objects.get_config('archive_choices'))
+
+ def test_page_posts_to_self(self):
+ custom_archives = setup_custom_archives()
+ archive_to_delete = custom_archives.pop()
+ delete_link = '%s?%s' % (
+ reverse('settings-delete-archive'),
+ urlencode({'archive': archive_to_delete}))
+ response = self.client.get(delete_link)
+ document = fromstring(response.content)
+ form_post_links = document.xpath(
+ "//form[@id='delete-archive']/@action")
+ self.assertEqual(
+ (httplib.OK, [delete_link]),
+ (response.status_code, form_post_links))
+
+ def test_page_errors_when_deleting_unknown_archive(self):
+ delete_link = '%s?%s' % (
+ reverse('settings-delete-archive'),
+ urlencode({'archive': 'http://unknown.archive'}))
+ response = self.client.post(delete_link)
+ response = self.client.get(extract_redirect(response))
+ self.assertEqual(
+ ["Unknown archive: 'http://unknown.archive'."],
+ [message.message for message in response.context['messages']])
+
+ def test_page_errors_when_deleting_builtin_archive(self):
+ delete_link = '%s?%s' % (
+ reverse('settings-delete-archive'),
+ urlencode({'archive': builtin_archive_choices[0][0]}))
+ response = self.client.post(delete_link)
+ response = self.client.get(extract_redirect(response))
+ self.assertEqual(
+ (
+ ["Cannot remove a built-in archive."],
+ builtin_archive_choices,
+ ),
+ (
+ [message.message for message in response.context['messages']],
+ Config.objects.get_config('archive_choices'),
+ ))
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-10-15 09:32:06 +0000
+++ src/maasserver/urls.py 2012-11-12 08:54:21 +0000
@@ -46,7 +46,8 @@
AccountsEdit,
AccountsView,
settings,
- settings_add_archive,
+ settings_delete_archive,
+ settings_manage_archives,
)
from maasserver.views.settings_clusters import (
ClusterDelete,
@@ -158,8 +159,11 @@
# /XXX
adminurl(r'^settings/$', settings, name='settings'),
adminurl(
- r'^settings/archives/add/$', settings_add_archive,
- name='settings-add-archive'),
+ r'^settings/archives/manage/$', settings_manage_archives,
+ name='settings-manage-archives'),
+ adminurl(
+ r'^settings/archives/delete/$',
+ settings_delete_archive, name='settings-delete-archive'),
adminurl(r'^accounts/add/$', AccountsAdd.as_view(), name='accounts-add'),
adminurl(
r'^accounts/(?P<username>\w+)/edit/$', AccountsEdit.as_view(),
=== modified file 'src/maasserver/views/settings.py'
--- src/maasserver/views/settings.py 2012-11-08 10:40:48 +0000
+++ src/maasserver/views/settings.py 2012-11-12 08:54:21 +0000
@@ -16,7 +16,8 @@
"AccountsEdit",
"AccountsView",
"settings",
- "settings_add_archive",
+ "settings_manage_archives",
+ "settings_delete_archive",
]
from django.contrib import messages
@@ -43,15 +44,17 @@
AddArchiveForm,
CommissioningForm,
EditUserForm,
+ GlobalKernelOptsForm,
MAASAndNetworkForm,
NewUserCreationForm,
UbuntuForm,
- GlobalKernelOptsForm,
)
from maasserver.models import (
+ Config,
NodeGroup,
UserProfile,
)
+from maasserver.models.config import builtin_archive_choices
from maasserver.views import process_form
@@ -230,17 +233,55 @@
context_instance=RequestContext(request))
-def settings_add_archive(request):
+def settings_manage_archives(request):
+ archive_choices = Config.objects.get_config('archive_choices')
+ custom_archive_choices = [
+ archive_choice for archive_choice in archive_choices
+ if archive_choice not in builtin_archive_choices]
if request.method == 'POST':
form = AddArchiveForm(request.POST)
if form.is_valid():
form.save()
messages.info(request, "Archive added.")
- return HttpResponseRedirect(reverse('settings'))
+ return HttpResponseRedirect(reverse('settings-manage-archives'))
else:
form = AddArchiveForm()
-
- return render_to_response(
- 'maasserver/settings_add_archive.html',
- {'form': form},
+ context = {
+ 'form': form,
+ 'custom_archive_choices': custom_archive_choices,
+ 'builtin_archive_choices': builtin_archive_choices,
+ }
+ return render_to_response(
+ 'maasserver/settings_manage_archives.html',
+ context,
+ context_instance=RequestContext(request))
+
+
+def settings_delete_archive(request):
+ archive_choices = Config.objects.get_config('archive_choices')
+ archive = request.GET.get('archive')
+ archive_choice = [archive, archive]
+
+ # Check that the selected archive is not a built-in archive.
+ if archive_choice in builtin_archive_choices:
+ messages.info(request, "Cannot remove a built-in archive.")
+ return HttpResponseRedirect(reverse('settings-manage-archives'))
+
+ # Check that the selected archive is in the list of archives.
+ if archive_choice not in archive_choices:
+ messages.info(request, "Unknown archive: '%s'." % archive)
+ return HttpResponseRedirect(reverse('settings-manage-archives'))
+
+ if request.method == 'POST':
+ archive_choices.remove([archive, archive])
+ Config.objects.set_config('archive_choices', archive_choices)
+ messages.info(request, "Archive removed.")
+ return HttpResponseRedirect(reverse('settings-manage-archives'))
+
+ context = {
+ 'archive': archive,
+ }
+ return render_to_response(
+ 'maasserver/settings_archive_delete.html',
+ context,
context_instance=RequestContext(request))
