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

CHANGELOG

@@ -1,6 +1,8 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.10.0 (unreleased)
+  - Update poltergeist to version 1.6.0 to support PhantomJS 2.0 (Zeger-Jan van de Weg)
+  - Fix cross references when usernames, milestones, or project names contain underscores (Stan Hu)
   - enable line wrapping per default and remove the checkbox to toggle it (Hannes Rosenögger)
   - extend the commit calendar to show the actual commits made on a date (Hannes Rosenögger)
   - Add a service to support external wikis (Hannes Rosenögger)

Gemfile

@@ -252,7 +252,7 @@ group :development, :test do
   gem 'rb-inotify', require: linux_only('rb-inotify')
 
   # PhantomJS driver for Capybara
-  gem 'poltergeist', '~> 1.5.1'
+  gem 'poltergeist', '~> 1.6.0'
 
   gem 'jasmine', '2.0.2'
 

Gemfile.lock

@@ -392,7 +392,7 @@ GEM
       ast (>= 1.1, < 3.0)
     pg (0.15.1)
     phantomjs (1.9.2.0)
-    poltergeist (1.5.1)
+    poltergeist (1.6.0)
       capybara (~> 2.1)
       cliver (~> 0.3.1)
       multi_json (~> 1.0)
@@ -650,7 +650,9 @@ GEM
     webmock (1.16.0)
       addressable (>= 2.2.7)
       crack (>= 0.3.2)
-    websocket-driver (0.3.3)
+    websocket-driver (0.5.3)
+      websocket-extensions (>= 0.1.0)
+    websocket-extensions (0.1.2)
     wikicloth (0.8.1)
       builder
       expression_parser
@@ -744,7 +746,7 @@ DEPENDENCIES
   omniauth-twitter
   org-ruby (= 0.9.12)
   pg
-  poltergeist (~> 1.5.1)
+  poltergeist (~> 1.6.0)
   pry-rails
   quiet_assets (~> 1.0.1)
   rack-attack

app/models/note.rb

@@ -59,7 +59,7 @@ class Note < ActiveRecord::Base
 
   class << self
     def create_status_change_note(noteable, project, author, status, source)
-      body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_"
+      body = "Status changed to #{status}#{' by ' + source.gfm_reference if source}"
 
       create(
         noteable: noteable,
@@ -95,9 +95,9 @@ class Note < ActiveRecord::Base
 
     def create_milestone_change_note(noteable, project, author, milestone)
       body = if milestone.nil?
-               '_Milestone removed_'
+               'Milestone removed'
              else
-               "_Milestone changed to #{milestone.title}_"
+               "Milestone changed to #{milestone.title}"
              end
 
       create(
@@ -110,7 +110,7 @@ class Note < ActiveRecord::Base
     end
 
     def create_assignee_change_note(noteable, project, author, assignee)
-      body = assignee.nil? ? '_Assignee removed_' : "_Reassigned to @#{assignee.username}_"
+      body = assignee.nil? ? 'Assignee removed' : "Reassigned to @#{assignee.username}"
 
       create({
         noteable: noteable,
@@ -140,7 +140,7 @@ class Note < ActiveRecord::Base
       end
 
       message << ' ' << 'label'.pluralize(labels_count)
-      body = "_#{message.capitalize}_"
+      body = "#{message.capitalize}"
 
       create(
         noteable: noteable,
@@ -170,14 +170,14 @@ class Note < ActiveRecord::Base
 
         commits_text = ActionController::Base.helpers.pluralize(existing_commits.length, 'commit')
 
-        branch = 
+        branch =
           if merge_request.for_fork?
             "#{merge_request.target_project_namespace}:#{merge_request.target_branch}"
           else
             merge_request.target_branch
           end
 
-        message = "* #{commit_ids} - _#{commits_text} from branch `#{branch}`_"
+        message = "* #{commit_ids} - #{commits_text} from branch `#{branch}`"
         body << message
         body << "\n"
       end
@@ -240,7 +240,7 @@ class Note < ActiveRecord::Base
                 where(noteable_id: noteable.id)
               end
 
-      notes.where('note like ?', cross_reference_note_content(gfm_reference)).
+      notes.where('note like ?', cross_reference_note_pattern(gfm_reference)).
         system.any?
     end
 
@@ -249,13 +249,18 @@ class Note < ActiveRecord::Base
     end
 
     def cross_reference_note_prefix
-      '_mentioned in '
+      'mentioned in '
     end
 
     private
 
     def cross_reference_note_content(gfm_reference)
-      cross_reference_note_prefix + "#{gfm_reference}_"
+      cross_reference_note_prefix + "#{gfm_reference}"
+    end
+
+    def cross_reference_note_pattern(gfm_reference)
+      # Older cross reference notes contained underscores for emphasis
+      "%" + cross_reference_note_content(gfm_reference) + "%"
     end
 
     # Prepend the mentioner's namespaced project path to the GFM reference for

app/services/notification_service.rb

@@ -120,7 +120,7 @@ class NotificationService
     return true unless note.noteable_type.present?
 
     # ignore gitlab service messages
-    return true if note.note.start_with?('_Status changed to closed_')
+    return true if note.note.start_with?('Status changed to closed')
     return true if note.cross_reference? && note.system == true
 
     opts = { noteable_type: note.noteable_type, project_id: note.project_id }

doc/api/merge_requests.md

@@ -375,7 +375,7 @@ Parameters:
     }
   },
   {
-    "note": "_Status changed to closed_",
+    "note": "Status changed to closed",
     "author": {
       "id": 11,
       "username": "admin",

doc/api/notes.md

@@ -21,7 +21,7 @@ Parameters:
 [
   {
     "id": 302,
-    "body": "_Status changed to closed_",
+    "body": "Status changed to closed",
     "attachment": null,
     "author": {
       "id": 1,

lib/tasks/gitlab/cleanup.rake

@@ -90,13 +90,14 @@ namespace :gitlab do
       warn_user_is_not_gitlab
       block_flag = ENV['BLOCK']
 
-      User.ldap.each do |ldap_user|
-        print "#{ldap_user.name} (#{ldap_user.extern_uid}) ..."
-        if Gitlab::LDAP::Access.allowed?(ldap_user)
+      User.find_each do |user|
+        next unless user.ldap_user?
+        print "#{user.name} (#{user.ldap_identity.extern_uid}) ..."
+        if Gitlab::LDAP::Access.allowed?(user)
           puts " [OK]".green
         else
           if block_flag
-            ldap_user.block! unless ldap_user.blocked?
+            user.block! unless user.blocked?
             puts " [BLOCKED]".red
           else
             puts " [NOT IN LDAP]".yellow

spec/lib/gitlab/reference_extractor_spec.rb

@@ -120,4 +120,18 @@ describe Gitlab::ReferenceExtractor do
       expect(extracted[0][1].message).to eq(commit.message)
     end
   end
+
+  context 'with a project with an underscore' do
+    let(:project) { create(:project, path: 'test_project') }
+    let(:issue) { create(:issue, project: project) }
+
+    it 'handles project issue references' do
+      subject.analyze("this refers issue #{project.path_with_namespace}##{issue.iid}",
+          project)
+      extracted = subject.issues_for(project)
+      expect(extracted.size).to eq(1)
+      expect(extracted).to eq([issue])
+    end
+
+  end
 end

spec/models/note_spec.rb

@@ -182,14 +182,14 @@ describe Note do
 
     describe '#note' do
       subject { super().note }
-      it { is_expected.to match(/Status changed to #{status}/) }
+      it { is_expected.to eq("Status changed to #{status}") }
     end
 
     it 'appends a back-reference if a closing mentionable is supplied' do
       commit = double('commit', gfm_reference: 'commit 123456')
       n = Note.create_status_change_note(thing, project, author, status, commit)
 
-      expect(n.note).to match(/Status changed to #{status} by commit 123456/)
+      expect(n.note).to eq("Status changed to #{status} by commit 123456")
     end
   end
 
@@ -197,7 +197,7 @@ describe Note do
     let(:project) { create(:project) }
     let(:thing) { create(:issue, project: project) }
     let(:author) { create(:user) }
-    let(:assignee) { create(:user) }
+    let(:assignee) { create(:user, username: "assigned_user") }
 
     subject { Note.create_assignee_change_note(thing, project, author, assignee) }
 
@@ -227,7 +227,7 @@ describe Note do
 
     describe '#note' do
       subject { super().note }
-      it { is_expected.to match(/Reassigned to @#{assignee.username}/) }
+      it { is_expected.to eq('Reassigned to @assigned_user') }
     end
 
     context 'assignee is removed' do
@@ -235,11 +235,95 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to match(/Assignee removed/) }
+        it { is_expected.to eq('Assignee removed') }
       end
     end
   end
 
+  describe '#create_labels_change_note' do
+    let(:project) { create(:project) }
+    let(:thing) { create(:issue, project: project) }
+    let(:author) { create(:user) }
+    let(:label1) { create(:label) }
+    let(:label2) { create(:label) }
+    let(:added_labels) { [label1, label2] }
+    let(:removed_labels) { [] }
+
+    subject { Note.create_labels_change_note(thing, project, author, added_labels, removed_labels) }
+
+    context 'creates and saves a Note' do
+      it { is_expected.to be_a Note }
+
+      describe '#id' do
+        subject { super().id }
+        it { is_expected.not_to be_nil }
+      end
+    end
+
+    describe '#noteable' do
+      subject { super().noteable }
+      it { is_expected.to eq(thing) }
+    end
+
+    describe '#project' do
+      subject { super().project }
+      it { is_expected.to eq(thing.project) }
+    end
+
+    describe '#author' do
+      subject { super().author }
+      it { is_expected.to eq(author) }
+    end
+
+    describe '#note' do
+      subject { super().note }
+      it { is_expected.to eq("Added ~#{label1.id} ~#{label2.id} labels") }
+    end
+
+    context 'label is removed' do
+      let(:added_labels) { [label1]  }
+      let(:removed_labels) { [label2] }
+
+      describe '#note' do
+        subject { super().note }
+        it { is_expected.to eq("Added ~#{label1.id} and removed ~#{label2.id} labels") }
+      end
+    end
+  end
+
+  describe '#create_milestone_change_note' do
+    let(:project) { create(:project) }
+    let(:thing) { create(:issue, project: project) }
+    let(:milestone) { create(:milestone, project: project, title: "first_milestone") }
+    let(:author) { create(:user) }
+
+    subject { Note.create_milestone_change_note(thing, project, author, milestone) }
+
+    context 'creates and saves a Note' do
+      it { is_expected.to be_a Note }
+
+      describe '#id' do
+        subject { super().id }
+        it { is_expected.not_to be_nil }
+      end
+    end
+
+    describe '#project' do
+      subject { super().project }
+      it { is_expected.to eq(thing.project) }
+    end
+
+    describe '#author' do
+      subject { super().author }
+      it { is_expected.to eq(author) }
+    end
+
+    describe '#note' do
+      subject { super().note }
+      it { is_expected.to eq("Milestone changed to first_milestone") }
+    end
+  end
+
   describe '#create_cross_reference_note' do
     let(:project)    { create(:project) }
     let(:author)     { create(:user) }
@@ -272,7 +356,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in merge request !#{mergereq.iid}_") }
+        it { is_expected.to eq("mentioned in merge request !#{mergereq.iid}") }
       end
     end
 
@@ -288,7 +372,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in commit #{commit.sha}_") }
+        it { is_expected.to eq("mentioned in commit #{commit.sha}") }
       end
     end
 
@@ -309,7 +393,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in issue ##{issue.iid}_") }
+        it { is_expected.to eq("mentioned in issue ##{issue.iid}") }
       end
     end
 
@@ -330,7 +414,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in merge request !#{mergereq.iid}_") }
+        it { is_expected.to eq("mentioned in merge request !#{mergereq.iid}") }
       end
     end
 
@@ -362,7 +446,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in issue ##{issue.iid}_") }
+        it { is_expected.to eq("mentioned in issue ##{issue.iid}") }
       end
     end
 
@@ -389,7 +473,7 @@ describe Note do
 
       describe '#note' do
         subject { super().note }
-        it { is_expected.to eq("_mentioned in commit #{parent_commit.id}_") }
+        it { is_expected.to eq("mentioned in commit #{parent_commit.id}") }
       end
     end
   end
@@ -421,6 +505,41 @@ describe Note do
       it { expect(Note.cross_reference_exists?(commit0, commit1)).to be_truthy }
       it { expect(Note.cross_reference_exists?(commit1, commit0)).to be_falsey }
     end
+
+    context 'legacy note with Markdown emphasis' do
+      let(:issue2) { create :issue, project: project }
+      let!(:note) do
+        create :note, system: true, noteable_id: issue2.id,
+                      noteable_type: "Issue", note: "_mentioned in issue " \
+                      "#{issue.project.path_with_namespace}##{issue.iid}_"
+      end
+
+      it 'detects if a mentionable with emphasis has been mentioned' do
+        expect(Note.cross_reference_exists?(issue2, issue)).to be_truthy
+      end
+    end
+  end
+
+  describe '#cross_references_with_underscores?' do
+    let(:project) { create :project, path: "first_project" }
+    let(:second_project) { create :project, path: "second_project" }
+
+    let(:author) { create :user }
+    let(:issue0) { create :issue, project: project }
+    let(:issue1) { create :issue, project: second_project }
+    let!(:note) { Note.create_cross_reference_note(issue0, issue1, author, project) }
+
+    it 'detects if a mentionable has already been mentioned' do
+      expect(Note.cross_reference_exists?(issue0, issue1)).to be_truthy
+    end
+
+    it 'detects if a mentionable has not already been mentioned' do
+      expect(Note.cross_reference_exists?(issue1, issue0)).to be_falsey
+    end
+
+    it 'detects that text has underscores' do
+      expect(note.note).to eq("mentioned in issue #{second_project.path_with_namespace}##{issue1.iid}")
+    end
   end
 
   describe '#system?' do
@@ -429,6 +548,8 @@ describe Note do
     let(:other)   { create(:issue, project: project) }
     let(:author)  { create(:user) }
     let(:assignee) { create(:user) }
+    let(:label) { create(:label) }
+    let(:milestone) { create(:milestone) }
 
     it 'should recognize user-supplied notes as non-system' do
       @note = create(:note_on_issue)
@@ -449,6 +570,16 @@ describe Note do
       @note = Note.create_assignee_change_note(issue, project, author, assignee)
       expect(@note).to be_system
     end
+
+    it 'should identify label-change notes as system notes' do
+      @note = Note.create_labels_change_note(issue, project, author, [label], [])
+      expect(@note).to be_system
+    end
+
+    it 'should identify milestone-change notes as system notes' do
+      @note = Note.create_milestone_change_note(issue, project, author, milestone)
+      expect(@note).to be_system
+    end
   end
 
   describe :authorization do