Add partial update method to update service and refactor specs

app/services/projects/operations/update_service.rb

@@ -30,6 +30,27 @@ module Projects
         settings = params[:error_tracking_setting_attributes]
         return {} if settings.blank?
 
+        if error_tracking_params_partial_updates?(settings)
+          error_tracking_params_for_partial_update(settings)
+        else
+          error_tracking_params_for_update(settings)
+        end
+      end
+
+      def error_tracking_params_partial_updates?(settings)
+        # Help from @splattael :bow:
+        # Make sure we're converting to symbols because
+        # * ActionController::Parameters#keys returns a list of strings
+        # * in specs we're using hashes with symbols as keys
+
+        settings.keys.map(&:to_sym) == %i[enabled]
+      end
+
+      def error_tracking_params_for_partial_update(settings)
+        { error_tracking_setting_attributes: settings }
+      end
+
+      def error_tracking_params_for_update(settings)
         api_url = ::ErrorTracking::ProjectErrorTrackingSetting.build_api_url_from(
           api_host: settings[:api_host],
           project_slug: settings.dig(:project, :slug),

lib/api/error_tracking.rb

@@ -39,18 +39,17 @@ module API
 
         not_found!('Error Tracking Setting') unless setting
 
-        # update_params = {
-        #   error_tracking_setting_attributes: { enabled: params[:active] }
-        # }
-
-        # result = ::Projects::Operations::UpdateService.new(user_project, current_user, update_params).execute
-        setting.enabled = params[:active]
-        setting.save!
-        # if result[:status] == :success
-        present setting, with: Entities::ErrorTracking::ProjectSetting
-        # else
-        # result
-        # end
+        update_params = {
+          error_tracking_setting_attributes: { enabled: params[:active] }
+        }
+
+        result = ::Projects::Operations::UpdateService.new(user_project, current_user, update_params).execute
+
+        if result[:status] == :success
+          present setting, with: Entities::ErrorTracking::ProjectSetting
+        else
+          result
+        end
       end
     end
   end

spec/factories/project_error_tracking_settings.rb

@@ -8,5 +8,9 @@ FactoryBot.define do
     token { 'access_token_123' }
     project_name { 'Sentry Project' }
     organization_name { 'Sentry Org' }
+
+    trait :disabled do
+      enabled { false }
+    end
   end
 end

spec/requests/api/error_tracking_spec.rb

@@ -17,52 +17,87 @@ describe API::ErrorTracking do
     end
 
     context 'when authenticated as maintainer' do
+
+      shared_examples 'returns project settings' do
+        it 'returns correct project settings' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to eq(
+            'active' => setting.reload.enabled,
+            'project_name' => setting.project_name,
+            'sentry_external_url' => setting.sentry_external_url,
+            'api_url' => setting.api_url
+          )
+        end
+      end
+
       before do
         project.add_maintainer(user)
       end
 
-      it 'returns project settings' do
-        make_request
+      context 'get settings' do
+        subject do
+          make_request
+        end
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response).to eq(
-          'active' => setting.enabled,
-          'project_name' => setting.project_name,
-          'sentry_external_url' => setting.sentry_external_url,
-          'api_url' => setting.api_url
-        )
+        it_behaves_like 'returns project settings'
       end
 
-      it 'returns active is invalid if non boolean' do
-        make_patch_request("randomstring")
+      context 'patch settings' do
+        subject do
+          make_patch_request(true)
+        end
+
+        it_behaves_like 'returns project settings'
 
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['error'])
-          .to eq('active is invalid')
+        subject do
+          make_patch_request(false)
+        end
+
+        it_behaves_like 'returns project settings'
+
+        it 'returns active is invalid if non boolean' do
+          make_patch_request("randomstring")
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['error'])
+            .to eq('active is invalid')
+        end
       end
     end
 
     context 'without a project setting' do
       let(:project) { create(:project) }
 
+      shared_examples 'returns 404' do
+        it 'returns correct project settings' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['message'])
+            .to eq('404 Error Tracking Setting Not Found')
+        end
+      end
+
       before do
         project.add_maintainer(user)
       end
 
-      it 'returns 404' do
-        make_request
+      context 'get settings' do
+        subject do
+          make_request
+        end
 
-        expect(response).to have_gitlab_http_status(:not_found)
-        expect(json_response['message'])
-          .to eq('404 Error Tracking Setting Not Found')
+        it_behaves_like 'returns 404'
       end
 
-      it 'returns 404 for update request' do
-        make_patch_request(true)
+      context 'patch settings' do
+        subject do
+          make_patch_request(true)
+        end
 
-        expect(response).to have_gitlab_http_status(:not_found)
-        expect(json_response['message'])
-          .to eq('404 Error Tracking Setting Not Found')
+        it_behaves_like 'returns 404'
       end
     end
 

spec/services/projects/operations/update_service_spec.rb

@@ -145,6 +145,41 @@ describe Projects::Operations::UpdateService do
         end
       end
 
+      context 'partial_update' do
+        let(:params) {
+          {
+            error_tracking_setting_attributes: {
+              enabled: true
+            }
+          }
+        }
+
+        context 'with setting' do
+          before do
+            create(:project_error_tracking_setting, :disabled, project: project)
+          end
+
+          it 'service succeeds' do
+            expect(result[:status]).to eq(:success)
+          end
+
+          it 'updates attributes' do
+            expect { result }
+              .to change { project.reload.error_tracking_setting.enabled }
+              .from(false)
+              .to(true)
+          end
+        end
+
+        context 'without setting' do
+          it 'does not create a setting' do
+            expect(result[:status]).to eq(:error)
+
+            expect(project.reload.error_tracking_setting).to be_nil
+          end
+        end
+      end
+
       context 'with masked param token' do
         let(:params) do
           {