Commit 2ec12c9bb81b28add0b1d7404822d8312ed08d77
1 parent
e2b39517
Exists in
master
and in
4 other branches
Impoved merge requests show page. Minor style improvements across project
Showing
13 changed files
with
101 additions
and
70 deletions
Show diff stats
app/assets/stylesheets/common.scss
| @@ -41,6 +41,10 @@ a:focus { | @@ -41,6 +41,10 @@ a:focus { | ||
| 41 | 41 | ||
| 42 | .label { | 42 | .label { |
| 43 | background-color: #474D57; | 43 | background-color: #474D57; |
| 44 | + | ||
| 45 | + &.pushed { | ||
| 46 | + background-color: $link_color; | ||
| 47 | + } | ||
| 44 | } | 48 | } |
| 45 | 49 | ||
| 46 | .pretty_label { | 50 | .pretty_label { |
| @@ -865,7 +869,7 @@ p.time { | @@ -865,7 +869,7 @@ p.time { | ||
| 865 | } | 869 | } |
| 866 | } | 870 | } |
| 867 | 871 | ||
| 868 | - padding: 10px 5px; | 872 | + padding: 15px 5px; |
| 869 | border-bottom: 1px solid #eee; | 873 | border-bottom: 1px solid #eee; |
| 870 | border-bottom: 1px solid rgba(0, 0, 0, 0.05); | 874 | border-bottom: 1px solid rgba(0, 0, 0, 0.05); |
| 871 | &:last-child { border:none } | 875 | &:last-child { border:none } |
| @@ -963,3 +967,17 @@ p.time { | @@ -963,3 +967,17 @@ p.time { | ||
| 963 | .highlight_word { | 967 | .highlight_word { |
| 964 | background:#EEDC94; | 968 | background:#EEDC94; |
| 965 | } | 969 | } |
| 970 | + | ||
| 971 | +.status_info { | ||
| 972 | + font-size:14px; | ||
| 973 | + padding:5px 15px; | ||
| 974 | + line-height:24px; | ||
| 975 | + width:60px; | ||
| 976 | + text-align:center; | ||
| 977 | + float:left; | ||
| 978 | + margin-right:20px; | ||
| 979 | +} | ||
| 980 | + | ||
| 981 | +.merge_request_status_holder { | ||
| 982 | + margin-bottom:20px; | ||
| 983 | +} |
app/controllers/merge_requests_controller.rb
| @@ -45,9 +45,6 @@ class MergeRequestsController < ApplicationController | @@ -45,9 +45,6 @@ class MergeRequestsController < ApplicationController | ||
| 45 | # or from cache if already merged | 45 | # or from cache if already merged |
| 46 | @commits = @merge_request.commits | 46 | @commits = @merge_request.commits |
| 47 | 47 | ||
| 48 | - # Close MR if nothing to merge | ||
| 49 | - #@merge_request.mark_as_merged! if @merge_request.probably_merged? | ||
| 50 | - | ||
| 51 | respond_to do |format| | 48 | respond_to do |format| |
| 52 | format.html | 49 | format.html |
| 53 | format.js | 50 | format.js |
| @@ -75,8 +72,7 @@ class MergeRequestsController < ApplicationController | @@ -75,8 +72,7 @@ class MergeRequestsController < ApplicationController | ||
| 75 | 72 | ||
| 76 | respond_to do |format| | 73 | respond_to do |format| |
| 77 | if @merge_request.save | 74 | if @merge_request.save |
| 78 | - @merge_request.reloaded_commits | ||
| 79 | - @merge_request.reloaded_diffs | 75 | + @merge_request.reload_code |
| 80 | format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully created.' } | 76 | format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully created.' } |
| 81 | format.json { render json: @merge_request, status: :created, location: @merge_request } | 77 | format.json { render json: @merge_request, status: :created, location: @merge_request } |
| 82 | else | 78 | else |
| @@ -89,6 +85,7 @@ class MergeRequestsController < ApplicationController | @@ -89,6 +85,7 @@ class MergeRequestsController < ApplicationController | ||
| 89 | def update | 85 | def update |
| 90 | respond_to do |format| | 86 | respond_to do |format| |
| 91 | if @merge_request.update_attributes(params[:merge_request].merge(:author_id_of_changes => current_user.id)) | 87 | if @merge_request.update_attributes(params[:merge_request].merge(:author_id_of_changes => current_user.id)) |
| 88 | + @merge_request.reload_code | ||
| 92 | format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully updated.' } | 89 | format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully updated.' } |
| 93 | format.json { head :ok } | 90 | format.json { head :ok } |
| 94 | else | 91 | else |
app/models/merge_request.rb
| @@ -41,12 +41,21 @@ class MergeRequest < ActiveRecord::Base | @@ -41,12 +41,21 @@ class MergeRequest < ActiveRecord::Base | ||
| 41 | where("title like :query", :query => "%#{query}%") | 41 | where("title like :query", :query => "%#{query}%") |
| 42 | end | 42 | end |
| 43 | 43 | ||
| 44 | + def self.find_all_by_branch(branch_name) | ||
| 45 | + where("source_branch like :branch or target_branch like :branch", :branch => branch_name) | ||
| 46 | + end | ||
| 47 | + | ||
| 44 | def validate_branches | 48 | def validate_branches |
| 45 | if target_branch == source_branch | 49 | if target_branch == source_branch |
| 46 | errors.add :base, "You can not use same branch for source and target branches" | 50 | errors.add :base, "You can not use same branch for source and target branches" |
| 47 | end | 51 | end |
| 48 | end | 52 | end |
| 49 | 53 | ||
| 54 | + def reload_code | ||
| 55 | + self.reloaded_commits | ||
| 56 | + self.reloaded_diffs | ||
| 57 | + end | ||
| 58 | + | ||
| 50 | def new? | 59 | def new? |
| 51 | today? && created_at == updated_at | 60 | today? && created_at == updated_at |
| 52 | end | 61 | end |
| @@ -72,10 +81,19 @@ class MergeRequest < ActiveRecord::Base | @@ -72,10 +81,19 @@ class MergeRequest < ActiveRecord::Base | ||
| 72 | commits.first | 81 | commits.first |
| 73 | end | 82 | end |
| 74 | 83 | ||
| 84 | + def merged? | ||
| 85 | + merged && merge_event | ||
| 86 | + end | ||
| 87 | + | ||
| 75 | def merge_event | 88 | def merge_event |
| 76 | self.project.events.where(:target_id => self.id, :target_type => "MergeRequest", :action => Event::Merged).last | 89 | self.project.events.where(:target_id => self.id, :target_type => "MergeRequest", :action => Event::Merged).last |
| 77 | end | 90 | end |
| 78 | 91 | ||
| 92 | + def closed_event | ||
| 93 | + self.project.events.where(:target_id => self.id, :target_type => "MergeRequest", :action => Event::Closed).last | ||
| 94 | + end | ||
| 95 | + | ||
| 96 | + | ||
| 79 | # Return the number of +1 comments (upvotes) | 97 | # Return the number of +1 comments (upvotes) |
| 80 | def upvotes | 98 | def upvotes |
| 81 | notes.select(&:upvote?).size | 99 | notes.select(&:upvote?).size |
| @@ -115,6 +133,17 @@ class MergeRequest < ActiveRecord::Base | @@ -115,6 +133,17 @@ class MergeRequest < ActiveRecord::Base | ||
| 115 | sort_by(&:created_at). | 133 | sort_by(&:created_at). |
| 116 | reverse | 134 | reverse |
| 117 | end | 135 | end |
| 136 | + | ||
| 137 | + def merge!(user_id) | ||
| 138 | + self.mark_as_merged! | ||
| 139 | + Event.create( | ||
| 140 | + :project => self.project, | ||
| 141 | + :action => Event::Merged, | ||
| 142 | + :target_id => self.id, | ||
| 143 | + :target_type => "MergeRequest", | ||
| 144 | + :author_id => user_id | ||
| 145 | + ) | ||
| 146 | + end | ||
| 118 | end | 147 | end |
| 119 | # == Schema Information | 148 | # == Schema Information |
| 120 | # | 149 | # |
app/models/project.rb
| @@ -80,35 +80,18 @@ class Project < ActiveRecord::Base | @@ -80,35 +80,18 @@ class Project < ActiveRecord::Base | ||
| 80 | def update_merge_requests(oldrev, newrev, ref, author_key_id) | 80 | def update_merge_requests(oldrev, newrev, ref, author_key_id) |
| 81 | return true unless ref =~ /heads/ | 81 | return true unless ref =~ /heads/ |
| 82 | branch_name = ref.gsub("refs/heads/", "") | 82 | branch_name = ref.gsub("refs/heads/", "") |
| 83 | - | ||
| 84 | - key = Key.find_by_identifier(author_key_id) | ||
| 85 | - user = key.user | ||
| 86 | - | 83 | + user = Key.find_by_identifier(author_key_id).user |
| 87 | c_ids = self.commits_between(oldrev, newrev).map(&:id) | 84 | c_ids = self.commits_between(oldrev, newrev).map(&:id) |
| 88 | 85 | ||
| 89 | - # update commits & diffs for existing MR | ||
| 90 | - mrs = self.merge_requests.opened.where(:source_branch => branch_name).all | ||
| 91 | - mrs.each do |merge_request| | ||
| 92 | - merge_request.reloaded_commits | ||
| 93 | - merge_request.reloaded_diffs | ||
| 94 | - end | 86 | + # Update code for merge requests |
| 87 | + mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all | ||
| 88 | + mrs.each { |merge_request| merge_request.reload_code } | ||
| 95 | 89 | ||
| 96 | # Close merge requests | 90 | # Close merge requests |
| 97 | mrs = self.merge_requests.opened.where(:target_branch => branch_name).all | 91 | mrs = self.merge_requests.opened.where(:target_branch => branch_name).all |
| 98 | - mrs.each do |merge_request| | ||
| 99 | - next unless merge_request.last_commit | ||
| 100 | - # Mark as merged & create event if merged | ||
| 101 | - if c_ids.include?(merge_request.last_commit.id) | ||
| 102 | - merge_request.mark_as_merged! | ||
| 103 | - Event.create( | ||
| 104 | - :project => self, | ||
| 105 | - :action => Event::Merged, | ||
| 106 | - :target_id => merge_request.id, | ||
| 107 | - :target_type => "MergeRequest", | ||
| 108 | - :author_id => user.id | ||
| 109 | - ) | ||
| 110 | - end | ||
| 111 | - end | 92 | + mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) } |
| 93 | + mrs.each { |merge_request| merge_request.merge!(user.id) } | ||
| 94 | + | ||
| 112 | true | 95 | true |
| 113 | end | 96 | end |
| 114 | 97 |
app/views/events/_event_changed_issue.html.haml
| 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 2 | %strong #{event.author_name} | 2 | %strong #{event.author_name} |
| 3 | -- if event.closed? | ||
| 4 | - closed | ||
| 5 | -- else | ||
| 6 | - reopened | ||
| 7 | -issue | 3 | +%span.label.important |
| 4 | + - if event.closed? | ||
| 5 | + closed | ||
| 6 | + - else | ||
| 7 | + reopened | ||
| 8 | + issue | ||
| 8 | = link_to project_issue_path(event.project, event.issue) do | 9 | = link_to project_issue_path(event.project, event.issue) do |
| 9 | %strong= truncate event.issue_title | 10 | %strong= truncate event.issue_title |
| 10 | at | 11 | at |
app/views/events/_event_changed_merge_request.html.haml
| 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 2 | %strong #{event.author_name} | 2 | %strong #{event.author_name} |
| 3 | -- if event.closed? | ||
| 4 | - closed | ||
| 5 | -- else | ||
| 6 | - reopened | ||
| 7 | -merge request | 3 | +%span.label.important |
| 4 | + - if event.closed? | ||
| 5 | + closed | ||
| 6 | + - else | ||
| 7 | + reopened | ||
| 8 | + merge request | ||
| 8 | = link_to project_merge_request_path(event.project, event.merge_request) do | 9 | = link_to project_merge_request_path(event.project, event.merge_request) do |
| 9 | %strong= truncate event.merge_request_title | 10 | %strong= truncate event.merge_request_title |
| 10 | at | 11 | at |
| @@ -12,7 +13,6 @@ at | @@ -12,7 +13,6 @@ at | ||
| 12 | %span.cgray | 13 | %span.cgray |
| 13 | = time_ago_in_words(event.created_at) | 14 | = time_ago_in_words(event.created_at) |
| 14 | ago. | 15 | ago. |
| 15 | -%br | ||
| 16 | %span.label= event.merge_request.source_branch | 16 | %span.label= event.merge_request.source_branch |
| 17 | → | 17 | → |
| 18 | %span.label= event.merge_request.target_branch | 18 | %span.label= event.merge_request.target_branch |
app/views/events/_event_new_issue.html.haml
| 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 2 | %strong #{event.author_name} | 2 | %strong #{event.author_name} |
| 3 | -created new issue | 3 | +%span.label.success created |
| 4 | + new issue | ||
| 4 | = link_to project_issue_path(event.project, event.issue) do | 5 | = link_to project_issue_path(event.project, event.issue) do |
| 5 | %strong= truncate event.issue_title | 6 | %strong= truncate event.issue_title |
| 6 | at | 7 | at |
app/views/events/_event_new_merge_request.html.haml
| 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 1 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 2 | %strong #{event.author_name} | 2 | %strong #{event.author_name} |
| 3 | -requested merge | 3 | +%span.label.success requested |
| 4 | + merge | ||
| 4 | = link_to project_merge_request_path(event.project, event.merge_request) do | 5 | = link_to project_merge_request_path(event.project, event.merge_request) do |
| 5 | %strong= truncate event.merge_request_title | 6 | %strong= truncate event.merge_request_title |
| 6 | at | 7 | at |
| @@ -8,7 +9,6 @@ at | @@ -8,7 +9,6 @@ at | ||
| 8 | %span.cgray | 9 | %span.cgray |
| 9 | = time_ago_in_words(event.created_at) | 10 | = time_ago_in_words(event.created_at) |
| 10 | ago. | 11 | ago. |
| 11 | -%br | ||
| 12 | %span.label= event.merge_request.source_branch | 12 | %span.label= event.merge_request.source_branch |
| 13 | → | 13 | → |
| 14 | %span.label= event.merge_request.target_branch | 14 | %span.label= event.merge_request.target_branch |
app/views/events/_event_push.html.haml
| 1 | - if event.new_branch? || event.new_tag? | 1 | - if event.new_branch? || event.new_tag? |
| 2 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 2 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 3 | %strong #{event.author_name} | 3 | %strong #{event.author_name} |
| 4 | - pushed new | 4 | + %span.label.pushed pushed |
| 5 | + new | ||
| 5 | - if event.new_tag? | 6 | - if event.new_tag? |
| 6 | tag | 7 | tag |
| 7 | = link_to project_commits_path(event.project, :ref => event.tag_name) do | 8 | = link_to project_commits_path(event.project, :ref => event.tag_name) do |
| @@ -18,7 +19,8 @@ | @@ -18,7 +19,8 @@ | ||
| 18 | - else | 19 | - else |
| 19 | = image_tag gravatar_icon(event.author_email), :class => "avatar" | 20 | = image_tag gravatar_icon(event.author_email), :class => "avatar" |
| 20 | %strong #{event.author_name} | 21 | %strong #{event.author_name} |
| 21 | - pushed to | 22 | + %span.label.pushed pushed |
| 23 | + to | ||
| 22 | = link_to project_commits_path(event.project, :ref => event.branch_name) do | 24 | = link_to project_commits_path(event.project, :ref => event.branch_name) do |
| 23 | %strong= event.branch_name | 25 | %strong= event.branch_name |
| 24 | at | 26 | at |
| @@ -30,10 +32,10 @@ | @@ -30,10 +32,10 @@ | ||
| 30 | = link_to compare_project_commits_path(event.project, :from => event.commits.first.prev_commit_id, :to => event.commits.last.id) do | 32 | = link_to compare_project_commits_path(event.project, :from => event.commits.first.prev_commit_id, :to => event.commits.last.id) do |
| 31 | Compare #{event.commits.first.commit.id[0..8]}...#{event.commits.last.id[0..8]} | 33 | Compare #{event.commits.first.commit.id[0..8]}...#{event.commits.last.id[0..8]} |
| 32 | - @project = event.project | 34 | - @project = event.project |
| 33 | - %ul.unstyled | ||
| 34 | - - if event.commits.size > 4 | ||
| 35 | - = render event.commits[0..2] | ||
| 36 | - %li ... and #{event.commits.size - 3} more commits | 35 | + %ul.unstyled.event_commits |
| 36 | + - if event.commits.size > 3 | ||
| 37 | + = render event.commits[0...2] | ||
| 38 | + %li ... and #{event.commits.size - 2} more commits | ||
| 37 | - else | 39 | - else |
| 38 | = render event.commits | 40 | = render event.commits |
| 39 | 41 |
app/views/issues/_show.html.haml
| @@ -15,7 +15,7 @@ | @@ -15,7 +15,7 @@ | ||
| 15 | - if issue.today? | 15 | - if issue.today? |
| 16 | %span.label.success today | 16 | %span.label.success today |
| 17 | - if issue.notes.any? | 17 | - if issue.notes.any? |
| 18 | - %span.label= pluralize issue.notes.count, 'note' | 18 | + %span.pretty_label= pluralize issue.notes.count, 'note' |
| 19 | - if issue.upvotes > 0 | 19 | - if issue.upvotes > 0 |
| 20 | %span.label.success= "+#{issue.upvotes}" | 20 | %span.label.success= "+#{issue.upvotes}" |
| 21 | 21 |
app/views/merge_requests/_how_to_merge.html.haml
| @@ -15,7 +15,7 @@ | @@ -15,7 +15,7 @@ | ||
| 15 | :javascript | 15 | :javascript |
| 16 | $(function(){ | 16 | $(function(){ |
| 17 | var modal = $('#modal_merge_info').modal({modal: true}); | 17 | var modal = $('#modal_merge_info').modal({modal: true}); |
| 18 | - $('.info_link').bind("click", function(){ | 18 | + $('.how_to_merge_link').bind("click", function(){ |
| 19 | modal.show(); | 19 | modal.show(); |
| 20 | }); | 20 | }); |
| 21 | $('.modal-header .close').bind("click", function(){ | 21 | $('.modal-header .close').bind("click", function(){ |
app/views/merge_requests/_merge_request.html.haml
| @@ -6,7 +6,7 @@ | @@ -6,7 +6,7 @@ | ||
| 6 | = time_ago_in_words(merge_request.created_at) | 6 | = time_ago_in_words(merge_request.created_at) |
| 7 | ago | 7 | ago |
| 8 | - if merge_request.notes.any? | 8 | - if merge_request.notes.any? |
| 9 | - %span.label= pluralize merge_request.notes.count, 'note' | 9 | + %span.pretty_label= pluralize merge_request.notes.count, 'note' |
| 10 | - if merge_request.upvotes > 0 | 10 | - if merge_request.upvotes > 0 |
| 11 | %span.label.success= "+#{merge_request.upvotes}" | 11 | %span.label.success= "+#{merge_request.upvotes}" |
| 12 | .right | 12 | .right |
app/views/merge_requests/show.html.haml
| @@ -26,23 +26,23 @@ | @@ -26,23 +26,23 @@ | ||
| 26 | 26 | ||
| 27 | 27 | ||
| 28 | %hr | 28 | %hr |
| 29 | -- if @merge_request.closed | ||
| 30 | - .alert-message.error Closed | ||
| 31 | - - if @merge_request.merged | ||
| 32 | - - event = @merge_request.merge_event | ||
| 33 | - %div | ||
| 34 | - %p | ||
| 35 | - %strong #{event.author_name} | ||
| 36 | - merged this request | ||
| 37 | - %span.cgray | ||
| 38 | - = time_ago_in_words(event.created_at) | ||
| 39 | - ago. | ||
| 40 | - %br | ||
| 41 | -- else | ||
| 42 | - .alert-message.success | ||
| 43 | - = link_to "#", :class => "info_link", :title => "How To Merge" do | ||
| 44 | - = image_tag "Info-UI.PNG" | ||
| 45 | - Open | 29 | +.merge_request_status_holder |
| 30 | + - if @merge_request.closed | ||
| 31 | + %h5 | ||
| 32 | + .alert-message.error.status_info Closed | ||
| 33 | + - if @merge_request.merged? | ||
| 34 | + %span | ||
| 35 | + Merged by #{@merge_request.merge_event.author_name} | ||
| 36 | + %small #{time_ago_in_words(@merge_request.merge_event.created_at)} ago. | ||
| 37 | + - elsif @merge_request.closed_event | ||
| 38 | + %span | ||
| 39 | + Closed by #{@merge_request.closed_event.author_name} | ||
| 40 | + %small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago. | ||
| 41 | + %br | ||
| 42 | + - else | ||
| 43 | + %h5 | ||
| 44 | + .alert-message.success.status_info Open | ||
| 45 | + = link_to "How to merge", "#", :class => "vlink how_to_merge_link", :title => "How To Merge" | ||
| 46 | 46 | ||
| 47 | = render "merge_requests/how_to_merge" | 47 | = render "merge_requests/how_to_merge" |
| 48 | 48 |