[anaconda:rhel7/master 4/5] Factor determination of field sensitivities into methods (#1076055)

mulhern amulhern at redhat.com
Wed Sep 24 16:27:26 UTC 2014


Related: rhbz#1076055

Use methods appropriately.

Each of the new methods is called in one of two contexts:
1) _populate_right_side or _on_selector_clicked (new_type is None)
2) on_device_type_changed (new_type is not None)

For context (1) the new methods behave exactly the same as the old
expressions they replace, except that:
* _size_entry_sensitivity() now takes into account the possibility
that the device might be a LUKS device.
* _config_button_sensitive() evaluates expressions on the raw device.
Previously expressions were evaluated on the device, which meant that
it was sensitive when there was, e.g., a non-existant LUKS device on
top of an existant RAID device, which seems wrong.
* _name_entry_sensitive() evaluates expressions on the raw device.
This makes a difference, supposing there is a non-existant LUKS
device on top of an existing device. Previously, the name field would
be sensitive in this case, now it is not. Under the, possibly incorrect,
hypothesis that the name of the LUKS device is derived from the name of
its slave, this seems a reasonable choice.

For context (2) the relevant expressions in the new methods are changed
somewhat, in this fashion:
*) Each checks "not device.exists", just like for context (1).
Currently this will have no effect, since in context (2) not device.exists
is always True (because device_type can not be changed unless _typeCombo is
sensitive and it is only sensitive when the device does not exist) and
the expression always forms part of a conjunction (for which True is the
identity). This seems to me correct.
*) _config_button_sensitive() checks "not device.protected" just like for
context (1). It's hard for me to imagine a context in which a device did
not exists but was also protected, so it may be that this check is redundant,
and should just be removed instead. But, if it is not redundant, it should
probably be in both.
*) When checking whether the type is correct, no change was made. For
context (1) the device type or the device format type is checked, for
context (2) it's new_type, which is an enumerated valued defined in
devicefactory. But the differences go a lot deeper than that, and I'm not
sure how to resolve them. Obviously no change will cause no observable
change in the behavior, but I think the differences point to something
actually wrong (or worth documenting a little more completely).

For context (1), fancy_set_sensitive() is called unconditionally on the
result of _name_entry_sensitive() previously it was called only if
use_dev.exists or use_dev.type == "btrfs volume" evaluated to True.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 pyanaconda/ui/gui/spokes/custom.py | 82 ++++++++++++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 16 deletions(-)

diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py
index d855518..8af7ef1 100644
--- a/pyanaconda/ui/gui/spokes/custom.py
+++ b/pyanaconda/ui/gui/spokes/custom.py
@@ -1781,6 +1781,63 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
 
         self._populate_right_side(selector)
 
+    def _size_entry_sensitive(self, device, new_type=None):
+        """ True if the size entry should be sensitive, otherwise False.
+
+            :param device: the device for which size is being displayed
+            :type device: :class:`blivet.devices.StorageDevice`
+            :param new_type: one of an enumeration of device types or None
+            :type new_type: int or NoneType
+            :returns: True if field should be sensitive, otherwise False
+            :rtype: bool
+
+            Note: Some devices can not be resized even if they do not exist.
+        """
+        raw_device = device.raw_device
+
+        # Resizability of underlying device
+        if new_type is not None:
+            raw_device_resizable = raw_device.resizable or (not raw_device.exists and new_type != DEVICE_TYPE_BTRFS)
+        else:
+            raw_device_resizable = raw_device.resizable or (not raw_device.exists and raw_device.format.type != "btrfs")
+
+        # Total resizability
+        return raw_device_resizable and (raw_device == device.raw_device or not device.exists)
+
+    def _name_entry_sensitive(self, device, new_type=None):
+        """ True if the name entry should be sensitive, otherwise False.
+
+            :param device: the device for which size is being displayed
+            :type device: :class:`blivet.devices.StorageDevice`
+            :param new_type: one of an enumeration of device types or None
+            :type new_type: int or NoneType
+            :returns: True if field should be sensitive, otherwise False
+            :rtype: bool
+        """
+        raw_device = device.raw_device
+
+        if new_type is not None:
+            return not raw_device.exists and new_type in (DEVICE_TYPE_BTRFS, DEVICE_TYPE_LVM, DEVICE_TYPE_MD, DEVICE_TYPE_LVM_THINP)
+
+        return not raw_device.exists and raw_device.type != "btrfs volume"
+
+    def _config_button_sensitive(self, device, new_type=None):
+        """ True if the config button should be sensitive, otherwise False.
+
+            :param device: the device for which size is being displayed
+            :type device: :class:`blivet.devices.StorageDevice`
+            :param new_type: one of an enumeration of device types or None
+            :type new_type: int or NoneType
+            :returns: True if button should be sensitive, otherwise False
+            :rtype: bool
+        """
+        raw_device = device.raw_device
+
+        if new_type is not None:
+            return not raw_device.exists and not raw_device.protected and new_type not in (DEVICE_TYPE_LVM, DEVICE_TYPE_LVM_THINP, DEVICE_TYPE_BTRFS)
+
+        return not raw_device.exists and not raw_device.protected and devicefactory.get_device_type(raw_device) in (DEVICE_TYPE_PARTITION, DEVICE_TYPE_MD)
+
     def _raid_level_visible(self, model, itr, user_data):
         device_type = self._get_current_device_type()
         raid_level_str = model[itr][1]
@@ -2087,11 +2144,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         if device_type in [DEVICE_TYPE_BTRFS, DEVICE_TYPE_LVM_THINP]:
             fancy_set_sensitive(self._encryptCheckbox, False)
 
-        # The size entry is only sensitive for resizable existing devices and
-        # new devices that are not btrfs subvolumes.
-        # Do this after the device type combo is set since
-        # on_device_type_changed doesn't account for device existence.
-        fancy_set_sensitive(self._sizeEntry, device.resizable or (not device.exists and device.format.type != "btrfs"))
+        fancy_set_sensitive(self._sizeEntry, self._size_entry_sensitive(device))
 
         if self._sizeEntry.get_sensitive():
             self._sizeEntry.props.has_tooltip = False
@@ -2102,9 +2155,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
 
         self._populate_raid(get_raid_level(device))
         self._populate_container(device=use_dev)
-        # do this last in case this was set sensitive in on_device_type_changed
-        if use_dev.exists or use_dev.type == "btrfs volume":
-            fancy_set_sensitive(self._nameEntry, False)
+        fancy_set_sensitive(self._nameEntry, self._name_entry_sensitive(device))
 
     ###
     ### SIGNAL HANDLERS
@@ -2812,9 +2863,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         self._current_selector = selector
 
         self._applyButton.set_sensitive(False)
-        self._configButton.set_sensitive(not selector._device.exists and
-                                         not selector._device.protected and
-                                         devicefactory.get_device_type(selector._device) in (DEVICE_TYPE_PARTITION, DEVICE_TYPE_MD))
+        self._configButton.set_sensitive(self._config_button_sensitive(selector._device))
         self._removeButton.set_sensitive(not selector._device.protected)
         return True
 
@@ -3047,9 +3096,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         elif new_type == DEVICE_TYPE_MD:
             raid_level = defaultRaidLevel(new_type)
 
-        # lvm uses the RHS to set disk set. no foolish minds here.
-        exists = self._current_selector and self._current_selector._device.exists
-        self._configButton.set_sensitive(not exists and new_type not in (DEVICE_TYPE_LVM, DEVICE_TYPE_LVM_THINP, DEVICE_TYPE_BTRFS))
+        device = self._current_selector._device
+
+        self._configButton.set_sensitive(self._config_button_sensitive(device, new_type))
 
         # this has to be done before calling populate_raid since it will need
         # the raid level combo to contain the relevant raid levels for the new
@@ -3059,9 +3108,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
         self._populate_raid(raid_level)
         self._populate_container()
 
-        fancy_set_sensitive(self._nameEntry, new_type in (DEVICE_TYPE_BTRFS, DEVICE_TYPE_LVM, DEVICE_TYPE_MD, DEVICE_TYPE_LVM_THINP))
+        fancy_set_sensitive(self._nameEntry, self._name_entry_sensitive(device, new_type))
         self._nameEntry.set_text(self._device_name_dict[new_type])
-        fancy_set_sensitive(self._sizeEntry, new_type != DEVICE_TYPE_BTRFS)
+
+        fancy_set_sensitive(self._sizeEntry, self._size_entry_sensitive(device, new_type))
 
         # begin btrfs magic
         model = self._fsCombo.get_model()
-- 
1.9.3



More information about the anaconda-patches mailing list