Merge pull request #496 from yast/addon-with-refused-license-m

Declining/refusal of an addon license (master)

package/yast2-registration.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu May 28 10:50:51 UTC 2020 - Martin Vidner <mvidner@suse.com>
+
+- Declining/refusal of an addon license means canceling all addons.
+  (bsc#1169577)
+- 4.3.1
+
 -------------------------------------------------------------------
 Tue May 12 14:43:44 UTC 2020 - josef Reidinger <jreidinger@localhost>
 

package/yast2-registration.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-registration
-Version:        4.3.0
+Version:        4.3.1
 Release:        0
 Summary:        YaST2 - Registration Module
 License:        GPL-2.0-only

src/lib/registration/addon.rb

@@ -26,7 +26,8 @@
 require "y2packager/resolvable"
 
 module Registration
-  # this is a wrapper class around SUSE::Connect::Product object
+  # A wrapper class around SUSE::Connect::Product object,
+  # https://rubydoc.info/github/SUSE/connect/SUSE/Connect/Product
   class Addon
     extend Yast::I18n
     include Yast::I18n
@@ -38,6 +39,7 @@ class << self
       # read the remote add-on from the registration server
       # @param registration [Registration::Registration] use this object for
       #  reading the remote add-ons
+      # @return [Array<Addon>]
       def find_all(registration)
         return @cached_addons if @cached_addons
 
@@ -83,7 +85,7 @@ def reset_auto_selected
         @auto_selected = nil
       end
 
-      # list of auto selected add-ons
+      # @return [Array<Addon>] auto selected add-ons
       def auto_selected
         @auto_selected ||= detect_auto_selection
       end
@@ -155,6 +157,7 @@ def create_addon_with_deps(root)
         result
       end
 
+      # @return [Array<Addon>]
       def load_addons(registration)
         pure_addons = registration.get_addon_list
         # get IDs of the already activated addons
@@ -172,6 +175,7 @@ def load_addons(registration)
         end
       end
 
+      # @return [Array<Addon>]
       def detect_auto_selection
         required = selected + registered
 

src/lib/registration/ui/addon_eula_dialog.rb

@@ -48,17 +48,23 @@ def run
           false
         )
 
+        # Default: no EULA specified => accepted
+        eula_ret = :accepted
+
         addons.each do |addon|
           next unless addon.eula_acceptance_needed?
           next if addon.registered?
 
           log.info "Addon '#{addon.name}' has an EULA at #{addon.eula_url}"
           eula_ret = accept_eula(addon)
 
-          return eula_ret if eula_ret != :next
+          # any declined license needs to be handled separately
+          break if eula_ret != :accepted
         end
 
-        :next
+        # go back if any EULA has not been accepted, let the user
+        # deselect the not accepted extension
+        eula_ret == :accepted ? :next : eula_ret
       end
 
     private
@@ -104,34 +110,35 @@ def setup_eula_dialog(addon, eula_reader, tmpdir)
         display_optional_info(File.join(tmpdir, "README.BETA"))
       end
 
-      # run the EULA agreement dialog
+      # Run the EULA agreement dialog.
       # @param [EulaReader] eula_reader read EULAs
-      # @return [Symbol] :accepted, :back, :abort, :halt - user input
+      # @return [Symbol] :accepted, :back
       def run_eula_dialog(eula_reader)
         base_product = false
-        cancel_action = "refuse"
+        cancel_action = "abort"
         ret = Yast::ProductLicense.HandleLicenseDialogRet(arg_ref(eula_reader.licenses),
           base_product, cancel_action)
+        ret = :back if ret == :abort
         log.debug "EULA dialog result: #{ret}"
         ret
       end
 
-      # ask user to accept an addon EULA
+      # Ask user to accept an addon EULA.
+      # Declining (refusing) the license is translated into Back
+      # which will fit nicely from the caller's point of view.
       # @param [Addon] addon the addon
-      # @return [Symbol] :back, :abort, :halt
+      # @return [Symbol] :accepted, :back
       def accept_eula(addon)
         Dir.mktmpdir("extension-eula-") do |tmpdir|
           return :back unless download_eula(addon, tmpdir)
 
           eula_reader = EulaReader.new(tmpdir)
           license = find_license(addon, eula_reader)
-          return :next if license && license.accepted?
+          return :accepted if license && license.accepted?
 
           setup_eula_dialog(addon, eula_reader, tmpdir)
           ret = run_eula_dialog(eula_reader)
           license.accept! if ret == :accepted
-
-          return :next if [:accepted, :refused].include?(ret)
           ret
         end
       ensure
@@ -144,6 +151,7 @@ def display_optional_info(info_file)
         Yast::InstShowInfo.show_info_txt(info_file) if File.exist?(info_file)
       end
 
+      # @return [Y2Packager::License]
       def find_license(addon, eula_reader)
         license_file = eula_reader.licenses[Y2Packager::License::DEFAULT_LANG]
         return nil unless license_file

test/registration/ui/addon_eula_dialog_test.rb

@@ -57,7 +57,7 @@
 
     context "when there are EULA acceptances pending" do
       let(:addons) { [addon_with_eula, second_addon_with_eula] }
-      let(:first_dialog_response) { :refused }
+      let(:first_dialog_response) { :accepted }
       let(:second_dialog_response) { :accepted }
 
       before do
@@ -76,8 +76,8 @@
       context "and the user wants to abort" do
         let(:first_dialog_response) { :abort }
 
-        it "returns :abort" do
-          expect(subject.run).to eq(:abort)
+        it "returns :back anyway" do
+          expect(subject.run).to eq(:back)
         end
       end
 
@@ -124,15 +124,15 @@
 
       before do
         allow(Yast::ProductLicense).to receive(:HandleLicenseDialogRet)
-          .and_return(:refused)
+          .and_return(:abort)
       end
 
       it "does not set it as accepted" do
         expect(product_license).to_not receive(:accept!)
       end
 
-      it "returns :next" do
-        expect(dialog.run).to eq(:next)
+      it "returns :back" do
+        expect(dialog.run).to eq(:back)
       end
     end
   end
@@ -198,8 +198,8 @@
     context "when the license was previously accepted" do
       let(:accepted?) { true }
 
-      it "returns :next" do
-        expect(dialog.send(:accept_eula, addon)).to eq(:next)
+      it "returns :accepted" do
+        expect(dialog.send(:accept_eula, addon)).to eq(:accepted)
       end
 
       it "does not show the eula" do