networks: Improvements to interface binding

- Fetch network interfaces list at the time of form creation instead of
  during definition.

- Add help text to interfaces list choice field.

- Don't write an explict check for requiring interface
  selection. required=True, which is default, takes care of that.

- Internationalize the empty selection text.

- Update the empty selection text.

- Re-order interface parameter in the same order of priority/appearance.

plinth/modules/networks/forms.py

@@ -22,7 +22,17 @@ from gettext import gettext as _
 from plinth import network
 from gi.repository import NM as nm
 
-DEFAULT_SELECT_MSG = 'please select'
+
+def _get_interface_choices(device_type):
+    """Return a list of choices for a given device type."""
+    interfaces = network.get_interface_list(device_type)
+    choices = [('', _('-- select --'))]
+    for interface, mac in interfaces.items():
+        display_string = _('{interface} ({mac})').format(interface=interface,
+                                                         mac=mac)
+        choices.append((interface, display_string))
+
+    return choices
 
 
 class ConnectionTypeSelectForm(forms.Form):
@@ -35,17 +45,12 @@ class ConnectionTypeSelectForm(forms.Form):
 
 class AddEthernetForm(forms.Form):
     """Form to create a new ethernet connection."""
-    interfaces = network.get_interface_list(nm.DeviceType.ETHERNET)
-    interfaces_list = (('', DEFAULT_SELECT_MSG), )
-    for interface, mac in interfaces.items():
-        displaystring = str(interface + ' (' + mac + ')')
-        newentry = interfaces_list + ((interface, displaystring), )
-        interfaces_list = newentry
-
     name = forms.CharField(label=_('Connection Name'))
     interface = forms.ChoiceField(
         label=_('Physical Interface'),
-        choices=interfaces_list)
+        choices=(),
+        help_text=_('The network device that this connection should be bound '
+                    'to.'))
     zone = forms.ChoiceField(
         label=_('Firewall Zone'),
         help_text=_('The firewall zone will control which services are \
@@ -61,26 +66,21 @@ available over this interfaces. Select Internal only for trusted networks.'),
         validators=[validators.validate_ipv4_address],
         required=False)
 
-    def clean(self):
+    def __init__(self, *args, **kwargs):
-        """ validate the form fields """
+        """Initialize the form, populate interface choices."""
-        cleaned_data = super(forms.Form, self).clean()
+        super(AddEthernetForm, self).__init__(*args, **kwargs)
-        if DEFAULT_SELECT_MSG == cleaned_data.get('interface'):
+        choices = _get_interface_choices(nm.DeviceType.ETHERNET)
-            raise forms.ValidationError('Please select a interface to be used')
+        self.fields['interface'].choices = choices
 
 
 class AddWifiForm(forms.Form):
     """Form to create a new wifi connection."""
-    interfaces = network.get_interface_list(nm.DeviceType.WIFI)
-    interfaces_list = (('', DEFAULT_SELECT_MSG), )
-    for interface, mac in interfaces.items():
-        displaystring = str(interface + ' (' + mac + ')')
-        newentry = interfaces_list + ((interface, displaystring), )
-        interfaces_list = newentry
-
     name = forms.CharField(label=_('Connection Name'))
     interface = forms.ChoiceField(
         label=_('Physical interface'),
-        choices=interfaces_list)
+        choices=(),
+        help_text=_('The network device that this connection should be bound '
+                    'to.'))
     zone = forms.ChoiceField(
         label=_('Firewall Zone'),
         help_text=_('The firewall zone will control which services are \
@@ -116,8 +116,8 @@ Point.'))
         validators=[validators.validate_ipv4_address],
         required=False)
 
-    def clean(self):
+    def __init__(self, *args, **kwargs):
-        """ validate the form fields """
+        """Initialize the form, populate interface choices."""
-        cleaned_data = super(forms.Form, self).clean()
+        super(AddWifiForm, self).__init__(*args, **kwargs)
-        if DEFAULT_SELECT_MSG == cleaned_data.get('interface'):
+        choices = _get_interface_choices(nm.DeviceType.WIFI)
-            raise forms.ValidationError('Please select a interface to be used')
+        self.fields['interface'].choices = choices

plinth/modules/networks/networks.py

@@ -75,16 +75,15 @@ def edit(request, uuid):
 
         if form.is_valid():
             name = form.cleaned_data['name']
+            interface = form.cleaned_data['interface']
             zone = form.cleaned_data['zone']
             ipv4_method = form.cleaned_data['ipv4_method']
             ipv4_address = form.cleaned_data['ipv4_address']
-            interface = form.cleaned_data['interface']
 
             if connection.get_connection_type() == '802-3-ethernet':
                 network.edit_ethernet_connection(
-                    connection,
+                    connection, name, interface, zone, ipv4_method,
-                    name, zone,
+                    ipv4_address)
-                    ipv4_method, ipv4_address, interface)
             elif connection.get_connection_type() == '802-11-wireless':
                 ssid = form.cleaned_data['ssid']
                 mode = form.cleaned_data['mode']
@@ -92,9 +91,8 @@ def edit(request, uuid):
                 passphrase = form.cleaned_data['passphrase']
 
                 network.edit_wifi_connection(
-                    connection, name, zone,
+                    connection, name, interface, zone, ssid, mode, auth_mode,
-                    ssid, mode, auth_mode, passphrase,
+                    passphrase, ipv4_method, ipv4_address)
-                    ipv4_method, ipv4_address, interface)
 
             return redirect(reverse_lazy('networks:index'))
         else:
@@ -105,13 +103,12 @@ def edit(request, uuid):
     else:
         settings_connection = connection.get_setting_connection()
         settings_ipv4 = connection.get_setting_ip4_config()
+        form_data['interface'] = connection.get_interface_name()
         try:
             form_data['zone'] = settings_connection.get_zone()
         except KeyError:
             form_data['zone'] = 'external'
 
-        form_data['interface'] = connection.get_interface_name()
-
         form_data['ipv4_method'] = settings_ipv4.get_method()
 
         if settings_ipv4.get_num_addresses():
@@ -215,14 +212,13 @@ def add_ethernet(request):
         form = AddEthernetForm(request.POST)
         if form.is_valid():
             name = form.cleaned_data['name']
+            interface = form.cleaned_data['interface']
             zone = form.cleaned_data['zone']
             ipv4_method = form.cleaned_data['ipv4_method']
             ipv4_address = form.cleaned_data['ipv4_address']
-            interface = form.cleaned_data['interface']
 
             network.add_ethernet_connection(
-                name, zone,
+                name, interface, zone, ipv4_method, ipv4_address)
-                ipv4_method, ipv4_address, interface)
             return redirect(reverse_lazy('networks:index'))
     else:
         form = AddEthernetForm()
@@ -250,6 +246,7 @@ def add_wifi(request, ssid=None):
         form = AddWifiForm(request.POST)
         if form.is_valid():
             name = form.cleaned_data['name']
+            interface = form.cleaned_data['interface']
             zone = form.cleaned_data['zone']
             ssid = form.cleaned_data['ssid']
             mode = form.cleaned_data['mode']
@@ -257,12 +254,10 @@ def add_wifi(request, ssid=None):
             passphrase = form.cleaned_data['passphrase']
             ipv4_method = form.cleaned_data['ipv4_method']
             ipv4_address = form.cleaned_data['ipv4_address']
-            interface = form.cleaned_data['interface']
 
             network.add_wifi_connection(
-                name, zone,
+                name, interface, zone, ssid, mode, auth_mode, passphrase,
-                ssid, mode, auth_mode, passphrase,
+                ipv4_method, ipv4_address)
-                ipv4_method, ipv4_address, interface)
             return redirect(reverse_lazy('networks:index'))
     else:
         if form_data:

plinth/network.py

@@ -64,12 +64,13 @@ def _commit_callback(connection, error, data=None):
     del data
 
 
-def get_interface_list(type):
+def get_interface_list(device_type):
-    """Get a list of Linux devices which are available on this system"""
+    """Get a list of network interface available on the system."""
     interfaces = {}
     for device in nm.Client.new(None).get_devices():
-        if device.get_device_type() == type:
+        if device.get_device_type() == device_type:
             interfaces[device.get_iface()] = device.get_hw_address()
+
     return interfaces
 
 
@@ -126,8 +127,8 @@ def get_active_connection(connection_uuid):
         raise ConnectionNotFound(connection_uuid)
 
 
-def _update_common_settings(connection, connection_uuid, name, type_, zone,
+def _update_common_settings(connection, connection_uuid, name, type_, interface,
-                            ipv4_method, ipv4_address, interface):
+                            zone, ipv4_method, ipv4_address):
     """Create/edit basic settings for network manager connections."""
     if not connection:
         connection = nm.SimpleConnection.new()
@@ -138,11 +139,12 @@ def _update_common_settings(connection, connection_uuid, name, type_, zone,
         settings = nm.SettingConnection.new()
         connection.add_setting(settings)
 
+    settings.set_property(nm.SETTING_CONNECTION_UUID, connection_uuid)
     settings.set_property(nm.SETTING_CONNECTION_ID, name)
     settings.set_property(nm.SETTING_CONNECTION_TYPE, type_)
-    settings.set_property(nm.SETTING_CONNECTION_ZONE, zone)
-    settings.set_property(nm.SETTING_CONNECTION_UUID, connection_uuid)
     settings.set_property(nm.SETTING_CONNECTION_INTERFACE_NAME, interface)
+    settings.set_property(nm.SETTING_CONNECTION_ZONE, zone)
+
     # IPv4
     settings = connection.get_setting_ip4_config()
     if not settings:
@@ -164,14 +166,14 @@ def _update_common_settings(connection, connection_uuid, name, type_, zone,
     return connection
 
 
-def _update_ethernet_settings(connection, connection_uuid, name, zone,
+def _update_ethernet_settings(connection, connection_uuid, name, interface,
-                              ipv4_method, ipv4_address, interface):
+                              zone, ipv4_method, ipv4_address):
     """Create/edit ethernet settings for network manager connections."""
     type_ = '802-3-ethernet'
 
     connection = _update_common_settings(connection, connection_uuid, name,
-                                         type_, zone, ipv4_method,
+                                         type_, interface, zone, ipv4_method,
-                                         ipv4_address, interface)
+                                         ipv4_address)
 
     # Ethernet
     settings = connection.get_setting_wired()
@@ -182,39 +184,39 @@ def _update_ethernet_settings(connection, connection_uuid, name, zone,
     return connection
 
 
-def add_ethernet_connection(name, zone, ipv4_method, ipv4_address, interface):
+def add_ethernet_connection(name, interface, zone, ipv4_method, ipv4_address):
     """Add an automatic ethernet connection in network manager.
 
     Return the UUID for the connection.
     """
     connection_uuid = str(uuid.uuid4())
     connection = _update_ethernet_settings(
-        None, connection_uuid, name, zone, ipv4_method,
+        None, connection_uuid, name, interface, zone, ipv4_method,
-        ipv4_address, interface)
+        ipv4_address)
     client = nm.Client.new(None)
     client.add_connection_async(connection, True, None, _callback, None)
     return connection_uuid
 
 
-def edit_ethernet_connection(connection, name, zone, ipv4_method,
+def edit_ethernet_connection(connection, name, interface, zone, ipv4_method,
-                             ipv4_address, interface):
+                             ipv4_address):
     """Edit an existing ethernet connection in network manager."""
     _update_ethernet_settings(
-        connection, connection.get_uuid(), name, zone, ipv4_method,
+        connection, connection.get_uuid(), name, interface, zone, ipv4_method,
-        ipv4_address, interface)
+        ipv4_address)
     connection.commit_changes(True)
 
 
-def _update_wifi_settings(connection, connection_uuid, name, zone, ssid, mode,
+def _update_wifi_settings(connection, connection_uuid, name, interface, zone,
-                          auth_mode, passphrase, ipv4_method,
+                          ssid, mode, auth_mode, passphrase, ipv4_method,
-                          ipv4_address, interface):
+                          ipv4_address):
     """Create/edit wifi settings for network manager connections."""
     type_ = '802-11-wireless'
     key_mgmt = 'wpa-psk'
 
     connection = _update_common_settings(connection, connection_uuid, name,
-                                         type_, zone, ipv4_method,
+                                         type_, interface, zone, ipv4_method,
-                                         ipv4_address, interface)
+                                         ipv4_address)
 
     # Wireless
     settings = connection.get_setting_wireless()
@@ -241,28 +243,27 @@ def _update_wifi_settings(connection, connection_uuid, name, zone, ssid, mode,
     return connection
 
 
-def add_wifi_connection(name, zone, ssid, mode, auth_mode, passphrase,
+def add_wifi_connection(name, interface, zone, ssid, mode, auth_mode,
-                        ipv4_method, ipv4_address, interface):
+                        passphrase, ipv4_method, ipv4_address):
     """Add an automatic Wi-Fi connection in network manager.
 
     Return the UUID for the connection.
     """
     connection_uuid = str(uuid.uuid4())
     connection = _update_wifi_settings(
-        None, connection_uuid, name, zone, ssid, mode, auth_mode, passphrase,
+        None, connection_uuid, name, interface, zone, ssid, mode, auth_mode,
-        ipv4_method, ipv4_address, interface)
+        passphrase, ipv4_method, ipv4_address)
     client = nm.Client.new(None)
     client.add_connection_async(connection, True, None, _callback, None)
     return connection_uuid
 
 
-def edit_wifi_connection(connection, name, zone,
+def edit_wifi_connection(connection, name, interface, zone, ssid, mode,
-                         ssid, mode, auth_mode, passphrase,
+                         auth_mode, passphrase, ipv4_method, ipv4_address):
-                         ipv4_method, ipv4_address, interface):
     """Edit an existing wifi connection in network manager."""
     _update_wifi_settings(
-        connection, connection.get_uuid(), name, zone, ssid, mode, auth_mode,
+        connection, connection.get_uuid(), name, interface, zone, ssid, mode,
-        passphrase, ipv4_method, ipv4_address, interface)
+        auth_mode, passphrase, ipv4_method, ipv4_address)
     connection.commit_changes(True)