Commit e6e0ad4e5ea0e44c1400052fb17b3df3396782e6
Exists in
spb-stable
and in
3 other branches
Merge branch 'feature/reopen_mr' into 'master'
Feature: reopen closed merge request
Showing
11 changed files
with
39 additions
and
15 deletions
Show diff stats
app/assets/stylesheets/sections/merge_requests.scss
| @@ -91,6 +91,7 @@ | @@ -91,6 +91,7 @@ | ||
| 91 | } | 91 | } |
| 92 | 92 | ||
| 93 | // hide mr close link for inline diff comment form | 93 | // hide mr close link for inline diff comment form |
| 94 | -.diff-file .close-mr-link { | ||
| 95 | - display: none; | 94 | +.diff-file .close-mr-link, |
| 95 | +.diff-file .reopen-mr-link { | ||
| 96 | + display: none; | ||
| 96 | } | 97 | } |
app/controllers/projects/merge_requests_controller.rb
| @@ -135,7 +135,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | @@ -135,7 +135,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | ||
| 135 | def automerge | 135 | def automerge |
| 136 | return access_denied! unless allowed_to_merge? | 136 | return access_denied! unless allowed_to_merge? |
| 137 | 137 | ||
| 138 | - if @merge_request.opened? && @merge_request.can_be_merged? | 138 | + if @merge_request.open? && @merge_request.can_be_merged? |
| 139 | @merge_request.should_remove_source_branch = params[:should_remove_source_branch] | 139 | @merge_request.should_remove_source_branch = params[:should_remove_source_branch] |
| 140 | @merge_request.automerge!(current_user, params[:merge_commit_message]) | 140 | @merge_request.automerge!(current_user, params[:merge_commit_message]) |
| 141 | @status = true | 141 | @status = true |
| @@ -230,7 +230,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | @@ -230,7 +230,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController | ||
| 230 | 230 | ||
| 231 | @merge_request_diff = @merge_request.merge_request_diff | 231 | @merge_request_diff = @merge_request.merge_request_diff |
| 232 | @allowed_to_merge = allowed_to_merge? | 232 | @allowed_to_merge = allowed_to_merge? |
| 233 | - @show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge | 233 | + @show_merge_controls = @merge_request.open? && @commits.any? && @allowed_to_merge |
| 234 | end | 234 | end |
| 235 | 235 | ||
| 236 | def allowed_to_merge? | 236 | def allowed_to_merge? |
app/models/concerns/issuable.rb
| @@ -23,7 +23,8 @@ module Issuable | @@ -23,7 +23,8 @@ module Issuable | ||
| 23 | scope :assigned, -> { where("assignee_id IS NOT NULL") } | 23 | scope :assigned, -> { where("assignee_id IS NOT NULL") } |
| 24 | scope :unassigned, -> { where("assignee_id IS NULL") } | 24 | scope :unassigned, -> { where("assignee_id IS NULL") } |
| 25 | scope :of_projects, ->(ids) { where(project_id: ids) } | 25 | scope :of_projects, ->(ids) { where(project_id: ids) } |
| 26 | - | 26 | + scope :opened, -> { with_state(:opened, :reopened) } |
| 27 | + scope :closed, -> { with_state(:closed) } | ||
| 27 | 28 | ||
| 28 | delegate :name, | 29 | delegate :name, |
| 29 | :email, | 30 | :email, |
app/models/issue.rb
| @@ -28,8 +28,6 @@ class Issue < ActiveRecord::Base | @@ -28,8 +28,6 @@ class Issue < ActiveRecord::Base | ||
| 28 | 28 | ||
| 29 | scope :of_group, ->(group) { where(project_id: group.project_ids) } | 29 | scope :of_group, ->(group) { where(project_id: group.project_ids) } |
| 30 | scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } | 30 | scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } |
| 31 | - scope :opened, -> { with_state(:opened, :reopened) } | ||
| 32 | - scope :closed, -> { with_state(:closed) } | ||
| 33 | 31 | ||
| 34 | attr_accessible :title, :assignee_id, :position, :description, | 32 | attr_accessible :title, :assignee_id, :position, :description, |
| 35 | :milestone_id, :label_list, :author_id_of_changes, | 33 | :milestone_id, :label_list, :author_id_of_changes, |
| @@ -50,9 +48,7 @@ class Issue < ActiveRecord::Base | @@ -50,9 +48,7 @@ class Issue < ActiveRecord::Base | ||
| 50 | end | 48 | end |
| 51 | 49 | ||
| 52 | state :opened | 50 | state :opened |
| 53 | - | ||
| 54 | state :reopened | 51 | state :reopened |
| 55 | - | ||
| 56 | state :closed | 52 | state :closed |
| 57 | end | 53 | end |
| 58 | 54 |
app/models/merge_request.rb
| @@ -100,8 +100,6 @@ class MergeRequest < ActiveRecord::Base | @@ -100,8 +100,6 @@ class MergeRequest < ActiveRecord::Base | ||
| 100 | 100 | ||
| 101 | scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } | 101 | scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } |
| 102 | scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))", team_project_ids: team.project_ids, team_member_ids: team.member_ids) } | 102 | scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))", team_project_ids: team.project_ids, team_member_ids: team.member_ids) } |
| 103 | - scope :opened, -> { with_state(:opened) } | ||
| 104 | - scope :closed, -> { with_state(:closed) } | ||
| 105 | scope :merged, -> { with_state(:merged) } | 103 | scope :merged, -> { with_state(:merged) } |
| 106 | scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } | 104 | scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } |
| 107 | scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } | 105 | scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } |
| @@ -160,6 +158,10 @@ class MergeRequest < ActiveRecord::Base | @@ -160,6 +158,10 @@ class MergeRequest < ActiveRecord::Base | ||
| 160 | MergeRequests::AutoMergeService.new.execute(self, current_user, commit_message) | 158 | MergeRequests::AutoMergeService.new.execute(self, current_user, commit_message) |
| 161 | end | 159 | end |
| 162 | 160 | ||
| 161 | + def open? | ||
| 162 | + opened? || reopened? | ||
| 163 | + end | ||
| 164 | + | ||
| 163 | def mr_and_commit_notes | 165 | def mr_and_commit_notes |
| 164 | # Fetch comments only from last 100 commits | 166 | # Fetch comments only from last 100 commits |
| 165 | commits_for_notes_limit = 100 | 167 | commits_for_notes_limit = 100 |
app/observers/merge_request_observer.rb
| @@ -22,6 +22,8 @@ class MergeRequestObserver < ActivityObserver | @@ -22,6 +22,8 @@ class MergeRequestObserver < ActivityObserver | ||
| 22 | create_event(merge_request, Event::REOPENED) | 22 | create_event(merge_request, Event::REOPENED) |
| 23 | create_note(merge_request) | 23 | create_note(merge_request) |
| 24 | execute_hooks(merge_request) | 24 | execute_hooks(merge_request) |
| 25 | + merge_request.reload_code | ||
| 26 | + merge_request.mark_as_unchecked | ||
| 25 | end | 27 | end |
| 26 | 28 | ||
| 27 | def after_update(merge_request) | 29 | def after_update(merge_request) |
app/views/projects/merge_requests/_show.html.haml
| @@ -2,7 +2,7 @@ | @@ -2,7 +2,7 @@ | ||
| 2 | = render "projects/merge_requests/show/mr_title" | 2 | = render "projects/merge_requests/show/mr_title" |
| 3 | = render "projects/merge_requests/show/how_to_merge" | 3 | = render "projects/merge_requests/show/how_to_merge" |
| 4 | = render "projects/merge_requests/show/mr_box" | 4 | = render "projects/merge_requests/show/mr_box" |
| 5 | - - if @merge_request.opened? | 5 | + - if @merge_request.open? |
| 6 | - if @merge_request.source_branch_exists? && @merge_request.target_branch_exists? | 6 | - if @merge_request.source_branch_exists? && @merge_request.target_branch_exists? |
| 7 | = render "projects/merge_requests/show/mr_accept" | 7 | = render "projects/merge_requests/show/mr_accept" |
| 8 | - else | 8 | - else |
| @@ -26,7 +26,8 @@ | @@ -26,7 +26,8 @@ | ||
| 26 | - if can?(current_user, :modify_merge_request, @merge_request) | 26 | - if can?(current_user, :modify_merge_request, @merge_request) |
| 27 | - unless @merge_request.closed? || @merge_request.merged? | 27 | - unless @merge_request.closed? || @merge_request.merged? |
| 28 | = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link", title: "Close merge request" | 28 | = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link", title: "Close merge request" |
| 29 | - | 29 | + - if @merge_request.closed? |
| 30 | + = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link", title: "Close merge request" | ||
| 30 | 31 | ||
| 31 | .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } | 32 | .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } |
| 32 | = render "projects/notes/notes_with_form" | 33 | = render "projects/notes/notes_with_form" |
app/views/projects/merge_requests/show/_mr_box.html.haml
| @@ -36,7 +36,7 @@ | @@ -36,7 +36,7 @@ | ||
| 36 | %i.icon-ok | 36 | %i.icon-ok |
| 37 | Merged by #{link_to_member(@project, @merge_request.merge_event.author)} | 37 | Merged by #{link_to_member(@project, @merge_request.merge_event.author)} |
| 38 | #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}. | 38 | #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}. |
| 39 | -- if !@closes_issues.empty? && @merge_request.opened? | 39 | +- if !@closes_issues.empty? && @merge_request.open? |
| 40 | .alert.alert-info.alert-info | 40 | .alert.alert-info.alert-info |
| 41 | %span | 41 | %span |
| 42 | %i.icon-ok | 42 | %i.icon-ok |
app/views/projects/merge_requests/show/_mr_title.html.haml
| @@ -3,7 +3,7 @@ | @@ -3,7 +3,7 @@ | ||
| 3 | 3 | ||
| 4 | %span.pull-right | 4 | %span.pull-right |
| 5 | - if can?(current_user, :modify_merge_request, @merge_request) | 5 | - if can?(current_user, :modify_merge_request, @merge_request) |
| 6 | - - if @merge_request.opened? | 6 | + - if @merge_request.open? |
| 7 | .btn-group.pull-left | 7 | .btn-group.pull-left |
| 8 | %a.btn.btn-grouped.dropdown-toggle{ data: {toggle: :dropdown} } | 8 | %a.btn.btn-grouped.dropdown-toggle{ data: {toggle: :dropdown} } |
| 9 | %i.icon-download-alt | 9 | %i.icon-download-alt |
| @@ -18,6 +18,8 @@ | @@ -18,6 +18,8 @@ | ||
| 18 | = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-grouped", id:"edit_merge_request" do | 18 | = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-grouped", id:"edit_merge_request" do |
| 19 | %i.icon-edit | 19 | %i.icon-edit |
| 20 | Edit | 20 | Edit |
| 21 | + - if @merge_request.closed? | ||
| 22 | + = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link", title: "Close merge request" | ||
| 21 | 23 | ||
| 22 | .votes-holder.hidden-sm.hidden-xs | 24 | .votes-holder.hidden-sm.hidden-xs |
| 23 | #votes= render 'votes/votes_block', votable: @merge_request | 25 | #votes= render 'votes/votes_block', votable: @merge_request |
features/project/merge_requests.feature
| @@ -29,6 +29,13 @@ Feature: Project Merge Requests | @@ -29,6 +29,13 @@ Feature: Project Merge Requests | ||
| 29 | And I click link "Close" | 29 | And I click link "Close" |
| 30 | Then I should see closed merge request "Bug NS-04" | 30 | Then I should see closed merge request "Bug NS-04" |
| 31 | 31 | ||
| 32 | + Scenario: I reopen merge request page | ||
| 33 | + Given I click link "Bug NS-04" | ||
| 34 | + And I click link "Close" | ||
| 35 | + Then I should see closed merge request "Bug NS-04" | ||
| 36 | + When I click link "Reopen" | ||
| 37 | + Then I should see reopened merge request "Bug NS-04" | ||
| 38 | + | ||
| 32 | Scenario: I submit new unassigned merge request | 39 | Scenario: I submit new unassigned merge request |
| 33 | Given I click link "New Merge Request" | 40 | Given I click link "New Merge Request" |
| 34 | And I submit new merge request "Wiki Feature" | 41 | And I submit new merge request "Wiki Feature" |
features/steps/project/project_merge_requests.rb
| @@ -170,6 +170,18 @@ class ProjectMergeRequests < Spinach::FeatureSteps | @@ -170,6 +170,18 @@ class ProjectMergeRequests < Spinach::FeatureSteps | ||
| 170 | end | 170 | end |
| 171 | end | 171 | end |
| 172 | 172 | ||
| 173 | + step 'I click link "Reopen"' do | ||
| 174 | + within '.page-title' do | ||
| 175 | + click_link "Reopen" | ||
| 176 | + end | ||
| 177 | + end | ||
| 178 | + | ||
| 179 | + step 'I should see reopened merge request "Bug NS-04"' do | ||
| 180 | + within '.state-label' do | ||
| 181 | + page.should have_content "Open" | ||
| 182 | + end | ||
| 183 | + end | ||
| 184 | + | ||
| 173 | def project | 185 | def project |
| 174 | @project ||= Project.find_by!(name: "Shop") | 186 | @project ||= Project.find_by!(name: "Shop") |
| 175 | end | 187 | end |