Added option to retry the block given (#428)

* Permit to retry when a timeout error occurs and also added a details button for timeout and json error exceptions

* Bump version & changelog.

package/yast2-registration.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Sun Mar 10 18:58:40 UTC 2019 - knut.anderssen@suse.com
+
+- Permit to retry the registration in case of a timeout or a json
+  parse error and hide the error details which will be accesible
+  through the 'details' button (bsc#1058375, bsc#1126045).
+- 4.1.20
+
 -------------------------------------------------------------------
 Fri Mar  1 09:50:20 UTC 2019 - lslezak@suse.cz
 

package/yast2-registration.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-registration
-Version:        4.1.19
+Version:        4.1.20
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/lib/registration/connect_helpers.rb

@@ -82,11 +82,10 @@ def self.catch_registration_errors(message_prefix: "",
         log.error "Timeout error: #{e.message}"
         # FIXME: to not break existing translation, this typo should be fixed
         # later after SP2: time -> timed
-        Yast::Report.Error(
-          error_with_details(message_prefix + _("Connection time out.") + "\n",
-            _("Make sure that the registration server is reachable and\n" \
-              "the connection is reliable."))
-        )
+        retry if report_error_and_retry?(message_prefix + _("Connection time out."),
+          _("Make sure that the registration server is reachable and\n" \
+            "the connection is reliable."))
+
         false
       rescue SUSE::Connect::ApiError => e
         log.error "Received error: #{e.response.inspect}"
@@ -142,7 +141,8 @@ def self.catch_registration_errors(message_prefix: "",
         log.error "JSON parse error"
         # update the message when an old SMT server is found
         check_smt_api(e.message)
-        report_error(message_prefix + _("Cannot parse the data from server."), e.message)
+        details_error(message_prefix + _("Cannot parse the data from server."), e.message)
+        false
       rescue StandardError => e
         log.error("SCC registration failed: #{e.class}: #{e}, #{e.backtrace}")
         Yast::Report.Error(
@@ -157,6 +157,24 @@ def self.report_error(msg, error_message)
       Yast::Report.Error(error_with_details(msg, error_message))
     end
 
+    def self.details_error(msg, error_message, retry_button: false)
+      if Yast::Mode.auto
+        report_error(msg, error_message)
+      else
+        buttons =
+          if retry_button
+            { retry: Yast::Label.RetryButton, cancel: Yast::Label.CancelButton }
+          else
+            :ok
+          end
+        Yast2::Popup.show(msg, details: error_message, headline: :error, buttons: buttons)
+      end
+    end
+
+    def self.report_error_and_retry?(msg, details_message)
+      details_error(msg, details_message, retry_button: true) == :retry
+    end
+
     # Report a pkg-bindings error. Display a message with error details from
     # libzypp.
     # @param msg [String] error message (translated)
@@ -295,7 +313,7 @@ def self.handle_network_error(message_prefix, e)
       if Yast::NetworkService.isNetworkRunning
         # FIXME: use a better message, this one has been reused after the text freeze
         report_error(message_prefix + _("Invalid URL."), e.message)
-      elsif Helpers.network_configurable && !(Yast::Mode.autoinst || Yast::Mode.autoupgrade)
+      elsif Helpers.network_configurable && !Yast::Mode.auto
         if Yast::Popup.YesNo(
           # Error popup
           _("Network is not configured, the registration server cannot be reached.\n" \
@@ -338,6 +356,7 @@ def self.add_update_hint(error_msg)
     end
 
     private_class_method :report_error, :error_with_details, :import_ssl_certificate,
-      :report_ssl_error, :check_smt_api, :handle_network_error
+      :report_ssl_error, :check_smt_api, :handle_network_error, :details_error,
+      :report_error_and_retry?
   end
 end

src/lib/registration/registration_ui.rb

@@ -54,8 +54,10 @@ def initialize(registration)
     #   items: boolean (true on success), remote service (or nil)
     def register_system_and_base_product
       product_service = nil
+      # TRANSLATORS: Popup error message prefix
+      error_options = { message_prefix: _("Registration failed.") + "\n\n" }
 
-      success = ConnectHelpers.catch_registration_errors do
+      success = ConnectHelpers.catch_registration_errors(error_options) do
         register_system if !Registration.is_registered?
 
         # then register the product(s)

test/connect_helpers_spec.rb

@@ -46,18 +46,51 @@ def api_error(code: 400, headers: {}, body: {})
       expect(ret).to eq(false)
     end
 
-    it "reports an error with details on timeout" do
-      details = _("Make sure that the registration server is reachable and\n" \
-        "the connection is reliable.")
-
-      expect(subject).to receive(:wrap_text).with("Details: #{details}", screen_width - 4)
-        .and_return("Details wrapped text")
-      expect(Yast::Report).to receive(:Error).with(
-        "Registration: " + _("Connection time out.") + "\n\n\nDetails wrapped text"
-      )
+    context "on timeout" do
+      let(:details) do
+        _("Make sure that the registration server is reachable and\n" \
+          "the connection is reliable.")
+      end
+      let(:autoinst) { false }
+
+      before do
+        allow(Yast::Mode).to receive(:autoinst).and_return(:autoinst?)
+      end
+
+      context "and in AutoYaST mode" do
+        let(:autoinst?) { true }
+
+        it "reports an error with details on timeout" do
+          expect(subject).to receive(:wrap_text).with("Details: #{details}", screen_width - 4)
+            .and_return("Details wrapped text")
+          expect(Yast::Report).to receive(:Error).with(
+            "Registration: " + _("Connection time out.") + "\n\nDetails wrapped text"
+          )
+
+          helpers.catch_registration_errors(message_prefix: "Registration: ") do
+            raise Timeout::Error
+          end
+        end
+      end
+
+      context "and in a common installation" do
+        it "shows an error dialog with a retry and details button" do
+          expect(helpers).to receive(:report_error_and_retry?)
+            .with("Registration: " + _("Connection time out."), details)
+
+          helpers.catch_registration_errors(message_prefix: "Registration: ") do
+            raise Timeout::Error
+          end
+        end
 
-      helpers.catch_registration_errors(message_prefix: "Registration: ") do
-        raise Timeout::Error
+        it "retries the given block if selected by the user" do
+          expect(helpers).to receive(:report_error_and_retry?).twice
+            .with("Registration: " + _("Connection time out."), details).and_return(true, false)
+
+          helpers.catch_registration_errors(message_prefix: "Registration: ") do
+            raise Timeout::Error
+          end
+        end
       end
     end
 
@@ -110,7 +143,28 @@ def api_error(code: 400, headers: {}, body: {})
     end
 
     context "JSON parse error is received" do
-      include_examples  "old registration server", JSON::ParserError.new("error message")
+      let(:autoinst?) { true }
+
+      before do
+        allow(Yast::Mode).to receive(:autoinst).and_return(autoinst?)
+        allow(Registration::UrlHelpers).to receive(:registration_url)
+          .and_return(SUSE::Connect::YaST::DEFAULT_URL)
+      end
+
+      include_examples "old registration server", JSON::ParserError.new("error message")
+
+      context "and in interactive installation" do
+        let(:autoinst?) { false }
+
+        it "hiddes the error details in a details button" do
+          expect(helpers).to receive(:details_error)
+            .with("Registration: " + _("Cannot parse the data from server."), "error message")
+
+          helpers.catch_registration_errors(message_prefix: "Registration: ") do
+            raise JSON::ParserError, "error message"
+          end
+        end
+      end
     end
 
     [400, 401, 500, 42].each do |error_code|