Add `to_reference` for models that support references

Now there is a single source of information for which attribute a model
uses to be referenced, and its special character.

app/models/commit.rb

@@ -1,9 +1,11 @@
 class Commit
-  include ActiveModel::Conversion
-  include StaticModel
   extend ActiveModel::Naming
+
+  include ActiveModel::Conversion
   include Mentionable
   include Participable
+  include Referable
+  include StaticModel
 
   attr_mentionable :safe_message
   participant :author, :committer, :notes, :mentioned_users
@@ -60,6 +62,14 @@ class Commit
     (self.class === other) && (raw == other.raw)
   end
 
+  def to_reference(from_project = nil)
+    if cross_project_reference?(from_project)
+      "#{project.to_reference}@#{id}"
+    else
+      id
+    end
+  end
+
   def diff_line_count
     @diff_line_count ||= Commit::diff_line_count(self.diffs)
     @diff_line_count
@@ -132,7 +142,7 @@ class Commit
 
   # Mentionable override.
   def gfm_reference
-    "commit #{id}"
+    "commit #{to_reference}"
   end
 
   def author

app/models/commit_range.rb

@@ -19,6 +19,7 @@
 #
 class CommitRange
   include ActiveModel::Conversion
+  include Referable
 
   attr_reader :sha_from, :notation, :sha_to
 
@@ -59,6 +60,14 @@ class CommitRange
     "#{sha_from[0..7]}#{notation}#{sha_to[0..7]}"
   end
 
+  def to_reference(from_project = nil)
+    if cross_project_reference?(from_project)
+      "#{project.to_reference}@#{to_s}"
+    else
+      to_s
+    end
+  end
+
   # Returns a String for use in a link's title attribute
   def reference_title
     "Commits #{suffixed_sha_from} through #{sha_to}"

app/models/external_issue.rb

@@ -1,4 +1,6 @@
 class ExternalIssue
+  include Referable
+
   def initialize(issue_identifier, project)
     @issue_identifier, @project = issue_identifier, project
   end
@@ -7,6 +9,10 @@ class ExternalIssue
     @issue_identifier.to_s
   end
 
+  def to_reference(_from_project = nil)
+    id
+  end
+
   def id
     @issue_identifier.to_s
   end

app/models/group.rb

@@ -17,6 +17,8 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class Group < Namespace
+  include Referable
+
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
   has_many :users, through: :group_members
 
@@ -36,6 +38,14 @@ class Group < Namespace
     def sort(method)
       order_by(method)
     end
+
+    def reference_prefix
+      '@'
+    end
+  end
+
+  def to_reference(_from_project = nil)
+    "#{self.class.reference_prefix}#{name}"
   end
 
   def human_name

app/models/issue.rb

@@ -21,10 +21,11 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class Issue < ActiveRecord::Base
-  include Issuable
   include InternalId
-  include Taskable
+  include Issuable
+  include Referable
   include Sortable
+  include Taskable
 
   ActsAsTaggableOn.strict_case_match = true
 
@@ -49,14 +50,28 @@ class Issue < ActiveRecord::Base
     state :closed
   end
 
+  def self.reference_prefix
+    '#'
+  end
+
   def hook_attrs
     attributes
   end
 
+  def to_reference(from_project = nil)
+    reference = "#{self.class.reference_prefix}#{iid}"
+
+    if cross_project_reference?(from_project)
+      reference = project.to_reference + reference
+    end
+
+    reference
+  end
+
   # Mentionable overrides.
 
   def gfm_reference
-    "issue ##{iid}"
+    "issue #{to_reference}"
   end
 
   # Reset issue events cache

app/models/label.rb

@@ -11,6 +11,8 @@
 #
 
 class Label < ActiveRecord::Base
+  include Referable
+
   DEFAULT_COLOR = '#428BCA'
 
   default_value_for :color, DEFAULT_COLOR
@@ -34,6 +36,31 @@ class Label < ActiveRecord::Base
 
   alias_attribute :name, :title
 
+  def self.reference_prefix
+    '~'
+  end
+
+  # Returns the String necessary to reference this Label in Markdown
+  #
+  # format - Symbol format to use (default: :id, optional: :name)
+  #
+  # Note that its argument differs from other objects implementing Referable. If
+  # a non-Symbol argument is given (such as a Project), it will default to :id.
+  #
+  # Examples:
+  #
+  #   Label.first.to_reference        # => "~1"
+  #   Label.first.to_reference(:name) # => "~\"bug\""
+  #
+  # Returns a String
+  def to_reference(format = :id)
+    if format == :name
+      %(#{self.class.reference_prefix}"#{name}")
+    else
+      "#{self.class.reference_prefix}#{id}"
+    end
+  end
+
   def open_issues_count
     issues.opened.count
   end

app/models/merge_request.rb

@@ -25,10 +25,11 @@ require Rails.root.join("app/models/commit")
 require Rails.root.join("lib/static_model")
 
 class MergeRequest < ActiveRecord::Base
-  include Issuable
-  include Taskable
   include InternalId
+  include Issuable
+  include Referable
   include Sortable
+  include Taskable
 
   belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
   belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
@@ -135,6 +136,20 @@ class MergeRequest < ActiveRecord::Base
   scope :closed, -> { with_states(:closed, :merged) }
   scope :declined, -> { with_states(:closed) }
 
+  def self.reference_prefix
+    '!'
+  end
+
+  def to_reference(from_project = nil)
+    reference = "#{self.class.reference_prefix}#{iid}"
+
+    if cross_project_reference?(from_project)
+      reference = project.to_reference + reference
+    end
+
+    reference
+  end
+
   def validate_branches
     if target_project == source_project && target_branch == source_branch
       errors.add :branch_conflict, "You can not use same project/branch for source and target"
@@ -291,7 +306,7 @@ class MergeRequest < ActiveRecord::Base
 
   # Mentionable override.
   def gfm_reference
-    "merge request !#{iid}"
+    "merge request #{to_reference}"
   end
 
   def target_project_path

app/models/project.rb

@@ -33,11 +33,12 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class Project < ActiveRecord::Base
-  include Sortable
+  include Gitlab::ConfigHelper
   include Gitlab::ShellAdapter
   include Gitlab::VisibilityLevel
-  include Gitlab::ConfigHelper
   include Rails.application.routes.url_helpers
+  include Referable
+  include Sortable
 
   extend Gitlab::ConfigHelper
   extend Enumerize
@@ -305,6 +306,10 @@ class Project < ActiveRecord::Base
     path
   end
 
+  def to_reference(_from_project = nil)
+    path_with_namespace
+  end
+
   def web_url
     [gitlab_config.url, path_with_namespace].join('/')
   end

app/models/snippet.rb

@@ -16,10 +16,11 @@
 #
 
 class Snippet < ActiveRecord::Base
-  include Sortable
-  include Linguist::BlobHelper
   include Gitlab::VisibilityLevel
+  include Linguist::BlobHelper
   include Participable
+  include Referable
+  include Sortable
 
   default_value_for :visibility_level, Snippet::PRIVATE
 
@@ -50,6 +51,20 @@ class Snippet < ActiveRecord::Base
 
   participant :author, :notes
 
+  def self.reference_prefix
+    '$'
+  end
+
+  def to_reference(from_project = nil)
+    reference = "#{self.class.reference_prefix}#{id}"
+
+    if cross_project_reference?(from_project)
+      reference = project.to_reference + reference
+    end
+
+    reference
+  end
+
   def self.content_types
     [
       ".rb", ".py", ".pl", ".scala", ".c", ".cpp", ".java",

app/models/user.rb

@@ -62,11 +62,13 @@ require 'carrierwave/orm/activerecord'
 require 'file_size_validator'
 
 class User < ActiveRecord::Base
-  include Sortable
-  include Gitlab::ConfigHelper
-  include TokenAuthenticatable
   extend Gitlab::ConfigHelper
+
+  include Gitlab::ConfigHelper
   include Gitlab::CurrentSettings
+  include Referable
+  include Sortable
+  include TokenAuthenticatable
 
   default_value_for :admin, false
   default_value_for :can_create_group, gitlab_config.default_can_create_group
@@ -247,6 +249,10 @@ class User < ActiveRecord::Base
     def build_user(attrs = {})
       User.new(attrs)
     end
+
+    def reference_prefix
+      '@'
+    end
   end
 
   #
@@ -257,6 +263,10 @@ class User < ActiveRecord::Base
     username
   end
 
+  def to_reference(_from_project = nil)
+    "#{self.class.reference_prefix}#{username}"
+  end
+
   def notification
     @notification ||= Notification.new(self)
   end

spec/models/commit_range_spec.rb

@@ -11,6 +11,29 @@ describe CommitRange do
     expect { described_class.new("Foo") }.to raise_error
   end
 
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Referable) }
+  end
+
+  describe '#to_reference' do
+    let(:project) { double('project', to_reference: 'namespace1/project') }
+
+    before do
+      range.project = project
+    end
+
+    it 'returns a String reference to the object' do
+      expect(range.to_reference).to eq range.to_s
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(range.to_reference(cross)).to eq "#{project.to_reference}@#{range.to_s}"
+    end
+  end
+
   describe '#to_s' do
     it 'is correct for three-dot syntax' do
       expect(range.to_s).to eq "#{sha_from[0..7]}...#{sha_to[0..7]}"

spec/models/commit_spec.rb

@@ -1,8 +1,28 @@
 require 'spec_helper'
 
 describe Commit do
-  let(:project) { create :project }
-  let(:commit) { project.commit }
+  let(:project) { create(:project) }
+  let(:commit)  { project.commit }
+
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Mentionable) }
+    it { is_expected.to include_module(Participable) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(StaticModel) }
+  end
+
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(commit.to_reference).to eq commit.id
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(commit.to_reference(cross)).to eq "#{project.to_reference}@#{commit.id}"
+    end
+  end
 
   describe '#title' do
     it "returns no_commit_message when safe_message is blank" do

spec/models/external_issue_spec.rb

@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe ExternalIssue do
+  let(:project) { double('project', to_reference: 'namespace1/project1') }
+  let(:issue)   { described_class.new('EXT-1234', project) }
+
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Referable) }
+  end
+
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(issue.to_reference).to eq issue.id
+    end
+  end
+
+  describe '#title' do
+    it 'returns a title' do
+      expect(issue.title).to eq "External Issue #{issue}"
+    end
+  end
+end

spec/models/group_spec.rb

@@ -18,16 +18,30 @@ require 'spec_helper'
 describe Group do
   let!(:group) { create(:group) }
 
-  describe "Associations" do
+  describe 'associations' do
     it { is_expected.to have_many :projects }
     it { is_expected.to have_many :group_members }
   end
 
-  it { is_expected.to validate_presence_of :name }
-  it { is_expected.to validate_uniqueness_of(:name) }
-  it { is_expected.to validate_presence_of :path }
-  it { is_expected.to validate_uniqueness_of(:path) }
-  it { is_expected.not_to validate_presence_of :owner }
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Referable) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of :name }
+    it { is_expected.to validate_uniqueness_of(:name) }
+    it { is_expected.to validate_presence_of :path }
+    it { is_expected.to validate_uniqueness_of(:path) }
+    it { is_expected.not_to validate_presence_of :owner }
+  end
+
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(group.to_reference).to eq "@#{group.name}"
+    end
+  end
 
   describe :users do
     it { expect(group.users).to eq(group.owners) }

spec/models/issue_spec.rb

@@ -24,15 +24,30 @@ describe Issue do
     it { is_expected.to belong_to(:milestone) }
   end
 
-  describe "Mass assignment" do
-  end
-
   describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(InternalId) }
     it { is_expected.to include_module(Issuable) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(Sortable) }
+    it { is_expected.to include_module(Taskable) }
   end
 
   subject { create(:issue) }
 
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(subject.to_reference).to eq "##{subject.iid}"
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(subject.to_reference(cross)).
+        to eq "#{subject.project.to_reference}##{subject.iid}"
+    end
+  end
+
   describe '#is_being_reassigned?' do
     it 'returns true if the issue assignee has changed' do
       subject.assignee = create(:user)

spec/models/label_spec.rb

@@ -14,30 +14,54 @@ require 'spec_helper'
 
 describe Label do
   let(:label) { create(:label) }
-  it { expect(label).to be_valid }
 
-  it { is_expected.to belong_to(:project) }
+  describe 'associations' do
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to have_many(:label_links).dependent(:destroy) }
+    it { is_expected.to have_many(:issues).through(:label_links).source(:target) }
+  end
+
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Referable) }
+  end
+
+  describe 'validation' do
+    it { is_expected.to validate_presence_of(:project) }
 
-  describe 'Validation' do
     it 'should validate color code' do
-      expect(build(:label, color: 'G-ITLAB')).not_to be_valid
-      expect(build(:label, color: 'AABBCC')).not_to be_valid
-      expect(build(:label, color: '#AABBCCEE')).not_to be_valid
-      expect(build(:label, color: '#GGHHII')).not_to be_valid
-      expect(build(:label, color: '#')).not_to be_valid
-      expect(build(:label, color: '')).not_to be_valid
+      expect(label).not_to allow_value('G-ITLAB').for(:color)
+      expect(label).not_to allow_value('AABBCC').for(:color)
+      expect(label).not_to allow_value('#AABBCCEE').for(:color)
+      expect(label).not_to allow_value('GGHHII').for(:color)
+      expect(label).not_to allow_value('#').for(:color)
+      expect(label).not_to allow_value('').for(:color)
 
-      expect(build(:label, color: '#AABBCC')).to be_valid
+      expect(label).to allow_value('#AABBCC').for(:color)
+      expect(label).to allow_value('#abcdef').for(:color)
     end
 
     it 'should validate title' do
-      expect(build(:label, title: 'G,ITLAB')).not_to be_valid
-      expect(build(:label, title: 'G?ITLAB')).not_to be_valid
-      expect(build(:label, title: 'G&ITLAB')).not_to be_valid
-      expect(build(:label, title: '')).not_to be_valid
+      expect(label).not_to allow_value('G,ITLAB').for(:title)
+      expect(label).not_to allow_value('G?ITLAB').for(:title)
+      expect(label).not_to allow_value('G&ITLAB').for(:title)
+      expect(label).not_to allow_value('').for(:title)
 
-      expect(build(:label, title: 'GITLAB')).to be_valid
-      expect(build(:label, title: 'gitlab')).to be_valid
+      expect(label).to allow_value('GITLAB').for(:title)
+      expect(label).to allow_value('gitlab').for(:title)
+      expect(label).to allow_value("customer's request").for(:title)
+    end
+  end
+
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(label.to_reference).to eq "~#{label.id}"
+      expect(label.to_reference(double)).to eq "~#{label.id}"
+    end
+
+    it 'returns a String reference to the object using its name' do
+      expect(label.to_reference(:name)).to eq %(~"#{label.name}")
     end
   end
 end

spec/models/merge_request_spec.rb

@@ -24,7 +24,26 @@
 require 'spec_helper'
 
 describe MergeRequest do
-  describe "Validation" do
+  subject { create(:merge_request) }
+
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(InternalId) }
+    it { is_expected.to include_module(Issuable) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(Sortable) }
+    it { is_expected.to include_module(Taskable) }
+  end
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:target_project).with_foreign_key(:target_project_id).class_name('Project') }
+    it { is_expected.to belong_to(:source_project).with_foreign_key(:source_project_id).class_name('Project') }
+
+    it { is_expected.to have_one(:merge_request_diff).dependent(:destroy) }
+  end
+
+  describe 'validation' do
     it { is_expected.to validate_presence_of(:target_branch) }
     it { is_expected.to validate_presence_of(:source_branch) }
   end
@@ -38,8 +57,15 @@ describe MergeRequest do
     it { is_expected.to respond_to(:cannot_be_merged?) }
   end
 
-  describe 'modules' do
-    it { is_expected.to include_module(Issuable) }
+  describe '#to_reference' do
+    it 'returns a String reference to the object' do
+      expect(subject.to_reference).to eq "!#{subject.iid}"
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(subject.to_reference(cross)).to eq "#{subject.source_project.to_reference}!#{subject.iid}"
+    end
   end
 
   describe "#mr_and_commit_notes" do

spec/models/project_spec.rb

@@ -32,7 +32,7 @@
 require 'spec_helper'
 
 describe Project do
-  describe 'Associations' do
+  describe 'associations' do
     it { is_expected.to belong_to(:group) }
     it { is_expected.to belong_to(:namespace) }
     it { is_expected.to belong_to(:creator).class_name('User') }
@@ -54,10 +54,17 @@ describe Project do
     it { is_expected.to have_one(:asana_service).dependent(:destroy) }
   end
 
-  describe 'Mass assignment' do
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Gitlab::ConfigHelper) }
+    it { is_expected.to include_module(Gitlab::ShellAdapter) }
+    it { is_expected.to include_module(Gitlab::VisibilityLevel) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(Sortable) }
   end
 
-  describe 'Validation' do
+  describe 'validation' do
     let!(:project) { create(:project) }
 
     it { is_expected.to validate_presence_of(:name) }
@@ -91,6 +98,14 @@ describe Project do
     it { is_expected.to respond_to(:path_with_namespace) }
   end
 
+  describe '#to_reference' do
+    let(:project) { create(:empty_project) }
+
+    it 'returns a String reference to the object' do
+      expect(project.to_reference).to eq project.path_with_namespace
+    end
+  end
+
   it 'should return valid url to repo' do
     project = Project.new(path: 'somewhere')
     expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git')

spec/models/snippet_spec.rb

@@ -18,7 +18,17 @@
 require 'spec_helper'
 
 describe Snippet do
-  describe "Associations" do
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Gitlab::VisibilityLevel) }
+    it { is_expected.to include_module(Linguist::BlobHelper) }
+    it { is_expected.to include_module(Participable) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(Sortable) }
+  end
+
+  describe 'associations' do
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to have_many(:notes).dependent(:destroy) }
   end
@@ -37,4 +47,18 @@ describe Snippet do
 
     it { is_expected.to validate_presence_of(:content) }
   end
+
+  describe '#to_reference' do
+    let(:project) { create(:empty_project) }
+    let(:snippet) { create(:snippet, project: project) }
+
+    it 'returns a String reference to the object' do
+      expect(snippet.to_reference).to eq "$#{snippet.id}"
+    end
+
+    it 'supports a cross-project reference' do
+      cross = double('project')
+      expect(snippet.to_reference(cross)).to eq "#{project.to_reference}$#{snippet.id}"
+    end
+  end
 end

spec/models/user_spec.rb

@@ -63,7 +63,17 @@ require 'spec_helper'
 describe User do
   include Gitlab::CurrentSettings
 
-  describe "Associations" do
+  describe 'modules' do
+    subject { described_class }
+
+    it { is_expected.to include_module(Gitlab::ConfigHelper) }
+    it { is_expected.to include_module(Gitlab::CurrentSettings) }
+    it { is_expected.to include_module(Referable) }
+    it { is_expected.to include_module(Sortable) }
+    it { is_expected.to include_module(TokenAuthenticatable) }
+  end
+
+  describe 'associations' do
     it { is_expected.to have_one(:namespace) }
     it { is_expected.to have_many(:snippets).class_name('Snippet').dependent(:destroy) }
     it { is_expected.to have_many(:project_members).dependent(:destroy) }
@@ -175,6 +185,14 @@ describe User do
     it { is_expected.to respond_to(:private_token) }
   end
 
+  describe '#to_reference' do
+    let(:user) { create(:user) }
+
+    it 'returns a String reference to the object' do
+      expect(user.to_reference).to eq "@#{user.username}"
+    end
+  end
+
   describe '#generate_password' do
     it "should execute callback when force_random_password specified" do
       user = build(:user, force_random_password: true)