Commit 8e85fac3 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix quick action permissions to match UI

This changes permissions on some quick actions to match what we check in
the issuable base service.

We recently changed the check in the service so that guests can set
labels, milestones, and other metadata when creating an issue. This
allows them to set those via quick actions.

Changelog: fixed
parent ae1b7af2
...@@ -17,7 +17,7 @@ module EE ...@@ -17,7 +17,7 @@ module EE
quick_action_target.supports_assignee? && quick_action_target.supports_assignee? &&
quick_action_target.allows_multiple_assignees? && quick_action_target.allows_multiple_assignees? &&
quick_action_target.persisted? && quick_action_target.persisted? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :reassign do |reassign_param| command :reassign do |reassign_param|
@updates[:assignee_ids] = extract_users(reassign_param).map(&:id) @updates[:assignee_ids] = extract_users(reassign_param).map(&:id)
......
...@@ -84,8 +84,7 @@ module Gitlab ...@@ -84,8 +84,7 @@ module Gitlab
params '~label1 ~"label 2"' params '~label1 ~"label 2"'
types Issuable types Issuable
condition do condition do
parent && current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", parent) &&
find_labels.any? find_labels.any?
end end
command :label do |labels_param| command :label do |labels_param|
...@@ -107,7 +106,7 @@ module Gitlab ...@@ -107,7 +106,7 @@ module Gitlab
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.labels.any? && quick_action_target.labels.any? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", parent) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :unlabel, :remove_label do |labels_param = nil| command :unlabel, :remove_label do |labels_param = nil|
if labels_param.present? if labels_param.present?
...@@ -139,7 +138,7 @@ module Gitlab ...@@ -139,7 +138,7 @@ module Gitlab
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.labels.any? && quick_action_target.labels.any? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", parent) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :relabel do |labels_param| command :relabel do |labels_param|
run_label_command(labels: find_labels(labels_param), command: :relabel, updates_key: :label_ids) run_label_command(labels: find_labels(labels_param), command: :relabel, updates_key: :label_ids)
......
...@@ -19,7 +19,7 @@ module Gitlab ...@@ -19,7 +19,7 @@ module Gitlab
types Issue types Issue
condition do condition do
quick_action_target.respond_to?(:due_date) && quick_action_target.respond_to?(:due_date) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
parse_params do |due_date_param| parse_params do |due_date_param|
Chronic.parse(due_date_param).try(:to_date) Chronic.parse(due_date_param).try(:to_date)
...@@ -40,7 +40,7 @@ module Gitlab ...@@ -40,7 +40,7 @@ module Gitlab
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.respond_to?(:due_date) && quick_action_target.respond_to?(:due_date) &&
quick_action_target.due_date? && quick_action_target.due_date? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :remove_due_date do command :remove_due_date do
@updates[:due_date] = nil @updates[:due_date] = nil
...@@ -54,7 +54,7 @@ module Gitlab ...@@ -54,7 +54,7 @@ module Gitlab
params '~"Target column"' params '~"Target column"'
types Issue types Issue
condition do condition do
current_user.can?(:"update_#{quick_action_target.to_ability_name}", quick_action_target) && current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target) &&
quick_action_target.project.boards.count == 1 quick_action_target.project.boards.count == 1
end end
command :board_move do |target_list_name| command :board_move do |target_list_name|
...@@ -86,7 +86,7 @@ module Gitlab ...@@ -86,7 +86,7 @@ module Gitlab
types Issue types Issue
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
current_user.can?(:"update_#{quick_action_target.to_ability_name}", quick_action_target) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :duplicate do |duplicate_param| command :duplicate do |duplicate_param|
canonical_issue = extract_references(duplicate_param, :issue).first canonical_issue = extract_references(duplicate_param, :issue).first
......
...@@ -26,7 +26,7 @@ module Gitlab ...@@ -26,7 +26,7 @@ module Gitlab
end end
types Issue, MergeRequest types Issue, MergeRequest
condition do condition do
quick_action_target.supports_assignee? && current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) quick_action_target.supports_assignee? && current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
parse_params do |assignee_param| parse_params do |assignee_param|
extract_users(assignee_param) extract_users(assignee_param)
...@@ -66,7 +66,7 @@ module Gitlab ...@@ -66,7 +66,7 @@ module Gitlab
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.assignees.any? && quick_action_target.assignees.any? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
parse_params do |unassign_param| parse_params do |unassign_param|
# When multiple users are assigned, all will be unassigned if multiple assignees are no longer allowed # When multiple users are assigned, all will be unassigned if multiple assignees are no longer allowed
...@@ -92,7 +92,7 @@ module Gitlab ...@@ -92,7 +92,7 @@ module Gitlab
types Issue, MergeRequest types Issue, MergeRequest
condition do condition do
quick_action_target.supports_milestone? && quick_action_target.supports_milestone? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) && current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target) &&
find_milestones(project, state: 'active').any? find_milestones(project, state: 'active').any?
end end
parse_params do |milestone_param| parse_params do |milestone_param|
...@@ -115,7 +115,7 @@ module Gitlab ...@@ -115,7 +115,7 @@ module Gitlab
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.milestone_id? && quick_action_target.milestone_id? &&
quick_action_target.supports_milestone? && quick_action_target.supports_milestone? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :remove_milestone do command :remove_milestone do
@updates[:milestone_id] = nil @updates[:milestone_id] = nil
...@@ -128,7 +128,7 @@ module Gitlab ...@@ -128,7 +128,7 @@ module Gitlab
params '#issue | !merge_request' params '#issue | !merge_request'
types Issue, MergeRequest types Issue, MergeRequest
condition do condition do
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
parse_params do |issuable_param| parse_params do |issuable_param|
extract_references(issuable_param, :issue).first || extract_references(issuable_param, :issue).first ||
...@@ -225,7 +225,7 @@ module Gitlab ...@@ -225,7 +225,7 @@ module Gitlab
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
!quick_action_target.discussion_locked? && !quick_action_target.discussion_locked? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :lock do command :lock do
@updates[:discussion_locked] = true @updates[:discussion_locked] = true
...@@ -238,7 +238,7 @@ module Gitlab ...@@ -238,7 +238,7 @@ module Gitlab
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
quick_action_target.discussion_locked? && quick_action_target.discussion_locked? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target) current_user.can?(:"set_#{quick_action_target.to_ability_name}_metadata", quick_action_target)
end end
command :unlock do command :unlock do
@updates[:discussion_locked] = false @updates[:discussion_locked] = false
......
...@@ -2553,4 +2553,32 @@ RSpec.describe QuickActions::InterpretService do ...@@ -2553,4 +2553,32 @@ RSpec.describe QuickActions::InterpretService do
end end
end end
end end
describe '#available_commands' do
context 'when Guest is creating a new issue' do
let_it_be(:guest) { create(:user) }
let(:issue) { build(:issue, project: public_project) }
let(:service) { described_class.new(project, guest) }
before_all do
public_project.add_guest(guest)
end
it 'includes commands to set metadata' do
# milestone action is only available when project has a milestone
milestone
available_commands = service.available_commands(issue)
expect(available_commands).to include(
a_hash_including(name: :label),
a_hash_including(name: :milestone),
a_hash_including(name: :copy_metadata),
a_hash_including(name: :assign),
a_hash_including(name: :due)
)
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment