address comments

5 files changed, 94 insertions, 31 deletions

diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb
index 2166350f579..3e1ac9fb2b2 100644
--- a/spec/lib/banzai/filter/user_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb
@@ -157,17 +157,20 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do
 
     it 'does not link a User' do
       doc = reference_filter("Hey #{reference}")
+
       expect(doc).not_to include('a')
     end
 
     context 'when skip_project_check set to true' do
       it 'links to a User' do
         doc = reference_filter("Hey #{reference}", skip_project_check: true)
+
         expect(doc.css('a').first.attr('href')).to eq urls.user_url(user)
       end
 
       it 'does not link users using @all reference' do
         doc = reference_filter("Hey #{User.reference_prefix}all", skip_project_check: true)
+
         expect(doc).not_to include('a')
       end
     end
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 0b138405f02..4d57efd3c53 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -172,32 +172,29 @@ describe Ability, lib: true do
   end
 
   describe '.users_that_can_read_personal_snippet' do
-    subject { Ability.users_that_can_read_personal_snippet(users, snippet) }
+    def users_for_snippet(snippet)
+      described_class.users_that_can_read_personal_snippet(users, snippet)
+    end
+
     let(:users)  { create_list(:user, 3) }
     let(:author) { users[0] }
 
-    context 'private snippet' do
-      let(:snippet) { create(:personal_snippet, :private, author: author) }
+    it 'private snippet is readable only by its author' do
+      snippet = create(:personal_snippet, :private, author: author)
 
-      it 'is readable only by its author' do
-        expect(subject).to match_array([author])
-      end
+      expect(users_for_snippet(snippet)).to match_array([author])
     end
 
-    context 'internal snippet' do
-      let(:snippet) { create(:personal_snippet, :public, author: author) }
+    it 'internal snippet is readable by all registered users' do
+      snippet = create(:personal_snippet, :public, author: author)
 
-      it 'is readable by all registered users' do
-        expect(subject).to match_array(users)
-      end
+      expect(users_for_snippet(snippet)).to match_array(users)
     end
 
-    context 'public snippet' do
-      let(:snippet) { create(:personal_snippet, :public, author: author) }
+    it 'public snippet is readable by all users' do
+      snippet = create(:personal_snippet, :public, author: author)
 
-      it 'is readable by all users' do
-        expect(subject).to match_array(users)
-      end
+      expect(users_for_snippet(snippet)).to match_array(users)
     end
   end
 
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 63eb7473c97..b73028f0bc0 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -35,17 +35,14 @@ describe Issue, "Mentionable" do
 
     subject { issue.mentioned_users }
 
-    it { is_expected.to include(user) }
-    it { is_expected.not_to include(user2) }
+    it { expect(subject).to contain_exactly(user) }
 
     context 'when a note on personal snippet' do
       let!(:note) { create(:note_on_personal_snippet, note: "#{user.to_reference} mentioned #{user3.to_reference}") }
 
       subject { note.mentioned_users }
 
-      it { is_expected.to include(user) }
-      it { is_expected.to include(user3) }
-      it { is_expected.not_to include(user2) }
+      it { expect(subject).to contain_exactly(user, user3) }
     end
   end
 
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 8017bf7a13c..1b8ae356f1f 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -59,7 +59,7 @@ describe Note, models: true do
     end
 
     context 'when noteable is a personal snippet' do
-      subject { create(:note_on_personal_snippet) }
+      subject { build(:note_on_personal_snippet) }
 
       it 'is valid without project' do
         is_expected.to be_valid
@@ -321,4 +321,70 @@ describe Note, models: true do
       end
     end
   end
+
+  describe '#for_personal_snippet?' do
+    it 'returns false for a project snippet note' do
+      expect(build(:note_on_project_snippet).for_personal_snippet?).to be_falsy
+    end
+
+    it 'returns true for a personal snippet note' do
+      expect(build(:note_on_personal_snippet).for_personal_snippet?).to be_truthy
+    end
+  end
+
+  describe '#to_ability_name' do
+    it 'returns snippet for a project snippet note' do
+      expect(build(:note_on_project_snippet).to_ability_name).to eq('snippet')
+    end
+
+    it 'returns personal_snippet for a personal snippet note' do
+      expect(build(:note_on_personal_snippet).to_ability_name).to eq('personal_snippet')
+    end
+
+    it 'returns merge_request for an MR note' do
+      expect(build(:note_on_merge_request).to_ability_name).to eq('merge_request')
+    end
+
+    it 'returns issue for an issue note' do
+      expect(build(:note_on_issue).to_ability_name).to eq('issue')
+    end
+
+    it 'returns issue for a commit note' do
+      expect(build(:note_on_commit).to_ability_name).to eq('commit')
+    end
+  end
+
+  describe '#cache_markdown_field' do
+    let(:html) { '<p>some html</p>'}
+
+    context 'note for a project snippet' do
+      let(:note) { build(:note_on_project_snippet) }
+
+      before do
+        expect(Banzai::Renderer).to receive(:cacheless_render_field).
+          with(note, :note, { skip_project_check: false }).and_return(html)
+
+        note.save
+      end
+
+      it 'creates a note' do
+        expect(note.note_html).to eq(html)
+      end
+    end
+
+    context 'note for a personal snippet' do
+      let(:note) { build(:note_on_personal_snippet) }
+
+      before do
+        expect(Banzai::Renderer).to receive(:cacheless_render_field).
+          with(note, :note, { skip_project_check: true }).and_return(html)
+
+        note.save
+      end
+
+      it 'creates a note' do
+        expect(note.note_html).to eq(html)
+      end
+    end
+  end
 end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index b99cdfbae2d..9c92a5080c6 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -15,25 +15,25 @@ describe Notes::CreateService, services: true do
 
     context "valid params" do
       it 'returns a valid note' do
-        note = Notes::CreateService.new(project, user, opts).execute
+        note = described_class.new(project, user, opts).execute
 
         expect(note).to be_valid
       end
 
       it 'returns a persisted note' do
-        note = Notes::CreateService.new(project, user, opts).execute
+        note = described_class.new(project, user, opts).execute
 
         expect(note).to be_persisted
       end
 
       it 'note has valid content' do
-        note = Notes::CreateService.new(project, user, opts).execute
+        note = described_class.new(project, user, opts).execute
 
         expect(note.note).to eq(opts[:note])
       end
 
       it 'note belongs to the correct project' do
-        note = Notes::CreateService.new(project, user, opts).execute
+        note = described_class.new(project, user, opts).execute
 
         expect(note.project).to eq(project)
       end
@@ -44,7 +44,7 @@ describe Notes::CreateService, services: true do
 
         expect_any_instance_of(TodoService).to receive(:new_note).with(note, user)
 
-        Notes::CreateService.new(project, user, opts).execute
+        described_class.new(project, user, opts).execute
       end
 
       it 'enqueues NewNoteWorker' do
@@ -53,7 +53,7 @@ describe Notes::CreateService, services: true do
 
         expect(NewNoteWorker).to receive(:perform_async).with(note.id)
 
-        Notes::CreateService.new(project, user, opts).execute
+        described_class.new(project, user, opts).execute
       end
     end
 
@@ -115,7 +115,7 @@ describe Notes::CreateService, services: true do
         noteable_type: 'Issue',
         noteable_id: issue.id
       }
-      note = Notes::CreateService.new(project, user, opts).execute
+      note = described_class.new(project, user, opts).execute
 
       expect(note).to be_valid
       expect(note.name).to eq('smile')
@@ -127,7 +127,7 @@ describe Notes::CreateService, services: true do
         noteable_type: 'Issue',
         noteable_id: issue.id
       }
-      note = Notes::CreateService.new(project, user, opts).execute
+      note = described_class.new(project, user, opts).execute
 
       expect(note).to be_valid
       expect(note.note).to eq(opts[:note])
@@ -142,7 +142,7 @@ describe Notes::CreateService, services: true do
 
       expect_any_instance_of(TodoService).to receive(:new_award_emoji).with(issue, user)
 
-      Notes::CreateService.new(project, user, opts).execute
+      described_class.new(project, user, opts).execute
     end
   end
 end