Merge pull request #3981 from DavidKang/kiwi-editor-flash-messages

Kiwi Editor - Add understandable flash message

src/api/app/assets/javascripts/webui/application/kiwi_editor.js

@@ -123,6 +123,7 @@ function closeDialog() {
   }
 
   fields.find(".ui-state-error").addClass('hidden');
+  fields.find('.kiwi_list_item').removeClass('has-error');
   dialog.removeClass('new_element');
 
   if (!canSave) {

src/api/app/assets/stylesheets/webui/application/kiwi_image.scss

@@ -75,5 +75,15 @@
     }
 
     small { margin: 0; }
+
+    &.has-error {
+      a { color: #c00; }
+    }
   }
 }
+
+.no-bullet {
+  list-style: none;
+  margin-top: 3px;
+}
+

src/api/app/controllers/webui/kiwi/images_controller.rb

@@ -20,7 +20,8 @@ def import_from_package
         if package.kiwi_image.blank? || package.kiwi_image.destroyed?
           package.kiwi_image = ::Kiwi::Image.build_from_xml(package.source_file(kiwi_file), package.kiwi_file_md5)
           unless package.save
-            errors = ["Kiwi File '#{kiwi_file}' has errors:", package.kiwi_image.errors.full_messages].join('<br />')
+            errors = package.kiwi_image.parsed_errors("Kiwi File '#{kiwi_file}' has errors:",
+                                                      [package.kiwi_image.package_groups.map(&:packages)].flatten.compact)
             redirect_to package_view_file_path(project: package.project, package: package, filename: kiwi_file), error: errors
             return
           end
@@ -45,9 +46,9 @@ def update
           @image.write_to_backend
         end
         redirect_to action: :show
-      rescue ActiveRecord::RecordInvalid, Timeout::Error => e
-        flash.now[:error] = "Cannot update kiwi image: #{@image.errors.full_messages.to_sentence} #{e.message}"
+      rescue ActiveRecord::RecordInvalid, Timeout::Error
         @package_groups = @image.package_groups.select(&:kiwi_type_image?).first
+        flash.now[:error] = @image.parsed_errors('Cannot update KIWI Image:', @package_groups.packages)
         render action: :show
       end
 

src/api/app/helpers/webui/flash_helper.rb

@@ -0,0 +1,20 @@
+module Webui::FlashHelper
+  def flash_content(flash)
+    if flash.is_a?(Hash)
+      capture_haml do
+        haml_tag(:span, flash.delete(:title))
+        haml_tag :ul do
+          flash.each do |name, messages|
+            haml_tag(:li, name, class: 'no-bullet')
+            haml_tag :ul do
+              messages.each { |message| haml_tag(:li, message) }
+            end
+          end
+        end
+      end
+    else
+      body = flash.gsub(/\\n/, '')
+      sanitize body, tags: %w(a b ul li br u), attributes: %w(href title)
+    end
+  end
+end

src/api/app/models/kiwi/image.rb

@@ -155,6 +155,25 @@ def self.binaries_available(project, use_project_repositories, repositories)
     end
   end
 
+  # This method is for parse the error messages and group them to make them more understandable.
+  # The nested errors messages are like `Model[0] attributes` and this is not easy to understand.
+  def parsed_errors(title, packages)
+    message = { title: title }
+    message["Image Errors:"] = errors.messages[:base] if errors.messages[:base].present?
+
+    { repository: repositories, package: packages }.each do |key, records|
+      records.each do |record|
+        if record.errors.present?
+          message["#{key.capitalize}: #{record.name}"] ||= []
+          message["#{key.capitalize}: #{record.name}"] << record.errors.messages.values
+          message["#{key.capitalize}: #{record.name}"].flatten!
+        end
+      end
+    end
+
+    message
+  end
+
   private
 
   def repositories_for_xml
@@ -169,7 +188,7 @@ def check_use_project_repositories
     return unless use_project_repositories? && repositories.present?
 
     errors.add(:base,
-               "A repository with source_path=\"obsrepositories:/\" has been set. If you want to use it, please remove the other repositories.")
+               "A repository with source_path \"obsrepositories:/\" has been set. If you want to use it, please remove the other repositories.")
   end
 
   def check_package_groups

src/api/app/models/kiwi/package.rb

@@ -2,7 +2,7 @@ class Kiwi::Package < ApplicationRecord
   belongs_to :package_group
   has_one :kiwi_image, through: :package_groups
 
-  validates :name, presence: true
+  validates :name, presence: { message: 'Package name can\'t be blank'}
 
   def to_h
     hash = { name: name }

src/api/app/models/kiwi/repository.rb

@@ -18,16 +18,17 @@ class Kiwi::Repository < ApplicationRecord
   #### Scopes (first the default_scope macro if is used)
 
   #### Validations macros
-  validates :alias, :source_path, uniqueness: { scope: :image }
-  validates :source_path, presence: true
+  validates :alias, :source_path, uniqueness: { scope: :image, message: "%{attribute} '%{value}' has already been taken."}
+  validates :source_path, presence: { message: '%{attribute} can\'t be nil.'}
   validate :source_path_format
-  validates :priority, numericality: { only_integer: true, allow_nil: true, greater_than_or_equal_to: 0, less_than: 100 }
+  validates :priority, numericality: { only_integer: true, allow_nil: true, greater_than_or_equal_to: 0,
+                                       less_than: 100, message: '%{attribute} must be between 0 and 99.' }
   validates :order, numericality: { only_integer: true, greater_than_or_equal_to: 1 }
   # TODO: repo_type value depends on packagemanager element
   # https://doc.opensuse.org/projects/kiwi/doc/#sec.description.repository
-  validates :repo_type, inclusion: { in: REPO_TYPES }
-  validates :replaceable, inclusion: { in: [true, false] }
-  validates :imageinclude, :prefer_license, inclusion: { in: [true, false] }, allow_nil: true
+  validates :repo_type, inclusion: { in: REPO_TYPES, message: "%{attribute} '%{value}' is not included in the list." }
+  validates :replaceable, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" }
+  validates :imageinclude, :prefer_license, inclusion: { in: [true, false], message: "%{attribute} has to be a boolean" }, allow_nil: true
   validates_associated :image, on: :update
 
   #### Class methods using self. (public and then private)
@@ -47,10 +48,10 @@ def source_path_format
     return if source_path =~ /\A#{URI.regexp(['ftp', 'http', 'https', 'plain'])}\z/
     if source_path_for_obs_repository?
       return if repo_type == 'rpm-md'
-      errors.add(:repo_type, "should be 'rpm-md' for obs:// repositories")
+      errors.add(:repo_type, "Repo type should be 'rpm-md' for obs:// repositories.")
     end
     return if source_path_for_opensuse_repository?
-    errors.add(:source_path, "has an invalid format")
+    errors.add(:source_path, "Source path has an invalid format.")
   end
 
   def to_xml

src/api/app/views/layouts/webui/_flash.html.haml

@@ -16,12 +16,10 @@
           - when :notice
             - flash_icon = 'ui-icon-info'
             - flash_header = 'ui-state-highlight'
-          - body = flash[flash_type].gsub(/\\n/, '')
-          - body = sanitize body, tags: %w(a b ul li br u), attributes: %w(href title)
           %div{ class: "#{flash_header} ui-corner-all ui-widget-shadow" }
             %p.flash-content
               %span{ class: "ui-icon #{flash_icon}" }
-                = body
+                = flash_content(flash[flash_type])
                 = link_to 'more info', '#', class: 'btn-more' if @more_info
             - if @more_info
               .more_info.hidden

src/api/app/views/webui/kiwi/images/_package_fields.html.haml

@@ -1,5 +1,5 @@
 .nested-fields.kiwi_fields
-  .kiwi_list_item
+  %div{ class: "kiwi_list_item #{'has-error' if f.object.errors.present?}"}
     %span
       = link_to '#', class: "package_edit kiwi_element_name" do
         = f.object.name

src/api/app/views/webui/kiwi/images/_repository_fields.html.haml

@@ -1,5 +1,5 @@
 .nested-fields.kiwi_fields
-  .kiwi_list_item
+  %div{ class: "kiwi_list_item #{'has-error' if f.object.errors.present?}"}
     %span
       = link_to f.object.name, '#', class: 'repository_edit kiwi_element_name'
     %span.kiwi_actions.hidden