Support only double quotes for multi-word label references

app/models/label.rb

@@ -41,16 +41,14 @@ class Label < ActiveRecord::Base
   end
 
   # Pattern used to extract label references from text
-  #
-  # TODO (rspeicher): Limit to double quotes (meh) or disallow single quotes in label names (bad).
   def self.reference_pattern
     %r{
       #{reference_prefix}
       (?:
-        (?<label_id>\d+)   | # Integer-based label ID, or
+        (?<label_id>\d+) | # Integer-based label ID, or
         (?<label_name>
-          [A-Za-z0-9_-]+   | # String-based single-word label title
-          ['"][^&\?,]+['"]   # String-based multi-word label surrounded in quotes
+          [A-Za-z0-9_-]+ | # String-based single-word label title, or
+          "[^&\?,]+"       # String-based multi-word label surrounded in quotes
         )
       )
     }x
@@ -70,7 +68,7 @@ class Label < ActiveRecord::Base
   #
   # Returns a String
   def to_reference(format = :id)
-    if format == :name
+    if format == :name && !name.include?('"')
       %(#{self.class.reference_prefix}"#{name}")
     else
       "#{self.class.reference_prefix}#{id}"

lib/gitlab/markdown/label_reference_filter.rb

@@ -72,8 +72,7 @@ module Gitlab
       # Returns a Hash.
       def label_params(id, name)
         if name
-          # TODO (rspeicher): Don't strip single quotes if we decide to only use double quotes for surrounding.
-          { name: name.tr('\'"', '') }
+          { name: name.tr('"', '') }
         else
           { id: id }
         end

spec/lib/gitlab/markdown/label_reference_filter_spec.rb

@@ -100,52 +100,26 @@ module Gitlab::Markdown
     end
 
     context 'String-based multi-word references in quotes' do
-      let(:label) { create(:label, name: 'gfm references', project: project) }
+      let(:label)     { create(:label, name: 'gfm references', project: project) }
+      let(:reference) { label.to_reference(:name) }
 
-      context 'in single quotes' do
-        let(:reference) { "#{Label.reference_prefix}'#{label.name}'" }
-
-        it 'links to a valid reference' do
-          doc = filter("See #{reference}")
-
-          expect(doc.css('a').first.attr('href')).to eq urls.
-            namespace_project_issues_url(project.namespace, project, label_name: label.name)
-          expect(doc.text).to eq 'See gfm references'
-        end
-
-        it 'links with adjacent text' do
-          doc = filter("Label (#{reference}.)")
-          expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\)))
-        end
-
-        it 'ignores invalid label names' do
-          exp = act = "Label #{Label.reference_prefix}'#{label.name.reverse}'"
+      it 'links to a valid reference' do
+        doc = filter("See #{reference}")
 
-          expect(filter(act).to_html).to eq exp
-        end
+        expect(doc.css('a').first.attr('href')).to eq urls.
+          namespace_project_issues_url(project.namespace, project, label_name: label.name)
+        expect(doc.text).to eq 'See gfm references'
       end
 
-      context 'in double quotes' do
-        let(:reference) { %(#{Label.reference_prefix}"#{label.name}") }
-
-        it 'links to a valid reference' do
-          doc = filter("See #{reference}")
-
-          expect(doc.css('a').first.attr('href')).to eq urls.
-            namespace_project_issues_url(project.namespace, project, label_name: label.name)
-          expect(doc.text).to eq 'See gfm references'
-        end
-
-        it 'links with adjacent text' do
-          doc = filter("Label (#{reference}.)")
-          expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\)))
-        end
+      it 'links with adjacent text' do
+        doc = filter("Label (#{reference}.)")
+        expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\)))
+      end
 
-        it 'ignores invalid label names' do
-          exp = act = %(Label #{Label.reference_prefix}"#{label.name.reverse}")
+      it 'ignores invalid label names' do
+        exp = act = %(Label #{Label.reference_prefix}"#{label.name.reverse}")
 
-          expect(filter(act).to_html).to eq exp
-        end
+        expect(filter(act).to_html).to eq exp
       end
     end
 

spec/models/label_spec.rb

@@ -55,13 +55,22 @@ describe Label do
   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}"
+    context 'using id' do
+      it 'returns a String reference to the object' do
+        expect(label.to_reference).to eq "~#{label.id}"
+        expect(label.to_reference(double('project'))).to eq "~#{label.id}"
+      end
     end
 
-    it 'returns a String reference to the object using its name' do
-      expect(label.to_reference(:name)).to eq %(~"#{label.name}")
+    context 'using name' do
+      it 'returns a String reference to the object' do
+        expect(label.to_reference(:name)).to eq %(~"#{label.name}")
+      end
+
+      it 'uses id when name contains double quote' do
+        label = create(:label, name: %q{"irony"})
+        expect(label.to_reference(:name)).to eq "~#{label.id}"
+      end
     end
   end
 end