Merge branch 'error-tracking-api-followup' into 'master'

Make follow up fixes for enable error tracking api

See merge request gitlab-org/gitlab!24892

app/models/error_tracking/project_error_tracking_setting.rb

@@ -27,6 +27,8 @@ module ErrorTracking
 
     validates :api_url, length: { maximum: 255 }, public_url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
 
+    validates :enabled, inclusion: { in: [true, false] }
+
     validates :api_url, presence: { message: 'is a required field' }, if: :enabled
 
     validate :validate_api_url_path, if: :enabled

changelogs/unreleased/error-tracking-api-followup.yml

+---
+title: Refactor error tracking specs and add validation to enabled field in error tracking model
+merge_request: 24892
+author: Rajendra Kadam
+type: added

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -19,6 +19,19 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
     it { is_expected.to allow_value("http://gitlab.com/api/0/projects/project1/something").for(:api_url) }
     it { is_expected.not_to allow_values("http://gitlab.com/api/0/projects/project1/something€").for(:api_url) }
 
+    it 'disallows non-booleans in enabled column' do
+      is_expected.not_to allow_value(
+        nil
+      ).for(:enabled)
+    end
+
+    it 'allows booleans in enabled column' do
+      is_expected.to allow_value(
+        true,
+        false
+      ).for(:enabled)
+    end
+
     it 'rejects invalid api_urls' do
       is_expected.not_to allow_values(
         "https://replaceme.com/'><script>alert(document.cookie)</script>", # unsafe

spec/requests/api/error_tracking_spec.rb

@@ -3,13 +3,13 @@
 require 'spec_helper'
 
 describe API::ErrorTracking do
-  let(:user) { create(:user) }
+  let_it_be(:user) { create(:user) }
   let(:setting) { create(:project_error_tracking_setting) }
   let(:project) { setting.project }
 
   shared_examples 'returns project settings' do
     it 'returns correct project settings' do
-      subject
+      make_request
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(json_response).to eq(
@@ -23,7 +23,7 @@ describe API::ErrorTracking do
 
   shared_examples 'returns 404' do
     it 'returns correct project settings' do
-      subject
+      make_request
 
       expect(response).to have_gitlab_http_status(:not_found)
       expect(json_response['message'])
@@ -32,7 +32,9 @@ describe API::ErrorTracking do
   end
 
   describe "PATCH /projects/:id/error_tracking/settings" do
-    def make_patch_request(**params)
+    let(:params) { { active: false } }
+
+    def make_request
       patch api("/projects/#{project.id}/error_tracking/settings", user), params: params
     end
 
@@ -42,26 +44,39 @@ describe API::ErrorTracking do
       end
 
       context 'patch settings' do
-        subject do
-          make_patch_request(active: false)
+        it_behaves_like 'returns project settings'
+
+        it 'updates enabled flag' do
+          expect(setting).to be_enabled
+
+          make_request
+
+          expect(json_response).to include('active' => false)
+          expect(setting.reload).not_to be_enabled
         end
 
-        it_behaves_like 'returns project settings'
+        context 'active is invalid' do
+          let(:params) { { active: "randomstring" } }
 
-        it 'returns active is invalid if non boolean' do
-          make_patch_request(active: "randomstring")
+          it 'returns active is invalid if non boolean' do
+            make_request
 
-          expect(response).to have_gitlab_http_status(:bad_request)
-          expect(json_response['error'])
-            .to eq('active is invalid')
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['error'])
+              .to eq('active is invalid')
+          end
         end
 
-        it 'returns 400 if active is empty' do
-          make_patch_request(active: '')
+        context 'active is empty' do
+          let(:params) { { active: '' } }
+
+          it 'returns 400' do
+            make_request
 
-          expect(response).to have_gitlab_http_status(:bad_request)
-          expect(json_response['error'])
-            .to eq('active is empty')
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['error'])
+              .to eq('active is empty')
+          end
         end
       end
 
@@ -73,10 +88,6 @@ describe API::ErrorTracking do
         end
 
         context 'patch settings' do
-          subject do
-            make_patch_request(active: true)
-          end
-
           it_behaves_like 'returns 404'
         end
       end
@@ -87,10 +98,12 @@ describe API::ErrorTracking do
         project.add_reporter(user)
       end
 
-      it 'returns 403 for update request' do
-        make_patch_request(active: true)
+      context 'patch request' do
+        it 'returns 403' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:forbidden)
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
       end
     end
 
@@ -99,28 +112,34 @@ describe API::ErrorTracking do
         project.add_developer(user)
       end
 
-      it 'returns 403 for update request' do
-        make_patch_request(active: true)
+      context 'patch request' do
+        it 'returns 403' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:forbidden)
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
       end
     end
 
     context 'when authenticated as non-member' do
-      it 'returns 404 for update request' do
-        make_patch_request(active: false)
+      context 'patch request' do
+        it 'returns 404' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:not_found)
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
 
     context 'when unauthenticated' do
       let(:user) { nil }
 
-      it 'returns 401 for update request' do
-        make_patch_request(active: true)
+      context 'patch request' do
+        it 'returns 401 for update request' do
+          make_request
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
       end
     end
   end
@@ -136,10 +155,6 @@ describe API::ErrorTracking do
       end
 
       context 'get settings' do
-        subject do
-          make_request
-        end
-
         it_behaves_like 'returns project settings'
       end
     end
@@ -152,10 +167,6 @@ describe API::ErrorTracking do
       end
 
       context 'get settings' do
-        subject do
-          make_request
-        end
-
         it_behaves_like 'returns 404'
       end
     end