From c339385ccf76ec26cf8db19df901cdfe59c712b8 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Tue, 24 Jan 2012 19:19:20 +0100 Subject: [PATCH] SONAR-1492 Improve rule & active rule edition page + use markdown instead of HTML + improve rule doc page --- .../resources/org/sonar/l10n/core.properties | 18 ++-- .../rules_configuration_controller.rb | 46 +++++----- .../WEB-INF/app/models/active_rule_note.rb | 9 +- .../webapp/WEB-INF/app/models/rule_note.rb | 9 +- .../WEB-INF/app/views/rules/show.html.erb | 44 ++++++---- .../_active_rule_note.html.erb | 58 ++++++++++++ .../_create_rule_note.html.erb | 48 ---------- .../views/rules_configuration/_rule.html.erb | 24 ++--- .../rules_configuration/_rule_note.html.erb | 83 +++++++++++------- .../main/webapp/images/reviews/comment.png | Bin 0 -> 1107 bytes .../src/main/webapp/stylesheets/style.css | 7 ++ 11 files changed, 199 insertions(+), 147 deletions(-) create mode 100644 sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_active_rule_note.html.erb delete 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/images/reviews/comment.png 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 802d8947e8f..225d92f67ed 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 @@ -985,16 +985,10 @@ 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.extend_description=Extend description +rules_configuration.update_description=Update description 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.
HTML tags can be used in this field. -rules_configuration.adding_activation_note_description=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.
HTML tags can be used in this field. -rules_configuration.create_note=Create note -rules_configuration.create_activation_note=Create activation note +rules_configuration.confirm_delete_note=Do you really want to delete this note? rules_configuration.rule_identification=Identification rules_configuration.rule_parameters=Parameters @@ -1004,9 +998,9 @@ rules_configuration.rule_parameters=Parameters # RULES DOCUMENTATION PAGE # #------------------------------------------------------------------------------ -rules.description=Description -rules.rule_activated_on_profile_x=Rule activated on profile "{0}" -rules.parameters=Parameters: +rules.more_about_rule_on_profile_x=More about this rule on profile "{0}" +rules.identification=Identification +rules.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 b8563fb4a70..91ad6b65236 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 @@ -30,7 +30,7 @@ class RulesConfigurationController < ApplicationController # 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', 'update_note', 'delete_note'], + :only => ['activate_rule', 'update_param', 'bulk_edit', 'create', 'update', 'delete', 'revert_rule', 'update_rule_note', 'update_active_rule_note', 'delete_active_rule_note'], :redirect_to => { :action => 'index' } before_filter :admin_required, :except => [ 'index', 'export' ] @@ -315,37 +315,41 @@ class RulesConfigurationController < ApplicationController end - def update_note - if params[:is_active_rule] - rule = ActiveRule.find(params[:rule_id]) - else - rule = Rule.find(params[:rule_id]) - end + def update_rule_note + rule = Rule.find(params[:rule_id]) note = rule.note unless note - if params[:is_active_rule] - note = ActiveRuleNote.new({:rule => rule}) - else - note = RuleNote.new({:rule => rule}) - end + note = RuleNote.new({:rule => rule}) # 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] } + render :partial => 'rule_note', :locals => {:rule => rule, :is_admin => true } end - - def delete_note - if params[:is_active_rule] - rule = ActiveRule.find(params[:rule_id]) - else - rule = Rule.find(params[:rule_id]) + + def update_active_rule_note + active_rule = ActiveRule.find(params[:active_rule_id]) + note = active_rule.note + unless note + note = ActiveRuleNote.new({:active_rule => active_rule}) + # set the note on the rule to avoid reloading the rule + active_rule.note = note end - rule.note.destroy if rule.note - render :text => '' + note.text = params[:text] + note.user_login = current_user.login + note.save! + render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :is_admin => true } + end + + + def delete_active_rule_note + active_rule = ActiveRule.find(params[:active_rule_id]) + active_rule.note.destroy if active_rule.note + active_rule.note = nil + render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :is_admin => true } end 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 b306bec7393..568b5461757 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,7 +20,6 @@ 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" @@ -32,5 +31,13 @@ class ActiveRuleNote < ActiveRecord::Base user_login ? User.find(:first, :conditions => ['login=?', user_login]) : nil end end + + def html_text + Api::Utils.markdown_to_html(text) + end + + def plain_text + Api::Utils.convert_string_to_unix_newlines(text) + 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 ca7c805a4f7..94791b88abf 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,7 +23,6 @@ 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 ||= @@ -31,5 +30,13 @@ class RuleNote < ActiveRecord::Base user_login ? User.find(:first, :conditions => ['login=?', user_login]) : nil end end + + def html_text + Api::Utils.markdown_to_html(text) + end + + def plain_text + Api::Utils.convert_string_to_unix_newlines(text) + end end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules/show.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules/show.html.erb index 1f9fb945606..4de01168813 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules/show.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules/show.html.erb @@ -1,26 +1,29 @@

<%= @rule.name %>

-
-Plugin: <%= @rule.plugin_name -%>    -Key: <%= @rule.plugin_rule_key %> -
-

<%= message('rules.description') -%>

-

<%= @rule.description %>

+ <% if @rule.description.strip.start_with?('

') %> + <%= @rule.description %> + <% else %> +

<%= @rule.description %>

+ <% end %> - <% if @rule.note %> -

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

+ <% if @rule.note && !@rule.note.text.strip.blank? %> +

<%= @rule.note.html_text -%>

<% end %> - <% if @profile %> -

<%= message('rules.rule_activated_on_profile_x', :params => @profile.name) -%>

+ <% if @profile && @active_rule %> +
+ + <%= message('rules.more_about_rule_on_profile_x', :params => @profile.name) %> + - <% if @active_rule %> -

<%= sanitize(@active_rule.note.text) if @active_rule.note %>

+ + <% end %> -
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_active_rule_note.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_active_rule_note.html.erb new file mode 100644 index 00000000000..f85e08f1d7b --- /dev/null +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_active_rule_note.html.erb @@ -0,0 +1,58 @@ +<% #locals = active_rule, is_admin + note = active_rule.note + active_note_detail_div_id = "and_" + active_rule.id.to_s + add_active_note_button_id = "aanb_" + active_rule.id.to_s + edit_active_note_link_id = "eanl_" + active_rule.id.to_s + delete_active_note_link_id = "danl_" + active_rule.id.to_s + active_note_form_div_id = "anf_" + active_rule.id.to_s + active_note_textarea_id = "ant_" + active_rule.id.to_s + submit_active_note_update_button_id = "sanub_" + active_rule.id.to_s +%> + +
+ <% if note %> +

+ <%= image_tag("reviews/comment.png") -%>  + <%= note.user.name -%>  + (<%= distance_of_time_in_words_to_now(note.updated_at) -%>)  +   + <%= image_tag 'sep12.png' -%> +   + <%= message('edit') %> +   + <%= link_to_remote message('delete'), + :url => {:action => 'delete_active_rule_note', :active_rule_id => active_rule.id }, + :update => "active_rule_note_#{active_rule.id}", + :confirm => message('rules_configuration.confirm_delete_note') -%> +

+

<%= note.html_text -%>

+ <% elsif is_admin %> + + <% end %> +
+ +<% if is_admin %> + +<% end %> \ No newline at end of file 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 deleted file mode 100644 index fc089848ad9..00000000000 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_create_rule_note.html.erb +++ /dev/null @@ -1,48 +0,0 @@ -<% #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 8bbe7a19236..a56225876bb 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 @@ -39,31 +39,18 @@ -

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

-

<%= rule.description %>

- <%= render :partial => 'rule_note', :locals => {:rule => rule, :is_admin => is_admin, :is_active_rule => false } %> + <%= render :partial => 'rule_note', :locals => {:rule => rule, :is_admin => is_admin } %>
<% if active_rule %>
- <%= render :partial => 'rule_note', :locals => {:rule => active_rule, :is_admin => is_admin, :is_active_rule => true } %> + <%= render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :is_admin => is_admin } %>
<% 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| @@ -87,6 +74,13 @@ <%= 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 %> +

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

+
+ + + + +
<%= message('plugin') -%>:<%= rule.plugin_name.capitalize %>
<%= message('key') -%>:<%= rule.plugin_rule_key %>
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 index 208a1fe98c4..ecda4d7b332 100644 --- 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 @@ -1,36 +1,53 @@ -<% #locals = rule, is_admin, is_active_rule - submit_note_update_button_id = (is_active_rule ? 'a' : '') + "snub_" + rule.id.to_s +<% #locals = rule, is_admin + note = rule.note + note_detail_div_id = "nd_" + rule.id.to_s + note_extend_link_id = "nel_" + rule.id.to_s + note_form_div_id = "nf_" + rule.id.to_s + note_textarea_id = "nt_" + rule.id.to_s + submit_note_update_button_id = "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 %> -
+
+ <% if rule.description.strip.start_with?('

') %> + <%= rule.description %> + <% else %> +

<%= rule.description %>

+ <% end %> + + <% if note && !note.text.strip.blank? %> +

<%= note.html_text -%>

+ <% end %> + + <% if is_admin %> +

+ » <%= message('rules_configuration.extend_description') %> +

+ <% end %> +
- -<% end %> \ No newline at end of file +<% if is_admin %> + +<% end %> diff --git a/sonar-server/src/main/webapp/images/reviews/comment.png b/sonar-server/src/main/webapp/images/reviews/comment.png new file mode 100644 index 0000000000000000000000000000000000000000..d2b4d6408511f505c93e4740621a0b092f48bbd8 GIT binary patch literal 1107 zcmV-Z1g!gsP)4Tx0C)j~RL^S@K@|QrZmG~B2wH0nvUrdpNm;9CMbtL^5n^i$+aIn^?(HA4aZWV5ov6ELTdbo0FI&wK{O>*+w4vx20?>!`FrQsdJlnHR>OPy zcd~b_n$otK2Za4V;76L-DzNVtaSB-y0*E}{p()372;bw_^6ZZ}PI-92wGS&j#91PI zKs7DSe@(bk%_Y-7gGe}(^>I=@oY#w#*Bu9GZf3^F5WP>3rn}7Ut74&?PWBFvy`A)a zPP5)V!Xd&78LdA?xQ(9mjMYElVd13a#D+Z_7&Y|xU=_C-srWU*6kiZcC!$nw*)9$7 zn6CX+@=AhmkT}X@VSsa5NKe;HZuq)~1$`#h6R+ZTR#D-3j}vF!)ZOnz+5)dI4jl{{ z44Mr{P!L4~VVJN`K!!XTF*LGrKO?IK8z<8w`3e3jI8lUGNUta*C8 zn(P`s>{pjD=7Kek#B;Fw@hxAK%$F&Q6vg9J^Xf~4by_hu-=A!MJ3Znq&n~srbFGPs zH&&aMXZ>nO`|hf|ljc?VPhR!${AbO?W8x_>CU%PFA&Hm8F7cAsOREdwU~R_;ot1_u z(ruCYB-LPGn!NQdT|ZlRy+(fw^-+`=%+gee_kY4FWHg<*4sZI8+sFJD270UUORdLHO0nA4V) z%{fwsET5CQ>B?eK%uw4yQc~9?*JVo2}ze(;aRcp*ceL#HUJSllrgm5wQKR zQu+C;QrUh^8rFfA`ftFz{YAidi-`aL010qNS#tmY3labT3lag+-G2N4009e0L_t(2 z6^&9m4udcZbt(on7B&_x!9WMp-k>)q+<=?F4ahB+P%DO3+=7i2CP3m-J0cLNrLCpp z+41vpl$g!)3}kT}`v?j7e^?NPAs0Lnh@!|(({!%u8pH2K9O6<9GU2dvOI+6l!!Uj} zCk}DZ#BHU(U&T^gZKTOV!eJQHaM#zgt!(pWLZWjP8*}IAj5CDGRC-V+u$C>D~t2z;|XP13SZec z&Ot!&Dj<_MPzk=W?|B~Nc@9m}tYpbi@lAk|C~`wM;ksj|GvcnTbD%Q%zQ1717gSSk Z{tI?8kSdDys}}$O002ovPDHLkV1gFG3@HEr literal 0 HcmV?d00001 diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css index 9d7aaa8e982..4296e0b7b14 100644 --- a/sonar-server/src/main/webapp/stylesheets/style.css +++ b/sonar-server/src/main/webapp/stylesheets/style.css @@ -2059,6 +2059,13 @@ ul.bullet li { margin-top:10px; } +.rule_detail .markdown-help { + vertical-align:top; + width: 90px; + padding-left: 10px; + white-space: nowrap; +} + .rule_detail > td > table { margin: 4px; } -- 2.39.5