From 9be3d5002969f11ced77c92d8a3161fc5d9fb49d Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 10 Jan 2019 17:11:59 +0100 Subject: [PATCH] Remove more "titles only" logic and adopts specs --- .../lib/acts_as_searchable.rb | 19 +- spec/controllers/search_controller_spec.rb | 162 +++++++++++++----- spec/models/project/activity_spec.rb | 96 +++++------ .../functional/search_controller_spec.rb | 70 -------- 4 files changed, 173 insertions(+), 174 deletions(-) diff --git a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb index e9fbcd85717..8cd9b350502 100644 --- a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb +++ b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb @@ -86,25 +86,22 @@ module Redmine find_order = "#{searchable_options[:order_column]} " + (options[:before] ? 'DESC' : 'ASC') columns = searchable_options[:columns] - columns = columns[0..0] if options[:titles_only] tsv_columns = searchable_options[:tsv_columns] token_clauses = columns.map { |column| "(LOWER(#{column}) LIKE ?)" } - unless options[:titles_only] - if EnterpriseToken.allows_to?(:attachment_filters) && OpenProject::Database.allows_tsv? - tsv_clauses = tsv_columns.map do |tsv_column| - OpenProject::FullTextSearch.tsv_where(tsv_column[:table_name], - tsv_column[:column_name], - tokens.join(' '), - concatenation: :and, - normalization: tsv_column[:normalization_type]) - end + if EnterpriseToken.allows_to?(:attachment_filters) && OpenProject::Database.allows_tsv? + tsv_clauses = tsv_columns.map do |tsv_column| + OpenProject::FullTextSearch.tsv_where(tsv_column[:table_name], + tsv_column[:column_name], + tokens.join(' '), + concatenation: :and, + normalization: tsv_column[:normalization_type]) end end - if !options[:titles_only] && searchable_options[:search_custom_fields] + if searchable_options[:search_custom_fields] searchable_custom_field_ids = CustomField.where(type: "#{name}CustomField", searchable: true).pluck(:id) if searchable_custom_field_ids.any? diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 311ff81de34..1bc75c89541 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -29,25 +29,73 @@ require 'spec_helper' describe SearchController, type: :controller do - let!(:project) { + let!(:project) do FactoryBot.create(:project, - name: 'eCookbook') - } - let(:user) { + name: 'eCookbook') + end + + let!(:other_project) do + FactoryBot.create(:project, + name: 'Other project') + end + + let!(:subproject) do + FactoryBot.create(:project, + name: 'Child project', + parent: project) + end + + let(:role) do + FactoryBot.create(:role, permissions: %i[view_wiki_pages view_work_packages]) + end + + let(:user) do FactoryBot.create(:user, - member_in_project: project) - } + member_in_projects: [project, subproject], + member_through_role: role) + end + + let!(:wiki_page) do + FactoryBot.create(:wiki_page, + title: "How to solve an issue", + wiki: project.wiki) + end + + let!(:work_package_1) do + FactoryBot.create(:work_package, + subject: 'This is a test issue', + project: project) + end + + let!(:work_package_2) do + FactoryBot.create(:work_package, + subject: 'Issue test 2', + project: project, + status: FactoryBot.create(:closed_status)) + end + + let!(:work_package_3) do + FactoryBot.create(:work_package, + subject: 'Issue test 3', + project: subproject) + end + + let!(:work_package_4) do + FactoryBot.create(:work_package, + subject: 'Issue test 4', + project: other_project) + end shared_examples_for 'successful search' do it { expect(response).to be_successful } it { expect(response).to render_template('index') } end - before do allow(User).to receive(:current).and_return user end + before { allow(User).to receive(:current).and_return user } describe 'project search' do context 'without a search parameter' do - before do get :index end + before { get :index } it_behaves_like 'successful search' end @@ -72,66 +120,90 @@ describe SearchController, type: :controller do end describe 'scoped project search' do - before do get :index, params: { project_id: project.id } end + before { get :index, params: { project_id: project.id } } it_behaves_like 'successful search' it { expect(assigns(:project).id).to be(project.id) } end - describe 'work package search' do - let!(:work_package_1) { - FactoryBot.create(:work_package, - subject: 'This is a test issue', - project: project) - } - let!(:work_package_2) { - FactoryBot.create(:work_package, - subject: 'Issue test 2', - project: project, - status: FactoryBot.create(:closed_status)) - } - - context 'when not searching for a note' do - before do get :index, params: { q: 'issue', work_packages: 1 } end + describe 'searching in all modules' do + context 'when searching in all projects' do + before { get :index, params: { q: 'issue', scope: 'all' } } it_behaves_like 'successful search' describe '#result' do - it { expect(assigns(:results).count).to be(2) } - + it { expect(assigns(:results).count).to be(4) } it { expect(assigns(:results)).to include(work_package_1) } - it { expect(assigns(:results)).to include(work_package_2) } + it { expect(assigns(:results)).to include(work_package_3) } + it { expect(assigns(:results)).to include(wiki_page) } + it { expect(assigns(:results)).to_not include(work_package_4) } + end - describe '#view' do - render_views + describe '#results_by_type' do + it { expect(assigns(:results_by_type)).to be_a(Hash) } + it { expect(assigns(:results_by_type)['work_packages']).to eql(3) } + end - it 'marks closed work packages' do - assert_select 'dt.work_package-closed' do - assert_select 'a', text: Regexp.new(work_package_2.status.name) - end + describe '#view' do + render_views + + it 'marks closed work packages' do + assert_select 'dt.work_package-closed' do + assert_select 'a', text: Regexp.new(work_package_2.status.name) end end end end + context 'when searching in project and its subprojects' do + before { get :index, params: { q: 'issue', project_id: project.id, scope: 'subprojects' } } + + it_behaves_like 'successful search' + + describe '#result' do + it { expect(assigns(:results).count).to be(4) } + it { expect(assigns(:results)).to include(work_package_1) } + it { expect(assigns(:results)).to include(work_package_2) } + it { expect(assigns(:results)).to include(work_package_3) } + it { expect(assigns(:results)).to include(wiki_page) } + it { expect(assigns(:results)).to_not include(work_package_4) } + end + end + + context 'when searching in project without its subprojects' do + before { get :index, params: { q: 'issue', project_id: project.id, scope: 'current_project' } } + + it_behaves_like 'successful search' + + describe '#result' do + it { expect(assigns(:results).count).to be(3) } + it { expect(assigns(:results)).to include(work_package_1) } + it { expect(assigns(:results)).to include(work_package_2) } + it { expect(assigns(:results)).to include(wiki_page) } + it { expect(assigns(:results)).to_not include(work_package_3) } + it { expect(assigns(:results)).to_not include(work_package_4) } + end + end + context 'when searching for a note' do - let!(:note_1) { + let!(:note_1) do FactoryBot.create :work_package_journal, - journable_id: work_package_1.id, - notes: 'Test note 1', - version: 2 - } + journable_id: work_package_1.id, + notes: 'Test note 1', + version: 2 + end - before do allow_any_instance_of(Journal).to receive_messages(predecessor: note_1) end + before { allow_any_instance_of(Journal).to receive_messages(predecessor: note_1) } - let!(:note_2) { + let!(:note_2) do FactoryBot.create :work_package_journal, - journable_id: work_package_1.id, - notes: 'Special note 2', - version: 3 - } + journable_id: work_package_1.id, + notes: 'Special note 2', + version: 3 + end describe 'second note predecessor' do subject { note_2.send :predecessor } @@ -142,7 +214,7 @@ describe SearchController, type: :controller do end before do - get :index, params: { q: 'note', work_packages: 1 } + get :index, params: { q: 'note'} end it_behaves_like 'successful search' diff --git a/spec/models/project/activity_spec.rb b/spec/models/project/activity_spec.rb index eaaa76dbeaa..620b17ab763 100644 --- a/spec/models/project/activity_spec.rb +++ b/spec/models/project/activity_spec.rb @@ -29,93 +29,93 @@ require 'spec_helper' describe Project::Activity, type: :model do - let(:project) { + let(:project) do FactoryBot.create(:project) - } + end let(:initial_time) { Time.now } - let(:work_package) { + let(:work_package) do FactoryBot.create(:work_package, - project: project) - } + project: project) + end - let(:work_package2) { + let(:work_package2) do FactoryBot.create(:work_package, - project: project) - } + project: project) + end - let(:wiki_content) { + let(:wiki_content) do project.reload page = FactoryBot.create(:wiki_page, - wiki: project.wiki) + wiki: project.wiki) FactoryBot.create(:wiki_content, - page: page) - } + page: page) + end - let(:wiki_content2) { + let(:wiki_content2) do project.reload page = FactoryBot.create(:wiki_page, - wiki: project.wiki) + wiki: project.wiki) FactoryBot.create(:wiki_content, - page: page) - } + page: page) + end - let(:news) { + let(:news) do FactoryBot.create(:news, - project: project) - } + project: project) + end - let(:news2) { + let(:news2) do FactoryBot.create(:news, - project: project) - } + project: project) + end - let(:repository) { + let(:repository) do FactoryBot.create(:repository_git, - project: project) - } + project: project) + end - let(:changeset) { + let(:changeset) do FactoryBot.create(:changeset, - repository: repository) - } + repository: repository) + end - let(:changeset2) { + let(:changeset2) do FactoryBot.create(:changeset, - repository: repository) - } + repository: repository) + end - let(:board) { + let(:board) do FactoryBot.create(:board, - project: project) - } + project: project) + end - let(:message) { + let(:message) do FactoryBot.create(:message, - board: board) - } + board: board) + end - let(:message2) { + let(:message2) do FactoryBot.create(:message, - board: board) - } + board: board) + end - let(:time_entry) { + let(:time_entry) do FactoryBot.create(:time_entry, - work_package: work_package, - project: project) - } + work_package: work_package, + project: project) + end - let(:time_entry2) { + let(:time_entry2) do FactoryBot.create(:time_entry, - work_package: work_package, - project: project) - } + work_package: work_package, + project: project) + end def latest_activity Project.with_latest_activity.find(project.id).latest_activity_at diff --git a/spec_legacy/functional/search_controller_spec.rb b/spec_legacy/functional/search_controller_spec.rb index d76bd3d3f37..c72e7530871 100644 --- a/spec_legacy/functional/search_controller_spec.rb +++ b/spec_legacy/functional/search_controller_spec.rb @@ -40,28 +40,6 @@ describe SearchController, type: :controller do User.current = nil end - it 'should search all projects' do - get :index, params: { q: 'recipe subproject commit', submit: 'Search' } - assert_response :success - assert_template 'index' - - assert assigns(:results).include?(WorkPackage.find(2)) - assert assigns(:results).include?(WorkPackage.find(5)) - assert assigns(:results).include?(Changeset.find(101)) - - assert assigns(:results_by_type).is_a?(Hash) - assert_equal 5, assigns(:results_by_type)['changesets'] - assert_select 'a', content: 'Changesets (5)' - end - - it 'should search project and subprojects' do - get :index, params: { project_id: 1, q: 'recipe subproject', scope: 'subprojects', submit: 'Search' } - assert_response :success - assert_template 'index' - assert assigns(:results).include?(WorkPackage.find(1)) - assert assigns(:results).include?(WorkPackage.find(5)) - end - it 'should search without searchable custom fields' do CustomField.update_all "searchable = #{ActiveRecord::Base.connection.quoted_false}" @@ -84,63 +62,15 @@ describe SearchController, type: :controller do assert results.include?(WorkPackage.find(7)) end - it 'should search all words' do - # 'all words' is on by default - get :index, params: { project_id: 1, q: 'recipe updating saving' } - results = assigns(:results) - refute_nil results - assert_equal 1, results.size - assert results.include?(WorkPackage.find(3)) - end - - it 'should search one of the words' do - get :index, params: { project_id: 1, q: 'recipe updating saving', submit: 'Search' } - results = assigns(:results) - refute_nil results - assert_equal 4, results.size - assert results.include?(WorkPackage.find(3)) - end - - it 'should search titles only without result' do - get :index, params: { project_id: 1, q: 'recipe updating saving', all_words: '1', titles_only: '1', submit: 'Search' } - results = assigns(:results) - refute_nil results - assert_equal 0, results.size - end - - it 'should search titles only' do - get :index, params: { project_id: 1, q: 'recipe', titles_only: '1', submit: 'Search' } - results = assigns(:results) - refute_nil results - assert_equal 2, results.size - end - it 'should search with invalid project id' do get :index, params: { project_id: 195, q: 'recipe' } assert_response 404 assert_nil assigns(:results) end - it 'should quick jump to work packages' do - # work_package of a public project - get :index, params: { q: '3' } - assert_redirected_to '/work_packages/3' - end - it 'should not jump to an invisible WP' do get :index, params: { q: '4' } assert_response :success assert_template 'index' end - - it 'should large integer' do - get :index, params: { q: '4615713488' } - assert_response :success - assert_template 'index' - end - - it 'should tokens with quotes' do - get :index, params: { project_id: 1, q: '"good bye" hello "bye bye"' } - assert_equal ['good bye', 'hello', 'bye bye'], assigns(:tokens) - end end