From 26891eec2cea8ca5e75a96a50d8785da6042cc5a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 25 Apr 2023 03:09:02 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/rules.gitlab-ci.yml | 43 +++++- .../deprecate_track_redis_hll_event.yml | 13 -- .../rspec/missing_feature_category.yml | 1 - .../components/profile_preferences.vue | 4 + .../concerns/product_analytics_tracking.rb | 20 ++- app/controllers/concerns/redis_tracking.rb | 49 ------- app/controllers/concerns/snippets_actions.rb | 4 +- app/controllers/concerns/wiki_actions.rb | 4 +- .../profiles/preferences_controller.rb | 4 +- .../projects/pipelines_controller.rb | 12 +- app/controllers/search_controller.rb | 2 +- app/controllers/users_controller.rb | 7 +- app/models/user.rb | 61 ++++---- app/services/users/update_service.rb | 29 +++- app/views/profiles/preferences/show.html.haml | 16 +++ app/views/users/show.html.haml | 2 +- .../development/disable_follow_users.yml | 8 ++ ...opulate_vulnerability_dismissal_fields.yml | 6 + ..._disabled_following_to_user_preferences.rb | 9 ++ ...0123709_validate_fk_projects_creator_id.rb | 15 ++ ..._to_null_dismissed_info_vulnerabilities.rb | 18 +++ ...12104514_add_index_to_group_group_links.rb | 16 +++ ...populate_vulnerability_dismissal_fields.rb | 29 ++++ db/schema_migrations/20230328165313 | 1 + db/schema_migrations/20230410123709 | 1 + db/schema_migrations/20230411011959 | 1 + db/schema_migrations/20230412104514 | 1 + db/schema_migrations/20230412185837 | 1 + db/structure.sql | 7 +- lib/api/entities/user.rb | 17 ++- lib/api/users.rb | 5 +- ...populate_vulnerability_dismissal_fields.rb | 90 ++++++++++++ locale/gitlab.pot | 12 ++ .../gitlab/deprecate_track_redis_hll_event.rb | 33 ----- .../concerns/redis_tracking_spec.rb | 135 ------------------ .../profiles/preferences_controller_spec.rb | 28 ++++ spec/frontend/fixtures/jobs.rb | 7 +- spec/lib/api/entities/user_spec.rb | 54 ++++++- ...ate_vulnerability_dismissal_fields_spec.rb | 114 +++++++++++++++ ...ate_vulnerability_dismissal_fields_spec.rb | 37 +++++ spec/models/user_spec.rb | 81 +++++++++++ spec/requests/api/users_spec.rb | 14 ++ spec/requests/users_controller_spec.rb | 29 ++++ .../deprecate_track_redis_hll_event_spec.rb | 17 --- spec/services/users/update_service_spec.rb | 43 ++++++ 45 files changed, 791 insertions(+), 309 deletions(-) delete mode 100644 .rubocop_todo/gitlab/deprecate_track_redis_hll_event.yml delete mode 100644 app/controllers/concerns/redis_tracking.rb create mode 100644 config/feature_flags/development/disable_follow_users.yml create mode 100644 db/docs/batched_background_migrations/populate_vulnerability_dismissal_fields.yml create mode 100644 db/migrate/20230328165313_add_disabled_following_to_user_preferences.rb create mode 100644 db/post_migrate/20230410123709_validate_fk_projects_creator_id.rb create mode 100644 db/post_migrate/20230411011959_add_temp_index_to_null_dismissed_info_vulnerabilities.rb create mode 100644 db/post_migrate/20230412104514_add_index_to_group_group_links.rb create mode 100644 db/post_migrate/20230412185837_queue_populate_vulnerability_dismissal_fields.rb create mode 100644 db/schema_migrations/20230328165313 create mode 100644 db/schema_migrations/20230410123709 create mode 100644 db/schema_migrations/20230411011959 create mode 100644 db/schema_migrations/20230412104514 create mode 100644 db/schema_migrations/20230412185837 create mode 100644 lib/gitlab/background_migration/populate_vulnerability_dismissal_fields.rb delete mode 100644 rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb delete mode 100644 spec/controllers/concerns/redis_tracking_spec.rb create mode 100644 spec/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields_spec.rb create mode 100644 spec/migrations/20230412185837_queue_populate_vulnerability_dismissal_fields_spec.rb delete mode 100644 spec/rubocop/cop/gitlab/deprecate_track_redis_hll_event_spec.rb diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 321993799e4..5ba0bc20a75 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -821,10 +821,23 @@ when: never - <<: *if-merge-request-targeting-stable-branch - <<: *if-merge-request-labels-run-review-app - - <<: *if-dot-com-gitlab-org-and-security-merge-request + - <<: *if-merge-request changes: *ci-build-images-patterns - - <<: *if-dot-com-gitlab-org-and-security-merge-request + - <<: *if-merge-request changes: *code-qa-patterns + # Rules to support .qa:rules:package-and-test-ee + - <<: *if-merge-request + changes: *dependency-patterns + - <<: *if-merge-request-labels-run-all-e2e + - <<: *if-dot-com-gitlab-org-and-security-merge-request-manual-ff-package-and-e2e + changes: *feature-flag-development-config-patterns + - <<: *if-merge-request + changes: *feature-flag-development-config-patterns + - <<: *if-merge-request + changes: *nodejs-patterns + - <<: *if-merge-request + changes: *ci-qa-patterns + - <<: *if-force-ci .build-images:rules:build-qa-image: rules: @@ -843,7 +856,6 @@ - <<: *if-dot-com-gitlab-org-schedule variables: ARCH: amd64,arm64 - - <<: *if-force-ci - <<: *if-ruby2-branch .build-images:rules:build-qa-image-as-if-foss: @@ -872,7 +884,6 @@ - <<: *if-merge-request-targeting-stable-branch - <<: *if-ruby2-branch - <<: *if-merge-request-labels-run-review-app - - <<: *if-merge-request-labels-run-all-e2e - <<: *if-auto-deploy-branches - !reference [".releases:rules:canonical-dot-com-gitlab-stable-branch-only-setup-test-env-patterns", rules] - <<: *if-default-refs @@ -1275,6 +1286,8 @@ ########## .notify:rules:create-issues-for-failing-tests: rules: + - <<: *if-not-canonical-namespace + when: never # Don't report child pipeline failures - if: '$CI_PIPELINE_SOURCE == "parent_pipeline"' when: never @@ -1331,14 +1344,32 @@ when: never - <<: *if-merge-request-targeting-stable-branch allow_failure: true - - <<: *if-dot-com-gitlab-org-and-security-merge-request + - <<: *if-merge-request changes: *code-backstage-qa-patterns allow_failure: true - <<: *if-dot-com-gitlab-org-schedule allow_failure: true + - <<: *if-ruby2-branch + # Rules to support .qa:rules:package-and-test-ee + - <<: *if-merge-request + changes: *dependency-patterns + allow_failure: true + - <<: *if-merge-request-labels-run-all-e2e + allow_failure: true + - <<: *if-dot-com-gitlab-org-and-security-merge-request-manual-ff-package-and-e2e + changes: *feature-flag-development-config-patterns + allow_failure: true + - <<: *if-merge-request + changes: *feature-flag-development-config-patterns + allow_failure: true + - <<: *if-merge-request + changes: *nodejs-patterns + allow_failure: true + - <<: *if-merge-request + changes: *ci-qa-patterns + allow_failure: true - <<: *if-force-ci allow_failure: true - - <<: *if-ruby2-branch .qa:rules:package-and-test-common: rules: diff --git a/.rubocop_todo/gitlab/deprecate_track_redis_hll_event.yml b/.rubocop_todo/gitlab/deprecate_track_redis_hll_event.yml deleted file mode 100644 index f5433dfd320..00000000000 --- a/.rubocop_todo/gitlab/deprecate_track_redis_hll_event.yml +++ /dev/null @@ -1,13 +0,0 @@ ---- -Gitlab/DeprecateTrackRedisHLLEvent: - Exclude: - - 'app/controllers/concerns/snippets_actions.rb' - - 'app/controllers/concerns/wiki_actions.rb' - - 'app/controllers/projects/blob_controller.rb' - - 'app/controllers/projects/pipelines_controller.rb' - - 'ee/app/controllers/admin/audit_logs_controller.rb' - - 'ee/app/controllers/admin/credentials_controller.rb' - - 'ee/app/controllers/groups/analytics/ci_cd_analytics_controller.rb' - - 'ee/app/controllers/groups/audit_events_controller.rb' - - 'ee/app/controllers/groups/epic_boards_controller.rb' - - 'spec/controllers/concerns/redis_tracking_spec.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 04cf1ec1a07..2941f2999d3 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -5494,7 +5494,6 @@ RSpec/MissingFeatureCategory: - 'spec/rubocop/cop/gitlab/change_timezone_spec.rb' - 'spec/rubocop/cop/gitlab/const_get_inherit_false_spec.rb' - 'spec/rubocop/cop/gitlab/delegate_predicate_methods_spec.rb' - - 'spec/rubocop/cop/gitlab/deprecate_track_redis_hll_event_spec.rb' - 'spec/rubocop/cop/gitlab/event_store_subscriber_spec.rb' - 'spec/rubocop/cop/gitlab/except_spec.rb' - 'spec/rubocop/cop/gitlab/feature_available_usage_spec.rb' diff --git a/app/assets/javascripts/profile/preferences/components/profile_preferences.vue b/app/assets/javascripts/profile/preferences/components/profile_preferences.vue index d0d947ddd6e..164ec46cdb9 100644 --- a/app/assets/javascripts/profile/preferences/components/profile_preferences.vue +++ b/app/assets/javascripts/profile/preferences/components/profile_preferences.vue @@ -131,6 +131,10 @@ export default { :message-url="view.message_url" :config="$options.integrationViewConfigs[view.name]" /> + + +
+
diff --git a/app/controllers/concerns/product_analytics_tracking.rb b/app/controllers/concerns/product_analytics_tracking.rb index fc33770b4d8..5424354b92c 100644 --- a/app/controllers/concerns/product_analytics_tracking.rb +++ b/app/controllers/concerns/product_analytics_tracking.rb @@ -2,7 +2,6 @@ module ProductAnalyticsTracking include Gitlab::Tracking::Helpers - include RedisTracking extend ActiveSupport::Concern class_methods do @@ -39,4 +38,23 @@ module ProductAnalyticsTracking **optional_arguments ) end + + def track_unique_redis_hll_event(event_name, &block) + custom_id = block ? yield(self) : nil + + unique_id = custom_id || visitor_id + + return unless unique_id + + Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values: unique_id) + end + + def visitor_id + return cookies[:visitor_id] if cookies[:visitor_id].present? + return unless current_user + + uuid = SecureRandom.uuid + cookies[:visitor_id] = { value: uuid, expires: 24.months } + uuid + end end diff --git a/app/controllers/concerns/redis_tracking.rb b/app/controllers/concerns/redis_tracking.rb deleted file mode 100644 index 445e72b8266..00000000000 --- a/app/controllers/concerns/redis_tracking.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -# Example: -# -# # In controller include module -# # Track event for index action -# -# include RedisTracking -# -# track_redis_hll_event :index, :show, name: 'i_analytics_dev_ops_score' -# -# You can also pass custom conditions using `if:`, using the same format as with Rails callbacks. -# You can also pass an optional block that calculates and returns a custom id to track. -module RedisTracking - include Gitlab::Tracking::Helpers - extend ActiveSupport::Concern - - class_methods do - def track_redis_hll_event(*controller_actions, name:, if: nil, &block) - custom_conditions = Array.wrap(binding.local_variable_get('if')) - conditions = [:trackable_html_request?, *custom_conditions] - - after_action only: controller_actions, if: conditions do - track_unique_redis_hll_event(name, &block) - end - end - end - - private - - def track_unique_redis_hll_event(event_name, &block) - custom_id = block ? yield(self) : nil - - unique_id = custom_id || visitor_id - - return unless unique_id - - Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values: unique_id) - end - - def visitor_id - return cookies[:visitor_id] if cookies[:visitor_id].present? - return unless current_user - - uuid = SecureRandom.uuid - cookies[:visitor_id] = { value: uuid, expires: 24.months } - uuid - end -end diff --git a/app/controllers/concerns/snippets_actions.rb b/app/controllers/concerns/snippets_actions.rb index 1bb81a46e50..62c5aee16e4 100644 --- a/app/controllers/concerns/snippets_actions.rb +++ b/app/controllers/concerns/snippets_actions.rb @@ -9,13 +9,13 @@ module SnippetsActions include Gitlab::NoteableMetadata include Snippets::SendBlob include SnippetsSort - include RedisTracking + include ProductAnalyticsTracking included do skip_before_action :verify_authenticity_token, if: -> { action_name == 'show' && js_request? } - track_redis_hll_event :show, name: 'i_snippets_show' + track_event :show, name: 'i_snippets_show' respond_to :html end diff --git a/app/controllers/concerns/wiki_actions.rb b/app/controllers/concerns/wiki_actions.rb index a007abacacc..265cf2a7698 100644 --- a/app/controllers/concerns/wiki_actions.rb +++ b/app/controllers/concerns/wiki_actions.rb @@ -5,7 +5,7 @@ module WikiActions include PreviewMarkdown include SendsBlob include Gitlab::Utils::StrongMemoize - include RedisTracking + include ProductAnalyticsTracking extend ActiveSupport::Concern RESCUE_GIT_TIMEOUTS_IN = %w[show edit history diff pages].freeze @@ -46,7 +46,7 @@ module WikiActions end end - track_redis_hll_event :show, name: 'wiki_action' + track_event :show, name: 'wiki_action' helper_method :view_file_button, :diff_file_html_data diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index 7786bad4251..b7e8432c039 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -37,7 +37,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController end def preferences_param_names - [ + preferences_param_names = [ :color_scheme_id, :diffs_deletion_color, :diffs_addition_color, @@ -60,6 +60,8 @@ class Profiles::PreferencesController < Profiles::ApplicationController :use_legacy_web_ide, :use_new_navigation ] + preferences_param_names << :enabled_following if ::Feature.enabled?(:disable_follow_users, user) + preferences_param_names end end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index a8107a46b4f..30f582f90a5 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -2,7 +2,7 @@ class Projects::PipelinesController < Projects::ApplicationController include ::Gitlab::Utils::StrongMemoize - include RedisTracking + include ProductAnalyticsTracking include ProductAnalyticsTracking include ProjectStatsRefreshConflictsGuard @@ -34,11 +34,11 @@ class Projects::PipelinesController < Projects::ApplicationController label: 'redis_hll_counters.analytics.analytics_total_unique_counts_monthly', destinations: %i[redis_hll snowplow] - track_redis_hll_event :charts, name: 'p_analytics_ci_cd_pipelines', if: -> { should_track_ci_cd_pipelines? } - track_redis_hll_event :charts, name: 'p_analytics_ci_cd_deployment_frequency', if: -> { should_track_ci_cd_deployment_frequency? } - track_redis_hll_event :charts, name: 'p_analytics_ci_cd_lead_time', if: -> { should_track_ci_cd_lead_time? } - track_redis_hll_event :charts, name: 'p_analytics_ci_cd_time_to_restore_service', if: -> { should_track_ci_cd_time_to_restore_service? } - track_redis_hll_event :charts, name: 'p_analytics_ci_cd_change_failure_rate', if: -> { should_track_ci_cd_change_failure_rate? } + track_event :charts, name: 'p_analytics_ci_cd_pipelines', conditions: -> { should_track_ci_cd_pipelines? } + track_event :charts, name: 'p_analytics_ci_cd_deployment_frequency', conditions: -> { should_track_ci_cd_deployment_frequency? } + track_event :charts, name: 'p_analytics_ci_cd_lead_time', conditions: -> { should_track_ci_cd_lead_time? } + track_event :charts, name: 'p_analytics_ci_cd_time_to_restore_service', conditions: -> { should_track_ci_cd_time_to_restore_service? } + track_event :charts, name: 'p_analytics_ci_cd_change_failure_rate', conditions: -> { should_track_ci_cd_change_failure_rate? } wrap_parameters Ci::Pipeline diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 688c56e56e0..a3c6499bc54 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -3,7 +3,7 @@ class SearchController < ApplicationController include ControllerWithCrossProjectAccessCheck include SearchHelper - include RedisTracking + include ProductAnalyticsTracking include ProductAnalyticsTracking include SearchRateLimitable diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e4354eaa452..02b29d42313 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -191,7 +191,12 @@ class UsersController < ApplicationController def follow followee = current_user.follow(user) - flash[:alert] = followee.errors.full_messages.join(', ') if followee&.errors&.any? + if followee + flash[:alert] = followee.errors.full_messages.join(', ') if followee&.errors&.any? + else + flash[:alert] = s_('Action not allowed.') + end + redirect_path = referer_path(request) || @user redirect_to redirect_path diff --git a/app/models/user.rb b/app/models/user.rb index 0aa509e58d7..cb1fbd97a00 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -349,29 +349,30 @@ class User < ApplicationRecord # User's role enum role: { software_developer: 0, development_team_lead: 1, devops_engineer: 2, systems_administrator: 3, security_analyst: 4, data_analyst: 5, product_manager: 6, product_designer: 7, other: 8 }, _suffix: true - delegate :notes_filter_for, - :set_notes_filter, - :first_day_of_week, :first_day_of_week=, - :timezone, :timezone=, - :time_display_relative, :time_display_relative=, - :time_format_in_24h, :time_format_in_24h=, - :show_whitespace_in_diffs, :show_whitespace_in_diffs=, - :view_diffs_file_by_file, :view_diffs_file_by_file=, - :pass_user_identities_to_ci_jwt, :pass_user_identities_to_ci_jwt=, - :tab_width, :tab_width=, - :sourcegraph_enabled, :sourcegraph_enabled=, - :gitpod_enabled, :gitpod_enabled=, - :setup_for_company, :setup_for_company=, - :render_whitespace_in_code, :render_whitespace_in_code=, - :markdown_surround_selection, :markdown_surround_selection=, - :markdown_automatic_lists, :markdown_automatic_lists=, - :diffs_deletion_color, :diffs_deletion_color=, - :diffs_addition_color, :diffs_addition_color=, - :use_legacy_web_ide, :use_legacy_web_ide=, - :use_new_navigation, :use_new_navigation=, - :pinned_nav_items, :pinned_nav_items=, - :achievements_enabled, :achievements_enabled=, - to: :user_preference + delegate :notes_filter_for, + :set_notes_filter, + :first_day_of_week, :first_day_of_week=, + :timezone, :timezone=, + :time_display_relative, :time_display_relative=, + :time_format_in_24h, :time_format_in_24h=, + :show_whitespace_in_diffs, :show_whitespace_in_diffs=, + :view_diffs_file_by_file, :view_diffs_file_by_file=, + :pass_user_identities_to_ci_jwt, :pass_user_identities_to_ci_jwt=, + :tab_width, :tab_width=, + :sourcegraph_enabled, :sourcegraph_enabled=, + :gitpod_enabled, :gitpod_enabled=, + :setup_for_company, :setup_for_company=, + :render_whitespace_in_code, :render_whitespace_in_code=, + :markdown_surround_selection, :markdown_surround_selection=, + :markdown_automatic_lists, :markdown_automatic_lists=, + :diffs_deletion_color, :diffs_deletion_color=, + :diffs_addition_color, :diffs_addition_color=, + :use_legacy_web_ide, :use_legacy_web_ide=, + :use_new_navigation, :use_new_navigation=, + :pinned_nav_items, :pinned_nav_items=, + :achievements_enabled, :achievements_enabled=, + :enabled_following, :enabled_following=, + to: :user_preference delegate :path, to: :namespace, allow_nil: true, prefix: true delegate :job_title, :job_title=, to: :user_detail, allow_nil: true @@ -1694,7 +1695,7 @@ class User < ApplicationRecord end def follow(user) - return false if self.id == user.id + return false unless following_users_allowed?(user) begin followee = Users::UserFollowUser.create(follower_id: self.id, followee_id: user.id) @@ -1713,6 +1714,18 @@ class User < ApplicationRecord end end + def following_users_allowed?(user) + return false if self.id == user.id + + following_users_enabled? && user.following_users_enabled? + end + + def following_users_enabled? + return true unless ::Feature.enabled?(:disable_follow_users, self) + + enabled_following + end + def forkable_namespaces strong_memoize(:forkable_namespaces) do personal_namespace = Namespace.where(id: namespace_id) diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 96018db5974..36c41c03303 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -6,6 +6,7 @@ module Users attr_reader :user, :identity_params ATTRS_REQUIRING_PASSWORD_CHECK = %w[email].freeze + BATCH_SIZE = 100 def initialize(current_user, params = {}) @current_user = current_user @@ -34,7 +35,7 @@ module Users reset_unconfirmed_email if @user.save(validate: validate) && update_status - notify_success(user_exists) + after_update(user_exists) else messages = @user.errors.full_messages + Array(@user.status&.errors&.full_messages) error(messages.uniq.join('. ')) @@ -80,8 +81,6 @@ module Users def notify_success(user_exists) notify_new_user(@user, nil) unless user_exists - - success end def discard_read_only_attributes @@ -118,6 +117,30 @@ module Users def provider_params identity_params.slice(*provider_attributes) end + + def after_update(user_exists) + notify_success(user_exists) + remove_followers_and_followee! if ::Feature.enabled?(:disable_follow_users, user) + + success + end + + def remove_followers_and_followee! + return false unless user.user_preference.enabled_following_previously_changed?(from: true, to: false) + + # rubocop: disable CodeReuse/ActiveRecord + loop do + inner_query = Users::UserFollowUser + .where(follower_id: user.id).or(Users::UserFollowUser.where(followee_id: user.id)) + .select(:follower_id, :followee_id) + .limit(BATCH_SIZE) + + deleted_records = Users::UserFollowUser.where('(follower_id, followee_id) IN (?)', inner_query).delete_all + + break if deleted_records == 0 + end + # rubocop: enable CodeReuse/ActiveRecord + end end end diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index c16469bbf79..8fb66cb3cd9 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -170,5 +170,21 @@ - if Feature.enabled?(:user_time_settings) .form-group = f.gitlab_ui_checkbox_component :time_format_in_24h, s_('Preferences|Display time in 24-hour format') + - if Feature.enabled?(:disable_follow_users, @user) + .row.js-preferences-form.js-search-settings-section + .col-sm-12 + %hr + .col-lg-4.profile-settings-sidebar#disabled_following + %h4.gl-mt-0 + = s_('Preferences|Disable follow users feature') + %p + = s_('Preferences|Turns off the ability to follow or be followed by other users.') + = succeed '.' do + = link_to _('Learn more'), help_page_path('user/profile/index', anchor: 'follow-users'), target: '_blank', rel: 'noopener noreferrer' + .col-lg-8 + .form-group + = f.gitlab_ui_checkbox_component :enabled_following, + s_('Preferences|Disable follow users') + #js-profile-preferences-app{ data: data_attributes } diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 70dccc4821b..e26b4de7b20 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -36,7 +36,7 @@ = render Pajamas::ButtonComponent.new(href: [:admin, @user], icon: 'user', button_options: { class: 'gl-flex-grow-1 gl-mx-1 has-tooltip', title: s_('UserProfile|View user in admin area'), data: {toggle: 'tooltip', placement: 'bottom', container: 'body'}}) - - if current_user && current_user.id != @user.id + - if current_user && current_user.following_users_allowed?(@user) - if current_user.following?(@user) = form_tag user_unfollow_path(@user, :json), class: link_classes + 'gl-display-inline-block' do = render Pajamas::ButtonComponent.new(type: :submit, button_options: { class: 'gl-w-full', data: { track_action: 'click_button', track_label: 'unfollow_from_profile' } }) do diff --git a/config/feature_flags/development/disable_follow_users.yml b/config/feature_flags/development/disable_follow_users.yml new file mode 100644 index 00000000000..9788ca520fd --- /dev/null +++ b/config/feature_flags/development/disable_follow_users.yml @@ -0,0 +1,8 @@ +--- +name: disable_follow_users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116023 +rollout_issue_url: +milestone: '16.0' +type: development +group: group::authentication and authorization +default_enabled: false diff --git a/db/docs/batched_background_migrations/populate_vulnerability_dismissal_fields.yml b/db/docs/batched_background_migrations/populate_vulnerability_dismissal_fields.yml new file mode 100644 index 00000000000..b51a6ab37d0 --- /dev/null +++ b/db/docs/batched_background_migrations/populate_vulnerability_dismissal_fields.yml @@ -0,0 +1,6 @@ +--- +migration_job_name: PopulateVulnerabilityDismissalFields +description: This populates missing dismissal info for vulnerabilities. +feature_category: vulnerability_management +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/405032 +milestone: 15.11 diff --git a/db/migrate/20230328165313_add_disabled_following_to_user_preferences.rb b/db/migrate/20230328165313_add_disabled_following_to_user_preferences.rb new file mode 100644 index 00000000000..0841829c957 --- /dev/null +++ b/db/migrate/20230328165313_add_disabled_following_to_user_preferences.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDisabledFollowingToUserPreferences < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + def change + add_column :user_preferences, :enabled_following, :boolean, default: true, null: false + end +end diff --git a/db/post_migrate/20230410123709_validate_fk_projects_creator_id.rb b/db/post_migrate/20230410123709_validate_fk_projects_creator_id.rb new file mode 100644 index 00000000000..7f522cd92ef --- /dev/null +++ b/db/post_migrate/20230410123709_validate_fk_projects_creator_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ValidateFkProjectsCreatorId < Gitlab::Database::Migration[2.1] + TABLE_NAME = :projects + COLUMN_NAME = :creator_id + FK_NAME = :fk_03ec10b0d3 + + def up + validate_foreign_key TABLE_NAME, COLUMN_NAME, name: FK_NAME + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20230411011959_add_temp_index_to_null_dismissed_info_vulnerabilities.rb b/db/post_migrate/20230411011959_add_temp_index_to_null_dismissed_info_vulnerabilities.rb new file mode 100644 index 00000000000..fe62260cb8f --- /dev/null +++ b/db/post_migrate/20230411011959_add_temp_index_to_null_dismissed_info_vulnerabilities.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddTempIndexToNullDismissedInfoVulnerabilities < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'tmp_index_vulnerability_dismissal_info' + + disable_ddl_transaction! + + def up + # Temporary index to be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/406653 + add_concurrent_index :vulnerabilities, :id, + where: "state = 2 AND (dismissed_at IS NULL OR dismissed_by_id IS NULL)", + name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :vulnerabilities, INDEX_NAME + end +end diff --git a/db/post_migrate/20230412104514_add_index_to_group_group_links.rb b/db/post_migrate/20230412104514_add_index_to_group_group_links.rb new file mode 100644 index 00000000000..655bff43b3c --- /dev/null +++ b/db/post_migrate/20230412104514_add_index_to_group_group_links.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddIndexToGroupGroupLinks < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_group_group_links_on_shared_with_group_and_group_access' + TABLE_NAME = :group_group_links + + def up + add_concurrent_index TABLE_NAME, [:shared_with_group_id, :group_access], name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name TABLE_NAME, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20230412185837_queue_populate_vulnerability_dismissal_fields.rb b/db/post_migrate/20230412185837_queue_populate_vulnerability_dismissal_fields.rb new file mode 100644 index 00000000000..d0924e8fdf8 --- /dev/null +++ b/db/post_migrate/20230412185837_queue_populate_vulnerability_dismissal_fields.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class QueuePopulateVulnerabilityDismissalFields < Gitlab::Database::Migration[2.1] + MIGRATION = "PopulateVulnerabilityDismissalFields" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 1_000 + MAX_BATCH_SIZE = 10_000 + SUB_BATCH_SIZE = 200 + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :vulnerabilities, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :vulnerabilities, :id, []) + end +end diff --git a/db/schema_migrations/20230328165313 b/db/schema_migrations/20230328165313 new file mode 100644 index 00000000000..a9ddacf5e48 --- /dev/null +++ b/db/schema_migrations/20230328165313 @@ -0,0 +1 @@ +ea52a177f82cc872b1f38490ff17bf166a885a4df7cf2c6c99dc7b1cccd14d33 \ No newline at end of file diff --git a/db/schema_migrations/20230410123709 b/db/schema_migrations/20230410123709 new file mode 100644 index 00000000000..2cf17b397dd --- /dev/null +++ b/db/schema_migrations/20230410123709 @@ -0,0 +1 @@ +d1e5678ea93b5b8f5d76b0332e79934badbaa84546754b66916bb2816cfaf307 \ No newline at end of file diff --git a/db/schema_migrations/20230411011959 b/db/schema_migrations/20230411011959 new file mode 100644 index 00000000000..830774fc029 --- /dev/null +++ b/db/schema_migrations/20230411011959 @@ -0,0 +1 @@ +31e8f1f8d65ea821efb158bd59657776de54ecbd5e10d3a64c5afe37706c5388 \ No newline at end of file diff --git a/db/schema_migrations/20230412104514 b/db/schema_migrations/20230412104514 new file mode 100644 index 00000000000..50c4a36e790 --- /dev/null +++ b/db/schema_migrations/20230412104514 @@ -0,0 +1 @@ +5a1245d37e10d03320a3cd8afda34226e54c6f6641c3abedfcb1333ea6ed69a0 \ No newline at end of file diff --git a/db/schema_migrations/20230412185837 b/db/schema_migrations/20230412185837 new file mode 100644 index 00000000000..d4c425e68fa --- /dev/null +++ b/db/schema_migrations/20230412185837 @@ -0,0 +1 @@ +f9c342816a6c656b1c13b8e9d0a771c1ee6a9847c03a76577c662f9cf238ad03 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1ba0ff9b13a..c366c1e3bb6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -23487,6 +23487,7 @@ CREATE TABLE user_preferences ( achievements_enabled boolean DEFAULT true NOT NULL, pinned_nav_items jsonb DEFAULT '{}'::jsonb NOT NULL, pass_user_identities_to_ci_jwt boolean DEFAULT false NOT NULL, + enabled_following boolean DEFAULT true NOT NULL, CONSTRAINT check_89bf269f41 CHECK ((char_length(diffs_deletion_color) <= 7)), CONSTRAINT check_d07ccd35f7 CHECK ((char_length(diffs_addition_color) <= 7)) ); @@ -30780,6 +30781,8 @@ CREATE UNIQUE INDEX index_group_deploy_tokens_on_group_and_deploy_token_ids ON g CREATE UNIQUE INDEX index_group_group_links_on_shared_group_and_shared_with_group ON group_group_links USING btree (shared_group_id, shared_with_group_id); +CREATE INDEX index_group_group_links_on_shared_with_group_and_group_access ON group_group_links USING btree (shared_with_group_id, group_access); + CREATE INDEX index_group_group_links_on_shared_with_group_and_shared_group ON group_group_links USING btree (shared_with_group_id, shared_group_id); CREATE INDEX index_group_import_states_on_group_id ON group_import_states USING btree (group_id); @@ -33010,6 +33013,8 @@ CREATE INDEX tmp_index_on_vulnerabilities_non_dismissed ON vulnerabilities USING CREATE INDEX tmp_index_project_statistics_cont_registry_size ON project_statistics USING btree (project_id) WHERE (container_registry_size = 0); +CREATE INDEX tmp_index_vulnerability_dismissal_info ON vulnerabilities USING btree (id) WHERE ((state = 2) AND ((dismissed_at IS NULL) OR (dismissed_by_id IS NULL))); + CREATE INDEX tmp_index_vulnerability_overlong_title_html ON vulnerabilities USING btree (id) WHERE (length(title_html) > 800); CREATE UNIQUE INDEX uniq_pkgs_deb_grp_architectures_on_distribution_id_and_name ON packages_debian_group_architectures USING btree (distribution_id, name); @@ -34470,7 +34475,7 @@ ALTER TABLE ONLY design_management_designs_versions ADD CONSTRAINT fk_03c671965c FOREIGN KEY (design_id) REFERENCES design_management_designs(id) ON DELETE CASCADE; ALTER TABLE ONLY projects - ADD CONSTRAINT fk_03ec10b0d3 FOREIGN KEY (creator_id) REFERENCES users(id) ON DELETE SET NULL NOT VALID; + ADD CONSTRAINT fk_03ec10b0d3 FOREIGN KEY (creator_id) REFERENCES users(id) ON DELETE SET NULL; ALTER TABLE ONLY analytics_dashboards_pointers ADD CONSTRAINT fk_05d96922bd FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/lib/api/entities/user.rb b/lib/api/entities/user.rb index 884f0f75d7a..689996be7fc 100644 --- a/lib/api/entities/user.rb +++ b/lib/api/entities/user.rb @@ -5,6 +5,7 @@ module API class User < UserBasic include UsersHelper include TimeZoneHelper + include Gitlab::Utils::StrongMemoize expose :created_at, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } expose :bio, :location, :public_email, :skype, :linkedin, :twitter, :discord, :website_url, :organization, :job_title, :pronouns @@ -12,18 +13,28 @@ module API expose :work_information do |user| work_information(user) end - expose :followers, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user| + expose :followers, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) && following_users_allowed(opts[:current_user], user) } do |user| user.followers.size end - expose :following, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user| + expose :following, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) && following_users_allowed(opts[:current_user], user) } do |user| user.followees.size end - expose :is_followed, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) && opts[:current_user] } do |user, opts| + expose :is_followed, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) && opts[:current_user] && following_users_allowed(opts[:current_user], user) } do |user, opts| user.followed_by?(opts[:current_user]) end expose :local_time do |user| local_time(user.timezone) end + + def following_users_allowed(current_user, user) + strong_memoize(:following_users_allowed) do + if current_user + current_user.following_users_allowed?(user) + else + true + end + end + end end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 48e13127712..505fcb2b38e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -195,12 +195,13 @@ module API not_found!('User') unless user followee = current_user.follow(user) + + not_modified! unless followee + if followee&.errors&.any? render_api_error!(followee.errors.full_messages.join(', '), 400) elsif followee&.persisted? present user, with: Entities::UserBasic - else - not_modified! end end diff --git a/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields.rb b/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields.rb new file mode 100644 index 00000000000..ee0f73cc3de --- /dev/null +++ b/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Populates missing dismissal information for vulnerabilities. + class PopulateVulnerabilityDismissalFields < BatchedMigrationJob + feature_category :vulnerability_management + scope_to ->(relation) { relation.where('state = 2 AND (dismissed_at IS NULL OR dismissed_by_id IS NULL)') } + operation_name :populate_vulnerability_dismissal_fields + + # rubocop:disable Style/Documentation + class Vulnerability < ApplicationRecord + self.table_name = 'vulnerabilities' + + has_one :finding, class_name: 'Finding' + + def copy_dismissal_information + return unless finding&.dismissal_feedback + + update_columns( + dismissed_at: finding.dismissal_feedback.created_at, + dismissed_by_id: finding.dismissal_feedback.author_id + ) + end + end + + class Finding < ApplicationRecord + self.table_name = 'vulnerability_occurrences' + + validates :details, json_schema: { filename: "filename" } + + def dismissal_feedback + Feedback.dismissal.where(finding_uuid: uuid).first + end + end + + class Feedback < ApplicationRecord + DISMISSAL_TYPE = 0 # dismissal + + self.table_name = 'vulnerability_feedback' + + scope :dismissal, -> { where(feedback_type: DISMISSAL_TYPE) } + end + # rubocop:enable Style/Documentation + + def perform + each_sub_batch do |sub_batch| + vulnerability_ids = sub_batch.pluck(:id) + Vulnerability.includes(:finding).where(id: vulnerability_ids).each do |vulnerability| + populate_for(vulnerability) + end + + log_info(vulnerability_ids) + end + end + + private + + def populate_for(vulnerability) + log_warning(vulnerability) unless vulnerability.copy_dismissal_information + rescue StandardError => error + log_error(error, vulnerability) + end + + def log_info(vulnerability_ids) + ::Gitlab::BackgroundMigration::Logger.info( + migrator: self.class.name, + message: 'Dismissal information has been copied', + count: vulnerability_ids.length + ) + end + + def log_warning(vulnerability) + ::Gitlab::BackgroundMigration::Logger.warn( + migrator: self.class.name, + message: 'Could not update vulnerability!', + vulnerability_id: vulnerability.id + ) + end + + def log_error(error, vulnerability) + ::Gitlab::BackgroundMigration::Logger.error( + migrator: self.class.name, + message: error.message, + vulnerability_id: vulnerability.id + ) + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c7c0ee8a54d..3d747645075 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2226,6 +2226,9 @@ msgstr "" msgid "Action" msgstr "" +msgid "Action not allowed." +msgstr "" + msgid "Action to take when receiving an alert. %{docsLink}" msgstr "" @@ -33326,6 +33329,12 @@ msgstr "" msgid "Preferences|Diff colors" msgstr "" +msgid "Preferences|Disable follow users" +msgstr "" + +msgid "Preferences|Disable follow users feature" +msgstr "" + msgid "Preferences|Display time in 24-hour format" msgstr "" @@ -33404,6 +33413,9 @@ msgstr "" msgid "Preferences|Time preferences" msgstr "" +msgid "Preferences|Turns off the ability to follow or be followed by other users." +msgstr "" + msgid "Preferences|Use relative times" msgstr "" diff --git a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb b/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb deleted file mode 100644 index 58411357eeb..00000000000 --- a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require 'rack/utils' - -module RuboCop - module Cop - module Gitlab - # This cop prevents from using deprecated `track_redis_hll_event` method. - # - # @example - # - # # bad - # track_redis_hll_event :show, name: 'p_analytics_valuestream' - # - # # good - # track_event :show, name: 'g_analytics_valuestream', destinations: [:redis_hll] - class DeprecateTrackRedisHLLEvent < RuboCop::Cop::Base - MSG = '`track_redis_hll_event` is deprecated. Use `track_event` helper instead. ' \ - 'See https://docs.gitlab.com/ee/development/service_ping/implement.html#add-new-events' - - def_node_matcher :track_redis_hll_event_used?, <<~PATTERN - (send _ :track_redis_hll_event ...) - PATTERN - - def on_send(node) - return unless track_redis_hll_event_used?(node) - - add_offense(node.loc.selector) - end - end - end - end -end diff --git a/spec/controllers/concerns/redis_tracking_spec.rb b/spec/controllers/concerns/redis_tracking_spec.rb deleted file mode 100644 index 0ad8fa79e5e..00000000000 --- a/spec/controllers/concerns/redis_tracking_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe RedisTracking do - include TrackingHelpers - - let(:user) { create(:user) } - - controller(ApplicationController) do - include RedisTracking - - skip_before_action :authenticate_user!, only: :show - track_redis_hll_event(:index, :show, - name: 'g_compliance_approval_rules', - if: [:custom_condition_one?, :custom_condition_two?]) { |controller| controller.get_custom_id } - - def index - render html: 'index' - end - - def new - render html: 'new' - end - - def show - render html: 'show' - end - - def get_custom_id - 'some_custom_id' - end - - private - - def custom_condition_one? - true - end - - def custom_condition_two? - true - end - end - - def expect_tracking - expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event) - .with('g_compliance_approval_rules', values: instance_of(String)) - end - - def expect_no_tracking - expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) - end - - context 'when user is logged in' do - before do - sign_in(user) - end - - it 'tracks the event' do - expect_tracking - - get :index - end - - it 'tracks the event if DNT is not enabled' do - stub_do_not_track('0') - - expect_tracking - - get :index - end - - it 'does not track the event if DNT is enabled' do - stub_do_not_track('1') - - expect_no_tracking - - get :index - end - - it 'does not track the event if the format is not HTML' do - expect_no_tracking - - get :index, format: :json - end - - it 'does not track the event if a custom condition returns false' do - expect(controller).to receive(:custom_condition_two?).and_return(false) - - expect_no_tracking - - get :index - end - - it 'does not track the event for untracked actions' do - expect_no_tracking - - get :new - end - end - - context 'when user is not logged in' do - let(:visitor_id) { SecureRandom.uuid } - - it 'tracks the event when there is a visitor id' do - cookies[:visitor_id] = { value: visitor_id, expires: 24.months } - - expect_tracking - - get :show, params: { id: 1 } - end - end - - context 'when user is not logged in and there is no visitor_id' do - it 'does not track the event' do - expect_no_tracking - - get :index - end - - it 'tracks the event when there is custom id' do - expect_tracking - - get :show, params: { id: 1 } - end - - it 'does not track the event when there is no custom id' do - expect(controller).to receive(:get_custom_id).and_return(nil) - - expect_no_tracking - - get :show, params: { id: 2 } - end - end -end diff --git a/spec/controllers/profiles/preferences_controller_spec.rb b/spec/controllers/profiles/preferences_controller_spec.rb index e2a216bb462..0554ab3184f 100644 --- a/spec/controllers/profiles/preferences_controller_spec.rb +++ b/spec/controllers/profiles/preferences_controller_spec.rb @@ -109,5 +109,33 @@ RSpec.describe Profiles::PreferencesController do expect(response.parsed_body['type']).to eq('alert') end end + + context 'on disable_follow_users feature flag' do + context 'with feature flag disabled' do + before do + stub_feature_flags(disable_follow_users: false) + end + + it 'does not update enabled_following preference of user' do + prefs = { enabled_following: false } + + go params: prefs + user.reload + + expect(user.enabled_following).to eq(true) + end + end + + context 'with feature flag enabled' do + it 'does not update enabled_following preference of user' do + prefs = { enabled_following: false } + + go params: prefs + user.reload + + expect(user.enabled_following).to eq(false) + end + end + end end end diff --git a/spec/frontend/fixtures/jobs.rb b/spec/frontend/fixtures/jobs.rb index 043d6a9db29..15bbec9b9f2 100644 --- a/spec/frontend/fixtures/jobs.rb +++ b/spec/frontend/fixtures/jobs.rb @@ -50,6 +50,7 @@ RSpec.describe 'Jobs (JavaScript fixtures)' do shared_examples 'graphql queries' do |path, jobs_query| let_it_be(:variables) { {} } + let_it_be(:success_path) { '' } let_it_be(:query) do get_graphql_query_as_string("#{path}/#{jobs_query}") @@ -57,9 +58,10 @@ RSpec.describe 'Jobs (JavaScript fixtures)' do fixtures_path = 'graphql/jobs/' - it "#{fixtures_path}#{jobs_query}.json" do + it "#{fixtures_path}#{jobs_query}.json", :aggregate_failures do post_graphql(query, current_user: user, variables: variables) + expect(graphql_data.dig(*success_path)).not_to be_nil expect_graphql_errors_to_be_empty end @@ -87,15 +89,18 @@ RSpec.describe 'Jobs (JavaScript fixtures)' do it_behaves_like 'graphql queries', 'jobs/components/table/graphql/queries', 'get_jobs.query.graphql' do let(:variables) { { fullPath: 'frontend-fixtures/builds-project' } } + let(:success_path) { %w[project jobs] } end it_behaves_like 'graphql queries', 'pages/admin/jobs/components/table/graphql/queries', 'get_all_jobs.query.graphql' do let(:user) { create(:admin) } + let(:success_path) { 'jobs' } end it_behaves_like 'graphql queries', 'pages/admin/jobs/components/table/graphql/queries', 'get_cancelable_jobs_count.query.graphql' do let(:variables) { { statuses: %w[PENDING RUNNING] } } let(:user) { create(:admin) } + let(:success_path) { %w[cancelable count] } end end diff --git a/spec/lib/api/entities/user_spec.rb b/spec/lib/api/entities/user_spec.rb index 3094fc748c9..6475dcd7618 100644 --- a/spec/lib/api/entities/user_spec.rb +++ b/spec/lib/api/entities/user_spec.rb @@ -30,21 +30,63 @@ RSpec.describe API::Entities::User do end %i(followers following is_followed).each do |relationship| - context 'when current user cannot read user profile' do - let(:can_read_user_profile) { false } - + shared_examples 'does not expose relationship' do it "does not expose #{relationship}" do expect(subject).not_to include(relationship) end end - context 'when current user can read user profile' do - let(:can_read_user_profile) { true } - + shared_examples 'exposes relationship' do it "exposes #{relationship}" do expect(subject).to include(relationship) end end + + context 'when current user cannot read user profile' do + let(:can_read_user_profile) { false } + + it_behaves_like 'does not expose relationship' + end + + context 'when current user can read user profile' do + let(:can_read_user_profile) { true } + + it_behaves_like 'exposes relationship' + end + + context 'when current user can read user profile and disable_follow_users is switched off' do + let(:can_read_user_profile) { true } + + before do + stub_feature_flags(disable_follow_users: false) + user.enabled_following = false + user.save! + end + + it_behaves_like 'exposes relationship' + end + + context 'when current user can read user profile, disable_follow_users is switched on and user disabled it for themself' do + let(:can_read_user_profile) { true } + + before do + user.enabled_following = false + user.save! + end + + it_behaves_like 'does not expose relationship' + end + + context 'when current user can read user profile, disable_follow_users is switched on and current user disabled it for themself' do + let(:can_read_user_profile) { true } + + before do + current_user.enabled_following = false + current_user.save! + end + + it_behaves_like 'does not expose relationship' + end end end diff --git a/spec/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields_spec.rb b/spec/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields_spec.rb new file mode 100644 index 00000000000..50380247c9f --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_vulnerability_dismissal_fields_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::PopulateVulnerabilityDismissalFields, schema: 20230412185837, feature_category: :vulnerability_management do # rubocop:disable Layout/LineLength + let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:vulnerabilities) { table(:vulnerabilities) } + let(:findings) { table(:vulnerability_occurrences) } + let(:scanners) { table(:vulnerability_scanners) } + let(:identifiers) { table(:vulnerability_identifiers) } + let(:feedback) { table(:vulnerability_feedback) } + + let(:user) { users.create!(name: 'test', email: 'test@example.com', projects_limit: 5) } + let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') } + let(:project) { projects.create!(namespace_id: namespace.id, name: 'foo', project_namespace_id: namespace.id) } + let(:vulnerability_1) do + vulnerabilities.create!(title: 'title', state: 2, severity: 0, + confidence: 5, report_type: 2, project_id: project.id, author_id: user.id + ) + end + + let(:vulnerability_2) do + vulnerabilities.create!(title: 'title', state: 2, severity: 0, + confidence: 5, report_type: 2, project_id: project.id, author_id: user.id + ) + end + + let(:scanner) { scanners.create!(project_id: project.id, external_id: 'foo', name: 'bar') } + let(:identifier) do + identifiers.create!(project_id: project.id, fingerprint: 'foo', + external_type: 'bar', external_id: 'zoo', name: 'identifier' + ) + end + + let(:uuid) { SecureRandom.uuid } + + before do + feedback.create!(feedback_type: 0, + category: 'sast', + project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1fa8', + project_id: project.id, + author_id: user.id, + created_at: Time.current, + finding_uuid: uuid + ) + + findings.create!(name: 'Finding', + report_type: 'sast', + project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1f98', + location_fingerprint: 'bar', + severity: 1, + confidence: 1, + metadata_version: 1, + raw_metadata: '', + details: {}, + uuid: uuid, + project_id: project.id, + vulnerability_id: vulnerability_1.id, + scanner_id: scanner.id, + primary_identifier_id: identifier.id + ) + + allow(::Gitlab::BackgroundMigration::Logger).to receive_messages(info: true, warn: true, error: true) + end + + subject do + described_class.new( + start_id: vulnerability_1.id, + end_id: vulnerability_2.id, + batch_table: :vulnerabilities, + batch_column: :id, + sub_batch_size: 200, + pause_ms: 2.minutes, + connection: ApplicationRecord.connection + ) + end + + describe '#perform' do + it 'updates the missing dismissal information of the vulnerability' do + expect { subject.perform }.to change { vulnerability_1.reload.dismissed_at } + .from(nil) + .and change { vulnerability_1.reload.dismissed_by_id }.from(nil).to(user.id) + end + + it 'writes log messages', :aggregate_failures do + subject.perform + + expect(::Gitlab::BackgroundMigration::Logger).to have_received(:info).with(migrator: described_class.name, + message: 'Dismissal information has been copied', + count: 2 + ) + expect(::Gitlab::BackgroundMigration::Logger).to have_received(:warn).with(migrator: described_class.name, + message: 'Could not update vulnerability!', + vulnerability_id: vulnerability_2.id + ) + end + + context 'when logger throws exception StandardError' do + before do + allow(::Gitlab::BackgroundMigration::Logger).to receive(:warn).and_raise(StandardError) + end + + it 'logs StandardError' do + expect(::Gitlab::BackgroundMigration::Logger).to receive(:error).with({ + migrator: described_class.name, message: StandardError.to_s, vulnerability_id: vulnerability_2.id + }) + + subject.perform + end + end + end +end diff --git a/spec/migrations/20230412185837_queue_populate_vulnerability_dismissal_fields_spec.rb b/spec/migrations/20230412185837_queue_populate_vulnerability_dismissal_fields_spec.rb new file mode 100644 index 00000000000..d39936abb90 --- /dev/null +++ b/spec/migrations/20230412185837_queue_populate_vulnerability_dismissal_fields_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueuePopulateVulnerabilityDismissalFields, feature_category: :vulnerability_management do + let!(:batched_migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :vulnerabilities, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(batched_migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c05b7274317..21cea5fde14 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3840,6 +3840,54 @@ RSpec.describe User, feature_category: :user_profile do expect(user.following?(followee)).to be_falsey end + + it 'does not follow if user disabled following' do + user = create(:user) + user.enabled_following = false + + followee = create(:user) + + expect(user.follow(followee)).to eq(false) + + expect(user.following?(followee)).to be_falsey + end + + it 'does not follow if followee user disabled following' do + user = create(:user) + + followee = create(:user) + followee.enabled_following = false + + expect(user.follow(followee)).to eq(false) + + expect(user.following?(followee)).to be_falsey + end + + context 'when disable_follow_users feature flag is off' do + before do + stub_feature_flags(disable_follow_users: false) + end + + it 'follows user even if user disabled following' do + user = create(:user) + user.enabled_following = false + + followee = create(:user) + + expect(user.follow(followee)).to be_truthy + expect(user.following?(followee)).to be_truthy + end + + it 'follows user even if followee user disabled following' do + user = create(:user) + + followee = create(:user) + followee.enabled_following = false + + expect(user.follow(followee)).to be_truthy + expect(user.following?(followee)).to be_truthy + end + end end describe '#unfollow' do @@ -3882,6 +3930,39 @@ RSpec.describe User, feature_category: :user_profile do end end + describe '#following_users_allowed?' do + using RSpec::Parameterized::TableSyntax + + let_it_be(:user) { create(:user) } + let_it_be(:followee) { create(:user) } + + where(:user_enabled_following, :followee_enabled_following, :feature_flag_status, :result) do + true | true | false | true + true | false | false | true + true | true | true | true + true | false | true | false + false | true | false | true + false | true | true | false + false | false | false | true + false | false | true | false + end + + with_them do + before do + user.enabled_following = user_enabled_following + followee.enabled_following = followee_enabled_following + followee.save! + stub_feature_flags(disable_follow_users: feature_flag_status) + end + + it { expect(user.following_users_allowed?(followee)).to eq result } + end + + it 'is false when user and followee is the same user' do + expect(user.following_users_allowed?(user)).to eq(false) + end + end + describe '#notification_email_or_default' do let(:email) { 'gonzo@muppets.com' } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 79e2755f34c..3ecd012be12 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1009,6 +1009,20 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile expect(response).to have_gitlab_http_status(:not_modified) end end + + context 'on a user with disabled following' do + before do + user.enabled_following = false + user.save! + end + + it 'does not change following' do + post api("/users/#{followee.id}/follow", user) + + expect(user.followees).to be_empty + expect(response).to have_gitlab_http_status(:not_modified) + end + end end describe 'POST /users/:id/unfollow' do diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 14e4fea0548..3367414b987 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -914,6 +914,35 @@ RSpec.describe UsersController, feature_category: :user_management do expect(user).not_to be_following(public_user) end end + + context 'when user or followee disabled following' do + before do + sign_in(user) + end + + it 'alerts and not follow if user disabled following' do + user.enabled_following = false + + post user_follow_url(username: public_user.username) + expect(response).to be_redirect + + expected_message = format(_('Action not allowed.')) + expect(flash[:alert]).to eq(expected_message) + expect(user).not_to be_following(public_user) + end + + it 'alerts and not follow if followee disabled following' do + public_user.enabled_following = false + public_user.save! + + post user_follow_url(username: public_user.username) + expect(response).to be_redirect + + expected_message = format(_('Action not allowed.')) + expect(flash[:alert]).to eq(expected_message) + expect(user).not_to be_following(public_user) + end + end end context 'token authentication' do diff --git a/spec/rubocop/cop/gitlab/deprecate_track_redis_hll_event_spec.rb b/spec/rubocop/cop/gitlab/deprecate_track_redis_hll_event_spec.rb deleted file mode 100644 index eed30e11a98..00000000000 --- a/spec/rubocop/cop/gitlab/deprecate_track_redis_hll_event_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop_spec_helper' -require_relative '../../../../rubocop/cop/gitlab/deprecate_track_redis_hll_event' - -RSpec.describe RuboCop::Cop::Gitlab::DeprecateTrackRedisHLLEvent do - it 'does not flag the use of track_event' do - expect_no_offenses('track_event :show, name: "p_analytics_insights"') - end - - it 'flags the use of track_redis_hll_event' do - expect_offense(<<~SOURCE) - track_redis_hll_event :show, name: 'p_analytics_valuestream' - ^^^^^^^^^^^^^^^^^^^^^ `track_redis_hll_event` is deprecated[...] - SOURCE - end -end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 1716685566c..d2a26425f52 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -185,6 +185,49 @@ RSpec.describe Users::UpdateService, feature_category: :user_profile do end.not_to raise_error end + describe 'updates the enabled_following' do + let(:user) { create(:user) } + + before do + 3.times do + user.follow(create(:user)) + create(:user).follow(user) + end + user.reload + end + + it 'removes followers and followees' do + expect do + update_user(user, enabled_following: false) + end.to change { user.followed_users.count }.from(3).to(0) + .and change { user.following_users.count }.from(3).to(0) + expect(user.enabled_following).to eq(false) + end + + it 'does not remove followers/followees if feature flag is off' do + stub_feature_flags(disable_follow_users: false) + + expect do + update_user(user, enabled_following: false) + end.to not_change { user.followed_users.count } + .and not_change { user.following_users.count } + end + + context 'when there is more followers/followees then batch limit' do + before do + stub_env('BATCH_SIZE', 1) + end + + it 'removes followers and followees' do + expect do + update_user(user, enabled_following: false) + end.to change { user.followed_users.count }.from(3).to(0) + .and change { user.following_users.count }.from(3).to(0) + expect(user.enabled_following).to eq(false) + end + end + end + def update_user(user, opts) described_class.new(user, opts.merge(user: user)).execute end