Update remaining endpoints

9 files changed, 18 insertions, 20 deletions

diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb
index f64da4ab77b..25152f30998 100644
--- a/lib/api/group_variables.rb
+++ b/lib/api/group_variables.rb
@@ -88,8 +88,7 @@ module API
         variable = user_group.variables.find_by(key: params[:key])
         not_found!('GroupVariable') unless variable
 
-        status 204
-        variable.destroy
+        destroy_conditionally!(variable)
       end
     end
   end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 8d4f8c01903..84980864151 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -14,7 +14,7 @@ module API
     def check_unmodified_since!(last_modified)
       if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) rescue nil
 
-      if if_unmodified_since && last_modified > if_unmodified_since
+      if if_unmodified_since && last_modified && last_modified > if_unmodified_since
         render_api_error!('412 Precondition Failed', 412)
       end
     end
diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb
index dbeaf9e17ef..e3123ef4e2d 100644
--- a/lib/api/pipeline_schedules.rb
+++ b/lib/api/pipeline_schedules.rb
@@ -117,8 +117,7 @@ module API
         not_found!('PipelineSchedule') unless pipeline_schedule
         authorize! :admin_pipeline_schedule, pipeline_schedule
 
-        status :accepted
-        present pipeline_schedule.destroy, with: Entities::PipelineScheduleDetails
+        destroy_conditionally!(pipeline_schedule)
       end
     end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 36fe3f243b9..78e25b6da03 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -1,7 +1,6 @@
 require_dependency 'declarative_policy'
 
 module API
-  # Projects API
   class Projects < Grape::API
     include PaginationParams
 
diff --git a/lib/api/protected_branches.rb b/lib/api/protected_branches.rb
index dccf4fa27a7..15fcb9e8e27 100644
--- a/lib/api/protected_branches.rb
+++ b/lib/api/protected_branches.rb
@@ -76,9 +76,7 @@ module API
       delete ':id/protected_branches/:name', requirements: BRANCH_ENDPOINT_REQUIREMENTS do
         protected_branch = user_project.protected_branches.find_by!(name: params[:name])
 
-        protected_branch.destroy
-
-        status 204
+        destroy_conditionally!(protected_branch)
       end
     end
   end
diff --git a/lib/api/services.rb b/lib/api/services.rb
index 2b5ef75b6bf..ff9ddd44439 100644
--- a/lib/api/services.rb
+++ b/lib/api/services.rb
@@ -655,15 +655,15 @@ module API
       end
       delete ":id/services/:service_slug" do
         service = user_project.find_or_initialize_service(params[:service_slug].underscore)
-        # Todo: Check if this done the right way
-        check_unmodified_since!(service.updated_at)
 
-        attrs = service_attributes(service).inject({}) do |hash, key|
-          hash.merge!(key => nil)
-        end
+        destroy_conditionally!(service) do
+          attrs = service_attributes(service).inject({}) do |hash, key|
+            hash.merge!(key => nil)
+          end
 
-        unless service.update_attributes(attrs.merge(active: false))
-          render_api_error!('400 Bad Request', 400)
+          unless service.update_attributes(attrs.merge(active: false))
+            render_api_error!('400 Bad Request', 400)
+          end
         end
       end
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index d7c7b9ae9c1..96f47bb618a 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -408,8 +408,11 @@ module API
             requires :impersonation_token_id, type: Integer, desc: 'The ID of the impersonation token'
           end
           delete ':impersonation_token_id' do
-            status 204
-            find_impersonation_token.revoke!
+            token = find_impersonation_token
+
+            destroy_conditionally!(token) do
+              token.revoke!
+            end
           end
         end
       end
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 7c0fdd3d1be..da71787abab 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -88,6 +88,7 @@ module API
         variable = user_project.variables.find_by(key: params[:key])
         not_found!('Variable') unless variable
 
+        # Variables don't have any timestamp. Therfore, destroy unconditionally.
         status 204
         variable.destroy
       end
diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb
index 1fc0ec528b9..150a391bb8d 100644
--- a/spec/requests/api/pipeline_schedules_spec.rb
+++ b/spec/requests/api/pipeline_schedules_spec.rb
@@ -267,8 +267,7 @@ describe API::PipelineSchedules do
           delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", 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')
+        expect(response).to have_http_status(204)
       end
 
       it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do