From 2def1c7217556388763dfb860f76e16f9af58571 Mon Sep 17 00:00:00 2001 From: randx Date: Fri, 29 Jun 2012 21:55:22 +0300 Subject: [PATCH] Merge Request -> show. Refactored. f5 support for diff --- Gemfile | 1 + Gemfile.lock | 2 + app/assets/javascripts/merge_requests.js | 55 ++++++- .../stylesheets/sections/merge_requests.scss | 10 ++ app/controllers/merge_requests_controller.rb | 35 +++-- app/views/commits/_diffs.html.haml | 3 +- app/views/merge_requests/_show.html.haml | 39 +++++ app/views/merge_requests/diffs.html.haml | 2 + app/views/merge_requests/diffs.js.haml | 2 +- app/views/merge_requests/show.html.haml | 145 +----------------- app/views/merge_requests/show.js.haml | 2 + .../{ => show}/_commits.html.haml | 0 .../{ => show}/_diffs.html.haml | 0 .../{ => show}/_how_to_merge.html.haml | 0 .../merge_requests/show/_mr_accept.html.haml | 42 +++++ .../merge_requests/show/_mr_box.html.haml | 31 ++++ .../merge_requests/show/_mr_title.html.haml | 28 ++++ db/schema.rb | 4 +- 18 files changed, 231 insertions(+), 170 deletions(-) create mode 100644 app/views/merge_requests/_show.html.haml create mode 100644 app/views/merge_requests/diffs.html.haml create mode 100644 app/views/merge_requests/show.js.haml rename app/views/merge_requests/{ => show}/_commits.html.haml (100%) rename app/views/merge_requests/{ => show}/_diffs.html.haml (100%) rename app/views/merge_requests/{ => show}/_how_to_merge.html.haml (100%) create mode 100644 app/views/merge_requests/show/_mr_accept.html.haml create mode 100644 app/views/merge_requests/show/_mr_box.html.haml create mode 100644 app/views/merge_requests/show/_mr_title.html.haml diff --git a/Gemfile b/Gemfile index 530f06b1..8bdc6426 100644 --- a/Gemfile +++ b/Gemfile @@ -39,6 +39,7 @@ gem "charlock_holmes" gem "foreman" gem "colored" gem 'resque_mailer' +gem 'tabs_on_rails' group :assets do gem "sass-rails", "3.2.5" diff --git a/Gemfile.lock b/Gemfile.lock index c3a45b59..897f640d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -331,6 +331,7 @@ GEM tilt (~> 1.1, != 1.3.0) sqlite3 (1.3.6) stamp (0.1.6) + tabs_on_rails (2.1.1) therubyracer (0.10.1) libv8 (~> 3.3.10) thin (1.3.1) @@ -420,6 +421,7 @@ DEPENDENCIES six sqlite3 stamp + tabs_on_rails therubyracer thin turn diff --git a/app/assets/javascripts/merge_requests.js b/app/assets/javascripts/merge_requests.js index 8df090cf..50092ae7 100644 --- a/app/assets/javascripts/merge_requests.js +++ b/app/assets/javascripts/merge_requests.js @@ -8,36 +8,77 @@ var MergeRequest = { var self = this; self.opts = opts; + self.initTabs(); + self.initMergeWidget(); + + $(".mr_show_all_commits").bind("click", function() { + self.showAllCommits(); + }); + + $(".line_note_link, .line_note_reply_link").live("click", function(e) { + var form = $(".per_line_form"); + $(this).parent().parent().after(form); + form.find("#note_line_code").val($(this).attr("line_code")); + form.show(); + return false; + }); + }, + + initMergeWidget: + function() { + var self = this; self.showState(self.opts.current_state); + if($(".automerge_widget").length && self.opts.check_enable){ $.get(opts.url_to_automerge_check, function(data){ self.showState(data.state); }, "json"); } + }, - $(".nav-tabs a").live("click", function() { - $(".nav-tabs a").parent().removeClass("active"); + initTabs: + function() { + $(".mr_nav_tabs a").live("click", function() { + $(".mr_nav_tabs a").parent().removeClass("active"); $(this).parent().addClass("active"); }); - $(".nav-tabs a.merge-notes-tab").live("click", function(e) { + var current_tab; + if(this.opts.action == "diffs") { + current_tab = $(".mr_nav_tabs .merge-diffs-tab"); + } else { + current_tab = $(".mr_nav_tabs .merge-notes-tab"); + } + current_tab.parent().addClass("active"); + + this.initNotesTab(); + this.initDiffTab(); + }, + + initNotesTab: + function() { + $(".mr_nav_tabs a.merge-notes-tab").live("click", function(e) { $(".merge-request-diffs").hide(); $(".merge_request_notes").show(); + var mr_path = $(".merge-notes-tab").attr("data-url"); + history.pushState({ path: mr_path }, '', mr_path); e.preventDefault(); }); + }, - $(".nav-tabs a.merge-diffs-tab").live("click", function(e) { + initDiffTab: + function() { + $(".mr_nav_tabs a.merge-diffs-tab").live("click", function(e) { if(!MergeRequest.diffs_loaded) { MergeRequest.loadDiff(); } $(".merge_request_notes").hide(); $(".merge-request-diffs").show(); + var mr_diff_path = $(".merge-diffs-tab").attr("data-url"); + history.pushState({ path: mr_diff_path }, '', mr_diff_path); e.preventDefault(); }); - $(".mr_show_all_commits").bind("click", function() { - MergeRequest.showAllCommits(); - }) }, showState: diff --git a/app/assets/stylesheets/sections/merge_requests.scss b/app/assets/stylesheets/sections/merge_requests.scss index de1ecf72..ad496238 100644 --- a/app/assets/stylesheets/sections/merge_requests.scss +++ b/app/assets/stylesheets/sections/merge_requests.scss @@ -72,3 +72,13 @@ @extend .primary; } } + +.mr_nav_tabs { + li { + a { + font-weight:bold; + padding:8px 20px; + text-align:center; + } + } +} diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b8643dce..bbb9dc33 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -3,6 +3,8 @@ class MergeRequestsController < ApplicationController before_filter :project before_filter :module_enabled before_filter :merge_request, :only => [:edit, :update, :destroy, :show, :commits, :diffs, :automerge, :automerge_check] + before_filter :validates_merge_request, :only => [:show, :diffs] + before_filter :define_show_vars, :only => [:show, :diffs] layout "project" # Authorize @@ -20,6 +22,7 @@ class MergeRequestsController < ApplicationController # Allow destroy merge_request before_filter :authorize_admin_merge_request!, :only => [:destroy] + def index @merge_requests = @project.merge_requests @@ -34,20 +37,6 @@ class MergeRequestsController < ApplicationController end def show - # Show git not found page if target branch doesnt exist - return git_not_found! unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) - - # Show git not found page if source branch doesnt exist - # and there is no saved commits between source & target branch - return git_not_found! if !@project.repo.heads.map(&:name).include?(@merge_request.source_branch) && @merge_request.commits.blank? - - # Build a note object for comment form - @note = @project.notes.new(:noteable => @merge_request) - - # Get commits from repository - # or from cache if already merged - @commits = @merge_request.commits - respond_to do |format| format.html format.js @@ -142,4 +131,22 @@ class MergeRequestsController < ApplicationController def module_enabled return render_404 unless @project.merge_requests_enabled end + + def validates_merge_request + # Show git not found page if target branch doesnt exist + return git_not_found! unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) + + # Show git not found page if source branch doesnt exist + # and there is no saved commits between source & target branch + return git_not_found! if !@project.repo.heads.map(&:name).include?(@merge_request.source_branch) && @merge_request.commits.blank? + end + + def define_show_vars + # Build a note object for comment form + @note = @project.notes.new(:noteable => @merge_request) + + # Get commits from repository + # or from cache if already merged + @commits = @merge_request.commits + end end diff --git a/app/views/commits/_diffs.html.haml b/app/views/commits/_diffs.html.haml index b28a435f..d16ad069 100644 --- a/app/views/commits/_diffs.html.haml +++ b/app/views/commits/_diffs.html.haml @@ -35,5 +35,4 @@ .diff_file_content_image{:class => image_diff_class(diff)} %img{:src => "data:#{file.mime_type};base64,#{Base64.encode64(file.data)}"} - else - %p - %center No preview for this file type + %p.nothing_here_message No preview for this file type diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml new file mode 100644 index 00000000..c0b3dd9f --- /dev/null +++ b/app/views/merge_requests/_show.html.haml @@ -0,0 +1,39 @@ += render "merge_requests/show/mr_title" += render "merge_requests/show/how_to_merge" += render "merge_requests/show/mr_box" += render "merge_requests/show/mr_accept" += render "merge_requests/show/commits" + +- if @commits.present? + %ul.nav.nav-tabs.mr_nav_tabs + %li + = link_to "#notes", "data-url" => project_merge_request_path(@project, @merge_request), :class => "merge-notes-tab tab" do + %i.icon-comment + Comments + %li + = link_to "#diffs", "data-url" => diffs_project_merge_request_path(@project, @merge_request), :class => "merge-diffs-tab tab" do + %i.icon-list-alt + Diff + +.merge_request_notes#notes{ :class => (controller.action_name == 'show') ? "" : "hide" } + = render("notes/notes", :tid => @merge_request.id, :tt => "merge_request") +.merge-request-diffs + = render "merge_requests/show/diffs" if @diffs +.status + += render "notes/per_line_form" + +:javascript + $(function(){ + MergeRequest.init({ + url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", + check_enable: #{@merge_request.state == MergeRequest::UNCHECKED ? "true" : "false"}, + current_state: "#{@merge_request.human_state}", + action: "#{controller.action_name}" + }); + + $(".edit_merge_request").live("ajax:beforeSend", function() { + $(this).replaceWith('#{image_tag "ajax_loader.gif"}'); + }) + }) + diff --git a/app/views/merge_requests/diffs.html.haml b/app/views/merge_requests/diffs.html.haml new file mode 100644 index 00000000..176b19bc --- /dev/null +++ b/app/views/merge_requests/diffs.html.haml @@ -0,0 +1,2 @@ += render "show" + diff --git a/app/views/merge_requests/diffs.js.haml b/app/views/merge_requests/diffs.js.haml index 68f0c84c..4be5ec13 100644 --- a/app/views/merge_requests/diffs.js.haml +++ b/app/views/merge_requests/diffs.js.haml @@ -1,4 +1,4 @@ :plain - $(".merge-request-diffs").html("#{escape_javascript(render(:partial => "diffs"))}"); + $(".merge-request-diffs").html("#{escape_javascript(render(:partial => "merge_requests/show/diffs"))}"); diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 6f345668..2a5b8b14 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -1,144 +1 @@ -%h3 - = "Merge Request ##{@merge_request.id}:" -   - %span.pretty_label.branch= @merge_request.source_branch - → - %span.pretty_label.branch= @merge_request.target_branch - - %span.right - - if @merge_request.merged? - %span.btn.small.disabled.padded - %strong - %i.icon-ok - = "MERGED" - - if can?(current_user, :modify_merge_request, @merge_request) - - if @merge_request.open? - = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn small padded danger", :title => "Close merge request" - = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn small padded" do - %i.icon-edit - Edit - - %br - - if @merge_request.upvotes > 0 - .upvotes#upvotes= "+#{pluralize @merge_request.upvotes, 'upvote'}" - -= render "merge_requests/how_to_merge" -.back_link - = link_to project_merge_requests_path(@project) do - ← To merge requests - -.main_box - .top_box_content - %h4 - - if @merge_request.closed - .alert-message.error.status_info Closed - - else - .alert-message.success.status_info Open - = @merge_request.title - - .middle_box_content - %div - %cite.cgray Created at #{@merge_request.created_at.stamp("Aug 21, 2011")} by - = image_tag gravatar_icon(@merge_request.author_email), :width => 16, :class => "lil_av" - %strong.author= link_to_merge_request_author(@merge_request) - - %cite.cgray and currently assigned to - = image_tag gravatar_icon(@merge_request.assignee_email), :width => 16, :class => "lil_av" - %strong.author= link_to_merge_request_assignee(@merge_request) - - - - if @merge_request.closed - .bottom_box_content - - if @merge_request.merged? - %span - Merged by #{@merge_request.merge_event.author_name} - %small #{time_ago_in_words(@merge_request.merge_event.created_at)} ago. - - elsif @merge_request.closed_event - %span - Closed by #{@merge_request.closed_event.author_name} - %small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago. - -- unless can?(current_user, :accept_mr, @project) - .alert-message - %strong Only masters can accept MR - - -- if @merge_request.open? && @commits.any? && can?(current_user, :accept_mr, @project) - .automerge_widget.can_be_merged{:style => "display:none"} - .alert.alert-success - %span - = form_for [:automerge, @project, @merge_request], :remote => true, :method => :get do |f| - %p - You can accept this request automatically. - If you still want to do it manually - - %strong= link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge" - for instructions - .accept_group - = f.submit "Accept Merge Request", :class => "btn small success accept_merge_request" - - unless @project.root_ref? @merge_request.source_branch - .remove_branch_holder - = label_tag :should_remove_source_branch, :class => "checkbox" do - = check_box_tag :should_remove_source_branch - Remove source-branch - .clearfix - - - .automerge_widget.cannot_be_merged{:style => "display:none"} - .alert.alert-info - %span - = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded", :title => "How To Merge" -   - %strong This request cant be merged with GitLab. You should do it manually - - .automerge_widget.unchecked - .alert-message - %strong - %i.icon-refresh - Checking for ability to automatically merge… - - .automerge_widget.already_cannot_be_merged{:style => "display:none"} - .alert.alert-info - %strong This merge request already can not be merged. Try to reload page. - -= render "merge_requests/commits" - -- unless @commits.empty? - .nav.nav-tabs - %li.active - = link_to "#notes", :class => "merge-notes-tab tab" do - Notes - %li - = link_to "#diffs", "data-url" => diffs_project_merge_request_path(@project, @merge_request), :class => "merge-diffs-tab tab" do - Diff - - -.merge_request_notes#notes= render "notes/notes", :tid => @merge_request.id, :tt => "merge_request" - -.merge-request-diffs -.status - -:javascript - $(function(){ - MergeRequest.init({ - url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", - check_enable: #{@merge_request.state == MergeRequest::UNCHECKED ? "true" : "false"}, - current_state: "#{@merge_request.human_state}" - }); - - $(".edit_merge_request").live("ajax:beforeSend", function() { - $(this).replaceWith('#{image_tag "ajax_loader.gif"}'); - }) - }) - -= render "notes/per_line_form" - -:javascript - $(document).ready(function(){ - $(".line_note_link, .line_note_reply_link").live("click", function(e) { - var form = $(".per_line_form"); - $(this).parent().parent().after(form); - form.find("#note_line_code").val($(this).attr("line_code")); - form.show(); - return false; - }); - }); += render "show" diff --git a/app/views/merge_requests/show.js.haml b/app/views/merge_requests/show.js.haml new file mode 100644 index 00000000..2922b3bc --- /dev/null +++ b/app/views/merge_requests/show.js.haml @@ -0,0 +1,2 @@ +:plain + $(".merge-request-notes").html("#{escape_javascript(render("notes/notes", :tid => @merge_request.id, :tt => "merge_request"))}"); diff --git a/app/views/merge_requests/_commits.html.haml b/app/views/merge_requests/show/_commits.html.haml similarity index 100% rename from app/views/merge_requests/_commits.html.haml rename to app/views/merge_requests/show/_commits.html.haml diff --git a/app/views/merge_requests/_diffs.html.haml b/app/views/merge_requests/show/_diffs.html.haml similarity index 100% rename from app/views/merge_requests/_diffs.html.haml rename to app/views/merge_requests/show/_diffs.html.haml diff --git a/app/views/merge_requests/_how_to_merge.html.haml b/app/views/merge_requests/show/_how_to_merge.html.haml similarity index 100% rename from app/views/merge_requests/_how_to_merge.html.haml rename to app/views/merge_requests/show/_how_to_merge.html.haml diff --git a/app/views/merge_requests/show/_mr_accept.html.haml b/app/views/merge_requests/show/_mr_accept.html.haml new file mode 100644 index 00000000..c158ef88 --- /dev/null +++ b/app/views/merge_requests/show/_mr_accept.html.haml @@ -0,0 +1,42 @@ +- unless can?(current_user, :accept_mr, @project) + .alert-message + %strong Only masters can accept MR + + +- if @merge_request.open? && @commits.any? && can?(current_user, :accept_mr, @project) + .automerge_widget.can_be_merged{:style => "display:none"} + .alert.alert-success + %span + = form_for [:automerge, @project, @merge_request], :remote => true, :method => :get do |f| + %p + You can accept this request automatically. + If you still want to do it manually - + %strong= link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge" + for instructions + .accept_group + = f.submit "Accept Merge Request", :class => "btn small success accept_merge_request" + - unless @project.root_ref? @merge_request.source_branch + .remove_branch_holder + = label_tag :should_remove_source_branch, :class => "checkbox" do + = check_box_tag :should_remove_source_branch + Remove source-branch + .clearfix + + + .automerge_widget.cannot_be_merged{:style => "display:none"} + .alert.alert-info + %span + = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded", :title => "How To Merge" +   + %strong This request cant be merged with GitLab. You should do it manually + + .automerge_widget.unchecked + .alert-message + %strong + %i.icon-refresh + Checking for ability to automatically merge… + + .automerge_widget.already_cannot_be_merged{:style => "display:none"} + .alert.alert-info + %strong This merge request already can not be merged. Try to reload page. + diff --git a/app/views/merge_requests/show/_mr_box.html.haml b/app/views/merge_requests/show/_mr_box.html.haml new file mode 100644 index 00000000..3027719d --- /dev/null +++ b/app/views/merge_requests/show/_mr_box.html.haml @@ -0,0 +1,31 @@ +.main_box + .top_box_content + %h4 + - if @merge_request.closed + .alert-message.error.status_info Closed + - else + .alert-message.success.status_info Open + = @merge_request.title + + .middle_box_content + %div + %cite.cgray Created at #{@merge_request.created_at.stamp("Aug 21, 2011")} by + = image_tag gravatar_icon(@merge_request.author_email), :width => 16, :class => "lil_av" + %strong.author= link_to_merge_request_author(@merge_request) + + %cite.cgray and currently assigned to + = image_tag gravatar_icon(@merge_request.assignee_email), :width => 16, :class => "lil_av" + %strong.author= link_to_merge_request_assignee(@merge_request) + + + - if @merge_request.closed + .bottom_box_content + - if @merge_request.merged? + %span + Merged by #{@merge_request.merge_event.author_name} + %small #{time_ago_in_words(@merge_request.merge_event.created_at)} ago. + - elsif @merge_request.closed_event + %span + Closed by #{@merge_request.closed_event.author_name} + %small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago. + diff --git a/app/views/merge_requests/show/_mr_title.html.haml b/app/views/merge_requests/show/_mr_title.html.haml new file mode 100644 index 00000000..837bbd9f --- /dev/null +++ b/app/views/merge_requests/show/_mr_title.html.haml @@ -0,0 +1,28 @@ +%h3 + = "Merge Request ##{@merge_request.id}:" +   + %span.pretty_label.branch= @merge_request.source_branch + → + %span.pretty_label.branch= @merge_request.target_branch + + %span.right + - if @merge_request.merged? + %span.btn.small.disabled.padded + %strong + %i.icon-ok + = "MERGED" + - if can?(current_user, :modify_merge_request, @merge_request) + - if @merge_request.open? + = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn small padded danger", :title => "Close merge request" + = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn small padded" do + %i.icon-edit + Edit + + %br + - if @merge_request.upvotes > 0 + .upvotes#upvotes= "+#{pluralize @merge_request.upvotes, 'upvote'}" + + +.back_link + = link_to project_merge_requests_path(@project) do + ← To merge requests diff --git a/db/schema.rb b/db/schema.rb index 7ce89c6b..f2bb1693 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -61,8 +61,8 @@ ActiveRecord::Schema.define(:version => 20120627145613) do t.boolean "closed", :default => false, :null => false t.datetime "created_at", :null => false t.datetime "updated_at", :null => false - t.text "st_commits", :limit => 4294967295 - t.text "st_diffs", :limit => 4294967295 + t.text "st_commits", :limit => 2147483647 + t.text "st_diffs", :limit => 2147483647 t.boolean "merged", :default => false, :null => false t.integer "state", :default => 1, :null => false end