From 8d327596480739737e24553a127b7e7cf5346641 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 2 Oct 2012 08:47:51 +0200 Subject: [PATCH] SONAR-3529 Improve code --- .../java/org/sonar/api/PropertyField.java | 3 ++ .../app/controllers/settings_controller.rb | 44 ++++++------------- .../webapp/WEB-INF/app/models/property.rb | 44 ++++++++++++------- .../app/views/settings/_error.html.erb | 8 ++-- .../app/views/settings/_multi_value.html.erb | 4 +- .../app/views/settings/_properties.html.erb | 8 ++-- 6 files changed, 53 insertions(+), 58 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/PropertyField.java b/sonar-plugin-api/src/main/java/org/sonar/api/PropertyField.java index c8114b724da..eb50526b67b 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/PropertyField.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/PropertyField.java @@ -34,6 +34,9 @@ import java.lang.annotation.Target; public @interface PropertyField { /** * Unique key within a property. It shouldn't be prefixed. + * Settings for this field are stored into the database with a composite key + * {key of parent property}.{key of the set}.{key of this field} + * eg. sonar.jira.servers.JIRA1.url */ String key(); diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb index bbb052d4261..83741be6f9f 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb @@ -40,53 +40,37 @@ class SettingsController < ApplicationController load_properties() @updated_properties = {} - save_properties(resource_id) - save_property_sets(resource_id) + update_properties(resource_id) + update_property_sets(resource_id) render :partial => 'settings/properties' end private - def save_property_sets(resource_id) - (params[:property_sets] || []).each do |key, value| - set_keys = drop_trailing_blank_values(value) + def update_properties(resource_id) + (params[:settings] || []).each do |key, value| + update_property(key, value, resource_id) + end + end + def update_property_sets(resource_id) + (params[:property_sets] || []).each do |key, set_keys| Property.with_key_prefix(key + '.').delete_all - Property.set(key, to_string(set_keys), resource_id) + update_property(key, set_keys, resource_id) params[key].each do |field_key, field_values| - field_values.each_with_index do |field_value, index| - set_key = set_keys[index] + field_values.zip(set_keys).each do |field_value, set_key| if set_key - field_property_key = "#{key}.#{set_key}.#{field_key}" - @updated_properties[field_property_key] = Property.set(field_property_key, field_value, resource_id) + update_property("#{key}.#{set_key}.#{field_key}", field_value, resource_id) end end end end end - def save_properties(resource_id) - (params[:settings] || []).each do |key, value| - if value.kind_of? Array - value = drop_trailing_blank_values(value) - end - - if value.blank? - Property.clear(key, resource_id) - else - @updated_properties[key] = Property.set(key, value, resource_id) - end - end - end - - def drop_trailing_blank_values(array) - array.reverse.drop_while(&:blank?).reverse - end - - def to_string(set_keys) - set_keys.map { |v| v.gsub(',', '%2C') }.join(',') + def update_property(key, value, resource_id) + @updated_properties[key] = Property.set(key, value, resource_id) end def load_properties diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb index 3ebf1126a2c..0b84d1ac9ea 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb @@ -48,6 +48,7 @@ class Property < ActiveRecord::Base all(key, resource_id, user_id).delete_all setGlobalProperty(key, nil, resource_id, user_id) end + prop end def self.clear_for_resources(key, value=nil) @@ -84,31 +85,36 @@ class Property < ActiveRecord::Base end def self.set(key, value, resource_id=nil, user_id=nil) - definition = Java::OrgSonarServerUi::JRubyFacade.getInstance().propertyDefinitions.get(key) - if definition && definition.multi_values - if value.kind_of? Array + if value.kind_of? Array + value = drop_trailing_blank_values(value) + + definition = Java::OrgSonarServerUi::JRubyFacade.getInstance().propertyDefinitions.get(key) + if definition && (definition.multi_values || !definition.fields.blank?) value = value.map { |v| v.gsub(',', '%2C') }.join(',') + else + value = value.first end - elsif value.kind_of? Array - value = value.first end text_value = value.to_s if defined? value text_value = nil if text_value.blank? + if text_value.blank? + return Property.clear(key, resource_id) + end + prop = by_key(key, resource_id, user_id) - if prop - if prop.text_value != text_value - prop.text_value = text_value - if prop.save - setGlobalProperty(key, text_value, resource_id, user_id) - end - end - else - prop = Property.new(:prop_key => key, :text_value => text_value, :resource_id => resource_id, :user_id => user_id) - if prop.save - setGlobalProperty(key, text_value, resource_id, user_id) - end + if prop && prop.text_value == text_value + return prop + end + + unless prop + prop = Property.new(:prop_key => key, :resource_id => resource_id, :user_id => user_id) + end + + prop.text_value = text_value + if prop.save + setGlobalProperty(key, text_value, resource_id, user_id) end prop @@ -167,6 +173,10 @@ class Property < ActiveRecord::Base Property.with_key(key).with_resource(resource_id).with_user(user_id) end + def self.drop_trailing_blank_values(array) + array.reverse.drop_while(&:blank?).reverse + end + def validate validate_property() validate_field() diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb index c1f5cf0a517..6dff575b2b0 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb @@ -1,7 +1,5 @@ -<% if @updated_properties -%> - <% p = @updated_properties[key] -%> +<% updated_property = @updated_properties[key] if @updated_properties -%> - <% if p && !p.valid? -%> -
<%= p.validation_error_message -%>
- <% end -%> +<% if updated_property && !updated_property.valid? -%> +
<%= updated_property.validation_error_message -%>
<% end -%> diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_multi_value.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_multi_value.html.erb index b2fb09af73d..06897360059 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_multi_value.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_multi_value.html.erb @@ -1,7 +1,5 @@
<%= render "settings/single_value", :property => property, :value => value -%> - <% if delete_link -%> - <%= message('delete') -%> - <% end -%> + <%= message('delete') -%>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb index 9626071b3ee..20da1396a43 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb @@ -28,11 +28,11 @@ <% value = property_value(property) -%> <% if property.multi_values -%> - <% value.each do |sub_value| -%> - <%= render "settings/multi_value", :property => property, :value => sub_value, :delete_link => true -%> + <% value.each_with_index do |sub_value, index| -%> + <%= render "settings/multi_value", :property => property, :value => sub_value, :hide_delete => ((index == 0) || (value == 1)) -%> <% end -%>
@@ -72,6 +72,8 @@ }); $j('.add_value').live('click', function () { + $j('.delete').show(); + var template = $j(this).siblings('.template'); template.before(template.html()); return false; -- 2.39.5