From ec8ce14230d1a796076ff3532d148e448a21c6a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Tue, 31 Jul 2018 16:15:12 +0100 Subject: [PATCH 1/9] Use CWM::ServiceWidget to handle the service start up configuration --- src/include/dns-server/dialog-main.rb | 142 ++++++++++++++------------ test/main_test.rb | 73 ++++--------- 2 files changed, 96 insertions(+), 119 deletions(-) diff --git a/src/include/dns-server/dialog-main.rb b/src/include/dns-server/dialog-main.rb index b365a56..c15200f 100644 --- a/src/include/dns-server/dialog-main.rb +++ b/src/include/dns-server/dialog-main.rb @@ -5,12 +5,15 @@ # Summary: Data for configuration of dns-server, input and output functions. # Authors: Jiri Srain -require "ui/service_status" +require "cwm/service_widget" +require "yast2/system_service" module Yast # Representation of the configuration of dns-server. # Input and output routines. module DnsServerDialogMainInclude + include Yast::Logger + def initialize_dns_server_dialog_main(include_target) textdomain "dns-server" @@ -259,20 +262,9 @@ def initialize_dns_server_dialog_main(include_target) @functions = { :abort => fun_ref(method(:confirmAbort), "boolean ()") } end - def InitStartUp(_key) - status_widget.refresh - nil - end + def handle_apply(_key, event) + SaveAndRestart(event) if event["ID"] == "apply" - def HandleStartUp(_key, event) - event_id = event["ID"] - if event_id == "apply" - SaveAndRestart() - else - if status_widget.handle_input(event_id) == :enabled_flag - DnsServer.SetStartService(status_widget.enabled_flag?) - end - end nil end @@ -895,7 +887,7 @@ def CheckOptionValue(option, value) _("Really set this\noption without any value?\n") ) return false - end + end # it is a YES or NO type elsif OptionsIsYesNoType(option) # it has not a yes/no value @@ -913,7 +905,7 @@ def CheckOptionValue(option, value) ) return false end - end + end # it must be a number elsif OptionsIsNumberType(option) # if has not a number value @@ -1876,38 +1868,64 @@ def HandleExpertZonesPage(key, event) end # Write settings dialog - # @return `abort if aborted and `next otherwise + # + # @return [Symbol] :next whether configuration is saved successfully + # :back if user decided to change settings + # :abort otherwise def WriteDialog - Wizard.RestoreHelp(Ops.get_string(@HELPS, "write", "")) - ret = DnsServer.Write - if ret - service.reload if service.running? && status_widget.reload_flag? - :next - else - if Popup.YesNo(_("Saving the configuration failed. Change the settings?")) - :back - else - :abort - end - end + log.info("Running write dialog") + + Wizard.RestoreHelp(write_help_text) + + return :next if write_settings + return :back if Popup.YesNo(_("Saving the configuration failed. Change the settings?")) + :abort end - # Writes settings and restores the dialog without exiting - def SaveAndRestart + # Writea settings without exiting + def SaveAndRestart(event) + return nil unless validate_and_save_widgets(event) + Wizard.CreateDialog - Wizard.RestoreHelp(Ops.get_string(@HELPS, "write", "")) - ret = DnsServer.Write - if ret - service.reload if service.running? && status_widget.reload_flag? - else - Report.Error(_("Saving the configuration failed")) - end - Builtins.sleep(1000) - UI.CloseDialog + Wizard.RestoreHelp(write_help_text) + Report.Error(_("Saving the configuration failed")) unless write_settings + Wizard.CloseDialog + + service_widget.refresh nil end + # Writes DNS server settings and save the service + # + # NOTE: currently, the DnsServer is a Perl module, reason why the write of + # settings is being performed in two steps. + # + # @return [Boolean] true if settings are saved successfully; false otherwise + def write_settings + DnsServer.Write && service.save + end + + # Validates and saves CWM widgets + # + # @param [Hash] event map that triggered saving + def validate_and_save_widgets(event) + return false unless CWM.validate_current_widgets(event) + + CWM.save_current_widgets(event) + + true + end + + # Returns the common help text during settings are being written + # + # @return [String] common help text, if any; empty string otherwise + def write_help_text + @HELPS.fetch("write") { "" } + rescue + "" + end + # Ask for exit without saving # @return event that should be handled, nil if user canceled the exit def confirmAbort @@ -1961,12 +1979,12 @@ def tabs "start_up" => { # FIXME: new startup "contents" => VBox( - status_widget.widget, + service_widget.contents, VSpacing(), "firewall", VStretch(), Right( - PushButton(Id("apply"), _("Apply Changes")) + "apply" ) ), # Dialog Label - DNS - expert settings @@ -1981,7 +1999,7 @@ def tabs # FIXME: new startup "widget_names" => DnsServer.ExpertUI ? # expert mode - ["start_up", "firewall"] : + ["start_up", "firewall", "apply"] : # simple mode ["start_up", "firewall", "set_icon"] }, @@ -2078,18 +2096,12 @@ def tabs # Returns a hash describing the UI widgets def new_widgets @new_widgets ||= { - "start_up" => { - "widget" => :custom, - "custom_widget" => VBox(), - "init" => fun_ref( - method(:InitStartUp), - "void (string)" - ), - "handle" => fun_ref( - method(:HandleStartUp), - "symbol (string, map)" - ), - "help" => status_widget.help + "start_up" => service_widget.cwm_definition, + "apply" => { + "widget" => :push_button, + "label" => _("Apply Changes"), + "handle" => fun_ref(method(:handle_apply), "symbol (string, map)"), + "help" => "" }, "firewall" => CWMFirewallInterfaces.CreateOpenFirewallWidget( { "services" => ["dns"], "display_details" => true } @@ -2225,20 +2237,20 @@ def new_widgets } end - # Returns the status widget for service - # - # @return [::UI::ServiceStatus] status widget + # Returns the 'named' system service # - # @see #service - def status_widget - @status_widget ||= ::UI::ServiceStatus.new(service) + # @return [Yast2::SystemService] 'named' system service + def service + @service ||= Yast2::SystemService.find("named") end - # Returns the 'named' systemd service + # Returns the status widget for service # - # @return [SystemdService] 'named' systemd service instance - def service - @service ||= SystemdService.find("named") + # @return [::CWM::ServiceWidget] service status widget + # + # @see #service + def service_widget + @service_widget ||= ::CWM::ServiceWidget.new(service) end end end diff --git a/test/main_test.rb b/test/main_test.rb index f1542bd..2f35d7b 100755 --- a/test/main_test.rb +++ b/test/main_test.rb @@ -11,6 +11,7 @@ class CurrentDialogMain attr_accessor :status_widget attr_accessor :service + def initialize Yast.include self, "dns-server/dialog-main.rb" @status_widget = "status_widget" @@ -23,75 +24,39 @@ def initialize end describe "#WriteDialog" do - let(:m) { CurrentDialogMain.new } - let(:written) { false } - let(:running) { true } + subject { CurrentDialogMain.new } before do allow(Yast::DnsServer).to receive(:Write).and_return written - allow(m.service).to receive(:running?).and_return running - end - - it "writes the DNS configuration" do - expect(Yast::DnsServer).to receive(:Write).and_return written - m.WriteDialog - end - - context "when the configuration is written" do - let(:written) { true } - - context "and the named service is running" do - context "and the config is marked to be reloaded" do - it "reloads the service" do - expect(m.status_widget).to receive(:reload_flag?).and_return true - expect(m.service).to receive(:reload) - expect(m.WriteDialog ).to eq(:next) - end - end - - context "and the config is not marked to be reloaded" do - it "does not restart nor reload the service" do - expect(m.status_widget).to receive(:reload_flag?).and_return false - expect(m.service).to_not receive(:reload) - expect(m.service).to_not receive(:restart) - expect(m.WriteDialog ).to eq(:next) - end - end - end - - context "and the named service is not running" do - let(:running) { false} - - before do - allow(m.status_widget).to receive(:reload_flag?).and_return true - end - - it "does not restart nor reload the service" do - expect(m.service).to_not receive(:restart) - expect(m.service).to_not receive(:reload) - expect(m.WriteDialog ).to eq(:next) - end - end end context "when the configuration is not written" do let(:written) { false } + let(:change_settings) { true } + + before do + allow(Yast::Popup).to receive(:YesNo).and_return(change_settings) + end it "aks for changing the current settings" do expect(Yast::Popup).to receive(:YesNo) - m.WriteDialog + + subject.WriteDialog end - it "returns :back if decided to change the current settings" do - expect(Yast::Popup).to receive(:YesNo).and_return true - expect(m.WriteDialog).to eq(:back) + context "and user decides to change the current setting" do + it "returns :back" do + expect(subject.WriteDialog).to eq(:back) + end end - it "returns :abort if canceled" do - expect(Yast::Popup).to receive(:YesNo).and_return false - expect(m.WriteDialog).to eq(:abort) + context "and user decides to cancel" do + let(:change_settings) { false } + + it "returns :abort" do + expect(subject.WriteDialog).to eq(:abort) + end end end end - end From 50a74a5800b21d1c135f02c545600a5be1ae7247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Wed, 1 Aug 2018 09:45:14 +0100 Subject: [PATCH 2/9] Use CWM::ServiceWidget also in the wizard installation (first run) --- .../dns-server/dialog-installwizard.rb | 45 ++++++++----------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/src/include/dns-server/dialog-installwizard.rb b/src/include/dns-server/dialog-installwizard.rb index c1cdb67..9a2ff71 100644 --- a/src/include/dns-server/dialog-installwizard.rb +++ b/src/include/dns-server/dialog-installwizard.rb @@ -9,6 +9,10 @@ # # Representation of the configuration of dns-server. # Input and output routines. + +require "cwm/service_widget" +require "yast2/system_service" + module Yast module DnsServerDialogInstallwizardInclude def initialize_dns_server_dialog_installwizard(include_target) @@ -23,6 +27,19 @@ def initialize_dns_server_dialog_installwizard(include_target) Yast.import "CWMFirewallInterfaces" end + def service + @service ||= Yast2::SystemService.find("named") + end + + def service_widget + @service_widget ||= ::CWM::ServiceWidget.new(service) + end + + def write_dns_settings + service_widget.store + service.save + end + def runInstallWizardForwardersDialog caption = # Dialog caption (before a colon) @@ -184,22 +201,7 @@ def runInstallWizardFinishDialog VBox( firewall_layout, ldap_support, - # Label for Radiobuttons - DNS starting - Left(Label(_("Start-up Behavior"))), - Left( - RadioButtonGroup( - Id("dns_server_type"), - VBox( - # Radiobutton label - DNS starting - Left( - RadioButton(Id(:on), _("O&n: Start Now and When Booting")) - ), - # Radiobutton label - DNS starting - Left(RadioButton(Id(:off), _("O&ff: Only Start Manually"))), - VSpacing(1) - ) - ) - ) + service_widget.contents, ), RichText(Id("installation_overview"), rich_text), VSpacing(2), @@ -222,13 +224,6 @@ def runInstallWizardFinishDialog SetDNSSErverIcon() Wizard.SetNextButton(:next, Label.FinishButton) - auto_start = DnsServer.GetStartService - UI.ChangeWidget( - Id("dns_server_type"), - :CurrentButton, - auto_start ? :on : :off - ) - use_ldap = false # only expert allows to store data in ldap if DnsServer.ExpertUI @@ -270,10 +265,8 @@ def runInstallWizardFinishDialog end if ret == :next || ret == :expert - DnsServer.SetModified + write_dns_settings - auto_start2 = UI.QueryWidget(Id("dns_server_type"), :CurrentButton) == :on - DnsServer.SetStartService(auto_start2) CWMFirewallInterfaces.OpenFirewallStore(firewall_widget, "", event) end From 58b9401a6205a61947c255c4ea1dcb16bc047e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Wed, 1 Aug 2018 09:46:17 +0100 Subject: [PATCH 3/9] Avoid DnsServer module to overlap with Yast2::SystemService Since Yast2::SystemService (used through the new ServiceWidget) is responsible to update properly the service status (start/stop/restart/...), related logic in the DnsServer Perl module it has been limited to be executed only for AutoYast (`auto` and `config` modes). --- src/modules/DnsServer.pm | 80 +++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/src/modules/DnsServer.pm b/src/modules/DnsServer.pm index 306654b..9978793 100644 --- a/src/modules/DnsServer.pm +++ b/src/modules/DnsServer.pm @@ -1525,44 +1525,48 @@ sub Write { $self->update_forwarding(); $ret = {}; - # named has to be started - if ($start_service) - { - my $success = 1; - if (! $write_only) - { - # named is running - if (Service->Status("named") == 0) { - y2milestone("Reloading service 'named'"); - $success = Service->Reload("named") - } else { - y2milestone("Restarting service 'named'"); - $success = Service->Restart("named") - } - } - Service->Enable ("named"); - if (! $success) - { - # Cannot start service 'named', because of error that follows Error:. Do not translate named. - Report->Error (__("Error occurred while starting service named.\n\n")); - $ok = 0; - # There's no 'named' running -> prevent from blocking DNS queries - $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind"; - y2warning("Local forwarder set to: ".GetLocalForwarder()); - } - } - # named has to be stopped - else - { - if (! $write_only) - { - y2milestone("Stopping service 'named'"); - Service->Stop("named"); - # There's no 'named' running. Reset dns forwarder again - $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind"; - y2warning("Local forwarder set to: ".GetLocalForwarder()); - } - Service->Disable ("named"); + + if (Mode->auto() || Mode->config()) + { + # named has to be started + if ($start_service) + { + my $success = 1; + if (! $write_only) + { + # named is running + if (Service->Status("named") == 0) { + y2milestone("Reloading service 'named'"); + $success = Service->Reload("named") + } else { + y2milestone("Restarting service 'named'"); + $success = Service->Restart("named") + } + } + Service->Enable ("named"); + if (! $success) + { + # Cannot start service 'named', because of error that follows Error:. Do not translate named. + Report->Error (__("Error occurred while starting service named.\n\n")); + $ok = 0; + # There's no 'named' running -> prevent from blocking DNS queries + $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind"; + y2warning("Local forwarder set to: ".GetLocalForwarder()); + } + } + # named has to be stopped + else + { + if (! $write_only) + { + y2milestone("Stopping service 'named'"); + Service->Stop("named"); + # There's no 'named' running. Reset dns forwarder again + $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind"; + y2warning("Local forwarder set to: ".GetLocalForwarder()); + } + Service->Disable ("named"); + } } # First run finished From f58b0d1396fbb203253cd0d91f515a8a1f23cd66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Wed, 1 Aug 2018 11:24:04 +0100 Subject: [PATCH 4/9] Move duplicated logic to helper module Much of the methods related to status service widget were duplicated in both dialog, reason why it has been moved to a helper module. --- package/yast2-dns-server.spec | 2 + src/.gitignore | 1 - src/Makefile.am | 6 ++- .../dns-server/dialog-installwizard.rb | 16 +++---- src/include/dns-server/dialog-main.rb | 20 +------- src/lib/dns-server/service_widget_helpers.rb | 22 +++++++++ test/service_widget_helpers_test.rb | 48 +++++++++++++++++++ 7 files changed, 85 insertions(+), 30 deletions(-) create mode 100644 src/lib/dns-server/service_widget_helpers.rb create mode 100644 test/service_widget_helpers_test.rb diff --git a/package/yast2-dns-server.spec b/package/yast2-dns-server.spec index 540fdc9..993c31d 100644 --- a/package/yast2-dns-server.spec +++ b/package/yast2-dns-server.spec @@ -82,6 +82,8 @@ This package contains the YaST2 component for DNS server configuration. %defattr(-,root,root) %dir %{yast_yncludedir}/dns-server %{yast_yncludedir}/dns-server/* +%dir %{yast_libdir}/dns-server +%{yast_libdir}/dns-server/*.rb %{yast_clientdir}/dns-server.rb %{yast_clientdir}/dns-server_*.rb %{yast_moduledir}/* diff --git a/src/.gitignore b/src/.gitignore index 5c5de1f..3739eed 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -1,3 +1,2 @@ .dep *.ybc -dns-server diff --git a/src/Makefile.am b/src/Makefile.am index 0e56315..eb929bc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -30,6 +30,10 @@ ynclude_DATA = \ include/dns-server/cmdline.rb \ include/dns-server/helps.rb +ylibdir = @ylibdir@/dns-server +ylib_DATA = \ + lib/dns-server/service_widget_helpers.rb + scrconf_DATA = \ scrconf/dns_zone.scr \ scrconf/cfg_named.scr \ @@ -49,6 +53,6 @@ schemafiles_DATA = \ desktop_DATA = \ desktop/dns-server.desktop -EXTRA_DIST = $(module_DATA) $(module1_DATA) $(client_DATA) $(ynclude_DATA) $(scrconf_DATA) $(agent_SCRIPTS) $(schemafiles_DATA) $(desktop_DATA) +EXTRA_DIST = $(module_DATA) $(module1_DATA) $(client_DATA) $(ynclude_DATA) $(scrconf_DATA) $(agent_SCRIPTS) $(schemafiles_DATA) $(desktop_DATA) $(ylib_DATA) include $(top_srcdir)/Makefile.am.common diff --git a/src/include/dns-server/dialog-installwizard.rb b/src/include/dns-server/dialog-installwizard.rb index 9a2ff71..ac55501 100644 --- a/src/include/dns-server/dialog-installwizard.rb +++ b/src/include/dns-server/dialog-installwizard.rb @@ -10,11 +10,12 @@ # Representation of the configuration of dns-server. # Input and output routines. -require "cwm/service_widget" -require "yast2/system_service" +require "dns-server/service_widget_helpers" module Yast module DnsServerDialogInstallwizardInclude + include Y2DnsServer::ServiceWidgetHelpers + def initialize_dns_server_dialog_installwizard(include_target) textdomain "dns-server" @@ -27,14 +28,9 @@ def initialize_dns_server_dialog_installwizard(include_target) Yast.import "CWMFirewallInterfaces" end - def service - @service ||= Yast2::SystemService.find("named") - end - - def service_widget - @service_widget ||= ::CWM::ServiceWidget.new(service) - end - + # Writes settings and save the service + # + # @return [Boolean] true if service is saved successfully; false otherwise def write_dns_settings service_widget.store service.save diff --git a/src/include/dns-server/dialog-main.rb b/src/include/dns-server/dialog-main.rb index c15200f..51f3c30 100644 --- a/src/include/dns-server/dialog-main.rb +++ b/src/include/dns-server/dialog-main.rb @@ -5,14 +5,14 @@ # Summary: Data for configuration of dns-server, input and output functions. # Authors: Jiri Srain -require "cwm/service_widget" -require "yast2/system_service" +require "dns-server/service_widget_helpers" module Yast # Representation of the configuration of dns-server. # Input and output routines. module DnsServerDialogMainInclude include Yast::Logger + include Y2DnsServer::ServiceWidgetHelpers def initialize_dns_server_dialog_main(include_target) textdomain "dns-server" @@ -2236,21 +2236,5 @@ def new_widgets } } end - - # Returns the 'named' system service - # - # @return [Yast2::SystemService] 'named' system service - def service - @service ||= Yast2::SystemService.find("named") - end - - # Returns the status widget for service - # - # @return [::CWM::ServiceWidget] service status widget - # - # @see #service - def service_widget - @service_widget ||= ::CWM::ServiceWidget.new(service) - end end end diff --git a/src/lib/dns-server/service_widget_helpers.rb b/src/lib/dns-server/service_widget_helpers.rb new file mode 100644 index 0000000..8a4ef2a --- /dev/null +++ b/src/lib/dns-server/service_widget_helpers.rb @@ -0,0 +1,22 @@ +require "cwm/service_widget" +require "yast2/system_service" + +module Y2DnsServer + module ServiceWidgetHelpers + # Returns the 'named' system service + # + # @return [Yast2::SystemService] 'named' system service + def service + @service ||= Yast2::SystemService.find("named") + end + + # Returns the status widget for service + # + # @return [::CWM::ServiceWidget] service status widget + # + # @see #service + def service_widget + @service_widget ||= ::CWM::ServiceWidget.new(service) + end + end +end diff --git a/test/service_widget_helpers_test.rb b/test/service_widget_helpers_test.rb new file mode 100644 index 0000000..a2a8151 --- /dev/null +++ b/test/service_widget_helpers_test.rb @@ -0,0 +1,48 @@ +#! /usr/bin/env rspec + +require_relative "test_helper" +require "dns-server/service_widget_helpers" + +describe Y2DnsServer::ServiceWidgetHelpers do + class Tester + include Y2DnsServer::ServiceWidgetHelpers + end + + subject { Tester.new } + + before do + allow(Yast2::SystemService).to receive(:find).with("named").and_return(service) + end + + describe "#service" do + context "when service is available" do + let(:service) { double("named") } + + it "returns a service" do + expect(subject.service).to be(service) + end + end + + context "when service is NOT available" do + let(:service) { nil } + + it "returns nil" do + expect(subject.service).to be_nil + end + end + end + + describe "#service_widget" do + let(:service) { double("named") } + + it "creates a service widget for \"named\" service" do + expect(CWM::ServiceWidget).to receive(:new).with(service) + + subject.service_widget + end + + it "returns a CWM::ServiceWidget" do + expect(subject.service_widget).to be_a(CWM::ServiceWidget) + end + end +end From 2b05d1e7fa8eba53ad77017fb6807395fe7ac501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Wed, 1 Aug 2018 12:23:06 +0100 Subject: [PATCH 5/9] Update Yast2 required version --- package/yast2-dns-server.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package/yast2-dns-server.spec b/package/yast2-dns-server.spec index 993c31d..1bcb24b 100644 --- a/package/yast2-dns-server.spec +++ b/package/yast2-dns-server.spec @@ -33,8 +33,8 @@ BuildRequires: yast2-perl-bindings BuildRequires: yast2-testsuite BuildRequires: rubygem(rspec) -# SuSEFirewall2 replaced by firewalld -BuildRequires: yast2 >= 4.0.39 +# Yast2::ServiceWidget +BuildRequires: yast2 >= 4.1.0 Requires: /usr/bin/host Requires: perl-gettext # Exporter Data::Dumper @@ -55,8 +55,8 @@ Requires: sed # FATE #303386: Network setup tools Requires: yast2-sysconfig -# SuSEFirewall2 replaced by firewalld -Requires: yast2 >= 4.0.39 +# Yast2::ServiceWidget +Requires: yast2 >= 4.1.0 BuildArch: noarch From f99c0fb5ffc8fb53dd57b472180cd119462a3467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Tue, 7 Aug 2018 11:10:07 +0100 Subject: [PATCH 6/9] Update from code review --- src/include/dns-server/dialog-main.rb | 16 ++++++++--- src/lib/dns-server/service_widget_helpers.rb | 28 +++++++++++++++++--- test/main_test.rb | 8 +++--- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/include/dns-server/dialog-main.rb b/src/include/dns-server/dialog-main.rb index 51f3c30..d4500c8 100644 --- a/src/include/dns-server/dialog-main.rb +++ b/src/include/dns-server/dialog-main.rb @@ -5,6 +5,8 @@ # Summary: Data for configuration of dns-server, input and output functions. # Authors: Jiri Srain +require "yast/logger" +require "yast2/popup" require "dns-server/service_widget_helpers" module Yast @@ -1878,11 +1880,11 @@ def WriteDialog Wizard.RestoreHelp(write_help_text) return :next if write_settings - return :back if Popup.YesNo(_("Saving the configuration failed. Change the settings?")) + return :back if back_to_change_setting? :abort end - # Writea settings without exiting + # Writes settings without exiting def SaveAndRestart(event) return nil unless validate_and_save_widgets(event) @@ -1898,7 +1900,7 @@ def SaveAndRestart(event) # Writes DNS server settings and save the service # - # NOTE: currently, the DnsServer is a Perl module, reason why the write of + # @note currently, the DnsServer is a Perl module, reason why the write of # settings is being performed in two steps. # # @return [Boolean] true if settings are saved successfully; false otherwise @@ -1906,6 +1908,14 @@ def write_settings DnsServer.Write && service.save end + # Shows a popup asking to user if wants to change settings + # + # @return [Boolean] true if user decides to go back to change settings; false otherwise + def back_to_change_setting? + change_settings_message = _("Saving the configuration failed. Change the settings?") + Yast2::Popup.show(change_settings_message, headline: :warning, buttons: :yes_no) == :yes + end + # Validates and saves CWM widgets # # @param [Hash] event map that triggered saving diff --git a/src/lib/dns-server/service_widget_helpers.rb b/src/lib/dns-server/service_widget_helpers.rb index 8a4ef2a..6e29186 100644 --- a/src/lib/dns-server/service_widget_helpers.rb +++ b/src/lib/dns-server/service_widget_helpers.rb @@ -1,3 +1,25 @@ +# encoding: utf-8 + +# Copyright (c) [2018] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. +# encoding: utf-8 + require "cwm/service_widget" require "yast2/system_service" @@ -10,13 +32,13 @@ def service @service ||= Yast2::SystemService.find("named") end - # Returns the status widget for service + # Widget to define status and start mode of the service # - # @return [::CWM::ServiceWidget] service status widget + # @return [CWM::ServiceWidget] # # @see #service def service_widget - @service_widget ||= ::CWM::ServiceWidget.new(service) + @service_widget ||= CWM::ServiceWidget.new(service) end end end diff --git a/test/main_test.rb b/test/main_test.rb index 2f35d7b..f7fa70e 100755 --- a/test/main_test.rb +++ b/test/main_test.rb @@ -32,14 +32,14 @@ def initialize context "when the configuration is not written" do let(:written) { false } - let(:change_settings) { true } + let(:change_settings) { :yes } before do - allow(Yast::Popup).to receive(:YesNo).and_return(change_settings) + allow(Yast2::Popup).to receive(:show).and_return(change_settings) end it "aks for changing the current settings" do - expect(Yast::Popup).to receive(:YesNo) + expect(Yast2::Popup).to receive(:show).with(instance_of(String), hash_including(buttons: :yes_no)) subject.WriteDialog end @@ -51,7 +51,7 @@ def initialize end context "and user decides to cancel" do - let(:change_settings) { false } + let(:change_settings) { :no } it "returns :abort" do expect(subject.WriteDialog).to eq(:abort) From 67c97a661fb6bcd4d370e54201bbfcb0ddf648e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Tue, 7 Aug 2018 13:15:46 +0100 Subject: [PATCH 7/9] Bump version and changes file --- package/yast2-dns-server.changes | 7 +++++++ package/yast2-dns-server.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2-dns-server.changes b/package/yast2-dns-server.changes index a8dbb2b..8577876 100644 --- a/package/yast2-dns-server.changes +++ b/package/yast2-dns-server.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Tue Aug 7 12:14:35 UTC 2018 - dgonzalez@suse.com + +- Use CWM::ServiceWidget to manage the service status + (part of fate#319428). +- 4.1.0 + ------------------------------------------------------------------- Wed Jul 25 18:20:26 UTC 2018 - knut.anderssen@suse.com diff --git a/package/yast2-dns-server.spec b/package/yast2-dns-server.spec index 1bcb24b..3479b1c 100644 --- a/package/yast2-dns-server.spec +++ b/package/yast2-dns-server.spec @@ -17,7 +17,7 @@ Name: yast2-dns-server -Version: 4.0.4 +Version: 4.1.0 Release: 0 Url: https://github.com/yast/yast-dns-server From b02905384c515b857eb516615e4739dfe716e944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Fri, 10 Aug 2018 11:26:00 +0100 Subject: [PATCH 8/9] Update unit tests --- test/main_test.rb | 45 ++++++++++++++++++++++++++++++--------------- test/test_helper.rb | 1 + 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/test/main_test.rb b/test/main_test.rb index f7fa70e..1c74d83 100755 --- a/test/main_test.rb +++ b/test/main_test.rb @@ -1,21 +1,19 @@ #! /usr/bin/env rspec require_relative "test_helper" -require "yast" -require "yast/rspec" +require_relative "../src/modules/DnsServerUI.rb" +require "dns-server/service_widget_helpers" + +require "yast2/system_service" describe "DnsServerDialogMainInclude" do class CurrentDialogMain include Yast::I18n include Yast::UIShortcuts - - attr_accessor :status_widget - attr_accessor :service + include Y2DnsServer::ServiceWidgetHelpers def initialize Yast.include self, "dns-server/dialog-main.rb" - @status_widget = "status_widget" - @service = "named.service" end end @@ -24,29 +22,46 @@ def initialize end describe "#WriteDialog" do - subject { CurrentDialogMain.new } + subject(:main_dialog) { CurrentDialogMain.new } before do - allow(Yast::DnsServer).to receive(:Write).and_return written + allow(Yast::DnsServer).to receive(:Write).and_return(dns_configuration_written) + allow(Yast2::SystemService).to receive(:find).and_return(service) end - context "when the configuration is not written" do - let(:written) { false } - let(:change_settings) { :yes } + let(:service) { instance_double(Yast2::SystemService, save: true) } + let(:dns_configuration_written) { true } + + context "when DNS configuration is written" do + it "saves the system service" do + expect(service).to receive(:save) + + main_dialog.WriteDialog + end + it "returns :next" do + expect(main_dialog.WriteDialog).to eq(:next) + end + end + + context "when the configuration is not written" do before do allow(Yast2::Popup).to receive(:show).and_return(change_settings) end + let(:change_settings) { :yes } + let(:dns_configuration_written) { false } + it "aks for changing the current settings" do - expect(Yast2::Popup).to receive(:show).with(instance_of(String), hash_including(buttons: :yes_no)) + expect(Yast2::Popup).to receive(:show) + .with(instance_of(String), hash_including(buttons: :yes_no)) - subject.WriteDialog + main_dialog.WriteDialog end context "and user decides to change the current setting" do it "returns :back" do - expect(subject.WriteDialog).to eq(:back) + expect(main_dialog.WriteDialog).to eq(:back) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 05c2211..48867d1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -21,6 +21,7 @@ ENV["Y2DIR"] = SRC_PATH require "yast" +require "yast/rspec" if ENV["COVERAGE"] require "simplecov" From fb6a30573c72b44a48dc008729c88b0fa29896a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz?= Date: Fri, 10 Aug 2018 11:50:17 +0100 Subject: [PATCH 9/9] Update from code review --- src/include/dns-server/dialog-installwizard.rb | 2 +- src/include/dns-server/dialog-main.rb | 10 +++++----- test/main_test.rb | 5 +++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/include/dns-server/dialog-installwizard.rb b/src/include/dns-server/dialog-installwizard.rb index ac55501..7d349ef 100644 --- a/src/include/dns-server/dialog-installwizard.rb +++ b/src/include/dns-server/dialog-installwizard.rb @@ -28,7 +28,7 @@ def initialize_dns_server_dialog_installwizard(include_target) Yast.import "CWMFirewallInterfaces" end - # Writes settings and save the service + # Writes settings and saves the service # # @return [Boolean] true if service is saved successfully; false otherwise def write_dns_settings diff --git a/src/include/dns-server/dialog-main.rb b/src/include/dns-server/dialog-main.rb index d4500c8..695574d 100644 --- a/src/include/dns-server/dialog-main.rb +++ b/src/include/dns-server/dialog-main.rb @@ -5,7 +5,7 @@ # Summary: Data for configuration of dns-server, input and output functions. # Authors: Jiri Srain -require "yast/logger" +require "yast" require "yast2/popup" require "dns-server/service_widget_helpers" @@ -1875,8 +1875,6 @@ def HandleExpertZonesPage(key, event) # :back if user decided to change settings # :abort otherwise def WriteDialog - log.info("Running write dialog") - Wizard.RestoreHelp(write_help_text) return :next if write_settings @@ -1898,7 +1896,7 @@ def SaveAndRestart(event) nil end - # Writes DNS server settings and save the service + # Writes DNS server settings and saves the service # # @note currently, the DnsServer is a Perl module, reason why the write of # settings is being performed in two steps. @@ -1908,7 +1906,7 @@ def write_settings DnsServer.Write && service.save end - # Shows a popup asking to user if wants to change settings + # Shows a popup asking to the user if wants to change settings # # @return [Boolean] true if user decides to go back to change settings; false otherwise def back_to_change_setting? @@ -1919,6 +1917,8 @@ def back_to_change_setting? # Validates and saves CWM widgets # # @param [Hash] event map that triggered saving + # + # @return [Boolean] false if validation fails; true otherwise def validate_and_save_widgets(event) return false unless CWM.validate_current_widgets(event) diff --git a/test/main_test.rb b/test/main_test.rb index 1c74d83..e97c227 100755 --- a/test/main_test.rb +++ b/test/main_test.rb @@ -1,10 +1,11 @@ #! /usr/bin/env rspec require_relative "test_helper" -require_relative "../src/modules/DnsServerUI.rb" -require "dns-server/service_widget_helpers" require "yast2/system_service" +require "dns-server/service_widget_helpers" + +Yast.import "DnsServerUI" describe "DnsServerDialogMainInclude" do class CurrentDialogMain