From 328abda6157af975f9251c854320f064dafa5a82 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Mon, 23 Jan 2012 18:03:19 +0100 Subject: [PATCH] SONAR-1492 Allow notes per quality rule Admin page implemented --- .../resources/org/sonar/l10n/core.properties | 14 ++- .../rules_configuration_controller.rb | 40 ++++++- .../WEB-INF/app/models/active_rule_note.rb | 9 ++ .../webapp/WEB-INF/app/models/rule_note.rb | 8 ++ .../_create_rule_note.html.erb | 48 ++++++++ .../views/rules_configuration/_rule.html.erb | 113 ++++++++++++------ .../rules_configuration/_rule_note.html.erb | 36 ++++++ .../rules_configuration/_rule_param.html.erb | 8 +- .../views/rules_configuration/index.html.erb | 9 +- .../src/main/webapp/stylesheets/style.css | 24 +++- 10 files changed, 257 insertions(+), 52 deletions(-) create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_create_rule_note.html.erb create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_note.html.erb diff --git a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties index c6363a34c7a..6be2ee69ff8 100644 --- a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -458,7 +458,7 @@ reviews.filtered_by.to=To date # #------------------------------------------------------------------------------ -action_plans.page_title=Manage Action plans +action_plans.page_title=Manage action plans action_plans.add_action_plan=Add action plan action_plans.col.status=St. action_plans.col.name=Name @@ -984,6 +984,18 @@ rules_configuration.rule_deleted=Rule deleted rules_configuration.unknown_rule=Unknown rule rules_configuration.x_rules_have_been_activated={0} rules have been activated. rules_configuration.x_rules_have_been_deactivated={0} rules have been deactivated. +rules_configuration.rule_description=Description +rules_configuration.rule_note=Note +rules_configuration.rule_activation_note=Activation note +rules_configuration.confirm_delete_note=Do you really want to delete this note? +rules_configuration.add_note=Add note +rules_configuration.add_activation_note=Add activation note +rules_configuration.adding_note_description=This note will be linked to the current rule (whatever the profile) and will be visible to every user. It can be used to extend the predefined rule description. +rules_configuration.adding_activation_note_decsription=This activation note will be linked to the active rule of the current profile and will be visible to every user. It can be used to explain why this rule was activated on this profile and/or why such parameter was specified. +rules_configuration.create_note=Create note +rules_configuration.create_activation_note=Create activation note +rules_configuration.rule_identification=Identification +rules_configuration.rule_parameters=Parameters #------------------------------------------------------------------------------ diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb index 4fb539ecf85..b8563fb4a70 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb @@ -29,7 +29,9 @@ class RulesConfigurationController < ApplicationController RULE_PRIORITIES = Sonar::RulePriority.as_options.reverse # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html) - verify :method => :post, :only => ['activate_rule', 'update_param', 'bulk_edit', 'create', 'update', 'delete', 'revert_rule'], :redirect_to => { :action => 'index' } + verify :method => :post, + :only => ['activate_rule', 'update_param', 'bulk_edit', 'create', 'update', 'delete', 'revert_rule', 'update_note', 'delete_note'], + :redirect_to => { :action => 'index' } before_filter :admin_required, :except => [ 'index', 'export' ] @@ -312,6 +314,42 @@ class RulesConfigurationController < ApplicationController :locals => {:parameter => rule_param, :active_parameter => active_param, :profile => profile, :active_rule => active_rule, :is_admin => is_admin } end + + def update_note + if params[:is_active_rule] + rule = ActiveRule.find(params[:rule_id]) + else + rule = Rule.find(params[:rule_id]) + end + note = rule.note + unless note + if params[:is_active_rule] + note = ActiveRuleNote.new({:rule => rule}) + else + note = RuleNote.new({:rule => rule}) + end + # set the note on the rule to avoid reloading the rule + rule.note = note + end + note.text = params[:text] + note.user_login = current_user.login + note.save! + render :partial => 'rule_note', :locals => {:rule => rule, :is_admin => true, :is_active_rule => params[:is_active_rule] } + end + + + def delete_note + if params[:is_active_rule] + rule = ActiveRule.find(params[:rule_id]) + else + rule = Rule.find(params[:rule_id]) + end + rule.note.destroy if rule.note + render :text => '' + end + + + private # return the number of newly activated rules diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/active_rule_note.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/active_rule_note.rb index 45bdfb4b8bb..b306bec7393 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/active_rule_note.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/active_rule_note.rb @@ -20,8 +20,17 @@ class ActiveRuleNote < ActiveRecord::Base belongs_to :active_rule alias_attribute :text, :data + alias_attribute :rule, :active_rule validates_presence_of :active_rule, :message => "can't be empty" validates_presence_of :user_login, :message => "can't be empty" + validates_length_of :data, :minimum => 1 + + def user + @user ||= + begin + user_login ? User.find(:first, :conditions => ['login=?', user_login]) : nil + end + end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_note.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_note.rb index c879875a95f..ca7c805a4f7 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule_note.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule_note.rb @@ -23,5 +23,13 @@ class RuleNote < ActiveRecord::Base validates_presence_of :rule, :message => "can't be empty" validates_presence_of :user_login, :message => "can't be empty" + validates_length_of :data, :minimum => 1 + + def user + @user ||= + begin + user_login ? User.find(:first, :conditions => ['login=?', user_login]) : nil + end + end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_create_rule_note.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_create_rule_note.html.erb new file mode 100644 index 00000000000..ebc2ddaaa9a --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_create_rule_note.html.erb @@ -0,0 +1,48 @@ +<% #locals = rule, active_rule, is_admin + create_buttons_div_id = "cb_" + rule.id.to_s + create_rule_note_button_div_id = "cnb_" + rule.id.to_s + create_active_rule_note_button_div_id = "canb_" + rule.id.to_s + create_rule_note_form_div_id = "crnf_" + rule.id.to_s + create_active_rule_note_form_div_id = "carnf_" + rule.id.to_s + submit_rule_note_id = "srn_" + rule.id.to_s + submit_active_rule_note_id = "sarn_" + rule.id.to_s + rule_note_text_id = "rnt_" + rule.id.to_s + active_rule_note_text_id = "arnt_" + rule.id.to_s +%> + +
+ +
+ > + > +
+ + + +<% if active_rule%> + +<% end %> + +
\ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb index 8ca525c5202..da6cad72dd4 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb @@ -1,5 +1,8 @@ - -
+<% color = cycle('even','odd') %> + + + + <% enable_modification = is_admin && !profile.provided? select_box_id = "levels_select_#{rule.id}" check_box_id = "levels_check_#{rule.id}" @@ -25,42 +28,76 @@ Overrides parent definition <% end %> <% end %> -
- - - <%= link_to_function("#{h rule.name}", nil, :class => "") do |page| + + + + <%= link_to_function("#{h rule.name}", nil, :class => "") do |page| page.toggle "desc_#{rule.id}" end - %> - - - -<%= rule.plugin_name.capitalize %> - - + + + + + + + + + + + + + +
+

<%= message('rules_configuration.rule_description') -%>

+

<%= rule.description %>

+
+ <%= render :partial => 'rule_note', :locals => {:rule => rule, :is_admin => is_admin, :is_active_rule => false } %> +
+ <% if active_rule %> +
+ <%= render :partial => 'rule_note', :locals => {:rule => active_rule, :is_admin => is_admin, :is_active_rule => true } %> +
+ <% end %> + <% if is_admin %> + <%= render :partial => 'create_rule_note', :locals => {:rule => rule, :active_rule => active_rule, :is_admin => is_admin } %> + <% end %> + 		+

<%= message('rules_configuration.rule_identification') -%>

+ + + + + +
<%= message('plugin') -%>:<%= rule.plugin_name.capitalize %>
<%= message('key') -%>:<%= rule.plugin_rule_key %>
+ <% unless rule.parameters.empty? %> +
+

<%= message('rules_configuration.rule_parameters') -%>

+ + <% rule.parameters.each do |parameter| + active_parameter = active_rule.active_param_by_param_id(parameter.id) if active_rule + %> + + <%= render :partial => 'rule_param', :object => nil, + :locals => {:parameter => parameter, :active_parameter => active_parameter, :profile => profile, :active_rule => active_rule, :is_admin => is_admin } %> + + <% end %> +
+ <% end %> + <% if is_admin %> + <% if rule.template? %> + <%= button_to message('rules_configuration.copy_rule'), {:action => 'new', :id => profile.id, :rule_id => rule.id}, :id => "copy-#{u rule.key}" %> + <% end %> + <% if rule.editable? %> + <%= button_to message('rules_configuration.edit_rule'), :action => 'edit', :id => profile.id, :rule_id => rule.id %> + <% end %> + <% if active_rule && active_rule.overrides? %> + <%= button_to message('rules_configuration.revert_to_parent_definition'), :overwrite_params => {:action => 'revert_rule', :id => profile.id, :active_rule_id => active_rule.id} %>
+ <% end %> + <% end %> +
+ + diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_note.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_note.html.erb new file mode 100644 index 00000000000..208a1fe98c4 --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_note.html.erb @@ -0,0 +1,36 @@ +<% #locals = rule, is_admin, is_active_rule + submit_note_update_button_id = (is_active_rule ? 'a' : '') + "snub_" + rule.id.to_s +%> + +<% + if rule.note + note = rule.note +%> +

<%= is_active_rule ? message('rules_configuration.rule_activation_note') : message('rules_configuration.rule_note') -%>

+
<%= note.user.name -%>, <%= l(note.updated_at) -%>
+
+

<%= sanitize(note.text) -%>

+ <% if is_admin %> +
+ + <%= button_to_remote message('delete'), + {:url => {:action => 'delete_note', :rule_id => rule.id, :is_active_rule => is_active_rule }, + :update => "#{'active_' if is_active_rule}rule_note_#{rule.id}", + :complete => "$('#{is_active_rule ? "canb_"+rule.rule.id.to_s : "cnb_"+rule.id.to_s}').show()", + :confirm => message('rules_configuration.confirm_delete_note')}, + {:class => 'red-button'} -%> +
+ <% end %> +
+ + +<% end %> \ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_param.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_param.html.erb index 0e011d5322e..0a2ee16ba06 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_param.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule_param.html.erb @@ -32,7 +32,13 @@ <%= active_parameter.errors.on 'value' %> <% end %> <% end %> - <%= h parameter.description -%> + + + + + <%= h parameter.description -%> + + diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb index 484fce0b348..be2e5c9b50c 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/index.html.erb @@ -20,7 +20,7 @@ return !localModifications; } function toggle_rules(){ - $$('.rule_desc').each(function(element) { + $$('.rule_detail').each(function(element) { element.toggle(); }); } @@ -106,7 +106,6 @@ <%= message('active') -%>/<%= message('severity') -%> <%= message('name') -%> [<%= link_to_function(message('rules_configuration.expand_collapse'), "toggle_rules()") %>] - <%= message('plugin') -%> @@ -119,9 +118,9 @@ @rules.each do |rule| active_rule = @profile.active_by_rule_id(rule.id) %> - - <%= render :partial => 'rule', :object => rule, :locals => {:profile => @profile, :active_rule => active_rule, :is_admin => is_admin} %> - + + <%= render :partial => 'rule', :object => rule, :locals => {:profile => @profile, :active_rule => active_rule, :is_admin => is_admin} %> + <% end %> diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css index 4bc8b09f896..b2438ffdec5 100644 --- a/sonar-server/src/main/webapp/stylesheets/style.css +++ b/sonar-server/src/main/webapp/stylesheets/style.css @@ -2046,32 +2046,44 @@ ul.bullet li { width: 100%; } -.rule_desc { +.rule_detail { color: #444; - padding: 2px 0; } -.rule_desc li { +.rule_detail > td > table { + margin: 4px; +} + +.rule_detail td { + vertical-align: text-top; +} + +.rule_detail li { list-style: disc outside; padding: 2px; } -.rule_desc ul { +.rule_detail ul { list-style: none outside; padding-left: 30px; } -.rule_desc pre, .rule_desc p { +.rule_detail pre, .rule_detail p { padding: 7px; } -.rule_desc pre { +.rule_detail pre { margin: 10px 0; font-family: "Courier New", Courier, monospace;; border: 1px dashed #aaa; font-size: 93%; } +.rule_detail .separator { + background: url('../images/sep12.png') center repeat-y; + min-width: 20px; +} + .tip:hover { background: #FFF; position: relative; -- 2.39.5