Merge branch 'master' into 8-9-stable

.rubocop.yml

@@ -349,7 +349,7 @@ Style/MultilineArrayBraceLayout:
 
 # Avoid multi-line chains of blocks.
 Style/MultilineBlockChain:
-  Enabled: false
+  Enabled: true
 
 # Ensures newlines after multiline block do statements.
 Style/MultilineBlockLayout:

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

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'

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

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

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

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)
 

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

app/views/projects/issues/_related_branches.html.haml

@@ -6,7 +6,7 @@
       %li
         - sha = @project.repository.find_branch(branch).target
         - pipeline = @project.pipeline(sha, branch) if sha
-        - if ci_copipelinemmit
+        - if pipeline
           %span.related-branch-ci-status
             = render_pipeline_status(pipeline)
         %span.related-branch-info

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)

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.

db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb

+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

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
@@ -751,8 +751,6 @@ ActiveRecord::Schema.define(version: 20160608155312) 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"
@@ -988,7 +986,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"

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

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.

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)

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

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

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,11 +37,11 @@ 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
 
-  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

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

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 }