SONAR-3529 Clean code

5 files changed, 26 insertions, 33 deletions

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 255294383cf..696296616d7 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
@@ -49,19 +49,15 @@ class SettingsController < ApplicationController
   # TODO: Validation
   def save_property_sets(resource_id)
     params[:property_sets].each do |key, value|
-      value = drop_trailing_blank_values(value)
+      set_keys = drop_trailing_blank_values(value)
 
-      # TODO: clear all
-      Property.set(key, value.map { |v| v.gsub(',', '%2C') }.join(','), resource_id)
+      Property.with_key_prefix(key + '.').delete_all
+      Property.set(key, to_string(set_keys), resource_id)
 
-      fields = params[key]
-
-      fields.each do |field_key, field_values|
+      params[key].each do |field_key, field_values|
         field_values.each_with_index do |field_value, index|
-          set_key = value[index]
-          if set_key
-            Property.set(key + "." + set_key + "." + field_key, field_value, resource_id)
-          end
+          set_key = set_keys[index]
+          Property.set("#{key}.#{set_key}.#{field_key}", field_value, resource_id) if set_key
         end
       end
     end
@@ -86,6 +82,10 @@ class SettingsController < ApplicationController
     array.reverse.drop_while(&:blank?).reverse
   end
 
+  def to_string(set_keys)
+    set_keys.map { |v| v.gsub(',', '%2C') }.join(',')
+  end
+
   def load_properties
     @category = params[:category] || 'general'
 
@@ -102,5 +102,4 @@ class SettingsController < ApplicationController
 
     not_found('category') unless @categories.include? @category
   end
-
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
index c97111ef077..3281dce96f5 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
@@ -44,9 +44,14 @@ module SettingsHelper
 
   # for backward-compatibility with properties that do not define the type TEXT
   def property_type(property, value)
+    unless property.fields.blank?
+      return 'PROPERTY_SET_DEFINITION'
+    end
+
     if property.getType().to_s=='STRING' && value && value.include?('\n')
       return 'TEXT'
     end
+
     property.getType()
   end
 
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 151657d11b0..366cc3b9844 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
@@ -21,7 +21,8 @@ class Property < ActiveRecord::Base
   validates_presence_of :prop_key
 
   named_scope :with_key, lambda { |value| {:conditions => {:prop_key, value}} }
-  named_scope :with_value, lambda { |value| {:conditions => ['text_value like ?', value] } }
+  named_scope :with_key_prefix, lambda { |value| {:conditions => ['prop_key like ?', value + '%']} }
+  named_scope :with_value, lambda { |value| {:conditions => ['text_value like ?', value]} }
   named_scope :with_resource, lambda { |value| {:conditions => {:resource_id => value}} }
   named_scope :with_user, lambda { |value| {:conditions => {:user_id => value}} }
   named_scope :with_resources, :conditions => 'resource_id is not null'
@@ -50,7 +51,7 @@ class Property < ActiveRecord::Base
   end
 
   def self.clear_for_resources(key, value=nil)
-    scope=Property.with_resources().with_key(key)
+    scope = Property.with_resources().with_key(key)
     if value
       scope.with_value(value)
     end
@@ -66,7 +67,7 @@ class Property < ActiveRecord::Base
   end
 
   def self.by_key_prefix(prefix)
-    Property.find(:all, :conditions => ['prop_key like ?', prefix + '%'])
+    Property.with_key_prefix(prefix)
   end
 
   def self.value(key, resource_id=nil, default_value=nil, user_id=nil)
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
index 0a6a04db28d..284a89f39cd 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
@@ -1,5 +1 @@
-<% if property.fields.blank? -%>
-  <%= render "settings/type_#{property_type(property, value)}", :property => property, :value => value -%>
-<% else -%>
-  <%= render "settings/type_PROPERTY_SET_DEFINITION", :property => property, :value => value -%>
-<% end -%>
\ No newline at end of file
+<%= render "settings/type_#{property_type(property, value)}", :property => property, :value => value -%>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_PROPERTY_SET_DEFINITION.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_PROPERTY_SET_DEFINITION.html.erb
index 029762ae2a7..3ebfdfffdf7 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_PROPERTY_SET_DEFINITION.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_PROPERTY_SET_DEFINITION.html.erb
@@ -1,21 +1,13 @@
-<% if Property.value('sonar.test.jira.servers').blank? -%>
-  <% Property.set('sonar.test.jira.servers', 'jira1,jira2') -%>
-  <% Property.set('sonar.test.jira.servers.jira1.url', 'http://url1') -%>
-  <% Property.set('sonar.test.jira.servers.jira1.port', '1234') -%>
-  <% Property.set('sonar.test.jira.servers.jira2.url', 'http://url2') -%>
-  <% Property.set('sonar.test.jira.servers.jira2.port', '9999') -%>
-<% end -%>
-
 <% resource_id = @resource.id if @resource -%>
 
 <% Property.values(property.key, resource_id).reject(&:blank?).each do |set_key| -%>
   <div class="multi_value">
-    <label>Key: </label><br/>
-    <input name="property_sets[<%= property.key -%>][]" value="<%= set_key -%>"/><br/>
+    <%= text_field_tag "property_sets[#{property.key}][]", set_key -%>
+    <br/>
 
     <% property.fields.each do |field| -%>
       <label><%= field.key -%>: </label><br/>
-      <input type="text" name="<%= property.key -%>[<%= field.key -%>][]" value="<%= Property.value("#{property.key}.#{set_key}.#{field.key}", resource_id) -%>"/>
+      <%= text_field_tag "#{property.key}[#{field.key}][]", Property.value("#{property.key}.#{set_key}.#{field.key}", resource_id) -%>
       <br/>
     <% end -%>
 
@@ -26,12 +18,12 @@
 
 <div class="template" style="display:none;">
   <div class="multi_value">
-    <label>Key: </label><br/>
-    <input name="property_sets[<%= property.key -%>][]" value=""/><br/>
+    <%= text_field_tag "property_sets[#{property.key}][]" %>
+    <br/>
 
     <% property.fields.each do |field| -%>
       <label><%= field.key -%>: </label><br/>
-      <input type="text" name="<%= property.key -%>[<%= field.key -%>][]" value=""/>
+      <%= text_field_tag "#{property.key}[#{field.key}][]" -%>
       <br/>
     <% end -%>