From ece627365e3a557c1c3c51b97cad8188bc01e0f2 Mon Sep 17 00:00:00 2001
From: Marc Saleiko <msaleiko@gitlab.com>
Date: Fri, 12 Apr 2024 08:46:18 +0000
Subject: [PATCH] Revert "ms-obfuscate-issue-email-participant-system-notes
into master"
This reverts merge request !141612
---
app/graphql/types/notes/note_type.rb | 24 ++++------
app/models/service_desk_setting.rb | 2 +-
app/models/system_note_metadata.rb | 1 -
app/presenters/note_presenter.rb | 28 -----------
app/serializers/note_entity.rb | 12 +----
.../system_notes/issuables_service.rb | 2 +-
doc/api/graphql/reference/index.md | 22 ++++-----
lib/api/entities/note.rb | 6 +--
lib/api/notes.rb | 2 +-
lib/gitlab/email/service_desk/custom_email.rb | 3 +-
lib/gitlab/utils/email.rb | 18 --------
spec/graphql/types/notes/note_type_spec.rb | 33 -------------
spec/lib/gitlab/utils/email_spec.rb | 16 -------
spec/presenters/note_presenter_spec.rb | 45 ------------------
spec/requests/api/graphql/notes/note_spec.rb | 41 +----------------
spec/requests/api/notes_spec.rb | 31 -------------
spec/serializers/note_entity_spec.rb | 46 +++----------------
.../create_service_spec.rb | 1 -
...tes_content_obfuscation_shared_examples.rb | 39 ----------------
19 files changed, 35 insertions(+), 337 deletions(-)
delete mode 100644 app/presenters/note_presenter.rb
delete mode 100644 spec/presenters/note_presenter_spec.rb
delete mode 100644 spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb
diff --git a/app/graphql/types/notes/note_type.rb b/app/graphql/types/notes/note_type.rb
index d1e917b73c79b..0f2a01d73901b 100644
--- a/app/graphql/types/notes/note_type.rb
+++ b/app/graphql/types/notes/note_type.rb
@@ -13,8 +13,6 @@ class NoteType < BaseObject
implements Types::ResolvableInterface
- present_using NotePresenter
-
field :max_access_level_of_author, GraphQL::Types::String,
null: true,
description: "Max access level of the note author in the project.",
@@ -30,11 +28,11 @@ class NoteType < BaseObject
field :author, Types::UserType,
null: true,
- description: 'User who wrote the note.'
+ description: 'User who wrote this note.'
field :system, GraphQL::Types::Boolean,
null: false,
- description: 'Indicates whether the note was created by the system or by a user.'
+ description: 'Indicates whether this note was created by the system or by a user.'
field :system_note_icon_name,
GraphQL::Types::String,
null: true,
@@ -51,7 +49,7 @@ class NoteType < BaseObject
field :confidential, GraphQL::Types::Boolean,
null: true,
- description: 'Indicates if the note is confidential.',
+ description: 'Indicates if this note is confidential.',
method: :confidential?,
deprecated: {
reason: :renamed,
@@ -61,7 +59,7 @@ class NoteType < BaseObject
field :internal, GraphQL::Types::Boolean,
null: true,
- description: 'Indicates if the note is internal.',
+ description: 'Indicates if this note is internal.',
method: :confidential?
field :created_at, Types::TimeType,
@@ -69,16 +67,16 @@ class NoteType < BaseObject
description: 'Timestamp of the note creation.'
field :discussion, Types::Notes::DiscussionType,
null: true,
- description: 'Discussion the note is a part of.'
+ description: 'Discussion this note is a part of.'
field :position, Types::Notes::DiffPositionType,
null: true,
- description: 'Position of the note on a diff.'
+ description: 'Position of this note on a diff.'
field :updated_at, Types::TimeType,
null: false,
description: "Timestamp of the note's last activity."
field :url, GraphQL::Types::String,
null: true,
- description: 'URL to view the note in the Web UI.'
+ description: 'URL to view this Note in the Web UI.'
field :last_edited_at, Types::TimeType,
null: true,
@@ -97,10 +95,7 @@ class NoteType < BaseObject
null: true,
description: 'Metadata for the given note if it is a system note.'
- field :body_html, GraphQL::Types::String,
- method: :note_html,
- null: true,
- description: "GitLab Flavored Markdown rendering of the content of the note."
+ markdown_field :body_html, null: true, method: :note
def url
::Gitlab::UrlBuilder.build(object)
@@ -123,8 +118,7 @@ def author
def id
return super unless object.is_a?(SyntheticNote)
- # object is a presenter, so object.object returns the concrete note object.
- ::Gitlab::GlobalId.build(object, model_name: object.object.class.to_s, id: object.discussion_id)
+ ::Gitlab::GlobalId.build(object, model_name: object.class.to_s, id: object.discussion_id)
end
end
end
diff --git a/app/models/service_desk_setting.rb b/app/models/service_desk_setting.rb
index eba39cd5f2b7a..095eb0b67f38a 100644
--- a/app/models/service_desk_setting.rb
+++ b/app/models/service_desk_setting.rb
@@ -23,7 +23,7 @@ class ServiceDeskSetting < ApplicationRecord
length: { maximum: 255 },
uniqueness: true,
allow_nil: true,
- format: Gitlab::Utils::Email::EMAIL_REGEXP_WITH_ANCHORS
+ format: /\A[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}\z/
validates :custom_email_credential,
presence: true,
diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb
index 16a19de0f9dcf..58b46a2038688 100644
--- a/app/models/system_note_metadata.rb
+++ b/app/models/system_note_metadata.rb
@@ -25,7 +25,6 @@ class SystemNoteMetadata < ApplicationRecord
tag due_date start_date_or_due_date pinned_embed cherry_pick health_status approved unapproved
status alert_issue_added relate unrelate new_alert_added severity contact timeline_event
issue_type relate_to_child unrelate_from_child relate_to_parent unrelate_from_parent override
- issue_email_participants
].freeze
validates :note, presence: true, unless: :importing?
diff --git a/app/presenters/note_presenter.rb b/app/presenters/note_presenter.rb
deleted file mode 100644
index 00eeded05b739..0000000000000
--- a/app/presenters/note_presenter.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# frozen_string_literal: true
-
-class NotePresenter < Gitlab::View::Presenter::Delegated # rubocop:disable Gitlab/NamespacedClass -- Note is not namespaced
- presents ::Note, as: :object # because we also have a #note method
-
- delegator_override :note
- def note
- obfuscate_participants_emails_in_system_note(object.note)
- end
-
- delegator_override :note_html
- def note_html
- # Always use `redacted_note_html` because it removes references
- # based on the current user context.
- # But fall back to `note_html` if redacted is not available (same behavior as `markdown_field`)
- obfuscate_participants_emails_in_system_note(object.redacted_note_html || object.note_html)
- end
-
- private
-
- def obfuscate_participants_emails_in_system_note(text)
- return text unless object.system?
- return text if can?(current_user, :read_external_emails, object.project)
- return text if object.system_note_metadata&.action != 'issue_email_participants'
-
- Gitlab::Utils::Email.obfuscate_emails_in_text(text)
- end
-end
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index 919d8f08a150c..a50d893d24459 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -17,13 +17,9 @@ class NoteEntity < API::Entities::Note
expose :author, using: NoteUserEntity
unexpose :note, as: :body
- expose :note do |note|
- note_presenter(note).note
- end
+ expose :note
- expose :note_html do |note|
- note_presenter(note).note_html
- end
+ expose :redacted_note_html, as: :note_html
expose :last_edited_at, if: -> (note, _) { note.edited? }
expose :last_edited_by, using: NoteUserEntity, if: -> (note, _) { note.edited? }
@@ -122,10 +118,6 @@ def external_author
Gitlab::Utils::Email.obfuscated_email(object.note_metadata.external_author, deform: true)
end
end
-
- def note_presenter(note)
- NotePresenter.new(note, current_user: current_user) # rubocop: disable CodeReuse/Presenter -- Directly instantiate NotePresenter because we don't have presenters for all subclasses of Note
- end
end
NoteEntity.prepend_mod_with('NoteEntity')
diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb
index 876e694dabf04..a5aae83c32a3f 100644
--- a/app/services/system_notes/issuables_service.rb
+++ b/app/services/system_notes/issuables_service.rb
@@ -440,7 +440,7 @@ def mark_duplicate_issue(canonical_issue)
end
def email_participants(body)
- create_note(NoteSummary.new(noteable, project, author, body, action: 'issue_email_participants'))
+ create_note(NoteSummary.new(noteable, project, author, body))
end
def discussion_lock
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index e8b9a9666d567..7a19052bb7e2c 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -2932,7 +2932,7 @@ Input type: `CreateDiffNoteInput`
| <a id="mutationcreatediffnoteconfidential"></a>`confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** This was renamed. Please use `internal`. Deprecated in GitLab 15.3. |
| <a id="mutationcreatediffnoteinternal"></a>`internal` | [`Boolean`](#boolean) | Internal flag for a note. Default is false. |
| <a id="mutationcreatediffnotenoteableid"></a>`noteableId` | [`NoteableID!`](#noteableid) | Global ID of the resource to add a note to. |
-| <a id="mutationcreatediffnoteposition"></a>`position` | [`DiffPositionInput!`](#diffpositioninput) | Position of the note on a diff. |
+| <a id="mutationcreatediffnoteposition"></a>`position` | [`DiffPositionInput!`](#diffpositioninput) | Position of this note on a diff. |
#### Fields
@@ -3007,7 +3007,7 @@ Input type: `CreateImageDiffNoteInput`
| <a id="mutationcreateimagediffnoteconfidential"></a>`confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** This was renamed. Please use `internal`. Deprecated in GitLab 15.3. |
| <a id="mutationcreateimagediffnoteinternal"></a>`internal` | [`Boolean`](#boolean) | Internal flag for a note. Default is false. |
| <a id="mutationcreateimagediffnotenoteableid"></a>`noteableId` | [`NoteableID!`](#noteableid) | Global ID of the resource to add a note to. |
-| <a id="mutationcreateimagediffnoteposition"></a>`position` | [`DiffImagePositionInput!`](#diffimagepositioninput) | Position of the note on a diff. |
+| <a id="mutationcreateimagediffnoteposition"></a>`position` | [`DiffImagePositionInput!`](#diffimagepositioninput) | Position of this note on a diff. |
#### Fields
@@ -7407,7 +7407,7 @@ Input type: `RepositionImageDiffNoteInput`
| ---- | ---- | ----------- |
| <a id="mutationrepositionimagediffnoteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationrepositionimagediffnoteid"></a>`id` | [`DiffNoteID!`](#diffnoteid) | Global ID of the DiffNote to update. |
-| <a id="mutationrepositionimagediffnoteposition"></a>`position` | [`UpdateDiffImagePositionInput!`](#updatediffimagepositioninput) | Position of the note on a diff. |
+| <a id="mutationrepositionimagediffnoteposition"></a>`position` | [`UpdateDiffImagePositionInput!`](#updatediffimagepositioninput) | Position of this note on a diff. |
#### Fields
@@ -8431,7 +8431,7 @@ Input type: `UpdateImageDiffNoteInput`
| <a id="mutationupdateimagediffnotebody"></a>`body` | [`String`](#string) | Content of the note. |
| <a id="mutationupdateimagediffnoteclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationupdateimagediffnoteid"></a>`id` | [`NoteID!`](#noteid) | Global ID of the note to update. |
-| <a id="mutationupdateimagediffnoteposition"></a>`position` | [`UpdateDiffImagePositionInput`](#updatediffimagepositioninput) | Position of the note on a diff. |
+| <a id="mutationupdateimagediffnoteposition"></a>`position` | [`UpdateDiffImagePositionInput`](#updatediffimagepositioninput) | Position of this note on a diff. |
#### Fields
@@ -24902,30 +24902,30 @@ Represents the network policy.
| Name | Type | Description |
| ---- | ---- | ----------- |
-| <a id="noteauthor"></a>`author` | [`UserCore`](#usercore) | User who wrote the note. |
+| <a id="noteauthor"></a>`author` | [`UserCore`](#usercore) | User who wrote this note. |
| <a id="noteauthoriscontributor"></a>`authorIsContributor` | [`Boolean`](#boolean) | Indicates whether the note author is a contributor. |
| <a id="noteawardemoji"></a>`awardEmoji` | [`AwardEmojiConnection`](#awardemojiconnection) | List of emoji reactions associated with the note. (see [Connections](#connections)) |
| <a id="notebody"></a>`body` | [`String!`](#string) | Content of the note. |
-| <a id="notebodyhtml"></a>`bodyHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of the content of the note. |
+| <a id="notebodyhtml"></a>`bodyHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `note`. |
| <a id="noteconfidential"></a>`confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated** in GitLab 15.5. This was renamed. Use: `internal`. |
| <a id="notecreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of the note creation. |
-| <a id="notediscussion"></a>`discussion` | [`Discussion`](#discussion) | Discussion the note is a part of. |
+| <a id="notediscussion"></a>`discussion` | [`Discussion`](#discussion) | Discussion this note is a part of. |
| <a id="noteid"></a>`id` | [`NoteID!`](#noteid) | ID of the note. |
-| <a id="noteinternal"></a>`internal` | [`Boolean`](#boolean) | Indicates if the note is internal. |
+| <a id="noteinternal"></a>`internal` | [`Boolean`](#boolean) | Indicates if this note is internal. |
| <a id="notelasteditedat"></a>`lastEditedAt` | [`Time`](#time) | Timestamp when note was last edited. |
| <a id="notelasteditedby"></a>`lastEditedBy` | [`UserCore`](#usercore) | User who last edited the note. |
| <a id="notemaxaccesslevelofauthor"></a>`maxAccessLevelOfAuthor` | [`String`](#string) | Max access level of the note author in the project. |
-| <a id="noteposition"></a>`position` | [`DiffPosition`](#diffposition) | Position of the note on a diff. |
+| <a id="noteposition"></a>`position` | [`DiffPosition`](#diffposition) | Position of this note on a diff. |
| <a id="noteproject"></a>`project` | [`Project`](#project) | Project associated with the note. |
| <a id="noteresolvable"></a>`resolvable` | [`Boolean!`](#boolean) | Indicates if the object can be resolved. |
| <a id="noteresolved"></a>`resolved` | [`Boolean!`](#boolean) | Indicates if the object is resolved. |
| <a id="noteresolvedat"></a>`resolvedAt` | [`Time`](#time) | Timestamp of when the object was resolved. |
| <a id="noteresolvedby"></a>`resolvedBy` | [`UserCore`](#usercore) | User who resolved the object. |
-| <a id="notesystem"></a>`system` | [`Boolean!`](#boolean) | Indicates whether the note was created by the system or by a user. |
+| <a id="notesystem"></a>`system` | [`Boolean!`](#boolean) | Indicates whether this note was created by the system or by a user. |
| <a id="notesystemnoteiconname"></a>`systemNoteIconName` | [`String`](#string) | Name of the icon corresponding to a system note. |
| <a id="notesystemnotemetadata"></a>`systemNoteMetadata` | [`SystemNoteMetadata`](#systemnotemetadata) | Metadata for the given note if it is a system note. |
| <a id="noteupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of the note's last activity. |
-| <a id="noteurl"></a>`url` | [`String`](#string) | URL to view the note in the Web UI. |
+| <a id="noteurl"></a>`url` | [`String`](#string) | URL to view this Note in the Web UI. |
| <a id="noteuserpermissions"></a>`userPermissions` | [`NotePermissions!`](#notepermissions) | Permissions for the current user on the resource. |
### `NotePermissions`
diff --git a/lib/api/entities/note.rb b/lib/api/entities/note.rb
index 97f4810866215..c693edc611bba 100644
--- a/lib/api/entities/note.rb
+++ b/lib/api/entities/note.rb
@@ -8,11 +8,7 @@ class Note < Grape::Entity
expose :id
expose :type
-
- expose :body do |note|
- NotePresenter.new(note, current_user: options[:current_user]).note
- end
-
+ expose :note, as: :body
expose :attachment_identifier, as: :attachment
expose :author, using: Entities::UserBasic
expose :created_at, :updated_at
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 138b2fddc8363..70b4a3735e3cc 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -51,7 +51,7 @@ class Notes < ::API::Base
notes = paginate(raw_notes)
notes = prepare_notes_for_rendering(notes)
notes = notes.select { |note| note.readable_by?(current_user) }
- present notes, with: Entities::Note, current_user: current_user
+ present notes, with: Entities::Note
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/lib/gitlab/email/service_desk/custom_email.rb b/lib/gitlab/email/service_desk/custom_email.rb
index 2d2ca20aa115e..1828f71984b8d 100644
--- a/lib/gitlab/email/service_desk/custom_email.rb
+++ b/lib/gitlab/email/service_desk/custom_email.rb
@@ -8,6 +8,7 @@ module ServiceDesk
# incoming_email and service_desk_email.
module CustomEmail
REPLY_ADDRESS_KEY_REGEXP = /\+([0-9a-f]{32})@/
+ EMAIL_REGEXP = /\A[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}\z/
class << self
def reply_address(issue, reply_key)
@@ -39,7 +40,7 @@ def key_from_reply_address(email)
def find_service_desk_setting_from_reply_address(email, key)
potential_custom_email = email.sub("+#{key}", '')
- return unless Gitlab::Utils::Email::EMAIL_REGEXP_WITH_ANCHORS.match?(potential_custom_email)
+ return unless EMAIL_REGEXP.match?(potential_custom_email)
ServiceDeskSetting.find_by_custom_email(potential_custom_email)
end
diff --git a/lib/gitlab/utils/email.rb b/lib/gitlab/utils/email.rb
index 6decb3da8f87f..5eb57a66c63d9 100644
--- a/lib/gitlab/utils/email.rb
+++ b/lib/gitlab/utils/email.rb
@@ -5,13 +5,6 @@ module Utils
module Email
extend self
- # Don't use Devise.email_regexp or URI::MailTo::EMAIL_REGEXP to be a bit more restrictive
- # on the format of an email. Especially for custom email addresses which cannot contain a `+`
- # in app/models/service_desk_setting.rb
- EMAIL_REGEXP = /[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}/
- EMAIL_REGEXP_WITH_CAPTURING_GROUP = /(#{EMAIL_REGEXP})/
- EMAIL_REGEXP_WITH_ANCHORS = /\A#{EMAIL_REGEXP.source}\z/
-
# Replaces most visible characters with * to obfuscate an email address
# deform adds a fix number of * to ensure the address cannot be guessed. Also obfuscates TLD with **
def obfuscated_email(email, deform: false)
@@ -21,17 +14,6 @@ def obfuscated_email(email, deform: false)
masker_class.new(email).masked
end
- # Runs email address obfuscation on the given text.
- def obfuscate_emails_in_text(text)
- return text unless text.present?
-
- matches = text.scan(EMAIL_REGEXP_WITH_CAPTURING_GROUP).flatten.index_with do |match|
- obfuscated_email(match, deform: true)
- end
-
- text.gsub(Regexp.union(matches.keys), matches)
- end
-
class Masker
attr_reader :local_part, :sub_domain, :toplevel_domain, :at, :dot
diff --git a/spec/graphql/types/notes/note_type_spec.rb b/spec/graphql/types/notes/note_type_spec.rb
index 02f311785db2b..8aabdb78562cf 100644
--- a/spec/graphql/types/notes/note_type_spec.rb
+++ b/spec/graphql/types/notes/note_type_spec.rb
@@ -3,8 +3,6 @@
require 'spec_helper'
RSpec.describe GitlabSchema.types['Note'], feature_category: :team_planning do
- include GraphqlHelpers
-
it 'exposes the expected fields' do
expected_fields = %i[
author
@@ -39,35 +37,4 @@
specify { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Note) }
specify { expect(described_class).to require_graphql_authorizations(:read_note) }
-
- context 'when system note with issue_email_participants action', feature_category: :service_desk do
- let_it_be(:user) { build_stubbed(:user) }
- let_it_be(:email) { 'user@example.com' }
- let_it_be(:note_text) { "added #{email}" }
- # Create project and issue separately because we need to public project.
- # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version
- let_it_be(:project) { create(:project, :public) }
- let_it_be(:issue) { create(:issue, project: project) }
- let_it_be(:note) do
- create(:note, :system, project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text)
- end
-
- let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) }
- # rubocop:enable RSpec/FactoryBot/AvoidCreate
-
- let(:prepared_note) { Notes::RenderService.new(user).execute([note]).first }
- let(:obfuscated_email) { 'us*****@e*****.c**' }
-
- describe '#body' do
- subject { resolve_field(:body, prepared_note, current_user: user) }
-
- it_behaves_like 'a note content field with obfuscated email address'
- end
-
- describe '#body_html' do
- subject { resolve_field(:body_html, prepared_note, current_user: user) }
-
- it_behaves_like 'a note content field with obfuscated email address'
- end
- end
end
diff --git a/spec/lib/gitlab/utils/email_spec.rb b/spec/lib/gitlab/utils/email_spec.rb
index 430002eb520fb..c81c2558f706b 100644
--- a/spec/lib/gitlab/utils/email_spec.rb
+++ b/spec/lib/gitlab/utils/email_spec.rb
@@ -51,20 +51,4 @@
end
end
end
-
- describe '.obfuscate_emails_in_text' do
- where(:input, :output) do
- nil | nil
- '' | ''
- 'added no email address' | 'added no email address'
- 'added user@example.com' | 'added us*****@e*****.c**'
- 'added user@example.com and hello@example.com' | 'added us*****@e*****.c** and he*****@e*****.c**'
- 'removed user@example.com, hello@example.com and bye@example.com' |
- 'removed us*****@e*****.c**, he*****@e*****.c** and by*****@e*****.c**'
- end
-
- with_them do
- it { expect(described_class.obfuscate_emails_in_text(input)).to eq(output) }
- end
- end
end
diff --git a/spec/presenters/note_presenter_spec.rb b/spec/presenters/note_presenter_spec.rb
deleted file mode 100644
index 2a8ae38f08274..0000000000000
--- a/spec/presenters/note_presenter_spec.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe NotePresenter, feature_category: :team_planning do
- let_it_be(:user) { build_stubbed(:user) }
- let_it_be(:email) { 'user@example.com' }
- let_it_be(:note_text) { "added #{email}" }
- # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version
- let_it_be_with_reload(:note) { create(:note, :system, author: Users::Internal.support_bot, note: note_text) }
- let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) }
- # rubocop:enable RSpec/FactoryBot/AvoidCreate
-
- let(:obfuscated_email) { 'us*****@e*****.c**' }
-
- subject(:presenter) do
- # The RenderService calls Banzai::ObjectRenderer which
- # populates `redacted_note_html`. We also do this in
- # API controllers.
- prepared_notes = Notes::RenderService.new(user).execute([note])
- described_class.new(prepared_notes.first, current_user: user)
- end
-
- describe '#note' do
- subject { presenter.note }
-
- it_behaves_like 'a note content field with obfuscated email address'
- end
-
- describe '#note_html' do
- subject { presenter.note_html }
-
- it_behaves_like 'a note content field with obfuscated email address'
-
- context 'when redacted_note_html is not present' do
- subject(:presenter) do
- described_class.new(note, current_user: user).note_html
- end
-
- it 'falls back to note_html and obfuscates emails' do
- is_expected.to include(obfuscated_email)
- end
- end
- end
-end
diff --git a/spec/requests/api/graphql/notes/note_spec.rb b/spec/requests/api/graphql/notes/note_spec.rb
index 2a2ea8ce49b1d..daceaec0b94cd 100644
--- a/spec/requests/api/graphql/notes/note_spec.rb
+++ b/spec/requests/api/graphql/notes/note_spec.rb
@@ -6,7 +6,6 @@
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
- let_it_be(:reporter_user) { create(:user) }
let_it_be(:project) { create(:project, :private) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:note) { create(:note, noteable: issue, project: project) }
@@ -20,10 +19,6 @@
graphql_query_for('note', note_params, note_fields)
end
- before_all do
- project.add_reporter(reporter_user)
- end
-
it_behaves_like 'a working graphql query' do
before do
post_graphql(query, current_user: current_user)
@@ -49,7 +44,7 @@
end
context 'when the user has access to read the note' do
- before_all do
+ before do
project.add_guest(current_user)
end
@@ -67,40 +62,6 @@
expect(note_data['id']).to eq(global_id_of(system_note).to_s)
end
-
- context 'with issue_email_participants action' do
- let_it_be(:email) { 'user@example.com' }
- let_it_be(:note_text) { "added #{email}" }
- let_it_be(:issue_email_participants_system_note) do
- create(:note, :system,
- project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text)
- end
-
- let_it_be(:system_note_metadata) do
- create(:system_note_metadata, note: issue_email_participants_system_note, action: :issue_email_participants)
- end
-
- let(:obfuscated_email) { 'us*****@e*****.c**' }
- let(:note_params) { { 'id' => global_id_of(issue_email_participants_system_note) } }
-
- it 'returns obfuscated email' do
- post_graphql(query, current_user: current_user)
-
- expect(note_data['id']).to eq(global_id_of(issue_email_participants_system_note).to_s)
- expect(note_data['body']).to include(obfuscated_email)
- expect(note_data['bodyHtml']).to include(obfuscated_email)
- end
-
- context 'when user has at least the reporter role in project' do
- it 'returns email' do
- post_graphql(query, current_user: reporter_user)
-
- expect(note_data['id']).to eq(global_id_of(issue_email_participants_system_note).to_s)
- expect(note_data['body']).to include(email)
- expect(note_data['bodyHtml']).to include(email)
- end
- end
- end
end
context 'and notes widget is not available' do
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index e7ff7baaa6ce0..497bbeced38b4 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -49,37 +49,6 @@
end
end
- context 'when system note with issue_email_participants action' do
- let!(:email) { 'user@example.com' }
- let!(:note_text) { "added #{email}" }
- let!(:note) do
- create(:note, :system, project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text)
- end
-
- let!(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) }
- let!(:another_user) { create(:user) }
-
- let(:obfuscated_email) { 'us*****@e*****.c**' }
-
- it 'returns obfuscated email' do
- get api("/projects/#{project.id}/issues/#{issue.iid}/notes", another_user)
-
- expect(response).to have_gitlab_http_status(:ok)
- expect(response).to include_pagination_headers
- expect(json_response.first['body']).to include(obfuscated_email)
- end
-
- context 'when user has at least the reporter role in project' do
- it 'returns email' do
- get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user)
-
- expect(response).to have_gitlab_http_status(:ok)
- expect(response).to include_pagination_headers
- expect(json_response.first['body']).to include(email)
- end
- end
- end
-
context "when referencing other project" do
# For testing the cross-reference of a private issue in a public project
let(:private_project) do
diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb
index 51335c5f73204..d5c4a31a93785 100644
--- a/spec/serializers/note_entity_spec.rb
+++ b/spec/serializers/note_entity_spec.rb
@@ -2,17 +2,14 @@
require 'spec_helper'
-RSpec.describe NoteEntity, feature_category: :team_planning do
+RSpec.describe NoteEntity do
include Gitlab::Routing
- # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Persisted records required
- let_it_be(:note) { create(:note) }
- let_it_be(:user) { create(:user) }
- # rubocop:enable RSpec/FactoryBot/AvoidCreate
-
let(:request) { double('request', current_user: user, noteable: note.noteable) }
let(:entity) { described_class.new(note, request: request) }
+ let(:note) { create(:note) }
+ let(:user) { create(:user) }
subject { entity.as_json }
@@ -56,8 +53,9 @@
end
end
- describe 'with email participant', feature_category: :service_desk do
- let_it_be(:note_metadata) { create(:note_metadata, note: note) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Persisted records required
+ describe 'with email participant' do
+ let_it_be(:note) { create(:note) }
+ let_it_be(:note_metadata) { create(:note_metadata, note: note) }
subject { entity.as_json[:external_author] }
@@ -68,36 +66,4 @@
it_behaves_like 'external author'
end
end
-
- context 'when system note with issue_email_participants action', feature_category: :service_desk do
- let_it_be(:email) { 'user@example.com' }
- let_it_be(:note_text) { "added #{email}" }
- # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version
- let_it_be(:note) { create(:note, :system, author: Users::Internal.support_bot, note: note_text) }
- let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) }
- # rubocop:enable RSpec/FactoryBot/AvoidCreate
-
- let(:obfuscated_email) { 'us*****@e*****.c**' }
- let(:expected_text) { obfuscated_email }
-
- subject(:entity_hash) do
- # The RenderService calls Banzai::ObjectRenderer which
- # populates `redacted_note_html`. We also do this in
- # API controllers.
- prepared_notes = Notes::RenderService.new(user).execute([note])
- described_class.new(prepared_notes.first, request: request).as_json
- end
-
- describe 'note' do
- subject { entity_hash[:note] }
-
- it_behaves_like 'a note content field with obfuscated email address'
- end
-
- describe 'note_html' do
- subject { entity_hash[:note_html] }
-
- it_behaves_like 'a note content field with obfuscated email address'
- 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 05c9162dde148..dc8d5a6ea74ec 100644
--- a/spec/services/issue_email_participants/create_service_spec.rb
+++ b/spec/services/issue_email_participants/create_service_spec.rb
@@ -12,7 +12,6 @@
note = issue.notes.last
expect(note.system?).to be true
expect(note.author).to eq(user)
- expect(note.system_note_metadata.action).to eq('issue_email_participants')
participants_emails = issue.email_participants_emails_downcase
diff --git a/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb b/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb
deleted file mode 100644
index 0f8f24a034f2e..0000000000000
--- a/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.shared_examples 'a note content field with obfuscated email address' do
- context 'when anonymous' do
- let(:user) { nil }
-
- it { is_expected.to include(obfuscated_email) }
- end
-
- context 'with signed in user' do
- before do
- stub_member_access_level(note.project, access_level => user) if access_level
- end
-
- context 'when user has no role in project' do
- let(:access_level) { nil }
-
- it { is_expected.to include(obfuscated_email) }
- end
-
- context 'when user has guest role in project' do
- let(:access_level) { :guest }
-
- it { is_expected.to include(obfuscated_email) }
- end
-
- context 'when user has reporter role in project' do
- let(:access_level) { :reporter }
-
- it { is_expected.to include(email) }
- end
-
- context 'when user has developer role in project' do
- let(:access_level) { :developer }
-
- it { is_expected.to include(email) }
- end
- end
-end
--
GitLab