Skip to content

Commit

Permalink
[webui] Improve Kiwi error message
Browse files Browse the repository at this point in the history
  • Loading branch information
David Kang committed Oct 18, 2017
1 parent 31d1a63 commit ba6550b
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,9 @@
}
}
}

.no-bullet {
list-style: none;
margin-top: 3px;
}

5 changes: 3 additions & 2 deletions src/api/app/controllers/webui/kiwi/images_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.messages.map { |_key, value| value }].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
Expand All @@ -46,8 +47,8 @@ def update
end
redirect_to action: :show
rescue ActiveRecord::RecordInvalid, Timeout::Error
flash.now[:error] = ['Cannot update kiwi image:', @image.errors.messages.map { |_key, value| value }].join('<br/>')
@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

Expand Down
19 changes: 19 additions & 0 deletions src/api/app/helpers/webui/webui_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,23 @@ def toggle_sliced_text(text, slice_length = 50, id = "toggle_sliced_text_#{Time.
end
short + long
end

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
21 changes: 20 additions & 1 deletion src/api/app/models/kiwi/image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/api/app/models/kiwi/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class Kiwi::Repository < ApplicationRecord
# 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, message: "%{attribute} '%{value}' is not included in the list." }
validates :replaceable, inclusion: { in: [true, false], message: "%{attribute} has to be 'true' or 'false'" }
validates :imageinclude, :prefer_license, inclusion: { in: [true, false], message: "%{attribute} has to be 'true' or 'false'" }, allow_nil: true
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)
Expand All @@ -48,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, "Repo type for '#{source_path}' 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, "Source path '#{source_path}' has an invalid format.")
errors.add(:source_path, "Source path has an invalid format.")
end

def to_xml
Expand Down
4 changes: 1 addition & 3 deletions src/api/app/views/layouts/webui/_flash.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions src/api/spec/controllers/webui/kiwi/images_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
filename: "#{package_with_kiwi_file.name}.kiwi"))
end

it { expect(flash[:error]).to end_with('please remove the other repositories.') }
it { expect(flash[:error]).to match('please remove the other repositories.') }
end
end

Expand All @@ -109,7 +109,10 @@
package: package_with_kiwi_file,
filename: "#{package_with_kiwi_file.name}.kiwi"))
end
it { expect(flash[:error]).to match(/Multiple package groups with same type are not allowed/) }

it do
expect(flash[:error]).to match_array(['Multiple package groups with same attributes is not allowed'])
end
end
end
end
Expand Down Expand Up @@ -161,7 +164,8 @@

subject { post :update, params: invalid_repositories_update_params }

it { expect(subject.request.flash[:error]).to match(/Source path 'htt:\/\/example.com' has an invalid format./) }
it { expect(subject.request.flash[:error]).to match(/Repository: 'htt:\/\/example.com'/) }
it { expect(subject.request.flash[:error]).to match(/Source path has an invalid format./) }
it { expect(subject.request.flash[:error]).to match(/Repo type 'apt2-deb' is not included in the list./) }
it { expect(subject).to have_http_status(:success) }
it { expect(subject).to render_template(:show) }
Expand Down Expand Up @@ -233,11 +237,13 @@
package_groups_attributes: {
'0' => {
id: kiwi_package.package_group.id,
kiwi_type: 'image',
packages_attributes: {
'0' => {
id: kiwi_package.id,
name: "",
arch: "x86"
id: kiwi_package.id,
name: "",
arch: "x86",
package_group_id: kiwi_package.package_group.id
}
}
}
Expand Down

0 comments on commit ba6550b

Please sign in to comment.