From 1dcb43623e00cad56b46ffe80b8bee71d1a1e3f5 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 11 Oct 2018 18:27:25 +0200 Subject: [PATCH] Add more highlighting specs for api v3 --- app/models/query/highlighting.rb | 48 +++++----- .../query_representer_generation_spec.rb | 58 +++++++++++- .../schemas/query_schema_representer_spec.rb | 46 ++++++++++ .../columns/work_package_column.rb | 39 ++++++++ spec/models/query_spec.rb | 90 ++++++++++++++++--- .../api/v3/parse_query_params_service_spec.rb | 15 ++++ .../update_query_from_params_service_spec.rb | 39 ++++++++ 7 files changed, 299 insertions(+), 36 deletions(-) create mode 100644 spec/models/queries/work_packages/columns/work_package_column.rb diff --git a/app/models/query/highlighting.rb b/app/models/query/highlighting.rb index 91771860f9..86fb9e3909 100644 --- a/app/models/query/highlighting.rb +++ b/app/models/query/highlighting.rb @@ -51,18 +51,12 @@ module Query::Highlighting validate :attributes_highlightable? - def available_highlighting_columns - @available_highlighting_columns ||= available_columns.select(&:highlightable?) - end - def valid_highlighting_subset! self.highlighted_attributes = valid_highlighting_subset end - def valid_highlighting_subset - available_names = available_highlighting_columns.map(&:name) - - highlighted_attributes & available_names + def available_highlighting_columns + @available_highlighting_columns ||= available_columns.select(&:highlightable?) end def highlighted_columns @@ -75,6 +69,8 @@ module Query::Highlighting end def highlighted_attributes + return [] unless EnterpriseToken.allows_to?(:conditional_highlighting) + val = super if val.present? @@ -96,6 +92,28 @@ module Query::Highlighting end end + def default_highlighting_mode + QUERY_HIGHLIGHTING_MODES.first + end + + def attributes_highlightable? + # Test that chosen attributes intersect with allowed columns + difference = highlighted_attributes - available_highlighting_columns.map { |col| col.name.to_sym } + if difference.any? + errors.add(:highlighted_attributes, + I18n.t(:error_attribute_not_highlightable, + attributes: difference.map(&:to_s).map(&:capitalize).join(', '))) + end + end + + private + + def valid_highlighting_subset + available_names = available_highlighting_columns.map(&:name) + + highlighted_attributes & available_names + end + def highlighted_attributes_from_setting settings = Setting.work_package_list_default_highlighted_attributes || [] values = settings.map(&:to_sym) @@ -112,19 +130,5 @@ module Query::Highlighting default_highlighting_mode end end - - def default_highlighting_mode - QUERY_HIGHLIGHTING_MODES.first - end - - def attributes_highlightable? - # Test that chosen attributes intersect with allowed columns - difference = highlighted_attributes - available_highlighting_columns.map { |col| col.name.to_sym } - if difference.any? - errors.add(:highlighted_attributes, - I18n.t(:error_attribute_not_highlightable, - attributes: difference.map(&:to_s).map(&:capitalize).join(', '))) - end - end end end diff --git a/spec/lib/api/v3/queries/query_representer_generation_spec.rb b/spec/lib/api/v3/queries/query_representer_generation_spec.rb index aa6c437aae..b969f08b13 100644 --- a/spec/lib/api/v3/queries/query_representer_generation_spec.rb +++ b/spec/lib/api/v3/queries/query_representer_generation_spec.rb @@ -517,8 +517,64 @@ describe ::API::V3::Queries::QueryRepresenter do it 'renders the default' do query.highlighting_mode = nil - + query.highlighted_attributes = nil is_expected.to be_json_eql('inline'.to_json).at_path('highlightingMode') + is_expected.to_not have_json_path('highlightedAttributes') + end + + context "inline attribute highlighting" do + let :status do + { + href: '/api/v3/queries/columns/status', + title: 'Status' + } + end + + let :type do + { + href: '/api/v3/queries/columns/type', + title: 'Type' + } + end + + let :priority do + { + href: '/api/v3/queries/columns/priority', + title: 'Priority' + } + end + + let :due_date do + { + href: '/api/v3/queries/columns/dueDate', + title: 'Finish date' + } + end + + let(:query) do + query = FactoryBot.build_stubbed(:query, project: project) + + query.highlighted_attributes = ['status', 'type', 'priority', 'due_date'] + + query + end + + let(:highlighted_attributes) do + [status, type, priority, due_date] + end + + it 'links an array of highlighted attributes' do + is_expected + .to be_json_eql(highlighted_attributes.to_json).at_path('_links/highlightedAttributes') + end + + it 'embeds selected inline attributes' do + query.highlighted_attributes[0..0].each_with_index do |attr, index| + is_expected + .to be_json_eql("/api/v3/queries/columns/#{attr}".to_json) + .at_path("_embedded/highlightedAttributes/#{index}/_links/self/href") + end + end end end diff --git a/spec/lib/api/v3/queries/schemas/query_schema_representer_spec.rb b/spec/lib/api/v3/queries/schemas/query_schema_representer_spec.rb index 8b39d4cea5..ad148fdccf 100644 --- a/spec/lib/api/v3/queries/schemas/query_schema_representer_spec.rb +++ b/spec/lib/api/v3/queries/schemas/query_schema_representer_spec.rb @@ -284,6 +284,20 @@ describe ::API::V3::Queries::Schemas::QuerySchemaRepresenter do it_behaves_like 'has no visibility property' end + describe 'highlighting_mode' do + let(:path) { 'highlightingMode' } + + it_behaves_like 'has basic schema properties' do + let(:type) { 'String' } + let(:name) { Query.human_attribute_name('highlighting_mode') } + let(:required) { false } + let(:writable) { true } + let(:has_default) { true } + end + + it_behaves_like 'has no visibility property' + end + describe 'columns' do let(:path) { 'columns' } @@ -332,6 +346,38 @@ describe ::API::V3::Queries::Schemas::QuerySchemaRepresenter do end end + describe 'show highlighted_attributes' do + let(:path) { 'highlightedAttributes' } + + it_behaves_like 'has basic schema properties' do + let(:type) { '[]QueryColumn' } + let(:name) { Query.human_attribute_name('highlighted_attributes') } + let(:required) { false } + let(:writable) { true } + let(:has_default) { true } + end + + it_behaves_like 'does not link to allowed values' + + context 'when embedding' do + let(:form_embedded) { true } + let(:type) { FactoryBot.build_stubbed(:type) } + let(:available_values) do + [Queries::WorkPackages::Columns::PropertyColumn.new(:bogus1, highlightable: true), + Queries::WorkPackages::Columns::PropertyColumn.new(:bogus2, highlightable: true)] + end + let(:available_values_method) { :available_columns } + + it_behaves_like 'has a collection of allowed values' do + let(:expected_hrefs) do + available_values.map do |value| + api_v3_paths.query_column(value.name.to_s.camelcase(:lower)) + end + end + end + end + end + describe 'filters' do let(:path) { 'filters' } diff --git a/spec/models/queries/work_packages/columns/work_package_column.rb b/spec/models/queries/work_packages/columns/work_package_column.rb new file mode 100644 index 0000000000..0ed44c3329 --- /dev/null +++ b/spec/models/queries/work_packages/columns/work_package_column.rb @@ -0,0 +1,39 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe Queries::WorkPackages::Columns::WorkPackageColumn, type: :model do + it "allows to be constructed with attribute highlightable" do + expect(described_class.new('foo', highlightable: true).highlightable?).to eq(true) + end + + it "allows to be constructed without attribute highlightable" do + expect(described_class.new('foo').highlightable?).to eq(false) + end +end diff --git a/spec/models/query_spec.rb b/spec/models/query_spec.rb index 330397079a..aaab81c5a7 100644 --- a/spec/models/query_spec.rb +++ b/spec/models/query_spec.rb @@ -82,25 +82,74 @@ describe Query, type: :model do end describe 'highlighting' do - it 'accepts valid values' do - %w(inline none status priority).each do |val| - query.highlighting_mode = val + context 'with EE' do + it '#hightlighting_mode accepts valid values' do + %w(inline none status type priority).each do |val| + query.highlighting_mode = val + expect(query).to be_valid + expect(query.highlighting_mode).to eq(val.to_sym) + end + end + + it '#hightlighting_mode accepts non-present values' do + query.highlighting_mode = nil + expect(query).to be_valid + + query.highlighting_mode = '' expect(query).to be_valid - expect(query.highlighting_mode).to eq(val.to_sym) end - end - it 'accepts non-present values' do - query.highlighting_mode = nil - expect(query).to be_valid + it '#hightlighting_mode rejects invalid values' do + query.highlighting_mode = 'bogus' + expect(query).not_to be_valid + end - query.highlighting_mode = '' - expect(query).to be_valid + it '#available_highlighting_columns returns highlightable Columns' do + available_columns = { + highlightable1: { + highlightable: true + }, + highlightable2: { + highlightable: true + }, + no_highlight: {} + } + + allow(Queries::WorkPackages::Columns::PropertyColumn).to receive(:property_columns) + .and_return(available_columns) + + expect(query.available_highlighting_columns.map(&:name)).to eq(%i{highlightable1 highlightable2}) + end + + describe '#highlighted_columns returns a valid subset of Columns' do + let(:highlighted_attributes) = { %i{status type priority due_date foo} } + + before do + query.highlighted_attributes = highlighted_attributes + end + + it 'removes the offending values' do + query.valid_subset! + + expect(query.highlighted_columns) + .to match_array %i{status type priority due_date} + end + end end - it 'rejects invalid values' do - query.highlighting_mode = 'bogus' - expect(query).not_to be_valid + context 'without EE' do + let(:conditional_highlighting_allowed) { false } + + it 'always returns :none as highlighting_mode' do + query.highlighting_mode = 'status' + expect(query.highlighting_mode).to eq(:none) + end + + it 'always returns nil as highlighted_attributes' do + query.highlighting_mode = 'inline' + query.highlighted_attributes = ['status'] + expect(query.highlighted_attributes).to be_empty + end end end @@ -531,6 +580,21 @@ describe Query, type: :model do end end end + + context 'highlighted_attributes' do + let(:highlighted_attributes) { %i{status type priority due_date foo} } + + before do + query.highlighted_attributes = highlighted_attributes + end + + it 'removes the offending values' do + query.valid_subset! + + expect(query.highlighted_attributes) + .to match_array %i{status type priority due_date} + end + end end describe '#filter_for' do diff --git a/spec/services/api/v3/parse_query_params_service_spec.rb b/spec/services/api/v3/parse_query_params_service_spec.rb index 3aee4f6051..3997df199d 100644 --- a/spec/services/api/v3/parse_query_params_service_spec.rb +++ b/spec/services/api/v3/parse_query_params_service_spec.rb @@ -127,6 +127,21 @@ describe ::API::V3::ParseQueryParamsService, end end + context 'with highlighted_attributes' do + it_behaves_like 'transforms' do + let(:params) { { highlightedAttributes: %w(status type priority dueDate) } } + # Please note, that dueDate is expected to get translated to due_date. + let(:expected) { { highlighted_attributes: %w(status type priority due_date) } } + end + end + + context 'without highlighted_attributes' do + it_behaves_like 'transforms' do + let(:params) { { highlightedAttributes: nil } } + let(:expected) { {} } + end + end + context 'with sort' do context 'as sortBy in comma separated value' do it_behaves_like 'transforms' do diff --git a/spec/services/update_query_from_params_service_spec.rb b/spec/services/update_query_from_params_service_spec.rb index ee53674de8..be14288ded 100644 --- a/spec/services/update_query_from_params_service_spec.rb +++ b/spec/services/update_query_from_params_service_spec.rb @@ -111,5 +111,44 @@ describe UpdateQueryFromParamsService, .to match_array(params[:columns].map(&:to_sym)) end end + + context 'highlighting mode', with_ee: %i[conditional_highlighting] do + let(:params) do + { highlighting_mode: 'status' } + end + + it 'sets the highlighting_mode' do + subject + + expect(query.highlighting_mode) + .to eq(:status) + end + end + + context 'default highlighting mode', with_ee: %i[conditional_highlighting] do + let(:params) do + { } + end + + it 'sets the highlighting_mode' do + subject + + expect(query.highlighting_mode) + .to eq(:inline) + end + end + + context 'highlighting mode without EE' do + let(:params) do + { highlighting_mode: 'status' } + end + + it 'sets the highlighting_mode' do + subject + + expect(query.highlighting_mode) + .to eq(:none) + end + end end end