Update from code review

Co-Authored-By: David Díaz González <dgonzalez@suse.de>
yast · Feb 8, 2019 · d838787 · d838787
1 parent ede7159
commit d838787
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 31 deletions.
diff --git a/src/include/users/dialogs.rb b/src/include/users/dialogs.rb
@@ -792,7 +792,7 @@ def EditUserDialog(what)
                       new_user_term,
                       HBox(
                         HSpacing(),
-                        Left(CheckBox(Id(:btrfs_subvolume), _("Btrfs subvolume"), btrfs_subvolume))
+                        Left(CheckBox(Id(:btrfs_subvolume), btrfs_label), btrfs_subvolume))
                       ),
                     )
                   ),
@@ -828,9 +828,8 @@ def EditUserDialog(what)
               ),
               text_mode ? Empty() : HSpacing(1)
             ),
-          ),
+          )
           HSpacing(1)
-        )
       end
 
       # generate contents for Plugins Dialog
@@ -1524,9 +1523,11 @@ def EditUserDialog(what)
           # Check if is a valid path when creating a Btfs subvolume
           if btrfs_subvolume && check_btrfs_path && !valid_btrfs_path?(new_home)
             Report.Error(
+              # TRANSLATORS: the error message when user try to create a Btrfs subvolume in a not
+              # valid location
               _("Given path is not a valid Btrfs location.\n\n" \
               "Choose another path for the home directory\n" \
-              "or uncheck the 'Btrfs subvolume' option.")
+              "or uncheck the '%{btrfs_option}' option.") % { btrfs_option: btrfs_label }
             )
             focus_tab.call(current, :home)
             next
@@ -1773,13 +1774,9 @@ def EditUserDialog(what)
           end
 
           if do_not_edit
-            UI.ChangeWidget(Id(:uid), :Enabled, false)
-            UI.ChangeWidget(Id(:home), :Enabled, false)
-            UI.ChangeWidget(Id(:move_home), :Enabled, false)
-            UI.ChangeWidget(Id(:shell), :Enabled, false)
-            UI.ChangeWidget(Id(:defaultgroup), :Enabled, false)
-            UI.ChangeWidget(Id(:browse), :Enabled, false)
-            UI.ChangeWidget(Id(:btrfs_subvolume), :Enabled, false)
+            [:uid, :home, :move_home, :shell, :defaultgroup, :browse, :btrfs_subvolume].each do |widget|
+              UI.ChangeWidget(Id(widget), :Enabled, false)
+            end
           end
           if user_type == "ldap" && !Ldap.file_server
             UI.ChangeWidget(Id(:browse), :Enabled, false)
@@ -1890,6 +1887,12 @@ def EditUserDialog(what)
       ret
     end
 
+    def btrfs_label
+      # TRANSLATORS: label for the checkbox that allows to create the user home directory as a
+      # Btrfs subvolume
+      _("Create a Separate Btrfs Subvolume for the Home Directory")
+    end
+
     # Returns clean path
     #
     # Also useful to remove the trailing backslash
@@ -1901,7 +1904,7 @@ def cleanpath(path)
       return path if path.empty?
 
       Pathname.new(path).cleanpath.to_s
-    rescue
+    rescue TypeError
       ""
     end
 
@@ -1923,8 +1926,7 @@ def valid_btrfs_path?(path)
     def btrfs_available?
       available_filesystems = Yast::Execute.locally!.stdout(
         ["/usr/bin/df", "--output=fstype"],
-        ["/usr/bin/tail", "-n", "+2"],
-        ["/usr/bin/uniq"]
+        ["/usr/bin/tail", "-n", "+2"]
       ).split("\n")
 
       available_filesystems.include?("btrfs")

diff --git a/src/modules/UsersRoutines.pm b/src/modules/UsersRoutines.pm
@@ -69,17 +69,6 @@ my $btrfs = "/usr/sbin/btrfs";
 ##-------------------------------------------------------------------------
 ##----------------- helper routines ---------------------------------------
 
-sub valid_btrfs {
-    my $path       = shift;
-    my $dirname    = dirname($path);
-    my $cmd        = "/usr/bin/stat -f --format %T $dirname";
-    my %cmd_output = %{ SCR->Execute( ".target.bash_output", $cmd ) };
-
-    chomp(%cmd_output);
-
-    return ( $cmd_output{"stdout"} eq "btrfs" );
-}
-
 sub btrfs_subvolume {
     my $path = shift;
     my $cmd  = "$btrfs subvolume show $path";
@@ -122,12 +111,6 @@ sub CreateHome {
 
     # Create the home as btrfs subvolume
     if ($use_btrfs) {
-        if (!valid_btrfs($home)) {
-            y2error("Error to create '$home' as btrfs subvolume in a not valid btrfs filesystem");
-
-            return 0;
-        }
-
         my $cmd = "btrfs subvolume create $home";
         my %cmd_out = %{ SCR->Execute( ".target.bash_output", $cmd ) };
         my $stderr = $cmd_out{"stderr"} || "";