Add spec (Halfway)

5 files changed, 216 insertions, 62 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 09d7d9ad349..81cf7039260 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -819,7 +819,7 @@ module API
 
     class Variable < Grape::Entity
       expose :key, :value
-      expose :protected?, as: :protected, if: -> (entity, options) { entity.respond_to?(protected?) }
+      expose :protected?, as: :protected, if: -> (entity, options) { entity.respond_to?(:protected?) }
     end
 
     class Pipeline < PipelineBasic
diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb
index e82b974c8cd..ffb9a09834d 100644
--- a/lib/api/pipeline_schedules.rb
+++ b/lib/api/pipeline_schedules.rb
@@ -120,74 +120,71 @@ module API
         destroy_conditionally!(pipeline_schedule)
       end
 
+      desc 'Create a new pipeline schedule variable' do
+        success Entities::Variable
+      end
       params do
         requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id'
+        requires :key, type: String, desc: 'The key of the variable'
+        requires :value, type: String, desc: 'The value of the variable'
       end
-      resource :variables, requirements: { pipeline_schedule_id: %r{[^/]+} } do
-        desc 'Create a new pipeline schedule variable' do
-          success Entities::PipelineScheduleDetails
-        end
-        params do
-          requires :key, type: String, desc: 'The key of the variable'
-          requires :value, type: String, desc: 'The value of the variable'
-        end
-        post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do
-          authorize! :read_pipeline_schedule, user_project
-
-          not_found!('PipelineSchedule') unless pipeline_schedule
-          authorize! :update_pipeline_schedule, pipeline_schedule
+      post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do
+        authorize! :read_pipeline_schedule, user_project
 
-          variable_params = declared_params(include_missing: false)
-          variable = pipeline_schedule.variables.create(variable_params)
+        not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :update_pipeline_schedule, pipeline_schedule
 
-          if variable.persisted?
-            present variable, with: Entities::Variable
-          else
-            render_validation_error!(variable)
-          end
+        variable_params = declared_params(include_missing: false)
+        variable = pipeline_schedule.variables.create(variable_params)
+        if variable.persisted?
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
         end
+      end
 
-        desc 'Edit a pipeline schedule variable' do
-          success Entities::PipelineScheduleDetails
-        end
-        params do
-          optional :key, type: String, desc: 'The key of the variable'
-          optional :value, type: String, desc: 'The value of the variable'
-        end
-        put ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
-          authorize! :read_pipeline_schedule, user_project
+      desc 'Edit a pipeline schedule variable' do
+        success Entities::Variable
+      end
+      params do
+        requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id'
+        requires :key, type: String, desc: 'The key of the variable'
+        optional :value, type: String, desc: 'The value of the variable'
+      end
+      put ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
+        authorize! :read_pipeline_schedule, user_project
 
-          not_found!('PipelineSchedule') unless pipeline_schedule
-          authorize! :update_pipeline_schedule, pipeline_schedule
+        not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :update_pipeline_schedule, pipeline_schedule
 
-          variable = pipeline_schedule.variables.find_by(key: params[:key])
-          not_found!('Variable') unless variable
+        variable = pipeline_schedule.variables.find_by(key: params[:key])
+        not_found!('Variable') unless variable
 
-          if variable.update(declared_params(include_missing: false))
-            present variable, with: Entities::Variable
-          else
-            render_validation_error!(variable)
-          end
+        if variable.update(declared_params(include_missing: false))
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
         end
+      end
 
-        desc 'Delete a pipeline schedule variable' do
-          success Entities::PipelineScheduleDetails
-        end
-        params do
-          requires :key, type: String, desc: 'The key of the variable'
-        end
-        delete ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
-          authorize! :read_pipeline_schedule, user_project
+      desc 'Delete a pipeline schedule variable' do
+        success Entities::Variable
+      end
+      params do
+        requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id'
+        requires :key, type: String, desc: 'The key of the variable'
+      end
+      delete ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
+        authorize! :read_pipeline_schedule, user_project
 
-          not_found!('PipelineSchedule') unless pipeline_schedule
-          authorize! :admin_pipeline_schedule, pipeline_schedule
+        not_found!('PipelineSchedule') unless pipeline_schedule
+        authorize! :admin_pipeline_schedule, pipeline_schedule
 
-          variable = pipeline_schedule.variables.find_by(key: params[:key])
-          not_found!('Variable') unless variable
+        variable = pipeline_schedule.variables.find_by(key: params[:key])
+        not_found!('Variable') unless variable
 
-          status :accepted
-          present variable, with: Entities::Variable
-        end
+        status :accepted
+        present variable, with: Entities::Variable
       end
     end
 
diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json
index 16bbeacddbd..c76c6945117 100644
--- a/spec/fixtures/api/schemas/pipeline_schedule.json
+++ b/spec/fixtures/api/schemas/pipeline_schedule.json
@@ -34,14 +34,7 @@
     },
     "variables": {
       "type": ["array", "null"],
-      "items": {
-        "type": ["object", "null"],
-        "properties": {
-          "key": { "type": "string" },
-          "value": { "type": "string" }
-        },
-        "additionalProperties": false
-      }
+      "items": { "$ref": "pipeline_schedule_variable.json" }
     }
   },
   "required": [
diff --git a/spec/fixtures/api/schemas/pipeline_schedule_variable.json b/spec/fixtures/api/schemas/pipeline_schedule_variable.json
new file mode 100644
index 00000000000..f7ccb2d44a0
--- /dev/null
+++ b/spec/fixtures/api/schemas/pipeline_schedule_variable.json
@@ -0,0 +1,8 @@
+{
+  "type": ["object", "null"],
+  "properties": {
+    "key": { "type": "string" },
+    "value": { "type": "string" }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb
index b6a5a7ffbb5..e55e3617b5a 100644
--- a/spec/requests/api/pipeline_schedules_spec.rb
+++ b/spec/requests/api/pipeline_schedules_spec.rb
@@ -299,4 +299,160 @@ describe API::PipelineSchedules do
       end
     end
   end
+
+  describe 'POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables' do
+    let(:params) { attributes_for(:ci_pipeline_schedule_variable) }
+
+    let(:pipeline_schedule) do
+      create(:ci_pipeline_schedule, project: project, owner: developer)
+    end
+
+    context 'authenticated user with valid permissions' do
+      context 'with required parameters' do
+        it 'creates pipeline_schedule_variable' do
+          expect do
+            post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer),
+              params
+          end.to change { pipeline_schedule.variables.count }.by(1)
+
+          expect(response).to have_http_status(:created)
+          expect(response).to match_response_schema('pipeline_schedule_variable')
+          expect(json_response['key']).to eq(params[:key])
+          expect(json_response['value']).to eq(params[:value])
+        end
+      end
+
+      context 'without required parameters' do
+        it 'does not create pipeline_schedule_variable' do
+          post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer)
+
+          expect(response).to have_http_status(:bad_request)
+        end
+      end
+
+      context 'when cron has validation error' do
+        it 'does not create pipeline_schedule_variable' do
+          post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer),
+            params.merge('key' => '!?!?')
+
+          expect(response).to have_http_status(:bad_request)
+          expect(json_response['message']).to have_key('key')
+        end
+      end
+    end
+
+    context 'authenticated user with invalid permissions' do
+      it 'does not create pipeline_schedule_variable' do
+        post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", user), params
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'unauthenticated user' do
+      it 'does not create pipeline_schedule_variable' do
+        post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables"), params
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
+    let(:pipeline_schedule) do
+      create(:ci_pipeline_schedule, project: project, owner: developer)
+    end
+
+    let(:pipeline_schedule_variable) do
+      create(:ci_pipeline_schedule_variable, pipeline_schedule: pipeline_schedule)
+    end
+
+    context 'authenticated user with valid permissions' do
+      it 'updates cron' do
+        put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer),
+          key: pipeline_schedule_variable.key, value: 'updated_value'
+
+        expect(response).to have_http_status(:ok)
+        expect(response).to match_response_schema('pipeline_schedule_variable')
+        expect(json_response['key']).to eq('updated_value')
+      end
+
+      context 'when cron has validation error' do
+        it 'does not update pipeline_schedule_variable' do
+          put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer),
+            key: '!?!?'
+
+          expect(response).to have_http_status(:bad_request)
+          expect(json_response['message']).to have_key('key')
+        end
+      end
+    end
+
+    context 'authenticated user with invalid permissions' do
+      it 'does not update pipeline_schedule_variable' do
+        put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", user)
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'unauthenticated user' do
+      it 'does not update pipeline_schedule_variable' do
+        put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}")
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do
+    let(:master) { create(:user) }
+
+    let!(:pipeline_schedule) do
+      create(:ci_pipeline_schedule, project: project, owner: developer)
+    end
+
+    let(:pipeline_schedule_variable) do
+      create(:ci_pipeline_schedule_variable, pipeline_schedule: pipeline_schedule)
+    end
+
+    before do
+      project.add_master(master)
+    end
+
+    context 'authenticated user with valid permissions' do
+      it 'deletes pipeline_schedule_variable' do
+        expect do
+          delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", master)
+        end.to change { project.pipeline_schedules.count }.by(-1)
+
+        expect(response).to have_http_status(:accepted)
+        expect(response).to match_response_schema('pipeline_schedule_variable')
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing pipeline_schedule_variable' do
+        delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/____", master)
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'authenticated user with invalid permissions' do # TODO:
+      let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master) }
+
+      it 'does not delete pipeline_schedule_variable' do
+        delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer)
+
+        expect(response).to have_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthenticated user' do
+      it 'does not delete pipeline_schedule_variable' do
+        delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}")
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
 end