Merge branch 'master' of https://gitlab.com/gitlab-org/gitlab-ce into...

Merge branch 'master' of https://gitlab.com/gitlab-org/gitlab-ce into issue_17479_todos_not_remove_when_leave_project

.rubocop.yml

@@ -276,7 +276,7 @@ Style/IdenticalConditionalBranches:
   Enabled: false
 
 # Checks the indentation of the first line of the right-hand-side of a
-# multi-line assignment. 
+# multi-line assignment.
 Style/IndentAssignment:
   Enabled: false
 
@@ -531,7 +531,7 @@ Style/SpaceAroundKeyword:
 
 # Use a single space around operators.
 Style/SpaceAroundOperators:
-  Enabled: false
+  Enabled: true
 
 # Checks that the left block brace has or doesn't have space before it.
 Style/SpaceBeforeBlockBraces:

CHANGELOG

@@ -49,6 +49,8 @@ v 8.8.0 (unreleased)
   - Total method execution timings are no longer tracked
   - Allow Admins to remove the Login with buttons for OAuth services and still be able to import !4034. (Andrei Gliga)
   - Add API endpoints for un/subscribing from/to a label. !4051 (Ahmad Sherif)
+  - Hide left sidebar on phone screens to give more space for content
+  - Redesign navigation for profile and group pages
 
 v 8.7.5
   - Fix relative links in wiki pages. !4050

Gemfile

@@ -323,7 +323,6 @@ gem "mail_room", "~> 0.7"
 gem 'email_reply_parser', '~> 0.5.8'
 
 ## CI
-gem 'activerecord-deprecated_finders', '~> 1.0.3'
 gem 'activerecord-session_store', '~> 0.1.0'
 gem "nested_form", '~> 0.3.2'
 

Gemfile.lock

@@ -33,7 +33,6 @@ GEM
       activemodel (= 4.2.6)
       activesupport (= 4.2.6)
       arel (~> 6.0)
-    activerecord-deprecated_finders (1.0.4)
     activerecord-session_store (0.1.2)
       actionpack (>= 4.0.0, < 5)
       activerecord (>= 4.0.0, < 5)
@@ -883,7 +882,6 @@ PLATFORMS
 DEPENDENCIES
   RedCloth (~> 4.2.9)
   ace-rails-ap (~> 4.0.2)
-  activerecord-deprecated_finders (~> 1.0.3)
   activerecord-session_store (~> 0.1.0)
   acts-as-taggable-on (~> 3.4)
   addressable (~> 2.3.8)

app/assets/javascripts/notes.js.coffee

@@ -285,6 +285,7 @@ class @Notes
     form.addClass "js-main-target-form"
 
     form.find("#note_line_code").remove()
+    form.find("#note_type").remove()
 
   ###
   General note form setup.
@@ -472,6 +473,7 @@ class @Notes
   setupDiscussionNoteForm: (dataHolder, form) =>
     # setup note target
     form.attr 'id', "new-discussion-note-form-#{dataHolder.data("discussionId")}"
+    form.find("#note_type").val dataHolder.data("noteType")
     form.find("#line_type").val dataHolder.data("lineType")
     form.find("#note_commit_id").val dataHolder.data("commitId")
     form.find("#note_line_code").val dataHolder.data("lineCode")

app/assets/stylesheets/framework/common.scss

@@ -289,7 +289,7 @@ table {
   text-shadow: none;
 
   @media (min-width: $screen-sm-min) {
-    margin-top: 11px;
+    margin-top: 8px;
   }
 }
 

app/assets/stylesheets/framework/gitlab-theme.scss

@@ -9,9 +9,6 @@
 @mixin gitlab-theme($color-light, $color, $color-darker, $color-dark) {
   .page-with-sidebar {
     .header-logo {
-      background-color: $color;
-      border-color: $color;
-
       a {
         color: $color-light;
 
@@ -21,7 +18,7 @@
       }
 
       &:hover {
-        background-color: $color-darker;
+        background-color: $color-dark;
         a {
           color: #fff;
 

app/assets/stylesheets/framework/header.scss

@@ -6,12 +6,12 @@ header {
   transition-duration: .3s;
 
   &.navbar-empty {
-    height: 58px;
+    height: $header-height;
     background: #fff;
     border-bottom: 1px solid $btn-gray-hover;
 
     .center-logo {
-      margin: 11px 0;
+      margin: 8px 0;
       text-align: center;
 
       #tanuki-logo, img {

app/assets/stylesheets/framework/nav.scss

@@ -209,6 +209,15 @@
     float: right;
     padding: 7px 0 0;
 
+    @media (max-width: $screen-xs-min) {
+      float: none;
+      padding: 0 9px;
+
+      .dropdown-new {
+        width: 100%;
+      }
+    }
+
     i {
       color: $layout-link-gray;
     }
@@ -225,6 +234,10 @@
 
     .dropdown {
       margin-left: 7px;
+
+      @media (max-width: $screen-xs-min) {
+        margin-left: 0;
+      }
     }
   }
 
@@ -260,4 +273,10 @@
 
 .page-with-layout-nav {
   margin-top: 50px;
+
+  &.controls-dropdown-visible {
+    @media (max-width: $screen-xs-min) {
+      margin-top: 96px;
+    }
+  }
 }

app/assets/stylesheets/framework/sidebar.scss

@@ -312,7 +312,7 @@
     }
 
     .nav-sidebar li a {
-      width: 230px;
+      width: $sidebar_width;
 
       &.back-link {
         i {

app/assets/stylesheets/framework/variables.scss

@@ -2,7 +2,7 @@
  * Layout
  */
 $sidebar_collapsed_width: 62px;
-$sidebar_width: 230px;
+$sidebar_width: 220px;
 $gutter_collapsed_width: 62px;
 $gutter_width: 290px;
 $gutter_inner_width: 258px;

app/assets/stylesheets/pages/issuable.scss

@@ -125,7 +125,7 @@
 
 .right-sidebar {
   position: fixed;
-  top: 58px;
+  top: $header-height;
   bottom: 0;
   right: 0;
   z-index: 10;

app/assets/stylesheets/pages/notes.scss

@@ -226,8 +226,7 @@ ul.notes {
   }
 }
 
-.note-action-button,
-.discussion-action-button {
+.note-action-button {
   display: inline-block;
   margin-left: 10px;
   line-height: 24px;

app/controllers/application_controller.rb

@@ -176,7 +176,7 @@ class ApplicationController < ActionController::Base
   end
 
   def check_password_expiration
-    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now  && !current_user.ldap_user?
+    if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user?
       redirect_to new_profile_password_path and return
     end
   end

app/controllers/concerns/creates_commit.rb

@@ -122,7 +122,7 @@ module CreatesCommit
       # Merge request from fork to this project
       @mr_source_project = @tree_edit_project
       @mr_target_project = @project
-      @mr_target_branch  ||= @ref
+      @mr_target_branch ||= @ref
     end
   end
 end

app/controllers/projects/commit_controller.rb

@@ -17,12 +17,12 @@ class Projects::CommitController < Projects::ApplicationController
   def show
     apply_diff_view_cookie!
 
-    @line_notes = commit.notes.inline
+    @grouped_diff_notes = commit.notes.grouped_diff_notes
+
     @note = @project.build_commit_note(commit)
-    @notes = commit.notes.not_inline.fresh
+    @notes = commit.notes.non_diff_notes.fresh
     @noteable = @commit
-    @comments_allowed = @reply_allowed = true
-    @comments_target  = {
+    @comments_target = {
       noteable_type: 'Commit',
       commit_id: @commit.id
     }
@@ -67,10 +67,10 @@ class Projects::CommitController < Projects::ApplicationController
     create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title} has been successfully reverted.",
                                           success_path: successful_change_path, failure_path: failed_change_path)
   end
-  
+
   def cherry_pick
     assign_change_commit_vars(@commit.cherry_pick_branch_name)
-    
+
     return render_404 if @target_branch.blank?
 
     create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title} has been successfully cherry-picked.",

app/controllers/projects/compare_controller.rb

@@ -22,7 +22,8 @@ class Projects::CompareController < Projects::ApplicationController
       @base_commit = @project.merge_base_commit(@base_ref, @head_ref)
       @diffs = compare.diffs(diff_options)
       @diff_refs = [@base_commit, @commit]
-      @line_notes = []
+      @diff_notes_disabled = true
+      @grouped_diff_notes = {}
     end
   end
 

app/controllers/projects/merge_requests_controller.rb

@@ -73,12 +73,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     # but we need it for the "View file @ ..." link by deleted files
     @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit
 
-    @comments_allowed = @reply_allowed = true
     @comments_target = {
       noteable_type: 'MergeRequest',
       noteable_id: @merge_request.id
     }
-    @line_notes = @merge_request.notes.where("line_code is not null")
+
+    @grouped_diff_notes = @merge_request.notes.grouped_diff_notes
 
     respond_to do |format|
       format.html
@@ -117,6 +117,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @commit = @merge_request.last_commit
     @base_commit = @merge_request.diff_base_commit
     @diffs = @merge_request.compare.diffs(diff_options) if @merge_request.compare
+    @diff_notes_disabled = true
 
     @ci_commit = @merge_request.ci_commit
     @statuses = @ci_commit.statuses if @ci_commit
@@ -300,7 +301,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     # Build a note object for comment form
     @note = @project.notes.new(noteable: @merge_request)
     @notes = @merge_request.mr_and_commit_notes.nonawards.inc_author.fresh
-    @discussions = Note.discussions_from_notes(@notes)
+    @discussions = @notes.discussions
     @noteable = @merge_request
 
     # Get commits from repository

app/controllers/projects/notes_controller.rb

@@ -96,7 +96,7 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def note_to_discussion_html(note)
-    return unless note.for_diff_line?
+    return unless note.diff_note?
 
     if params[:view] == 'parallel'
       template = "projects/notes/_diff_notes_with_reply_parallel"
@@ -120,7 +120,7 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def note_to_discussion_with_diff_html(note)
-    return unless note.for_diff_line?
+    return unless note.diff_note?
 
     render_to_string(
       "projects/notes/_discussion",
@@ -158,7 +158,7 @@ class Projects::NotesController < Projects::ApplicationController
   def note_params
     params.require(:note).permit(
       :note, :noteable, :noteable_id, :noteable_type, :project_id,
-      :attachment, :line_code, :commit_id
+      :attachment, :line_code, :commit_id, :type
     )
   end
 

app/finders/notes_finder.rb

@@ -10,7 +10,7 @@ class NotesFinder
     notes =
       case target_type
       when "commit"
-        project.notes.for_commit_id(target_id).not_inline
+        project.notes.for_commit_id(target_id).non_diff_notes
       when "issue"
         project.issues.find(target_id).notes.nonawards.inc_author
       when "merge_request"

app/helpers/diff_helper.rb

@@ -55,22 +55,18 @@ module DiffHelper
     end
   end
 
-  def line_comments
-    @line_comments ||= @line_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
-  end
-
-  def organize_comments(type_left, type_right, line_code_left, line_code_right)
-    comments_left = comments_right = nil
+  def organize_comments(left, right)
+    notes_left = notes_right = nil
 
-    unless type_left.nil? && type_right == 'new'
-      comments_left = line_comments[line_code_left]
+    unless left[:type].nil? && right[:type] == 'new'
+      notes_left = @grouped_diff_notes[left[:line_code]]
     end
 
-    unless type_left.nil? && type_right.nil?
-      comments_right = line_comments[line_code_right]
+    unless left[:type].nil? && right[:type].nil?
+      notes_right = @grouped_diff_notes[right[:line_code]]
     end
 
-    [comments_left, comments_right]
+    [notes_left, notes_right]
   end
 
   def inline_diff_btn
@@ -96,8 +92,8 @@ module DiffHelper
     ].join(' ').html_safe
   end
 
-  def commit_for_diff(diff)
-    if diff.deleted_file
+  def commit_for_diff(diff_file)
+    if diff_file.deleted_file
       @base_commit || @commit.parent || @commit
     else
       @commit

app/helpers/nav_helper.rb

@@ -43,4 +43,12 @@ module NavHelper
     class_name += " with-horizontal-nav" if defined?(nav) && nav
     class_name
   end
+
+  def layout_nav_class
+    "page-with-layout-nav" if defined?(nav) && nav
+  end
+
+  def layout_dropdown_class
+    "controls-dropdown-visible" if current_user
+  end
 end

app/helpers/notes_helper.rb

 module NotesHelper
   # Helps to distinguish e.g. commit notes in mr notes list
   def note_for_main_target?(note)
-    (@noteable.class.name == note.noteable_type && !note.for_diff_line?)
+    @noteable.class.name == note.noteable_type && !note.diff_note?
   end
 
   def note_target_fields(note)
@@ -15,16 +15,6 @@ module NotesHelper
     note.editable? && can?(current_user, :admin_note, note)
   end
 
-  def link_to_commit_diff_line_note(note)
-    if note.for_commit_diff_line?
-      link_to(
-        "#{note.diff_file_name}:L#{note.diff_new_line}",
-        namespace_project_commit_path(@project.namespace, @project,
-                                      note.noteable, anchor: note.line_code)
-      )
-    end
-  end
-
   def noteable_json(noteable)
     {
       id: noteable.id,
@@ -35,7 +25,7 @@ module NotesHelper
   end
 
   def link_to_new_diff_note(line_code, line_type = nil)
-    discussion_id = Note.build_discussion_id(
+    discussion_id = LegacyDiffNote.build_discussion_id(
       @comments_target[:noteable_type],
       @comments_target[:noteable_id] || @comments_target[:commit_id],
       line_code
@@ -45,9 +35,10 @@ module NotesHelper
       noteable_type: @comments_target[:noteable_type],
       noteable_id:   @comments_target[:noteable_id],
       commit_id:     @comments_target[:commit_id],
+      line_type:     line_type,
       line_code:     line_code,
-      discussion_id: discussion_id,
-      line_type:     line_type
+      note_type:     LegacyDiffNote.name,
+      discussion_id: discussion_id
     }
 
     button_tag(class: 'btn add-diff-note js-add-diff-note-button',
@@ -57,18 +48,24 @@ module NotesHelper
     end
   end
 
-  def link_to_reply_diff(note, line_type = nil)
+  def link_to_reply_discussion(note, line_type = nil)
     return unless current_user
 
     data = {
       noteable_type: note.noteable_type,
       noteable_id:   note.noteable_id,
       commit_id:     note.commit_id,
-      line_code:     note.line_code,
       discussion_id: note.discussion_id,
       line_type:     line_type
     }
 
+    if note.diff_note?
+      data.merge!(
+        line_code: note.line_code,
+        note_type: LegacyDiffNote.name
+      )
+    end
+
     button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
                            data: data, title: 'Add a reply'
   end

app/helpers/search_helper.rb

@@ -59,7 +59,7 @@ module SearchHelper
   # Autocomplete results for the current project, if it's defined
   def project_autocomplete
     if @project && @project.repository.exists? && @project.repository.root_ref
-      ref    = @ref || @project.repository.root_ref
+      ref = @ref || @project.repository.root_ref
 
       [
         { category: "Current Project", label: "Files",          url: namespace_project_tree_path(@project.namespace, @project, ref) },

app/helpers/selects_helper.rb

@@ -18,7 +18,7 @@ module SelectsHelper
         first_user: first_user,
         current_user: opts[:current_user] || false,
         "push-code-to-protected-branches" => opts[:push_code_to_protected_branches],
-        author_id: opts[:author_id]       || ''
+        author_id: opts[:author_id] || ''
       }
     }
 

app/models/legacy_diff_note.rb

+class LegacyDiffNote < Note
+  serialize :st_diff
+
+  validates :line_code, presence: true, line_code: true
+
+  before_create :set_diff
+
+  class << self
+    def build_discussion_id(noteable_type, noteable_id, line_code, active = true)
+      [super(noteable_type, noteable_id), line_code, active].join("-")
+    end
+  end
+
+  def diff_note?
+    true
+  end
+
+  def legacy_diff_note?
+    true
+  end
+
+  def discussion_id
+    @discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, line_code, active?)
+  end
+
+  def diff_file_hash
+    line_code.split('_')[0] if line_code
+  end
+
+  def diff_old_line
+    line_code.split('_')[1].to_i if line_code
+  end
+
+  def diff_new_line
+    line_code.split('_')[2].to_i if line_code
+  end
+
+  def diff
+    @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
+  end
+
+  def diff_file_path
+    diff.new_path.presence || diff.old_path
+  end
+
+  def diff_lines
+    @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
+  end
+
+  def diff_line
+    @diff_line ||= diff_lines.find { |line| generate_line_code(line) == self.line_code }
+  end
+
+  def diff_line_text
+    diff_line.try(:text)
+  end
+
+  def diff_line_type
+    diff_line.try(:type)
+  end
+
+  def highlighted_diff_lines
+    Gitlab::Diff::Highlight.new(diff_lines).highlight
+  end
+
+  def truncated_diff_lines
+    max_number_of_lines = 16
+    prev_match_line = nil
+    prev_lines = []
+
+    highlighted_diff_lines.each do |line|
+      if line.type == "match"
+        prev_lines.clear
+        prev_match_line = line
+      else
+        prev_lines << line
+
+        break if generate_line_code(line) == self.line_code
+
+        prev_lines.shift if prev_lines.length >= max_number_of_lines
+      end
+    end
+
+    prev_lines
+  end
+
+  # Check if this note is part of an "active" discussion
+  #
+  # This will always return true for anything except MergeRequest noteables,
+  # which have special logic.
+  #
+  # If the note's current diff cannot be matched in the MergeRequest's current
+  # diff, it's considered inactive.
+  def active?
+    return @active if defined?(@active)
+    return true if for_commit?
+    return true unless self.diff
+    return false unless noteable
+
+    noteable_diff = find_noteable_diff
+
+    if noteable_diff
+      parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
+
+      @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line_text }
+    else
+      @active = false
+    end
+
+    @active
+  end
+
+  private
+
+  def find_diff
+    return nil unless noteable
+    return @diff if defined?(@diff)
+
+    @diff = noteable.diffs(Commit.max_diff_options).find do |d|
+      d.new_path && Digest::SHA1.hexdigest(d.new_path) == diff_file_hash
+    end
+  end
+
+  def set_diff
+    # First lets find notes with same diff
+    # before iterating over all mr diffs
+    diff = diff_for_line_code unless for_merge_request?
+    diff ||= find_diff
+
+    self.st_diff = diff.to_hash if diff
+  end
+
+  def diff_for_line_code
+    attributes = {
+      noteable_type: noteable_type,
+      line_code: line_code
+    }
+
+    if for_commit?
+      attributes[:commit_id] = commit_id
+    else
+      attributes[:noteable_id] = noteable_id
+    end
+
+    self.class.where(attributes).last.try(:diff)
+  end
+
+  def generate_line_code(line)
+    Gitlab::Diff::LineCode.generate(diff_file_path, line.new_pos, line.old_pos)
+  end
+
+  # Find the diff on noteable that matches our own
+  def find_noteable_diff
+    diffs = noteable.diffs(Commit.max_diff_options)
+    diffs.find { |d| d.new_path == self.diff.new_path }
+  end
+end

app/models/note.rb

-require 'carrierwave/orm/activerecord'
-
 class Note < ActiveRecord::Base
+  extend ActiveModel::Naming
   include Gitlab::CurrentSettings
   include Participable
   include Mentionable
@@ -22,12 +21,10 @@ class Note < ActiveRecord::Base
   delegate :name, :email, to: :author, prefix: true
 
   before_validation :set_award!
-  before_validation :clear_blank_line_code!
 
   validates :note, :project, presence: true
   validates :note, uniqueness: { scope: [:author, :noteable_type, :noteable_id] }, if: ->(n) { n.is_award }
   validates :note, inclusion: { in: Emoji.emojis_names }, if: ->(n) { n.is_award }
-  validates :line_code, line_code: true, allow_blank: true
   # Attachments are deprecated and are handled by Markdown uploader
   validates :attachment, file_size: { maximum: :max_attachment_size }
 
@@ -41,8 +38,6 @@ class Note < ActiveRecord::Base
   scope :awards, ->{ where(is_award: true) }
   scope :nonawards, ->{ where(is_award: false) }
   scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) }
-  scope :inline, ->{ where("line_code IS NOT NULL") }
-  scope :not_inline, ->{ where(line_code: nil) }
   scope :system, ->{ where(system: true) }
   scope :user, ->{ where(system: false) }
   scope :common, ->{ where(noteable_type: ["", nil]) }
@@ -50,38 +45,31 @@ class Note < ActiveRecord::Base
   scope :inc_author_project, ->{ includes(:project, :author) }
   scope :inc_author, ->{ includes(:author) }
 
+  scope :legacy_diff_notes, ->{ where(type: 'LegacyDiffNote') }
+  scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
+
   scope :with_associations, -> do
     includes(:author, :noteable, :updated_by,
              project: [:project_members, { group: [:group_members] }])
   end
 
-  serialize :st_diff
-  before_create :set_diff, if: ->(n) { n.line_code.present? }
+  before_validation :clear_blank_line_code!
 
   class << self
-    def discussions_from_notes(notes)
-      discussion_ids = []
-      discussions = []
-
-      notes.each do |note|
-        next if discussion_ids.include?(note.discussion_id)
-
-        # don't group notes for the main target
-        if !note.for_diff_line? && note.for_merge_request?
-          discussions << [note]
-        else
-          discussions << notes.select do |other_note|
-            note.discussion_id == other_note.discussion_id
-          end
-          discussion_ids << note.discussion_id
-        end
-      end
+    def model_name
+      ActiveModel::Name.new(self, nil, 'note')
+    end
+
+    def build_discussion_id(noteable_type, noteable_id)
+      [:discussion, noteable_type.try(:underscore), noteable_id].join("-")
+    end
 
-      discussions
+    def discussions
+      all.group_by(&:discussion_id).values
     end
 
-    def build_discussion_id(type, id, line_code)
-      [:discussion, type.try(:underscore), id, line_code].join("-").to_sym
+    def grouped_diff_notes
+      legacy_diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
     end
 
     # Searches for notes matching the given query.
@@ -116,167 +104,39 @@ class Note < ActiveRecord::Base
     system && SystemNoteService.cross_reference?(note)
   end
 
-  def max_attachment_size
-    current_application_settings.max_attachment_size.megabytes.to_i
-  end
-
-  def find_diff
-    return nil unless noteable
-    return @diff if defined?(@diff)
-
-    # Don't use ||= because nil is a valid value for @diff
-    @diff = noteable.diffs(Commit.max_diff_options).find do |d|
-      Digest::SHA1.hexdigest(d.new_path) == diff_file_index if d.new_path
-    end
-  end
-
-  def hook_attrs
-    attributes
+  def diff_note?
+    false
   end
 
-  def set_diff
-    # First lets find notes with same diff
-    # before iterating over all mr diffs
-    diff = diff_for_line_code unless for_merge_request?
-    diff ||= find_diff
-
-    self.st_diff = diff.to_hash if diff
+  def legacy_diff_note?
+    false
   end
 
-  def diff
-    @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
-  end
-
-  def diff_for_line_code
-    Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
-  end
-
-  # Check if this note is part of an "active" discussion
-  #
-  # This will always return true for anything except MergeRequest noteables,
-  # which have special logic.
-  #
-  # If the note's current diff cannot be matched in the MergeRequest's current
-  # diff, it's considered inactive.
   def active?
-    return true unless self.diff
-    return false unless noteable
-    return @active if defined?(@active)
-
-    noteable_diff = find_noteable_diff
-
-    if noteable_diff
-      parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
-
-      @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line }
-    else
-      @active = false
-    end
-
-    @active
-  end
-
-  def diff_file_index
-    line_code.split('_')[0] if line_code
-  end
-
-  def diff_file_name
-    diff.new_path if diff
+    true
   end
 
-  def file_path
-    if diff.new_path.present?
-      diff.new_path
-    elsif diff.old_path.present?
-      diff.old_path
-    end
-  end
-
-  def diff_old_line
-    line_code.split('_')[1].to_i if line_code
-  end
-
-  def diff_new_line
-    line_code.split('_')[2].to_i if line_code
-  end
-
-  def generate_line_code(line)
-    Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
-  end
-
-  def diff_line
-    return @diff_line if @diff_line
-
-    if diff
-      diff_lines.each do |line|
-        if generate_line_code(line) == self.line_code
-          @diff_line = line.text
-        end
-      end
-    end
-
-    @diff_line
-  end
-
-  def diff_line_type
-    return @diff_line_type if @diff_line_type
-
-    if diff
-      diff_lines.each do |line|
-        if generate_line_code(line) == self.line_code
-          @diff_line_type = line.type
-        end
-      end
-    end
-
-    @diff_line_type
-  end
-
-  def truncated_diff_lines
-    max_number_of_lines = 16
-    prev_match_line = nil
-    prev_lines = []
-
-    highlighted_diff_lines.each do |line|
-      if line.type == "match"
-        prev_lines.clear
-        prev_match_line = line
+  def discussion_id
+    @discussion_id ||=
+      if for_merge_request?
+        [:discussion, :note, id].join("-")
       else
-        prev_lines << line
-
-        break if generate_line_code(line) == self.line_code
-
-        prev_lines.shift if prev_lines.length >= max_number_of_lines
+        self.class.build_discussion_id(noteable_type, noteable_id || commit_id)
       end
-    end
-
-    prev_lines
-  end
-
-  def diff_lines
-    @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
   end
 
-  def highlighted_diff_lines
-    Gitlab::Diff::Highlight.new(diff_lines).highlight
+  def max_attachment_size
+    current_application_settings.max_attachment_size.megabytes.to_i
   end
 
-  def discussion_id
-    @discussion_id ||= Note.build_discussion_id(noteable_type, noteable_id || commit_id, line_code)
+  def hook_attrs
+    attributes
   end
 
   def for_commit?
     noteable_type == "Commit"
   end
 
-  def for_commit_diff_line?
-    for_commit? && for_diff_line?
-  end
-
-  def for_diff_line?
-    line_code.present?
-  end
-
   def for_issue?
     noteable_type == "Issue"
   end
@@ -285,10 +145,6 @@ class Note < ActiveRecord::Base
     noteable_type == "MergeRequest"
   end
 
-  def for_merge_request_diff_line?
-    for_merge_request? && for_diff_line?
-  end
-
   def for_snippet?
     noteable_type == "Snippet"
   end
@@ -361,14 +217,8 @@ class Note < ActiveRecord::Base
     self.line_code = nil if self.line_code.blank?
   end
 
-  # Find the diff on noteable that matches our own
-  def find_noteable_diff
-    diffs = noteable.diffs(Commit.max_diff_options)
-    diffs.find { |d| d.new_path == self.diff.new_path }
-  end
-
   def awards_supported?
-    (for_issue? || for_merge_request?) && !for_diff_line?
+    (for_issue? || for_merge_request?) && !diff_note?
   end
 
   def contains_emoji_only?

app/models/project_services/external_wiki_service.rb

@@ -25,7 +25,7 @@ class ExternalWikiService < Service
 
   def execute(_data)
     @response = HTTParty.get(properties['external_wiki_url'], verify: true) rescue nil
-    if @response !=200
+    if @response != 200
       nil
     end
   end

app/models/project_wiki.rb

@@ -113,7 +113,7 @@ class ProjectWiki
   end
 
   def page_title_and_dir(title)
-    title_array =  title.split("/")
+    title_array = title.split("/")
     title = title_array.pop
     [title, title_array.join("/")]
   end

app/models/repository.rb

@@ -795,7 +795,7 @@ class Repository
   def check_revert_content(commit, base_branch)
     source_sha = find_branch(base_branch).target
     args       = [commit.id, source_sha]
-    args       << { mainline: 1 } if commit.merge_commit?
+    args << { mainline: 1 } if commit.merge_commit?
 
     revert_index = rugged.revert_commit(*args)
     return false if revert_index.conflicts?
@@ -809,7 +809,7 @@ class Repository
   def check_cherry_pick_content(commit, base_branch)
     source_sha = find_branch(base_branch).target
     args       = [commit.id, source_sha]
-    args       << 1 if commit.merge_commit?
+    args << 1 if commit.merge_commit?
 
     cherry_pick_index = rugged.cherrypick_commit(*args)
     return false if cherry_pick_index.conflicts?

app/services/merge_requests/build_service.rb

@@ -66,7 +66,7 @@ module MergeRequests
       commits = merge_request.compare_commits
       if commits && commits.count == 1
         commit = commits.first
-        merge_request.title       = commit.title
+        merge_request.title = commit.title
         merge_request.description ||= commit.description.try(:strip)
       elsif iid && (issue = merge_request.target_project.get_issue(iid)) && !issue.try(:confidential?)
         case issue

app/services/system_hooks_service.rb

@@ -85,7 +85,7 @@ class SystemHooksService
       path_with_namespace: model.path_with_namespace,
       project_id: model.id,
       owner_name: owner.name,
-      owner_email: owner.respond_to?(:email) ?  owner.email : "",
+      owner_email: owner.respond_to?(:email) ? owner.email : "",
       project_visibility: Project.visibility_levels.key(model.visibility_level_field).downcase
     }
   end

app/views/layouts/_page.html.haml

 .page-with-sidebar{ class: "#{page_sidebar_class} #{page_gutter_class}" }
-  = render "layouts/broadcast"
   .sidebar-wrapper.nicescroll{ class: nav_sidebar_class }
     .header-logo
       %a#logo
@@ -26,7 +25,8 @@
     .layout-nav
       .container-fluid
         = render "layouts/nav/#{nav}"
-  .content-wrapper{ class: ('page-with-layout-nav' if defined?(nav) && nav) }
+  .content-wrapper{ class: "#{layout_nav_class} #{layout_dropdown_class}" }
+    = render "layouts/broadcast"
     = render "layouts/flash"
     = yield :flash_message
     %div{ class: (container_class unless @no_container) }

app/views/layouts/nav/_group_settings.html.haml

 - if current_user
   - if access = @group.users.find_by(id: current_user.id)
     .controls
-      %span.dropdown.group-settings-dropdown
+      .dropdown.group-settings-dropdown
         %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'}
           = icon('cog')
           = icon('caret-down')

app/views/notify/note_merge_request_email.html.haml

-- if @note.diff_file_name
+- if @note.legacy_diff_note?
   %p.details
     New comment on diff for
-    = link_to @note.diff_file_name, @target_url
+    = link_to @note.diff_file_path, @target_url
     \:
 
 = render 'note_message'

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

@@ -15,7 +15,7 @@
         = link_text
       - else
         = link_to "", "##{line_code}", id: line_code, data: { linenumber: link_text }
-      - if @comments_allowed && can?(current_user, :create_note, @project)
+      - if !@diff_notes_disabled && can?(current_user, :create_note, @project)
         = link_to_new_diff_note(line_code)
     %td.new_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } }
       - link_text = type == "old" ? " ".html_safe : line.new_pos

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

@@ -16,7 +16,7 @@
         - else
           %td.old_line.diff-line-num{id: left[:line_code], class: "#{left[:type]} #{'empty-cell' if !left[:number]}"}
             = link_to raw(left[:number]), "##{left[:line_code]}", id: left[:line_code]
-            - if @comments_allowed && can?(current_user, :create_note, @project)
+            - if !@diff_notes_disabled && can?(current_user, :create_note, @project)
               = link_to_new_diff_note(left[:line_code], 'old')
           %td.line_content{class: "parallel noteable_line #{left[:type]} #{left[:line_code]} #{'empty-cell' if left[:text].empty?}", data: { line_code: left[:line_code] }}= diff_line_content(left[:text])
 
@@ -29,14 +29,14 @@
 
           %td.new_line.diff-line-num{id: new_line_code, class: "#{new_line_class} #{'empty-cell' if !right[:number]}", data: { linenumber: right[:number] }}
             = link_to raw(right[:number]), "##{new_line_code}", id: new_line_code
-            - if @comments_allowed && can?(current_user, :create_note, @project)
-              = link_to_new_diff_note(right[:line_code], 'new')
+            - if !@diff_notes_disabled && can?(current_user, :create_note, @project)
+              = link_to_new_diff_note(new_line_code, 'new')
           %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code} #{'empty-cell' if right[:text].empty?}", data: { line_code: new_line_code }}= diff_line_content(right[:text])
 
-      - if @reply_allowed
-        - comments_left, comments_right = organize_comments(left[:type], right[:type], left[:line_code], right[:line_code])
-        - if comments_left.present? || comments_right.present?
-          = render "projects/notes/diff_notes_with_reply_parallel", notes_left: comments_left, notes_right: comments_right
+      - unless @diff_notes_disabled
+        - notes_left, notes_right = organize_comments(left, right)
+        - if notes_left.present? || notes_right.present?
+          = render "projects/notes/diff_notes_with_reply_parallel", notes_left: notes_left, notes_right: notes_right
 
 - if diff_file.diff.diff.blank? && diff_file.mode_changed?
   .file-mode-changed

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

@@ -6,16 +6,15 @@
 %table.text-file.code.js-syntax-highlight{ class: too_big ? 'hide' : '' }
 
   - last_line = 0
-  - raw_diff_lines = diff_file.diff_lines.to_a
   - diff_file.highlighted_diff_lines.each_with_index do |line, index|
     - line_code = generate_line_code(diff_file.file_path, line)
     - last_line = line.new_pos
     = render "projects/diffs/line", {line: line, diff_file: diff_file, line_code: line_code}
 
-    - if @reply_allowed
-      - comments = @line_notes.select { |n| n.line_code == line_code && n.active? }.sort_by(&:created_at)
-      - unless comments.empty?
-        = render "projects/notes/diff_notes_with_reply", notes: comments, line: raw_diff_lines[index].text
+    - unless @diff_notes_disabled
+      - diff_notes = @grouped_diff_notes[line_code]
+      - if diff_notes
+        = render "projects/notes/diff_notes_with_reply", notes: diff_notes
 
   - if last_line > 0
     = render "projects/diffs/match_line", { line: "",

app/views/projects/notes/_diff_notes_with_reply.html.haml

-- note = notes.first # example note
--# Check if line want not changed since comment was left
-- if !defined?(line) || line == note.diff_line
-  %tr.notes_holder
-    %td.notes_line{ colspan: 2 }
-    %td.notes_content
-      %ul.notes{ data: { discussion_id: note.discussion_id } }
-        = render notes
-      .discussion-reply-holder
-        = link_to_reply_diff(note)
+- note = notes.first
+%tr.notes_holder
+  %td.notes_line{ colspan: 2 }
+  %td.notes_content
+    %ul.notes{ data: { discussion_id: note.discussion_id } }
+      = render partial: "projects/notes/note", collection: notes, as: :note
+    .discussion-reply-holder
+      = link_to_reply_discussion(note)

app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml

-- note1 = notes_left.present? ? notes_left.first : nil
-- note2 = notes_right.present? ? notes_right.first : nil
+- note_left = notes_left.present? ? notes_left.first : nil
+- note_right = notes_right.present? ? notes_right.first : nil
 
 %tr.notes_holder
-  - if note1
+  - if note_left
     %td.notes_line.old
     %td.notes_content.parallel.old
-      %ul.notes{ data: { discussion_id: note1.discussion_id } }
-        = render notes_left
+      %ul.notes{ data: { discussion_id: note_left.discussion_id } }
+        = render partial: "projects/notes/note", collection: notes_left, as: :note
 
       .discussion-reply-holder
-        = link_to_reply_diff(note1, 'old')
+        = link_to_reply_discussion(note_left, 'old')
   - else
     %td.notes_line.old= ""
     %td.notes_content.parallel.old= ""
 
-  - if note2
+  - if note_right
     %td.notes_line.new
     %td.notes_content.parallel.new
-      %ul.notes{ data: { discussion_id: note2.discussion_id } }
-        = render notes_right
+      %ul.notes{ data: { discussion_id: note_right.discussion_id } }
+        = render partial: "projects/notes/note", collection: notes_right, as: :note
 
       .discussion-reply-holder
-        = link_to_reply_diff(note2, 'new')
+        = link_to_reply_discussion(note_right, 'new')
   - else
     %td.notes_line.new= ""
     %td.notes_content.parallel.new= ""

app/views/projects/notes/_discussion.html.haml

 - note = discussion_notes.first
+- expanded = !note.diff_note? || note.active?
 %li.note.note-discussion.timeline-entry
   .timeline-entry-inner
     .timeline-icon
       = link_to user_path(note.author) do
-        = image_tag avatar_icon(note.author_email), class: "avatar s40"
+        = image_tag avatar_icon(note.author), class: "avatar s40"
     .timeline-content
-      - if note.for_merge_request?
-        - (active_notes, outdated_notes) = discussion_notes.partition(&:active?)
-        = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0
-        = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0
-      - else
-        = render "projects/notes/discussions/commit", discussion_notes: discussion_notes
+      .discussion.js-toggle-container{ class: note.discussion_id }
+        .discussion-header
+          = link_to_member(@project, note.author, avatar: false)
+
+          .inline.discussion-headline-light
+            = note.author.to_reference
+            started a discussion on
+
+            - if note.for_commit?
+              - commit = note.noteable
+              - if commit
+                commit
+                = link_to commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code), class: 'monospace'
+              - else
+                a deleted commit
+            - else
+              - if note.active?
+                = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
+                  the diff
+              - else
+                an outdated diff
+
+            = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "note-created-ago")
+
+          .discussion-actions
+            = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
+              - if expanded
+                = icon("chevron-up")
+              - else
+                = icon("chevron-down")
+
+              Toggle discussion
+
+        .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
+          - if note.diff_note?
+            = render "projects/notes/discussions/diff_with_notes", discussion_notes: discussion_notes
+          - else
+            = render "projects/notes/discussions/notes", discussion_notes: discussion_notes

app/views/projects/notes/_form.html.haml

@@ -6,6 +6,7 @@
   = f.hidden_field :line_code
   = f.hidden_field :noteable_id
   = f.hidden_field :noteable_type
+  = f.hidden_field :type
 
   = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do
     = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text', placeholder: "Write a comment or drag your files here..."

app/views/projects/notes/_note.html.haml

+- return unless note.author
+- return if note.cross_reference_not_visible_for?(current_user)
+
 - note_editable = note_editable?(note)
-%li.timeline-entry{ id: dom_id(note), class: [dom_class(note), "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable} }
+%li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable} }
   .timeline-entry-inner
     .timeline-icon
       %a{href: user_path(note.author)}
@@ -8,8 +11,8 @@
       .note-header
         = link_to_member(note.project, note.author, avatar: false)
         .inline.note-headline-light
-          = "#{note.author.to_reference}"
-          - if !note.system
+          = note.author.to_reference
+          - unless note.system
             commented
           %a{ href: "##{dom_id(note)}" }
             = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')

app/views/projects/notes/_notes.html.haml

@@ -2,14 +2,9 @@
   - @discussions.each do |discussion_notes|
     - note = discussion_notes.first
     - if note_for_main_target?(note)
-      - next if note.cross_reference_not_visible_for?(current_user)
-
-      = render discussion_notes
+      = render partial: "projects/notes/note", object: note, as: :note
     - else
       = render 'projects/notes/discussion', discussion_notes: discussion_notes
 - else
   - @notes.each do |note|
-    - next unless note.author
-    - next if note.cross_reference_not_visible_for?(current_user)
-
-    = render note
+    = render partial: "projects/notes/note", object: note, as: :note

app/views/projects/notes/discussions/_active.html.haml

-- note = discussion_notes.first
-.discussion.js-toggle-container{ class: note.discussion_id }
-  .discussion-header
-    = link_to_member(@project, note.author, avatar: false)
-    .inline.discussion-headline-light
-      = "#{note.author.to_reference} started a discussion"
-      = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
-        on the diff
-      = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago")
-    .discussion-actions
-      = link_to "#", class: "discussion-action-button discussion-toggle-button js-toggle-button" do
-        %i.fa.fa-chevron-up
-        Show/hide discussion
-
-  .discussion-body.js-toggle-content
-    = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note

app/views/projects/notes/discussions/_commit.html.haml

-- note = discussion_notes.first
-- commit = note.noteable
-- commit_description = commit ? 'commit' : 'a deleted commit'
-.discussion.js-toggle-container{ class: note.discussion_id }
-  .discussion-header
-    = link_to_member(@project, note.author, avatar: false)
-    .inline.discussion-headline-light
-      = "#{note.author.to_reference} started a discussion on #{commit_description}"
-      - if commit
-        = link_to(commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable), class: 'monospace')
-      = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago")
-    .discussion-actions
-      = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
-        %i.fa.fa-chevron-up
-        Show/hide discussion
-  .discussion-body.js-toggle-content
-    - if note.for_diff_line?
-      = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note
-    - else
-      .panel.panel-default
-        .notes{ data: { discussion_id: discussion_notes.first.discussion_id } }
-          %ul.notes.timeline
-            = render discussion_notes
-        .discussion-reply-holder
-          = link_to_reply_diff(discussion_notes.first)

app/views/projects/notes/discussions/_diff.html.haml

-- diff = note.diff
-- if diff
-  .diff-file
-    .diff-header
-      %span
-        - if diff.deleted_file
-          = diff.old_path
-        - else
-          = diff.new_path
-          - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
-            %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}"
-    .diff-content.code.js-syntax-highlight
-      %table
-        - note.truncated_diff_lines.each do |line|
-          - type = line.type
-          - line_code = generate_line_code(note.file_path, line)
-          %tr.line_holder{ id: line_code, class: "#{type}" }
-            - if type == "match"
-              %td.old_line.diff-line-num= "..."
-              %td.new_line.diff-line-num= "..."
-              %td.line_content.match= line.text
-            - else
-              %td.old_line.diff-line-num{ data: { linenumber: type == "new" ? " ".html_safe : line.old_pos } }
-              %td.new_line.diff-line-num{ data: { linenumber: type == "old" ? " ".html_safe : line.new_pos } }
-              %td.line_content{ class: ['noteable_line', type, line_code], line_code: line_code }= diff_line_content(line.text, type)
-
-              - if line_code == note.line_code
-                = render "projects/notes/diff_notes_with_reply", notes: discussion_notes

app/views/projects/notes/discussions/_diff_with_notes.html.haml

+- note = discussion_notes.first
+- diff = note.diff
+- return unless diff
+
+.diff-file
+  .diff-header
+    %span
+      - if diff.deleted_file
+        = diff.old_path
+      - else
+        = diff.new_path
+        - if diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
+          %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}"
+  .diff-content.code.js-syntax-highlight
+    %table
+      - note.truncated_diff_lines.each do |line|
+        - type = line.type
+        - line_code = generate_line_code(note.diff_file_path, line)
+        %tr.line_holder{ id: line_code, class: "#{type}" }
+          - if type == "match"
+            %td.old_line.diff-line-num= "..."
+            %td.new_line.diff-line-num= "..."
+            %td.line_content.match= line.text
+          - else
+            %td.old_line.diff-line-num{ data: { linenumber: type == "new" ? " ".html_safe : line.old_pos } }
+            %td.new_line.diff-line-num{ data: { linenumber: type == "old" ? " ".html_safe : line.new_pos } }
+            %td.line_content{ class: ['noteable_line', type, line_code], line_code: line_code }= diff_line_content(line.text, type)
+
+            - if line_code == note.line_code
+              = render "projects/notes/diff_notes_with_reply", notes: discussion_notes

app/views/projects/notes/discussions/_notes.html.haml

+- note = discussion_notes.first
+.panel.panel-default
+  .notes{ data: { discussion_id: note.discussion_id } }
+    %ul.notes.timeline
+      = render partial: "projects/notes/note", collection: discussion_notes, as: :note
+  .discussion-reply-holder
+    = link_to_reply_discussion(note)

app/views/projects/notes/discussions/_outdated.html.haml

-- note = discussion_notes.first
-.discussion.js-toggle-container{ class: note.discussion_id }
-  .discussion-header
-    = link_to_member(@project, note.author, avatar: false)
-    .inline.discussion-headline-light
-      = "#{note.author.to_reference} started a discussion"
-      on the outdated diff
-      = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago")
-    .discussion-actions
-      = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
-        %i.fa.fa-chevron-down
-        Show/hide discussion
-  .discussion-body.js-toggle-content.hide
-    = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note

config/environments/test.rb

@@ -20,7 +20,7 @@ Rails.application.configure do
   config.action_dispatch.show_exceptions = false
 
   # Disable request forgery protection in test environment
-  config.action_controller.allow_forgery_protection    = false
+  config.action_controller.allow_forgery_protection = false
 
   # Tell Action Mailer not to deliver emails to the real world.
   # The :test delivery method accumulates sent emails in the

config/initializers/1_settings.rb

@@ -126,7 +126,7 @@ end
 
 
 Settings['omniauth'] ||= Settingslogic.new({})
-Settings.omniauth['enabled']      = false if Settings.omniauth['enabled'].nil?
+Settings.omniauth['enabled'] = false if Settings.omniauth['enabled'].nil?
 Settings.omniauth['auto_sign_in_with_provider'] = false if Settings.omniauth['auto_sign_in_with_provider'].nil?
 Settings.omniauth['allow_single_sign_on'] = false if Settings.omniauth['allow_single_sign_on'].nil?
 Settings.omniauth['external_providers'] = [] if Settings.omniauth['external_providers'].nil?
@@ -134,7 +134,7 @@ Settings.omniauth['block_auto_created_users'] = true if Settings.omniauth['block
 Settings.omniauth['auto_link_ldap_user'] = false if Settings.omniauth['auto_link_ldap_user'].nil?
 Settings.omniauth['auto_link_saml_user'] = false if Settings.omniauth['auto_link_saml_user'].nil?
 
-Settings.omniauth['providers']  ||= []
+Settings.omniauth['providers'] ||= []
 Settings.omniauth['cas3'] ||= Settingslogic.new({})
 Settings.omniauth.cas3['session_duration'] ||= 8.hours
 Settings.omniauth['session_tickets'] ||= Settingslogic.new({})
@@ -168,7 +168,7 @@ end
 Settings['shared'] ||= Settingslogic.new({})
 Settings.shared['path'] = File.expand_path(Settings.shared['path'] || "shared", Rails.root)
 
-Settings['issues_tracker']  ||= {}
+Settings['issues_tracker'] ||= {}
 
 #
 # GitLab
@@ -183,7 +183,7 @@ Settings.gitlab['ssh_host']   ||= Settings.gitlab.host
 Settings.gitlab['https']        = false if Settings.gitlab['https'].nil?
 Settings.gitlab['port']       ||= Settings.gitlab.https ? 443 : 80
 Settings.gitlab['relative_url_root'] ||= ENV['RAILS_RELATIVE_URL_ROOT'] || ''
-Settings.gitlab['protocol']   ||= Settings.gitlab.https ? "https" : "http"
+Settings.gitlab['protocol'] ||= Settings.gitlab.https ? "https" : "http"
 Settings.gitlab['email_enabled'] ||= true if Settings.gitlab['email_enabled'].nil?
 Settings.gitlab['email_from'] ||= ENV['GITLAB_EMAIL_FROM'] || "gitlab@#{Settings.gitlab.host}"
 Settings.gitlab['email_display_name'] ||= ENV['GITLAB_EMAIL_DISPLAY_NAME'] || 'GitLab'
@@ -196,7 +196,7 @@ Settings.gitlab['user_home']  ||= begin
 rescue ArgumentError # no user configured
   '/home/' + Settings.gitlab['user']
 end
-Settings.gitlab['time_zone']  ||= nil
+Settings.gitlab['time_zone'] ||= nil
 Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled'].nil?
 Settings.gitlab['signin_enabled'] ||= true if Settings.gitlab['signin_enabled'].nil?
 Settings.gitlab['restricted_visibility_levels'] = Settings.send(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], [])
@@ -211,7 +211,7 @@ Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.g
 Settings.gitlab.default_projects_features['wiki']           = true if Settings.gitlab.default_projects_features['wiki'].nil?
 Settings.gitlab.default_projects_features['snippets']       = false if Settings.gitlab.default_projects_features['snippets'].nil?
 Settings.gitlab.default_projects_features['builds']         = true if Settings.gitlab.default_projects_features['builds'].nil?
-Settings.gitlab.default_projects_features['visibility_level']    = Settings.send(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE)
+Settings.gitlab.default_projects_features['visibility_level'] = Settings.send(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE)
 Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') if Settings.gitlab['repository_downloads_path'].nil?
 Settings.gitlab['restricted_signup_domains'] ||= []
 Settings.gitlab['import_sources'] ||= ['github','bitbucket','gitlab','gitorious','google_code','fogbugz','git']
@@ -225,8 +225,8 @@ Settings['gitlab_ci'] ||= Settingslogic.new({})
 Settings.gitlab_ci['shared_runners_enabled'] = true if Settings.gitlab_ci['shared_runners_enabled'].nil?
 Settings.gitlab_ci['all_broken_builds']     = true if Settings.gitlab_ci['all_broken_builds'].nil?
 Settings.gitlab_ci['add_pusher']            = false if Settings.gitlab_ci['add_pusher'].nil?
-Settings.gitlab_ci['url']                   ||= Settings.send(:build_gitlab_ci_url)
 Settings.gitlab_ci['builds_path']           = File.expand_path(Settings.gitlab_ci['builds_path'] || "builds/", Rails.root)
+Settings.gitlab_ci['url']                 ||= Settings.send(:build_gitlab_ci_url)
 
 #
 # Reply by email
@@ -240,7 +240,7 @@ Settings.incoming_email['enabled'] = false if Settings.incoming_email['enabled']
 Settings['artifacts'] ||= Settingslogic.new({})
 Settings.artifacts['enabled']      = true if Settings.artifacts['enabled'].nil?
 Settings.artifacts['path']         = File.expand_path(Settings.artifacts['path'] || File.join(Settings.shared['path'], "artifacts"), Rails.root)
-Settings.artifacts['max_size']    ||= 100 # in megabytes
+Settings.artifacts['max_size']   ||= 100 # in megabytes
 
 #
 # Git LFS
@@ -298,7 +298,7 @@ Settings['backup'] ||= Settingslogic.new({})
 Settings.backup['keep_time']  ||= 0
 Settings.backup['pg_schema']    = nil
 Settings.backup['path']         = File.expand_path(Settings.backup['path'] || "tmp/backups/", Rails.root)
-Settings.backup['archive_permissions']          ||= 0600
+Settings.backup['archive_permissions'] ||= 0600
 Settings.backup['upload'] ||= Settingslogic.new({ 'remote_directory' => nil, 'connection' => nil })
 # Convert upload connection settings to use symbol keys, to make Fog happy
 if Settings.backup['upload']['connection']

config/initializers/carrierwave.rb

@@ -20,7 +20,7 @@ if File.exist?(aws_file)
     config.fog_public = false
 
     # optional, defaults to {}
-    config.fog_attributes = { 'Cache-Control'=>'max-age=315576000' }
+    config.fog_attributes = { 'Cache-Control' => 'max-age=315576000' }
 
     # optional time (in seconds) that authenticated urls will be valid.
     # when fog_public is false and provider is AWS or Google, defaults to 600

config/initializers/devise.rb

@@ -243,7 +243,7 @@ Devise.setup do |config|
     when Hash
       # Add procs for handling SLO
       if provider['name'] == 'cas3'
-        provider['args'][:on_single_sign_out]  = lambda do |request|
+        provider['args'][:on_single_sign_out] = lambda do |request|
           ticket = request.params[:session_index]
           raise "Service Ticket not found." unless Gitlab::OAuth::Session.valid?(:cas3, ticket)
           Gitlab::OAuth::Session.destroy(:cas3, ticket)

config/routes.rb

@@ -74,7 +74,7 @@ Rails.application.routes.draw do
   end
 
   # Health check
-  get 'health_check(/:checks)'  => 'health_check#index', as: :health_check
+  get 'health_check(/:checks)' => 'health_check#index', as: :health_check
 
   # Enable Grack support
   mount Grack::AuthSpawner, at: '/', constraints: lambda { |request| /[-\/\w\.]+\.git\//.match(request.path_info) }, via: [:get, :post, :put]
@@ -83,7 +83,7 @@ Rails.application.routes.draw do
   get 'help'                  => 'help#index'
   get 'help/:category/:file'  => 'help#show', as: :help_page, constraints: { category: /.*/, file: /[^\/\.]+/ }
   get 'help/shortcuts'
-  get 'help/ui'               => 'help#ui'
+  get 'help/ui' => 'help#ui'
 
   #
   # Global snippets

db/migrate/20160508215820_add_type_to_notes.rb

+class AddTypeToNotes < ActiveRecord::Migration
+  def change
+    add_column :notes, :type, :string
+  end
+end

db/migrate/20160508221410_set_type_on_legacy_diff_notes.rb

+class SetTypeOnLegacyDiffNotes < ActiveRecord::Migration
+  def change
+    execute "UPDATE notes SET type = 'LegacyDiffNote' WHERE line_code IS NOT NULL"
+  end
+end

db/schema.rb

@@ -632,10 +632,11 @@ ActiveRecord::Schema.define(version: 20160509201028) do
     t.string   "line_code"
     t.string   "commit_id"
     t.integer  "noteable_id"
-    t.boolean  "system",        default: false, null: false
+    t.boolean  "system",            default: false, null: false
     t.text     "st_diff"
     t.integer  "updated_by_id"
-    t.boolean  "is_award",      default: false, null: false
+    t.boolean  "is_award",          default: false, null: false
+    t.string   "type"
   end
 
   add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree

doc/administration/environment_variables.md

@@ -58,4 +58,4 @@ to the naming scheme `GITLAB_#{name in 1_settings.rb in upper case}`.
 
 It's possible to preconfigure the GitLab docker image by adding the environment
 variable `GITLAB_OMNIBUS_CONFIG` to the `docker run` command.
-For more information see the ['preconfigure-docker-container' section in the Omnibus documentation](http://doc.gitlab.com/omnibus/docker/#preconfigure-docker-container).
+For more information see the ['preconfigure-docker-container' section in the Omnibus documentation](http://docs.gitlab.com/omnibus/docker/#preconfigure-docker-container).

doc/administration/high_availability/load_balancer.md

@@ -60,4 +60,4 @@ Read more on high-availability configuration:
       configure custom domains with custom SSL, which would not be possible
       if SSL was terminated at the load balancer.
 
-[gitlab-pages]: http://doc.gitlab.com/ee/pages/administration.html
+[gitlab-pages]: http://docs.gitlab.com/ee/pages/administration.html

doc/administration/high_availability/nfs.md

@@ -113,4 +113,4 @@ Read more on high-availability configuration:
 1. [Configure the GitLab application servers](gitlab.md)
 1. [Configure the load balancers](load_balancer.md)
 
-[udp-log-shipping]: http://doc.gitlab.com/omnibus/settings/logs.html#udp-log-shipping-gitlab-enterprise-edition-only "UDP log shipping"
+[udp-log-shipping]: http://docs.gitlab.com/omnibus/settings/logs.html#udp-log-shipping-gitlab-enterprise-edition-only "UDP log shipping"

doc/api/services.md

@@ -491,7 +491,7 @@ Jira issue tracker
 
 Set JIRA service for a project.
 
-> Setting `project_url`, `issues_url` and `new_issue_url` will allow a user to easily navigate to the Jira issue tracker. See the [integration doc](http://doc.gitlab.com/ce/integration/external-issue-tracker.html) for details.  Support for referencing commits and automatic closing of Jira issues directly from GitLab is [available in GitLab EE.](http://doc.gitlab.com/ee/integration/jira.html)
+> Setting `project_url`, `issues_url` and `new_issue_url` will allow a user to easily navigate to the Jira issue tracker. See the [integration doc](http://docs.gitlab.com/ce/integration/external-issue-tracker.html) for details.  Support for referencing commits and automatic closing of Jira issues directly from GitLab is [available in GitLab EE.](http://docs.gitlab.com/ee/integration/jira.html)
 
 ```
 PUT /projects/:id/services/jira

doc/development/doc_styleguide.md

@@ -127,7 +127,7 @@ Inside the document:
     ```
   If the document you are editing resides in a place other than the GitLab CE/EE
   `doc/` directory, instead of the relative link, use the full path:
-  `http://doc.gitlab.com/ce/administration/restart_gitlab.html`.
+  `http://docs.gitlab.com/ce/administration/restart_gitlab.html`.
   Replace `reconfigure` with `restart` where appropriate.
 
 ## Installation guide
@@ -266,5 +266,5 @@ curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -d "restricted_signup_domai
 
 [cURL]: http://curl.haxx.se/ "cURL website"
 [single spaces]: http://www.slate.com/articles/technology/technology/2011/01/space_invaders.html
-[gfm]: http://doc.gitlab.com/ce/markdown/markdown.html#newlines "GitLab flavored markdown documentation"
+[gfm]: http://docs.gitlab.com/ce/markdown/markdown.html#newlines "GitLab flavored markdown documentation"
 [doc-restart]: ../administration/restart_gitlab.md "GitLab restart documentation"

doc/gitlab-basics/create-issue.md

@@ -24,4 +24,4 @@ You may assign the Issue to a user, add a milestone and add labels (they are all
 
 ![Submit new issue](basicsimages/submit_new_issue.png)
 
-Your Issue will now be added to the Issue Tracker and will be ready to be reviewed. You can comment on it and mention the people involved. You can also link Issues to the Merge Requests where the Issues are solved. To do this, you can use an [Issue closing pattern](http://doc.gitlab.com/ce/customization/issue_closing.html).
+Your Issue will now be added to the Issue Tracker and will be ready to be reviewed. You can comment on it and mention the people involved. You can also link Issues to the Merge Requests where the Issues are solved. To do this, you can use an [Issue closing pattern](http://docs.gitlab.com/ce/customization/issue_closing.html).

doc/gitlab-basics/create-project.md

@@ -14,7 +14,7 @@ Fill out the required information:
 
 1. Select a [visibility level](https://gitlab.com/help/public_access/public_access)
 
-1. You can also [import your existing projects](http://doc.gitlab.com/ce/workflow/importing/README.html)
+1. You can also [import your existing projects](http://docs.gitlab.com/ce/workflow/importing/README.html)
 
 1. Click on "create project"
 

doc/hooks/custom_hooks.md

@@ -2,7 +2,7 @@
 
 **Note: Custom git hooks must be configured on the filesystem of the GitLab
 server. Only GitLab server administrators will be able to complete these tasks.
-Please explore [webhooks](../web_hooks/web_hooks.md) as an option if you do not have filesystem access. For a user configurable Git Hooks interface, please see [GitLab Enterprise Edition Git Hooks](http://doc.gitlab.com/ee/git_hooks/git_hooks.html).**
+Please explore [webhooks](../web_hooks/web_hooks.md) as an option if you do not have filesystem access. For a user configurable Git Hooks interface, please see [GitLab Enterprise Edition Git Hooks](http://docs.gitlab.com/ee/git_hooks/git_hooks.html).**
 
 Git natively supports hooks that are executed on different actions.
 Examples of server-side git hooks include pre-receive, post-receive, and update.

doc/install/installation.md

@@ -6,7 +6,7 @@ Since an installation from source is a lot of work and error prone we strongly r
 
 One reason the Omnibus package is more reliable is its use of Runit to restart any of the GitLab processes in case one crashes.
 On heavily used GitLab instances the memory usage of the Sidekiq background worker will grow over time.
-Omnibus packages solve this by [letting the Sidekiq terminate gracefully](http://doc.gitlab.com/ce/operations/sidekiq_memory_killer.html) if it uses too much memory.
+Omnibus packages solve this by [letting the Sidekiq terminate gracefully](http://docs.gitlab.com/ce/operations/sidekiq_memory_killer.html) if it uses too much memory.
 After this termination Runit will detect Sidekiq is not running and will start it.
 Since installations from source don't have Runit, Sidekiq can't be terminated and its memory usage will grow over time.
 

doc/install/relative_url.md

@@ -132,5 +132,5 @@ To disable the relative URL:
 1.  Follow the same as above starting from 2. and set up the
     GitLab URL to one that doesn't contain a relative path.
 
-[omnibus-rel]: http://doc.gitlab.com/omnibus/settings/configuration.html#configuring-a-relative-url-for-gitlab "How to setup relative URL in Omnibus GitLab"
+[omnibus-rel]: http://docs.gitlab.com/omnibus/settings/configuration.html#configuring-a-relative-url-for-gitlab "How to setup relative URL in Omnibus GitLab"
 [restart gitlab]: ../administration/restart_gitlab.md#installations-from-source "How to restart GitLab"

doc/integration/README.md

@@ -19,7 +19,7 @@ See the documentation below for details on how to configure these services.
 
 GitLab Enterprise Edition contains [advanced Jenkins support][jenkins].
 
-[jenkins]: http://doc.gitlab.com/ee/integration/jenkins.html
+[jenkins]: http://docs.gitlab.com/ee/integration/jenkins.html
 
 
 ## Project services

doc/intro/README.md

@@ -39,4 +39,4 @@ Install and update your GitLab installation.
 
 - [Install GitLab](https://about.gitlab.com/installation/)
 - [Update GitLab](https://about.gitlab.com/update/)
-- [Explore Omnibus GitLab configuration options](http://doc.gitlab.com/omnibus/settings/configuration.html)
+- [Explore Omnibus GitLab configuration options](http://docs.gitlab.com/omnibus/settings/configuration.html)

doc/logs/logs.md

 ## Log system
 GitLab has advanced log system so everything is logging and you can analize your instance using various system log files.
-In addition to system log files, GitLab Enterprise Edition comes with Audit Events. Find more about them [in Audit Events documentation](http://doc.gitlab.com/ee/administration/audit_events.html)
+In addition to system log files, GitLab Enterprise Edition comes with Audit Events. Find more about them [in Audit Events documentation](http://docs.gitlab.com/ee/administration/audit_events.html)
 
 System log files are typically plain text in a standard log file format. This guide talks about how to read and use these system log files.
 

doc/raketasks/README.md

@@ -8,4 +8,4 @@
 - [User management](user_management.md)
 - [Webhooks](web_hooks.md)
 - [Import](import.md) of git repositories in bulk
-- [Rebuild authorized_keys file](http://doc.gitlab.com/ce/raketasks/maintenance.html#rebuild-authorized_keys-file) task for administrators
+- [Rebuild authorized_keys file](http://docs.gitlab.com/ce/raketasks/maintenance.html#rebuild-authorized_keys-file) task for administrators

doc/update/README.md

@@ -29,7 +29,7 @@ Based on your installation, choose a section below that fits your needs.
 
 ## Omnibus Packages
 
-- The [Omnibus update guide](http://doc.gitlab.com/omnibus/update/README.html)
+- The [Omnibus update guide](http://docs.gitlab.com/omnibus/update/README.html)
   contains the steps needed to update an Omnibus GitLab package.
 
 ## Installation from source
@@ -86,10 +86,10 @@ possible.
   information about configuring GitLab to work with a MySQL database.
 - [Restoring from backup after a failed upgrade](restore_after_failure.md)
 
-[omnidocker]: http://doc.gitlab.com/omnibus/docker/README.html
+[omnidocker]: http://docs.gitlab.com/omnibus/docker/README.html
 [source-ee]: https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc/update
 [source-ce]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/update
 [ee-ce]: ../downgrade_ee_to_ce/README.md
 [ce]: https://about.gitlab.com/features/#community
 [ee]: https://about.gitlab.com/features/#enterprise
-[omni-ce-ee]: http://doc.gitlab.com/omnibus/update/README.html#from-community-edition-to-enterprise-edition
+[omni-ce-ee]: http://docs.gitlab.com/omnibus/update/README.html#from-community-edition-to-enterprise-edition

doc/update/patch_versions.md

@@ -57,10 +57,10 @@ sudo -u git -H make
 cd /home/git/gitlab
 
 # PostgreSQL
-sudo -u git -H bundle install --without development test mysql --with postgres --deployment
+sudo -u git -H bundle install --without development test mysql --deployment
 
 # MySQL
-sudo -u git -H bundle install --without development test postgres --with mysql --deployment
+sudo -u git -H bundle install --without development test postgres --deployment
 
 # Optional: clean up old gems
 sudo -u git -H bundle clean

doc/workflow/gitlab_flow.md

@@ -131,7 +131,7 @@ When you feel comfortable with it to be merged you assign it to the person that
 There is room for more feedback and after the assigned person feels comfortable with the result the branch is merged.
 If the assigned person does not feel comfortable they can close the merge request without merging.
 
-In GitLab it is common to protect the long-lived branches (e.g. the master branch) so that normal developers [can't modify these protected branches](http://doc.gitlab.com/ce/permissions/permissions.html).
+In GitLab it is common to protect the long-lived branches (e.g. the master branch) so that normal developers [can't modify these protected branches](http://docs.gitlab.com/ce/permissions/permissions.html).
 So if you want to merge it into a protected branch you assign it to someone with master authorizations.
 
 ## Issues with GitLab flow
@@ -187,7 +187,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.
-In GitLab EE and .com you can also [rebase before merge](http://doc.gitlab.com/ee/workflow/rebase_before_merge.html) from the web interface.
+In GitLab EE and .com you can also [rebase before merge](http://docs.gitlab.com/ee/workflow/rebase_before_merge.html) from the web interface.
 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.

doc/workflow/groups.md

@@ -54,7 +54,7 @@ If necessary, you can increase the access level of an individual user for a spec
 ## Managing group memberships via LDAP
 
 In GitLab Enterprise Edition it is possible to manage GitLab group memberships using LDAP groups.
-See [the GitLab Enterprise Edition documentation](http://doc.gitlab.com/ee/integration/ldap.html) for more information.
+See [the GitLab Enterprise Edition documentation](http://docs.gitlab.com/ee/integration/ldap.html) for more information.
 
 ## Allowing only admins to create groups
 

doc/workflow/importing/import_projects_from_github.md

@@ -44,5 +44,5 @@ case the namespace is taken, the project will be imported on the user's
 namespace.
 
 [gh-import]: ../../integration/github.md "GitHub integration"
-[ee-gh]: http://doc.gitlab.com/ee/integration/github.html "GitHub integration for GitLab EE"
+[ee-gh]: http://docs.gitlab.com/ee/integration/github.html "GitHub integration for GitLab EE"
 [new-project]: ../../gitlab-basics/create-project.md "How to create a new project in GitLab"

doc/workflow/importing/import_projects_from_gitlab_com.md

@@ -2,7 +2,7 @@
 
 You can import your existing GitLab.com projects to your GitLab instance. But keep in mind that it is possible only if
 GitLab support is enabled on your GitLab instance. 
-You can read more about GitLab support [here](http://doc.gitlab.com/ce/integration/gitlab.html)
+You can read more about GitLab support [here](http://docs.gitlab.com/ce/integration/gitlab.html)
 To get to the importer page you need to go to "New project" page.
 
 ![New project page](gitlab_importer/new_project_page.png)

doc/workflow/lfs/manage_large_binaries_with_git_lfs.md

@@ -4,7 +4,7 @@ Managing large files such as audio, video and graphics files has always been one
 of the shortcomings of Git. The general recommendation is to not have Git repositories
 larger than 1GB to preserve performance.
 
-GitLab already supports [managing large files with git annex](http://doc.gitlab.com/ee/workflow/git_annex.html)
+GitLab already supports [managing large files with git annex](http://docs.gitlab.com/ee/workflow/git_annex.html)
 (EE only), however in certain environments it is not always convenient to use
 different commands to differentiate between the large files and regular ones.
 

features/steps/dashboard/issues.rb

@@ -74,7 +74,7 @@ class Spinach::Features::DashboardIssues < Spinach::FeatureSteps
 
   def project
     @project ||= begin
-                   project =create :project
+                   project = create :project
                    project.team << [current_user, :master]
                    project
                  end

features/steps/dashboard/merge_requests.rb

@@ -100,7 +100,7 @@ class Spinach::Features::DashboardMergeRequests < Spinach::FeatureSteps
 
   def project
     @project ||= begin
-                   project =create :project
+                   project = create :project
                    project.team << [current_user, :master]
                    project
                  end

features/steps/profile/profile.rb

@@ -166,7 +166,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps
   end
 
   step 'I have group with projects' do
-    @group   = create(:group)
+    @group = create(:group)
     @group.add_owner(current_user)
     @project = create(:project, namespace: @group)
     @event   = create(:closed_issue_event, project: @project)

features/steps/project/team_management.rb

@@ -126,7 +126,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
 
   step 'I share project with group "OpenSource"' do
     project = Project.find_by(name: 'Shop')
-    os_group   = create(:group, name: 'OpenSource')
+    os_group = create(:group, name: 'OpenSource')
     create(:project, group: os_group)
     @os_user1 = create(:user)
     @os_user2 = create(:user)

features/steps/shared/diff_note.rb

@@ -23,7 +23,7 @@ module SharedDiffNote
     page.within(diff_file_selector) do
       click_diff_line(sample_commit.line_code)
 
-      page.within("form[id$='#{sample_commit.line_code}']") do
+      page.within("form[id$='#{sample_commit.line_code}-true']") do
         fill_in "note[note]", with: "Typo, please fix"
         find(".js-comment-button").trigger("click")
         sleep 0.05
@@ -33,7 +33,7 @@ module SharedDiffNote
 
   step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do
     click_parallel_diff_line(sample_commit.line_code, 'old')
-    page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}']") do
+    page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do
       fill_in "note[note]", with: "Old comment"
       find(".js-comment-button").trigger("click")
     end
@@ -41,7 +41,7 @@ module SharedDiffNote
 
   step 'I leave a diff comment in a parallel view on the right side like "New comment"' do
     click_parallel_diff_line(sample_commit.line_code, 'new')
-    page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}']") do
+    page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do
       fill_in "note[note]", with: "New comment"
       find(".js-comment-button").trigger("click")
     end
@@ -51,7 +51,7 @@ module SharedDiffNote
     page.within(diff_file_selector) do
       click_diff_line(sample_commit.line_code)
 
-      page.within("form[id$='#{sample_commit.line_code}']") do
+      page.within("form[id$='#{sample_commit.line_code}-true']") do
         fill_in "note[note]", with: "Should fix it :smile:"
         find('.js-md-preview-button').click
       end
@@ -62,7 +62,7 @@ module SharedDiffNote
     page.within(diff_file_selector) do
       click_diff_line(sample_commit.del_line_code)
 
-      page.within("form[id$='#{sample_commit.del_line_code}']") do
+      page.within("form[id$='#{sample_commit.del_line_code}-true']") do
         fill_in "note[note]", with: "DRY this up"
         find('.js-md-preview-button').click
       end
@@ -91,7 +91,7 @@ module SharedDiffNote
     page.within(diff_file_selector) do
       click_diff_line(sample_commit.line_code)
 
-      page.within("form[id$='#{sample_commit.line_code}']") do
+      page.within("form[id$='#{sample_commit.line_code}-true']") do
         fill_in 'note[note]', with: ':smile:'
         click_button('Comment')
       end

lib/api/commits.rb

@@ -107,6 +107,8 @@ module API
 
             break if opts[:line_code]
           end
+
+          opts[:type] = LegacyDiffNote.name if opts[:line_code]
         end
 
         note = ::Notes::CreateService.new(user_project, current_user, opts).execute

lib/api/entities.rb

@@ -227,9 +227,9 @@ module API
 
     class CommitNote < Grape::Entity
       expose :note
-      expose(:path) { |note| note.diff_file_name }
-      expose(:line) { |note| note.diff_new_line }
-      expose(:line_type) { |note| note.diff_line_type }
+      expose(:path) { |note| note.diff_file_path if note.legacy_diff_note? }
+      expose(:line) { |note| note.diff_new_line if note.legacy_diff_note? }
+      expose(:line_type) { |note| note.diff_line_type if note.legacy_diff_note? }
       expose :author, using: Entities::UserBasic
       expose :created_at
     end

lib/api/helpers.rb

@@ -2,7 +2,7 @@ module API
   module Helpers
     PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
     PRIVATE_TOKEN_PARAM = :private_token
-    SUDO_HEADER ="HTTP_SUDO"
+    SUDO_HEADER = "HTTP_SUDO"
     SUDO_PARAM = :sudo
 
     def parse_boolean(value)

lib/gitlab/email/reply_parser.rb

@@ -65,7 +65,7 @@ module Gitlab
                    (l =~ /On \w+ \d+,? \d+,?.*wrote:/)
 
           # Headers on subsequent lines
-          break if (0..2).all? { |off| lines[idx+off] =~ REPLYING_HEADER_REGEX }
+          break if (0..2).all? { |off| lines[idx + off] =~ REPLYING_HEADER_REGEX }
           # Headers on the same line
           break if REPLYING_HEADER_LABELS.count { |label| l.include?(label) } >= 3
 

lib/gitlab/redis.rb

@@ -25,7 +25,7 @@ module Gitlab
       end
       @pool.with { |redis| yield redis }
     end
-    
+
     def self.redis_store_options
       url = new.url
       redis_config_hash = ::Redis::Store::Factory.extract_host_options_from_uri(url)
@@ -40,10 +40,10 @@ module Gitlab
     def initialize(rails_env=nil)
       rails_env ||= Rails.env
       config_file = File.expand_path('../../../config/resque.yml', __FILE__)
-  
+
       @url = "redis://localhost:6379"
       if File.exist?(config_file)
-        @url =YAML.load_file(config_file)[rails_env]
+        @url = YAML.load_file(config_file)[rails_env]
       end
     end
   end

spec/factories/notes.rb

@@ -9,10 +9,10 @@ FactoryGirl.define do
     author
 
     factory :note_on_commit,             traits: [:on_commit]
-    factory :note_on_commit_diff,        traits: [:on_commit, :on_diff]
+    factory :note_on_commit_diff,        traits: [:on_commit, :on_diff], class: LegacyDiffNote
     factory :note_on_issue,              traits: [:on_issue], aliases: [:votable_note]
     factory :note_on_merge_request,      traits: [:on_merge_request]
-    factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff]
+    factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff], class: LegacyDiffNote
     factory :note_on_project_snippet,    traits: [:on_project_snippet]
     factory :system_note,                traits: [:system]
     factory :downvote_note,              traits: [:award, :downvote]

spec/features/notes_on_merge_requests_spec.rb

@@ -192,7 +192,7 @@ describe 'Comments', feature: true do
         end
 
         it 'should be removed when canceled' do
-          page.within(".diff-file form[id$='#{line_code}']") do
+          page.within(".diff-file form[id$='#{line_code}-true']") do
             find('.js-close-discussion-note-form').trigger('click')
           end
 

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -443,12 +443,12 @@ module Ci
       context 'when job variables are defined' do
         context 'when syntax is correct' do
           it 'returns job variables' do
-            variables =  {
+            variables = {
               KEY1: 'value1',
               SOME_KEY_2: 'value2'
             }
 
-            config =  YAML.dump(
+            config = YAML.dump(
               { before_script: ['pwd'],
                 rspec: {
                   variables: variables,

spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb

@@ -10,8 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
       'path/dir_1/subdir/subfile' => { size: 10 },
       'path/second_dir' => {},
       'path/second_dir/dir_3/file_2' => { size: 10 },
-      'path/second_dir/dir_3/file_3'=> { size: 10 },
-      'another_directory/'=> {},
+      'path/second_dir/dir_3/file_3' => { size: 10 },
+      'another_directory/' => {},
       'another_file' => {},
       '/file/with/absolute_path' => {} }
   end

spec/lib/gitlab/lfs/lfs_router_spec.rb

@@ -26,8 +26,8 @@ describe Gitlab::Lfs::Router, lib: true do
 
   let(:sample_oid) { "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a17f80" }
   let(:sample_size) { 499013 }
-  let(:respond_with_deprecated) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
-  let(:respond_with_disabled) {[ 501, { "Content-Type"=>"application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
+  let(:respond_with_deprecated) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
+  let(:respond_with_disabled) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]}
 
   describe 'when lfs is disabled' do
     before do

spec/models/commit_spec.rb

@@ -56,7 +56,7 @@ describe Commit, models: true do
     end
 
     it "does not truncates a message with a newline after 80 but less 100 characters" do
-      message =<<eos
+      message = <<eos
 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit.
 Vivamus egestas lacinia lacus, sed rutrum mauris.
 eos

spec/models/hooks/service_hook_spec.rb

@@ -34,14 +34,14 @@ describe ServiceHook, models: true do
     it "POSTs to the webhook URL" do
       @service_hook.execute(@data)
       expect(WebMock).to have_requested(:post, @service_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' }
       ).once
     end
 
     it "POSTs the data as JSON" do
       @service_hook.execute(@data)
       expect(WebMock).to have_requested(:post, @service_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' }
       ).once
     end
 

spec/models/hooks/system_hook_spec.rb

@@ -33,7 +33,7 @@ describe SystemHook, models: true do
       Projects::CreateService.new(user, name: 'empty').execute
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /project_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -42,7 +42,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /project_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -51,7 +51,7 @@ describe SystemHook, models: true do
       
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -60,7 +60,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -69,7 +69,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_add_to_team/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -79,7 +79,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_remove_from_team/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -88,7 +88,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /group_create/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -97,7 +97,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /group_destroy/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -106,7 +106,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_add_to_group/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
 
@@ -116,7 +116,7 @@ describe SystemHook, models: true do
 
       expect(WebMock).to have_requested(:post, system_hook.url).with(
         body: /user_remove_from_group/,
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' }
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' }
       ).once
     end
   end

spec/models/legacy_diff_note_spec.rb

+require 'spec_helper'
+
+describe LegacyDiffNote, models: true do
+  describe "Commit diff line notes" do
+    let!(:note) { create(:note_on_commit_diff, note: "+1 from me") }
+    let!(:commit) { note.noteable }
+
+    it "should save a valid note" do
+      expect(note.commit_id).to eq(commit.id)
+      expect(note.noteable.id).to eq(commit.id)
+    end
+
+    it "should be recognized by #legacy_diff_note?" do
+      expect(note).to be_legacy_diff_note
+    end
+  end
+
+  describe '#active?' do
+    it 'is always true when the note has no associated diff' do
+      note = build(:note_on_merge_request_diff)
+
+      expect(note).to receive(:diff).and_return(nil)
+
+      expect(note).to be_active
+    end
+
+    it 'is never true when the note has no noteable associated' do
+      note = build(:note_on_merge_request_diff)
+
+      expect(note).to receive(:diff).and_return(double)
+      expect(note).to receive(:noteable).and_return(nil)
+
+      expect(note).not_to be_active
+    end
+
+    it 'returns the memoized value if defined' do
+      note = build(:note_on_merge_request_diff)
+
+      note.instance_variable_set(:@active, 'foo')
+      expect(note).not_to receive(:find_noteable_diff)
+
+      expect(note.active?).to eq 'foo'
+    end
+
+    context 'for a merge request noteable' do
+      it 'is false when noteable has no matching diff' do
+        merge = build_stubbed(:merge_request, :simple)
+        note = build(:note_on_merge_request_diff, noteable: merge)
+
+        allow(note).to receive(:diff).and_return(double)
+        expect(note).to receive(:find_noteable_diff).and_return(nil)
+
+        expect(note).not_to be_active
+      end
+
+      it 'is true when noteable has a matching diff' do
+        merge = create(:merge_request, :simple)
+
+        # Generate a real line_code value so we know it will match. We use a
+        # random line from a random diff just for funsies.
+        diff = merge.diffs.to_a.sample
+        line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
+        code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
+
+        # We're persisting in order to trigger the set_diff callback
+        note = create(:note_on_merge_request_diff, noteable: merge, line_code: code)
+
+        # Make sure we don't get a false positive from a guard clause
+        expect(note).to receive(:find_noteable_diff).and_call_original
+        expect(note).to be_active
+      end
+    end
+  end
+end

spec/models/note_spec.rb

@@ -34,24 +34,6 @@ describe Note, models: true do
     end
   end
 
-  describe "Commit diff line notes" do
-    let!(:note) { create(:note_on_commit_diff, note: "+1 from me") }
-    let!(:commit) { note.noteable }
-
-    it "should save a valid note" do
-      expect(note.commit_id).to eq(commit.id)
-      expect(note.noteable.id).to eq(commit.id)
-    end
-
-    it "should be recognized by #for_diff_line?" do
-      expect(note).to be_for_diff_line
-    end
-
-    it "should be recognized by #for_commit_diff_line?" do
-      expect(note).to be_for_commit_diff_line
-    end
-  end
-
   describe 'authorization' do
     before do
       @p1 = create(:project)
@@ -148,66 +130,6 @@ describe Note, models: true do
     end
   end
 
-  describe '#active?' do
-    it 'is always true when the note has no associated diff' do
-      note = build(:note)
-
-      expect(note).to receive(:diff).and_return(nil)
-
-      expect(note).to be_active
-    end
-
-    it 'is never true when the note has no noteable associated' do
-      note = build(:note)
-
-      expect(note).to receive(:diff).and_return(double)
-      expect(note).to receive(:noteable).and_return(nil)
-
-      expect(note).not_to be_active
-    end
-
-    it 'returns the memoized value if defined' do
-      note = build(:note)
-
-      expect(note).to receive(:diff).and_return(double)
-      expect(note).to receive(:noteable).and_return(double)
-
-      note.instance_variable_set(:@active, 'foo')
-      expect(note).not_to receive(:find_noteable_diff)
-
-      expect(note.active?).to eq 'foo'
-    end
-
-    context 'for a merge request noteable' do
-      it 'is false when noteable has no matching diff' do
-        merge = build_stubbed(:merge_request, :simple)
-        note = build(:note, noteable: merge)
-
-        allow(note).to receive(:diff).and_return(double)
-        expect(note).to receive(:find_noteable_diff).and_return(nil)
-
-        expect(note).not_to be_active
-      end
-
-      it 'is true when noteable has a matching diff' do
-        merge = create(:merge_request, :simple)
-
-        # Generate a real line_code value so we know it will match. We use a
-        # random line from a random diff just for funsies.
-        diff = merge.diffs.to_a.sample
-        line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
-        code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
-
-        # We're persisting in order to trigger the set_diff callback
-        note = create(:note, noteable: merge, line_code: code)
-
-        # Make sure we don't get a false positive from a guard clause
-        expect(note).to receive(:find_noteable_diff).and_call_original
-        expect(note).to be_active
-      end
-    end
-  end
-
   describe "editable?" do
     it "returns true" do
       note = build(:note)
@@ -258,7 +180,7 @@ describe Note, models: true do
     end
 
     it "is not an award emoji when comment is on a diff" do
-      note = create(:note, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2")
+      note = create(:note_on_merge_request_diff, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2")
       note = note.reload
 
       expect(note.note).to eq(":blowfish:")