Merge branch 'move-permission-check-manual-actions-on-deployments' into 'master'

Move permission check of manual actions of deployments

Closes #52412

See merge request gitlab-org/gitlab-ce!24660

17 files changed, 48 insertions, 50 deletions

diff --git a/app/assets/javascripts/environments/components/container.vue b/app/assets/javascripts/environments/components/container.vue
index bd402c0eea5..6ece8b92a30 100644
--- a/app/assets/javascripts/environments/components/container.vue
+++ b/app/assets/javascripts/environments/components/container.vue
@@ -22,10 +22,6 @@ export default {
       type: Object,
       required: true,
     },
-    canCreateDeployment: {
-      type: Boolean,
-      required: true,
-    },
     canReadEnvironment: {
       type: Boolean,
       required: true,
@@ -51,11 +47,7 @@ export default {
     <slot name="emptyState"></slot>
 
     <div v-if="!isLoading && environments.length > 0" class="table-holder">
-      <environment-table
-        :environments="environments"
-        :can-create-deployment="canCreateDeployment"
-        :can-read-environment="canReadEnvironment"
-      />
+      <environment-table :environments="environments" :can-read-environment="canReadEnvironment" />
 
       <table-pagination
         v-if="pagination && pagination.totalPages > 1"
diff --git a/app/assets/javascripts/environments/components/environment_item.vue b/app/assets/javascripts/environments/components/environment_item.vue
index f44806d82a6..503c1b38f71 100644
--- a/app/assets/javascripts/environments/components/environment_item.vue
+++ b/app/assets/javascripts/environments/components/environment_item.vue
@@ -47,12 +47,6 @@ export default {
       default: () => ({}),
     },
 
-    canCreateDeployment: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-
     canReadEnvironment: {
       type: Boolean,
       required: false,
@@ -151,7 +145,7 @@ export default {
     },
 
     actions() {
-      if (!this.model || !this.model.last_deployment || !this.canCreateDeployment) {
+      if (!this.model || !this.model.last_deployment) {
         return [];
       }
 
@@ -561,7 +555,7 @@ export default {
         />
 
         <rollback-component
-          v-if="canRetry && canCreateDeployment"
+          v-if="canRetry"
           :is-last-deployment="isLastDeployment"
           :retry-url="retryUrl"
         />
diff --git a/app/assets/javascripts/environments/components/environments_app.vue b/app/assets/javascripts/environments/components/environments_app.vue
index 87c1d44dd40..aa2417d3194 100644
--- a/app/assets/javascripts/environments/components/environments_app.vue
+++ b/app/assets/javascripts/environments/components/environments_app.vue
@@ -24,10 +24,6 @@ export default {
       type: Boolean,
       required: true,
     },
-    canCreateDeployment: {
-      type: Boolean,
-      required: true,
-    },
     canReadEnvironment: {
       type: Boolean,
       required: true,
@@ -106,7 +102,6 @@ export default {
       :is-loading="isLoading"
       :environments="state.environments"
       :pagination="state.paginationInformation"
-      :can-create-deployment="canCreateDeployment"
       :can-read-environment="canReadEnvironment"
       @onChangePage="onChangePage"
     >
diff --git a/app/assets/javascripts/environments/components/environments_table.vue b/app/assets/javascripts/environments/components/environments_table.vue
index 75bdf87137f..e2c304de00a 100644
--- a/app/assets/javascripts/environments/components/environments_table.vue
+++ b/app/assets/javascripts/environments/components/environments_table.vue
@@ -23,12 +23,6 @@ export default {
       required: false,
       default: false,
     },
-
-    canCreateDeployment: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
   },
   methods: {
     folderUrl(model) {
@@ -64,7 +58,6 @@ export default {
         is="environment-item"
         :key="`environment-item-${i}`"
         :model="model"
-        :can-create-deployment="canCreateDeployment"
         :can-read-environment="canReadEnvironment"
       />
 
@@ -79,7 +72,6 @@ export default {
             v-for="(children, index) in model.children"
             :key="`env-item-${i}-${index}`"
             :model="children"
-            :can-create-deployment="canCreateDeployment"
             :can-read-environment="canReadEnvironment"
           />
 
diff --git a/app/assets/javascripts/environments/folder/environments_folder_bundle.js b/app/assets/javascripts/environments/folder/environments_folder_bundle.js
index 982e550e73c..56e7f69cad6 100644
--- a/app/assets/javascripts/environments/folder/environments_folder_bundle.js
+++ b/app/assets/javascripts/environments/folder/environments_folder_bundle.js
@@ -18,7 +18,6 @@ export default () =>
         endpoint: environmentsData.environmentsDataEndpoint,
         folderName: environmentsData.environmentsDataFolderName,
         cssContainerClass: environmentsData.cssClass,
-        canCreateDeployment: parseBoolean(environmentsData.environmentsDataCanCreateDeployment),
         canReadEnvironment: parseBoolean(environmentsData.environmentsDataCanReadEnvironment),
       };
     },
@@ -28,7 +27,6 @@ export default () =>
           endpoint: this.endpoint,
           folderName: this.folderName,
           cssContainerClass: this.cssContainerClass,
-          canCreateDeployment: this.canCreateDeployment,
           canReadEnvironment: this.canReadEnvironment,
         },
       });
diff --git a/app/assets/javascripts/environments/folder/environments_folder_view.vue b/app/assets/javascripts/environments/folder/environments_folder_view.vue
index d6f0b6115a6..80f0e00400b 100644
--- a/app/assets/javascripts/environments/folder/environments_folder_view.vue
+++ b/app/assets/javascripts/environments/folder/environments_folder_view.vue
@@ -23,10 +23,6 @@ export default {
       type: String,
       required: true,
     },
-    canCreateDeployment: {
-      type: Boolean,
-      required: true,
-    },
     canReadEnvironment: {
       type: Boolean,
       required: true,
@@ -55,7 +51,6 @@ export default {
       :is-loading="isLoading"
       :environments="state.environments"
       :pagination="state.paginationInformation"
-      :can-create-deployment="canCreateDeployment"
       :can-read-environment="canReadEnvironment"
       @onChangePage="onChangePage"
     />
diff --git a/app/assets/javascripts/environments/index.js b/app/assets/javascripts/environments/index.js
index d366e7550b7..6af66d0f86e 100644
--- a/app/assets/javascripts/environments/index.js
+++ b/app/assets/javascripts/environments/index.js
@@ -20,7 +20,6 @@ export default () =>
         helpPagePath: environmentsData.helpPagePath,
         cssContainerClass: environmentsData.cssClass,
         canCreateEnvironment: parseBoolean(environmentsData.canCreateEnvironment),
-        canCreateDeployment: parseBoolean(environmentsData.canCreateDeployment),
         canReadEnvironment: parseBoolean(environmentsData.canReadEnvironment),
       };
     },
@@ -32,7 +31,6 @@ export default () =>
           helpPagePath: this.helpPagePath,
           cssContainerClass: this.cssContainerClass,
           canCreateEnvironment: this.canCreateEnvironment,
-          canCreateDeployment: this.canCreateDeployment,
           canReadEnvironment: this.canReadEnvironment,
         },
       });
diff --git a/app/helpers/environments_helper.rb b/app/helpers/environments_helper.rb
index b3935ae350d..365b94f5a3e 100644
--- a/app/helpers/environments_helper.rb
+++ b/app/helpers/environments_helper.rb
@@ -11,7 +11,6 @@ module EnvironmentsHelper
     {
       "endpoint" => folder_project_environments_path(@project, @folder, format: :json),
       "folder-name" => @folder,
-      "can-create-deployment" => can?(current_user, :create_deployment, @project).to_s,
       "can-read-environment" => can?(current_user, :read_environment, @project).to_s
     }
   end
diff --git a/app/serializers/deployment_entity.rb b/app/serializers/deployment_entity.rb
index aa1d9e6292c..34ae06278c8 100644
--- a/app/serializers/deployment_entity.rb
+++ b/app/serializers/deployment_entity.rb
@@ -24,6 +24,12 @@ class DeploymentEntity < Grape::Entity
   expose :user, using: UserEntity
   expose :commit, using: CommitEntity
   expose :deployable, using: JobEntity
-  expose :manual_actions, using: JobEntity
-  expose :scheduled_actions, using: JobEntity
+  expose :manual_actions, using: JobEntity, if: -> (*) { can_create_deployment? }
+  expose :scheduled_actions, using: JobEntity, if: -> (*) { can_create_deployment? }
+
+  private
+
+  def can_create_deployment?
+    can?(request.current_user, :create_deployment, request.project)
+  end
 end
diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml
index d66de7ab698..99cbbc11acd 100644
--- a/app/views/projects/environments/index.html.haml
+++ b/app/views/projects/environments/index.html.haml
@@ -2,7 +2,6 @@
 - page_title _("Environments")
 
 #environments-list-view{ data: { environments_data: environments_list_data,
-  "can-create-deployment" => can?(current_user, :create_deployment, @project).to_s,
   "can-read-environment" => can?(current_user, :read_environment, @project).to_s,
   "can-create-environment" => can?(current_user, :create_environment, @project).to_s,
   "new-environment-path" => new_project_environment_path(@project),
diff --git a/changelogs/unreleased/move-permission-check-manual-actions-on-deployments.yml b/changelogs/unreleased/move-permission-check-manual-actions-on-deployments.yml
new file mode 100644
index 00000000000..9e979b48ad1
--- /dev/null
+++ b/changelogs/unreleased/move-permission-check-manual-actions-on-deployments.yml
@@ -0,0 +1,5 @@
+---
+title: Move permission check of manual actions of deployments
+merge_request: 24660
+author:
+type: other
diff --git a/spec/javascripts/environments/environment_item_spec.js b/spec/javascripts/environments/environment_item_spec.js
index 7618c2f50ce..a89e50045da 100644
--- a/spec/javascripts/environments/environment_item_spec.js
+++ b/spec/javascripts/environments/environment_item_spec.js
@@ -25,7 +25,6 @@ describe('Environment item', () => {
       component = new EnvironmentItem({
         propsData: {
           model: mockItem,
-          canCreateDeployment: false,
           canReadEnvironment: true,
           service: {},
         },
@@ -117,7 +116,6 @@ describe('Environment item', () => {
       component = new EnvironmentItem({
         propsData: {
           model: environment,
-          canCreateDeployment: true,
           canReadEnvironment: true,
           service: {},
         },
diff --git a/spec/javascripts/environments/environment_table_spec.js b/spec/javascripts/environments/environment_table_spec.js
index 0e5e50a59a5..52895f35f3a 100644
--- a/spec/javascripts/environments/environment_table_spec.js
+++ b/spec/javascripts/environments/environment_table_spec.js
@@ -26,7 +26,6 @@ describe('Environment table', () => {
 
     vm = mountComponent(Component, {
       environments: [mockItem],
-      canCreateDeployment: false,
       canReadEnvironment: true,
     });
 
diff --git a/spec/javascripts/environments/environments_app_spec.js b/spec/javascripts/environments/environments_app_spec.js
index e2d81eb454a..9220f7a264f 100644
--- a/spec/javascripts/environments/environments_app_spec.js
+++ b/spec/javascripts/environments/environments_app_spec.js
@@ -9,7 +9,6 @@ describe('Environment', () => {
   const mockData = {
     endpoint: 'environments.json',
     canCreateEnvironment: true,
-    canCreateDeployment: true,
     canReadEnvironment: true,
     cssContainerClass: 'container',
     newEnvironmentPath: 'environments/new',
diff --git a/spec/javascripts/environments/folder/environments_folder_view_spec.js b/spec/javascripts/environments/folder/environments_folder_view_spec.js
index 7f0a9475d5f..d9ee7e74e28 100644
--- a/spec/javascripts/environments/folder/environments_folder_view_spec.js
+++ b/spec/javascripts/environments/folder/environments_folder_view_spec.js
@@ -13,7 +13,6 @@ describe('Environments Folder View', () => {
   const mockData = {
     endpoint: 'environments.json',
     folderName: 'review',
-    canCreateDeployment: true,
     canReadEnvironment: true,
     cssContainerClass: 'container',
   };
diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb
index cfa5414b40f..894fd7a0a12 100644
--- a/spec/serializers/deployment_entity_spec.rb
+++ b/spec/serializers/deployment_entity_spec.rb
@@ -1,14 +1,22 @@
 require 'spec_helper'
 
 describe DeploymentEntity do
-  let(:user) { create(:user) }
+  let(:user) { developer }
+  let(:developer) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:project) { create(:project) }
   let(:request) { double('request') }
-  let(:deployment) { create(:deployment) }
+  let(:deployment) { create(:deployment, deployable: build, project: project) }
+  let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
+  let(:pipeline) { create(:ci_pipeline, project: project, user: user) }
   let(:entity) { described_class.new(deployment, request: request) }
   subject { entity.as_json }
 
   before do
+    project.add_developer(developer)
+    project.add_reporter(reporter)
     allow(request).to receive(:current_user).and_return(user)
+    allow(request).to receive(:project).and_return(project)
   end
 
   it 'exposes internal deployment id' do
@@ -23,6 +31,24 @@ describe DeploymentEntity do
     expect(subject).to include(:created_at)
   end
 
+  context 'when the pipeline has another manual action' do
+    let(:other_build) { create(:ci_build, :manual, name: 'another deploy', pipeline: pipeline) }
+    let!(:other_deployment) { create(:deployment, deployable: other_build) }
+
+    it 'returns another manual action' do
+      expect(subject[:manual_actions].count).to eq(1)
+      expect(subject[:manual_actions].first[:name]).to eq('another deploy')
+    end
+
+    context 'when user is a reporter' do
+      let(:user) { reporter }
+
+      it 'returns another manual action' do
+        expect(subject[:manual_actions]).not_to be_present
+      end
+    end
+  end
+
   describe 'scheduled_actions' do
     let(:project) { create(:project, :repository) }
     let(:pipeline) { create(:ci_pipeline, project: project, user: user) }
diff --git a/spec/serializers/environment_serializer_spec.rb b/spec/serializers/environment_serializer_spec.rb
index 87493a28d1f..3541bd5f12e 100644
--- a/spec/serializers/environment_serializer_spec.rb
+++ b/spec/serializers/environment_serializer_spec.rb
@@ -10,6 +10,10 @@ describe EnvironmentSerializer do
       .represent(resource)
   end
 
+  before do
+    project.add_developer(user)
+  end
+
   context 'when there is a single object provided' do
     let(:project) { create(:project, :repository) }
     let(:deployable) { create(:ci_build) }