From 9c4db595cbb5ab928fa7a075986d935aa0c1d449 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 25 Jul 2012 22:24:58 +0600 Subject: [PATCH 01/46] [refs #579] new line comment icon --- app/assets/images/line_comment.png | Bin 0 -> 3233 bytes app/assets/stylesheets/design/custom.scss | 18 ++++++++ lib/ext/git/inline_callback.rb | 49 +++++++++++++++++----- 3 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 app/assets/images/line_comment.png diff --git a/app/assets/images/line_comment.png b/app/assets/images/line_comment.png new file mode 100644 index 0000000000000000000000000000000000000000..544cb0714c16cbd67ee26939ee8fff9c429e8e8b GIT binary patch literal 3233 zcmV;S3|{kzP)Oz@Z0f2-7z;ux~O9+4z z06=<WDR*FRcSTFz-W=q650N5=6FiBTtNC2?60Km==3$g$R3;-}uh=nNt1bYBr z$Ri_o0EC$U6h`t_Jn<{85a%iY0C<_QJh>z}MS)ugEpZ1|S1ukX&Pf+56gFW3VVXcL z!g-k)GJ!M?;PcD?0HBc-5#WRK{dmp}uFlRjj{U%*%WZ25jX{P*?XzTzZ-GF^d31o+^>%=Ap99M6&ogks$0k4OBs3;+Bb z(;~!4V!2o<6ys46agIcqjPo+3B8fthDa9qy|77CdEc*jK-!%ZRYCZvbku9iQV*~a} zClFY4z~c7+0P?$U!PF=S1Au6Q;m>#f??3%Vpd|o+W=WE9003S@Bra6Svp>fO002aw zfhw>;8}z{#EWidF!3EsG3;bXU&9EIRU@z1_9W=mE zXoiz;4lcq~xDGvV5BgyUp1~-*fe8db$Osc*A=-!mVv1NJjtCc-h4>-CNCXm#Bp}I% z6j35eku^v$Qi@a{RY)E3J#qp$hg?Rwkvqr$GJ^buyhkyVfwECO)C{#lxu`c9ghrwZ z&}4KmnvWKso6vH!8a<3Qq36)6Xb;+tK10Vaz~~qUGsJ8#F2=(`u{bOVlVi)VBCHIn z#u~6ztOL7=^<&SmcLWlFMZgI*1b0FpVIDz9SWH+>*hr`#93(Um+6gxa1B6k+CnA%m zOSC4s5&6UzVlpv@SV$}*))J2sFA#f(L&P^E5{W}HC%KRUNwK6<(h|}}(r!{C=`5+6 zG)NjFlgZj-YqAG9lq?`C$c5yc>d>VnA`E_*3F2Qp##d8RZb=H01_m zm@+|Cqnc9PsG(F5HIG_Ct)aG3uTh7n6Et<2In9F>NlT@zqLtGcXcuVrX|L#Xx)I%# z9!{6gSJKPrN9dR61N3(c4Tcqi$B1Vr8Jidf7-t!G7_XR2rWwr)$3XQ?}=hpK0&Z&W{|ep&sA23f;Q!%st`QJ}G3cbou<7-yIK2z4nfCCCtN2-XOGSWo##{8Q{ATu zrxr~;I`ytDs%xbip}RzPziy}Qn4Z2~fSycmr`~zJ=lUFdFa1>gZThG6M+{g7vkW8# z+YHVaJjFF}Z#*3@$J_ByLtVo_L#1JrVVB{Ak-5=4qt!-@Mh}c>#$4kh<88)m#-k<% zCLtzEP3leVno>={htGUuD;o7bD)w_sX$S}eAxwzy?U zvgBH(S?;#HZiQMoS*2K2T3xe7t(~nU*1N5{rxB;QPLocnp4Ml>u<^FZwyC!nu;thW z+pe~4wtZn|Vi#w(#jeBdlf9FDx_yoPJqHbk*$%56S{;6Kv~mM9!g3B(KJ}#RZ#@)!hR|78Dq z|Iq-afF%KE1Brn_fm;Im_u$xr8UFki1L{Ox>G0o)(&RAZ;=|I=wN2l97;cLaHH6 zleTB-XXa*h%dBOEvi`+xi?=Txl?TadvyiL>SuF~-LZ;|cS}4~l2eM~nS7yJ>iOM;a ztDY;(?aZ^v+mJV$@1Ote62cPUlD4IWOIIx&SmwQ~YB{nzae3Pc;}r!fhE@iwJh+Os zDs9zItL;~pu715HdQEGAUct(O!LkCy1<%NCg+}G`0PgpNm-?d@-hMgNe6^V+j z6x$b<6@S<$+<4_1hi}TincS4LsjI}fWY1>OX6feMEuLErma3QLmkw?X+1j)X-&VBk z_4Y;EFPF_I+q;9dL%E~BJh;4Nr^(LEJ3myURP{Rblsw%57T)g973 zR8o)DE9*xN#~;4_o$q%o4K@u`jhx2fBXC4{U8Qn{*%*B z$Ge=nny$HAYq{=vy|sI0_vss+H_qMky?OB#|JK!>IX&II^LlUh#rO5!7TtbwC;iUL zyV-Xq?ybB}ykGP{?LpZ?-G|jbTmIbG@7#ZCz;~eY(cDM(28Dyq{*m>M4?_iynUBkc z4TkHUI6gT!;y-fz>HMcd&t%Ugo)`Y2{>!cx7B7DI)$7;J(U{Spm-3gBzioV_{p!H$ z8L!*M!p0uH$#^p{Ui4P`?ZJ24cOCDe-w#jZd?0@)|7iKK^;6KN`;!@ylm7$*nDhK& zGcDTy000JJOGiWi000000Qp0^e*gdg32;bRa{vGf6951U69E94oEQKA00(qQO+^RX z2N@6(7svzE)Bpeh!%0LzR4C7lkv(fvK@^7HGkZTan~&XWqKH|vGZIun8eIe{tB5uh zwn0&_6Cr8DKVXqoMT8WVAz;u5ibX86F^CaCF<^{|vT--p%}r*#cW35U%;x4pL-wg= znDfk>^A0gHHkhfX$%WdpQnVb_V*>yLw0L45?+x~(&mT&6d1NceYG2m$$js`Kd8r}| zK_GxOE@QwjKnFYB;o}`QdNU1e_O}{*QC?b!;Sne){|C*`xO$@f?#Y}86<>T6ECn%q zvBTk^!Zs~MaOuJO)zQyVO#O)FN`K%AGy?%Cy8kJ3%2L7bZtvIyjV*4T2~?zVaxD(T z&UtJd4;8iv7K42mM_lOfkM8$j^WGB%G+*y+BWC^(sB_PMeXrKx3hW*v3`6m?V;%R3 zxyHZEg?i*|_3<}pVgo@)0!sR4&ikob{n?v+t()I%c~h>Bf2~a~M`fv{X2Yn|RKu*B z%YE60r?a-w+1f?1+bwpx09GQFiZ$cP>)_@5Iy_ ... ... @@ -17,37 +18,53 @@ module Git end def addline(line) + set_line_number " #{line.new_number} -
#{render_line(line)}
+ + #{line_comment} +
#{render_line(line)}
+ " end - + def remline(line) + set_line_number " #{line.old_number} -
#{render_line(line)}
+ + #{line_comment} +
#{render_line(line)}
+ " end def modline(line) + set_line_number " #{line.old_number} #{line.new_number} -
#{render_line(line)}
+ + #{line_comment} +
#{render_line(line)}
+ " end - + def unmodline(line) + set_line_number " #{line.old_number} #{line.new_number} -
#{render_line(line)}
+ + #{line_comment} +
#{render_line(line)}
+ " end - + def sepline(line) " … @@ -55,12 +72,16 @@ module Git " end - + def nonewlineline(line) + set_line_number " #{line.old_number} #{line.new_number} -
#{render_line(line)}
+ + #{line_comment} +
#{render_line(line)}
+ " end @@ -78,9 +99,17 @@ module Git res += escape(line) end res += '' - + res end + + def set_line_number + @num_line = (@num_line || -1).succ + end + + def line_comment + "Add Comment" + end end end end From 527dd210bf17631a5ee27f5f4890863729149d7f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 00:40:12 +0600 Subject: [PATCH 02/46] [refs #579] alpha version of inline comments for commit --- app/assets/javascripts/extra/comment.js | 32 ++- app/assets/javascripts/extra/preview.js | 2 +- app/assets/stylesheets/design/custom.scss | 32 ++- .../projects/comments_controller.rb | 5 + .../projects/git/commits_controller.rb | 2 + .../projects/projects_controller.rb | 4 +- app/helpers/diff_helper.rb | 193 +++++++++++++++++- app/models/ability.rb | 2 +- app/models/comment.rb | 3 +- .../projects/comments/_line_list.html.haml | 11 + .../projects/comments/new_line.html.haml | 10 + .../git/commits/_commit_diff.html.haml | 3 +- app/views/projects/git/commits/show.html.haml | 6 +- config/routes.rb | 1 + lib/ext/git/inline_callback.rb | 83 +++++++- 15 files changed, 356 insertions(+), 33 deletions(-) create mode 100644 app/views/projects/comments/_line_list.html.haml create mode 100644 app/views/projects/comments/new_line.html.haml diff --git a/app/assets/javascripts/extra/comment.js b/app/assets/javascripts/extra/comment.js index 5bcf3aa20..7be01e4ef 100644 --- a/app/assets/javascripts/extra/comment.js +++ b/app/assets/javascripts/extra/comment.js @@ -1,17 +1,19 @@ $(document).ready(function() { - $('.buttons a.edit_comment').live('click', function() { + var new_comment = $('#open-comment.comment.hidden.new_line_comment'); + + $(document).on('click', '.buttons a.edit_comment', function() { $(this).parent().parent().parent().hide(); $('#open-comment'+'.comment.'+$(this).attr('id')).show(); return false; }); - $('.cancel_edit_comment.button').live('click', function() { + $(document).on('click', '.cancel_edit_comment.button', function() { $(this).parent().parent().parent().hide(); $('.buttons a.edit_comment#'+$(this).attr('id')).parent().parent().parent().show(); return false; }); - $('form.edit_comment').live('submit', function() { + $(document).on('submit', 'form.edit_comment', function() { var form = $(this); form.parent().find('.flash').remove(); $.ajax({ @@ -30,5 +32,29 @@ $(document).ready(function() { return false; }); + $('.add_line-comment').on('click', function() { + function ProcessData(data) { + var str = ""+""+data+""; + par.after(str); + line.addClass('new_comment_exists'); + par.parent().find('#md_tabs.nav.nav-tabs').each(function(i) { $(this).find('a:first').tab('show') }); + } + var line = $(this); + var par = line.parent().parent(); + if(line.hasClass('new_comment_exists')) { + $('#open-comment.new_line_comment').parent().parent().show(); + } + else { + $('#open-comment.new_line_comment').parent().parent().remove(); + $.get(line.attr('href'), null, ProcessData); + } + $('#new_line_edit_input').focus(); + return false; + }); + $(document).on('click', '.cancel_inline_comment.button', function() { + $(this).parent().parent().parent().parent().parent().hide(); + return false; + }); }); + diff --git a/app/assets/javascripts/extra/preview.js b/app/assets/javascripts/extra/preview.js index 5f9002ea0..4eb397844 100644 --- a/app/assets/javascripts/extra/preview.js +++ b/app/assets/javascripts/extra/preview.js @@ -2,7 +2,7 @@ $(document).ready(function() { var preview_url = $('#preview_url').val(); $('#md_tabs.nav.nav-tabs').each(function(i) { $(this).find('a:first').tab('show') }); - $('#md_tabs a[data-toggle="tab"]').on('shown', function (e) { + $(document).on('shown','#md_tabs a[data-toggle="tab"]', function (e) { if(e.relatedTarget) { var hash = e.relatedTarget.hash; } else { var hash = e.currentTarget.hash; } var el = $(hash+'_input'); diff --git a/app/assets/stylesheets/design/custom.scss b/app/assets/stylesheets/design/custom.scss index c5580c890..92e5cf64c 100644 --- a/app/assets/stylesheets/design/custom.scss +++ b/app/assets/stylesheets/design/custom.scss @@ -38,14 +38,6 @@ div.description-top div.name input { height: 100%; } -article div.activity { - border: 1px solid #D6D6D6; - border-radius: 5px 5px 5px 5px; - color: #333333; - margin-top: 15px; - padding: 6px; -} - article div.messages div.activity { margin-top: 0; margin-bottom: 10px; @@ -1600,3 +1592,27 @@ table.tablesorter.platform-maintainers.static-search thead tr.search th input[ty -moz-opacity:1; } +div.file .inline-comments { + margin: 5px; + + .top { + height:auto; + background: none; + box-shadow: none; + } + + .activity { + border-radius: 0; + max-width: 764px; + } +} + +td.line_comments { + border: solid #DDD; + border-width: 1px 0; +} + +#open-comment.comment.new_line_comment { + max-width: 750px; + margin-left: 10px; +} diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index 003ed166e..ccfdc85be 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -9,6 +9,7 @@ class Projects::CommentsController < Projects::BaseController include CommentsHelper def create + @comment.data = {:path => params[:path], :line => params[:line]} if params[:path].present? && params[:line].present? if @comment.save flash[:notice] = I18n.t("flash.comment.saved") redirect_to project_commentable_path(@project, @commentable) @@ -37,6 +38,10 @@ class Projects::CommentsController < Projects::BaseController redirect_to project_commentable_path(@project, @commentable) end + def new_line + render :layout => false + end + protected def find_commentable diff --git a/app/controllers/projects/git/commits_controller.rb b/app/controllers/projects/git/commits_controller.rb index 31816996c..0cd232c9a 100644 --- a/app/controllers/projects/git/commits_controller.rb +++ b/app/controllers/projects/git/commits_controller.rb @@ -10,6 +10,8 @@ class Projects::Git::CommitsController < Projects::Git::BaseController def show @commit = @project.repo.commit(params[:id]) + @comments = Comment.for_commit(@commit) + respond_to do |format| format.html format.diff { render :text => (@commit.diffs.map(&:diff).join("\n") rescue ''), :content_type => "text/plain" } diff --git a/app/controllers/projects/projects_controller.rb b/app/controllers/projects/projects_controller.rb index 0b8842670..3c323c041 100644 --- a/app/controllers/projects/projects_controller.rb +++ b/app/controllers/projects/projects_controller.rb @@ -104,7 +104,7 @@ class Projects::ProjectsController < Projects::BaseController end def preview - render :inline => view_context.markdown(params[:text]), :layout => false + render :inline => view_context.markdown(params[:text] || ''), :layout => false end protected @@ -122,7 +122,7 @@ class Projects::ProjectsController < Projects::BaseController if groups.present? || owners.present? projects = projects.by_owners(groups, owners) end - + projects = projects.search(params[:sSearch]).search_order if params[:sSearch].present? res[:filtered_count] = projects.count diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 5cd707e1a..b3010ff09 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,18 +1,201 @@ # -*- encoding : utf-8 -*- module DiffHelper - + #include Git::Diff::InlineCallback def render_diff(diff) diff_display ||= Diff::Display::Unified.new(diff.diff) - - #res = "" + prepare(diff) res = "" res += "" - res += diff_display.render(Git::Diff::InlineCallback.new) + res += renderer diff_display.data #diff_display.render(Git::Diff::InlineCallback.new comments) res += "" res += "
" - res.html_safe end + ######################################################## + # FIXME: Just to dev, remove to lib + ######################################################## + def prepare(diff) + @num_line, @path = -1, diff.a_path + @line_comments = @comments.select{|c| c.data.try('[]', :path) == @path} + end + + def headerline(line) + set_line_number + " + ... + ... + #{line} + " + end + + def addline(line) + set_line_number + " + + #{line.new_number} + + #{line_comment} +
#{render_line(line)}
+ + + #{render_line_comments}" + end + + def remline(line) + set_line_number + " + #{line.old_number} + + + #{line_comment} +
#{render_line(line)}
+ + + #{render_line_comments}" + end + + def modline(line) + set_line_number + " + #{line.old_number} + #{line.new_number} + + #{line_comment} +
#{render_line(line)}
+ + + #{render_line_comments}" + end + + def unmodline(line) + set_line_number + " + #{line.old_number} + #{line.new_number} + + #{line_comment} +
#{render_line(line)}
+ + + #{render_line_comments}" + end + + def sepline(line) + " + … + … + + " + end + + def nonewlineline(line) + set_line_number + " + #{line.old_number} + #{line.new_number} + + #{line_comment} +
#{render_line(line)}
+ + + #{render_line_comments}" + end + + def before_headerblock(block) + end + + def after_headerblock(block) + end + + def before_unmodblock(block) + end + + def before_modblock(block) + end + + def before_remblock(block) + end + + def before_addblock(block) + end + + def before_sepblock(block) + end + + def before_nonewlineblock(block) + end + + def after_unmodblock(block) + end + + def after_modblock(block) + end + + def after_remblock(block) + end + + def after_addblock(block) + end + + def after_sepblock(block) + end + + def after_nonewlineblock(block) + end + + def new_line + "" + end + + def renderer(data) + result = [] + data.each do |block| + result << send("before_" + classify(block), block) + result << block.map { |line| send(classify(line), line) } + result << send("after_" + classify(block), block) + end + result.compact.join(new_line) + end + + protected + + def classify(object) + object.class.name[/\w+$/].downcase + end + + def escape(str) + str.to_s.gsub('&', '&').gsub('<', '<').gsub('>', '>').gsub('"', '"') + end + + def render_line(line) + res = '' + if line.inline_changes? + prefix, changed, postfix = line.segments.map{|segment| escape(segment) } + res += "#{prefix}#{changed}#{postfix}" + else + res += escape(line) + end + res += '' + + res + end + + def set_line_number + @num_line = @num_line.succ + end + + def line_comment + #"Add Comment" + link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @path, :line => @num_line), :class => 'add_line-comment' + end + + def render_line_comments + comments = @line_comments.select{|c| c.data.try('[]', :line) == @num_line.to_s} + " + #{comments.count} + #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commit)} + " if comments.count > 0 + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 50fd62693..1be8b86f4 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -120,7 +120,7 @@ class Ability can([:update, :destroy], Issue) {|issue| issue.user_id == user.id or local_admin?(issue.project)} cannot :manage, Issue, :project => {:has_issues => false} # switch off issues - can(:create, Comment) {|comment| can? :read, comment.project} + can([:create, :new_line], Comment) {|comment| can? :read, comment.project} can(:update, Comment) {|comment| comment.user == user or comment.project.owner == user or local_admin?(comment.project)} cannot :manage, Comment, :commentable_type => 'Issue', :commentable => {:project => {:has_issues => false}} # switch off issues end diff --git a/app/models/comment.rb b/app/models/comment.rb index 6960860cc..39956b622 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -3,6 +3,7 @@ class Comment < ActiveRecord::Base belongs_to :commentable, :polymorphic => true belongs_to :user belongs_to :project + serialize :data validates :body, :user_id, :commentable_id, :commentable_type, :project_id, :presence => true @@ -12,7 +13,7 @@ class Comment < ActiveRecord::Base after_create :subscribe_on_reply, :unless => lambda {|c| c.commit_comment?} after_create :subscribe_users - attr_accessible :body + attr_accessible :body, :data def commentable # raise commentable_id.inspect diff --git a/app/views/projects/comments/_line_list.html.haml b/app/views/projects/comments/_line_list.html.haml new file mode 100644 index 000000000..9b3113d58 --- /dev/null +++ b/app/views/projects/comments/_line_list.html.haml @@ -0,0 +1,11 @@ +- list.each do |comment| + .inline-comments + - CommentPresenter.present(comment, :project => project, :commentable => commentable) do |presenter| + = render 'shared/feed_message', :presenter => presenter + #open-comment.comment.hidden{:class => "comment-#{comment.id}"} + %h3.tmargin0= t("layout.comments.edit_header") + = form_for comment, :url => project_commentable_comment_path(project, commentable, comment), :html => { :class => 'form edit_comment' } do |f| + = render "projects/comments/form", :f => f, :id => "edit_#{comment.id}" + .comment-left + =link_to t('layout.cancel'), '#', :id => "comment-#{comment.id}", :class => 'cancel_edit_comment button' + .both diff --git a/app/views/projects/comments/new_line.html.haml b/app/views/projects/comments/new_line.html.haml new file mode 100644 index 000000000..2c295b479 --- /dev/null +++ b/app/views/projects/comments/new_line.html.haml @@ -0,0 +1,10 @@ +#open-comment.comment.view.new_line_comment + =link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' + %h3.tmargin0= t("layout.comments.new_header") + = form_for :comment, :url => project_commit_comments_path(@project, @commentable), :method => :post, :html => { :class => :form } do |f| + = render "projects/comments/form", :f => f, :id => 'new_line' + .comment-left + =link_to t('layout.cancel'), '#', :class => 'cancel_inline_comment button' + =hidden_field_tag :path, params[:path] + =hidden_field_tag :line, params[:line] + .both diff --git a/app/views/projects/git/commits/_commit_diff.html.haml b/app/views/projects/git/commits/_commit_diff.html.haml index 46f639af9..8932d8b61 100644 --- a/app/views/projects/git/commits/_commit_diff.html.haml +++ b/app/views/projects/git/commits/_commit_diff.html.haml @@ -6,5 +6,6 @@ - if commit_diff.b_path.present? .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, commit_diff.b_path) .clear + -comments = @comments.select{|c| c.data.try('[]', :path) == commit_diff.a_path} + .diff_data= render_diff(commit_diff) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? - .diff_data= render_diff(commit_diff) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? \ No newline at end of file diff --git a/app/views/projects/git/commits/show.html.haml b/app/views/projects/git/commits/show.html.haml index 5cc736823..7da6e3afb 100644 --- a/app/views/projects/git/commits/show.html.haml +++ b/app/views/projects/git/commits/show.html.haml @@ -2,17 +2,15 @@ = render 'submenu' = render 'about_block', :project => @project - %h3= t("layout.projects.last_commit") - GitPresenters::CommitAsMessagePresenter.present(@commit, :branch => @branch, :project => @project) do |presenter| = render :partial => 'shared/feed_message', :locals => {:presenter => presenter, :item_no => 1} .both - #repo-wrapper = render 'show' - - = render "projects/comments/list", :list => Comment.for_commit(@commit), :project => @project, :commentable => @commit + -comments = @comments.select {|c| c.data.blank? } # dont work @comments.where(:data => nil) + = render "projects/comments/list", :list => comments, :project => @project, :commentable => @commit = render "projects/comments/add", :project => @project, :commentable => @commit if current_user =hidden_field_tag :preview_url, project_md_preview_path(@project) diff --git a/config/routes.rb b/config/routes.rb index a41f66da9..bc0c7575e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -210,6 +210,7 @@ Rosa::Application.routes.draw do get '/commit/:commit_id/comments/:id(.:format)' => "comments#edit", :as => :edit_project_commit_comment put '/commit/:commit_id/comments/:id(.:format)' => "comments#update", :as => :project_commit_comment delete '/commit/:commit_id/comments/:id(.:format)' => "comments#destroy" + get '/commit/:commit_id/add_line_comments(.:format)' => "comments#new_line", :as => :new_line_commit_comment # Commit subscribes post '/commit/:commit_id/subscribe' => "commit_subscribes#create", :as => :subscribe_commit delete '/commit/:commit_id/unsubscribe' => "commit_subscribes#destroy", :as => :unsubscribe_commit diff --git a/lib/ext/git/inline_callback.rb b/lib/ext/git/inline_callback.rb index 706d4b7c8..0692ca19a 100644 --- a/lib/ext/git/inline_callback.rb +++ b/lib/ext/git/inline_callback.rb @@ -1,11 +1,9 @@ # -*- encoding : utf-8 -*- module Git module Diff - class InlineCallback < ::Diff::Renderer::Base - def before_headerblock(block) - end - - def after_headerblock(block) + module InlineCallback + def prepare(comments) + @num_line, @line_comments = -1, comments end def headerline(line) @@ -26,7 +24,8 @@ module Git #{line_comment}
#{render_line(line)}
- " + + #{render_line_comments @num_line}" end def remline(line) @@ -85,7 +84,68 @@ module Git " end + def before_headerblock(block) + end + + def after_headerblock(block) + end + + def before_unmodblock(block) + end + + def before_modblock(block) + end + + def before_remblock(block) + end + + def before_addblock(block) + end + + def before_sepblock(block) + end + + def before_nonewlineblock(block) + end + + def after_unmodblock(block) + end + + def after_modblock(block) + end + + def after_remblock(block) + end + + def after_addblock(block) + end + + def after_sepblock(block) + end + + def after_nonewlineblock(block) + end + + def new_line + "" + end + + def renderer(data) + result = [] + data.each do |block| + result << send("before_" + classify(block), block) + result << block.map { |line| send(classify(line), line) } + result << send("after_" + classify(block), block) + end + result.compact.join(new_line) + end + protected + + def classify(object) + object.class.name[/\w+$/].downcase + end + def escape(str) str.to_s.gsub('&', '&').gsub('<', '<').gsub('>', '>').gsub('"', '"') end @@ -104,12 +164,21 @@ module Git end def set_line_number - @num_line = (@num_line || -1).succ + @num_line = @num_line.succ end def line_comment "Add Comment" end + + def render_line_comments line_number + comments = @line_comments.select{|c| c.data.try('[]', :line_number) == line_number} + + " + + #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commit)} + " if comments.count + end end end end From 21a5115137e1c0cde057d26dacbbe2b8781da9a8 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 12:13:14 +0600 Subject: [PATCH 03/46] [refs #579] fixed destroing comments --- app/models/ability.rb | 2 +- app/presenters/comment_presenter.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 1be8b86f4..c37361e65 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -121,7 +121,7 @@ class Ability cannot :manage, Issue, :project => {:has_issues => false} # switch off issues can([:create, :new_line], Comment) {|comment| can? :read, comment.project} - can(:update, Comment) {|comment| comment.user == user or comment.project.owner == user or local_admin?(comment.project)} + can([:update, :destroy], Comment) {|comment| comment.user == user or comment.project.owner == user or local_admin?(comment.project)} cannot :manage, Comment, :commentable_type => 'Issue', :commentable => {:project => {:has_issues => false}} # switch off issues end diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index 687316cc5..3461090a7 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -43,7 +43,7 @@ class CommentPresenter < ApplicationPresenter res << link_to(t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal') res << link_to(t("layout.edit"), ep, :id => "comment-#{comment.id}", :class => "edit_comment").html_safe end - if controller.can? :delete, @comment + if controller.can? :destroy, @comment res << link_to(t("layout.delete"), dp, :method => "delete", :confirm => t("layout.comments.confirm_delete")).html_safe end From a24ea9a4b1f91a7acc5c57acd7bc62e4e4b22908 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 14:16:07 +0600 Subject: [PATCH 04/46] [refs #579] fixed & refactoring markdown help --- app/presenters/comment_presenter.rb | 1 - app/views/projects/comments/_add.html.haml | 1 + app/views/projects/comments/_button_md_help.html.haml | 0 app/views/projects/comments/_list.html.haml | 1 + app/views/projects/comments/new_line.html.haml | 2 +- app/views/projects/issues/_title_body.html.haml | 2 +- 6 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 app/views/projects/comments/_button_md_help.html.haml diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index 3461090a7..18603abe6 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -40,7 +40,6 @@ class CommentPresenter < ApplicationPresenter res = [] if controller.can? :update, @comment - res << link_to(t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal') res << link_to(t("layout.edit"), ep, :id => "comment-#{comment.id}", :class => "edit_comment").html_safe end if controller.can? :destroy, @comment diff --git a/app/views/projects/comments/_add.html.haml b/app/views/projects/comments/_add.html.haml index e0ab3eb65..513a8a666 100644 --- a/app/views/projects/comments/_add.html.haml +++ b/app/views/projects/comments/_add.html.haml @@ -1,4 +1,5 @@ #open-comment.comment.view + =render 'projects/comments/button_md_help' =link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' %h3.tmargin0= t("layout.comments.new_header") - if Comment.issue_comment?(commentable.class) diff --git a/app/views/projects/comments/_button_md_help.html.haml b/app/views/projects/comments/_button_md_help.html.haml new file mode 100644 index 000000000..e69de29bb diff --git a/app/views/projects/comments/_list.html.haml b/app/views/projects/comments/_list.html.haml index aa27c0955..bc92a2cb5 100644 --- a/app/views/projects/comments/_list.html.haml +++ b/app/views/projects/comments/_list.html.haml @@ -5,6 +5,7 @@ - CommentPresenter.present(comment, :project => project, :commentable => commentable) do |presenter| = render 'shared/feed_message', :presenter => presenter #open-comment.comment.hidden{:class => "comment-#{comment.id}"} + =render 'projects/comments/button_md_help' %h3.tmargin0= t("layout.comments.edit_header") = form_for comment, :url => project_commentable_comment_path(project, commentable, comment), :html => { :class => 'form edit_comment' } do |f| = render "projects/comments/form", :f => f, :id => "edit_#{comment.id}" diff --git a/app/views/projects/comments/new_line.html.haml b/app/views/projects/comments/new_line.html.haml index 2c295b479..331315c69 100644 --- a/app/views/projects/comments/new_line.html.haml +++ b/app/views/projects/comments/new_line.html.haml @@ -1,5 +1,5 @@ #open-comment.comment.view.new_line_comment - =link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' + =render 'projects/comments/button_md_help' %h3.tmargin0= t("layout.comments.new_header") = form_for :comment, :url => project_commit_comments_path(@project, @commentable), :method => :post, :html => { :class => :form } do |f| = render "projects/comments/form", :f => f, :id => 'new_line' diff --git a/app/views/projects/issues/_title_body.html.haml b/app/views/projects/issues/_title_body.html.haml index 806f9c16a..7f35fe7d6 100644 --- a/app/views/projects/issues/_title_body.html.haml +++ b/app/views/projects/issues/_title_body.html.haml @@ -2,7 +2,7 @@ %h3.tmargin0= t 'activerecord.attributes.issue.title' .wrapper= f.text_area :title, :cols => 80, :rows => 1 #open-comment.comment.view - =link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' + =render 'projects/comments/button_md_help' %h3.tmargin0= t 'activerecord.attributes.issue.body' =render 'projects/comments/body', :f => f, :id => id .both From 1bc76a7f18d182e6a4a5d8a42e33fa374df57ef3 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 15:34:55 +0600 Subject: [PATCH 05/46] [refs #579] fresh schema --- db/schema.rb | 112 +++++++++++++++++++++++++-------------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 97372c6ea..0d2a29289 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -17,8 +17,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.integer "user_id", :null => false t.string "kind" t.text "data" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "advisories", :force => true do |t| @@ -53,8 +53,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "arches", :force => true do |t| t.string "name", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "arches", ["name"], :name => "index_arches_on_name", :unique => true @@ -63,8 +63,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.integer "user_id" t.string "provider" t.string "uid" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "authentications", ["provider", "uid"], :name => "index_authentications_on_provider_and_uid", :unique => true @@ -75,8 +75,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.integer "level" t.integer "status" t.integer "build_list_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "version" end @@ -110,8 +110,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.integer "project_id" t.integer "arch_id" t.datetime "notified_at" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "is_circle", :default => false t.text "additional_repos" t.string "name" @@ -142,8 +142,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "commentable_type" t.integer "user_id" t.text "body" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.decimal "commentable_id", :precision => 50, :scale => 0 t.integer "project_id" end @@ -160,8 +160,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "controller" t.string "action" t.text "message" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "flash_notifies", :force => true do |t| @@ -175,8 +175,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "groups", :force => true do |t| t.integer "owner_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "uname" t.integer "own_projects_count", :default => 0, :null => false t.text "description" @@ -193,8 +193,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "title" t.text "body" t.string "status", :default => "open" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "user_id" t.datetime "closed_at" t.integer "closed_by" @@ -254,14 +254,14 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "description" t.string "name", :null => false t.integer "parent_platform_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "released", :default => false, :null => false t.integer "owner_id" t.string "owner_type" t.string "visibility", :default => "open", :null => false t.string "platform_type", :default => "main", :null => false - t.string "distrib_type", :null => false + t.string "distrib_type" end add_index "platforms", ["name"], :name => "index_platforms_on_name", :unique => true, :case_sensitive => false @@ -270,16 +270,16 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.integer "platform_id" t.string "login" t.string "password" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "user_id" end create_table "product_build_lists", :force => true do |t| t.integer "product_id" t.integer "status", :default => 2, :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end add_index "product_build_lists", ["product_id"], :name => "index_product_build_lists_on_product_id" @@ -287,8 +287,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "products", :force => true do |t| t.string "name", :null => false t.integer "platform_id", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "build_script" t.text "counter" t.text "ks" @@ -307,8 +307,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "name" t.string "version" t.datetime "file_mtime" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "platform_id" end @@ -317,25 +317,25 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "project_to_repositories", :force => true do |t| t.integer "project_id" t.integer "repository_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "projects", :force => true do |t| t.string "name" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "owner_id" t.string "owner_type" t.string "visibility", :default => "open" t.text "description" t.string "ancestry" t.boolean "has_issues", :default => true + t.boolean "has_wiki", :default => false t.string "srpm_file_name" + t.string "srpm_content_type" t.integer "srpm_file_size" t.datetime "srpm_updated_at" - t.string "srpm_content_type" - t.boolean "has_wiki", :default => false t.string "default_branch", :default => "master" t.boolean "is_package", :default => true, :null => false t.integer "average_build_time", :default => 0, :null => false @@ -363,8 +363,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "token" t.boolean "approved", :default => false t.boolean "rejected", :default => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "interest" t.text "more" t.string "language" @@ -378,16 +378,16 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "actor_type" t.integer "target_id" t.string "target_type" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "role" end create_table "repositories", :force => true do |t| t.string "description", :null => false t.integer "platform_id", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "name", :null => false t.boolean "publish_without_qa", :default => true end @@ -399,8 +399,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.boolean "new_comment_reply", :default => true t.boolean "new_issue", :default => true t.boolean "issue_assign", :default => true - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "new_comment_commit_owner", :default => true t.boolean "new_comment_commit_repo_owner", :default => true t.boolean "new_comment_commit_commentor", :default => true @@ -411,8 +411,8 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "subscribes", :force => true do |t| t.string "subscribeable_type" t.integer "user_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "status", :default => true t.integer "project_id" t.decimal "subscribeable_id", :precision => 50, :scale => 0 @@ -420,18 +420,21 @@ ActiveRecord::Schema.define(:version => 20121003154246) do create_table "users", :force => true do |t| t.string "name" - t.string "email", :default => "", :null => false - t.string "encrypted_password", :limit => 128, :default => "", :null => false + t.string "email", :default => "", :null => false + t.string "encrypted_password", :default => "", :null => false t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "ssh_key" t.string "uname" t.string "role" - t.string "language", :default => "en" - t.integer "own_projects_count", :default => 0, :null => false + t.string "language", :default => "en" + t.integer "own_projects_count", :default => 0, :null => false + t.string "confirmation_token" + t.datetime "confirmed_at" + t.datetime "confirmation_sent_at" t.text "professional_experience" t.string "site" t.string "company" @@ -440,14 +443,11 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.string "avatar_content_type" t.integer "avatar_file_size" t.datetime "avatar_updated_at" - t.integer "failed_attempts", :default => 0 + t.integer "failed_attempts", :default => 0 t.string "unlock_token" t.datetime "locked_at" - t.string "confirmation_token" - t.datetime "confirmed_at" - t.datetime "confirmation_sent_at" t.string "authentication_token" - t.integer "build_priority", :default => 50 + t.integer "build_priority", :default => 50 end add_index "users", ["authentication_token"], :name => "index_users_on_authentication_token" From afa6dd07060b828a0954c9755653ede137e191a6 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 15:39:07 +0600 Subject: [PATCH 06/46] [refs #579] fixed style --- app/assets/stylesheets/design/custom.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/design/custom.scss b/app/assets/stylesheets/design/custom.scss index 5f0a0ed99..828a45654 100644 --- a/app/assets/stylesheets/design/custom.scss +++ b/app/assets/stylesheets/design/custom.scss @@ -1652,6 +1652,7 @@ td.line_comments { #open-comment.comment.new_line_comment { max-width: 750px; margin-left: 10px; +} #repo-wrapper .edit_form.issue div.comment div.wrapper { background: 0; From 2360a25a424edf4cad790bd9ad6a8d57a29bd03d Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 17:02:25 +0600 Subject: [PATCH 07/46] [refs #579] fixed many errors --- app/helpers/diff_helper.rb | 12 ++++++------ app/models/ability.rb | 4 +--- app/views/projects/comments/_add.html.haml | 1 - .../projects/comments/_button_md_help.html.haml | 2 ++ app/views/projects/comments/_line_list.html.haml | 10 +--------- db/migrate/20121005100158_add_data_to_comments.rb | 5 +++++ db/schema.rb | 3 ++- lib/ext/git/inline_callback.rb | 6 +++--- 8 files changed, 20 insertions(+), 23 deletions(-) create mode 100644 db/migrate/20121005100158_add_data_to_comments.rb diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 9b86af520..db8c9228f 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -23,12 +23,12 @@ module DiffHelper #include Git::Diff::InlineCallback def render_diff(diff, diff_counter) diff_display ||= Diff::Display::Unified.new(diff.diff) - path = if @pull + url = if @pull @pull.id ? polymorphic_path([@project, @pull]) : '' elsif @commit commit_path @project, @commit end - prepare(diff, path) + prepare(diff, url, diff_counter) res = "" res += "" @@ -41,9 +41,9 @@ module DiffHelper ######################################################## # FIXME: Just to dev, remove to lib ######################################################## - def prepare(diff, comments, url, diff_counter) - @num_line, @url, @diff_counter = -1, url, diff_counter - @line_comments = @comments.select{|c| c.data.try('[]', :path) == @path} + def prepare(diff, url, diff_counter) + @num_line, @filepath, @url, @diff_counter = -1, diff.a_path, url, diff_counter + @line_comments = @comments.select{|c| c.data.try('[]', :path) == @filepath} end def headerline(line) @@ -212,7 +212,7 @@ module DiffHelper end def line_comment - link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @diff.a_path, :line => @num_line), :class => 'add_line-comment' + link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @filepath, :line => @num_line), :class => 'add_line-comment' end def render_line_comments diff --git a/app/models/ability.rb b/app/models/ability.rb index da812b061..57e4999b5 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -132,9 +132,7 @@ class Ability can([:create, :new_line], Comment) {|comment| can? :read, comment.project} can([:update, :destroy], Comment) {|comment| comment.user == user or comment.project.owner == user or local_admin?(comment.project)} - cannot :manage, Comment do |c| - c.commentable_type == 'Issue' && !c.project.has_issues && !c.commentable.pull_request # when switch off issues - end + cannot :manage, Comment, :commentable_type => 'Issue', :project => {:has_issues => false} end # Shared cannot rights for all users (registered, admin) diff --git a/app/views/projects/comments/_add.html.haml b/app/views/projects/comments/_add.html.haml index 513a8a666..132f81541 100644 --- a/app/views/projects/comments/_add.html.haml +++ b/app/views/projects/comments/_add.html.haml @@ -1,6 +1,5 @@ #open-comment.comment.view =render 'projects/comments/button_md_help' - =link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' %h3.tmargin0= t("layout.comments.new_header") - if Comment.issue_comment?(commentable.class) - new_path = project_issue_comments_path(project, commentable) diff --git a/app/views/projects/comments/_button_md_help.html.haml b/app/views/projects/comments/_button_md_help.html.haml index e69de29bb..3cc040988 100644 --- a/app/views/projects/comments/_button_md_help.html.haml +++ b/app/views/projects/comments/_button_md_help.html.haml @@ -0,0 +1,2 @@ +=link_to t('layout.comments.md_cheatsheet_header'), '#md_help', 'data-toggle' => 'modal', :style => 'float:right;' + diff --git a/app/views/projects/comments/_line_list.html.haml b/app/views/projects/comments/_line_list.html.haml index 9b3113d58..929c219ca 100644 --- a/app/views/projects/comments/_line_list.html.haml +++ b/app/views/projects/comments/_line_list.html.haml @@ -1,11 +1,3 @@ - list.each do |comment| .inline-comments - - CommentPresenter.present(comment, :project => project, :commentable => commentable) do |presenter| - = render 'shared/feed_message', :presenter => presenter - #open-comment.comment.hidden{:class => "comment-#{comment.id}"} - %h3.tmargin0= t("layout.comments.edit_header") - = form_for comment, :url => project_commentable_comment_path(project, commentable, comment), :html => { :class => 'form edit_comment' } do |f| - = render "projects/comments/form", :f => f, :id => "edit_#{comment.id}" - .comment-left - =link_to t('layout.cancel'), '#', :id => "comment-#{comment.id}", :class => 'cancel_edit_comment button' - .both + = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable diff --git a/db/migrate/20121005100158_add_data_to_comments.rb b/db/migrate/20121005100158_add_data_to_comments.rb new file mode 100644 index 000000000..c33ed6a8b --- /dev/null +++ b/db/migrate/20121005100158_add_data_to_comments.rb @@ -0,0 +1,5 @@ +class AddDataToComments < ActiveRecord::Migration + def change + add_column :comments, :data, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d2a29289..bd9ee3cdb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20121003154246) do +ActiveRecord::Schema.define(:version => 20121005100158) do create_table "activity_feeds", :force => true do |t| t.integer "user_id", :null => false @@ -146,6 +146,7 @@ ActiveRecord::Schema.define(:version => 20121003154246) do t.datetime "updated_at", :null => false t.decimal "commentable_id", :precision => 50, :scale => 0 t.integer "project_id" + t.text "data" end create_table "event_logs", :force => true do |t| diff --git a/lib/ext/git/inline_callback.rb b/lib/ext/git/inline_callback.rb index 896672b6d..c95b5913a 100644 --- a/lib/ext/git/inline_callback.rb +++ b/lib/ext/git/inline_callback.rb @@ -2,8 +2,8 @@ module Git module Diff class InlineCallback < ::Diff::Renderer::Base - def initialize diff, comments, url, diff_counter - @num_line, @url, @diff_counter = -1, url, diff_counter + def initialize diff, url, diff_counter + @num_line, @filepath, @url, @diff_counter = -1, diff.a_path, url, diff_counter @line_comments = @comments.select{|c| c.data.try('[]', :path) == @path} end @@ -173,7 +173,7 @@ module Git end def line_comment - link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @diff.a_path, :line => @num_line), :class => 'add_line-comment' + link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @filepath, :line => @num_line), :class => 'add_line-comment' end def render_line_comments From 80a91b21a9f7f2268b1f6556f733916d5dbbd63c Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 5 Oct 2012 18:32:56 +0600 Subject: [PATCH 08/46] [refs #579] fixed bug with new_line access --- app/controllers/projects/comments_controller.rb | 2 +- app/models/ability.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index ccfdc85be..b65904986 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -4,7 +4,7 @@ class Projects::CommentsController < Projects::BaseController load_and_authorize_resource :project before_filter :find_commentable before_filter :find_or_build_comment - load_and_authorize_resource #:through => :commentable + load_and_authorize_resource :new => :new_line include CommentsHelper diff --git a/app/models/ability.rb b/app/models/ability.rb index 57e4999b5..da812b061 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -132,7 +132,9 @@ class Ability can([:create, :new_line], Comment) {|comment| can? :read, comment.project} can([:update, :destroy], Comment) {|comment| comment.user == user or comment.project.owner == user or local_admin?(comment.project)} - cannot :manage, Comment, :commentable_type => 'Issue', :project => {:has_issues => false} + cannot :manage, Comment do |c| + c.commentable_type == 'Issue' && !c.project.has_issues && !c.commentable.pull_request # when switch off issues + end end # Shared cannot rights for all users (registered, admin) From ed7aec602ffaf175144ea09ce2d837f839cee894 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 6 Oct 2012 01:19:05 +0600 Subject: [PATCH 09/46] [refs #579] alpha version of the pull inline comments --- .../projects/comments_controller.rb | 36 +++++++++++++++++-- .../projects/git/commits_controller.rb | 2 +- .../projects/pull_requests_controller.rb | 4 ++- app/helpers/diff_helper.rb | 23 +++++++++--- app/models/activity_feed_observer.rb | 2 +- app/models/pull_request.rb | 2 +- .../projects/comments/_comment.html.haml | 2 +- .../projects/comments/_line_list.html.haml | 2 +- app/views/projects/comments/_list.html.haml | 2 +- .../projects/comments/new_line.html.haml | 2 +- .../pull_requests/_activity.html.haml | 7 ++-- .../projects/pull_requests/show.html.haml | 2 +- config/routes.rb | 1 + 13 files changed, 67 insertions(+), 20 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index b65904986..a257855e5 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -9,15 +9,14 @@ class Projects::CommentsController < Projects::BaseController include CommentsHelper def create - @comment.data = {:path => params[:path], :line => params[:line]} if params[:path].present? && params[:line].present? + res = set_additional_data if @comment.save flash[:notice] = I18n.t("flash.comment.saved") - redirect_to project_commentable_path(@project, @commentable) else flash[:error] = I18n.t("flash.comment.save_error") flash[:warning] = @comment.errors.full_messages.join('. ') - redirect_to project_commentable_path(@project, @commentable) end + redirect_to project_commentable_path(@project, @commentable) end def edit @@ -39,6 +38,11 @@ class Projects::CommentsController < Projects::BaseController end def new_line + @path = if @commentable.class == Issue + project_issue_comments_path(@project, @commentable) + elsif @commentable.class == Grit::Commit + project_commit_comments_path(@project, @commentable) + end render :layout => false end @@ -53,4 +57,30 @@ class Projects::CommentsController < Projects::BaseController @comment = params[:id].present? && Comment.find(params[:id]) || current_user.comments.build(params[:comment]) {|c| c.commentable = @commentable; c.project = @project} end + + def set_additional_data + return true unless params[:path].present? && params[:line].present? + @comment.data = {:path => params[:path], :line => params[:line]} + if @commentable.class == Issue && pull = @commentable.pull_request + repo = Grit::Repo.new(pull.path) + base_commit = pull.common_ancestor + head_commit = repo.commits(pull.head_branch).first + diff = pull.diff repo, base_commit, head_commit + return false unless diff_path = diff.select {|d| d.a_path == params[:path]} + comment_line = params[:line].to_i + return false if comment_line == 0 # NB! also dont want create comment to the diff header + + line_number, line_presence = -1, false + diff_path[0].diff.each_line do |line| + line_number = line_number.succ + # Save 2 lines above and bottom of the diff comment line + break if line_number > comment_line + 2 + if (comment_line-2..comment_line+2).include? line_number + @comment.data["line#{line_number-comment_line}"] = line.chomp + line_presence = true + end + end + return line_presence + end + end end diff --git a/app/controllers/projects/git/commits_controller.rb b/app/controllers/projects/git/commits_controller.rb index 0cd232c9a..513807a24 100644 --- a/app/controllers/projects/git/commits_controller.rb +++ b/app/controllers/projects/git/commits_controller.rb @@ -9,7 +9,7 @@ class Projects::Git::CommitsController < Projects::Git::BaseController end def show - @commit = @project.repo.commit(params[:id]) + @commit = @commentable = @project.repo.commit(params[:id]) @comments = Comment.for_commit(@commit) respond_to do |format| diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index 17e4f16ef..f634b5013 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -47,7 +47,7 @@ class Projects::PullRequestsController < Projects::BaseController flash.now[:error] = I18n.t('projects.pull_requests.up_to_date', :to_ref => @pull.to_ref, :from_ref => @pull.from_ref) render :new else - @pull.send(@pull.status) + @pull.send(@pull.status == 'blocked' ? 'block' : @pull.status) redirect_to project_pull_request_path(@pull.to_project, @pull) end else @@ -138,6 +138,8 @@ class Projects::PullRequestsController < Projects::BaseController @diff = @pull.diff repo, @base_commit, @head_commit @stats = @pull.diff_stats repo, @base_commit, @head_commit + @comments = @issue.comments + @commentable = @issue end def find_destination_project bang=true diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index db8c9228f..59d775b1e 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -42,7 +42,7 @@ module DiffHelper # FIXME: Just to dev, remove to lib ######################################################## def prepare(diff, url, diff_counter) - @num_line, @filepath, @url, @diff_counter = -1, diff.a_path, url, diff_counter + @diff, @num_line, @filepath, @url, @diff_counter = diff, -1, diff.a_path, url, diff_counter @line_comments = @comments.select{|c| c.data.try('[]', :path) == @filepath} end @@ -212,14 +212,29 @@ module DiffHelper end def line_comment - link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @filepath, :line => @num_line), :class => 'add_line-comment' + path = if @commentable.class == Issue + project_new_line_pull_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + elsif @commentable.class == Grit::Commit + new_line_commit_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + end + link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), path, :class => 'add_line-comment' end def render_line_comments - comments = @line_comments.select{|c| c.data.try('[]', :line) == @num_line.to_s} + comments = @line_comments.select do |c| + next false if c.data.try('[]', :line) != @num_line.to_s + next true if c.commentable_type == 'Grit::Commit' + #diff = Diff::Display::Unified.new(@diff.diff) + res, ind = true, 0 + @diff.diff.each_line do |line| + res = false if (@num_line-2..@num_line+2).include?(ind) && c.data.try('[]', "line#{ind-@num_line}") != line.chomp + ind = ind + 1 + end + res + end " - + " if comments.count > 0 end diff --git a/app/models/activity_feed_observer.rb b/app/models/activity_feed_observer.rb index 25ae5046f..715450e03 100644 --- a/app/models/activity_feed_observer.rb +++ b/app/models/activity_feed_observer.rb @@ -71,7 +71,7 @@ class ActivityFeedObserver < ActiveRecord::Observer when 'GitHook' return unless record.project - record.project.pull_requests.needed_checking.each {|pull| pull.check} + PullRequest.where("from_project_id = ? OR to_project_id = ?", record.project, record.project).needed_checking.each {|pull| pull.check} change_type = record.change_type branch_name = record.refname.split('/').last diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 2cdef2dfe..ffd0346a5 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -175,7 +175,7 @@ class PullRequest < ActiveRecord::Base system 'git', 'remote', 'add', 'head', from_project.path end end - clean + clean # Need testing end Dir.chdir(path) do diff --git a/app/views/projects/comments/_comment.html.haml b/app/views/projects/comments/_comment.html.haml index 33fddf41c..41b1ba286 100644 --- a/app/views/projects/comments/_comment.html.haml +++ b/app/views/projects/comments/_comment.html.haml @@ -4,7 +4,7 @@ =render 'projects/comments/button_md_help' %h3.tmargin0= t("layout.comments.edit_header") = form_for comment, :url => project_commentable_comment_path(project, commentable, comment), :html => { :class => 'form edit_comment' } do |f| - = render "projects/comments/form", :f => f, :id => "edit_#{comment.id}" + = render "projects/comments/form", :f => f, :id => "#{add_id.presence}edit_#{comment.id}" .comment-left =link_to t('layout.cancel'), '#', :id => "comment-#{comment.id}", :class => 'cancel_edit_comment button' .both diff --git a/app/views/projects/comments/_line_list.html.haml b/app/views/projects/comments/_line_list.html.haml index 929c219ca..03a9aa4fd 100644 --- a/app/views/projects/comments/_line_list.html.haml +++ b/app/views/projects/comments/_line_list.html.haml @@ -1,3 +1,3 @@ - list.each do |comment| .inline-comments - = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable + = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable, :add_id => 'inline' diff --git a/app/views/projects/comments/_list.html.haml b/app/views/projects/comments/_list.html.haml index 868533dab..6b2e447d5 100644 --- a/app/views/projects/comments/_list.html.haml +++ b/app/views/projects/comments/_list.html.haml @@ -2,5 +2,5 @@ .hr %h3#block-list= t("layout.comments.comments_header") - list.each do |comment| - = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable + = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable, :add_id => nil = render "projects/comments/markdown_help" diff --git a/app/views/projects/comments/new_line.html.haml b/app/views/projects/comments/new_line.html.haml index 331315c69..64b69a4b2 100644 --- a/app/views/projects/comments/new_line.html.haml +++ b/app/views/projects/comments/new_line.html.haml @@ -1,7 +1,7 @@ #open-comment.comment.view.new_line_comment =render 'projects/comments/button_md_help' %h3.tmargin0= t("layout.comments.new_header") - = form_for :comment, :url => project_commit_comments_path(@project, @commentable), :method => :post, :html => { :class => :form } do |f| + = form_for :comment, :url => @path, :method => :post, :html => { :class => :form } do |f| = render "projects/comments/form", :f => f, :id => 'new_line' .comment-left =link_to t('layout.cancel'), '#', :class => 'cancel_inline_comment button' diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index 3d70ef7c1..870edb92d 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -3,21 +3,20 @@ %h3#block-list= t("layout.comments.comments_header") - commits_queue = [] -- merge_activity(@issue.comments, @commits).each do |item| # +- merge_activity(@comments, @commits).each do |item| # - if item.is_a?(Comment) - if commits_queue.present? = render 'projects/git/commits/commits_small', :commits => commits_queue - commits_queue = [] - = render 'projects/comments/comment', :comment => item, :project => @project, :commentable => @issue + = render 'projects/comments/comment', :comment => item, :project => @project, :commentable => @issue, :add_id => nil - else - commits_queue << item - comments_for_commit = Comment.for_commit(item) - if comments_for_commit.present? = render 'projects/git/commits/commits_small', :commits => commits_queue - comments_for_commit.each do |comment| - = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => item + = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => item, :add_id => nil - commits_queue = [] - if commits_queue.present? = render 'projects/git/commits/commits_small', :commits => commits_queue -= render "projects/comments/markdown_help" diff --git a/app/views/projects/pull_requests/show.html.haml b/app/views/projects/pull_requests/show.html.haml index 07c8997f8..0e525d1cd 100644 --- a/app/views/projects/pull_requests/show.html.haml +++ b/app/views/projects/pull_requests/show.html.haml @@ -16,4 +16,4 @@ =render 'status' =render 'diff_commits_tabs' unless @pull.already? =hidden_field_tag :preview_url, project_md_preview_path(@project) - += render "projects/comments/markdown_help" diff --git a/config/routes.rb b/config/routes.rb index d50f5825f..5dca2bd09 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -212,6 +212,7 @@ Rosa::Application.routes.draw do end post '/preview' => 'projects#preview', :as => 'md_preview' post 'refs_list' => 'projects#refs_list', :as => 'refs_list' + get '/pull_requests/:issue_id/add_line_comments(.:format)' => "comments#new_line", :as => :new_line_pull_comment end # Resource From f12634cee88c73178e9269e9831b566ecc49ac53 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 15:14:31 +0600 Subject: [PATCH 10/46] [refs #579] add link to comments --- app/presenters/comment_presenter.rb | 27 ++++++++++--------- .../_new_comment_commit_notification.haml | 2 +- .../partials/_new_comment_notification.haml | 2 +- .../projects/comments/_comment.html.haml | 2 +- app/views/shared/_feed_message.html.haml | 2 +- config/locales/en.yml | 1 + config/locales/ru.yml | 1 + 7 files changed, 20 insertions(+), 17 deletions(-) diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index 18603abe6..e5da30342 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -27,23 +27,17 @@ class CommentPresenter < ApplicationPresenter def caption? false end - def buttons - project = options[:project] - commentable = options[:commentable] - (ep, dp) = if Comment.issue_comment?(commentable.class) - [edit_project_issue_comment_path(project, commentable, comment), - project_issue_comment_path(project, commentable, comment)] - elsif Comment.commit_comment?(commentable.class) - [edit_project_commit_comment_path(project, commentable, comment), - project_commit_comment_path(project, commentable, comment)] - end - res = [] + def buttons + project, commentable = options[:project], options[:commentable] + path = helpers.project_commentable_comment_path(project, commentable, comment) + + res = [link_to(t("layout.link"), "#{helpers.project_commentable_path(project, commentable)}##{comment_anchor}", :class => "link_to_comment").html_safe] if controller.can? :update, @comment - res << link_to(t("layout.edit"), ep, :id => "comment-#{comment.id}", :class => "edit_comment").html_safe + res << link_to(t("layout.edit"), path, :id => "comment-#{comment.id}", :class => "edit_comment").html_safe end if controller.can? :destroy, @comment - res << link_to(t("layout.delete"), dp, :method => "delete", + res << link_to(t("layout.delete"), path, :method => "delete", :confirm => t("layout.comments.confirm_delete")).html_safe end res @@ -69,4 +63,11 @@ class CommentPresenter < ApplicationPresenter def comment_id @comment.id end + + def comment_anchor + # check for pull diff inline comment + before = (@options[:add_anchor].present? && @comment.commentable_type == 'Issue') ? 'diff-' : '' + "#{before}comment#{@comment.id}" + end + end diff --git a/app/views/activity_feeds/partials/_new_comment_commit_notification.haml b/app/views/activity_feeds/partials/_new_comment_commit_notification.haml index 003ffd448..2f276d098 100644 --- a/app/views/activity_feeds/partials/_new_comment_commit_notification.haml +++ b/app/views/activity_feeds/partials/_new_comment_commit_notification.haml @@ -4,7 +4,7 @@ .text %span = raw t("notifications.bodies.new_comment_notification.title", :user_link => link_to(user_name, user_path(user_id)) ) - = raw t("notifications.bodies.new_comment_notification.commit_content", {:commit_link => link_to(commit_message, commit_path(project_owner, project_name, commit_id) + "#comment##{comment_id}")}) + = raw t("notifications.bodies.new_comment_notification.commit_content", {:commit_link => link_to(commit_message, commit_path(project_owner, project_name, commit_id) + "#comment#{comment_id}")}) = raw t("notifications.bodies.project", :project_link => link_to("#{project_owner}/#{project_name}", project_path(project_owner, project_name)) ) .both %span.date= activity_feed.created_at diff --git a/app/views/activity_feeds/partials/_new_comment_notification.haml b/app/views/activity_feeds/partials/_new_comment_notification.haml index 567aae9a3..bc626889d 100644 --- a/app/views/activity_feeds/partials/_new_comment_notification.haml +++ b/app/views/activity_feeds/partials/_new_comment_notification.haml @@ -4,7 +4,7 @@ .text %span = raw t("notifications.bodies.new_comment_notification.title", {:user_link => link_to(user_name, user_path(user_id))}) - = raw t("notifications.bodies.new_comment_notification.content", {:issue_link => link_to(issue_title, project_issue_path(project_owner, project_name, issue_serial_id) + "#comment##{comment_id}")}) + = raw t("notifications.bodies.new_comment_notification.content", {:issue_link => link_to(issue_title, project_issue_path(project_owner, project_name, issue_serial_id) + "#comment#{comment_id}")}) = raw t("notifications.bodies.project", :project_link => link_to("#{project_owner}/#{project_name}", project_path(project_owner, project_name)) ) .both %span.date= activity_feed.created_at diff --git a/app/views/projects/comments/_comment.html.haml b/app/views/projects/comments/_comment.html.haml index 41b1ba286..6a478dbb7 100644 --- a/app/views/projects/comments/_comment.html.haml +++ b/app/views/projects/comments/_comment.html.haml @@ -1,4 +1,4 @@ -- CommentPresenter.present(comment, :project => project, :commentable => commentable) do |presenter| +- CommentPresenter.present(comment, :project => project, :commentable => commentable, :add_anchor => add_id.presence) do |presenter| = render 'shared/feed_message', :presenter => presenter #open-comment.comment.hidden{:class => "comment-#{comment.id}"} =render 'projects/comments/button_md_help' diff --git a/app/views/shared/_feed_message.html.haml b/app/views/shared/_feed_message.html.haml index 0e7e8e420..414b4cc9b 100644 --- a/app/views/shared/_feed_message.html.haml +++ b/app/views/shared/_feed_message.html.haml @@ -1,4 +1,4 @@ -.activity{:id => presenter.comment_id? ? "comment##{presenter.comment_id}" : ''} +.activity{:id => presenter.comment_id? ? presenter.comment_anchor : ''} .top - if presenter.buttons? %span.buttons= raw presenter.buttons.join(' | ').html_safe diff --git a/config/locales/en.yml b/config/locales/en.yml index 77afe09e7..3513e0485 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -48,6 +48,7 @@ en: invalid_content_type: incorrect type atom_link_tag_title: Private feed for %{nickname} | %{app_name} preview: Preview + link: Link settings: label: Settings diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 0f9e45378..3a0f829f7 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -48,6 +48,7 @@ ru: invalid_content_type: имеет неверный тип atom_link_tag_title: Приватная лента для %{nickname} | %{app_name} preview: Предосмотр + link: Ссылка settings: label: 'Настройки' From 824ee79452f3a382d5fb88d3dfbb046103899582 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 15:20:45 +0600 Subject: [PATCH 11/46] [refs #579] redirect to created comment --- app/controllers/projects/comments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index a257855e5..be8fb5645 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -16,7 +16,7 @@ class Projects::CommentsController < Projects::BaseController flash[:error] = I18n.t("flash.comment.save_error") flash[:warning] = @comment.errors.full_messages.join('. ') end - redirect_to project_commentable_path(@project, @commentable) + redirect_to project_commentable_path(@project, @commentable) + "#comment#{@comment.id}" end def edit From 548f6985268a65453a5496c820e818e35e479326 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 20:42:38 +0600 Subject: [PATCH 12/46] [refs #579] add a line comment button --- app/assets/javascripts/extra/comment.js | 69 ++++++++++++++++--- app/assets/stylesheets/design/custom.scss | 17 +++-- app/helpers/diff_helper.rb | 25 ++++--- .../projects/comments/_line_list.html.haml | 2 +- config/locales/models/comment.en.yml | 1 + config/locales/models/comment.ru.yml | 1 + 6 files changed, 90 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/extra/comment.js b/app/assets/javascripts/extra/comment.js index 7be01e4ef..549e78240 100644 --- a/app/assets/javascripts/extra/comment.js +++ b/app/assets/javascripts/extra/comment.js @@ -32,20 +32,66 @@ $(document).ready(function() { return false; }); + $('.new_inline_comment.button').on('click', function() { + var tmp = $(this).parents('tr').prev('tr') + tmp = tmp.find("a[href='"+$(this).attr('href')+"']"); + var tmp = $(this).parents('tr').prev('tr').find("a[href='"+$(this).attr('href')+"']"); + $(this).parents('tr').prev('tr').find("a[href='"+$(this).attr('href')+"']").click(); + return false; + }); + $('.add_line-comment').on('click', function() { function ProcessData(data) { - var str = ""+""; - par.after(str); - line.addClass('new_comment_exists'); + if(yield) { + var str = ""+""; + par.after(str); + } + else { + par.find('td:last').append(data); + } + $("a[href='"+line.attr('href')+"']").addClass('new_comment_exists'); par.parent().find('#md_tabs.nav.nav-tabs').each(function(i) { $(this).find('a:first').tab('show') }); } var line = $(this); - var par = line.parent().parent(); - if(line.hasClass('new_comment_exists')) { - $('#open-comment.new_line_comment').parent().parent().show(); + var tmp = line.parents('tr'); + var yield = false; + var par = null; + + if(tmp.hasClass('inline-comments')) { + par = tmp; + } + else { + par = tmp.next('tr.inline-comments'); + } + + if(par.length == 0) { + par = tmp; + yield = true; + } + + // Hide visible new comment form + $('#open-comment.new_line_comment').parents('.inline-comments').each(function(i) { + if($(this).find('.line-comments').length > 0) { + $(this).find('#open-comment.new_line_comment').hide(); + $(this).find('.new_inline_comment.button').show(); + } + else { + $(this).hide(); + } + }); + par.find('.new_inline_comment.button').hide(); + + // Show existing new comment form or load them. + if(line.hasClass('new_comment_exists')) { + var tr = line.parents('tr').next(); + if(tr.find('.line-comments').length > 0) { + tr.find('#open-comment.new_line_comment').show(); + } + else { + tr.show(); + } } else { - $('#open-comment.new_line_comment').parent().parent().remove(); $.get(line.attr('href'), null, ProcessData); } $('#new_line_edit_input').focus(); @@ -53,7 +99,14 @@ $(document).ready(function() { }); $(document).on('click', '.cancel_inline_comment.button', function() { - $(this).parent().parent().parent().parent().parent().hide(); + var tr = $(this).parents('.inline-comments'); + if(tr.find('.line-comments').length > 0) { + tr.find('#open-comment.new_line_comment').hide(); + tr.find('.new_inline_comment.button').show(); + } + else { + tr.hide(); + } return false; }); }); diff --git a/app/assets/stylesheets/design/custom.scss b/app/assets/stylesheets/design/custom.scss index 828a45654..c357af743 100644 --- a/app/assets/stylesheets/design/custom.scss +++ b/app/assets/stylesheets/design/custom.scss @@ -1630,8 +1630,6 @@ table.tablesorter.platform-maintainers.static-search thead tr.search th input[ty } div.file .inline-comments { - margin: 5px; - .top { height:auto; background: none; @@ -1640,18 +1638,19 @@ div.file .inline-comments { .activity { border-radius: 0; - max-width: 764px; + max-width: 752px; + margin-left: 10px; } } -td.line_comments { +tr.inline-comments td { border: solid #DDD; border-width: 1px 0; } -#open-comment.comment.new_line_comment { - max-width: 750px; - margin-left: 10px; +.inline-comments #open-comment { + max-width: 754px; + margin: 10px; } #repo-wrapper .edit_form.issue div.comment div.wrapper { @@ -1695,3 +1694,7 @@ form#new_pull_request { margin: 0; padding: 0; } + +.new_inline_comment.button { + margin: 10px 0 10px 10px; +} diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 59d775b1e..8fd7d4f36 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -212,12 +212,7 @@ module DiffHelper end def line_comment - path = if @commentable.class == Issue - project_new_line_pull_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) - elsif @commentable.class == Grit::Commit - new_line_commit_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) - end - link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), path, :class => 'add_line-comment' + link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_comment_path, :class => 'add_line-comment' end def render_line_comments @@ -232,13 +227,25 @@ module DiffHelper end res end - " - - + " + + + " if comments.count > 0 end def td_line_link id, num "" end + + def new_comment_path + if @commentable.class == Issue + project_new_line_pull_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + elsif @commentable.class == Grit::Commit + new_line_commit_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + end + end end diff --git a/app/views/projects/comments/_line_list.html.haml b/app/views/projects/comments/_line_list.html.haml index 03a9aa4fd..4a5e070a5 100644 --- a/app/views/projects/comments/_line_list.html.haml +++ b/app/views/projects/comments/_line_list.html.haml @@ -1,3 +1,3 @@ - list.each do |comment| - .inline-comments + .line-comments = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable, :add_id => 'inline' diff --git a/config/locales/models/comment.en.yml b/config/locales/models/comment.en.yml index 845864a64..8f3e5a4a6 100644 --- a/config/locales/models/comment.en.yml +++ b/config/locales/models/comment.en.yml @@ -3,6 +3,7 @@ en: comments: confirm_delete: Are you sure you want to delete the comment? new_header: New comment + new_inline: Add a line comment edit_header: Editing a comment has_commented: added a note notifications_are: Notifications for new comments are diff --git a/config/locales/models/comment.ru.yml b/config/locales/models/comment.ru.yml index 89d87f2b4..f481d904c 100644 --- a/config/locales/models/comment.ru.yml +++ b/config/locales/models/comment.ru.yml @@ -3,6 +3,7 @@ ru: comments: confirm_delete: Вы уверены, что хотите удалить комментарий? new_header: Новый комментарий + new_inline: Добавить комментарий к строке edit_header: Редактирование комментария has_commented: оставил комментарий notifications_are: Уведомления о последующих комментариях From 694037af7e6716b7254096162dd566a81ee79315 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 20:47:03 +0600 Subject: [PATCH 13/46] [refs #579] remove unused lib --- lib/ext/git/inline_callback.rb | 192 --------------------------------- 1 file changed, 192 deletions(-) delete mode 100644 lib/ext/git/inline_callback.rb diff --git a/lib/ext/git/inline_callback.rb b/lib/ext/git/inline_callback.rb deleted file mode 100644 index c95b5913a..000000000 --- a/lib/ext/git/inline_callback.rb +++ /dev/null @@ -1,192 +0,0 @@ -# -*- encoding : utf-8 -*- -module Git - module Diff - class InlineCallback < ::Diff::Renderer::Base - def initialize diff, url, diff_counter - @num_line, @filepath, @url, @diff_counter = -1, diff.a_path, url, diff_counter - @line_comments = @comments.select{|c| c.data.try('[]', :path) == @path} - end - - def headerline(line) - set_line_number - " - - - - " - end - - def addline(line) - set_line_number - " - - #{td_line_link "diff-F#{@diff_counter}R#{line.new_number}", line.new_number} - - - #{render_line_comments}" - end - - def remline(line) - set_line_number - " - #{td_line_link "diff-F#{@diff_counter}L#{line.old_number}", line.old_number} - - - - #{render_line_comments}" - end - - def modline(line) - set_line_number - " - #{td_line_link "diff-F#{@diff_counter}L#{line.old_number}", line.old_number} - #{td_line_link "diff-F#{@diff_counter}R#{line.new_number}", line.new_number} - - - #{render_line_comments}" - end - - def unmodline(line) - set_line_number - " - #{td_line_link "diff-F#{@diff_counter}L#{line.old_number}", line.old_number} - #{td_line_link "diff-F#{@diff_counter}R#{line.new_number}", line.new_number} - - - #{render_line_comments}" - end - - def sepline(line) - " - - - - " - end - - def nonewlineline(line) - set_line_number - " - #{td_line_link "diff-F#{@diff_counter}L#{line.old_number}", line.old_number} - #{td_line_link "diff-F#{@diff_counter}R#{line.new_number}", line.new_number} - - - #{render_line_comments}" - end - - def before_headerblock(block) - end - - def after_headerblock(block) - end - - def before_unmodblock(block) - end - - def before_modblock(block) - end - - def before_remblock(block) - end - - def before_addblock(block) - end - - def before_sepblock(block) - end - - def before_nonewlineblock(block) - end - - def after_unmodblock(block) - end - - def after_modblock(block) - end - - def after_remblock(block) - end - - def after_addblock(block) - end - - def after_sepblock(block) - end - - def after_nonewlineblock(block) - end - - def new_line - "" - end - - def renderer(data) - result = [] - data.each do |block| - result << send("before_" + classify(block), block) - result << block.map { |line| send(classify(line), line) } - result << send("after_" + classify(block), block) - end - result.compact.join(new_line) - end - - protected - - def classify(object) - object.class.name[/\w+$/].downcase - end - - def escape(str) - str.to_s.gsub('&', '&').gsub('<', '<').gsub('>', '>').gsub('"', '"') - end - - def render_line(line) - res = '' - if line.inline_changes? - prefix, changed, postfix = line.segments.map{|segment| escape(segment) } - res += "#{prefix}#{changed}#{postfix}" - else - res += escape(line) - end - res += '' - - res - end - - def set_line_number - @num_line = @num_line.succ - end - - def line_comment - link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_line_commit_comment_path(@project, @commit, :path => @filepath, :line => @num_line), :class => 'add_line-comment' - end - - def render_line_comments - comments = @line_comments.select{|c| c.data.try('[]', :line) == @num_line.to_s} - " - - - " if comments.count > 0 - end - - def td_line_link id, num - "" - end - end - end -end From 3241b7a55ac9193aa313927cf1e67ee6b5f1eb83 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 20:54:36 +0600 Subject: [PATCH 14/46] [refs #579] save additional info for pull inline comments --- app/controllers/projects/comments_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index be8fb5645..c0a2a0416 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -63,8 +63,9 @@ class Projects::CommentsController < Projects::BaseController @comment.data = {:path => params[:path], :line => params[:line]} if @commentable.class == Issue && pull = @commentable.pull_request repo = Grit::Repo.new(pull.path) - base_commit = pull.common_ancestor - head_commit = repo.commits(pull.head_branch).first + base_commit, head_commit = pull.common_ancestor, repo.commits(pull.head_branch).first + @comment.data[:base_commit], @comment.data[:head_commit] = base_commit, head_commit + diff = pull.diff repo, base_commit, head_commit return false unless diff_path = diff.select {|d| d.a_path == params[:path]} comment_line = params[:line].to_i From 21b7b0d0b1b3aa189e795ac2d7308f0d9c99ef70 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 21:31:30 +0600 Subject: [PATCH 15/46] [refs #579] dont create wrong comment --- app/controllers/projects/comments_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index c0a2a0416..eeb7947d3 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -9,8 +9,9 @@ class Projects::CommentsController < Projects::BaseController include CommentsHelper def create - res = set_additional_data - if @comment.save + if !set_additional_data + flash[:error] = I18n.t("flash.comment.save_error") + elsif @comment.save flash[:notice] = I18n.t("flash.comment.saved") else flash[:error] = I18n.t("flash.comment.save_error") From 70e8a1fe0d825a7dd140fa1468c5a9997667e9e2 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 21:33:43 +0600 Subject: [PATCH 16/46] [refs #579] unless -> if --- app/controllers/projects/comments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index eeb7947d3..f9197aacf 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -60,7 +60,7 @@ class Projects::CommentsController < Projects::BaseController end def set_additional_data - return true unless params[:path].present? && params[:line].present? + return true if params[:path].blank? && params[:line].blank? # not inline comment @comment.data = {:path => params[:path], :line => params[:line]} if @commentable.class == Issue && pull = @commentable.pull_request repo = Grit::Repo.new(pull.path) From fcfdf1b26be659663505fbae0bce0db285b0b124 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 21:36:23 +0600 Subject: [PATCH 17/46] [refs #579] change 'base' & 'head' -> 'to' & 'from' --- app/controllers/projects/comments_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index f9197aacf..832e8f7f0 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -64,10 +64,10 @@ class Projects::CommentsController < Projects::BaseController @comment.data = {:path => params[:path], :line => params[:line]} if @commentable.class == Issue && pull = @commentable.pull_request repo = Grit::Repo.new(pull.path) - base_commit, head_commit = pull.common_ancestor, repo.commits(pull.head_branch).first - @comment.data[:base_commit], @comment.data[:head_commit] = base_commit, head_commit + to_commit, from_commit = pull.common_ancestor, repo.commits(pull.head_branch).first + @comment.data[:to_commit], @comment.data[:from_commit] = to_commit, from_commit - diff = pull.diff repo, base_commit, head_commit + diff = pull.diff repo, to_commit, from_commit return false unless diff_path = diff.select {|d| d.a_path == params[:path]} comment_line = params[:line].to_i return false if comment_line == 0 # NB! also dont want create comment to the diff header From a5477926bc1514994b2c642fadd263634c7f9c71 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 8 Oct 2012 21:36:46 +0600 Subject: [PATCH 18/46] [refs #579] fix checking path --- app/controllers/projects/comments_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index 832e8f7f0..61f464118 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -68,7 +68,8 @@ class Projects::CommentsController < Projects::BaseController @comment.data[:to_commit], @comment.data[:from_commit] = to_commit, from_commit diff = pull.diff repo, to_commit, from_commit - return false unless diff_path = diff.select {|d| d.a_path == params[:path]} + diff_path = diff.select {|d| d.a_path == params[:path]} + return false if diff_path.blank? comment_line = params[:line].to_i return false if comment_line == 0 # NB! also dont want create comment to the diff header From e7cd5f008aa8ea11cc14cfcbf02bb57aff882a18 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 9 Oct 2012 20:15:10 +0600 Subject: [PATCH 19/46] Revert "[refs #579] save additional info for pull inline comments" This reverts commit 0de497c30f15720ae3b2f31c19e7ed2fdc390ad2. Conflicts: app/controllers/projects/comments_controller.rb --- app/controllers/projects/comments_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index 61f464118..cdf4755f5 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -65,7 +65,6 @@ class Projects::CommentsController < Projects::BaseController if @commentable.class == Issue && pull = @commentable.pull_request repo = Grit::Repo.new(pull.path) to_commit, from_commit = pull.common_ancestor, repo.commits(pull.head_branch).first - @comment.data[:to_commit], @comment.data[:from_commit] = to_commit, from_commit diff = pull.diff repo, to_commit, from_commit diff_path = diff.select {|d| d.a_path == params[:path]} From 993454fc291cb96dc7e2f8817a09356ae4a44f38 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 11 Oct 2012 23:50:40 +0600 Subject: [PATCH 20/46] [refs #579] alpha version of the pull discussion --- .../projects/comments_controller.rb | 21 +++++--- .../projects/pull_requests_controller.rb | 10 ++-- app/helpers/diff_helper.rb | 50 +++++++++++-------- app/helpers/pull_request_helper.rb | 4 +- app/models/comment.rb | 30 +++++++++++ app/models/pull_request.rb | 1 + .../git/commits/_commit_diff.html.haml | 2 +- .../pull_requests/_activity.html.haml | 19 ++----- .../_discussion_comments.html.haml | 13 +++++ .../pull_requests/_pull_diff.html.haml | 2 +- app/views/projects/wiki/_diff_data.html.haml | 2 +- 11 files changed, 102 insertions(+), 52 deletions(-) create mode 100644 app/views/projects/pull_requests/_discussion_comments.html.haml diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index cdf4755f5..94b780dba 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -68,21 +68,30 @@ class Projects::CommentsController < Projects::BaseController diff = pull.diff repo, to_commit, from_commit diff_path = diff.select {|d| d.a_path == params[:path]} - return false if diff_path.blank? - comment_line = params[:line].to_i - return false if comment_line == 0 # NB! also dont want create comment to the diff header + return false unless @comment.actual_inline_comment?(diff, true) - line_number, line_presence = -1, false + comment_line, line_number, strings = params[:line].to_i, -1, [] diff_path[0].diff.each_line do |line| line_number = line_number.succ # Save 2 lines above and bottom of the diff comment line break if line_number > comment_line + 2 if (comment_line-2..comment_line+2).include? line_number @comment.data["line#{line_number-comment_line}"] = line.chomp - line_presence = true + end + + # Save lines from the closest header for rendering in the discussion + if line_number < comment_line - 2 + # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)" + if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/ + strings = line + else + strings << line + end end end - return line_presence + @comment.data[:strings] = strings + @comment.data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) + return true end end end diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index f634b5013..58512f8e5 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -128,16 +128,16 @@ class Projects::PullRequestsController < Projects::BaseController end def load_diff_commits_data - repo = Grit::Repo.new(@pull.path) + @repo = Grit::Repo.new(@pull.path) @base_commit = @pull.common_ancestor - @head_commit = repo.commits(@pull.head_branch).first + @head_commit = @repo.commits(@pull.head_branch).first - @commits = repo.commits_between(repo.commits(@pull.to_ref).first, @head_commit) + @commits = @repo.commits_between(@repo.commits(@pull.to_ref).first, @head_commit) @total_commits = @commits.count @commits = @commits.last(100) - @diff = @pull.diff repo, @base_commit, @head_commit - @stats = @pull.diff_stats repo, @base_commit, @head_commit + @diff = @pull.diff @repo, @base_commit, @head_commit + @stats = @pull.diff_stats @repo, @base_commit, @head_commit @comments = @issue.comments @commentable = @issue end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 8fd7d4f36..d1a2ce889 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -21,18 +21,29 @@ module DiffHelper end #include Git::Diff::InlineCallback - def render_diff(diff, diff_counter) - diff_display ||= Diff::Display::Unified.new(diff.diff) + def render_diff(diff, diff_counter, comments, diffpath = nil) + if diff.respond_to?(:diff) + filepath = diff.a_path + diff = diff.diff + in_discussion = false + comments = comments.select{|c| c.data.try('[]', :path) == filepath} + else + filepath = diffpath + in_discussion = true + end + + diff_display ||= Diff::Display::Unified.new(diff) url = if @pull @pull.id ? polymorphic_path([@project, @pull]) : '' elsif @commit commit_path @project, @commit end - prepare(diff, url, diff_counter) + prepare(url, diff_counter, filepath, comments, in_discussion) res = "
#{comments.count}#{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commit)}#{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)}
"+data+"
"+data+"
#{comments.count}#{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)}
#{comments.count} + #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)} + #{link_to t('layout.comments.new_inline'), new_comment_path, :class => 'new_inline_comment button'} +
#{num}
......#{line}
- #{line_comment} -
#{render_line(line)}
-
- #{line_comment} -
#{render_line(line)}
-
- #{line_comment} -
#{render_line(line)}
-
- #{line_comment} -
#{render_line(line)}
-
- #{line_comment} -
#{render_line(line)}
-
#{comments.count}#{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commit)}
#{num}
" res += "" res += renderer diff_display.data #diff_display.render(Git::Diff::InlineCallback.new comments, path) + res += tr_line_comments(comments) if in_discussion res += "" res += "
" res.html_safe @@ -41,9 +52,9 @@ module DiffHelper ######################################################## # FIXME: Just to dev, remove to lib ######################################################## - def prepare(diff, url, diff_counter) - @diff, @num_line, @filepath, @url, @diff_counter = diff, -1, diff.a_path, url, diff_counter - @line_comments = @comments.select{|c| c.data.try('[]', :path) == @filepath} + def prepare(url, diff_counter, filepath, comments, in_discussion) + @num_line, @url, @diff_counter, @in_discussion = -1, url, diff_counter, in_discussion + @filepath, @line_comments = filepath, comments end def headerline(line) @@ -216,29 +227,26 @@ module DiffHelper end def render_line_comments - comments = @line_comments.select do |c| - next false if c.data.try('[]', :line) != @num_line.to_s - next true if c.commentable_type == 'Grit::Commit' - #diff = Diff::Display::Unified.new(@diff.diff) - res, ind = true, 0 - @diff.diff.each_line do |line| - res = false if (@num_line-2..@num_line+2).include?(ind) && c.data.try('[]', "line#{ind-@num_line}") != line.chomp - ind = ind + 1 + unless @in_discussion + comments = @line_comments.select do |c| + c.data.try('[]', :line) == @num_line.to_s && c.actual_inline_comment?(@diff) end - res + tr_line_comments(comments) if comments.count > 0 end + end + + def td_line_link id, num + "#{num}" + end + + def tr_line_comments comments " #{comments.count} #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)} #{link_to t('layout.comments.new_inline'), new_comment_path, :class => 'new_inline_comment button'} - - " if comments.count > 0 - end - - def td_line_link id, num - "#{num}" + " end def new_comment_path diff --git a/app/helpers/pull_request_helper.rb b/app/helpers/pull_request_helper.rb index cfc0e9ce3..264d10db2 100644 --- a/app/helpers/pull_request_helper.rb +++ b/app/helpers/pull_request_helper.rb @@ -1,9 +1,9 @@ # -*- encoding : utf-8 -*- module PullRequestHelper def merge_activity comments, commits - issue_comments = @issue.comments.map{ |c| [c.created_at, c] } + pull_comments = comments.group_by(&:data).map{|data, c| [c.first.created_at, [data || {}, [c].flatten]]} commits = @commits.map{ |c| [(c.committed_date || c.authored_date), c] } - (issue_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] } + (pull_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] } end def pull_status_label pull diff --git a/app/models/comment.rb b/app/models/comment.rb index 39956b622..1923eab9b 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -54,6 +54,36 @@ class Comment < ActiveRecord::Base User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify end + def actual_inline_comment?(diff, force = false) + return data[:actual] if data[:actual].present? && !force + filepath, line_number = data[:path], data[:line] + diff_path = diff.select {|d| d.a_path == data[:path]} + comment_line = data[:line].to_i + # NB! also dont create a comment to the diff header + return data[:actual] = false if diff_path.blank? || comment_line == 0 + return data[:actual] = true if commentable_type == 'Grit::Commit' + res, ind = true, 0 + diff_path[0].diff.each_line do |line| + if self.persisted? && (comment_line-2..comment_line+2).include?(ind) && data.try('[]', "line#{ind-comment_line}") != line.chomp + break res = false + end + ind = ind + 1 + end + if ind < comment_line + return data[:actual] = false + else + return data[:actual] = res + end + end + + def inline_diff(repo) + text = data[:strings] + Rails.logger.debug "Comment id is #{id}; text class is #{text.class.name}; text is #{text}" + closest = [] + (-2..0).each {|shift| closest << data["line#{shift}"]} + text << closest.join("\n") + end + protected def subscribe_on_reply diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index ffd0346a5..6f92784dc 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -70,6 +70,7 @@ class PullRequest < ActiveRecord::Base else send(new_status) end + self.comments.each {|c| c.data[:actual]=nil; c.save} # maybe need add new column 'actual'? else self.status = new_status == 'block' ? 'blocked' : new_status end diff --git a/app/views/projects/git/commits/_commit_diff.html.haml b/app/views/projects/git/commits/_commit_diff.html.haml index ce9178850..f745bfa47 100644 --- a/app/views/projects/git/commits/_commit_diff.html.haml +++ b/app/views/projects/git/commits/_commit_diff.html.haml @@ -6,4 +6,4 @@ - if commit_diff.b_path.present? .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, commit_diff.b_path) .clear - .diff_data= render_diff(commit_diff, commit_diff_counter) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? + .diff_data= render_diff(commit_diff, commit_diff_counter, @comments) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index 870edb92d..32f0e873f 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -4,19 +4,8 @@ - commits_queue = [] - merge_activity(@comments, @commits).each do |item| # - - if item.is_a?(Comment) - - if commits_queue.present? - = render 'projects/git/commits/commits_small', :commits => commits_queue - - commits_queue = [] - = render 'projects/comments/comment', :comment => item, :project => @project, :commentable => @issue, :add_id => nil - - else - - commits_queue << item - - comments_for_commit = Comment.for_commit(item) - - if comments_for_commit.present? - = render 'projects/git/commits/commits_small', :commits => commits_queue - - comments_for_commit.each do |comment| - = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => item, :add_id => nil - - commits_queue = [] -- if commits_queue.present? - = render 'projects/git/commits/commits_small', :commits => commits_queue + - if item.is_a? Grit::Commit + = render 'projects/git/commits/commits_small', :commits => [item] + - elsif item.is_a? Array + = render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml new file mode 100644 index 000000000..e875b7292 --- /dev/null +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -0,0 +1,13 @@ +-if item[0].blank? + -item[1].each do |comment| + = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => @issue, :add_id => nil +-else + -comment = item[1].first + .file + %a{:name => "#"} + .top + .l=comment.data[:view_path] + .r=link_to "view full changes", '#' + .clear + .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) + diff --git a/app/views/projects/pull_requests/_pull_diff.html.haml b/app/views/projects/pull_requests/_pull_diff.html.haml index a5c5c8816..b4c249c86 100644 --- a/app/views/projects/pull_requests/_pull_diff.html.haml +++ b/app/views/projects/pull_requests/_pull_diff.html.haml @@ -6,4 +6,4 @@ - if pull_diff.b_path.present? .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, pull_diff.b_path) .clear - .diff_data= render_diff(pull_diff, pull_diff_counter) unless (Grit::Repo.new(@pull.path).tree(commit_id) / pull_diff.b_path).binary? + .diff_data= render_diff(pull_diff, pull_diff_counter, @comments) unless (Grit::Repo.new(@pull.path).tree(commit_id) / pull_diff.b_path).binary? diff --git a/app/views/projects/wiki/_diff_data.html.haml b/app/views/projects/wiki/_diff_data.html.haml index 1c00ff781..897a05f5f 100644 --- a/app/views/projects/wiki/_diff_data.html.haml +++ b/app/views/projects/wiki/_diff_data.html.haml @@ -1,4 +1,4 @@ .blob_header .size= h(diff.deleted_file ? diff.a_path : diff.b_path) .clear -.diff_data.highlight= render_diff(diff) \ No newline at end of file +.diff_data.highlight= render_diff(diff, diff_counter, []) From bb475676280833547ac8cff16642ab3be83396e7 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 13 Oct 2012 00:12:04 +0600 Subject: [PATCH 21/46] [refs #579] small improvements of the discussion --- app/assets/javascripts/extra/pull.js | 5 +++++ app/helpers/diff_helper.rb | 17 ++++++++++------- app/presenters/comment_presenter.rb | 8 ++++++-- app/views/projects/comments/_comment.html.haml | 6 +++--- .../projects/comments/_line_list.html.haml | 3 --- app/views/projects/comments/_list.html.haml | 2 +- .../_discussion_comments.html.haml | 8 +++++--- config/locales/models/pull_request.en.yml | 1 + config/locales/models/pull_request.ru.yml | 5 +++-- 9 files changed, 34 insertions(+), 21 deletions(-) delete mode 100644 app/views/projects/comments/_line_list.html.haml diff --git a/app/assets/javascripts/extra/pull.js b/app/assets/javascripts/extra/pull.js index b69eeb18e..b802f251a 100644 --- a/app/assets/javascripts/extra/pull.js +++ b/app/assets/javascripts/extra/pull.js @@ -24,4 +24,9 @@ $(document).ready(function() { location.hash = href; } }); + + var diff_tab = $('#pull_tabs a[href="#diff"]'); + $('.link_to_full_changes').on('click', function(){ + diff_tab.tab('show'); + }); }); diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index d1a2ce889..3c5e810b0 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -50,7 +50,7 @@ module DiffHelper end ######################################################## - # FIXME: Just to dev, remove to lib + # FIXME: Just to dev, remove to lib. Really need it? ######################################################## def prepare(url, diff_counter, filepath, comments, in_discussion) @num_line, @url, @diff_counter, @in_discussion = -1, url, diff_counter, in_discussion @@ -240,13 +240,16 @@ module DiffHelper end def tr_line_comments comments - " + res=" #{comments.count} - - #{render("projects/comments/line_list", :list => comments, :project => @project, :commentable => @commentable)} - #{link_to t('layout.comments.new_inline'), new_comment_path, :class => 'new_inline_comment button'} - - " + " + comments.each do |comment| + res << "
+ #{render 'projects/comments/comment', :comment => comment, :data => {:project => @project, :commentable => @commentable, :add_anchor => 'inline', :in_discussion => @in_discussion}} +
" + end + res << link_to(t('layout.comments.new_inline'), new_comment_path, :class => 'new_inline_comment button') + res << "" end def new_comment_path diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index e5da30342..ac9564c31 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -32,7 +32,7 @@ class CommentPresenter < ApplicationPresenter project, commentable = options[:project], options[:commentable] path = helpers.project_commentable_comment_path(project, commentable, comment) - res = [link_to(t("layout.link"), "#{helpers.project_commentable_path(project, commentable)}##{comment_anchor}", :class => "link_to_comment").html_safe] + res = [link_to(t("layout.link"), "#{helpers.project_commentable_path(project, commentable)}##{comment_anchor}", :class => "#{@options[:in_discussion].present? ? 'in_discussion_' : ''}link_to_comment").html_safe] if controller.can? :update, @comment res << link_to(t("layout.edit"), path, :id => "comment-#{comment.id}", :class => "edit_comment").html_safe end @@ -66,7 +66,11 @@ class CommentPresenter < ApplicationPresenter def comment_anchor # check for pull diff inline comment - before = (@options[:add_anchor].present? && @comment.commentable_type == 'Issue') ? 'diff-' : '' + before = if @options[:add_anchor].present? && !@options[:in_discussion] + 'diff-' + else + '' + end "#{before}comment#{@comment.id}" end diff --git a/app/views/projects/comments/_comment.html.haml b/app/views/projects/comments/_comment.html.haml index 6a478dbb7..0c25b2ac3 100644 --- a/app/views/projects/comments/_comment.html.haml +++ b/app/views/projects/comments/_comment.html.haml @@ -1,10 +1,10 @@ -- CommentPresenter.present(comment, :project => project, :commentable => commentable, :add_anchor => add_id.presence) do |presenter| +- CommentPresenter.present(comment, data) do |presenter| = render 'shared/feed_message', :presenter => presenter #open-comment.comment.hidden{:class => "comment-#{comment.id}"} =render 'projects/comments/button_md_help' %h3.tmargin0= t("layout.comments.edit_header") - = form_for comment, :url => project_commentable_comment_path(project, commentable, comment), :html => { :class => 'form edit_comment' } do |f| - = render "projects/comments/form", :f => f, :id => "#{add_id.presence}edit_#{comment.id}" + = form_for comment, :url => project_commentable_comment_path(data[:project], data[:commentable], comment), :html => { :class => 'form edit_comment' } do |f| + = render "projects/comments/form", :f => f, :id => "#{data[:add_id]}edit_#{comment.id}" .comment-left =link_to t('layout.cancel'), '#', :id => "comment-#{comment.id}", :class => 'cancel_edit_comment button' .both diff --git a/app/views/projects/comments/_line_list.html.haml b/app/views/projects/comments/_line_list.html.haml deleted file mode 100644 index 4a5e070a5..000000000 --- a/app/views/projects/comments/_line_list.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -- list.each do |comment| - .line-comments - = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable, :add_id => 'inline' diff --git a/app/views/projects/comments/_list.html.haml b/app/views/projects/comments/_list.html.haml index 6b2e447d5..e63505aa7 100644 --- a/app/views/projects/comments/_list.html.haml +++ b/app/views/projects/comments/_list.html.haml @@ -2,5 +2,5 @@ .hr %h3#block-list= t("layout.comments.comments_header") - list.each do |comment| - = render 'projects/comments/comment', :comment => comment, :project => project, :commentable => commentable, :add_id => nil + = render 'projects/comments/comment', :comment => comment, :data => {:project => project, :commentable => commentable} = render "projects/comments/markdown_help" diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index e875b7292..105ffabff 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -1,13 +1,15 @@ -if item[0].blank? -item[1].each do |comment| - = render 'projects/comments/comment', :comment => comment, :project => @project, :commentable => @issue, :add_id => nil + = render 'projects/comments/comment', :comment => comment, :data => {:project => @project, :commentable => @issue} -else -comment = item[1].first + -anchor = "#{comment.actual_inline_comment?(@diff) ? 'diff-' : ''}comment#{comment.id}" .file - %a{:name => "#"} .top .l=comment.data[:view_path] - .r=link_to "view full changes", '#' + .r=link_to t("layout.pull_requests.view_full_changes"), + "#{project_commentable_path(@project, @commentable)}##{anchor}", + :class => 'link_to_full_changes' .clear .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) diff --git a/config/locales/models/pull_request.en.yml b/config/locales/models/pull_request.en.yml index 268e4bcbc..7bc790a7e 100644 --- a/config/locales/models/pull_request.en.yml +++ b/config/locales/models/pull_request.en.yml @@ -53,3 +53,4 @@ en: layout: pull_requests: search: Find pull request... + view_full_changes: View full changes diff --git a/config/locales/models/pull_request.ru.yml b/config/locales/models/pull_request.ru.yml index 7ff23c518..6aff6b4c8 100644 --- a/config/locales/models/pull_request.ru.yml +++ b/config/locales/models/pull_request.ru.yml @@ -2,7 +2,7 @@ ru: projects: pull_requests: new: - header: 'Создать пул реквест' + header: Создать пул реквест submit: Создать пул реквест update: Обновить коммиты show: @@ -23,7 +23,7 @@ ru: %{user} смержил %{to_ref} с %{from_ref} в %{time}' closed: '%{user} закрыл пул реквест в %{time}' - is_big: Этот пул реквест слишком большой! Мы показываем только последние %{count} комитов. + is_big: Этот пул реквест слишком большой! Мы показываем только последние %{count} коммитов. open: '' statuses: blocked: Заблокирован @@ -55,3 +55,4 @@ ru: layout: pull_requests: search: Найти пул реквест... + view_full_changes: Посмотреть все изменения From c5d6707b8f745c8aa7c24fa3d74c90284e00a8aa Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 13 Oct 2012 01:56:02 +0600 Subject: [PATCH 22/46] [refs #579] fixed commit comments errors --- app/controllers/projects/comments_controller.rb | 6 ++++-- .../projects/git/commits_controller.rb | 4 ++-- app/helpers/comments_helper.rb | 4 ++++ app/models/comment.rb | 16 +++++++++++----- app/views/projects/git/commits/_show.html.haml | 2 +- .../pull_requests/_discussion_comments.html.haml | 3 +-- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index 94b780dba..171686d7e 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -9,15 +9,17 @@ class Projects::CommentsController < Projects::BaseController include CommentsHelper def create + anchor = '' if !set_additional_data flash[:error] = I18n.t("flash.comment.save_error") elsif @comment.save flash[:notice] = I18n.t("flash.comment.saved") + anchor = view_context.comment_anchor(@comment) else flash[:error] = I18n.t("flash.comment.save_error") flash[:warning] = @comment.errors.full_messages.join('. ') end - redirect_to project_commentable_path(@project, @commentable) + "#comment#{@comment.id}" + redirect_to "#{project_commentable_path(@project, @commentable)}##{anchor}" end def edit @@ -91,7 +93,7 @@ class Projects::CommentsController < Projects::BaseController end @comment.data[:strings] = strings @comment.data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) - return true end + return true end end diff --git a/app/controllers/projects/git/commits_controller.rb b/app/controllers/projects/git/commits_controller.rb index 513807a24..122884e97 100644 --- a/app/controllers/projects/git/commits_controller.rb +++ b/app/controllers/projects/git/commits_controller.rb @@ -10,11 +10,11 @@ class Projects::Git::CommitsController < Projects::Git::BaseController def show @commit = @commentable = @project.repo.commit(params[:id]) - @comments = Comment.for_commit(@commit) + @comments, @diff = Comment.for_commit(@commit), @commit.diffs respond_to do |format| format.html - format.diff { render :text => (@commit.diffs.map(&:diff).join("\n") rescue ''), :content_type => "text/plain" } + format.diff { render :text => (@diff.map(&:diff).join("\n") rescue ''), :content_type => "text/plain" } format.patch { render :text => (@commit.to_patch rescue ''), :content_type => "text/plain" } end end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 329f70ee4..c05cd3339 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -17,4 +17,8 @@ module CommentsHelper commit_path project, commentable.id end end + + def comment_anchor c + "#{(c.data.present? && c.actual_inline_comment?(@diff)) ? 'diff-' : ''}comment#{c.id}" + end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 1923eab9b..36f551989 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -55,9 +55,13 @@ class Comment < ActiveRecord::Base end def actual_inline_comment?(diff, force = false) - return data[:actual] if data[:actual].present? && !force + unless force + return true if + raise "This is not inline comment!" if data.blank? # for debug + return data[:actual] if data[:actual].present? + end filepath, line_number = data[:path], data[:line] - diff_path = diff.select {|d| d.a_path == data[:path]} + diff_path = (diff || commentable.diffs ).select {|d| d.a_path == data[:path]} comment_line = data[:line].to_i # NB! also dont create a comment to the diff header return data[:actual] = false if diff_path.blank? || comment_line == 0 @@ -77,13 +81,15 @@ class Comment < ActiveRecord::Base end def inline_diff(repo) - text = data[:strings] - Rails.logger.debug "Comment id is #{id}; text class is #{text.class.name}; text is #{text}" - closest = [] + text, closest = data[:strings], [] (-2..0).each {|shift| closest << data["line#{shift}"]} text << closest.join("\n") end + def pull_comment? + return true if commentable.class == Issue && commentable.pull_request.present? + end + protected def subscribe_on_reply diff --git a/app/views/projects/git/commits/_show.html.haml b/app/views/projects/git/commits/_show.html.haml index 9815b5e2d..82683c270 100644 --- a/app/views/projects/git/commits/_show.html.haml +++ b/app/views/projects/git/commits/_show.html.haml @@ -13,6 +13,6 @@ -begin = render_commit_stats(stats) - = render :partial => 'commit_diff', :collection => @commit.diffs + = render :partial => 'commit_diff', :collection => @diff - rescue Grit::Git::GitTimeout %p= t 'layout.git.repositories.commit_diff_too_big' diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index 105ffabff..a1bfcab49 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -3,12 +3,11 @@ = render 'projects/comments/comment', :comment => comment, :data => {:project => @project, :commentable => @issue} -else -comment = item[1].first - -anchor = "#{comment.actual_inline_comment?(@diff) ? 'diff-' : ''}comment#{comment.id}" .file .top .l=comment.data[:view_path] .r=link_to t("layout.pull_requests.view_full_changes"), - "#{project_commentable_path(@project, @commentable)}##{anchor}", + "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", :class => 'link_to_full_changes' .clear .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) From b9d69a966be53305dff275546bce1ae97465f84c Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Sat, 13 Oct 2012 01:59:14 +0600 Subject: [PATCH 23/46] [refs #579] add 404 for wrong commit hash --- app/controllers/projects/git/commits_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/git/commits_controller.rb b/app/controllers/projects/git/commits_controller.rb index 122884e97..049d7f677 100644 --- a/app/controllers/projects/git/commits_controller.rb +++ b/app/controllers/projects/git/commits_controller.rb @@ -9,7 +9,7 @@ class Projects::Git::CommitsController < Projects::Git::BaseController end def show - @commit = @commentable = @project.repo.commit(params[:id]) + @commit = @commentable = @project.repo.commit(params[:id]) || raise(ActiveRecord::RecordNotFound) @comments, @diff = Comment.for_commit(@commit), @commit.diffs respond_to do |format| From 00323cec82ce3eff64aef89c5de1f9bcdfb1a792 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 16:03:35 +0600 Subject: [PATCH 24/46] [refs #579] grouped commits --- app/views/projects/pull_requests/_activity.html.haml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index 32f0e873f..804644410 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -5,7 +5,10 @@ - commits_queue = [] - merge_activity(@comments, @commits).each do |item| # - if item.is_a? Grit::Commit - = render 'projects/git/commits/commits_small', :commits => [item] + -commits_queue << item - elsif item.is_a? Array + = render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? + -commits_queue.clear = render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil += render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? From 1ee9e3f00ecb9ebdaf5689a9c77cffcdee1eb6c1 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 18:30:34 +0600 Subject: [PATCH 25/46] [refs #579] fixed updating comment --- app/assets/javascripts/extra/comment.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/extra/comment.js b/app/assets/javascripts/extra/comment.js index 549e78240..18f7ba52a 100644 --- a/app/assets/javascripts/extra/comment.js +++ b/app/assets/javascripts/extra/comment.js @@ -2,14 +2,14 @@ $(document).ready(function() { var new_comment = $('#open-comment.comment.hidden.new_line_comment'); $(document).on('click', '.buttons a.edit_comment', function() { - $(this).parent().parent().parent().hide(); - $('#open-comment'+'.comment.'+$(this).attr('id')).show(); + $(this).parents('div.activity').hide() + .next().show(); return false; }); $(document).on('click', '.cancel_edit_comment.button', function() { - $(this).parent().parent().parent().hide(); - $('.buttons a.edit_comment#'+$(this).attr('id')).parent().parent().parent().show(); + $(this).parents('#open-comment.comment').hide() + .prev().show(); return false; }); @@ -22,8 +22,9 @@ $(document).ready(function() { data: form.serialize(), success: function(data){ var cancel_button = form.find('.cancel_edit_comment.button'); + var id = cancel_button.attr('id').match(/\d+$/)[0]; cancel_button.click(); - $('.buttons a.edit_comment#'+cancel_button.attr('id')).parent().parent().find('.cm-s-default.md_and_cm').html(data).find('code').each(function (code) { CodeMirrorRun(this); }) + $('#comment'+id+', #diff-comment'+id).parent().find('.cm-s-default.md_and_cm').html(data).find('code').each(function (code) { CodeMirrorRun(this); }) }, error: function(data){ form.before(data.responseText); From fb1288e382d6f2f1f23855e33e99d383acec7919 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 22:33:16 +0600 Subject: [PATCH 26/46] [refs #579] ugly sorting common pull comments --- app/helpers/pull_request_helper.rb | 8 ++++--- .../pull_requests/_activity.html.haml | 23 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/app/helpers/pull_request_helper.rb b/app/helpers/pull_request_helper.rb index 264d10db2..db6ee030d 100644 --- a/app/helpers/pull_request_helper.rb +++ b/app/helpers/pull_request_helper.rb @@ -1,9 +1,11 @@ # -*- encoding : utf-8 -*- module PullRequestHelper def merge_activity comments, commits - pull_comments = comments.group_by(&:data).map{|data, c| [c.first.created_at, [data || {}, [c].flatten]]} - commits = @commits.map{ |c| [(c.committed_date || c.authored_date), c] } - (pull_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] } + common_comments, pull_comments = comments.partition {|c| c.data.blank?} + common_comments = common_comments.map{ |c| [c.created_at, c] } + pull_comments = pull_comments.group_by(&:data).map{|data, c| [c.first.created_at, [data || {}, [c].flatten]]} + commits = commits.map{ |c| [(c.committed_date || c.authored_date), c] } + (common_comments + pull_comments + commits).sort_by{ |c| c[0] }.map{ |c| c[1] } end def pull_status_label pull diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index 804644410..58c4e8ad4 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -1,14 +1,15 @@ -%a{ :name => "comments" } .hr -%h3#block-list= t("layout.comments.comments_header") - -- commits_queue = [] -- merge_activity(@comments, @commits).each do |item| # - - if item.is_a? Grit::Commit - -commits_queue << item - - elsif item.is_a? Array - = render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? +-commits_queue = [] +-merge_activity(@comments, @commits).each do |item| # + -if item.is_a? Comment + =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? -commits_queue.clear - = render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil -= render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? + =render 'projects/comments/comment', :comment => item, :data => {:project => @project, :commentable => @commentable} + -elsif item.is_a? Grit::Commit + -commits_queue << item + -elsif item.is_a? Array + =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? + -commits_queue.clear + =render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil +=render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? From ef37180825d0ab72d45e60d91bc4e567a26d84fc Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 22:37:31 +0600 Subject: [PATCH 27/46] [refs #579] small activity refactoring --- .../pull_requests/_activity.html.haml | 2 +- .../_discussion_comments.html.haml | 22 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index 58c4e8ad4..bd131b305 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -10,6 +10,6 @@ -elsif item.is_a? Array =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? -commits_queue.clear - =render 'projects/pull_requests/discussion_comments', :item => item, :project => @project, :commentable => @issue, :add_id => nil + =render 'projects/pull_requests/discussion_comments', :item => item, :add_id => nil =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index a1bfcab49..15c6bf63d 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -1,14 +1,10 @@ --if item[0].blank? - -item[1].each do |comment| - = render 'projects/comments/comment', :comment => comment, :data => {:project => @project, :commentable => @issue} --else - -comment = item[1].first - .file - .top - .l=comment.data[:view_path] - .r=link_to t("layout.pull_requests.view_full_changes"), - "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", - :class => 'link_to_full_changes' - .clear - .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) +-comment = item[1].first +.file + .top + .l=comment.data[:view_path] + .r=link_to t("layout.pull_requests.view_full_changes"), + "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", + :class => 'link_to_full_changes' + .clear + .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) From e3a0005bd3cd77055c9f5d1d0a281ff6c9fe046a Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 23:42:28 +0600 Subject: [PATCH 28/46] [refs #579] small refactoring --- app/models/pull_request.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 6f92784dc..156b691e4 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -20,10 +20,6 @@ class PullRequest < ActiveRecord::Base scope :needed_checking, includes(:issue).where(:issues => {:status => ['open', 'blocked', 'ready']}) state_machine :status, :initial => :open do - #after_transition [:ready, :blocked] => [:merged, :closed] do |pull, transition| - # FileUtils.rm_rf(pull.path) # What about diff? - #end - event :ready do transition [:ready, :open, :blocked] => :ready end @@ -65,11 +61,7 @@ class PullRequest < ActiveRecord::Base end if do_transaction - if new_status == 'already' - ready; merging - else - send(new_status) - end + new_status == 'already' ? (ready; merging) : send(new_status) self.comments.each {|c| c.data[:actual]=nil; c.save} # maybe need add new column 'actual'? else self.status = new_status == 'block' ? 'blocked' : new_status From 66f0cc3244474b65be5048c5aa4838d7ed55c718 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 23:45:01 +0600 Subject: [PATCH 29/46] [refs #579] fixed setting status to inline comments --- app/models/pull_request.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 156b691e4..41f1e8c5c 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -62,7 +62,7 @@ class PullRequest < ActiveRecord::Base if do_transaction new_status == 'already' ? (ready; merging) : send(new_status) - self.comments.each {|c| c.data[:actual]=nil; c.save} # maybe need add new column 'actual'? + self.update_inline_comments else self.status = new_status == 'block' ? 'blocked' : new_status end @@ -201,4 +201,17 @@ class PullRequest < ActiveRecord::Base def clean_dir FileUtils.rm_rf path end + + def update_inline_comments + if self.comments.count > 0 + repo = Grit::Repo.new self.path + diff = self.diff repo, self.common_ancestor, repo.commits(self.head_branch).first + end + self.comments.each do |c| + if c.data.present? # maybe need add new column 'actual'? + c.actual_inline_comment? diff, true + c.save + end + end + end end From 8d4311e4197ea18c987aa3792facaeb6814b21ee Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Mon, 15 Oct 2012 23:46:44 +0600 Subject: [PATCH 30/46] [refs #579] remove a wrong line --- app/models/comment.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 36f551989..30f42c1ec 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -56,7 +56,6 @@ class Comment < ActiveRecord::Base def actual_inline_comment?(diff, force = false) unless force - return true if raise "This is not inline comment!" if data.blank? # for debug return data[:actual] if data[:actual].present? end From cef112efc3502e14b37ead4fa624d25744f74e0c Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 00:14:43 +0600 Subject: [PATCH 31/46] [refs #579] first version of the outdated diff --- app/views/projects/pull_requests/_activity.html.haml | 10 +++++++++- config/locales/models/pull_request.en.yml | 3 ++- config/locales/models/pull_request.ru.yml | 3 ++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/views/projects/pull_requests/_activity.html.haml b/app/views/projects/pull_requests/_activity.html.haml index bd131b305..3f868e607 100644 --- a/app/views/projects/pull_requests/_activity.html.haml +++ b/app/views/projects/pull_requests/_activity.html.haml @@ -10,6 +10,14 @@ -elsif item.is_a? Array =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? -commits_queue.clear - =render 'projects/pull_requests/discussion_comments', :item => item, :add_id => nil + -unless item[1].first.data[:actual] + -exp_id = "expand-comment#{item[1].first.id}" + .activity + =t '.show_outdated_diff' + %span.data-expander.collapsed{:id => exp_id}   + .hidden{:id => "content-#{exp_id}"} + =render 'projects/pull_requests/discussion_comments', :item => item, :add_id => nil + -else + =render 'projects/pull_requests/discussion_comments', :item => item, :add_id => nil =render 'projects/git/commits/commits_small', :commits => commits_queue if commits_queue.present? diff --git a/config/locales/models/pull_request.en.yml b/config/locales/models/pull_request.en.yml index 7bc790a7e..c9f002c1c 100644 --- a/config/locales/models/pull_request.en.yml +++ b/config/locales/models/pull_request.en.yml @@ -28,7 +28,8 @@ en: ready: Ready merged: Merged closed: Closed - + activity: + show_outdated_diff: Show outdated_diff pull_requests: tabs: discussion: Discussion diff --git a/config/locales/models/pull_request.ru.yml b/config/locales/models/pull_request.ru.yml index 6aff6b4c8..ccc46fd70 100644 --- a/config/locales/models/pull_request.ru.yml +++ b/config/locales/models/pull_request.ru.yml @@ -30,7 +30,8 @@ ru: ready: Готов к мержу merged: Смержен closed: Закрыт - + activity: + show_outdated_diff: Отобразить устаревшие изменения pull_requests: tabs: discussion: Дискуссия From 5c7fb21511eda9e003c3210b603bc6ea92d0266e Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 00:16:01 +0600 Subject: [PATCH 32/46] [refs #579] add cursor pointer to arrow --- app/assets/stylesheets/design/custom.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/design/custom.scss b/app/assets/stylesheets/design/custom.scss index c357af743..c60144e2a 100644 --- a/app/assets/stylesheets/design/custom.scss +++ b/app/assets/stylesheets/design/custom.scss @@ -118,6 +118,7 @@ div.activity, .commits_activity { margin-left: 10px; display: inline-block; width: 12px; + cursor: pointer; } .data-expander.collapsed { background: #FFF image-url('expand-gray.png') no-repeat; From a807a222306d71818a478f43eeea7ade826102b4 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 14:07:26 +0600 Subject: [PATCH 33/46] [refs #579] remove debug code --- app/assets/javascripts/extra/comment.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/assets/javascripts/extra/comment.js b/app/assets/javascripts/extra/comment.js index 18f7ba52a..eb5c59386 100644 --- a/app/assets/javascripts/extra/comment.js +++ b/app/assets/javascripts/extra/comment.js @@ -34,9 +34,6 @@ $(document).ready(function() { }); $('.new_inline_comment.button').on('click', function() { - var tmp = $(this).parents('tr').prev('tr') - tmp = tmp.find("a[href='"+$(this).attr('href')+"']"); - var tmp = $(this).parents('tr').prev('tr').find("a[href='"+$(this).attr('href')+"']"); $(this).parents('tr').prev('tr').find("a[href='"+$(this).attr('href')+"']").click(); return false; }); From b0604f89d174a785a8def21ba45f8625b6ae11d4 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 15:50:21 +0600 Subject: [PATCH 34/46] [refs #579] move logick to model --- .../projects/comments_controller.rb | 38 +------------------ app/models/comment.rb | 36 ++++++++++++++++++ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index 171686d7e..c925b33b4 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -10,7 +10,7 @@ class Projects::CommentsController < Projects::BaseController def create anchor = '' - if !set_additional_data + if !@comment.set_additional_data params flash[:error] = I18n.t("flash.comment.save_error") elsif @comment.save flash[:notice] = I18n.t("flash.comment.saved") @@ -60,40 +60,4 @@ class Projects::CommentsController < Projects::BaseController @comment = params[:id].present? && Comment.find(params[:id]) || current_user.comments.build(params[:comment]) {|c| c.commentable = @commentable; c.project = @project} end - - def set_additional_data - return true if params[:path].blank? && params[:line].blank? # not inline comment - @comment.data = {:path => params[:path], :line => params[:line]} - if @commentable.class == Issue && pull = @commentable.pull_request - repo = Grit::Repo.new(pull.path) - to_commit, from_commit = pull.common_ancestor, repo.commits(pull.head_branch).first - - diff = pull.diff repo, to_commit, from_commit - diff_path = diff.select {|d| d.a_path == params[:path]} - return false unless @comment.actual_inline_comment?(diff, true) - - comment_line, line_number, strings = params[:line].to_i, -1, [] - diff_path[0].diff.each_line do |line| - line_number = line_number.succ - # Save 2 lines above and bottom of the diff comment line - break if line_number > comment_line + 2 - if (comment_line-2..comment_line+2).include? line_number - @comment.data["line#{line_number-comment_line}"] = line.chomp - end - - # Save lines from the closest header for rendering in the discussion - if line_number < comment_line - 2 - # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)" - if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/ - strings = line - else - strings << line - end - end - end - @comment.data[:strings] = strings - @comment.data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) - end - return true - end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 30f42c1ec..ff1e0f802 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -89,6 +89,42 @@ class Comment < ActiveRecord::Base return true if commentable.class == Issue && commentable.pull_request.present? end + def set_additional_data params + return true if params[:path].blank? && params[:line].blank? # not inline comment + self.data = {:path => params[:path], :line => params[:line]} + if commentable.class == Issue && pull = commentable.pull_request + repo = Grit::Repo.new(pull.path) + to_commit, from_commit = pull.common_ancestor, repo.commits(pull.head_branch).first + + diff = pull.diff repo, to_commit, from_commit + diff_path = diff.select {|d| d.a_path == params[:path]} + return false unless actual_inline_comment?(diff, true) + + comment_line, line_number, strings = params[:line].to_i, -1, [] + diff_path[0].diff.each_line do |line| + line_number = line_number.succ + # Save 2 lines above and bottom of the diff comment line + break if line_number > comment_line + 2 + if (comment_line-2..comment_line+2).include? line_number + data["line#{line_number-comment_line}"] = line.chomp + end + + # Save lines from the closest header for rendering in the discussion + if line_number < comment_line - 2 + # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)" + if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/ + strings = line + else + strings << line + end + end + end + data[:strings] = strings + data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) + end + return true + end + protected def subscribe_on_reply From 957973450695bfe91e84730b4f540973d1015372 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 16:19:53 +0600 Subject: [PATCH 35/46] [refs #579] some optimization by getting pull repo --- app/controllers/projects/pull_requests_controller.rb | 9 ++++----- app/models/comment.rb | 6 ++---- app/models/pull_request.rb | 8 +++++--- app/views/projects/pull_requests/_pull_diff.html.haml | 2 +- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index 34c8c6f0f..f1b0139d4 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -128,16 +128,15 @@ class Projects::PullRequestsController < Projects::BaseController end def load_diff_commits_data - @repo = Grit::Repo.new(@pull.path) @base_commit = @pull.common_ancestor - @head_commit = @repo.commits(@pull.head_branch).first + @head_commit = @pull.repo.commits(@pull.head_branch).first - @commits = @repo.commits_between(@repo.commits(@pull.to_ref).first, @head_commit) + @commits = @pull.repo.commits_between(@pull.repo.commits(@pull.to_ref).first, @head_commit) @total_commits = @commits.count @commits = @commits.last(100) - @diff = @pull.diff @repo, @base_commit, @head_commit - @stats = @pull.diff_stats @repo, @base_commit, @head_commit + @diff = @pull.diff @pull.repo, @base_commit, @head_commit + @stats = @pull.diff_stats @pull.repo, @base_commit, @head_commit @comments = @issue.comments @commentable = @issue end diff --git a/app/models/comment.rb b/app/models/comment.rb index ff1e0f802..61aabdef9 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -93,10 +93,8 @@ class Comment < ActiveRecord::Base return true if params[:path].blank? && params[:line].blank? # not inline comment self.data = {:path => params[:path], :line => params[:line]} if commentable.class == Issue && pull = commentable.pull_request - repo = Grit::Repo.new(pull.path) - to_commit, from_commit = pull.common_ancestor, repo.commits(pull.head_branch).first - - diff = pull.diff repo, to_commit, from_commit + to_commit, from_commit = pull.common_ancestor, pull.repo.commits(pull.head_branch).first + diff = pull.diff pull.repo, to_commit, from_commit diff_path = diff.select {|d| d.a_path == params[:path]} return false unless actual_inline_comment?(diff, true) diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 41f1e8c5c..d6148580a 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -96,7 +96,6 @@ class PullRequest < ActiveRecord::Base def common_ancestor return @common_ancestor if @common_ancestor - repo = Grit::Repo.new(path) base_commit = repo.commits(to_ref).first head_commit = repo.commits(head_branch).first @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, head_commit)) || base_commit @@ -147,6 +146,10 @@ class PullRequest < ActiveRecord::Base end end + def repo + return @repo if @repo.present? #&& !id_changed? + @repo = Grit::Repo.new path + end protected def merge @@ -204,8 +207,7 @@ class PullRequest < ActiveRecord::Base def update_inline_comments if self.comments.count > 0 - repo = Grit::Repo.new self.path - diff = self.diff repo, self.common_ancestor, repo.commits(self.head_branch).first + diff = self.diff self.repo, self.common_ancestor, repo.commits(self.head_branch).first end self.comments.each do |c| if c.data.present? # maybe need add new column 'actual'? diff --git a/app/views/projects/pull_requests/_pull_diff.html.haml b/app/views/projects/pull_requests/_pull_diff.html.haml index dc707a042..931084e7e 100644 --- a/app/views/projects/pull_requests/_pull_diff.html.haml +++ b/app/views/projects/pull_requests/_pull_diff.html.haml @@ -6,5 +6,5 @@ - if pull_diff.b_path.present? .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, pull_diff.b_path) .clear - -if pull_diff.diff.present? && !(Grit::Repo.new(@pull.path).tree(commit_id) / pull_diff.b_path).binary? + -if pull_diff.diff.present? && !(@pull.repo.tree(commit_id) / pull_diff.b_path).binary? .diff_data=render_diff(pull_diff, pull_diff_counter, @comments) From f060d919d978eab53c7b55f8068daf1c77ea2abf Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 19:20:55 +0600 Subject: [PATCH 36/46] [refs #579] add new helper --- .../projects/comments_controller.rb | 6 +----- app/helpers/comments_helper.rb | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/comments_controller.rb b/app/controllers/projects/comments_controller.rb index c925b33b4..e1f7f1073 100644 --- a/app/controllers/projects/comments_controller.rb +++ b/app/controllers/projects/comments_controller.rb @@ -41,11 +41,7 @@ class Projects::CommentsController < Projects::BaseController end def new_line - @path = if @commentable.class == Issue - project_issue_comments_path(@project, @commentable) - elsif @commentable.class == Grit::Commit - project_commit_comments_path(@project, @commentable) - end + @path = view_context.project_commentable_comments_path(@project, @commentable) render :layout => false end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index c05cd3339..204348df6 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -1,23 +1,29 @@ # -*- encoding : utf-8 -*- module CommentsHelper def project_commentable_comment_path(project, commentable, comment) - case - when Comment.issue_comment?(commentable.class) + if Comment.issue_comment?(commentable.class) project_issue_comment_path(project, commentable, comment) - when Comment.commit_comment?(commentable.class) + elsif Comment.commit_comment?(commentable.class) project_commit_comment_path(project, commentable, comment) end end def project_commentable_path(project, commentable) - case - when Comment.issue_comment?(commentable.class) + if Comment.issue_comment?(commentable.class) polymorphic_path [project, commentable.pull_request ? commentable.pull_request : commentable] - when Comment.commit_comment?(commentable.class) + elsif Comment.commit_comment?(commentable.class) commit_path project, commentable.id end end + def project_commentable_comments_path(project, commentable) + if commentable.class == Issue + project_issue_comments_path(@project, @commentable) + elsif commentable.class == Grit::Commit + project_commit_comments_path(@project, @commentable) + end + end + def comment_anchor c "#{(c.data.present? && c.actual_inline_comment?(@diff)) ? 'diff-' : ''}comment#{c.id}" end From b2fd34835159319f13737e19c8598b6b1aeb7b82 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 19:21:47 +0600 Subject: [PATCH 37/46] [refs #579] fixed bug with saving around lines --- app/models/comment.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 61aabdef9..e93d84a22 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -111,13 +111,13 @@ class Comment < ActiveRecord::Base if line_number < comment_line - 2 # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)" if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/ - strings = line + strings = [line] else strings << line end end end - data[:strings] = strings + data[:strings] = strings.join '' data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) end return true From 3868fbf4f31102ee9e9bb38e56c147f2ed5be8c6 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 16 Oct 2012 19:22:19 +0600 Subject: [PATCH 38/46] [refs #579] forgot change to the pull.repo --- app/models/pull_request.rb | 1 + app/views/projects/pull_requests/_discussion_comments.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index d6148580a..761c64381 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -150,6 +150,7 @@ class PullRequest < ActiveRecord::Base return @repo if @repo.present? #&& !id_changed? @repo = Grit::Repo.new path end + protected def merge diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index 15c6bf63d..f7aca9f6e 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -6,5 +6,5 @@ "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", :class => 'link_to_full_changes' .clear - .diff_data=render_diff(comment.inline_diff(@repo), 0, item[1], comment.data[:path]) + .diff_data=render_diff(comment.inline_diff(@pull.repo), 0, item[1], comment.data[:path]) From 7870ffef7d42bc06162691f0067c052a81e28782 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 00:38:42 +0600 Subject: [PATCH 39/46] [refs #579] fixed some bugs --- app/helpers/diff_helper.rb | 13 ++++++++--- app/models/comment.rb | 22 ++++++++++++------- .../projects/comments/new_line.html.haml | 1 + .../_discussion_comments.html.haml | 13 +++++++---- config/locales/models/pull_request.en.yml | 1 + config/locales/models/pull_request.ru.yml | 3 +-- 6 files changed, 36 insertions(+), 17 deletions(-) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 3c5e810b0..fac745b78 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -53,8 +53,13 @@ module DiffHelper # FIXME: Just to dev, remove to lib. Really need it? ######################################################## def prepare(url, diff_counter, filepath, comments, in_discussion) - @num_line, @url, @diff_counter, @in_discussion = -1, url, diff_counter, in_discussion + @url, @diff_counter, @in_discussion = url, diff_counter, in_discussion @filepath, @line_comments = filepath, comments + @add_reply_id, @num_line = if @in_discussion + [@line_comments[0].id, @line_comments[0].data[:line].to_i - @line_comments[0].data[:strings].lines.count.to_i-1] + else + [nil, -1] + end end def headerline(line) @@ -223,6 +228,7 @@ module DiffHelper end def line_comment + return if @in_discussion && @add_reply_id && @line_comments[0].data[:line].to_i != @num_line link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_comment_path, :class => 'add_line-comment' end @@ -253,10 +259,11 @@ module DiffHelper end def new_comment_path + hash = {:path => @filepath, :line => @num_line} if @commentable.class == Issue - project_new_line_pull_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + project_new_line_pull_comment_path(@project, @commentable, hash.merge({:in_reply => @add_reply_id})) elsif @commentable.class == Grit::Commit - new_line_commit_comment_path(@project, @commentable, :path => @filepath, :line => @num_line) + new_line_commit_comment_path(@project, @commentable, hash) end end end diff --git a/app/models/comment.rb b/app/models/comment.rb index e93d84a22..aa6e29ca3 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -57,7 +57,7 @@ class Comment < ActiveRecord::Base def actual_inline_comment?(diff, force = false) unless force raise "This is not inline comment!" if data.blank? # for debug - return data[:actual] if data[:actual].present? + return data[:actual] unless data[:actual].nil? end filepath, line_number = data[:path], data[:line] diff_path = (diff || commentable.diffs ).select {|d| d.a_path == data[:path]} @@ -79,10 +79,8 @@ class Comment < ActiveRecord::Base end end - def inline_diff(repo) - text, closest = data[:strings], [] - (-2..0).each {|shift| closest << data["line#{shift}"]} - text << closest.join("\n") + def inline_diff + data[:strings] + data['line0'] end def pull_comment? @@ -91,6 +89,10 @@ class Comment < ActiveRecord::Base def set_additional_data params return true if params[:path].blank? && params[:line].blank? # not inline comment + if params[:in_reply].present? && reply = Comment.where(:id => params[:in_reply]).first + self.data = reply.data + return true + end self.data = {:path => params[:path], :line => params[:line]} if commentable.class == Issue && pull = commentable.pull_request to_commit, from_commit = pull.common_ancestor, pull.repo.commits(pull.head_branch).first @@ -108,16 +110,20 @@ class Comment < ActiveRecord::Base end # Save lines from the closest header for rendering in the discussion - if line_number < comment_line - 2 + if line_number < comment_line # Header is the line like "@@ -47,9 +50,8 @@ def initialize(user)" - if line =~ /^@@ [+-]([0-9]+)(?:,([0-9]+))? [+-]([0-9]+)(?:,([0-9]+))? @@/ + if line =~ Diff::Display::Unified::Generator::LINE_NUM_RE strings = [line] else strings << line end end end - data[:strings] = strings.join '' + ## Bug with numbers of diff lines, now store all diff + data[:strings] = strings.join + # Limit stored diff to 10 lines (see inline_diff) + #data[:strings] = ((strings.count) <= 9 ? strings : [strings[0]] + strings.last(8)).join + ## data[:view_path] = h(diff_path[0].renamed_file ? "#{diff_path[0].a_path.rtruncate 60} -> #{diff_path[0].b_path.rtruncate 60}" : diff_path[0].a_path.rtruncate(120)) end return true diff --git a/app/views/projects/comments/new_line.html.haml b/app/views/projects/comments/new_line.html.haml index 64b69a4b2..15dc609be 100644 --- a/app/views/projects/comments/new_line.html.haml +++ b/app/views/projects/comments/new_line.html.haml @@ -7,4 +7,5 @@ =link_to t('layout.cancel'), '#', :class => 'cancel_inline_comment button' =hidden_field_tag :path, params[:path] =hidden_field_tag :line, params[:line] + =hidden_field_tag :in_reply, params[:in_reply] if params[:in_reply].present? .both diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index f7aca9f6e..96713f014 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -2,9 +2,14 @@ .file .top .l=comment.data[:view_path] - .r=link_to t("layout.pull_requests.view_full_changes"), - "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", - :class => 'link_to_full_changes' + -if comment.actual_inline_comment? @diff + .r=link_to t("layout.pull_requests.view_full_changes"), + "#{project_commentable_path(@project, @commentable)}##{comment_anchor(comment)}", + :class => 'link_to_full_changes' + -else + .r + =t'projects.pull_requests.outdated_diff' + =image_tag 'x.png' .clear - .diff_data=render_diff(comment.inline_diff(@pull.repo), 0, item[1], comment.data[:path]) + .diff_data=render_diff(comment.inline_diff, 0, item[1], comment.data[:path]) diff --git a/config/locales/models/pull_request.en.yml b/config/locales/models/pull_request.en.yml index c9f002c1c..8c1842f18 100644 --- a/config/locales/models/pull_request.en.yml +++ b/config/locales/models/pull_request.en.yml @@ -28,6 +28,7 @@ en: ready: Ready merged: Merged closed: Closed + outdated_diff: Outdated diff activity: show_outdated_diff: Show outdated_diff pull_requests: diff --git a/config/locales/models/pull_request.ru.yml b/config/locales/models/pull_request.ru.yml index ccc46fd70..65eb34bfd 100644 --- a/config/locales/models/pull_request.ru.yml +++ b/config/locales/models/pull_request.ru.yml @@ -7,8 +7,6 @@ ru: update: Обновить коммиты show: pull: Пул реквест - into: из - from: в header: Пул реквест status: close: Закрыть пул реквест @@ -30,6 +28,7 @@ ru: ready: Готов к мержу merged: Смержен closed: Закрыт + outdated_diff: Устаревшие изменения activity: show_outdated_diff: Отобразить устаревшие изменения pull_requests: From 62ca030110573089568852d96ebae3712f760b7a Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 17:46:57 +0600 Subject: [PATCH 40/46] [refs #90] head_branch -> from_branch --- .../projects/pull_requests_controller.rb | 2 +- app/models/pull_request.rb | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index f1b0139d4..2ed7241f4 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -129,7 +129,7 @@ class Projects::PullRequestsController < Projects::BaseController def load_diff_commits_data @base_commit = @pull.common_ancestor - @head_commit = @pull.repo.commits(@pull.head_branch).first + @head_commit = @pull.repo.commits(@pull.from_branch).first @commits = @pull.repo.commits_between(@pull.repo.commits(@pull.to_ref).first, @head_commit) @total_commits = @commits.count diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 761c64381..1f09879a2 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -86,7 +86,7 @@ class PullRequest < ActiveRecord::Base File.join(APP_CONFIG['root_path'], 'pull_requests', to_project.owner.uname, to_project.name, filename) end - def head_branch + def from_branch if to_project != from_project "head_#{from_ref}" else @@ -97,7 +97,7 @@ class PullRequest < ActiveRecord::Base def common_ancestor return @common_ancestor if @common_ancestor base_commit = repo.commits(to_ref).first - head_commit = repo.commits(head_branch).first + head_commit = repo.commits(from_branch).first @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, head_commit)) || base_commit end @@ -156,7 +156,7 @@ class PullRequest < ActiveRecord::Base def merge clone message = "Merge pull request ##{serial_id} from #{from_project.name_with_owner}:#{from_ref}\r\n #{title}" - %x(cd #{path} && git checkout #{to_ref} && git merge --no-ff #{head_branch} -m '#{message}') + %x(cd #{path} && git checkout #{to_ref} && git merge --no-ff #{from_branch} -m '#{message}') end def clone @@ -182,7 +182,7 @@ class PullRequest < ActiveRecord::Base system 'git', 'checkout', from_ref system 'git', 'pull', 'origin', from_ref else - system 'git', 'fetch', 'head', "+#{from_ref}:#{head_branch}" + system 'git', 'fetch', 'head', "+#{from_ref}:#{from_branch}" end end # TODO catch errors @@ -194,10 +194,10 @@ class PullRequest < ActiveRecord::Base system 'git', 'checkout', to_ref to_project.repo.branches.each do |branch| - system 'git', 'branch', '-D', branch.name unless [to_ref, head_branch].include? branch.name + system 'git', 'branch', '-D', branch.name unless [to_ref, from_branch].include? branch.name end to_project.repo.tags.each do |tag| - system 'git', 'tag', '-d', tag.name unless [to_ref, head_branch].include? tag.name + system 'git', 'tag', '-d', tag.name unless [to_ref, from_branch].include? tag.name end end end @@ -208,7 +208,7 @@ class PullRequest < ActiveRecord::Base def update_inline_comments if self.comments.count > 0 - diff = self.diff self.repo, self.common_ancestor, repo.commits(self.head_branch).first + diff = self.diff self.repo, self.common_ancestor, repo.commits(self.from_branch).first end self.comments.each do |c| if c.data.present? # maybe need add new column 'actual'? From e15bf13c4fdb41db14b2dd11918f3bed202f1c50 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 18:09:12 +0600 Subject: [PATCH 41/46] [refs #90] small refactoring of the diff method --- .../projects/pull_requests_controller.rb | 11 +++-------- app/models/comment.rb | 6 ++---- app/models/pull_request.rb | 16 ++++++++++------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index 2ed7241f4..5fa552e35 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -128,17 +128,12 @@ class Projects::PullRequestsController < Projects::BaseController end def load_diff_commits_data - @base_commit = @pull.common_ancestor - @head_commit = @pull.repo.commits(@pull.from_branch).first - - @commits = @pull.repo.commits_between(@pull.repo.commits(@pull.to_ref).first, @head_commit) + @commits = @pull.repo.commits_between(@pull.from_commit, @pull.to_commit) @total_commits = @commits.count @commits = @commits.last(100) - @diff = @pull.diff @pull.repo, @base_commit, @head_commit - @stats = @pull.diff_stats @pull.repo, @base_commit, @head_commit - @comments = @issue.comments - @commentable = @issue + @diff, @stats = @pull.diff, @pull.diff_stats + @comments, @commentable = @issue.comments, @issue end def find_destination_project bang=true diff --git a/app/models/comment.rb b/app/models/comment.rb index aa6e29ca3..1f8884273 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -95,10 +95,8 @@ class Comment < ActiveRecord::Base end self.data = {:path => params[:path], :line => params[:line]} if commentable.class == Issue && pull = commentable.pull_request - to_commit, from_commit = pull.common_ancestor, pull.repo.commits(pull.head_branch).first - diff = pull.diff pull.repo, to_commit, from_commit - diff_path = diff.select {|d| d.a_path == params[:path]} - return false unless actual_inline_comment?(diff, true) + diff_path = pull.diff.select {|d| d.a_path == params[:path]} + return false unless actual_inline_comment?(pull.diff, true) comment_line, line_number, strings = params[:line].to_i, -1, [] diff_path[0].diff.each_line do |line| diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index 1f09879a2..dcee28272 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -97,14 +97,14 @@ class PullRequest < ActiveRecord::Base def common_ancestor return @common_ancestor if @common_ancestor base_commit = repo.commits(to_ref).first - head_commit = repo.commits(from_branch).first - @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, head_commit)) || base_commit + @common_ancestor = repo.commit(repo.git.merge_base({}, base_commit, from_commit)) || base_commit end + alias_method :to_commit, :common_ancestor - def diff_stats(repo, a,b) + def diff_stats stats = [] Dir.chdir(path) do - lines = repo.git.native(:diff, {:numstat => true, :M => true}, "#{a.id}...#{b.id}").split("\n") + lines = repo.git.native(:diff, {:numstat => true, :M => true}, "#{to_commit.id}...#{from_commit.id}").split("\n") while !lines.empty? files = [] while lines.first =~ /^([-\d]+)\s+([-\d]+)\s+(.+)/ @@ -119,8 +119,8 @@ class PullRequest < ActiveRecord::Base end # FIXME maybe move to warpc/grit? - def diff(repo, a, b) - diff = repo.git.native('diff', {:M => true}, "#{a}...#{b}") + def diff + diff = repo.git.native('diff', {:M => true}, "#{to_commit.id}...#{from_commit.id}") if diff =~ /diff --git a/ diff = diff.sub(/.*?(diff --git a)/m, '\1') @@ -151,6 +151,10 @@ class PullRequest < ActiveRecord::Base @repo = Grit::Repo.new path end + def from_commit + repo.commits(from_branch).first + end + protected def merge From c6f45956ccfc716b03db8a7683cb7ba25aed1512 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 18:50:43 +0600 Subject: [PATCH 42/46] [refs #90] fixed error by 24fd99b2d5cab --- app/controllers/projects/pull_requests_controller.rb | 2 +- app/views/projects/pull_requests/_pull_diff.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index 5fa552e35..20e06f79d 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/app/controllers/projects/pull_requests_controller.rb @@ -128,7 +128,7 @@ class Projects::PullRequestsController < Projects::BaseController end def load_diff_commits_data - @commits = @pull.repo.commits_between(@pull.from_commit, @pull.to_commit) + @commits = @pull.repo.commits_between(@pull.to_commit, @pull.from_commit) @total_commits = @commits.count @commits = @commits.last(100) diff --git a/app/views/projects/pull_requests/_pull_diff.html.haml b/app/views/projects/pull_requests/_pull_diff.html.haml index 931084e7e..e4b694993 100644 --- a/app/views/projects/pull_requests/_pull_diff.html.haml +++ b/app/views/projects/pull_requests/_pull_diff.html.haml @@ -1,4 +1,4 @@ -- commit_id = pull_diff.deleted_file ? @base_commit.id : @head_commit.id +- commit_id = pull_diff.deleted_file ? @pull.to_commit.id : @pull.from_commit.id .file %a{:name => "diff-#{pull_diff_counter}"} .top From ecf8e7e5eec8c9f08248d9ef24d6ee78c59b989b Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 18:51:28 +0600 Subject: [PATCH 43/46] [refs #579] fixed wiki & refactoring --- app/helpers/diff_helper.rb | 32 +++++++++---------- .../git/commits/_commit_diff.html.haml | 3 +- .../_discussion_comments.html.haml | 2 +- .../pull_requests/_pull_diff.html.haml | 2 +- app/views/projects/wiki/_diff_data.html.haml | 3 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index fac745b78..7f7bca252 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -21,15 +21,12 @@ module DiffHelper end #include Git::Diff::InlineCallback - def render_diff(diff, diff_counter, comments, diffpath = nil) + def render_diff(diff, args = {})#diff_counter, comments, opts = nil diffpath = nil) if diff.respond_to?(:diff) - filepath = diff.a_path - diff = diff.diff - in_discussion = false - comments = comments.select{|c| c.data.try('[]', :path) == filepath} + diff, filepath, in_discussion = diff.diff, diff.a_path, false + comments = (args[:comments] || []).select{|c| c.data.try('[]', :path) == filepath} else - filepath = diffpath - in_discussion = true + filepath, in_discussion, comments = args[:diffpath], true, args[:comments] end diff_display ||= Diff::Display::Unified.new(diff) @@ -38,7 +35,7 @@ module DiffHelper elsif @commit commit_path @project, @commit end - prepare(url, diff_counter, filepath, comments, in_discussion) + prepare(args.merge({:filepath => filepath, :comments => comments, :in_discussion => in_discussion})) res = "" res += "" @@ -52,14 +49,14 @@ module DiffHelper ######################################################## # FIXME: Just to dev, remove to lib. Really need it? ######################################################## - def prepare(url, diff_counter, filepath, comments, in_discussion) - @url, @diff_counter, @in_discussion = url, diff_counter, in_discussion - @filepath, @line_comments = filepath, comments + def prepare(args) + @url, @diff_counter, @in_discussion = args[:url], args[:diff_counter], args[:in_discussion] + @filepath, @line_comments, @in_wiki = args[:filepath], args[:comments], args[:in_wiki] @add_reply_id, @num_line = if @in_discussion - [@line_comments[0].id, @line_comments[0].data[:line].to_i - @line_comments[0].data[:strings].lines.count.to_i-1] - else - [nil, -1] - end + [@line_comments[0].id, @line_comments[0].data[:line].to_i - @line_comments[0].data[:strings].lines.count.to_i-1] + else + [nil, -1] + end end def headerline(line) @@ -228,12 +225,12 @@ module DiffHelper end def line_comment - return if @in_discussion && @add_reply_id && @line_comments[0].data[:line].to_i != @num_line + return if @in_wiki || (@in_discussion && @add_reply_id && @line_comments[0].data[:line].to_i != @num_line) link_to image_tag('line_comment.png', :alt => t('layout.comments.new_header')), new_comment_path, :class => 'add_line-comment' end def render_line_comments - unless @in_discussion + unless @in_wiki || @in_discussion comments = @line_comments.select do |c| c.data.try('[]', :line) == @num_line.to_s && c.actual_inline_comment?(@diff) end @@ -246,6 +243,7 @@ module DiffHelper end def tr_line_comments comments + return if @in_wiki res=""+""; par.after(str); + par = par.next(); } else { par.find('td:last').append(data); } - $("a[href='"+line.attr('href')+"']").addClass('new_comment_exists'); - par.parent().find('#md_tabs.nav.nav-tabs').each(function(i) { $(this).find('a:first').tab('show') }); + par.find('#md_tabs.nav.nav-tabs').each(function(i) { + $(this).find('a:first').tab('show'); + $(this).parent().find('#new_line_edit_input').focus(); + }); } var line = $(this); var tmp = line.parents('tr'); @@ -70,40 +73,27 @@ $(document).ready(function() { // Hide visible new comment form $('#open-comment.new_line_comment').parents('.inline-comments').each(function(i) { if($(this).find('.line-comments').length > 0) { - $(this).find('#open-comment.new_line_comment').hide(); + $(this).find('#open-comment.new_line_comment').remove(); $(this).find('.new_inline_comment.button').show(); } else { - $(this).hide(); + $(this).remove(); } }); par.find('.new_inline_comment.button').hide(); - // Show existing new comment form or load them. - if(line.hasClass('new_comment_exists')) { - var tr = line.parents('tr').next(); - if(tr.find('.line-comments').length > 0) { - tr.find('#open-comment.new_line_comment').show(); - } - else { - tr.show(); - } - } - else { - $.get(line.attr('href'), null, ProcessData); - } - $('#new_line_edit_input').focus(); + $.get(line.attr('href'), null, ProcessData); return false; }); $(document).on('click', '.cancel_inline_comment.button', function() { var tr = $(this).parents('.inline-comments'); if(tr.find('.line-comments').length > 0) { - tr.find('#open-comment.new_line_comment').hide(); + tr.find('#open-comment.new_line_comment').remove(); tr.find('.new_inline_comment.button').show(); } else { - tr.hide(); + tr.remove(); } return false; });
#{comments.count} " diff --git a/app/views/projects/git/commits/_commit_diff.html.haml b/app/views/projects/git/commits/_commit_diff.html.haml index f745bfa47..c02a04395 100644 --- a/app/views/projects/git/commits/_commit_diff.html.haml +++ b/app/views/projects/git/commits/_commit_diff.html.haml @@ -6,4 +6,5 @@ - if commit_diff.b_path.present? .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, commit_diff.b_path) .clear - .diff_data= render_diff(commit_diff, commit_diff_counter, @comments) unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? + -unless (@project.repo.tree(commit_id) / commit_diff.b_path).binary? + .diff_data=render_diff(commit_diff, :diff_counter => commit_diff_counter, :comments => @comments) diff --git a/app/views/projects/pull_requests/_discussion_comments.html.haml b/app/views/projects/pull_requests/_discussion_comments.html.haml index 96713f014..a1d4ecf42 100644 --- a/app/views/projects/pull_requests/_discussion_comments.html.haml +++ b/app/views/projects/pull_requests/_discussion_comments.html.haml @@ -11,5 +11,5 @@ =t'projects.pull_requests.outdated_diff' =image_tag 'x.png' .clear - .diff_data=render_diff(comment.inline_diff, 0, item[1], comment.data[:path]) + .diff_data=render_diff(comment.inline_diff, :diff_counter => 0, :comments => item[1], :filepath => comment.data[:path]) diff --git a/app/views/projects/pull_requests/_pull_diff.html.haml b/app/views/projects/pull_requests/_pull_diff.html.haml index e4b694993..60fbe55ff 100644 --- a/app/views/projects/pull_requests/_pull_diff.html.haml +++ b/app/views/projects/pull_requests/_pull_diff.html.haml @@ -7,4 +7,4 @@ .r= link_to "view file @ #{short_hash_id(commit_id)}", blob_path(@project, commit_id, pull_diff.b_path) .clear -if pull_diff.diff.present? && !(@pull.repo.tree(commit_id) / pull_diff.b_path).binary? - .diff_data=render_diff(pull_diff, pull_diff_counter, @comments) + .diff_data=render_diff(pull_diff, :diff_counter => pull_diff_counter, :comments => @comments) diff --git a/app/views/projects/wiki/_diff_data.html.haml b/app/views/projects/wiki/_diff_data.html.haml index a215c7995..952b6ea45 100644 --- a/app/views/projects/wiki/_diff_data.html.haml +++ b/app/views/projects/wiki/_diff_data.html.haml @@ -3,4 +3,5 @@ .top .l= h((diff.deleted_file ? diff.a_path : diff.b_path).rtruncate 100) .clear - .diff_data.highlight= render_diff(diff, diff_counter, []) + .diff_data.highlight= render_diff(diff, :diff_counter => diff_counter, :in_wiki => true) + From ea1ec80d297b71a6c940498ceb3963111c9ebbb8 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Thu, 18 Oct 2012 23:37:52 +0600 Subject: [PATCH 44/46] [refs #579] fixed bug with updating comments --- app/models/pull_request.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb index dcee28272..d43e3f325 100644 --- a/app/models/pull_request.rb +++ b/app/models/pull_request.rb @@ -120,6 +120,7 @@ class PullRequest < ActiveRecord::Base # FIXME maybe move to warpc/grit? def diff + return @diff if @diff.present? diff = repo.git.native('diff', {:M => true}, "#{to_commit.id}...#{from_commit.id}") if diff =~ /diff --git a/ @@ -127,7 +128,7 @@ class PullRequest < ActiveRecord::Base else diff = '' end - Grit::Diff.list_from_string(repo, diff) + @diff = Grit::Diff.list_from_string(repo, diff) end def set_user_and_time user @@ -211,9 +212,6 @@ class PullRequest < ActiveRecord::Base end def update_inline_comments - if self.comments.count > 0 - diff = self.diff self.repo, self.common_ancestor, repo.commits(self.from_branch).first - end self.comments.each do |c| if c.data.present? # maybe need add new column 'actual'? c.actual_inline_comment? diff, true From 54f462f408911cec85421c523853a08f3b0e1457 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 19 Oct 2012 15:50:15 +0600 Subject: [PATCH 45/46] [refs #579] small refactoring --- app/helpers/comments_helper.rb | 4 ++-- app/helpers/diff_helper.rb | 4 ++-- app/models/comment.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 204348df6..fbc6fb905 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -17,9 +17,9 @@ module CommentsHelper end def project_commentable_comments_path(project, commentable) - if commentable.class == Issue + if commentable.is_a? Issue project_issue_comments_path(@project, @commentable) - elsif commentable.class == Grit::Commit + elsif commentable.is_a? Grit::Commit project_commit_comments_path(@project, @commentable) end end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 7f7bca252..d9f41984f 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -258,9 +258,9 @@ module DiffHelper def new_comment_path hash = {:path => @filepath, :line => @num_line} - if @commentable.class == Issue + if @commentable.is_a? Issue project_new_line_pull_comment_path(@project, @commentable, hash.merge({:in_reply => @add_reply_id})) - elsif @commentable.class == Grit::Commit + elsif @commentable.is_a? Grit::Commit new_line_commit_comment_path(@project, @commentable, hash) end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 1f8884273..a0358a70a 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -84,7 +84,7 @@ class Comment < ActiveRecord::Base end def pull_comment? - return true if commentable.class == Issue && commentable.pull_request.present? + return true if commentable.is_a?(Issue) && commentable.pull_request.present? end def set_additional_data params @@ -94,7 +94,7 @@ class Comment < ActiveRecord::Base return true end self.data = {:path => params[:path], :line => params[:line]} - if commentable.class == Issue && pull = commentable.pull_request + if commentable.is_a?(Issue) && pull = commentable.pull_request diff_path = pull.diff.select {|d| d.a_path == params[:path]} return false unless actual_inline_comment?(pull.diff, true) From 9bc9eab4f161acccf36f5521b228a040c16d691f Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Fri, 19 Oct 2012 18:39:11 +0600 Subject: [PATCH 46/46] [refs #579] remove new comment form after close & fix focus --- app/assets/javascripts/extra/comment.js | 30 +++++++++---------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/extra/comment.js b/app/assets/javascripts/extra/comment.js index eb5c59386..da683f382 100644 --- a/app/assets/javascripts/extra/comment.js +++ b/app/assets/javascripts/extra/comment.js @@ -43,12 +43,15 @@ $(document).ready(function() { if(yield) { var str = "
"+data+"