summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorClement Ho <clemmakesapps@gmail.com>2018-05-01 16:21:47 +0000
committerClement Ho <clemmakesapps@gmail.com>2018-05-01 16:21:47 +0000
commitf9e2b4730f58ba630344c9554eb907ab003abbd5 (patch)
tree8ea283d80956ce9117f40cd58469e7dc26a4bb44
parentb49ac65e3fcfcac87157810b08d20efd8b8e5e73 (diff)
parent5b92d405fd6e52e6bf1ab1d440ece5a5c1654198 (diff)
downloadgitlab-ce-f9e2b4730f58ba630344c9554eb907ab003abbd5.tar.gz
Merge branch 'master' into 'bootstrap4'
# Conflicts: # app/views/projects/branches/_branch.html.haml
-rw-r--r--CHANGELOG.md22
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--app/assets/javascripts/notes/components/discussion_counter.vue2
-rw-r--r--app/assets/javascripts/sidebar/lib/sidebar_move_issue.js3
-rw-r--r--app/assets/javascripts/vue_shared/components/identicon.vue2
-rw-r--r--app/assets/stylesheets/pages/notes.scss4
-rw-r--r--app/controllers/omniauth_callbacks_controller.rb4
-rw-r--r--app/controllers/projects/notes_controller.rb4
-rw-r--r--app/finders/groups_finder.rb8
-rw-r--r--app/helpers/application_helper.rb74
-rw-r--r--app/helpers/auth_helper.rb8
-rw-r--r--app/helpers/avatars_helper.rb74
-rw-r--r--app/helpers/system_note_helper.rb6
-rw-r--r--app/mailers/notify.rb1
-rw-r--r--app/models/commit.rb4
-rw-r--r--app/models/diff_note.rb15
-rw-r--r--app/services/notes/resolve_service.rb9
-rw-r--r--app/services/repository_archive_clean_up_service.rb5
-rw-r--r--app/views/projects/branches/_branch.html.haml11
-rw-r--r--changelogs/unreleased/dm-commit-trailer-without-gravatar.yml5
-rw-r--r--changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml5
-rw-r--r--changelogs/unreleased/fix-inconsistent-protected-branch-pill-baseline.yml5
-rw-r--r--changelogs/unreleased/helm-add-alpine-mirrors.yml5
-rw-r--r--changelogs/unreleased/jprovazn-commit-notes-api.yml5
-rw-r--r--changelogs/unreleased/osw-use-cached-highlighted-content-for-discussions.yml5
-rw-r--r--changelogs/unreleased/revert-discussion-counter-height.yml5
-rw-r--r--changelogs/unreleased/security-45689-fix-archive-cache-bug.yml5
-rw-r--r--changelogs/unreleased/security_issue_42029.yml5
-rw-r--r--changelogs/unreleased/update-timeline-icon-for-description-edit.yml5
-rw-r--r--doc/api/discussions.md585
-rw-r--r--doc/api/groups.md4
-rw-r--r--doc/api/notes.md9
-rw-r--r--lib/api/discussions.rb88
-rw-r--r--lib/api/entities.rb19
-rw-r--r--lib/api/groups.rb16
-rw-r--r--lib/api/helpers.rb4
-rw-r--r--lib/api/helpers/custom_attributes.rb3
-rw-r--r--lib/api/helpers/notes_helpers.rb56
-rw-r--r--lib/api/notes.rb30
-rw-r--r--lib/banzai/filter/commit_trailers_filter.rb1
-rw-r--r--lib/gitlab/diff/file_collection/base.rb2
-rw-r--r--lib/gitlab/diff/position.rb4
-rw-r--r--lib/gitlab/git/raw_diff_change.rb4
-rw-r--r--lib/gitlab/git/repository.rb51
-rw-r--r--lib/gitlab/kubernetes/helm/base_command.rb3
-rw-r--r--spec/features/dashboard/groups_list_spec.rb22
-rw-r--r--spec/features/projects/issues/user_toggles_subscription_spec.rb8
-rw-r--r--spec/finders/groups_finder_spec.rb84
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/notes.json5
-rw-r--r--spec/helpers/application_helper_spec.rb139
-rw-r--r--spec/helpers/auth_helper_spec.rb24
-rw-r--r--spec/helpers/avatars_helper_spec.rb139
-rw-r--r--spec/javascripts/sidebar/mock_data.js2
-rw-r--r--spec/javascripts/sidebar/sidebar_move_issue_spec.js9
-rw-r--r--spec/lib/banzai/filter/commit_trailers_filter_spec.rb40
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb87
-rw-r--r--spec/lib/gitlab/kubernetes/helm/base_command_spec.rb18
-rw-r--r--spec/lib/gitlab/kubernetes/helm/init_command_spec.rb20
-rw-r--r--spec/lib/gitlab/kubernetes/helm/install_command_spec.rb66
-rw-r--r--spec/models/diff_note_spec.rb33
-rw-r--r--spec/requests/api/discussions_spec.rb33
-rw-r--r--spec/services/notes/resolve_service_spec.rb23
-rw-r--r--spec/services/repository_archive_clean_up_service_spec.rb68
-rw-r--r--spec/support/commit_trailers_spec_helper.rb2
-rw-r--r--spec/support/shared_examples/helm_generated_script.rb19
-rw-r--r--spec/support/shared_examples/requests/api/diff_discussions.rb57
-rw-r--r--spec/support/shared_examples/requests/api/resolvable_discussions.rb87
67 files changed, 1658 insertions, 514 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8278119cf10..e7db98858e8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,14 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 10.7.2 (2018-04-25)
+
+### Security (2 changes)
+
+- Serve archive requests with the correct file in all cases.
+- Sanitizes user name to avoid XSS attacks.
+
+
## 10.7.1 (2018-04-23)
### Fixed (11 changes)
@@ -237,6 +245,13 @@ entry.
- Upgrade Gitaly to upgrade its charlock_holmes.
+## 10.6.5 (2018-04-24)
+
+### Security (1 change)
+
+- Sanitizes user name to avoid XSS attacks.
+
+
## 10.6.4 (2018-04-09)
### Fixed (8 changes, 1 of them is from the community)
@@ -478,6 +493,13 @@ entry.
- Use host URL to build JIRA remote link icon.
+## 10.5.8 (2018-04-24)
+
+### Security (1 change)
+
+- Sanitizes user name to avoid XSS attacks.
+
+
## 10.5.7 (2018-04-03)
### Security (2 changes)
diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index ee74734aa22..6aba2b245a8 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-4.1.0
+4.2.0
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index a75af1e5322..a6d5adaff12 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -86,7 +86,7 @@ export default {
v-html="resolveSvg"
></span>
</span>
- <span class=".line-resolve-text">
+ <span class="line-resolve-text">
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ countText }} resolved
</span>
</div>
diff --git a/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
index 1eadebc7004..b267422cd97 100644
--- a/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
+++ b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js
@@ -1,4 +1,5 @@
import $ from 'jquery';
+import _ from 'underscore';
function isValidProjectId(id) {
return id > 0;
@@ -43,7 +44,7 @@ class SidebarMoveIssue {
renderRow: project => `
<li>
<a href="#" class="js-move-issue-dropdown-item">
- ${project.name_with_namespace}
+ ${_.escape(project.name_with_namespace)}
</a>
</li>
`,
diff --git a/app/assets/javascripts/vue_shared/components/identicon.vue b/app/assets/javascripts/vue_shared/components/identicon.vue
index 0a30f467b08..23010f40f26 100644
--- a/app/assets/javascripts/vue_shared/components/identicon.vue
+++ b/app/assets/javascripts/vue_shared/components/identicon.vue
@@ -17,7 +17,7 @@ export default {
},
computed: {
/**
- * This method is based on app/helpers/application_helper.rb#project_identicon
+ * This method is based on app/helpers/avatars_helper.rb#project_identicon
*/
identiconStyles() {
const allowedColors = [
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 08a14f0a8a8..f568c5d7036 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -776,7 +776,3 @@ ul.notes {
height: auto;
}
}
-
-.line-resolve-text {
- vertical-align: middle;
-}
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 9137bc92810..40d9fa18a10 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -8,8 +8,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
omniauth_flow(Gitlab::Auth::OAuth)
end
- Gitlab.config.omniauth.providers.each do |provider|
- alias_method provider['name'], :handle_omniauth
+ AuthHelper.providers_for_base_controller.each do |provider|
+ alias_method provider, :handle_omniauth
end
# Extend the standard implementation to also increment
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 86c50d88a2a..bc13b8ad7ba 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -33,9 +33,7 @@ class Projects::NotesController < Projects::ApplicationController
def resolve
return render_404 unless note.resolvable?
- note.resolve!(current_user)
-
- MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable)
+ Notes::ResolveService.new(project, current_user).execute(note)
discussion = note.discussion
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index 0282b378d88..0754123a3cf 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -39,7 +39,7 @@ class GroupsFinder < UnionFinder
def all_groups
return [owned_groups] if params[:owned]
- return [Group.all] if current_user&.full_private_access?
+ return [Group.all] if current_user&.full_private_access? && all_available?
groups = []
groups << Gitlab::GroupHierarchy.new(groups_for_ancestors, groups_for_descendants).all_groups if current_user
@@ -67,6 +67,10 @@ class GroupsFinder < UnionFinder
end
def include_public_groups?
- current_user.nil? || params.fetch(:all_available, true)
+ current_user.nil? || all_available?
+ end
+
+ def all_available?
+ params.fetch(:all_available, true)
end
end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 228c8d2e8f9..6aa307b4db4 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -32,80 +32,6 @@ module ApplicationHelper
args.any? { |v| v.to_s.downcase == action_name }
end
- def project_icon(project_id, options = {})
- project =
- if project_id.respond_to?(:avatar_url)
- project_id
- else
- Project.find_by_full_path(project_id)
- end
-
- if project.avatar_url
- image_tag project.avatar_url, options
- else # generated icon
- project_identicon(project, options)
- end
- end
-
- def project_identicon(project, options = {})
- allowed_colors = {
- red: 'FFEBEE',
- purple: 'F3E5F5',
- indigo: 'E8EAF6',
- blue: 'E3F2FD',
- teal: 'E0F2F1',
- orange: 'FBE9E7',
- gray: 'EEEEEE'
- }
-
- options[:class] ||= ''
- options[:class] << ' identicon'
- bg_key = project.id % 7
- style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
-
- content_tag(:div, class: options[:class], style: style) do
- project.name[0, 1].upcase
- end
- end
-
- # Takes both user and email and returns the avatar_icon by
- # user (preferred) or email.
- def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
- if user
- avatar_icon_for_user(user, size, scale, only_path: only_path)
- elsif email
- avatar_icon_for_email(email, size, scale, only_path: only_path)
- else
- default_avatar
- end
- end
-
- def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
- user = User.find_by_any_email(email.try(:downcase))
- if user
- avatar_icon_for_user(user, size, scale, only_path: only_path)
- else
- gravatar_icon(email, size, scale)
- end
- end
-
- def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
- if user
- user.avatar_url(size: size, only_path: only_path) || default_avatar
- else
- gravatar_icon(nil, size, scale)
- end
- end
-
- def gravatar_icon(user_email = '', size = nil, scale = 2)
- GravatarService.new.execute(user_email, size, scale) ||
- default_avatar
- end
-
- def default_avatar
- asset_path('no_avatar.png')
- end
-
def last_commit(project)
if project.repo_exists?
time_ago_with_tooltip(project.repository.commit.committed_date)
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index c109954f3a3..d2daee22aba 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -1,6 +1,6 @@
module AuthHelper
PROVIDERS_WITH_ICONS = %w(twitter github gitlab bitbucket google_oauth2 facebook azure_oauth2 authentiq).freeze
- FORM_BASED_PROVIDERS = [/\Aldap/, 'crowd'].freeze
+ LDAP_PROVIDER = /\Aldap/
def ldap_enabled?
Gitlab::Auth::LDAP::Config.enabled?
@@ -23,7 +23,7 @@ module AuthHelper
end
def form_based_provider?(name)
- FORM_BASED_PROVIDERS.any? { |pattern| pattern === name.to_s }
+ [LDAP_PROVIDER, 'crowd'].any? { |pattern| pattern === name.to_s }
end
def form_based_providers
@@ -38,6 +38,10 @@ module AuthHelper
auth_providers.reject { |provider| form_based_provider?(provider) }
end
+ def providers_for_base_controller
+ auth_providers.reject { |provider| LDAP_PROVIDER === provider }
+ end
+
def enabled_button_based_providers
disabled_providers = Gitlab::CurrentSettings.disabled_oauth_sign_in_sources || []
diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 8d96569bbb0..43d92bde064 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -1,4 +1,78 @@
module AvatarsHelper
+ def project_icon(project_id, options = {})
+ project =
+ if project_id.respond_to?(:avatar_url)
+ project_id
+ else
+ Project.find_by_full_path(project_id)
+ end
+
+ if project.avatar_url
+ image_tag project.avatar_url, options
+ else # generated icon
+ project_identicon(project, options)
+ end
+ end
+
+ def project_identicon(project, options = {})
+ allowed_colors = {
+ red: 'FFEBEE',
+ purple: 'F3E5F5',
+ indigo: 'E8EAF6',
+ blue: 'E3F2FD',
+ teal: 'E0F2F1',
+ orange: 'FBE9E7',
+ gray: 'EEEEEE'
+ }
+
+ options[:class] ||= ''
+ options[:class] << ' identicon'
+ bg_key = project.id % 7
+ style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555"
+
+ content_tag(:div, class: options[:class], style: style) do
+ project.name[0, 1].upcase
+ end
+ end
+
+ # Takes both user and email and returns the avatar_icon by
+ # user (preferred) or email.
+ def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true)
+ if user
+ avatar_icon_for_user(user, size, scale, only_path: only_path)
+ elsif email
+ avatar_icon_for_email(email, size, scale, only_path: only_path)
+ else
+ default_avatar
+ end
+ end
+
+ def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true)
+ user = User.find_by_any_email(email.try(:downcase))
+ if user
+ avatar_icon_for_user(user, size, scale, only_path: only_path)
+ else
+ gravatar_icon(email, size, scale)
+ end
+ end
+
+ def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
+ if user
+ user.avatar_url(size: size, only_path: only_path) || default_avatar
+ else
+ gravatar_icon(nil, size, scale)
+ end
+ end
+
+ def gravatar_icon(user_email = '', size = nil, scale = 2)
+ GravatarService.new.execute(user_email, size, scale) ||
+ default_avatar
+ end
+
+ def default_avatar
+ ActionController::Base.helpers.image_path('no_avatar.png')
+ end
+
def author_avatar(commit_or_event, options = {})
user_avatar(options.merge({
user: commit_or_event.author,
diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb
index 00fe67d6ffb..5b4a141dbcf 100644
--- a/app/helpers/system_note_helper.rb
+++ b/app/helpers/system_note_helper.rb
@@ -1,14 +1,14 @@
module SystemNoteHelper
ICON_NAMES_BY_ACTION = {
'commit' => 'commit',
- 'description' => 'pencil',
+ 'description' => 'pencil-square',
'merge' => 'git-merge',
'merged' => 'git-merge',
'opened' => 'issue-open',
'closed' => 'issue-close',
'time_tracking' => 'timer',
'assignee' => 'user',
- 'title' => 'pencil',
+ 'title' => 'pencil-square',
'task' => 'task-done',
'label' => 'label',
'cross_reference' => 'comment-dots',
@@ -18,7 +18,7 @@ module SystemNoteHelper
'milestone' => 'clock',
'discussion' => 'comment',
'moved' => 'arrow-right',
- 'outdated' => 'pencil',
+ 'outdated' => 'pencil-square',
'duplicate' => 'issue-duplicate',
'locked' => 'lock',
'unlocked' => 'lock-open'
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index e4212775956..3646e08a15f 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -16,6 +16,7 @@ class Notify < BaseMailer
helper BlobHelper
helper EmailsHelper
helper MembersHelper
+ helper AvatarsHelper
helper GitlabRoutingHelper
def test_email(recipient_email, subject, body)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 9750e9298ec..32f3d90595a 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -105,6 +105,10 @@ class Commit
end
end
end
+
+ def parent_class
+ ::Project
+ end
end
attr_accessor :raw
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 15122cbc693..616a626419b 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -54,7 +54,20 @@ class DiffNote < Note
end
def diff_file
- @diff_file ||= self.original_position.diff_file(self.project.repository)
+ @diff_file ||=
+ begin
+ if created_at_diff?(noteable.diff_refs)
+ # We're able to use the already persisted diffs (Postgres) if we're
+ # presenting a "current version" of the MR discussion diff.
+ # So no need to make an extra Gitaly diff request for it.
+ # As an extra benefit, the returned `diff_file` already
+ # has `highlighted_diff_lines` data set from Redis on
+ # `Diff::FileCollection::MergeRequestDiff`.
+ noteable.diffs(paths: original_position.paths, expanded: true).diff_files.first
+ else
+ original_position.diff_file(self.project.repository)
+ end
+ end
end
def diff_line
diff --git a/app/services/notes/resolve_service.rb b/app/services/notes/resolve_service.rb
new file mode 100644
index 00000000000..0db8ee809a9
--- /dev/null
+++ b/app/services/notes/resolve_service.rb
@@ -0,0 +1,9 @@
+module Notes
+ class ResolveService < ::BaseService
+ def execute(note)
+ note.resolve!(current_user)
+
+ ::MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable)
+ end
+ end
+end
diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb
index 9a88459b511..ba7be4b3f89 100644
--- a/app/services/repository_archive_clean_up_service.rb
+++ b/app/services/repository_archive_clean_up_service.rb
@@ -20,11 +20,12 @@ class RepositoryArchiveCleanUpService
private
def clean_up_old_archives
- run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete))
+ run(%W(find #{path} -mindepth 1 -maxdepth 3 -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -mmin +#{mmin} -delete))
end
def clean_up_empty_directories
- run(%W(find #{path} -not -path #{path} -type d -empty -name \*.git -maxdepth 1 -delete))
+ run(%W(find #{path} -mindepth 2 -maxdepth 2 -type d -empty -delete))
+ run(%W(find #{path} -mindepth 1 -maxdepth 1 -type d -empty -delete))
end
def run(cmd)
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 54767bdf757..85cde721607 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -8,18 +8,17 @@
%li{ class: "branch-item js-branch-#{branch.name}" }
.branch-info
.branch-title
- = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name' do
- = sprite_icon('fork', size: 12)
+ = sprite_icon('fork', size: 12)
+ = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name prepend-left-8' do
= branch.name
- &nbsp;
- if branch.name == @repository.root_ref
- %span.badge.badge-primary default
+ %span.badge.badge-primary.prepend-left-5 default
- elsif merged
- %span.badge.badge-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } }
+ %span.badge.badge-info.has-tooltip.prepend-left-5{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } }
= s_('Branches|merged')
- if protected_branch?(@project, branch)
- %span.badge.badge-success
+ %span.badge.badge-success.prepend-left-5
= s_('Branches|protected')
.block-truncated
diff --git a/changelogs/unreleased/dm-commit-trailer-without-gravatar.yml b/changelogs/unreleased/dm-commit-trailer-without-gravatar.yml
new file mode 100644
index 00000000000..9f057c67122
--- /dev/null
+++ b/changelogs/unreleased/dm-commit-trailer-without-gravatar.yml
@@ -0,0 +1,5 @@
+---
+title: Fix commit trailer rendering when Gravatar is disabled
+merge_request:
+author:
+type: fixed
diff --git a/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml b/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml
new file mode 100644
index 00000000000..6e2273ed9af
--- /dev/null
+++ b/changelogs/unreleased/feature-show-only-groups-user-is-member-of-in-dashboard.yml
@@ -0,0 +1,5 @@
+---
+title: For group dashboard, we no longer show groups which the visitor is not a member of (this applies to admins and auditors)
+merge_request: 17884
+author: Roger Rüttimann
+type: changed
diff --git a/changelogs/unreleased/fix-inconsistent-protected-branch-pill-baseline.yml b/changelogs/unreleased/fix-inconsistent-protected-branch-pill-baseline.yml
new file mode 100644
index 00000000000..fb6dffaf226
--- /dev/null
+++ b/changelogs/unreleased/fix-inconsistent-protected-branch-pill-baseline.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed inconsistent protected branch pill baseline
+merge_request:
+author:
+type: fixed
diff --git a/changelogs/unreleased/helm-add-alpine-mirrors.yml b/changelogs/unreleased/helm-add-alpine-mirrors.yml
new file mode 100644
index 00000000000..656c4f911d0
--- /dev/null
+++ b/changelogs/unreleased/helm-add-alpine-mirrors.yml
@@ -0,0 +1,5 @@
+---
+title: Increase cluster applications installer availability using alpine linux mirrors
+merge_request:
+author:
+type: performance
diff --git a/changelogs/unreleased/jprovazn-commit-notes-api.yml b/changelogs/unreleased/jprovazn-commit-notes-api.yml
new file mode 100644
index 00000000000..4665d800ccf
--- /dev/null
+++ b/changelogs/unreleased/jprovazn-commit-notes-api.yml
@@ -0,0 +1,5 @@
+---
+title: Add discussion API for merge requests and commits
+merge_request:
+author:
+type: added
diff --git a/changelogs/unreleased/osw-use-cached-highlighted-content-for-discussions.yml b/changelogs/unreleased/osw-use-cached-highlighted-content-for-discussions.yml
new file mode 100644
index 00000000000..03a11a3038a
--- /dev/null
+++ b/changelogs/unreleased/osw-use-cached-highlighted-content-for-discussions.yml
@@ -0,0 +1,5 @@
+---
+title: Use persisted diff data instead fetching Git on discussions
+merge_request:
+author:
+type: performance
diff --git a/changelogs/unreleased/revert-discussion-counter-height.yml b/changelogs/unreleased/revert-discussion-counter-height.yml
new file mode 100644
index 00000000000..331ff997009
--- /dev/null
+++ b/changelogs/unreleased/revert-discussion-counter-height.yml
@@ -0,0 +1,5 @@
+---
+title: Revert discussion counter height
+merge_request: 18656
+author: George Tsiolis
+type: changed
diff --git a/changelogs/unreleased/security-45689-fix-archive-cache-bug.yml b/changelogs/unreleased/security-45689-fix-archive-cache-bug.yml
new file mode 100644
index 00000000000..0103a7fc430
--- /dev/null
+++ b/changelogs/unreleased/security-45689-fix-archive-cache-bug.yml
@@ -0,0 +1,5 @@
+---
+title: Serve archive requests with the correct file in all cases
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security_issue_42029.yml b/changelogs/unreleased/security_issue_42029.yml
new file mode 100644
index 00000000000..0772e33f930
--- /dev/null
+++ b/changelogs/unreleased/security_issue_42029.yml
@@ -0,0 +1,5 @@
+---
+title: Sanitizes user name to avoid XSS attacks
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/update-timeline-icon-for-description-edit.yml b/changelogs/unreleased/update-timeline-icon-for-description-edit.yml
new file mode 100644
index 00000000000..560db00e503
--- /dev/null
+++ b/changelogs/unreleased/update-timeline-icon-for-description-edit.yml
@@ -0,0 +1,5 @@
+---
+title: Update timeline icon for description edit
+merge_request: 18633
+author: George Tsiolis
+type: changed
diff --git a/doc/api/discussions.md b/doc/api/discussions.md
index c341b7f2009..65e2f9d6cd9 100644
--- a/doc/api/discussions.md
+++ b/doc/api/discussions.md
@@ -1,6 +1,6 @@
# Discussions API
-Discussions are set of related notes on snippets or issues.
+Discussions are set of related notes on snippets, issues, merge requests or commits.
## Issues
@@ -61,7 +61,8 @@ GET /projects/:id/issues/:issue_iid/discussions
"system": false,
"noteable_id": 3,
"noteable_type": "Issue",
- "noteable_iid": null
+ "noteable_iid": null,
+ "resolvable": false
}
]
},
@@ -87,7 +88,8 @@ GET /projects/:id/issues/:issue_iid/discussions
"system": false,
"noteable_id": 3,
"noteable_type": "Issue",
- "noteable_iid": null
+ "noteable_iid": null,
+ "resolvable": false
}
]
}
@@ -265,7 +267,8 @@ GET /projects/:id/snippets/:snippet_id/discussions
"system": false,
"noteable_id": 3,
"noteable_type": "Snippet",
- "noteable_id": null
+ "noteable_id": null,
+ "resolvable": false
}
]
},
@@ -291,7 +294,8 @@ GET /projects/:id/snippets/:snippet_id/discussions
"system": false,
"noteable_id": 3,
"noteable_type": "Snippet",
- "noteable_id": null
+ "noteable_id": null,
+ "resolvable": false
}
]
}
@@ -409,3 +413,574 @@ Parameters:
```bash
curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/636
```
+
+## Merge requests
+
+### List project merge request discussions
+
+Gets a list of all discussions for a single merge request.
+
+```
+GET /projects/:id/merge_requests/:merge_request_iid/discussions
+```
+
+| Attribute | Type | Required | Description |
+| ------------------- | ---------------- | ---------- | ------------ |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+
+```json
+[
+ {
+ "id": "6a9c1750b37d513a43987b574953fceb50b03ce7",
+ "individual_note": false,
+ "notes": [
+ {
+ "id": 1126,
+ "type": "DiscussionNote",
+ "body": "discussion text",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-03T21:54:39.668Z",
+ "updated_at": "2018-03-03T21:54:39.668Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Merge request",
+ "noteable_iid": null,
+ "resolved": false,
+ "resolvable": true,
+ "resolved_by": null
+ },
+ {
+ "id": 1129,
+ "type": "DiscussionNote",
+ "body": "reply to the discussion",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T13:38:02.127Z",
+ "updated_at": "2018-03-04T13:38:02.127Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Merge request",
+ "noteable_iid": null,
+ "resolved": false,
+ "resolvable": true,
+ "resolved_by": null
+ }
+ ]
+ },
+ {
+ "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6",
+ "individual_note": true,
+ "notes": [
+ {
+ "id": 1128,
+ "type": null,
+ "body": "a single comment",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T09:17:22.520Z",
+ "updated_at": "2018-03-04T09:17:22.520Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Merge request",
+ "noteable_iid": null,
+ "resolved": false,
+ "resolvable": true,
+ "resolved_by": null
+ }
+ ]
+ }
+]
+```
+
+Diff comments contain also position:
+
+```json
+[
+ {
+ "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6",
+ "individual_note": false,
+ "notes": [
+ {
+ "id": 1128,
+ "type": DiffNote,
+ "body": "diff comment",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T09:17:22.520Z",
+ "updated_at": "2018-03-04T09:17:22.520Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Merge request",
+ "noteable_iid": null,
+ "position": {
+ "base_sha": "b5d6e7b1613fca24d250fa8e5bc7bcc3dd6002ef",
+ "start_sha": "7c9c2ead8a320fb7ba0b4e234bd9529a2614e306",
+ "head_sha": "4803c71e6b1833ca72b8b26ef2ecd5adc8a38031",
+ "old_path": "package.json",
+ "new_path": "package.json",
+ "position_type": "text",
+ "old_line": 27,
+ "new_line": 27
+ },
+ "resolved": false,
+ "resolvable": true,
+ "resolved_by": null
+ }
+ ]
+ }
+]
+```
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions
+```
+
+### Get single merge request discussion
+
+Returns a single discussion for a specific project merge request
+
+```
+GET /projects/:id/merge_requests/:merge_request_iid/discussions/:discussion_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `discussion_id` | integer | yes | The ID of a discussion |
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7
+```
+
+### Create new merge request discussion
+
+Creates a new discussion to a single project merge request. This is similar to creating
+a note but but another comments (replies) can be added to it later.
+
+```
+POST /projects/:id/merge_requests/:merge_request_iid/discussions
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `body` | string | yes | The content of a discussion |
+| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `position` | hash | no | Position when creating a diff note |
+| `position[base_sha]` | string | yes | Base commit SHA in the source branch |
+| `position[start_sha]` | string | yes | SHA referencing commit in target branch |
+| `position[head_sha]` | string | yes | SHA referencing HEAD of this merge request |
+| `position[position_type]` | string | yes | Type of the position reference', allowed values: 'text' or 'image' |
+| `position[new_path]` | string | no | File path after change |
+| `position[new_line]` | integer | no | Line number after change (for 'text' diff notes) |
+| `position[old_path]` | string | no | File path before change |
+| `position[old_line]` | integer | no | Line number before change (for 'text' diff notes) |
+| `position[width]` | integer | no | Width of the image (for 'image' diff notes) |
+| `position[height]` | integer | no | Height of the image (for 'image' diff notes) |
+| `position[x]` | integer | no | X coordinate (for 'image' diff notes) |
+| `position[y]` | integer | no | Y coordinate (for 'image' diff notes) |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions?body=comment
+```
+
+### Resolve a merge request discussion
+
+Resolve/unresolve whole discussion of a merge request.
+
+```
+PUT /projects/:id/merge_requests/:merge_request_iid/discussions/:discussion_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `resolved` | boolean | yes | Resolve/unresolve the discussion |
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7?resolved=true
+```
+
+
+### Add note to existing merge request discussion
+
+Adds a new note to the discussion.
+
+```
+POST /projects/:id/merge_requests/:merge_request_iid/discussions/:discussion_id/notes
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+| `body` | string | yes | The content of a discussion |
+| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
+```
+
+### Modify an existing merge request discussion note
+
+Modify or resolve an existing discussion note of a merge request.
+
+```
+PUT /projects/:id/merge_requests/:merge_request_iid/discussions/:discussion_id/notes/:note_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+| `body` | string | no | The content of a discussion (exactly one of `body` or `resolved` must be set |
+| `resolved` | boolean | no | Resolve/unresolve the note (exactly one of `body` or `resolved` must be set |
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment
+```
+
+Resolving a note:
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?resolved=true
+```
+
+### Delete a merge request discussion note
+
+Deletes an existing discussion note of a merge request.
+
+```
+DELETE /projects/:id/merge_requests/:merge_request_iid/discussions/:discussion_id/notes/:note_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+
+```bash
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/636
+```
+
+## Commits
+
+### List project commit discussions
+
+Gets a list of all discussions for a single commit.
+
+```
+GET /projects/:id/commits/:commit_id/discussions
+```
+
+| Attribute | Type | Required | Description |
+| ------------------- | ---------------- | ---------- | ------------ |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+
+```json
+[
+ {
+ "id": "6a9c1750b37d513a43987b574953fceb50b03ce7",
+ "individual_note": false,
+ "notes": [
+ {
+ "id": 1126,
+ "type": "DiscussionNote",
+ "body": "discussion text",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-03T21:54:39.668Z",
+ "updated_at": "2018-03-03T21:54:39.668Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Commit",
+ "noteable_iid": null,
+ "resolvable": false
+ },
+ {
+ "id": 1129,
+ "type": "DiscussionNote",
+ "body": "reply to the discussion",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T13:38:02.127Z",
+ "updated_at": "2018-03-04T13:38:02.127Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Commit",
+ "noteable_iid": null,
+ "resolvable": false
+ }
+ ]
+ },
+ {
+ "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6",
+ "individual_note": true,
+ "notes": [
+ {
+ "id": 1128,
+ "type": null,
+ "body": "a single comment",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T09:17:22.520Z",
+ "updated_at": "2018-03-04T09:17:22.520Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Commit",
+ "noteable_iid": null,
+ "resolvable": false
+ }
+ ]
+ }
+]
+```
+
+Diff comments contain also position:
+
+```json
+[
+ {
+ "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6",
+ "individual_note": false,
+ "notes": [
+ {
+ "id": 1128,
+ "type": DiffNote,
+ "body": "diff comment",
+ "attachment": null,
+ "author": {
+ "id": 1,
+ "name": "root",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon",
+ "web_url": "http://localhost:3000/root"
+ },
+ "created_at": "2018-03-04T09:17:22.520Z",
+ "updated_at": "2018-03-04T09:17:22.520Z",
+ "system": false,
+ "noteable_id": 3,
+ "noteable_type": "Commit",
+ "noteable_iid": null,
+ "position": {
+ "base_sha": "b5d6e7b1613fca24d250fa8e5bc7bcc3dd6002ef",
+ "start_sha": "7c9c2ead8a320fb7ba0b4e234bd9529a2614e306",
+ "head_sha": "4803c71e6b1833ca72b8b26ef2ecd5adc8a38031",
+ "old_path": "package.json",
+ "new_path": "package.json",
+ "position_type": "text",
+ "old_line": 27,
+ "new_line": 27
+ },
+ "resolvable": false
+ }
+ ]
+ }
+]
+```
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions
+```
+
+### Get single commit discussion
+
+Returns a single discussion for a specific project commit
+
+```
+GET /projects/:id/commits/:commit_id/discussions/:discussion_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+| `discussion_id` | integer | yes | The ID of a discussion |
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7
+```
+
+### Create new commit discussion
+
+Creates a new discussion to a single project commit. This is similar to creating
+a note but but another comments (replies) can be added to it later.
+
+```
+POST /projects/:id/commits/:commit_id/discussions
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+| `body` | string | yes | The content of a discussion |
+| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `position` | hash | no | Position when creating a diff note |
+| `position[base_sha]` | string | yes | Base commit SHA in the source branch |
+| `position[start_sha]` | string | yes | SHA referencing commit in target branch |
+| `position[head_sha]` | string | yes | SHA referencing HEAD of this commit |
+| `position[position_type]` | string | yes | Type of the position reference', allowed values: 'text' or 'image' |
+| `position[new_path]` | string | no | File path after change |
+| `position[new_line]` | integer | no | Line number after change |
+| `position[old_path]` | string | no | File path before change |
+| `position[old_line]` | integer | no | Line number before change |
+| `position[width]` | integer | no | Width of the image (for 'image' diff notes) |
+| `position[height]` | integer | no | Height of the image (for 'image' diff notes) |
+| `position[x]` | integer | no | X coordinate (for 'image' diff notes) |
+| `position[y]` | integer | no | Y coordinate (for 'image' diff notes) |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions?body=comment
+```
+
+### Add note to existing commit discussion
+
+Adds a new note to the discussion.
+
+```
+POST /projects/:id/commits/:commit_id/discussions/:discussion_id/notes
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+| `body` | string | yes | The content of a discussion |
+| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+
+```bash
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
+```
+
+### Modify an existing commit discussion note
+
+Modify or resolve an existing discussion note of a commit.
+
+```
+PUT /projects/:id/commits/:commit_id/discussions/:discussion_id/notes/:note_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+| `body` | string | no | The content of a note |
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment
+```
+
+Resolving a note:
+
+```bash
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?resolved=true
+```
+
+### Delete a commit discussion note
+
+Deletes an existing discussion note of a commit.
+
+```
+DELETE /projects/:id/commits/:commit_id/discussions/:discussion_id/notes/:note_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `commit_id` | integer | yes | The ID of a commit |
+| `discussion_id` | integer | yes | The ID of a discussion |
+| `note_id` | integer | yes | The ID of a discussion note |
+
+```bash
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/636
+```
diff --git a/doc/api/groups.md b/doc/api/groups.md
index 1aed8aac64e..923fd662a5b 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -10,7 +10,7 @@ Parameters:
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `skip_groups` | array of integers | no | Skip the group IDs passed |
-| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) |
+| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) |
| `search` | string | no | Return the list of authorized groups matching the search criteria |
| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
| `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
@@ -94,7 +94,7 @@ Parameters:
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) of the parent group |
| `skip_groups` | array of integers | no | Skip the group IDs passed |
-| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users) |
+| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) |
| `search` | string | no | Return the list of authorized groups matching the search criteria |
| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
| `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
diff --git a/doc/api/notes.md b/doc/api/notes.md
index aa38d22845c..d29c5b94915 100644
--- a/doc/api/notes.md
+++ b/doc/api/notes.md
@@ -39,7 +39,8 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at
"system": true,
"noteable_id": 377,
"noteable_type": "Issue",
- "noteable_iid": 377
+ "noteable_iid": 377,
+ "resolvable": false
},
{
"id": 305,
@@ -58,7 +59,8 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at
"system": true,
"noteable_id": 121,
"noteable_type": "Issue",
- "noteable_iid": 121
+ "noteable_iid": 121,
+ "resolvable": false
}
]
```
@@ -314,7 +316,8 @@ Parameters:
"system": false,
"noteable_id": 2,
"noteable_type": "MergeRequest",
- "noteable_iid": 2
+ "noteable_iid": 2,
+ "resolvable": false
}
```
diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb
index 7975f35ab1e..13c34e3473a 100644
--- a/lib/api/discussions.rb
+++ b/lib/api/discussions.rb
@@ -5,11 +5,12 @@ module API
before { authenticate! }
- NOTEABLE_TYPES = [Issue, Snippet].freeze
+ NOTEABLE_TYPES = [Issue, Snippet, MergeRequest, Commit].freeze
NOTEABLE_TYPES.each do |noteable_type|
parent_type = noteable_type.parent_class.to_s.underscore
noteables_str = noteable_type.to_s.underscore.pluralize
+ noteables_path = noteable_type == Commit ? "repository/#{noteables_str}" : noteables_str
params do
requires :id, type: String, desc: "The ID of a #{parent_type}"
@@ -19,14 +20,12 @@ module API
success Entities::Discussion
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
use :pagination
end
- get ":id/#{noteables_str}/:noteable_id/discussions" do
+ get ":id/#{noteables_path}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
- break not_found!("Discussions") unless can?(current_user, noteable_read_ability_name(noteable), noteable)
-
notes = noteable.notes
.inc_relations_for_view
.includes(:noteable)
@@ -43,13 +42,13 @@ module API
end
params do
requires :discussion_id, type: String, desc: 'The ID of a discussion'
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
end
- get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id" do
+ get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
- if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
+ if notes.empty?
break not_found!("Discussion")
end
@@ -62,19 +61,36 @@ module API
success Entities::Discussion
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
requires :body, type: String, desc: 'The content of a note'
optional :created_at, type: String, desc: 'The creation date of the note'
+ optional :position, type: Hash do
+ requires :base_sha, type: String, desc: 'Base commit SHA in the source branch'
+ requires :start_sha, type: String, desc: 'SHA referencing commit in target branch'
+ requires :head_sha, type: String, desc: 'SHA referencing HEAD of this merge request'
+ requires :position_type, type: String, desc: 'Type of the position reference', values: %w(text image)
+ optional :new_path, type: String, desc: 'File path after change'
+ optional :new_line, type: Integer, desc: 'Line number after change'
+ optional :old_path, type: String, desc: 'File path before change'
+ optional :old_line, type: Integer, desc: 'Line number before change'
+ optional :width, type: Integer, desc: 'Width of the image'
+ optional :height, type: Integer, desc: 'Height of the image'
+ optional :x, type: Integer, desc: 'X coordinate in the image'
+ optional :y, type: Integer, desc: 'Y coordinate in the image'
+ end
end
- post ":id/#{noteables_str}/:noteable_id/discussions" do
+ post ":id/#{noteables_path}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+ type = params[:position] ? 'DiffNote' : 'DiscussionNote'
+ id_key = noteable.is_a?(Commit) ? :commit_id : :noteable_id
opts = {
note: params[:body],
created_at: params[:created_at],
- type: 'DiscussionNote',
+ type: type,
noteable_type: noteables_str.classify,
- noteable_id: noteable.id
+ position: params[:position],
+ id_key => noteable.id
}
note = create_note(noteable, opts)
@@ -91,13 +107,13 @@ module API
end
params do
requires :discussion_id, type: String, desc: 'The ID of a discussion'
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
end
- get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do
+ get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
- if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
+ if notes.empty?
break not_found!("Notes")
end
@@ -108,12 +124,12 @@ module API
success Entities::Note
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :body, type: String, desc: 'The content of a note'
optional :created_at, type: String, desc: 'The creation date of the note'
end
- post ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do
+ post ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
@@ -139,11 +155,11 @@ module API
success Entities::Note
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
end
- get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+ get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
get_note(noteable, params[:note_id])
@@ -153,30 +169,52 @@ module API
success Entities::Note
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
- requires :body, type: String, desc: 'The content of a note'
+ optional :body, type: String, desc: 'The content of a note'
+ optional :resolved, type: Boolean, desc: 'Mark note resolved/unresolved'
+ exactly_one_of :body, :resolved
end
- put ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+ put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
- update_note(noteable, params[:note_id])
+ if params[:resolved].nil?
+ update_note(noteable, params[:note_id])
+ else
+ resolve_note(noteable, params[:note_id], params[:resolved])
+ end
end
desc "Delete a comment in a #{noteable_type.to_s.downcase} discussion" do
success Entities::Note
end
params do
- requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
end
- delete ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+ delete ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
delete_note(noteable, params[:note_id])
end
+
+ if Noteable::RESOLVABLE_TYPES.include?(noteable_type.to_s)
+ desc "Resolve/unresolve an existing #{noteable_type.to_s.downcase} discussion" do
+ success Entities::Discussion
+ end
+ params do
+ requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
+ requires :discussion_id, type: String, desc: 'The ID of a discussion'
+ requires :resolved, type: Boolean, desc: 'Mark discussion resolved/unresolved'
+ end
+ put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
+ noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
+
+ resolve_discussion(noteable, params[:discussion_id], params[:resolved])
+ end
+ end
end
end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 8aad320e376..75d56b82424 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -286,6 +286,10 @@ module API
end
end
+ class DiffRefs < Grape::Entity
+ expose :base_sha, :head_sha, :start_sha
+ end
+
class Commit < Grape::Entity
expose :id, :short_id, :title, :created_at
expose :parent_ids
@@ -601,6 +605,8 @@ module API
merge_request.metrics&.pipeline
end
+ expose :diff_refs, using: Entities::DiffRefs
+
def build_available?(options)
options[:project]&.feature_available?(:builds, options[:current_user])
end
@@ -642,6 +648,11 @@ module API
expose :id, :key, :created_at
end
+ class DiffPosition < Grape::Entity
+ expose :base_sha, :start_sha, :head_sha, :old_path, :new_path,
+ :position_type
+ end
+
class Note < Grape::Entity
# Only Issue and MergeRequest have iid
NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze
@@ -655,6 +666,14 @@ module API
expose :system?, as: :system
expose :noteable_id, :noteable_type
+ expose :position, if: ->(note, options) { note.diff_note? } do |note|
+ note.position.to_h
+ end
+
+ expose :resolvable?, as: :resolvable
+ expose :resolved?, as: :resolved, if: ->(note, options) { note.resolvable? }
+ expose :resolved_by, using: Entities::UserBasic, if: ->(note, options) { note.resolvable? }
+
# Avoid N+1 queries as much as possible
expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) }
end
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 4a4df1b8b9e..92e3d5cc10a 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -37,13 +37,11 @@ module API
use :pagination
end
- def find_groups(params)
- find_params = {
- all_available: params[:all_available],
- custom_attributes: params[:custom_attributes],
- owned: params[:owned]
- }
- find_params[:parent] = find_group!(params[:id]) if params[:id]
+ def find_groups(params, parent_id = nil)
+ find_params = params.slice(:all_available, :custom_attributes, :owned)
+ find_params[:parent] = find_group!(parent_id) if parent_id
+ find_params[:all_available] =
+ find_params.fetch(:all_available, current_user&.full_private_access?)
groups = GroupsFinder.new(current_user, find_params).execute
groups = groups.search(params[:search]) if params[:search].present?
@@ -85,7 +83,7 @@ module API
use :with_custom_attributes
end
get do
- groups = find_groups(params)
+ groups = find_groups(declared_params(include_missing: false), params[:id])
present_groups params, groups
end
@@ -213,7 +211,7 @@ module API
use :with_custom_attributes
end
get ":id/subgroups" do
- groups = find_groups(params)
+ groups = find_groups(declared_params(include_missing: false), params[:id])
present_groups params, groups
end
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index b8657cd7ee4..2ed331d4fd2 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -171,6 +171,10 @@ module API
MergeRequestsFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
end
+ def find_project_commit(id)
+ user_project.commit_by(oid: id)
+ end
+
def find_project_snippet(id)
finder_params = { project: user_project }
SnippetsFinder.new(current_user, finder_params).find(id)
diff --git a/lib/api/helpers/custom_attributes.rb b/lib/api/helpers/custom_attributes.rb
index 70e4eda95f8..10d652e33f5 100644
--- a/lib/api/helpers/custom_attributes.rb
+++ b/lib/api/helpers/custom_attributes.rb
@@ -7,6 +7,9 @@ module API
helpers do
params :with_custom_attributes do
optional :with_custom_attributes, type: Boolean, default: false, desc: 'Include custom attributes in the response'
+
+ optional :custom_attributes, type: Hash,
+ desc: 'Filter with custom attributes'
end
def with_custom_attributes(collection_or_resource, options = {})
diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb
index b74b8149834..b4bfb677d72 100644
--- a/lib/api/helpers/notes_helpers.rb
+++ b/lib/api/helpers/notes_helpers.rb
@@ -21,6 +21,23 @@ module API
end
end
+ def resolve_note(noteable, note_id, resolved)
+ note = noteable.notes.find(note_id)
+
+ authorize! :resolve_note, note
+
+ bad_request!("Note is not resolvable") unless note.resolvable?
+
+ if resolved
+ parent = noteable_parent(noteable)
+ ::Notes::ResolveService.new(parent, current_user).execute(note)
+ else
+ note.unresolve!
+ end
+
+ present note, with: Entities::Note
+ end
+
def delete_note(noteable, note_id)
note = noteable.notes.find(note_id)
@@ -35,7 +52,7 @@ module API
def get_note(noteable, note_id)
note = noteable.notes.with_metadata.find(params[:note_id])
- can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
+ can_read_note = !note.cross_reference_not_visible_for?(current_user)
if can_read_note
present note, with: Entities::Note
@@ -49,7 +66,20 @@ module API
end
def find_noteable(parent, noteables_str, noteable_id)
- public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
+ noteable = public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
+
+ readable =
+ if noteable.is_a?(Commit)
+ # for commits there is not :read_commit policy, check if user
+ # has :read_note permission on the commit's project
+ can?(current_user, :read_note, user_project)
+ else
+ can?(current_user, noteable_read_ability_name(noteable), noteable)
+ end
+
+ return not_found!(noteables_str) unless readable
+
+ noteable
end
def noteable_parent(noteable)
@@ -57,11 +87,8 @@ module API
end
def create_note(noteable, opts)
- noteables_str = noteable.model_name.to_s.underscore.pluralize
-
- return not_found!(noteables_str) unless can?(current_user, noteable_read_ability_name(noteable), noteable)
-
- authorize! :create_note, noteable
+ policy_object = noteable.is_a?(Commit) ? user_project : noteable
+ authorize!(:create_note, policy_object)
parent = noteable_parent(noteable)
@@ -73,6 +100,21 @@ module API
project = parent if parent.is_a?(Project)
::Notes::CreateService.new(project, current_user, opts).execute
end
+
+ def resolve_discussion(noteable, discussion_id, resolved)
+ discussion = noteable.find_discussion(discussion_id)
+
+ forbidden! unless discussion.can_resolve?(current_user)
+
+ if resolved
+ parent = noteable_parent(noteable)
+ ::Discussions::ResolveService.new(parent, current_user, merge_request: noteable).execute(discussion)
+ else
+ discussion.unresolve!
+ end
+
+ present discussion, with: Entities::Discussion
+ end
end
end
end
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 69f1df6b341..39923e6d5b5 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -31,23 +31,19 @@ module API
get ":id/#{noteables_str}/:noteable_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
- if can?(current_user, noteable_read_ability_name(noteable), noteable)
- # We exclude notes that are cross-references and that cannot be viewed
- # by the current user. By doing this exclusion at this level and not
- # at the DB query level (which we cannot in that case), the current
- # page can have less elements than :per_page even if
- # there's more than one page.
- raw_notes = noteable.notes.with_metadata.reorder(params[:order_by] => params[:sort])
- notes =
- # paginate() only works with a relation. This could lead to a
- # mismatch between the pagination headers info and the actual notes
- # array returned, but this is really a edge-case.
- paginate(raw_notes)
- .reject { |n| n.cross_reference_not_visible_for?(current_user) }
- present notes, with: Entities::Note
- else
- not_found!("Notes")
- end
+ # We exclude notes that are cross-references and that cannot be viewed
+ # by the current user. By doing this exclusion at this level and not
+ # at the DB query level (which we cannot in that case), the current
+ # page can have less elements than :per_page even if
+ # there's more than one page.
+ raw_notes = noteable.notes.with_metadata.reorder(params[:order_by] => params[:sort])
+ notes =
+ # paginate() only works with a relation. This could lead to a
+ # mismatch between the pagination headers info and the actual notes
+ # array returned, but this is really a edge-case.
+ paginate(raw_notes)
+ .reject { |n| n.cross_reference_not_visible_for?(current_user) }
+ present notes, with: Entities::Note
end
desc "Get a single #{noteable_type.to_s.downcase} note" do
diff --git a/lib/banzai/filter/commit_trailers_filter.rb b/lib/banzai/filter/commit_trailers_filter.rb
index ef16df1f3ae..7b55e8b36f6 100644
--- a/lib/banzai/filter/commit_trailers_filter.rb
+++ b/lib/banzai/filter/commit_trailers_filter.rb
@@ -13,7 +13,6 @@ module Banzai
# * https://git.wiki.kernel.org/index.php/CommitMessageConventions
class CommitTrailersFilter < HTML::Pipeline::Filter
include ActionView::Helpers::TagHelper
- include ApplicationHelper
include AvatarsHelper
TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze
diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb
index a6007ebf531..c79d8d3cb21 100644
--- a/lib/gitlab/diff/file_collection/base.rb
+++ b/lib/gitlab/diff/file_collection/base.rb
@@ -36,6 +36,8 @@ module Gitlab
private
def decorate_diff!(diff)
+ return diff if diff.is_a?(File)
+
Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs, fallback_diff_refs: fallback_diff_refs)
end
end
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index 690b27cde81..978962ab2eb 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -12,6 +12,10 @@ module Gitlab
:head_sha,
:old_line,
:new_line,
+ :width,
+ :height,
+ :x,
+ :y,
:position_type, to: :formatter
# A position can belong to a text line or to an image coordinate
diff --git a/lib/gitlab/git/raw_diff_change.rb b/lib/gitlab/git/raw_diff_change.rb
index eb3d8819239..92f6c45ce25 100644
--- a/lib/gitlab/git/raw_diff_change.rb
+++ b/lib/gitlab/git/raw_diff_change.rb
@@ -38,7 +38,9 @@ module Gitlab
end
def extract_operation
- case @raw_operation&.first(1)
+ return :unknown unless @raw_operation
+
+ case @raw_operation[0]
when 'A'
:added
when 'C'
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 0ad2d385051..de0044fc149 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -391,18 +391,6 @@ module Gitlab
nil
end
- def archive_prefix(ref, sha, append_sha:)
- append_sha = (ref != sha) if append_sha.nil?
-
- project_name = self.name.chomp('.git')
- formatted_ref = ref.tr('/', '-')
-
- prefix_segments = [project_name, formatted_ref]
- prefix_segments << sha if append_sha
-
- prefix_segments.join('-')
- end
-
def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:)
ref ||= root_ref
commit = Gitlab::Git::Commit.find(self, ref)
@@ -413,12 +401,44 @@ module Gitlab
{
'RepoPath' => path,
'ArchivePrefix' => prefix,
- 'ArchivePath' => archive_file_path(prefix, storage_path, format),
+ 'ArchivePath' => archive_file_path(storage_path, commit.id, prefix, format),
'CommitId' => commit.id
}
end
- def archive_file_path(name, storage_path, format = "tar.gz")
+ # This is both the filename of the archive (missing the extension) and the
+ # name of the top-level member of the archive under which all files go
+ #
+ # FIXME: The generated prefix is incorrect for projects with hashed
+ # storage enabled
+ def archive_prefix(ref, sha, append_sha:)
+ append_sha = (ref != sha) if append_sha.nil?
+
+ project_name = self.name.chomp('.git')
+ formatted_ref = ref.tr('/', '-')
+
+ prefix_segments = [project_name, formatted_ref]
+ prefix_segments << sha if append_sha
+
+ prefix_segments.join('-')
+ end
+ private :archive_prefix
+
+ # The full path on disk where the archive should be stored. This is used
+ # to cache the archive between requests.
+ #
+ # The path is a global namespace, so needs to be globally unique. This is
+ # achieved by including `gl_repository` in the path.
+ #
+ # Archives relating to a particular ref when the SHA is not present in the
+ # filename must be invalidated when the ref is updated to point to a new
+ # SHA. This is achieved by including the SHA in the path.
+ #
+ # As this is a full path on disk, it is not "cloud native". This should
+ # be resolved by either removing the cache, or moving the implementation
+ # into Gitaly and removing the ArchivePath parameter from the git-archive
+ # senddata response.
+ def archive_file_path(storage_path, sha, name, format = "tar.gz")
# Build file path
return nil unless name
@@ -436,8 +456,9 @@ module Gitlab
end
file_name = "#{name}.#{extension}"
- File.join(storage_path, self.name, file_name)
+ File.join(storage_path, self.gl_repository, sha, file_name)
end
+ private :archive_file_path
# Return repo size in megabytes
def size
diff --git a/lib/gitlab/kubernetes/helm/base_command.rb b/lib/gitlab/kubernetes/helm/base_command.rb
index 6e4df05aa7e..3d778da90c7 100644
--- a/lib/gitlab/kubernetes/helm/base_command.rb
+++ b/lib/gitlab/kubernetes/helm/base_command.rb
@@ -15,6 +15,9 @@ module Gitlab
def generate_script
<<~HEREDOC
set -eo pipefail
+ ALPINE_VERSION=$(cat /etc/alpine-release | cut -d '.' -f 1,2)
+ echo http://mirror.clarkson.edu/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories
+ echo http://mirror1.hs-esslingen.de/pub/Mirrors/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories
apk add -U ca-certificates openssl >/dev/null
wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v#{Gitlab::Kubernetes::Helm::HELM_VERSION}-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
mv /tmp/linux-amd64/helm /usr/bin/
diff --git a/spec/features/dashboard/groups_list_spec.rb b/spec/features/dashboard/groups_list_spec.rb
index a71020002dc..ed47f7ed390 100644
--- a/spec/features/dashboard/groups_list_spec.rb
+++ b/spec/features/dashboard/groups_list_spec.rb
@@ -40,7 +40,7 @@ feature 'Dashboard Groups page', :js do
expect(page).to have_content(nested_group.name)
end
- describe 'when filtering groups', :nested_groups do
+ context 'when filtering groups', :nested_groups do
before do
group.add_owner(user)
nested_group.add_owner(user)
@@ -75,7 +75,7 @@ feature 'Dashboard Groups page', :js do
end
end
- describe 'group with subgroups', :nested_groups do
+ context 'with subgroups', :nested_groups do
let!(:subgroup) { create(:group, :public, parent: group) }
before do
@@ -106,7 +106,7 @@ feature 'Dashboard Groups page', :js do
end
end
- describe 'when using pagination' do
+ context 'when using pagination' do
let(:group) { create(:group, created_at: 5.days.ago) }
let(:group2) { create(:group, created_at: 2.days.ago) }
@@ -141,4 +141,20 @@ feature 'Dashboard Groups page', :js do
expect(page).not_to have_selector("#group-#{group2.id}")
end
end
+
+ context 'when signed in as admin' do
+ let(:admin) { create(:admin) }
+
+ it 'shows only groups admin is member of' do
+ group.add_owner(admin)
+ expect(another_group).to be_persisted
+
+ sign_in(admin)
+ visit dashboard_groups_path
+ wait_for_requests
+
+ expect(page).to have_content(group.name)
+ expect(page).not_to have_content(another_group.name)
+ end
+ end
end
diff --git a/spec/features/projects/issues/user_toggles_subscription_spec.rb b/spec/features/projects/issues/user_toggles_subscription_spec.rb
index 117a614b980..c2b2a193682 100644
--- a/spec/features/projects/issues/user_toggles_subscription_spec.rb
+++ b/spec/features/projects/issues/user_toggles_subscription_spec.rb
@@ -1,9 +1,9 @@
require "spec_helper"
describe "User toggles subscription", :js do
- set(:project) { create(:project_empty_repo, :public) }
- set(:user) { create(:user) }
- set(:issue) { create(:issue, project: project, author: user) }
+ let(:project) { create(:project_empty_repo, :public) }
+ let(:user) { create(:user) }
+ let(:issue) { create(:issue, project: project, author: user) }
before do
project.add_developer(user)
@@ -12,7 +12,7 @@ describe "User toggles subscription", :js do
visit(project_issue_path(project, issue))
end
- it "unsibscribes from issue" do
+ it "unsubscribes from issue" do
subscription_button = find(".js-issuable-subscribe-button")
# Check we're subscribed.
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index abc470788e1..16c0d418d98 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -2,43 +2,71 @@ require 'spec_helper'
describe GroupsFinder do
describe '#execute' do
- let(:user) { create(:user) }
-
- context 'root level groups' do
- let!(:private_group) { create(:group, :private) }
- let!(:internal_group) { create(:group, :internal) }
- let!(:public_group) { create(:group, :public) }
-
- context 'without a user' do
- subject { described_class.new.execute }
-
- it { is_expected.to eq([public_group]) }
+ let(:user) { create(:user) }
+
+ describe 'root level groups' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:user_type, :params, :results) do
+ nil | { all_available: true } | %i(public_group user_public_group)
+ nil | { all_available: false } | %i(public_group user_public_group)
+ nil | {} | %i(public_group user_public_group)
+
+ :regular | { all_available: true } | %i(public_group internal_group user_public_group user_internal_group
+ user_private_group)
+ :regular | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :regular | {} | %i(public_group internal_group user_public_group user_internal_group user_private_group)
+
+ :external | { all_available: true } | %i(public_group user_public_group user_internal_group user_private_group)
+ :external | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :external | {} | %i(public_group user_public_group user_internal_group user_private_group)
+
+ :admin | { all_available: true } | %i(public_group internal_group private_group user_public_group
+ user_internal_group user_private_group)
+ :admin | { all_available: false } | %i(user_public_group user_internal_group user_private_group)
+ :admin | {} | %i(public_group internal_group private_group user_public_group user_internal_group
+ user_private_group)
end
- context 'with a user' do
- subject { described_class.new(user).execute }
-
- context 'normal user' do
- it { is_expected.to contain_exactly(public_group, internal_group) }
- end
-
- context 'external user' do
- let(:user) { create(:user, external: true) }
-
- it { is_expected.to contain_exactly(public_group) }
+ with_them do
+ before do
+ # Fixme: Because of an issue: https://github.com/tomykaira/rspec-parameterized/issues/8#issuecomment-381888428
+ # The groups need to be created here, not with let syntax, and also compared by name and not ids
+
+ @groups = {
+ private_group: create(:group, :private, name: 'private_group'),
+ internal_group: create(:group, :internal, name: 'internal_group'),
+ public_group: create(:group, :public, name: 'public_group'),
+
+ user_private_group: create(:group, :private, name: 'user_private_group'),
+ user_internal_group: create(:group, :internal, name: 'user_internal_group'),
+ user_public_group: create(:group, :public, name: 'user_public_group')
+ }
+
+ if user_type
+ user =
+ case user_type
+ when :regular
+ create(:user)
+ when :external
+ create(:user, external: true)
+ when :admin
+ create(:user, :admin)
+ end
+ @groups.values_at(:user_private_group, :user_internal_group, :user_public_group).each do |group|
+ group.add_developer(user)
+ end
+ end
end
- context 'user is member of the private group' do
- before do
- private_group.add_guest(user)
- end
+ subject { described_class.new(User.last, params).execute.to_a }
- it { is_expected.to contain_exactly(public_group, internal_group, private_group) }
- end
+ it { is_expected.to match_array(@groups.values_at(*results)) }
end
end
context 'subgroups', :nested_groups do
+ let(:user) { create(:user) }
let!(:parent_group) { create(:group, :public) }
let!(:public_subgroup) { create(:group, :public, parent: parent_group) }
let!(:internal_subgroup) { create(:group, :internal, parent: parent_group) }
diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json
index 4c4ca3b582f..0f9c221fd6d 100644
--- a/spec/fixtures/api/schemas/public_api/v4/notes.json
+++ b/spec/fixtures/api/schemas/public_api/v4/notes.json
@@ -24,7 +24,10 @@
"system": { "type": "boolean" },
"noteable_id": { "type": "integer" },
"noteable_iid": { "type": "integer" },
- "noteable_type": { "type": "string" }
+ "noteable_type": { "type": "string" },
+ "resolved": { "type": "boolean" },
+ "resolvable": { "type": "boolean" },
+ "resolved_by": { "type": ["string", "null"] }
},
"required": [
"id", "body", "attachment", "author", "created_at", "updated_at",
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 43cb0dfe163..5e454f8b310 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -2,8 +2,6 @@
require 'spec_helper'
describe ApplicationHelper do
- include UploadHelpers
-
describe 'current_controller?' do
it 'returns true when controller matches argument' do
stub_controller_name('foo')
@@ -54,143 +52,6 @@ describe ApplicationHelper do
end
end
- describe 'project_icon' do
- it 'returns an url for the avatar' do
- project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
-
- expect(helper.project_icon(project.full_path).to_s)
- .to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
- end
- end
-
- describe 'avatar_icon_for' do
- let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
- let(:email) { 'foo@example.com' }
- let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
-
- it 'prefers the user to retrieve the avatar_url' do
- expect(helper.avatar_icon_for(user, email).to_s)
- .to eq(user.avatar.url)
- end
-
- it 'falls back to email lookup if no user given' do
- expect(helper.avatar_icon_for(nil, email).to_s)
- .to eq(another_user.avatar.url)
- end
- end
-
- describe 'avatar_icon_for_email' do
- let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
-
- context 'using an email' do
- context 'when there is a matching user' do
- it 'returns a relative URL for the avatar' do
- expect(helper.avatar_icon_for_email(user.email).to_s)
- .to eq(user.avatar.url)
- end
- end
-
- context 'when no user exists for the email' do
- it 'calls gravatar_icon' do
- expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
-
- helper.avatar_icon_for_email('foo@example.com', 20, 2)
- end
- end
-
- context 'without an email passed' do
- it 'calls gravatar_icon' do
- expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
-
- helper.avatar_icon_for_email(nil, 20, 2)
- end
- end
- end
- end
-
- describe 'avatar_icon_for_user' do
- let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
-
- context 'with a user object passed' do
- it 'returns a relative URL for the avatar' do
- expect(helper.avatar_icon_for_user(user).to_s)
- .to eq(user.avatar.url)
- end
- end
-
- context 'without a user object passed' do
- it 'calls gravatar_icon' do
- expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
-
- helper.avatar_icon_for_user(nil, 20, 2)
- end
- end
- end
-
- describe 'gravatar_icon' do
- let(:user_email) { 'user@email.com' }
-
- context 'with Gravatar disabled' do
- before do
- stub_application_setting(gravatar_enabled?: false)
- end
-
- it 'returns a generic avatar' do
- expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png')
- end
- end
-
- context 'with Gravatar enabled' do
- before do
- stub_application_setting(gravatar_enabled?: true)
- end
-
- it 'returns a generic avatar when email is blank' do
- expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png')
- end
-
- it 'returns a valid Gravatar URL' do
- stub_config_setting(https: false)
-
- expect(helper.gravatar_icon(user_email))
- .to match('https://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118')
- end
-
- it 'uses HTTPs when configured' do
- stub_config_setting(https: true)
-
- expect(helper.gravatar_icon(user_email))
- .to match('https://secure.gravatar.com')
- end
-
- it 'returns custom gravatar path when gravatar_url is set' do
- stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
-
- expect(gravatar_icon(user_email, 20))
- .to eq('http://example.local/?s=40&hash=b58c6f14d292556214bd64909bcdb118')
- end
-
- it 'accepts a custom size argument' do
- expect(helper.gravatar_icon(user_email, 64)).to include '?s=128'
- end
-
- it 'defaults size to 40@2x when given an invalid size' do
- expect(helper.gravatar_icon(user_email, nil)).to include '?s=80'
- end
-
- it 'accepts a scaling factor' do
- expect(helper.gravatar_icon(user_email, 40, 3)).to include '?s=120'
- end
-
- it 'ignores case and surrounding whitespace' do
- normal = helper.gravatar_icon('foo@example.com')
- upcase = helper.gravatar_icon(' FOO@EXAMPLE.COM ')
-
- expect(normal).to eq upcase
- end
- end
- end
-
describe 'simple_sanitize' do
let(:a_tag) { '<a href="#">Foo</a>' }
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
index c94fedd615b..120b23e66ac 100644
--- a/spec/helpers/auth_helper_spec.rb
+++ b/spec/helpers/auth_helper_spec.rb
@@ -18,6 +18,30 @@ describe AuthHelper do
end
end
+ describe "providers_for_base_controller" do
+ it 'returns all enabled providers from devise' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :github] }
+ expect(helper.providers_for_base_controller).to include(*[:twitter, :github])
+ end
+
+ it 'excludes ldap providers' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+ expect(helper.providers_for_base_controller).not_to include(:ldapmain)
+ end
+ end
+
+ describe "form_based_providers" do
+ it 'includes LDAP providers' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+ expect(helper.form_based_providers).to eq %i(ldapmain)
+ end
+
+ it 'includes crowd provider' do
+ allow(helper).to receive(:auth_providers) { [:twitter, :crowd] }
+ expect(helper.form_based_providers).to eq %i(crowd)
+ end
+ end
+
describe 'enabled_button_based_providers' do
before do
allow(helper).to receive(:auth_providers) { [:twitter, :github] }
diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb
index 04c6d259135..5856bccb5b8 100644
--- a/spec/helpers/avatars_helper_spec.rb
+++ b/spec/helpers/avatars_helper_spec.rb
@@ -1,10 +1,147 @@
require 'rails_helper'
describe AvatarsHelper do
- include ApplicationHelper
+ include UploadHelpers
let(:user) { create(:user) }
+ describe '#project_icon' do
+ it 'returns an url for the avatar' do
+ project = create(:project, :public, avatar: File.open(uploaded_image_temp_path))
+
+ expect(helper.project_icon(project.full_path).to_s)
+ .to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />"
+ end
+ end
+
+ describe '#avatar_icon_for' do
+ let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
+ let(:email) { 'foo@example.com' }
+ let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
+
+ it 'prefers the user to retrieve the avatar_url' do
+ expect(helper.avatar_icon_for(user, email).to_s)
+ .to eq(user.avatar.url)
+ end
+
+ it 'falls back to email lookup if no user given' do
+ expect(helper.avatar_icon_for(nil, email).to_s)
+ .to eq(another_user.avatar.url)
+ end
+ end
+
+ describe '#avatar_icon_for_email' do
+ let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+
+ context 'using an email' do
+ context 'when there is a matching user' do
+ it 'returns a relative URL for the avatar' do
+ expect(helper.avatar_icon_for_email(user.email).to_s)
+ .to eq(user.avatar.url)
+ end
+ end
+
+ context 'when no user exists for the email' do
+ it 'calls gravatar_icon' do
+ expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
+
+ helper.avatar_icon_for_email('foo@example.com', 20, 2)
+ end
+ end
+
+ context 'without an email passed' do
+ it 'calls gravatar_icon' do
+ expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
+
+ helper.avatar_icon_for_email(nil, 20, 2)
+ end
+ end
+ end
+ end
+
+ describe '#avatar_icon_for_user' do
+ let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+
+ context 'with a user object passed' do
+ it 'returns a relative URL for the avatar' do
+ expect(helper.avatar_icon_for_user(user).to_s)
+ .to eq(user.avatar.url)
+ end
+ end
+
+ context 'without a user object passed' do
+ it 'calls gravatar_icon' do
+ expect(helper).to receive(:gravatar_icon).with(nil, 20, 2)
+
+ helper.avatar_icon_for_user(nil, 20, 2)
+ end
+ end
+ end
+
+ describe '#gravatar_icon' do
+ let(:user_email) { 'user@email.com' }
+
+ context 'with Gravatar disabled' do
+ before do
+ stub_application_setting(gravatar_enabled?: false)
+ end
+
+ it 'returns a generic avatar' do
+ expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png')
+ end
+ end
+
+ context 'with Gravatar enabled' do
+ before do
+ stub_application_setting(gravatar_enabled?: true)
+ end
+
+ it 'returns a generic avatar when email is blank' do
+ expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png')
+ end
+
+ it 'returns a valid Gravatar URL' do
+ stub_config_setting(https: false)
+
+ expect(helper.gravatar_icon(user_email))
+ .to match('https://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118')
+ end
+
+ it 'uses HTTPs when configured' do
+ stub_config_setting(https: true)
+
+ expect(helper.gravatar_icon(user_email))
+ .to match('https://secure.gravatar.com')
+ end
+
+ it 'returns custom gravatar path when gravatar_url is set' do
+ stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
+
+ expect(gravatar_icon(user_email, 20))
+ .to eq('http://example.local/?s=40&hash=b58c6f14d292556214bd64909bcdb118')
+ end
+
+ it 'accepts a custom size argument' do
+ expect(helper.gravatar_icon(user_email, 64)).to include '?s=128'
+ end
+
+ it 'defaults size to 40@2x when given an invalid size' do
+ expect(helper.gravatar_icon(user_email, nil)).to include '?s=80'
+ end
+
+ it 'accepts a scaling factor' do
+ expect(helper.gravatar_icon(user_email, 40, 3)).to include '?s=120'
+ end
+
+ it 'ignores case and surrounding whitespace' do
+ normal = helper.gravatar_icon('foo@example.com')
+ upcase = helper.gravatar_icon(' FOO@EXAMPLE.COM ')
+
+ expect(normal).to eq upcase
+ end
+ end
+ end
+
describe '#user_avatar' do
subject { helper.user_avatar(user: user) }
diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js
index 8b6e8b24f00..fcd7bea3f6d 100644
--- a/spec/javascripts/sidebar/mock_data.js
+++ b/spec/javascripts/sidebar/mock_data.js
@@ -138,7 +138,7 @@ const RESPONSE_MAP = {
},
{
id: 20,
- name_with_namespace: 'foo / bar',
+ name_with_namespace: '<img src=x onerror=alert(document.domain)> foo / bar',
},
],
},
diff --git a/spec/javascripts/sidebar/sidebar_move_issue_spec.js b/spec/javascripts/sidebar/sidebar_move_issue_spec.js
index 1777053d370..8f35b9ca437 100644
--- a/spec/javascripts/sidebar/sidebar_move_issue_spec.js
+++ b/spec/javascripts/sidebar/sidebar_move_issue_spec.js
@@ -71,6 +71,15 @@ describe('SidebarMoveIssue', function () {
expect($.fn.glDropdown).toHaveBeenCalled();
});
+
+ it('escapes html from project name', (done) => {
+ this.$toggleButton.dropdown('toggle');
+
+ setTimeout(() => {
+ expect(this.$content.find('.js-move-issue-dropdown-item')[1].innerHTML.trim()).toEqual('&lt;img src=x onerror=alert(document.domain)&gt; foo / bar');
+ done();
+ });
+ });
});
describe('onConfirmClicked', () => {
diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
index 1fd145116df..068cdc85a07 100644
--- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
+++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
@@ -47,16 +47,36 @@ describe Banzai::Filter::CommitTrailersFilter do
)
end
- it 'non GitLab users and replaces them with mailto links' do
- _, message_html = build_commit_message(
- trailer: trailer,
- name: FFaker::Name.name,
- email: email
- )
+ context 'non GitLab users' do
+ shared_examples 'mailto links' do
+ it 'replaces them with mailto links' do
+ _, message_html = build_commit_message(
+ trailer: trailer,
+ name: FFaker::Name.name,
+ email: email
+ )
- doc = filter(message_html)
+ doc = filter(message_html)
- expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+ expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: trailer)
+ end
+ end
+
+ context 'when Gravatar is disabled' do
+ before do
+ stub_application_setting(gravatar_enabled: false)
+ end
+
+ it_behaves_like 'mailto links'
+ end
+
+ context 'when Gravatar is enabled' do
+ before do
+ stub_application_setting(gravatar_enabled: true)
+ end
+
+ it_behaves_like 'mailto links'
+ end
end
it 'multiple trailers in the same message' do
@@ -69,7 +89,7 @@ describe Banzai::Filter::CommitTrailersFilter do
doc = filter(message)
expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer)
- expect_to_have_mailto_link(doc, email: email, trailer: different_trailer)
+ expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: different_trailer)
end
context 'special names' do
@@ -90,7 +110,7 @@ describe Banzai::Filter::CommitTrailersFilter do
doc = filter(message_html)
- expect_to_have_mailto_link(doc, email: email, trailer: trailer)
+ expect_to_have_mailto_link_with_avatar(doc, email: email, trailer: trailer)
expect(doc.text).to match Regexp.escape(message)
end
end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index da1a6229ccf..9924641f829 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -234,59 +234,72 @@ describe Gitlab::Git::Repository, seed_helper: true do
it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :tag_names
end
- shared_examples 'archive check' do |extenstion|
- it { expect(metadata['ArchivePath']).to match(%r{tmp/gitlab-git-test.git/gitlab-git-test-master-#{SeedRepo::LastCommit::ID}}) }
- it { expect(metadata['ArchivePath']).to end_with extenstion }
- end
+ describe '#archive_metadata' do
+ let(:storage_path) { '/tmp' }
+ let(:cache_key) { File.join(repository.gl_repository, SeedRepo::LastCommit::ID) }
- describe '#archive_prefix' do
- let(:project_name) { 'project-name'}
+ let(:append_sha) { true }
+ let(:ref) { 'master' }
+ let(:format) { nil }
- before do
- expect(repository).to receive(:name).once.and_return(project_name)
- end
+ let(:expected_extension) { 'tar.gz' }
+ let(:expected_filename) { "#{expected_prefix}.#{expected_extension}" }
+ let(:expected_path) { File.join(storage_path, cache_key, expected_filename) }
+ let(:expected_prefix) { "gitlab-git-test-#{ref}-#{SeedRepo::LastCommit::ID}" }
- it 'returns parameterised string for a ref containing slashes' do
- prefix = repository.archive_prefix('test/branch', 'SHA', append_sha: nil)
+ subject(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: append_sha) }
- expect(prefix).to eq("#{project_name}-test-branch-SHA")
+ it 'sets RepoPath to the repository path' do
+ expect(metadata['RepoPath']).to eq(repository.path)
end
- it 'returns correct string for a ref containing dots' do
- prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: nil)
-
- expect(prefix).to eq("#{project_name}-test.branch-SHA")
+ it 'sets CommitId to the commit SHA' do
+ expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID)
end
- it 'returns string with sha when append_sha is false' do
- prefix = repository.archive_prefix('test.branch', 'SHA', append_sha: false)
-
- expect(prefix).to eq("#{project_name}-test.branch")
+ it 'sets ArchivePrefix to the expected prefix' do
+ expect(metadata['ArchivePrefix']).to eq(expected_prefix)
end
- end
- describe '#archive' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', append_sha: true) }
+ it 'sets ArchivePath to the expected globally-unique path' do
+ # This is really important from a security perspective. Think carefully
+ # before changing it: https://gitlab.com/gitlab-org/gitlab-ce/issues/45689
+ expect(expected_path).to include(File.join(repository.gl_repository, SeedRepo::LastCommit::ID))
- it_should_behave_like 'archive check', '.tar.gz'
- end
-
- describe '#archive_zip' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'zip', append_sha: true) }
+ expect(metadata['ArchivePath']).to eq(expected_path)
+ end
- it_should_behave_like 'archive check', '.zip'
- end
+ context 'append_sha varies archive path and filename' do
+ where(:append_sha, :ref, :expected_prefix) do
+ sha = SeedRepo::LastCommit::ID
- describe '#archive_bz2' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'tbz2', append_sha: true) }
+ true | 'master' | "gitlab-git-test-master-#{sha}"
+ true | sha | "gitlab-git-test-#{sha}-#{sha}"
+ false | 'master' | "gitlab-git-test-master"
+ false | sha | "gitlab-git-test-#{sha}"
+ nil | 'master' | "gitlab-git-test-master-#{sha}"
+ nil | sha | "gitlab-git-test-#{sha}"
+ end
- it_should_behave_like 'archive check', '.tar.bz2'
- end
+ with_them do
+ it { expect(metadata['ArchivePrefix']).to eq(expected_prefix) }
+ it { expect(metadata['ArchivePath']).to eq(expected_path) }
+ end
+ end
- describe '#archive_fallback' do
- let(:metadata) { repository.archive_metadata('master', '/tmp', 'madeup', append_sha: true) }
+ context 'format varies archive path and filename' do
+ where(:format, :expected_extension) do
+ nil | 'tar.gz'
+ 'madeup' | 'tar.gz'
+ 'tbz2' | 'tar.bz2'
+ 'zip' | 'zip'
+ end
- it_should_behave_like 'archive check', '.tar.gz'
+ with_them do
+ it { expect(metadata['ArchivePrefix']).to eq(expected_prefix) }
+ it { expect(metadata['ArchivePath']).to eq(expected_path) }
+ end
+ end
end
describe '#size' do
diff --git a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
index 3cfdae794f6..7be8be54d5e 100644
--- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb
@@ -4,22 +4,10 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do
let(:application) { create(:clusters_applications_helm) }
let(:base_command) { described_class.new(application.name) }
- describe '#generate_script' do
- let(:helm_version) { Gitlab::Kubernetes::Helm::HELM_VERSION }
- let(:command) do
- <<~HEREDOC
- set -eo pipefail
- apk add -U ca-certificates openssl >/dev/null
- wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v#{helm_version}-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
- mv /tmp/linux-amd64/helm /usr/bin/
- HEREDOC
- end
-
- subject { base_command.generate_script }
+ subject { base_command }
- it 'should return a command that prepares the environment for helm-cli' do
- expect(subject).to eq(command)
- end
+ it_behaves_like 'helm commands' do
+ let(:commands) { '' }
end
describe '#pod_resource' do
diff --git a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
index e6920b0a76f..89e36a298f8 100644
--- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb
@@ -2,23 +2,9 @@ require 'spec_helper'
describe Gitlab::Kubernetes::Helm::InitCommand do
let(:application) { create(:clusters_applications_helm) }
- let(:init_command) { described_class.new(application.name) }
+ let(:commands) { 'helm init >/dev/null' }
- describe '#generate_script' do
- let(:command) do
- <<~MSG.chomp
- set -eo pipefail
- apk add -U ca-certificates openssl >/dev/null
- wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.0-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
- mv /tmp/linux-amd64/helm /usr/bin/
- helm init >/dev/null
- MSG
- end
+ subject { described_class.new(application.name) }
- subject { init_command.generate_script }
-
- it 'should return the appropriate command' do
- is_expected.to eq(command)
- end
- end
+ it_behaves_like 'helm commands'
end
diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
index 137b8f718de..547f3f1752c 100644
--- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
+++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb
@@ -12,50 +12,36 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do
)
end
- describe '#generate_script' do
- let(:command) do
- <<~MSG
- set -eo pipefail
- apk add -U ca-certificates openssl >/dev/null
- wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.0-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
- mv /tmp/linux-amd64/helm /usr/bin/
- helm init --client-only >/dev/null
- helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null
- MSG
- end
-
- subject { install_command.generate_script }
+ subject { install_command }
- it 'should return appropriate command' do
- is_expected.to eq(command)
+ it_behaves_like 'helm commands' do
+ let(:commands) do
+ <<~EOS
+ helm init --client-only >/dev/null
+ helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null
+ EOS
end
+ end
- context 'with an application with a repository' do
- let(:ci_runner) { create(:ci_runner) }
- let(:application) { create(:clusters_applications_runner, runner: ci_runner) }
- let(:install_command) do
- described_class.new(
- application.name,
- chart: application.chart,
- values: application.values,
- repository: application.repository
- )
- end
-
- let(:command) do
- <<~MSG
- set -eo pipefail
- apk add -U ca-certificates openssl >/dev/null
- wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.0-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
- mv /tmp/linux-amd64/helm /usr/bin/
- helm init --client-only >/dev/null
- helm repo add #{application.name} #{application.repository}
- helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null
- MSG
- end
+ context 'with an application with a repository' do
+ let(:ci_runner) { create(:ci_runner) }
+ let(:application) { create(:clusters_applications_runner, runner: ci_runner) }
+ let(:install_command) do
+ described_class.new(
+ application.name,
+ chart: application.chart,
+ values: application.values,
+ repository: application.repository
+ )
+ end
- it 'should return appropriate command' do
- is_expected.to eq(command)
+ it_behaves_like 'helm commands' do
+ let(:commands) do
+ <<~EOS
+ helm init --client-only >/dev/null
+ helm repo add #{application.name} #{application.repository}
+ helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null
+ EOS
end
end
end
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index 2705421e540..fb51c0172ab 100644
--- a/spec/models/diff_note_spec.rb
+++ b/spec/models/diff_note_spec.rb
@@ -85,12 +85,35 @@ describe DiffNote do
end
describe "#diff_file" do
- it "returns the correct diff file" do
- diff_file = subject.diff_file
+ context 'when the discussion was created in the diff' do
+ it 'returns correct diff file' do
+ diff_file = subject.diff_file
- expect(diff_file.old_path).to eq(position.old_path)
- expect(diff_file.new_path).to eq(position.new_path)
- expect(diff_file.diff_refs).to eq(position.diff_refs)
+ expect(diff_file.old_path).to eq(position.old_path)
+ expect(diff_file.new_path).to eq(position.new_path)
+ expect(diff_file.diff_refs).to eq(position.diff_refs)
+ end
+ end
+
+ context 'when discussion is outdated or not created in the diff' do
+ let(:diff_refs) { project.commit(sample_commit.id).diff_refs }
+ let(:position) do
+ Gitlab::Diff::Position.new(
+ old_path: "files/ruby/popen.rb",
+ new_path: "files/ruby/popen.rb",
+ old_line: nil,
+ new_line: 14,
+ diff_refs: diff_refs
+ )
+ end
+
+ it 'returns the correct diff file' do
+ diff_file = subject.diff_file
+
+ expect(diff_file.old_path).to eq(position.old_path)
+ expect(diff_file.new_path).to eq(position.new_path)
+ expect(diff_file.diff_refs).to eq(position.diff_refs)
+ end
end
end
diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb
index 4a44b219a67..ef34192f888 100644
--- a/spec/requests/api/discussions_spec.rb
+++ b/spec/requests/api/discussions_spec.rb
@@ -2,32 +2,53 @@ require 'spec_helper'
describe API::Discussions do
let(:user) { create(:user) }
- let!(:project) { create(:project, :public, namespace: user.namespace) }
+ let!(:project) { create(:project, :public, :repository, namespace: user.namespace) }
let(:private_user) { create(:user) }
before do
- project.add_reporter(user)
+ project.add_developer(user)
end
- context "when noteable is an Issue" do
+ context 'when noteable is an Issue' do
let!(:issue) { create(:issue, project: project, author: user) }
let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) }
- it_behaves_like "discussions API", 'projects', 'issues', 'iid' do
+ it_behaves_like 'discussions API', 'projects', 'issues', 'iid' do
let(:parent) { project }
let(:noteable) { issue }
let(:note) { issue_note }
end
end
- context "when noteable is a Snippet" do
+ context 'when noteable is a Snippet' do
let!(:snippet) { create(:project_snippet, project: project, author: user) }
let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) }
- it_behaves_like "discussions API", 'projects', 'snippets', 'id' do
+ it_behaves_like 'discussions API', 'projects', 'snippets', 'id' do
let(:parent) { project }
let(:noteable) { snippet }
let(:note) { snippet_note }
end
end
+
+ context 'when noteable is a Merge Request' do
+ let!(:noteable) { create(:merge_request_with_diffs, source_project: project, target_project: project, author: user) }
+ let!(:note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project, author: user) }
+ let!(:diff_note) { create(:diff_note_on_merge_request, noteable: noteable, project: project, author: user) }
+ let(:parent) { project }
+
+ it_behaves_like 'discussions API', 'projects', 'merge_requests', 'iid'
+ it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid'
+ it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid'
+ end
+
+ context 'when noteable is a Commit' do
+ let!(:noteable) { create(:commit, project: project, author: user) }
+ let!(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+ let!(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+ let(:parent) { project }
+
+ it_behaves_like 'discussions API', 'projects', 'repository/commits', 'id'
+ it_behaves_like 'diff discussions API', 'projects', 'repository/commits', 'id'
+ end
end
diff --git a/spec/services/notes/resolve_service_spec.rb b/spec/services/notes/resolve_service_spec.rb
new file mode 100644
index 00000000000..b54d40a7a5c
--- /dev/null
+++ b/spec/services/notes/resolve_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Notes::ResolveService do
+ let(:merge_request) { create(:merge_request) }
+ let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) }
+ let(:user) { merge_request.author }
+
+ describe '#execute' do
+ it "resolves the note" do
+ described_class.new(merge_request.project, user).execute(note)
+ note.reload
+
+ expect(note.resolved?).to be true
+ expect(note.resolved_by).to eq(user)
+ end
+
+ it "sends notifications if all discussions are resolved" do
+ expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request)
+
+ described_class.new(merge_request.project, user).execute(note)
+ end
+ end
+end
diff --git a/spec/services/repository_archive_clean_up_service_spec.rb b/spec/services/repository_archive_clean_up_service_spec.rb
index 2d7fa3f80f7..ab1c638fc39 100644
--- a/spec/services/repository_archive_clean_up_service_spec.rb
+++ b/spec/services/repository_archive_clean_up_service_spec.rb
@@ -1,15 +1,47 @@
require 'spec_helper'
describe RepositoryArchiveCleanUpService do
- describe '#execute' do
- subject(:service) { described_class.new }
+ subject(:service) { described_class.new }
+ describe '#execute (new archive locations)' do
+ let(:sha) { "0" * 40 }
+
+ it 'removes outdated archives and directories in a new-style path' do
+ in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 3.hours) do |dirname, files|
+ service.execute
+
+ files.each { |filename| expect(File.exist?(filename)).to be_falsy }
+ expect(File.directory?(dirname)).to be_falsy
+ expect(File.directory?(File.dirname(dirname))).to be_falsy
+ end
+ end
+
+ it 'does not remove directories when they contain outdated non-archives' do
+ in_directory_with_files("project-999/#{sha}", %w[tar conf rb], 2.hours) do |dirname, files|
+ service.execute
+
+ expect(File.directory?(dirname)).to be_truthy
+ end
+ end
+
+ it 'does not remove in-date archives in a new-style path' do
+ in_directory_with_files("project-999/#{sha}", %w[tar tar.bz2 tar.gz zip], 1.hour) do |dirname, files|
+ service.execute
+
+ files.each { |filename| expect(File.exist?(filename)).to be_truthy }
+ end
+ end
+ end
+
+ describe '#execute (legacy archive locations)' do
context 'when the downloads directory does not exist' do
it 'does not remove any archives' do
path = '/invalid/path/'
stub_repository_downloads_path(path)
+ allow(File).to receive(:directory?).and_call_original
expect(File).to receive(:directory?).with(path).and_return(false)
+
expect(service).not_to receive(:clean_up_old_archives)
expect(service).not_to receive(:clean_up_empty_directories)
@@ -19,7 +51,7 @@ describe RepositoryArchiveCleanUpService do
context 'when the downloads directory exists' do
shared_examples 'invalid archive files' do |dirname, extensions, mtime|
- it 'does not remove files and directoy' do
+ it 'does not remove files and directory' do
in_directory_with_files(dirname, extensions, mtime) do |dir, files|
service.execute
@@ -43,7 +75,7 @@ describe RepositoryArchiveCleanUpService do
end
context 'with files older than 2 hours inside invalid directories' do
- it_behaves_like 'invalid archive files', 'john_doe/sample.git', %w[conf rb tar tar.gz], 2.hours
+ it_behaves_like 'invalid archive files', 'john/doe/sample.git', %w[conf rb tar tar.gz], 2.hours
end
context 'with files newer than 2 hours that matches valid archive extensions' do
@@ -58,24 +90,24 @@ describe RepositoryArchiveCleanUpService do
it_behaves_like 'invalid archive files', 'sample.git', %w[conf rb tar tar.gz], 1.hour
end
end
+ end
- def in_directory_with_files(dirname, extensions, mtime)
- Dir.mktmpdir do |tmpdir|
- stub_repository_downloads_path(tmpdir)
- dir = File.join(tmpdir, dirname)
- files = create_temporary_files(dir, extensions, mtime)
+ def in_directory_with_files(dirname, extensions, mtime)
+ Dir.mktmpdir do |tmpdir|
+ stub_repository_downloads_path(tmpdir)
+ dir = File.join(tmpdir, dirname)
+ files = create_temporary_files(dir, extensions, mtime)
- yield(dir, files)
- end
+ yield(dir, files)
end
+ end
- def stub_repository_downloads_path(path)
- allow(Gitlab.config.gitlab).to receive(:repository_downloads_path).and_return(path)
- end
+ def stub_repository_downloads_path(path)
+ allow(Gitlab.config.gitlab).to receive(:repository_downloads_path).and_return(path)
+ end
- def create_temporary_files(dir, extensions, mtime)
- FileUtils.mkdir_p(dir)
- FileUtils.touch(extensions.map { |ext| File.join(dir, "sample.#{ext}") }, mtime: Time.now - mtime)
- end
+ def create_temporary_files(dir, extensions, mtime)
+ FileUtils.mkdir_p(dir)
+ FileUtils.touch(extensions.map { |ext| File.join(dir, "sample.#{ext}") }, mtime: Time.now - mtime)
end
end
diff --git a/spec/support/commit_trailers_spec_helper.rb b/spec/support/commit_trailers_spec_helper.rb
index add359946db..efa317fd2f9 100644
--- a/spec/support/commit_trailers_spec_helper.rb
+++ b/spec/support/commit_trailers_spec_helper.rb
@@ -8,7 +8,7 @@ module CommitTrailersSpecHelper
expect(wrapper.attribute('data-user').value).to eq user.id.to_s
end
- def expect_to_have_mailto_link(doc, email:, trailer:)
+ def expect_to_have_mailto_link_with_avatar(doc, email:, trailer:)
wrapper = find_user_wrapper(doc, trailer)
expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email)
diff --git a/spec/support/shared_examples/helm_generated_script.rb b/spec/support/shared_examples/helm_generated_script.rb
new file mode 100644
index 00000000000..56e86a87ab9
--- /dev/null
+++ b/spec/support/shared_examples/helm_generated_script.rb
@@ -0,0 +1,19 @@
+shared_examples 'helm commands' do
+ describe '#generate_script' do
+ let(:helm_setup) do
+ <<~EOS
+ set -eo pipefail
+ ALPINE_VERSION=$(cat /etc/alpine-release | cut -d '.' -f 1,2)
+ echo http://mirror.clarkson.edu/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories
+ echo http://mirror1.hs-esslingen.de/pub/Mirrors/alpine/v$ALPINE_VERSION/main >> /etc/apk/repositories
+ apk add -U ca-certificates openssl >/dev/null
+ wget -q -O - https://kubernetes-helm.storage.googleapis.com/helm-v2.7.0-linux-amd64.tar.gz | tar zxC /tmp >/dev/null
+ mv /tmp/linux-amd64/helm /usr/bin/
+ EOS
+ end
+
+ it 'should return appropriate command' do
+ expect(subject.generate_script).to eq(helm_setup + commands)
+ end
+ end
+end
diff --git a/spec/support/shared_examples/requests/api/diff_discussions.rb b/spec/support/shared_examples/requests/api/diff_discussions.rb
new file mode 100644
index 00000000000..85a4bd8ca27
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/diff_discussions.rb
@@ -0,0 +1,57 @@
+shared_examples 'diff discussions API' do |parent_type, noteable_type, id_name|
+ describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+ it "includes diff discussions" do
+ get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user)
+
+ discussion = json_response.find { |record| record['id'] == diff_note.discussion_id }
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(discussion).not_to be_nil
+ expect(discussion['individual_note']).to eq(false)
+ expect(discussion['notes'].first['body']).to eq(diff_note.note)
+ end
+ end
+
+ describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+ it "returns a discussion by id" do
+ get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{diff_note.discussion_id}", user)
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['id']).to eq(diff_note.discussion_id)
+ expect(json_response['notes'].first['body']).to eq(diff_note.note)
+ expect(json_response['notes'].first['position']).to eq(diff_note.position.to_h.stringify_keys)
+ end
+ end
+
+ describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+ it "creates a new diff note" do
+ position = diff_note.position.to_h
+
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['notes'].first['body']).to eq('hi!')
+ expect(json_response['notes'].first['type']).to eq('DiffNote')
+ expect(json_response['notes'].first['position']).to eq(position.stringify_keys)
+ end
+
+ it "returns a 400 bad request error when position is invalid" do
+ position = diff_note.position.to_h.merge(new_line: '100000')
+
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+ end
+
+ describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do
+ it 'adds a new note to the diff discussion' do
+ post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{diff_note.discussion_id}/notes", user), body: 'hi!'
+
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['body']).to eq('hi!')
+ expect(json_response['type']).to eq('DiffNote')
+ end
+ end
+end
diff --git a/spec/support/shared_examples/requests/api/resolvable_discussions.rb b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
new file mode 100644
index 00000000000..408ad08cc48
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
@@ -0,0 +1,87 @@
+shared_examples 'resolvable discussions API' do |parent_type, noteable_type, id_name|
+ describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+ it "resolves discussion if resolved is true" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user), resolved: true
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['notes'].size).to eq(1)
+ expect(json_response['notes'][0]['resolved']).to eq(true)
+ end
+
+ it "unresolves discussion if resolved is false" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user), resolved: false
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['notes'].size).to eq(1)
+ expect(json_response['notes'][0]['resolved']).to eq(false)
+ end
+
+ it "returns a 400 bad request error if resolved parameter is not passed" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", user)
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+
+ it "returns a 401 unauthorized error if user is not authenticated" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}"), resolved: true
+
+ expect(response).to have_gitlab_http_status(401)
+ end
+
+ it "returns a 403 error if user resolves discussion of someone else" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+
+ context 'when user does not have access to read the discussion' do
+ before do
+ parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+ end
+
+ it 'responds with 404' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+ end
+ end
+
+ describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+ it 'returns resolved note when resolved parameter is true' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", user), resolved: true
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response['resolved']).to eq(true)
+ end
+
+ it 'returns a 404 error when note id not found' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/12345", user),
+ body: 'Hello!'
+
+ expect(response).to have_gitlab_http_status(404)
+ end
+
+ it 'returns a 400 bad request error if neither body nor resolved parameter is given' do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", user)
+
+ expect(response).to have_gitlab_http_status(400)
+ end
+
+ it "returns a 403 error if user resolves note of someone else" do
+ put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+ "discussions/#{note.discussion_id}/notes/#{note.id}", private_user), resolved: true
+
+ expect(response).to have_gitlab_http_status(403)
+ end
+ end
+end