From 2fd009d23810939a77ddc1716ab4250ee18826fa Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Wed, 8 Jun 2016 16:56:44 +0200
Subject: [PATCH 01/23] Retry spinach tests in case of failure using rerun
 reporter

---
 .gitlab-ci.yml | 4 +---
 CHANGELOG      | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 5ef3081395..965de5e50e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -89,9 +89,7 @@ update-knapsack:
     - export KNAPSACK_REPORT_PATH=knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
     - export KNAPSACK_GENERATE_REPORT=true
     - cp knapsack/spinach_report.json ${KNAPSACK_REPORT_PATH}
-    - knapsack spinach "-r rerun"
-    # retry failed tests 3 times
-    - retry '[ ! -e tmp/spinach-rerun.txt ] || bin/spinach -r rerun $(cat tmp/spinach-rerun.txt)'
+    - knapsack spinach "-r rerun" || retry '[ ! -e tmp/spinach-rerun.txt ] || bin/spinach -r rerun $(cat tmp/spinach-rerun.txt)'
   artifacts:
     paths:
     - knapsack/
diff --git a/CHANGELOG b/CHANGELOG
index 0506854599..874b942f46 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 8.9.0 (unreleased)
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects
   - Redesign navigation for project pages
   - Fix groups API to list only user's accessible projects
+  - Retry spinach tests in case of failure using rerun reporter
   - Redesign account and email confirmation emails
   - Bump nokogiri to 1.6.8
   - Use gitlab-shell v3.0.0

From 0e896ffe4eebb8bcf04bc1327d498bb041faed56 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Fri, 10 Jun 2016 14:51:16 +0200
Subject: [PATCH 02/23] Improve Gitlab::Auth method names

Auth.find was a very generic name for a very specific method.
Auth.find_in_gitlab_or_ldap was inaccurate in GitLab EE where it also
looks in Kerberos.
---
 app/controllers/jwt_controller.rb               |  2 +-
 app/controllers/projects/git_http_controller.rb |  2 +-
 config/initializers/doorkeeper.rb               |  2 +-
 lib/api/session.rb                              |  2 +-
 lib/gitlab/auth.rb                              |  6 +++---
 lib/gitlab/backend/grack_auth.rb                |  2 +-
 spec/lib/gitlab/auth_spec.rb                    | 16 ++++++++--------
 spec/requests/jwt_controller_spec.rb            |  2 +-
 8 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 131a16dad9..014b9b43ff 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -42,7 +42,7 @@ class JwtController < ApplicationController
   end
 
   def authenticate_user(login, password)
-    user = Gitlab::Auth.find_in_gitlab_or_ldap(login, password)
+    user = Gitlab::Auth.find_with_user_password(login, password)
     Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login)
     user
   end
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 348d6cf4d9..f907d63258 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -43,7 +43,7 @@ class Projects::GitHttpController < Projects::ApplicationController
     return if project && project.public? && upload_pack?
 
     authenticate_or_request_with_http_basic do |login, password|
-      auth_result = Gitlab::Auth.find(login, password, project: project, ip: request.ip)
+      auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
       if auth_result.type == :ci && upload_pack?
         @ci = true
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 8dc8e270af..618dba7415 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -12,7 +12,7 @@ Doorkeeper.configure do
   end
 
   resource_owner_from_credentials do |routes|
-    Gitlab::Auth.find_in_gitlab_or_ldap(params[:username], params[:password])
+    Gitlab::Auth.find_with_user_password(params[:username], params[:password])
   end
 
   # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
diff --git a/lib/api/session.rb b/lib/api/session.rb
index 56e69b2366..56c202f129 100644
--- a/lib/api/session.rb
+++ b/lib/api/session.rb
@@ -11,7 +11,7 @@ module API
     # Example Request:
     #  POST /session
     post "/session" do
-      user = Gitlab::Auth.find_in_gitlab_or_ldap(params[:email] || params[:login], params[:password])
+      user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password])
 
       return unauthorized! unless user
       present user, with: Entities::UserLogin
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 076e2af7d3..db1704af75 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -3,14 +3,14 @@ module Gitlab
     Result = Struct.new(:user, :type)
 
     class << self
-      def find(login, password, project:, ip:)
+      def find_for_git_client(login, password, project:, ip:)
         raise "Must provide an IP for rate limiting" if ip.nil?
 
         result = Result.new
 
         if valid_ci_request?(login, password, project)
           result.type = :ci
-        elsif result.user = find_in_gitlab_or_ldap(login, password)
+        elsif result.user = find_with_user_password(login, password)
           result.type = :gitlab_or_ldap
         elsif result.user = oauth_access_token_check(login, password)
           result.type = :oauth
@@ -20,7 +20,7 @@ module Gitlab
         result
       end
 
-      def find_in_gitlab_or_ldap(login, password)
+      def find_with_user_password(login, password)
         user = User.by_login(login)
 
         # If no user is found, or it's an LDAP server, try LDAP.
diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb
index 9e09d2e118..adbf5941a9 100644
--- a/lib/gitlab/backend/grack_auth.rb
+++ b/lib/gitlab/backend/grack_auth.rb
@@ -95,7 +95,7 @@ module Grack
     end
 
     def authenticate_user(login, password)
-      user = Gitlab::Auth.find_in_gitlab_or_ldap(login, password)
+      user = Gitlab::Auth.find_with_user_password(login, password)
 
       unless user
         user = oauth_access_token_check(login, password)
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index a814ad2a4e..f081d550ec 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -41,7 +41,7 @@ describe Gitlab::Auth, lib: true do
     end
   end
 
-  describe 'find_in_gitlab_or_ldap' do
+  describe 'find_with_user_password' do
     let!(:user) do
       create(:user,
         username: username,
@@ -52,25 +52,25 @@ describe Gitlab::Auth, lib: true do
     let(:password) { 'my-secret' }
 
     it "should find user by valid login/password" do
-      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).to eql user
+      expect( gl_auth.find_with_user_password(username, password) ).to eql user
     end
 
     it 'should find user by valid email/password with case-insensitive email' do
-      expect(gl_auth.find_in_gitlab_or_ldap(user.email.upcase, password)).to eql user
+      expect(gl_auth.find_with_user_password(user.email.upcase, password)).to eql user
     end
 
     it 'should find user by valid username/password with case-insensitive username' do
-      expect(gl_auth.find_in_gitlab_or_ldap(username.upcase, password)).to eql user
+      expect(gl_auth.find_with_user_password(username.upcase, password)).to eql user
     end
 
     it "should not find user with invalid password" do
       password = 'wrong'
-      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user
+      expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
     end
 
     it "should not find user with invalid login" do
       user = 'wrong'
-      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user
+      expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
     end
 
     context "with ldap enabled" do
@@ -81,13 +81,13 @@ describe Gitlab::Auth, lib: true do
       it "tries to autheticate with db before ldap" do
         expect(Gitlab::LDAP::Authentication).not_to receive(:login)
 
-        gl_auth.find_in_gitlab_or_ldap(username, password)
+        gl_auth.find_with_user_password(username, password)
       end
 
       it "uses ldap as fallback to for authentication" do
         expect(Gitlab::LDAP::Authentication).to receive(:login)
 
-        gl_auth.find_in_gitlab_or_ldap('ldap_user', 'password')
+        gl_auth.find_with_user_password('ldap_user', 'password')
       end
     end
   end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index c995993a85..d2d4a9eca1 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -44,7 +44,7 @@ describe JwtController do
       let(:user) { create(:user) }
       let(:headers) { { authorization: credentials('user', 'password') } }
 
-      before { expect(Gitlab::Auth).to receive(:find_in_gitlab_or_ldap).with('user', 'password').and_return(user) }
+      before { expect(Gitlab::Auth).to receive(:find_with_user_password).with('user', 'password').and_return(user) }
 
       subject! { get '/jwt/auth', parameters, headers }
 

From 16927e3fadfb5b9b2c3b12ffdf7ab217971b8cca Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 30 May 2016 11:05:17 +0200
Subject: [PATCH 03/23] Enable Style/MultilineBlockChain rubocop style cop

See #17478
---
 .rubocop.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 678f7db025..72db034254 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -348,7 +348,7 @@ Style/MultilineArrayBraceLayout:
 
 # Avoid multi-line chains of blocks.
 Style/MultilineBlockChain:
-  Enabled: false
+  Enabled: true
 
 # Ensures newlines after multiline block do statements.
 Style/MultilineBlockLayout:

From dc7734f4bf919e67323eafdaa1c875a84433fdf9 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 10 Jun 2016 22:08:06 +0200
Subject: [PATCH 04/23] Use bundle exec to run spinach

---
 .gitlab-ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index aa84a9e4da..b558515847 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -91,7 +91,7 @@ update-knapsack:
     - export KNAPSACK_REPORT_PATH=knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
     - export KNAPSACK_GENERATE_REPORT=true
     - cp knapsack/spinach_report.json ${KNAPSACK_REPORT_PATH}
-    - knapsack spinach "-r rerun" || retry '[ ! -e tmp/spinach-rerun.txt ] || bin/spinach -r rerun $(cat tmp/spinach-rerun.txt)'
+    - knapsack spinach "-r rerun" || retry '[ ! -e tmp/spinach-rerun.txt ] || bundle exec spinach -r rerun $(cat tmp/spinach-rerun.txt)'
   artifacts:
     paths:
     - knapsack/

From f395ed242add9c21536a6e4d9ea7f0154351e503 Mon Sep 17 00:00:00 2001
From: Connor Shea <connor.james.shea@gmail.com>
Date: Sun, 12 Jun 2016 18:25:21 -0600
Subject: [PATCH 05/23] Update brakeman from 3.2.1 to 3.3.2

Removes a few dependencies.

Changelog: https://github.com/presidentbeef/brakeman/blob/master/CHANGES
---
 Gemfile      |  2 +-
 Gemfile.lock | 28 +++++-----------------------
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/Gemfile b/Gemfile
index b2660144f2..6d8a33c2ee 100644
--- a/Gemfile
+++ b/Gemfile
@@ -245,7 +245,7 @@ end
 
 group :development do
   gem "foreman"
-  gem 'brakeman', '~> 3.2.0', require: false
+  gem 'brakeman', '~> 3.3.0', require: false
 
   gem 'letter_opener_web', '~> 1.3.0'
   gem 'quiet_assets', '~> 1.0.2'
diff --git a/Gemfile.lock b/Gemfile.lock
index dfc1570049..2ba2676efa 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -97,16 +97,7 @@ GEM
     bootstrap-sass (3.3.6)
       autoprefixer-rails (>= 5.2.1)
       sass (>= 3.3.4)
-    brakeman (3.2.1)
-      erubis (~> 2.6)
-      haml (>= 3.0, < 5.0)
-      highline (>= 1.6.20, < 2.0)
-      ruby2ruby (~> 2.3.0)
-      ruby_parser (~> 3.8.1)
-      safe_yaml (>= 1.0)
-      sass (~> 3.0)
-      slim (>= 1.3.6, < 4.0)
-      terminal-table (~> 1.4)
+    brakeman (3.3.2)
     browser (2.0.3)
     builder (3.2.2)
     bullet (5.0.0)
@@ -338,7 +329,6 @@ GEM
     hashie (3.4.3)
     health_check (1.5.1)
       rails (>= 2.3.0)
-    highline (1.7.8)
     hipchat (1.5.2)
       httparty
       mimemagic
@@ -642,10 +632,7 @@ GEM
     ruby-saml (1.1.2)
       nokogiri (>= 1.5.10)
       uuid (~> 2.3)
-    ruby2ruby (2.3.0)
-      ruby_parser (~> 3.1)
-      sexp_processor (~> 4.0)
-    ruby_parser (3.8.1)
+    ruby_parser (3.8.2)
       sexp_processor (~> 4.1)
     rubyntlm (0.5.2)
     rubypants (0.2.0)
@@ -655,7 +642,7 @@ GEM
     safe_yaml (1.0.4)
     sanitize (2.1.0)
       nokogiri (>= 1.4.4)
-    sass (3.4.21)
+    sass (3.4.22)
     sass-rails (5.0.4)
       railties (>= 4.0.0, < 5.0)
       sass (~> 3.1)
@@ -704,9 +691,6 @@ GEM
       tilt (>= 1.3, < 3)
     six (0.2.0)
     slack-notifier (1.2.1)
-    slim (3.0.6)
-      temple (~> 0.7.3)
-      tilt (>= 1.3.3, < 2.1)
     slop (3.6.0)
     spinach (0.8.10)
       colorize
@@ -747,10 +731,8 @@ GEM
       railties (>= 3.2.5, < 6)
     teaspoon-jasmine (2.2.0)
       teaspoon (>= 1.0.0)
-    temple (0.7.6)
     term-ansicolor (1.3.2)
       tins (~> 1.0)
-    terminal-table (1.5.2)
     test_after_commit (0.4.2)
       activerecord (>= 3.2)
     thin (1.6.4)
@@ -759,7 +741,7 @@ GEM
       rack (~> 1.0)
     thor (0.19.1)
     thread_safe (0.3.5)
-    tilt (2.0.2)
+    tilt (2.0.5)
     timecop (0.8.1)
     timfel-krb5-auth (0.8.3)
     tinder (1.10.1)
@@ -848,7 +830,7 @@ DEPENDENCIES
   better_errors (~> 1.0.1)
   binding_of_caller (~> 0.7.2)
   bootstrap-sass (~> 3.3.0)
-  brakeman (~> 3.2.0)
+  brakeman (~> 3.3.0)
   browser (~> 2.0.3)
   bullet
   bundler-audit

From 0e50fa24a5b4ff7665a68da2a1221b5cae9e5633 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 13 Jun 2016 12:32:53 +0300
Subject: [PATCH 06/23] Remove counters from Pipeline navigation

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 app/views/projects/pipelines/_head.html.haml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml
index f278d4e053..d0ba0d27d7 100644
--- a/app/views/projects/pipelines/_head.html.haml
+++ b/app/views/projects/pipelines/_head.html.haml
@@ -5,11 +5,9 @@
         = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do
           %span
             Pipelines
-            %span.badge.count.ci_counter= number_with_delimiter(@project.pipelines.running_or_pending.count)
 
     - if project_nav_tab? :builds
       = nav_link(controller: %w(builds)) do
         = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do
           %span
             Builds
-            %span.badge.count.builds_counter= number_with_delimiter(@project.running_or_pending_build_count)

From f73cf3e937b92d29753e468dac8a17470253c791 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <jacob@gitlab.com>
Date: Mon, 13 Jun 2016 15:38:25 +0200
Subject: [PATCH 07/23] Also rename "find" in the specs

---
 spec/lib/gitlab/auth_spec.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index f081d550ec..7bec136715 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::Auth, lib: true do
   let(:gl_auth) { described_class }
 
-  describe 'find' do
+  describe 'find_for_git_client' do
     it 'recognizes CI' do
       token = '123'
       project = create(:empty_project)
@@ -11,7 +11,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token')
-      expect(gl_auth.find('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci))
+      expect(gl_auth.find_for_git_client('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci))
     end
 
     it 'recognizes master passwords' do
@@ -19,7 +19,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
-      expect(gl_auth.find(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
+      expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
     end
 
     it 'recognizes OAuth tokens' do
@@ -29,7 +29,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2')
-      expect(gl_auth.find("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth))
+      expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth))
     end
 
     it 'returns double nil for invalid credentials' do
@@ -37,7 +37,7 @@ describe Gitlab::Auth, lib: true do
       ip = 'ip'
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
-      expect(gl_auth.find(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
+      expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
     end
   end
 

From f9bbc4d485d32a8233894a75eae4945a2807188b Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 13 Jun 2016 18:23:55 +0200
Subject: [PATCH 08/23] Revert CHANGELOG

---
 CHANGELOG | 1 -
 1 file changed, 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index f0b0bf4a2f..8f897b4a34 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -20,7 +20,6 @@ v 8.9.0 (unreleased)
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects
   - Redesign navigation for project pages
   - Fix groups API to list only user's accessible projects
-  - Retry spinach tests in case of failure using rerun reporter
   - Redesign account and email confirmation emails
   - `git clone https://host/namespace/project` now works, in addition to using the `.git` suffix
   - Bump nokogiri to 1.6.8

From af33338bbf752ab5f4dba5b981644b399ae3563b Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <amatyushentsev@gmail.com>
Date: Sun, 15 May 2016 00:22:01 -0700
Subject: [PATCH 09/23] Add more information into RSS fead for issues

---
 CHANGELOG                                   |  2 ++
 app/views/issues/_issue.atom.builder        | 20 +++++++++++-
 spec/features/atom/dashboard_issues_spec.rb | 35 +++++++++++++++------
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 7a6a14919d..80b19bbd04 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -123,6 +123,8 @@ v 8.8.1
 
 v 8.8.0
   - Implement GFM references for milestones (Alejandro Rodríguez)
+v 8.8.0 (unreleased)
+  - Add more information into RSS fead for issues.
   - Snippets tab under user profile. !4001 (Long Nguyen)
   - Fix error when using link to uploads in global snippets
   - Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref
diff --git a/app/views/issues/_issue.atom.builder b/app/views/issues/_issue.atom.builder
index 68a2d19e58..9683187414 100644
--- a/app/views/issues/_issue.atom.builder
+++ b/app/views/issues/_issue.atom.builder
@@ -5,10 +5,28 @@ xml.entry do
   xml.updated issue.created_at.xmlschema
   xml.media   :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(issue.author_email))
 
-  xml.author do |author|
+  xml.author do
     xml.name issue.author_name
     xml.email issue.author_email
   end
 
   xml.summary issue.title
+  xml.description issue.description if issue.description
+  xml.milestone issue.milestone.title if issue.milestone
+  xml.due_date issue.due_date if issue.due_date
+
+  unless issue.labels.empty?
+    xml.labels do
+      issue.labels.each do |label|
+        xml.label label.name
+      end
+    end
+  end
+
+  if issue.assignee
+    xml.assignee do
+      xml.name issue.assignee.name
+      xml.email issue.assignee.email
+    end
+  end
 end
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index b710cb3c72..87b478adb8 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -2,15 +2,18 @@ require 'spec_helper'
 
 describe "Dashboard Issues Feed", feature: true  do
   describe "GET /issues" do
-    let!(:user)     { create(:user) }
-    let!(:project1) { create(:project) }
-    let!(:project2) { create(:project) }
-    let!(:issue1)   { create(:issue, author: user, assignee: user, project: project1) }
-    let!(:issue2)   { create(:issue, author: user, assignee: user, project: project2) }
+    let!(:user)         { create(:user) }
+    let!(:project1)     { create(:project) }
+    let!(:project2)     { create(:project) }
+    let!(:milestone1)   { create(:milestone, project: project1, title: 'v1') }
+    let!(:label1)       { create(:label, project: project1, title: 'label1') }
+    let!(:issue1)       { create(:issue, author: user, assignee: user, project: project1, milestone: milestone1) }
+    let!(:issue2)       { create(:issue, author: user, assignee: user, project: project2, description: 'test desc') }
 
     before do
       project1.team << [user, :master]
       project2.team << [user, :master]
+      issue1.labels << label1
     end
 
     describe "atom feed" do
@@ -20,10 +23,24 @@ describe "Dashboard Issues Feed", feature: true  do
         expect(response_headers['Content-Type']).
           to have_content('application/atom+xml')
         expect(body).to have_selector('title', text: "#{user.name} issues")
-        expect(body).to have_selector('author email', text: issue1.author_email)
-        expect(body).to have_selector('entry summary', text: issue1.title)
-        expect(body).to have_selector('author email', text: issue2.author_email)
-        expect(body).to have_selector('entry summary', text: issue2.title)
+
+        entry_1 = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]")
+        expect(entry_1).to be_present
+
+        entry_2 = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]")
+        expect(entry_2).to be_present
+
+        expect(entry_1).to have_selector('author email', text: issue1.author_email)
+        expect(entry_1).to have_selector('assignee email', text: issue1.author_email)
+        expect(entry_1).to have_selector('labels label', text: label1.title)
+        expect(entry_1).to have_selector('milestone', text: milestone1.title)
+        expect(entry_1).not_to have_selector('description')
+
+        expect(entry_2).to have_selector('author email', text: issue2.author_email)
+        expect(entry_2).to have_selector('assignee email', text: issue2.author_email)
+        expect(entry_2).not_to have_selector('labels')
+        expect(entry_2).not_to have_selector('milestone')
+        expect(entry_2).to have_selector('description', text: issue1.description)
       end
     end
   end

From 5328930e3f7d013791417ba38805cfab9d029dbd Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <amatyushentsev@gmail.com>
Date: Sat, 11 Jun 2016 20:01:16 -0700
Subject: [PATCH 10/23] Move change description to proper release and fix typo

---
 CHANGELOG | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG b/CHANGELOG
index 80b19bbd04..1e611ef481 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.9.0 (unreleased)
   - Fix Error 500 when using closes_issues API with an external issue tracker
+  - Add more information into RSS feed for issues.
   - Bulk assign/unassign labels to issues.
   - Ability to prioritize labels !4009 / !3205 (Thijs Wouters)
   - Fix endless redirections when accessing user OAuth applications when they are disabled
@@ -124,7 +125,6 @@ v 8.8.1
 v 8.8.0
   - Implement GFM references for milestones (Alejandro Rodríguez)
 v 8.8.0 (unreleased)
-  - Add more information into RSS fead for issues.
   - Snippets tab under user profile. !4001 (Long Nguyen)
   - Fix error when using link to uploads in global snippets
   - Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref

From fcbb14f6b79ea4d97078ebae9df9a0fc4bba021e Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <amatyushentsev@gmail.com>
Date: Sat, 11 Jun 2016 20:03:39 -0700
Subject: [PATCH 11/23] Move issue rendering tests into separate contexts

---
 spec/features/atom/dashboard_issues_spec.rb | 58 +++++++++++++--------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index 87b478adb8..9a4eb8f950 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -5,42 +5,58 @@ describe "Dashboard Issues Feed", feature: true  do
     let!(:user)         { create(:user) }
     let!(:project1)     { create(:project) }
     let!(:project2)     { create(:project) }
-    let!(:milestone1)   { create(:milestone, project: project1, title: 'v1') }
-    let!(:label1)       { create(:label, project: project1, title: 'label1') }
-    let!(:issue1)       { create(:issue, author: user, assignee: user, project: project1, milestone: milestone1) }
-    let!(:issue2)       { create(:issue, author: user, assignee: user, project: project2, description: 'test desc') }
 
     before do
       project1.team << [user, :master]
       project2.team << [user, :master]
-      issue1.labels << label1
     end
 
     describe "atom feed" do
       it "should render atom feed via private token" do
         visit issues_dashboard_path(:atom, private_token: user.private_token)
 
-        expect(response_headers['Content-Type']).
-          to have_content('application/atom+xml')
+        expect(response_headers['Content-Type']).to have_content('application/atom+xml')
         expect(body).to have_selector('title', text: "#{user.name} issues")
+      end
 
-        entry_1 = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]")
-        expect(entry_1).to be_present
+      context "issue with basic fields" do
+        let!(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'test desc') }
 
-        entry_2 = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]")
-        expect(entry_2).to be_present
+        it "should render issue fields" do
+          visit issues_dashboard_path(:atom, private_token: user.private_token)
 
-        expect(entry_1).to have_selector('author email', text: issue1.author_email)
-        expect(entry_1).to have_selector('assignee email', text: issue1.author_email)
-        expect(entry_1).to have_selector('labels label', text: label1.title)
-        expect(entry_1).to have_selector('milestone', text: milestone1.title)
-        expect(entry_1).not_to have_selector('description')
+          entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]")
 
-        expect(entry_2).to have_selector('author email', text: issue2.author_email)
-        expect(entry_2).to have_selector('assignee email', text: issue2.author_email)
-        expect(entry_2).not_to have_selector('labels')
-        expect(entry_2).not_to have_selector('milestone')
-        expect(entry_2).to have_selector('description', text: issue1.description)
+          expect(entry).to be_present
+          expect(entry).to have_selector('author email', text: issue2.author_email)
+          expect(entry).to have_selector('assignee email', text: issue2.author_email)
+          expect(entry).not_to have_selector('labels')
+          expect(entry).not_to have_selector('milestone')
+          expect(entry).to have_selector('description', text: issue2.description)
+        end
+      end
+
+      context "issue with label and milestone" do
+        let!(:milestone1) { create(:milestone, project: project1, title: 'v1') }
+        let!(:label1)     { create(:label, project: project1, title: 'label1') }
+        let!(:issue1)     { create(:issue, author: user, assignee: user, project: project1, milestone: milestone1) }
+
+        before do
+          issue1.labels << label1
+        end
+
+        it "should render issue label and milestone info" do
+          visit issues_dashboard_path(:atom, private_token: user.private_token)
+
+          entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]")
+
+          expect(entry).to be_present
+          expect(entry).to have_selector('author email', text: issue1.author_email)
+          expect(entry).to have_selector('assignee email', text: issue1.author_email)
+          expect(entry).to have_selector('labels label', text: label1.title)
+          expect(entry).to have_selector('milestone', text: milestone1.title)
+          expect(entry).not_to have_selector('description')
+        end
       end
     end
   end

From e8bf8ec40725a0af21677211f2d73d9b516c184a Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <amatyushentsev@gmail.com>
Date: Mon, 13 Jun 2016 09:51:55 -0700
Subject: [PATCH 12/23] Apply reviewer notes: update CHANGELOG, adjust code
 formatting

---
 CHANGELOG                                   |  3 +--
 spec/features/atom/dashboard_issues_spec.rb | 12 ++++++------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 1e611ef481..01c7f3c2ef 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,7 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.9.0 (unreleased)
   - Fix Error 500 when using closes_issues API with an external issue tracker
-  - Add more information into RSS feed for issues.
+  - Add more information into RSS feed for issues (Alexander Matyushentsev)
   - Bulk assign/unassign labels to issues.
   - Ability to prioritize labels !4009 / !3205 (Thijs Wouters)
   - Fix endless redirections when accessing user OAuth applications when they are disabled
@@ -124,7 +124,6 @@ v 8.8.1
 
 v 8.8.0
   - Implement GFM references for milestones (Alejandro Rodríguez)
-v 8.8.0 (unreleased)
   - Snippets tab under user profile. !4001 (Long Nguyen)
   - Fix error when using link to uploads in global snippets
   - Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index 9a4eb8f950..4dd9548cfc 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -2,9 +2,9 @@ require 'spec_helper'
 
 describe "Dashboard Issues Feed", feature: true  do
   describe "GET /issues" do
-    let!(:user)         { create(:user) }
-    let!(:project1)     { create(:project) }
-    let!(:project2)     { create(:project) }
+    let!(:user)     { create(:user) }
+    let!(:project1) { create(:project) }
+    let!(:project2) { create(:project) }
 
     before do
       project1.team << [user, :master]
@@ -12,7 +12,7 @@ describe "Dashboard Issues Feed", feature: true  do
     end
 
     describe "atom feed" do
-      it "should render atom feed via private token" do
+      it "renders atom feed via private token" do
         visit issues_dashboard_path(:atom, private_token: user.private_token)
 
         expect(response_headers['Content-Type']).to have_content('application/atom+xml')
@@ -22,7 +22,7 @@ describe "Dashboard Issues Feed", feature: true  do
       context "issue with basic fields" do
         let!(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'test desc') }
 
-        it "should render issue fields" do
+        it "renders issue fields" do
           visit issues_dashboard_path(:atom, private_token: user.private_token)
 
           entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]")
@@ -45,7 +45,7 @@ describe "Dashboard Issues Feed", feature: true  do
           issue1.labels << label1
         end
 
-        it "should render issue label and milestone info" do
+        it "renders issue label and milestone info" do
           visit issues_dashboard_path(:atom, private_token: user.private_token)
 
           entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]")

From 747989c115c86b090612f805170beb175fe1672a Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 13 Jun 2016 17:40:36 -0300
Subject: [PATCH 13/23] =?UTF-8?q?Schema=20doesn=E2=80=99t=20reflect=20the?=
 =?UTF-8?q?=20changes=20of=20the=20last=203=20migrations?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The schema doesn’t reflect the changes of the last 3 migrations:

* 20160610140403_remove_notification_setting_not_null_constraints.rb
* 20160610201627_migrate_users_notification_level.rb
* 20160610301627_remove_notification_level_from_users.rb
---
 db/schema.rb | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/db/schema.rb b/db/schema.rb
index aac327797e..3c947d62e8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160608155312) do
+ActiveRecord::Schema.define(version: 20160610301627) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -670,8 +670,8 @@ ActiveRecord::Schema.define(version: 20160608155312) do
 
   create_table "notification_settings", force: :cascade do |t|
     t.integer  "user_id",                 null: false
-    t.integer  "source_id",               null: false
-    t.string   "source_type",             null: false
+    t.integer  "source_id"
+    t.string   "source_type"
     t.integer  "level",       default: 0, null: false
     t.datetime "created_at",              null: false
     t.datetime "updated_at",              null: false
@@ -988,7 +988,6 @@ ActiveRecord::Schema.define(version: 20160608155312) do
     t.boolean  "can_create_team",             default: true,  null: false
     t.string   "state"
     t.integer  "color_scheme_id",             default: 1,     null: false
-    t.integer  "notification_level",          default: 1,     null: false
     t.datetime "password_expires_at"
     t.integer  "created_by_id"
     t.datetime "last_credential_check_at"

From 0568b90c97dcbad3ab100e060fef91e0786aafe8 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 10 Jun 2016 16:53:20 -0300
Subject: [PATCH 14/23] Remove deprecated issues_tracker and issues_tracker_id
 from project

---
 app/models/project.rb                         |  5 -----
 .../project_services/issue_tracker_service.rb | 18 +++------------
 ...ed_issues_tracker_columns_from_projects.rb |  6 +++++
 db/schema.rb                                  |  2 --
 spec/factories/projects.rb                    |  6 -----
 spec/helpers/issues_helper_spec.rb            | 16 +++-----------
 spec/models/project_spec.rb                   | 22 -------------------
 7 files changed, 12 insertions(+), 63 deletions(-)
 create mode 100644 db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb

diff --git a/app/models/project.rb b/app/models/project.rb
index e2f7ffe493..dfa99fe0df 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -146,7 +146,6 @@ class Project < ActiveRecord::Base
               message: Gitlab::Regex.project_path_regex_message }
   validates :issues_enabled, :merge_requests_enabled,
             :wiki_enabled, inclusion: { in: [true, false] }
-  validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true
   validates :namespace, presence: true
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
@@ -589,10 +588,6 @@ class Project < ActiveRecord::Base
     update_column(:has_external_issue_tracker, services.external_issue_trackers.any?)
   end
 
-  def can_have_issues_tracker_id?
-    self.issues_enabled && !self.default_issues_tracker?
-  end
-
   def build_missing_services
     services_templates = Service.where(template: true)
 
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index 6ae9b16d3c..87ecb3b8b8 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -38,9 +38,9 @@ class IssueTrackerService < Service
       if enabled_in_gitlab_config
         self.properties = {
           title: issues_tracker['title'],
-          project_url: add_issues_tracker_id(issues_tracker['project_url']),
-          issues_url: add_issues_tracker_id(issues_tracker['issues_url']),
-          new_issue_url: add_issues_tracker_id(issues_tracker['new_issue_url'])
+          project_url: issues_tracker['project_url'],
+          issues_url: issues_tracker['issues_url'],
+          new_issue_url: issues_tracker['new_issue_url']
         }
       else
         self.properties = {}
@@ -83,16 +83,4 @@ class IssueTrackerService < Service
   def issues_tracker
     Gitlab.config.issues_tracker[to_param]
   end
-
-  def add_issues_tracker_id(url)
-    if self.project
-      id = self.project.issues_tracker_id
-
-      if id
-        url = url.gsub(":issues_tracker_id", id)
-      end
-    end
-
-    url
-  end
 end
diff --git a/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb
new file mode 100644
index 0000000000..477b2106de
--- /dev/null
+++ b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb
@@ -0,0 +1,6 @@
+class RemoveDeprecatedIssuesTrackerColumnsFromProjects < ActiveRecord::Migration
+  def change
+    remove_column :projects, :issues_tracker, :string, default: 'gitlab', null: false
+    remove_column :projects, :issues_tracker_id, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3c947d62e8..3dccbbd50b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -751,8 +751,6 @@ ActiveRecord::Schema.define(version: 20160610301627) do
     t.boolean  "merge_requests_enabled",             default: true,     null: false
     t.boolean  "wiki_enabled",                       default: true,     null: false
     t.integer  "namespace_id"
-    t.string   "issues_tracker",                     default: "gitlab", null: false
-    t.string   "issues_tracker_id"
     t.boolean  "snippets_enabled",                   default: true,     null: false
     t.datetime "last_activity_at"
     t.string   "import_url"
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index da8d97c9f8..5c8ddbebf0 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -67,9 +67,6 @@ FactoryGirl.define do
           'new_issue_url' => 'http://redmine/projects/project_name_in_redmine/issues/new'
         }
       )
-
-      project.issues_tracker = 'redmine'
-      project.issues_tracker_id = 'project_name_in_redmine'
     end
   end
 
@@ -84,9 +81,6 @@ FactoryGirl.define do
           'new_issue_url' => 'http://jira.example/secure/CreateIssue.jspa'
         }
       )
-
-      project.issues_tracker = 'jira'
-      project.issues_tracker_id = 'project_name_in_jira'
     end
   end
 end
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index eae61a54df..831ae7fb69 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -7,10 +7,7 @@ describe IssuesHelper do
 
   describe "url_for_project_issues" do
     let(:project_url) { ext_project.external_issue_tracker.project_url }
-    let(:ext_expected) do
-      project_url.gsub(':project_id', ext_project.id.to_s)
-                 .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { project_url.gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { polymorphic_path([@project.namespace, project]) }
 
     it "should return internal path if used internal tracker" do
@@ -56,11 +53,7 @@ describe IssuesHelper do
 
   describe "url_for_issue" do
     let(:issues_url) { ext_project.external_issue_tracker.issues_url}
-    let(:ext_expected) do
-      issues_url.gsub(':id', issue.iid.to_s)
-        .gsub(':project_id', ext_project.id.to_s)
-        .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { issues_url.gsub(':id', issue.iid.to_s).gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { polymorphic_path([@project.namespace, project, issue]) }
 
     it "should return internal path if used internal tracker" do
@@ -106,10 +99,7 @@ describe IssuesHelper do
 
   describe 'url_for_new_issue' do
     let(:issues_url) { ext_project.external_issue_tracker.new_issue_url }
-    let(:ext_expected) do
-      issues_url.gsub(':project_id', ext_project.id.to_s)
-        .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { issues_url.gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { new_namespace_project_issue_path(project.namespace, project) }
 
     it "should return internal path if used internal tracker" do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f3590f72cf..de8815f5a3 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -53,7 +53,6 @@ describe Project, models: true do
     it { is_expected.to validate_length_of(:path).is_within(0..255) }
     it { is_expected.to validate_length_of(:description).is_within(0..2000) }
     it { is_expected.to validate_presence_of(:creator) }
-    it { is_expected.to validate_length_of(:issues_tracker_id).is_within(0..255) }
     it { is_expected.to validate_presence_of(:namespace) }
 
     it 'should not allow new projects beyond user limits' do
@@ -321,27 +320,6 @@ describe Project, models: true do
     end
   end
 
-  describe :can_have_issues_tracker_id? do
-    let(:project) { create(:project) }
-    let(:ext_project) { create(:redmine_project) }
-
-    it 'should be true for projects with external issues tracker if issues enabled' do
-      expect(ext_project.can_have_issues_tracker_id?).to be_truthy
-    end
-
-    it 'should be false for projects with internal issue tracker if issues enabled' do
-      expect(project.can_have_issues_tracker_id?).to be_falsey
-    end
-
-    it 'should be always false if issues disabled' do
-      project.issues_enabled = false
-      ext_project.issues_enabled = false
-
-      expect(project.can_have_issues_tracker_id?).to be_falsey
-      expect(ext_project.can_have_issues_tracker_id?).to be_falsey
-    end
-  end
-
   describe :open_branches do
     let(:project) { create(:project) }
 

From 4c716de450ecc2959022c67f99f188fff540c6c4 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 10 Jun 2016 16:53:48 -0300
Subject: [PATCH 15/23] Update CHANGELOG

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 7a6a14919d..3387394de5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -72,6 +72,7 @@ v 8.9.0 (unreleased)
   - Cache on the database if a project has an active external issue tracker.
   - Put project Labels and Milestones pages links under Issues and Merge Requests tabs as subnav
   - All classes in the Banzai::ReferenceParser namespace are now instrumented
+  - Remove deprecated issues_tracker and issues_tracker_id from project model
 
 v 8.8.5 (unreleased)
   - Ensure branch cleanup regardless of whether the GitHub import process succeeds

From af8500f43010f42176b2ec1814f0fe7248258b05 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 6 Jun 2016 15:56:04 -0300
Subject: [PATCH 16/23] Allow users to create confidential issues in private
 projects

---
 app/views/shared/issuable/_form.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 17e2a7e929..d503026f91 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -35,7 +35,7 @@
       .clearfix
       .error-alert
 
-- if issuable.is_a?(Issue) && !issuable.project.private?
+- if issuable.is_a?(Issue)
   .form-group
     .col-sm-offset-2.col-sm-10
       .checkbox

From b56c45675019baaaf47615d51c08d5caa0734ad3 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 6 Jun 2016 16:13:31 -0300
Subject: [PATCH 17/23] Project members with guest role can't access
 confidential issues

---
 app/finders/snippets_finder.rb                |  2 +-
 app/models/ability.rb                         |  2 +-
 app/models/issue.rb                           | 10 +++++++-
 app/models/note.rb                            |  2 +-
 app/models/project_team.rb                    | 10 ++++++--
 app/models/user.rb                            | 25 +++++++++++++------
 app/views/shared/issuable/_form.html.haml     |  2 +-
 .../projects/issues_controller_spec.rb        | 19 +++++++++++++-
 .../lib/banzai/filter/redactor_filter_spec.rb | 12 +++++++++
 .../lib/gitlab/project_search_results_spec.rb | 12 +++++++++
 spec/lib/gitlab/search_results_spec.rb        | 16 ++++++++++++
 spec/models/concerns/milestoneish_spec.rb     | 14 +++++++++++
 spec/models/event_spec.rb                     |  6 +++++
 spec/models/note_spec.rb                      | 15 ++++++++---
 spec/models/project_team_spec.rb              |  6 +++++
 spec/requests/api/issues_spec.rb              | 25 ++++++++++++++++++-
 spec/requests/api/milestones_spec.rb          | 13 ++++++++++
 spec/services/notification_service_spec.rb    | 11 ++++++++
 .../projects/autocomplete_service_spec.rb     | 12 +++++++++
 spec/services/todo_service_spec.rb            | 18 ++++++++++---
 20 files changed, 208 insertions(+), 24 deletions(-)

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index 01cbf91c65..00ff161103 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -51,7 +51,7 @@ class SnippetsFinder
     snippets = project.snippets.fresh
 
     if current_user
-      if project.team.member?(current_user.id) || current_user.admin?
+      if project.team.member?(current_user) || current_user.admin?
         snippets
       else
         snippets.public_and_internal
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 44515550d9..aea946f922 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -533,7 +533,7 @@ class Ability
     def filter_confidential_issues_abilities(user, issue, rules)
       return rules if user.admin? || !issue.confidential?
 
-      unless issue.author == user || issue.assignee == user || issue.project.team.member?(user.id)
+      unless issue.author == user || issue.assignee == user || issue.project.team.member?(user, Gitlab::Access::REPORTER)
         rules.delete(:admin_issue)
         rules.delete(:read_issue)
         rules.delete(:update_issue)
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 235922710a..6ecb353535 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -54,7 +54,15 @@ class Issue < ActiveRecord::Base
     return where(confidential: false) if user.blank?
     return all if user.admin?
 
-    where('issues.confidential = false OR (issues.confidential = true AND (issues.author_id = :user_id OR issues.assignee_id = :user_id OR issues.project_id IN(:project_ids)))', user_id: user.id, project_ids: user.authorized_projects.select(:id))
+    where('
+      issues.confidential IS NULL
+      OR issues.confidential IS FALSE
+      OR (issues.confidential = TRUE
+        AND (issues.author_id = :user_id
+          OR issues.assignee_id = :user_id
+          OR issues.project_id IN(:project_ids)))',
+      user_id: user.id,
+      project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
   end
 
   def self.reference_prefix
diff --git a/app/models/note.rb b/app/models/note.rb
index 585d8c4ad8..8ce2b6fa53 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -100,7 +100,7 @@ class Note < ActiveRecord::Base
             OR issues.assignee_id = :user_id
             OR issues.project_id IN(:project_ids)))',
           user_id: as_user.id,
-          project_ids: as_user.authorized_projects.select(:id))
+          project_ids: as_user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
       else
         found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE')
       end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 70a8bbaba6..e29e854860 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -131,8 +131,14 @@ class ProjectTeam
     max_member_access(user.id) == Gitlab::Access::MASTER
   end
 
-  def member?(user_id)
-    !!find_member(user_id)
+  def member?(user, min_member_access = nil)
+    member = !!find_member(user.id)
+
+    if min_member_access
+      member && max_member_access(user.id) >= min_member_access
+    else
+      member
+    end
   end
 
   def human_max_access(user_id)
diff --git a/app/models/user.rb b/app/models/user.rb
index 7afbfbf112..69c1bf4bc3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -405,8 +405,8 @@ class User < ActiveRecord::Base
   end
 
   # Returns projects user is authorized to access.
-  def authorized_projects
-    Project.where("projects.id IN (#{projects_union.to_sql})")
+  def authorized_projects(min_access_level = nil)
+    Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
   end
 
   def viewable_starred_projects
@@ -824,11 +824,22 @@ class User < ActiveRecord::Base
 
   private
 
-  def projects_union
-    Gitlab::SQL::Union.new([personal_projects.select(:id),
-                            groups_projects.select(:id),
-                            projects.select(:id),
-                            groups.joins(:shared_projects).select(:project_id)])
+  def projects_union(min_access_level = nil)
+    relations = if min_access_level
+                  scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
+
+                  [personal_projects.select(:id),
+                   groups_projects.where(members: scope).select(:id),
+                   projects.where(members: scope).select(:id),
+                   groups.joins(:shared_projects).where(members: scope).select(:project_id)]
+                else
+                  [personal_projects.select(:id),
+                   groups_projects.select(:id),
+                   projects.select(:id),
+                   groups.joins(:shared_projects).select(:project_id)]
+                end
+
+    Gitlab::SQL::Union.new(relations)
   end
 
   def ci_projects_union
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index d503026f91..c30bdb0ae9 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -41,7 +41,7 @@
       .checkbox
         = f.label :confidential do
           = f.check_box :confidential
-          This issue is confidential and should only be visible to team members
+          This issue is confidential and should only be visible to team members with at least Reporter access.
 
 - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
   - has_due_date = issuable.has_attribute?(:due_date)
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 78be7e3dc3..cbaa3e0b7b 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -105,6 +105,15 @@ describe Projects::IssuesController do
         expect(assigns(:issues)).to eq [issue]
       end
 
+      it 'should not list confidential issues for project members with guest role' do
+        sign_in(member)
+        project.team << [member, :guest]
+
+        get_issues
+
+        expect(assigns(:issues)).to eq [issue]
+      end
+
       it 'should list confidential issues for author' do
         sign_in(author)
         get_issues
@@ -148,7 +157,7 @@ describe Projects::IssuesController do
 
     shared_examples_for 'restricted action' do |http_status|
       it 'returns 404 for guests' do
-        sign_out :user
+        sign_out(:user)
         go(id: unescaped_parameter_value.to_param)
 
         expect(response).to have_http_status :not_found
@@ -161,6 +170,14 @@ describe Projects::IssuesController do
         expect(response).to have_http_status :not_found
       end
 
+      it 'returns 404 for project members with guest role' do
+        sign_in(member)
+        project.team << [member, :guest]
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status :not_found
+      end
+
       it "returns #{http_status[:success]} for author" do
         sign_in(author)
         go(id: unescaped_parameter_value.to_param)
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index 697d10bbf7..f181125156 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -69,6 +69,18 @@ describe Banzai::Filter::RedactorFilter, lib: true do
         expect(doc.css('a').length).to eq 0
       end
 
+      it 'removes references for project members with guest role' do
+        member = create(:user)
+        project = create(:empty_project, :public)
+        project.team << [member, :guest]
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
+        doc = filter(link, current_user: member)
+
+        expect(doc.css('a').length).to eq 0
+      end
+
       it 'allows references for author' do
         author = create(:user)
         project = create(:empty_project, :public)
diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb
index db0ff95b4f..270b89972d 100644
--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/spec/lib/gitlab/project_search_results_spec.rb
@@ -43,6 +43,18 @@ describe Gitlab::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 1
     end
 
+    it 'should not list project confidential issues for project members with guest role' do
+      project.team << [member, :guest]
+
+      results = described_class.new(member, project, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 1
+    end
+
     it 'should list project confidential issues for author' do
       results = described_class.new(author, project, query)
       issues = results.objects('issues')
diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb
index f4afe597e8..1bb444bf34 100644
--- a/spec/lib/gitlab/search_results_spec.rb
+++ b/spec/lib/gitlab/search_results_spec.rb
@@ -86,6 +86,22 @@ describe Gitlab::SearchResults do
       expect(results.issues_count).to eq 1
     end
 
+    it 'should not list confidential issues for project members with guest role' do
+      project_1.team << [member, :guest]
+      project_2.team << [member, :guest]
+
+      results = described_class.new(member, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 1
+    end
+
     it 'should list confidential issues for author' do
       results = described_class.new(author, limit_projects, query)
       issues = results.objects('issues')
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 47c3be673c..7e9ab8940c 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -5,6 +5,7 @@ describe Milestone, 'Milestoneish' do
   let(:assignee) { create(:user) }
   let(:non_member) { create(:user) }
   let(:member) { create(:user) }
+  let(:guest) { create(:user) }
   let(:admin) { create(:admin) }
   let(:project) { create(:project, :public) }
   let(:milestone) { create(:milestone, project: project) }
@@ -21,6 +22,7 @@ describe Milestone, 'Milestoneish' do
 
   before do
     project.team << [member, :developer]
+    project.team << [guest, :guest]
   end
 
   describe '#closed_items_count' do
@@ -28,6 +30,10 @@ describe Milestone, 'Milestoneish' do
       expect(milestone.closed_items_count(non_member)).to eq 2
     end
 
+    it 'should not count confidential issues for project members with guest role' do
+      expect(milestone.closed_items_count(guest)).to eq 2
+    end
+
     it 'should count confidential issues for author' do
       expect(milestone.closed_items_count(author)).to eq 4
     end
@@ -50,6 +56,10 @@ describe Milestone, 'Milestoneish' do
       expect(milestone.total_items_count(non_member)).to eq 4
     end
 
+    it 'should not count confidential issues for project members with guest role' do
+      expect(milestone.total_items_count(guest)).to eq 4
+    end
+
     it 'should count confidential issues for author' do
       expect(milestone.total_items_count(author)).to eq 7
     end
@@ -85,6 +95,10 @@ describe Milestone, 'Milestoneish' do
       expect(milestone.percent_complete(non_member)).to eq 50
     end
 
+    it 'should not count confidential issues for project members with guest role' do
+      expect(milestone.percent_complete(guest)).to eq 50
+    end
+
     it 'should count confidential issues for author' do
       expect(milestone.percent_complete(author)).to eq 57
     end
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index b0e76fec69..166a1dc4dd 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -50,6 +50,7 @@ describe Event, models: true do
     let(:project) { create(:empty_project, :public) }
     let(:non_member) { create(:user) }
     let(:member)  { create(:user) }
+    let(:guest)  { create(:user) }
     let(:author) { create(:author) }
     let(:assignee) { create(:user) }
     let(:admin) { create(:admin) }
@@ -61,6 +62,7 @@ describe Event, models: true do
 
     before do
       project.team << [member, :developer]
+      project.team << [guest, :guest]
     end
 
     context 'issue event' do
@@ -71,6 +73,7 @@ describe Event, models: true do
         it { expect(event.visible_to_user?(author)).to eq true }
         it { expect(event.visible_to_user?(assignee)).to eq true }
         it { expect(event.visible_to_user?(member)).to eq true }
+        it { expect(event.visible_to_user?(guest)).to eq true }
         it { expect(event.visible_to_user?(admin)).to eq true }
       end
 
@@ -81,6 +84,7 @@ describe Event, models: true do
         it { expect(event.visible_to_user?(author)).to eq true }
         it { expect(event.visible_to_user?(assignee)).to eq true }
         it { expect(event.visible_to_user?(member)).to eq true }
+        it { expect(event.visible_to_user?(guest)).to eq false }
         it { expect(event.visible_to_user?(admin)).to eq true }
       end
     end
@@ -93,6 +97,7 @@ describe Event, models: true do
         it { expect(event.visible_to_user?(author)).to eq true }
         it { expect(event.visible_to_user?(assignee)).to eq true }
         it { expect(event.visible_to_user?(member)).to eq true }
+        it { expect(event.visible_to_user?(guest)).to eq true }
         it { expect(event.visible_to_user?(admin)).to eq true }
       end
 
@@ -103,6 +108,7 @@ describe Event, models: true do
         it { expect(event.visible_to_user?(author)).to eq true }
         it { expect(event.visible_to_user?(assignee)).to eq true }
         it { expect(event.visible_to_user?(member)).to eq true }
+        it { expect(event.visible_to_user?(guest)).to eq false }
         it { expect(event.visible_to_user?(admin)).to eq true }
       end
     end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index f15e96714b..285ab19cfa 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -162,16 +162,23 @@ describe Note, models: true do
     end
 
     context "confidential issues" do
-      let(:user) { create :user }
-      let(:confidential_issue) { create(:issue, :confidential, author: user) }
-      let(:confidential_note) { create :note, note: "Random", noteable: confidential_issue, project: confidential_issue.project }
+      let(:user) { create(:user) }
+      let(:project) { create(:project) }
+      let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) }
+      let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) }
 
       it "returns notes with matching content if user can see the issue" do
         expect(described_class.search(confidential_note.note, as_user: user)).to eq([confidential_note])
       end
 
       it "does not return notes with matching content if user can not see the issue" do
-        user = create :user
+        user = create(:user)
+        expect(described_class.search(confidential_note.note, as_user: user)).to be_empty
+      end
+
+      it "does not return notes with matching content for project members with guest role" do
+        user = create(:user)
+        project.team << [user, :guest]
         expect(described_class.search(confidential_note.note, as_user: user)).to be_empty
       end
 
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index bacb17a888..8bebd6a944 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -29,6 +29,9 @@ describe ProjectTeam, models: true do
       it { expect(project.team.master?(nonmember)).to be_falsey }
       it { expect(project.team.member?(nonmember)).to be_falsey }
       it { expect(project.team.member?(guest)).to be_truthy }
+      it { expect(project.team.member?(reporter, Gitlab::Access::REPORTER)).to be_truthy }
+      it { expect(project.team.member?(guest, Gitlab::Access::REPORTER)).to be_falsey }
+      it { expect(project.team.member?(nonmember, Gitlab::Access::GUEST)).to be_falsey }
     end
   end
 
@@ -64,6 +67,9 @@ describe ProjectTeam, models: true do
       it { expect(project.team.master?(nonmember)).to be_falsey }
       it { expect(project.team.member?(nonmember)).to be_falsey }
       it { expect(project.team.member?(guest)).to be_truthy }
+      it { expect(project.team.member?(guest, Gitlab::Access::MASTER)).to be_truthy }
+      it { expect(project.team.member?(reporter, Gitlab::Access::MASTER)).to be_falsey }
+      it { expect(project.team.member?(nonmember, Gitlab::Access::GUEST)).to be_falsey }
     end
   end
 
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index bb92617259..59e557c5b2 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -5,6 +5,7 @@ describe API::API, api: true  do
   let(:user)        { create(:user) }
   let(:user2)       { create(:user) }
   let(:non_member)  { create(:user) }
+  let(:guest)       { create(:user) }
   let(:author)      { create(:author) }
   let(:assignee)    { create(:assignee) }
   let(:admin)       { create(:user, :admin) }
@@ -41,7 +42,10 @@ describe API::API, api: true  do
   end
   let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) }
 
-  before { project.team << [user, :reporter] }
+  before do
+    project.team << [user, :reporter]
+    project.team << [guest, :guest]
+  end
 
   describe "GET /issues" do
     context "when unauthenticated" do
@@ -144,6 +148,14 @@ describe API::API, api: true  do
       expect(json_response.first['title']).to eq(issue.title)
     end
 
+    it 'should return project issues without confidential issues for project members with guest role' do
+      get api("#{base_url}/issues", guest)
+      expect(response.status).to eq(200)
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(2)
+      expect(json_response.first['title']).to eq(issue.title)
+    end
+
     it 'should return project confidential issues for author' do
       get api("#{base_url}/issues", author)
       expect(response.status).to eq(200)
@@ -278,6 +290,11 @@ describe API::API, api: true  do
         expect(response.status).to eq(404)
       end
 
+      it "should return 404 for project members with guest role" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest)
+        expect(response.status).to eq(404)
+      end
+
       it "should return confidential issue for project members" do
         get api("/projects/#{project.id}/issues/#{confidential_issue.id}", user)
         expect(response.status).to eq(200)
@@ -413,6 +430,12 @@ describe API::API, api: true  do
         expect(response.status).to eq(403)
       end
 
+      it "should return 403 for project members with guest role" do
+        put api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest),
+          title: 'updated title'
+        expect(response.status).to eq(403)
+      end
+
       it "should update a confidential issue for project members" do
         put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
           title: 'updated title'
diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb
index 241995041b..0154d1c62c 100644
--- a/spec/requests/api/milestones_spec.rb
+++ b/spec/requests/api/milestones_spec.rb
@@ -146,6 +146,7 @@ describe API::API, api: true  do
       let(:milestone) { create(:milestone, project: public_project) }
       let(:issue) { create(:issue, project: public_project) }
       let(:confidential_issue) { create(:issue, confidential: true, project: public_project) }
+
       before do
         public_project.team << [user, :developer]
         milestone.issues << issue << confidential_issue
@@ -160,6 +161,18 @@ describe API::API, api: true  do
         expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id)
       end
 
+      it 'does not return confidential issues to team members with guest role' do
+        member = create(:user)
+        project.team << [member, :guest]
+
+        get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", member)
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(1)
+        expect(json_response.map { |issue| issue['id'] }).to include(issue.id)
+      end
+
       it 'does not return confidential issues to regular users' do
         get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", create(:user))
 
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index b99e02ba67..e871a103d4 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -132,12 +132,14 @@ describe NotificationService, services: true do
       let(:assignee) { create(:user) }
       let(:non_member) { create(:user) }
       let(:member) { create(:user) }
+      let(:guest) { create(:user) }
       let(:admin) { create(:admin) }
       let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
       let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") }
 
       it 'filters out users that can not read the issue' do
         project.team << [member, :developer]
+        project.team << [guest, :guest]
 
         expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times
 
@@ -146,6 +148,7 @@ describe NotificationService, services: true do
         notification.new_note(note)
 
         should_not_email(non_member)
+        should_not_email(guest)
         should_email(author)
         should_email(assignee)
         should_email(member)
@@ -322,17 +325,20 @@ describe NotificationService, services: true do
         let(:assignee) { create(:user) }
         let(:non_member) { create(:user) }
         let(:member) { create(:user) }
+        let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
 
         it "emails subscribers of the issue's labels that can read the issue" do
           project.team << [member, :developer]
+          project.team << [guest, :guest]
 
           label = create(:label, issues: [confidential_issue])
           label.toggle_subscription(non_member)
           label.toggle_subscription(author)
           label.toggle_subscription(assignee)
           label.toggle_subscription(member)
+          label.toggle_subscription(guest)
           label.toggle_subscription(admin)
 
           ActionMailer::Base.deliveries.clear
@@ -341,6 +347,7 @@ describe NotificationService, services: true do
 
           should_not_email(non_member)
           should_not_email(author)
+          should_not_email(guest)
           should_email(assignee)
           should_email(member)
           should_email(admin)
@@ -490,6 +497,7 @@ describe NotificationService, services: true do
         let(:assignee) { create(:user) }
         let(:non_member) { create(:user) }
         let(:member) { create(:user) }
+        let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
         let!(:label_1) { create(:label, issues: [confidential_issue]) }
@@ -497,11 +505,13 @@ describe NotificationService, services: true do
 
         it "emails subscribers of the issue's labels that can read the issue" do
           project.team << [member, :developer]
+          project.team << [guest, :guest]
 
           label_2.toggle_subscription(non_member)
           label_2.toggle_subscription(author)
           label_2.toggle_subscription(assignee)
           label_2.toggle_subscription(member)
+          label_2.toggle_subscription(guest)
           label_2.toggle_subscription(admin)
 
           ActionMailer::Base.deliveries.clear
@@ -509,6 +519,7 @@ describe NotificationService, services: true do
           notification.relabeled_issue(confidential_issue, [label_2], @u_disabled)
 
           should_not_email(non_member)
+          should_not_email(guest)
           should_email(author)
           should_email(assignee)
           should_email(member)
diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb
index 6108c26a78..0971fec2e9 100644
--- a/spec/services/projects/autocomplete_service_spec.rb
+++ b/spec/services/projects/autocomplete_service_spec.rb
@@ -33,6 +33,18 @@ describe Projects::AutocompleteService, services: true do
         expect(issues.count).to eq 1
       end
 
+      it 'should not list project confidential issues for project members with guest role' do
+        project.team << [member, :guest]
+
+        autocomplete = described_class.new(project, non_member)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).not_to include security_issue_1.iid
+        expect(issues).not_to include security_issue_2.iid
+        expect(issues.count).to eq 1
+      end
+
       it 'should list project confidential issues for author' do
         autocomplete = described_class.new(project, author)
         issues = autocomplete.issues.map(&:iid)
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 489c920f19..549a936b06 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -5,13 +5,15 @@ describe TodoService, services: true do
   let(:assignee) { create(:user) }
   let(:non_member) { create(:user) }
   let(:member) { create(:user) }
+  let(:guest) { create(:user) }
   let(:admin) { create(:admin) }
   let(:john_doe) { create(:user) }
   let(:project) { create(:project) }
-  let(:mentions) { [author, assignee, john_doe, member, non_member, admin].map(&:to_reference).join(' ') }
+  let(:mentions) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') }
   let(:service) { described_class.new }
 
   before do
+    project.team << [guest, :guest]
     project.team << [author, :developer]
     project.team << [member, :developer]
     project.team << [john_doe, :developer]
@@ -41,18 +43,20 @@ describe TodoService, services: true do
         service.new_issue(issue, author)
 
         should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
       end
 
-      it 'does not create todo for non project members when issue is confidential' do
+      it 'does not create todo if user can not see the issue when issue is confidential' do
         service.new_issue(confidential_issue, john_doe)
 
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::ASSIGNED)
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
@@ -81,6 +85,7 @@ describe TodoService, services: true do
         service.update_issue(issue, author)
 
         should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
@@ -92,13 +97,14 @@ describe TodoService, services: true do
         expect { service.update_issue(issue, author) }.not_to change(member.todos, :count)
       end
 
-      it 'does not create todo for non project members when issue is confidential' do
+      it 'does not create todo if user can not see the issue when issue is confidential' do
         service.update_issue(confidential_issue, john_doe)
 
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
@@ -192,18 +198,20 @@ describe TodoService, services: true do
         service.new_note(note, john_doe)
 
         should_create_todo(user: member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+        should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_create_todo(user: author, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
       end
 
-      it 'does not create todo for non project members when leaving a note on a confidential issue' do
+      it 'does not create todo if user can not see the issue when leaving a note on a confidential issue' do
         service.new_note(note_on_confidential_issue, john_doe)
 
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
       end
 
@@ -245,6 +253,7 @@ describe TodoService, services: true do
         service.new_merge_request(mr_assigned, author)
 
         should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
@@ -256,6 +265,7 @@ describe TodoService, services: true do
         service.update_merge_request(mr_assigned, author)
 
         should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)

From 149176758393c8d89d996c62c11511b3e86b3f8d Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 10 Jun 2016 11:26:47 -0300
Subject: [PATCH 18/23] Use Issue.visible_to_user in Notes.search to avoid
 query duplication

---
 app/models/issue.rb |  2 +-
 app/models/note.rb  | 19 +++----------------
 app/models/user.rb  | 21 +++++++++------------
 3 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6ecb353535..1bdf9c011b 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -51,7 +51,7 @@ class Issue < ActiveRecord::Base
   end
 
   def self.visible_to_user(user)
-    return where(confidential: false) if user.blank?
+    return where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank?
     return all if user.admin?
 
     where('
diff --git a/app/models/note.rb b/app/models/note.rb
index 8ce2b6fa53..58133f1581 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -88,22 +88,9 @@ class Note < ActiveRecord::Base
       table   = arel_table
       pattern = "%#{query}%"
 
-      found_notes = joins('LEFT JOIN issues ON issues.id = noteable_id').
-        where(table[:note].matches(pattern))
-
-      if as_user
-        found_notes.where('
-          issues.confidential IS NULL
-          OR issues.confidential IS FALSE
-          OR (issues.confidential IS TRUE
-            AND (issues.author_id = :user_id
-            OR issues.assignee_id = :user_id
-            OR issues.project_id IN(:project_ids)))',
-          user_id: as_user.id,
-          project_ids: as_user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
-      else
-        found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE')
-      end
+      Note.joins('LEFT JOIN issues ON issues.id = noteable_id').
+        where(table[:note].matches(pattern)).
+        merge(Issue.visible_to_user(as_user))
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 69c1bf4bc3..a5b3c8afe5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -825,19 +825,16 @@ class User < ActiveRecord::Base
   private
 
   def projects_union(min_access_level = nil)
-    relations = if min_access_level
-                  scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
+    relations = [personal_projects.select(:id),
+                 groups_projects.select(:id),
+                 projects.select(:id),
+                 groups.joins(:shared_projects).select(:project_id)]
 
-                  [personal_projects.select(:id),
-                   groups_projects.where(members: scope).select(:id),
-                   projects.where(members: scope).select(:id),
-                   groups.joins(:shared_projects).where(members: scope).select(:project_id)]
-                else
-                  [personal_projects.select(:id),
-                   groups_projects.select(:id),
-                   projects.select(:id),
-                   groups.joins(:shared_projects).select(:project_id)]
-                end
+
+    if min_access_level
+      scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
+      relations = [relations.shift] + relations.map { |relation| relation.where(members: scope) }
+    end
 
     Gitlab::SQL::Union.new(relations)
   end

From 6789d2ebf37d5f0537bea72ba99d3b7711e70728 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 8 Jun 2016 16:09:03 -0300
Subject: [PATCH 19/23] Update CHANGELOG

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 3387394de5..92666e6325 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -73,6 +73,7 @@ v 8.9.0 (unreleased)
   - Put project Labels and Milestones pages links under Issues and Merge Requests tabs as subnav
   - All classes in the Banzai::ReferenceParser namespace are now instrumented
   - Remove deprecated issues_tracker and issues_tracker_id from project model
+  - Allow users to create confidential issues in private projects
 
 v 8.8.5 (unreleased)
   - Ensure branch cleanup regardless of whether the GitHub import process succeeds

From 84b07f7054bca5820bc54a99014538506718201e Mon Sep 17 00:00:00 2001
From: Benjamin Schmid <benjamin.schmid@exxcellent.de>
Date: Thu, 19 May 2016 17:08:10 +0200
Subject: [PATCH 20/23] Honor credentials on calling Bamboo CI trigger
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This improves the Bamboo Service and provides a fix for situations,
where the build trigger won't work, because Bamboo is requiring
authentication also for the trigger GET.

The change now does provide additional HTTP Basic Auth parameters
if user credentials were provided and appends an request parameter
indicating the HTTP Basic Authentication should be used.
This aligns interaction with Bamboo with the other calls this service
executes.

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 app/models/project_services/bamboo_service.rb | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 1d1780dcfb..8c9c52ac7d 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -1,6 +1,4 @@
 class BambooService < CiService
-  include HTTParty
-
   prop_accessor :bamboo_url, :build_key, :username, :password
 
   validates :bamboo_url, presence: true, url: true, if: :activated?
@@ -112,8 +110,19 @@ class BambooService < CiService
   def execute(data)
     return unless supported_events.include?(data[:object_kind])
 
-    # Bamboo requires a GET and does not take any data.
+    # Bamboo requires a GET and does take authentification
     url = URI.join(bamboo_url, "/updateAndBuild.action?buildKey=#{build_key}").to_s
-    self.class.get(url, verify: false)
+
+    if username.blank? && password.blank?
+      HTTParty.get(url, verify: false)
+    else
+      url << '&os_authType=basic'
+      auth = {
+        username: username,
+        password: password
+      }
+      HTTParty.get(url, verify: false, basic_auth: auth)
+    end
+
   end
 end

From 46f3cd7c65b871d4efa6c33fbfccbc01fdf36649 Mon Sep 17 00:00:00 2001
From: Benjamin Schmid <benjamin.schmid@exxcellent.de>
Date: Mon, 30 May 2016 12:30:35 +0200
Subject: [PATCH 21/23] Fix broken URI joining for `bamboo_url` with suffixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If one had configured a `bamboo_url` like http://foo.bar/bamboo in the
previous implementation the plugin directed it's request i.e. to
http://foo.bar/rest/... instead of http://foo.bar/bamboo/rest/...

`URI.join` only works correctly, if the prefix URL has
  - at least one or more  trailing '/'
  - the appended parts are _not_ prefixed with '/'

The current implementation should work with all sorts of Bamboo base URLs.

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 CHANGELOG                                     | 1 +
 app/models/project_services/bamboo_service.rb | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 3387394de5..bba6991f18 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -38,6 +38,7 @@ v 8.9.0 (unreleased)
   - Add option to project to only allow merge requests to be merged if the build succeeds (Rui Santos)
   - Fix issues filter when ordering by milestone
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
+  - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
   - Todos will display target state if issuable target is 'Closed' or 'Merged'
   - Fix bug when sorting issues by milestone due date and filtering by two or more labels
   - Add support for using Yubikeys (U2F) for two-factor authentication
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 8c9c52ac7d..cb215b595f 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -59,7 +59,7 @@ class BambooService < CiService
   end
 
   def build_info(sha)
-    url = URI.join(bamboo_url, "/rest/api/latest/result?label=#{sha}").to_s
+    url = URI.join("#{bamboo_url}/", "rest/api/latest/result?label=#{sha}").to_s
 
     if username.blank? && password.blank?
       @response = HTTParty.get(url, verify: false)
@@ -78,11 +78,11 @@ class BambooService < CiService
 
     if @response.code != 200 || @response['results']['results']['size'] == '0'
       # If actual build link can't be determined, send user to build summary page.
-      URI.join(bamboo_url, "/browse/#{build_key}").to_s
+      URI.join("#{bamboo_url}/", "browse/#{build_key}").to_s
     else
       # If actual build link is available, go to build result page.
       result_key = @response['results']['results']['result']['planResultKey']['key']
-      URI.join(bamboo_url, "/browse/#{result_key}").to_s
+      URI.join("#{bamboo_url}/", "browse/#{result_key}").to_s
     end
   end
 
@@ -111,7 +111,7 @@ class BambooService < CiService
     return unless supported_events.include?(data[:object_kind])
 
     # Bamboo requires a GET and does take authentification
-    url = URI.join(bamboo_url, "/updateAndBuild.action?buildKey=#{build_key}").to_s
+    url = URI.join("#{bamboo_url}/", "updateAndBuild.action?buildKey=#{build_key}").to_s
 
     if username.blank? && password.blank?
       HTTParty.get(url, verify: false)

From 17c32ee8d0b2dafa61b3f509d48f7ee8a8dbea14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 1 Jun 2016 16:43:40 +0200
Subject: [PATCH 22/23] Factorize duplicated code into a method in
 BambooService and update specs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 app/models/project_services/bamboo_service.rb | 37 +++++++++----------
 .../project_services/bamboo_service_spec.rb   | 12 +++---
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index cb215b595f..b5c76e4d4f 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -59,18 +59,7 @@ class BambooService < CiService
   end
 
   def build_info(sha)
-    url = URI.join("#{bamboo_url}/", "rest/api/latest/result?label=#{sha}").to_s
-
-    if username.blank? && password.blank?
-      @response = HTTParty.get(url, verify: false)
-    else
-      url << '&os_authType=basic'
-      auth = {
-        username: username,
-        password: password
-      }
-      @response = HTTParty.get(url, verify: false, basic_auth: auth)
-    end
+    @response = get_path("rest/api/latest/result?label=#{sha}")
   end
 
   def build_page(sha, ref)
@@ -110,19 +99,27 @@ class BambooService < CiService
   def execute(data)
     return unless supported_events.include?(data[:object_kind])
 
-    # Bamboo requires a GET and does take authentification
-    url = URI.join("#{bamboo_url}/", "updateAndBuild.action?buildKey=#{build_key}").to_s
+    get_path("updateAndBuild.action?buildKey=#{build_key}")
+  end
+
+  private
+
+  def build_url(path)
+    URI.join("#{bamboo_url}/", path).to_s
+  end
+
+  def get_path(path)
+    url = build_url(path)
 
     if username.blank? && password.blank?
       HTTParty.get(url, verify: false)
     else
       url << '&os_authType=basic'
-      auth = {
-        username: username,
-        password: password
-      }
-      HTTParty.get(url, verify: false, basic_auth: auth)
+      HTTParty.get(url, verify: false,
+                        basic_auth: {
+                          username: username,
+                          password: password
+                        })
     end
-
   end
 end
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index ec81f05fc7..9ae461f8c2 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -126,25 +126,25 @@ describe BambooService, models: true do
     it 'returns a specific URL when status is 500' do
       stub_request(status: 500)
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
     end
 
     it 'returns a specific URL when response has no results' do
       stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/foo')
     end
 
     it 'returns a build URL when bamboo_url has no trailing slash' do
       stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
 
-      expect(service(bamboo_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+      expect(service(bamboo_url: 'http://gitlab.com/bamboo').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
     end
 
     it 'returns a build URL when bamboo_url has a trailing slash' do
       stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
 
-      expect(service(bamboo_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+      expect(service(bamboo_url: 'http://gitlab.com/bamboo/').build_page('123', 'unused')).to eq('http://gitlab.com/bamboo/browse/42')
     end
   end
 
@@ -192,7 +192,7 @@ describe BambooService, models: true do
     end
   end
 
-  def service(bamboo_url: 'http://gitlab.com')
+  def service(bamboo_url: 'http://gitlab.com/bamboo')
     described_class.create(
       project: create(:empty_project),
       properties: {
@@ -205,7 +205,7 @@ describe BambooService, models: true do
   end
 
   def stub_request(status: 200, body: nil, build_state: 'success')
-    bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
+    bamboo_full_url = 'http://mic:password@gitlab.com/bamboo/rest/api/latest/result?label=123&os_authType=basic'
     body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
 
     WebMock.stub_request(:get, bamboo_full_url).to_return(

From 2f7b2057f25d4390d063e4c4fce3f4f12ea58463 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Wed, 1 Jun 2016 16:44:39 +0200
Subject: [PATCH 23/23] Fix broken URI joining for `teamcity_url` with suffixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If one had configured a `teamcity_url` like http://foo.bar/teamcity in
the previous implementation the plugin directed it's request i.e. to
http://foo.bar/httpAuth/... instead of http://foo.bar/teamcity/httpAuth/...

`URI.join` only works correctly, if the prefix URL has
  - at least one or more  trailing '/'
  - the appended parts are _not_ prefixed with '/'

The current implementation should work with all sorts of TeamCity base
URLs.

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 CHANGELOG                                     |  1 +
 .../project_services/teamcity_service.rb      | 37 ++++++++++---------
 .../project_services/teamcity_service_spec.rb | 10 ++---
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index bba6991f18..3d46901ba6 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -39,6 +39,7 @@ v 8.9.0 (unreleased)
   - Fix issues filter when ordering by milestone
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
   - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
+  - TeamCity Service: Fix URL handling when base URL contains a path
   - Todos will display target state if issuable target is 'Closed' or 'Merged'
   - Fix bug when sorting issues by milestone due date and filtering by two or more labels
   - Add support for using Yubikeys (U2F) for two-factor authentication
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index b0dcb52eba..a4a967c9bc 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -1,6 +1,4 @@
 class TeamcityService < CiService
-  include HTTParty
-
   prop_accessor :teamcity_url, :build_type, :username, :password
 
   validates :teamcity_url, presence: true, url: true, if: :activated?
@@ -64,15 +62,7 @@ class TeamcityService < CiService
   end
 
   def build_info(sha)
-    url = URI.join(
-      teamcity_url,
-      "/httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}"
-    ).to_s
-    auth = {
-      username: username,
-      password: password
-    }
-    @response = HTTParty.get(url, verify: false, basic_auth: auth)
+    @response = get_path("httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}")
   end
 
   def build_page(sha, ref)
@@ -81,14 +71,11 @@ class TeamcityService < CiService
     if @response.code != 200
       # If actual build link can't be determined,
       # send user to build summary page.
-      URI.join(teamcity_url, "/viewLog.html?buildTypeId=#{build_type}").to_s
+      build_url("viewLog.html?buildTypeId=#{build_type}")
     else
       # If actual build link is available, go to build result page.
       built_id = @response['build']['id']
-      URI.join(
-        teamcity_url,
-        "/viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}"
-      ).to_s
+      build_url("viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}")
     end
   end
 
@@ -123,8 +110,8 @@ class TeamcityService < CiService
 
     branch = Gitlab::Git.ref_name(data[:ref])
 
-    self.class.post(
-      URI.join(teamcity_url, '/httpAuth/app/rest/buildQueue').to_s,
+    HTTParty.post(
+      build_url('httpAuth/app/rest/buildQueue'),
       body: "<build branchName=\"#{branch}\">"\
             "<buildType id=\"#{build_type}\"/>"\
             '</build>',
@@ -132,4 +119,18 @@ class TeamcityService < CiService
       basic_auth: auth
     )
   end
+
+  private
+
+  def build_url(path)
+    URI.join("#{teamcity_url}/", path).to_s
+  end
+
+  def get_path(path)
+    HTTParty.get(build_url(path), verify: false,
+                                  basic_auth: {
+                                    username: username,
+                                    password: password
+                                  })
+  end
 end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index 24a708ca84..474715d24c 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -126,19 +126,19 @@ describe TeamcityService, models: true do
     it 'returns a specific URL when status is 500' do
       stub_request(status: 500)
 
-      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo')
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
     end
 
     it 'returns a build URL when teamcity_url has no trailing slash' do
       stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+      expect(service(teamcity_url: 'http://gitlab.com/teamcity').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
     end
 
     it 'returns a build URL when teamcity_url has a trailing slash' do
       stub_request(body: %Q({"build":{"id":"666"}}))
 
-      expect(service(teamcity_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+      expect(service(teamcity_url: 'http://gitlab.com/teamcity/').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
     end
   end
 
@@ -180,7 +180,7 @@ describe TeamcityService, models: true do
     end
   end
 
-  def service(teamcity_url: 'http://gitlab.com')
+  def service(teamcity_url: 'http://gitlab.com/teamcity')
     described_class.create(
       project: create(:empty_project),
       properties: {
@@ -193,7 +193,7 @@ describe TeamcityService, models: true do
   end
 
   def stub_request(status: 200, body: nil, build_status: 'success')
-    teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
+    teamcity_full_url = 'http://mic:password@gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
     body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
 
     WebMock.stub_request(:get, teamcity_full_url).to_return(