Commit 0fd3ed99df400067fcee9860e090d6038f7673bb
Merge branch '6-6-5-patch' into '6-6-stable'
6.6.5 patch * adds ability to reopen mr * adds ability to remove issue assignee * removes close button from diff comment form
Showing
14 changed files
with
113 additions
and
17 deletions
Show diff stats
CHANGELOG
app/assets/javascripts/project_users_select.js.coffee
| ... | ... | @@ -10,6 +10,16 @@ |
| 10 | 10 | query: (query) -> |
| 11 | 11 | Api.projectUsers project_id, query.term, (users) -> |
| 12 | 12 | data = { results: users } |
| 13 | + | |
| 14 | + nullUser = { | |
| 15 | + name: 'Unassigned', | |
| 16 | + avatar: null, | |
| 17 | + username: 'none', | |
| 18 | + id: '' | |
| 19 | + } | |
| 20 | + | |
| 21 | + data.results.unshift(nullUser) | |
| 22 | + | |
| 13 | 23 | query.callback(data) |
| 14 | 24 | |
| 15 | 25 | initSelection: (element, callback) -> |
| ... | ... | @@ -35,8 +45,13 @@ |
| 35 | 45 | else |
| 36 | 46 | avatar = gon.relative_url_root + "/assets/no_avatar.png" |
| 37 | 47 | |
| 48 | + if user.id == '' | |
| 49 | + avatarMarkup = '' | |
| 50 | + else | |
| 51 | + avatarMarkup = "<div class='user-image'><img class='avatar s24' src='#{avatar}'></div>" | |
| 52 | + | |
| 38 | 53 | "<div class='user-result'> |
| 39 | - <div class='user-image'><img class='avatar s24' src='#{avatar}'></div> | |
| 54 | + #{avatarMarkup} | |
| 40 | 55 | <div class='user-name'>#{user.name}</div> |
| 41 | 56 | <div class='user-username'>#{user.username}</div> |
| 42 | 57 | </div>" | ... | ... |
app/assets/stylesheets/sections/merge_requests.scss
app/controllers/projects/merge_requests_controller.rb
| ... | ... | @@ -131,7 +131,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController |
| 131 | 131 | def automerge |
| 132 | 132 | return access_denied! unless allowed_to_merge? |
| 133 | 133 | |
| 134 | - if @merge_request.opened? && @merge_request.can_be_merged? | |
| 134 | + if @merge_request.open? && @merge_request.can_be_merged? | |
| 135 | 135 | @merge_request.should_remove_source_branch = params[:should_remove_source_branch] |
| 136 | 136 | @merge_request.automerge!(current_user, params[:merge_commit_message]) |
| 137 | 137 | @status = true |
| ... | ... | @@ -226,7 +226,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController |
| 226 | 226 | |
| 227 | 227 | @merge_request_diff = @merge_request.merge_request_diff |
| 228 | 228 | @allowed_to_merge = allowed_to_merge? |
| 229 | - @show_merge_controls = @merge_request.opened? && @commits.any? && @allowed_to_merge | |
| 229 | + @show_merge_controls = @merge_request.open? && @commits.any? && @allowed_to_merge | |
| 230 | 230 | end |
| 231 | 231 | |
| 232 | 232 | def allowed_to_merge? | ... | ... |
app/models/concerns/issuable.rb
| ... | ... | @@ -23,7 +23,8 @@ module Issuable |
| 23 | 23 | scope :assigned, -> { where("assignee_id IS NOT NULL") } |
| 24 | 24 | scope :unassigned, -> { where("assignee_id IS NULL") } |
| 25 | 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 | 29 | delegate :name, |
| 29 | 30 | :email, | ... | ... |
app/models/issue.rb
| ... | ... | @@ -28,8 +28,6 @@ class Issue < ActiveRecord::Base |
| 28 | 28 | |
| 29 | 29 | scope :of_group, ->(group) { where(project_id: group.project_ids) } |
| 30 | 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 | 32 | attr_accessible :title, :assignee_id, :position, :description, |
| 35 | 33 | :milestone_id, :label_list, :author_id_of_changes, |
| ... | ... | @@ -50,9 +48,7 @@ class Issue < ActiveRecord::Base |
| 50 | 48 | end |
| 51 | 49 | |
| 52 | 50 | state :opened |
| 53 | - | |
| 54 | 51 | state :reopened |
| 55 | - | |
| 56 | 52 | state :closed |
| 57 | 53 | end |
| 58 | 54 | ... | ... |
app/models/merge_request.rb
| ... | ... | @@ -100,8 +100,6 @@ class MergeRequest < ActiveRecord::Base |
| 100 | 100 | |
| 101 | 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 | 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 | 103 | scope :merged, -> { with_state(:merged) } |
| 106 | 104 | scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } |
| 107 | 105 | scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } |
| ... | ... | @@ -160,6 +158,10 @@ class MergeRequest < ActiveRecord::Base |
| 160 | 158 | MergeRequests::AutoMergeService.new.execute(self, current_user, commit_message) |
| 161 | 159 | end |
| 162 | 160 | |
| 161 | + def open? | |
| 162 | + opened? || reopened? | |
| 163 | + end | |
| 164 | + | |
| 163 | 165 | def mr_and_commit_notes |
| 164 | 166 | # Fetch comments only from last 100 commits |
| 165 | 167 | commits_for_notes_limit = 100 | ... | ... |
app/observers/merge_request_observer.rb
| ... | ... | @@ -22,6 +22,8 @@ class MergeRequestObserver < ActivityObserver |
| 22 | 22 | create_event(merge_request, Event::REOPENED) |
| 23 | 23 | create_note(merge_request) |
| 24 | 24 | execute_hooks(merge_request) |
| 25 | + merge_request.reload_code | |
| 26 | + merge_request.mark_as_unchecked | |
| 25 | 27 | end |
| 26 | 28 | |
| 27 | 29 | def after_update(merge_request) | ... | ... |
app/views/projects/merge_requests/_show.html.haml
| ... | ... | @@ -2,7 +2,7 @@ |
| 2 | 2 | = render "projects/merge_requests/show/mr_title" |
| 3 | 3 | = render "projects/merge_requests/show/how_to_merge" |
| 4 | 4 | = render "projects/merge_requests/show/mr_box" |
| 5 | - - if @merge_request.opened? | |
| 5 | + - if @merge_request.open? | |
| 6 | 6 | - if @merge_request.source_branch_exists? && @merge_request.target_branch_exists? |
| 7 | 7 | = render "projects/merge_requests/show/mr_accept" |
| 8 | 8 | - else |
| ... | ... | @@ -24,9 +24,10 @@ |
| 24 | 24 | |
| 25 | 25 | - content_for :note_actions do |
| 26 | 26 | - if can?(current_user, :modify_merge_request, @merge_request) |
| 27 | - - unless @merge_request.closed? | |
| 28 | - = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn grouped btn-close", title: "Close merge request" | |
| 29 | - | |
| 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 grouped btn-close close-mr-link", title: "Close merge request" | |
| 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 grouped btn-reopen reopen-mr-link", title: "Close merge request" | |
| 30 | 31 | |
| 31 | 32 | .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } |
| 32 | 33 | = render "projects/notes/notes_with_form" | ... | ... |
app/views/projects/merge_requests/show/_mr_box.html.haml
| ... | ... | @@ -37,7 +37,7 @@ |
| 37 | 37 | %i.icon-ok |
| 38 | 38 | Merged by #{link_to_member(@project, @merge_request.merge_event.author)} |
| 39 | 39 | #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}. |
| 40 | -- if !@closes_issues.empty? && @merge_request.opened? | |
| 40 | +- if !@closes_issues.empty? && @merge_request.open? | |
| 41 | 41 | .alert.alert-info.alert-info |
| 42 | 42 | %span |
| 43 | 43 | %i.icon-ok | ... | ... |
app/views/projects/merge_requests/show/_mr_title.html.haml
| ... | ... | @@ -3,8 +3,8 @@ |
| 3 | 3 | |
| 4 | 4 | %span.pull-right |
| 5 | 5 | - if can?(current_user, :modify_merge_request, @merge_request) |
| 6 | - - if @merge_request.opened? | |
| 7 | - .left.btn-group | |
| 6 | + - if @merge_request.open? | |
| 7 | + .btn-group.pull-left | |
| 8 | 8 | %a.btn.grouped.dropdown-toggle{ data: {toggle: :dropdown} } |
| 9 | 9 | %i.icon-download-alt |
| 10 | 10 | Download as |
| ... | ... | @@ -18,6 +18,8 @@ |
| 18 | 18 | = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn grouped", id:"edit_merge_request" do |
| 19 | 19 | %i.icon-edit |
| 20 | 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 grouped btn-reopen reopen-mr-link", title: "Close merge request" | |
| 21 | 23 | |
| 22 | 24 | .votes-holder.hidden-sm.hidden-xs |
| 23 | 25 | #votes= render 'votes/votes_block', votable: @merge_request | ... | ... |
features/project/merge_requests.feature
| ... | ... | @@ -29,6 +29,13 @@ Feature: Project Merge Requests |
| 29 | 29 | And I click link "Close" |
| 30 | 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 | 39 | Scenario: I submit new unassigned merge request |
| 33 | 40 | Given I click link "New Merge Request" |
| 34 | 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 | 170 | end |
| 171 | 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 | 185 | def project |
| 174 | 186 | @project ||= Project.find_by!(name: "Shop") |
| 175 | 187 | end | ... | ... |
spec/features/issues_spec.rb
| ... | ... | @@ -43,6 +43,31 @@ describe "Issues" do |
| 43 | 43 | page.should have_content project.name |
| 44 | 44 | end |
| 45 | 45 | end |
| 46 | + | |
| 47 | + end | |
| 48 | + | |
| 49 | + describe "Editing issue assignee" do | |
| 50 | + let!(:issue) do | |
| 51 | + create(:issue, | |
| 52 | + author: @user, | |
| 53 | + assignee: @user, | |
| 54 | + project: project) | |
| 55 | + end | |
| 56 | + | |
| 57 | + it 'allows user to select unasigned', :js => true do | |
| 58 | + visit edit_project_issue_path(project, issue) | |
| 59 | + | |
| 60 | + page.should have_content "Assign to #{@user.name}" | |
| 61 | + | |
| 62 | + page.first('#s2id_issue_assignee_id').click | |
| 63 | + sleep 2 # wait for ajax stuff to complete | |
| 64 | + page.first('.user-result').click | |
| 65 | + | |
| 66 | + click_button "Save changes" | |
| 67 | + | |
| 68 | + page.should have_content "Assignee: Select assignee" | |
| 69 | + issue.reload.assignee.should be_nil | |
| 70 | + end | |
| 46 | 71 | end |
| 47 | 72 | |
| 48 | 73 | describe "Filter issue" do |
| ... | ... | @@ -245,6 +270,28 @@ describe "Issues" do |
| 245 | 270 | page.should have_content milestone.title |
| 246 | 271 | end |
| 247 | 272 | end |
| 273 | + | |
| 274 | + describe 'removing assignee' do | |
| 275 | + let(:user2) { create(:user) } | |
| 276 | + | |
| 277 | + before :each do | |
| 278 | + issue.assignee = user2 | |
| 279 | + issue.save | |
| 280 | + end | |
| 281 | + | |
| 282 | + it 'allows user to remove assignee', :js => true do | |
| 283 | + visit project_issue_path(project, issue) | |
| 284 | + page.should have_content "Assignee: #{user2.name}" | |
| 285 | + | |
| 286 | + page.first('#s2id_issue_assignee_id').click | |
| 287 | + sleep 2 # wait for ajax stuff to complete | |
| 288 | + page.first('.user-result').click | |
| 289 | + | |
| 290 | + page.should have_content "Assignee: Unassigned" | |
| 291 | + sleep 2 # wait for ajax stuff to complete | |
| 292 | + issue.reload.assignee.should be_nil | |
| 293 | + end | |
| 294 | + end | |
| 248 | 295 | end |
| 249 | 296 | |
| 250 | 297 | def first_issue | ... | ... |