diff options
author | Patrick Derichs <pderichs@gitlab.com> | 2019-07-30 21:25:49 +0300 |
---|---|---|
committer | Patrick Derichs <pderichs@gitlab.com> | 2019-08-01 11:42:42 +0300 |
commit | 0e99daae4afdb90d74c4b0bfe5cb3e482bbb422e (patch) | |
tree | f6170c10e6e6e657425c3d484eabd04d57c87bb0 /spec/finders/notes_finder_spec.rb | |
parent | 533237a097281dbe4fb1c821d5823c4de8c8f6af (diff) |
Use NotesFinder in IssuableActions module
Remove project from NotesFinder constructor
Add project parameter to specs
Also look for methods in private scope
Fix specs to match new NotesFinder constructor
Diffstat (limited to 'spec/finders/notes_finder_spec.rb')
-rw-r--r-- | spec/finders/notes_finder_spec.rb | 123 |
1 files changed, 70 insertions, 53 deletions
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 87bde4ca2f6..88906adfeeb 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -14,7 +14,7 @@ describe NotesFinder do let!(:system_note) { create(:note_on_issue, project: project, system: true) } it 'returns only user notes when using only_comments filter' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_comments]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_comments]) notes = finder.execute @@ -22,7 +22,7 @@ describe NotesFinder do end it 'returns only system notes when using only_activity filters' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_activity]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_activity]) notes = finder.execute @@ -30,7 +30,7 @@ describe NotesFinder do end it 'gets all notes' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:all_activity]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:all_activity]) notes = finder.execute @@ -41,7 +41,7 @@ describe NotesFinder do it 'finds notes on merge requests' do create(:note_on_merge_request, project: project) - notes = described_class.new(project, user).execute + notes = described_class.new(user, project: project).execute expect(notes.count).to eq(1) end @@ -49,7 +49,7 @@ describe NotesFinder do it 'finds notes on snippets' do create(:note_on_project_snippet, project: project) - notes = described_class.new(project, user).execute + notes = described_class.new(user, project: project).execute expect(notes.count).to eq(1) end @@ -59,13 +59,13 @@ describe NotesFinder do note = create(:note_on_commit, project: project) params = { target_type: 'commit', target_id: note.noteable.id } - notes = described_class.new(project, create(:user), params).execute + notes = described_class.new(create(:user), params).execute expect(notes.count).to eq(0) end it 'succeeds when no notes found' do - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -82,7 +82,7 @@ describe NotesFinder do it 'publicly excludes notes on merge requests' do create(:note_on_merge_request, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -90,7 +90,7 @@ describe NotesFinder do it 'publicly excludes notes on issues' do create(:note_on_issue, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -98,7 +98,7 @@ describe NotesFinder do it 'publicly excludes notes on snippets' do create(:note_on_project_snippet, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -110,7 +110,7 @@ describe NotesFinder do let!(:note2) { create :note_on_commit, project: project } it 'finds only notes for the selected type' do - notes = described_class.new(project, user, target_type: 'issue').execute + notes = described_class.new(user, project: project, target_type: 'issue').execute expect(notes).to eq([note1]) end @@ -118,56 +118,51 @@ describe NotesFinder do context 'for target' do let(:project) { create(:project, :repository) } - let(:note1) { create :note_on_commit, project: project } - let(:note2) { create :note_on_commit, project: project } + let!(:note1) { create :note_on_commit, project: project } + let!(:note2) { create :note_on_commit, project: project } let(:commit) { note1.noteable } - let(:params) { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } - - before do - note1 - note2 - end + let(:params) { { project: project, target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } it 'finds all notes' do - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.size).to eq(2) end it 'finds notes on merge requests' do note = create(:note_on_merge_request, project: project) - params = { target_type: 'merge_request', target_id: note.noteable.id } + params = { project: project, target_type: 'merge_request', target_id: note.noteable.id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes).to include(note) end it 'finds notes on snippets' do note = create(:note_on_project_snippet, project: project) - params = { target_type: 'snippet', target_id: note.noteable.id } + params = { project: project, target_type: 'snippet', target_id: note.noteable.id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.count).to eq(1) end it 'finds notes on personal snippets' do note = create(:note_on_personal_snippet) - params = { target_type: 'personal_snippet', target_id: note.noteable_id } + params = { project: project, target_type: 'personal_snippet', target_id: note.noteable_id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.count).to eq(1) end it 'raises an exception for an invalid target_type' do params[:target_type] = 'invalid' - expect { described_class.new(project, user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") + expect { described_class.new(user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") end it 'filters out old notes' do note2.update_attribute(:updated_at, 2.hours.ago) - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes).to eq([note1]) end @@ -175,25 +170,47 @@ describe NotesFinder do let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } let!(:confidential_note) { create(:note, noteable: confidential_issue, project: confidential_issue.project) } - let(:params) { { target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } } + let(:params) { { project: confidential_issue.project, target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } } it 'returns notes if user can see the issue' do - expect(described_class.new(project, user, params).execute).to eq([confidential_note]) + expect(described_class.new(user, params).execute).to eq([confidential_note]) end it 'raises an error if user can not see the issue' do user = create(:user) - expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end it 'raises an error for project members with guest role' do user = create(:user) project.add_guest(user) - expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end end end + + context 'for explicit target' do + let(:project) { create(:project, :repository) } + let!(:note1) { create :note_on_commit, project: project, created_at: 1.day.ago, updated_at: 2.hours.ago } + let!(:note2) { create :note_on_commit, project: project } + let(:commit) { note1.noteable } + let(:params) { { project: project, target: commit } } + + it 'returns the expected notes' do + expect(described_class.new(user, params).execute).to eq([note1, note2]) + end + + it 'returns the expected notes when last_fetched_at is given' do + params = { project: project, target: commit, last_fetched_at: 1.hour.ago.to_i } + expect(described_class.new(user, params).execute).to eq([note2]) + end + + it 'fails when nil is provided' do + params = { project: project, target: nil } + expect { described_class.new(user, params).execute }.to raise_error(RuntimeError) + end + end end describe '.search' do @@ -201,17 +218,17 @@ describe NotesFinder do let(:note) { create(:note_on_issue, note: 'WoW', project: project) } it 'returns notes with matching content' do - expect(described_class.new(note.project, nil, search: note.note).execute).to eq([note]) + expect(described_class.new(nil, project: note.project, search: note.note).execute).to eq([note]) end it 'returns notes with matching content regardless of the casing' do - expect(described_class.new(note.project, nil, search: 'WOW').execute).to eq([note]) + expect(described_class.new(nil, project: note.project, search: 'WOW').execute).to eq([note]) end it 'returns commit notes user can access' do note = create(:note_on_commit, project: project) - expect(described_class.new(note.project, create(:user), search: note.note).execute).to eq([note]) + expect(described_class.new(create(:user), project: note.project, search: note.note).execute).to eq([note]) end context "confidential issues" do @@ -220,27 +237,27 @@ describe NotesFinder do let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) } it "returns notes with matching content if user can see the issue" do - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to eq([confidential_note]) + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to eq([confidential_note]) end it "does not return notes with matching content if user can not see the issue" do user = create(:user) - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end it "does not return notes with matching content for project members with guest role" do user = create(:user) project.add_guest(user) - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end it "does not return notes with matching content for unauthenticated users" do - expect(described_class.new(confidential_note.project, nil, search: confidential_note.note).execute).to be_empty + expect(described_class.new(nil, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end end context 'inlines SQL filters on subqueries for performance' do - let(:sql) { described_class.new(note.project, nil, search: note.note).execute.to_sql } + let(:sql) { described_class.new(nil, project: note.project, search: note.note).execute.to_sql } let(:number_of_noteable_types) { 4 } specify 'project_id check' do @@ -254,11 +271,11 @@ describe NotesFinder do end describe '#target' do - subject { described_class.new(project, user, params) } + subject { described_class.new(user, params) } context 'for a issue target' do let(:issue) { create(:issue, project: project) } - let(:params) { { target_type: 'issue', target_id: issue.id } } + let(:params) { { project: project, target_type: 'issue', target_id: issue.id } } it 'returns the issue' do expect(subject.target).to eq(issue) @@ -267,7 +284,7 @@ describe NotesFinder do context 'for a merge request target' do let(:merge_request) { create(:merge_request, source_project: project) } - let(:params) { { target_type: 'merge_request', target_id: merge_request.id } } + let(:params) { { project: project, target_type: 'merge_request', target_id: merge_request.id } } it 'returns the merge_request' do expect(subject.target).to eq(merge_request) @@ -276,7 +293,7 @@ describe NotesFinder do context 'for a snippet target' do let(:snippet) { create(:project_snippet, project: project) } - let(:params) { { target_type: 'snippet', target_id: snippet.id } } + let(:params) { { project: project, target_type: 'snippet', target_id: snippet.id } } it 'returns the snippet' do expect(subject.target).to eq(snippet) @@ -286,7 +303,7 @@ describe NotesFinder do context 'for a commit target' do let(:project) { create(:project, :repository) } let(:commit) { project.commit } - let(:params) { { target_type: 'commit', target_id: commit.id } } + let(:params) { { project: project, target_type: 'commit', target_id: commit.id } } it 'returns the commit' do expect(subject.target).to eq(commit) @@ -298,24 +315,24 @@ describe NotesFinder do let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } it 'finds issues by iid' do - iid_params = { target_type: 'issue', target_iid: issue.iid } - expect(described_class.new(project, user, iid_params).target).to eq(issue) + iid_params = { project: project, target_type: 'issue', target_iid: issue.iid } + expect(described_class.new(user, iid_params).target).to eq(issue) end it 'finds merge requests by iid' do - iid_params = { target_type: 'merge_request', target_iid: merge_request.iid } - expect(described_class.new(project, user, iid_params).target).to eq(merge_request) + iid_params = { project: project, target_type: 'merge_request', target_iid: merge_request.iid } + expect(described_class.new(user, iid_params).target).to eq(merge_request) end it 'returns nil if both target_id and target_iid are not given' do - params_without_any_id = { target_type: 'issue' } - expect(described_class.new(project, user, params_without_any_id).target).to be_nil + params_without_any_id = { project: project, target_type: 'issue' } + expect(described_class.new(user, params_without_any_id).target).to be_nil end it 'prioritizes target_id over target_iid' do issue2 = create(:issue, project: project) - iid_params = { target_type: 'issue', target_id: issue2.id, target_iid: issue.iid } - expect(described_class.new(project, user, iid_params).target).to eq(issue2) + iid_params = { project: project, target_type: 'issue', target_id: issue2.id, target_iid: issue.iid } + expect(described_class.new(user, iid_params).target).to eq(issue2) end end end |