Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into fix/atom-url-issue

CHANGELOG

@@ -15,9 +15,19 @@ v 8.5.0 (unreleased)
   - Track project import failure
   - Fix visibility level text in admin area (Zeger-Jan van de Weg)
   - Update the ExternalIssue regex pattern (Blake Hitchcock)
+  - Revert "Add IP check against DNSBLs at account sign-up"
   - Deprecate API "merge_request/:merge_request_id/comments". Use "merge_requests/:merge_request_id/notes" instead
   - Deprecate API "merge_request/:merge_request_id/...". Use "merge_requests/:merge_request_id/..." instead
   - Prevent parse error when name of project ends with .atom and prevent path issues
+  - Mark inline difference between old and new paths when a file is renamed
+
+v 8.4.3
+  - Increase lfs_objects size column to 8-byte integer to allow files larger
+    than 2.1GB
+  - Correctly highlight MR diff when MR has merge conflicts
+  - Fix highlighting in blame view
+  - Update sentry-raven gem to prevent "Not a git repository" console output
+    when running certain commands
 
 v 8.4.2
   - Bump required gitlab-workhorse version to bring in a fix for missing
@@ -28,7 +38,6 @@ v 8.4.2
     improvement when checking if a repository was empty
   - Add instrumentation for Gitlab::Git::Repository instance methods so we can
     track them in Performance Monitoring.
-  - Correctly highlight MR diff when MR has merge conflicts
   - Increase contrast between highlighted code comments and inline diff marker
   - Fix method undefined when using external commit status in builds
   - Fix highlighting in blame view.
@@ -38,6 +47,8 @@ v 8.4.1
     and Nokogiri (1.6.7.2)
   - Fix redirect loop during import
   - Fix diff highlighting for all syntax themes
+  - Warn admin during OAuth of granting admin rights (Zeger-Jan van de Weg)
+  - Delete project and associations in a background worker
 
 v 8.4.0
   - Allow LDAP users to change their email if it was not set by the LDAP server
@@ -81,7 +92,7 @@ v 8.4.0
   - Show 'All' tab by default in the builds page
   - Add Open Graph and Twitter Card data to all pages
   - Fix API project lookups when querying with a namespace with dots (Stan Hu)
-  - Enable forcing Two-Factor authentication sitewide, with optional grace period
+  - Enable forcing Two-factor authentication sitewide, with optional grace period
   - Import GitHub Pull Requests into GitLab
   - Change single user API endpoint to return more detailed data (Michael Potthoff)
   - Update version check images to use SVG

CONTRIBUTING.md

@@ -147,7 +147,7 @@ sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true)
 sudo gitlab-rake gitlab:env:info)
 
 (For installations from source run and paste the output of:
-sudo -u git -H bundle exec rake gitlab:env:info)
+sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production)
 
 ## Possible fixes
 
@@ -255,6 +255,8 @@ For examples of feedback on merge requests please look at already
 request feel free to mention one of the Merge Marshalls of the [core team][].
 Please ensure that your merge request meets the contribution acceptance criteria.
 
+When having your code reviewed and when reviewing merge requests please take the [thoughtbot code review guidelines](https://github.com/thoughtbot/guides/tree/master/code-review) into account.
+
 ## Definition of done
 
 If you contribute to GitLab please know that changes involve more than just

app/assets/javascripts/awards_handler.coffee

@@ -4,6 +4,7 @@ class @AwardsHandler
       event.stopPropagation()
       event.preventDefault()
       $(".emoji-menu").show()
+      $("#emoji_search").focus()
 
     $("html").on 'click', (event) ->
       if !$(event.target).closest(".emoji-menu").length

app/assets/stylesheets/framework/files.scss

@@ -36,6 +36,20 @@
       }
     }
 
+    .filename {
+      &.old {
+        span.idiff {
+          background-color: #f8cbcb;
+        }
+      }
+
+      &.new {
+        span.idiff {
+          background-color: #a6f3a6;
+        }
+      }
+    }
+
     .left-options {
       margin-top: -3px;
     }
@@ -158,3 +172,15 @@
     }
   }
 }
+
+span.idiff {
+  &.left {
+    border-top-left-radius: 2px;
+    border-bottom-left-radius: 2px;
+  }
+
+  &.right {
+    border-top-right-radius: 2px;
+    border-bottom-right-radius: 2px;
+  }
+}

app/controllers/admin/application_settings_controller.rb

@@ -74,8 +74,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :metrics_timeout,
       :metrics_method_call_threshold,
       :metrics_sample_interval,
-      :ip_blocking_enabled,
-      :dnsbl_servers_list,
       :recaptcha_enabled,
       :recaptcha_site_key,
       :recaptcha_private_key,

app/controllers/projects_controller.rb

@@ -93,6 +93,10 @@ class ProjectsController < ApplicationController
       return
     end
 
+    if @project.pending_delete?
+      flash[:alert] = "Project queued for delete."
+    end
+
     respond_to do |format|
       format.html do
         if @project.repository_exists?
@@ -120,8 +124,8 @@ class ProjectsController < ApplicationController
   def destroy
     return access_denied! unless can?(current_user, :remove_project, @project)
 
-    ::Projects::DestroyService.new(@project, current_user, {}).execute
-    flash[:alert] = "Project '#{@project.name}' was deleted."
+    ::Projects::DestroyService.new(@project, current_user, {}).pending_delete!
+    flash[:alert] = "Project '#{@project.name}' will be deleted."
 
     redirect_to dashboard_projects_path
   rescue Projects::DestroyService::DestroyError => ex

app/controllers/registrations_controller.rb

@@ -8,11 +8,6 @@ class RegistrationsController < Devise::RegistrationsController
 
   def create
     if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
-      if Gitlab::IpCheck.new(request.remote_ip).spam?
-        flash[:alert] = 'Could not create an account. This IP is listed for spam.'
-        return render action: 'new'
-      end
-
       super
     else
       flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code."

app/helpers/diff_helper.rb

 module DiffHelper
+  def mark_inline_diffs(old_line, new_line)
+    old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs
+
+    marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs)
+    marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs)
+
+    [marked_old_line, marked_new_line]
+  end
+
   def diff_view
     params[:view] == 'parallel' ? 'parallel' : 'inline'
   end
@@ -55,7 +64,7 @@ module DiffHelper
     if line.blank?
       "  ".html_safe
     else
-      line.html_safe
+      line
     end
   end
 

app/models/application_setting.rb

@@ -43,8 +43,6 @@
 #  metrics_port                      :integer          default(8089)
 #  sentry_enabled                    :boolean          default(FALSE)
 #  sentry_dsn                        :string
-#  ip_blocking_enabled               :boolean          default(FALSE)
-#  dns_blacklist_threshold           :float            default(0.33)
 #
 
 class ApplicationSetting < ActiveRecord::Base

app/models/broadcast_message.rb

@@ -26,8 +26,10 @@ class BroadcastMessage < ActiveRecord::Base
   default_value_for :font,  '#FFFFFF'
 
   def self.current
+    Rails.cache.fetch("broadcast_message_current", expires_in: 1.minute) do
       where("ends_at > :now AND starts_at <= :now", now: Time.zone.now).last
     end
+  end
 
   def active?
     started? && !ended?

app/models/commit.rb

@@ -68,18 +68,18 @@ class Commit
 
   # Pattern used to extract commit references from text
   #
-  # The SHA can be between 6 and 40 hex characters.
+  # The SHA can be between 7 and 40 hex characters.
   #
   # This pattern supports cross-project references.
   def self.reference_pattern
     %r{
       (?:#{Project.reference_pattern}#{reference_prefix})?
-      (?<commit>\h{6,40})
+      (?<commit>\h{7,40})
     }x
   end
 
   def self.link_reference_pattern
-    super("commit", /(?<commit>\h{6,40})/)
+    super("commit", /(?<commit>\h{7,40})/)
   end
 
   def to_reference(from_project = nil)

app/models/commit_range.rb

@@ -32,8 +32,8 @@ class CommitRange
   PATTERN = /#{REF_PATTERN}\.{2,3}#{REF_PATTERN}/
 
   # In text references, the beginning and ending refs can only be SHAs
-  # between 6 and 40 hex characters.
-  STRICT_PATTERN = /\h{6,40}\.{2,3}\h{6,40}/
+  # between 7 and 40 hex characters.
+  STRICT_PATTERN = /\h{7,40}\.{2,3}\h{7,40}/
 
   def self.reference_prefix
     '@'

app/models/project.rb

@@ -36,6 +36,7 @@
 #  build_coverage_regex   :string
 #  build_allow_git_fetch  :boolean          default(TRUE), not null
 #  build_timeout          :integer          default(3600), not null
+#  pending_delete         :boolean
 #
 
 require 'carrierwave/orm/activerecord'

app/models/wiki_page.rb

@@ -110,7 +110,7 @@ class WikiPage
   # Returns boolean True or False if this instance
   # is an old version of the page.
   def historical?
-    @page.historical?
+    @page.historical? && versions.first.sha != version.sha
   end
 
   # Returns boolean True or False if this instance

app/services/delete_user_service.rb

@@ -13,7 +13,7 @@ class DeleteUserService
       user.personal_projects.each do |project|
         # Skip repository removal because we remove directory with namespace
         # that contain all this repositories
-        ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
+        ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
       end
 
       user.destroy

app/services/destroy_group_service.rb

@@ -9,7 +9,7 @@ class DestroyGroupService
     @group.projects.each do |project|
       # Skip repository removal because we remove directory with namespace
       # that contain all this repositories
-      ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
+      ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete!
     end
 
     @group.destroy

app/services/notes/create_service.rb

@@ -6,27 +6,12 @@ module Notes
       note.system = false
 
       if note.save
-        notification_service.new_note(note)
-
-        # Skip system notes, like status changes and cross-references and awards
-        unless note.system || note.is_award
-          event_service.leave_note(note, note.author)
-          note.create_cross_references!
-          execute_hooks(note)
-        end
+        # Finish the harder work in the background
+        NewNoteWorker.perform_in(2.seconds, note.id, params)
       end
 
       note
     end
 
-    def hook_data(note)
-      Gitlab::NoteDataBuilder.build(note, current_user)
-    end
-
-    def execute_hooks(note)
-      note_data = hook_data(note)
-      note.project.execute_hooks(note_data, :note_hooks)
-      note.project.execute_services(note_data, :note_hooks)
-    end
   end
 end

app/services/notes/post_process_service.rb

+module Notes
+  class PostProcessService
+
+    attr_accessor :note
+
+    def initialize(note)
+      @note = note
+    end
+
+    def execute
+      # Skip system notes, like status changes and cross-references and awards
+      unless @note.system || @note.is_award
+        EventCreateService.new.leave_note(@note, @note.author)
+        @note.create_cross_references!
+        execute_note_hooks
+      end
+    end
+
+    def hook_data
+      Gitlab::NoteDataBuilder.build(@note, @note.author)
+    end
+
+    def execute_note_hooks
+      note_data = hook_data
+      @note.project.execute_hooks(note_data, :note_hooks)
+      @note.project.execute_services(note_data, :note_hooks)
+    end
+
+  end
+end

app/services/projects/destroy_service.rb

@@ -6,6 +6,12 @@ module Projects
 
     DELETED_FLAG = '+deleted'
 
+    def pending_delete!
+      project.update_attribute(:pending_delete, true)
+
+      ProjectDestroyWorker.perform_in(1.minute, project.id, current_user.id, params)
+    end
+
     def execute
       return false unless can?(current_user, :remove_project, project)
 

app/views/admin/application_settings/_form.html.haml

@@ -105,14 +105,14 @@
             = f.check_box :signin_enabled
             Sign-in enabled
     .form-group
-      = f.label :two_factor_authentication, 'Two-Factor authentication', class: 'control-label col-sm-2'
+      = f.label :two_factor_authentication, 'Two-factor authentication', class: 'control-label col-sm-2'
       .col-sm-10
         .checkbox
           = f.label :require_two_factor_authentication do
             = f.check_box :require_two_factor_authentication
-            Require all users to setup Two-Factor authentication
+            Require all users to setup Two-factor authentication
     .form-group
-      = f.label :two_factor_authentication, 'Two-Factor grace period (hours)', class: 'control-label col-sm-2'
+      = f.label :two_factor_authentication, 'Two-factor grace period (hours)', class: 'control-label col-sm-2'
       .col-sm-10
         = f.number_field :two_factor_grace_period, min: 0, class: 'form-control', placeholder: '0'
         .help-block Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication
@@ -212,22 +212,6 @@
 
   %fieldset
     %legend Spam and Anti-bot Protection
-    .form-group
-      .col-sm-offset-2.col-sm-10
-        .checkbox
-          = f.label :ip_blocking_enabled do
-            = f.check_box :ip_blocking_enabled
-            Enable IP check against blacklist at sign-up
-          .help-block Helps preventing accounts creation from 'known spam sources'
-
-    .form-group
-      = f.label :dnsbl_servers_list, class: 'control-label col-sm-2' do
-        DNSBL servers list
-      .col-sm-10
-        = f.text_field :dnsbl_servers_list, class: 'form-control'
-        .help-block
-          Please enter DNSBL servers separated with comma
-
     .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox

app/views/doorkeeper/authorizations/new.html.haml

@@ -4,6 +4,15 @@
     Authorize
     %strong.text-info= @pre_auth.client.name
     to use your account?
+
+  - if current_user.admin?
+    .text-warning.prepend-top-20
+      %p
+        = icon("exclamation-triangle fw")
+        You are an admin, which means granting access to
+        %strong #{@pre_auth.client.name}
+        will allow them to interact with GitLab as an admin as well. Proceed with caution.
+
   - if @pre_auth.scopes
     #oauth-permissions
       %p This application will be able to:

app/views/profiles/two_factor_auths/new.html.haml

 - page_title 'Two-factor Authentication', 'Account'
 
-%h2.page-title Two-Factor Authentication (2FA)
+%h2.page-title Two-factor Authentication (2FA)
 %p
   Download the Google Authenticator application from App Store for iOS or Google
   Play for Android and scan this code.

app/views/projects/diffs/_file.html.haml

@@ -7,16 +7,20 @@
           = submodule_link(blob, @commit.id, project.repository)
     - else
       = blob_icon blob.mode, blob.name
+
       = link_to "#diff-#{i}" do
+        - if diff_file.renamed_file
+          - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
+          %strong.filename.old
+            = old_path
+          →
+          %strong.filename.new
+            = new_path
+        - else
           %strong
             = diff_file.new_path
-
           - if diff_file.deleted_file
             deleted
-      - elsif diff_file.renamed_file
-        renamed from
-        %strong
-          = diff_file.old_path
 
       - if diff_file.mode_changed?
         %small

app/views/votes/_votes_block.html.haml

@@ -7,7 +7,7 @@
 
   - if current_user
     .awards-controls
-      %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"}
+      %a.add-award{"href" => "#"}
         = icon('smile-o')
       .emoji-menu
         .emoji-menu-content

app/workers/new_note_worker.rb

+class NewNoteWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(note_id, note_params)
+    note = Note.find(note_id)
+
+    NotificationService.new.new_note(note)
+    Notes::PostProcessService.new(note).execute
+  end
+end

app/workers/project_destroy_worker.rb

+class ProjectDestroyWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(project_id, user_id, params)
+    begin
+      project = Project.find(project_id)
+    rescue ActiveRecord::RecordNotFound
+      return
+    end
+
+    user = User.find(user_id)
+
+    ::Projects::DestroyService.new(project, user, params).execute
+  end
+end

config/routes.rb

@@ -490,7 +490,7 @@ Rails.application.routes.draw do
         end
 
         resource  :avatar, only: [:show, :destroy]
-        resources :commit, only: [:show], constraints: { id: /[[:alnum:]]{6,40}/ } do
+        resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
           member do
             get :branches
             get :builds

db/migrate/20160122185421_add_pending_delete_to_project.rb

+class AddPendingDeleteToProject < ActiveRecord::Migration
+  def change
+    add_column :projects, :pending_delete, :boolean, default: false
+  end
+end

db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb

+class RemoveIpBlockingSettingsFromApplicationSettings < ActiveRecord::Migration
+  def change
+    remove_column :application_settings, :ip_blocking_enabled, :boolean, default: false
+    remove_column :application_settings, :dnsbl_servers_list, :text
+  end
+end

db/migrate/20160128233227_change_lfs_objects_size_column.rb

+class ChangeLfsObjectsSizeColumn < ActiveRecord::Migration
+  def change
+    change_column :lfs_objects, :size, :integer, limit: 8
+  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: 20160120172143) do
+ActiveRecord::Schema.define(version: 20160128233227) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -64,8 +64,6 @@ ActiveRecord::Schema.define(version: 20160120172143) do
     t.integer  "metrics_sample_interval",           default: 15
     t.boolean  "sentry_enabled",                    default: false
     t.string   "sentry_dsn"
-    t.boolean  "ip_blocking_enabled",               default: false
-    t.text     "dnsbl_servers_list"
   end
 
   create_table "audit_events", force: :cascade do |t|
@@ -448,7 +446,7 @@ ActiveRecord::Schema.define(version: 20160120172143) do
 
   create_table "lfs_objects", force: :cascade do |t|
     t.string   "oid",                  null: false
-    t.integer  "size",       null: false
+    t.integer  "size",       limit: 8, null: false
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "file"
@@ -679,6 +677,7 @@ ActiveRecord::Schema.define(version: 20160120172143) do
     t.string   "build_coverage_regex"
     t.boolean  "build_allow_git_fetch",  default: true,     null: false
     t.integer  "build_timeout",          default: 3600,     null: false
+    t.boolean  "pending_delete",         default: false
   end
 
   add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree

doc/api/builds.md

@@ -18,7 +18,7 @@ GET /projects/:id/builds
 ### Example of request
 
 ```
-curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds"
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds"
 ```
 
 ### Example of response
@@ -123,7 +123,7 @@ GET /projects/:id/repository/commits/:sha/builds
 ### Example of request
 
 ```
-curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds"
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds"
 ```
 
 ### Example of response
@@ -213,7 +213,7 @@ GET /projects/:id/builds/:build_id
 ### Example of request
 
 ```
-curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/8"
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/8"
 ```
 
 ### Example of response
@@ -277,7 +277,7 @@ POST /projects/:id/builds/:build_id/cancel
 ### Example of request
 
 ```
-curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/cancel"
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/cancel"
 ```
 
 ### Example of response
@@ -327,7 +327,7 @@ POST /projects/:id/builds/:build_id/retry
 ### Example of request
 
 ```
-curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/retry"
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/retry"
 ```
 
 ### Example of response

doc/security/README.md

@@ -7,4 +7,4 @@
 - [Reset your root password](reset_root_password.md)
 - [User File Uploads](user_file_uploads.md)
 - [How we manage the CRIME vulnerability](crime_vulnerability.md)
-- [Enforce Two-Factor authentication](two_factor_authentication.md)
+- [Enforce Two-factor authentication](two_factor_authentication.md)

doc/workflow/gitlab_flow.md

@@ -152,9 +152,10 @@ The name of this branch should start with the issue number, for example '15-requ
 
 When you are done or want to discuss the code you open a merge request.
 This is an online place to discuss the change and review the code.
-Creating a branch is a manual action since you do not always want to merge a new branch you push, it could be a long-running environment or release branch.
-If you create the merge request but do not assign it to anyone it is a 'work-in-process' merge request.
+Opening a merge request is a manual action since you do not always want to merge a new branch you push, it could be a long-running environment or release branch.
+If you open the merge request but do not assign it to anyone it is a 'Work In Progress' merge request.
 These are used to discuss the proposed implementation but are not ready for inclusion in the master branch yet.
+_Pro tip:_ Start the title of the merge request with `[WIP]` or `WIP:` to prevent it from being merged before it's ready.
 
 When the author thinks the code is ready the merge request is assigned to reviewer.
 The reviewer presses the merge button when they think the code is ready for inclusion in the master branch.
@@ -185,7 +186,7 @@ If you have an issue that spans across multiple repositories, the best thing is
 
 ![Vim screen showing the rebase view](rebase.png)
 
-With git you can use an interactive rebase (rebase -i) to squash multiple commits into one and reorder them.
+With git you can use an interactive rebase (`rebase -i`) to squash multiple commits into one and reorder them.
 This functionality is useful if you made a couple of commits for small changes during development and want to replace them with a single commit or if you want to make the order more logical.
 However you should never rebase commits you have pushed to a remote server.
 Somebody can have referred to the commits or cherry-picked them.

features/project/issues/award_emoji.feature

@@ -9,6 +9,7 @@ Feature: Award Emoji
   @javascript
   Scenario: I add and remove award in the issue
     Given I click to emoji-picker
+    Then The search field is focused
     And I click to emoji in the picker
     Then I have award added
     And I can remove it by clicking to icon
@@ -16,11 +17,13 @@ Feature: Award Emoji
   @javascript
   Scenario: I can see the list of emoji categories
     Given I click to emoji-picker
+    Then The search field is focused
     Then I can see the activity and food categories
 
   @javascript
   Scenario: I can search emoji
     Given I click to emoji-picker
+    Then The search field is focused
     And I search "hand"
     Then I see search result for "hand"
 

features/steps/project/issues/award_emoji.rb

@@ -66,4 +66,8 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
       expect(page).to have_selector '[data-emoji="raised_hand"]'
     end
   end
+
+  step 'The search field is focused' do
+    page.evaluate_script("document.activeElement.id").should eq "emoji_search"
+  end
 end

lib/api/projects.rb

@@ -246,7 +246,7 @@ module API
       #   DELETE /projects/:id
       delete ":id" do
         authorize! :remove_project, user_project
-        ::Projects::DestroyService.new(user_project, current_user, {}).execute
+        ::Projects::DestroyService.new(user_project, current_user, {}).pending_delete!
       end
 
       # Mark this project as forked from another

lib/dnsxl_check.rb

-require 'resolv'
-
-class DNSXLCheck
-
-  class Resolver
-    def self.search(query)
-      begin
-        Resolv.getaddress(query)
-        true
-      rescue Resolv::ResolvError
-        false
-      end
-    end
-  end
-
-  IP_REGEXP = /\A(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\z/
-  DEFAULT_THRESHOLD = 0.33
-
-  def self.create_from_list(list)
-    dnsxl_check = DNSXLCheck.new
-
-    list.each do |entry|
-      dnsxl_check.add_list(entry.domain, entry.weight)
-    end
-
-    dnsxl_check
-  end
-
-  def test(ip)
-    if use_threshold?
-      test_with_threshold(ip)
-    else
-      test_strict(ip)
-    end
-  end
-
-  def test_with_threshold(ip)
-    return false if lists.empty?
-
-    search(ip)
-    final_score >= threshold
-  end
-
-  def test_strict(ip)
-    return false if lists.empty?
-
-    search(ip)
-    @score > 0
-  end
-
-  def use_threshold=(value)
-    @use_threshold = value == true
-  end
-
-  def use_threshold?
-    @use_threshold &&= true
-  end
-
-  def threshold=(threshold)
-    raise ArgumentError, "'threshold' value must be grather than 0 and less than or equal to 1" unless threshold > 0 && threshold <= 1
-    @threshold = threshold
-  end
-
-  def threshold
-    @threshold ||= DEFAULT_THRESHOLD
-  end
-
-  def add_list(domain, weight)
-    @lists ||= []
-    @lists << { domain: domain, weight: weight }
-  end
-
-  def lists
-    @lists ||= []
-  end
-
-  private
-
-  def search(ip)
-    raise ArgumentError, "'ip' value must be in #{IP_REGEXP} format" unless ip.match(IP_REGEXP)
-
-    @score = 0
-
-    reversed = reverse_ip(ip)
-    search_in_rbls(reversed)
-  end
-
-  def reverse_ip(ip)
-    ip.split('.').reverse.join('.')
-  end
-
-  def search_in_rbls(reversed_ip)
-    lists.each do |rbl|
-      query = "#{reversed_ip}.#{rbl[:domain]}"
-      @score += rbl[:weight] if Resolver.search(query)
-    end
-  end
-
-  def final_score
-    weights = lists.map{ |rbl| rbl[:weight] }.reduce(:+).to_i
-    return 0 if weights == 0
-
-    (@score.to_f / weights.to_f).round(2)
-  end
-end

lib/gitlab/diff/highlight.rb

@@ -21,13 +21,13 @@ module Gitlab
           # ignore highlighting for "match" lines
           next diff_line if diff_line.type == 'match' || diff_line.type == 'nonewline'
 
-          rich_line = highlight_line(diff_line, i)
+          rich_line = highlight_line(diff_line) || diff_line.text
 
           if line_inline_diffs = inline_diffs[i]
             rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs)
           end
 
-          diff_line.text = rich_line.html_safe
+          diff_line.text = rich_line
 
           diff_line
         end
@@ -35,8 +35,8 @@ module Gitlab
 
       private
 
-      def highlight_line(diff_line, index)
-        return html_escape(diff_line.text) unless diff_file && diff_file.diff_refs
+      def highlight_line(diff_line)
+        return unless diff_file && diff_file.diff_refs
 
         line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' '
 
@@ -49,11 +49,11 @@ module Gitlab
 
         # Only update text if line is found. This will prevent
         # issues with submodules given the line only exists in diff content.
-        rich_line ? line_prefix + rich_line : html_escape(diff_line.text)
+        "#{line_prefix}#{rich_line}".html_safe if rich_line
       end
 
       def inline_diffs
-        @inline_diffs ||= InlineDiff.new(@raw_lines).inline_diffs
+        @inline_diffs ||= InlineDiff.for_lines(@raw_lines)
       end
 
       def old_lines
@@ -72,11 +72,6 @@ module Gitlab
 
         [ref.project.repository, ref.id, path]
       end
-
-      def html_escape(str)
-        replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
-        str.gsub(/[&"'><]/, replacements)
-      end
     end
   end
 end

lib/gitlab/diff/inline_diff.rb

 module Gitlab
   module Diff
     class InlineDiff
-      attr_accessor :lines
+      attr_accessor :old_line, :new_line, :offset
 
-      def initialize(lines)
-        @lines = lines
-      end
+      def self.for_lines(lines)
+        local_edit_indexes = self.find_local_edits(lines)
 
-      def inline_diffs
         inline_diffs = []
 
         local_edit_indexes.each do |index|
           old_index = index
           new_index = index + 1
-          old_line = @lines[old_index]
-          new_line = @lines[new_index]
+          old_line = lines[old_index]
+          new_line = lines[new_index]
+
+          old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
+
+          inline_diffs[old_index] = old_diffs
+          inline_diffs[new_index] = new_diffs
+        end
+
+        inline_diffs
+      end
+
+      def initialize(old_line, new_line, offset: 0)
+        @old_line = old_line[offset..-1]
+        @new_line = new_line[offset..-1]
+        @offset = offset
+      end
 
+      def inline_diffs
         # Skip inline diff if empty line was replaced with content
-          next if old_line[1..-1] == ""
+        return if old_line == ""
 
-          # Add one, because this is based on the prefixless version
-          lcp = longest_common_prefix(old_line[1..-1], new_line[1..-1]) + 1
+        lcp = longest_common_prefix(old_line, new_line)
         lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1])
 
-          old_diff_range = lcp..(old_line.length - lcs - 1)
-          new_diff_range = lcp..(new_line.length - lcs - 1)
+        lcp += offset
+        old_length = old_line.length + offset
+        new_length = new_line.length + offset
 
-          inline_diffs[old_index] = [old_diff_range] if old_diff_range.begin <= old_diff_range.end
-          inline_diffs[new_index] = [new_diff_range] if new_diff_range.begin <= new_diff_range.end
-        end
+        old_diff_range = lcp..(old_length - lcs - 1)
+        new_diff_range = lcp..(new_length - lcs - 1)
 
-        inline_diffs
+        old_diffs = [old_diff_range] if old_diff_range.begin <= old_diff_range.end
+        new_diffs = [new_diff_range] if new_diff_range.begin <= new_diff_range.end
+
+        [old_diffs, new_diffs]
       end
 
       private
 
-      # Find runs of single line edits
-      def local_edit_indexes
-        line_prefixes = @lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' }
+      def self.find_local_edits(lines)
+        line_prefixes = lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' }
         joined_line_prefixes = " #{line_prefixes.join} "
 
         offset = 0

lib/gitlab/diff/inline_diff_marker.rb

@@ -5,10 +5,12 @@ module Gitlab
 
       def initialize(raw_line, rich_line = raw_line)
         @raw_line = raw_line
-        @rich_line = rich_line
+        @rich_line = ERB::Util.html_escape(rich_line)
       end
 
       def mark(line_inline_diffs)
+        return rich_line unless line_inline_diffs
+
         marker_ranges = []
         line_inline_diffs.each do |inline_diff_range|
           # Map the inline-diff range based on the raw line to character positions in the rich line
@@ -19,11 +21,15 @@ module Gitlab
 
         offset = 0
         # Mark each range
-        marker_ranges.each do |range|
-          offset = insert_around_range(rich_line, range, "<span class='idiff'>", "</span>", offset)
+        marker_ranges.each_with_index do |range, i|
+          class_names = ["idiff"]
+          class_names << "left"   if i == 0
+          class_names << "right"  if i == marker_ranges.length - 1
+
+          offset = insert_around_range(rich_line, range, "<span class='#{class_names.join(" ")}'>", "</span>", offset)
         end
 
-        rich_line
+        rich_line.html_safe
       end
 
       private

lib/gitlab/ip_check.rb

-module Gitlab
-  class IpCheck
-
-    def initialize(ip)
-      @ip = ip
-
-      application_settings = ApplicationSetting.current
-      @ip_blocking_enabled =  application_settings.ip_blocking_enabled
-      @dnsbl_servers_list = application_settings.dnsbl_servers_list
-    end
-
-    def spam?
-      @ip_blocking_enabled && blacklisted?
-    end
-
-    private
-
-    def blacklisted?
-      on_dns_blacklist?
-    end
-
-    def on_dns_blacklist?
-      dnsbl_check = DNSXLCheck.new
-      prepare_dnsbl_list(dnsbl_check)
-      dnsbl_check.test(@ip)
-    end
-
-    def prepare_dnsbl_list(dnsbl_check)
-      @dnsbl_servers_list.split(',').map(&:strip).reject(&:empty?).each do |domain|
-        dnsbl_check.add_list(domain, 1)
-      end
-    end
-  end
-end

spec/fixtures/parallel_diff_result.yml

@@ -55,7 +55,7 @@
     :type: new
     :number: 9
     :text: |
-      +<span id="LC9" class="line">      <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
+      +<span id="LC9" class="line">      <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
     :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9
 - :left:
     :type:

spec/helpers/diff_helper_spec.rb

@@ -104,8 +104,7 @@ describe DiffHelper do
     end
   end
 
-  describe 'diff_line_content' do
-
+  describe '#diff_line_content' do
     it 'should return non breaking space when line is empty' do
       expect(diff_line_content(nil)).to eq('  ')
     end
@@ -116,9 +115,19 @@ describe DiffHelper do
       expect(diff_line_content(diff_file.diff_lines.first.type)).to eq('match')
       expect(diff_file.diff_lines.first.new_pos).to eq(6)
     end
+  end
+
+  describe "#mark_inline_diffs" do
+    let(:old_line) { %{abc 'def'} }
+    let(:new_line) { %{abc "def"} }
+
+    it "returns strings with marked inline diffs" do
+      marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
 
-    it 'should return safe HTML' do
-      expect(diff_line_content(diff_file.diff_lines.first.text)).to be_html_safe
+      expect(marked_old_line).to eq("abc <span class='idiff left right'>&#39;def&#39;</span>")
+      expect(marked_old_line).to be_html_safe
+      expect(marked_new_line).to eq("abc <span class='idiff left right'>&quot;def&quot;</span>")
+      expect(marked_new_line).to be_html_safe
     end
   end
 end

spec/lib/banzai/filter/commit_reference_filter_spec.rb

@@ -21,7 +21,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do
     let(:reference) { commit.id }
 
     # Let's test a variety of commit SHA sizes just to be paranoid
-    [6, 8, 12, 18, 20, 32, 40].each do |size|
+    [7, 8, 12, 18, 20, 32, 40].each do |size|
       it "links to a valid reference of #{size} characters" do
         doc = reference_filter("See #{reference[0...size]}")
 
@@ -35,7 +35,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do
       doc = reference_filter("See #{commit.id}")
       expect(doc.text).to eq "See #{commit.short_id}"
 
-      doc = reference_filter("See #{commit.id[0...6]}")
+      doc = reference_filter("See #{commit.id[0...7]}")
       expect(doc.text).to eq "See #{commit.short_id}"
     end
 

spec/lib/dnsxl_check_spec.rb

-require 'spec_helper'
-require 'ostruct'
-
-describe 'DNSXLCheck', lib: true, no_db: true do
-  let(:spam_ip)    { '127.0.0.2' }
-  let(:no_spam_ip) { '127.0.0.3' }
-  let(:invalid_ip) { 'a.b.c.d' }
-  let!(:dnsxl_check) { DNSXLCheck.create_from_list([OpenStruct.new({ domain: 'test', weight: 1 })]) }
-
-  before(:context) do
-    class DNSXLCheck::Resolver
-      class << self
-        alias_method :old_search, :search
-        def search(query)
-          return false if query.match(/always\.failing\.domain\z/)
-          return true  if query.match(/\A2\.0\.0\.127\./)
-          return false if query.match(/\A3\.0\.0\.127\./)
-        end
-      end
-    end
-  end
-
-  describe '#test' do
-    before do
-      dnsxl_check.threshold = 0.75
-      dnsxl_check.add_list('always.failing.domain', 1)
-    end
-
-    context 'when threshold is used' do
-      before { dnsxl_check.use_threshold= true }
-
-      it { expect(dnsxl_check.test(spam_ip)).to be_falsey }
-    end
-
-    context 'when threshold is not used' do
-      before { dnsxl_check.use_threshold= false }
-
-      it { expect(dnsxl_check.test(spam_ip)).to be_truthy }
-    end
-  end
-
-  describe '#test_with_threshold' do
-    it { expect{ dnsxl_check.test_with_threshold(invalid_ip) }.to raise_error(ArgumentError) }
-
-    it { expect(dnsxl_check.test_with_threshold(spam_ip)).to    be_truthy }
-    it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey }
-  end
-
-  describe '#test_strict' do
-    before do
-      dnsxl_check.threshold = 1
-      dnsxl_check.add_list('always.failing.domain', 1)
-    end
-
-    it { expect{ dnsxl_check.test_strict(invalid_ip) }.to raise_error(ArgumentError) }
-
-    it { expect(dnsxl_check.test_with_threshold(spam_ip)).to    be_falsey }
-    it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey }
-    it { expect(dnsxl_check.test_strict(spam_ip)).to    be_truthy }
-    it { expect(dnsxl_check.test_strict(no_spam_ip)).to be_falsey }
-  end
-
-  describe '#threshold=' do
-    it { expect{ dnsxl_check.threshold = 0   }.to     raise_error(ArgumentError) }
-    it { expect{ dnsxl_check.threshold = 1.1 }.to     raise_error(ArgumentError) }
-    it { expect{ dnsxl_check.threshold = 0.5 }.not_to raise_error }
-  end
-end

spec/lib/gitlab/diff/highlight_spec.rb

@@ -9,33 +9,69 @@ describe Gitlab::Diff::Highlight, lib: true do
   let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) }
 
   describe '#highlight' do
-    let(:diff_lines) { Gitlab::Diff::Highlight.new(diff_file).highlight }
+    context "with a diff file" do
+      let(:subject) { Gitlab::Diff::Highlight.new(diff_file).highlight }
 
       it 'should return Gitlab::Diff::Line elements' do
-      expect(diff_lines.first).to be_an_instance_of(Gitlab::Diff::Line)
+        expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
       end
 
       it 'should not modify "match" lines' do
-      expect(diff_lines[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
-      expect(diff_lines[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
+        expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
+        expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
       end
 
-    it 'should highlight unchanged lines' do
+      it 'highlights and marks unchanged lines' do
         code = %Q{ <span id="LC7" class="line">  <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n}
 
-      expect(diff_lines[2].text).to eq(code)
+        expect(subject[2].text).to eq(code)
       end
 
-    it 'should highlight removed lines' do
+      it 'highlights and marks removed lines' do
         code = %Q{-<span id="LC9" class="line">      <span class="k">raise</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>\n}
 
-      expect(diff_lines[4].text).to eq(code)
+        expect(subject[4].text).to eq(code)
       end
 
-    it 'should highlight added lines' do
-      code = %Q{+<span id="LC9" class="line">      <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>\n}
+      it 'highlights and marks added lines' do
+        code = %Q{+<span id="LC9" class="line">      <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>\n}
 
-      expect(diff_lines[5].text).to eq(code)
+        expect(subject[5].text).to eq(code)
+      end
+    end
+
+    context "with diff lines" do
+      let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines).highlight }
+
+      it 'should return Gitlab::Diff::Line elements' do
+        expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line)
+      end
+
+      it 'should not modify "match" lines' do
+        expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen')
+        expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen')
+      end
+
+      it 'marks unchanged lines' do
+        code = %Q{   def popen(cmd, path=nil)}
+
+        expect(subject[2].text).to eq(code)
+        expect(subject[2].text).not_to be_html_safe
+      end
+
+      it 'marks removed lines' do
+        code = %Q{-      raise "System commands must be given as an array of strings"}
+
+        expect(subject[4].text).to eq(code)
+        expect(subject[4].text).not_to be_html_safe
+      end
+
+      it 'marks added lines' do
+        code = %Q{+      raise <span class='idiff left right'>RuntimeError, </span>&quot;System commands must be given as an array of strings&quot;}
+
+        expect(subject[5].text).to eq(code)
+        expect(subject[5].text).to be_html_safe
+      end
     end
   end
 end

spec/lib/gitlab/diff/inline_diff_marker_spec.rb

@@ -2,14 +2,28 @@ require 'spec_helper'
 
 describe Gitlab::Diff::InlineDiffMarker, lib: true do
   describe '#inline_diffs' do
+
+    context "when the rich text is html safe" do
       let(:raw)  { "abc 'def'" }
-    let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&#39;def&#39;</span>} }
+      let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&#39;def&#39;</span>}.html_safe }
       let(:inline_diffs) { [2..5] }
-
       let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) }
 
       it 'marks the inline diffs' do
-      expect(subject).to eq(%{<span class="abc">ab<span class='idiff'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff'>&#39;d</span>ef&#39;</span>})
+        expect(subject).to eq(%{<span class="abc">ab<span class='idiff left'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff right'>&#39;d</span>ef&#39;</span>})
+        expect(subject).to be_html_safe
+      end
+    end
+
+    context "when the text text is not html safe" do
+      let(:raw)  { "abc 'def'" }
+      let(:inline_diffs) { [2..5] }
+      let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw).mark(inline_diffs) }
+
+      it 'marks the inline diffs' do
+        expect(subject).to eq(%{ab<span class='idiff left right'>c &#39;d</span>ef&#39;})
+        expect(subject).to be_html_safe
+      end
     end
   end
 end

spec/lib/gitlab/diff/inline_diff_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Diff::InlineDiff, lib: true do
-  describe '#inline_diffs' do
+  describe '.for_lines' do
     let(:diff) do
       <<eos
  class Test
@@ -13,7 +13,7 @@ describe Gitlab::Diff::InlineDiff, lib: true do
 eos
     end
 
-    let(:subject) { Gitlab::Diff::InlineDiff.new(diff.lines).inline_diffs }
+    let(:subject) { described_class.for_lines(diff.lines) }
 
     it 'finds all inline diffs' do
       expect(subject[0]).to be_nil
@@ -24,4 +24,17 @@ eos
       expect(subject[5]).to be_nil
     end
   end
+
+  describe "#inline_diffs" do
+    let(:old_line) { "XXX def initialize(test = true)" }
+    let(:new_line) { "YYY def initialize(test = false)" }
+    let(:subject) { described_class.new(old_line, new_line, offset: 3).inline_diffs }
+
+    it "finds the inline diff" do
+      old_diffs, new_diffs = subject
+
+      expect(old_diffs).to eq([26..28])
+      expect(new_diffs).to eq([26..29])
+    end
+  end
 end

spec/models/hooks/system_hook_spec.rb

@@ -36,7 +36,7 @@ describe SystemHook, models: true do
     it "project_destroy hook" do
       user = create(:user)
       project = create(:empty_project, namespace: user.namespace)
-      Projects::DestroyService.new(project, user, {}).execute
+      Projects::DestroyService.new(project, user, {}).pending_delete!
       expect(WebMock).to have_requested(:post, @system_hook.url).with(
         body: /project_destroy/,
         headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }

spec/models/wiki_page_spec.rb

@@ -189,6 +189,38 @@ describe WikiPage, models: true do
     end
   end
 
+  describe '#historical?' do
+    before do
+      create_page('Update', 'content')
+      @page = wiki.find_page('Update')
+      3.times { |i| @page.update("content #{i}") }
+    end
+
+    after do
+      destroy_page('Update')
+    end
+
+    it 'returns true when requesting an old version' do
+      old_version = @page.versions.last.to_s
+      old_page = wiki.find_page('Update', old_version)
+
+      expect(old_page.historical?).to eq true
+    end
+
+    it 'returns false when requesting latest version' do
+      latest_version = @page.versions.first.to_s
+      latest_page = wiki.find_page('Update', latest_version)
+
+      expect(latest_page.historical?).to eq false
+    end
+
+    it 'returns false when version is nil' do
+      latest_page = wiki.find_page('Update', nil)
+
+      expect(latest_page.historical?).to eq false
+    end
+  end
+
   private
 
   def remove_temp_repo(path)

spec/routing/project_routing_spec.rb

@@ -321,12 +321,12 @@ describe Projects::HooksController, 'routing' do
   end
 end
 
-# project_commit GET    /:project_id/commit/:id(.:format) commit#show {id: /[[:alnum:]]{6,40}/, project_id: /[^\/]+/}
+# project_commit GET    /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/}
 describe Projects::CommitController, 'routing' do
   it 'to #show' do
-    expect(get('/gitlab/gitlabhq/commit/4246fb')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb')
-    expect(get('/gitlab/gitlabhq/commit/4246fb.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'diff')
-    expect(get('/gitlab/gitlabhq/commit/4246fb.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'patch')
+    expect(get('/gitlab/gitlabhq/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd')
+    expect(get('/gitlab/gitlabhq/commit/4246fbd.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'diff')
+    expect(get('/gitlab/gitlabhq/commit/4246fbd.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'patch')
     expect(get('/gitlab/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5')
   end
 end

spec/services/notes/create_service_spec.rb

@@ -15,8 +15,6 @@ describe Notes::CreateService, services: true do
           noteable_id: issue.id
         }
         
-        expect(project).to receive(:execute_hooks)
-        expect(project).to receive(:execute_services)
         @note = Notes::CreateService.new(project, user, opts).execute
       end
 

spec/services/notes/post_process_service_spec.rb

+require 'spec_helper'
+
+describe Notes::PostProcessService, services: true do
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+  let(:user) { create(:user) }
+
+  describe :execute do
+    before do
+      project.team << [user, :master]
+      note_opts = {
+        note: 'Awesome comment',
+        noteable_type: 'Issue',
+        noteable_id: issue.id
+      }
+
+      @note = Notes::CreateService.new(project, user, note_opts).execute
+    end
+
+    it do
+      expect(project).to receive(:execute_hooks)
+      expect(project).to receive(:execute_services)
+      Notes::PostProcessService.new(@note).execute
+    end
+  end
+end

spec/support/filter_spec_helper.rb

@@ -67,9 +67,9 @@ module FilterSpecHelper
     if reference =~ /\A(.+)?.\d+\z/
       # Integer-based reference with optional project prefix
       reference.gsub(/\d+\z/) { |i| i.to_i + 1 }
-    elsif reference =~ /\A(.+@)?(\h{6,40}\z)/
+    elsif reference =~ /\A(.+@)?(\h{7,40}\z)/
       # SHA-based reference with optional prefix
-      reference.gsub(/\h{6,40}\z/) { |v| v.reverse }
+      reference.gsub(/\h{7,40}\z/) { |v| v.reverse }
     else
       reference.gsub(/\w+\z/) { |v| v.reverse }
     end