diff options
23 files changed, 385 insertions, 111 deletions
diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue index 1da4a8fea73..a5fa85f1ed5 100644 --- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue +++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue @@ -216,7 +216,7 @@ export default { v-if="currentRequest" :current-request="currentRequest" :requests="requests" - class="ml-auto" + class="gl-ml-auto" @change-current-request="changeCurrentRequest" /> <add-request v-on="$listeners" /> diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index abbfd4d66d4..de54102bdc8 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -63,11 +63,11 @@ module AlertManagement end def process_new_alert + return if resolving_alert? + if alert.save alert.execute_integrations SystemNoteService.create_new_alert(alert, alert_source) - - process_resolved_alert if resolving_alert? else logger.warn( message: "Unable to create AlertManagement::Alert from #{alert_source}", diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 2ab4bb47462..019246dfc9f 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -25,14 +25,18 @@ class EventCreateService def open_mr(merge_request, current_user) create_record_event(merge_request, current_user, :created).tap do track_event(event_action: :created, event_target: MergeRequest, author_id: current_user.id) - track_mr_snowplow_event(merge_request, current_user, :create) + track_snowplow_event(merge_request, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION, + :create, 'merge_requests_users') end end def close_mr(merge_request, current_user) create_record_event(merge_request, current_user, :closed).tap do track_event(event_action: :closed, event_target: MergeRequest, author_id: current_user.id) - track_mr_snowplow_event(merge_request, current_user, :close) + track_snowplow_event(merge_request, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION, + :close, 'merge_requests_users') end end @@ -43,7 +47,9 @@ class EventCreateService def merge_mr(merge_request, current_user) create_record_event(merge_request, current_user, :merged).tap do track_event(event_action: :merged, event_target: MergeRequest, author_id: current_user.id) - track_mr_snowplow_event(merge_request, current_user, :merge) + track_snowplow_event(merge_request, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION, + :merge, 'merge_requests_users') end end @@ -67,7 +73,9 @@ class EventCreateService create_record_event(note, current_user, :commented).tap do if note.is_a?(DiffNote) && note.for_merge_request? track_event(event_action: :commented, event_target: MergeRequest, author_id: current_user.id) - track_mr_snowplow_event(note, current_user, :comment) + track_snowplow_event(note, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION, + :comment, 'merge_requests_users') end end end @@ -100,12 +108,27 @@ class EventCreateService records = create.zip([:created].cycle) + update.zip([:updated].cycle) return [] if records.empty? + if create.any? + track_snowplow_event(create.first, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, + :create, 'design_users') + end + + if update.any? + track_snowplow_event(update.first, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, + :update, 'design_users') + end + create_record_events(records, current_user) end def destroy_designs(designs, current_user) return [] unless designs.present? + track_snowplow_event(designs.first, current_user, + Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION, + :destroy, 'design_users') create_record_events(designs.zip([:destroyed].cycle), current_user) end @@ -230,14 +253,14 @@ class EventCreateService Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(**params) end - def track_mr_snowplow_event(record, current_user, action) + def track_snowplow_event(record, current_user, category, action, label) return unless Feature.enabled?(:route_hll_to_snowplow_phase2) project = record.project Gitlab::Tracking.event( - Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION.to_s, + category.to_s, action.to_s, - label: 'merge_requests_users', + label: label, project: project, namespace: project.namespace, user: current_user diff --git a/config/events/1655179428_design_actions_create_.yml b/config/events/1655179428_design_actions_create_.yml new file mode 100644 index 00000000000..25e03a08a7a --- /dev/null +++ b/config/events/1655179428_design_actions_create_.yml @@ -0,0 +1,25 @@ +--- +description: Triggered from backend layer when design is created +category: design_actions +action: create +label_description: Constant string that match with ServicePing metric name of action_monthly_active_users_design_management +property_description: +value_description: +extra_properties: +identifiers: +- project +- user +- namespace +product_section: dev +product_stage: plan +product_group: group::product_planning +product_category: design_management +milestone: "15.1" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90107 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate diff --git a/config/events/1655179485_design_actions_update_.yml b/config/events/1655179485_design_actions_update_.yml new file mode 100644 index 00000000000..7648e51f0ae --- /dev/null +++ b/config/events/1655179485_design_actions_update_.yml @@ -0,0 +1,25 @@ +--- +description: Triggered from backend layer when design is updated +category: design_actions +action: update +label_description: Constant string that match with ServicePing metric name of action_monthly_active_users_design_management +property_description: +value_description: +extra_properties: +identifiers: +- project +- user +- namespace +product_section: dev +product_stage: plan +product_group: group::product_planning +product_category: design_management +milestone: "15.1" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90107 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate diff --git a/config/events/1655179517_design_actions_destroy_.yml b/config/events/1655179517_design_actions_destroy_.yml new file mode 100644 index 00000000000..fd75f698e5e --- /dev/null +++ b/config/events/1655179517_design_actions_destroy_.yml @@ -0,0 +1,25 @@ +--- +description: Triggered from backend layer when design is deleted +category: design_actions +action: destroy +label_description: Constant string that match with ServicePing metric name of action_monthly_active_users_design_management +property_description: +value_description: +extra_properties: +identifiers: +- project +- user +- namespace +product_section: dev +product_stage: plan +product_group: group::product_planning +product_category: design_management +milestone: "15.1" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/90107 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate diff --git a/doc/api/users.md b/doc/api/users.md index 5e41a0f6258..a84b236d1e3 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -2000,3 +2000,33 @@ Example response: } ] ``` + +## Disable two factor authentication (administrator only) + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/295260) in GitLab 15.2. + +Disables two factor authentication (2FA) for the specified user. Only administrators can disable 2FA for users. + +Administrators cannot disable 2FA for their own user account or other administrators using the API. Instead, they can disable an +administrator's 2FA [using the Rails console](../security/two_factor_authentication.md#for-a-single-user). + +```plaintext +PATCH /user/:id/disable_two_factor +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ------- | -------- | --------------------- | +| `id` | integer | yes | The ID of the user | + +```shell +curl --request PATCH --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/user/1/disable_two_factor" +``` + +Returns: + +- `204 No content` on success. +- `400 Bad request` if two factor authentication is not enabled for the specified user. +- `403 Forbidden` if not authenticated as an administrator. +- `404 User Not Found` if user cannot be found. diff --git a/doc/development/code_review.md b/doc/development/code_review.md index a6976271ddf..5e576acc15d 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -275,7 +275,7 @@ This saves reviewers time and helps authors catch mistakes earlier. Verify that the merge request meets all [contribution acceptance criteria](contributing/merge_request_workflow.md#contribution-acceptance-criteria). If a merge request is too large, fixes more than one issue, or implements more -than one feature, you should guide the author towards spltting the merge request +than one feature, you should guide the author towards splitting the merge request into smaller merge requests. When you are confident @@ -300,11 +300,18 @@ Because a maintainer's job only depends on their knowledge of the overall GitLab codebase, and not that of any specific domain, they can review, approve, and merge merge requests from any team and in any product area. -If a merge request is too large, fixes more than one issue, or implements more -than one feature, the maintainer can ask the author to make the merge request -smaller. Request the previous reviewer, or a merge request coach to help guide -the author on how to split the merge request, and to review the resulting -changes. +A maintainer should ask the author to make a merge request smaller if it is: + +- Too large. +- Fixes more than one issue. +- Implements more than one feature. +- Has a high complexity resulting in additional risk. + +The maintainer, any of the +reviewers, or a merge request coach can step up to help the author to divide work +into smaller iterations, and guide the author on how to split the merge request. +The author may choose to request that the current maintainers and reviewers review the split MRs +or request a new group of maintainers and reviewers. Maintainers do their best to also review the specifics of the chosen solution before merging, but as they are not necessarily [domain experts](#domain-experts), they may be poorly diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index d14263f660a..16185310d99 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -1754,7 +1754,7 @@ Be sure to create a full database backup before attempting any changes. #### Disable user two-factor authentication (2FA) Users with 2FA enabled can't sign in to GitLab. In that case, you must -[disable 2FA for everyone](../security/two_factor_authentication.md#disable-2fa-for-everyone), +[disable 2FA for everyone](../security/two_factor_authentication.md#for-all-users), after which users must reactivate 2FA. #### Reset CI/CD variables diff --git a/doc/security/two_factor_authentication.md b/doc/security/two_factor_authentication.md index ae13881fe6f..bcc50b01b0e 100644 --- a/doc/security/two_factor_authentication.md +++ b/doc/security/two_factor_authentication.md @@ -81,14 +81,33 @@ The following are important notes about 2FA: This action causes all subgroups with 2FA requirements to stop requiring that from their members. -## Disable 2FA for everyone +## Disable 2FA WARNING: -Disabling 2FA for everyone does not disable the [enforce 2FA for all users](#enforce-2fa-for-all-users) +Disabling 2FA for users does not disable the [enforce 2FA for all users](#enforce-2fa-for-all-users) or [enforce 2FA for all users in a group](#enforce-2fa-for-all-users-in-a-group) settings. You must also disable any enforced 2FA settings so users aren't asked to set up 2FA again when they next sign in to GitLab. +WARNING: +This is a permanent and irreversible action. Users must reactivate 2FA to use it again. + +### For a single user + +To disable 2FA for non-administrator users, we recommend using the [API endpoint](../api/users.md#disable-two-factor-authentication-administrator-only) +instead of the Rails console. +Using the [Rails console](../administration/operations/rails_console.md), 2FA for a single user can be disabled. +Connect to the Rails console and run: + +```ruby +admin = User.find_by_username('<USERNAME>') +user_to_disable = User.find_by_username('<USERNAME>') + +TwoFactor::DestroyService.new(admin, user: user_to_disable).execute +``` + +### For all users + There may be some special situations where you want to disable 2FA for everyone even when forced 2FA is disabled. There is a Rake task for that: @@ -100,10 +119,6 @@ sudo gitlab-rake gitlab:two_factor:disable_for_all_users sudo -u git -H bundle exec rake gitlab:two_factor:disable_for_all_users RAILS_ENV=production ``` -WARNING: -This is a permanent and irreversible action. Users have to -reactivate 2FA from scratch if they want to use it again. - ## 2FA for Git over SSH operations **(PREMIUM)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/270554) in GitLab 13.7. diff --git a/lib/api/users.rb b/lib/api/users.rb index 93df9413119..48bb81dafeb 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -325,6 +325,30 @@ module API end # rubocop: enable CodeReuse/ActiveRecord + desc "Disable two factor authentication for a user. Available only for admins" do + detail 'This feature was added in GitLab 15.2' + success Entities::UserWithAdmin + end + params do + requires :id, type: Integer, desc: 'The ID of the user' + end + patch ":id/disable_two_factor", feature_category: :authentication_and_authorization do + authenticated_as_admin! + + user = User.find_by_id(params[:id]) + not_found!('User') unless user + + forbidden!('Two-factor authentication for admins cannot be disabled via the API. Use the Rails console') if user.admin? + + result = TwoFactor::DestroyService.new(current_user, user: user).execute + + if result[:status] == :success + no_content! + else + bad_request!(result[:message]) + end + end + desc "Delete a user's identity. Available only for admins" do success Entities::UserWithAdmin end diff --git a/lib/sidebars/projects/menus/monitor_menu.rb b/lib/sidebars/projects/menus/monitor_menu.rb index c35bc1f5481..311c44f5f80 100644 --- a/lib/sidebars/projects/menus/monitor_menu.rb +++ b/lib/sidebars/projects/menus/monitor_menu.rb @@ -10,7 +10,6 @@ module Sidebars add_item(metrics_dashboard_menu_item) add_item(logs_menu_item) - add_item(tracing_menu_item) add_item(error_tracking_menu_item) add_item(alert_management_menu_item) add_item(incidents_menu_item) @@ -72,21 +71,6 @@ module Sidebars ) end - def tracing_menu_item - if !Feature.enabled?(:monitor_tracing, context.project) || - !can?(context.current_user, :read_environment, context.project) || - !can?(context.current_user, :admin_project, context.project) - return ::Sidebars::NilMenuItem.new(item_id: :tracing) - end - - ::Sidebars::MenuItem.new( - title: _('Tracing'), - link: project_tracing_path(context.project), - active_routes: { path: 'tracings#show' }, - item_id: :tracing - ) - end - def error_tracking_menu_item unless can?(context.current_user, :read_sentry_issue, context.project) return ::Sidebars::NilMenuItem.new(item_id: :error_tracking) diff --git a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb index 89150c73069..440e8e1238c 100644 --- a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb +++ b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb @@ -146,7 +146,7 @@ module QA issue_comments: issues.sum { |_k, v| v[:comments].length } } }, - not_imported: { + diff: { mrs: @mr_diff, issues: @issue_diff } @@ -256,11 +256,12 @@ module QA count_msg = "Expected to contain same amount of #{type}s. Source: #{expected.length}, Target: #{actual.length}" expect(actual.length).to eq(expected.length), count_msg - missing_comments = verify_comments(type, actual, expected) + comment_diff = verify_comments(type, actual, expected) { - "#{type}s": (expected.keys - actual.keys).map { |it| actual[it]&.slice(:title, :url) }.compact, - "#{type}_comments": missing_comments + "missing_#{type}s": (expected.keys - actual.keys).map { |it| actual[it]&.slice(:title, :url) }.compact, + "extra_#{type}s": (actual.keys - expected.keys).map { |it| expected[it]&.slice(:title, :url) }.compact, + "#{type}_comments": comment_diff } end @@ -271,7 +272,7 @@ module QA # @param [Hash] expected # @return [Hash] def verify_comments(type, actual, expected) - actual.each_with_object([]) do |(key, actual_item), missing_comments| + actual.each_with_object([]) do |(key, actual_item), diff| expected_item = expected[key] title = actual_item[:title] msg = "expected #{type} with title '#{title}' to have" @@ -305,16 +306,18 @@ module QA expect(actual_comments.length).to eq(expected_comments.length), comment_count_msg expect(actual_comments).to match_array(expected_comments) - # Save missing comments + # Save comment diff # - comment_diff = expected_comments - actual_comments - next if comment_diff.empty? + missing_comments = expected_comments - actual_comments + extra_comments = actual_comments - expected_comments + next if missing_comments.empty? && extra_comments.empty? - missing_comments << { + diff << { title: title, target_url: actual_item[:url], source_url: expected_item[:url], - missing_comments: comment_diff + missing_comments: missing_comments, + extra_comments: extra_comments } end end diff --git a/spec/graphql/mutations/design_management/delete_spec.rb b/spec/graphql/mutations/design_management/delete_spec.rb index 93fff5e5103..79196d4965d 100644 --- a/spec/graphql/mutations/design_management/delete_spec.rb +++ b/spec/graphql/mutations/design_management/delete_spec.rb @@ -86,9 +86,11 @@ RSpec.describe Mutations::DesignManagement::Delete do end end - it 'runs no more than 29 queries' do + it 'runs no more than 30 queries' do + allow(Gitlab::Tracking).to receive(:event) # rubocop:disable RSpec/ExpectGitlabTracking + filenames.each(&:present?) # ignore setup - # Queries: as of 2021-07-22 + # Queries: as of 2022-06-15 # ------------- # 01. routing query # 02. find project by id @@ -106,20 +108,21 @@ RSpec.describe Mutations::DesignManagement::Delete do # 15. current designs by filename and issue # 16, 17 project.authorizations for user (same query as 5) # 18. find route by id and source_type + # 19. find plan for standard context # ------------- our queries are below: - # 19. start transaction 1 - # 20. start transaction 2 - # 21. find version by sha and issue - # 22. exists version with sha and issue? - # 23. leave transaction 2 - # 24. create version with sha and issue - # 25. create design-version links - # 26. validate version.actions.present? - # 27. validate version.issue.present? - # 28. validate version.sha is unique - # 29. leave transaction 1 + # 20. start transaction 1 + # 21. start transaction 2 + # 22. find version by sha and issue + # 23. exists version with sha and issue? + # 24. leave transaction 2 + # 25. create version with sha and issue + # 26. create design-version links + # 27. validate version.actions.present? + # 28. validate version.issue.present? + # 29. validate version.sha is unique + # 30. leave transaction 1 # - expect { run_mutation }.not_to exceed_query_limit(29) + expect { run_mutation }.not_to exceed_query_limit(30) end end diff --git a/spec/lib/sidebars/projects/menus/monitor_menu_spec.rb b/spec/lib/sidebars/projects/menus/monitor_menu_spec.rb index b11c9db4e46..9d5845d20e5 100644 --- a/spec/lib/sidebars/projects/menus/monitor_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/monitor_menu_spec.rb @@ -82,20 +82,6 @@ RSpec.describe Sidebars::Projects::Menus::MonitorMenu do end end - describe 'Tracing' do - let(:item_id) { :tracing } - - it_behaves_like 'access rights checks' - - context 'when feature disabled' do - before do - stub_feature_flags(monitor_tracing: false) - end - - specify { is_expected.to be_nil } - end - end - describe 'Error Tracking' do let(:item_id) { :error_tracking } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index d4dc7375e9e..3b0f1032288 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -17,6 +17,8 @@ RSpec.describe API::Users do let(:deactivated_user) { create(:user, state: 'deactivated') } let(:banned_user) { create(:user, :banned) } let(:internal_user) { create(:user, :bot) } + let(:user_with_2fa) { create(:user, :two_factor_via_otp) } + let(:admin_with_2fa) { create(:admin, :two_factor_via_otp) } context 'admin notes' do let_it_be(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } @@ -81,6 +83,79 @@ RSpec.describe API::Users do end end + describe "PATCH /users/:id/disable_two_factor" do + context "when current user is an admin" do + it "returns a 204 when 2FA is disabled for the target user" do + expect do + patch api("/users/#{user_with_2fa.id}/disable_two_factor", admin) + end.to change { user_with_2fa.reload.two_factor_enabled? } + .from(true) + .to(false) + expect(response).to have_gitlab_http_status(:no_content) + end + + it "uses TwoFactor Destroy Service" do + destroy_service = instance_double(TwoFactor::DestroyService, execute: nil) + expect(TwoFactor::DestroyService).to receive(:new) + .with(admin, user: user_with_2fa) + .and_return(destroy_service) + expect(destroy_service).to receive(:execute) + + patch api("/users/#{user_with_2fa.id}/disable_two_factor", admin) + end + + it "returns a 400 if 2FA is not enabled for the target user" do + expect(TwoFactor::DestroyService).to receive(:new).and_call_original + + expect do + patch api("/users/#{user.id}/disable_two_factor", admin) + end.not_to change { user.reload.two_factor_enabled? } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - Two-factor authentication is not enabled for this user") + end + + it "returns a 403 if the target user is an admin" do + expect(TwoFactor::DestroyService).to receive(:new).never + + expect do + patch api("/users/#{admin_with_2fa.id}/disable_two_factor", admin) + end.not_to change { admin_with_2fa.reload.two_factor_enabled? } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq("403 Forbidden - Two-factor authentication for admins cannot be disabled via the API. Use the Rails console") + end + + it "returns a 404 if the target user cannot be found" do + expect(TwoFactor::DestroyService).to receive(:new).never + + patch api("/users/#{non_existing_record_id}/disable_two_factor", admin) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq("404 User Not Found") + end + end + + context "when current user is not an admin" do + it "returns a 403" do + expect do + patch api("/users/#{user_with_2fa.id}/disable_two_factor", user) + end.not_to change { user_with_2fa.reload.two_factor_enabled? } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq("403 Forbidden") + end + end + + context "when unauthenticated" do + it "returns a 401" do + patch api("/users/#{user_with_2fa.id}/disable_two_factor") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + describe 'GET /users/' do context 'when unauthenticated' do it "does not contain certain fields" do diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 56da85cc4a0..e66b413a5c9 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -379,10 +379,14 @@ RSpec.describe EventCreateService, :clean_gitlab_redis_cache, :clean_gitlab_redi end end - describe 'design events' do + describe 'design events', :snowplow do let_it_be(:design) { create(:design, project: project) } let_it_be(:author) { user } + before do + allow(Gitlab::Tracking).to receive(:event) # rubocop:disable RSpec/ExpectGitlabTracking + end + describe '#save_designs' do let_it_be(:updated) { create_list(:design, 5) } let_it_be(:created) { create_list(:design, 3) } @@ -411,6 +415,44 @@ RSpec.describe EventCreateService, :clean_gitlab_redis_cache, :clean_gitlab_redi it_behaves_like "it records the event in the event counter" do let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION } end + + it 'records correct create payload with Snowplow event' do + service.save_designs(author, create: [design]) + + expect_snowplow_event( + category: Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION.to_s, + action: 'create', + namespace: design.project.namespace, + user: author, + project: design.project, + label: 'design_users' + ) + end + + it 'records correct update payload with Snowplow event' do + service.save_designs(author, update: [design]) + + expect_snowplow_event( + category: Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION.to_s, + action: 'update', + namespace: design.project.namespace, + user: author, + project: design.project, + label: 'design_users' + ) + end + + context 'when FF is disabled' do + before do + stub_feature_flags(route_hll_to_snowplow_phase2: false) + end + + it 'doesnt emit snowwplow events', :snowplow do + subject + + expect_no_snowplow_event + end + end end describe '#destroy_designs' do @@ -434,6 +476,31 @@ RSpec.describe EventCreateService, :clean_gitlab_redis_cache, :clean_gitlab_redi it_behaves_like "it records the event in the event counter" do let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION } end + + it 'records correct payload with Snowplow event' do + service.destroy_designs([design], author) + + expect_snowplow_event( + category: Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION.to_s, + action: 'destroy', + namespace: design.project.namespace, + user: author, + project: design.project, + label: 'design_users' + ) + end + + context 'when FF is disabled' do + before do + stub_feature_flags(route_hll_to_snowplow_phase2: false) + end + + it 'doesnt emit snowwplow events', :snowplow do + subject + + expect_no_snowplow_event + end + end end end diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index 0d0bb317df2..6f760e6dbfa 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -228,12 +228,14 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do context 'when payload exceeds max amount of processable alerts' do # We are defining 2 alerts in payload_raw above let(:max_alerts) { 1 } + let(:fingerprint) { prometheus_alert_payload_fingerprint(alert_resolved) } before do stub_const("#{described_class}::PROCESS_MAX_ALERTS", max_alerts) create(:prometheus_integration, project: project) create(:project_alerting_setting, project: project, token: token) + create(:alert_management_alert, project: project, fingerprint: fingerprint) allow(Gitlab::AppLogger).to receive(:warn) end diff --git a/spec/support/helpers/prometheus_helpers.rb b/spec/support/helpers/prometheus_helpers.rb index d49abbf3f19..2f6b36f43c0 100644 --- a/spec/support/helpers/prometheus_helpers.rb +++ b/spec/support/helpers/prometheus_helpers.rb @@ -267,6 +267,13 @@ module PrometheusHelpers } end + def prometheus_alert_payload_fingerprint(prometheus_alert) + # timestamp is hard-coded in #prometheus_map_alert_payload + fingerprint = "#{prometheus_alert.id}/2018-09-24T08:57:31.095725221Z" + + Gitlab::AlertManagement::Fingerprint.generate(fingerprint) + end + private def prometheus_map_alert_payload(status, alert) diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index d277a45584d..50e6d4aad1b 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -84,7 +84,6 @@ RSpec.shared_context 'project navbar structure' do nav_sub_items: [ _('Metrics'), _('Logs'), - _('Tracing'), _('Error Tracking'), _('Alerts'), _('Incidents'), diff --git a/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb b/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb index 86e7da5bcbe..f8e096297d3 100644 --- a/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb +++ b/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb @@ -56,7 +56,7 @@ RSpec.shared_examples 'processes recovery alert' do context 'seen for the first time' do let(:alert) { AlertManagement::Alert.last } - include_examples 'processes never-before-seen recovery alert' + it_behaves_like 'alerts service responds with an error and takes no actions', :bad_request end context 'for an existing alert with the same fingerprint' do @@ -107,7 +107,7 @@ RSpec.shared_examples 'processes recovery alert' do context 'which is resolved' do let_it_be(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: gitlab_fingerprint, monitoring_tool: source) } - include_examples 'processes never-before-seen recovery alert' + it_behaves_like 'alerts service responds with an error and takes no actions', :bad_request end end end diff --git a/spec/support/shared_examples/services/alert_management_shared_examples.rb b/spec/support/shared_examples/services/alert_management_shared_examples.rb index f644f1a1687..571cb7dc03d 100644 --- a/spec/support/shared_examples/services/alert_management_shared_examples.rb +++ b/spec/support/shared_examples/services/alert_management_shared_examples.rb @@ -68,14 +68,14 @@ RSpec.shared_examples 'processes one firing and one resolved prometheus alerts' expect(Gitlab::AppLogger).not_to receive(:warn) expect { subject } - .to change(AlertManagement::Alert, :count).by(2) - .and change(Note, :count).by(4) + .to change(AlertManagement::Alert, :count).by(1) + .and change(Note, :count).by(1) expect(subject).to be_success expect(subject.payload[:alerts]).to all(be_a_kind_of(AlertManagement::Alert)) - expect(subject.payload[:alerts].size).to eq(2) + expect(subject.payload[:alerts].size).to eq(1) end it_behaves_like 'processes incident issues' - it_behaves_like 'sends alert notification emails', count: 2 + it_behaves_like 'sends alert notification emails' end diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb index 3943355bffd..7f961b3a4e9 100644 --- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb @@ -437,32 +437,6 @@ RSpec.describe 'layouts/nav/sidebar/_project' do end end - describe 'Tracing' do - it 'has a link to the tracing page' do - render - - expect(rendered).to have_link('Tracing', href: project_tracing_path(project)) - end - - context 'without project.tracing_external_url' do - it 'has a link to the tracing page' do - render - - expect(rendered).to have_link('Tracing', href: project_tracing_path(project)) - end - end - - describe 'when the user does not have access' do - let(:user) { nil } - - it 'does not have a link to the tracing page' do - render - - expect(rendered).not_to have_text 'Tracing' - end - end - end - describe 'Error Tracking' do it 'has a link to the error tracking page' do render |