[PATCH installer v4 24/30] tui: network: factor out name pinning into advanced options dialog

[Christoph Heiss <c.heiss@proxmox.com>]

This dialog can now be easily extended with additional panels for other,
advanced network-related options.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v3 -> v4:
  * no changes

Changes v2 -> v3:
  * no changes

Changes v1 -> v2:
  * no changes

 proxinstall                                |   4 +-
 proxmox-tui-installer/src/views/network.rs | 272 +++++++++++----------
 2 files changed, 140 insertions(+), 136 deletions(-)

diff --git a/proxinstall b/proxinstall
index 2622871..079b227 100755
--- a/proxinstall
+++ b/proxinstall
@@ -395,12 +395,12 @@ my sub create_network_interface_pin_frame {
 
     my $enable_checkbox = Gtk3::CheckButton->new('Pin network interface names');
     $enable_checkbox->set_active($gtk_state->{network_pinning_enabled});
-    $vbox->pack_start($enable_checkbox, 1, 0, 5);
+    $vbox->pack_start($enable_checkbox, 0, 0, 5);
 
     my $grid = Gtk3::Grid->new();
     $grid->set_column_spacing(10);
     $grid->set_row_spacing(10);
-    $vbox->pack_start($grid, 1, 1, 5);
+    $vbox->pack_start($grid, 0, 0, 5);
 
     my $interfaces = Proxmox::Install::RunEnv::get()->{network}->{interfaces};
     my $mapping = Proxmox::Install::Config::get_network_interface_pin_map();
diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index 6401174..f040881 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -1,8 +1,9 @@
+use anyhow::{Result, anyhow, bail};
 use cursive::{
     Cursive, View,
     view::{Nameable, Resizable, ViewWrapper},
     views::{
-        Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, ResizedView,
+        Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, Panel, ResizedView,
         ScrollView, SelectView, TextView,
     },
 };
@@ -22,10 +23,10 @@ use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
 use super::{CidrAddressEditView, FormView};
 
 struct NetworkViewOptions {
-    selected_mac: MacAddress,
     pinning_enabled: bool,
-    // For UI purposes, we want to always save the mapping, to save the state
-    // between toggling the checkbox
+    interfaces: Vec<Interface>,
+    // For UI purposes, we want to always save the state for the below options, even if not enabled
+    // by the user - to preserve the state between toggling the checkbox
     pinning_options: NetworkInterfacePinningOptions,
 }
 
@@ -40,32 +41,29 @@ pub struct NetworkOptionsView {
 }
 
 impl NetworkOptionsView {
-    const PINNING_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+    const FORM_VIEW_NAME: &str = "network-management-form";
     const MGMT_IFNAME_SELECTVIEW_NAME: &str = "network-management-ifname-selectview";
+    const ADVANCED_OPTIONS_BUTTON_NAME: &str = "network-pinning-options-button";
+
+    const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
 
     pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
-        let mut ifaces = network_info
+        let mut interfaces = network_info
             .interfaces
             .values()
-            .collect::<Vec<&Interface>>();
+            .cloned()
+            .collect::<Vec<Interface>>();
 
         // First, sort interfaces by their link state and then name
-        ifaces.sort_unstable_by_key(|x| (&x.state, &x.name));
-
-        let selected_mac = network_info
-            .interfaces
-            .get(&options.ifname)
-            .map(|iface| iface.mac)
-            .unwrap_or_else(|| ifaces.first().expect("at least one network interface").mac);
+        interfaces.sort_unstable_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name)));
 
         let options_ref = Arc::new(Mutex::new(NetworkViewOptions {
-            selected_mac,
             pinning_enabled: options.pinning_opts.is_some(),
+            interfaces,
             pinning_options: options.pinning_opts.clone().unwrap_or_default(),
         }));
 
-        let iface_selection =
-            Self::build_mgmt_ifname_selectview(ifaces.clone(), options_ref.clone());
+        let iface_selection = Self::build_mgmt_ifname_selectview(options_ref.clone(), None);
 
         let form = FormView::<()>::new()
             .child(
@@ -89,36 +87,13 @@ impl NetworkOptionsView {
                 EditView::new().content(options.dns_server.to_string()),
             );
 
-        let pinning_checkbox = LinearLayout::horizontal()
-            .child(Checkbox::new().checked().on_change({
-                let ifaces = ifaces
-                    .iter()
-                    .map(|iface| (*iface).clone())
-                    .collect::<Vec<Interface>>();
-                let options_ref = options_ref.clone();
-                move |siv, enable_pinning| {
-                    siv.call_on_name(Self::PINNING_OPTIONS_BUTTON_NAME, {
-                        let options_ref = options_ref.clone();
-                        move |view: &mut Button| {
-                            view.set_enabled(enable_pinning);
-
-                            options_ref.lock().expect("unpoisoned lock").pinning_enabled =
-                                enable_pinning;
-                        }
-                    });
-
-                    Self::refresh_ifname_selectview(siv, &ifaces, options_ref.clone());
-                }
-            }))
-            .child(TextView::new(" Pin network interface names").no_wrap())
+        let advanced_options = LinearLayout::horizontal()
             .child(DummyView.full_width())
             .child(
-                Button::new("Pinning options", {
+                Button::new("Advanced options", {
                     let options_ref = options_ref.clone();
-                    let network_info = network_info.clone();
                     move |siv| {
-                        let mut view =
-                            Self::custom_name_mapping_view(&network_info, options_ref.clone());
+                        let mut view = Self::advanced_options_view(options_ref.clone());
 
                         // Pre-compute the child's layout, since it might depend on the size. Without this,
                         // the view will be empty until focused.
@@ -128,13 +103,13 @@ impl NetworkOptionsView {
                         siv.add_layer(view);
                     }
                 })
-                .with_name(Self::PINNING_OPTIONS_BUTTON_NAME),
+                .with_name(Self::ADVANCED_OPTIONS_BUTTON_NAME),
             );
 
         let view = LinearLayout::vertical()
-            .child(form)
+            .child(form.with_name(Self::FORM_VIEW_NAME))
             .child(DummyView.full_width())
-            .child(pinning_checkbox);
+            .child(advanced_options);
 
         Self {
             view,
@@ -145,13 +120,10 @@ impl NetworkOptionsView {
     pub fn get_values(&mut self) -> Result<NetworkOptions, String> {
         let form = self
             .view
-            .get_child(0)
-            .and_then(|v| v.downcast_ref::<FormView>())
-            .ok_or("failed to retrieve network options form")?;
-
-        let iface = form
-            .get_value::<NamedView<SelectView<Interface>>, _>(0)
-            .ok_or("failed to retrieve management interface name")?;
+            .get_child_mut(0)
+            .and_then(|v| v.downcast_mut::<NamedView<FormView>>())
+            .ok_or("failed to retrieve network options form")?
+            .get_mut();
 
         let fqdn = form
             .get_value::<EditView, _>(1)
@@ -208,83 +180,69 @@ impl NetworkOptionsView {
         }
     }
 
-    fn custom_name_mapping_view(
-        network_info: &NetworkInfo,
-        options_ref: NetworkViewOptionsRef,
-    ) -> impl View {
-        const DIALOG_NAME: &str = "network-interface-name-pinning-dialog";
+    fn advanced_options_view(options_ref: NetworkViewOptionsRef) -> impl View {
+        let inner = ScrollView::new(
+            LinearLayout::vertical().child(Panel::new(
+                InterfacePinningOptionsView::new(options_ref.clone())
+                    .with_name(Self::INTERFACE_PINNING_VIEW_NAME),
+            )),
+        );
 
-        let mut interfaces = network_info
-            .interfaces
-            .values()
-            .collect::<Vec<&Interface>>();
-
-        interfaces.sort_by(|a, b| (&a.state, &a.name).cmp(&(&b.state, &b.name)));
-
-        Dialog::around(InterfacePinningOptionsView::new(
-            &interfaces,
-            options_ref.clone(),
-        ))
-        .title("Interface Name Pinning Options")
-        .button("Ok", {
-            let interfaces = interfaces
-                .iter()
-                .map(|v| (*v).clone())
-                .collect::<Vec<Interface>>();
-            move |siv| {
-                let options = siv
-                    .call_on_name(DIALOG_NAME, |view: &mut Dialog| {
-                        view.get_content_mut()
-                            .downcast_mut::<InterfacePinningOptionsView>()
-                            .map(InterfacePinningOptionsView::get_values)
-                    })
-                    .flatten();
-
-                let options = match options {
-                    Some(Ok(options)) => options,
-                    Some(Err(err)) => {
-                        siv.add_layer(Dialog::info(err));
-                        return;
+        Dialog::around(inner)
+            .title("Advanced Network Options")
+            .button("Ok", {
+                move |siv| {
+                    match Self::get_view_values(siv) {
+                        Ok(pin_opts) => {
+                            siv.pop_layer();
+                            let options = &mut options_ref.lock().expect("unpoisoned lock");
+                            options.pinning_options = pin_opts;
+                        }
+                        Err(err) => {
+                            siv.pop_layer();
+                            siv.add_layer(Dialog::info(err.to_string()));
+                            return;
+                        }
                     }
-                    None => {
-                        siv.add_layer(Dialog::info(
-                            "Failed to retrieve network interface name pinning options view",
-                        ));
-                        return;
-                    }
-                };
 
-                siv.pop_layer();
-                options_ref.lock().expect("unpoisoned lock").pinning_options = options;
-
-                Self::refresh_ifname_selectview(siv, &interfaces, options_ref.clone());
-            }
-        })
-        .with_name(DIALOG_NAME)
-        .max_size((80, 40))
+                    Self::refresh_ifname_selectview(siv, options_ref.clone());
+                }
+            })
+            .max_size((80, 40))
     }
 
-    fn refresh_ifname_selectview(
-        siv: &mut Cursive,
-        ifaces: &[Interface],
-        options_ref: NetworkViewOptionsRef,
-    ) {
+    fn get_view_values(siv: &mut Cursive) -> Result<NetworkInterfacePinningOptions> {
+        let pin_opts = siv
+            .call_on_name(
+                Self::INTERFACE_PINNING_VIEW_NAME,
+                InterfacePinningOptionsView::get_values,
+            )
+            .unwrap_or_else(|| {
+                bail!("failed to retrieve network interface name pinning options")
+            })?;
+
+        Ok(pin_opts)
+    }
+
+    fn refresh_ifname_selectview(siv: &mut Cursive, options_ref: NetworkViewOptionsRef) {
         siv.call_on_name(
             Self::MGMT_IFNAME_SELECTVIEW_NAME,
             |view: &mut SelectView<Interface>| {
-                *view = Self::build_mgmt_ifname_selectview(ifaces.iter().collect(), options_ref);
+                let selected_mac = view.selection().map(|iface| iface.mac);
+                *view = Self::build_mgmt_ifname_selectview(options_ref, selected_mac);
             },
         );
     }
 
     fn build_mgmt_ifname_selectview(
-        ifaces: Vec<&Interface>,
         options_ref: NetworkViewOptionsRef,
+        selected_mac: Option<MacAddress>,
     ) -> SelectView<Interface> {
         let options = options_ref.lock().expect("unpoisoned lock");
 
         // Map all interfaces to a list of (human-readable interface name, [Interface]) pairs
-        let ifnames = ifaces
+        let ifnames = options
+            .interfaces
             .iter()
             .map(|iface| {
                 if options.pinning_enabled
@@ -297,20 +255,20 @@ impl NetworkOptionsView {
             })
             .collect::<Vec<(String, Interface)>>();
 
-        let mut view = SelectView::new()
-            .popup()
-            .with_all(ifnames.clone())
-            .on_submit({
-                let options_ref = options_ref.clone();
-                move |_, iface| {
-                    options_ref.lock().expect("unpoisoned lock").selected_mac = iface.mac;
-                }
-            });
+        let mut view = SelectView::new().popup().with_all(ifnames.clone());
+
+        let selected_mac = selected_mac.unwrap_or_else(|| {
+            options
+                .interfaces
+                .first()
+                .expect("at least one network interface")
+                .mac
+        });
 
         // Finally, (try to) select the current one
         let selected = view
             .iter()
-            .position(|(_label, iface)| iface.mac == options.selected_mac)
+            .position(|(_label, iface)| iface.mac == selected_mac)
             .unwrap_or(0); // we sort UP interfaces first, so select the first UP interface
         //
         view.set_selection(selected);
@@ -324,19 +282,44 @@ impl ViewWrapper for NetworkOptionsView {
 }
 
 struct InterfacePinningOptionsView {
-    view: ScrollView<NamedView<FormView<MacAddress>>>,
+    view: LinearLayout,
 }
 
 impl InterfacePinningOptionsView {
+    const ENABLED_CHECKBOX_NAME: &str = "network-interface-name-pinning-enabled";
     const FORM_NAME: &str = "network-interface-name-pinning-form";
 
-    fn new(interfaces: &[&Interface], options_ref: NetworkViewOptionsRef) -> Self {
+    fn new(options_ref: NetworkViewOptionsRef) -> Self {
         let options = options_ref.lock().expect("unpoisoned lock");
 
         // Filter out all non-physical links, as it does not make sense to pin their names
         // in this way.
         // The low-level installer will skip them anyway.
-        let interfaces = interfaces.iter().filter(|iface| iface.pinned_id.is_some());
+        let interfaces = options
+            .interfaces
+            .iter()
+            .filter(|iface| iface.pinned_id.is_some());
+
+        let mut view = LinearLayout::vertical()
+            .child(
+                LinearLayout::horizontal()
+                    .child(
+                        Checkbox::new()
+                            .with_checked(options.pinning_enabled)
+                            .on_change({
+                                let options_ref = options_ref.clone();
+                                move |siv, enabled| {
+                                    options_ref.lock().expect("unpoisoned lock").pinning_enabled =
+                                        enabled;
+
+                                    Self::toggle_all(siv, enabled);
+                                }
+                            })
+                            .with_name(Self::ENABLED_CHECKBOX_NAME),
+                    )
+                    .child(TextView::new(" Pin network interface names").no_wrap()),
+            )
+            .child(DummyView::new());
 
         let mut form = FormView::<MacAddress>::new();
 
@@ -356,7 +339,7 @@ impl InterfacePinningOptionsView {
                                 .expect("always pinnable interface")
                                 .name,
                         )
-                        .max_content_width(MAX_IFNAME_LEN)
+                        .with_enabled(options.pinning_enabled)
                         .fixed_width(MAX_IFNAME_LEN),
                 );
 
@@ -375,13 +358,31 @@ impl InterfacePinningOptionsView {
             }
         }
 
-        Self {
-            view: ScrollView::new(form.with_name(Self::FORM_NAME)),
-        }
+        view.add_child(form.with_name(Self::FORM_NAME));
+
+        Self { view }
     }
 
-    fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions, String> {
-        let form = self.view.get_inner_mut().get_mut();
+    fn toggle_all(siv: &mut Cursive, enabled: bool) {
+        siv.call_on_name(Self::FORM_NAME, |v: &mut FormView<MacAddress>| {
+            v.call_on_childs(&|v: &mut LinearLayout| {
+                if let Some(v) = v
+                    .get_child_mut(1)
+                    .and_then(|v| v.downcast_mut::<ResizedView<EditView>>())
+                {
+                    v.get_inner_mut().set_enabled(enabled);
+                }
+            });
+        });
+    }
+
+    fn get_values(&mut self) -> Result<NetworkInterfacePinningOptions> {
+        let form = self
+            .view
+            .get_child_mut(2)
+            .and_then(|v| v.downcast_mut::<NamedView<FormView<MacAddress>>>())
+            .ok_or(anyhow!("failed to retrieve interface pinning form"))?
+            .get_mut();
 
         let mut mapping = HashMap::new();
 
@@ -395,18 +396,21 @@ impl InterfacePinningOptionsView {
                 .get_child(1)
                 .and_then(|v| v.downcast_ref::<ResizedView<EditView>>())
                 .map(|v| v.get_inner().get_content())
-                .ok_or_else(|| format!("failed to retrieve pinning ID for interface {}", mac))?;
+                .ok_or(anyhow!(
+                    "failed to retrieve pinning ID for interface {}",
+                    mac
+                ))?;
 
             mapping.insert(*mac, (*name).clone());
         }
 
         let opts = NetworkInterfacePinningOptions { mapping };
-        opts.verify().map_err(|err| err.to_string())?;
+        opts.verify()?;
 
         Ok(opts)
     }
 }
 
 impl ViewWrapper for InterfacePinningOptionsView {
-    cursive::wrap_impl!(self.view: ScrollView<NamedView<FormView<MacAddress>>>);
+    cursive::wrap_impl!(self.view: LinearLayout);
 }
-- 
2.55.0