From df27b3a1742048358674733d9cb3392348d5e964 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 9 Feb 2016 12:02:52 +0100 Subject: [PATCH] Issuable can be assigned to author Closes #9014 The only difference with #9014 is that I thoughed the author should also be able to assign the issue. If this is unwanted behavior Ill revert it. --- CHANGELOG | 1 + app/assets/javascripts/users_select.js.coffee | 4 +++- app/controllers/autocomplete_controller.rb | 8 +++++++- app/controllers/projects/issues_controller.rb | 4 ++-- app/helpers/selects_helper.rb | 16 +++++++++------- app/models/concerns/issuable.rb | 5 +++++ app/services/issuable_base_service.rb | 11 ++++++++--- app/services/issues/base_service.rb | 4 ++-- app/services/merge_requests/base_service.rb | 4 ++-- app/views/shared/issuable/_sidebar.html.haml | 7 +++++-- spec/controllers/autocomplete_controller_spec.rb | 13 +++++++++++++ spec/models/concerns/issuable_spec.rb | 14 ++++++++++++++ 12 files changed, 71 insertions(+), 20 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 77a21e757c4..e93979fd468 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -78,6 +78,7 @@ v 8.5.0 (unreleased) - Add label description (Nuttanart Pornprasitsakul) - Show label row when filtering issues or merge requests by label (Nuttanart Pornprasitsakul) - Add Todos + - Allow issues and merge requests to be assigned to the author(Zeger-Jan van de Weg) v 8.4.4 - Update omniauth-saml gem to 1.4.2 diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee index 9467011799f..93c0c7adfee 100644 --- a/app/assets/javascripts/users_select.js.coffee +++ b/app/assets/javascripts/users_select.js.coffee @@ -7,11 +7,12 @@ class @UsersSelect @projectId = $(select).data('project-id') @groupId = $(select).data('group-id') @showCurrentUser = $(select).data('current-user') + @authorId = $(select).data('author-id') showNullUser = $(select).data('null-user') showAnyUser = $(select).data('any-user') showEmailUser = $(select).data('email-user') firstUser = $(select).data('first-user') - + $(select).select2 placeholder: "Search for a user" multiple: $(select).hasClass('multiselect') @@ -112,6 +113,7 @@ class @UsersSelect project_id: @projectId group_id: @groupId current_user: @showCurrentUser + author_id: @authorId dataType: "json" ).done (users) -> callback(users) diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 77c8dafc012..23f2ab45ff4 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -12,8 +12,14 @@ class AutocompleteController < ApplicationController if params[:search].blank? # Include current user if available to filter by "Me" if params[:current_user] && current_user - @users = [*@users, current_user].uniq + @users = [*@users, current_user] end + + if params[:author_id] + @users = [User.find(params[:author_id]), *@users] + end + + @users.uniq! end render json: @users, only: [:name, :username, :id], methods: [:avatar_url] diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 67faa1e4437..8aa85e448c4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -59,8 +59,8 @@ class Projects::IssuesController < Projects::ApplicationController end def show - @note = @project.notes.new(noteable: @issue) - @notes = @issue.notes.nonawards.with_associations.fresh + @note = @project.notes.new(noteable: @issue) + @notes = @issue.notes.nonawards.with_associations.fresh @noteable = @issue @merge_requests = @issue.referenced_merge_requests(current_user) diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 05386d790ca..368762b8265 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -6,12 +6,13 @@ module SelectsHelper value = opts[:selected] || '' placeholder = opts[:placeholder] || 'Search for a user' - null_user = opts[:null_user] || false - any_user = opts[:any_user] || false - email_user = opts[:email_user] || false - first_user = opts[:first_user] && current_user ? current_user.username : false - current_user = opts[:current_user] || false - project = opts[:project] || @project + null_user = opts[:null_user] || false + any_user = opts[:any_user] || false + email_user = opts[:email_user] || false + first_user = opts[:first_user] && current_user ? current_user.username : false + current_user = opts[:current_user] || false + author_id = opts[:author_id] || false + project = opts[:project] || @project html = { class: css_class, @@ -21,7 +22,8 @@ module SelectsHelper any_user: any_user, email_user: email_user, first_user: first_user, - current_user: current_user + current_user: current_user, + author_id: author_id } } diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index e5f089fb8a0..f9314a241b2 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -116,6 +116,11 @@ module Issuable assignee_id_changed? end + def can_assign_user?(current_user) + author == current_user || + Ability.abilities.allowed?(current_user, :"admin_#{to_ability_name}", project) + end + def open? opened? || reopened? end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ca87dca4a70..99a28b8c637 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -33,7 +33,7 @@ class IssuableBaseService < BaseService end end - def filter_params(issuable_ability_name = :issue) + def filter_params(issuable_ability_name, issuable) params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE @@ -42,13 +42,18 @@ class IssuableBaseService < BaseService unless can?(current_user, ability, project) params.delete(:milestone_id) params.delete(:label_ids) - params.delete(:assignee_id) + + # The author of an issue can be assigned, to signal the ball being in his/her + # court. This allow him/her to reassign the issue back to the reviewer. + if issuable && !(issuable.author == current_user) + params.delete(:assignee_id) + end end end def update(issuable) change_state(issuable) - filter_params + filter_params(issuable) old_labels = issuable.labels.to_a if params.present? && issuable.update_attributes(params.merge(updated_by: current_user)) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 770f32de944..4527d1c74e0 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -10,8 +10,8 @@ module Issues private - def filter_params - super(:issue) + def filter_params(issuable = nil) + super(:issue, issuable) end def execute_hooks(issue, action = 'open') diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 7b306a8a531..2afbdfae664 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -23,8 +23,8 @@ module MergeRequests private - def filter_params - super(:merge_request) + def filter_params(issueable = nil) + super(:merge_request, issueable) end end end diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index a45775f36b5..1df44eaa64f 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -30,7 +30,7 @@ .title %label Assignee - - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) + - if issuable.can_assign_user?(current_user) .pull-right = link_to 'Edit', '#', class: 'edit-link' .value @@ -43,7 +43,10 @@ .light None .selectbox - = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, first_user: true) + = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", + placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', + selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, + first_user: true, author_id: issuable.author_id) .block.milestone .sidebar-collapsed-icon diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 85379a8e984..0754a2889fc 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -143,4 +143,17 @@ describe AutocompleteController do it { expect(body.size).to eq 0 } end end + + context 'author of issuable included' do + before do + sign_in(user) + get(:users, author_id: non_member.id) + end + + let(:body) { JSON.parse(response.body) } + + it 'should also return the author' do + expect(body.first["username"]).to eq non_member.username + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 600089802b2..16fc1a6069a 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -111,6 +111,20 @@ describe Issue, "Issuable" do end end + describe "#can_assign_user?" do + let(:author) { build(:user) } + let(:issue) { build(:issue, author: author)} + + it "Allows the author to change the assignee" do + expect(issue.can_assign_user?(author)).to be_truthy + end + + it "Doesn't allow others, non-team members" do + other_user = build(:user) + expect(issue.can_assign_user?(other_user)).to be_falsey + end + end + describe "votes" do before do author = create :user -- GitLab