From 15a6633999c81387245cabf129dd2fbb04650c95 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg <zegerjan@gitlab.com> Date: Wed, 17 Feb 2016 12:49:38 +0100 Subject: [PATCH] Revert authors ability to assign anyone --- app/controllers/autocomplete_controller.rb | 2 +- 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 | 2 +- spec/controllers/autocomplete_controller_spec.rb | 4 ++-- spec/models/concerns/issuable_spec.rb | 14 -------------- 8 files changed, 11 insertions(+), 35 deletions(-) diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 5d81a996fba..1e4fc612a3c 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -15,7 +15,7 @@ class AutocompleteController < ApplicationController @users = [*@users, current_user] end - unless params[:author_id] == "false" + if params[:author_id] && params[:author_id] != "false" @users = [User.find(params[:author_id]), *@users] end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f9314a241b2..e5f089fb8a0 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -116,11 +116,6 @@ 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 99a28b8c637..ca87dca4a70 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, issuable) + def filter_params(issuable_ability_name = :issue) params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE @@ -42,18 +42,13 @@ class IssuableBaseService < BaseService unless can?(current_user, ability, project) params.delete(:milestone_id) params.delete(:label_ids) - - # 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 + params.delete(:assignee_id) end end def update(issuable) change_state(issuable) - filter_params(issuable) + filter_params 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 4527d1c74e0..770f32de944 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(issuable = nil) - super(:issue, issuable) + def filter_params + super(:issue) 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 e472381ba36..7b306a8a531 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(issuable = nil) - super(:merge_request, issuable) + def filter_params + super(:merge_request) end end end diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 1df44eaa64f..ef351fe8093 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 issuable.can_assign_user?(current_user) + - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) .pull-right = link_to 'Edit', '#', class: 'edit-link' .value diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 0754a2889fc..612e344c411 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -12,13 +12,13 @@ describe AutocompleteController do project.team << [user, :master] end - let(:body) { JSON.parse(response.body) } - describe 'GET #users with project ID' do before do get(:users, project_id: project.id) end + let(:body) { JSON.parse(response.body) } + it { expect(body).to be_kind_of(Array) } it { expect(body.size).to eq 1 } it { expect(body.first["username"]).to eq user.username } diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 16fc1a6069a..600089802b2 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -111,20 +111,6 @@ 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