Merge pull request #73 from yast/default_to_restart

Propose 'restart' as the default service widget action when running and no user connected

package/yast2-samba-server.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Thu Mar  5 13:59:38 UTC 2020 - Knut Anderssen <kanderssen@suse.com>
+
+- Propose to restart samba services by default when writing the
+  configuration with services already running. Reload is still used
+  if some user is connected to the samba server (bsc#1165638)
+- 4.1.4
+
 -------------------------------------------------------------------
 Wed Dec 12 12:47:46 UTC 2018 - Josef Reidinger <jreidinger@suse.com>
 

package/yast2-samba-server.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-samba-server
-Version:        4.1.3
+Version:        4.1.4
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/include/samba-server/complex.rb

@@ -87,13 +87,7 @@ def WriteDialog
       # If there some connected users, SAMBA is running and should be running
       # also after the Write() operation and the Progress was turned on before
       # Writing SAMBA conf
-      connected_users = SambaService.ConnectedUsers.count
-      Builtins.y2milestone("Number of connected users: %1", connected_users)
-
-      switch_to_reload = connected_users > 0 &&
-        SambaService.GetServiceRunning &&
-        SambaService.GetServiceAutoStart &&
-        ProgressStatus()
+      switch_to_reload = need_to_restart? && connected_users? && ProgressStatus()
 
       ret = save_status(switch_to_reload)
 
@@ -112,6 +106,26 @@ def WriteDialog
       ret ? :next : :abort
     end
 
+    # Convenience method to check whether a restart or reload should be used
+    # after writing the configuration
+    #
+    # @return [Boolean] true if the service is running; false otherwise
+    def need_to_restart?
+      # could be partialy active. i.e: smb is running and nmb is not
+      services.currently_active?
+    end
+
+    # Convenience method to check whether there are users connected to samba
+    # service or not
+    #
+    # @return [Boolean] true if some user is connected to the service; false
+    #   otherwise
+    def connected_users?
+      connected_users = SambaService.ConnectedUsers.count
+      Builtins.y2milestone("Number of connected users: %1", connected_users)
+      connected_users > 0
+    end
+
     # Saves service status (start mode and starts/stops the service)
     #
     # @param switch_to_reload [Boolean] indicates if restart action must be

src/include/samba-server/dialogs.rb

@@ -100,7 +100,10 @@ def initialize_samba_server_dialogs(include_target)
     #
     # @return [::CWM::ServiceWidget]
     def service_widget
-      @service_widget ||= ::CWM::ServiceWidget.new(services)
+      return @service_widget if @service_widget
+      @service_widget = ::CWM::ServiceWidget.new(services)
+      @service_widget.default_action = :restart if need_to_restart? && !connected_users
+      @service_widget
     end
 
     # routines

src/modules/SambaService.pm

@@ -145,7 +145,7 @@ sub StartStopNow {
     my ($self, $on) = @_;
     my $error = 0;
 
-    # Zero connected users -> restart, einther -> reload
+    # Zero connected users -> restart, either -> reload
     my $connected_users = $self->ConnectedUsers();
     my $nr_connected_users = scalar(@$connected_users);
 

test/dialog_complex_test.rb

@@ -42,17 +42,18 @@ def initialize
     allow(Yast2::SystemService).to receive(:find).with(anything).and_return(service)
     allow(Yast2::CompoundService).to receive(:new).and_return(services)
     allow(services).to receive(:action).and_return(action)
+    allow(services).to receive(:currently_active?).and_return(service_running)
   end
 
   let(:service) { instance_double("Yast2::SystemService", save: true, is_a?: true) }
   let(:services) { instance_double("Yast2::CompoundService", save: true) }
+  let(:service_running) { false }
   let(:action) { :start }
 
   describe "#WriteDialog" do
     subject(:samba) { TestComplexDialog.new }
 
     let(:connected_users) { ["john", "jane"] }
-    let(:service_running) { false }
     let(:service_on_boot) { false }
 
     let(:auto) { false }