From b2a6b6b3aa1e799fc8e664a3a1a36124d7360713 Mon Sep 17 00:00:00 2001 From: Igor Artemenko Date: Tue, 3 Sep 2024 21:27:14 +0100 Subject: [PATCH] [JEDI-340] Filter out inactive users (#382) https://runway.powerhrg.com/backlog_items/JEDI-340 --------- Co-authored-by: Carlos Palhares --- audiences/Gemfile.lock | 2 +- audiences/docs/CHANGELOG.md | 4 ++++ audiences/gemfiles/rails_6_1.gemfile.lock | 2 +- audiences/gemfiles/rails_7_0.gemfile.lock | 2 +- audiences/gemfiles/rails_7_1.gemfile.lock | 2 +- audiences/lib/audiences/configuration.rb | 3 ++- audiences/lib/audiences/scim/resource.rb | 14 ++++++++++++-- audiences/lib/audiences/version.rb | 2 +- audiences/spec/lib/audiences_spec.rb | 18 +++++++++--------- .../models/audiences/context_users_spec.rb | 2 +- .../spec/models/audiences/criterion_spec.rb | 5 +++-- .../models/audiences/criterion_users_spec.rb | 14 +++++++------- audiences/spec/requests/contexts_api_spec.rb | 12 ++++++------ 13 files changed, 49 insertions(+), 33 deletions(-) diff --git a/audiences/Gemfile.lock b/audiences/Gemfile.lock index 8eaede36..43b088c1 100644 --- a/audiences/Gemfile.lock +++ b/audiences/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - audiences (1.2.2) + audiences (1.3.0) rails (>= 6.0) GEM diff --git a/audiences/docs/CHANGELOG.md b/audiences/docs/CHANGELOG.md index 18b29d8f..302922b5 100644 --- a/audiences/docs/CHANGELOG.md +++ b/audiences/docs/CHANGELOG.md @@ -1,5 +1,9 @@ # Unreleased +# Version 1.3.0 (2024-09-03) + +- Filter out inactive users by default [#382](https://github.com/powerhome/audiences/pull/382) + # Version 1.2.2 (2024-08-21) - Permit configured resource attributes [#375](https://github.com/powerhome/audiences/pull/375) diff --git a/audiences/gemfiles/rails_6_1.gemfile.lock b/audiences/gemfiles/rails_6_1.gemfile.lock index e636bd2b..fec26e94 100644 --- a/audiences/gemfiles/rails_6_1.gemfile.lock +++ b/audiences/gemfiles/rails_6_1.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - audiences (1.2.2) + audiences (1.3.0) rails (>= 6.0) GEM diff --git a/audiences/gemfiles/rails_7_0.gemfile.lock b/audiences/gemfiles/rails_7_0.gemfile.lock index 88bfcbcd..952b170a 100644 --- a/audiences/gemfiles/rails_7_0.gemfile.lock +++ b/audiences/gemfiles/rails_7_0.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - audiences (1.2.2) + audiences (1.3.0) rails (>= 6.0) GEM diff --git a/audiences/gemfiles/rails_7_1.gemfile.lock b/audiences/gemfiles/rails_7_1.gemfile.lock index 3f144055..246dc3cc 100644 --- a/audiences/gemfiles/rails_7_1.gemfile.lock +++ b/audiences/gemfiles/rails_7_1.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - audiences (1.2.2) + audiences (1.3.0) rails (>= 6.0) GEM diff --git a/audiences/lib/audiences/configuration.rb b/audiences/lib/audiences/configuration.rb index 7c8b85b3..3dd4f59d 100644 --- a/audiences/lib/audiences/configuration.rb +++ b/audiences/lib/audiences/configuration.rb @@ -46,7 +46,8 @@ module Audiences # @see `resource`. # config_accessor :resources do - { Users: Scim::Resource.new(type: :Users, attributes: ["photos" => %w[type value]]) } + { Users: Scim::Resource.new(type: :Users, attributes: ["active", { "photos" => %w[type value] }], + filter: "active eq true") } end # diff --git a/audiences/lib/audiences/scim/resource.rb b/audiences/lib/audiences/scim/resource.rb index e71db5fc..bce2193f 100644 --- a/audiences/lib/audiences/scim/resource.rb +++ b/audiences/lib/audiences/scim/resource.rb @@ -3,17 +3,20 @@ module Audiences module Scim class Resource - attr_accessor :options, :type, :attributes + attr_accessor :options, :type, :attributes, :filter - def initialize(type:, attributes: [], **options) + def initialize(type:, attributes: [], filter: nil, **options) @type = type @options = options @attributes = ["id", "externalId", "displayName", *attributes] + @filter = filter end def query(**options) + options_filter = options.delete(:filter) ResourcesQuery.new(Scim.client, resource: self, attributes: scim_attributes, + filter: merged_filter(options_filter), **@options, **options) end @@ -29,6 +32,13 @@ def scim_attributes end end.join(",") end + + def merged_filter(filter) + return @filter unless filter + return filter unless @filter + + "(#{@filter}) and (#{filter})" + end end end end diff --git a/audiences/lib/audiences/version.rb b/audiences/lib/audiences/version.rb index a139961e..e1024771 100644 --- a/audiences/lib/audiences/version.rb +++ b/audiences/lib/audiences/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Audiences - VERSION = "1.2.2" + VERSION = "1.3.0" end diff --git a/audiences/spec/lib/audiences_spec.rb b/audiences/spec/lib/audiences_spec.rb index 25458751..32bead25 100644 --- a/audiences/spec/lib/audiences_spec.rb +++ b/audiences/spec/lib/audiences_spec.rb @@ -8,7 +8,7 @@ let(:token) { Audiences::Context.for(baseball_club).signed_key } it "updates an audience context from a given key and params" do - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") .with(query: {}) .to_return(status: 200, body: { "Resources" => [] }.to_json) @@ -26,17 +26,17 @@ end it "updates group criterion" do - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") - .with(query: { filter: "groups.value eq 1 OR groups.value eq 2" }) + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") + .with(query: { filter: "(active eq true) and (groups.value eq 1 OR groups.value eq 2)" }) .to_return(status: 200, body: { "Resources" => [] }.to_json) - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") - .with(query: { filter: "groups.value eq 3 OR groups.value eq 4" }) + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") + .with(query: { filter: "(active eq true) and (groups.value eq 3 OR groups.value eq 4)" }) .to_return(status: 200, body: { "Resources" => [] }.to_json) - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") - .with(query: { filter: "groups.value eq 5 OR groups.value eq 6" }) + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") + .with(query: { filter: "(active eq true) and (groups.value eq 5 OR groups.value eq 6)" }) .to_return(status: 200, body: { "Resources" => [] }.to_json) - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") - .with(query: { filter: "groups.value eq 7 OR groups.value eq 8" }) + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") + .with(query: { filter: "(active eq true) and (groups.value eq 7 OR groups.value eq 8)" }) .to_return(status: 200, body: { "Resources" => [] }.to_json) updated_context = Audiences.update( diff --git a/audiences/spec/models/audiences/context_users_spec.rb b/audiences/spec/models/audiences/context_users_spec.rb index 9b778087..ab8e2bd6 100644 --- a/audiences/spec/models/audiences/context_users_spec.rb +++ b/audiences/spec/models/audiences/context_users_spec.rb @@ -11,7 +11,7 @@ { "externalId" => 1414 }, ], } - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") .to_return(status: 200, body: response.to_json) context = Audiences::Context.new(match_all: true) diff --git a/audiences/spec/models/audiences/criterion_spec.rb b/audiences/spec/models/audiences/criterion_spec.rb index 2ecca8b2..51191b29 100644 --- a/audiences/spec/models/audiences/criterion_spec.rb +++ b/audiences/spec/models/audiences/criterion_spec.rb @@ -39,8 +39,9 @@ let(:context) { Audiences::Context.for(owner) } it "fetches the criteria matching users" do - attrs = "id,externalId,displayName,photos.type,photos.value" - stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}&filter=groups.value eq 123") + attrs = "id,externalId,displayName,active,photos.type,photos.value" + stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ + "&filter=(active eq true) and (groups.value eq 123)") .to_return(status: 200, body: { "Resources" => [{ "externalId" => 13 }] }.to_json) criterion = context.criteria.create!(groups: { Departments: [{ id: 123 }] }) diff --git a/audiences/spec/models/audiences/criterion_users_spec.rb b/audiences/spec/models/audiences/criterion_users_spec.rb index a28a41ba..4332f763 100644 --- a/audiences/spec/models/audiences/criterion_users_spec.rb +++ b/audiences/spec/models/audiences/criterion_users_spec.rb @@ -12,9 +12,9 @@ ], } - attrs = "id,externalId,displayName,photos.type,photos.value" + attrs = "id,externalId,displayName,active,photos.type,photos.value" stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 1 OR groups.value eq 3") + "&filter=(active eq true) and (groups.value eq 1 OR groups.value eq 3)") .to_return(status: 200, body: response.to_json) users = Audiences::CriterionUsers.new(criterion).to_a @@ -45,12 +45,12 @@ ], } - attrs = "id,externalId,displayName,photos.type,photos.value" + attrs = "id,externalId,displayName,active,photos.type,photos.value" stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 1 OR groups.value eq 3") + "&filter=(active eq true) and (groups.value eq 1 OR groups.value eq 3)") .to_return(status: 200, body: response1or3.to_json) stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 5 OR groups.value eq 6") + "&filter=(active eq true) and (groups.value eq 5 OR groups.value eq 6)") .to_return(status: 200, body: response5or6.to_json) users = Audiences::CriterionUsers.new(criterion).to_a @@ -80,9 +80,9 @@ ], } - attrs = "id,externalId,displayName,photos.type,photos.value" + attrs = "id,externalId,displayName,active,photos.type,photos.value" stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 1 OR groups.value eq 3") + "&filter=(active eq true) and (groups.value eq 1 OR groups.value eq 3)") .to_return(status: 200, body: response1or3.to_json) stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}&filter=") diff --git a/audiences/spec/requests/contexts_api_spec.rb b/audiences/spec/requests/contexts_api_spec.rb index 2ba8a265..94885758 100644 --- a/audiences/spec/requests/contexts_api_spec.rb +++ b/audiences/spec/requests/contexts_api_spec.rb @@ -26,7 +26,7 @@ end it "updates the audience context to match all" do - stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,photos.type,photos.value") + stub_request(:get, "http://example.com/scim/v2/Users?attributes=id,externalId,displayName,active,photos.type,photos.value&filter=active%20eq%20true") .to_return(status: 200, body: users_response.to_json, headers: {}) put audience_context_path(example_owner, :members), as: :json, params: { match_all: true } @@ -78,18 +78,18 @@ end it "allows updating the group criteria" do - attrs = "id,externalId,displayName,photos.type,photos.value" + attrs = "id,externalId,displayName,active,photos.type,photos.value" stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 123") + "&filter=(active eq true) and (groups.value eq 123)") .to_return(status: 200, body: users_response.to_json, headers: {}) stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 321") + "&filter=(active eq true) and (groups.value eq 321)") .to_return(status: 200, body: users_response.to_json, headers: {}) stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 789") + "&filter=(active eq true) and (groups.value eq 789)") .to_return(status: 200, body: users_response.to_json, headers: {}) stub_request(:get, "http://example.com/scim/v2/Users?attributes=#{attrs}" \ - "&filter=groups.value eq 987") + "&filter=(active eq true) and (groups.value eq 987)") .to_return(status: 200, body: users_response.to_json, headers: {}) put audience_context_path(example_owner, :members),