Commit b4036a8f authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '37081-epics-premium' into 'master'

Add subepics feature to license and services

See merge request gitlab-org/gitlab!24170
parents cabac822 47f6d9c3
......@@ -6,17 +6,16 @@ module EpicRelations
include IssuableLinks
included do
before_action :check_epics_available!
before_action :authorize_read_epic!, only: :index
before_action :authorize_admin_epic!, only: [:create, :destroy, :update]
before_action :authorize_read!, only: :index
before_action :authorize_admin!, only: [:create, :destroy, :update]
end
def authorize_read_epic!
render_404 unless can?(current_user, :read_epic, epic)
def authorize_read!
render_403 unless can?(current_user, :read_epic, epic)
end
def authorize_admin_epic!
render_403 unless can?(current_user, :admin_epic, epic)
def authorize_admin!
render_403 unless can?(current_user, "admin_#{authorized_object}", epic)
end
def epic
......@@ -24,4 +23,8 @@ module EpicRelations
group.epics.find_by_iid(params[:epic_id])
end
end
def authorized_object
raise NotImplementedError
end
end
......@@ -3,6 +3,7 @@
class Groups::EpicIssuesController < Groups::ApplicationController
include EpicRelations
before_action :check_epics_available!
before_action :authorize_issue_link_association!, only: [:destroy, :update]
def update
......@@ -32,4 +33,8 @@ class Groups::EpicIssuesController < Groups::ApplicationController
def link
@link ||= EpicIssue.find(params[:id])
end
def authorized_object
'epic'
end
end
......@@ -3,6 +3,9 @@
class Groups::EpicLinksController < Groups::ApplicationController
include EpicRelations
before_action :check_epics_available!, only: :index
before_action :check_subepics_available!, only: [:create, :destroy, :update]
def update
result = EpicLinks::UpdateService.new(child_epic, current_user, params[:epic]).execute
......@@ -28,4 +31,8 @@ class Groups::EpicLinksController < Groups::ApplicationController
def child_epic
@child_epic ||= Epic.find(params[:id])
end
def authorized_object
'epic_link'
end
end
......@@ -124,6 +124,7 @@ class License < ApplicationRecord
report_approver_rules
sast
security_dashboard
subepics
threat_monitoring
tracing
web_ide_terminal
......
......@@ -9,6 +9,7 @@ module EE
with_scope :subject
condition(:ldap_synced) { @subject.ldap_synced? }
condition(:epics_available) { @subject.feature_available?(:epics) }
condition(:subepics_available) { @subject.feature_available?(:subepics) }
condition(:contribution_analytics_available) do
@subject.feature_available?(:contribution_analytics)
end
......@@ -95,6 +96,10 @@ module EE
enable :update_epic
end
rule { reporter & subepics_available }.policy do
enable :admin_epic_link
end
rule { owner & epics_available }.enable :destroy_epic
rule { ~can?(:read_cross_project) }.policy do
......
......@@ -3,7 +3,7 @@
module EpicLinks
class CreateService < IssuableLinks::CreateService
def execute
unless can?(current_user, :admin_epic, issuable.group)
unless can?(current_user, :admin_epic_link, issuable.group)
return error(issuables_not_found_message, 404)
end
......
......@@ -26,8 +26,8 @@ module EpicLinks
def permission_to_remove_relation?
child_epic.present? &&
parent_epic.present? &&
can?(current_user, :admin_epic, parent_epic) &&
can?(current_user, :admin_epic, child_epic)
can?(current_user, :admin_epic_link, parent_epic) &&
can?(current_user, :admin_epic_link, child_epic)
end
def not_found_message
......
......@@ -12,6 +12,10 @@ module EpicLinks
end
def execute
unless can?(current_user, :admin_epic_link, epic.group)
return error('Epic not found for given params', 404)
end
move_epic!
success
rescue ActiveRecord::RecordNotFound
......
......@@ -29,7 +29,7 @@ module API
optional :move_after_id, type: Integer, desc: 'The id of the epic issue association that should be positioned after the actual issue'
end
put ':id/(-/)epics/:epic_iid/issues/:epic_issue_id' do
authorize_can_admin!
authorize_can_admin_epic!
update_params = {
move_before_id: params[:move_before_id],
......@@ -71,7 +71,7 @@ module API
end
# rubocop: disable CodeReuse/ActiveRecord
post ':id/(-/)epics/:epic_iid/issues/:issue_id' do
authorize_can_admin!
authorize_can_admin_epic!
issue = Issue.find(params[:issue_id])
......@@ -97,7 +97,7 @@ module API
requires :epic_issue_id, type: Integer, desc: 'The id of the association'
end
delete ':id/(-/)epics/:epic_iid/issues/:epic_issue_id' do
authorize_can_admin!
authorize_can_admin_epic!
result = ::EpicIssues::DestroyService.new(link, current_user).execute
......
......@@ -6,7 +6,6 @@ module API
before do
authenticate!
authorize_epics_feature!
end
helpers ::API::Helpers::EpicsHelpers
......@@ -43,6 +42,7 @@ module API
success EE::API::Entities::Epic
end
get ':id/(-/)epics/:epic_iid/epics' do
authorize_epics_feature!
authorize_can_read!
present child_epics, with: EE::API::Entities::Epic
......@@ -55,7 +55,8 @@ module API
use :child_epic_id
end
post ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
authorize_can_admin!
authorize_subepics_feature!
authorize_can_admin_epic_link!
create_params = { target_issuable: child_epic }
......@@ -75,7 +76,8 @@ module API
requires :title, type: String, desc: 'The title of a child epic'
end
post ':id/(-/)epics/:epic_iid/epics' do
authorize_can_admin!
authorize_subepics_feature!
authorize_can_admin_epic_link!
create_params = { parent_id: epic.id, title: params[:title] }
......@@ -93,7 +95,8 @@ module API
use :child_epic_id
end
delete ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
authorize_can_admin!
authorize_subepics_feature!
authorize_can_admin_epic_link!
updated_epic = ::Epics::UpdateService.new(user_group, current_user, { parent: nil }).execute(child_epic)
......@@ -107,7 +110,8 @@ module API
optional :move_after_id, type: Integer, desc: 'The id of the epic that should be positioned after the child epic'
end
put ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
authorize_can_admin!
authorize_subepics_feature!
authorize_can_admin_epic_link!
update_params = params.slice(:move_before_id, :move_after_id)
......
......@@ -101,7 +101,7 @@ module API
put ':id/(-/)epics/:epic_iid' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/194104')
authorize_can_admin!
authorize_can_admin_epic!
update_params = declared_params(include_missing: false)
update_params.delete(:epic_iid)
......
......@@ -7,14 +7,22 @@ module API
forbidden! unless user_group.feature_available?(:epics)
end
def authorize_subepics_feature!
forbidden! unless user_group.feature_available?(:subepics)
end
def authorize_can_read!
authorize!(:read_epic, epic)
end
def authorize_can_admin!
def authorize_can_admin_epic!
authorize!(:admin_epic, epic)
end
def authorize_can_admin_epic_link!
authorize!(:admin_epic_link, epic)
end
def authorize_can_create!
authorize!(:admin_epic, user_group)
end
......
......@@ -90,7 +90,7 @@ module EE
end
def action_allowed?
quick_action_target.group&.feature_available?(:epics) &&
quick_action_target.group&.feature_available?(:subepics) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target)
end
......
......@@ -24,8 +24,8 @@ describe Groups::EpicIssuesController do
subject
end
it 'returns 400 status' do
expect(response).to have_gitlab_http_status(:not_found)
it 'returns 403 status' do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
......
......@@ -8,32 +8,35 @@ describe Groups::EpicLinksController do
let(:epic1) { create(:epic, group: group) }
let(:epic2) { create(:epic, group: group) }
let(:user) { create(:user) }
let(:features_when_forbidden) { { epics: true, subepics: false } }
before do
sign_in(user)
end
shared_examples 'unlicensed epics action' do
shared_examples 'unlicensed subepics action' do
before do
stub_licensed_features(epics: false)
stub_licensed_features(features_when_forbidden)
group.add_developer(user)
subject
end
it 'returns 400 status' do
expect(response).to have_gitlab_http_status(:not_found)
it 'returns 403 status' do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
describe 'GET #index' do
let(:features_when_forbidden) { { epics: false } }
before do
epic1.update(parent: parent_epic)
end
subject { get :index, params: { group_id: group, epic_id: parent_epic.to_param } }
it_behaves_like 'unlicensed epics action'
it_behaves_like 'unlicensed subepics action'
context 'when epics are enabled' do
before do
......@@ -74,11 +77,11 @@ describe Groups::EpicLinksController do
post :create, params: { group_id: group, epic_id: parent_epic.to_param, issuable_references: reference }
end
it_behaves_like 'unlicensed epics action'
it_behaves_like 'unlicensed subepics action'
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user has permissions to create requested association' do
......@@ -125,11 +128,11 @@ describe Groups::EpicLinksController do
put :update, params: { group_id: group, epic_id: parent_epic.to_param, id: epic1.id, epic: { move_before_id: move_before_epic.id } }
end
it_behaves_like 'unlicensed epics action'
it_behaves_like 'unlicensed subepics action'
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user has permissions to reorder epics' do
......@@ -175,11 +178,11 @@ describe Groups::EpicLinksController do
subject { delete :destroy, params: { group_id: group, epic_id: parent_epic.to_param, id: epic1.id } }
it_behaves_like 'unlicensed epics action'
it_behaves_like 'unlicensed subepics action'
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user has permissions to update the parent epic' do
......
......@@ -28,7 +28,7 @@ describe 'Epic Issues', :js do
end
def visit_epic
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
sign_in(user)
visit group_epic_path(group, epic)
......@@ -182,7 +182,7 @@ describe 'Epic Issues', :js do
let(:epic4) { create(:epic, group: group, parent_id: epic3.id) }
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
sign_in(user)
visit group_epic_path(group, epic4)
......
......@@ -6,9 +6,12 @@ describe API::EpicLinks do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:epic) { create(:epic, group: group) }
let(:features_when_forbidden) { { epics: true, subepics: false } }
shared_examples 'user does not have access' do
it 'returns 403 when epics feature is disabled' do
it 'returns 403 when subepics feature is disabled' do
stub_licensed_features(features_when_forbidden)
group.add_developer(user)
subject
......@@ -37,14 +40,15 @@ describe API::EpicLinks do
describe 'GET /groups/:id/epics/:epic_iid/epics' do
let(:url) { "/groups/#{group.path}/epics/#{epic.iid}/epics" }
let(:features_when_forbidden) { { epics: false } }
subject { get api(url, user) }
it_behaves_like 'user does not have access'
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
let!(:child_epic1) { create(:epic, group: group, parent: epic, relative_position: 200) }
......@@ -70,9 +74,9 @@ describe API::EpicLinks do
it_behaves_like 'user does not have access'
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user is guest' do
......@@ -119,9 +123,9 @@ describe API::EpicLinks do
it_behaves_like 'user does not have access'
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user is guest' do
......@@ -176,9 +180,9 @@ describe API::EpicLinks do
it_behaves_like 'user does not have access'
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user has permissions to reorder epics' do
......@@ -215,9 +219,9 @@ describe API::EpicLinks do
it_behaves_like 'user does not have access'
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when user is guest' do
......
......@@ -55,15 +55,19 @@ describe EpicLinks::CreateService do
described_class.new(epic, user, params).execute
end
context 'when epics feature is disabled' do
context 'when subepics feature is disabled' do
before do
stub_licensed_features(epics: true, subepics: false)
end
subject { add_epic([valid_reference]) }
include_examples 'returns an error'
end
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when an error occurs' do
......
......@@ -45,15 +45,19 @@ describe EpicLinks::DestroyService do
described_class.new(child_epic, user).execute
end
context 'when epics feature is disabled' do
context 'when subepics feature is disabled' do
before do
stub_licensed_features(epics: true, subepics: false)
end
subject { remove_epic_relation(child_epic) }
include_examples 'returns not found error'
end
context 'when epics feature is enabled' do
context 'when subepics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when the user has no permissions to remove epic relation' do
......
......@@ -54,7 +54,7 @@ describe EpicLinks::ListService do
context 'when epics feature is enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'group member can see all child epics' do
......
......@@ -13,6 +13,7 @@ describe EpicLinks::UpdateService do
let(:child_epic4) { create(:epic, group: group, parent: parent_epic, relative_position: 400) }
let(:epic_to_move) { child_epic3 }
let(:params) { {} }
subject do
described_class.new(epic_to_move, user, params).execute
......@@ -23,6 +24,10 @@ describe EpicLinks::UpdateService do
end
describe '#execute' do
before do
group.add_developer(user)
end
shared_examples 'updating timestamps' do
it 'does not update moved epic' do
updated_at = epic_to_move.updated_at
......@@ -39,68 +44,82 @@ describe EpicLinks::UpdateService do
end
end
context 'when params are nil' do
let(:params) { { move_before_id: nil, move_after_id: nil } }
context 'when subepics feature is not available' do
it 'returns an error' do
stub_licensed_features(epics: true, subepics: false)
it 'does not change order of child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic3, child_epic4])
expect(subject).to eq(message: 'Epic not found for given params', status: :error, http_status: 404)
end
end
context 'when moving to start' do
let(:params) { { move_before_id: nil, move_after_id: child_epic1.id } }
context 'when subepics feature is available' do
before do
stub_licensed_features(epics: true, subepics: true)
end
it_behaves_like 'updating timestamps'
context 'when params are nil' do
let(:params) { { move_before_id: nil, move_after_id: nil } }
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic3, child_epic1, child_epic2, child_epic4])
it 'does not change order of child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic3, child_epic4])
end
end
end
context 'when moving to end' do
let(:params) { { move_before_id: child_epic4.id, move_after_id: nil } }
context 'when moving to start' do
let(:params) { { move_before_id: nil, move_after_id: child_epic1.id } }
it_behaves_like 'updating timestamps'
it_behaves_like 'updating timestamps'
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic4, child_epic3])
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic3, child_epic1, child_epic2, child_epic4])
end
end
end
context 'when moving between siblings' do
let(:params) { { move_before_id: child_epic1.id, move_after_id: child_epic2.id } }
context 'when moving to end' do
let(:params) { { move_before_id: child_epic4.id, move_after_id: nil } }
it_behaves_like 'updating timestamps'
it_behaves_like 'updating timestamps'
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic3, child_epic2, child_epic4])
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic4, child_epic3])
end
end
end
context 'when params are invalid' do
let(:other_epic) { create(:epic, group: group) }
context 'when moving between siblings' do
let(:params) { { move_before_id: child_epic1.id, move_after_id: child_epic2.id } }
shared_examples 'returns error' do
it 'does not change order of child epics and returns error' do
expect(subject).to include(message: 'Epic not found for given params', status: :error, http_status: 404)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic3, child_epic4])
it_behaves_like 'updating timestamps'
it 'reorders child epics' do
expect(subject).to include(status: :success)
expect(ordered_epics).to eq([child_epic1, child_epic3, child_epic2, child_epic4])
end
end
context 'when move_before_id is not a child of parent epic' do
let(:params) { { move_before_id: other_epic.id, move_after_id: child_epic2.id } }
context 'when params are invalid' do
let(:other_epic) { create(:epic, group: group) }
it_behaves_like 'returns error'
end
shared_examples 'returns error' do
it 'does not change order of child epics and returns error' do
expect(subject).to include(message: 'Epic not found for given params', status: :error, http_status: 404)
expect(ordered_epics).to eq([child_epic1, child_epic2, child_epic3, child_epic4])
end
end
context 'when move_after_id is not a child of parent epic' do
let(:params) { { move_before_id: child_epic1.id, move_after_id: other_epic.id } }
context 'when move_before_id is not a child of parent epic' do
let(:params) { { move_before_id: other_epic.id, move_after_id: child_epic2.id } }
it_behaves_like 'returns error'
it_behaves_like 'returns error'
end
context 'when move_after_id is not a child of parent epic' do
let(:params) { { move_before_id: child_epic1.id, move_after_id: other_epic.id } }
it_behaves_like 'returns error'
end
end
end
end
......
......@@ -101,17 +101,33 @@ describe Groups::AutocompleteService do
let(:parent_epic) { create(:epic, group: group, author: user) }
let(:epic) { create(:epic, group: group, author: user, parent: parent_epic) }
before do
stub_licensed_features(epics: true)
context 'with subepics feature enabled' do
before do
stub_licensed_features(epics: true, subepics: true)
end
it 'returns available commands' do
available_commands = [
:todo, :unsubscribe, :award, :shrug, :tableflip, :cc, :title, :close,
:child_epic, :remove_child_epic, :parent_epic, :remove_parent_epic
]
expect(subject.commands(epic).map { |c| c[:name] }).to match_array(available_commands)
end
end
it 'returns available commands' do
available_commands = [
:todo, :unsubscribe, :award, :shrug, :tableflip, :cc, :title, :close,
:child_epic, :remove_child_epic, :parent_epic, :remove_parent_epic
]
context 'with subepics feature disabled' do
before do
stub_licensed_features(epics: true, subepics: false)
end
it 'returns available commands' do
available_commands = [
:todo, :unsubscribe, :award, :shrug, :tableflip, :cc, :title, :close
]
expect(subject.commands(epic).map { |c| c[:name] }).to match_array(available_commands)
expect(subject.commands(epic).map { |c| c[:name] }).to match_array(available_commands)
end
end
end
end
......
......@@ -368,9 +368,9 @@ describe QuickActions::InterpretService do
end
end
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
context 'when a user does not have permissions to add epic relations' do
......@@ -501,9 +501,9 @@ describe QuickActions::InterpretService do
end
end
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
epic.reload
end
......@@ -595,8 +595,9 @@ describe QuickActions::InterpretService do
end
end
context 'when epics are disabled' do
context 'when subepics are disabled' do
before do
stub_licensed_features(epics: true, subepics: false)
group.add_developer(current_user)
end
......@@ -671,9 +672,9 @@ describe QuickActions::InterpretService do
end
end
context 'when epics are enabled' do
context 'when subepics are enabled' do
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
end
it 'unassigns an issue from an epic' do
......@@ -978,7 +979,7 @@ describe QuickActions::InterpretService do
let(:epic2) { create(:epic, group: group) }
before do
stub_licensed_features(epics: true)
stub_licensed_features(epics: true, subepics: true)
group.add_developer(current_user)
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