diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules.vue b/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules.vue
index d03de91ea074e50d0a8c78353751a54107002dc6..6695c6179cf8b86c6ab66b53aba95dcc96019073 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules.vue
+++ b/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules.vue
@@ -70,10 +70,12 @@ export default {
},
update(data) {
const { pipelineSchedules: { nodes: list = [], count } = {} } = data.project || {};
+ const currentUser = data.currentUser || {};
return {
list,
count,
+ currentUser,
};
},
error() {
@@ -279,6 +281,7 @@ export default {
<pipeline-schedules-table
v-else
:schedules="schedules.list"
+ :current-user="schedules.currentUser"
@showTakeOwnershipModal="setTakeOwnershipModal"
@showDeleteModal="setDeleteModal"
@playPipelineSchedule="playPipelineSchedule"
diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue b/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
index 45b4f618e1763ea9df217e7314f46cd754872c0f..5bd58bfd95dce30a9a9c7d3cba7312d725e89fe7 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
+++ b/app/assets/javascripts/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue
@@ -23,13 +23,20 @@ export default {
type: Object,
required: true,
},
+ currentUser: {
+ type: Object,
+ required: true,
+ },
},
computed: {
canPlay() {
return this.schedule.userPermissions.playPipelineSchedule;
},
+ isCurrentUserOwner() {
+ return this.schedule.owner.username === this.currentUser.username;
+ },
canTakeOwnership() {
- return this.schedule.userPermissions.takeOwnershipPipelineSchedule;
+ return !this.isCurrentUserOwner && this.schedule.userPermissions.adminPipelineSchedule;
},
canUpdate() {
return this.schedule.userPermissions.updatePipelineSchedule;
diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/table/pipeline_schedules_table.vue b/app/assets/javascripts/ci/pipeline_schedules/components/table/pipeline_schedules_table.vue
index e8cfc5b29f39a39012dd5d03ef5ce3d507e16f91..0b95e2037e852adb68272fd3dc42b7ea6fd7afa2 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/components/table/pipeline_schedules_table.vue
+++ b/app/assets/javascripts/ci/pipeline_schedules/components/table/pipeline_schedules_table.vue
@@ -59,6 +59,10 @@ export default {
type: Array,
required: true,
},
+ currentUser: {
+ type: Object,
+ required: true,
+ },
},
};
</script>
@@ -94,6 +98,7 @@ export default {
<template #cell(actions)="{ item }">
<pipeline-schedule-actions
:schedule="item"
+ :current-user="currentUser"
@showTakeOwnershipModal="$emit('showTakeOwnershipModal', $event)"
@showDeleteModal="$emit('showDeleteModal', $event)"
@playPipelineSchedule="$emit('playPipelineSchedule', $event)"
diff --git a/app/assets/javascripts/ci/pipeline_schedules/graphql/queries/get_pipeline_schedules.query.graphql b/app/assets/javascripts/ci/pipeline_schedules/graphql/queries/get_pipeline_schedules.query.graphql
index 9f6cb429cca9280a8146403b4b4921da0b852057..6167c7dc57752a59e79b006ca75b774c9ecc68c9 100644
--- a/app/assets/javascripts/ci/pipeline_schedules/graphql/queries/get_pipeline_schedules.query.graphql
+++ b/app/assets/javascripts/ci/pipeline_schedules/graphql/queries/get_pipeline_schedules.query.graphql
@@ -1,4 +1,8 @@
query getPipelineSchedulesQuery($projectPath: ID!, $status: PipelineScheduleStatus) {
+ currentUser {
+ id
+ username
+ }
project(fullPath: $projectPath) {
id
pipelineSchedules(status: $status) {
@@ -25,13 +29,13 @@ query getPipelineSchedulesQuery($projectPath: ID!, $status: PipelineScheduleStat
realNextRun
owner {
id
+ username
avatarUrl
name
webPath
}
userPermissions {
playPipelineSchedule
- takeOwnershipPipelineSchedule
updatePipelineSchedule
adminPipelineSchedule
}
diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index bbc62c0395747bc88c2b3f658865012d692bd98d..fb332fec3b5c371c0cc329e76ebfa12ff3824436 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -8,8 +8,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
before_action :authorize_read_pipeline_schedule!
before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
before_action :authorize_update_pipeline_schedule!, only: [:edit, :update]
- before_action :authorize_take_ownership_pipeline_schedule!, only: [:take_ownership]
- before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
+ before_action :authorize_admin_pipeline_schedule!, only: [:take_ownership, :destroy]
before_action :push_schedule_feature_flag, only: [:index, :new, :edit]
feature_category :continuous_integration
@@ -111,10 +110,6 @@ def authorize_update_pipeline_schedule!
return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
end
- def authorize_take_ownership_pipeline_schedule!
- return access_denied! unless can?(current_user, :take_ownership_pipeline_schedule, schedule)
- end
-
def authorize_admin_pipeline_schedule!
return access_denied! unless can?(current_user, :admin_pipeline_schedule, schedule)
end
diff --git a/app/graphql/mutations/ci/pipeline_schedule/take_ownership.rb b/app/graphql/mutations/ci/pipeline_schedule/take_ownership.rb
index 2e4312f0045de3793ce442d41a37517efb99e9b6..d71ef738cabe66c8753472560906c6d01eed2bdb 100644
--- a/app/graphql/mutations/ci/pipeline_schedule/take_ownership.rb
+++ b/app/graphql/mutations/ci/pipeline_schedule/take_ownership.rb
@@ -6,7 +6,7 @@ module PipelineSchedule
class TakeOwnership < Base
graphql_name 'PipelineScheduleTakeOwnership'
- authorize :take_ownership_pipeline_schedule
+ authorize :admin_pipeline_schedule
field :pipeline_schedule,
Types::Ci::PipelineScheduleType,
diff --git a/app/graphql/types/permission_types/ci/pipeline_schedules.rb b/app/graphql/types/permission_types/ci/pipeline_schedules.rb
index 268ac6096d0799d325c9625994c7e2daa1bb00f5..dd9d94aa57831b0b641d5d60481ae3e0bb9d5c71 100644
--- a/app/graphql/types/permission_types/ci/pipeline_schedules.rb
+++ b/app/graphql/types/permission_types/ci/pipeline_schedules.rb
@@ -6,11 +6,16 @@ module Ci
class PipelineSchedules < BasePermissionType
graphql_name 'PipelineSchedulePermissions'
- abilities :take_ownership_pipeline_schedule,
- :update_pipeline_schedule,
+ abilities :update_pipeline_schedule,
:admin_pipeline_schedule
ability_field :play_pipeline_schedule, calls_gitaly: true
+ ability_field :take_ownership_pipeline_schedule,
+ deprecated: {
+ reason: 'Use admin_pipeline_schedule permission to determine if the user can take ownership ' \
+ 'of a pipeline schedule',
+ milestone: '15.9'
+ }
end
end
end
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index 3a674bfef92b3ee1b787eba4a425ffde67d908d6..7b0d484f9f7f02c4e06125285b1f24801c76903c 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -23,6 +23,10 @@ class PipelineSchedulePolicy < PipelinePolicy
enable :update_pipeline_schedule
end
+ # `take_ownership_pipeline_schedule` is deprecated, and should not be used. It can be removed in 17.0
+ # once the deprecated field `take_ownership_pipeline_schedule` is removed from the GraphQL type
+ # `PermissionTypes::Ci::PipelineSchedules`.
+ # Use `admin_pipeline_schedule` to decide if a user has the ability to take ownership of a pipeline schedule.
rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do
enable :take_ownership_pipeline_schedule
end
diff --git a/app/services/ci/pipeline_schedules/take_ownership_service.rb b/app/services/ci/pipeline_schedules/take_ownership_service.rb
index 9b4001c74bdee02fc474be3bedeaaca7fdb0ad70..b4d193cb8754232929e52567489545a1efbc9488 100644
--- a/app/services/ci/pipeline_schedules/take_ownership_service.rb
+++ b/app/services/ci/pipeline_schedules/take_ownership_service.rb
@@ -23,7 +23,7 @@ def execute
attr_reader :schedule, :user
def allowed?
- user.can?(:take_ownership_pipeline_schedule, schedule)
+ user.can?(:admin_pipeline_schedule, schedule)
end
def forbidden
diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
index 0de31f59033a9b2ee39349b4d1852651faf80950..37b2b3ecfdeb296c4b1238b089621cec29010455 100644
--- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
+++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml
@@ -37,7 +37,7 @@
- if can?(current_user, :play_pipeline_schedule, pipeline_schedule)
= link_to play_pipeline_schedule_path(pipeline_schedule), method: :post, title: _('Play'), class: 'btn gl-button btn-default btn-icon' do
= sprite_icon('play')
- - if can?(current_user, :take_ownership_pipeline_schedule, pipeline_schedule)
+ - if can?(current_user, :admin_pipeline_schedule, pipeline_schedule) && pipeline_schedule.owner != current_user
= render Pajamas::ButtonComponent.new(button_options: { class: 'js-take-ownership-button has-tooltip', title: s_('PipelineSchedule|Take ownership to edit'), data: { url: take_ownership_pipeline_schedule_path(pipeline_schedule) } }) do
= s_('PipelineSchedules|Take ownership')
- if can?(current_user, :update_pipeline_schedule, pipeline_schedule)
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index b91310cc9baf0089cffd50f6d420827c72837c86..ff663ef315632e1d2aa25cee5cc5cffebc36efc8 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -17615,7 +17615,7 @@ Represents a pipeline schedule.
| ---- | ---- | ----------- |
| <a id="pipelineschedulepermissionsadminpipelineschedule"></a>`adminPipelineSchedule` | [`Boolean!`](#boolean) | Indicates the user can perform `admin_pipeline_schedule` on this resource. |
| <a id="pipelineschedulepermissionsplaypipelineschedule"></a>`playPipelineSchedule` | [`Boolean!`](#boolean) | Indicates the user can perform `play_pipeline_schedule` on this resource. |
-| <a id="pipelineschedulepermissionstakeownershippipelineschedule"></a>`takeOwnershipPipelineSchedule` | [`Boolean!`](#boolean) | Indicates the user can perform `take_ownership_pipeline_schedule` on this resource. |
+| <a id="pipelineschedulepermissionstakeownershippipelineschedule"></a>`takeOwnershipPipelineSchedule` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 15.9. Use admin_pipeline_schedule permission to determine if the user can take ownership of a pipeline schedule. |
| <a id="pipelineschedulepermissionsupdatepipelineschedule"></a>`updatePipelineSchedule` | [`Boolean!`](#boolean) | Indicates the user can perform `update_pipeline_schedule` on this resource. |
### `PipelineScheduleVariable`
diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb
index afb3754f2ae0a2ba0267e5816b9ef0993eda97ac..e27ec24fb441e2ff71abf0beb09620772ce2f79e 100644
--- a/lib/api/ci/pipeline_schedules.rb
+++ b/lib/api/ci/pipeline_schedules.rb
@@ -141,9 +141,12 @@ class PipelineSchedules < ::API::Base
requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id', documentation: { example: 13 }
end
post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do
- authorize! :take_ownership_pipeline_schedule, pipeline_schedule
+ authorize! :admin_pipeline_schedule, pipeline_schedule
- if pipeline_schedule.own!(current_user)
+ if pipeline_schedule.owned_by?(current_user)
+ status(:ok) # Set response code to 200 if schedule is already owned by current user
+ present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails
+ elsif pipeline_schedule.own!(current_user)
present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails
else
render_validation_error!(pipeline_schedule)
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index a628c1ab23060d044117307d68b33e1927406315..6d810fdcd511790f65cb8330d461772be887f6c0 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -410,9 +410,9 @@ def go
it { expect { go }.to be_denied_for(:visitor) }
context 'when user is schedule owner' do
- it { expect { go }.to be_denied_for(:owner).of(project).own(pipeline_schedule) }
- it { expect { go }.to be_denied_for(:maintainer).of(project).own(pipeline_schedule) }
- it { expect { go }.to be_denied_for(:developer).of(project).own(pipeline_schedule) }
+ it { expect { go }.to be_allowed_for(:owner).of(project).own(pipeline_schedule) }
+ it { expect { go }.to be_allowed_for(:maintainer).of(project).own(pipeline_schedule) }
+ it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:reporter).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:guest).of(project).own(pipeline_schedule) }
it { expect { go }.to be_denied_for(:user).own(pipeline_schedule) }
diff --git a/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_spec.js b/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_spec.js
index 611993556e30f57101c106fd83ee767ebb3d7800..97d8f420bbf246b2f77ee33cbd1d04b1141cbfaf 100644
--- a/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_spec.js
+++ b/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_spec.js
@@ -16,6 +16,7 @@ import getPipelineSchedulesQuery from '~/ci/pipeline_schedules/graphql/queries/g
import {
mockGetPipelineSchedulesGraphQLResponse,
mockPipelineScheduleNodes,
+ mockPipelineScheduleCurrentUser,
deleteMutationResponse,
playMutationResponse,
takeOwnershipMutationResponse,
@@ -115,6 +116,7 @@ describe('Pipeline schedules app', () => {
await waitForPromises();
expect(findTable().props('schedules')).toEqual(mockPipelineScheduleNodes);
+ expect(findTable().props('currentUser')).toEqual(mockPipelineScheduleCurrentUser);
});
it('shows query error alert', async () => {
diff --git a/spec/frontend/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions_spec.js b/spec/frontend/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions_spec.js
index 6fb6a8bc33b8f5f66c67c829cd3be0a8fd249038..82ced866983a31a69478c3265afde4bc577bda70 100644
--- a/spec/frontend/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions_spec.js
+++ b/spec/frontend/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions_spec.js
@@ -3,6 +3,7 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import PipelineScheduleActions from '~/ci/pipeline_schedules/components/table/cells/pipeline_schedule_actions.vue';
import {
mockPipelineScheduleNodes,
+ mockPipelineScheduleCurrentUser,
mockPipelineScheduleAsGuestNodes,
mockTakeOwnershipNodes,
} from '../../../mock_data';
@@ -12,6 +13,7 @@ describe('Pipeline schedule actions', () => {
const defaultProps = {
schedule: mockPipelineScheduleNodes[0],
+ currentUser: mockPipelineScheduleCurrentUser,
};
const createComponent = (props = defaultProps) => {
@@ -31,14 +33,17 @@ describe('Pipeline schedule actions', () => {
wrapper.destroy();
});
- it('displays action buttons', () => {
+ it('displays buttons when user is the owner of schedule and has adminPipelineSchedule permissions', () => {
createComponent();
expect(findAllButtons()).toHaveLength(3);
});
- it('does not display action buttons', () => {
- createComponent({ schedule: mockPipelineScheduleAsGuestNodes[0] });
+ it('does not display action buttons when user is not owner and does not have adminPipelineSchedule permission', () => {
+ createComponent({
+ schedule: mockPipelineScheduleAsGuestNodes[0],
+ currentUser: mockPipelineScheduleCurrentUser,
+ });
expect(findAllButtons()).toHaveLength(0);
});
@@ -54,7 +59,10 @@ describe('Pipeline schedule actions', () => {
});
it('take ownership button emits showTakeOwnershipModal event and schedule id', () => {
- createComponent({ schedule: mockTakeOwnershipNodes[0] });
+ createComponent({
+ schedule: mockTakeOwnershipNodes[0],
+ currentUser: mockPipelineScheduleCurrentUser,
+ });
findTakeOwnershipBtn().vm.$emit('click');
diff --git a/spec/frontend/ci/pipeline_schedules/components/table/pipeline_schedules_table_spec.js b/spec/frontend/ci/pipeline_schedules/components/table/pipeline_schedules_table_spec.js
index 316b3bcf9265d0bba767221cc17de1e3a070dbb8..c777c4bc9cb4a2ef1ea57b5ba737957438c41232 100644
--- a/spec/frontend/ci/pipeline_schedules/components/table/pipeline_schedules_table_spec.js
+++ b/spec/frontend/ci/pipeline_schedules/components/table/pipeline_schedules_table_spec.js
@@ -1,13 +1,14 @@
import { GlTableLite } from '@gitlab/ui';
import { mountExtended } from 'helpers/vue_test_utils_helper';
import PipelineSchedulesTable from '~/ci/pipeline_schedules/components/table/pipeline_schedules_table.vue';
-import { mockPipelineScheduleNodes } from '../../mock_data';
+import { mockPipelineScheduleNodes, mockPipelineScheduleCurrentUser } from '../../mock_data';
describe('Pipeline schedules table', () => {
let wrapper;
const defaultProps = {
schedules: mockPipelineScheduleNodes,
+ currentUser: mockPipelineScheduleCurrentUser,
};
const createComponent = (props = defaultProps) => {
diff --git a/spec/frontend/ci/pipeline_schedules/mock_data.js b/spec/frontend/ci/pipeline_schedules/mock_data.js
index 2826c054249a6ded606d8d91857c73260f70c16c..1485f6beea48a20c6a8e3517c0b73266e454b478 100644
--- a/spec/frontend/ci/pipeline_schedules/mock_data.js
+++ b/spec/frontend/ci/pipeline_schedules/mock_data.js
@@ -5,6 +5,7 @@ import mockGetPipelineSchedulesTakeOwnershipGraphQLResponse from 'test_fixtures/
const {
data: {
+ currentUser,
project: {
pipelineSchedules: { nodes },
},
@@ -28,6 +29,7 @@ const {
} = mockGetPipelineSchedulesTakeOwnershipGraphQLResponse;
export const mockPipelineScheduleNodes = nodes;
+export const mockPipelineScheduleCurrentUser = currentUser;
export const mockPipelineScheduleAsGuestNodes = guestNodes;
diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb
index 9aa50876b55ccdc4cb4193b9ee2d508db1c7da99..92ad37145c08b77577e05bcb4033ede4740e97f1 100644
--- a/spec/policies/ci/pipeline_schedule_policy_spec.rb
+++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb
@@ -104,7 +104,7 @@
end
it 'includes abilities to take ownership' do
- expect(policy).to be_allowed :take_ownership_pipeline_schedule
+ expect(policy).to be_allowed :admin_pipeline_schedule
end
end
end
diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb
index 2a2c5f65aee702ca280184eb1446b43e5c8ef600..d760e4ddf2887fea28ea7d9e07fdca998d9e2566 100644
--- a/spec/requests/api/ci/pipeline_schedules_spec.rb
+++ b/spec/requests/api/ci/pipeline_schedules_spec.rb
@@ -473,12 +473,12 @@ def active?(str)
end
context 'as the existing owner of the schedule' do
- it 'rejects the request and leaves the schedule unchanged' do
+ it 'accepts the request and leaves the schedule unchanged' do
expect do
post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer)
end.not_to change { pipeline_schedule.reload.owner }
- expect(response).to have_gitlab_http_status(:forbidden)
+ expect(response).to have_gitlab_http_status(:success)
end
end
end
diff --git a/spec/views/projects/pipeline_schedules/_pipeline_schedule.html.haml_spec.rb b/spec/views/projects/pipeline_schedules/_pipeline_schedule.html.haml_spec.rb
index 37c9908af1d56aa5ad6d2a6b693943a9243fb7c9..13ec7207ec912925e56621a37c4f8471b3375768 100644
--- a/spec/views/projects/pipeline_schedules/_pipeline_schedule.html.haml_spec.rb
+++ b/spec/views/projects/pipeline_schedules/_pipeline_schedule.html.haml_spec.rb
@@ -22,7 +22,7 @@
let(:user) { maintainer }
before do
- allow(view).to receive(:can?).with(maintainer, :take_ownership_pipeline_schedule, pipeline_schedule).and_return(true)
+ allow(view).to receive(:can?).with(maintainer, :admin_pipeline_schedule, pipeline_schedule).and_return(true)
end
it 'non-owner can take ownership of pipeline' do
@@ -36,7 +36,7 @@
let(:user) { owner }
before do
- allow(view).to receive(:can?).with(owner, :take_ownership_pipeline_schedule, pipeline_schedule).and_return(false)
+ allow(view).to receive(:can?).with(owner, :admin_pipeline_schedule, pipeline_schedule).and_return(false)
end
it 'owner cannot take ownership of pipeline' do