From 25ba0c04e90a470bfdf3fe3a5b044a73157565d2 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 3 Jan 2024 00:07:15 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../organizations/organizations_resolver.rb | 19 +++ app/graphql/types/query_type.rb | 5 + app/models/issue_email_participant.rb | 3 + .../issue_email_participants/base_service.rb | 45 ++++++ .../create_service.rb | 42 +---- .../destroy_service.rb | 44 ++++++ app/services/system_note_service.rb | 4 +- .../system_notes/issuables_service.rb | 2 +- .../i_quickactions_remove_email_multiple.yml | 24 +++ .../i_quickactions_remove_email_single.yml | 24 +++ ...quickactions_remove_email_multiple_28d.yml | 27 ++++ ...i_quickactions_remove_email_single_28d.yml | 27 ++++ ...1227103059_replace_fk_on_epics_issue_id.rb | 20 +++ ...k_epics_issue_id_with_on_delete_nullify.rb | 16 ++ ...20231227104711_remove_fk_epics_issue_id.rb | 21 +++ db/schema_migrations/20231227103059 | 1 + db/schema_migrations/20231227104408 | 1 + db/schema_migrations/20231227104711 | 1 + db/structure.sql | 6 +- doc/api/graphql/reference/index.md | 14 ++ .../blueprints/ai_gateway/index.md | 6 +- .../troubleshooting_dependency_scanning.md | 4 - doc/user/project/merge_requests/changes.md | 7 + .../merge_when_pipeline_succeeds.md | 3 + lib/gitlab/quick_actions/issue_actions.rb | 20 +++ .../quick_action_activity_unique_counter.rb | 2 + locale/gitlab.pot | 16 +- qa/Gemfile | 2 +- qa/Gemfile.lock | 6 +- spec/models/issue_email_participant_spec.rb | 16 +- .../organizations/organizations_query_spec.rb | 56 +++++++ .../create_service_spec.rb | 10 +- .../destroy_service_spec.rb | 147 ++++++++++++++++++ .../quick_actions/interpret_service_spec.rb | 98 +++++++++++- spec/services/system_note_service_spec.rb | 12 ++ .../system_notes/issuables_service_spec.rb | 8 + .../types/query_type_shared_context.rb | 2 + 37 files changed, 701 insertions(+), 60 deletions(-) create mode 100644 app/graphql/resolvers/organizations/organizations_resolver.rb create mode 100644 app/services/issue_email_participants/base_service.rb create mode 100644 app/services/issue_email_participants/destroy_service.rb create mode 100644 config/events/i_quickactions_remove_email_multiple.yml create mode 100644 config/events/i_quickactions_remove_email_single.yml create mode 100644 config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_multiple_28d.yml create mode 100644 config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_single_28d.yml create mode 100644 db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb create mode 100644 db/migrate/20231227104408_validate_fk_epics_issue_id_with_on_delete_nullify.rb create mode 100644 db/migrate/20231227104711_remove_fk_epics_issue_id.rb create mode 100644 db/schema_migrations/20231227103059 create mode 100644 db/schema_migrations/20231227104408 create mode 100644 db/schema_migrations/20231227104711 create mode 100644 spec/requests/api/graphql/organizations/organizations_query_spec.rb create mode 100644 spec/services/issue_email_participants/destroy_service_spec.rb diff --git a/app/graphql/resolvers/organizations/organizations_resolver.rb b/app/graphql/resolvers/organizations/organizations_resolver.rb new file mode 100644 index 00000000000..ab21a84645b --- /dev/null +++ b/app/graphql/resolvers/organizations/organizations_resolver.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Resolvers + module Organizations + class OrganizationsResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + + type Types::Organizations::OrganizationType.connection_type, null: true + authorize :read_organization + + def resolve + # For the Organization MVC, all the organizations are public. We need to change this to only accessible + # organizations once we start supporting private organizations. + # See https://gitlab.com/groups/gitlab-org/-/epics/10649. + ::Organizations::Organization.all + end + end + end +end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 0e39ff2c030..2042eee0116 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -122,6 +122,11 @@ module Types resolver: Resolvers::Organizations::OrganizationResolver, description: "Find an organization.", alpha: { milestone: '16.4' } + field :organizations, Types::Organizations::OrganizationType.connection_type, + null: true, + resolver: Resolvers::Organizations::OrganizationsResolver, + description: "List organizations.", + alpha: { milestone: '16.8' } field :package, description: 'Find a package. This field can only be resolved for one query in any single request. Returns `null` if a package has no `default` status.', resolver: Resolvers::PackageDetailsResolver diff --git a/app/models/issue_email_participant.rb b/app/models/issue_email_participant.rb index 9d7e2afa1d9..bb03b3d72e6 100644 --- a/app/models/issue_email_participant.rb +++ b/app/models/issue_email_participant.rb @@ -3,6 +3,7 @@ class IssueEmailParticipant < ApplicationRecord include BulkInsertSafe include Presentable + include CaseSensitivity belongs_to :issue @@ -10,6 +11,8 @@ class IssueEmailParticipant < ApplicationRecord validates :issue, presence: true validate :validate_email_format + scope :with_emails, ->(emails) { iwhere(email: emails) } + def validate_email_format self.errors.add(:email, I18n.t(:invalid, scope: 'valid_email.validations.email')) unless ValidateEmail.valid?(self.email) end diff --git a/app/services/issue_email_participants/base_service.rb b/app/services/issue_email_participants/base_service.rb new file mode 100644 index 00000000000..c9847bae537 --- /dev/null +++ b/app/services/issue_email_participants/base_service.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module IssueEmailParticipants + class BaseService < ::BaseProjectService + MAX_NUMBER_OF_EMAILS = 6 + + attr_reader :target, :emails + + def initialize(target:, current_user:, emails:) + super(project: target.project, current_user: current_user) + + @target = target + @emails = emails + end + + private + + def response_from_guard_checks + return error_feature_flag unless Feature.enabled?(:issue_email_participants, target.project) + return error_underprivileged unless current_user.can?(:"admin_#{target.to_ability_name}", target) + + nil + end + + def add_system_note(emails) + message = format(system_note_text, emails: emails.to_sentence) + ::SystemNoteService.email_participants(target, project, current_user, message) + + message + end + + def error(message) + ServiceResponse.error(message: message) + end + + def error_feature_flag + # Don't translate feature flag error because it's temporary. + error("Feature flag issue_email_participants is not enabled for this project.") + end + + def error_underprivileged + error(_("You don't have permission to manage email participants.")) + end + end +end diff --git a/app/services/issue_email_participants/create_service.rb b/app/services/issue_email_participants/create_service.rb index 52c59b2b8fe..aac396ba226 100644 --- a/app/services/issue_email_participants/create_service.rb +++ b/app/services/issue_email_participants/create_service.rb @@ -1,25 +1,15 @@ # frozen_string_literal: true module IssueEmailParticipants - class CreateService < ::BaseProjectService + class CreateService < BaseService include Gitlab::Utils::StrongMemoize - MAX_NUMBER_OF_EMAILS = 6 MAX_NUMBER_OF_RECORDS = 10 - attr_reader :target, :emails - - def initialize(target:, current_user:, emails:) - super(project: target.project, current_user: current_user) - - @target = target - @emails = emails - end - def execute - return error_feature_flag unless Feature.enabled?(:issue_email_participants, target.project) - return error_underprivileged unless current_user.can?(:"admin_#{target.to_ability_name}", target) - return error_no_participants unless emails.present? + response = response_from_guard_checks + return response unless response.nil? + return error_no_participants_added unless emails.present? added_emails = add_participants(deduplicate_and_limit_emails) @@ -27,7 +17,7 @@ module IssueEmailParticipants message = add_system_note(added_emails) ServiceResponse.success(message: message.upcase_first << ".") else - error_no_participants + error_no_participants_added end end @@ -60,13 +50,6 @@ module IssueEmailParticipants added_emails end - def add_system_note(added_emails) - message = format(_("added %{emails}"), emails: added_emails.to_sentence) - ::SystemNoteService.add_email_participants(target, project, current_user, message) - - message - end - def existing_emails target.email_participants_emails_downcase end @@ -78,20 +61,11 @@ module IssueEmailParticipants end end - def error(message) - ServiceResponse.error(message: message) + def system_note_text + _("added %{emails}") end - def error_feature_flag - # Don't translate feature flag error because it's temporary. - error("Feature flag issue_email_participants is not enabled for this project.") - end - - def error_underprivileged - error(_("You don't have permission to add email participants.")) - end - - def error_no_participants + def error_no_participants_added error(_("No email participants were added. Either none were provided, or they already exist.")) end end diff --git a/app/services/issue_email_participants/destroy_service.rb b/app/services/issue_email_participants/destroy_service.rb new file mode 100644 index 00000000000..8cd0178da00 --- /dev/null +++ b/app/services/issue_email_participants/destroy_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module IssueEmailParticipants + class DestroyService < BaseService + def execute + response = response_from_guard_checks + return response unless response.nil? + return error_no_participants_removed unless emails.present? + + removed_emails = remove_participants(emails.first(MAX_NUMBER_OF_EMAILS)) + + if removed_emails.any? + message = add_system_note(removed_emails) + ServiceResponse.success(message: message.upcase_first << ".") + else + error_no_participants_removed + end + end + + private + + def remove_participants(emails_to_remove) + participants = target + .issue_email_participants + .with_emails(emails_to_remove) + .load # to avoid additional query + + emails = participants.map(&:email) + return [] if emails.empty? + + participants.delete_all + + emails + end + + def system_note_text + _("removed %{emails}") + end + + def error_no_participants_removed + error(_("No email participants were removed. Either none were provided, or they don't exist.")) + end + end +end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 5f71b7ac9e9..9175d91119e 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -282,8 +282,8 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).mark_canonical_issue_of_duplicate(duplicate_issue) end - def add_email_participants(noteable, project, author, body) - ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).add_email_participants(body) + def email_participants(noteable, project, author, body) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).email_participants(body) end def discussion_lock(issuable, author) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index c584d5ccca3..7857bf20c8f 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -431,7 +431,7 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end - def add_email_participants(body) + def email_participants(body) create_note(NoteSummary.new(noteable, project, author, body)) end diff --git a/config/events/i_quickactions_remove_email_multiple.yml b/config/events/i_quickactions_remove_email_multiple.yml new file mode 100644 index 00000000000..707624db394 --- /dev/null +++ b/config/events/i_quickactions_remove_email_multiple.yml @@ -0,0 +1,24 @@ +--- +description: Count usage of /remove_email quickaction with multiple arguments +category: InternalEventTracking +action: i_quickactions_remove_email_multiple +label_description: +property_description: +value_description: +extra_properties: +identifiers: +- project +- user +- namespace +product_section: seg +product_stage: service +product_group: respond +milestone: "16.7" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138178 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate diff --git a/config/events/i_quickactions_remove_email_single.yml b/config/events/i_quickactions_remove_email_single.yml new file mode 100644 index 00000000000..fea63d2ecd3 --- /dev/null +++ b/config/events/i_quickactions_remove_email_single.yml @@ -0,0 +1,24 @@ +--- +description: Count usage of /remove_email quickaction with a single argument +category: InternalEventTracking +action: i_quickactions_remove_email_single +label_description: +property_description: +value_description: +extra_properties: +identifiers: +- project +- user +- namespace +product_section: seg +product_stage: service +product_group: respond +milestone: "16.7" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138178 +distributions: +- ce +- ee +tiers: +- free +- premium +- ultimate diff --git a/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_multiple_28d.yml b/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_multiple_28d.yml new file mode 100644 index 00000000000..fdccb143c14 --- /dev/null +++ b/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_multiple_28d.yml @@ -0,0 +1,27 @@ +--- +key_path: count_distinct_user_id_from_i_quickactions_remove_email_multiple_28d +description: Unique users using the /remove_email quick action to remove multiple email participants from an issue within 28 days +product_section: seg +product_stage: service +product_group: respond +performance_indicator_type: [] +value_type: number +status: active +milestone: "16.7" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138178 +time_frame: 28d +data_source: internal_events +data_category: optional +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - i_quickactions_remove_email_multiple +events: + - name: i_quickactions_remove_email_multiple + unique: user.id diff --git a/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_single_28d.yml b/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_single_28d.yml new file mode 100644 index 00000000000..2be6ace6b38 --- /dev/null +++ b/config/metrics/counts_28d/count_distinct_user_id_from_i_quickactions_remove_email_single_28d.yml @@ -0,0 +1,27 @@ +--- +key_path: count_distinct_user_id_from_i_quickactions_remove_email_single_28d +description: Unique users using the /remove_email quick action to remove a single email participant from an issue within 28 days +product_section: seg +product_stage: service +product_group: respond +performance_indicator_type: [] +value_type: number +status: active +milestone: "16.7" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138178 +time_frame: 28d +data_source: internal_events +data_category: optional +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +options: + events: + - i_quickactions_remove_email_single +events: + - name: i_quickactions_remove_email_single + unique: user.id diff --git a/db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb b/db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb new file mode 100644 index 00000000000..e5dd4e868f0 --- /dev/null +++ b/db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class ReplaceFkOnEpicsIssueId < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '16.8' + + FK_NAME = :fk_epics_issue_id_with_on_delete_nullify + + def up + # This will replace the existing fk_893ee302e5 + add_concurrent_foreign_key(:epics, :issues, column: :issue_id, on_delete: :nullify, validate: false, name: FK_NAME) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(:epics, column: :issue_id, on_delete: :nullify, name: FK_NAME) + end + end +end diff --git a/db/migrate/20231227104408_validate_fk_epics_issue_id_with_on_delete_nullify.rb b/db/migrate/20231227104408_validate_fk_epics_issue_id_with_on_delete_nullify.rb new file mode 100644 index 00000000000..9f7601a704b --- /dev/null +++ b/db/migrate/20231227104408_validate_fk_epics_issue_id_with_on_delete_nullify.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class ValidateFkEpicsIssueIdWithOnDeleteNullify < Gitlab::Database::Migration[2.2] + milestone '16.8' + + FK_NAME = :fk_epics_issue_id_with_on_delete_nullify + + # foreign key added in db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb + def up + validate_foreign_key(:epics, :issue_id, name: FK_NAME) + end + + def down + # no-op + end +end diff --git a/db/migrate/20231227104711_remove_fk_epics_issue_id.rb b/db/migrate/20231227104711_remove_fk_epics_issue_id.rb new file mode 100644 index 00000000000..29752e8741c --- /dev/null +++ b/db/migrate/20231227104711_remove_fk_epics_issue_id.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class RemoveFkEpicsIssueId < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '16.8' + + FK_NAME = :fk_893ee302e5 + + # new foreign key added in db/migrate/20231227103059_replace_fk_on_epics_issue_id.rb + # and validated in db/migrate/20231227104408_validate_fk_epics_issue_id_with_on_delete_nullify.rb + def up + with_lock_retries do + remove_foreign_key_if_exists(:epics, column: :issue_id, on_delete: :cascade, name: FK_NAME) + end + end + + def down + add_concurrent_foreign_key(:epics, :issues, column: :issue_id, on_delete: :cascade, validate: false, name: FK_NAME) + end +end diff --git a/db/schema_migrations/20231227103059 b/db/schema_migrations/20231227103059 new file mode 100644 index 00000000000..50f5df69ad5 --- /dev/null +++ b/db/schema_migrations/20231227103059 @@ -0,0 +1 @@ +d9f963d252141e1fe5bab5dd8f6b67964253788771d45bf343459014864919b5 \ No newline at end of file diff --git a/db/schema_migrations/20231227104408 b/db/schema_migrations/20231227104408 new file mode 100644 index 00000000000..1ac79ac87b1 --- /dev/null +++ b/db/schema_migrations/20231227104408 @@ -0,0 +1 @@ +ffeb813c94ff0fdefae162e32f56083125248e8b3f34535f9f4252dcb09b1412 \ No newline at end of file diff --git a/db/schema_migrations/20231227104711 b/db/schema_migrations/20231227104711 new file mode 100644 index 00000000000..006bb2e1865 --- /dev/null +++ b/db/schema_migrations/20231227104711 @@ -0,0 +1 @@ +22e7c4fe8821e07a6ec5c48c32007849faa673eee203689dd51753bf38004077 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 77723578965..e2fbc5a6e23 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -37895,9 +37895,6 @@ ALTER TABLE ONLY bulk_import_entities ALTER TABLE ONLY requirements_management_test_reports ADD CONSTRAINT fk_88f30752fc FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE; -ALTER TABLE ONLY epics - ADD CONSTRAINT fk_893ee302e5 FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE; - ALTER TABLE ONLY issues ADD CONSTRAINT fk_899c8f3231 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -38393,6 +38390,9 @@ ALTER TABLE ONLY approval_group_rules_groups ALTER TABLE ONLY emails ADD CONSTRAINT fk_emails_user_id FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY epics + ADD CONSTRAINT fk_epics_issue_id_with_on_delete_nullify FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE SET NULL; + ALTER TABLE ONLY clusters ADD CONSTRAINT fk_f05c5e5a42 FOREIGN KEY (management_project_id) REFERENCES projects(id) ON DELETE SET NULL; diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 57a33473b19..6318ef85f85 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -687,6 +687,20 @@ Returns [`Organization`](#organization). | ---- | ---- | ----------- | | `id` | [`OrganizationsOrganizationID!`](#organizationsorganizationid) | ID of the organization. | +### `Query.organizations` + +List organizations. + +WARNING: +**Introduced** in 16.8. +This feature is an Experiment. It can be changed or removed at any time. + +Returns [`OrganizationConnection`](#organizationconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#connection-pagination-arguments): +`before: String`, `after: String`, `first: Int`, and `last: Int`. + ### `Query.package` Find a package. This field can only be resolved for one query in any single request. Returns `null` if a package has no `default` status. diff --git a/doc/architecture/blueprints/ai_gateway/index.md b/doc/architecture/blueprints/ai_gateway/index.md index c09f8aaa621..e40861139d6 100644 --- a/doc/architecture/blueprints/ai_gateway/index.md +++ b/doc/architecture/blueprints/ai_gateway/index.md @@ -103,7 +103,7 @@ GitLab instances, JSON API, and gRPC differ on these items: | + A new Ruby-gRPC server for vscode: likely faster because we can limit dependencies to load ([modular monolith](https://gitlab.com/gitlab-org/gitlab/-/issues/365293)) | - Existing Grape API for vscode: meaning slow boot time and unneeded resources loaded | | + Bi-directional streaming | - Straight forward way to stream requests and responses (could still be added) | | - A new Python-gRPC server: we don't have experience running gRPC-Python servers | + Existing Python fastapi server, already running for Code Suggestions to extend | -| - Hard to pass on unknown messages from vscode through GitLab to ai-gateway | + Easier support for newer vscode + newer ai-gatway, through old GitLab instance | +| - Hard to pass on unknown messages from vscode through GitLab to ai-gateway | + Easier support for newer VS Code + newer AI-gateway, through old GitLab instance | | - Unknown support for gRPC in other clients (vscode, jetbrains, other editors) | + Support in all external clients | | - Possible protocol mismatch (VSCode --REST--> Rails --gRPC--> AI gateway) | + Same protocol across the stack | @@ -264,7 +264,7 @@ Another example use case includes 2 versions of a prompt passed in the `prompt_c a field in the gateway, and keep them around for at least 2 major versions of GitLab.** -A good practise that might help support backwards compatibility is to provide building blocks for the prompt inside the `prompt_components` rather then a complete prompt. By moving responsibility of compiling prompt out of building blocks on the AI-Gateway, one can achive more flexibility in terms of prompt adjustments in the future. +A good practice that might help support backward compatibility: provide building blocks for the prompt inside the `prompt_components`, rather then a complete prompt. By moving responsibility of compiling the prompt out of building blocks and into the AI-Gateway, more flexible prompt adjustments are possible in the future. #### Example feature: Code Suggestions @@ -503,7 +503,7 @@ It is deployed to a Kubernetes cluster in it's own project. There is a staging environment that is currently used directly by engineers for testing. -In the future, this will be deloyed using +In the future, this will be deployed using [Runway](https://gitlab.com/gitlab-com/gl-infra/platform/runway/). At that time, there will be a production and staging deployment. The staging deployment can be used for automated QA-runs that will have diff --git a/doc/user/application_security/dependency_scanning/troubleshooting_dependency_scanning.md b/doc/user/application_security/dependency_scanning/troubleshooting_dependency_scanning.md index 77579a04c7e..dae72e1a555 100644 --- a/doc/user/application_security/dependency_scanning/troubleshooting_dependency_scanning.md +++ b/doc/user/application_security/dependency_scanning/troubleshooting_dependency_scanning.md @@ -65,10 +65,6 @@ Consider updating to Docker `19.03.1` or greater. Older versions are not affected. Read more in [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/13830#note_211354992 "Current SAST container fails"). -## Getting warning message `gl-dependency-scanning-report.json: no matching files` - -For information, see the [general Application Security troubleshooting section](../../../ci/jobs/job_artifacts_troubleshooting.md#error-message-no-files-to-upload). - ## Limitation when using rules:exists The [dependency scanning CI template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml) diff --git a/doc/user/project/merge_requests/changes.md b/doc/user/project/merge_requests/changes.md index 780041ac411..094d2cf5730 100644 --- a/doc/user/project/merge_requests/changes.md +++ b/doc/user/project/merge_requests/changes.md @@ -162,6 +162,13 @@ per conflicted file on the merge request diff: ![Example of a conflict alert shown in a merge request diff](img/conflict_ui_v15_6.png) +## Show scanner findings in diff **(ULTIMATE ALL)** + +You can show scanner findings in the diff. For details, see: + +- [Code Quality findings](../../../ci/testing/code_quality.md#merge-request-changes-view) +- [Static Analysis findings](../../application_security/sast/index.md#merge-request-changes-view) + ## Add a comment to a merge request file > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123515) in GitLab 16.1 [with a flag](../../../administration/feature_flags.md) named `comment_on_files`. Enabled by default. diff --git a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md index a9cad78449b..3a2729bd64b 100644 --- a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md +++ b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md @@ -23,6 +23,9 @@ author can either retry any failed jobs, or push new commits to fix the failure: - If a retried job succeeds on the second try, the merge request is merged. - If new commits are added to the merge request, GitLab cancels the request to ensure the new changes are reviewed before merge. +- If new commits are added to the target branch of the merge request and + fast-forward only merge request is configured, GitLab cancels the request + to prevent merge conflicts. ## Auto-merge a merge request diff --git a/lib/gitlab/quick_actions/issue_actions.rb b/lib/gitlab/quick_actions/issue_actions.rb index c79432f36cc..b3f56e8590a 100644 --- a/lib/gitlab/quick_actions/issue_actions.rb +++ b/lib/gitlab/quick_actions/issue_actions.rb @@ -240,6 +240,26 @@ module Gitlab @execution_message[:invite_email] = response.message end + desc { _('Remove email participant(s)') } + explanation { _('Removes email participant(s).') } + params 'email1@example.com email2@example.com (up to 6 emails)' + types Issue + condition do + quick_action_target.persisted? && + Feature.enabled?(:issue_email_participants, parent) && + current_user.can?(:"admin_#{quick_action_target.to_ability_name}", quick_action_target) && + quick_action_target.issue_email_participants.any? + end + command :remove_email do |emails = ""| + response = ::IssueEmailParticipants::DestroyService.new( + target: quick_action_target, + current_user: current_user, + emails: emails.split(' ') + ).execute + + @execution_message[:remove_email] = response.message + end + desc { _('Promote issue to incident') } explanation { _('Promotes issue to incident') } execution_message { _('Issue has been promoted to incident') } diff --git a/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter.rb index 2e92afb5439..6e4c5d4e845 100644 --- a/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/quick_action_activity_unique_counter.rb @@ -37,6 +37,8 @@ module Gitlab event_name_for_unlabel(args) when 'invite_email' 'invite_email' + event_name_quantifier(args.split) + when 'remove_email' + 'remove_email' + event_name_quantifier(args.split) else name end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 427a1fcd0d0..c511d6bd5b5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32229,6 +32229,9 @@ msgstr "" msgid "No email participants were added. Either none were provided, or they already exist." msgstr "" +msgid "No email participants were removed. Either none were provided, or they don't exist." +msgstr "" + msgid "No endpoint provided" msgstr "" @@ -40280,6 +40283,9 @@ msgstr "" msgid "Remove due date" msgstr "" +msgid "Remove email participant(s)" +msgstr "" + msgid "Remove epic reference" msgstr "" @@ -40463,6 +40469,9 @@ msgstr "" msgid "Removes an issue from an epic." msgstr "" +msgid "Removes email participant(s)." +msgstr "" + msgid "Removes link with %{issue_ref}." msgstr "" @@ -56189,10 +56198,10 @@ msgstr "" msgid "You don't have any recent searches" msgstr "" -msgid "You don't have permission to add email participants." +msgid "You don't have permission to approve this deployment. Contact the project or group owner for help." msgstr "" -msgid "You don't have permission to approve this deployment. Contact the project or group owner for help." +msgid "You don't have permission to manage email participants." msgstr "" msgid "You don't have permission to view this epic" @@ -58870,6 +58879,9 @@ msgstr "" msgid "remove weight" msgstr "" +msgid "removed %{emails}" +msgstr "" + msgid "removed a %{link_type} link" msgstr "" diff --git a/qa/Gemfile b/qa/Gemfile index 65ee52e44aa..ca919fd7c00 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -18,7 +18,7 @@ gem 'rspec-retry', '~> 0.6.2', require: 'rspec/retry' gem 'rspec_junit_formatter', '~> 0.6.0' gem 'faker', '~> 3.2', '>= 3.2.2' gem 'knapsack', '~> 4.0' -gem 'parallel_tests', '~> 4.3' +gem 'parallel_tests', '~> 4.4' gem 'rotp', '~> 6.3.0' gem 'parallel', '~> 1.24' gem 'rainbow', '~> 3.1.1' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index eb847eb04ec..8f9f3a7ccda 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -222,7 +222,7 @@ GEM sawyer (~> 0.9) os (1.1.4) parallel (1.24.0) - parallel_tests (4.3.0) + parallel_tests (4.4.0) parallel parser (3.2.2.1) ast (~> 2.4.1) @@ -367,7 +367,7 @@ DEPENDENCIES nokogiri (~> 1.15, >= 1.15.5) octokit (~> 8.0.0) parallel (~> 1.24) - parallel_tests (~> 4.3) + parallel_tests (~> 4.4) pry-byebug (~> 3.10.1) rainbow (~> 3.1.1) rake (~> 13, >= 13.1.0) @@ -385,4 +385,4 @@ DEPENDENCIES zeitwerk (~> 2.6, >= 2.6.12) BUNDLED WITH - 2.5.1 + 2.5.2 diff --git a/spec/models/issue_email_participant_spec.rb b/spec/models/issue_email_participant_spec.rb index 8ddc9a5f478..760af974275 100644 --- a/spec/models/issue_email_participant_spec.rb +++ b/spec/models/issue_email_participant_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe IssueEmailParticipant do +RSpec.describe IssueEmailParticipant, feature_category: :service_desk do describe "Associations" do it { is_expected.to belong_to(:issue) } end @@ -27,4 +27,18 @@ RSpec.describe IssueEmailParticipant do expect(subject).to be_invalid end end + + describe 'Scopes' do + describe '.with_emails' do + let!(:participant) { create(:issue_email_participant, email: 'user@example.com') } + let!(:participant1) { create(:issue_email_participant, email: 'user1@example.com') } + let!(:participant2) { create(:issue_email_participant, email: 'user2@example.com') } + + it 'returns only participant with matching emails' do + expect(described_class.with_emails([participant.email, participant1.email])).to match_array( + [participant, participant1] + ) + end + end + end end diff --git a/spec/requests/api/graphql/organizations/organizations_query_spec.rb b/spec/requests/api/graphql/organizations/organizations_query_spec.rb new file mode 100644 index 00000000000..12d81ed7412 --- /dev/null +++ b/spec/requests/api/graphql/organizations/organizations_query_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting organizations information', feature_category: :cell do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + + let(:query) { graphql_query_for(:organizations, organizations_fields) } + let(:organizations) { graphql_data_at(:organizations, :nodes) } + let(:organizations_fields) do + <<~FIELDS + nodes { + id + path + } + FIELDS + end + + before_all { create_list(:organization, 3) } + + subject(:request_organization) { post_graphql(query, current_user: current_user) } + + context 'without authenticated user' do + let(:current_user) { nil } + + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + end + + context 'with authenticated user' do + let(:current_user) { user } + + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + + it_behaves_like 'sorted paginated query' do + include_context 'no sort argument' + + let(:first_param) { 2 } + let(:data_path) { [:organizations] } + let(:all_records) { Organizations::Organization.order(id: :desc).map { |o| global_id_of(o).to_s } } + end + + def pagination_query(params) + graphql_query_for(:organizations, params, "#{page_info} nodes { id }") + end + end +end diff --git a/spec/services/issue_email_participants/create_service_spec.rb b/spec/services/issue_email_participants/create_service_spec.rb index fcfdeeb08f3..dc8d5a6ea74 100644 --- a/spec/services/issue_email_participants/create_service_spec.rb +++ b/spec/services/issue_email_participants/create_service_spec.rb @@ -41,8 +41,8 @@ RSpec.describe IssueEmailParticipants::CreateService, feature_category: :service let(:expected_emails) { emails } let(:error_feature_flag) { "Feature flag issue_email_participants is not enabled for this project." } - let(:error_underprivileged) { _("You don't have permission to add email participants.") } - let(:error_no_participants) do + let(:error_underprivileged) { _("You don't have permission to manage email participants.") } + let(:error_no_participants_added) do _("No email participants were added. Either none were provided, or they already exist.") end @@ -58,7 +58,7 @@ RSpec.describe IssueEmailParticipants::CreateService, feature_category: :service end context 'when no emails are provided' do - let(:error_message) { error_no_participants } + let(:error_message) { error_no_participants_added } it_behaves_like 'a failed service execution' end @@ -69,7 +69,7 @@ RSpec.describe IssueEmailParticipants::CreateService, feature_category: :service it_behaves_like 'a successful service execution' context 'when email is already a participant of the issue' do - let(:error_message) { error_no_participants } + let(:error_message) { error_no_participants_added } before do issue.issue_email_participants.create!(email: emails.first) @@ -89,7 +89,7 @@ RSpec.describe IssueEmailParticipants::CreateService, feature_category: :service end let(:emails) { ['over-max@example.com'] } - let(:error_message) { error_no_participants } + let(:error_message) { error_no_participants_added } it_behaves_like 'a failed service execution' diff --git a/spec/services/issue_email_participants/destroy_service_spec.rb b/spec/services/issue_email_participants/destroy_service_spec.rb new file mode 100644 index 00000000000..70e09bb8d3b --- /dev/null +++ b/spec/services/issue_email_participants/destroy_service_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe IssueEmailParticipants::DestroyService, feature_category: :service_desk do + shared_examples 'a successful service execution' do + it 'removes participants', :aggregate_failures do + expect(response).to be_success + + issue.reset + note = issue.notes.last + expect(note.system?).to be true + expect(note.author).to eq(user) + + participants_emails = issue.email_participants_emails_downcase + + expected_emails.each do |email| + expect(participants_emails).not_to include(email) + expect(response.message).to include(email) + expect(note.note).to include(email) + end + end + end + + shared_examples 'a failed service execution' do + it 'returns error ServiceResponse with message', :aggregate_failures do + expect(response).to be_error + expect(response.message).to eq(error_message) + end + end + + describe '#execute' do + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:issue) { create(:issue, project: project) } + + let(:emails) { nil } + let(:service) { described_class.new(target: issue, current_user: user, emails: emails) } + let(:expected_emails) { emails } + + let(:error_feature_flag) { "Feature flag issue_email_participants is not enabled for this project." } + let(:error_underprivileged) { _("You don't have permission to manage email participants.") } + let(:error_no_participants_removed) do + _("No email participants were removed. Either none were provided, or they don't exist.") + end + + subject(:response) { service.execute } + + context 'when the user is not a project member' do + let(:error_message) { error_underprivileged } + + it_behaves_like 'a failed service execution' + end + + context 'when user has reporter role in project' do + before_all do + project.add_reporter(user) + end + + context 'when no emails are provided' do + let(:error_message) { error_no_participants_removed } + + it_behaves_like 'a failed service execution' + end + + context 'when one email is provided' do + let(:emails) { ['user@example.com'] } + let(:error_message) { error_no_participants_removed } + + it_behaves_like 'a failed service execution' + + context 'when email is a participant of the issue' do + before do + issue.issue_email_participants.create!(email: 'user@example.com') + end + + it_behaves_like 'a successful service execution' + + context 'when email is formatted in a different case' do + let(:emails) { ['USER@example.com'] } + let(:expected_emails) { emails.map(&:downcase) } + let(:error_message) { error_no_participants_removed } + + it_behaves_like 'a successful service execution' + end + end + end + + context 'when multiple emails are provided' do + let(:emails) { ['user@example.com', 'user2@example.com'] } + let(:error_message) { error_no_participants_removed } + + it_behaves_like 'a failed service execution' + + context 'when duplicate email provided' do + let(:emails) { ['user@example.com', 'user@example.com'] } + let(:expected_emails) { emails[...-1] } + + it_behaves_like 'a failed service execution' + end + + context 'when one email is a participant of the issue' do + let(:expected_emails) { emails[...-1] } + + before do + issue.issue_email_participants.create!(email: emails.first) + end + + it_behaves_like 'a successful service execution' + end + + context 'when both emails are a participant of the issue' do + before do + emails.each do |email| + issue.issue_email_participants.create!(email: email) + end + end + + it_behaves_like 'a successful service execution' + end + end + + context 'when more than the allowed number of emails are provided' do + let(:emails) { (1..7).map { |i| "user#{i}@example.com" } } + let(:expected_emails) { emails[...-1] } + + before do + emails.each do |email| + issue.issue_email_participants.create!(email: email) + end + end + + it_behaves_like 'a successful service execution' + end + end + + context 'when feature flag issue_email_participants is disabled' do + let(:error_message) { error_feature_flag } + + before do + stub_feature_flags(issue_email_participants: false) + end + + it_behaves_like 'a failed service execution' + end + end +end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index e96a7c7fc5c..e04f14e752f 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -2324,7 +2324,7 @@ RSpec.describe QuickActions::InterpretService, feature_category: :team_planning end end - context 'invite_email command' do + describe 'invite_email command' do let_it_be(:issuable) { issue } it_behaves_like 'failed command', "No email participants were added. Either none were provided, or they already exist." do @@ -2454,6 +2454,102 @@ RSpec.describe QuickActions::InterpretService, feature_category: :team_planning end end + describe 'remove_email command' do + let_it_be_with_reload(:issuable) { issue } + + it 'is not part of the available commands' do + expect(service.available_commands(issuable)).not_to include(a_hash_including(name: :remove_email)) + end + + context 'with existing email participant' do + let(:content) { '/remove_email user@example.com' } + + subject(:remove_email) { service.execute(content, issuable) } + + before do + issuable.issue_email_participants.create!(email: "user@example.com") + end + + it 'returns message' do + _, _, message = service.execute(content, issuable) + + expect(message).to eq('Removed user@example.com.') + end + + it 'removes 1 participant' do + expect { remove_email }.to change { issue.issue_email_participants.count }.by(-1) + end + + context 'with mixed case email' do + let(:content) { '/remove_email FirstLast@GitLab.com' } + + before do + issuable.issue_email_participants.create!(email: "FirstLast@GitLab.com") + end + + it 'returns correctly cased message' do + _, _, message = service.execute(content, issuable) + + expect(message).to eq('Removed FirstLast@GitLab.com.') + end + + it 'removes 1 participant' do + expect { remove_email }.to change { issue.issue_email_participants.count }.by(-1) + end + end + + context 'with invalid email' do + let(:content) { '/remove_email user@example.com bad_email' } + + it 'only removes valid emails' do + expect { remove_email }.to change { issue.issue_email_participants.count }.by(-1) + end + end + + context 'with non-existing email address' do + let(:content) { '/remove_email NonExistent@gitlab.com' } + + it 'returns message' do + _, _, message = service.execute(content, issuable) + + expect(message).to eq("No email participants were removed. Either none were provided, or they don't exist.") + end + end + + context 'with more than the max number of emails' do + let(:content) { '/remove_email user@example.com user1@example.com' } + + before do + stub_const("IssueEmailParticipants::DestroyService::MAX_NUMBER_OF_EMAILS", 1) + # user@example.com has already been added above + issuable.issue_email_participants.create!(email: "user1@example.com") + end + + it 'only removes the max allowed number of emails' do + expect { remove_email }.to change { issue.issue_email_participants.count }.by(-1) + end + end + end + + context 'with non-persisted issue' do + let(:issuable) { build(:issue) } + + it 'is not part of the available commands' do + expect(service.available_commands(issuable)).not_to include(a_hash_including(name: :remove_email)) + end + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(issue_email_participants: false) + end + + it 'is not part of the available commands' do + expect(service.available_commands(issuable)).not_to include(a_hash_including(name: :remove_email)) + end + end + end + context 'severity command' do let_it_be_with_reload(:issuable) { create(:incident, project: project) } diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 1eb11c80264..8c11270d6fd 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -515,6 +515,18 @@ RSpec.describe SystemNoteService, feature_category: :shared do end end + describe '.email_participants' do + let(:body) { 'added user@example.com' } + + it 'calls IssuableService' do + expect_next_instance_of(::SystemNotes::IssuablesService) do |service| + expect(service).to receive(:email_participants).with(body) + end + + described_class.email_participants(noteable, project, author, body) + end + end + describe '.discussion_lock' do let(:issuable) { double } diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index 0ba20ee5be1..2d7cbd30db8 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -770,6 +770,14 @@ RSpec.describe ::SystemNotes::IssuablesService, feature_category: :team_planning end end + describe '#email_participants' do + let(:body) { "added user@example.com" } + + subject(:system_note) { service.email_participants(body) } + + it { expect(system_note.note).to eq(body) } + end + describe '#discussion_lock' do subject { service.discussion_lock } diff --git a/spec/support/shared_contexts/graphql/types/query_type_shared_context.rb b/spec/support/shared_contexts/graphql/types/query_type_shared_context.rb index 6ab41d87f44..391336526e3 100644 --- a/spec/support/shared_contexts/graphql/types/query_type_shared_context.rb +++ b/spec/support/shared_contexts/graphql/types/query_type_shared_context.rb @@ -26,6 +26,8 @@ RSpec.shared_context 'with FOSS query type fields' do :milestone, :namespace, :note, + :organization, + :organizations, :package, :project, :projects,