diff --git a/CHANGELOG b/CHANGELOG
index b6c39ae5f71f49eb668e4915d85108e3aa019548..0897a712eaf3f63b684f96fca7e9ab1d1facf577 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
Please view this file on the master branch, on stable branches it's out of date.
v 7.10.0 (unreleased)
+ - 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)
diff --git a/app/models/note.rb b/app/models/note.rb
index 649e9b4e852e480bf47fbe170e393e2062bfee8d..27b583a869abd25ea2cbc110f792a74a0123ad1c 100644
--- a/app/models/note.rb
+++ b/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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 848ed77ebf8373f2e4e948de477f8b927f8d1cb4..cc5853144c56ef21d9e7f3b4660dc9e237b454ef 100644
--- a/app/services/notification_service.rb
+++ b/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 }
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 1f3fd26a241644d09cd7ecc2880ba94732a98afc..6a272539e45fb7db43877b74652fe24dbffcb0ed 100644
--- a/doc/api/merge_requests.md
+++ b/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",
diff --git a/doc/api/notes.md b/doc/api/notes.md
index c22e493562ab1f0b8df4fbca94582ec3f73419fa..ee2f9fa0eac7192ebe6f8d7b4fa7cc46d039d254 100644
--- a/doc/api/notes.md
+++ b/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,
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 034f8ee7c45f599c441e0071f81579e8e732e05f..5ebe44f6fb79297f903c7e492819883ddf3567f4 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/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
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 17cb439c90e9850d8f40bf71ebcf15554116ee7c..a7bf5081d5b8a9c98176301132c712ab8e5823de 100644
--- a/spec/models/note_spec.rb
+++ b/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