diff --git a/.rubocop_todo/gitlab/avoid_gitlab_instance_checks.yml b/.rubocop_todo/gitlab/avoid_gitlab_instance_checks.yml index 2567ff440001726fdb7a102fedd798462161290f..e1d9880a0dce4156a9b84e9eecb7c28a9cc547bb 100644 --- a/.rubocop_todo/gitlab/avoid_gitlab_instance_checks.yml +++ b/.rubocop_todo/gitlab/avoid_gitlab_instance_checks.yml @@ -68,7 +68,6 @@ Gitlab/AvoidGitlabInstanceChecks: - 'ee/app/workers/gitlab_subscriptions/refresh_seats_worker.rb' - 'ee/app/workers/gitlab_subscriptions/schedule_refresh_seats_worker.rb' - 'ee/app/workers/update_all_mirrors_worker.rb' - - 'ee/config/initializers/gitlab_suggested_reviewers_secret.rb' - 'ee/lib/api/code_suggestions.rb' - 'ee/lib/api/scim/instance_scim.rb' - 'ee/lib/ee/api/namespaces.rb' diff --git a/.rubocop_todo/gitlab/bounded_contexts.yml b/.rubocop_todo/gitlab/bounded_contexts.yml index 5a696cc9ec320874e33b5beca0d4744ad5531f22..7cd23cb15f143bd9b0f0b9081bb962bc68e596c9 100644 --- a/.rubocop_todo/gitlab/bounded_contexts.yml +++ b/.rubocop_todo/gitlab/bounded_contexts.yml @@ -2371,7 +2371,6 @@ Gitlab/BoundedContexts: - 'ee/app/graphql/types/app_sec/fuzzing/api/scan_mode_enum.rb' - 'ee/app/graphql/types/app_sec/fuzzing/api/scan_profile_type.rb' - 'ee/app/graphql/types/app_sec/fuzzing/coverage/corpus_type.rb' - - 'ee/app/graphql/types/applied_ml/suggested_reviewers_type.rb' - 'ee/app/graphql/types/approval_rule_type.rb' - 'ee/app/graphql/types/approval_rule_type_enum.rb' - 'ee/app/graphql/types/branch_protections/unprotect_access_level_type.rb' diff --git a/.rubocop_todo/gitlab/feature_flag_without_actor.yml b/.rubocop_todo/gitlab/feature_flag_without_actor.yml index 60336dabeb50f8ceefb861f92cc9cb6b30bf6a6f..48ec024468228af61fd880b2c5a66e3e62e04cf9 100644 --- a/.rubocop_todo/gitlab/feature_flag_without_actor.yml +++ b/.rubocop_todo/gitlab/feature_flag_without_actor.yml @@ -85,7 +85,6 @@ Gitlab/FeatureFlagWithoutActor: - 'ee/app/views/projects/on_demand_scans/index.html.haml' - 'ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml' - 'ee/lib/api/code_suggestions.rb' - - 'ee/lib/api/internal/suggested_reviewers.rb' - 'ee/lib/ee/api/entities/application_setting.rb' - 'ee/lib/ee/api/geo.rb' - 'ee/lib/ee/api/internal/base.rb' diff --git a/.rubocop_todo/layout/empty_line_after_magic_comment.yml b/.rubocop_todo/layout/empty_line_after_magic_comment.yml index 7f45fd2da904bdfb70397893842b1dcafcdc103f..f5a4f04549d97d49a12897ef3a20a8083b8bd1f3 100644 --- a/.rubocop_todo/layout/empty_line_after_magic_comment.yml +++ b/.rubocop_todo/layout/empty_line_after_magic_comment.yml @@ -132,7 +132,6 @@ Layout/EmptyLineAfterMagicComment: - 'ee/app/policies/ee/environment_policy.rb' - 'ee/app/policies/security/finding_policy.rb' - 'ee/app/policies/vulnerabilities/finding_policy.rb' - - 'ee/app/presenters/applied_ml/suggested_reviewers_presenter.rb' - 'ee/app/serializers/audit_event_serializer.rb' - 'ee/app/serializers/ee/issue_board_entity.rb' - 'ee/app/serializers/ee/issue_entity.rb' diff --git a/.rubocop_todo/lint/unused_block_argument.yml b/.rubocop_todo/lint/unused_block_argument.yml index 71322685638fe00e3af14f0ba8987c1df39e6768..0d133edd9812c952ee084d85abca9675c3349ab4 100644 --- a/.rubocop_todo/lint/unused_block_argument.yml +++ b/.rubocop_todo/lint/unused_block_argument.yml @@ -101,7 +101,6 @@ Lint/UnusedBlockArgument: - 'ee/lib/gitlab/authority_analyzer.rb' - 'ee/lib/gitlab/insights/reducers/count_per_label_reducer.rb' - 'ee/lib/gitlab/proxy.rb' - - 'ee/lib/tasks/contracts/merge_requests.rake' - 'ee/lib/tasks/gitlab/indexer.rake' - 'ee/lib/tasks/gitlab/seed/insights.rake' - 'ee/spec/factories/approvers.rb' diff --git a/.rubocop_todo/rake/require.yml b/.rubocop_todo/rake/require.yml index 24bb67e63fdb1e648b2dcbe1be64b2c914360a19..d0c54a08abd571dba4393f6a701d298e88c59364 100644 --- a/.rubocop_todo/rake/require.yml +++ b/.rubocop_todo/rake/require.yml @@ -1,7 +1,6 @@ --- Rake/Require: Exclude: - - 'ee/lib/tasks/contracts/merge_requests.rake' - 'lib/tasks/contracts/merge_requests.rake' - 'lib/tasks/contracts/pipeline_schedules.rake' - 'lib/tasks/contracts/pipelines.rake' diff --git a/.rubocop_todo/rspec/before_all_role_assignment.yml b/.rubocop_todo/rspec/before_all_role_assignment.yml index 3b3a9f9ff7617849c128fe86fb9f7553003f68c1..3f843602fd3091af3d6440dd46d7a3ef3dba2e36 100644 --- a/.rubocop_todo/rspec/before_all_role_assignment.yml +++ b/.rubocop_todo/rspec/before_all_role_assignment.yml @@ -580,9 +580,7 @@ RSpec/BeforeAllRoleAssignment: - 'ee/spec/services/merge_trains/create_pipeline_service_spec.rb' - 'ee/spec/services/product_analytics/cube_data_query_service_spec.rb' - 'ee/spec/services/product_analytics/initialize_stack_service_spec.rb' - - 'ee/spec/services/projects/deregister_suggested_reviewers_project_service_spec.rb' - 'ee/spec/services/projects/group_links/create_service_spec.rb' - - 'ee/spec/services/projects/register_suggested_reviewers_project_service_spec.rb' - 'ee/spec/services/projects/transfer_service_spec.rb' - 'ee/spec/services/projects/update_service_spec.rb' - 'ee/spec/services/protected_environments/base_service_spec.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index 7214fcfcac7f2af778be50bee28972642cca1164..24d5abce8d3614084a18f4996f47181b1aac96da 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -292,7 +292,6 @@ RSpec/NamedSubject: - 'ee/spec/lib/gitlab/analytics/cycle_analytics/summary/stage_time_summary_spec.rb' - 'ee/spec/lib/gitlab/analytics/cycle_analytics/summary/time_to_restore_service_spec.rb' - 'ee/spec/lib/gitlab/analytics/type_of_work/tasks_by_type_spec.rb' - - 'ee/spec/lib/gitlab/applied_ml/suggested_reviewers/client_spec.rb' - 'ee/spec/lib/gitlab/audit/events/preloader_spec.rb' - 'ee/spec/lib/gitlab/audit/levels/group_spec.rb' - 'ee/spec/lib/gitlab/audit/levels/instance_spec.rb' @@ -672,7 +671,6 @@ RSpec/NamedSubject: - 'ee/spec/requests/api/internal/app_sec/dast/site_validations_spec.rb' - 'ee/spec/requests/api/internal/base_spec.rb' - 'ee/spec/requests/api/internal/kubernetes_spec.rb' - - 'ee/spec/requests/api/internal/suggested_reviewers_spec.rb' - 'ee/spec/requests/api/issues_spec.rb' - 'ee/spec/requests/api/members_spec.rb' - 'ee/spec/requests/api/merge_trains_spec.rb' @@ -1042,15 +1040,11 @@ RSpec/NamedSubject: - 'ee/spec/workers/ldap_sync_worker_spec.rb' - 'ee/spec/workers/llm/completion_worker_spec.rb' - 'ee/spec/workers/members_destroyer/clean_up_group_protected_branch_rules_worker_spec.rb' - - 'ee/spec/workers/merge_requests/capture_suggested_reviewers_accepted_worker_spec.rb' - - 'ee/spec/workers/merge_requests/fetch_suggested_reviewers_worker_spec.rb' - 'ee/spec/workers/merge_requests/sync_code_owner_approval_rules_worker_spec.rb' - 'ee/spec/workers/personal_access_tokens/groups/policy_worker_spec.rb' - 'ee/spec/workers/personal_access_tokens/instance/policy_worker_spec.rb' - 'ee/spec/workers/project_import_schedule_worker_spec.rb' - - 'ee/spec/workers/projects/deregister_suggested_reviewers_project_worker_spec.rb' - 'ee/spec/workers/projects/disable_legacy_open_source_license_for_inactive_projects_worker_spec.rb' - - 'ee/spec/workers/projects/register_suggested_reviewers_project_worker_spec.rb' - 'ee/spec/workers/repository_import_worker_spec.rb' - 'ee/spec/workers/repository_update_mirror_worker_spec.rb' - 'ee/spec/workers/requirements_management/import_requirements_csv_worker_spec.rb' diff --git a/.rubocop_todo/rspec/receive_messages.yml b/.rubocop_todo/rspec/receive_messages.yml index a226ceecf2ffa5b373fe1d3a855f63cb54205f69..8d27bc86416d7b07c51d6de8a50574a9a181962d 100644 --- a/.rubocop_todo/rspec/receive_messages.yml +++ b/.rubocop_todo/rspec/receive_messages.yml @@ -174,7 +174,6 @@ RSpec/ReceiveMessages: - 'ee/spec/views/layouts/application.html.haml_spec.rb' - 'ee/spec/views/layouts/header/_ee_subscribable_banner.html.haml_spec.rb' - 'ee/spec/views/projects/project_members/index.html.haml_spec.rb' - - 'ee/spec/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml_spec.rb' - 'ee/spec/views/registrations/groups/new.html.haml_spec.rb' - 'ee/spec/views/registrations/welcome/show.html.haml_spec.rb' - 'ee/spec/views/search/results/_issuable.html.haml_spec.rb' diff --git a/.rubocop_todo/sidekiq/enforce_database_health_signal_deferral.yml b/.rubocop_todo/sidekiq/enforce_database_health_signal_deferral.yml index ac4842b4544c1a64671e39be46aab36258a96008..d6723c3a3241863b82a2354da8df60744e68b579 100644 --- a/.rubocop_todo/sidekiq/enforce_database_health_signal_deferral.yml +++ b/.rubocop_todo/sidekiq/enforce_database_health_signal_deferral.yml @@ -174,10 +174,8 @@ Sidekiq/EnforceDatabaseHealthSignalDeferral: - 'ee/app/workers/llm/namespace_access_cache_reset_worker.rb' - 'ee/app/workers/members/delete_pending_members_worker.rb' - 'ee/app/workers/merge_requests/approval_metrics_event_worker.rb' - - 'ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb' - 'ee/app/workers/merge_requests/create_approvals_reset_note_worker.rb' - 'ee/app/workers/merge_requests/duo_code_review_chat_worker.rb' - - 'ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb' - 'ee/app/workers/merge_requests/notify_approvers_worker.rb' - 'ee/app/workers/merge_requests/process_merge_audit_event_worker.rb' - 'ee/app/workers/merge_requests/stream_approval_audit_event_worker.rb' @@ -189,9 +187,7 @@ Sidekiq/EnforceDatabaseHealthSignalDeferral: - 'ee/app/workers/package_metadata/advisories_sync_worker.rb' - 'ee/app/workers/package_metadata/cve_enrichment_sync_worker.rb' - 'ee/app/workers/package_metadata/licenses_sync_worker.rb' - - 'ee/app/workers/projects/deregister_suggested_reviewers_project_worker.rb' - 'ee/app/workers/projects/disable_legacy_open_source_license_for_inactive_projects_worker.rb' - - 'ee/app/workers/projects/register_suggested_reviewers_project_worker.rb' - 'ee/app/workers/sbom/create_occurrences_vulnerabilities_worker.rb' - 'ee/app/workers/sbom/process_vulnerabilities_worker.rb' - 'ee/app/workers/search/elastic_default_branch_changed_worker.rb' diff --git a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml index 2a0402bbaaa6b5a5956a161627c9332367789315..00f97c0483ff0ce481d20e31764de0906b9761c8 100644 --- a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml +++ b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml @@ -317,8 +317,6 @@ SidekiqLoadBalancing/WorkerDataConsistency: - 'ee/app/workers/ldap_sync_worker.rb' - 'ee/app/workers/members_destroyer/clean_up_group_protected_branch_rules_worker.rb' - 'ee/app/workers/merge_request_reset_approvals_worker.rb' - - 'ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb' - - 'ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb' - 'ee/app/workers/merge_requests/sync_code_owner_approval_rules_worker.rb' - 'ee/app/workers/merge_trains/refresh_worker.rb' - 'ee/app/workers/namespaces/sync_namespace_name_worker.rb' @@ -330,7 +328,6 @@ SidekiqLoadBalancing/WorkerDataConsistency: - 'ee/app/workers/package_metadata/licenses_sync_worker.rb' - 'ee/app/workers/personal_access_tokens/groups/policy_worker.rb' - 'ee/app/workers/personal_access_tokens/instance/policy_worker.rb' - - 'ee/app/workers/projects/register_suggested_reviewers_project_worker.rb' - 'ee/app/workers/refresh_license_compliance_checks_worker.rb' - 'ee/app/workers/requirements_management/import_requirements_csv_worker.rb' - 'ee/app/workers/requirements_management/process_requirements_reports_worker.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index b4c0594b755147e0a68096b3b4e74a3ae3a41201..c4c81a589932a835bcb469fdba371c539ea29032 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -1053,7 +1053,6 @@ Style/InlineDisableAnnotation: - 'ee/app/graphql/types/analytics/value_stream_dashboard/count_type.rb' - 'ee/app/graphql/types/app_sec/fuzzing/api/ci_configuration_type.rb' - 'ee/app/graphql/types/app_sec/fuzzing/api/scan_profile_type.rb' - - 'ee/app/graphql/types/applied_ml/suggested_reviewers_type.rb' - 'ee/app/graphql/types/boards/board_epic_type.rb' - 'ee/app/graphql/types/boards/epic_list_metadata_type.rb' - 'ee/app/graphql/types/boards/epic_user_preferences_type.rb' @@ -1447,7 +1446,6 @@ Style/InlineDisableAnnotation: - 'ee/lib/gitlab/analytics/cycle_analytics/summary/base_time.rb' - 'ee/lib/gitlab/analytics/type_of_work/tasks_by_type.rb' - 'ee/lib/gitlab/analytics/value_stream_dashboard/namespace_cursor.rb' - - 'ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_pb.rb' - 'ee/lib/gitlab/auth/group_saml/identity_linker.rb' - 'ee/lib/gitlab/auth/group_saml/membership_updater.rb' - 'ee/lib/gitlab/auth/saml/membership_updater.rb' @@ -1551,7 +1549,6 @@ Style/InlineDisableAnnotation: - 'ee/spec/requests/api/graphql/boards/board_lists_query_spec.rb' - 'ee/spec/requests/api/graphql/mutations/boards/epic_boards/epic_move_list_spec.rb' - 'ee/spec/requests/api/graphql/mutations/geo/registries/update_spec.rb' - - 'ee/spec/requests/api/graphql/project/merge_request_spec.rb' - 'ee/spec/requests/api/graphql/project/product_analytics/product_analytics_spec.rb' - 'ee/spec/requests/api/groups_spec.rb' - 'ee/spec/requests/api/internal/base_spec.rb' @@ -1595,7 +1592,6 @@ Style/InlineDisableAnnotation: - 'ee/spec/workers/ldap_all_groups_sync_worker_spec.rb' - 'ee/spec/workers/ldap_group_sync_worker_spec.rb' - 'ee/spec/workers/ldap_sync_worker_spec.rb' - - 'ee/spec/workers/projects/register_suggested_reviewers_project_worker_spec.rb' - 'ee/spec/workers/update_all_mirrors_worker_spec.rb' - 'lib/api/access_requests.rb' - 'lib/api/admin/plan_limits.rb' diff --git a/app/assets/javascripts/sidebar/components/assignees/constants.js b/app/assets/javascripts/sidebar/components/assignees/constants.js index 01267dab435d016ead31fa860446ff1fb040a58e..7914ba31e36715a2f17b1195ed12d59aa4cc2a05 100644 --- a/app/assets/javascripts/sidebar/components/assignees/constants.js +++ b/app/assets/javascripts/sidebar/components/assignees/constants.js @@ -13,7 +13,6 @@ export const userTypes = { automation_bot: 'AUTOMATION_BOT', security_policy_bot: 'SECURITY_POLICY_BOT', admin_bot: 'ADMIN_BOT', - suggested_reviewers_bot: 'SUGGESTED_REVIEWERS_BOT', service_account: 'SERVICE_ACCOUNT', llm_bot: 'LLM_BOT', placeholder: 'PLACEHOLDER', diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 437f167424afa4a80ed231f9459b5eeb3fdc2c9e..1df5c723777afcfe0d10c5a4b90512390dc9910c 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -61,12 +61,11 @@ export default class SidebarMediator { async saveReviewers(field) { const selectedReviewers = this.store.reviewers; const selectedIds = selectedReviewers.map((u) => u.id); - const suggestedSelectedIds = selectedReviewers.filter((u) => u.suggested).map((u) => u.id); // If there are no ids, that means we have to unassign (which is id = 0) // And it only accepts an array, hence [0] const reviewers = selectedIds.length === 0 ? [0] : selectedIds; - const data = { reviewer_ids: reviewers, suggested_reviewer_ids: suggestedSelectedIds }; + const data = { reviewer_ids: reviewers }; try { const res = await this.service.update(field, data); diff --git a/app/assets/javascripts/users_select/index.js b/app/assets/javascripts/users_select/index.js index b619c661ea990502bd5a1b8e2e4a3daeb1805f86..ade694affc2ac5b45e580eaf1f24b5b47b78ad2f 100644 --- a/app/assets/javascripts/users_select/index.js +++ b/app/assets/javascripts/users_select/index.js @@ -47,7 +47,6 @@ function UsersSelect(currentUser, els, options = {}) { issuableType: $dropdown.data('issuableType'), targetBranch: $dropdown.data('targetBranch'), authorId: $dropdown.data('authorId'), - showSuggested: $dropdown.data('showSuggested'), }; const showNullUser = $dropdown.data('nullUser'); const defaultNullUser = $dropdown.data('nullUserDefault'); @@ -69,22 +68,6 @@ function UsersSelect(currentUser, els, options = {}) { let assigneeTemplate; let collapsedAssigneeTemplate; - const suggestedReviewersHelpPath = $dropdown.data('suggestedReviewersHelpPath'); - const suggestedReviewersHeaderTemplate = template( - `
- <%- header %> - - ${spriteIcon('question-o', 'gl-ml-3 gl-icon s16')} - -
`, - ); - if (selectedId === undefined) { selectedId = selectedIdDefault; } @@ -363,16 +346,6 @@ function UsersSelect(currentUser, els, options = {}) { if ($dropdown.hasClass('js-multiselect')) { const selected = getSelected().filter((i) => i !== 0); - if ($dropdown.data('showSuggested')) { - const suggested = this.suggestedUsers(users); - if (suggested.length) { - users = users.filter( - (u) => !u.suggested || (u.suggested && selected.indexOf(u.id) !== -1), - ); - users.splice(showDivider + 1, 0, ...suggested); - } - } - if (selected.length > 0) { if ($dropdown.data('dropdownHeader')) { showDivider += 1; @@ -403,24 +376,6 @@ function UsersSelect(currentUser, els, options = {}) { $dropdown.data('deprecatedJQueryDropdown').positionMenuAbove(); } }, - suggestedUsers(users) { - const selected = getSelected().filter((i) => i !== 0); - const suggestedUsers = users.filter((u) => u.suggested && selected.indexOf(u.id) === -1); - - if (!suggestedUsers.length) return []; - - const items = [ - { - type: 'header', - content: suggestedReviewersHeaderTemplate({ - header: $dropdown.data('suggestedReviewersHeader'), - }), - }, - ...suggestedUsers, - { type: 'header', content: $dropdown.data('allMembersHeader') }, - ]; - return items; - }, filterable: true, filterRemote: true, search: { @@ -675,10 +630,6 @@ UsersSelect.prototype.users = function (query, options, callback) { params.approval_rules = true; } - if (isMergeRequest && options.showSuggested) { - params.show_suggested = true; - } - if (isNewMergeRequest) { params.target_branch = options.targetBranch || null; } @@ -711,14 +662,13 @@ UsersSelect.prototype.renderRow = function ( const tooltipAttributes = tooltip ? `data-container="body" data-placement="left" data-title="${tooltip}"` : ''; - const dataUserSuggested = user.suggested ? `data-user-suggested=${user.suggested}` : ''; const busyBadge = user?.availability && isUserBusy(user.availability) ? `${__('Busy')}` : ''; return ` -
  • +
  • ${this.renderRowAvatar(issuableType, user, img)} diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 370daf8c54304e8e58b40c76c068bfa38e32bbbc..ea51fe3f97a808d42a34b4a68398e4a63787221b 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -28,20 +28,7 @@ def users .new(params: params, current_user: current_user, project: project, group: group) .execute - presented_users = UserSerializer - .new(params.merge({ current_user: current_user })) - .represent(users, project: project) - - extra_users = presented_suggested_users - - if extra_users.present? - presented_users.reject! do |user| - extra_users.any? { |suggested_user| suggested_user[:id] == user[:id] } - end - presented_users += extra_users - end - - render json: presented_users + render json: UserSerializer.new(params.merge({ current_user: current_user })).represent(users, project: project) end def user diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index 5c291e5e0511112c25fb9741cee6f3112e17c299..688d4797e56fea648aa93cd674260c71ddce9a5b 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -116,15 +116,9 @@ def reviewers_dropdown_options(issuable_type, iid = nil, target_branch = nil) dropdown_data = multiple_reviewers_dropdown_options(dropdown_data) if merge_request_supports_multiple_reviewers? - dropdown_data[:data].merge!(reviewers_dropdown_options_for_suggested_reviewers) dropdown_data end - # Overwritten - def reviewers_dropdown_options_for_suggested_reviewers - {} - end - # Overwritten def issue_supports_multiple_assignees? false diff --git a/app/models/concerns/has_user_type.rb b/app/models/concerns/has_user_type.rb index d77aa8821b0413105a108a85ad7dbc41a0432308..bcf8f4b0314876111e02c5350d24c03967afbd52 100644 --- a/app/models/concerns/has_user_type.rb +++ b/app/models/concerns/has_user_type.rb @@ -16,7 +16,6 @@ module HasUserType automation_bot: 9, security_policy_bot: 10, admin_bot: 11, - suggested_reviewers_bot: 12, service_account: 13, llm_bot: 14, placeholder: 15, @@ -34,7 +33,6 @@ module HasUserType automation_bot security_policy_bot admin_bot - suggested_reviewers_bot service_account llm_bot duo_code_review_bot diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index fb3b393be790e398af0ebf53d2baf4c1c2ce6bd9..d85d46e5174aab4d677fb395514b402d17eee1d1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -87,11 +87,6 @@ class MergeRequest < ApplicationRecord belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff' manual_inverse_association :latest_merge_request_diff, :merge_request - # method overridden in EE - def suggested_reviewer_users - User.none - end - # This is the same as latest_merge_request_diff unless: # 1. There are arguments - in which case we might be trying to force-reload. # 2. This association is already loaded. @@ -2485,10 +2480,6 @@ def execute_merge_checks(checks, params: {}, execute_all: false) # rubocop: enable CodeReuse/ServiceClass end - def can_suggest_reviewers? - false # overridden in EE - end - def hidden? author&.banned? end diff --git a/app/models/project.rb b/app/models/project.rb index b9704c4c938ae812fc47ef836ca6931a04dda5f5..d31f373bf755d2bf3a91d37a1a995f5c4c8fdc10 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3506,16 +3506,6 @@ def pages_domain_present?(domain_url) pages_url == domain_url || pages_domains.any? { |domain| domain.url == domain_url } end - # overridden in EE - def can_suggest_reviewers? - false - end - - # overridden in EE - def suggested_reviewers_available? - false - end - def crm_enabled? return false unless group diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 1e38a8975a4dba3875d83d801e077df01d6da240..5bcc672b72f852da7b25fc63c1f42d13f0a52758 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -17,6 +17,7 @@ class ProjectSetting < ApplicationRecord ignore_column :pages_multiple_versions_enabled, remove_with: '17.9', remove_after: '2025-02-20' ignore_column :pages_default_domain_redirect, remove_with: '17.9', remove_after: '2025-02-20' + ignore_column :suggested_reviewers_enabled, remove_with: '18.5', remove_after: '2025-09-19' scope :for_projects, ->(projects) { where(project_id: projects) } scope :with_namespace, -> { joins(project: :namespace) } @@ -43,7 +44,6 @@ class ProjectSetting < ApplicationRecord validates :squash_commit_template, length: { maximum: Project::MAX_COMMIT_TEMPLATE_LENGTH } validates :issue_branch_template, length: { maximum: Issue::MAX_BRANCH_TEMPLATE } validates :target_platforms, inclusion: { in: ALLOWED_TARGET_PLATFORMS } - validates :suggested_reviewers_enabled, inclusion: { in: [true, false] } validates :merge_request_title_regex_description, length: { maximum: Project::MAX_MERGE_REQUEST_TITLE_REGEX_DESCRIPTION } validates :merge_request_title_regex, untrusted_regexp: true, diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 896630a54cedbc9f5ab8b9348a803dea017f14e5..0993a8a1d2d66bae9fd08b5d6ff0e62929b15ddd 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -186,7 +186,6 @@ def filter_params(merge_request) end filter_reviewer(merge_request) - filter_suggested_reviewers end def filter_reviewer(merge_request) @@ -215,10 +214,6 @@ def filter_reviewer(merge_request) end end - def filter_suggested_reviewers - # Implemented in EE - end - def set_reviewers_approved(merge_request, new_reviewers) approval_users = merge_request.approvals_for_user_ids(new_reviewers.map(&:id)) diff --git a/app/views/projects/settings/merge_requests/show.html.haml b/app/views/projects/settings/merge_requests/show.html.haml index 463cce865033b93869982582950430e90b9a1b53..672e6ae48c63c59d650c869af3052468a68e9a20 100644 --- a/app/views/projects/settings/merge_requests/show.html.haml +++ b/app/views/projects/settings/merge_requests/show.html.haml @@ -17,6 +17,5 @@ = f.submit _('Save changes'), class: "rspec-save-merge-request-changes", data: { testid: 'save-merge-request-changes-button' }, pajamas_button: true = render_if_exists 'projects/settings/merge_requests/merge_request_approvals_settings', expanded: true -= render_if_exists 'projects/settings/merge_requests/suggested_reviewers_settings', expanded: true = render_if_exists 'projects/settings/merge_requests/duo_code_review_settings', expanded: true = render_if_exists 'projects/settings/merge_requests/target_branch_rules_settings' diff --git a/config/feature_flags/ops/suggested_reviewers_internal_api.yml b/config/feature_flags/ops/suggested_reviewers_internal_api.yml deleted file mode 100644 index f19beefa58a5ffa4360a44a281e07cce0621fcfa..0000000000000000000000000000000000000000 --- a/config/feature_flags/ops/suggested_reviewers_internal_api.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: suggested_reviewers_internal_api -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106648 -rollout_issue_url: -milestone: '15.7' -type: ops -group: group::ai framework -default_enabled: true diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 3e80fcf8b30ce296639bab3d32e66be054f80222..692f9ef8a407d219ee024a04197e7b5582e9facd 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1336,11 +1336,6 @@ production: &base # The client timeout in seconds for the KAS API (used by the GitLab backend) # client_timeout_seconds: 5 - suggested_reviewers: - # File that contains the secret key for verifying access to GitLab internal API for Suggested Reviewers. - # Default is '.gitlab_suggested_reviewers_secret' relative to Rails.root (i.e. root of the GitLab app). - # secret_file: /home/git/gitlab/.gitlab_suggested_reviewers_secret - zoekt: # Files that contain username and password for basic auth for Zoekt # Default is '.gitlab_zoekt_username' and '.gitlab_zoekt_password' in Rails.root diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 90ecc1cf996ee4667471c5b89a8f410fec6138dd..e94b2b291f6193d0c703de36f015b4faba3fedc9 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -617,8 +617,6 @@ - 1 - - merge_requests_approval_metrics_event - 1 -- - merge_requests_capture_suggested_reviewers_accepted - - 1 - - merge_requests_cleanup_ref - 1 - - merge_requests_close_issue @@ -635,8 +633,6 @@ - 1 - - merge_requests_execute_approval_hooks - 1 -- - merge_requests_fetch_suggested_reviewers - - 1 - - merge_requests_handle_assignees_change - 1 - - merge_requests_mergeability_check_batch @@ -761,8 +757,6 @@ - 1 - - projects_delete_branch - 1 -- - projects_deregister_suggested_reviewers_project - - 1 - - projects_finalize_project_statistics_refresh - 1 - - projects_forks_sync @@ -793,8 +787,6 @@ - 1 - - projects_refresh_build_artifacts_size_statistics - 1 -- - projects_register_suggested_reviewers_project - - 1 - - projects_repository_destroy - 1 - - projects_schedule_bulk_repository_shard_moves diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 88c12e06e058e15b3d06db206b6238c9c01644b1..baa7826cd243e5d53cd74dca802f3cd3d5308374 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -32365,7 +32365,6 @@ Defines which user roles, users, or groups can merge into a protected branch. | `squashReadOnly` | [`Boolean!`](#boolean) | Indicates if `squashReadOnly` is enabled. | | `state` | [`MergeRequestState!`](#mergerequeststate) | State of the merge request. | | `subscribed` | [`Boolean!`](#boolean) | Indicates if the currently logged in user is subscribed to the merge request. | -| `suggestedReviewers` | [`SuggestedReviewersType`](#suggestedreviewerstype) | Suggested reviewers for merge request. | | `supportsLockOnMerge` | [`Boolean!`](#boolean) | Indicates if the merge request supports locked labels. | | `targetBranch` | [`String!`](#string) | Target branch of the merge request. | | `targetBranchExists` | [`Boolean!`](#boolean) | Indicates if the target branch of the merge request exists. | @@ -41145,19 +41144,6 @@ Represents an entry from the future subscriptions. | `type` | [`String!`](#string) | Type of license the subscription will yield. | | `usersInLicenseCount` | [`Int`](#int) | Number of paid user seats. | -### `SuggestedReviewersType` - -Represents a Suggested Reviewers result set. - -#### Fields - -| Name | Type | Description | -| ---- | ---- | ----------- | -| `accepted` | [`[String!]`](#string) | List of accepted reviewer usernames. | -| `createdAt` | [`Time!`](#time) | Timestamp of when the suggestions were created. | -| `suggested` | [`[String!]!`](#string) | List of suggested reviewer usernames. | -| `updatedAt` | [`Time!`](#time) | Timestamp of when the suggestions were updated. | - ### `SystemNoteMetadata` #### Fields @@ -48073,7 +48059,6 @@ Possible types of user. | `SECURITY_POLICY_BOT` | Security policy bot. | | `SERVICE_ACCOUNT` | Service account. | | `SERVICE_USER` | Service user. | -| `SUGGESTED_REVIEWERS_BOT` | Suggested reviewers bot. | | `SUPPORT_BOT` | Support bot. | | `VISUAL_REVIEW_BOT` | Visual review bot. | diff --git a/ee/app/controllers/ee/autocomplete_controller.rb b/ee/app/controllers/ee/autocomplete_controller.rb index a5c5d813e80fa953e5eb69b0bcbb44767ba44ebf..c0d2501808ebfaea643514a26fb3236718680b8a 100644 --- a/ee/app/controllers/ee/autocomplete_controller.rb +++ b/ee/app/controllers/ee/autocomplete_controller.rb @@ -40,26 +40,5 @@ def namespace_routes render json: RouteSerializer.new.represent(routes) end - - private - - def suggested_reviewers_available? - project.can_suggest_reviewers? - end - - def presented_suggested_users - return [] unless params[:search].blank? && params[:merge_request_iid].present? - return [] unless suggested_reviewers_available? - - merge_request = project.merge_requests.find_by_iid!(params[:merge_request_iid]) - return [] unless merge_request&.open? - - suggested_users = merge_request.suggested_reviewer_users - return [] if suggested_users.empty? - - ::UserSerializer - .new(params.merge({ current_user: current_user, suggested: true })) - .represent(suggested_users, project: project) - end end end diff --git a/ee/app/controllers/ee/projects/merge_requests/application_controller.rb b/ee/app/controllers/ee/projects/merge_requests/application_controller.rb index 3b7abdc03c5bc336daea9613a866969464eb2fbc..b525feb9cb694ef1ef5c83c3105a266ff68aa1aa 100644 --- a/ee/app/controllers/ee/projects/merge_requests/application_controller.rb +++ b/ee/app/controllers/ee/projects/merge_requests/application_controller.rb @@ -30,8 +30,7 @@ def merge_request_params_attributes :approvals_before_merge, :approver_group_ids, :approver_ids, - :reset_approval_rules_to_defaults, - { suggested_reviewer_ids: [] } + :reset_approval_rules_to_defaults ) end diff --git a/ee/app/controllers/ee/projects/settings/merge_requests_controller.rb b/ee/app/controllers/ee/projects/settings/merge_requests_controller.rb index f5c266e7351b97d12116fd178127fed37983ac99..637548d43651435fdce7e820b22f16442d6a2b0f 100644 --- a/ee/app/controllers/ee/projects/settings/merge_requests_controller.rb +++ b/ee/app/controllers/ee/projects/settings/merge_requests_controller.rb @@ -31,7 +31,7 @@ def project_params_attributes override :project_setting_attributes def project_setting_attributes - attributes = %i[prevent_merge_without_jira_issue suggested_reviewers_enabled auto_duo_code_review_enabled] + attributes = %i[prevent_merge_without_jira_issue auto_duo_code_review_enabled] attributes << :only_allow_merge_if_all_status_checks_passed if allow_external_status_checks? diff --git a/ee/app/graphql/ee/types/merge_request_type.rb b/ee/app/graphql/ee/types/merge_request_type.rb index f4253bf0d4fac5065d6272e0455b46dc12999d7f..38a44567dc6f8365534734b40c727dda705341c5 100644 --- a/ee/app/graphql/ee/types/merge_request_type.rb +++ b/ee/app/graphql/ee/types/merge_request_type.rb @@ -56,10 +56,6 @@ module MergeRequestType description: 'Approval settings that are overridden by the policies for the merge request.', resolver: ::Resolvers::SecurityOrchestration::PolicyApprovalSettingsOverrideResolver - field :suggested_reviewers, ::Types::AppliedMl::SuggestedReviewersType, - null: true, - description: 'Suggested reviewers for merge request.' - field :blocking_merge_requests, ::Types::MergeRequests::BlockingMergeRequestsType, null: true, experiment: { milestone: '16.5' }, @@ -116,12 +112,6 @@ def merge_train_index object.merge_train_car&.index end - def suggested_reviewers - return unless object.project.can_suggest_reviewers? - - object.predictions - end - def base_merge_request object end diff --git a/ee/app/graphql/types/applied_ml/suggested_reviewers_type.rb b/ee/app/graphql/types/applied_ml/suggested_reviewers_type.rb deleted file mode 100644 index c84529b2d75f2657ae117b3279227f836ae1b236..0000000000000000000000000000000000000000 --- a/ee/app/graphql/types/applied_ml/suggested_reviewers_type.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -module Types - module AppliedMl - # SuggestedReviewersType have their authorization enforced by MergeRequestType - # rubocop: disable Graphql/AuthorizeTypes - class SuggestedReviewersType < BaseObject - graphql_name 'SuggestedReviewersType' - description 'Represents a Suggested Reviewers result set' - - present_using ::AppliedMl::SuggestedReviewersPresenter - - field :accepted, [GraphQL::Types::String], null: true, description: 'List of accepted reviewer usernames.' - field :created_at, Types::TimeType, null: false, description: 'Timestamp of when the suggestions were created.' - field :suggested, [GraphQL::Types::String], null: false, description: 'List of suggested reviewer usernames.' - field :updated_at, Types::TimeType, null: false, description: 'Timestamp of when the suggestions were updated.' - end - # rubocop: enable Graphql/AuthorizeTypes - end -end diff --git a/ee/app/helpers/ee/form_helper.rb b/ee/app/helpers/ee/form_helper.rb index 26df3f419c94df9f67bb9608044da8e4fee8a699..315947e743d17c48afdb62010786a3f12fa3ebfa 100644 --- a/ee/app/helpers/ee/form_helper.rb +++ b/ee/app/helpers/ee/form_helper.rb @@ -2,21 +2,6 @@ module EE module FormHelper - # Overwritten - def reviewers_dropdown_options_for_suggested_reviewers - suggested_reviewers_help_path = help_page_path( - 'user/project/merge_requests/reviews/_index.md', - anchor: 'request-a-review' - ) - - { - show_suggested: true, - suggested_reviewers_help_path: suggested_reviewers_help_path, - suggested_reviewers_header: _('Suggestion(s)'), - all_members_header: _('All project members') - } - end - def issue_supports_multiple_assignees? current_board_parent.feature_available?(:multiple_issue_assignees) end diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 87f360969722b416bf2dfdae954e8515c77e2e34..0f2f6ff54f318e514c624ce05dc65521e653d34f 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -575,31 +575,6 @@ def diff_head_pipeline?(pipeline) pipeline.source_sha == diff_head_sha || pipeline.sha == diff_head_sha end - override :can_suggest_reviewers? - def can_suggest_reviewers? - open? && modified_paths.any? - end - - override :suggested_reviewer_users - def suggested_reviewer_users - return ::User.none unless predictions && predictions.suggested_reviewers.is_a?(Hash) - - usernames = Array.wrap(suggested_reviewers["reviewers"]) - return ::User.none if usernames.empty? - - # Preserve the original order of suggested usernames - join_sql = ::MergeRequest.sanitize_sql_array( - [ - 'JOIN UNNEST(ARRAY[?]::varchar[]) WITH ORDINALITY AS t(username, ord) USING(username)', - usernames - ] - ) - - project.authorized_users.with_state(:active).human - .joins(Arel.sql(join_sql)) - .order('t.ord') - end - def rebase_commit_is_different?(newrev) rebase_commit_sha != newrev end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 67f7485c98ed3b9fb598513b7add199c7da8e8bd..bf46c4a6197b6b619944ac69b90b6bc7569ea313 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -463,7 +463,6 @@ def lock_for_confirmation!(id) delegate :prevent_merge_without_jira_issue, :prevent_merge_without_jira_issue=, :selective_code_owner_removals, - :suggested_reviewers_enabled, :only_allow_merge_if_all_status_checks_passed, :only_allow_merge_if_all_status_checks_passed=, :mirror_branch_regex, @@ -543,18 +542,6 @@ def configured_to_create_issues_from_vulnerabilities? !!jira_integration&.configured_to_create_issues_from_vulnerabilities? end - def can_suggest_reviewers? - suggested_reviewers_available? && suggested_reviewers_enabled - end - - def suggested_reviewers_available? - strong_memoize(:suggested_reviewers_available) do - ::Feature.disabled?(:hide_suggested_reviewers, self, type: :beta) && - ::Gitlab.com? && - licensed_feature_available?(:suggested_reviewers) - end - end - def member_usernames_among(users) members_among(users).pluck(:username) end diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index 7c3b81007e6134b746f78206e93207047004eb65..d8800fcf69f873da3c7a48d8430051244b81c501 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -285,7 +285,6 @@ class Features summarize_mr_changes stale_runner_cleanup_for_namespace status_page - suggested_reviewers subepics observability unique_project_download_limit diff --git a/ee/app/policies/ee/base_policy.rb b/ee/app/policies/ee/base_policy.rb index 05c2b1a6dbe08736d68ba8a2cfebf76cd6401602..574cfa75ce8cb9ed0845791c11cbc2ebd58972a4 100644 --- a/ee/app/policies/ee/base_policy.rb +++ b/ee/app/policies/ee/base_policy.rb @@ -16,10 +16,6 @@ module BasePolicy with_scope :user condition(:visual_review_bot, score: 0) { @user&.visual_review_bot? } - desc "User is suggested reviewers bot" - with_scope :user - condition(:suggested_reviewers_bot, score: 0) { @user&.suggested_reviewers_bot? } - with_scope :global condition(:license_block) { License.block_changes? } diff --git a/ee/app/policies/ee/policy_actor.rb b/ee/app/policies/ee/policy_actor.rb index 6052c5269dbfadfa5ef1874c052e069b429f0035..98e40ef22cbc0544ce1a1557ca6659e810372573 100644 --- a/ee/app/policies/ee/policy_actor.rb +++ b/ee/app/policies/ee/policy_actor.rb @@ -10,10 +10,6 @@ def visual_review_bot? false end - def suggested_reviewers_bot? - false - end - def group_sso?(_) false end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 7219570cc158e1b798c49600455502e0db350302..66cfad442a9d79c35f0103ff3181245c7ae2c851 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -274,11 +274,6 @@ module ProjectPolicy end end - with_scope :subject - condition(:suggested_reviewers_available) do - @subject.can_suggest_reviewers? - end - condition(:summarize_new_merge_request_enabled) do ::Feature.enabled?(:add_ai_summary_for_new_mr, subject) && ::Gitlab::Llm::FeatureAuthorizer.new( @@ -1045,11 +1040,6 @@ module ProjectPolicy enable :create_key_result end - rule { suggested_reviewers_bot & suggested_reviewers_available & resource_access_token_feature_available & resource_access_token_creation_allowed }.policy do - enable :admin_project_member - enable :create_resource_access_tokens - end - rule { custom_role_enables_manage_project_access_tokens & resource_access_token_feature_available & resource_access_token_creation_allowed }.policy do enable :read_resource_access_tokens enable :create_resource_access_tokens diff --git a/ee/app/presenters/applied_ml/suggested_reviewers_presenter.rb b/ee/app/presenters/applied_ml/suggested_reviewers_presenter.rb deleted file mode 100644 index 851f841da1c0c2cfa1776bb8b000717342fc396e..0000000000000000000000000000000000000000 --- a/ee/app/presenters/applied_ml/suggested_reviewers_presenter.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true -# -module AppliedMl - class SuggestedReviewersPresenter < Gitlab::View::Presenter::Delegated - presents ::MergeRequest::Predictions, as: :predictions - - delegator_override :accepted - def accepted - accepted_reviewer_usernames - end - - delegator_override :suggested - def suggested - suggested_reviewer_usernames - end - end -end diff --git a/ee/app/services/ee/merge_requests/after_create_service.rb b/ee/app/services/ee/merge_requests/after_create_service.rb index efdd2a926fd89cc87e6235f440e577a2cfc7af26..b41fc6c1d38456ebc2e7ac0adc8b28e1a9748bd8 100644 --- a/ee/app/services/ee/merge_requests/after_create_service.rb +++ b/ee/app/services/ee/merge_requests/after_create_service.rb @@ -18,7 +18,7 @@ def prepare_merge_request(merge_request) record_onboarding_progress(merge_request) merge_request.schedule_policy_synchronization - schedule_fetch_suggested_reviewers(merge_request) + schedule_approval_notifications(merge_request) schedule_duo_code_review(merge_request) track_usage_event if merge_request.project.scan_result_policy_reads.any? @@ -45,13 +45,6 @@ def schedule_approval_notifications(merge_request) ::MergeRequests::NotifyApproversWorker.perform_in(APPROVERS_NOTIFICATION_DELAY, merge_request.id) end - def schedule_fetch_suggested_reviewers(merge_request) - return unless merge_request.project.can_suggest_reviewers? - return unless merge_request.can_suggest_reviewers? - - ::MergeRequests::FetchSuggestedReviewersWorker.perform_async(merge_request.id) - end - # NOTE: If it was requested, Duo Code Review needs to be triggered after merge_request_diff gets created. def schedule_duo_code_review(merge_request) return unless merge_request.reviewers.duo_code_review_bot.any? diff --git a/ee/app/services/ee/merge_requests/base_service.rb b/ee/app/services/ee/merge_requests/base_service.rb index a26607257c8042a1fa0b2f4ae747f855d302015f..243c2916c3eaf71b6e2b946f71fb71c75743d368 100644 --- a/ee/app/services/ee/merge_requests/base_service.rb +++ b/ee/app/services/ee/merge_requests/base_service.rb @@ -7,7 +7,7 @@ module BaseService private - attr_accessor :blocking_merge_requests_params, :suggested_reviewer_ids + attr_accessor :blocking_merge_requests_params def assign_duo_as_reviewer(merge_request) project = merge_request.project @@ -26,7 +26,7 @@ def assign_duo_as_reviewer(merge_request) def handle_reviewers_change(merge_request, old_reviewers) super new_reviewers = merge_request.reviewers - old_reviewers - capture_suggested_reviewers_accepted(merge_request) + set_requested_changes(merge_request, new_reviewers) if new_reviewers.any? request_duo_code_review(merge_request) if new_reviewers.any?(&:duo_code_review_bot?) end @@ -55,14 +55,6 @@ def filter_params(merge_request) super end - override :filter_suggested_reviewers - def filter_suggested_reviewers - suggested_reviewer_ids_from_params = params.delete(:suggested_reviewer_ids) - return if suggested_reviewer_ids_from_params.blank? - - self.suggested_reviewer_ids = suggested_reviewer_ids_from_params & params[:reviewer_ids] - end - def set_requested_changes(merge_request, new_reviewers) requested_changes_users = merge_request.requested_changes_for_users(new_reviewers.map(&:id)) @@ -132,13 +124,6 @@ def publish_approvals_reset_event(merge_request, cause, approver_ids) ) end - def capture_suggested_reviewers_accepted(merge_request) - return if suggested_reviewer_ids.blank? - - ::MergeRequests::CaptureSuggestedReviewersAcceptedWorker - .perform_async(merge_request.id, suggested_reviewer_ids) - end - def audit_security_policy_branch_bypass(merge_request) matching_policies = merge_request.security_policies_with_branch_exceptions diff --git a/ee/app/services/ee/merge_requests/refresh_service.rb b/ee/app/services/ee/merge_requests/refresh_service.rb index d51043f4add630ca10c6c4fa6027ceb2b04af7b7..42aec6f97859fc7cdfdca58818ea4e3ee1376f54 100644 --- a/ee/app/services/ee/merge_requests/refresh_service.rb +++ b/ee/app/services/ee/merge_requests/refresh_service.rb @@ -19,22 +19,11 @@ def refresh_merge_requests! reset_approvals_for_merge_requests(push.ref, push.newrev) - trigger_suggested_reviewers_fetch sync_any_merge_request_approval_rules sync_preexisting_states_approval_rules sync_unenforceable_approval_rules end - def trigger_suggested_reviewers_fetch - return unless project.can_suggest_reviewers? - - merge_requests_for_source_branch.each do |mr| - next unless mr.can_suggest_reviewers? - - ::MergeRequests::FetchSuggestedReviewersWorker.perform_async(mr.id) - end - end - def reset_approvals_for_merge_requests(ref, newrev) # Add a flag that prevents unverified changes from getting through in the # 10 second window below diff --git a/ee/app/services/ee/projects/update_service.rb b/ee/app/services/ee/projects/update_service.rb index 5816b76ed0003a90f3229927257f6116919187ff..7bdf775b4946af479defc926d5791a54cfc6c17c 100644 --- a/ee/app/services/ee/projects/update_service.rb +++ b/ee/app/services/ee/projects/update_service.rb @@ -31,13 +31,6 @@ def execute return update_failed! if project.errors.any? - if params[:project_setting_attributes].present? - suggested_reviewers_already_enabled = project.suggested_reviewers_enabled - unless project.suggested_reviewers_available? - params[:project_setting_attributes].delete(:suggested_reviewers_enabled) - end - end - prepare_analytics_dashboards_params! validate_web_based_commit_signing_enabled @@ -56,8 +49,6 @@ def execute project.remove_import_data if project.previous_changes.include?('mirror') && !project.mirror? update_amazon_q_service_account! update_duo_workflow_service_account! - - suggested_reviewers_already_enabled ? trigger_project_deregistration : trigger_project_registration end result @@ -96,28 +87,6 @@ def prepare_analytics_dashboards_params! end end - def trigger_project_registration - return unless params[:project_setting_attributes].present? && - params[:project_setting_attributes][:suggested_reviewers_enabled] == '1' - - return unless can_update_suggested_reviewers_setting? - - ::Projects::RegisterSuggestedReviewersProjectWorker.perform_async(project.id, current_user.id) - end - - def trigger_project_deregistration - return unless params[:project_setting_attributes].present? && - params[:project_setting_attributes][:suggested_reviewers_enabled] == '0' - - return unless project.suggested_reviewers_available? - - ::Projects::DeregisterSuggestedReviewersProjectWorker.perform_async(project.id, current_user.id) - end - - def can_update_suggested_reviewers_setting? - project.suggested_reviewers_available? && current_user.can?(:create_resource_access_tokens, project) - end - override :remove_unallowed_params def remove_unallowed_params super diff --git a/ee/app/services/merge_requests/capture_suggested_reviewers_accepted_service.rb b/ee/app/services/merge_requests/capture_suggested_reviewers_accepted_service.rb deleted file mode 100644 index 44c66ea75362796f07abf550421720d09ebd7581..0000000000000000000000000000000000000000 --- a/ee/app/services/merge_requests/capture_suggested_reviewers_accepted_service.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class CaptureSuggestedReviewersAcceptedService < BaseProjectService - # Capture suggested reviewers whom are accepted as reviewers - # - # @param [MergeRequest] merge_request - # @param [Array] reviewer_ids - # - # The list of accepted users will be normalised with the suggested list and - # existing accepted list. - # - # suggested_usernames: [a, b, c] - # existing_accepted_usernames: [a] - # new_accepted_usernames: [b, d] - # accepted_usernames: [a, b] - # - # @return [ServiceResponse] an instance of a ServiceResponse object - def execute(merge_request, reviewer_ids) - return ServiceResponse.error(message: 'Reviewer IDs are empty') if reviewer_ids.blank? - return ServiceResponse.error(message: 'Merge request is not eligible') unless merge_request.can_suggest_reviewers? - - predictions = merge_request.predictions - return ServiceResponse.error(message: 'No predictions are recorded') if predictions.blank? - - accepted_usernames = calculate_accepted_usernames(predictions, reviewer_ids) - predictions.update!(accepted_reviewers: { reviewers: accepted_usernames }) - - ServiceResponse.success( - payload: { - project_id: project.id, - merge_request_id: merge_request.id, - reviewers: predictions.accepted_reviewer_usernames - } - ) - rescue ActiveRecord::RecordInvalid => err - ServiceResponse.error(message: err.message) - end - - private - - def calculate_accepted_usernames(predictions, reviewer_ids) - suggested_usernames = predictions.suggested_reviewer_usernames - existing_accepted_usernames = predictions.accepted_reviewer_usernames - new_accepted_usernames = project.member_usernames_among(User.id_in(reviewer_ids)) - - suggested_usernames & (existing_accepted_usernames | new_accepted_usernames) - end - end -end diff --git a/ee/app/services/merge_requests/fetch_suggested_reviewers_service.rb b/ee/app/services/merge_requests/fetch_suggested_reviewers_service.rb deleted file mode 100644 index 6338f6ddda8e2888f452f0d7b601d5687f3a91e1..0000000000000000000000000000000000000000 --- a/ee/app/services/merge_requests/fetch_suggested_reviewers_service.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class FetchSuggestedReviewersService < BaseProjectService - def execute(merge_request) - suggested_reviewers(merge_request) - end - - private - - def suggested_reviewers(merge_request) - changes = merge_request.modified_paths - return error('Merge request contains no modified files') if changes.empty? - - model_input = { - project_id: merge_request.project_id, - merge_request_iid: merge_request.iid, - changes: changes, - author_username: merge_request.author.username - } - - result = ::Gitlab::AppliedMl::SuggestedReviewers::Client.new.suggested_reviewers(**model_input) - success(result) - end - end -end diff --git a/ee/app/services/projects/deregister_suggested_reviewers_project_service.rb b/ee/app/services/projects/deregister_suggested_reviewers_project_service.rb deleted file mode 100644 index 6613371e0bc8c15ffe3778c001550cf2892d2e6c..0000000000000000000000000000000000000000 --- a/ee/app/services/projects/deregister_suggested_reviewers_project_service.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module Projects - class DeregisterSuggestedReviewersProjectService < BaseProjectService - def execute - unless project_deregistration_available? - return ServiceResponse.error( - message: 'Suggested Reviewers deregistration is unavailable', - reason: :feature_unavailable - ) - end - - result = ::Gitlab::AppliedMl::SuggestedReviewers::Client.new.deregister_project( - project_id: project.id - ) - - ServiceResponse.success(payload: result) - rescue Gitlab::AppliedMl::Errors::ProjectNotFound - ServiceResponse.error(message: 'Project is not found', reason: :project_not_found) - rescue Gitlab::AppliedMl::Errors::ResourceNotAvailable - ServiceResponse.error(message: 'Failed to deregister project', reason: :client_request_failed) - end - - private - - def project_deregistration_available? - project.suggested_reviewers_available? && !project.suggested_reviewers_enabled - end - end -end diff --git a/ee/app/services/projects/register_suggested_reviewers_project_service.rb b/ee/app/services/projects/register_suggested_reviewers_project_service.rb deleted file mode 100644 index 31b6a167aeb1e20a2ebdccf14a544dc3a38ad77e..0000000000000000000000000000000000000000 --- a/ee/app/services/projects/register_suggested_reviewers_project_service.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -module Projects - class RegisterSuggestedReviewersProjectService < BaseProjectService - def execute - unless project_registration_available? - return ServiceResponse.error( - message: 'Suggested Reviewers feature is unavailable', - reason: :feature_unavailable - ) - end - - token_response = create_access_token - if token_response.error? - return ServiceResponse.error( - message: 'Failed to create access token', - reason: :token_creation_failed - ) - end - - access_token = token_response.payload[:access_token].token - registration_input = { - project_id: project.id, - project_name: project.name, - project_namespace: project.namespace.full_path, - access_token: access_token - } - result = ::Gitlab::AppliedMl::SuggestedReviewers::Client.new.register_project(**registration_input) - - ServiceResponse.success(payload: result) - rescue Gitlab::AppliedMl::Errors::ProjectAlreadyExists - ServiceResponse.error(message: 'Project is already registered', reason: :project_already_registered) - rescue Gitlab::AppliedMl::Errors::ResourceNotAvailable - ServiceResponse.error(message: 'Failed to register project', reason: :client_request_failed) - end - - private - - def project_registration_available? - project.suggested_reviewers_available? && - project.suggested_reviewers_enabled && - current_user.can?(:create_resource_access_tokens, project) - end - - def create_access_token - token_params = { - name: 'Suggested reviewers token', - scopes: [Gitlab::Auth::READ_API_SCOPE], - expires_at: 95.days.from_now - } - - ResourceAccessTokens::CreateService.new(current_user, project, token_params).execute - end - end -end diff --git a/ee/app/views/projects/settings/merge_requests/_merge_requests_suggested_reviewers_form.html.haml b/ee/app/views/projects/settings/merge_requests/_merge_requests_suggested_reviewers_form.html.haml deleted file mode 100644 index 90570fd826a9efb94fc9ddb5727754f766498d47..0000000000000000000000000000000000000000 --- a/ee/app/views/projects/settings/merge_requests/_merge_requests_suggested_reviewers_form.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -= gitlab_ui_form_for @project, url: project_settings_merge_requests_path(@project), html: { multipart: true, class: "merge-request-settings-form" }, authenticity_token: true do |f| - .form-group - = f.fields_for :project_setting do |settings| - = settings.gitlab_ui_checkbox_component :suggested_reviewers_enabled, s_('ProjectSettings|Enable suggested reviewers'), help_text: s_('SuggestedReviewers|Suggestions appear in the Reviewer section of the right sidebar') - = f.submit _('Save changes'), pajamas_button: true diff --git a/ee/app/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml b/ee/app/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml deleted file mode 100644 index eec721e7556d8a77372d3f13a806699cbbadd935..0000000000000000000000000000000000000000 --- a/ee/app/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -- return unless @project.feature_available?(:merge_requests, current_user) -- return unless @project.suggested_reviewers_available? - -- if current_user.can?(:create_resource_access_tokens, @project) - = render ::Layouts::SettingsSectionComponent.new(s_('SuggestedReviewers|Suggested reviewers'), - id: 'js-merge-request-suggested-reviewers-settings', - options: { class: 'merge-requests-feature' }) do |c| - - c.with_description do - = s_("SuggestedReviewers|Get suggestions for reviewers based on GitLab's machine learning tool.") - = link_to _("Learn more."), help_page_path("user/project/merge_requests/reviews/_index.md"), target: '_blank', rel: 'noopener noreferrer' - - c.with_body do - = render 'projects/settings/merge_requests/merge_requests_suggested_reviewers_form' diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 0b60c6b4905d7e62551ad69be6fa2020e677e166..c66417896b9de9775865a8d3ab2d030ea0909e2d 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2683,16 +2683,6 @@ :idempotent: true :tags: [] :queue_namespace: -- :name: merge_requests_capture_suggested_reviewers_accepted - :worker_name: MergeRequests::CaptureSuggestedReviewersAcceptedWorker - :feature_category: :code_review_workflow - :has_external_dependencies: false - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :queue_namespace: - :name: merge_requests_create_approvals_reset_note :worker_name: MergeRequests::CreateApprovalsResetNoteWorker :feature_category: :code_review_workflow @@ -2713,16 +2703,6 @@ :idempotent: false :tags: [] :queue_namespace: -- :name: merge_requests_fetch_suggested_reviewers - :worker_name: MergeRequests::FetchSuggestedReviewersWorker - :feature_category: :code_review_workflow - :has_external_dependencies: true - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :queue_namespace: - :name: merge_requests_notify_approvers :worker_name: MergeRequests::NotifyApproversWorker :feature_category: :code_review_workflow @@ -2964,26 +2944,6 @@ :tags: - :import_shared_storage :queue_namespace: -- :name: projects_deregister_suggested_reviewers_project - :worker_name: Projects::DeregisterSuggestedReviewersProjectWorker - :feature_category: :code_review_workflow - :has_external_dependencies: true - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :queue_namespace: -- :name: projects_register_suggested_reviewers_project - :worker_name: Projects::RegisterSuggestedReviewersProjectWorker - :feature_category: :code_review_workflow - :has_external_dependencies: true - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: true - :tags: [] - :queue_namespace: - :name: projects_repository_destroy :worker_name: Projects::RepositoryDestroyWorker :feature_category: :importers diff --git a/ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb b/ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb deleted file mode 100644 index 4687ffc4b8dbb1f4056c4fb9b406397fb9d6de04..0000000000000000000000000000000000000000 --- a/ee/app/workers/merge_requests/capture_suggested_reviewers_accepted_worker.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class CaptureSuggestedReviewersAcceptedWorker - include ApplicationWorker - - data_consistency :always - feature_category :code_review_workflow - urgency :low - deduplicate :until_executed - - sidekiq_options retry: 3 - - idempotent! - - def perform(merge_request_id, reviewer_ids) - return if reviewer_ids.blank? - - merge_request = MergeRequest.find_by_id(merge_request_id) - return unless merge_request&.can_suggest_reviewers? - - response = ::MergeRequests::CaptureSuggestedReviewersAcceptedService - .new(project: merge_request.project) - .execute(merge_request, reviewer_ids) - - handle_success(response) if response.success? - end - - private - - def handle_success(response) - log_extra_metadata_on_done(:project_id, response.payload[:project_id]) - log_extra_metadata_on_done(:merge_request_id, response.payload[:merge_request_id]) - log_extra_metadata_on_done(:reviewers, response.payload[:reviewers]) - end - end -end diff --git a/ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb b/ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb deleted file mode 100644 index b6f0323f262a8889dbab674b087fe9f8966840bc..0000000000000000000000000000000000000000 --- a/ee/app/workers/merge_requests/fetch_suggested_reviewers_worker.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class FetchSuggestedReviewersWorker - include ApplicationWorker - - data_consistency :always - feature_category :code_review_workflow - urgency :low - deduplicate :until_executed - - sidekiq_options retry: 3 - - idempotent! - - # MergeRequests::FetchSuggestedReviewersWorker makes an external RPC request - worker_has_external_dependencies! - - def perform(merge_request_id) - merge_request = MergeRequest.find_by_id(merge_request_id) - return unless merge_request - return if merge_request.modified_paths.empty? - - result = ::MergeRequests::FetchSuggestedReviewersService - .new(project: merge_request.project) - .execute(merge_request) - - if result && result[:status] == :success - merge_request.build_predictions unless merge_request.predictions - merge_request.predictions.update(suggested_reviewers: result.except(:status)) - else - logger.error(structured_payload({ result: result })) - end - rescue Gitlab::AppliedMl::Errors::ResourceNotAvailable => e - Gitlab::ErrorTracking.log_exception( - e, - project_id: merge_request.project.id, - merge_request_id: merge_request.id - ) - - nil - end - end -end diff --git a/ee/app/workers/projects/deregister_suggested_reviewers_project_worker.rb b/ee/app/workers/projects/deregister_suggested_reviewers_project_worker.rb deleted file mode 100644 index 40e9524625a2eacc50ce4cae99492b0e290a59ac..0000000000000000000000000000000000000000 --- a/ee/app/workers/projects/deregister_suggested_reviewers_project_worker.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -module Projects - class DeregisterSuggestedReviewersProjectWorker - include ApplicationWorker - - data_consistency :delayed - feature_category :code_review_workflow - urgency :low - deduplicate :until_executed - - sidekiq_options retry: 3 - - idempotent! - - # ::Projects::DeregisterSuggestedReviewersProjectService makes an external RPC request - worker_has_external_dependencies! - - def perform(project_id, user_id) - project = Project.find_by_id(project_id) - return unless project && project.suggested_reviewers_available? && !project.suggested_reviewers_enabled - - user = User.find_by_id(user_id) - return unless user - - response = ::Projects::DeregisterSuggestedReviewersProjectService - .new(project: project, current_user: user) - .execute - - handle_result(response, project_id) - end - - private - - def handle_result(response, project_id) - return if response.error? && response.reason != :client_request_failed - return response.track_and_raise_exception(project_id: project_id) if response.error? - - log_hash_metadata_on_done( - project_id: response.payload[:project_id], - deregistered_at: response.payload[:deregistered_at] - ) - end - end -end diff --git a/ee/app/workers/projects/register_suggested_reviewers_project_worker.rb b/ee/app/workers/projects/register_suggested_reviewers_project_worker.rb deleted file mode 100644 index dfbd4f1029fc0695c9d008eff8ac34cec203d5bb..0000000000000000000000000000000000000000 --- a/ee/app/workers/projects/register_suggested_reviewers_project_worker.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -module Projects - class RegisterSuggestedReviewersProjectWorker - include ApplicationWorker - - data_consistency :always - feature_category :code_review_workflow - urgency :low - deduplicate :until_executed - - sidekiq_options retry: 3 - - idempotent! - - # ::Projects::RegisterSuggestedReviewersProjectService makes an external RPC request - worker_has_external_dependencies! - - def perform(project_id, user_id) - project = Project.find_by_id(project_id) - return unless project && project.suggested_reviewers_available? && project.suggested_reviewers_enabled - - user = User.find_by_id(user_id) - return unless user - - response = ::Projects::RegisterSuggestedReviewersProjectService.new(project: project, current_user: user).execute - if response.error? - handle_error(response, project_id) - else - handle_success(response) - end - end - - private - - def handle_error(response, project_id) - case response.reason - when :client_request_failed - response.track_and_raise_exception(project_id: project_id) - when :project_already_registered - # ignore - response - else - response.track_exception(project_id: project_id) - end - end - - def handle_success(response) - log_extra_metadata_on_done(:project_id, response.payload[:project_id]) - log_extra_metadata_on_done(:registered_at, response.payload[:registered_at]) - end - end -end diff --git a/ee/config/feature_flags/beta/hide_suggested_reviewers.yml b/ee/config/feature_flags/beta/hide_suggested_reviewers.yml deleted file mode 100644 index e18a80b104ade5535f627bcf99e568cf83c0e7c8..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/beta/hide_suggested_reviewers.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: hide_suggested_reviewers -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/478831 -introduced_by_url: -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/505999 -milestone: '17.7' -group: group::code review -type: beta -default_enabled: false diff --git a/ee/config/initializers/gitlab_suggested_reviewers_secret.rb b/ee/config/initializers/gitlab_suggested_reviewers_secret.rb deleted file mode 100644 index e71a0e9e49a2db115995ed5293728d4deda3d1c4..0000000000000000000000000000000000000000 --- a/ee/config/initializers/gitlab_suggested_reviewers_secret.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -return unless Gitlab.com? - -Gitlab::AppliedMl::SuggestedReviewers.ensure_secret! diff --git a/ee/lib/api/internal/suggested_reviewers.rb b/ee/lib/api/internal/suggested_reviewers.rb deleted file mode 100644 index 78b1f13fe0905e1d58ba988ecac41e8e17028442..0000000000000000000000000000000000000000 --- a/ee/lib/api/internal/suggested_reviewers.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -module API - # Suggested Reviewers Internal API - module Internal - class SuggestedReviewers < ::API::Base - feature_category :code_review_workflow - - before do - check_feature_enabled - authenticate_gitlab_suggested_reviewers_request! - end - - helpers do - def check_feature_enabled - not_found! unless Feature.enabled?(:suggested_reviewers_internal_api, type: :ops) - not_found! if ::Feature.enabled?(:hide_suggested_reviewers, type: :beta) - end - - def authenticate_gitlab_suggested_reviewers_request! - return if Gitlab::AppliedMl::SuggestedReviewers.verify_api_request(headers) - - render_api_error!('Suggested Reviewers JWT authentication invalid', :unauthorized) - end - - def create_access_token(project) - token_params = { - name: 'Suggested reviewers token', - access_level: Gitlab::Access::REPORTER, - scopes: [Gitlab::Auth::READ_API_SCOPE], - expires_at: 1.day.from_now - } - - ::ResourceAccessTokens::CreateService.new( - ::Users::Internal.suggested_reviewers_bot, - project, - token_params - ).execute - end - end - - namespace 'internal' do - namespace 'suggested_reviewers' do - resource :tokens do - desc 'Create a project access token' do - detail 'Creates a new access token for a project.' - tags 'project_access_tokens' - success Entities::ResourceAccessTokenWithToken - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 404, message: 'Not found' } - ] - end - params do - requires :project_id, type: Integer, desc: 'The ID of the project' - end - post do - ::Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/503887', - new_threshold: 110) - - project = find_project(params[:project_id]) - not_found! unless project&.can_suggest_reviewers? - - token_response = create_access_token(project) - if token_response.success? - present( - token_response.payload[:access_token], - with: Entities::ResourceAccessTokenWithToken, - resource: project - ) - else - bad_request!(token_response.message) - end - end - end - end - end - end - end -end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index 3d9993000cf4ba4840813fdbb8408b3c9cecd95f..e5eb6d3d2275605056287094448d658309075cbb 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -90,7 +90,6 @@ module API mount ::API::Internal::AppSec::Dast::SiteValidations mount ::API::Internal::Search::Zoekt - mount ::API::Internal::SuggestedReviewers mount ::API::Internal::Ai::XRay::Scan mount ::API::Internal::Observability diff --git a/ee/lib/ee/users/internal.rb b/ee/lib/ee/users/internal.rb index 4ca636d18572799ecae742ecd263a3ed71977398..bbdf074fd367df91a512a38ed304bfeb342a334d 100644 --- a/ee/lib/ee/users/internal.rb +++ b/ee/lib/ee/users/internal.rb @@ -9,7 +9,7 @@ module Internal extend Forwardable # Delegate to an instance method of the class - def_delegators :new, :visual_review_bot, :suggested_reviewers_bot + def_delegators :new, :visual_review_bot end # rubocop:disable CodeReuse/ActiveRecord -- Need to instantiate a record here @@ -24,17 +24,6 @@ def visual_review_bot end end - def suggested_reviewers_bot - email_pattern = "suggested-reviewers-bot%s@#{Settings.gitlab.host}" - - unique_internal( - ::User.where(user_type: :suggested_reviewers_bot), 'suggested-reviewers-bot', email_pattern) do |u| - u.bio = 'The GitLab suggested reviewers bot used for suggested reviewers' - u.name = 'GitLab Suggested Reviewers Bot' - u.confirmed_at = Time.zone.now - u.private_profile = true - end - end # rubocop:enable CodeReuse/ActiveRecord end end diff --git a/ee/lib/gitlab/applied_ml/suggested_reviewers.rb b/ee/lib/gitlab/applied_ml/suggested_reviewers.rb deleted file mode 100644 index 65a05c26a38aa8cefa4c575e26a0303db6fea0b1..0000000000000000000000000000000000000000 --- a/ee/lib/gitlab/applied_ml/suggested_reviewers.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module AppliedMl - module SuggestedReviewers - INTERNAL_API_REQUEST_HEADER = 'Gitlab-Suggested-Reviewers-Api-Request' - JWT_ISSUER = 'gitlab-suggested-reviewers' - EXPIRATION = 5.minutes - - include JwtAuthenticatable - - class << self - def verify_api_request(request_headers) - token = request_headers[INTERNAL_API_REQUEST_HEADER] - return unless token - - decode_jwt( - token, - issuer: JWT_ISSUER, - iat_after: Time.current - EXPIRATION - ) - rescue JWT::DecodeError - nil - end - - def secret_path - Gitlab.config.suggested_reviewers.secret_file - end - - def ensure_secret! - return if File.exist?(secret_path) - - write_secret - end - end - end - end -end diff --git a/ee/lib/gitlab/applied_ml/suggested_reviewers/client.rb b/ee/lib/gitlab/applied_ml/suggested_reviewers/client.rb deleted file mode 100644 index f650ad422710aaa710e32f1524c4541dc6400d9b..0000000000000000000000000000000000000000 --- a/ee/lib/gitlab/applied_ml/suggested_reviewers/client.rb +++ /dev/null @@ -1,150 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module AppliedMl - module SuggestedReviewers - class Client - include Gitlab::AppliedMl::SuggestedReviewers::RecommenderPb - include Gitlab::AppliedMl::SuggestedReviewers::RecommenderServicesPb - - DEFAULT_TIMEOUT = 15 - DEFAULT_CERTS = ::Gitlab::X509::Certificate.ca_certs_bundle - - JWT_ISSUER = "gitlab-issuer" - JWT_AUDIENCE = "gitlab-suggested-reviewers" - SECRET_NAME = "SUGGESTED_REVIEWERS_SECRET" - SECRET_LENGTH = 64 - - NETWORK_ERRORS = [ - GRPC::DeadlineExceeded, - GRPC::Unavailable - ].freeze - - def self.default_rpc_url - if Gitlab.dev_or_test_env? - 'suggested-reviewer.dev:443' - else - 'api.unreview.io:443' - end - end - - def initialize(rpc_url: self.class.default_rpc_url, certs: DEFAULT_CERTS) - raise Errors::ConfigurationError, "gRPC host unknown" if rpc_url.blank? - - @rpc_url = rpc_url - @certs = certs - @secret = read_secret! - end - - def suggested_reviewers(merge_request_iid:, project_id:, changes:, author_username:, top_n: 5) - raise Errors::ArgumentError, "Changes empty" if changes.blank? - - model_input = { - merge_request_iid: merge_request_iid, - top_n: top_n, - project_id: project_id, - changes: changes, - author_username: author_username - } - response = get_reviewers(model_input) - - { - version: response.version, - top_n: response.top_n, - reviewers: response.reviewers - } - rescue *NETWORK_ERRORS => e - raise Errors::ConnectionFailed, e - rescue GRPC::BadStatus => e - raise Errors::ResourceNotAvailable, e - end - - def register_project(project_id:, project_name:, project_namespace:, access_token:) - registration_input = { - project_id: project_id, - project_name: project_name, - project_namespace: project_namespace, - access_token: access_token - } - response = send_register_project(registration_input) - - { - project_id: response.project_id, - registered_at: response.registered_at - } - rescue GRPC::AlreadyExists => e - raise Errors::ProjectAlreadyExists, e - rescue GRPC::BadStatus => e - raise Errors::ResourceNotAvailable, e - end - - def deregister_project(project_id:) - deregistration_input = { - project_id: project_id - } - response = send_deregister_project(deregistration_input) - - { - project_id: response.project_id, - deregistered_at: response.deregistered_at - } - rescue GRPC::NotFound => e - raise Errors::ProjectNotFound, e - rescue GRPC::BadStatus => e - raise Errors::ResourceNotAvailable, e - end - - private - - attr_reader :rpc_url, :certs, :secret - - def read_secret! - secret = ENV[SECRET_NAME] - - raise Errors::ConfigurationError, "Variable #{SECRET_NAME} is missing" if secret.blank? - - if secret.length != SECRET_LENGTH - raise Errors::ConfigurationError, "Secret must contain #{SECRET_LENGTH} bytes" - end - - secret - end - - def get_reviewers(model_input) - request = MergeRequestRecommendationsReqV2.new(model_input) - client = Stub.new(rpc_url, credentials, timeout: DEFAULT_TIMEOUT) - client.merge_request_recommendations_v2(request) - end - - def send_register_project(registration_input) - request = RegisterProjectReq.new(registration_input) - client = Stub.new(rpc_url, credentials, timeout: DEFAULT_TIMEOUT) - client.register_project(request) - end - - def send_deregister_project(deregistration_input) - request = DeregisterProjectReq.new(deregistration_input) - client = Stub.new(rpc_url, credentials, timeout: DEFAULT_TIMEOUT) - client.deregister_project(request) - end - - def credentials - ssl_creds = GRPC::Core::ChannelCredentials.new(certs) - - auth_header = { authorization: "Bearer #{token}" } - auth_proc = proc { auth_header } - call_creds = GRPC::Core::CallCredentials.new(auth_proc) - - ssl_creds.compose(call_creds) - end - - def token - JSONWebToken::HMACToken.new(secret).tap do |token| - token.issuer = JWT_ISSUER - token.audience = JWT_AUDIENCE - end.encoded - end - end - end - end -end diff --git a/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_pb.rb b/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_pb.rb deleted file mode 100644 index aa6eb687253f8cc01a09117327a3a6c32a85e724..0000000000000000000000000000000000000000 --- a/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_pb.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module AppliedMl - module SuggestedReviewers - module RecommenderPb - Google::Protobuf::DescriptorPool.generated_pool.build do - add_file("bot/recommender.proto", syntax: :proto3) do - add_message "bot.MergeRequestRecommendationsReqV2" do - optional :merge_request_iid, :int64, 1 - optional :top_n, :int32, 2 - optional :project_id, :int64, 3 - repeated :changes, :string, 4 - optional :author_username, :string, 5 - end - add_message "bot.MergeRequestRecommendationsResV2" do - optional :version, :string, 1 - optional :top_n, :int32, 2 - repeated :reviewers, :string, 3 - end - add_message "bot.RegisterProjectReq" do - optional :project_id, :int64, 1 - optional :project_name, :string, 2 - optional :project_namespace, :string, 3 - optional :access_token, :string, 4 - end - add_message "bot.RegisterProjectRes" do - optional :project_id, :int64, 1 - optional :registered_at, :string, 2 - end - add_message "bot.DeregisterProjectReq" do - optional :project_id, :int64, 1 - end - add_message "bot.DeregisterProjectRes" do - optional :project_id, :int64, 1 - optional :deregistered_at, :string, 2 - end - end - rescue Google::Protobuf::TypeError - 'Log' - end - - # rubocop: disable Layout/LineLength - MergeRequestRecommendationsReqV2 = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.MergeRequestRecommendationsReqV2").msgclass - MergeRequestRecommendationsResV2 = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.MergeRequestRecommendationsResV2").msgclass - RegisterProjectReq = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.RegisterProjectReq").msgclass - RegisterProjectRes = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.RegisterProjectRes").msgclass - DeregisterProjectReq = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.DeregisterProjectReq").msgclass - DeregisterProjectRes = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("bot.DeregisterProjectRes").msgclass - # rubocop: enable Layout/LineLength - end - end - end -end diff --git a/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_services_pb.rb b/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_services_pb.rb deleted file mode 100644 index bd11467bb0a97d9549bd49d8ae8db0aa31a35b7d..0000000000000000000000000000000000000000 --- a/ee/lib/gitlab/applied_ml/suggested_reviewers/recommender_services_pb.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module AppliedMl - module SuggestedReviewers - module RecommenderServicesPb - class Service - include ::GRPC::GenericService - include Gitlab::AppliedMl::SuggestedReviewers::RecommenderPb - - self.marshal_class_method = :encode - self.unmarshal_class_method = :decode - self.service_name = 'bot.RecommenderService' - - rpc :MergeRequestRecommendationsV2, MergeRequestRecommendationsReqV2, MergeRequestRecommendationsResV2 - rpc :RegisterProject, RegisterProjectReq, RegisterProjectRes - rpc :DeregisterProject, DeregisterProjectReq, DeregisterProjectRes - end - - Stub = Service.rpc_stub_class - end - end - end -end diff --git a/ee/lib/tasks/contracts/merge_requests.rake b/ee/lib/tasks/contracts/merge_requests.rake deleted file mode 100644 index 60fb7b51fce73ea3f43761c9ebd5f4ea9e1b4164..0000000000000000000000000000000000000000 --- a/ee/lib/tasks/contracts/merge_requests.rake +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -return if Rails.env.production? - -require 'pact/tasks/verification_task' - -provider = File.expand_path('../../../spec/contracts/provider', __dir__) - -namespace :contracts do - require_relative "../../../../spec/contracts/provider/helpers/contract_source_helper" - - namespace :merge_requests do - Pact::VerificationTask.new(:get_suggested_reviewers) do |pact| - pact_helper_location = 'pact_helpers/project/merge_requests/show/get_suggested_reviewers_helper.rb' - - pact.uri( - Provider::ContractSourceHelper.contract_location( - requester: :rake, - file_path: pact_helper_location, - edition: :ee - ), - pact_helper: "#{provider}/#{pact_helper_location}" - ) - end - - desc 'Run all merge request contract tests' - task 'test:merge_requests', :contract_merge_requests do |_t, arg| - errors = %w[get_suggested_reviewers].each_with_object([]) do |task, err| - Rake::Task["contracts:merge_requests:pact:verify:#{task}"].execute - rescue StandardError, SystemExit - err << "contracts:merge_requests:pact:verify:#{task}" - end - - raise StandardError, "Errors in tasks #{errors.join(', ')}" unless errors.empty? - end - end -end diff --git a/ee/spec/contracts/.gitignore b/ee/spec/contracts/.gitignore deleted file mode 100644 index cb89d4102d384ea13d3a1a176379a0e796a9fd1d..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -logs/ -consumer/node_modules diff --git a/ee/spec/contracts/consumer/fixtures/project/merge_requests/suggested_reviewers.fixture.js b/ee/spec/contracts/consumer/fixtures/project/merge_requests/suggested_reviewers.fixture.js deleted file mode 100644 index 06841998c97441e4aebddfe175eb58fb47855700..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/consumer/fixtures/project/merge_requests/suggested_reviewers.fixture.js +++ /dev/null @@ -1,56 +0,0 @@ -import { Matchers } from '@pact-foundation/pact'; -import { - AUTOCOMPLETE_USERS_URL, - TEST_PROJECT_ID, - TEST_MERGE_REQUEST_IID, -} from '../../../test_constants'; - -const userIdMatchExample1 = 6954442; -const userIdMatchExample2 = 6954441; - -const body = [ - { - id: Matchers.integer(userIdMatchExample1), - username: Matchers.string('user1'), - name: Matchers.string('A User'), - }, - { - id: Matchers.integer(userIdMatchExample2), - username: Matchers.string('gitlab-qa'), - name: Matchers.string('Contract Test User'), - suggested: Matchers.boolean(true), - }, -]; - -export const suggestedReviewersFixture = { - body: Matchers.extractPayload(body), - - success: { - status: 200, - headers: { - 'Content-Type': 'application/json; charset=utf-8', - }, - body, - }, - - scenario: { - state: 'a merge request exists with suggested reviewers available for selection', - uponReceiving: 'a request for suggested reviewers', - }, - - request: { - withRequest: { - method: 'GET', - path: AUTOCOMPLETE_USERS_URL, - query: { - active: 'true', - project_id: Matchers.string(TEST_PROJECT_ID), - merge_request_iid: Matchers.string(TEST_MERGE_REQUEST_IID), - current_user: 'true', - }, - headers: { - Accept: '*/*', - }, - }, - }, -}; diff --git a/ee/spec/contracts/consumer/resources/api/project/autocomplete_users.js b/ee/spec/contracts/consumer/resources/api/project/autocomplete_users.js deleted file mode 100644 index 875e39bba5dec33492d40d8a0ddac24efbfc1410..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/consumer/resources/api/project/autocomplete_users.js +++ /dev/null @@ -1,24 +0,0 @@ -import axios from 'axios'; - -import { - AUTOCOMPLETE_USERS_URL, - TEST_PROJECT_ID, - TEST_MERGE_REQUEST_IID, -} from '../../../test_constants'; - -export async function getSuggestedReviewers(endpoint) { - const { url } = endpoint; - - return axios({ - method: 'GET', - baseURL: url, - url: AUTOCOMPLETE_USERS_URL, - headers: { Accept: '*/*' }, - params: { - active: 'true', - project_id: TEST_PROJECT_ID, - merge_request_iid: TEST_MERGE_REQUEST_IID, - current_user: 'true', - }, - }).then((response) => response.data); -} diff --git a/ee/spec/contracts/consumer/specs/project/merge_requests/show.spec.js b/ee/spec/contracts/consumer/specs/project/merge_requests/show.spec.js deleted file mode 100644 index cefed6020e5c1a7f6c22f17bf2e7d5e136fd85cb..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/consumer/specs/project/merge_requests/show.spec.js +++ /dev/null @@ -1,41 +0,0 @@ -import path from 'path'; -import { pactWith } from 'jest-pact'; -import { suggestedReviewersFixture } from '../../../fixtures/project/merge_requests/suggested_reviewers.fixture'; -import { getSuggestedReviewers } from '../../../resources/api/project/autocomplete_users'; - -const ROOT_PATH = path.resolve(__dirname, '../../..'); -const CONSUMER_NAME = 'MergeRequests#show'; -const CONSUMER_LOG = path.join(ROOT_PATH, '../logs/consumer.log'); -const CONTRACT_DIR = path.join(ROOT_PATH, '../contracts/project/merge_requests/show'); -const SUGGESTED_REVIEWERS_PROVIDER_NAME = 'GET suggested reviewers'; - -// API endpoint: /autocomplete/users.json -pactWith( - { - consumer: CONSUMER_NAME, - provider: SUGGESTED_REVIEWERS_PROVIDER_NAME, - log: CONSUMER_LOG, - dir: CONTRACT_DIR, - }, - - (provider) => { - describe(SUGGESTED_REVIEWERS_PROVIDER_NAME, () => { - beforeEach(() => { - const interaction = { - ...suggestedReviewersFixture.scenario, - ...suggestedReviewersFixture.request, - willRespondWith: suggestedReviewersFixture.success, - }; - provider.addInteraction(interaction); - }); - - it('return a successful body', async () => { - const suggestedReviewers = await getSuggestedReviewers({ - url: provider.mockService.baseUrl, - }); - - expect(suggestedReviewers).toEqual(suggestedReviewersFixture.body); - }); - }); - }, -); diff --git a/ee/spec/contracts/consumer/test_constants.js b/ee/spec/contracts/consumer/test_constants.js deleted file mode 100644 index c1db52945baffbba40704ef0fe232f1dc44109c5..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/consumer/test_constants.js +++ /dev/null @@ -1,3 +0,0 @@ -export const AUTOCOMPLETE_USERS_URL = '/-/autocomplete/users.json'; -export const TEST_PROJECT_ID = '12345'; -export const TEST_MERGE_REQUEST_IID = '54321'; diff --git a/ee/spec/contracts/contracts/project/merge_requests/show/mergerequests#show-get_suggested_reviewers.json b/ee/spec/contracts/contracts/project/merge_requests/show/mergerequests#show-get_suggested_reviewers.json deleted file mode 100644 index 50ff43b3cf76bff96ead840a86ebe12e307f0578..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/contracts/project/merge_requests/show/mergerequests#show-get_suggested_reviewers.json +++ /dev/null @@ -1,77 +0,0 @@ -{ - "consumer": { - "name": "MergeRequests#show" - }, - "provider": { - "name": "GET suggested reviewers" - }, - "interactions": [ - { - "description": "a request for suggested reviewers", - "providerState": "a merge request exists with suggested reviewers available for selection", - "request": { - "method": "GET", - "path": "/-/autocomplete/users.json", - "query": "active=true&project_id=12345&merge_request_iid=54321¤t_user=true", - "headers": { - "Accept": "*/*" - }, - "matchingRules": { - "$.query.project_id[0]": { - "match": "type" - }, - "$.query.merge_request_iid[0]": { - "match": "type" - } - } - }, - "response": { - "status": 200, - "headers": { - "Content-Type": "application/json; charset=utf-8" - }, - "body": [ - { - "id": 6954442, - "username": "user1", - "name": "A User" - }, - { - "id": 6954441, - "username": "gitlab-qa", - "name": "Contract Test User", - "suggested": true - } - ], - "matchingRules": { - "$.body[0].id": { - "match": "type" - }, - "$.body[0].username": { - "match": "type" - }, - "$.body[0].name": { - "match": "type" - }, - "$.body[1].id": { - "match": "type" - }, - "$.body[1].username": { - "match": "type" - }, - "$.body[1].name": { - "match": "type" - }, - "$.body[1].suggested": { - "match": "type" - } - } - } - } - ], - "metadata": { - "pactSpecification": { - "version": "2.0.0" - } - } -} \ No newline at end of file diff --git a/ee/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_suggested_reviewers_helper.rb b/ee/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_suggested_reviewers_helper.rb deleted file mode 100644 index 51392b31c3bb1cd279010ce23d3593b965e25fb8..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/provider/pact_helpers/project/merge_requests/show/get_suggested_reviewers_helper.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_relative '../../../../../../../../spec/contracts/provider/helpers/contract_source_helper' -require_relative '../../../../states/project/merge_requests/show_state' -require_relative '../../../../../../../../spec/contracts/provider/spec_helper' -require_relative '../../../../../../../../spec/contracts/provider/environments/test' - -module Provider - module SuggestedReviewersHelper - Pact.service_provider "GET suggested reviewers" do - app { Environments::Test.app } - - honours_pact_with 'MergeRequests#show' do - pact_uri Provider::ContractSourceHelper.contract_location(requester: :spec, file_path: __FILE__, edition: :ee) - end - end - end -end diff --git a/ee/spec/contracts/provider/spec_helper.rb b/ee/spec/contracts/provider/spec_helper.rb deleted file mode 100644 index ea703683987c6be6d4e033681459653fc26a1e22..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/provider/spec_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require Rails.root.join("config/initializers/0_inject_enterprise_edition_module.rb") -require Rails.root.join("ee/spec/support/helpers/ee/license_helpers.rb") -require Rails.root.join("spec/support/helpers/license_helper.rb") - -Pact.configure do |config| - config.include LicenseHelpers -end diff --git a/ee/spec/contracts/provider/states/project/merge_requests/show_state.rb b/ee/spec/contracts/provider/states/project/merge_requests/show_state.rb deleted file mode 100644 index f70cf87ed4d0c8db0685d2b06bb1357a92249cbb..0000000000000000000000000000000000000000 --- a/ee/spec/contracts/provider/states/project/merge_requests/show_state.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -Pact.provider_states_for "MergeRequests#show" do - provider_state "a merge request exists with suggested reviewers available for selection" do - set_up do - # Suggested Reviewers is a SaaS feature, but we can't use the `:saas` RSpec metadata like we do in other specs - allow(::Gitlab).to receive(:com?).and_return(true) - - stub_licensed_features(suggested_reviewers: true) - - user = User.find_by(name: Provider::UsersHelper::CONTRACT_USER_NAME) - namespace = create(:namespace, name: 'gitlab-org') - project = create(:project, id: 12345, name: 'gitlab-qa', namespace: namespace) - project.add_maintainer(user) - project.project_setting.update!(suggested_reviewers_enabled: true) - merge_request = create(:merge_request, iid: 54321, source_project: project, author: user) - - merge_request.build_predictions - merge_request.predictions.update!(suggested_reviewers: { reviewers: [user.username] }) - end - end -end diff --git a/ee/spec/controllers/autocomplete_controller_spec.rb b/ee/spec/controllers/autocomplete_controller_spec.rb index 5daa6bc8e2518611eea80578e73275286476465f..91e3d6d47146b82f1a41042da3b028647936ced4 100644 --- a/ee/spec/controllers/autocomplete_controller_spec.rb +++ b/ee/spec/controllers/autocomplete_controller_spec.rb @@ -6,100 +6,6 @@ let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, namespace: user.namespace) } - context 'GET users', feature_category: :user_profile do - let_it_be(:user2) { create(:user) } - let_it_be(:non_member) { create(:user) } - - context 'project members' do - before do - project.add_developer(user2) - sign_in(user) - end - - describe 'GET #users with suggested users' do - let_it_be(:suggested_user) { create(:user) } - let_it_be(:merge_request) { create(:merge_request, source_project: project) } - - let(:request_common_params) do - { - active: 'true', - project_id: project.id, - merge_request_iid: merge_request.iid, - current_user: true - } - end - - let(:request_params) { request_common_params } - - before do - project.add_developer(suggested_user) - merge_request.build_predictions - merge_request.predictions.update!(suggested_reviewers: { reviewers: [suggested_user.username] }) - - allow(controller).to receive(:suggested_reviewers_available?).and_return(true) - end - - shared_examples 'feature available' do - it 'returns the suggested reviewers' do - get(:users, params: request_params) - - expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq(3) - expect(json_response.map { |user| user['suggested'] }).to match_array([nil, nil, true]) - end - end - - shared_examples 'feature unavailable' do - it 'returns no suggested reviewers' do - get(:users, params: request_params) - - expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq(3) - expect(json_response.map { |user| user['suggested'] }).to match_array([nil, nil, nil]) - end - end - - include_examples 'feature available' - - context 'when suggested reviewers is unavailable for project' do - before do - allow(controller).to receive(:suggested_reviewers_available?).and_return(false) - end - - include_examples 'feature unavailable' - end - - context 'when search param is not blank' do - let(:request_params) { request_common_params.merge(search: suggested_user.username) } - - it 'returns no suggested reviewers' do - get(:users, params: request_params) - - expect(json_response.map { |user| user['suggested'] }).to match_array([nil]) - end - end - - context 'when merge_request_iid is blank' do - let(:request_params) { request_common_params.except(:merge_request_iid) } - - include_examples 'feature unavailable' - end - - context 'when merge_request is closed' do - let_it_be(:merge_request) { create(:merge_request, :closed, source_project: project) } - - include_examples 'feature unavailable' - end - - context 'when merge_request has been merged' do - let_it_be(:merge_request) { create(:merge_request, :merged, source_project: project) } - - include_examples 'feature unavailable' - end - end - end - end - context 'groups', feature_category: :groups_and_projects do before do sign_in(user) diff --git a/ee/spec/controllers/projects/settings/merge_requests_controller_spec.rb b/ee/spec/controllers/projects/settings/merge_requests_controller_spec.rb index bf5ccf695a318f6ca9b7c6f6e9fb9ed3c64ed3e8..e0fe73814aa96ba0724f346d24526885fa603fd7 100644 --- a/ee/spec/controllers/projects/settings/merge_requests_controller_spec.rb +++ b/ee/spec/controllers/projects/settings/merge_requests_controller_spec.rb @@ -110,21 +110,6 @@ end end - context 'when suggested_reviewers_enabled param is specified' do - let(:params) { { project_setting_attributes: { suggested_reviewers_enabled: '1' } } } - - let(:request) do - put :update, params: { namespace_id: project.namespace, project_id: project, project: params } - end - - it 'updates the attribute' do - allow_any_instance_of(Project).to receive(:suggested_reviewers_available?).and_return(true) # rubocop:disable RSpec/AnyInstanceOf - - request - expect(project.reload.suggested_reviewers_enabled).to be(true) - end - end - context 'when auto_duo_code_review_enabled param is specified' do let(:params) { { project_setting_attributes: { auto_duo_code_review_enabled: '1' } } } diff --git a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb index 772168ebcc85c9f423238b206d135f16f2196c58..af8d51d267745195de10fb93ad6dbf94dc066746 100644 --- a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb +++ b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb @@ -62,44 +62,4 @@ end end end - - context 'suggested reviewers', :js, :saas do - let_it_be(:suggested_user) { create(:user) } - - before do - stub_licensed_features(suggested_reviewers: true) - stub_feature_flags(hide_suggested_reviewers: false) - - target_project.project_setting.update!(suggested_reviewers_enabled: true) - - merge_request.project.add_developer(suggested_user) - merge_request.build_predictions - merge_request.predictions.update!(suggested_reviewers: { reviewers: [suggested_user.username] }) - end - - it 'displays suggested reviewers in reviewer dropdown', :aggregate_failures do - find('.js-reviewer-search').click - wait_for_requests - - help_page_path = help_page_path('user/project/merge_requests/reviews/_index.md', anchor: 'request-a-review') - - page.within '.dropdown-menu-reviewer' do - expect(page).to have_content('Suggestion(s)') - expect(page).to have_link(title: 'Learn about suggested reviewers', href: %r{#{help_page_path}}) - expect(page).to have_content(suggested_user.name) - expect(page).to have_content(suggested_user.username) - expect(page).to have_css("[data-user-suggested='true']") - end - end - - it 'removes headers in reviewer dropdown' do - find('.js-reviewer-search').click - wait_for_requests - - page.within '.dropdown-menu-reviewer' do - click_on suggested_user.name - expect(page).not_to have_content('Suggestion(s)') - end - end - end end diff --git a/ee/spec/graphql/ee/types/merge_request_type_spec.rb b/ee/spec/graphql/ee/types/merge_request_type_spec.rb index 06161f59ad0409bb66758fabd614451c2f7d280c..c093ae1e9e2ea5c0b0e582031b79c4bc8a41c20a 100644 --- a/ee/spec/graphql/ee/types/merge_request_type_spec.rb +++ b/ee/spec/graphql/ee/types/merge_request_type_spec.rb @@ -16,7 +16,6 @@ it { expect(described_class).to have_graphql_field(:approvals_left, complexity: 2, calls_gitaly?: true) } it { expect(described_class).to have_graphql_field(:has_security_reports, calls_gitaly?: true) } it { expect(described_class).to have_graphql_field(:security_reports_up_to_date_on_target_branch, calls_gitaly?: true) } - it { expect(described_class).to have_graphql_field(:suggested_reviewers) } it { expect(described_class).to have_graphql_field(:blocking_merge_requests) } it { expect(described_class).to have_graphql_field(:merge_request_diffs) } it { expect(described_class).to have_graphql_field(:policy_violations) } diff --git a/ee/spec/graphql/types/applied_ml/suggested_reviewers_type_spec.rb b/ee/spec/graphql/types/applied_ml/suggested_reviewers_type_spec.rb deleted file mode 100644 index 153e0ad657707769ec3577777538b78902732fe1..0000000000000000000000000000000000000000 --- a/ee/spec/graphql/types/applied_ml/suggested_reviewers_type_spec.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe GitlabSchema.types['SuggestedReviewersType'], feature_category: :code_review_workflow do - include GraphqlHelpers - - let(:fields) { %i[accepted created_at suggested updated_at] } - - it { expect(described_class).to have_graphql_fields(fields) } - - describe 'field values' do - let_it_be(:predictions) do - create( - :predictions, - accepted_reviewers: { reviewers: %w[bmarley] }, - suggested_reviewers: { reviewers: %w[bmarley swayne] } - ) - end - - let_it_be(:user) { build(:user) } - - subject { resolve_field(field_name, predictions, current_user: user) } - - describe 'accepted' do - let(:field_name) { :accepted } - - it { is_expected.to eq(['bmarley']) } - end - - describe 'suggested' do - let(:field_name) { :suggested } - - it { is_expected.to eq(%w[bmarley swayne]) } - end - end -end diff --git a/ee/spec/initializers/gitlab_suggested_reviewers_secret_spec.rb b/ee/spec/initializers/gitlab_suggested_reviewers_secret_spec.rb deleted file mode 100644 index 2bac226e8ff8db22db26696b1eca842f386ba879..0000000000000000000000000000000000000000 --- a/ee/spec/initializers/gitlab_suggested_reviewers_secret_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Suggested Reviewers secret initialization for GitLab EE', feature_category: :code_review_workflow do - subject { Gitlab::Application.config } - - let(:load_suggested_reviewers_secret) do - load Rails.root.join('ee/config/initializers/gitlab_suggested_reviewers_secret.rb') - end - - context 'when not SAAS' do - it 'does not load secret' do - expect(Gitlab::AppliedMl::SuggestedReviewers).not_to receive(:ensure_secret!) - - load_suggested_reviewers_secret - end - end - - context 'when SAAS', :saas do - it 'loads secret' do - expect(Gitlab::AppliedMl::SuggestedReviewers).to receive(:ensure_secret!) - - load_suggested_reviewers_secret - end - end -end diff --git a/ee/spec/lib/ee/users/internal_spec.rb b/ee/spec/lib/ee/users/internal_spec.rb deleted file mode 100644 index 026d76fef9fb996c5d1ac4a176576df1e7600b7a..0000000000000000000000000000000000000000 --- a/ee/spec/lib/ee/users/internal_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Users::Internal, feature_category: :user_profile do - let_it_be(:first_organization) { create(:organization) } - - shared_examples 'bot users' do |bot_type| - it 'creates the user if it does not exist' do - expect do - described_class.public_send(bot_type) - end.to change { User.where(user_type: bot_type).count }.by(1) - end - - it 'creates a route for the namespace of the created user' do - bot_user = described_class.public_send(bot_type) - - expect(bot_user.namespace.route).to be_present - end - - it 'does not create a new user if it already exists' do - described_class.public_send(bot_type) - - expect do - described_class.public_send(bot_type) - end.not_to change { User.count } - end - end - - it_behaves_like 'bot users', :suggested_reviewers_bot -end diff --git a/ee/spec/lib/gitlab/applied_ml/suggested_reviewers/client_spec.rb b/ee/spec/lib/gitlab/applied_ml/suggested_reviewers/client_spec.rb deleted file mode 100644 index 55aa9379f4a060518098dec090acfffd4446fc8b..0000000000000000000000000000000000000000 --- a/ee/spec/lib/gitlab/applied_ml/suggested_reviewers/client_spec.rb +++ /dev/null @@ -1,357 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::AppliedMl::SuggestedReviewers::Client, feature_category: :code_review_workflow do - include AfterNextHelpers - - let(:stub_class) { Gitlab::AppliedMl::SuggestedReviewers::RecommenderServicesPb::Stub } - - let(:rpc_url) { 'example.org:1234' } - let(:certs) { 'arandomstring' } - let(:secret) { SecureRandom.hex(32) } - let(:client_arguments) { {} } - - let(:client) { described_class.new(**client_arguments) } - - shared_examples 'respecting channel credentials' do - it 'uses a ChannelCredentials object' do - allow(GRPC::Core::ChannelCredentials).to receive(:new).and_call_original - - subject - - expect(stub_class).to have_received(:new) - .with( - rpc_url, - instance_of(GRPC::Core::ChannelCredentials), - timeout: described_class::DEFAULT_TIMEOUT - ) - end - - it 'uses a CallCredentials object' do - allow(GRPC::Core::CallCredentials).to receive(:new).and_call_original - - subject - - expect(GRPC::Core::CallCredentials).to have_received(:new).with(instance_of(Proc)) - end - - it 'creates a JWT HMAC token', :aggregate_failures do - token = instance_spy(JSONWebToken::HMACToken, encoded: 'test-token') - allow(JSONWebToken::HMACToken).to receive(:new).with(secret).and_return(token) - - subject - - expect(token).to have_received(:issuer=).with(described_class::JWT_ISSUER) - expect(token).to have_received(:audience=).with(described_class::JWT_AUDIENCE) - expect(token).to have_received(:encoded) - end - end - - shared_examples 'respecting environment configuration' do - it 'uses a development URL' do - allow(Gitlab).to receive(:dev_or_test_env?).and_return(true) - - subject - - expect(stub_class).to have_received(:new).with('suggested-reviewer.dev:443', any_args) - end - - it 'uses a production URL' do - allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) - - subject - - expect(stub_class).to have_received(:new).with('api.unreview.io:443', any_args) - end - - context 'with an invalid gRPC URL configured' do - let(:client_arguments) { { rpc_url: '' } } - - it 'raises a configuration error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ConfigurationError, 'gRPC host unknown') - end - end - - context 'with no secret configured' do - let(:secret) { nil } - - it 'raises a configuration error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ConfigurationError, - 'Variable SUGGESTED_REVIEWERS_SECRET is missing') - end - end - - context 'with an invalid secret configured' do - let(:secret) { '@s3cr3tunt0ld' } - - it 'raises a configuration error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ConfigurationError, 'Secret must contain 64 bytes') - end - end - end - - describe '#suggested_reviewers' do - let(:response_class) { Gitlab::AppliedMl::SuggestedReviewers::RecommenderPb::MergeRequestRecommendationsResV2 } - - let(:suggested_reviewers_request) do - { - project_id: 42, - merge_request_iid: 7, - top_n: 5, - changes: ['db', 'ee/db'], - author_username: 'joe' - } - end - - let(:suggested_reviewers_response) do - response_class.new( - { - version: "0.7.1", - top_n: 4, - reviewers: %w[john jane] - } - ) - end - - subject do - client.suggested_reviewers(**suggested_reviewers_request) - end - - before do - stub_env('SUGGESTED_REVIEWERS_SECRET', secret) - end - - context 'when configuration and input is healthy' do - let(:client_arguments) { { rpc_url: rpc_url, certs: certs } } - let(:suggested_reviewers_result) do - { - version: "0.7.1", - top_n: 4, - reviewers: %w[john jane] - } - end - - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:merge_request_recommendations_v2).and_return(suggested_reviewers_response) - end - end - - it { is_expected.to eq(suggested_reviewers_result) } - - it_behaves_like 'respecting channel credentials' - end - - context 'when a grpc connection error is received' do - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:merge_request_recommendations_v2).and_raise(GRPC::Unavailable) - end - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ConnectionFailed) - end - end - - context 'when a grpc bad status is received' do - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:merge_request_recommendations_v2).and_raise(GRPC::Internal) - end - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - end - end - - context 'with no changes' do - let(:suggested_reviewers_request) do - { - project_id: 42, - merge_request_iid: 7, - top_n: 5, - changes: [], - author_username: 'joe' - } - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ArgumentError) - end - end - - describe 'gRPC configuration' do - let(:client_arguments) { {} } - - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:merge_request_recommendations_v2).and_return(suggested_reviewers_response) - end - end - - it_behaves_like 'respecting environment configuration' - end - end - - describe '#register_project' do - let(:response_class) { Gitlab::AppliedMl::SuggestedReviewers::RecommenderPb::RegisterProjectRes } - let(:register_project_request) do - { - project_id: 42, - project_name: 'foo', - project_namespace: 'bar/zoo', - access_token: 'secret' - } - end - - let(:register_project_response) do - response_class.new( - { - project_id: 42, - registered_at: '2022-01-01 20:22' - } - ) - end - - subject do - client.register_project(**register_project_request) - end - - before do - stub_env('SUGGESTED_REVIEWERS_SECRET', secret) - end - - context 'when configuration and input is healthy' do - let(:client_arguments) { { rpc_url: rpc_url, certs: certs } } - let(:register_project_result) do - { - project_id: 42, - registered_at: '2022-01-01 20:22' - } - end - - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:register_project).and_return(register_project_response) - end - end - - it { is_expected.to eq(register_project_result) } - - it_behaves_like 'respecting channel credentials' - end - - context 'when a grpc already exists is received' do - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:register_project).and_raise(GRPC::AlreadyExists) - end - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ProjectAlreadyExists) - end - end - - context 'when a grpc bad status is received' do - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:register_project).and_raise(GRPC::Unavailable) - end - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - end - end - - describe 'with gRPC configuration' do - let(:client_arguments) { {} } - - before do - allow_next_instance_of(stub_class) do |stub| - allow(stub).to receive(:register_project).and_return(register_project_response) - end - end - - it_behaves_like 'respecting environment configuration' - end - end - - describe '#deregister_project' do - let(:response_class) { Gitlab::AppliedMl::SuggestedReviewers::RecommenderPb::DeregisterProjectRes } - let(:deregister_project_request) do - { - project_id: 42 - } - end - - let(:deregister_project_response) do - response_class.new( - { - project_id: 42, - deregistered_at: '2022-01-01 20:22' - } - ) - end - - subject do - client.deregister_project(**deregister_project_request) - end - - before do - stub_env('SUGGESTED_REVIEWERS_SECRET', secret) - end - - context 'when configuration and input is healthy' do - let(:client_arguments) { { rpc_url: rpc_url, certs: certs } } - let(:deregister_project_result) do - { - project_id: 42, - deregistered_at: '2022-01-01 20:22' - } - end - - before do - allow_next(stub_class).to receive(:deregister_project).and_return(deregister_project_response) - end - - it { is_expected.to eq(deregister_project_result) } - - it_behaves_like 'respecting channel credentials' - end - - context 'when a grpc not found is received' do - before do - allow_next(stub_class).to receive(:deregister_project).and_raise(GRPC::NotFound) - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ProjectNotFound) - end - end - - context 'when a grpc bad status is received' do - before do - allow_next(stub_class).to receive(:deregister_project).and_raise(GRPC::Unavailable) - end - - it 'raises a new error' do - expect { subject }.to raise_error(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - end - end - - describe 'with gRPC configuration' do - let(:client_arguments) { {} } - - before do - allow_next(stub_class).to receive(:deregister_project).and_return(deregister_project_response) - end - - it_behaves_like 'respecting environment configuration' - end - end -end diff --git a/ee/spec/lib/gitlab/applied_ml/suggested_reviewers_spec.rb b/ee/spec/lib/gitlab/applied_ml/suggested_reviewers_spec.rb deleted file mode 100644 index 1b9faf86c91ff22ea23e69ce7575b52572342495..0000000000000000000000000000000000000000 --- a/ee/spec/lib/gitlab/applied_ml/suggested_reviewers_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::AppliedMl::SuggestedReviewers, feature_category: :code_review_workflow do - let(:secret) { SecureRandom.random_bytes(described_class::SECRET_LENGTH) } - - before do - allow(described_class).to receive(:secret).and_return(secret) - end - - describe '.verify_api_request' do - let(:iat) { 1.minute.ago.to_i } - let(:payload) { { 'iss' => described_class::JWT_ISSUER, 'iat' => iat } } - - subject(:decoded_token) do - described_class.verify_api_request(headers) - end - - context 'when header is not set' do - let(:headers) { {} } - - it { is_expected.to be_nil } - end - - context 'when token is encoded with a wrong secret' do - let(:headers) do - encoded_token = JWT.encode(payload, 'wrongsecret', 'HS256') - { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token } - end - - it { is_expected.to be_nil } - end - - context 'when header is included a token encoded with a correct secret' do - let(:headers) do - encoded_token = JWT.encode(payload, secret, 'HS256') - { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token } - end - - it { is_expected.to match_array([{ 'iss' => described_class::JWT_ISSUER, 'iat' => iat }, { 'alg' => 'HS256' }]) } - end - end - - describe '.secret_path' do - it 'returns default gitlab config' do - expect(described_class.secret_path).to eq(Gitlab.config.suggested_reviewers.secret_file) - end - end - - describe '.ensure_secret!' do - context 'when secret file exists' do - before do - allow(File).to receive(:exist?).with(Gitlab.config.suggested_reviewers.secret_file).and_return(true) - end - - it 'does not call write_secret' do - expect(described_class).not_to receive(:write_secret) - - described_class.ensure_secret! - end - end - - context 'when secret file does not exist' do - before do - allow(File).to receive(:exist?).with(Gitlab.config.suggested_reviewers.secret_file).and_return(false) - end - - it 'calls write_secret' do - expect(described_class).to receive(:write_secret) - - described_class.ensure_secret! - end - end - end -end diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index fd60ba26e7c8aa55e9ce0345ba651232fe1a21eb..ab324077f76fc3b56e8cf53537fd85bf63730d06 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -4390,87 +4390,6 @@ def stub_default_url_options(host) end end - describe '#suggested_reviewers_available?' do - subject { project.suggested_reviewers_available? } - - context 'on Gitlab.com', :saas do - context 'when licensed features are available', :saas do - before do - stub_licensed_features(suggested_reviewers: true) - end - - it { is_expected.to eq false } - end - - context 'when licensed features are unavailable', :saas do - before do - stub_licensed_features(suggested_reviewers: false) - end - - it { is_expected.to eq false } - end - - context 'when hide_suggested_reviewers feature flag is disabled', :saas do - before do - stub_licensed_features(suggested_reviewers: true) - stub_feature_flags(hide_suggested_reviewers: false) - end - - it { is_expected.to eq true } - end - end - - context 'on self managed' do - context 'when licensed features are available' do - before do - stub_licensed_features(suggested_reviewers: true) - end - - it { is_expected.to eq false } - end - end - end - - describe '#can_suggest_reviewers?' do - subject { project.can_suggest_reviewers? } - - context 'when available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when enabled' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(true) - end - - it { is_expected.to eq true } - end - - context 'when not enabled' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(false) - end - - it { is_expected.to eq false } - end - end - - context 'when not available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - context 'when enabled' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(true) - end - - it { is_expected.to eq false } - end - end - end - describe '#any_external_status_checks_not_passed?' do let(:protected_branch) { create(:protected_branch, name: 'main', project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project, target_branch: protected_branch.name) } diff --git a/ee/spec/models/merge_request/suggested_reviewers_merge_request_spec.rb b/ee/spec/models/merge_request/suggested_reviewers_merge_request_spec.rb deleted file mode 100644 index 22c746681746d6831829d9d9c456341841052204..0000000000000000000000000000000000000000 --- a/ee/spec/models/merge_request/suggested_reviewers_merge_request_spec.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequest, feature_category: :code_review_workflow do - let_it_be(:project) { build(:project) } - - subject(:merge_request) { build(:merge_request, project: project) } - - describe '#can_suggest_reviewers?' do - subject { merge_request.can_suggest_reviewers? } - - before do - allow(merge_request).to receive(:modified_paths).and_return(['foo/bar.txt']) - end - - context 'when open' do - before do - allow(merge_request).to receive(:open?).and_return(true) - end - - it { is_expected.to be(true) } - - context 'when modified_paths is empty' do - before do - allow(merge_request).to receive(:modified_paths).and_return([]) - end - - it { is_expected.to be(false) } - end - end - - context 'when not open' do - before do - allow(merge_request).to receive(:open?).and_return(false) - end - - it { is_expected.to be(false) } - end - end - - describe '#suggested_reviewer_users' do - subject(:suggested_reviewer_users) { merge_request.suggested_reviewer_users } - - shared_examples 'blank suggestions' do - it 'returns an empty relation' do - expect(suggested_reviewer_users).to be_empty - end - end - - context 'when predictions is nil' do - it_behaves_like 'blank suggestions' - end - - context 'when predictions is not nil' do - before do - merge_request.build_predictions - end - - context 'when predictions is a non hash' do - before do - merge_request.build_predictions - merge_request.predictions.suggested_reviewers = 1 - end - - it_behaves_like 'blank suggestions' - end - - context 'when predictions is an empty hash' do - before do - merge_request.predictions.suggested_reviewers = {} - end - - it_behaves_like 'blank suggestions' - end - - context 'when suggests a user who is not a member' do - let_it_be(:non_member) { create(:user) } - - before do - merge_request.predictions.suggested_reviewers = { 'reviewers' => [non_member.username] } - end - - it_behaves_like 'blank suggestions' - end - - context 'when suggests users who are members' do - let_it_be(:first_member) { create(:user) } - let_it_be(:second_member) { create(:user) } - let_it_be(:bot_member) { create(:user, :project_bot) } - let_it_be(:service_member) { create(:user, :service_user) } - - before_all do - project.add_developer(first_member) - project.add_developer(second_member) - project.add_reporter(bot_member) - project.add_reporter(service_member) - end - - before do - merge_request.predictions.suggested_reviewers = { - 'reviewers' => [ - second_member.username, - first_member.username, - bot_member.username, - service_member.username - ] - } - end - - context 'when a user is inactive' do - before do - second_member.deactivate - end - - it 'returns only active human users' do - expect(suggested_reviewer_users).to eq([first_member]) - end - end - - context 'when all users are active' do - it 'returns human users in correct suggested order' do - expect(suggested_reviewer_users).to eq([second_member, first_member]) - end - end - end - end - end -end diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 4d756d73ee5f08328cef00455d3a7351939cbfe1..972bdd106d3aa446c86c9a6a26bcea3500f480bd 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3325,57 +3325,6 @@ def create_member_role(member, abilities = member_role_abilities) end end - describe 'permissions for suggested reviewers bot', :saas do - let(:permissions) { [:admin_project_member, :create_resource_access_tokens] } - let(:namespace) { build_stubbed(:namespace) } - let(:project) { build_stubbed(:project, namespace: namespace) } - - context 'when user is suggested_reviewers_bot' do - let(:current_user) { Users::Internal.suggested_reviewers_bot } - - where(:suggested_reviewers_available, :token_creation_allowed, :allowed) do - false | false | false - false | true | false - true | false | false - true | true | true - end - - with_them do - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(suggested_reviewers_available) - - allow(::Gitlab::CurrentSettings) - .to receive(:personal_access_tokens_disabled?) - .and_return(!token_creation_allowed) - end - - it 'always allows permissions except when feature disabled' do - if allowed - expect_allowed(*permissions) - else - expect_disallowed(*permissions) - end - end - end - end - - context 'when user is not suggested_reviewers_bot' do - let(:current_user) { developer } - - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(true) - - allow(::Gitlab::CurrentSettings) - .to receive(:personal_access_tokens_disabled?) - .and_return(false) - end - - it 'does not allow permissions' do - expect_disallowed(*permissions) - end - end - end - describe 'read_project_runners' do context 'with auditor' do let(:current_user) { auditor } diff --git a/ee/spec/presenters/applied_ml/suggested_reviewers_presenter_spec.rb b/ee/spec/presenters/applied_ml/suggested_reviewers_presenter_spec.rb deleted file mode 100644 index 072a8c00bf9b400b906e6b75fa3a1928f02dc3ac..0000000000000000000000000000000000000000 --- a/ee/spec/presenters/applied_ml/suggested_reviewers_presenter_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe AppliedMl::SuggestedReviewersPresenter, feature_category: :code_review_workflow do - let(:presenter) { described_class.new(predictions) } - - let(:predictions) do - build( - :predictions, - accepted_reviewers: { reviewers: %w[bmarley] }, - suggested_reviewers: { reviewers: %w[bmarley swayne] } - ) - end - - describe '#accepted' do - subject { presenter.accepted } - - it { is_expected.to eq(['bmarley']) } - end - - describe '#suggested' do - subject { presenter.suggested } - - it { is_expected.to eq(%w[bmarley swayne]) } - end -end diff --git a/ee/spec/requests/api/graphql/project/merge_request_spec.rb b/ee/spec/requests/api/graphql/project/merge_request_spec.rb deleted file mode 100644 index b36e9f5ac2dcf1789f79a380e1487ddde3d6d4d1..0000000000000000000000000000000000000000 --- a/ee/spec/requests/api/graphql/project/merge_request_spec.rb +++ /dev/null @@ -1,90 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'getting merge request information nested in a project', feature_category: :code_review_workflow do - include GraphqlHelpers - - let_it_be(:current_user) { create(:user) } - let_it_be(:project) { create(:project, :repository, :public) } - - let(:merge_request) { create(:merge_request, source_project: project) } - let(:merge_request_graphql_data) { graphql_data_at(:project, :merge_request) } - let(:mr_fields) { "suggestedReviewers { #{all_graphql_fields_for('SuggestedReviewersType')} }" } - let(:suggested_reviewers) do - { - 'version' => '0.0.0', - 'top_n' => 1, - 'reviewers' => %w[bmarley swayne] - } - end - - let(:accepted_reviewers) do - { - 'reviewers' => %w[bmarley] - } - end - - let(:api_result) do - { - 'accepted' => %w[bmarley], - 'suggested' => %w[bmarley swayne] - } - end - - let(:query) do - graphql_query_for( - :project, - { full_path: project.full_path }, - query_graphql_field(:merge_request, { iid: merge_request.iid.to_s }, mr_fields) - ) - end - - describe 'suggestedReviewers' do - before do - merge_request.build_predictions - merge_request.predictions.update!( - suggested_reviewers: suggested_reviewers, - accepted_reviewers: accepted_reviewers - ) - allow_any_instance_of(Project) # rubocop:disable RSpec/AnyInstanceOf - .to receive(:can_suggest_reviewers?).and_return(available) - end - - shared_examples 'feature available' do - it 'returns the right suggested reviewers' do - post_graphql(query, current_user: current_user) - - expected_data = { - 'suggestedReviewers' => a_hash_including(api_result) - } - - expect(merge_request_graphql_data).to include(expected_data) - end - end - - shared_examples 'feature unavailable' do - it 'returns nil' do - post_graphql(query, current_user: current_user) - - expected_data = { - 'suggestedReviewers' => nil - } - - expect(merge_request_graphql_data).to include(expected_data) - end - end - - context 'when suggested reviewers is available for the project' do - let(:available) { true } - - include_examples 'feature available' - end - - context 'when suggested reviewers is not available for the project' do - let(:available) { false } - - include_examples 'feature unavailable' - end - end -end diff --git a/ee/spec/requests/api/internal/suggested_reviewers_spec.rb b/ee/spec/requests/api/internal/suggested_reviewers_spec.rb deleted file mode 100644 index 014b3dd896af23eac2b032eab5710f48f0b8eba4..0000000000000000000000000000000000000000 --- a/ee/spec/requests/api/internal/suggested_reviewers_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe API::Internal::SuggestedReviewers, feature_category: :code_review_workflow do - describe 'POST /internal/suggested_reviewers/tokens' do - let_it_be(:organization) { create(:organization) } - let_it_be_with_reload(:project) { create(:project) } - let_it_be(:secret) do - SecureRandom.random_bytes(Gitlab::AppliedMl::SuggestedReviewers::SECRET_LENGTH) - end - - let(:url) { '/internal/suggested_reviewers/tokens' } - let(:params) { { project_id: project.id } } - let(:headers) { {} } - - subject do - post api(url), params: params, headers: headers - end - - before do - allow(Gitlab::AppliedMl::SuggestedReviewers).to receive(:secret).and_return(secret) - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(suggested_reviewers_internal_api: false) - end - - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when hide_suggested_reviewers is enabled' do - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when feature flag is enabled' do - before do - stub_feature_flags(suggested_reviewers_internal_api: true) - stub_feature_flags(hide_suggested_reviewers: false) - end - - context 'when authentication header is not set', :aggregate_failures do - it 'returns 401' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(json_response).to eq('message' => 'Suggested Reviewers JWT authentication invalid') - end - end - - context 'when authentication header is set' do - let(:headers) do - jwt_token = JWT.encode( - { 'iss' => Gitlab::AppliedMl::SuggestedReviewers::JWT_ISSUER, 'iat' => 1.minute.ago.to_i }, - secret, 'HS256' - ) - - { Gitlab::AppliedMl::SuggestedReviewers::INTERNAL_API_REQUEST_HEADER => jwt_token } - end - - context 'when project is not allowed to suggest reviewers' do - before do - stub_licensed_features(suggested_reviewers: false) - end - - it 'returns 404' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when project is allowed to suggest reviewers', :saas do - before do - stub_licensed_features(suggested_reviewers: true) - project.project_setting.update!(suggested_reviewers_enabled: true) - end - - context 'when create token service fails' do - let(:service) { instance_spy(ResourceAccessTokens::CreateService) } - let(:error_response) do - ServiceResponse.error(message: 'something went wrong') - end - - before do - allow(service).to receive(:execute).and_return(error_response) - allow(ResourceAccessTokens::CreateService).to receive(:new).and_return(service) - end - - it 'returns 400', :aggregate_failures do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response).to eq('message' => '400 Bad request - something went wrong') - end - end - - context 'when create token service succeeds' do - it 'returns 200', [:freeze_time, :aggregate_failures] do - expires_at = 1.day.from_now.to_date.to_s - - subject - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to include( - 'name' => 'Suggested reviewers token', - 'access_level' => Gitlab::Access::REPORTER, - 'scopes' => ['read_api'], - 'expires_at' => expires_at, - 'token' => kind_of(String) - ) - end - end - end - end - end - end -end diff --git a/ee/spec/services/ee/merge_requests/after_create_service_spec.rb b/ee/spec/services/ee/merge_requests/after_create_service_spec.rb index 8afa93b8960a5fae08661d9171329e720627c2ac..6ff7487b20ec6cd5f70af286613118af4a262e59 100644 --- a/ee/spec/services/ee/merge_requests/after_create_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/after_create_service_spec.rb @@ -38,64 +38,6 @@ it_behaves_like 'audits security policy branch bypass' - describe 'suggested reviewers' do - before do - allow(MergeRequests::FetchSuggestedReviewersWorker).to receive(:perform_async) - allow(merge_request).to receive(:ensure_merge_request_diff) - end - - context 'when suggested reviewers is available for project' do - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(true) - end - - context 'when merge request can suggest reviewers' do - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(true) - end - - it 'calls fetch worker for the merge request' do - execute - - expect(merge_request).to have_received(:ensure_merge_request_diff).ordered - expect(MergeRequests::FetchSuggestedReviewersWorker).to have_received(:perform_async) - .with(merge_request.id) - .ordered - end - end - - context 'when merge request cannot suggest reviewers' do - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(false) - end - - it 'does not call fetch worker for the merge request' do - execute - - expect(MergeRequests::FetchSuggestedReviewersWorker).not_to have_received(:perform_async) - end - end - end - - context 'when suggested reviewers is not available for project' do - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(false) - end - - context 'when merge request can suggest reviewers' do - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(true) - end - - it 'does not call fetch worker for the merge request' do - execute - - expect(MergeRequests::FetchSuggestedReviewersWorker).not_to have_received(:perform_async) - end - end - end - end - describe 'schedule_duo_code_review' do let(:merge_request) { create(:merge_request) } let(:current_user) { merge_request.author } diff --git a/ee/spec/services/ee/merge_requests/refresh_service_spec.rb b/ee/spec/services/ee/merge_requests/refresh_service_spec.rb index 7e37769c82c537bcfe50aba288c87d9b66d3e6a3..1d61cd631399634b6728a4f7cb5ac60c82bca6bd 100644 --- a/ee/spec/services/ee/merge_requests/refresh_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/refresh_service_spec.rb @@ -152,36 +152,6 @@ end end - describe '#trigger_suggested_reviewers_fetch' do - using RSpec::Parameterized::TableSyntax - - where(:project_can_suggest, :merge_request_can_suggest, :triggered) do - true | true | true - true | false | false - false | true | false - false | false | false - end - - with_them do - before do - allow(project).to receive(:can_suggest_reviewers?).and_return(project_can_suggest) - - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(merge_request_can_suggest) - allow(service).to receive(:merge_requests_for_source_branch).and_return([merge_request]) - end - - it do - if triggered - expect(::MergeRequests::FetchSuggestedReviewersWorker).to receive(:perform_async).with(merge_request.id) - else - expect(::MergeRequests::FetchSuggestedReviewersWorker).not_to receive(:perform_async).with(merge_request.id) - end - - subject - end - end - end - describe '#sync_any_merge_request_approval_rules' do let(:merge_request_1) { merge_request } let(:merge_request_2) { another_merge_request } diff --git a/ee/spec/services/ee/merge_requests/update_service_spec.rb b/ee/spec/services/ee/merge_requests/update_service_spec.rb index 6f311cf7b8b4c775fb980bc57b88558782b4cc16..745d434f7fed9c0f139d2aafb17a2792fbcf0944 100644 --- a/ee/spec/services/ee/merge_requests/update_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/update_service_spec.rb @@ -511,42 +511,6 @@ def update_merge_request(opts) end end - describe 'capture suggested_reviewer_ids', feature_category: :code_review_workflow do - shared_examples 'not capturing suggested_reviewer_ids' do - it 'does not capture the suggested_reviewer_ids and raise update error', :aggregate_failures do - expect(MergeRequests::CaptureSuggestedReviewersAcceptedWorker).not_to receive(:perform_async) - - expect { update_merge_request(opts) }.not_to raise_error - end - end - - context 'when reviewer_ids is present' do - context 'when suggested_reviewer_ids is present' do - let(:opts) { { reviewer_ids: [user.id, user2.id], suggested_reviewer_ids: [user.id] } } - - it 'captures the suggested_reviewer_ids and does not raise update error', :aggregate_failures do - expect(MergeRequests::CaptureSuggestedReviewersAcceptedWorker) - .to receive(:perform_async) - .with(merge_request.id, [user.id]) - - expect { update_merge_request(opts) }.not_to raise_error - end - end - - context 'when suggested_reviewer_ids is blank' do - let(:opts) { { reviewer_ids: [user.id, user2.id] } } - - it_behaves_like 'not capturing suggested_reviewer_ids' - end - end - - context 'when reviewer_ids is blank' do - let(:opts) { { reviewer_ids: [], suggested_reviewer_ids: [user.id] } } - - it_behaves_like 'not capturing suggested_reviewer_ids' - end - end - describe '#sync_any_merge_request_approval_rules' do let(:opts) { { target_branch: 'feature-2' } } let!(:scan_result_policy_read) { create(:scan_result_policy_read, :targeting_commits, project: project) } diff --git a/ee/spec/services/merge_requests/capture_suggested_reviewers_accepted_service_spec.rb b/ee/spec/services/merge_requests/capture_suggested_reviewers_accepted_service_spec.rb deleted file mode 100644 index b06df4ec437800961b69f1a198ccb748992fd7f6..0000000000000000000000000000000000000000 --- a/ee/spec/services/merge_requests/capture_suggested_reviewers_accepted_service_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequests::CaptureSuggestedReviewersAcceptedService, feature_category: :code_review_workflow do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - subject(:service) { described_class.new(project: project, current_user: user) } - - before_all do - project.add_maintainer(user) - end - - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(true) - end - - describe '#execute' do - subject(:result) { service.execute(merge_request, reviewer_ids) } - - context 'when the reviewer IDs param is empty' do - let(:reviewer_ids) { [] } - - it 'returns an error response', :aggregate_failures do - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Reviewer IDs are empty') - end - end - - context 'when the merge request is not eligible' do - let(:reviewer_ids) { [1, 2] } - - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(false) - end - - it 'returns an error response', :aggregate_failures do - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Merge request is not eligible') - end - end - - context 'when there is no existing predictions' do - let(:reviewer_ids) { [1, 2] } - - it 'returns an error response', :aggregate_failures do - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('No predictions are recorded') - end - end - - context 'when there is a validation error' do - let(:reviewer_ids) { [1, 2] } - let(:predictions) { build(:predictions, merge_request: merge_request) } - - before do - allow(predictions).to receive(:update!).and_raise(ActiveRecord::RecordInvalid) - allow(merge_request).to receive(:predictions).and_return(predictions) - end - - it 'returns an error response', :aggregate_failures do - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Record invalid') - end - end - - context 'when successful' do - using RSpec::Parameterized::TableSyntax - - let(:reviewer_ids) { [1, 2] } - - where(:suggested, :existing_accepted, :new_accepted, :accepted) do - %w[bob mary donald] | %w[] | %w[donald] | %w[donald] - %w[bob mary donald] | %w[bob] | %w[donald] | %w[bob donald] - %w[bob mary donald] | %w[bob] | %w[mickey] | %w[bob] - %w[bob mary donald] | %w[bob donald] | %w[bob] | %w[bob donald] - end - - with_them do - let(:predictions) do - build( - :predictions, - merge_request: merge_request, - suggested_reviewers: { reviewers: suggested }, - accepted_reviewers: { reviewers: existing_accepted } - ) - end - - before do - merge_request.predictions = predictions - - allow(project).to receive(:member_usernames_among).and_return(new_accepted) - end - - it 'returns a success response and updates predictions', :aggregate_failures do - expect(result).to be_a(ServiceResponse) - expect(result).to be_success - expect(result.payload).to eq( - { - project_id: project.id, - merge_request_id: merge_request.id, - reviewers: accepted - } - ) - - expect(predictions.reload.accepted_reviewer_usernames).to eq(accepted) - end - end - end - end -end diff --git a/ee/spec/services/merge_requests/fetch_suggested_reviewers_service_spec.rb b/ee/spec/services/merge_requests/fetch_suggested_reviewers_service_spec.rb deleted file mode 100644 index 8e8365885a28b4993b28e8fb33bf5ce65cb41eb0..0000000000000000000000000000000000000000 --- a/ee/spec/services/merge_requests/fetch_suggested_reviewers_service_spec.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequests::FetchSuggestedReviewersService, feature_category: :code_review_workflow do - let(:merge_request) { create(:merge_request) } - let(:project) { merge_request.project } - - subject(:service) { described_class.new(project: project).execute(merge_request) } - - describe '#execute' do - let(:example_model_result) do - { - version: '0.1.0', - top_n: 1, - reviewers: ['root'] - } - end - - let(:example_result) do - example_model_result.merge({ status: :success }) - end - - let(:model_input) do - { - project_id: merge_request.project_id, - merge_request_iid: merge_request.iid, - changes: [ - 'bar/branch-test.txt', - 'custom-highlighting/test.gitlab-custom', - 'encoding/iso8859.txt', - 'files/images/wm.svg', - 'files/js/commit.coffee', - 'files/js/commit.js.coffee', - 'files/lfs/lfs_object.iso', - 'files/ruby/popen.rb', - 'files/ruby/regex.rb', - 'files/.DS_Store', - 'files/whitespace', - 'foo/bar/.gitkeep', - 'with space/README.md', - '.DS_Store', - '.gitattributes', - '.gitignore', - '.gitmodules', - 'CHANGELOG', - 'README', - 'gitlab-grack', - 'gitlab-shell' - ], - author_username: merge_request.author.username - } - end - - it 'sends the machine learning model input to the suggested reviewers client' do - stub_env('SUGGESTED_REVIEWERS_SECRET', SecureRandom.hex(32)) - - expect_next_instance_of(Gitlab::AppliedMl::SuggestedReviewers::Client) do |client| - expect(client).to receive(:suggested_reviewers).with(model_input).and_return(example_model_result) - end - - expect(service[:status]).to eq(:success) - end - - it 'returns an empty result when changes are empty' do - allow(merge_request).to receive(:modified_paths).and_return([]) - - expect(service) - .to eq({ status: :error, message: 'Merge request contains no modified files' }) - end - end -end diff --git a/ee/spec/services/projects/deregister_suggested_reviewers_project_service_spec.rb b/ee/spec/services/projects/deregister_suggested_reviewers_project_service_spec.rb deleted file mode 100644 index 9c73112d9346c3d0da5df4cffeee37ac0584275d..0000000000000000000000000000000000000000 --- a/ee/spec/services/projects/deregister_suggested_reviewers_project_service_spec.rb +++ /dev/null @@ -1,114 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::DeregisterSuggestedReviewersProjectService, :saas, feature_category: :code_review_workflow do - include AfterNextHelpers - - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - subject(:service) { described_class.new(project: project, current_user: user) } - - describe '#execute' do - let(:deregistration_result) do - { - project_id: project.project_id, - deregistered_at: '2022-01-01 01:01' - } - end - - let(:deregistration_input) do - { - project_id: project.project_id - } - end - - subject(:result) { service.execute } - - shared_examples 'an unavailable response' do - it 'returns an error response without calling client', :aggregate_failures do - expect(Gitlab::AppliedMl::SuggestedReviewers::Client).not_to receive(:new) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Suggested Reviewers deregistration is unavailable') - expect(result.reason).to eq(:feature_unavailable) - end - end - - before do - project.add_maintainer(user) - end - - context 'when the suggested reviewers is not available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - it_behaves_like 'an unavailable response' - end - - context 'when the suggested reviewers is available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when the suggested reviewers is enabled' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(true) - end - - it_behaves_like 'an unavailable response' - end - - context 'when the suggested reviewers is not enabled' do - before do - stub_env('SUGGESTED_REVIEWERS_SECRET', SecureRandom.hex(32)) - allow(project).to receive(:suggested_reviewers_enabled).and_return(false) - end - - context 'when project is not found' do - it 'returns an error response', :aggregate_failures do - allow_next(Gitlab::AppliedMl::SuggestedReviewers::Client) - .to receive(:deregister_project) - .with(deregistration_input) - .and_raise(Gitlab::AppliedMl::Errors::ProjectNotFound) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Project is not found') - expect(result.reason).to eq(:project_not_found) - end - end - - context 'when suggested reviewers client fails' do - it 'returns an error response', :aggregate_failures do - allow_next(Gitlab::AppliedMl::SuggestedReviewers::Client) - .to receive(:deregister_project) - .with(deregistration_input) - .and_raise(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Failed to deregister project') - expect(result.reason).to eq(:client_request_failed) - end - end - - context 'when suggested reviewers client succeeds' do - it 'returns a success response', :aggregate_failures do - allow_next(Gitlab::AppliedMl::SuggestedReviewers::Client) - .to receive(:deregister_project) - .with(hash_including(deregistration_input)) - .and_return(deregistration_result) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_success - expect(result.payload).to eq(deregistration_result) - end - end - end - end - end -end diff --git a/ee/spec/services/projects/register_suggested_reviewers_project_service_spec.rb b/ee/spec/services/projects/register_suggested_reviewers_project_service_spec.rb deleted file mode 100644 index 62f9cf3b90fe6a9fb6e37faebf48269aa2391cd2..0000000000000000000000000000000000000000 --- a/ee/spec/services/projects/register_suggested_reviewers_project_service_spec.rb +++ /dev/null @@ -1,180 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::RegisterSuggestedReviewersProjectService, :saas, feature_category: :code_review_workflow do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - subject(:service) { described_class.new(project: project, current_user: user) } - - describe '#execute' do - let(:registration_result) do - { - project_id: project.project_id, - registered_at: '2022-01-01 01:01' - } - end - - let(:registration_input) do - { - project_id: project.project_id, - project_name: project.name, - project_namespace: project.namespace.full_path, - access_token: a_kind_of(String) - } - end - - subject(:result) { service.execute } - - shared_examples 'an unavailable response' do - it 'returns an error response without calling client', :aggregate_failures do - expect(ResourceAccessTokens::CreateService).not_to receive(:new) - expect(Gitlab::AppliedMl::SuggestedReviewers::Client).not_to receive(:new) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Suggested Reviewers feature is unavailable') - expect(result.reason).to eq(:feature_unavailable) - end - end - - before do - project.add_maintainer(user) - end - - context 'when the suggested reviewers is not available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - it_behaves_like 'an unavailable response' - end - - context 'when the suggested reviewers is available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when the suggested reviewers is not enabled' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(false) - end - - it_behaves_like 'an unavailable response' - end - - context 'when the suggested reviewers is enabled' do - before do - stub_env('SUGGESTED_REVIEWERS_SECRET', SecureRandom.hex(32)) - allow(project).to receive(:suggested_reviewers_enabled).and_return(true) - - allow_next_instance_of(ResourceAccessTokens::CreateService, user, project, anything) do |token_service| - allow(token_service).to receive(:execute).and_return(token_response) - end - end - - context 'when the user cannot create access tokens' do - before do - project.add_developer(user) - end - - it_behaves_like 'an unavailable response' - end - - context 'when the user can create access tokens' do - before do - project.add_maintainer(user) - end - - it 'creates an access token', :aggregate_failures do - freeze_time do - token_params = { - name: 'Suggested reviewers token', - scopes: [Gitlab::Auth::READ_API_SCOPE], - expires_at: 95.days.from_now - } - - expect_next_instance_of( - ResourceAccessTokens::CreateService, user, project, token_params - ) do |token_service| - expect(token_service).to receive(:execute).and_call_original - end - - expect_next_instance_of(Gitlab::AppliedMl::SuggestedReviewers::Client) do |client| - expect(client).to receive(:register_project) - .with(registration_input) - .and_return(registration_result) - end - - result - end - end - - context 'when token creation succeeds', :aggregate_failures do - let(:token_response) do - ServiceResponse.success(payload: { access_token: create(:personal_access_token) }) - end - - context 'when suggested reviewers client succeeds' do - it 'returns a success response', :aggregate_failures do - allow_next_instance_of(Gitlab::AppliedMl::SuggestedReviewers::Client) do |client| - allow(client).to receive(:register_project) - .with(hash_including(registration_input)) - .and_return(registration_result) - end - - expect(result).to be_a(ServiceResponse) - expect(result).to be_success - expect(result.payload).to eq(registration_result) - end - end - - context 'when suggested reviewers client fails' do - it 'returns an error response', :aggregate_failures do - allow_next_instance_of(Gitlab::AppliedMl::SuggestedReviewers::Client) do |client| - allow(client).to receive(:register_project) - .with(registration_input) - .and_raise(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - end - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Failed to register project') - expect(result.reason).to eq(:client_request_failed) - end - end - - context 'when project is already registered', :aggregate_failures do - it 'returns an error response', :aggregate_failures do - allow_next_instance_of(Gitlab::AppliedMl::SuggestedReviewers::Client) do |client| - allow(client).to receive(:register_project) - .with(registration_input) - .and_raise(Gitlab::AppliedMl::Errors::ProjectAlreadyExists) - end - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Project is already registered') - expect(result.reason).to eq(:project_already_registered) - end - end - end - - context 'when token creation fails', :aggregate_failures do - let(:token_response) { ServiceResponse.error(message: 'No joy') } - - it 'returns an error response and does not call suggested reviewers client', :aggregate_failures do - expect(Gitlab::AppliedMl::SuggestedReviewers::Client).not_to receive(:new) - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - expect(result.message).to eq('Failed to create access token') - expect(result.reason).to eq(:token_creation_failed) - end - end - end - end - end - end -end diff --git a/ee/spec/services/projects/update_service_spec.rb b/ee/spec/services/projects/update_service_spec.rb index 20a8afc34ca933296a28b7c9a9937d6ec0ba1058..6b37216afadadd88f91477d92559a65f77cbb516 100644 --- a/ee/spec/services/projects/update_service_spec.rb +++ b/ee/spec/services/projects/update_service_spec.rb @@ -567,195 +567,6 @@ def update_default_branch(branch = 'feature') end end - context 'triggering suggested reviewer project registrations' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - - let(:opts) { { project_setting_attributes: { suggested_reviewers_enabled: '1' } } } - - before do - group.add_maintainer(user) - project.add_maintainer(user) - end - - shared_examples 'calling registration worker' do - it 'calls perform_async' do - expect(::Projects::RegisterSuggestedReviewersProjectWorker) - .to receive(:perform_async) - .with(project.id, user.id) - - update_project(project, user, opts) - end - end - - shared_examples 'not calling registration worker' do - it 'does not call perform_async' do - expect(::Projects::RegisterSuggestedReviewersProjectWorker).not_to receive(:perform_async) - - update_project(project, user, opts) - end - end - - context 'when available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when enabled' do - before do - project.project_setting.update! suggested_reviewers_enabled: true - end - - it_behaves_like 'not calling registration worker' - end - - context 'when not enabled' do - before do - project.project_setting.update! suggested_reviewers_enabled: false - end - - context 'when not allowed to create access token' do - before do - group.namespace_settings.update! resource_access_token_creation_allowed: false - end - - it_behaves_like 'not calling registration worker' - end - - context 'when allowed to create access token', :saas do - before do - group.namespace_settings.update! resource_access_token_creation_allowed: true - end - - it_behaves_like 'calling registration worker' - - it 'sets the setting' do - expect { update_project(project, user, opts) } - .to change { project.reload.suggested_reviewers_enabled }.from(false).to(true) - end - - context 'when form param is set to false' do - let(:opts) { { project_setting_attributes: { suggested_reviewers_enabled: '0' } } } - - it_behaves_like 'not calling registration worker' - end - end - end - end - - context 'when not available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - context 'when enabled' do - before do - project.project_setting.update! suggested_reviewers_enabled: true - end - - it_behaves_like 'not calling registration worker' - end - - context 'when not enabled' do - before do - project.project_setting.update! suggested_reviewers_enabled: false - end - - it_behaves_like 'not calling registration worker' - - it 'does not set the setting' do - expect { update_project(project, user, opts) }.not_to change { project.reload.suggested_reviewers_enabled } - end - end - end - end - - context 'when triggering suggested reviewers project deregistrations' do - let_it_be_with_reload(:project) { create(:project) } - - let(:opts) { { project_setting_attributes: { suggested_reviewers_enabled: '0' } } } - - before_all do - project.add_maintainer(user) - end - - shared_examples 'calling deregistration worker' do - it 'calls perform_async' do - expect(::Projects::DeregisterSuggestedReviewersProjectWorker) - .to receive(:perform_async).with(project.id, user.id) - - update_project(project, user, opts) - end - - it 'changes the setting' do - expect { update_project(project, user, opts) } - .to change { project.reload.suggested_reviewers_enabled }.from(true).to(false) - end - end - - shared_examples 'not calling deregistration worker' do - it 'does not call perform_async' do - expect(::Projects::DeregisterSuggestedReviewersProjectWorker).not_to receive(:perform_async) - - update_project(project, user, opts) - end - - it 'does not change the setting' do - expect { update_project(project, user, opts) }.not_to change { project.reload.suggested_reviewers_enabled } - end - end - - context 'when available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when not enabled' do - before do - project.project_setting.update!(suggested_reviewers_enabled: false) - end - - it_behaves_like 'not calling deregistration worker' - end - - context 'when enabled', :saas do - before do - project.project_setting.update!(suggested_reviewers_enabled: true) - end - - it_behaves_like 'calling deregistration worker' - - context 'when form param is set to true' do - let(:opts) { { project_setting_attributes: { suggested_reviewers_enabled: '1' } } } - - it_behaves_like 'not calling deregistration worker' - end - end - end - - context 'when not available' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - context 'when not enabled' do - before do - project.project_setting.update!(suggested_reviewers_enabled: false) - end - - it_behaves_like 'not calling deregistration worker' - end - - context 'when enabled' do - before do - project.project_setting.update!(suggested_reviewers_enabled: true) - end - - it_behaves_like 'not calling deregistration worker' - end - end - end - it 'returns an error result when record cannot be updated' do admin = create(:admin) diff --git a/ee/spec/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml_spec.rb b/ee/spec/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml_spec.rb deleted file mode 100644 index 0064ad12490576873b09abfc2f5bd1e3aa5818fc..0000000000000000000000000000000000000000 --- a/ee/spec/views/projects/settings/merge_requests/_suggested_reviewers_settings.html.haml_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'projects/settings/merge_requests/_suggested_reviewers_settings', - feature_category: :code_review_workflow do - let_it_be(:user) { build(:user) } - let_it_be(:project) { build(:project) } - - before do - assign(:project, project) - - allow(view).to receive(:expanded).and_return(true) - allow(view).to receive(:current_user).and_return(user) - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when user can create access tokens' do - before do - allow(user).to receive(:can?).with(:create_resource_access_tokens, project).and_return(true) - end - - it 'renders the settings title', :aggregate_failures do - render - - expect(rendered).to have_content 'Suggested reviewers' - expect(rendered).to have_content "Get suggestions for reviewers based on GitLab's machine learning tool." - end - - it 'renders the settings form', :aggregate_failures do - expect(view).to receive(:gitlab_ui_form_for) - .with(project, a_hash_including(url: project_settings_merge_requests_path(project))) - .and_call_original - - render - - expect(rendered).to have_css('input[id=project_project_setting_attributes_suggested_reviewers_enabled]') - end - end - - context 'when user cannot create access tokens' do - before do - allow(user).to receive(:can?).with(:create_resource_access_tokens, project).and_return(false) - end - - it 'does not render the settings section' do - render - - expect(rendered).not_to have_content 'Suggested reviewers' - end - end -end diff --git a/ee/spec/workers/merge_requests/capture_suggested_reviewers_accepted_worker_spec.rb b/ee/spec/workers/merge_requests/capture_suggested_reviewers_accepted_worker_spec.rb deleted file mode 100644 index f483231ee478b00ae4cc2d557157c05fdfbb3113..0000000000000000000000000000000000000000 --- a/ee/spec/workers/merge_requests/capture_suggested_reviewers_accepted_worker_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequests::CaptureSuggestedReviewersAcceptedWorker, feature_category: :code_review_workflow do - let_it_be(:reviewers) { create_list(:user, 3) } - let_it_be(:merge_request) { create(:merge_request) } - let_it_be(:project) { merge_request.project } - let_it_be(:predictions) do - create( - :predictions, - merge_request: merge_request, - suggested_reviewers: { reviewers: reviewers.map(&:username) } - ) - end - - let(:reviewer_ids) { [reviewers.first(2).map(&:id)] } - - subject(:worker) { described_class.new } - - before_all do - project.add_members(reviewers, :developer) - end - - it_behaves_like 'an idempotent worker' do - let(:job_args) { [merge_request.id, reviewer_ids] } - - it 'updates the accepted reviewers' do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(true) - - subject - - expect(predictions.reload.accepted_reviewer_usernames).to eq(reviewers.first(2).map(&:username)) - end - end - - describe '#perform' do - context 'when merge request is not found' do - it 'returns without calling the capture suggested reviewer service' do - expect(MergeRequests::CaptureSuggestedReviewersAcceptedService).not_to receive(:new) - - worker.perform(non_existing_record_id, reviewer_ids) - end - end - - context 'when merge request is found' do - before do - allow(MergeRequest).to receive(:find_by_id).and_return(merge_request) - end - - context 'when merge request is not eligible' do - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(false) - end - - it 'returns without calling the capture suggested reviewer service' do - expect(MergeRequests::CaptureSuggestedReviewersAcceptedService).not_to receive(:new) - - worker.perform(merge_request.id, reviewer_ids) - end - end - - context 'when merge request is eligible' do - before do - allow(merge_request).to receive(:can_suggest_reviewers?).and_return(true) - end - - context 'when reviewer ids is blank' do - let(:reviewer_ids) { [] } - - it 'returns without calling the capture suggested reviewer service' do - expect(MergeRequests::CaptureSuggestedReviewersAcceptedService).not_to receive(:new) - - worker.perform(merge_request.id, reviewer_ids) - end - end - - context 'when reviewer ids is not blank' do - let(:reviewer_ids) { [1, 2] } - - context 'when service returns error' do - let(:response) do - ServiceResponse.error(message: 'an error has occurred') - end - - it 'returns without logging extra metadata' do - allow_next_instance_of(::MergeRequests::CaptureSuggestedReviewersAcceptedService) do |instance| - allow(instance).to receive(:execute).with(merge_request, reviewer_ids).and_return(response) - end - - expect(worker).not_to receive(:log_extra_metadata_on_done) - - worker.perform(merge_request.id, reviewer_ids) - end - end - - context 'when service returns success' do - let(:response) do - ServiceResponse.success( - payload: { - project_id: merge_request.project.id, - merge_request_id: merge_request.id, - reviewers: %w[bob mary] - } - ) - end - - it 'attempts to capture suggested reviewers accepted' do - expect_next_instance_of(::MergeRequests::CaptureSuggestedReviewersAcceptedService) do |instance| - expect(instance).to receive(:execute).with(merge_request, reviewer_ids).and_return(response) - end - - worker.perform(merge_request.id, reviewer_ids) - end - - it 'logs with extra metadata', :aggregate_failures do - allow_next_instance_of(::MergeRequests::CaptureSuggestedReviewersAcceptedService) do |instance| - allow(instance).to receive(:execute).and_return(response) - end - - expect(worker).to receive(:log_extra_metadata_on_done) - .with(:project_id, response.payload[:project_id]) - expect(worker).to receive(:log_extra_metadata_on_done) - .with(:merge_request_id, response.payload[:merge_request_id]) - expect(worker).to receive(:log_extra_metadata_on_done) - .with(:reviewers, response.payload[:reviewers]) - - worker.perform(merge_request.id, reviewer_ids) - end - end - end - end - end - end -end diff --git a/ee/spec/workers/merge_requests/fetch_suggested_reviewers_worker_spec.rb b/ee/spec/workers/merge_requests/fetch_suggested_reviewers_worker_spec.rb deleted file mode 100644 index 59a4fbda419b5c0441553c64322f896b54a8940e..0000000000000000000000000000000000000000 --- a/ee/spec/workers/merge_requests/fetch_suggested_reviewers_worker_spec.rb +++ /dev/null @@ -1,119 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequests::FetchSuggestedReviewersWorker, feature_category: :code_review_workflow do - include AfterNextHelpers - - let_it_be(:merge_request) { create(:merge_request) } - - subject { described_class.new } - - describe '#perform' do - context 'when merge request is not found' do - it 'returns without calling the fetch suggested reviewer service' do - expect(MergeRequests::FetchSuggestedReviewersService).not_to receive(:new) - - subject.perform(non_existing_record_id) - end - end - - context 'when merge request does not have changes' do - let_it_be(:merge_request) { create(:merge_request, :without_diffs) } - - it 'returns without calling the fetch suggested reviewer service' do - expect(MergeRequests::FetchSuggestedReviewersService).not_to receive(:new) - - subject.perform(merge_request.id) - end - end - - context 'when merge request is found' do - let(:example_model_result) do - { - version: '0.1.0', - top_n: 1, - reviewers: ['root'] - } - end - - let(:example_success_result) do - example_model_result.merge({ status: :success }) - end - - let(:example_error_result) do - example_model_result.merge({ status: :error }) - end - - context 'with a happy path' do - it 'attempts to fetch suggested reviewers' do - expect_next_instance_of(::MergeRequests::FetchSuggestedReviewersService) do |instance| - expect(instance).to receive(:execute) - end - - subject.perform(merge_request.id) - end - - it 'updates the merge request with a successful result' do - allow_next_instance_of(::MergeRequests::FetchSuggestedReviewersService) do |instance| - allow(instance).to receive(:execute).and_return(example_success_result) - end - - subject.perform(merge_request.id) - - expect(merge_request.predictions.suggested_reviewers).to eq(example_model_result.stringify_keys) - end - end - - context 'when issues occur' do - let_it_be(:merge_request2) { create(:merge_request) } - - let(:logger) { subject.send(:logger) } - - it 'with an error result does not update the merge request predictions', :aggregate_failures do - allow_next_instance_of(::MergeRequests::FetchSuggestedReviewersService) do |instance| - allow(instance).to receive(:execute).and_return(example_error_result) - end - - log_params = { - result: { - reviewers: ["root"], - status: :error, - top_n: 1, - version: "0.1.0" - } - } - expect(logger).to receive(:error).with(hash_including(log_params.stringify_keys)).and_call_original - - subject.perform(merge_request2.id) - - expect(merge_request2.predictions&.suggested_reviewers).to be_nil - end - end - - context 'when exceptions are raised' do - it 're-raises exception when it is retriable' do - allow_next(::MergeRequests::FetchSuggestedReviewersService) - .to receive(:execute) - .and_raise(Gitlab::AppliedMl::Errors::ConnectionFailed) - - expect { subject.perform(merge_request.id) }.to raise_error(Gitlab::AppliedMl::Errors::ConnectionFailed) - end - - it 'does not raise but logs exception when it is swallowable', :aggregate_failures do - allow_next(::MergeRequests::FetchSuggestedReviewersService) - .to receive(:execute) - .and_raise(Gitlab::AppliedMl::Errors::ResourceNotAvailable) - - expect(Gitlab::ErrorTracking).to receive(:log_exception).with( - Gitlab::AppliedMl::Errors::ResourceNotAvailable, - project_id: merge_request.project.id, - merge_request_id: merge_request.id - ) - - expect { subject.perform(merge_request.id) }.not_to raise_error - end - end - end - end -end diff --git a/ee/spec/workers/projects/deregister_suggested_reviewers_project_worker_spec.rb b/ee/spec/workers/projects/deregister_suggested_reviewers_project_worker_spec.rb deleted file mode 100644 index 823d2dae28261e47ea397d3521ce08afff534496..0000000000000000000000000000000000000000 --- a/ee/spec/workers/projects/deregister_suggested_reviewers_project_worker_spec.rb +++ /dev/null @@ -1,139 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::DeregisterSuggestedReviewersProjectWorker, feature_category: :code_review_workflow do - let(:project) { build_stubbed(:project) } - let(:user) { build_stubbed(:user) } - let(:service) { instance_spy(::Projects::DeregisterSuggestedReviewersProjectService) } - - subject { described_class.new } - - describe '#perform' do - before do - allow(::Project).to receive(:find_by_id).and_return(project) - allow(::User).to receive(:find_by_id).and_return(user) - - allow(::Projects::DeregisterSuggestedReviewersProjectService).to receive(:new).and_return(service) - end - - context 'when project is not found' do - it 'returns without calling the deregister service' do - subject.perform(non_existing_record_id, user.id) - - expect(service).not_to have_received(:execute) - end - end - - context 'when project is found' do - context 'when user is not found' do - it 'returns without calling the deregister service' do - subject.perform(project.id, non_existing_record_id) - - expect(service).not_to have_received(:execute) - end - end - - context 'when user is found' do - context 'when suggested reviews is not available for the project' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(false) - end - - it 'returns without calling the deregister service', :aggregate_failures do - subject.perform(project.id, user.id) - - expect(service).not_to have_received(:execute) - end - end - - context 'when suggested reviews is available for the project' do - before do - allow(project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when suggested reviews is enabled for the project' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(true) - end - - it 'returns without calling the deregister service', :aggregate_failures do - subject.perform(project.id, user.id) - - expect(service).not_to have_received(:execute) - end - end - - context 'when suggested reviews is not enabled for the project' do - before do - allow(project).to receive(:suggested_reviewers_enabled).and_return(false) - end - - context 'when service returns success' do - let(:deregistration_result) do - { - project_id: project.id, - deregistered_at: '2022-01-01 09:00' - } - end - - let(:response) do - ServiceResponse.success(payload: deregistration_result) - end - - it 'calls deregister service and logs an info with payload', :aggregate_failures do - allow(service).to receive(:execute).and_return(response) - - expect(subject) - .to receive(:log_hash_metadata_on_done) - .with( - project_id: response.payload[:project_id], - deregistered_at: response.payload[:deregistered_at] - ) - - subject.perform(project.id, user.id) - end - end - - context 'when service returns error' do - context 'when error is swallowable' do - let(:response) do - ServiceResponse.error( - message: 'Project is not found', - reason: :project_not_found - ) - end - - it 'swallows the error' do - allow(service).to receive(:execute).and_return(response) - - expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - - subject.perform(project.id, user.id) - end - end - - context 'when error is trackable and raisable' do - let(:response) do - ServiceResponse.error(message: 'Failed to deregister project', reason: :client_request_failed) - end - - it 'tracks and raises the error', :aggregate_failures do - allow(service).to receive(:execute).and_return(response) - - expect(Gitlab::ErrorTracking) - .to receive(:track_and_raise_exception) - .with(an_instance_of(StandardError), { project_id: project.id }) - .and_call_original - - expect { subject.perform(project.id, user.id) } - .to raise_error(StandardError, 'Failed to deregister project') - end - end - end - end - end - end - end - end -end diff --git a/ee/spec/workers/projects/register_suggested_reviewers_project_worker_spec.rb b/ee/spec/workers/projects/register_suggested_reviewers_project_worker_spec.rb deleted file mode 100644 index 439de63c178591b773b7e219e3f3632300f8040e..0000000000000000000000000000000000000000 --- a/ee/spec/workers/projects/register_suggested_reviewers_project_worker_spec.rb +++ /dev/null @@ -1,153 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::RegisterSuggestedReviewersProjectWorker, feature_category: :code_review_workflow do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - subject { described_class.new } - - # rubocop:disable RSpec/AnyInstanceOf - describe '#perform' do - context 'when project is not found' do - it 'returns without calling the fetch suggested reviewer service' do - expect(::Projects::RegisterSuggestedReviewersProjectService).not_to receive(:new) - - subject.perform(non_existing_record_id, user.id) - end - end - - context 'when project is found' do - context 'when user is not found' do - it 'returns without calling the fetch suggested reviewer service' do - expect(::Projects::RegisterSuggestedReviewersProjectService).not_to receive(:new) - - subject.perform(project.id, non_existing_record_id) - end - end - - context 'when user is found' do - context 'when suggested reviews is not available for the project' do - before do - allow_any_instance_of(::Project).to receive(:suggested_reviewers_available?).and_return(false) - end - - it 'returns without calling the fetch suggested reviewer service', :aggregate_failures do - expect(::Projects::RegisterSuggestedReviewersProjectService).not_to receive(:new) - - subject.perform(project.id, user.id) - end - end - - context 'when suggested reviews is available for the project' do - before do - allow_any_instance_of(::Project).to receive(:suggested_reviewers_available?).and_return(true) - end - - context 'when suggested reviews is not enabled for the project' do - before do - allow_any_instance_of(::Project).to receive(:suggested_reviewers_enabled).and_return(false) - end - - it 'returns without calling the fetch suggested reviewer service', :aggregate_failures do - expect(::Projects::RegisterSuggestedReviewersProjectService).not_to receive(:new) - - subject.perform(project.id, user.id) - end - end - - context 'when suggested reviews is enabled for the project' do - before do - allow_any_instance_of(::Project).to receive(:suggested_reviewers_enabled).and_return(true) - end - - context 'when service returns success' do - let(:registration_result) do - { - project_id: project.id, - registered_at: '2022-01-01 09:00' - } - end - - let(:response) do - ServiceResponse.success(payload: registration_result) - end - - it 'calls project register service and logs an info with payload', :aggregate_failures do - allow_next_instance_of(::Projects::RegisterSuggestedReviewersProjectService) do |instance| - allow(instance).to receive(:execute).and_return(response) - end - - expect(subject).to receive(:log_extra_metadata_on_done) - .with(:project_id, response.payload[:project_id]) - expect(subject).to receive(:log_extra_metadata_on_done) - .with(:registered_at, response.payload[:registered_at]) - - subject.perform(project.id, user.id) - end - end - - context 'when service returns error' do - context 'when error is trackable' do - let(:response) do - ServiceResponse.error(message: 'Failed to create access token', reason: :token_creation_failed) - end - - it 'tracks the error' do - allow_next_instance_of(::Projects::RegisterSuggestedReviewersProjectService) do |instance| - allow(instance).to receive(:execute).and_return(response) - end - - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with(an_instance_of(StandardError), { project_id: project.id }) - .and_call_original - - subject.perform(project.id, user.id) - end - end - - context 'when error is swallowable' do - let(:response) do - ServiceResponse.error(message: 'Project is already registered', reason: :project_already_registered) - end - - it 'swallows the error' do - allow_next_instance_of(::Projects::RegisterSuggestedReviewersProjectService) do |instance| - allow(instance).to receive(:execute).and_return(response) - end - - expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - - subject.perform(project.id, user.id) - end - end - - context 'when error is trackable and raisable' do - let(:response) do - ServiceResponse.error(message: 'Failed to register project', reason: :client_request_failed) - end - - it 'tracks and raises the error', :aggregate_failures do - allow_next_instance_of(::Projects::RegisterSuggestedReviewersProjectService) do |instance| - allow(instance).to receive(:execute).and_return(response) - end - - expect(Gitlab::ErrorTracking) - .to receive(:track_and_raise_exception) - .with(an_instance_of(StandardError), { project_id: project.id }) - .and_call_original - - expect { subject.perform(project.id, user.id) } - .to raise_error(StandardError, 'Failed to register project') - end - end - end - end - end - end - end - end - # rubocop:enable RSpec/AnyInstanceOf -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 57a8d6b3e6572c321feb20524366a2f87077fe3e..6829e1c73bd73f3c29ffebd3988a7a5bf550120f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6645,9 +6645,6 @@ msgstr "" msgid "All policy rules must be satisfied." msgstr "" -msgid "All project members" -msgstr "" - msgid "All projects" msgstr "" @@ -49126,9 +49123,6 @@ msgstr "" msgid "ProjectSettings|Enable sending email notifications for this project" msgstr "" -msgid "ProjectSettings|Enable suggested reviewers" -msgstr "" - msgid "ProjectSettings|Encourage" msgstr "" @@ -61481,27 +61475,9 @@ msgstr "" msgid "SuggestedColors|Titanium yellow" msgstr "" -msgid "SuggestedReviewers|Get suggestions for reviewers based on GitLab's machine learning tool." -msgstr "" - -msgid "SuggestedReviewers|Learn about suggested reviewers" -msgstr "" - -msgid "SuggestedReviewers|Suggested reviewers" -msgstr "" - -msgid "SuggestedReviewers|Suggested reviewers help link" -msgstr "" - -msgid "SuggestedReviewers|Suggestions appear in the Reviewer section of the right sidebar" -msgstr "" - msgid "Suggestion is not applicable as the suggestion was not found." msgstr "" -msgid "Suggestion(s)" -msgstr "" - msgid "Suggestions are not applicable as one or more suggestions were not found." msgstr "" diff --git a/spec/contracts/consumer/package.json b/spec/contracts/consumer/package.json index 60f268806de19715655343a9aa7b36105ab17c33..6d3feaa6d4c870e01f5da36c55731d7da232bc34 100644 --- a/spec/contracts/consumer/package.json +++ b/spec/contracts/consumer/package.json @@ -22,8 +22,5 @@ "devDependencies": { "@babel/preset-env": "^7.18.2", "babel-jest": "^28.1.1" - }, - "config": { - "pact_do_not_track": true } } diff --git a/spec/frontend/sidebar/sidebar_mediator_spec.js b/spec/frontend/sidebar/sidebar_mediator_spec.js index 4dc285fc3c86d9684a294c6b45ef39fd6b61b700..f107ae670b9ea96ea5585e4396a1dd08737c9655 100644 --- a/spec/frontend/sidebar/sidebar_mediator_spec.js +++ b/spec/frontend/sidebar/sidebar_mediator_spec.js @@ -71,7 +71,6 @@ describe('Sidebar mediator', () => { it('sends correct data to service', () => { const data = { reviewer_ids: [1, 2], - suggested_reviewer_ids: [1], }; mediator.saveReviewers(field); diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 4fe713c148b00a7655161d05f697ab57c8bfc540..bf03e9e3edf329d9062f91682d7b1a4906863bac 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -13,7 +13,7 @@ specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot - visual_review_bot migration_bot automation_bot security_policy_bot admin_bot suggested_reviewers_bot + visual_review_bot migration_bot automation_bot security_policy_bot admin_bot service_account llm_bot placeholder duo_code_review_bot import_user]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 7fe9eebb940d440d3d50f524da0c2465938f4113..9733551c34837c6a647eb7c7a23d7ed12254c94f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -6688,24 +6688,6 @@ def transition! end end - describe '#can_suggest_reviewers?' do - let_it_be(:merge_request) { build(:merge_request, :opened, project: project) } - - subject(:can_suggest_reviewers) { merge_request.can_suggest_reviewers? } - - it 'returns false' do - expect(can_suggest_reviewers).to be(false) - end - end - - describe '#suggested_reviewer_users' do - let_it_be(:merge_request) { build(:merge_request, project: project) } - - subject(:suggested_reviewer_users) { merge_request.suggested_reviewer_users } - - it { is_expected.to be_empty } - end - describe '#hidden?', feature_category: :insider_threat do let_it_be(:author) { create(:user) } let(:merge_request) { build_stubbed(:merge_request, author: author) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 037deb450288478cb8b3ef3cbf050b99b2557ad4..de01ad04701c0891ec9ce0032aba6fb6e019dedf 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -9676,22 +9676,6 @@ def create_project_statistics_with_size(project, size) end end - describe '#can_suggest_reviewers?' do - let_it_be(:project) { create(:project) } - - subject(:can_suggest_reviewers) { project.can_suggest_reviewers? } - - it { is_expected.to be(false) } - end - - describe '#suggested_reviewers_available?' do - let_it_be(:project) { create(:project) } - - subject(:suggested_reviewers_available) { project.suggested_reviewers_available? } - - it { is_expected.to be(false) } - end - describe '.cascading_with_parent_namespace' do let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) } let_it_be_with_reload(:subgroup) { create(:group, parent: group) } diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index fd6f423ecdfb2e8f5c7b9f4bbb0ad90ead8640b4..9e08c318fc3606668f2218555e1367fba7a8d455 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -176,7 +176,6 @@ project_setting: - target_platforms - selective_code_owner_removals - show_diff_preview_in_email - - suggested_reviewers_enabled - mirror_branch_regex - allow_pipeline_trigger_approve_deployment - pages_unique_domain_enabled diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index b09faae4cac6533bc1b0c1dc9212339f72706827..f0fdcfa10cc8b74baf5170dd61129e18c470c391 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -359,12 +359,10 @@ 'MergeRequestCleanupRefsWorker' => 3, 'MergeRequestMergeabilityCheckWorker' => 3, 'MergeRequestResetApprovalsWorker' => 3, - 'MergeRequests::CaptureSuggestedReviewersAcceptedWorker' => 3, 'MergeRequests::CleanupRefWorker' => 3, 'MergeRequests::CreatePipelineWorker' => 3, 'MergeRequests::DeleteSourceBranchWorker' => 3, 'MergeRequests::DuoCodeReviewChatWorker' => 3, - 'MergeRequests::FetchSuggestedReviewersWorker' => 3, 'MergeRequests::HandleAssigneesChangeWorker' => 3, 'MergeRequests::MergeabilityCheckBatchWorker' => 3, 'MergeRequests::ResolveTodosWorker' => 3, @@ -410,7 +408,6 @@ 'ProjectExportWorker' => false, 'ProjectImportScheduleWorker' => 1, 'ProjectTemplateExportWorker' => false, - 'Projects::DeregisterSuggestedReviewersProjectWorker' => 3, 'Projects::DisableLegacyOpenSourceLicenseForInactiveProjectsWorker' => 3, 'Projects::GitGarbageCollectWorker' => false, 'Projects::ImportExport::RelationExportWorker' => 6, @@ -420,7 +417,6 @@ 'Projects::ScheduleBulkRepositoryShardMovesWorker' => 3, 'Projects::UpdateRepositoryStorageWorker' => 3, 'Projects::RefreshBuildArtifactsSizeStatisticsWorker' => 0, - 'Projects::RegisterSuggestedReviewersProjectWorker' => 3, 'PropagateIntegrationGroupWorker' => 3, 'PropagateIntegrationInheritDescendantWorker' => 3, 'PropagateIntegrationInheritWorker' => 3, diff --git a/tooling/danger/internal_users.rb b/tooling/danger/internal_users.rb index 6614ee668fb7f19b1db4bfe5d3ebc6188eb1d51f..57ff942c9a2d8a998a74355a3559e4c7703bd609 100644 --- a/tooling/danger/internal_users.rb +++ b/tooling/danger/internal_users.rb @@ -40,7 +40,6 @@ module InternalUsers automation_bot security_policy_bot admin_bot - suggested_reviewers_bot llm_bot placeholder duo_code_review_bot