Default reply confidentiality

changelogs/unreleased/dry-up-notes-build-service-spec.yml

+---
+title: Dry up notes build service spec
+merge_request: 54632
+author: Lee Tickett @leetickett
+type: other

spec/services/notes/build_service_spec.rb

@@ -8,26 +8,33 @@ RSpec.describe Notes::BuildService do
   let(:note) { create(:discussion_note_on_issue) }
   let(:project) { note.project }
   let(:author) { note.author }
+  let(:user) { author }
   let(:merge_request) { create(:merge_request, source_project: project) }
-  let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: author) }
+  let(:mr_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note.author) }
+  let(:base_params) { { note: 'Test' } }
+  let(:params) { {} }
+
+  subject(:new_note) { described_class.new(project, user, base_params.merge(params)).execute }
 
   describe '#execute' do
     context 'when in_reply_to_discussion_id is specified' do
+      let(:params) { { in_reply_to_discussion_id: note.discussion_id } }
+
       context 'when a note with that original discussion ID exists' do
         it 'sets the note up to be in reply to that note' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
           expect(new_note).to be_valid
           expect(new_note.in_reply_to?(note)).to be_truthy
           expect(new_note.resolved?).to be_falsey
         end
 
         context 'when discussion is resolved' do
+          let(:params) { { in_reply_to_discussion_id: mr_note.discussion_id } }
+
           before do
             mr_note.resolve!(author)
           end
 
           it 'resolves the note' do
-            new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: mr_note.discussion_id).execute
             expect(new_note).to be_valid
             expect(new_note.resolved?).to be_truthy
           end
@@ -36,24 +43,23 @@ RSpec.describe Notes::BuildService do
 
       context 'when a note with that discussion ID exists' do
         it 'sets the note up to be in reply to that note' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
           expect(new_note).to be_valid
           expect(new_note.in_reply_to?(note)).to be_truthy
         end
       end
 
       context 'when no note with that discussion ID exists' do
+        let(:params) { { in_reply_to_discussion_id: 'foo' } }
+
         it 'sets an error' do
-          new_note = described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: 'foo').execute
           expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found')
         end
       end
 
       context 'when user has no access to discussion' do
-        it 'sets an error' do
-          another_user = create(:user)
-          new_note = described_class.new(project, another_user, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute
+        let(:user) { create(:user) }
 
+        it 'sets an error' do
           expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found')
         end
       end
@@ -129,22 +135,21 @@ RSpec.describe Notes::BuildService do
 
     context 'when replying to individual note' do
       let(:note) { create(:note_on_issue) }
-
-      subject { described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute }
+      let(:params) { { in_reply_to_discussion_id: note.discussion_id } }
 
       it 'sets the note up to be in reply to that note' do
-        expect(subject).to be_valid
-        expect(subject).to be_a(DiscussionNote)
-        expect(subject.discussion_id).to eq(note.discussion_id)
+        expect(new_note).to be_valid
+        expect(new_note).to be_a(DiscussionNote)
+        expect(new_note.discussion_id).to eq(note.discussion_id)
       end
 
       context 'when noteable does not support replies' do
         let(:note) { create(:note_on_commit) }
 
         it 'builds another individual note' do
-          expect(subject).to be_valid
-          expect(subject).to be_a(Note)
-          expect(subject.discussion_id).not_to eq(note.discussion_id)
+          expect(new_note).to be_valid
+          expect(new_note).to be_a(Note)
+          expect(new_note.discussion_id).not_to eq(note.discussion_id)
         end
       end
     end
@@ -156,124 +161,92 @@ RSpec.describe Notes::BuildService do
 
       context 'when replying to a confidential comment' do
         let(:note) { create(:note_on_issue, confidential: true) }
+        let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: false } }
 
         context 'when the user can read confidential comments' do
-          subject do
-            described_class.new(
-              project,
-              author,
-              note: 'Test',
-              in_reply_to_discussion_id: note.discussion_id,
-              confidential: false
-            ).execute
-          end
-
           it '`confidential` param is ignored and set to `true`' do
-            expect(subject.confidential).to be_truthy
+            expect(new_note.confidential).to be_truthy
           end
         end
 
         context 'when the user cannot read confidential comments' do
-          let(:another_user) { create(:user) }
-
-          subject do
-            described_class.new(
-              project,
-              another_user,
-              note: 'Test',
-              in_reply_to_discussion_id: note.discussion_id,
-              confidential: false
-            ).execute
-          end
+          let(:user) { create(:user) }
 
           it 'returns `Discussion to reply to cannot be found` error' do
-            expect(subject.errors.first).to include("Discussion to reply to cannot be found")
+            expect(new_note.errors.first).to include("Discussion to reply to cannot be found")
           end
         end
       end
 
       context 'when replying to a public comment' do
         let(:note) { create(:note_on_issue, confidential: false) }
-
-        subject do
-          described_class.new(
-            project,
-            author,
-            note: 'Test',
-            in_reply_to_discussion_id: note.discussion_id,
-            confidential: true
-          ).execute
-        end
+        let(:params) { { in_reply_to_discussion_id: note.discussion_id, confidential: true } }
 
         it '`confidential` param is ignored and set to `false`' do
-          expect(subject.confidential).to be_falsey
+          expect(new_note.confidential).to be_falsey
         end
       end
 
       context 'when creating a new comment' do
         context 'when the `confidential` note flag is set to `true`' do
           context 'when the user is allowed (reporter)' do
-            subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is allowed (issuable author)' do
-            let(:another_user) { create(:user) }
-            let(:issue) { create(:issue, author: another_user) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: issue, confidential: true).execute }
+            let(:user) { create(:user) }
+            let(:issue) { create(:issue, author: user) }
+            let(:params) { { confidential: true, noteable: issue } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is allowed (admin)' do
             before do
-              enable_admin_mode!(another_user)
+              enable_admin_mode!(admin)
             end
 
-            let(:another_user) { create(:admin) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:admin) { create(:admin) }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `true`' do
-              expect(subject.confidential).to be_truthy
+              expect(new_note.confidential).to be_truthy
             end
           end
 
           context 'when the user is not allowed' do
-            let(:another_user) { create(:user) }
-
-            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+            let(:user) { create(:user) }
+            let(:params) { { confidential: true, noteable: merge_request } }
 
             it 'note `confidential` flag is set to `false`' do
-              expect(subject.confidential).to be_falsey
+              expect(new_note.confidential).to be_falsey
             end
           end
         end
 
         context 'when the `confidential` note flag is set to `false`' do
-          subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: false).execute }
+          let(:params) { { confidential: false, noteable: merge_request } }
 
           it 'note `confidential` flag is set to `false`' do
-            expect(subject.confidential).to be_falsey
+            expect(new_note.confidential).to be_falsey
           end
         end
       end
     end
 
+    context 'when noteable is not set' do
+      let(:params) { { noteable_type: note.noteable_type, noteable_id: note.noteable_id } }
+
       it 'builds a note without saving it' do
-      new_note = described_class.new(project,
-                                    author,
-                                    noteable_type: note.noteable_type,
-                                    noteable_id: note.noteable_id,
-                                    note: 'Test').execute
         expect(new_note).to be_valid
         expect(new_note).not_to be_persisted
       end
     end
+  end
 end