Merge branch 'master' into awardables

.rubocop.yml

@@ -937,10 +937,9 @@ Lint/Void:
 
 ##################### Performance ############################
 
-# TODO: Enable Casecmp Cop.
 # Use `casecmp` rather than `downcase ==`.
 Performance/Casecmp:
-  Enabled: false
+  Enabled: true
 
 # TODO: Enable DoubleStartEndWith Cop.
 # Use `str.{start,end}_with?(x, ..., y, ...)` instead of
@@ -990,11 +989,12 @@ Performance/RedundantSortBy:
 # string.
 Performance/StartWith:
   Enabled: false
+
 # Use `tr` instead of `gsub` when you are replacing the same number of
 # characters. Use `delete` instead of `gsub` when you are deleting
 # characters.
 Performance/StringReplacement:
-  Enabled: false
+  Enabled: true
 
 # TODO: Enable TimesMap Cop.
 # Checks for `.times.map` calls.

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.8.0 (unreleased)
+  - Fix error when using link to uploads in global snippets
   - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
   - Use a case-insensitive comparison in sanitizing URI schemes
   - Project#open_branches has been cleaned up and no longer loads entire records into memory.
@@ -20,6 +21,7 @@ v 8.8.0 (unreleased)
   - Fix error when visiting commit builds page before build was updated
   - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project
   - Update SVG sanitizer to conform to SVG 1.1
+  - Speed up push emails with multiple recipients by only generating the email once
   - Updated search UI
   - Display informative message when new milestone is created
   - Sanitize milestones and labels titles
@@ -31,6 +33,7 @@ v 8.8.0 (unreleased)
   - API: Expose Issue#user_notes_count. !3126 (Anton Popov)
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
   - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
+  - Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724
   - Added multiple colors for labels in dropdowns when dups happen.
   - Improve description for the Two-factor Authentication sign-in screen. (Connor Shea)
   - API support for the 'since' and 'until' operators on commit requests (Paco Guzman)
@@ -38,10 +41,16 @@ v 8.8.0 (unreleased)
   - Expire repository exists? and has_visible_content? caches after a push if necessary
   - Fix unintentional filtering bug in issues sorted by milestone due (Takuya Noguchi)
   - Fix adding a todo for private group members (Ahmad Sherif)
+  - Bump ace-rails-ap gem version from 2.0.1 to 4.0.2 which upgrades Ace Editor from 1.1.2 to 1.2.3
 
 v 8.7.4
-  - Fix always showing build notification message when switching between merge requests
-  - Links for Redmine issue references are generated correctly again (Benedikt Huss)
+  - Links for Redmine issue references are generated correctly again !4048 (Benedikt Huss)
+  - Fix setting trusted proxies !3970
+  - Fix BitBucket importer bug when throwing exceptions !3941
+  - Use sign out path only if not empty !3989
+  - Running rake gitlab:db:drop_tables now drops tables with cascade !4020
+  - Running rake gitlab:db:drop_tables uses "IF EXISTS" as a precaution !4100
+  - Use a case-insensitive comparison in sanitizing URI schemes
 
 v 8.7.3
   - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented

Gemfile

@@ -197,7 +197,7 @@ gem 'licensee', '~> 8.0.0'
 gem "rack-attack", '~> 4.3.1'
 
 # Ace editor
-gem 'ace-rails-ap', '~> 2.0.1'
+gem 'ace-rails-ap', '~> 4.0.2'
 
 # Keyboard shortcuts
 gem 'mousetrap-rails', '~> 1.4.6'
@@ -218,7 +218,6 @@ gem 'gitlab_emoji',       '~> 0.3.0'
 gem 'gon',                '~> 6.0.1'
 gem 'jquery-atwho-rails', '~> 1.3.2'
 gem 'jquery-rails',       '~> 4.1.0'
-gem 'jquery-scrollto-rails', '~> 1.4.3'
 gem 'jquery-ui-rails',    '~> 5.0.0'
 gem 'raphael-rails',      '~> 2.1.2'
 gem 'request_store',      '~> 1.3.0'

Gemfile.lock

@@ -3,7 +3,7 @@ GEM
   specs:
     CFPropertyList (2.3.2)
     RedCloth (4.2.9)
-    ace-rails-ap (2.0.1)
+    ace-rails-ap (4.0.2)
     actionmailer (4.2.6)
       actionpack (= 4.2.6)
       actionview (= 4.2.6)
@@ -432,8 +432,6 @@ GEM
       rails-dom-testing (>= 1, < 3)
       railties (>= 4.2.0)
       thor (>= 0.14, < 2.0)
-    jquery-scrollto-rails (1.4.3)
-      railties (> 3.1, < 5.0)
     jquery-turbolinks (2.1.0)
       railties (>= 3.1.0)
       turbolinks
@@ -882,7 +880,7 @@ PLATFORMS
 
 DEPENDENCIES
   RedCloth (~> 4.2.9)
-  ace-rails-ap (~> 2.0.1)
+  ace-rails-ap (~> 4.0.2)
   activerecord-deprecated_finders (~> 1.0.3)
   activerecord-session_store (~> 0.1.0)
   acts-as-taggable-on (~> 3.4)
@@ -953,7 +951,6 @@ DEPENDENCIES
   influxdb (~> 0.2)
   jquery-atwho-rails (~> 1.3.2)
   jquery-rails (~> 4.1.0)
-  jquery-scrollto-rails (~> 1.4.3)
   jquery-turbolinks (~> 2.1.0)
   jquery-ui-rails (~> 5.0.0)
   kaminari (~> 0.16.3)

app/assets/javascripts/ci/application.js.coffee

-# This is a manifest file that'll be compiled into application.js, which will include all the files
-# listed below.
-#
-# Any JavaScript/Coffee file within this directory, lib/assets/javascripts, vendor/assets/javascripts,
-# or vendor/assets/javascripts of plugins, if any, can be referenced here using a relative path.
-#
-# It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the
-# the compiled file.
-#
-# WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD
-# GO AFTER THE REQUIRES BELOW.
-#
 #= require pager
 #= require jquery_nested_form
 #= require_tree .
-#
-$(document).on 'click', '.edit-runner-link', (event) ->
-  event.preventDefault()
-
-  descr = $(this).closest('.runner-description').first()
-  descr.addClass('hide')
-  form = descr.next('.runner-description-form')
-  descrInput = form.find('input.description')
-  originalValue = descrInput.val()
-  form.removeClass('hide')
-  form.find('.cancel').on 'click', (event) ->
-    event.preventDefault()
-
-    form.addClass('hide')
-    descrInput.val(originalValue)
-    descr.removeClass('hide')
 
 $(document).on 'click', '.assign-all-runner', ->
   $(this).replaceWith('<i class="fa fa-refresh fa-spin"></i> Assign in progress..')

app/assets/javascripts/shortcuts_issuable.coffee

@@ -6,6 +6,10 @@ class @ShortcutsIssuable extends ShortcutsNavigation
     super()
     Mousetrap.bind('a', @openSidebarDropdown.bind(@, 'assignee'))
     Mousetrap.bind('m', @openSidebarDropdown.bind(@, 'milestone'))
+    Mousetrap.bind('r', =>
+      @replyWithSelectedText()
+      return false
+    )
     Mousetrap.bind('j', =>
       @prevIssue()
       return false

app/controllers/dashboard/labels_controller.rb

 class Dashboard::LabelsController < Dashboard::ApplicationController
   def index
-    labels = Label.where(project_id: projects).select(:title, :color).uniq(:title)
+    labels = Label.where(project_id: projects).select(:id, :title, :color).uniq(:title)
 
     respond_to do |format|
       format.json { render json: labels }

app/controllers/dashboard/projects_controller.rb

@@ -28,7 +28,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   end
 
   def starred
-    @projects = current_user.starred_projects.sorted_by_activity
+    @projects = current_user.viewable_starred_projects.sorted_by_activity
     @projects = filter_projects(@projects)
     @projects = @projects.includes(:namespace, :forked_from_project, :tags)
     @projects = @projects.sort(@sort = params[:sort])

app/controllers/dashboard_controller.rb

@@ -25,7 +25,7 @@ class DashboardController < Dashboard::ApplicationController
   def load_events
     projects =
       if params[:filter] == "starred"
-        current_user.starred_projects
+        current_user.viewable_starred_projects
       else
         current_user.authorized_projects
       end

app/controllers/projects/hooks_controller.rb

@@ -27,8 +27,10 @@ class Projects::HooksController < Projects::ApplicationController
     if !@project.empty_repo?
       status, message = TestHookService.new.execute(hook, current_user)
 
-      if status
-        flash[:notice] = 'Hook successfully executed.'
+      if status && status >= 200 && status < 400
+        flash[:notice] = "Hook executed successfully: HTTP #{status}"
+      elsif status
+        flash[:alert] = "Hook executed successfully but returned HTTP #{status} #{message}"
       else
         flash[:alert] = "Hook execution failed: #{message}"
       end

app/mailers/emails/projects.rb

@@ -59,9 +59,9 @@ module Emails
            subject: subject("Project was moved"))
     end
 
-    def repository_push_email(project_id, recipient, opts = {})
+    def repository_push_email(project_id, opts = {})
       @message =
-        Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts)
+        Gitlab::Email::Message::RepositoryPush.new(self, project_id, opts)
 
       # used in notify layout
       @target_url = @message.target_url
@@ -72,7 +72,6 @@ module Emails
 
       mail(from:      sender(@message.author_id, @message.send_from_committer_email?),
            reply_to:  @message.reply_to,
-           to:        @message.recipient,
            subject:   @message.subject)
     end
   end

app/models/hooks/web_hook.rb

@@ -38,7 +38,7 @@ class WebHook < ActiveRecord::Base
                               basic_auth: auth)
     end
 
-    [(response.code >= 200 && response.code < 300), ActionView::Base.full_sanitizer.sanitize(response.to_s)]
+    [response.code, response.to_s]
   rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
     logger.error("WebHook Error => #{e}")
     [false, e.to_s]

app/models/repository.rb

@@ -81,7 +81,7 @@ class Repository
   def commit(id = 'HEAD')
     return nil unless exists?
     commit = Gitlab::Git::Commit.find(raw_repository, id)
-    commit = Commit.new(commit, @project) if commit
+    commit = ::Commit.new(commit, @project) if commit
     commit
   rescue Rugged::OdbError
     nil
@@ -453,7 +453,7 @@ class Repository
   def version
     cache.fetch(:version) do
       tree(:head).blobs.find do |file|
-        file.name.downcase == 'version'
+        file.name.casecmp('version').zero?
       end
     end
   end

app/models/user.rb

@@ -382,6 +382,11 @@ class User < ActiveRecord::Base
     Project.where("projects.id IN (#{projects_union.to_sql})")
   end
 
+  def viewable_starred_projects
+    starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})",
+                           [Project::PUBLIC, Project::INTERNAL])
+  end
+
   def owned_projects
     @owned_projects ||=
       Project.where('namespace_id IN (?) OR namespace_id = ?',

app/services/projects/destroy_service.rb

@@ -35,7 +35,7 @@ module Projects
         end
       end
 
-      log_info("Project \"#{project.name}\" was removed")
+      log_info("Project \"#{project.path_with_namespace}\" was removed")
       system_hook_service.execute_hooks_for(project, :destroy)
       true
     end

app/views/admin/runners/_runner.html.haml

@@ -11,18 +11,10 @@
     = link_to admin_runner_path(runner) do
       = runner.short_sha
   %td
-    .runner-description
-      = runner.description
-      %span (#{link_to 'edit', '#', class: 'edit-runner-link'})
-    .runner-description-form.hide
-      = form_for [:admin, runner], remote: true, html: { class: 'form-inline' } do |f|
-        .form-group
-          = f.text_field :description, class: 'form-control'
-        = f.submit 'Save', class: 'btn'
-        %span (#{link_to 'cancel', '#', class: 'cancel'})
+    = runner.description
   %td
     - if runner.shared?
-      \-
+      n/a
     - else
       = runner.projects.count(:all)
   %td

app/views/layouts/header/_default.html.haml

@@ -27,8 +27,9 @@
             %li
               = link_to dashboard_todos_path, title: 'Todos', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
                 = icon('bell fw')
-                %span.badge.todos-pending-count
-                  = todos_pending_count
+                - unless todos_pending_count == 0
+                  %span.badge.todos-pending-count
+                    = todos_pending_count
             - if current_user.can_create_project?
               %li
                 = link_to new_project_path, title: 'New project', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do

app/workers/emails_on_push_worker.rb

 class EmailsOnPushWorker
   include Sidekiq::Worker
 
+  attr_reader :email, :skip_premailer
+
   def perform(project_id, recipients, push_data, options = {})
     options.symbolize_keys!
     options.reverse_merge!(
@@ -41,11 +43,11 @@ class EmailsOnPushWorker
       end
     end
 
-    recipients.split(" ").each do |recipient|
+    recipients.split.each do |recipient|
       begin
-        Notify.repository_push_email(
-          project_id,
+        send_email(
           recipient,
+          project_id,
           author_id:                  author_id,
           ref:                        ref,
           action:                     action,
@@ -53,14 +55,29 @@ class EmailsOnPushWorker
           reverse_compare:            reverse_compare,
           send_from_committer_email:  send_from_committer_email,
           disable_diffs:              disable_diffs
-        ).deliver_now
+        )
+
       # These are input errors and won't be corrected even if Sidekiq retries
       rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e
         logger.info("Failed to send e-mail for project '#{project.name_with_namespace}' to #{recipient}: #{e}")
       end
     end
   ensure
+    @email = nil
     compare = nil
     GC.start
   end
+
+  private
+
+  def send_email(recipient, project_id, options)
+    # Generating the body of this email can be expensive, so only do it once
+    @skip_premailer ||= email.present?
+    @email ||= Notify.repository_push_email(project_id, options)
+
+    email.to = recipient
+    email.add_message_id
+    email.header[:skip_premailer] = true if skip_premailer
+    email.deliver_now
+  end
 end

config/application.rb

 require File.expand_path('../boot', __FILE__)
 
 require 'rails/all'
-require 'devise'
-I18n.config.enforce_available_locales = false
+
 Bundler.require(:default, Rails.env)
-require_relative '../lib/gitlab/redis'
 
 module Gitlab
   class Application < Rails::Application
+    require_dependency Rails.root.join('lib/gitlab/redis')
+
     # Settings in config/environments/* take precedence over those specified here.
     # Application configuration should go into files in config/initializers
     # -- all .rb files in that directory are automatically loaded.
 
-    # Custom directories with classes and modules you want to be autoloadable.
-    config.autoload_paths.push(*%W(#{config.root}/lib
-                                   #{config.root}/app/models/hooks
-                                   #{config.root}/app/models/concerns
-                                   #{config.root}/app/models/project_services
-                                   #{config.root}/app/models/members))
+    # Sidekiq uses eager loading, but directories not in the standard Rails
+    # directories must be added to the eager load paths:
+    # https://github.com/mperham/sidekiq/wiki/FAQ#why-doesnt-sidekiq-autoload-my-rails-application-code
+    # Also, there is no need to add `lib` to autoload_paths since autoloading is
+    # configured to check for eager loaded paths:
+    # https://github.com/rails/rails/blob/v4.2.6/railties/lib/rails/engine.rb#L687
+    # This is a nice reference article on autoloading/eager loading:
+    # http://blog.arkency.com/2014/11/dont-forget-about-eager-load-when-extending-autoload
+    config.eager_load_paths.push(*%W(#{config.root}/lib
+                                     #{config.root}/app/models/ci
+                                     #{config.root}/app/models/hooks
+                                     #{config.root}/app/models/members
+                                     #{config.root}/app/models/project_services))
 
     # Only load the plugins named here, in the order given (default is alphabetical).
     # :all can be used as a placeholder for all plugins not explicitly named.

config/initializers/1_settings.rb

-require 'gitlab' # Load lib/gitlab.rb as soon as possible
+require_dependency Rails.root.join('lib/gitlab') # Load Gitlab as soon as possible
 
 class Settings < Settingslogic
   source ENV.fetch('GITLAB_CONFIG') { "#{Rails.root}/config/gitlab.yml" }

config/initializers/5_backend.rb

 # GIT over HTTP
-require Rails.root.join("lib", "gitlab", "backend", "grack_auth")
+require_dependency Rails.root.join('lib/gitlab/backend/grack_auth')
 
 # GIT over SSH
-require Rails.root.join("lib", "gitlab", "backend", "shell")
+require_dependency Rails.root.join('lib/gitlab/backend/shell')
 
 # GitLab shell adapter
-require Rails.root.join("lib", "gitlab", "backend", "shell_adapter")
+require_dependency Rails.root.join('lib/gitlab/backend/shell_adapter')
 
 required_version = Gitlab::VersionInfo.parse(Gitlab::Shell.version_required)
 current_version = Gitlab::VersionInfo.parse(Gitlab::Shell.new.version)

config/initializers/monkey_patch.rb

-## This patch is from rails 4.2-stable. Remove it when 4.2.6 is released
-## https://github.com/rails/rails/issues/21108
-
-module ActiveRecord
-  module ConnectionAdapters
-    class AbstractMysqlAdapter < AbstractAdapter
-      # SHOW VARIABLES LIKE 'name'
-      def show_variable(name)
-        variables = select_all("select @@#{name} as 'Value'", 'SCHEMA')
-        variables.first['Value'] unless variables.empty?
-      rescue ActiveRecord::StatementInvalid
-        nil
-      end
-
-      
-      # MySQL is too stupid to create a temporary table for use subquery, so we have
-      # to give it some prompting in the form of a subsubquery. Ugh!
-      def subquery_for(key, select)
-        subsubselect = select.clone
-        subsubselect.projections = [key]
-
-        subselect = Arel::SelectManager.new(select.engine)
-        subselect.project Arel.sql(key.name)
-        # Materialized subquery by adding distinct
-        # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
-        subselect.from subsubselect.distinct.as('__active_record_temp')
-      end
-    end
-  end
-end
-
-module ActiveRecord
-  module ConnectionAdapters
-    class MysqlAdapter < AbstractMysqlAdapter
-      ADAPTER_NAME = 'MySQL'.freeze
-
-      # Get the client encoding for this database
-      def client_encoding
-        return @client_encoding if @client_encoding
-
-        result = exec_query(
-          "select @@character_set_client",
-          'SCHEMA')
-        @client_encoding = ENCODINGS[result.rows.last.last]
-      end
-    end
-  end
-end

doc/web_hooks/web_hooks.md

@@ -13,6 +13,19 @@ You can configure webhooks to listen for specific events like pushes, issues or
 
 Webhooks can be used to update an external issue tracker, trigger CI builds, update a backup mirror, or even deploy to your production server.
 
+## Webhook endpoint tips
+
+If you are writing your own endpoint (web server) that will receive
+GitLab webhooks keep in mind the following things:
+
+-   Your endpoint should send its HTTP response as fast as possible. If
+    you wait too long, GitLab may decide the hook failed and retry it.
+-   Your endpoint should ALWAYS return a valid HTTP response. If you do
+    not do this then GitLab will think the hook failed and retry it.
+    Most HTTP libraries take care of this for you automatically but if
+    you are writing a low-level hook this is important to remember.
+-   GitLab ignores the HTTP status code returned by your endpoint.
+
 ## SSL Verification
 
 By default, the SSL certificate of the webhook endpoint is verified based on

features/steps/project/hooks.rb

@@ -59,7 +59,7 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps
   step 'hook should be triggered' do
     expect(current_path).to eq namespace_project_hooks_path(current_project.namespace, current_project)
     expect(page).to have_selector '.flash-notice',
-                              text: 'Hook successfully executed.'
+                              text: 'Hook executed successfully: HTTP 200'
   end
 
   step 'I should see hook error message' do

lib/api/api.rb

-Dir["#{Rails.root}/lib/api/*.rb"].each {|file| require file}
-
 module API
   class API < Grape::API
     include APIGuard
@@ -25,38 +23,39 @@ module API
     format :json
     content_type :txt, "text/plain"
 
-    helpers Helpers
-
-    mount Groups
-    mount GroupMembers
-    mount Users
-    mount Projects
-    mount Repositories
-    mount Issues
-    mount Milestones
-    mount Session
-    mount MergeRequests
-    mount Notes
-    mount Internal
-    mount SystemHooks
-    mount ProjectSnippets
-    mount ProjectMembers
-    mount DeployKeys
-    mount ProjectHooks
-    mount Services
-    mount Files
-    mount Commits
-    mount CommitStatus
-    mount Namespaces
-    mount Branches
-    mount Labels
-    mount Settings
-    mount Keys
-    mount Tags
-    mount Triggers
-    mount Builds
-    mount Variables
-    mount Runners
-    mount Licenses
+    # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
+    helpers ::API::Helpers
+
+    mount ::API::Groups
+    mount ::API::GroupMembers
+    mount ::API::Users
+    mount ::API::Projects
+    mount ::API::Repositories
+    mount ::API::Issues
+    mount ::API::Milestones
+    mount ::API::Session
+    mount ::API::MergeRequests
+    mount ::API::Notes
+    mount ::API::Internal
+    mount ::API::SystemHooks
+    mount ::API::ProjectSnippets
+    mount ::API::ProjectMembers
+    mount ::API::DeployKeys
+    mount ::API::ProjectHooks
+    mount ::API::Services
+    mount ::API::Files
+    mount ::API::Commits
+    mount ::API::CommitStatuses
+    mount ::API::Namespaces
+    mount ::API::Branches
+    mount ::API::Labels
+    mount ::API::Settings
+    mount ::API::Keys
+    mount ::API::Tags
+    mount ::API::Triggers
+    mount ::API::Builds
+    mount ::API::Variables
+    mount ::API::Runners
+    mount ::API::Licenses
   end
 end

lib/api/api_guard.rb

@@ -2,171 +2,175 @@
 
 require 'rack/oauth2'
 
-module APIGuard
-  extend ActiveSupport::Concern
+module API
+  module APIGuard
+    extend ActiveSupport::Concern
 
-  included do |base|
-    # OAuth2 Resource Server Authentication
-    use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
-      # The authenticator only fetches the raw token string
+    included do |base|
+      # OAuth2 Resource Server Authentication
+      use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
+        # The authenticator only fetches the raw token string
 
-      # Must yield access token to store it in the env
-      request.access_token
-    end
+        # Must yield access token to store it in the env
+        request.access_token
+      end
 
-    helpers HelperMethods
+      helpers HelperMethods
 
-    install_error_responders(base)
-  end
+      install_error_responders(base)
+    end
 
-  # Helper Methods for Grape Endpoint
-  module HelperMethods
-    # Invokes the doorkeeper guard.
-    #
-    # If token is presented and valid, then it sets @current_user.
-    #
-    # If the token does not have sufficient scopes to cover the requred scopes,
-    # then it raises InsufficientScopeError.
-    #
-    # If the token is expired, then it raises ExpiredError.
-    #
-    # If the token is revoked, then it raises RevokedError.
-    #
-    # If the token is not found (nil), then it raises TokenNotFoundError.
-    #
-    # Arguments:
-    #
-    #   scopes: (optional) scopes required for this guard.
-    #           Defaults to empty array.
-    #
-    def doorkeeper_guard!(scopes: [])
-      if (access_token = find_access_token).nil?
-        raise TokenNotFoundError
-
-      else
-        case validate_access_token(access_token, scopes)
-        when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-          raise InsufficientScopeError.new(scopes)
-        when Oauth2::AccessTokenValidationService::EXPIRED
-          raise ExpiredError
-        when Oauth2::AccessTokenValidationService::REVOKED
-          raise RevokedError
-        when Oauth2::AccessTokenValidationService::VALID
-          @current_user = User.find(access_token.resource_owner_id)
+    # Helper Methods for Grape Endpoint
+    module HelperMethods
+      # Invokes the doorkeeper guard.
+      #
+      # If token is presented and valid, then it sets @current_user.
+      #
+      # If the token does not have sufficient scopes to cover the requred scopes,
+      # then it raises InsufficientScopeError.
+      #
+      # If the token is expired, then it raises ExpiredError.
+      #
+      # If the token is revoked, then it raises RevokedError.
+      #
+      # If the token is not found (nil), then it raises TokenNotFoundError.
+      #
+      # Arguments:
+      #
+      #   scopes: (optional) scopes required for this guard.
+      #           Defaults to empty array.
+      #
+      def doorkeeper_guard!(scopes: [])
+        if (access_token = find_access_token).nil?
+          raise TokenNotFoundError
+
+        else
+          case validate_access_token(access_token, scopes)
+          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
+            raise InsufficientScopeError.new(scopes)
+          when Oauth2::AccessTokenValidationService::EXPIRED
+            raise ExpiredError
+          when Oauth2::AccessTokenValidationService::REVOKED
+            raise RevokedError
+          when Oauth2::AccessTokenValidationService::VALID
+            @current_user = User.find(access_token.resource_owner_id)
+          end
         end
       end
-    end
 
-    def doorkeeper_guard(scopes: [])
-      if access_token = find_access_token
-        case validate_access_token(access_token, scopes)
-        when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
-          raise InsufficientScopeError.new(scopes)
+      def doorkeeper_guard(scopes: [])
+        if access_token = find_access_token
+          case validate_access_token(access_token, scopes)
+          when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE
+            raise InsufficientScopeError.new(scopes)
 
-        when Oauth2::AccessTokenValidationService::EXPIRED
-          raise ExpiredError
+          when Oauth2::AccessTokenValidationService::EXPIRED
+            raise ExpiredError
 
-        when Oauth2::AccessTokenValidationService::REVOKED
-          raise RevokedError
+          when Oauth2::AccessTokenValidationService::REVOKED
+            raise RevokedError
 
-        when Oauth2::AccessTokenValidationService::VALID
-          @current_user = User.find(access_token.resource_owner_id)
+          when Oauth2::AccessTokenValidationService::VALID
+            @current_user = User.find(access_token.resource_owner_id)
+          end
         end
       end
-    end
 
-    def current_user
-      @current_user
-    end
+      def current_user
+        @current_user
+      end
 
-    private
-    def find_access_token
-      @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
-    end
+      private
 
-    def doorkeeper_request
-      @doorkeeper_request ||= ActionDispatch::Request.new(env)
-    end
+      def find_access_token
+        @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
+      end
 
-    def validate_access_token(access_token, scopes)
-      Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
-    end
-  end
+      def doorkeeper_request
+        @doorkeeper_request ||= ActionDispatch::Request.new(env)
+      end
 
-  module ClassMethods
-    # Installs the doorkeeper guard on the whole Grape API endpoint.
-    #
-    # Arguments:
-    #
-    #   scopes: (optional) scopes required for this guard.
-    #           Defaults to empty array.
-    #
-    def guard_all!(scopes: [])
-      before do
-        guard! scopes: scopes
+      def validate_access_token(access_token, scopes)
+        Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
       end
     end
 
-    private
-    def install_error_responders(base)
-      error_classes = [ MissingTokenError, TokenNotFoundError,
-                        ExpiredError, RevokedError, InsufficientScopeError]
+    module ClassMethods
+      # Installs the doorkeeper guard on the whole Grape API endpoint.
+      #
+      # Arguments:
+      #
+      #   scopes: (optional) scopes required for this guard.
+      #           Defaults to empty array.
+      #
+      def guard_all!(scopes: [])
+        before do
+          guard! scopes: scopes
+        end
+      end
 
-      base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler
-    end
+      private
 
-    def oauth2_bearer_token_error_handler
-      Proc.new do |e|
-        response =
-          case e
-          when MissingTokenError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new
-
-          when TokenNotFoundError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Bad Access Token.")
-
-          when ExpiredError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Token is expired. You can either do re-authorization or token refresh.")
-
-          when RevokedError
-            Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
-              :invalid_token,
-              "Token was revoked. You have to re-authorize from the user.")
-
-          when InsufficientScopeError
-            # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
-            # does not include WWW-Authenticate header, which breaks the standard.
-            Rack::OAuth2::Server::Resource::Bearer::Forbidden.new(
-              :insufficient_scope,
-              Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope],
-              { scope: e.scopes })
-          end
+      def install_error_responders(base)
+        error_classes = [ MissingTokenError, TokenNotFoundError,
+                          ExpiredError, RevokedError, InsufficientScopeError]
 
-        response.finish
+        base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler
+      end
+
+      def oauth2_bearer_token_error_handler
+        Proc.new do |e|
+          response =
+            case e
+            when MissingTokenError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new
+
+            when TokenNotFoundError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Bad Access Token.")
+
+            when ExpiredError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token is expired. You can either do re-authorization or token refresh.")
+
+            when RevokedError
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token was revoked. You have to re-authorize from the user.")
+
+            when InsufficientScopeError
+              # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
+              # does not include WWW-Authenticate header, which breaks the standard.
+              Rack::OAuth2::Server::Resource::Bearer::Forbidden.new(
+                :insufficient_scope,
+                Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope],
+                { scope: e.scopes })
+            end
+
+          response.finish
+        end
       end
     end
-  end
 
-  #
-  # Exceptions
-  #
+    #
+    # Exceptions
+    #
 
-  class MissingTokenError < StandardError; end
+    class MissingTokenError < StandardError; end
 
-  class TokenNotFoundError < StandardError; end
+    class TokenNotFoundError < StandardError; end
 
-  class ExpiredError < StandardError; end
+    class ExpiredError < StandardError; end
 
-  class RevokedError < StandardError; end
+    class RevokedError < StandardError; end
 
-  class InsufficientScopeError < StandardError
-    attr_reader :scopes
-    def initialize(scopes)
-      @scopes = scopes
+    class InsufficientScopeError < StandardError
+      attr_reader :scopes
+      def initialize(scopes)
+        @scopes = scopes
+      end
     end
   end
 end

lib/api/commit_statuses.rb

@@ -2,7 +2,7 @@ require 'mime/types'
 
 module API
   # Project commit statuses API
-  class CommitStatus < Grape::API
+  class CommitStatuses < Grape::API
     resource :projects do
       before { authenticate! }
 

lib/api/projects.rb

@@ -44,7 +44,7 @@ module API
       # Example Request:
       #   GET /projects/starred
       get '/starred' do
-        @projects = current_user.starred_projects
+        @projects = current_user.viewable_starred_projects
         @projects = filter_projects(@projects)
         @projects = paginate @projects
         present @projects, with: Entities::Project

lib/banzai/filter/upload_link_filter.rb

@@ -8,6 +8,8 @@ module Banzai
     #
     class UploadLinkFilter < HTML::Pipeline::Filter
       def call
+        return doc unless project
+
         doc.search('a').each do |el|
           process_link_attr el.attribute('href')
         end
@@ -31,7 +33,11 @@ module Banzai
       end
 
       def build_url(uri)
-        File.join(Gitlab.config.gitlab.url, context[:project].path_with_namespace, uri)
+        File.join(Gitlab.config.gitlab.url, project.path_with_namespace, uri)
+      end
+
+      def project
+        context[:project]
       end
 
       # Ensure that a :project key exists in context

lib/ci/api/api.rb

-Dir["#{Rails.root}/lib/ci/api/*.rb"].each {|file| require file}
-
 module Ci
   module API
     class API < Grape::API
-      include APIGuard
+      include ::API::APIGuard
       version 'v1', using: :path
 
       rescue_from ActiveRecord::RecordNotFound do
@@ -31,9 +29,9 @@ module Ci
       helpers ::API::Helpers
       helpers Gitlab::CurrentSettings
 
-      mount Builds
-      mount Runners
-      mount Triggers
+      mount ::Ci::API::Builds
+      mount ::Ci::API::Runners
+      mount ::Ci::API::Triggers
     end
   end
 end

lib/gitlab.rb

-require 'gitlab/git'
+require_dependency 'gitlab/git'
 
 module Gitlab
   def self.com?

lib/gitlab/database.rb

@@ -5,11 +5,11 @@ module Gitlab
     end
 
     def self.mysql?
-      adapter_name.downcase == 'mysql2'
+      adapter_name.casecmp('mysql2').zero?
     end
 
     def self.postgresql?
-      adapter_name.downcase == 'postgresql'
+      adapter_name.casecmp('postgresql').zero?
     end
 
     def self.version

lib/gitlab/diff/parser.rb

@@ -18,7 +18,7 @@ module Gitlab
           @lines.each do |line|
             next if filename?(line)
   
-            full_line = line.gsub(/\n/, '')
+            full_line = line.delete("\n")
   
             if line.match(/^@@ -/)
               type = "match"

lib/gitlab/email/message/repository_push.rb

@@ -2,7 +2,6 @@ module Gitlab
   module Email
     module Message
       class RepositoryPush
-        attr_accessor :recipient
         attr_reader :author_id, :ref, :action
 
         include Gitlab::Routing.url_helpers
@@ -11,13 +10,12 @@ module Gitlab
         delegate :name, to: :author, prefix: :author
         delegate :username, to: :author, prefix: :author
 
-        def initialize(notify, project_id, recipient, opts = {})
+        def initialize(notify, project_id, opts = {})
           raise ArgumentError, 'Missing options: author_id, ref, action' unless
             opts[:author_id] && opts[:ref] && opts[:action]
 
           @notify = notify
           @project_id = project_id
-          @recipient = recipient
           @opts = opts.dup
 
           @author_id = @opts.delete(:author_id)

lib/gitlab/markup_helper.rb

@@ -40,7 +40,7 @@ module Gitlab
     # Returns boolean
     def plain?(filename)
       filename.downcase.end_with?('.txt') ||
-        filename.downcase == 'readme'
+        filename.casecmp('readme').zero?
     end
 
     def previewable?(filename)

spec/features/merge_requests/user_lists_merge_requests_spec.rb

+require 'spec_helper'
+
+describe 'Projects > Merge requests > User lists merge requests', feature: true do
+  include SortingHelper
+
+  let(:project) { create(:project, :public) }
+  let(:user) { create(:user) }
+
+  before do
+    @fix = create(:merge_request,
+                  title: 'fix',
+                  source_project: project,
+                  source_branch: 'fix',
+                  assignee: user,
+                  milestone: create(:milestone, due_date: '2013-12-11'),
+                  created_at: 1.minute.ago,
+                  updated_at: 1.minute.ago)
+    create(:merge_request,
+           title: 'markdown',
+           source_project: project,
+           source_branch: 'markdown',
+           assignee: user,
+           milestone: create(:milestone, due_date: '2013-12-12'),
+           created_at: 2.minutes.ago,
+           updated_at: 2.minutes.ago)
+    create(:merge_request,
+           title: 'lfs',
+           source_project: project,
+           source_branch: 'lfs',
+           created_at: 3.minutes.ago,
+           updated_at: 10.seconds.ago)
+  end
+
+  it 'filters on no assignee' do
+    visit_merge_requests(project, assignee_id: IssuableFinder::NONE)
+
+    expect(current_path).to eq(namespace_project_merge_requests_path(project.namespace, project))
+    expect(page).to have_content 'lfs'
+    expect(page).not_to have_content 'fix'
+    expect(page).not_to have_content 'markdown'
+  end
+
+  it 'filters on a specific assignee' do
+    visit_merge_requests(project, assignee_id: user.id)
+
+    expect(page).not_to have_content 'lfs'
+    expect(page).to have_content 'fix'
+    expect(page).to have_content 'markdown'
+  end
+
+  it 'sorts by newest' do
+    visit_merge_requests(project, sort: sort_value_recently_created)
+
+    expect(first_merge_request).to include('lfs')
+    expect(last_merge_request).to include('fix')
+  end
+
+  it 'sorts by oldest' do
+    visit_merge_requests(project, sort: sort_value_oldest_created)
+
+    expect(first_merge_request).to include('fix')
+    expect(last_merge_request).to include('lfs')
+  end
+
+  it 'sorts by last updated' do
+    visit_merge_requests(project, sort: sort_value_recently_updated)
+
+    expect(first_merge_request).to include('lfs')
+  end
+
+  it 'sorts by oldest updated' do
+    visit_merge_requests(project, sort: sort_value_oldest_updated)
+
+    expect(first_merge_request).to include('markdown')
+  end
+
+  it 'sorts by milestone due soon' do
+    visit_merge_requests(project, sort: sort_value_milestone_soon)
+
+    expect(first_merge_request).to include('fix')
+  end
+
+  it 'sorts by milestone due later' do
+    visit_merge_requests(project, sort: sort_value_milestone_later)
+
+    expect(first_merge_request).to include('markdown')
+  end
+
+  it 'filters on one label and sorts by due soon' do
+    label = create(:label, project: project)
+    create(:label_link, label: label, target: @fix)
+
+    visit_merge_requests(project, label_name: [label.name],
+                                  sort: sort_value_due_date_soon)
+
+    expect(first_merge_request).to include('fix')
+  end
+
+  context 'while filtering on two labels' do
+    let(:label) { create(:label, project: project) }
+    let(:label2) { create(:label, project: project) }
+
+    before do
+      create(:label_link, label: label, target: @fix)
+      create(:label_link, label: label2, target: @fix)
+    end
+
+    it 'sorts by due soon' do
+      visit_merge_requests(project, label_name: [label.name, label2.name],
+                                    sort: sort_value_due_date_soon)
+
+      expect(first_merge_request).to include('fix')
+    end
+
+    context 'filter on assignee and' do
+      it 'sorts by due soon' do
+        visit_merge_requests(project, label_name: [label.name, label2.name],
+                                      assignee_id: user.id,
+                                      sort: sort_value_due_date_soon)
+
+        expect(first_merge_request).to include('fix')
+      end
+    end
+  end
+
+  def visit_merge_requests(project, opts = {})
+    visit namespace_project_merge_requests_path(project.namespace, project, opts)
+  end
+
+  def first_merge_request
+    page.all('ul.mr-list > li').first.text
+  end
+
+  def last_merge_request
+    page.all('ul.mr-list > li').last.text
+  end
+end

spec/lib/banzai/filter/upload_link_filter_spec.rb

@@ -8,6 +8,10 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do
       project: project
     })
 
+    raw_filter(doc, contexts)
+  end
+
+  def raw_filter(doc, contexts = {})
     described_class.call(doc, contexts)
   end
 
@@ -70,4 +74,18 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do
       expect(doc.at_css('img')['src']).to match "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/%ED%95%9C%EA%B8%80.png"
     end
   end
+
+  context 'when project context does not exist' do
+    let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') }
+
+    it 'does not raise error' do
+      expect { raw_filter(upload_link, project: nil) }.not_to raise_error
+    end
+
+    it 'does not rewrite link' do
+      doc = raw_filter(upload_link, project: nil)
+
+      expect(doc.to_html).to eq upload_link
+    end
+  end
 end

spec/lib/gitlab/email/message/repository_push_spec.rb

@@ -8,7 +8,7 @@ describe Gitlab::Email::Message::RepositoryPush do
   let!(:author) { create(:author, name: 'Author') }
 
   let(:message) do
-    described_class.new(Notify, project.id, 'recipient@example.com', opts)
+    described_class.new(Notify, project.id, opts)
   end
 
   context 'new commits have been pushed to repository' do

spec/mailers/notify_spec.rb

@@ -593,7 +593,7 @@ describe Notify do
     let(:user) { create(:user) }
     let(:tree_path) { namespace_project_tree_path(project.namespace, project, "master") }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :create) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :create) }
 
     it_behaves_like 'it should not have Gmail Actions links'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -606,10 +606,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /Pushed new branch master/
     end
@@ -624,7 +620,7 @@ describe Notify do
     let(:user) { create(:user) }
     let(:tree_path) { namespace_project_tree_path(project.namespace, project, "v1.0") }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :create) }
 
     it_behaves_like 'it should not have Gmail Actions links'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -637,10 +633,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /Pushed new tag v1\.0/
     end
@@ -654,7 +646,7 @@ describe Notify do
     let(:example_site_path) { root_path }
     let(:user) { create(:user) }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :delete) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :delete) }
 
     it_behaves_like 'it should not have Gmail Actions links'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -667,10 +659,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /Deleted branch master/
     end
@@ -680,7 +668,7 @@ describe Notify do
     let(:example_site_path) { root_path }
     let(:user) { create(:user) }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) }
 
     it_behaves_like 'it should not have Gmail Actions links'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -693,10 +681,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /Deleted tag v1\.0/
     end
@@ -710,7 +694,7 @@ describe Notify do
     let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) }
     let(:send_from_committer_email) { false }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) }
 
     it_behaves_like 'it should not have Gmail Actions links'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -723,10 +707,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /\[#{project.path_with_namespace}\]\[master\] #{commits.length} commits:/
     end
@@ -818,7 +798,7 @@ describe Notify do
     let(:commits) { Commit.decorate(compare.commits, nil) }
     let(:diff_path) { namespace_project_commit_path(project.namespace, project, commits.first) }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) }
+    subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) }
 
     it_behaves_like 'it should show Gmail Actions View Commit link'
     it_behaves_like "a user cannot unsubscribe through footer link"
@@ -831,10 +811,6 @@ describe Notify do
       expect(sender.address).to eq(gitlab_sender)
     end
 
-    it 'is sent to recipient' do
-      is_expected.to deliver_to 'devs@company.name'
-    end
-
     it 'has the correct subject' do
       is_expected.to have_subject /#{commits.first.title}/
     end

spec/models/hooks/web_hook_spec.rb

@@ -95,13 +95,13 @@ describe WebHook, models: true do
     it "handles 200 status code" do
       WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "Success")
 
-      expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+      expect(project_hook.execute(@data, 'push_hooks')).to eq([200, 'Success'])
     end
 
     it "handles 2xx status codes" do
       WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: "Success")
 
-      expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+      expect(project_hook.execute(@data, 'push_hooks')).to eq([201, 'Success'])
     end
   end
 end

spec/models/user_spec.rb

@@ -783,4 +783,23 @@ describe User, models: true do
 
     it { is_expected.to eq([private_project]) }
   end
+
+  describe '#viewable_starred_projects' do
+    let(:user) { create(:user) }
+    let(:public_project) { create(:empty_project, :public) }
+    let(:private_project) { create(:empty_project, :private) }
+    let(:private_viewable_project) { create(:empty_project, :private) }
+
+    before do
+      private_viewable_project.team << [user, Gitlab::Access::MASTER]
+
+      [public_project, private_project, private_viewable_project].each do |project|
+        user.toggle_star(project)
+      end
+    end
+
+    it 'returns only starred projects the user can view' do
+      expect(user.viewable_starred_projects).not_to include(private_project)
+    end
+  end
 end

spec/requests/api/commit_status_spec.rb → spec/requests/api/commit_statuses_spec.rb

 require 'spec_helper'
 
-describe API::CommitStatus, api: true do
+describe API::CommitStatuses, api: true do
   include ApiHelpers
 
   let!(:project) { create(:project) }

spec/requests/api/projects_spec.rb

@@ -10,20 +10,20 @@ describe API::API, api: true  do
   let(:admin) { create(:admin) }
   let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
   let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) }
-  let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) }
   let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') }
   let(:project_member) { create(:project_member, :master, user: user, project: project) }
   let(:project_member2) { create(:project_member, :developer, user: user3, project: project) }
   let(:user4) { create(:user) }
   let(:project3) do
     create(:project,
+    :private,
     name: 'second_project',
     path: 'second_project',
     creator_id: user.id,
     namespace: user.namespace,
     merge_requests_enabled: false,
     issues_enabled: false, wiki_enabled: false,
-    snippets_enabled: false, visibility_level: 0)
+    snippets_enabled: false)
   end
   let(:project_member3) do
     create(:project_member,
@@ -164,21 +164,18 @@ describe API::API, api: true  do
   end
 
   describe 'GET /projects/starred' do
+    let(:public_project) { create(:project, :public) }
+
     before do
-      admin.starred_projects << project
-      admin.save!
+      project_member2
+      user3.update_attributes(starred_projects: [project, project2, project3, public_project])
     end
 
-    it 'should return the starred projects' do
-      get api('/projects/all', admin)
+    it 'should return the starred projects viewable by the user' do
+      get api('/projects/starred', user3)
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
-
-      expect(json_response).to satisfy do |response|
-        response.one? do |entry|
-          entry['name'] == project.name
-        end
-      end
+      expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, public_project.id)
     end
   end
 

spec/workers/emails_on_push_worker_spec.rb

@@ -6,29 +6,66 @@ describe EmailsOnPushWorker do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+  let(:recipients) { user.email }
+  let(:perform) { subject.perform(project.id, recipients, data.stringify_keys) }
 
   subject { EmailsOnPushWorker.new }
 
-  before do
-    allow(Project).to receive(:find).and_return(project)
-  end
-
   describe "#perform" do
-    it "sends mail" do
-      subject.perform(project.id, user.email, data.stringify_keys)
+    context "when there are no errors in sending" do
+      let(:email) { ActionMailer::Base.deliveries.last }
+
+      before { perform }
 
-      email = ActionMailer::Base.deliveries.last
-      expect(email.subject).to include('Change some files')
-      expect(email.to).to eq([user.email])
+      it "sends a mail with the correct subject" do
+        expect(email.subject).to include('Change some files')
+      end
+
+      it "sends the mail to the correct recipient" do
+        expect(email.to).to eq([user.email])
+      end
     end
 
-    it "gracefully handles an input SMTP error" do
-      ActionMailer::Base.deliveries.clear
-      allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError)
+    context "when there is an SMTP error" do
+      before do
+        ActionMailer::Base.deliveries.clear
+        allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError)
+        perform
+      end
+
+      it "gracefully handles an input SMTP error" do
+        expect(ActionMailer::Base.deliveries.count).to eq(0)
+      end
+    end
+
+    context "when there are multiple recipients" do
+      let(:recipients) do
+        1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n")
+      end
+
+      before do
+        # This is a hack because we modify the mail object before sending, for efficency,
+        # but the TestMailer adapter just appends the objects to an array. To clone a mail
+        # object, create a new one!
+        #   https://github.com/mikel/mail/issues/314#issuecomment-12750108
+        allow_any_instance_of(Mail::TestMailer).to receive(:deliver!).and_wrap_original do |original, mail|
+          original.call(Mail.new(mail.encoded))
+        end
+
+        ActionMailer::Base.deliveries.clear
+      end
 
-      subject.perform(project.id, user.email, data.stringify_keys)
+      it "sends the mail to each of the recipients" do
+        perform
+        expect(ActionMailer::Base.deliveries.count).to eq(5)
+        expect(ActionMailer::Base.deliveries.map(&:to).flatten).to contain_exactly(*recipients.split)
+      end
 
-      expect(ActionMailer::Base.deliveries.count).to eq(0)
+      it "only generates the mail once" do
+        expect(Notify).to receive(:repository_push_email).once.and_call_original
+        expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original
+        perform
+      end
     end
   end
 end

vendor/assets/javascripts/jquery.scrollTo.js

+/*!
+ * jQuery.scrollTo
+ * Copyright (c) 2007-2015 Ariel Flesler - aflesler<a>gmail<d>com | http://flesler.blogspot.com
+ * Licensed under MIT
+ * http://flesler.blogspot.com/2007/10/jqueryscrollto.html
+ * @projectDescription Lightweight, cross-browser and highly customizable animated scrolling with jQuery
+ * @author Ariel Flesler
+ * @version 2.1.2
+ */
+;(function(factory) {
+	'use strict';
+	if (typeof define === 'function' && define.amd) {
+		// AMD
+		define(['jquery'], factory);
+	} else if (typeof module !== 'undefined' && module.exports) {
+		// CommonJS
+		module.exports = factory(require('jquery'));
+	} else {
+		// Global
+		factory(jQuery);
+	}
+})(function($) {
+	'use strict';
+
+	var $scrollTo = $.scrollTo = function(target, duration, settings) {
+		return $(window).scrollTo(target, duration, settings);
+	};
+
+	$scrollTo.defaults = {
+		axis:'xy',
+		duration: 0,
+		limit:true
+	};
+
+	function isWin(elem) {
+		return !elem.nodeName ||
+			$.inArray(elem.nodeName.toLowerCase(), ['iframe','#document','html','body']) !== -1;
+	}		
+
+	$.fn.scrollTo = function(target, duration, settings) {
+		if (typeof duration === 'object') {
+			settings = duration;
+			duration = 0;
+		}
+		if (typeof settings === 'function') {
+			settings = { onAfter:settings };
+		}
+		if (target === 'max') {
+			target = 9e9;
+		}
+
+		settings = $.extend({}, $scrollTo.defaults, settings);
+		// Speed is still recognized for backwards compatibility
+		duration = duration || settings.duration;
+		// Make sure the settings are given right
+		var queue = settings.queue && settings.axis.length > 1;
+		if (queue) {
+			// Let's keep the overall duration
+			duration /= 2;
+		}
+		settings.offset = both(settings.offset);
+		settings.over = both(settings.over);
+
+		return this.each(function() {
+			// Null target yields nothing, just like jQuery does
+			if (target === null) return;
+
+			var win = isWin(this),
+				elem = win ? this.contentWindow || window : this,
+				$elem = $(elem),
+				targ = target, 
+				attr = {},
+				toff;
+
+			switch (typeof targ) {
+				// A number will pass the regex
+				case 'number':
+				case 'string':
+					if (/^([+-]=?)?\d+(\.\d+)?(px|%)?$/.test(targ)) {
+						targ = both(targ);
+						// We are done
+						break;
+					}
+					// Relative/Absolute selector
+					targ = win ? $(targ) : $(targ, elem);
+					/* falls through */
+				case 'object':
+					if (targ.length === 0) return;
+					// DOMElement / jQuery
+					if (targ.is || targ.style) {
+						// Get the real position of the target
+						toff = (targ = $(targ)).offset();
+					}
+			}
+
+			var offset = $.isFunction(settings.offset) && settings.offset(elem, targ) || settings.offset;
+
+			$.each(settings.axis.split(''), function(i, axis) {
+				var Pos	= axis === 'x' ? 'Left' : 'Top',
+					pos = Pos.toLowerCase(),
+					key = 'scroll' + Pos,
+					prev = $elem[key](),
+					max = $scrollTo.max(elem, axis);
+
+				if (toff) {// jQuery / DOMElement
+					attr[key] = toff[pos] + (win ? 0 : prev - $elem.offset()[pos]);
+
+					// If it's a dom element, reduce the margin
+					if (settings.margin) {
+						attr[key] -= parseInt(targ.css('margin'+Pos), 10) || 0;
+						attr[key] -= parseInt(targ.css('border'+Pos+'Width'), 10) || 0;
+					}
+
+					attr[key] += offset[pos] || 0;
+
+					if (settings.over[pos]) {
+						// Scroll to a fraction of its width/height
+						attr[key] += targ[axis === 'x'?'width':'height']() * settings.over[pos];
+					}
+				} else {
+					var val = targ[pos];
+					// Handle percentage values
+					attr[key] = val.slice && val.slice(-1) === '%' ?
+						parseFloat(val) / 100 * max
+						: val;
+				}
+
+				// Number or 'number'
+				if (settings.limit && /^\d+$/.test(attr[key])) {
+					// Check the limits
+					attr[key] = attr[key] <= 0 ? 0 : Math.min(attr[key], max);
+				}
+
+				// Don't waste time animating, if there's no need.
+				if (!i && settings.axis.length > 1) {
+					if (prev === attr[key]) {
+						// No animation needed
+						attr = {};
+					} else if (queue) {
+						// Intermediate animation
+						animate(settings.onAfterFirst);
+						// Don't animate this axis again in the next iteration.
+						attr = {};
+					}
+				}
+			});
+
+			animate(settings.onAfter);
+
+			function animate(callback) {
+				var opts = $.extend({}, settings, {
+					// The queue setting conflicts with animate()
+					// Force it to always be true
+					queue: true,
+					duration: duration,
+					complete: callback && function() {
+						callback.call(elem, targ, settings);
+					}
+				});
+				$elem.animate(attr, opts);
+			}
+		});
+	};
+
+	// Max scrolling position, works on quirks mode
+	// It only fails (not too badly) on IE, quirks mode.
+	$scrollTo.max = function(elem, axis) {
+		var Dim = axis === 'x' ? 'Width' : 'Height',
+			scroll = 'scroll'+Dim;
+
+		if (!isWin(elem))
+			return elem[scroll] - $(elem)[Dim.toLowerCase()]();
+
+		var size = 'client' + Dim,
+			doc = elem.ownerDocument || elem.document,
+			html = doc.documentElement,
+			body = doc.body;
+
+		return Math.max(html[scroll], body[scroll]) - Math.min(html[size], body[size]);
+	};
+
+	function both(val) {
+		return $.isFunction(val) || $.isPlainObject(val) ? val : { top:val, left:val };
+	}
+
+	// Add special hooks so that window scroll properties can be animated
+	$.Tween.propHooks.scrollLeft = 
+	$.Tween.propHooks.scrollTop = {
+		get: function(t) {
+			return $(t.elem)[t.prop]();
+		},
+		set: function(t) {
+			var curr = this.get(t);
+			// If interrupt is true and user scrolled, stop animating
+			if (t.options.interrupt && t._last && t._last !== curr) {
+				return $(t.elem).stop();
+			}
+			var next = Math.round(t.now);
+			// Don't waste CPU
+			// Browsers don't render floating point scroll
+			if (curr !== next) {
+				$(t.elem)[t.prop](next);
+				t._last = this.get(t);
+			}
+		}
+	};
+
+	// AMD requirement
+	return $scrollTo;
+});