Merge 13e5236 into 5f79ea9

package/yast2-registration.changes

@@ -1,3 +1,19 @@
+-------------------------------------------------------------------
+Tue Jan 28 16:19:45 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- Improves the online search mechanism (jsc#SLE-9109):
+  - Do not crash when a package does not belong to an addon.
+  - Display dependencies when enabling a module/extension.
+  - Handle multiple packages with the same name properly.
+  - Show an error when a package cannot be selected for
+    installation.
+  - After selecting a package, unselect the addon if it is not
+    needed anymore.
+  - Forces a minimal number of characters before performing the
+    search.
+  - Removes the 'Back' button.
+- 4.2.29
+
 -------------------------------------------------------------------
 Thu Jan 23 11:58:16 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
 

package/yast2-registration.spec

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

src/lib/registration/addon.rb

@@ -356,6 +356,17 @@ def eula_acceptance_needed?
       !eula_url.to_s.strip.empty?
     end
 
+    # Returns all the dependencies
+    #
+    # Includes all dependencies in a recursive way.
+    #
+    # @return [Array<Addon>]
+    def dependencies
+      return [] if depends_on.nil?
+
+      [depends_on] + depends_on.dependencies
+    end
+
     def self.dump_addons
       # dump the downloaded data to a file for easier debugging,
       # avoid write failures when running as an unprivileged user (rspec tests)

src/lib/registration/clients/online_search.rb

@@ -18,6 +18,7 @@
 # find current contact information at www.suse.com.
 
 require "yast"
+require "yast2/popup"
 require "registration/dialogs/online_search"
 require "registration/addon"
 require "registration/registration"
@@ -53,12 +54,14 @@ def initialize
       # Sequence workflow aliases
       #
       # @see #find_addons
-      # @see #package_search
-      # @see #commit
+      # @see #search_packages
+      # @see #display_eula
+      # @see #register_addons
+      # @see #select_packages
       def workflow_aliases
         {
           "find_addons"     => ->() { find_addons },
-          "search"          => ->() { search_packages },
+          "search_packages" => ->() { search_packages },
           "display_eula"    => ->() { display_eula },
           "register_addons" => ->() { register_addons },
           "select_packages" => ->() { select_packages }
@@ -72,17 +75,18 @@ def workflow_aliases
       #   1. Find the addons
       #   2. Search for packages (UI)
       #   3. Register the addons
-      #   4. Select the packages for installation
+      #   4. Display the EULA if needed
+      #   5. Select the packages for installation
       #
       # @return [Symbol] Sequence's result (:next or :abort)
       def run
         sequence = {
           "ws_start"        => "find_addons",
           "find_addons"     => {
             abort: :abort,
-            next:  "search"
+            next:  "search_packages"
           },
-          "search"          => {
+          "search_packages" => {
             abort: :abort,
             next:  "display_eula"
           },
@@ -106,32 +110,50 @@ def run
 
     private
 
+      # Find all available addons
+      #
+      # @return [:next]
       def find_addons
         ::Registration::Addon.find_all(registration)
         :next
       end
 
       # Opens the online search dialog
+      #
+      # @return [:next]
       def search_packages
         reset_selected_addons_cache!
         package_search_dialog.run
       end
 
-      # display EULAs for the selected addons
+      # Display EULAs for the selected addons
+      #
+      # @return [Symbol] User input (:next, :back, :abort)
+      #   or :next if there are not licenses to accept
+      #
       def display_eula
         return :next if selected_addons.empty?
         ::Registration::UI::AddonEulaDialog.run(selected_addons)
       end
 
+      # Registers addons
+      #
+      # @return [Symbol] User input (:next, :back, :abort)
+      #   or :next if there are not licenses to accept
       def register_addons
         return :next if selected_addons.empty?
         registration_ui.register_addons(selected_addons, {})
       end
 
+      # Selects packages for installation
+      #
+      # It displays an error when a package cannot be selected.
+      #
+      # @return [:next]
       def select_packages
         ::Registration::SwMgmt.select_addon_products
         package_search_dialog.selected_packages.each do |pkg|
-          Yast::Pkg.PkgInstall(pkg.name)
+          pkg_install_error_message(pkg.name) unless Yast::Pkg.PkgInstall(pkg.name)
         end
         :next
       end
@@ -159,6 +181,17 @@ def selected_addons
       def reset_selected_addons_cache!
         @selected_addons = nil
       end
+
+      def pkg_install_error_message(name)
+        Yast2::Popup.show(
+          format(
+            # TRANSLATORS: 'name' is the package's name
+            _("Package %{name} could not be selected for installation."),
+            name: name
+          ),
+          headline: :error
+        )
+      end
     end
   end
 end

src/lib/registration/dialogs/online_search.rb

@@ -61,6 +61,11 @@ def run
         ret
       end
 
+      # @macro seeDialog
+      def back_button
+        ""
+      end
+
     private
 
       # Package search widget

src/lib/registration/package_search.rb

@@ -59,6 +59,7 @@ def packages
 
         remote_packages = pkg["products"].map do |product|
           RemotePackage.new(
+            id:      pkg["id"],
             name:    pkg["name"],
             version: pkg["version"],
             release: pkg["release"],

src/lib/registration/remote_package.rb

@@ -24,20 +24,24 @@ module Registration
   #
   # @example Find the status
   class RemotePackage
-    attr_reader :name, :arch, :version, :release, :addon
+    attr_reader :id, :name, :arch, :version, :release, :addon
 
+    # @param id      [Integer] Package ID
     # @param name    [String] Package name
     # @param arch    [String] Architecture
     # @param version [String] Version number
     # @param release [String] Release number
     # @param addon   [Addon]  Addon which the package belongs to
-    def initialize(name:, arch:, version:, release:, addon:)
+    # rubocop:disable Metrics/ParameterLists
+    def initialize(id:, name:, arch:, version:, release:, addon:)
+      @id = id
       @name = name
       @arch = arch
       @version = version
       @release = release
       @addon = addon
     end
+    # rubocop:enable Metrics/ParameterLists
 
     def full_version
       "#{version}-#{release}"

src/lib/registration/widgets/package_search.rb

@@ -23,8 +23,10 @@
 require "registration/widgets/remote_packages_table"
 require "registration/widgets/remote_package_details"
 require "registration/package_search"
+require "yast2/popup"
 
 Yast.import "Popup"
+Yast.import "HTML"
 
 module Registration
   module Widgets
@@ -138,12 +140,13 @@ def start_search_event?(event)
       #
       # @param text [String] Text to search for
       def search_package(text)
+        return unless valid_search_text?(text)
         @search = ::Registration::PackageSearch.new(text: text)
         # TRANSLATORS: searching for packages
         Yast::Popup.Feedback(_("Searching..."), _("Searching for packages")) do
-          selected_package_names = @selected_packages.map(&:name)
+          selected_package_ids = selected_packages.map(&:id)
           @search.packages.each do |pkg|
-            pkg.select! if selected_package_names.include?(pkg.name)
+            pkg.select! if selected_package_ids.include?(pkg.id)
           end
         end
         packages_table.change_items(@search.packages)
@@ -155,7 +158,8 @@ def search_package(text)
       # @return [RemotePackage,nil]
       def find_current_package
         return unless search && packages_table.value
-        search.packages.find { |p| p.name == packages_table.value }
+        selected_id = packages_table.value
+        search.packages.find { |p| p.id == selected_id }
       end
 
       # Selects/unselects the current package for installation
@@ -177,20 +181,62 @@ def toggle_package
 
       # Selects the current package for installation
       #
-      # If required, it selects the addon for registration.
+      # If required, it selects the corresponding addon
+      #
+      # @param package [RemotePackage] Package to select
       def select_package(package)
         addon = package.addon
-        return unless addon.registered? || addon.selected? || enable_addon?(addon)
-
-        addon.selected unless addon.registered? || addon.selected?
-        package.select!
-        @selected_packages << package
+        select_addon(addon) if addon
+        set_package_as_selected(package) if addon.nil? || addon.selected? || addon.registered?
       end
 
       # Unselects the current package for installation
+      #
+      # If not needed, unselects the corresponding addon
+      #
+      # @param package [RemotePackage] Package to unselect
+      #
+      # @see #unselect_addon
+      # @see #unselect_package!
       def unselect_package(package)
+        unset_package_as_selected(package)
+        unselect_addon(package.addon) if package.addon
+      end
+
+      # Selects the given addon if needed
+      #
+      # @param addon [Addon] Addon to select
+      def select_addon(addon)
+        return if addon.registered? || addon.selected? || addon.auto_selected?
+        addon.selected if enable_addon?(addon)
+      end
+
+      # Unselects the given addon if required
+      #
+      # @param addon [Addon] Addon to unselect
+      def unselect_addon(addon)
+        return if addon.registered? || needed_addon?(addon)
+        addon.unselected if disable_addon?(addon)
+      end
+
+      # Sets the package as selected
+      #
+      # Marks the package as selected and adds it to the list of selected packages.
+      #
+      # @param package [RemotePackage] Package to add
+      def set_package_as_selected(package)
+        package.select!
+        selected_packages << package
+      end
+
+      # Unsets the package as selected
+      #
+      # Marks the package as not selected and removes it from the list of selected packages.
+      #
+      # @param package [RemotePackage] Package to remove
+      def unset_package_as_selected(package)
         package.unselect!
-        @selected_packages.delete(package)
+        selected_packages.delete(package)
       end
 
       # Updates the package details widget
@@ -201,16 +247,73 @@ def update_details
 
       # Asks the user to enable the addon
       #
-      # It omits the question if the addon is already registered or selected for registration.
-      #
       # @param addon [Addon] Addon to ask about
       def enable_addon?(addon)
+        description = Yast::HTML.Para(
+          format(
+            _("The selected package is provided by the '%{name}', " \
+              "which is not enabled on this system yet."),
+            name: addon.name
+          )
+        )
+
+        unselected_deps = addon.dependencies.reject { |d| d.selected? || d.registered? }
+        if !unselected_deps.empty?
+          description << Yast::HTML.Para(
+            format(
+              _("Additionally, '%{name}' depends on the following modules/extensions:"),
+              name: addon.name
+            )
+          )
+          description << Yast::HTML.List(unselected_deps.map(&:name))
+        end
+        # TRANSLATORS: 'it' and 'them' refers to the modules/extensions to enable
+        question = n_(
+          "Do you want to enable it?", "Do you want to enable them?", unselected_deps.size + 1
+        )
+        yes_no_popup(description + question)
+      end
+
+      # Asks the user to disable the addon
+      #
+      # @param addon [Addon] Addon to ask about
+      def disable_addon?(addon)
         message = format(
-          _("'%{name}' module/extension is not enabled for this system.\n" \
-            "Do you want to enable it?"),
+          _("The '%{name}' is not needed anymore.\n" \
+            "Do you want to unselect it?"),
           name: addon.name
         )
-        Yast::Popup.YesNo(message)
+        yes_no_popup(message)
+      end
+
+      MINIMAL_SEARCH_TEXT_SIZE = 2
+
+      # Determines whether the search text is valid or not
+      #
+      # @param text [String] Text to search for
+      def valid_search_text?(text)
+        return true if text.to_s.size >= MINIMAL_SEARCH_TEXT_SIZE
+        Yast2::Popup.show(
+          format(
+            # TRANSLATORS: the minimal size of the text to search for package names
+            _("Please, type at least %{minimal_size} characters to search for."),
+            minimal_size: MINIMAL_SEARCH_TEXT_SIZE
+          )
+        )
+        false
+      end
+
+      # Determines whether the addon is still needed
+      def needed_addon?(addon)
+        selected_packages.any? { |pkg| pkg.addon == addon }
+      end
+
+      # Asks a yes/no question
+      #
+      # @return [Boolean] true if the answer is affirmative; false otherwise
+      def yes_no_popup(message)
+        ret = Yast2::Popup.show(message, richtext: true, buttons: :yes_no)
+        ret == :yes
       end
     end
   end