-
-
- {{ getNoteableData.source_branch }}
-
-
- {{ getNoteableData.target_branch }}
-
-
+
+
+
+ {{ sourceName }}
+
+
+ {{ targetName }}
+
+
+
{{
__('Create commit')
diff --git a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
index 0d1fd26777f..3f33b0c900e 100644
--- a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
+++ b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
@@ -58,14 +58,18 @@ export const diffCompareDropdownTargetVersions = (state, getters) => {
export const diffCompareDropdownSourceVersions = (state, getters) => {
// Appended properties here are to make the compare_dropdown_layout easier to reason about
- return state.mergeRequestDiffs.map((v, i) => ({
- ...v,
- href: v.version_path,
- commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count),
- versionName:
- i === 0
+ return state.mergeRequestDiffs.map((v, i) => {
+ const isLatestVersion = i === 0;
+
+ return {
+ ...v,
+ href: v.version_path,
+ commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count),
+ isLatestVersion,
+ versionName: isLatestVersion
? __('latest version')
: sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }),
- selected: v.version_index === getters.selectedSourceIndex,
- }));
+ selected: v.version_index === getters.selectedSourceIndex,
+ };
+ });
};
diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb
index 83b8a834801..78fb20650e9 100644
--- a/app/graphql/types/issue_type.rb
+++ b/app/graphql/types/issue_type.rb
@@ -121,6 +121,9 @@ module Types
field :moved_to, Types::IssueType, null: true,
description: 'Updated Issue after it got moved to another project'
+ field :create_note_email, GraphQL::STRING_TYPE, null: true,
+ description: 'User specific email address for the issue'
+
def author
Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
end
@@ -140,6 +143,10 @@ module Types
def discussion_locked
!!object.discussion_locked
end
+
+ def create_note_email
+ object.creatable_note_email_address(context[:current_user])
+ end
end
end
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 15842dec3dd..da142cbed0e 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -442,7 +442,8 @@ module IssuablesHelper
fullPath: issuable[:project_full_path],
iid: issuable[:iid],
severity: issuable[:severity],
- timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours
+ timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours,
+ createNoteEmail: issuable[:create_note_email]
}
end
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 2dbe9360d42..f3cc68e4b85 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -19,6 +19,11 @@ module Noteable
def resolvable_types
%w(MergeRequest DesignManagement::Design)
end
+
+ # `Noteable` class names that support creating/forwarding individual notes.
+ def email_creatable_types
+ %w(Issue)
+ end
end
# The timestamp of the note (e.g. the :created_at or :updated_at attribute if provided via
@@ -55,6 +60,10 @@ module Noteable
supports_discussions? && self.class.replyable_types.include?(base_class_name)
end
+ def supports_creating_notes_by_email?
+ self.class.email_creatable_types.include?(base_class_name)
+ end
+
def supports_suggestion?
false
end
@@ -158,6 +167,18 @@ module Noteable
def after_note_destroyed(_note)
# no-op
end
+
+ # Email address that an authorized user can send/forward an email to be added directly
+ # to an issue or merge request.
+ # example: incoming+h5bp-html5-boilerplate-8-1234567890abcdef123456789-issue-34@localhost.com
+ def creatable_note_email_address(author)
+ return unless supports_creating_notes_by_email?
+
+ project_email = project.new_issuable_address(author, self.class.name.underscore)
+ return unless project_email
+
+ project_email.sub('@', "-#{iid}@")
+ end
end
Noteable.extend(Noteable::ClassMethods)
diff --git a/app/serializers/issuable_sidebar_basic_entity.rb b/app/serializers/issuable_sidebar_basic_entity.rb
index 53c123c06fd..4b3d6f21d6d 100644
--- a/app/serializers/issuable_sidebar_basic_entity.rb
+++ b/app/serializers/issuable_sidebar_basic_entity.rb
@@ -103,6 +103,10 @@ class IssuableSidebarBasicEntity < Grape::Entity
issuable.project.emails_disabled?
end
+ expose :create_note_email do |issuable|
+ issuable.creatable_note_email_address(current_user)
+ end
+
expose :supports_time_tracking?, as: :supports_time_tracking
expose :supports_milestone?, as: :supports_milestone
expose :supports_severity?, as: :supports_severity
diff --git a/app/views/projects/branches/new.html.haml b/app/views/projects/branches/new.html.haml
index 24dfb59dc85..17314cd7c5a 100644
--- a/app/views/projects/branches/new.html.haml
+++ b/app/views/projects/branches/new.html.haml
@@ -2,9 +2,12 @@
- default_ref = params[:ref] || @project.default_branch
- if @error
- .alert.alert-danger
- %button.close{ type: "button", "data-dismiss" => "alert" } ×
- = @error
+ .gl-alert.gl-alert-danger
+ = sprite_icon('error', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
+ %button.js-close.gl-alert-dismiss{ type: 'button', 'aria-label' => _('Dismiss') }
+ = sprite_icon('close', size: 16, css_class: 'gl-icon')
+ .gl-alert-body
+ = @error
%h3.page-title
New Branch
%hr
diff --git a/changelogs/unreleased/18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment.yml b/changelogs/unreleased/18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment.yml
new file mode 100644
index 00000000000..7e139c69ee3
--- /dev/null
+++ b/changelogs/unreleased/18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment.yml
@@ -0,0 +1,5 @@
+---
+title: New user/issue specific email address for creating/forwarding to an issue
+merge_request: 49050
+author:
+type: added
diff --git a/changelogs/unreleased/233658-migrate-alert-on-new-branch-page.yml b/changelogs/unreleased/233658-migrate-alert-on-new-branch-page.yml
new file mode 100644
index 00000000000..801b45b2bf6
--- /dev/null
+++ b/changelogs/unreleased/233658-migrate-alert-on-new-branch-page.yml
@@ -0,0 +1,5 @@
+---
+title: Migrates the alert on the new branch page
+merge_request: 50307
+author:
+type: other
diff --git a/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
new file mode 100644
index 00000000000..6d721b9cbaa
--- /dev/null
+++ b/changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml
@@ -0,0 +1,5 @@
+---
+title: Remove diff display preferences and file tree from changes empty state
+merge_request: 43467
+author:
+type: fixed
diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql
index 18a52e93af2..b264b6127e7 100644
--- a/doc/api/graphql/reference/gitlab_schema.graphql
+++ b/doc/api/graphql/reference/gitlab_schema.graphql
@@ -8665,6 +8665,11 @@ type EpicIssue implements CurrentUserTodos & Noteable {
"""
confidential: Boolean!
+ """
+ User specific email address for the issue
+ """
+ createNoteEmail: String
+
"""
Timestamp of when the issue was created
"""
@@ -11636,6 +11641,11 @@ type Issue implements CurrentUserTodos & Noteable {
"""
confidential: Boolean!
+ """
+ User specific email address for the issue
+ """
+ createNoteEmail: String
+
"""
Timestamp of when the issue was created
"""
diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json
index 5d0f350ae31..9fd74b1cc53 100644
--- a/doc/api/graphql/reference/gitlab_schema.json
+++ b/doc/api/graphql/reference/gitlab_schema.json
@@ -24014,6 +24014,20 @@
"isDeprecated": false,
"deprecationReason": null
},
+ {
+ "name": "createNoteEmail",
+ "description": "User specific email address for the issue",
+ "args": [
+
+ ],
+ "type": {
+ "kind": "SCALAR",
+ "name": "String",
+ "ofType": null
+ },
+ "isDeprecated": false,
+ "deprecationReason": null
+ },
{
"name": "createdAt",
"description": "Timestamp of when the issue was created",
@@ -31845,6 +31859,20 @@
"isDeprecated": false,
"deprecationReason": null
},
+ {
+ "name": "createNoteEmail",
+ "description": "User specific email address for the issue",
+ "args": [
+
+ ],
+ "type": {
+ "kind": "SCALAR",
+ "name": "String",
+ "ofType": null
+ },
+ "isDeprecated": false,
+ "deprecationReason": null
+ },
{
"name": "createdAt",
"description": "Timestamp of when the issue was created",
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 72f94d12c16..6a51437f8b2 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -1425,6 +1425,7 @@ Relationship between an epic and an issue.
| `blockedByCount` | Int | Count of issues blocking this issue. |
| `closedAt` | Time | Timestamp of when the issue was closed |
| `confidential` | Boolean! | Indicates the issue is confidential |
+| `createNoteEmail` | String | User specific email address for the issue |
| `createdAt` | Time! | Timestamp of when the issue was created |
| `currentUserTodos` | TodoConnection! | Todos for the current user |
| `description` | String | Description of the issue |
@@ -1768,6 +1769,7 @@ Represents a recorded measurement (object count) for the Admins.
| `blockedByCount` | Int | Count of issues blocking this issue. |
| `closedAt` | Time | Timestamp of when the issue was closed |
| `confidential` | Boolean! | Indicates the issue is confidential |
+| `createNoteEmail` | String | User specific email address for the issue |
| `createdAt` | Time! | Timestamp of when the issue was created |
| `currentUserTodos` | TodoConnection! | Todos for the current user |
| `description` | String | Description of the issue |
diff --git a/doc/subscriptions/self_managed/index.md b/doc/subscriptions/self_managed/index.md
index a4affff92e4..1c869584783 100644
--- a/doc/subscriptions/self_managed/index.md
+++ b/doc/subscriptions/self_managed/index.md
@@ -43,7 +43,7 @@ billable user, with the following exceptions:
count toward overages in the subscribed seat count.
- Users who are [pending approval](../../user/admin_area/approving_users.md).
- Members with Guest permissions on an Ultimate subscription.
-- GitLab-created service accounts: `Ghost User` and bots (`Support Bot`, [`Project bot users`](../../user/project/settings/project_access_tokens.md#project-bot-users), and so on).
+- GitLab-created service accounts: `Ghost User` and bots [(`Support Bot`](../../user/project/service_desk.md#support-bot-user), [`Project bot users`](../../user/project/settings/project_access_tokens.md#project-bot-users), and so on).
### Tips for managing users and subscription seats
diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb
index 1b8421d34f3..e71ea154355 100644
--- a/lib/gitlab/email/handler.rb
+++ b/lib/gitlab/email/handler.rb
@@ -11,6 +11,7 @@ module Gitlab
[
CreateNoteHandler,
CreateIssueHandler,
+ CreateNoteOnIssuableHandler,
UnsubscribeHandler,
CreateMergeRequestHandler,
ServiceDeskHandler
diff --git a/lib/gitlab/email/handler/create_note_on_issuable_handler.rb b/lib/gitlab/email/handler/create_note_on_issuable_handler.rb
new file mode 100644
index 00000000000..aed3647744a
--- /dev/null
+++ b/lib/gitlab/email/handler/create_note_on_issuable_handler.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'gitlab/email/handler/base_handler'
+
+# Handles comment creation emails when sent/forwarded by an authorized
+# user. Attachments are allowed. Quoted material is _not_ stripped, just like
+# create issue emails
+# Supports these formats:
+# incoming+gitlab-org-gitlab-ce-20-Author_Token12345678-issue-34@incoming.gitlab.com
+module Gitlab
+ module Email
+ module Handler
+ class CreateNoteOnIssuableHandler < BaseHandler
+ include ReplyProcessing
+
+ attr_reader :issuable_iid
+
+ HANDLER_REGEX = /\A#{HANDLER_ACTION_BASE_REGEX}-(?.+)-issue-(?\d+)\z/.freeze
+
+ def initialize(mail, mail_key)
+ super(mail, mail_key)
+
+ if (matched = HANDLER_REGEX.match(mail_key.to_s))
+ @project_slug = matched[:project_slug]
+ @project_id = matched[:project_id]&.to_i
+ @incoming_email_token = matched[:incoming_email_token]
+ @issuable_iid = matched[:issuable_iid]&.to_i
+ end
+ end
+
+ def can_handle?
+ incoming_email_token && project_id && issuable_iid
+ end
+
+ def execute
+ raise ProjectNotFound unless project
+
+ validate_permission!(:create_note)
+
+ raise NoteableNotFoundError unless noteable
+ raise EmptyEmailError if message_including_reply.blank?
+
+ verify_record!(
+ record: create_note,
+ invalid_exception: InvalidNoteError,
+ record_name: 'comment')
+ end
+
+ def metrics_event
+ :receive_email_create_note_issuable
+ end
+
+ def noteable
+ return unless issuable_iid
+
+ @noteable ||= project&.issues&.find_by_iid(issuable_iid)
+ end
+
+ private
+
+ # rubocop: disable CodeReuse/ActiveRecord
+ def author
+ @author ||= User.find_by(incoming_email_token: incoming_email_token)
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
+ def create_note
+ Notes::CreateService.new(project, author, note_params).execute
+ end
+
+ def note_params
+ {
+ noteable_type: noteable.class.to_s,
+ noteable_id: noteable.id,
+ note: message_including_reply
+ }
+ end
+ end
+ end
+ end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 9e8b50f39d7..db1f67a8f77 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -18935,7 +18935,7 @@ msgstr ""
msgid "No changes"
msgstr ""
-msgid "No changes between %{sourceBranch} and %{targetBranch}"
+msgid "No changes between %{source} and %{target}"
msgstr ""
msgid "No child epics match applied filters"
diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb
index 8930c55a28c..8999c4d6656 100644
--- a/spec/features/merge_request/user_sees_versions_spec.rb
+++ b/spec/features/merge_request/user_sees_versions_spec.rb
@@ -191,7 +191,7 @@ RSpec.describe 'Merge request > User sees versions', :js do
find('.btn-default').click
click_link 'version 1'
end
- expect(page).to have_content '0 files'
+ expect(page).to have_content 'No changes between version 1 and version 1'
end
end
@@ -217,7 +217,7 @@ RSpec.describe 'Merge request > User sees versions', :js do
expect(page).to have_content 'version 1'
end
- expect(page).to have_content '0 files'
+ expect(page).to have_content 'No changes between version 1 and version 1'
end
end
diff --git a/spec/fixtures/emails/valid_note_on_issuable.eml b/spec/fixtures/emails/valid_note_on_issuable.eml
new file mode 100644
index 00000000000..29308c9d969
--- /dev/null
+++ b/spec/fixtures/emails/valid_note_on_issuable.eml
@@ -0,0 +1,24 @@
+Return-Path:
+Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
+Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
+Date: Thu, 13 Jun 2013 17:03:48 -0400
+From: Jake the Dog
+To: incoming+gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo
+Message-ID:
+Subject: New Issue comment by email
+Mime-Version: 1.0
+Content-Type: text/plain;
+ charset=ISO-8859-1
+Content-Transfer-Encoding: 7bit
+X-Sieve: CMU Sieve 2.2
+X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
+ 13 Jun 2013 14:03:48 -0700 (PDT)
+X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
+
+This should create a new comment on the issue.
+
+- Jake out
+
+> This quoted content will be included in the comment.
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 56cc0c8b8cf..d50f5133e04 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/spec/frontend/diffs/components/app_spec.js
@@ -147,31 +147,21 @@ describe('diffs/components/app', () => {
});
});
- it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
- createComponent({}, ({ state }) => {
- state.diffs.showTreeList = false;
- state.diffs.isParallelView = false;
- });
+ it.each`
+ props | state | expected
+ ${{ isFluidLayout: true }} | ${{ isParallelView: false }} | ${false}
+ ${{}} | ${{ isParallelView: false }} | ${true}
+ ${{}} | ${{ showTreeList: true, diffFiles: [{}], isParallelView: false }} | ${false}
+ ${{}} | ${{ showTreeList: false, diffFiles: [{}], isParallelView: false }} | ${true}
+ ${{}} | ${{ showTreeList: false, diffFiles: [], isParallelView: false }} | ${true}
+ `(
+ 'uses container-limiting classes ($expected) with state ($state) and props ($props)',
+ ({ props, state, expected }) => {
+ createComponent(props, ({ state: origState }) => Object.assign(origState.diffs, state));
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(true);
- });
-
- it('does not add container-limiting classes when showFileTree is false with inline diffs', () => {
- createComponent({}, ({ state }) => {
- state.diffs.showTreeList = true;
- state.diffs.isParallelView = false;
- });
-
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false);
- });
-
- it('does not add container-limiting classes when isFluidLayout', () => {
- createComponent({ isFluidLayout: true }, ({ state }) => {
- state.diffs.isParallelView = false;
- });
-
- expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false);
- });
+ expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(expected);
+ },
+ );
it('displays loading icon on loading', () => {
createComponent({}, ({ state }) => {
@@ -249,7 +239,9 @@ describe('diffs/components/app', () => {
});
it('sets width of tree list', () => {
- createComponent();
+ createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
+ });
expect(wrapper.find('.js-diff-tree-list').element.style.width).toEqual('320px');
});
@@ -283,15 +275,6 @@ describe('diffs/components/app', () => {
expect(wrapper.find(NoChanges).exists()).toBe(false);
expect(wrapper.findAll(DiffFile).length).toBe(1);
});
-
- it('does not render empty state when versions match', () => {
- createComponent({}, ({ state }) => {
- state.diffs.startVersion = mergeRequestDiff;
- state.diffs.mergeRequestDiff = mergeRequestDiff;
- });
-
- expect(wrapper.find(NoChanges).exists()).toBe(false);
- });
});
describe('keyboard shortcut navigation', () => {
@@ -517,6 +500,7 @@ describe('diffs/components/app', () => {
describe('diffs', () => {
it('should render compare versions component', () => {
createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
state.diffs.mergeRequestDiffs = diffsMockData;
state.diffs.targetBranchName = 'target-branch';
state.diffs.mergeRequestDiff = mergeRequestDiff;
@@ -525,7 +509,8 @@ describe('diffs/components/app', () => {
expect(wrapper.find(CompareVersions).exists()).toBe(true);
expect(wrapper.find(CompareVersions).props()).toEqual(
expect.objectContaining({
- mergeRequestDiffs: diffsMockData,
+ isLimitedContainer: false,
+ diffFilesCountText: null,
}),
);
});
@@ -593,9 +578,17 @@ describe('diffs/components/app', () => {
expect(wrapper.find(DiffFile).exists()).toBe(true);
});
- it('should render tree list', () => {
+ it("doesn't render tree list when no changes exist", () => {
createComponent();
+ expect(wrapper.find(TreeList).exists()).toBe(false);
+ });
+
+ it('should render tree list', () => {
+ createComponent({}, ({ state }) => {
+ state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }];
+ });
+
expect(wrapper.find(TreeList).exists()).toBe(true);
});
});
diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js
index cd7697676c4..949cc855200 100644
--- a/spec/frontend/diffs/components/compare_versions_spec.js
+++ b/spec/frontend/diffs/components/compare_versions_spec.js
@@ -11,10 +11,26 @@ localVue.use(Vuex);
describe('CompareVersions', () => {
let wrapper;
+ let store;
const targetBranchName = 'tmp-wine-dev';
const createWrapper = (props) => {
- const store = createStore();
+ wrapper = mount(CompareVersionsComponent, {
+ localVue,
+ store,
+ propsData: {
+ mergeRequestDiffs: diffsMockData,
+ diffFilesCountText: '1',
+ ...props,
+ },
+ });
+ };
+ const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width');
+ const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown');
+ const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown');
+
+ beforeEach(() => {
+ store = createStore();
const mergeRequestDiff = diffsMockData[0];
store.state.diffs.addedLines = 10;
@@ -23,20 +39,6 @@ describe('CompareVersions', () => {
store.state.diffs.targetBranchName = targetBranchName;
store.state.diffs.mergeRequestDiff = mergeRequestDiff;
store.state.diffs.mergeRequestDiffs = diffsMockData;
-
- wrapper = mount(CompareVersionsComponent, {
- localVue,
- store,
- propsData: {
- mergeRequestDiffs: diffsMockData,
- diffFilesCountText: null,
- ...props,
- },
- });
- };
-
- beforeEach(() => {
- createWrapper();
});
afterEach(() => {
@@ -45,6 +47,10 @@ describe('CompareVersions', () => {
});
describe('template', () => {
+ beforeEach(() => {
+ createWrapper();
+ });
+
it('should render Tree List toggle button with correct attribute values', () => {
const treeListBtn = wrapper.find('.js-toggle-tree-list');
@@ -54,8 +60,8 @@ describe('CompareVersions', () => {
});
it('should render comparison dropdowns with correct values', () => {
- const sourceDropdown = wrapper.find('.mr-version-dropdown');
- const targetDropdown = wrapper.find('.mr-version-compare-dropdown');
+ const sourceDropdown = findCompareSourceDropdown();
+ const targetDropdown = findCompareTargetDropdown();
expect(sourceDropdown.exists()).toBe(true);
expect(targetDropdown.exists()).toBe(true);
@@ -63,16 +69,6 @@ describe('CompareVersions', () => {
expect(targetDropdown.find('button').html()).toContain(targetBranchName);
});
- it('should not render comparison dropdowns if no mergeRequestDiffs are specified', () => {
- createWrapper({ mergeRequestDiffs: [] });
-
- const sourceDropdown = wrapper.find('.mr-version-dropdown');
- const targetDropdown = wrapper.find('.mr-version-compare-dropdown');
-
- expect(sourceDropdown.exists()).toBe(false);
- expect(targetDropdown.exists()).toBe(false);
- });
-
it('should render view types buttons with correct values', () => {
const inlineBtn = wrapper.find('#inline-diff-btn');
const parallelBtn = wrapper.find('#parallel-diff-btn');
@@ -88,22 +84,34 @@ describe('CompareVersions', () => {
it('adds container-limiting classes when showFileTree is false with inline diffs', () => {
createWrapper({ isLimitedContainer: true });
- const limitedContainer = wrapper.find('.container-limited.limit-container-width');
-
- expect(limitedContainer.exists()).toBe(true);
+ expect(findLimitedContainer().exists()).toBe(true);
});
it('does not add container-limiting classes when showFileTree is false with inline diffs', () => {
createWrapper({ isLimitedContainer: false });
- const limitedContainer = wrapper.find('.container-limited.limit-container-width');
+ expect(findLimitedContainer().exists()).toBe(false);
+ });
+ });
- expect(limitedContainer.exists()).toBe(false);
+ describe('noChangedFiles', () => {
+ beforeEach(() => {
+ store.state.diffs.diffFiles = [];
+ });
+
+ it('should not render Tree List toggle button when there are no changes', () => {
+ createWrapper();
+
+ const treeListBtn = wrapper.find('.js-toggle-tree-list');
+
+ expect(treeListBtn.exists()).toBe(false);
});
});
describe('setInlineDiffViewType', () => {
it('should persist the view type in the url', () => {
+ createWrapper();
+
const viewTypeBtn = wrapper.find('#inline-diff-btn');
viewTypeBtn.trigger('click');
@@ -113,6 +121,7 @@ describe('CompareVersions', () => {
describe('setParallelDiffViewType', () => {
it('should persist the view type in the url', () => {
+ createWrapper();
const viewTypeBtn = wrapper.find('#parallel-diff-btn');
viewTypeBtn.trigger('click');
@@ -121,11 +130,14 @@ describe('CompareVersions', () => {
});
describe('commit', () => {
- beforeEach((done) => {
- wrapper.vm.$store.state.diffs.commit = getDiffWithCommit().commit;
- wrapper.mergeRequestDiffs = [];
+ beforeEach(() => {
+ store.state.diffs.commit = getDiffWithCommit().commit;
+ createWrapper();
+ });
- wrapper.vm.$nextTick(done);
+ it('does not render compare dropdowns', () => {
+ expect(findCompareSourceDropdown().exists()).toBe(false);
+ expect(findCompareTargetDropdown().exists()).toBe(false);
});
it('renders latest version button', () => {
@@ -137,4 +149,16 @@ describe('CompareVersions', () => {
expect(wrapper.text()).toContain(wrapper.vm.commit.short_id);
});
});
+
+ describe('with no versions', () => {
+ beforeEach(() => {
+ store.state.diffs.mergeRequestDiffs = [];
+ createWrapper();
+ });
+
+ it('does not render compare dropdowns', () => {
+ expect(findCompareSourceDropdown().exists()).toBe(false);
+ expect(findCompareTargetDropdown().exists()).toBe(false);
+ });
+ });
});
diff --git a/spec/frontend/diffs/components/no_changes_spec.js b/spec/frontend/diffs/components/no_changes_spec.js
index 5d00a04d415..df9af51f9cf 100644
--- a/spec/frontend/diffs/components/no_changes_spec.js
+++ b/spec/frontend/diffs/components/no_changes_spec.js
@@ -1,20 +1,22 @@
-import { createLocalVue, shallowMount } from '@vue/test-utils';
+import { createLocalVue, shallowMount, mount } from '@vue/test-utils';
import Vuex from 'vuex';
import { GlButton } from '@gitlab/ui';
import { createStore } from '~/mr_notes/stores';
import NoChanges from '~/diffs/components/no_changes.vue';
+import diffsMockData from '../mock_data/merge_request_diffs';
+
+const localVue = createLocalVue();
+localVue.use(Vuex);
+
+const TEST_TARGET_BRANCH = 'foo';
+const TEST_SOURCE_BRANCH = 'dev/update';
describe('Diff no changes empty state', () => {
- let vm;
+ let wrapper;
+ let store;
- function createComponent(extendStore = () => {}) {
- const localVue = createLocalVue();
- localVue.use(Vuex);
-
- const store = createStore();
- extendStore(store);
-
- vm = shallowMount(NoChanges, {
+ function createComponent(mountFn = shallowMount) {
+ wrapper = mountFn(NoChanges, {
localVue,
store,
propsData: {
@@ -23,26 +25,61 @@ describe('Diff no changes empty state', () => {
});
}
- afterEach(() => {
- vm.destroy();
+ beforeEach(() => {
+ store = createStore();
+ store.state.diffs.mergeRequestDiff = {};
+ store.state.notes.noteableData = {
+ target_branch: TEST_TARGET_BRANCH,
+ source_branch: TEST_SOURCE_BRANCH,
+ };
+ store.state.diffs.mergeRequestDiffs = diffsMockData;
});
- it('prevents XSS', () => {
- createComponent((store) => {
- // eslint-disable-next-line no-param-reassign
- store.state.notes.noteableData = {
- source_branch: '',
- target_branch: '',
- };
- });
+ afterEach(() => {
+ wrapper.destroy();
+ wrapper = null;
+ });
- expect(vm.find('script').exists()).toBe(false);
+ const findMessage = () => wrapper.find('[data-testid="no-changes-message"]');
+
+ it('prevents XSS', () => {
+ store.state.notes.noteableData = {
+ source_branch: '',
+ target_branch: '',
+ };
+
+ createComponent();
+
+ expect(wrapper.find('script').exists()).toBe(false);
});
describe('Renders', () => {
it('Show create commit button', () => {
createComponent();
- expect(vm.find(GlButton).exists()).toBe(true);
+
+ expect(wrapper.find(GlButton).exists()).toBe(true);
});
+
+ it.each`
+ expectedText | sourceIndex | targetIndex
+ ${`No changes between ${TEST_SOURCE_BRANCH} and ${TEST_TARGET_BRANCH}`} | ${null} | ${null}
+ ${`No changes between ${TEST_SOURCE_BRANCH} and version 1`} | ${diffsMockData[0].version_index} | ${1}
+ ${`No changes between version 3 and version 2`} | ${3} | ${2}
+ ${`No changes between version 3 and ${TEST_TARGET_BRANCH}`} | ${3} | ${-1}
+ `(
+ 'renders text "$expectedText" (sourceIndex=$sourceIndex and targetIndex=$targetIndex)',
+ ({ expectedText, targetIndex, sourceIndex }) => {
+ if (targetIndex !== null) {
+ store.state.diffs.startVersion = { version_index: targetIndex };
+ }
+ if (sourceIndex !== null) {
+ store.state.diffs.mergeRequestDiff.version_index = sourceIndex;
+ }
+
+ createComponent(mount);
+
+ expect(findMessage().text()).toBe(expectedText);
+ },
+ );
});
});
diff --git a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
index 19f3d28e45e..f7954515422 100644
--- a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
+++ b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
@@ -136,6 +136,7 @@ describe('Compare diff version dropdowns', () => {
...firstDiff,
href: firstDiff.version_path,
commitsText: `${firstDiff.commits_count} commits,`,
+ isLatestVersion: true,
versionName: 'latest version',
selected: true,
};
@@ -144,6 +145,9 @@ describe('Compare diff version dropdowns', () => {
selectedSourceIndex: expectedShape.version_index,
});
expect(sourceVersions[0]).toEqual(expectedShape);
- expect(sourceVersions[1].selected).toBe(false);
+ expect(sourceVersions[1]).toMatchObject({
+ selected: false,
+ isLatestVersion: false,
+ });
});
});
diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb
index 558fc479af1..21fc530149c 100644
--- a/spec/graphql/types/issue_type_spec.rb
+++ b/spec/graphql/types/issue_type_spec.rb
@@ -17,7 +17,8 @@ RSpec.describe GitlabSchema.types['Issue'] do
fields = %i[id iid title description state reference author assignees updated_by participants labels milestone due_date
confidential discussion_locked upvotes downvotes user_notes_count user_discussions_count web_path web_url relative_position
emails_disabled subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status
- design_collection alert_management_alert severity current_user_todos moved moved_to]
+ design_collection alert_management_alert severity current_user_todos moved moved_to
+ create_note_email]
fields.each do |field_name|
expect(described_class).to have_graphql_field(field_name)
diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
index ef448ee96a4..8872800069a 100644
--- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
@@ -4,146 +4,50 @@ require 'spec_helper'
RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
include_context :email_shared_context
+
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project, :public, :repository) }
+
+ let(:noteable) { note.noteable }
+ let(:note) { create(:diff_note_on_merge_request, project: project) }
+ let(:email_raw) { fixture_file('emails/valid_reply.eml') }
let!(:sent_notification) do
SentNotification.record_note(note, user.id, mail_key)
end
- let(:noteable) { note.noteable }
- let(:note) { create(:diff_note_on_merge_request, project: project) }
- let(:user) { create(:user) }
- let(:project) { create(:project, :public, :repository) }
- let(:email_raw) { fixture_file('emails/valid_reply.eml') }
-
it_behaves_like :reply_processing_shared_examples
+ it_behaves_like :note_handler_shared_examples do
+ let(:recipient) { sent_notification.recipient }
+
+ let(:update_commands_only) { fixture_file('emails/update_commands_only_reply.eml')}
+ let(:no_content) { fixture_file('emails/no_content_reply.eml') }
+ let(:commands_in_reply) { fixture_file('emails/commands_in_reply.eml') }
+ let(:with_quick_actions) { fixture_file('emails/valid_reply_with_quick_actions.eml') }
+ end
+
before do
stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo")
stub_config_setting(host: 'localhost')
end
- context "when the recipient address doesn't include a mail key" do
- let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") }
+ context 'when the recipient address does not include a mail key' do
+ let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, '') }
- it "raises a UnknownIncomingEmail" do
+ it 'raises a UnknownIncomingEmail' do
expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
end
end
- context "when no sent notification for the mail key could be found" do
+ context 'when no sent notification for the mail key could be found' do
let(:email_raw) { fixture_file('emails/wrong_mail_key.eml') }
- it "raises a SentNotificationNotFoundError" do
+ it 'raises a SentNotificationNotFoundError' do
expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError)
end
end
- context "when the noteable could not be found" do
- before do
- noteable.destroy
- end
-
- it "raises a NoteableNotFoundError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::NoteableNotFoundError)
- end
- end
-
- context "when the note could not be saved" do
- before do
- allow_any_instance_of(Note).to receive(:persisted?).and_return(false)
- end
-
- it "raises an InvalidNoteError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
- end
-
- context 'because the note was update commands only' do
- let!(:email_raw) { fixture_file("emails/update_commands_only_reply.eml") }
-
- context 'and current user cannot update noteable' do
- it 'raises a CommandsOnlyNoteError' do
- expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
- end
- end
-
- context "and current user can update noteable" do
- before do
- project.add_developer(user)
- end
-
- it 'does not raise an error' do
- expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1)
-
- expect(noteable.reload).to be_closed
- end
- end
- end
- end
-
- context 'when the note contains quick actions' do
- let!(:email_raw) { fixture_file("emails/commands_in_reply.eml") }
-
- context 'and current user cannot update the noteable' do
- it 'only executes the commands that the user can perform' do
- expect { receiver.execute }
- .to change { noteable.notes.user.count }.by(1)
- .and change { user.todos_pending_count }.from(0).to(1)
-
- expect(noteable.reload).to be_open
- end
- end
-
- context 'and current user can update noteable' do
- before do
- project.add_developer(user)
- end
-
- it 'posts a note and updates the noteable' do
- expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
-
- expect { receiver.execute }
- .to change { noteable.notes.user.count }.by(1)
- .and change { user.todos_pending_count }.from(0).to(1)
-
- expect(noteable.reload).to be_closed
- end
- end
- end
-
- context "when the reply is blank" do
- let!(:email_raw) { fixture_file("emails/no_content_reply.eml") }
-
- it "raises an EmptyEmailError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
- end
- end
-
- shared_examples "checks permissions on noteable" do
- context "when user has access" do
- before do
- project.add_reporter(user)
- end
-
- it "creates a comment" do
- expect { receiver.execute }.to change { noteable.notes.count }.by(1)
- end
- end
-
- context "when user does not have access" do
- it "raises UserNotAuthorizedError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
- end
- end
- end
-
- context "when discussion is locked" do
- before do
- noteable.update_attribute(:discussion_locked, true)
- end
-
- it_behaves_like "checks permissions on noteable"
- end
-
- context "when issue is confidential" do
+ context 'when issue is confidential' do
let(:issue) { create(:issue, project: project) }
let(:note) { create(:note, noteable: issue, project: project) }
@@ -151,17 +55,17 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
issue.update_attribute(:confidential, true)
end
- it_behaves_like "checks permissions on noteable"
+ it_behaves_like :checks_permissions_on_noteable_examples
end
shared_examples 'a reply to existing comment' do
- it "creates a comment" do
+ it 'creates a comment' do
expect { receiver.execute }.to change { noteable.notes.count }.by(1)
new_note = noteable.notes.last
expect(new_note.author).to eq(sent_notification.recipient)
expect(new_note.position).to eq(note.position)
- expect(new_note.note).to include("I could not disagree more.")
+ expect(new_note.note).to include('I could not disagree more.')
expect(new_note.in_reply_to?(note)).to be_truthy
if note.part_of_discussion?
@@ -172,32 +76,14 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
end
end
- context "when everything is fine" do
+ # additional shared tests in :reply_processing_shared_examples
+ context 'when everything is fine' do
before do
setup_attachment
end
it_behaves_like 'a reply to existing comment'
- it "adds all attachments" do
- expect_next_instance_of(Gitlab::Email::AttachmentUploader) do |uploader|
- expect(uploader).to receive(:execute).with(upload_parent: project, uploader_class: FileUploader).and_return(
- [
- {
- url: "uploads/image.png",
- alt: "image",
- markdown: markdown
- }
- ]
- )
- end
-
- receiver.execute
-
- note = noteable.notes.last
- expect(note.note).to include(markdown)
- end
-
context 'when sub-addressing is not supported' do
before do
stub_incoming_email_setting(enabled: true, address: nil)
@@ -228,75 +114,9 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
end
end
- context "when note is not a discussion" do
+ context 'when note is not a discussion' do
let(:note) { create(:note_on_merge_request, project: project) }
it_behaves_like 'a reply to existing comment'
end
-
- context 'when the service desk' do
- let(:project) { create(:project, :public, service_desk_enabled: true) }
- let(:support_bot) { User.support_bot }
- let(:noteable) { create(:issue, project: project, author: support_bot, title: 'service desk issue') }
- let(:note) { create(:note, project: project, noteable: noteable) }
- let(:email_raw) { fixture_file('emails/valid_reply_with_quick_actions.eml') }
-
- let!(:sent_notification) do
- SentNotification.record_note(note, support_bot.id, mail_key)
- end
-
- context 'is enabled' do
- before do
- allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
- project.project_feature.update!(issues_access_level: issues_access_level)
- end
-
- context 'when issues are enabled for everyone' do
- let(:issues_access_level) { ProjectFeature::ENABLED }
-
- it 'creates a comment' do
- expect { receiver.execute }.to change { noteable.notes.count }.by(1)
- end
-
- context 'when quick actions are present' do
- it 'encloses quick actions with code span markdown' do
- receiver.execute
- noteable.reload
-
- note = Note.last
- expect(note.note).to include("Jake out\n\n`/close`\n`/title test`")
- expect(noteable.title).to eq('service desk issue')
- expect(noteable).to be_opened
- end
- end
- end
-
- context 'when issues are protected members only' do
- let(:issues_access_level) { ProjectFeature::PRIVATE }
-
- it 'creates a comment' do
- expect { receiver.execute }.to change { noteable.notes.count }.by(1)
- end
- end
-
- context 'when issues are disabled' do
- let(:issues_access_level) { ProjectFeature::DISABLED }
-
- it 'does not create a comment' do
- expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
- end
- end
- end
-
- context 'is disabled' do
- before do
- allow(Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
- allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(false)
- end
-
- it 'does not create a comment' do
- expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
- end
- end
- end
end
diff --git a/spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb
new file mode 100644
index 00000000000..94f28d3399a
--- /dev/null
+++ b/spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Email::Handler::CreateNoteOnIssuableHandler do
+ include_context :email_shared_context
+
+ let_it_be(:user) { create(:user, email: 'jake@adventuretime.ooo', incoming_email_token: 'auth_token') }
+ let_it_be(:namespace) { create(:namespace, path: 'gitlabhq') }
+ let_it_be(:project) { create(:project, :public, namespace: namespace, path: 'gitlabhq') }
+
+ let!(:noteable) { create(:issue, project: project) }
+ let(:email_raw) { email_fixture('emails/valid_note_on_issuable.eml') }
+
+ before do
+ stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo")
+ stub_config_setting(host: 'localhost')
+ end
+
+ it_behaves_like :reply_processing_shared_examples
+
+ it_behaves_like :note_handler_shared_examples, true do
+ let_it_be(:recipient) { user }
+
+ let(:update_commands_only) { email_reply_fixture('emails/update_commands_only_reply.eml') }
+ let(:no_content) { email_reply_fixture('emails/no_content_reply.eml') }
+ let(:commands_in_reply) { email_reply_fixture('emails/commands_in_reply.eml') }
+ let(:with_quick_actions) { email_reply_fixture('emails/valid_reply_with_quick_actions.eml') }
+ end
+
+ context 'when the recipient address does not include a mail key' do
+ let(:mail_key) { 'gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid' }
+ let(:email_raw) { fixture_file('emails/valid_note_on_issuable.eml').gsub(mail_key, '') }
+
+ it 'raises an UnknownIncomingEmail' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
+ end
+ end
+
+ context 'when issue is confidential' do
+ before do
+ noteable.update_attribute(:confidential, true)
+ end
+
+ it_behaves_like :checks_permissions_on_noteable_examples
+ end
+
+ def email_fixture(path)
+ fixture_file(path)
+ .gsub('project_id', project.project_id.to_s)
+ .gsub('issue_iid', noteable.iid.to_s)
+ end
+
+ def email_reply_fixture(path)
+ reply_address = 'reply+59d8df8370b7e95c5a49fbf86aeb2c93'
+ note_address = "incoming+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-issue-#{noteable.iid}"
+
+ fixture_file(path)
+ .gsub(reply_address, note_address)
+ end
+end
diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb
index 2cd8c31e6b2..eff6fb63a5f 100644
--- a/spec/lib/gitlab/email/handler_spec.rb
+++ b/spec/lib/gitlab/email/handler_spec.rb
@@ -60,8 +60,9 @@ RSpec.describe Gitlab::Email::Handler do
describe 'regexps are set properly' do
let(:addresses) do
- %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path-to-project-123-user_email_token-merge-request) +
- %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY} sent_notification_key path-to-project-123-user_email_token-issue) +
+ %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}) +
+ %w(sent_notification_key path-to-project-123-user_email_token-merge-request) +
+ %w(path-to-project-123-user_email_token-issue path-to-project-123-user_email_token-issue-123) +
%w(path/to/project+user_email_token path/to/project+merge-request+user_email_token some/project)
end
diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb
index bb7374bf46c..a7117af81a2 100644
--- a/spec/models/concerns/noteable_spec.rb
+++ b/spec/models/concerns/noteable_spec.rb
@@ -25,8 +25,8 @@ RSpec.describe Noteable do
let(:active_position2) do
Gitlab::Diff::Position.new(
- old_path: "files/ruby/popen.rb",
- new_path: "files/ruby/popen.rb",
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
old_line: 16,
new_line: 22,
diff_refs: subject.diff_refs
@@ -35,11 +35,11 @@ RSpec.describe Noteable do
let(:outdated_position) do
Gitlab::Diff::Position.new(
- old_path: "files/ruby/popen.rb",
- new_path: "files/ruby/popen.rb",
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
old_line: nil,
new_line: 9,
- diff_refs: project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs
+ diff_refs: project.commit('874797c3a73b60d2187ed6e2fcabd289ff75171e').diff_refs
)
end
@@ -80,7 +80,7 @@ RSpec.describe Noteable do
describe '#grouped_diff_discussions' do
let(:grouped_diff_discussions) { subject.grouped_diff_discussions }
- it "includes active discussions" do
+ it 'includes active discussions' do
discussions = grouped_diff_discussions.values.flatten
expect(discussions.count).to eq(2)
@@ -91,17 +91,17 @@ RSpec.describe Noteable do
expect(discussions.last.notes).to eq([active_diff_note3])
end
- it "doesn't include outdated discussions" do
+ it 'does not include outdated discussions' do
expect(grouped_diff_discussions.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
end
- it "groups the discussions by line code" do
+ it 'groups the discussions by line code' do
expect(grouped_diff_discussions[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id)
expect(grouped_diff_discussions[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id)
end
end
- context "discussion status" do
+ context 'discussion status' do
let(:first_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
let(:second_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
let(:third_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion }
@@ -110,56 +110,56 @@ RSpec.describe Noteable do
allow(subject).to receive(:resolvable_discussions).and_return([first_discussion, second_discussion, third_discussion])
end
- describe "#discussions_resolvable?" do
- context "when all discussions are unresolvable" do
+ describe '#discussions_resolvable?' do
+ context 'when all discussions are unresolvable' do
before do
allow(first_discussion).to receive(:resolvable?).and_return(false)
allow(second_discussion).to receive(:resolvable?).and_return(false)
allow(third_discussion).to receive(:resolvable?).and_return(false)
end
- it "returns false" do
+ it 'returns false' do
expect(subject.discussions_resolvable?).to be false
end
end
- context "when some discussions are unresolvable and some discussions are resolvable" do
+ context 'when some discussions are unresolvable and some discussions are resolvable' do
before do
allow(first_discussion).to receive(:resolvable?).and_return(true)
allow(second_discussion).to receive(:resolvable?).and_return(false)
allow(third_discussion).to receive(:resolvable?).and_return(true)
end
- it "returns true" do
+ it 'returns true' do
expect(subject.discussions_resolvable?).to be true
end
end
- context "when all discussions are resolvable" do
+ context 'when all discussions are resolvable' do
before do
allow(first_discussion).to receive(:resolvable?).and_return(true)
allow(second_discussion).to receive(:resolvable?).and_return(true)
allow(third_discussion).to receive(:resolvable?).and_return(true)
end
- it "returns true" do
+ it 'returns true' do
expect(subject.discussions_resolvable?).to be true
end
end
end
- describe "#discussions_resolved?" do
- context "when discussions are not resolvable" do
+ describe '#discussions_resolved?' do
+ context 'when discussions are not resolvable' do
before do
allow(subject).to receive(:discussions_resolvable?).and_return(false)
end
- it "returns false" do
+ it 'returns false' do
expect(subject.discussions_resolved?).to be false
end
end
- context "when discussions are resolvable" do
+ context 'when discussions are resolvable' do
before do
allow(subject).to receive(:discussions_resolvable?).and_return(true)
@@ -168,31 +168,31 @@ RSpec.describe Noteable do
allow(third_discussion).to receive(:resolvable?).and_return(true)
end
- context "when all resolvable discussions are resolved" do
+ context 'when all resolvable discussions are resolved' do
before do
allow(first_discussion).to receive(:resolved?).and_return(true)
allow(third_discussion).to receive(:resolved?).and_return(true)
end
- it "returns true" do
+ it 'returns true' do
expect(subject.discussions_resolved?).to be true
end
end
- context "when some resolvable discussions are not resolved" do
+ context 'when some resolvable discussions are not resolved' do
before do
allow(first_discussion).to receive(:resolved?).and_return(true)
allow(third_discussion).to receive(:resolved?).and_return(false)
end
- it "returns false" do
+ it 'returns false' do
expect(subject.discussions_resolved?).to be false
end
end
end
end
- describe "#discussions_to_be_resolved" do
+ describe '#discussions_to_be_resolved' do
before do
allow(first_discussion).to receive(:to_be_resolved?).and_return(true)
allow(second_discussion).to receive(:to_be_resolved?).and_return(false)
@@ -245,6 +245,12 @@ RSpec.describe Noteable do
end
end
+ describe '.email_creatable_types' do
+ it 'exposes the email creatable types' do
+ expect(described_class.email_creatable_types).to include('Issue')
+ end
+ end
+
describe '#capped_notes_count' do
context 'notes number < 10' do
it 'the number of notes is returned' do
@@ -263,13 +269,13 @@ RSpec.describe Noteable do
end
end
- describe "#has_any_diff_note_positions?" do
- let(:source_branch) { "compare-with-merge-head-source" }
- let(:target_branch) { "compare-with-merge-head-target" }
+ describe '#has_any_diff_note_positions?' do
+ let(:source_branch) { 'compare-with-merge-head-source' }
+ let(:target_branch) { 'compare-with-merge-head-target' }
let(:merge_request) { create(:merge_request, source_branch: source_branch, target_branch: target_branch) }
let!(:note) do
- path = "files/markdown/ruby-style-guide.md"
+ path = 'files/markdown/ruby-style-guide.md'
position = Gitlab::Diff::Position.new(
old_path: path,
@@ -286,20 +292,54 @@ RSpec.describe Noteable do
Discussions::CaptureDiffNotePositionsService.new(merge_request).execute
end
- it "returns true when it has diff note positions" do
+ it 'returns true when it has diff note positions' do
expect(merge_request.has_any_diff_note_positions?).to be(true)
end
- it "returns false when it has notes but no diff note positions" do
+ it 'returns false when it has notes but no diff note positions' do
DiffNotePosition.where(note: note).find_each(&:delete)
expect(merge_request.has_any_diff_note_positions?).to be(false)
end
- it "returns false when it has no notes" do
+ it 'returns false when it has no notes' do
merge_request.notes.find_each(&:destroy)
expect(merge_request.has_any_diff_note_positions?).to be(false)
end
end
+
+ describe '#creatable_note_email_address' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:source_branch) { 'compare-with-merge-head-source' }
+
+ let(:issue) { create(:issue, project: project) }
+ let(:snippet) { build(:snippet) }
+
+ context 'incoming email enabled' do
+ before do
+ stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab")
+ end
+
+ it 'returns the address to create a note' do
+ address = "p+#{project.full_path_slug}-#{project.project_id}-#{user.incoming_email_token}-issue-#{issue.iid}@gl.ab"
+
+ expect(issue.creatable_note_email_address(user)).to eq(address)
+ end
+
+ it 'returns nil for unsupported types' do
+ expect(snippet.creatable_note_email_address(user)).to be_nil
+ end
+ end
+
+ context 'incoming email disabled' do
+ before do
+ stub_incoming_email_setting(enabled: false)
+ end
+
+ it 'returns nil' do
+ expect(issue.creatable_note_email_address(user)).to be_nil
+ end
+ end
+ end
end
diff --git a/spec/support/shared_contexts/email_shared_context.rb b/spec/support/shared_contexts/email_shared_context.rb
index 298e03162c4..9dffea7c94e 100644
--- a/spec/support/shared_contexts/email_shared_context.rb
+++ b/spec/support/shared_contexts/email_shared_context.rb
@@ -1,16 +1,16 @@
# frozen_string_literal: true
RSpec.shared_context :email_shared_context do
- let(:mail_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
+ let(:mail_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' }
let(:receiver) { Gitlab::Email::Receiver.new(email_raw) }
- let(:markdown) { "" }
+ let(:markdown) { '' }
def setup_attachment
allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return(
[
{
- url: "uploads/image.png",
- alt: "image",
+ url: 'uploads/image.png',
+ alt: 'image',
markdown: markdown
}
]
@@ -19,23 +19,252 @@ RSpec.shared_context :email_shared_context do
end
RSpec.shared_examples :reply_processing_shared_examples do
- context "when the user could not be found" do
+ context 'when the user could not be found' do
before do
user.destroy!
end
- it "raises a UserNotFoundError" do
+ it 'raises a UserNotFoundError' do
expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError)
end
end
- context "when the user is not authorized to the project" do
+ context 'when the user is not authorized to the project' do
before do
project.update_attribute(:visibility_level, Project::PRIVATE)
end
- it "raises a ProjectNotFound" do
+ it 'raises a ProjectNotFound' do
expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
end
end
end
+
+RSpec.shared_examples :checks_permissions_on_noteable_examples do
+ context 'when user has access' do
+ before do
+ project.add_reporter(user)
+ end
+
+ it 'creates a comment' do
+ expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+ end
+ end
+
+ context 'when user does not have access' do
+ it 'raises UserNotAuthorizedError' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
+ end
+ end
+end
+
+RSpec.shared_examples :note_handler_shared_examples do |forwardable|
+ context 'when the noteable could not be found' do
+ before do
+ noteable.destroy!
+ end
+
+ it 'raises a NoteableNotFoundError' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::NoteableNotFoundError)
+ end
+ end
+
+ context 'when the note could not be saved' do
+ before do
+ allow_any_instance_of(Note).to receive(:persisted?).and_return(false)
+ end
+
+ it 'raises an InvalidNoteError' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
+ end
+
+ context 'because the note was update commands only' do
+ let!(:email_raw) { update_commands_only }
+
+ context 'and current user cannot update noteable' do
+ it 'raises a CommandsOnlyNoteError' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
+ end
+ end
+
+ context 'and current user can update noteable' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'does not raise an error', unless: forwardable do
+ expect { receiver.execute }.to change { noteable.resource_state_events.count }.by(1)
+
+ expect(noteable.reload).to be_closed
+ end
+
+ it 'raises an InvalidNoteError', if: forwardable do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
+ end
+ end
+ end
+ end
+
+ context 'when the note contains quick actions' do
+ let!(:email_raw) { commands_in_reply }
+
+ context 'and current user cannot update the noteable' do
+ it 'only executes the commands that the user can perform' do
+ expect { receiver.execute }
+ .to change { noteable.notes.user.count }.by(1)
+ .and change { user.todos_pending_count }.from(0).to(1)
+
+ expect(noteable.reload).to be_open
+ end
+ end
+
+ context 'and current user can update noteable' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'posts a note and updates the noteable' do
+ expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
+
+ expect { receiver.execute }
+ .to change { noteable.notes.user.count }.by(1)
+ .and change { user.todos_pending_count }.from(0).to(1)
+
+ expect(noteable.reload).to be_closed
+ end
+ end
+ end
+
+ context 'when the reply is blank' do
+ let!(:email_raw) { no_content }
+
+ it 'raises an EmptyEmailError', unless: forwardable do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
+ end
+
+ it 'allows email to only have quoted text', if: forwardable do
+ expect { receiver.execute }.not_to raise_error(Gitlab::Email::EmptyEmailError)
+ end
+ end
+
+ context 'when discussion is locked' do
+ before do
+ noteable.update_attribute(:discussion_locked, true)
+ end
+
+ it_behaves_like :checks_permissions_on_noteable_examples
+ end
+
+ context 'when everything is fine' do
+ before do
+ setup_attachment
+ end
+
+ it 'adds all attachments' do
+ expect_next_instance_of(Gitlab::Email::AttachmentUploader) do |uploader|
+ expect(uploader).to receive(:execute).with(upload_parent: project, uploader_class: FileUploader).and_return(
+ [
+ {
+ url: 'uploads/image.png',
+ alt: 'image',
+ markdown: markdown
+ }
+ ]
+ )
+ end
+
+ receiver.execute
+
+ note = noteable.notes.last
+ expect(note.note).to include(markdown)
+ expect(note.note).to include('Jake out')
+ end
+ end
+
+ context 'when the service desk' do
+ let(:project) { create(:project, :public, service_desk_enabled: true) }
+ let(:support_bot) { User.support_bot }
+ let(:noteable) { create(:issue, project: project, author: support_bot, title: 'service desk issue') }
+ let!(:note) { create(:note, project: project, noteable: noteable) }
+ let(:email_raw) { with_quick_actions }
+
+ let!(:sent_notification) do
+ SentNotification.record_note(note, support_bot.id, mail_key)
+ end
+
+ context 'is enabled' do
+ before do
+ allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
+ project.project_feature.update!(issues_access_level: issues_access_level)
+ end
+
+ context 'when issues are enabled for everyone' do
+ let(:issues_access_level) { ProjectFeature::ENABLED }
+
+ it 'creates a comment' do
+ expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+ end
+
+ context 'when quick actions are present' do
+ before do
+ receiver.execute
+ noteable.reload
+ end
+
+ context 'when author is a support_bot', unless: forwardable do
+ it 'encloses quick actions with code span markdown' do
+ note = Note.last
+ expect(note.note).to include("Jake out\n\n`/close`\n`/title test`")
+ expect(noteable.title).to eq('service desk issue')
+ expect(noteable).to be_opened
+ end
+ end
+
+ context 'when author is a normal user', if: forwardable do
+ it 'extracted the quick actions' do
+ note = Note.last
+ expect(note.note).to include('Jake out')
+ expect(note.note).not_to include("`/close`\n`/title test`")
+ end
+ end
+ end
+ end
+
+ context 'when issues are protected members only' do
+ let(:issues_access_level) { ProjectFeature::PRIVATE }
+
+ before do
+ if recipient.support_bot?
+ @changed_by = 1
+ else
+ @changed_by = 2
+ project.add_developer(recipient)
+ end
+ end
+
+ it 'creates a comment' do
+ expect { receiver.execute }.to change { noteable.notes.count }.by(@changed_by)
+ end
+ end
+
+ context 'when issues are disabled' do
+ let(:issues_access_level) { ProjectFeature::DISABLED }
+
+ it 'does not create a comment' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
+ end
+ end
+ end
+
+ context 'is disabled', unless: forwardable do
+ before do
+ allow(Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
+ allow(Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(false)
+ end
+
+ it 'does not create a comment' do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+ end
+ end
+ end
+end