kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

Browse source
This commit is contained in:
GitLab Bot 2021-01-05 03:10:19 +00:00
parent b28aa8bd7d
commit eb29a43da9
31 changed files with 828 additions and 394 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
app/assets/javascripts/diffs/components/app.vue
View file

@ -145,8 +145,6 @@ export default {
isBatchLoading: (state) => state.diffs.isBatchLoading,
diffFiles: (state) => state.diffs.diffFiles,
diffViewType: (state) => state.diffs.diffViewType,
mergeRequestDiffs: (state) => state.diffs.mergeRequestDiffs,
mergeRequestDiff: (state) => state.diffs.mergeRequestDiff,
commit: (state) => state.diffs.commit,
renderOverflowWarning: (state) => state.diffs.renderOverflowWarning,
numTotalFiles: (state) => state.diffs.realSize,
@ -186,17 +184,16 @@ export default {
return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request;
},
renderDiffFiles() {
return (
this.diffFiles.length > 0 ||
(this.startVersion &&
this.startVersion.version_index === this.mergeRequestDiff.version_index)
);
return this.diffFiles.length > 0;
},
renderFileTree() {
return this.renderDiffFiles && this.showTreeList;
},
hideFileStats() {
return this.treeWidth <= TREE_HIDE_STATS_WIDTH;
},
isLimitedContainer() {
return !this.showTreeList && !this.isParallelView && !this.isFluidLayout;
return !this.renderFileTree && !this.isParallelView && !this.isFluidLayout;
},
isDiffHead() {
return parseBoolean(getParameterByName('diff_head'));
@ -259,7 +256,7 @@ export default {
this.adjustView();
},
isLoading: 'adjustView',
showTreeList: 'adjustView',
renderFileTree: 'adjustView',
},
mounted() {
this.setBaseConfig({
@ -467,7 +464,6 @@ export default {
<div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div>
<div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane">
<compare-versions
:merge-request-diffs="mergeRequestDiffs"
:is-limited-container="isLimitedContainer"
:diff-files-count-text="numTotalFiles"
/>
@ -495,7 +491,7 @@ export default {
class="files d-flex gl-mt-2"
>
<div
v-if="showTreeList"
v-if="renderFileTree"
:style="{ width: `${treeWidth}px` }"
class="diff-tree-list js-diff-tree-list px-3 pr-md-0"
>

27
app/assets/javascripts/diffs/components/compare_versions.vue
View file

@ -22,10 +22,6 @@ export default {
GlTooltip: GlTooltipDirective,
},
props: {
mergeRequestDiffs: {
type: Array,
required: true,
},
isLimitedContainer: {
type: Boolean,
required: false,
@ -44,6 +40,7 @@ export default {
'diffCompareDropdownSourceVersions',
]),
...mapState('diffs', [
'diffFiles',
'commit',
'showTreeList',
'startVersion',
@ -51,12 +48,15 @@ export default {
'addedLines',
'removedLines',
]),
showDropdowns() {
return !this.commit && this.mergeRequestDiffs.length;
},
toggleFileBrowserTitle() {
return this.showTreeList ? __('Hide file browser') : __('Show file browser');
},
hasChanges() {
return this.diffFiles.length > 0;
},
hasSourceVersions() {
return this.diffCompareDropdownSourceVersions.length > 0;
},
},
created() {
this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES;
@ -82,6 +82,7 @@ export default {
}"
>
<gl-button
v-if="hasChanges"
v-gl-tooltip.hover
variant="default"
icon="file-tree"
@ -90,8 +91,12 @@ export default {
:selected="showTreeList"
@click="setShowTreeList({ showTreeList: !showTreeList })"
/>
<div v-if="commit">
{{ __('Viewing commit') }}
<gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link>
</div>
<gl-sprintf
v-if="showDropdowns"
v-else-if="hasSourceVersions"
class="d-flex align-items-center compare-versions-container"
:message="s__('MergeRequest|Compare %{target} and %{source}')"
>
@ -109,11 +114,7 @@ export default {
/>
</template>
</gl-sprintf>
<div v-else-if="commit">
{{ __('Viewing commit') }}
<gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link>
</div>
<div class="inline-parallel-buttons d-none d-md-flex ml-auto">
<div v-if="hasChanges" class="inline-parallel-buttons d-none d-md-flex ml-auto">
<diff-stats
:diff-files-count-text="diffFilesCountText"
:added-lines="addedLines"

42
app/assets/javascripts/diffs/components/no_changes.vue
View file

@ -14,7 +14,31 @@ export default {
},
},
computed: {
...mapGetters('diffs', [
'diffCompareDropdownTargetVersions',
'diffCompareDropdownSourceVersions',
]),
...mapGetters(['getNoteableData']),
selectedSourceVersion() {
return this.diffCompareDropdownSourceVersions.find((x) => x.selected);
},
sourceName() {
if (!this.selectedSourceVersion || this.selectedSourceVersion.isLatestVersion) {
return this.getNoteableData.source_branch;
}
return this.selectedSourceVersion.versionName;
},
selectedTargetVersion() {
return this.diffCompareDropdownTargetVersions.find((x) => x.selected);
},
targetName() {
if (!this.selectedTargetVersion || this.selectedTargetVersion.version_index < 0) {
return this.getNoteableData.target_branch;
}
return this.selectedTargetVersion.versionName || '';
},
},
};
</script>
@ -26,14 +50,16 @@ export default {
</div>
<div class="col-12">
<div class="text-content text-center">
<gl-sprintf :message="__('No changes between %{sourceBranch} and %{targetBranch}')">
<template #sourceBranch>
<span class="ref-name">{{ getNoteableData.source_branch }}</span>
</template>
<template #targetBranch>
<span class="ref-name">{{ getNoteableData.target_branch }}</span>
</template>
</gl-sprintf>
<div data-testid="no-changes-message">
<gl-sprintf :message="__('No changes between %{source} and %{target}')">
<template #source>
<span class="ref-name">{{ sourceName }}</span>
</template>
<template #target>
<span class="ref-name">{{ targetName }}</span>
</template>
</gl-sprintf>
</div>
<div class="text-center">
<gl-button :href="getNoteableData.new_blob_path" variant="success" category="primary">{{
__('Create commit')

20
app/assets/javascripts/diffs/store/getters_versions_dropdowns.js
View file

@ -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,
};
});
};

7
app/graphql/types/issue_type.rb
View file

@ -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

3
app/helpers/issuables_helper.rb
View file

@ -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

21
app/models/concerns/noteable.rb
View file

@ -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)

4
app/serializers/issuable_sidebar_basic_entity.rb
View file

@ -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

9
app/views/projects/branches/new.html.haml
View file

@ -2,9 +2,12 @@
- default_ref = params[:ref] || @project.default_branch
- if @error
.alert.alert-danger
%button.close{ type: "button", "data-dismiss" => "alert" } &times;
= @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

5
changelogs/unreleased/18816-forward-an-e-mail-to-an-existing-issue-as-a-new-comment.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: New user/issue specific email address for creating/forwarding to an issue
merge_request: 49050
author:
type: added

5
changelogs/unreleased/233658-migrate-alert-on-new-branch-page.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Migrates the alert on the new branch page
merge_request: 50307
author:
type: other

5
changelogs/unreleased/256035-empty-changes-tab-on-merge-requests-should-not-show-file-browser-a.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Remove diff display preferences and file tree from changes empty state
merge_request: 43467
author:
type: fixed

10
doc/api/graphql/reference/gitlab_schema.graphql
View file

@ -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
"""

28
doc/api/graphql/reference/gitlab_schema.json
View file

@ -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",

2
doc/api/graphql/reference/index.md
View file

@ -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 |

2
doc/subscriptions/self_managed/index.md
View file

@ -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

1
lib/gitlab/email/handler.rb
View file

@ -11,6 +11,7 @@ module Gitlab
[
CreateNoteHandler,
CreateIssueHandler,
CreateNoteOnIssuableHandler,
UnsubscribeHandler,
CreateMergeRequestHandler,
ServiceDeskHandler

81
lib/gitlab/email/handler/create_note_on_issuable_handler.rb Normal file
View file

@ -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}-(?<incoming_email_token>.+)-issue-(?<issuable_iid>\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

2
locale/gitlab.pot
View file

@ -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"

4
spec/features/merge_request/user_sees_versions_spec.rb
View file

@ -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

24
spec/fixtures/emails/valid_note_on_issuable.eml vendored Normal file
View file

@ -0,0 +1,24 @@
Return-Path: <jake@adventuretime.ooo>
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 <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo>; 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 <jake@adventuretime.ooo>
To: incoming+gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
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.

65
spec/frontend/diffs/components/app_spec.js
View file

@ -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);
});
});

96
spec/frontend/diffs/components/compare_versions_spec.js
View file

@ -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);
});
});
});

81
spec/frontend/diffs/components/no_changes_spec.js
View file

@ -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: '<script>alert("test");</script>',
target_branch: '<script>alert("test");</script>',
};
});
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: '<script>alert("test");</script>',
target_branch: '<script>alert("test");</script>',
};
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);
},
);
});
});

6
spec/frontend/diffs/store/getters_versions_dropdowns_spec.js
View file

@ -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,
});
});
});

3
spec/graphql/types/issue_type_spec.rb
View file

@ -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)

236
spec/lib/gitlab/email/handler/create_note_handler_spec.rb
View file

@ -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

61
spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb Normal file
View file

@ -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

5
spec/lib/gitlab/email/handler_spec.rb
View file

@ -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

104
spec/models/concerns/noteable_spec.rb
View file

@ -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

245
spec/support/shared_contexts/email_shared_context.rb
View file

@ -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) { "![image](uploads/image.png)" }
let(:markdown) { '![image](uploads/image.png)' }
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