Commit ff6d63d6 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'ce-to-ee-2018-03-27' into 'master'

CE upstream - 2018-03-27 08:13 UTC

See merge request gitlab-org/gitlab-ee!5121
parents 1aec54db f165091c
......@@ -49,7 +49,7 @@ gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos
gem 'omniauth-oauth2-generic', '~> 0.2.2'
gem 'omniauth-saml', '~> 1.10'
gem 'omniauth-shibboleth', '~> 1.2.0'
gem 'omniauth-twitter', '~> 1.2.0'
gem 'omniauth-twitter', '~> 1.4'
gem 'omniauth_crowd', '~> 2.2.0'
gem 'omniauth-authentiq', '~> 0.3.1'
gem 'rack-oauth2', '~> 1.2.1'
......
......@@ -547,7 +547,7 @@ GEM
nokogiri (1.8.2)
mini_portile2 (~> 2.3.0)
numerizer (0.1.1)
oauth (0.5.1)
oauth (0.5.4)
oauth2 (1.4.0)
faraday (>= 0.8, < 0.13)
jwt (~> 1.0)
......@@ -602,9 +602,9 @@ GEM
ruby-saml (~> 1.7)
omniauth-shibboleth (1.2.1)
omniauth (>= 1.0.0)
omniauth-twitter (1.2.1)
json (~> 1.3)
omniauth-twitter (1.4.0)
omniauth-oauth (~> 1.1)
rack
omniauth_crowd (2.2.3)
activesupport
nokogiri (>= 1.4.4)
......@@ -1157,7 +1157,7 @@ DEPENDENCIES
omniauth-oauth2-generic (~> 0.2.2)
omniauth-saml (~> 1.10)
omniauth-shibboleth (~> 1.2.0)
omniauth-twitter (~> 1.2.0)
omniauth-twitter (~> 1.4)
omniauth_crowd (~> 2.2.0)
org-ruby (~> 0.9.12)
peek (~> 1.0.1)
......
......@@ -11,6 +11,14 @@ module Emails
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def push_to_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil, new_commits: [], existing_commits: [])
setup_merge_request_mail(merge_request_id, recipient_id)
@new_commits = new_commits
@existing_commits = existing_commits
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
......
......@@ -175,7 +175,7 @@ class Commit
if safe_message.blank?
no_commit_message
else
safe_message.split("\n", 2).first
safe_message.split(/[\r\n]/, 2).first
end
end
......
......@@ -48,7 +48,7 @@ class NotificationRecipient
when :custom
custom_enabled? || %i[participating mention].include?(@type)
when :watch, :participating
!excluded_watcher_action?
!action_excluded?
when :mention
@type == :mention
else
......@@ -96,13 +96,22 @@ class NotificationRecipient
end
end
def action_excluded?
excluded_watcher_action? || excluded_participating_action?
end
def excluded_watcher_action?
return false unless @custom_action
return false if notification_level == :custom
return false unless @custom_action && notification_level == :watch
NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
end
def excluded_participating_action?
return false unless @custom_action && notification_level == :participating
NotificationSetting::EXCLUDED_PARTICIPATING_EVENTS.include?(@custom_action)
end
private
def read_ability
......
......@@ -33,6 +33,7 @@ class NotificationSetting < ActiveRecord::Base
:close_issue,
:reassign_issue,
:new_merge_request,
:push_to_merge_request,
:reopen_merge_request,
:close_merge_request,
:reassign_merge_request,
......@@ -41,10 +42,14 @@ class NotificationSetting < ActiveRecord::Base
:success_pipeline
].freeze
EXCLUDED_WATCHER_EVENTS = [
EXCLUDED_PARTICIPATING_EVENTS = [
:success_pipeline
].freeze
EXCLUDED_WATCHER_EVENTS = [
:push_to_merge_request
].push(*EXCLUDED_PARTICIPATING_EVENTS).freeze
def self.find_or_create_for(source)
setting = find_or_initialize_by(source: source)
......
......@@ -641,9 +641,7 @@ class User < ActiveRecord::Base
end
def owned_projects
@owned_projects ||=
Project.where('namespace_id IN (?) OR namespace_id = ?',
owned_groups.select(:id), namespace.id).joins(:namespace)
@owned_projects ||= Project.from("(#{owned_projects_union.to_sql}) AS projects")
end
# Returns projects which user can admin issues on (for example to move an issue to that project).
......@@ -1218,6 +1216,15 @@ class User < ActiveRecord::Base
private
def owned_projects_union
Gitlab::SQL::Union.new([
Project.where(namespace: namespace),
Project.joins(:project_authorizations)
.where("projects.namespace_id <> ?", namespace.id)
.where(project_authorizations: { user_id: id, access_level: Gitlab::Access::OWNER })
], remove_duplicates: false)
end
def ci_projects_union
scope = { access_level: [Gitlab::Access::MASTER, Gitlab::Access::OWNER] }
groups = groups_projects.where(members: scope)
......
......@@ -21,7 +21,7 @@ module MergeRequests
comment_mr_branch_presence_changed
end
comment_mr_with_commits
notify_about_push
mark_mr_as_wip_from_commits
execute_mr_web_hooks
reset_approvals_for_merge_requests
......@@ -158,8 +158,8 @@ module MergeRequests
end
end
# Add comment about pushing new commits to merge requests
def comment_mr_with_commits
# Add comment about pushing new commits to merge requests and send nofitication emails
def notify_about_push
return unless @commits.present?
merge_requests_for_source_branch.each do |merge_request|
......@@ -172,6 +172,8 @@ module MergeRequests
SystemNoteService.add_commits(merge_request, merge_request.project,
@current_user, new_commits,
existing_commits, @oldrev)
notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
end
end
......
......@@ -116,6 +116,16 @@ class NotificationService
new_resource_email(merge_request, :new_merge_request_email)
end
def push_to_merge_request(merge_request, current_user, new_commits: [], existing_commits: [])
new_commits = new_commits.map { |c| { short_id: c.short_id, title: c.title } }
existing_commits = existing_commits.map { |c| { short_id: c.short_id, title: c.title } }
recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: "push_to")
recipients.each do |recipient|
mailer.send(:push_to_merge_request_email, recipient.user.id, merge_request.id, current_user.id, recipient.reason, new_commits: new_commits, existing_commits: existing_commits).deliver_later
end
end
# When merge request text is updated, we should send an email to:
#
# * newly mentioned project team members with notification level higher than Participating
......
......@@ -3,6 +3,7 @@
- if License.feature_available?(:repository_mirrors)
= render partial: 'repository_mirrors_form', locals: { f: f }
%fieldset
%legend Continuous Integration and Deployment
.form-group
......
......@@ -42,10 +42,6 @@
= link_to "(?)", help_page_path("integration/bitbucket")
and GitLab.com
= link_to "(?)", help_page_path("integration/gitlab")
.form-group
= f.label :default_branch_protection, class: 'control-label col-sm-2'
.col-sm-10
= f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, @application_setting.default_branch_protection), {}, class: 'form-control'
-# EE-only
- if ldap_enabled?
......
%h3
New commits were pushed to the merge request
= link_to(@merge_request.to_reference, project_merge_request_url(@merge_request.target_project, @merge_request))
by #{@current_user.name}
- if @existing_commits.any?
- count = @existing_commits.size
%ul
%li
- if count.one?
- commit_id = @existing_commits.first[:short_id]
= link_to(commit_id, project_commit_url(@merge_request.target_project, commit_id))
- else
= link_to(project_compare_url(@merge_request.target_project, from: @existing_commits.first[:short_id], to: @existing_commits.last[:short_id])) do
#{@existing_commits.first[:short_id]}...#{@existing_commits.last[:short_id]}
= precede '&nbsp;- ' do
- commits_text = "#{count} commit".pluralize(count)
#{commits_text} from branch `#{@merge_request.target_branch}`
- if @new_commits.any?
%ul
- @new_commits.each do |commit|
%li
= link_to(commit[:short_id], project_commit_url(@merge_request.target_project, commit[:short_id]))
= precede ' - ' do
#{commit[:title]}
New commits were pushed to the merge request #{@merge_request.to_reference} by #{@current_user.name}
\
#{url_for(project_merge_request_url(@merge_request.target_project, @merge_request))}
\
- if @existing_commits.any?
- count = @existing_commits.size
- commits_id = count.one? ? @existing_commits.first[:short_id] : "#{@existing_commits.first[:short_id]}...#{@existing_commits.last[:short_id]}"
- commits_text = "#{count} commit".pluralize(count)
* #{commits_id} - #{commits_text} from branch `#{@merge_request.target_branch}`
\
- @new_commits.each do |commit|
* #{commit[:short_id]} - #{raw commit[:title]}
---
title: Send notification emails when push to a merge request
merge_request: 7610
author: YarNayar
type: feature
---
title: Improves the performance of projects list page
merge_request: 17934
author:
type: performance
---
title: 'Cloning a repository over HTTPS with LDAP credentials causes a HTTP 401 Access denied'
merge_request: !17988
author: Horatiu Eugen Vlad
type: fixed
class AddPushToMergeRequestToNotificationSettings < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :notification_settings, :push_to_merge_request, :boolean
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20180320182229) do
ActiveRecord::Schema.define(version: 20180323150945) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -1708,6 +1708,7 @@ ActiveRecord::Schema.define(version: 20180320182229) do
t.boolean "merge_merge_request"
t.boolean "failed_pipeline"
t.boolean "success_pipeline"
t.boolean "push_to_merge_request"
end
add_index "notification_settings", ["source_id", "source_type"], name: "index_notification_settings_on_source_id_and_source_type", using: :btree
......
......@@ -24,6 +24,7 @@ reopen_issue
close_issue
reassign_issue
new_merge_request
push_to_merge_request
reopen_merge_request
close_merge_request
reassign_merge_request
......@@ -75,6 +76,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab
| `close_issue` | boolean | no | Enable/disable this notification |
| `reassign_issue` | boolean | no | Enable/disable this notification |
| `new_merge_request` | boolean | no | Enable/disable this notification |
| `push_to_merge_request` | boolean | no | Enable/disable this notification |
| `reopen_merge_request` | boolean | no | Enable/disable this notification |
| `close_merge_request` | boolean | no | Enable/disable this notification |
| `reassign_merge_request` | boolean | no | Enable/disable this notification |
......@@ -141,6 +143,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab
| `close_issue` | boolean | no | Enable/disable this notification |
| `reassign_issue` | boolean | no | Enable/disable this notification |
| `new_merge_request` | boolean | no | Enable/disable this notification |
| `push_to_merge_request` | boolean | no | Enable/disable this notification |
| `reopen_merge_request` | boolean | no | Enable/disable this notification |
| `close_merge_request` | boolean | no | Enable/disable this notification |
| `reassign_merge_request` | boolean | no | Enable/disable this notification |
......@@ -164,6 +167,7 @@ Example responses:
"close_issue": false,
"reassign_issue": false,
"new_merge_request": false,
"push_to_merge_request": false,
"reopen_merge_request": false,
"close_merge_request": false,
"reassign_merge_request": false,
......
......@@ -67,7 +67,7 @@ Below is the table of events users can be notified of:
### Issue / Merge request events
In all of the below cases, the notification will be sent to:
In most of the below cases, the notification will be sent to:
- Participants:
- the author and assignee of the issue/merge request
- authors of comments on the issue/merge request
......@@ -87,6 +87,7 @@ In all of the below cases, the notification will be sent to:
| Reassign issue | The above, plus the old assignee |
| Reopen issue | |
| New merge request | |
| Push to merge request | Participants and Custom notification level with this event selected |
| Reassign merge request | The above, plus the old assignee |
| Close merge request | |
| Reopen merge request | |
......
......@@ -71,7 +71,11 @@ module Gitlab
authenticators.compact!
user if authenticators.find { |auth| auth.login(login, password) }
# return found user that was authenticated first for given login credentials
authenticators.find do |auth|
authenticated_user = auth.login(login, password)
break authenticated_user if authenticated_user
end
end
end
......
......@@ -8,7 +8,7 @@ module Gitlab
def login(login, password)
return false unless Gitlab::CurrentSettings.password_authentication_enabled_for_git?
user&.valid_password?(password)
return user if user&.valid_password?(password)
end
end
end
......
......@@ -12,30 +12,26 @@ module Gitlab
return unless Gitlab::Auth::LDAP::Config.enabled?
return unless login.present? && password.present?
auth = nil
# loop through providers until valid bind
# return found user that was authenticated by first provider for given login credentials
providers.find do |provider|
auth = new(provider)
auth.login(login, password) # true will exit the loop
break auth.user if auth.login(login, password) # true will exit the loop
end
# If (login, password) was invalid for all providers, the value of auth is now the last
# Gitlab::Auth::LDAP::Authentication instance we tried.
auth.user
end
def self.providers
Gitlab::Auth::LDAP::Config.providers
end
attr_accessor :ldap_user
def login(login, password)
@ldap_user = adapter.bind_as(
result = adapter.bind_as(
filter: user_filter(login),
size: 1,
password: password
)
return unless result
@user = Gitlab::Auth::LDAP::User.find_by_uid_and_provider(result.dn, provider)
end
def adapter
......@@ -56,12 +52,6 @@ module Gitlab
filter
end
def user
return unless ldap_user
Gitlab::Auth::LDAP::User.find_by_uid_and_provider(ldap_user.dn, provider)
end
end
end
end
......
......@@ -12,6 +12,7 @@ module Gitlab
@user = user
end
# Implementation must return user object if login successful
def login(login, password)
raise NotImplementedError
end
......
......@@ -3,7 +3,7 @@
cd "$(dirname "$0")/.."
# Use long options (e.g. --header instead of -H) for curl examples in documentation.
echo 'Checking for curl short options...'
echo '=> Checking for cURL short options...'
grep --extended-regexp --recursive --color=auto 'curl (.+ )?-[^- ].*' doc/ >/dev/null 2>&1
if [ $? == 0 ]
then
......@@ -15,7 +15,7 @@ fi
# Ensure that the CHANGELOG.md does not contain duplicate versions
DUPLICATE_CHANGELOG_VERSIONS=$(grep --extended-regexp '^## .+' CHANGELOG.md | sed -E 's| \(.+\)||' | sort -r | uniq -d)
echo 'Checking for CHANGELOG.md duplicate entries...'
echo '=> Checking for CHANGELOG.md duplicate entries...'
if [ "${DUPLICATE_CHANGELOG_VERSIONS}" != "" ]
then
echo '✖ ERROR: Duplicate versions in CHANGELOG.md:' >&2
......@@ -25,7 +25,7 @@ fi
# Make sure no files in doc/ are executable
EXEC_PERM_COUNT=$(find doc/ app/ -type f -perm 755 | wc -l)
echo 'Checking for executable permissions...'
echo '=> Checking for executable permissions...'
if [ "${EXEC_PERM_COUNT}" -ne 0 ]
then
echo '✖ ERROR: Executable permissions should not be used in documentation! Use `chmod 644` to the files in question:' >&2
......@@ -33,5 +33,33 @@ then
exit 1
fi
# Do not use 'README.md', instead use 'index.md'
# Number of 'README.md's as of 2018-03-26
NUMBER_READMES_CE=42
NUMBER_READMES_EE=46
FIND_READMES=$(find doc/ -name "README.md" | wc -l)
echo '=> Checking for new README.md files...'
if [ "${CI_PROJECT_NAME}" == 'gitlab-ce' ]
then
if [ ${FIND_READMES} -ne ${NUMBER_READMES_CE} ]
then
echo
echo ' ✖ ERROR: New README.md file(s) detected, prefer index.md over README.md.' >&2
echo ' https://docs.gitlab.com/ee/development/writing_documentation.html#location-and-naming-documents'
echo
exit 1
fi
elif [ "${CI_PROJECT_NAME}" == 'gitlab-ee' ]
then
if [ ${FIND_READMES} -ne $NUMBER_READMES_EE ]
then
echo
echo ' ✖ ERROR: New README.md file(s) detected, prefer index.md over README.md.' >&2
echo ' https://docs.gitlab.com/ee/development/writing_documentation.html#location-and-naming-documents'
echo
exit 1
fi
fi
echo "✔ Linting passed"
exit 0
......@@ -315,13 +315,19 @@ describe Gitlab::Auth do
it "tries to autheticate with db before ldap" do
expect(Gitlab::Auth::LDAP::Authentication).not_to receive(:login)
gl_auth.find_with_user_password(username, password)
expect(gl_auth.find_with_user_password(username, password)).to eq(user)
end
it "does not find user by using ldap as fallback to for authentication" do
expect(Gitlab::Auth::LDAP::Authentication).to receive(:login).and_return(nil)
expect(gl_auth.find_with_user_password('ldap_user', 'password')).to be_nil
end
it "uses ldap as fallback to for authentication" do
expect(Gitlab::Auth::LDAP::Authentication).to receive(:login)
it "find new user by using ldap as fallback to for authentication" do
expect(Gitlab::Auth::LDAP::Authentication).to receive(:login).and_return(user)
gl_auth.find_with_user_password('ldap_user', 'password')
expect(gl_auth.find_with_user_password('ldap_user', 'password')).to eq(user)
end
end
......
......@@ -24,6 +24,14 @@ describe MergeRequests::RefreshService do
merge_when_pipeline_succeeds: true,
merge_user: @user)
@another_merge_request = create(:merge_request,
source_project: @project,
source_branch: 'master',
target_branch: 'test',
target_project: @project,
merge_when_pipeline_succeeds: true,
merge_user: @user)
@fork_merge_request = create(:merge_request,
source_project: @fork_project,
source_branch: 'master',
......@@ -55,9 +63,11 @@ describe MergeRequests::RefreshService do
context 'push to origin repo source branch' do
let(:refresh_service) { service.new(@project, @user) }
let(:notification_service) { spy('notification_service') }
before do
allow(refresh_service).to receive(:execute_hooks)
allow(NotificationService).to receive(:new) { notification_service }
end
it 'executes hooks with update action' do
......@@ -67,6 +77,11 @@ describe MergeRequests::RefreshService do
expect(refresh_service).to have_received(:execute_hooks)
.with(@merge_request, 'update', old_rev: @oldrev)
expect(notification_service).to have_received(:push_to_merge_request)
.with(@merge_request, @user, new_commits: anything, existing_commits: anything)
expect(notification_service).to have_received(:push_to_merge_request)
.with(@another_merge_request, @user, new_commits: anything, existing_commits: anything)
expect(@merge_request.notes).not_to be_empty
expect(@merge_request).to be_open
expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey
......@@ -125,11 +140,13 @@ describe MergeRequests::RefreshService do
context 'push to origin repo source branch when an MR was reopened' do
let(:refresh_service) { service.new(@project, @user) }
let(:notification_service) { spy('notification_service') }
before do
@merge_request.update(state: :opened)
allow(refresh_service).to receive(:execute_hooks)
allow(NotificationService).to receive(:new) { notification_service }
refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
reload_mrs
end
......@@ -137,6 +154,10 @@ describe MergeRequests::RefreshService do
it 'executes hooks with update action' do
expect(refresh_service).to have_received(:execute_hooks)
.with(@merge_request, 'update', old_rev: @oldrev)
expect(notification_service).to have_received(:push_to_merge_request)
.with(@merge_request, @user, new_commits: anything, existing_commits: anything)
expect(notification_service).to have_received(:push_to_merge_request)
.with(@another_merge_request, @user, new_commits: anything, existing_commits: anything)
expect(@merge_request.notes).not_to be_empty
expect(@merge_request).to be_open
......
......@@ -1132,6 +1132,36 @@ describe NotificationService, :mailer do
end
end
describe '#push_to_merge_request' do
before do
update_custom_notification(:push_to_merge_request, @u_guest_custom, resource: project)
update_custom_notification(:push_to_merge_request, @u_custom_global)
end
it do
notification.push_to_merge_request(merge_request, @u_disabled)
should_email(merge_request.assignee)
should_email(@u_guest_custom)
should_email(@u_custom_global)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_not_email(@u_watcher)
should_not_email(@u_guest_watcher)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
should_not_email(@u_lazy_participant)
end
it_behaves_like 'participating notifications' do
let(:participant) { create(:user, username: 'user-participant') }
let(:issuable) { merge_request }
let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) }
end
end
describe '#relabel_merge_request' do
let(:group_label_1) { create(:group_label, group: group, title: 'Group Label 1', merge_requests: [merge_request]) }
let(:group_label_2) { create(:group_label, group: group, title: 'Group Label 2') }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment