Add latest changes from gitlab-org/gitlab@master

.rubocop_todo.yml

@@ -1144,9 +1144,6 @@ Rails/SaveBang:
     - 'spec/services/issuable/clone/attributes_rewriter_spec.rb'
     - 'spec/services/issuable/common_system_notes_service_spec.rb'
     - 'spec/services/labels/promote_service_spec.rb'
-    - 'spec/services/milestones/destroy_service_spec.rb'
-    - 'spec/services/milestones/promote_service_spec.rb'
-    - 'spec/services/milestones/transfer_service_spec.rb'
     - 'spec/services/notes/create_service_spec.rb'
     - 'spec/services/notification_recipients/build_service_spec.rb'
     - 'spec/services/notification_service_spec.rb'

app/models/project_statistics.rb

@@ -37,6 +37,8 @@ class ProjectStatistics < ApplicationRecord
   end
 
   def refresh!(only: [])
+    return if Gitlab::Database.read_only?
+
     COLUMNS_TO_REFRESH.each do |column, generator|
       if only.empty? || only.include?(column)
         public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend

app/models/snippet_statistics.rb

@@ -34,6 +34,8 @@ class SnippetStatistics < ApplicationRecord
   end
 
   def refresh!
+    return if Gitlab::Database.read_only?
+
     update_commit_count
     update_repository_size
     update_file_count

app/services/feature_flags/base_service.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class BaseService < ::BaseService
+    include Gitlab::Utils::StrongMemoize
+
+    AUDITABLE_ATTRIBUTES = %w(name description active).freeze
+
+    protected
+
+    def audit_event(feature_flag)
+      message = audit_message(feature_flag)
+
+      return if message.blank?
+
+      details =
+        {
+          custom_message: message,
+          target_id: feature_flag.id,
+          target_type: feature_flag.class.name,
+          target_details: feature_flag.name
+        }
+
+      ::AuditEventService.new(
+        current_user,
+        feature_flag.project,
+        details
+      )
+    end
+
+    def save_audit_event(audit_event)
+      return unless audit_event
+
+      audit_event.security_event
+    end
+
+    def created_scope_message(scope)
+      "Created rule <strong>#{scope.environment_scope}</strong> "\
+      "and set it as <strong>#{scope.active ? "active" : "inactive"}</strong> "\
+      "with strategies <strong>#{scope.strategies}</strong>."
+    end
+
+    def feature_flag_by_name
+      strong_memoize(:feature_flag_by_name) do
+        project.operations_feature_flags.find_by_name(params[:name])
+      end
+    end
+
+    def feature_flag_scope_by_environment_scope
+      strong_memoize(:feature_flag_scope_by_environment_scope) do
+        feature_flag_by_name.scopes.find_by_environment_scope(params[:environment_scope])
+      end
+    end
+  end
+end

app/services/feature_flags/create_service.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class CreateService < FeatureFlags::BaseService
+    def execute
+      return error('Access Denied', 403) unless can_create?
+      return error('Version is invalid', :bad_request) unless valid_version?
+      return error('New version feature flags are not enabled for this project', :bad_request) unless flag_version_enabled?
+
+      ActiveRecord::Base.transaction do
+        feature_flag = project.operations_feature_flags.new(params)
+
+        if feature_flag.save
+          save_audit_event(audit_event(feature_flag))
+
+          success(feature_flag: feature_flag)
+        else
+          error(feature_flag.errors.full_messages, 400)
+        end
+      end
+    end
+
+    private
+
+    def audit_message(feature_flag)
+      message_parts = ["Created feature flag <strong>#{feature_flag.name}</strong>",
+                       "with description <strong>\"#{feature_flag.description}\"</strong>."]
+
+      message_parts += feature_flag.scopes.map do |scope|
+        created_scope_message(scope)
+      end
+
+      message_parts.join(" ")
+    end
+
+    def can_create?
+      Ability.allowed?(current_user, :create_feature_flag, project)
+    end
+
+    def valid_version?
+      !params.key?(:version) || Operations::FeatureFlag.versions.key?(params[:version])
+    end
+
+    def flag_version_enabled?
+      params[:version] != 'new_version_flag' || new_version_feature_flags_enabled?
+    end
+
+    def new_version_feature_flags_enabled?
+      ::Feature.enabled?(:feature_flags_new_version, project, default_enabled: true)
+    end
+  end
+end

app/services/feature_flags/destroy_service.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class DestroyService < FeatureFlags::BaseService
+    def execute(feature_flag)
+      destroy_feature_flag(feature_flag)
+    end
+
+    private
+
+    def destroy_feature_flag(feature_flag)
+      return error('Access Denied', 403) unless can_destroy?(feature_flag)
+
+      ActiveRecord::Base.transaction do
+        if feature_flag.destroy
+          save_audit_event(audit_event(feature_flag))
+
+          success(feature_flag: feature_flag)
+        else
+          error(feature_flag.errors.full_messages)
+        end
+      end
+    end
+
+    def audit_message(feature_flag)
+      "Deleted feature flag <strong>#{feature_flag.name}</strong>."
+    end
+
+    def can_destroy?(feature_flag)
+      Ability.allowed?(current_user, :destroy_feature_flag, feature_flag)
+    end
+  end
+end

app/services/feature_flags/disable_service.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class DisableService < BaseService
+    def execute
+      return error('Feature Flag not found', 404) unless feature_flag_by_name
+      return error('Feature Flag Scope not found', 404) unless feature_flag_scope_by_environment_scope
+      return error('Strategy not found', 404) unless strategy_exist_in_persisted_data?
+
+      ::FeatureFlags::UpdateService
+        .new(project, current_user, update_params)
+        .execute(feature_flag_by_name)
+    end
+
+    private
+
+    def update_params
+      if remaining_strategies.empty?
+        params_to_destroy_scope
+      else
+        params_to_update_scope
+      end
+    end
+
+    def remaining_strategies
+      strong_memoize(:remaining_strategies) do
+        feature_flag_scope_by_environment_scope.strategies.reject do |strategy|
+          strategy['name'] == params[:strategy]['name'] &&
+            strategy['parameters'] == params[:strategy]['parameters']
+        end
+      end
+    end
+
+    def strategy_exist_in_persisted_data?
+      feature_flag_scope_by_environment_scope.strategies != remaining_strategies
+    end
+
+    def params_to_destroy_scope
+      { scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, _destroy: true }] }
+    end
+
+    def params_to_update_scope
+      { scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, strategies: remaining_strategies }] }
+    end
+  end
+end

app/services/feature_flags/enable_service.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class EnableService < BaseService
+    def execute
+      if feature_flag_by_name
+        update_feature_flag
+      else
+        create_feature_flag
+      end
+    end
+
+    private
+
+    def create_feature_flag
+      ::FeatureFlags::CreateService
+        .new(project, current_user, create_params)
+        .execute
+    end
+
+    def update_feature_flag
+      ::FeatureFlags::UpdateService
+        .new(project, current_user, update_params)
+        .execute(feature_flag_by_name)
+    end
+
+    def create_params
+      if params[:environment_scope] == '*'
+        params_to_create_flag_with_default_scope
+      else
+        params_to_create_flag_with_additional_scope
+      end
+    end
+
+    def update_params
+      if feature_flag_scope_by_environment_scope
+        params_to_update_scope
+      else
+        params_to_create_scope
+      end
+    end
+
+    def params_to_create_flag_with_default_scope
+      {
+        name: params[:name],
+        scopes_attributes: [
+          {
+            active: true,
+            environment_scope: '*',
+            strategies: [params[:strategy]]
+          }
+        ]
+      }
+    end
+
+    def params_to_create_flag_with_additional_scope
+      {
+        name: params[:name],
+        scopes_attributes: [
+          {
+            active: false,
+            environment_scope: '*'
+          },
+          {
+            active: true,
+            environment_scope: params[:environment_scope],
+            strategies: [params[:strategy]]
+          }
+        ]
+      }
+    end
+
+    def params_to_create_scope
+      {
+        scopes_attributes: [{
+          active: true,
+          environment_scope: params[:environment_scope],
+          strategies: [params[:strategy]]
+        }]
+      }
+    end
+
+    def params_to_update_scope
+      {
+        scopes_attributes: [{
+          id: feature_flag_scope_by_environment_scope.id,
+          active: true,
+          strategies: feature_flag_scope_by_environment_scope.strategies | [params[:strategy]]
+        }]
+      }
+    end
+  end
+end

app/services/feature_flags/update_service.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  class UpdateService < FeatureFlags::BaseService
+    AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES = {
+      'active' => 'active state',
+      'environment_scope' => 'environment scope',
+      'strategies' => 'strategies'
+    }.freeze
+
+    def execute(feature_flag)
+      return error('Access Denied', 403) unless can_update?(feature_flag)
+
+      ActiveRecord::Base.transaction do
+        feature_flag.assign_attributes(params)
+
+        feature_flag.strategies.each do |strategy|
+          if strategy.name_changed? && strategy.name_was == ::Operations::FeatureFlags::Strategy::STRATEGY_GITLABUSERLIST
+            strategy.user_list = nil
+          end
+        end
+
+        audit_event = audit_event(feature_flag)
+
+        if feature_flag.save
+          save_audit_event(audit_event)
+
+          success(feature_flag: feature_flag)
+        else
+          error(feature_flag.errors.full_messages, :bad_request)
+        end
+      end
+    end
+
+    private
+
+    def audit_message(feature_flag)
+      changes = changed_attributes_messages(feature_flag)
+      changes += changed_scopes_messages(feature_flag)
+
+      return if changes.empty?
+
+      "Updated feature flag <strong>#{feature_flag.name}</strong>. " + changes.join(" ")
+    end
+
+    def changed_attributes_messages(feature_flag)
+      feature_flag.changes.slice(*AUDITABLE_ATTRIBUTES).map do |attribute_name, changes|
+        "Updated #{attribute_name} "\
+        "from <strong>\"#{changes.first}\"</strong> to "\
+        "<strong>\"#{changes.second}\"</strong>."
+      end
+    end
+
+    def changed_scopes_messages(feature_flag)
+      feature_flag.scopes.map do |scope|
+        if scope.new_record?
+          created_scope_message(scope)
+        elsif scope.marked_for_destruction?
+          deleted_scope_message(scope)
+        else
+          updated_scope_message(scope)
+        end
+      end.compact # updated_scope_message can return nil if nothing has been changed
+    end
+
+    def deleted_scope_message(scope)
+      "Deleted rule <strong>#{scope.environment_scope}</strong>."
+    end
+
+    def updated_scope_message(scope)
+      changes = scope.changes.slice(*AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES.keys)
+      return if changes.empty?
+
+      message = "Updated rule <strong>#{scope.environment_scope}</strong> "
+      message += changes.map do |attribute_name, change|
+        name = AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES[attribute_name]
+        "#{name} from <strong>#{change.first}</strong> to <strong>#{change.second}</strong>"
+      end.join(' ')
+
+      message + '.'
+    end
+
+    def can_update?(feature_flag)
+      Ability.allowed?(current_user, :update_feature_flag, feature_flag)
+    end
+  end
+end

changelogs/unreleased/250041-does-not-refresh-project-snippet-statistics-on-a-read-only-instanc.yml

@@ -0,0 +1,5 @@
+---
+title: Does not refresh project/snippet statistics on a read-only instance
+merge_request: 42417
+author:
+type: fixed

changelogs/unreleased/rails-save-bang-33.yml

@@ -0,0 +1,5 @@
+---
+title: Fix Rails/SaveBang offenses for spec files in spec/services/milestones/*
+merge_request: 42775
+author: Rajendra Kadam
+type: other

spec/models/project_statistics_spec.rb

@@ -201,6 +201,23 @@ RSpec.describe ProjectStatistics do
         statistics.refresh!(only: [:commit_count])
       end
     end
+
+    context 'when the database is read-only' do
+      it 'does nothing' do
+        allow(Gitlab::Database).to receive(:read_only?) { true }
+
+        expect(statistics).not_to receive(:update_commit_count)
+        expect(statistics).not_to receive(:update_repository_size)
+        expect(statistics).not_to receive(:update_wiki_size)
+        expect(statistics).not_to receive(:update_lfs_objects_size)
+        expect(statistics).not_to receive(:update_snippets_size)
+        expect(statistics).not_to receive(:save!)
+        expect(Namespaces::ScheduleAggregationWorker)
+          .not_to receive(:perform_async)
+
+        statistics.refresh!
+      end
+    end
   end
 
   describe '#update_commit_count' do

spec/models/snippet_statistics_spec.rb

@@ -75,15 +75,28 @@ RSpec.describe SnippetStatistics do
   end
 
   describe '#refresh!' do
-    subject { statistics.refresh! }
-
     it 'retrieves and saves statistic data from repository' do
       expect(statistics).to receive(:update_commit_count)
       expect(statistics).to receive(:update_file_count)
       expect(statistics).to receive(:update_repository_size)
       expect(statistics).to receive(:save!)
 
-      subject
+      statistics.refresh!
+    end
+
+    context 'when the database is read-only' do
+      it 'does nothing' do
+        allow(Gitlab::Database).to receive(:read_only?) { true }
+
+        expect(statistics).not_to receive(:update_commit_count)
+        expect(statistics).not_to receive(:update_file_count)
+        expect(statistics).not_to receive(:update_repository_size)
+        expect(statistics).not_to receive(:save!)
+        expect(Namespaces::ScheduleAggregationWorker)
+          .not_to receive(:perform_async)
+
+        statistics.refresh!
+      end
     end
   end
 

spec/services/feature_flags/create_service_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeatureFlags::CreateService do
+  let(:project) { create(:project) }
+  let(:developer) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:user) { developer }
+
+  before do
+    project.add_developer(developer)
+    project.add_reporter(reporter)
+  end
+
+  describe '#execute' do
+    subject do
+      described_class.new(project, user, params).execute
+    end
+
+    let(:feature_flag) { subject[:feature_flag] }
+
+    context 'when feature flag can not be created' do
+      let(:params) { {} }
+
+      it 'returns status error' do
+        expect(subject[:status]).to eq(:error)
+      end
+
+      it 'returns validation errors' do
+        expect(subject[:message]).to include("Name can't be blank")
+      end
+
+      it 'does not create audit log' do
+        expect { subject }.not_to change { AuditEvent.count }
+      end
+    end
+
+    context 'when feature flag is saved correctly' do
+      let(:params) do
+        {
+          name: 'feature_flag',
+          description: 'description',
+          scopes_attributes: [{ environment_scope: '*', active: true },
+                              { environment_scope: 'production', active: false }]
+        }
+      end
+
+      it 'returns status success' do
+        expect(subject[:status]).to eq(:success)
+      end
+
+      it 'creates feature flag' do
+        expect { subject }.to change { Operations::FeatureFlag.count }.by(1)
+      end
+
+      it 'creates audit event' do
+        expected_message = 'Created feature flag <strong>feature_flag</strong> '\
+                           'with description <strong>"description"</strong>. '\
+                           'Created rule <strong>*</strong> and set it as <strong>active</strong> '\
+                           'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>. '\
+                           'Created rule <strong>production</strong> and set it as <strong>inactive</strong> '\
+                           'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>.'
+
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(AuditEvent.last.details[:custom_message]).to eq(expected_message)
+      end
+
+      context 'when user is reporter' do
+        let(:user) { reporter }
+
+        it 'returns error status' do
+          expect(subject[:status]).to eq(:error)
+          expect(subject[:message]).to eq('Access Denied')
+        end
+      end
+    end
+  end
+end

spec/services/feature_flags/destroy_service_spec.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeatureFlags::DestroyService do
+  include FeatureFlagHelpers
+
+  let(:project) { create(:project) }
+  let(:developer) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:user) { developer }
+  let!(:feature_flag) { create(:operations_feature_flag, project: project) }
+
+  before do
+    project.add_developer(developer)
+    project.add_reporter(reporter)
+  end
+
+  describe '#execute' do
+    subject { described_class.new(project, user, params).execute(feature_flag) }
+
+    let(:audit_event_message) { AuditEvent.last.details[:custom_message] }
+    let(:params) { {} }
+
+    it 'returns status success' do
+      expect(subject[:status]).to eq(:success)
+    end
+
+    it 'destroys feature flag' do
+      expect { subject }.to change { Operations::FeatureFlag.count }.by(-1)
+    end
+
+    it 'creates audit log' do
+      expect { subject }.to change { AuditEvent.count }.by(1)
+      expect(audit_event_message).to eq("Deleted feature flag <strong>#{feature_flag.name}</strong>.")
+    end
+
+    context 'when user is reporter' do
+      let(:user) { reporter }
+
+      it 'returns error status' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to eq('Access Denied')
+      end
+    end
+
+    context 'when feature flag can not be destroyed' do
+      before do
+        allow(feature_flag).to receive(:destroy).and_return(false)
+      end
+
+      it 'returns status error' do
+        expect(subject[:status]).to eq(:error)
+      end
+
+      it 'does not create audit log' do
+        expect { subject }.not_to change { AuditEvent.count }
+      end
+    end
+  end
+end

spec/services/feature_flags/disable_service_spec.rb

@@ -0,0 +1,91 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeatureFlags::DisableService do
+  include FeatureFlagHelpers
+
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:service) { described_class.new(project, user, params) }
+  let(:params) { {} }
+
+  before do
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'with params to disable default strategy on prd scope' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd',
+          strategy: { name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys
+        }
+      end
+
+      context 'when there is a persisted feature flag' do
+        let!(:feature_flag) { create_flag(project, params[:name]) }
+
+        context 'when there is a persisted scope' do
+          let!(:scope) do
+            create_scope(feature_flag, params[:environment_scope], true, strategies)
+          end
+
+          context 'when there is a persisted strategy' do
+            let(:strategies) do
+              [
+                { name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys,
+                { name: 'userWithId', parameters: { 'userIds': 'User:2' } }.deep_stringify_keys
+              ]
+            end
+
+            it 'deletes the specified strategy' do
+              subject
+
+              scope.reload
+              expect(scope.strategies.count).to eq(1)
+              expect(scope.strategies).not_to include(params[:strategy])
+            end
+
+            context 'when strategies will be empty' do
+              let(:strategies) { [params[:strategy]] }
+
+              it 'deletes the persisted scope' do
+                subject
+
+                expect(feature_flag.scopes.exists?(environment_scope: params[:environment_scope]))
+                  .to eq(false)
+              end
+            end
+          end
+
+          context 'when there is no persisted strategy' do
+            let(:strategies) { [{ name: 'default', parameters: {} }] }
+
+            it 'returns error' do
+              expect(subject[:status]).to eq(:error)
+              expect(subject[:message]).to include('Strategy not found')
+            end
+          end
+        end
+
+        context 'when there is no persisted scope' do
+          it 'returns error' do
+            expect(subject[:status]).to eq(:error)
+            expect(subject[:message]).to include('Feature Flag Scope not found')
+          end
+        end
+      end
+
+      context 'when there is no persisted feature flag' do
+        it 'returns error' do
+          expect(subject[:status]).to eq(:error)
+          expect(subject[:message]).to include('Feature Flag not found')
+        end
+      end
+    end
+  end
+end

spec/services/feature_flags/enable_service_spec.rb

@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeatureFlags::EnableService do
+  include FeatureFlagHelpers
+
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:service) { described_class.new(project, user, params) }
+  let(:params) { {} }
+
+  before do
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'with params to enable default strategy on prd scope' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      context 'when there is no persisted feature flag' do
+        it 'creates a new feature flag with scope' do
+          feature_flag = subject[:feature_flag]
+          scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+          expect(subject[:status]).to eq(:success)
+          expect(feature_flag.name).to eq(params[:name])
+          expect(feature_flag.default_scope).not_to be_active
+          expect(scope).to be_active
+          expect(scope.strategies).to include(params[:strategy])
+        end
+
+        context 'when params include default scope' do
+          let(:params) do
+            {
+              name: 'awesome',
+              environment_scope: '*',
+              strategy: { name: 'userWithId', parameters: { 'userIds': 'abc' } }.deep_stringify_keys
+            }
+          end
+
+          it 'create a new feature flag with an active default scope with the specified strategy' do
+            feature_flag = subject[:feature_flag]
+            expect(subject[:status]).to eq(:success)
+            expect(feature_flag.default_scope).to be_active
+            expect(feature_flag.default_scope.strategies).to include(params[:strategy])
+          end
+        end
+      end
+
+      context 'when there is a persisted feature flag' do
+        let!(:feature_flag) { create_flag(project, params[:name]) }
+
+        context 'when there is no persisted scope' do
+          it 'creates a new scope for the persisted feature flag' do
+            feature_flag = subject[:feature_flag]
+            scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+            expect(subject[:status]).to eq(:success)
+            expect(feature_flag.name).to eq(params[:name])
+            expect(scope).to be_active
+            expect(scope.strategies).to include(params[:strategy])
+          end
+        end
+
+        context 'when there is a persisted scope' do
+          let!(:feature_flag_scope) do
+            create_scope(feature_flag, params[:environment_scope], active, strategies)
+          end
+
+          let(:active) { true }
+
+          context 'when the persisted scope does not have the specified strategy yet' do
+            let(:strategies) { [{ name: 'userWithId', parameters: { 'userIds': 'abc' } }] }
+
+            it 'adds the specified strategy to the scope' do
+              subject
+
+              feature_flag_scope.reload
+              expect(feature_flag_scope.strategies).to include(params[:strategy])
+            end
+
+            context 'when the persisted scope is inactive' do
+              let(:active) { false }
+
+              it 'reactivates the scope' do
+                expect { subject }
+                  .to change { feature_flag_scope.reload.active }.from(false).to(true)
+              end
+            end
+          end
+
+          context 'when the persisted scope has the specified strategy already' do
+            let(:strategies) { [params[:strategy]] }
+
+            it 'does not add a duplicated strategy to the scope' do
+              expect { subject }
+                .not_to change { feature_flag_scope.reload.strategies.count }
+            end
+          end
+        end
+      end
+    end
+
+    context 'when strategy is not specified in params' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd'
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include('Scopes strategies must be an array of strategy hashes')
+      end
+    end
+
+    context 'when environment scope is not specified in params' do
+      let(:params) do
+        {
+          name: 'awesome',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include("Scopes environment scope can't be blank")
+      end
+    end
+
+    context 'when name is not specified in params' do
+      let(:params) do
+        {
+          environment_scope: 'prd',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include("Name can't be blank")
+      end
+    end
+  end
+end

spec/services/feature_flags/update_service_spec.rb

@@ -0,0 +1,250 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe FeatureFlags::UpdateService do
+  let(:project) { create(:project) }
+  let(:developer) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:user) { developer }
+  let(:feature_flag) { create(:operations_feature_flag, project: project, active: true) }
+
+  before do
+    project.add_developer(developer)
+    project.add_reporter(reporter)
+  end
+
+  describe '#execute' do
+    subject { described_class.new(project, user, params).execute(feature_flag) }
+
+    let(:params) { { name: 'new_name' } }
+    let(:audit_event_message) do
+      AuditEvent.last.details[:custom_message]
+    end
+
+    it 'returns success status' do
+      expect(subject[:status]).to eq(:success)
+    end
+
+    it 'creates audit event with correct message' do
+      name_was = feature_flag.name
+
+      expect { subject }.to change { AuditEvent.count }.by(1)
+      expect(audit_event_message).to(
+        eq("Updated feature flag <strong>new_name</strong>. "\
+           "Updated name from <strong>\"#{name_was}\"</strong> "\
+           "to <strong>\"new_name\"</strong>.")
+      )
+    end
+
+    context 'with invalid params' do
+      let(:params) { { name: nil } }
+
+      it 'returns error status' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:http_status]).to eq(:bad_request)
+      end
+
+      it 'returns error messages' do
+        expect(subject[:message]).to include("Name can't be blank")
+      end
+
+      it 'does not create audit event' do
+        expect { subject }.not_to change { AuditEvent.count }
+      end
+    end
+
+    context 'when user is reporter' do
+      let(:user) { reporter }
+
+      it 'returns error status' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to eq('Access Denied')
+      end
+    end
+
+    context 'when nothing is changed' do
+      let(:params) { {} }
+
+      it 'returns success status' do
+        expect(subject[:status]).to eq(:success)
+      end
+
+      it 'does not create audit event' do
+        expect { subject }.not_to change { AuditEvent.count }
+      end
+    end
+
+    context 'description is being changed' do
+      let(:params) { { description: 'new description' } }
+
+      it 'creates audit event with changed description' do
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to(
+          include("Updated description from <strong>\"\"</strong>"\
+                  " to <strong>\"new description\"</strong>.")
+        )
+      end
+    end
+
+    context 'when flag active state is changed' do
+      let(:params) do
+        {
+          active: false
+        }
+      end
+
+      it 'creates audit event about changing active state' do
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to(
+          include('Updated active from <strong>"true"</strong> to <strong>"false"</strong>.')
+        )
+      end
+    end
+
+    context 'when scope active state is changed' do
+      let(:params) do
+        {
+          scopes_attributes: [{ id: feature_flag.scopes.first.id, active: false }]
+        }
+      end
+
+      it 'creates audit event about changing active state' do
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to(
+          include("Updated rule <strong>*</strong> active state "\
+                  "from <strong>true</strong> to <strong>false</strong>.")
+        )
+      end
+    end
+
+    context 'when scope is renamed' do
+      let(:changed_scope) { feature_flag.scopes.create!(environment_scope: 'review', active: true) }
+      let(:params) do
+        {
+          scopes_attributes: [{ id: changed_scope.id, environment_scope: 'staging' }]
+        }
+      end
+
+      it 'creates audit event with changed name' do
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to(
+          include("Updated rule <strong>staging</strong> environment scope "\
+                  "from <strong>review</strong> to <strong>staging</strong>.")
+        )
+      end
+
+      context 'when scope can not be updated' do
+        let(:params) do
+          {
+            scopes_attributes: [{ id: changed_scope.id, environment_scope: '' }]
+          }
+        end
+
+        it 'returns error status' do
+          expect(subject[:status]).to eq(:error)
+        end
+
+        it 'returns error messages' do
+          expect(subject[:message]).to include("Scopes environment scope can't be blank")
+        end
+
+        it 'does not create audit event' do
+          expect { subject }.not_to change { AuditEvent.count }
+        end
+      end
+    end
+
+    context 'when scope is deleted' do
+      let(:deleted_scope) { feature_flag.scopes.create!(environment_scope: 'review', active: true) }
+      let(:params) do
+        {
+          scopes_attributes: [{ id: deleted_scope.id, '_destroy': true }]
+        }
+      end
+
+      it 'creates audit event with deleted scope' do
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to include("Deleted rule <strong>review</strong>.")
+      end
+
+      context 'when scope can not be deleted' do
+        before do
+          allow(deleted_scope).to receive(:destroy).and_return(false)
+        end
+
+        it 'does not create audit event' do
+          expect do
+            subject
+          end.to not_change { AuditEvent.count }.and raise_error(ActiveRecord::RecordNotDestroyed)
+        end
+      end
+    end
+
+    context 'when new scope is being added' do
+      let(:new_environment_scope) { 'review' }
+      let(:params) do
+        {
+          scopes_attributes: [{ environment_scope: new_environment_scope, active: true }]
+        }
+      end
+
+      it 'creates audit event with new scope' do
+        expected = 'Created rule <strong>review</strong> and set it as <strong>active</strong> '\
+                   'with strategies <strong>[{"name"=>"default", "parameters"=>{}}]</strong>.'
+
+        subject
+
+        expect(audit_event_message).to include(expected)
+      end
+
+      context 'when scope can not be created' do
+        let(:new_environment_scope) { '' }
+
+        it 'returns error status' do
+          expect(subject[:status]).to eq(:error)
+        end
+
+        it 'returns error messages' do
+          expect(subject[:message]).to include("Scopes environment scope can't be blank")
+        end
+
+        it 'does not create audit event' do
+          expect { subject }.not_to change { AuditEvent.count }
+        end
+      end
+    end
+
+    context 'when the strategy is changed' do
+      let(:scope) do
+        create(:operations_feature_flag_scope,
+               feature_flag: feature_flag,
+               environment_scope: 'sandbox',
+               strategies: [{ name: "default", parameters: {} }])
+      end
+
+      let(:params) do
+        {
+          scopes_attributes: [{
+            id: scope.id,
+            environment_scope: 'sandbox',
+            strategies: [{
+              name: 'gradualRolloutUserId',
+              parameters: {
+                groupId: 'mygroup',
+                percentage: "40"
+              }
+            }]
+          }]
+        }
+      end
+
+      it 'creates an audit event' do
+        expected = %r{Updated rule <strong>sandbox</strong> strategies from <strong>.*</strong> to <strong>.*</strong>.}
+
+        expect { subject }.to change { AuditEvent.count }.by(1)
+        expect(audit_event_message).to match(expected)
+      end
+    end
+  end
+end

spec/services/milestones/destroy_service_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Milestones::DestroyService do
       let(:group_milestone) { create(:milestone, group: group) }
 
       before do
-        project.update(namespace: group)
+        project.update!(namespace: group)
         group.add_developer(user)
       end
 

spec/services/milestones/promote_service_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe Milestones::PromoteService do
       end
 
       it 'raises error if project does not belong to a group' do
-        project.update(namespace: user.namespace)
+        project.update!(namespace: user.namespace)
 
         expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
       end

spec/services/milestones/transfer_service_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe Milestones::TransferService do
         new_group.add_maintainer(user)
         project.add_maintainer(user)
         # simulate project transfer
-        project.update(group: new_group)
+        project.update!(group: new_group)
       end
 
       context 'without existing milestone at the new group level' do