From 73cb4802e257320fbc4c0c1fa576749415372b22 Mon Sep 17 00:00:00 2001 From: ulferts Date: Wed, 8 Dec 2021 13:25:15 +0100 Subject: [PATCH] Feature/increased principal id filter values (#9959) * extend principal id filter to also take non `me` values * remove unused methods --- .../members/filters/principal_filter.rb | 4 - .../queries/principals/filters/id_filter.rb | 28 +++--- .../filter/principal_base_filter.rb | 4 - .../v3/principals/principals_resource_spec.rb | 88 ++++++++----------- 4 files changed, 51 insertions(+), 73 deletions(-) diff --git a/app/models/queries/members/filters/principal_filter.rb b/app/models/queries/members/filters/principal_filter.rb index 83a79c6f34..6065757888 100644 --- a/app/models/queries/members/filters/principal_filter.rb +++ b/app/models/queries/members/filters/principal_filter.rb @@ -51,10 +51,6 @@ class Queries::Members::Filters::PrincipalFilter < Queries::Members::Filters::Me true end - def principal_resource? - true - end - def where operator_strategy.sql_for_field(values_replaced, self.class.model.table_name, :user_id) end diff --git a/app/models/queries/principals/filters/id_filter.rb b/app/models/queries/principals/filters/id_filter.rb index dff49d7d09..2c28a14de8 100644 --- a/app/models/queries/principals/filters/id_filter.rb +++ b/app/models/queries/principals/filters/id_filter.rb @@ -29,8 +29,14 @@ #++ class Queries::Principals::Filters::IdFilter < Queries::Principals::Filters::PrincipalFilter + include Queries::WorkPackages::Filter::MeValueFilterMixin + def allowed_values - [["me", "me"]] # Not the whole truth but performs better than checking all IDs + raise NotImplementedError, 'There would be too many candidates' + end + + def where + operator_strategy.sql_for_field(values_replaced, self.class.model.table_name, self.class.key) end def type @@ -41,21 +47,17 @@ class Queries::Principals::Filters::IdFilter < Queries::Principals::Filters::Pri :id end - def where - operator_strategy.sql_for_field(values_replaced, self.class.model.table_name, self.class.key) + def allowed_values_subset + Principal.visible.pluck(:id).map(&:to_s) + [me_value_key] end - def values_replaced - vals = values.clone + private - if vals.delete('me') - if User.current.logged? - vals.push(User.current.id.to_s) - else - vals.push('0') - end - end + def type_strategy + @type_strategy ||= Queries::Filters::Strategies::HugeList.new(self) + end - vals + def ar_object_filter? + true end end diff --git a/app/models/queries/work_packages/filter/principal_base_filter.rb b/app/models/queries/work_packages/filter/principal_base_filter.rb index 2a89fc3ee5..6abe954afb 100644 --- a/app/models/queries/work_packages/filter/principal_base_filter.rb +++ b/app/models/queries/work_packages/filter/principal_base_filter.rb @@ -46,10 +46,6 @@ class Queries::WorkPackages::Filter::PrincipalBaseFilter < true end - def principal_resource? - true - end - def where operator_strategy.sql_for_field(values_replaced, self.class.model.table_name, self.class.key) end diff --git a/spec/requests/api/v3/principals/principals_resource_spec.rb b/spec/requests/api/v3/principals/principals_resource_spec.rb index 154c84e569..56a7d79cf1 100644 --- a/spec/requests/api/v3/principals/principals_resource_spec.rb +++ b/spec/requests/api/v3/principals/principals_resource_spec.rb @@ -33,21 +33,11 @@ describe 'API v3 Principals resource', type: :request do include Rack::Test::Methods include API::V3::Utilities::PathHelper - describe '#get principals' do - let(:path) do - path = api_v3_paths.principals - - query_props = [] - - if order - query_props << "sortBy=#{JSON.dump(order.map { |(k, v)| [k, v] })}" - end + describe '#GET /api/v3/principals' do + subject(:response) { last_response } - if filter - query_props << "filters=#{CGI.escape(JSON.dump(filter))}" - end - - "#{path}?#{query_props.join('&')}" + let(:path) do + api_v3_paths.path_for :principals, filters: filter, sort_by: order end let(:order) { { name: :desc } } let(:filter) { nil } @@ -102,84 +92,78 @@ describe 'API v3 Principals resource', type: :request do end it 'succeeds' do - expect(last_response.status) - .to eql(200) + expect(response.status) + .to eq(200) end it_behaves_like 'API V3 collection response', 4, 4 do - let(:response) { last_response } - - it 'has the group as the last and the placeholder as the second to last element', :aggregate_failures do - is_expected - .to be_json_eql('PlaceholderUser'.to_json) - .at_path('_embedded/elements/0/_type') - - is_expected - .to be_json_eql('Group'.to_json) - .at_path('_embedded/elements/1/_type') - - is_expected - .to be_json_eql('User'.to_json) - .at_path('_embedded/elements/2/_type') - end + let(:elements) { [placeholder_user, group, other_user, user] } end - context 'provide filter for project the user is member in' do + context 'with a filter for project the user is member in' do let(:filter) do [{ member: { operator: '=', values: [project.id.to_s] } }] end - it_behaves_like 'API V3 collection response', 3, 3 do - let(:response) { last_response } - end + it_behaves_like 'API V3 collection response', 3, 3 end - context 'provide filter for type "User"' do + context 'with a filter for type "User"' do let(:filter) do [{ type: { operator: '=', values: ['User'] } }] end - it_behaves_like 'API V3 collection response', 2, 2, nil do - let(:response) { last_response } - end + it_behaves_like 'API V3 collection response', 2, 2, nil end - context 'provide filter for type "Group"' do + context 'with a filter for type "Group"' do let(:filter) do [{ type: { operator: '=', values: ['Group'] } }] end - it_behaves_like 'API V3 collection response', 1, 1, 'Group' do - let(:response) { last_response } - end + it_behaves_like 'API V3 collection response', 1, 1, 'Group' end - context 'provide filter for type "PlaceholderUser"' do + context 'with a filter for type "PlaceholderUser"' do let(:filter) do [{ type: { operator: '=', values: ['PlaceholderUser'] } }] end - it_behaves_like 'API V3 collection response', 1, 1, 'PlaceholderUser' do - let(:response) { last_response } - end + it_behaves_like 'API V3 collection response', 1, 1, 'PlaceholderUser' end - context 'user without a project membership' do + context 'with a without a project membership' do let(:user) { FactoryBot.create(:user) } # The user herself + it_behaves_like 'API V3 collection response', 1, 1, 'User' + end + + context 'with a filter for any name attribute' do + let(:filter) do + [{ any_name_attribute: { operator: '~', values: ['aaaa@example.com'] } }] + end + + it_behaves_like 'API V3 collection response', 1, 1, 'User' + end + + context 'with a filter for id' do + let(:filter) do + [{ id: { operator: '=', values: [user.id.to_s] } }] + end + it_behaves_like 'API V3 collection response', 1, 1, 'User' do - let(:response) { last_response } + let(:elements) { [user] } end end - context 'provide filter for any name attribute' do + context 'with a filter for id with the `me` value' do let(:filter) do - [{ any_name_attribute: { operator: '~', values: ['aaaa@example.com'] } }] + [{ id: { operator: '=', values: ['me'] } }] end it_behaves_like 'API V3 collection response', 1, 1, 'User' do - let(:response) { last_response } + let(:elements) { [current_user] } end end end