diff --git a/ee/app/controllers/projects/feature_flags_controller.rb b/ee/app/controllers/projects/feature_flags_controller.rb index 009d3c4f214092afa58849457b9d44351938a526..ed8329de89b78ce8aef7bbed1cdd5883dba7977c 100644 --- a/ee/app/controllers/projects/feature_flags_controller.rb +++ b/ee/app/controllers/projects/feature_flags_controller.rb @@ -28,7 +28,6 @@ def index end def new - @feature_flag = project.operations_feature_flags.new end def show @@ -36,23 +35,21 @@ def show format.json do Gitlab::PollingInterval.set_header(response, interval: 10_000) - render_success_json + render_success_json(feature_flag) end end end def create - @feature_flag = project.operations_feature_flags.create(create_params) + result = FeatureFlags::CreateService.new(project, current_user, create_params).execute - if @feature_flag.persisted? + if result[:status] == :success respond_to do |format| - format.html { redirect_to_index(notice: 'Feature flag was successfully created.') } - format.json { render_success_json } + format.json { render_success_json(result[:feature_flag]) } end else respond_to do |format| - format.html { render :new } - format.json { render_error_json } + format.json { render_error_json(result[:message]) } end end end @@ -61,29 +58,31 @@ def edit end def update - if feature_flag.update(update_params) + result = FeatureFlags::UpdateService.new(project, current_user, update_params).execute(feature_flag) + + if result[:status] == :success respond_to do |format| - format.html { redirect_to_index(notice: 'Feature flag was successfully updated.') } - format.json { render_success_json } + format.json { render_success_json(result[:feature_flag]) } end else respond_to do |format| - format.html { render :edit } - format.json { render_error_json } + format.json { render_error_json(result[:message]) } end end end def destroy - if feature_flag.destroy + result = FeatureFlags::DestroyService.new(project, current_user).execute(feature_flag) + + if result[:status] == :success respond_to do |format| format.html { redirect_to_index(notice: 'Feature flag was successfully removed.') } - format.json { render_success_json } + format.json { render_success_json(feature_flag) } end else respond_to do |format| format.html { redirect_to_index(alert: 'Feature flag was not removed.') } - format.json { render_error_json } + format.json { render_error_json(result[:message]) } end end end @@ -106,7 +105,7 @@ def update_params scopes_attributes: [:id, :environment_scope, :active, :_destroy]) end - def feature_flag_json + def feature_flag_json(feature_flag) FeatureFlagSerializer .new(project: @project, current_user: @current_user) .represent(feature_flag) @@ -129,12 +128,12 @@ def redirect_to_index(**args) redirect_to project_feature_flags_path(@project), status: :found, **args end - def render_success_json - render json: feature_flag_json, status: :ok + def render_success_json(feature_flag) + render json: feature_flag_json(feature_flag), status: :ok end - def render_error_json - render json: { message: feature_flag.errors.full_messages }, + def render_error_json(messages) + render json: { message: messages }, status: :bad_request end end diff --git a/ee/app/services/feature_flags/base_service.rb b/ee/app/services/feature_flags/base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..b5a05900f6e6465d5f0dc418b4ec13e98c9eb976 --- /dev/null +++ b/ee/app/services/feature_flags/base_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module FeatureFlags + class BaseService < ::BaseService + AUDITABLE_ATTRIBUTES = %w(name description).freeze + + protected + + def audit_enabled? + Feature.enabled?(:feature_flag_audit, project, default_enabled: true) + end + + def audit_event(feature_flag) + return unless audit_enabled? + + 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 # feature_flag_audit is disabled or audit_message is blank + + audit_event.security_event + end + + def created_scope_message(scope) + "Created rule #{scope.environment_scope} "\ + "and set it as #{scope.active ? "active" : "inactive"}." + end + end +end diff --git a/ee/app/services/feature_flags/create_service.rb b/ee/app/services/feature_flags/create_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..bcc0c0972768016984cdeb234d2c614f05bcf3ee --- /dev/null +++ b/ee/app/services/feature_flags/create_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module FeatureFlags + class CreateService < FeatureFlags::BaseService + def execute + 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) + end + end + end + + private + + def audit_message(feature_flag) + message_parts = ["Created feature flag #{feature_flag.name}", + "with description \"#{feature_flag.description}\"."] + + message_parts += feature_flag.scopes.map do |scope| + created_scope_message(scope) + end + + message_parts.join(" ") + end + end +end diff --git a/ee/app/services/feature_flags/destroy_service.rb b/ee/app/services/feature_flags/destroy_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..c80d869787cdeb25cd0c033b3a5032ee44450755 --- /dev/null +++ b/ee/app/services/feature_flags/destroy_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module FeatureFlags + class DestroyService < FeatureFlags::BaseService + def execute(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 + + private + + def audit_message(feature_flag) + "Deleted feature flag #{feature_flag.name}." + end + end +end diff --git a/ee/app/services/feature_flags/update_service.rb b/ee/app/services/feature_flags/update_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..a9612a7f4a5a761821831ed0fac620fb5f8d6c06 --- /dev/null +++ b/ee/app/services/feature_flags/update_service.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module FeatureFlags + class UpdateService < FeatureFlags::BaseService + AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES = { + 'active' => 'active state', + 'environment_scope' => 'environment scope' + }.freeze + + def execute(feature_flag) + ActiveRecord::Base.transaction do + feature_flag.assign_attributes(params) + + 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) + 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 #{feature_flag.name}. " + changes.join(" ") + end + + def changed_attributes_messages(feature_flag) + feature_flag.changes.slice(*AUDITABLE_ATTRIBUTES).map do |attribute_name, changes| + "Updated #{attribute_name} "\ + "from \"#{changes.first}\" to "\ + "\"#{changes.second}\"." + 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 #{scope.environment_scope}." + end + + def updated_scope_message(scope) + changes = scope.changes.slice(*AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES.keys) + return if changes.empty? + + message = "Updated rule #{scope.environment_scope} " + message += changes.map do |attribute_name, change| + name = AUDITABLE_SCOPE_ATTRIBUTES_HUMAN_NAMES[attribute_name] + "#{name} from #{change.first} to #{change.second}" + end.join(' ') + + message + '.' + end + end +end diff --git a/ee/changelogs/unreleased/feature-flags-audit-events.yml b/ee/changelogs/unreleased/feature-flags-audit-events.yml new file mode 100644 index 0000000000000000000000000000000000000000..b7353e5aebd898bc308b641e1f6211ba1a688db0 --- /dev/null +++ b/ee/changelogs/unreleased/feature-flags-audit-events.yml @@ -0,0 +1,5 @@ +--- +title: Add audit log for managing feature flags +merge_request: 9487 +author: +type: added diff --git a/ee/spec/controllers/projects/feature_flags_controller_spec.rb b/ee/spec/controllers/projects/feature_flags_controller_spec.rb index e1f5744b1018c0c936893e4eba2afe13e3b17c2d..5eac73734645881e1b3e67837027a8168c0e0ee0 100644 --- a/ee/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/ee/spec/controllers/projects/feature_flags_controller_spec.rb @@ -300,24 +300,6 @@ end end - describe 'POST create' do - render_views - - subject { post(:create, params: params) } - - context 'when creating a new feature flag' do - let(:params) do - view_params.merge(operations_feature_flag: { name: 'my_feature_flag', active: true }) - end - - it 'creates and redirects to list' do - subject - - expect(response).to redirect_to(project_feature_flags_path(project)) - end - end - end - describe 'POST create.json' do subject { post(:create, params: params, format: :json) } @@ -435,29 +417,6 @@ end end - describe 'PUT update' do - let!(:feature_flag) { create(:operations_feature_flag, project: project, name: 'my_feature_flag') } - - render_views - - subject { put(:update, params: params) } - - context 'when updating an existing feature flag' do - let(:params) do - view_params.merge( - id: feature_flag.id, - operations_feature_flag: { name: 'my_feature_flag_v2', active: true } - ) - end - - it 'updates and redirects to list' do - subject - - expect(response).to redirect_to(project_feature_flags_path(project)) - end - end - end - describe 'DELETE destroy.json' do subject { delete(:destroy, params: params, format: :json) } diff --git a/ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb b/ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb index ac62f2c330b688967e41bb9d734a33c600ce35be..b166affba44e2c8a21d674607fa7a01b7acbbe6f 100644 --- a/ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb +++ b/ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb @@ -15,113 +15,182 @@ end context 'when creates without changing scopes' do - before do - visit(new_project_feature_flag_path(project)) - set_feature_flag_info('ci_live_trace', 'For live trace') - click_button 'Create feature flag' - end + shared_examples 'succesfully creates feature flag' do + before do + visit(new_project_feature_flag_path(project)) + set_feature_flag_info('ci_live_trace', 'For live trace') + click_button 'Create feature flag' + expect(page).to have_current_path(project_feature_flags_path(project)) + end - it 'shows the created feature flag' do - within_feature_flag_row(1) do - expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') - expect(page).to have_css('.js-feature-flag-status .badge-success') + it 'shows the created feature flag' do + within_feature_flag_row(1) do + expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') + expect(page).to have_css('.js-feature-flag-status .badge-success') - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(1)')).to have_content('*') - expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(1)')).to have_content('*') + expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') + end end end end + + context 'when feature flag audit enabled' do + include_examples 'succesfully creates feature flag' + + it 'records audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_text("Created feature flag ci live trace with description \"For live trace\".") + end + end + + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + include_examples 'succesfully creates feature flag' + + it 'does not record audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_no_text("Created feature flag") + end + end end context 'when creates with disabling the default scope' do - before do - visit(new_project_feature_flag_path(project)) - set_feature_flag_info('ci_live_trace', 'For live trace') + shared_examples 'succesfully creates feature flag' do + before do + visit(new_project_feature_flag_path(project)) + set_feature_flag_info('ci_live_trace', 'For live trace') - within_scope_row(1) do - within_status { find('.project-feature-toggle').click } + within_scope_row(1) do + within_status { find('.project-feature-toggle').click } + end + + click_button 'Create feature flag' end - click_button 'Create feature flag' + it 'shows the created feature flag' do + within_feature_flag_row(1) do + expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') + expect(page).to have_css('.js-feature-flag-status .badge-danger') + + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(1)')).to have_content('*') + expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive') + end + end + end end - it 'shows the created feature flag' do - within_feature_flag_row(1) do - expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') - expect(page).to have_css('.js-feature-flag-status .badge-danger') + context 'when feature flag audit enabled' do + include_examples 'succesfully creates feature flag' + end - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(1)')).to have_content('*') - expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive') - end + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) end + + include_examples 'succesfully creates feature flag' end end context 'when creates with an additional scope' do - before do - visit(new_project_feature_flag_path(project)) - set_feature_flag_info('mr_train', '') - - within_scope_row(2) do - within_environment_spec do - find('.js-env-input').set("review/*") - find('.js-create-button').click + shared_examples 'succesfully creates feature flag' do + before do + visit(new_project_feature_flag_path(project)) + set_feature_flag_info('mr_train', '') + + within_scope_row(2) do + within_environment_spec do + find('.js-env-input').set("review/*") + find('.js-create-button').click + end end - end - within_scope_row(2) do - within_status { find('.project-feature-toggle').click } + within_scope_row(2) do + within_status { find('.project-feature-toggle').click } + end + + click_button 'Create feature flag' end - click_button 'Create feature flag' + it 'shows the created feature flag' do + within_feature_flag_row(1) do + expect(page.find('.feature-flag-name')).to have_content('mr_train') + expect(page).to have_css('.js-feature-flag-status .badge-success') + + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(1)')).to have_content('*') + expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') + expect(page.find('.badge:nth-child(2)')).to have_content('review/*') + expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active') + end + end + end end - it 'shows the created feature flag' do - within_feature_flag_row(1) do - expect(page.find('.feature-flag-name')).to have_content('mr_train') - expect(page).to have_css('.js-feature-flag-status .badge-success') + context 'when feature flag audit enabled' do + include_examples 'succesfully creates feature flag' + end - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(1)')).to have_content('*') - expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') - expect(page.find('.badge:nth-child(2)')).to have_content('review/*') - expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active') - end + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) end + + include_examples 'succesfully creates feature flag' end end context 'when searches an environment name for scope creation' do let!(:environment) { create(:environment, name: 'production', project: project) } - before do - visit(new_project_feature_flag_path(project)) - set_feature_flag_info('mr_train', '') + shared_examples 'succesfully creates feature flag' do + before do + visit(new_project_feature_flag_path(project)) + set_feature_flag_info('mr_train', '') - within_scope_row(2) do - within_environment_spec do - find('.js-env-input').set('prod') - click_button 'production' + within_scope_row(2) do + within_environment_spec do + find('.js-env-input').set('prod') + click_button 'production' + end end + + click_button 'Create feature flag' end - click_button 'Create feature flag' + it 'shows the created feature flag' do + within_feature_flag_row(1) do + expect(page.find('.feature-flag-name')).to have_content('mr_train') + expect(page).to have_css('.js-feature-flag-status .badge-success') + + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(1)')).to have_content('*') + expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') + expect(page.find('.badge:nth-child(2)')).to have_content('production') + expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive') + end + end + end end - it 'shows the created feature flag' do - within_feature_flag_row(1) do - expect(page.find('.feature-flag-name')).to have_content('mr_train') - expect(page).to have_css('.js-feature-flag-status .badge-success') + context 'when feature flag audit enabled' do + include_examples 'succesfully creates feature flag' + end - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(1)')).to have_content('*') - expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active') - expect(page.find('.badge:nth-child(2)')).to have_content('production') - expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive') - end + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) end + + include_examples 'succesfully creates feature flag' end end diff --git a/ee/spec/features/projects/feature_flags/user_deletes_feature_flag_spec.rb b/ee/spec/features/projects/feature_flags/user_deletes_feature_flag_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..86dc6dbd31d80b9dc1733dbb97dae5e64b4e88d0 --- /dev/null +++ b/ee/spec/features/projects/feature_flags/user_deletes_feature_flag_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User deletes feature flag', :js do + include FeatureFlagHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace) } + + let!(:feature_flag) do + create_flag(project, 'ci_live_trace', false, + description: 'For live trace feature') + end + + before do + project.add_developer(user) + stub_licensed_features(feature_flags: true) + sign_in(user) + + visit(project_feature_flags_path(project)) + + find('.js-feature-flag-delete-button').click + click_button('Delete feature flag') + expect(page).to have_current_path(project_feature_flags_path(project)) + end + + it 'user does not see feature flag' do + expect(page).to have_no_content('ci_live_trace') + end + + it 'records audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_text("Deleted feature flag ci live trace.") + end +end diff --git a/ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb b/ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb index 1b2fcee5a8b29320644d161e1d58592238a177be..a9d908fc6f759edd56d9c592a4efb38217678c84 100644 --- a/ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb +++ b/ee/spec/features/projects/feature_flags/user_updates_feature_flag_spec.rb @@ -37,66 +37,153 @@ end context 'when user updates a status of a scope' do - before do - within_scope_row(2) do - within_status { find('.project-feature-toggle').click } + shared_examples 'succesfully updates feature flag' do + before do + within_scope_row(2) do + within_status { find('.project-feature-toggle').click } + end + + click_button 'Save changes' + expect(page).to have_current_path(project_feature_flags_path(project)) end - click_button 'Save changes' + it 'shows the updated feature flag' do + within_feature_flag_row(1) do + expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') + expect(page).to have_css('.js-feature-flag-status .badge-danger') + + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(1)')).to have_content('*') + expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive') + expect(page.find('.badge:nth-child(2)')).to have_content('review/*') + expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive') + end + end + end end - it 'shows the updated feature flag' do - within_feature_flag_row(1) do - expect(page.find('.feature-flag-name')).to have_content('ci_live_trace') - expect(page).to have_css('.js-feature-flag-status .badge-danger') + context 'when feature flag audit enabled' do + include_examples 'succesfully updates feature flag' - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(1)')).to have_content('*') - expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive') - expect(page.find('.badge:nth-child(2)')).to have_content('review/*') - expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive') - end + it 'records audit event' do + visit(project_audit_events_path(project)) + + expect(page).to( + have_text("Updated feature flag ci live trace. Updated rule review/* active state from true to false.") + ) + end + end + + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + include_examples 'succesfully updates feature flag' + + it 'does not record audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_no_text("Updated feature flag") end end end context 'when user adds a new scope' do - before do - within_scope_row(3) do - within_environment_spec do - find('.js-env-input').set('production') - find('.js-create-button').click + shared_examples 'succesfully updates feature flag' do + before do + within_scope_row(3) do + within_environment_spec do + find('.js-env-input').set('production') + find('.js-create-button').click + end end + + click_button 'Save changes' + expect(page).to have_current_path(project_feature_flags_path(project)) end - click_button 'Save changes' + it 'shows the newly created scope' do + within_feature_flag_row(1) do + within_feature_flag_scopes do + expect(page.find('.badge:nth-child(3)')).to have_content('production') + expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive') + end + end + end end - it 'shows the newly created scope' do - within_feature_flag_row(1) do - within_feature_flag_scopes do - expect(page.find('.badge:nth-child(3)')).to have_content('production') - expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive') - end + context 'when feature flag audit enabled' do + include_examples 'succesfully updates feature flag' + + it 'records audit event' do + visit(project_audit_events_path(project)) + + expect(page).to( + have_text("Updated feature flag ci live trace") + ) + end + end + + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + include_examples 'succesfully updates feature flag' + + it 'does not record audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_no_text("Updated feature flag") end end end context 'when user deletes a scope' do - before do - within_scope_row(2) do - within_delete { find('.js-delete-scope').click } + shared_examples 'succesfully updates feature flag' do + before do + within_scope_row(2) do + within_delete { find('.js-delete-scope').click } + end + + click_button 'Save changes' + expect(page).to have_current_path(project_feature_flags_path(project)) + end + + it 'shows the updated feature flag' do + within_feature_flag_row(1) do + within_feature_flag_scopes do + expect(page).to have_css('.badge:nth-child(1)') + expect(page).not_to have_css('.badge:nth-child(2)') + end + end end + end + + context 'when feature flag audit enabled' do + include_examples 'succesfully updates feature flag' - click_button 'Save changes' + it 'records audit event' do + visit(project_audit_events_path(project)) + + expect(page).to( + have_text("Updated feature flag ci live trace") + ) + end end - it 'shows the updated feature flag' do - within_feature_flag_row(1) do - within_feature_flag_scopes do - expect(page).to have_css('.badge:nth-child(1)') - expect(page).not_to have_css('.badge:nth-child(2)') - end + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + include_examples 'succesfully updates feature flag' + + it 'does not record audit event' do + visit(project_audit_events_path(project)) + + expect(page).to have_no_text("Updated feature flag") end end end diff --git a/ee/spec/services/feature_flags/create_service_spec.rb b/ee/spec/services/feature_flags/create_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..16c4554527f76c7645d4e07accb21d469d679259 --- /dev/null +++ b/ee/spec/services/feature_flags/create_service_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe FeatureFlags::CreateService do + let(:project) { create(:project) } + let(:user) { create(:user) } + + 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 feature flag "\ + "with description \"description\". "\ + "Created rule * and set it as active. "\ + "Created rule production and set it as inactive." + + expect { subject }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.present.action).to eq(expected_message) + end + + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + it { expect(subject[:status]).to eq(:success) } + + it 'creates feature flag' do + expect { subject }.to change { Operations::FeatureFlag.count }.by(1) + end + + it 'does not create audit log' do + expect { subject }.not_to change { AuditEvent.count } + end + end + end + end +end diff --git a/ee/spec/services/feature_flags/destroy_service_spec.rb b/ee/spec/services/feature_flags/destroy_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..302a91b57c4f1d3df6936bae24a196e6ea2687c9 --- /dev/null +++ b/ee/spec/services/feature_flags/destroy_service_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe FeatureFlags::DestroyService do + let(:project) { create(:project) } + let(:user) { create(:user) } + let!(:feature_flag) { create(:operations_feature_flag) } + + describe '#execute' do + subject { described_class.new(project, user).execute(feature_flag) } + let(:audit_event_message) { AuditEvent.last.present.action } + + 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 #{feature_flag.name.tr('_', ' ')}.") + end + + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + it 'works successfully' 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 '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 diff --git a/ee/spec/services/feature_flags/update_service_spec.rb b/ee/spec/services/feature_flags/update_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fcb951fcb0bc1eb7b1e275dc393770b045ac5160 --- /dev/null +++ b/ee/spec/services/feature_flags/update_service_spec.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe FeatureFlags::UpdateService do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:feature_flag) { create(:operations_feature_flag) } + + 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.present.action + 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 new name. "\ + "Updated name from \"#{name_was.tr('_', ' ')}\" "\ + "to \"new name\".") + ) + end + + shared_examples 'disabled feature flag audit' do + context 'when feature flag audit is disabled' do + before do + stub_feature_flags(feature_flag_audit: false) + end + + 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 + end + + include_examples 'disabled feature flag audit' + + context 'with invalid params' do + let(:params) { { name: nil } } + + it 'returns error status' do + expect(subject[:status]).to eq(:error) + 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 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 \"\""\ + " to \"new description\".") + ) + end + + include_examples 'disabled feature flag audit' + end + + context 'when 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 stae' do + expect { subject }.to change { AuditEvent.count }.by(1) + expect(audit_event_message).to( + include("Updated rule * active state "\ + "from true to false.") + ) + end + + include_examples 'disabled feature flag audit' + 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 staging environment scope "\ + "from review to staging.") + ) + end + + include_examples 'disabled feature flag audit' + + 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 review.") + end + + include_examples 'disabled feature flag audit' + + context 'when scope can not be deleted' do + RSpec::Matchers.define_negated_matcher :not_change, :change + + 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 + subject + expect(audit_event_message).to( + include("Created rule review and set it as active.") + ) + end + + include_examples 'disabled feature flag audit' + + 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 + end +end