From 152117fc553d0984daa7a1ceb4c5773bf18fab3c Mon Sep 17 00:00:00 2001 From: vcorrea Date: Wed, 12 Nov 2025 23:54:36 -0300 Subject: [PATCH 1/4] Allow explicitly blocking actors and groups --- lib/flipper.rb | 4 +-- lib/flipper/dsl.rb | 16 ++++++++++++ lib/flipper/feature.rb | 53 +++++++++++++++++++++++++++++++++++--- lib/flipper/gate.rb | 7 +++++ lib/flipper/gate_values.rb | 8 +++++- lib/flipper/gates/actor.rb | 12 +++++++++ lib/flipper/gates/group.rb | 14 ++++++++++ 7 files changed, 108 insertions(+), 6 deletions(-) diff --git a/lib/flipper.rb b/lib/flipper.rb index 4f05e285e..630a0d8d3 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -59,8 +59,8 @@ def instance=(flipper) :enabled?, :enable, :disable, :enable_expression, :disable_expression, :expression, :add_expression, :remove_expression, - :enable_actor, :disable_actor, - :enable_group, :disable_group, + :enable_actor, :disable_actor, :block_actor, :unblock_actor, + :enable_group, :disable_group, :block_group, :unblock_group, :enable_percentage_of_actors, :disable_percentage_of_actors, :enable_percentage_of_time, :disable_percentage_of_time, :features, :feature, :[], :preload, :preload_all, diff --git a/lib/flipper/dsl.rb b/lib/flipper/dsl.rb index 858e5fa63..84582f077 100644 --- a/lib/flipper/dsl.rb +++ b/lib/flipper/dsl.rb @@ -181,6 +181,22 @@ def disable_percentage_of_actors(name) feature(name).disable_percentage_of_actors end + def block_actor(name, actor) + feature(name).block_actor(actor) + end + + def block_group(name, group) + feature(name).block_group(group) + end + + def unblock_actor(name, actor) + feature(name).unblock_actor(actor) + end + + def unblock_group(name, group) + feature(name).unblock_group(group) + end + # Public: Add a feature. # # name - The String or Symbol name of the feature. diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index a4dcc63ec..0e29a3612 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -68,6 +68,32 @@ def disable(thing = false) end end + def block(thing) + instrument(:block) do |payload| + adapter.add self + + blocking_gate = gate_for(thing, blocking: true) + wrapped_thing = blocking_gate.wrap(thing) + payload[:gate_name] = blocking_gate.name + payload[:thing] = wrapped_thing + + adapter.enable self, blocking_gate, wrapped_thing + end + end + + def unblock(thing) + instrument(:unblock) do |payload| + adapter.add self + + blocking_gate = gate_for(thing, blocking: true) + wrapped_thing = blocking_gate.wrap(thing) + payload[:gate_name] = blocking_gate.name + payload[:thing] = wrapped_thing + + adapter.disable self, blocking_gate, wrapped_thing + end + end + # Public: Adds this feature. # # Returns the result of Adapter#add. @@ -118,7 +144,10 @@ def enabled?(*actors) actors: actors ) - if open_gate = gates.detect { |gate| gate.open?(context) } + if blocking_gate = gates.detect { |gate| gate.blocks?(context) } + payload[:gate_name] = blocking_gate.name + false + elsif open_gate = gates.detect { |gate| gate.open?(context) } payload[:gate_name] = open_gate.name true else @@ -250,6 +279,22 @@ def disable_percentage_of_actors disable Types::PercentageOfActors.new(0) end + def block_actor(actor) + block Types::Actor.wrap(actor) + end + + def block_group(group) + block Types::Group.wrap(group) + end + + def unblock_actor(actor) + unblock Types::Actor.wrap(actor) + end + + def unblock_group(group) + unblock Types::Group.wrap(group) + end + # Public: Returns state for feature (:on, :off, or :conditional). def state values = gate_values @@ -413,6 +458,8 @@ def gates_hash percentage_of_actors: Gates::PercentageOfActors.new, percentage_of_time: Gates::PercentageOfTime.new, group: Gates::Group.new, + blocking_actor: Gates::Actor.new({ blocking: true }), + blocking_group: Gates::Group.new({ blocking: true }), }.freeze end @@ -429,8 +476,8 @@ def gate(name) # # Returns a Flipper::Gate. # Raises Flipper::GateNotFound if no gate found for actor - def gate_for(actor) - gates.detect { |gate| gate.protects?(actor) } || raise(GateNotFound, actor) + def gate_for(actor, blocking: false) + gates.detect { |gate| gate.protects?(actor) && gate.blocking == blocking } || raise(GateNotFound, actor) end private diff --git a/lib/flipper/gate.rb b/lib/flipper/gate.rb index 3b55dc5a2..0ddbc8349 100644 --- a/lib/flipper/gate.rb +++ b/lib/flipper/gate.rb @@ -1,7 +1,10 @@ module Flipper class Gate + attr_reader :blocking + # Public def initialize(options = {}) + @blocking = options.fetch(:blocking, false) end # Public: The name of the gate. Implemented in subclass. @@ -30,6 +33,10 @@ def open?(context) false end + def blocks?(context) + false + end + # Internal: Check if a gate is protects an actor. Implemented in subclass. # # Returns true if gate protects actor, false if not. diff --git a/lib/flipper/gate_values.rb b/lib/flipper/gate_values.rb index 1d9df3b16..25a7c3ce8 100644 --- a/lib/flipper/gate_values.rb +++ b/lib/flipper/gate_values.rb @@ -9,6 +9,8 @@ class GateValues attr_reader :expression attr_reader :percentage_of_actors attr_reader :percentage_of_time + attr_reader :blocking_actors + attr_reader :blocking_groups def initialize(adapter_values) @boolean = Typecast.to_boolean(adapter_values[:boolean]) @@ -17,6 +19,8 @@ def initialize(adapter_values) @expression = adapter_values[:expression] @percentage_of_actors = Typecast.to_number(adapter_values[:percentage_of_actors]) @percentage_of_time = Typecast.to_number(adapter_values[:percentage_of_time]) + @blocking_actors = Typecast.to_set(adapter_values[:blocking_actors]) + @blocking_groups = Typecast.to_set(adapter_values[:blocking_groups]) end def eql?(other) @@ -26,7 +30,9 @@ def eql?(other) groups == other.groups && expression == other.expression && percentage_of_actors == other.percentage_of_actors && - percentage_of_time == other.percentage_of_time + percentage_of_time == other.percentage_of_time && + blocking_actors == other.blocking_actors && + blocking_groups == other.blocking_groups end alias_method :==, :eql? end diff --git a/lib/flipper/gates/actor.rb b/lib/flipper/gates/actor.rb index ed3d84150..d7a139565 100644 --- a/lib/flipper/gates/actor.rb +++ b/lib/flipper/gates/actor.rb @@ -3,11 +3,15 @@ module Gates class Actor < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name + return :blocking_actor if blocking + :actor end # Internal: Name converted to value safe for adapter. def key + return :blocking_actors if blocking + :actors end @@ -30,6 +34,14 @@ def open?(context) end end + def blocks?(context) + return false unless context.actors? + + context.actors.any? do |actor| + context.values.blocking_actors.include?(actor.value) + end + end + def wrap(actor) Types::Actor.wrap(actor) end diff --git a/lib/flipper/gates/group.rb b/lib/flipper/gates/group.rb index 0f966b2ef..971368a8a 100644 --- a/lib/flipper/gates/group.rb +++ b/lib/flipper/gates/group.rb @@ -3,11 +3,15 @@ module Gates class Group < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name + return :blocking_group if blocking + :group end # Internal: Name converted to value safe for adapter. def key + return :blocking_groups if blocking + :groups end @@ -32,6 +36,16 @@ def open?(context) end end + def blocks?(context) + return false unless context.actors? + + context.values.blocking_groups.any? do |name| + context.actors.any? do |actor| + Flipper.group(name).match?(actor, context) + end + end + end + def wrap(thing) Types::Group.wrap(thing) end From 3db8a62bee681121b836667ab55216a1494eec34 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Thu, 26 Feb 2026 09:23:33 -0500 Subject: [PATCH 2/4] Add block_actor and block_group gates for deny list functionality Adds two new gates that allow blocking specific actors and groups from accessing a feature, even when the feature is fully enabled. Block gates are evaluated before allow gates, so a blocked actor is always denied. Key changes: - New BlockActor and BlockGroup gates with higher priority than allow gates - Deny list UI card separate from allow gates, visible regardless of toggle state - Block values preserved through boolean enable/disable operations - API endpoints for managing blocked actors and groups - Shared adapter specs (RSpec + Minitest) for block gate compliance Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 + lib/flipper/adapter.rb | 2 + lib/flipper/adapters/http.rb | 8 +- lib/flipper/adapters/rollout.rb | 2 + lib/flipper/api/middleware.rb | 2 + .../api/v1/actions/block_actors_gate.rb | 44 ++++++ .../api/v1/actions/block_groups_gate.rb | 54 +++++++ lib/flipper/feature.rb | 140 ++++++++++++----- lib/flipper/gate.rb | 5 +- lib/flipper/gate_values.rb | 12 +- lib/flipper/gates/actor.rb | 12 -- lib/flipper/gates/block_actor.rb | 43 ++++++ lib/flipper/gates/block_group.rb | 45 ++++++ lib/flipper/gates/group.rb | 14 -- lib/flipper/spec/shared_adapter_specs.rb | 73 ++++++++- lib/flipper/test/shared_adapter_test.rb | 61 ++++++++ lib/flipper/ui/actions/block_actors_gate.rb | 41 +++++ lib/flipper/ui/actions/block_groups_gate.rb | 35 +++++ lib/flipper/ui/decorators/feature.rb | 32 ++-- lib/flipper/ui/middleware.rb | 2 + lib/flipper/ui/views/feature.erb | 144 ++++++++++++++++++ spec/flipper/adapter_spec.rb | 2 + spec/flipper/adapters/read_only_spec.rb | 2 + spec/flipper/adapters/rollout_spec.rb | 6 + .../api/v1/actions/block_actors_gate_spec.rb | 113 ++++++++++++++ .../api/v1/actions/block_groups_gate_spec.rb | 137 +++++++++++++++++ spec/flipper/api/v1/actions/feature_spec.rb | 40 +++++ spec/flipper/api/v1/actions/features_spec.rb | 32 ++++ spec/flipper/cloud_spec.rb | 24 +-- spec/flipper/exporters/json/v1_spec.rb | 6 +- spec/flipper/feature_spec.rb | 6 +- spec/flipper/gates/block_actor_spec.rb | 59 +++++++ spec/flipper/gates/block_group_spec.rb | 84 ++++++++++ .../ui/actions/block_actors_gate_spec.rb | 135 ++++++++++++++++ .../ui/actions/block_groups_gate_spec.rb | 124 +++++++++++++++ spec/flipper/ui/actions/export_spec.rb | 8 +- 36 files changed, 1433 insertions(+), 118 deletions(-) create mode 100644 lib/flipper/api/v1/actions/block_actors_gate.rb create mode 100644 lib/flipper/api/v1/actions/block_groups_gate.rb create mode 100644 lib/flipper/gates/block_actor.rb create mode 100644 lib/flipper/gates/block_group.rb create mode 100644 lib/flipper/ui/actions/block_actors_gate.rb create mode 100644 lib/flipper/ui/actions/block_groups_gate.rb create mode 100644 spec/flipper/api/v1/actions/block_actors_gate_spec.rb create mode 100644 spec/flipper/api/v1/actions/block_groups_gate_spec.rb create mode 100644 spec/flipper/gates/block_actor_spec.rb create mode 100644 spec/flipper/gates/block_group_spec.rb create mode 100644 spec/flipper/ui/actions/block_actors_gate_spec.rb create mode 100644 spec/flipper/ui/actions/block_groups_gate_spec.rb diff --git a/CLAUDE.md b/CLAUDE.md index dc70fe896..48bc87ae0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,3 +91,5 @@ For outbound HTTP requests, use `Flipper::Adapters::Http::Client` instead of raw Uses both RSpec (currently preferred for new tests) and Minitest. Shared adapter specs ensure consistency across all storage backends. Extensive testing across multiple Rails versions (5.0-8.0). `Flipper.configuration` is reset to nil before each spec (in `spec/spec_helper.rb`), but `Flipper::UI.configuration` is **not** globally reset. When modifying UI config in tests, set the value in `before` and reset it in `after` to match the existing pattern throughout the spec suite. + +**Important**: `lib/flipper/spec/shared_adapter_specs.rb` (RSpec) and `lib/flipper/test/shared_adapter_test.rb` (Minitest) must always be kept in sync. They contain identical test coverage for adapter compliance — one for RSpec, one for Minitest. Any time one is changed, the other must be updated to match. diff --git a/lib/flipper/adapter.rb b/lib/flipper/adapter.rb index 33f6c1fd1..c9c067eb1 100644 --- a/lib/flipper/adapter.rb +++ b/lib/flipper/adapter.rb @@ -15,6 +15,8 @@ def default_config expression: nil, percentage_of_actors: nil, percentage_of_time: nil, + block_actors: Set.new, + block_groups: Set.new, } end diff --git a/lib/flipper/adapters/http.rb b/lib/flipper/adapters/http.rb index 0879ecdca..ba97c5824 100644 --- a/lib/flipper/adapters/http.rb +++ b/lib/flipper/adapters/http.rb @@ -132,7 +132,7 @@ def remove(feature) def enable(feature, gate, thing) body = request_body_for_gate(gate, thing.value) - query_string = gate.key == :groups ? "?allow_unregistered_groups=true" : "" + query_string = %i[groups block_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" response = @client.post("/features/#{feature.key}/#{gate.key}#{query_string}", body) raise Error, response unless response.is_a?(Net::HTTPOK) true @@ -140,7 +140,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) body = request_body_for_gate(gate, thing.value) - query_string = gate.key == :groups ? "?allow_unregistered_groups=true" : "" + query_string = %i[groups block_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" response = case gate.key when :percentage_of_actors, :percentage_of_time @client.post("/features/#{feature.key}/#{gate.key}#{query_string}", body) @@ -171,9 +171,9 @@ def request_body_for_gate(gate, value) data = case gate.key when :boolean {} - when :groups + when :groups, :block_groups { name: value.to_s } - when :actors + when :actors, :block_actors { flipper_id: value.to_s } when :percentage_of_actors, :percentage_of_time { percentage: value.to_s } diff --git a/lib/flipper/adapters/rollout.rb b/lib/flipper/adapters/rollout.rb index 8d35b28ef..12c58dac3 100644 --- a/lib/flipper/adapters/rollout.rb +++ b/lib/flipper/adapters/rollout.rb @@ -50,6 +50,8 @@ def get(feature) percentage_of_actors: percentage_of_actors, percentage_of_time: nil, expression: nil, + block_actors: Set.new, + block_groups: Set.new, } end diff --git a/lib/flipper/api/middleware.rb b/lib/flipper/api/middleware.rb index ae3a166c7..0f274b9ee 100644 --- a/lib/flipper/api/middleware.rb +++ b/lib/flipper/api/middleware.rb @@ -19,6 +19,8 @@ def initialize(app, options = {}) @action_collection.add Api::V1::Actions::PercentageOfActorsGate @action_collection.add Api::V1::Actions::ActorsGate @action_collection.add Api::V1::Actions::GroupsGate + @action_collection.add Api::V1::Actions::BlockActorsGate + @action_collection.add Api::V1::Actions::BlockGroupsGate @action_collection.add Api::V1::Actions::BooleanGate @action_collection.add Api::V1::Actions::ClearFeature @action_collection.add Api::V1::Actions::Actors diff --git a/lib/flipper/api/v1/actions/block_actors_gate.rb b/lib/flipper/api/v1/actions/block_actors_gate.rb new file mode 100644 index 000000000..0fbb6ce34 --- /dev/null +++ b/lib/flipper/api/v1/actions/block_actors_gate.rb @@ -0,0 +1,44 @@ +require 'flipper/api/action' +require 'flipper/api/v1/decorators/feature' + +module Flipper + module Api + module V1 + module Actions + class BlockActorsGate < Api::Action + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/block_actors/?\Z} + + def post + ensure_valid_params + feature = flipper[feature_name] + actor = Actor.new(flipper_id) + feature.block_actor(actor) + decorated_feature = Decorators::Feature.new(feature) + json_response(decorated_feature.as_json, 200) + end + + def delete + ensure_valid_params + feature = flipper[feature_name] + actor = Actor.new(flipper_id) + feature.unblock_actor(actor) + decorated_feature = Decorators::Feature.new(feature) + json_response(decorated_feature.as_json, 200) + end + + private + + def ensure_valid_params + json_error_response(:flipper_id_invalid) if flipper_id.nil? + end + + def flipper_id + @flipper_id ||= params['flipper_id'] + end + end + end + end + end +end diff --git a/lib/flipper/api/v1/actions/block_groups_gate.rb b/lib/flipper/api/v1/actions/block_groups_gate.rb new file mode 100644 index 000000000..a915b82dc --- /dev/null +++ b/lib/flipper/api/v1/actions/block_groups_gate.rb @@ -0,0 +1,54 @@ +require 'flipper/api/action' +require 'flipper/api/v1/decorators/feature' + +module Flipper + module Api + module V1 + module Actions + class BlockGroupsGate < Api::Action + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/block_groups/?\Z} + + def post + ensure_valid_params + feature = flipper[feature_name] + feature.block_group(group_name) + decorated_feature = Decorators::Feature.new(feature) + json_response(decorated_feature.as_json, 200) + end + + def delete + ensure_valid_params + feature = flipper[feature_name] + feature.unblock_group(group_name) + decorated_feature = Decorators::Feature.new(feature) + json_response(decorated_feature.as_json, 200) + end + + private + + def ensure_valid_params + if group_name.nil? || group_name.empty? + json_error_response(:name_invalid) + end + + return if allow_unregistered_groups? + return if Flipper.group_exists?(group_name) + + json_error_response(:group_not_registered) + end + + def allow_unregistered_groups? + allow_unregistered_groups = params['allow_unregistered_groups'] + allow_unregistered_groups && allow_unregistered_groups == 'true' + end + + def group_name + @group_name ||= params['name'] + end + end + end + end + end +end diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index 0e29a3612..6f6566722 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -48,7 +48,10 @@ def enable(thing = true) payload[:gate_name] = gate.name payload[:thing] = wrapped_thing - adapter.enable self, gate, wrapped_thing + saved_blocks = preserve_block_values if gate.data_type == :boolean + result = adapter.enable self, gate, wrapped_thing + restore_block_values(saved_blocks) if saved_blocks + result end end @@ -64,34 +67,51 @@ def disable(thing = false) payload[:gate_name] = gate.name payload[:thing] = wrapped_thing - adapter.disable self, gate, wrapped_thing + saved_blocks = preserve_block_values if gate.data_type == :boolean + result = adapter.disable self, gate, wrapped_thing + restore_block_values(saved_blocks) if saved_blocks + result end end - def block(thing) - instrument(:block) do |payload| - adapter.add self - - blocking_gate = gate_for(thing, blocking: true) - wrapped_thing = blocking_gate.wrap(thing) - payload[:gate_name] = blocking_gate.name - payload[:thing] = wrapped_thing - - adapter.enable self, blocking_gate, wrapped_thing - end + # Public: Block an actor from this feature. + # + # actor - a Flipper::Types::Actor instance or an object that responds + # to flipper_id. + # + # Returns result of Adapter#enable. + def block_actor(actor) + block gate(:block_actor), Types::Actor.wrap(actor) end - def unblock(thing) - instrument(:unblock) do |payload| - adapter.add self + # Public: Block a group from this feature. + # + # group - a Flipper::Types::Group instance or a String or Symbol name of a + # registered group. + # + # Returns result of Adapter#enable. + def block_group(group) + block gate(:block_group), Types::Group.wrap(group) + end - blocking_gate = gate_for(thing, blocking: true) - wrapped_thing = blocking_gate.wrap(thing) - payload[:gate_name] = blocking_gate.name - payload[:thing] = wrapped_thing + # Public: Unblock an actor from this feature. + # + # actor - a Flipper::Types::Actor instance or an object that responds + # to flipper_id. + # + # Returns result of Adapter#disable. + def unblock_actor(actor) + unblock gate(:block_actor), Types::Actor.wrap(actor) + end - adapter.disable self, blocking_gate, wrapped_thing - end + # Public: Unblock a group from this feature. + # + # group - a Flipper::Types::Group instance or a String or Symbol name of a + # registered group. + # + # Returns result of Adapter#disable. + def unblock_group(group) + unblock gate(:block_group), Types::Group.wrap(group) end # Public: Adds this feature. @@ -279,22 +299,6 @@ def disable_percentage_of_actors disable Types::PercentageOfActors.new(0) end - def block_actor(actor) - block Types::Actor.wrap(actor) - end - - def block_group(group) - block Types::Group.wrap(group) - end - - def unblock_actor(actor) - unblock Types::Actor.wrap(actor) - end - - def unblock_group(group) - unblock Types::Group.wrap(group) - end - # Public: Returns state for feature (:on, :off, or :conditional). def state values = gate_values @@ -393,6 +397,20 @@ def percentage_of_time_value gate_values.percentage_of_time end + # Public: Get the adapter value for the block actors gate. + # + # Returns Set of String flipper_id's. + def block_actors_value + gate_values.block_actors + end + + # Public: Get the adapter value for the block groups gate. + # + # Returns Set of String group names. + def block_groups_value + gate_values.block_groups + end + # Public: Get the gates that have been enabled for the feature. # # Returns an Array of Flipper::Gate instances. @@ -458,8 +476,8 @@ def gates_hash percentage_of_actors: Gates::PercentageOfActors.new, percentage_of_time: Gates::PercentageOfTime.new, group: Gates::Group.new, - blocking_actor: Gates::Actor.new({ blocking: true }), - blocking_group: Gates::Group.new({ blocking: true }), + block_actor: Gates::BlockActor.new, + block_group: Gates::BlockGroup.new, }.freeze end @@ -476,12 +494,50 @@ def gate(name) # # Returns a Flipper::Gate. # Raises Flipper::GateNotFound if no gate found for actor - def gate_for(actor, blocking: false) - gates.detect { |gate| gate.protects?(actor) && gate.blocking == blocking } || raise(GateNotFound, actor) + def gate_for(actor) + gates.detect { |gate| gate.protects?(actor) } || raise(GateNotFound, actor) end private + def block(gate, thing) + instrument(:block) do |payload| + adapter.add self + payload[:gate_name] = gate.name + payload[:thing] = thing + adapter.enable self, gate, thing + end + end + + def unblock(gate, thing) + instrument(:unblock) do |payload| + adapter.add self + payload[:gate_name] = gate.name + payload[:thing] = thing + adapter.disable self, gate, thing + end + end + + def preserve_block_values + values = gate_values + block_actors = values.block_actors + block_groups = values.block_groups + return if block_actors.empty? && block_groups.empty? + { block_actors: block_actors, block_groups: block_groups } + end + + def restore_block_values(saved) + block_actor_gate = gate(:block_actor) + saved[:block_actors].each do |flipper_id| + adapter.enable self, block_actor_gate, Types::Actor.wrap(Flipper::Actor.new(flipper_id)) + end + + block_group_gate = gate(:block_group) + saved[:block_groups].each do |group_name| + adapter.enable self, block_group_gate, Types::Group.wrap(group_name) + end + end + # Private: Instrument a feature operation. def instrument(operation, initial_payload = {}) @instrumenter.instrument(InstrumentationName, initial_payload) do |payload| diff --git a/lib/flipper/gate.rb b/lib/flipper/gate.rb index 0ddbc8349..b8523fd14 100644 --- a/lib/flipper/gate.rb +++ b/lib/flipper/gate.rb @@ -1,10 +1,7 @@ module Flipper class Gate - attr_reader :blocking - # Public def initialize(options = {}) - @blocking = options.fetch(:blocking, false) end # Public: The name of the gate. Implemented in subclass. @@ -68,3 +65,5 @@ def inspect require 'flipper/gates/percentage_of_actors' require 'flipper/gates/percentage_of_time' require 'flipper/gates/expression' +require 'flipper/gates/block_actor' +require 'flipper/gates/block_group' diff --git a/lib/flipper/gate_values.rb b/lib/flipper/gate_values.rb index 25a7c3ce8..e8d78c67d 100644 --- a/lib/flipper/gate_values.rb +++ b/lib/flipper/gate_values.rb @@ -9,8 +9,8 @@ class GateValues attr_reader :expression attr_reader :percentage_of_actors attr_reader :percentage_of_time - attr_reader :blocking_actors - attr_reader :blocking_groups + attr_reader :block_actors + attr_reader :block_groups def initialize(adapter_values) @boolean = Typecast.to_boolean(adapter_values[:boolean]) @@ -19,8 +19,8 @@ def initialize(adapter_values) @expression = adapter_values[:expression] @percentage_of_actors = Typecast.to_number(adapter_values[:percentage_of_actors]) @percentage_of_time = Typecast.to_number(adapter_values[:percentage_of_time]) - @blocking_actors = Typecast.to_set(adapter_values[:blocking_actors]) - @blocking_groups = Typecast.to_set(adapter_values[:blocking_groups]) + @block_actors = Typecast.to_set(adapter_values[:block_actors]) + @block_groups = Typecast.to_set(adapter_values[:block_groups]) end def eql?(other) @@ -31,8 +31,8 @@ def eql?(other) expression == other.expression && percentage_of_actors == other.percentage_of_actors && percentage_of_time == other.percentage_of_time && - blocking_actors == other.blocking_actors && - blocking_groups == other.blocking_groups + block_actors == other.block_actors && + block_groups == other.block_groups end alias_method :==, :eql? end diff --git a/lib/flipper/gates/actor.rb b/lib/flipper/gates/actor.rb index d7a139565..ed3d84150 100644 --- a/lib/flipper/gates/actor.rb +++ b/lib/flipper/gates/actor.rb @@ -3,15 +3,11 @@ module Gates class Actor < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name - return :blocking_actor if blocking - :actor end # Internal: Name converted to value safe for adapter. def key - return :blocking_actors if blocking - :actors end @@ -34,14 +30,6 @@ def open?(context) end end - def blocks?(context) - return false unless context.actors? - - context.actors.any? do |actor| - context.values.blocking_actors.include?(actor.value) - end - end - def wrap(actor) Types::Actor.wrap(actor) end diff --git a/lib/flipper/gates/block_actor.rb b/lib/flipper/gates/block_actor.rb new file mode 100644 index 000000000..9d69edc20 --- /dev/null +++ b/lib/flipper/gates/block_actor.rb @@ -0,0 +1,43 @@ +module Flipper + module Gates + class BlockActor < Gate + # Internal: The name of the gate. Used for instrumentation, etc. + def name + :block_actor + end + + # Internal: Name converted to value safe for adapter. + def key + :block_actors + end + + def data_type + :set + end + + def enabled?(value) + !value.empty? + end + + def open?(context) + false + end + + def blocks?(context) + return false unless context.actors? + + context.actors.any? do |actor| + context.values.block_actors.include?(actor.value) + end + end + + def wrap(actor) + Types::Actor.wrap(actor) + end + + def protects?(thing) + Types::Actor.wrappable?(thing) + end + end + end +end diff --git a/lib/flipper/gates/block_group.rb b/lib/flipper/gates/block_group.rb new file mode 100644 index 000000000..fc38d22e1 --- /dev/null +++ b/lib/flipper/gates/block_group.rb @@ -0,0 +1,45 @@ +module Flipper + module Gates + class BlockGroup < Gate + # Internal: The name of the gate. Used for instrumentation, etc. + def name + :block_group + end + + # Internal: Name converted to value safe for adapter. + def key + :block_groups + end + + def data_type + :set + end + + def enabled?(value) + !value.empty? + end + + def open?(context) + false + end + + def blocks?(context) + return false unless context.actors? + + context.values.block_groups.any? do |name| + context.actors.any? do |actor| + Flipper.group(name).match?(actor, context) + end + end + end + + def wrap(thing) + Types::Group.wrap(thing) + end + + def protects?(thing) + thing.is_a?(Types::Group) || thing.is_a?(Symbol) + end + end + end +end diff --git a/lib/flipper/gates/group.rb b/lib/flipper/gates/group.rb index 971368a8a..0f966b2ef 100644 --- a/lib/flipper/gates/group.rb +++ b/lib/flipper/gates/group.rb @@ -3,15 +3,11 @@ module Gates class Group < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name - return :blocking_group if blocking - :group end # Internal: Name converted to value safe for adapter. def key - return :blocking_groups if blocking - :groups end @@ -36,16 +32,6 @@ def open?(context) end end - def blocks?(context) - return false unless context.actors? - - context.values.blocking_groups.any? do |name| - context.actors.any? do |actor| - Flipper.group(name).match?(actor, context) - end - end - end - def wrap(thing) Types::Group.wrap(thing) end diff --git a/lib/flipper/spec/shared_adapter_specs.rb b/lib/flipper/spec/shared_adapter_specs.rb index ba126b02e..328d45bcc 100644 --- a/lib/flipper/spec/shared_adapter_specs.rb +++ b/lib/flipper/spec/shared_adapter_specs.rb @@ -4,12 +4,14 @@ let(:flipper) { Flipper.new(subject) } let(:feature) { flipper[:stats] } - let(:boolean_gate) { feature.gate(:boolean) } - let(:expression_gate) { feature.gate(:expression) } - let(:group_gate) { feature.gate(:group) } - let(:actor_gate) { feature.gate(:actor) } - let(:actors_gate) { feature.gate(:percentage_of_actors) } - let(:time_gate) { feature.gate(:percentage_of_time) } + let(:boolean_gate) { feature.gate(:boolean) } + let(:expression_gate) { feature.gate(:expression) } + let(:group_gate) { feature.gate(:group) } + let(:actor_gate) { feature.gate(:actor) } + let(:actors_gate) { feature.gate(:percentage_of_actors) } + let(:time_gate) { feature.gate(:percentage_of_time) } + let(:block_actor_gate) { feature.gate(:block_actor) } + let(:block_group_gate) { feature.gate(:block_group) } before do Flipper.register(:admins) do |actor| @@ -332,6 +334,65 @@ expect(subject.get(feature)).to eq(subject.default_config.merge(boolean: "true")) end + it 'can enable, disable and get value for block actor gate' do + actor22 = Flipper::Actor.new('22') + actor_asdf = Flipper::Actor.new('asdf') + + expect(subject.enable(feature, block_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) + expect(subject.enable(feature, block_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) + + result = subject.get(feature) + expect(result[:block_actors]).to eq(Set['22', 'asdf']) + + expect(subject.disable(feature, block_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) + result = subject.get(feature) + expect(result[:block_actors]).to eq(Set['22']) + + expect(subject.disable(feature, block_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) + result = subject.get(feature) + expect(result[:block_actors]).to eq(Set.new) + end + + it 'can enable, disable and get value for block group gate' do + expect(subject.enable(feature, block_group_gate, flipper.group(:admins))).to eq(true) + expect(subject.enable(feature, block_group_gate, flipper.group(:early_access))).to eq(true) + + result = subject.get(feature) + expect(result[:block_groups]).to eq(Set['admins', 'early_access']) + + expect(subject.disable(feature, block_group_gate, flipper.group(:early_access))).to eq(true) + result = subject.get(feature) + expect(result[:block_groups]).to eq(Set['admins']) + + expect(subject.disable(feature, block_group_gate, flipper.group(:admins))).to eq(true) + result = subject.get(feature) + expect(result[:block_groups]).to eq(Set.new) + end + + it 'preserves block actors and groups when feature is enabled' do + actor = Flipper::Actor.new('blocked_user') + feature.block_actor(actor) + feature.block_group(:admins) + + feature.enable + + expect(feature.block_actors_value).to eq(Set['blocked_user']) + expect(feature.block_groups_value).to eq(Set['admins']) + expect(feature.enabled?(actor)).to be(false) + end + + it 'preserves block actors and groups when feature is disabled' do + actor = Flipper::Actor.new('blocked_user') + feature.enable_actor(actor) + feature.block_actor(actor) + feature.block_group(:admins) + + feature.disable + + expect(feature.block_actors_value).to eq(Set['blocked_user']) + expect(feature.block_groups_value).to eq(Set['admins']) + end + it 'can import and export' do adapter = Flipper::Adapters::Memory.new source_flipper = Flipper.new(adapter) diff --git a/lib/flipper/test/shared_adapter_test.rb b/lib/flipper/test/shared_adapter_test.rb index c3f825ead..2edbef69e 100644 --- a/lib/flipper/test/shared_adapter_test.rb +++ b/lib/flipper/test/shared_adapter_test.rb @@ -11,6 +11,8 @@ def setup @actor_gate = @feature.gate(:actor) @actors_gate = @feature.gate(:percentage_of_actors) @time_gate = @feature.gate(:percentage_of_time) + @block_actor_gate = @feature.gate(:block_actor) + @block_group_gate = @feature.gate(:block_group) Flipper.register(:admins) do |actor| actor.respond_to?(:admin?) && actor.admin? @@ -329,6 +331,65 @@ def test_clears_other_gate_values_on_enable assert_equal @adapter.default_config.merge(boolean: "true"), @adapter.get(@feature) end + def test_can_enable_disable_and_get_value_for_block_actor_gate + actor22 = Flipper::Actor.new('22') + actor_asdf = Flipper::Actor.new('asdf') + + assert_equal true, @adapter.enable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor22)) + assert_equal true, @adapter.enable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor_asdf)) + + result = @adapter.get(@feature) + assert_equal Set['22', 'asdf'], result[:block_actors] + + assert_equal true, @adapter.disable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor_asdf)) + result = @adapter.get(@feature) + assert_equal Set['22'], result[:block_actors] + + assert_equal true, @adapter.disable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor22)) + result = @adapter.get(@feature) + assert_equal Set.new, result[:block_actors] + end + + def test_can_enable_disable_and_get_value_for_block_group_gate + assert_equal true, @adapter.enable(@feature, @block_group_gate, @flipper.group(:admins)) + assert_equal true, @adapter.enable(@feature, @block_group_gate, @flipper.group(:early_access)) + + result = @adapter.get(@feature) + assert_equal Set['admins', 'early_access'], result[:block_groups] + + assert_equal true, @adapter.disable(@feature, @block_group_gate, @flipper.group(:early_access)) + result = @adapter.get(@feature) + assert_equal Set['admins'], result[:block_groups] + + assert_equal true, @adapter.disable(@feature, @block_group_gate, @flipper.group(:admins)) + result = @adapter.get(@feature) + assert_equal Set.new, result[:block_groups] + end + + def test_preserves_block_actors_and_groups_when_feature_is_enabled + actor = Flipper::Actor.new('blocked_user') + @feature.block_actor(actor) + @feature.block_group(:admins) + + @feature.enable + + assert_equal Set['blocked_user'], @feature.block_actors_value + assert_equal Set['admins'], @feature.block_groups_value + refute @feature.enabled?(actor) + end + + def test_preserves_block_actors_and_groups_when_feature_is_disabled + actor = Flipper::Actor.new('blocked_user') + @feature.enable_actor(actor) + @feature.block_actor(actor) + @feature.block_group(:admins) + + @feature.disable + + assert_equal Set['blocked_user'], @feature.block_actors_value + assert_equal Set['admins'], @feature.block_groups_value + end + def test_can_import_and_export adapter = Flipper::Adapters::Memory.new source_flipper = Flipper.new(adapter) diff --git a/lib/flipper/ui/actions/block_actors_gate.rb b/lib/flipper/ui/actions/block_actors_gate.rb new file mode 100644 index 000000000..6bcd727fd --- /dev/null +++ b/lib/flipper/ui/actions/block_actors_gate.rb @@ -0,0 +1,41 @@ +require 'flipper/ui/action' +require 'flipper/ui/decorators/feature' +require 'flipper/ui/util' + +module Flipper + module UI + module Actions + class BlockActorsGate < UI::Action + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/block_actors/?\Z} + + def post + render_read_only if read_only? + + feature = flipper[feature_name] + value = params['value'].to_s.strip + values = value.split(UI.configuration.actors_separator).map(&:strip).uniq + + if values.empty? + error = "#{value.inspect} is not a valid actor value." + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/block_actors?error=#{Flipper::UI::Util.escape error}") + end + + values.each do |value| + actor = Flipper::Actor.new(value) + + case params['operation'] + when 'block' + feature.block_actor actor + when 'unblock' + feature.unblock_actor actor + end + end + + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}") + end + end + end + end +end diff --git a/lib/flipper/ui/actions/block_groups_gate.rb b/lib/flipper/ui/actions/block_groups_gate.rb new file mode 100644 index 000000000..85a88d810 --- /dev/null +++ b/lib/flipper/ui/actions/block_groups_gate.rb @@ -0,0 +1,35 @@ +require 'flipper/ui/action' +require 'flipper/ui/decorators/feature' + +module Flipper + module UI + module Actions + class BlockGroupsGate < UI::Action + include FeatureNameFromRoute + + route %r{\A/features/(?.*)/block_groups/?\Z} + + def post + render_read_only if read_only? + + feature = flipper[feature_name] + value = params['value'].to_s.strip + + if Flipper.group_exists?(value) + case params['operation'] + when 'block' + feature.block_group value + when 'unblock' + feature.unblock_group value + end + + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}") + else + error = "The group named #{value.inspect} has not been registered." + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/block_groups?error=#{Flipper::UI::Util.escape error}") + end + end + end + end + end +end diff --git a/lib/flipper/ui/decorators/feature.rb b/lib/flipper/ui/decorators/feature.rb index 6e0b0e3f7..a9c287fe0 100644 --- a/lib/flipper/ui/decorators/feature.rb +++ b/lib/flipper/ui/decorators/feature.rb @@ -36,24 +36,34 @@ def color_class end def gates_in_words - return "Fully Enabled" if feature.boolean_value - statuses = [] - if feature.actors_value.count > 0 - statuses << %Q() + Util.pluralize(feature.actors_value.count, 'actor', 'actors') + "" - end + if feature.boolean_value + statuses << "Fully Enabled" + else + if feature.actors_value.count > 0 + statuses << %Q() + Util.pluralize(feature.actors_value.count, 'actor', 'actors') + "" + end + + if feature.groups_value.count > 0 + statuses << %Q() + Util.pluralize(feature.groups_value.count, 'group', 'groups') + "" + end + + if feature.percentage_of_actors_value > 0 + statuses << "#{feature.percentage_of_actors_value}% of actors" + end - if feature.groups_value.count > 0 - statuses << %Q() + Util.pluralize(feature.groups_value.count, 'group', 'groups') + "" + if feature.percentage_of_time_value > 0 + statuses << "#{feature.percentage_of_time_value}% of time" + end end - if feature.percentage_of_actors_value > 0 - statuses << "#{feature.percentage_of_actors_value}% of actors" + if feature.block_actors_value.count > 0 + statuses << %Q() + Util.pluralize(feature.block_actors_value.count, 'blocked actor', 'blocked actors') + "" end - if feature.percentage_of_time_value > 0 - statuses << "#{feature.percentage_of_time_value}% of time" + if feature.block_groups_value.count > 0 + statuses << %Q() + Util.pluralize(feature.block_groups_value.count, 'blocked group', 'blocked groups') + "" end Util.to_sentence(statuses) diff --git a/lib/flipper/ui/middleware.rb b/lib/flipper/ui/middleware.rb index bb7c4fe95..8e3a60de7 100644 --- a/lib/flipper/ui/middleware.rb +++ b/lib/flipper/ui/middleware.rb @@ -20,6 +20,8 @@ def initialize(app, options = {}) @action_collection.add UI::Actions::AddFeature @action_collection.add UI::Actions::ActorsGate @action_collection.add UI::Actions::GroupsGate + @action_collection.add UI::Actions::BlockActorsGate + @action_collection.add UI::Actions::BlockGroupsGate @action_collection.add UI::Actions::BooleanGate @action_collection.add UI::Actions::PercentageOfTimeGate @action_collection.add UI::Actions::PercentageOfActorsGate diff --git a/lib/flipper/ui/views/feature.erb b/lib/flipper/ui/views/feature.erb index 5ad227a63..f540addc9 100644 --- a/lib/flipper/ui/views/feature.erb +++ b/lib/flipper/ui/views/feature.erb @@ -298,6 +298,150 @@ +
+
+
+
+

Deny List

+

Blocked actors and groups are always denied access, even when the feature is fully enabled. These rules are not affected by enabling or disabling the feature.

+
+ + <%# Blocked Actors Info and Form %> +
+
+
+
+ "> + <% if @feature.block_actors_value.count > 0 %> + <%= Util.pluralize @feature.block_actors_value.count, 'blocked actor', 'blocked actors' %> + <% else %> + No blocked actors + <% end %> + +
+
+ <% if write_allowed? %> +
+ +
+
+
+ <%== csrf_input_tag %> + +
+ +
+
+ +
+
+
+
+ +
+ <% end %> +
+
+ + <%# Blocked Actors list %> + <% @feature.block_actors_value.each do |item| %> +
+
+
+
+ <% if Flipper::UI::Util.present?(@feature.actor_names[item]) %> + <%== Sanitize.fragment("#{@feature.actor_names[item]} (#{item})", Sanitize::Config::BASIC) %> + <% else %> + <%= item %> + <% end %> +
+
+
+ <% if write_allowed? %> +
+ <%== csrf_input_tag %> + + + +
+ <% end %> +
+
+
+ <% end %> + + <%# Blocked Groups Info and Form %> +
+
+
+
+ "> + <% if @feature.block_groups_value.count > 0 %> + <%= Util.pluralize @feature.block_groups_value.count, 'blocked group', 'blocked groups' %> + <% else %> + No blocked groups + <% end %> + +
+
+ <% if write_allowed? %> +
+ +
+
+ <% if Flipper.groups.empty? %> + No groups available. + <% else %> +
+ <%== csrf_input_tag %> + +
+ +
+
+
+ <% end %> +
+
+ +
+ <% end %> +
+
+ + <%# Blocked Groups list %> + <% @feature.block_groups_value.each do |item| %> +
+
+
+
<%= item %>
+
+
+ <% if write_allowed? %> +
+ <%== csrf_input_tag %> + + + +
+ <% end %> +
+
+
+ <% end %> +
+
+
+ <% if write_allowed? && Flipper::UI.configuration.feature_removal_enabled %>
diff --git a/spec/flipper/adapter_spec.rb b/spec/flipper/adapter_spec.rb index ca0910549..38f060b4e 100644 --- a/spec/flipper/adapter_spec.rb +++ b/spec/flipper/adapter_spec.rb @@ -9,6 +9,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, + block_actors: Set.new, + block_groups: Set.new, } end diff --git a/spec/flipper/adapters/read_only_spec.rb b/spec/flipper/adapters/read_only_spec.rb index a14c3b423..0648b7245 100644 --- a/spec/flipper/adapters/read_only_spec.rb +++ b/spec/flipper/adapters/read_only_spec.rb @@ -63,6 +63,8 @@ }, percentage_of_actors: '25', percentage_of_time: '45', + block_actors: Set.new, + block_groups: Set.new, }) end diff --git a/spec/flipper/adapters/rollout_spec.rb b/spec/flipper/adapters/rollout_spec.rb index e01b4245d..a1b27c48e 100644 --- a/spec/flipper/adapters/rollout_spec.rb +++ b/spec/flipper/adapters/rollout_spec.rb @@ -36,6 +36,8 @@ expression: nil, percentage_of_actors: 20.0, percentage_of_time: nil, + block_actors: Set.new, + block_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end @@ -50,6 +52,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, + block_actors: Set.new, + block_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end @@ -66,6 +70,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, + block_actors: Set.new, + block_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end diff --git a/spec/flipper/api/v1/actions/block_actors_gate_spec.rb b/spec/flipper/api/v1/actions/block_actors_gate_spec.rb new file mode 100644 index 000000000..1f07afadf --- /dev/null +++ b/spec/flipper/api/v1/actions/block_actors_gate_spec.rb @@ -0,0 +1,113 @@ +RSpec.describe Flipper::Api::V1::Actions::BlockActorsGate do + let(:app) { build_api(flipper) } + let(:actor) { Flipper::Actor.new('1') } + + describe 'block' do + before do + post '/features/my_feature/block_actors', flipper_id: actor.flipper_id + end + + it 'blocks actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].block_actors_value).to include('1') + end + + it 'returns decorated feature with block_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'unblock' do + before do + flipper[:my_feature].block_actor(actor) + delete '/features/my_feature/block_actors', flipper_id: actor.flipper_id + end + + it 'unblocks actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].block_actors_value).to be_empty + end + + it 'returns decorated feature with empty block_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } + expect(gate['value']).to be_empty + end + end + + describe 'block with slash in feature name' do + before do + post '/features/my/feature/block_actors', flipper_id: actor.flipper_id + end + + it 'blocks actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].block_actors_value).to include('1') + end + + it 'returns decorated feature with block_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'block with space in feature name' do + before do + post '/features/sp%20ace/block_actors', flipper_id: actor.flipper_id + end + + it 'blocks actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper["sp ace"].block_actors_value).to include('1') + end + + it 'returns decorated feature with block_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'block missing flipper_id parameter' do + before do + post '/features/my_feature/block_actors' + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'unblock missing flipper_id parameter' do + before do + delete '/features/my_feature/block_actors' + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'block nil flipper_id parameter' do + before do + post '/features/my_feature/block_actors', flipper_id: nil + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'unblock nil flipper_id parameter' do + before do + delete '/features/my_feature/block_actors', flipper_id: nil + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end +end diff --git a/spec/flipper/api/v1/actions/block_groups_gate_spec.rb b/spec/flipper/api/v1/actions/block_groups_gate_spec.rb new file mode 100644 index 000000000..ca00e4aa1 --- /dev/null +++ b/spec/flipper/api/v1/actions/block_groups_gate_spec.rb @@ -0,0 +1,137 @@ +RSpec.describe Flipper::Api::V1::Actions::BlockGroupsGate do + let(:app) { build_api(flipper) } + + describe 'block' do + before do + Flipper.register(:admins) do |actor| + actor.respond_to?(:admin?) && actor.admin? + end + post '/features/my_feature/block_groups', name: 'admins' + end + + it 'blocks group for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].block_groups_value).to include('admins') + end + + it 'returns decorated feature with block_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + expect(group_gate['value']).to eq(['admins']) + end + end + + describe 'block feature with slash in name' do + before do + Flipper.register(:admins) do |actor| + actor.respond_to?(:admin?) && actor.admin? + end + post '/features/my/feature/block_groups', name: 'admins' + end + + it 'blocks group for feature' do + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].block_groups_value).to include('admins') + end + + it 'returns decorated feature with block_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + expect(group_gate['value']).to eq(['admins']) + end + end + + describe 'block without name params' do + before do + post '/features/my_feature/block_groups' + end + + it 'returns correct status code' do + expect(last_response.status).to eq(422) + end + + it 'returns formatted error' do + expected = { + 'code' => 5, + 'message' => 'Required parameter name is missing.', + 'more_info' => api_error_code_reference_url, + } + expect(json_response).to eq(expected) + end + end + + describe 'unblock' do + before do + Flipper.register(:admins) do |actor| + actor.respond_to?(:admin?) && actor.admin? + end + flipper[:my_feature].block_group(:admins) + delete '/features/my_feature/block_groups', name: 'admins' + end + + it 'unblocks group for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].block_groups_value).to be_empty + end + + it 'returns decorated feature with empty block_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + expect(group_gate['value']).to eq([]) + end + end + + describe 'unblock for group not registered' do + before do + delete '/features/my_feature/block_groups', name: 'admins' + end + + it '404s with correct error response when group not registered' do + expect(last_response.status).to eq(404) + expected = { + 'code' => 2, + 'message' => 'Group not registered.', + 'more_info' => api_error_code_reference_url, + } + expect(json_response).to eq(expected) + end + end + + describe 'block for group not registered when allow_unregistered_groups is true' do + before do + Flipper.unregister_groups + post '/features/my_feature/block_groups', name: 'admins', allow_unregistered_groups: 'true' + end + + it 'responds successfully' do + expect(last_response.status).to eq(200) + end + + it 'returns decorated feature with group in block_groups set' do + group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + expect(group_gate['value']).to eq(['admins']) + end + + it 'blocks group' do + expect(flipper[:my_feature].block_groups_value).to eq(Set["admins"]) + end + end + + describe 'unblock for group not registered when allow_unregistered_groups is true' do + before do + Flipper.unregister_groups + flipper[:my_feature].block_group(:admins) + delete '/features/my_feature/block_groups', name: 'admins', allow_unregistered_groups: 'true' + end + + it 'responds successfully' do + expect(last_response.status).to eq(200) + end + + it 'returns decorated feature with group not in block_groups set' do + group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + expect(group_gate['value']).to eq([]) + end + + it 'unblocks group' do + expect(flipper[:my_feature].block_groups_value).to be_empty + end + end +end diff --git a/spec/flipper/api/v1/actions/feature_spec.rb b/spec/flipper/api/v1/actions/feature_spec.rb index 4abe5818f..4e64bc7d6 100644 --- a/spec/flipper/api/v1/actions/feature_spec.rb +++ b/spec/flipper/api/v1/actions/feature_spec.rb @@ -45,6 +45,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], } @@ -94,6 +104,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], } @@ -159,6 +179,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], } @@ -208,6 +238,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], } diff --git a/spec/flipper/api/v1/actions/features_spec.rb b/spec/flipper/api/v1/actions/features_spec.rb index ee69e4af0..3e6e8861e 100644 --- a/spec/flipper/api/v1/actions/features_spec.rb +++ b/spec/flipper/api/v1/actions/features_spec.rb @@ -49,6 +49,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], }, ], @@ -70,6 +80,8 @@ {'key' => 'percentage_of_actors', 'value' => nil}, { 'key' => 'percentage_of_time', 'value' => nil}, { 'key' => 'groups', 'value' => []}, + { 'key' => 'block_actors', 'value' => []}, + { 'key' => 'block_groups', 'value' => []}, ], }, ], @@ -170,6 +182,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], }, ], @@ -226,6 +248,16 @@ 'name' => 'group', 'value' => [], }, + { + 'key' => 'block_actors', + 'name' => 'block_actor', + 'value' => [], + }, + { + 'key' => 'block_groups', + 'name' => 'block_group', + 'value' => [], + }, ], } expect(json_response).to eq(expected_response) diff --git a/spec/flipper/cloud_spec.rb b/spec/flipper/cloud_spec.rb index ef436105e..3aa6a46cc 100644 --- a/spec/flipper/cloud_spec.rb +++ b/spec/flipper/cloud_spec.rb @@ -122,10 +122,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5"}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) @@ -148,10 +148,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5"}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) @@ -173,10 +173,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5"}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) diff --git a/spec/flipper/exporters/json/v1_spec.rb b/spec/flipper/exporters/json/v1_spec.rb index 616dabf1f..3634acfaa 100644 --- a/spec/flipper/exporters/json/v1_spec.rb +++ b/spec/flipper/exporters/json/v1_spec.rb @@ -25,9 +25,9 @@ export = subject.call(adapter) expect(export.features).to eq({ - "google_analytics" => {actors: Set.new, boolean: nil, expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil}, - "plausible" => {actors: Set.new, boolean: "true", expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil}, - "search" => {actors: Set["User;1", "User;100"], boolean: nil, expression: nil, groups: Set["admins", "employees"], percentage_of_actors: "10", percentage_of_time: "15"}, + "google_analytics" => {actors: Set.new, boolean: nil, expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "plausible" => {actors: Set.new, boolean: "true", expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "search" => {actors: Set["User;1", "User;100"], boolean: nil, expression: nil, groups: Set["admins", "employees"], percentage_of_actors: "10", percentage_of_time: "15", block_actors: Set.new, block_groups: Set.new}, }) end end diff --git a/spec/flipper/feature_spec.rb b/spec/flipper/feature_spec.rb index 600d25db3..d76fcb7f0 100644 --- a/spec/flipper/feature_spec.rb +++ b/spec/flipper/feature_spec.rb @@ -129,7 +129,7 @@ def actor.nil? instance.gates.each do |gate| expect(gate).to be_a(Flipper::Gate) end - expect(instance.gates.size).to be(6) + expect(instance.gates.size).to be(8) end end @@ -1214,6 +1214,8 @@ def actor.nil? :boolean, :group, :expression, + :block_actor, + :block_group, ]) expect(subject.disabled_gate_names.to_set).to eq(Set[ @@ -1221,6 +1223,8 @@ def actor.nil? :boolean, :group, :expression, + :block_actor, + :block_group, ]) end end diff --git a/spec/flipper/gates/block_actor_spec.rb b/spec/flipper/gates/block_actor_spec.rb new file mode 100644 index 000000000..620324b20 --- /dev/null +++ b/spec/flipper/gates/block_actor_spec.rb @@ -0,0 +1,59 @@ +RSpec.describe Flipper::Gates::BlockActor do + let(:feature_name) { :search } + + subject do + described_class.new + end + + def context(set, actors: [Flipper::Types::Actor.new(Flipper::Actor.new('5'))]) + Flipper::FeatureCheckContext.new( + feature_name: feature_name, + values: Flipper::GateValues.new(block_actors: set), + actors: actors + ) + end + + describe '#blocks?' do + it 'returns true when actor is in blocked set' do + expect(subject.blocks?(context(Set['5']))).to be(true) + end + + it 'returns false when actor is not in blocked set' do + expect(subject.blocks?(context(Set['10']))).to be(false) + end + + it 'returns false when blocked set is empty' do + expect(subject.blocks?(context(Set.new))).to be(false) + end + + it 'returns false when no actors in context' do + expect(subject.blocks?(context(Set['5'], actors: []))).to be(false) + end + end + + describe '#open?' do + it 'always returns false' do + expect(subject.open?(context(Set['5']))).to be(false) + end + end + + describe '#wrap' do + it 'returns actor instance' do + actor = Flipper::Actor.new('5') + result = subject.wrap(actor) + expect(result).to be_instance_of(Flipper::Types::Actor) + expect(result.value).to eq('5') + end + end + + describe '#protects?' do + it 'returns true for actor' do + actor = Flipper::Actor.new('5') + expect(subject.protects?(actor)).to be(true) + end + + it 'returns false for non-actor' do + expect(subject.protects?(:admins)).to be(false) + end + end +end diff --git a/spec/flipper/gates/block_group_spec.rb b/spec/flipper/gates/block_group_spec.rb new file mode 100644 index 000000000..aff24728b --- /dev/null +++ b/spec/flipper/gates/block_group_spec.rb @@ -0,0 +1,84 @@ +RSpec.describe Flipper::Gates::BlockGroup do + let(:feature_name) { :search } + + subject do + described_class.new + end + + def context(set) + Flipper::FeatureCheckContext.new( + feature_name: feature_name, + values: Flipper::GateValues.new(block_groups: set), + actors: [Flipper::Types::Actor.new(Flipper::Actor.new('5'))] + ) + end + + describe '#blocks?' do + context 'with a matching registered group' do + before do + Flipper.register(:staff) { |actor| true } + end + + it 'returns true when actor matches blocked group' do + expect(subject.blocks?(context(Set[:staff]))).to be(true) + end + end + + context 'with a non-matching registered group' do + before do + Flipper.register(:staff) { |actor| false } + end + + it 'returns false when actor does not match blocked group' do + expect(subject.blocks?(context(Set[:staff]))).to be(false) + end + end + + it 'returns false when blocked set is empty' do + expect(subject.blocks?(context(Set.new))).to be(false) + end + + context 'with a group in adapter, but not registered' do + before do + Flipper.register(:staff) { |actor| true } + end + + it 'ignores unregistered group' do + expect(subject.blocks?(context(Set[:newbs]))).to be(false) + end + end + end + + describe '#open?' do + before do + Flipper.register(:staff) { |actor| true } + end + + it 'always returns false' do + expect(subject.open?(context(Set[:staff]))).to be(false) + end + end + + describe '#wrap' do + it 'returns group instance for symbol' do + group = Flipper.register(:admins) {} + expect(subject.wrap(:admins)).to eq(group) + end + + it 'returns group instance for group instance' do + group = Flipper.register(:admins) {} + expect(subject.wrap(group)).to eq(group) + end + end + + describe '#protects?' do + it 'returns true for group' do + group = Flipper.register(:admins) {} + expect(subject.protects?(group)).to be(true) + end + + it 'returns true for symbol' do + expect(subject.protects?(:admins)).to be(true) + end + end +end diff --git a/spec/flipper/ui/actions/block_actors_gate_spec.rb b/spec/flipper/ui/actions/block_actors_gate_spec.rb new file mode 100644 index 000000000..0a50d84a3 --- /dev/null +++ b/spec/flipper/ui/actions/block_actors_gate_spec.rb @@ -0,0 +1,135 @@ +RSpec.describe Flipper::UI::Actions::BlockActorsGate do + let(:token) do + if Rack::Protection::AuthenticityToken.respond_to?(:random_token) + Rack::Protection::AuthenticityToken.random_token + else + 'a' + end + end + let(:session) do + { :csrf => token, 'csrf' => token, '_csrf_token' => token } + end + + describe 'POST /features/:feature/block_actors' do + context 'blocking an actor' do + let(:value) { 'User;6' } + let(:multi_value) { 'User;5, User;7, User;9, User;12' } + + before do + post 'features/search/block_actors', + { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'adds item to blocked actors' do + expect(flipper[:search].block_actors_value).to include(value) + end + + it 'adds multiple items to blocked actors' do + post 'features/search/block_actors', + { 'value' => multi_value, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + + expect(flipper[:search].block_actors_value).to include('User;5') + expect(flipper[:search].block_actors_value).to include('User;7') + expect(flipper[:search].block_actors_value).to include('User;9') + expect(flipper[:search].block_actors_value).to include('User;12') + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search') + end + + context 'when feature name contains space' do + before do + post 'features/sp+ace/block_actors', + { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'adds item to blocked actors' do + expect(flipper["sp ace"].block_actors_value).to include('User;6') + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/sp+ace') + end + end + + context 'value contains whitespace' do + let(:value) { ' User;6 ' } + + it 'adds item without whitespace' do + expect(flipper[:search].block_actors_value).to include('User;6') + end + end + + context 'for an invalid actor value' do + context 'empty value' do + let(:value) { '' } + + it 'redirects back with error' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search/block_actors?error=%22%22+is+not+a+valid+actor+value.') + end + end + + context 'nil value' do + let(:value) { nil } + + it 'redirects back with error' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search/block_actors?error=%22%22+is+not+a+valid+actor+value.') + end + end + end + end + + context 'when a readonly adapter is configured' do + let(:value) { 'User;6' } + + before do + allow(flipper).to receive(:read_only?) { true } + end + + it 'does not allow an actor to be blocked' do + post 'features/search/block_actors', + { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + + expect(flipper[:search].block_actors_value).not_to include('User;6') + expect(last_response.body).to include("The UI is currently in read only mode.") + end + end + + context 'unblocking an actor' do + let(:value) { 'User;6' } + + before do + flipper[:search].block_actor Flipper::Actor.new(value) + post 'features/search/block_actors', + { 'value' => value, 'operation' => 'unblock', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'removes item from blocked actors' do + expect(flipper[:search].block_actors_value).not_to include(value) + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search') + end + + context 'value contains whitespace' do + let(:value) { ' User;6 ' } + + it 'removes item without whitespace' do + expect(flipper[:search].block_actors_value).not_to include('User;6') + end + end + end + end +end diff --git a/spec/flipper/ui/actions/block_groups_gate_spec.rb b/spec/flipper/ui/actions/block_groups_gate_spec.rb new file mode 100644 index 000000000..d97ddeff9 --- /dev/null +++ b/spec/flipper/ui/actions/block_groups_gate_spec.rb @@ -0,0 +1,124 @@ +RSpec.describe Flipper::UI::Actions::BlockGroupsGate do + let(:token) do + if Rack::Protection::AuthenticityToken.respond_to?(:random_token) + Rack::Protection::AuthenticityToken.random_token + else + 'a' + end + end + + let(:session) do + { :csrf => token, 'csrf' => token, '_csrf_token' => token } + end + + describe 'POST /features/:feature/block_groups' do + let(:group_name) { 'admins' } + + before do + Flipper.register(:admins, &:admin?) + end + + after do + Flipper.unregister_groups + end + + context 'blocking a group' do + before do + post 'features/search/block_groups', + { 'value' => group_name, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'adds item to blocked groups' do + expect(flipper[:search].block_groups_value).to include('admins') + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search') + end + + context 'feature name contains space' do + before do + post 'features/sp+ace/block_groups', + { 'value' => group_name, 'operation' => 'block', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'adds item to blocked groups' do + expect(flipper["sp ace"].block_groups_value).to include('admins') + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/sp+ace') + end + end + + context 'group name contains whitespace' do + let(:group_name) { ' admins ' } + + it 'adds item without whitespace' do + expect(flipper[:search].block_groups_value).to include('admins') + end + end + + context 'for an unregistered group' do + context 'unknown group name' do + let(:group_name) { 'not_here' } + + it 'redirects back with error' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22not_here%22+has+not+been+registered.') + end + end + + context 'empty group name' do + let(:group_name) { '' } + + it 'redirects back with error' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22%22+has+not+been+registered.') + end + end + + context 'nil group name' do + let(:group_name) { nil } + + it 'redirects back with error' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22%22+has+not+been+registered.') + end + end + end + end + + context 'unblocking a group' do + let(:group_name) { 'admins' } + + before do + flipper[:search].block_group :admins + post 'features/search/block_groups', + { 'value' => group_name, 'operation' => 'unblock', 'authenticity_token' => token }, + 'rack.session' => session + end + + it 'removes item from blocked groups' do + expect(flipper[:search].block_groups_value).not_to include('admins') + end + + it 'redirects back to feature' do + expect(last_response.status).to be(302) + expect(last_response.headers['location']).to eq('/features/search') + end + + context 'group name contains whitespace' do + let(:group_name) { ' admins ' } + + it 'removes item without whitespace' do + expect(flipper[:search].block_groups_value).not_to include('admins') + end + end + end + end +end diff --git a/spec/flipper/ui/actions/export_spec.rb b/spec/flipper/ui/actions/export_spec.rb index e59bef4bf..ce3f3d584 100644 --- a/spec/flipper/ui/actions/export_spec.rb +++ b/spec/flipper/ui/actions/export_spec.rb @@ -41,10 +41,10 @@ expect(last_response.headers['content-type']).to eq('application/json') expect(data['version']).to eq(1) expect(data['features']).to eq({ - "analytics" => {"boolean"=>nil, "expression"=>{"Equal"=>[{"Property"=>["plan"]}, "basic"]}, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil}, - "search"=> {"boolean"=>nil, "expression"=>nil, "groups"=>["admins", "employees"], "actors"=>["User;1", "User;100"], "percentage_of_actors"=>"10", "percentage_of_time"=>"15"}, - "plausible"=> {"boolean"=>"true", "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil}, - "google_analytics"=> {"boolean"=>nil, "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil}, + "analytics" => {"boolean"=>nil, "expression"=>{"Equal"=>[{"Property"=>["plan"]}, "basic"]}, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, + "search"=> {"boolean"=>nil, "expression"=>nil, "groups"=>["admins", "employees"], "actors"=>["User;1", "User;100"], "percentage_of_actors"=>"10", "percentage_of_time"=>"15", "block_actors"=>[], "block_groups"=>[]}, + "plausible"=> {"boolean"=>"true", "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, + "google_analytics"=> {"boolean"=>nil, "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, }) end end From 17996f3cef4507da181849bd9a1d381256458715 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sat, 7 Mar 2026 20:50:32 -0500 Subject: [PATCH 3/4] Rename block gates to deny gates Rename block_actor/block_group to deny_actor/deny_group throughout the codebase. The inverse operation is now "permit" instead of "unblock". This better communicates the intent of the deny list feature. Co-Authored-By: Claude Opus 4.6 --- lib/flipper.rb | 4 +- lib/flipper/adapter.rb | 14 +- lib/flipper/adapters/active_record.rb | 12 +- lib/flipper/adapters/http.rb | 8 +- lib/flipper/adapters/memory.rb | 11 +- lib/flipper/adapters/moneta.rb | 11 +- lib/flipper/adapters/mongo.rb | 18 +- lib/flipper/adapters/pstore.rb | 11 +- lib/flipper/adapters/redis.rb | 17 +- lib/flipper/adapters/rollout.rb | 4 +- lib/flipper/adapters/sequel.rb | 10 +- .../adapters/sync/feature_synchronizer.rb | 43 ++- lib/flipper/api/middleware.rb | 4 +- ...ock_actors_gate.rb => deny_actors_gate.rb} | 8 +- ...ock_groups_gate.rb => deny_groups_gate.rb} | 8 +- lib/flipper/cli.rb | 55 ++++ lib/flipper/dsl.rb | 16 +- lib/flipper/feature.rb | 80 ++---- lib/flipper/gate.rb | 10 +- lib/flipper/gate_values.rb | 12 +- .../gates/{block_actor.rb => deny_actor.rb} | 12 +- .../gates/{block_group.rb => deny_group.rb} | 12 +- lib/flipper/spec/shared_adapter_specs.rb | 56 ++-- lib/flipper/test/shared_adapter_test.rb | 56 ++-- ...ock_actors_gate.rb => deny_actors_gate.rb} | 14 +- ...ock_groups_gate.rb => deny_groups_gate.rb} | 14 +- lib/flipper/ui/decorators/feature.rb | 8 +- lib/flipper/ui/middleware.rb | 4 +- lib/flipper/ui/public/css/application.css | 20 ++ lib/flipper/ui/views/feature.erb | 252 ++++++++++-------- spec/flipper/adapter_spec.rb | 4 +- spec/flipper/adapters/read_only_spec.rb | 4 +- spec/flipper/adapters/rollout_spec.rb | 12 +- .../api/v1/actions/block_actors_gate_spec.rb | 113 -------- .../api/v1/actions/deny_actors_gate_spec.rb | 113 ++++++++ ..._gate_spec.rb => deny_groups_gate_spec.rb} | 74 ++--- spec/flipper/api/v1/actions/feature_spec.rb | 32 +-- spec/flipper/api/v1/actions/features_spec.rb | 28 +- spec/flipper/cli_spec.rb | 54 +++- spec/flipper/cloud_spec.rb | 24 +- spec/flipper/exporters/json/v1_spec.rb | 6 +- spec/flipper/feature_spec.rb | 8 +- ...block_actor_spec.rb => deny_actor_spec.rb} | 10 +- ...block_group_spec.rb => deny_group_spec.rb} | 10 +- ..._gate_spec.rb => deny_actors_gate_spec.rb} | 64 ++--- ..._gate_spec.rb => deny_groups_gate_spec.rb} | 44 +-- spec/flipper/ui/actions/export_spec.rb | 8 +- 47 files changed, 825 insertions(+), 587 deletions(-) rename lib/flipper/api/v1/actions/{block_actors_gate.rb => deny_actors_gate.rb} (83%) rename lib/flipper/api/v1/actions/{block_groups_gate.rb => deny_groups_gate.rb} (86%) rename lib/flipper/gates/{block_actor.rb => deny_actor.rb} (79%) rename lib/flipper/gates/{block_group.rb => deny_group.rb} (82%) rename lib/flipper/ui/actions/{block_actors_gate.rb => deny_actors_gate.rb} (73%) rename lib/flipper/ui/actions/{block_groups_gate.rb => deny_groups_gate.rb} (69%) delete mode 100644 spec/flipper/api/v1/actions/block_actors_gate_spec.rb create mode 100644 spec/flipper/api/v1/actions/deny_actors_gate_spec.rb rename spec/flipper/api/v1/actions/{block_groups_gate_spec.rb => deny_groups_gate_spec.rb} (53%) rename spec/flipper/gates/{block_actor_spec.rb => deny_actor_spec.rb} (82%) rename spec/flipper/gates/{block_group_spec.rb => deny_group_spec.rb} (86%) rename spec/flipper/ui/actions/{block_actors_gate_spec.rb => deny_actors_gate_spec.rb} (55%) rename spec/flipper/ui/actions/{block_groups_gate_spec.rb => deny_groups_gate_spec.rb} (64%) diff --git a/lib/flipper.rb b/lib/flipper.rb index 630a0d8d3..9537c1882 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -59,8 +59,8 @@ def instance=(flipper) :enabled?, :enable, :disable, :enable_expression, :disable_expression, :expression, :add_expression, :remove_expression, - :enable_actor, :disable_actor, :block_actor, :unblock_actor, - :enable_group, :disable_group, :block_group, :unblock_group, + :enable_actor, :disable_actor, :deny_actor, :permit_actor, + :enable_group, :disable_group, :deny_group, :permit_group, :enable_percentage_of_actors, :disable_percentage_of_actors, :enable_percentage_of_time, :disable_percentage_of_time, :features, :feature, :[], :preload, :preload_all, diff --git a/lib/flipper/adapter.rb b/lib/flipper/adapter.rb index c9c067eb1..6f46bd44d 100644 --- a/lib/flipper/adapter.rb +++ b/lib/flipper/adapter.rb @@ -15,11 +15,17 @@ def default_config expression: nil, percentage_of_actors: nil, percentage_of_time: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } end + # Gate keys for deny gates. Deny gate values are preserved when + # toggling the boolean gate. + def deny_gate_keys + [:deny_actors, :deny_groups].freeze + end + def from(source) return source if source.is_a?(Flipper::Adapter) source.adapter @@ -71,6 +77,10 @@ def default_config self.class.default_config end + def deny_gate_keys + self.class.deny_gate_keys + end + # Public: default name of the adapter def name @name ||= self.class.name.split('::').last.split(/(?=[A-Z])/).join('_').downcase.to_sym diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index 6733b46d4..6da82e2a3 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -162,7 +162,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) when :integer set(feature, gate, thing) when :json @@ -187,6 +187,14 @@ def unsupported_data_type(data_type) private + def clear_allow_gates(feature) + with_write_connection(@gate_class) do + @gate_class.where(feature_key: feature.key) + .where.not(key: deny_gate_keys.map(&:to_s)) + .destroy_all + end + end + def set(feature, gate, thing, options = {}) clear_feature = options.fetch(:clear, false) json_feature = options.fetch(:json, false) @@ -195,7 +203,7 @@ def set(feature, gate, thing, options = {}) with_write_connection(@gate_class) do @gate_class.transaction(requires_new: true) do - clear(feature) if clear_feature + clear_allow_gates(feature) if clear_feature delete(feature, gate) begin @gate_class.create! do |g| diff --git a/lib/flipper/adapters/http.rb b/lib/flipper/adapters/http.rb index ba97c5824..717e4c177 100644 --- a/lib/flipper/adapters/http.rb +++ b/lib/flipper/adapters/http.rb @@ -132,7 +132,7 @@ def remove(feature) def enable(feature, gate, thing) body = request_body_for_gate(gate, thing.value) - query_string = %i[groups block_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" + query_string = %i[groups deny_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" response = @client.post("/features/#{feature.key}/#{gate.key}#{query_string}", body) raise Error, response unless response.is_a?(Net::HTTPOK) true @@ -140,7 +140,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) body = request_body_for_gate(gate, thing.value) - query_string = %i[groups block_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" + query_string = %i[groups deny_groups].include?(gate.key) ? "?allow_unregistered_groups=true" : "" response = case gate.key when :percentage_of_actors, :percentage_of_time @client.post("/features/#{feature.key}/#{gate.key}#{query_string}", body) @@ -171,9 +171,9 @@ def request_body_for_gate(gate, value) data = case gate.key when :boolean {} - when :groups, :block_groups + when :groups, :deny_groups { name: value.to_s } - when :actors, :block_actors + when :actors, :deny_actors { flipper_id: value.to_s } when :percentage_of_actors, :percentage_of_time { percentage: value.to_s } diff --git a/lib/flipper/adapters/memory.rb b/lib/flipper/adapters/memory.rb index 67a312af9..5aa0e2deb 100644 --- a/lib/flipper/adapters/memory.rb +++ b/lib/flipper/adapters/memory.rb @@ -65,7 +65,7 @@ def enable(feature, gate, thing) case gate.data_type when :boolean - @source[feature.key] = default_config + clear_allow_gates(feature) @source[feature.key][gate.key] = thing.value.to_s when :integer @source[feature.key][gate.key] = thing.value.to_s @@ -88,7 +88,7 @@ def disable(feature, gate, thing) case gate.data_type when :boolean - @source[feature.key] = default_config + clear_allow_gates(feature) when :integer @source[feature.key][gate.key] = thing.value.to_s when :set @@ -122,6 +122,13 @@ def import(source) private + def clear_allow_gates(feature) + existing = @source[feature.key] || default_config + preserved = {} + deny_gate_keys.each { |k| preserved[k] = existing[k] } + @source[feature.key] = default_config.merge(preserved) + end + def reset @pid = Process.pid @lock&.unlock if @lock&.locked? diff --git a/lib/flipper/adapters/moneta.rb b/lib/flipper/adapters/moneta.rb index 2a1f9ca09..ad2046975 100644 --- a/lib/flipper/adapters/moneta.rb +++ b/lib/flipper/adapters/moneta.rb @@ -54,7 +54,7 @@ def get(feature) def enable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) result = get(feature) result[gate.key] = thing.value.to_s moneta[key(feature.key)] = result @@ -84,7 +84,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) when :integer result = get(feature) result[gate.key] = thing.value.to_s @@ -103,6 +103,13 @@ def disable(feature, gate, thing) private + def clear_allow_gates(feature) + existing = get(feature) + preserved = {} + deny_gate_keys.each { |k| preserved[k] = existing[k] } + moneta[key(feature.key)] = default_config.merge(preserved) + end + def key(feature_key) "#{FEATURES_KEY}/#{feature_key}" end diff --git a/lib/flipper/adapters/mongo.rb b/lib/flipper/adapters/mongo.rb index 598aebf71..30fc30d5f 100644 --- a/lib/flipper/adapters/mongo.rb +++ b/lib/flipper/adapters/mongo.rb @@ -66,7 +66,7 @@ def get_all(**kwargs) def enable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) update feature.key, '$set' => { gate.key.to_s => thing.value.to_s, } @@ -99,7 +99,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - delete feature.key + clear_allow_gates(feature) when :integer update feature.key, '$set' => { gate.key.to_s => thing.value.to_s, @@ -121,6 +121,20 @@ def disable(feature, gate, thing) private + def clear_allow_gates(feature) + doc = find(feature.key) + return if doc.empty? + + fields_to_unset = doc.keys.reject do |k| + k == '_id' || deny_gate_keys.any? { |pk| pk.to_s == k } + end + + if fields_to_unset.any? + unset_hash = fields_to_unset.each_with_object({}) { |f, h| h[f] = 1 } + update feature.key, '$unset' => unset_hash + end + end + def read_feature_keys find(@features_key).fetch('features') { Set.new }.to_set end diff --git a/lib/flipper/adapters/pstore.rb b/lib/flipper/adapters/pstore.rb index d9b844609..de98f45fb 100644 --- a/lib/flipper/adapters/pstore.rb +++ b/lib/flipper/adapters/pstore.rb @@ -78,7 +78,7 @@ def enable(feature, gate, thing) @store.transaction do case gate.data_type when :boolean - clear_gates(feature) + clear_allow_gates(feature) write key(feature, gate), thing.value.to_s when :integer write key(feature, gate), thing.value.to_s @@ -98,7 +98,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + @store.transaction { clear_allow_gates(feature) } when :integer @store.transaction do write key(feature, gate), thing.value.to_s @@ -136,6 +136,13 @@ def clear_gates(feature) end end + def clear_allow_gates(feature) + feature.gates.each do |gate| + next if gate.deny? + delete key(feature, gate) + end + end + def read_feature_keys set_members @features_key end diff --git a/lib/flipper/adapters/redis.rb b/lib/flipper/adapters/redis.rb index b196b42e7..a65e92f7f 100644 --- a/lib/flipper/adapters/redis.rb +++ b/lib/flipper/adapters/redis.rb @@ -92,7 +92,7 @@ def enable(feature, gate, thing) feature_key = key_for(feature.key) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } when :integer with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } @@ -118,7 +118,7 @@ def disable(feature, gate, thing) feature_key = key_for(feature.key) case gate.data_type when :boolean - with_connection { |conn| conn.del feature_key } + clear_allow_gates(feature) when :integer with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } when :set @@ -134,6 +134,19 @@ def disable(feature, gate, thing) private + def clear_allow_gates(feature) + feature_key = key_for(feature.key) + with_connection do |conn| + all_fields = conn.hkeys(feature_key) + fields_to_delete = all_fields.reject do |field| + deny_gate_keys.any? do |key| + field == key.to_s || field.start_with?("#{key}/") + end + end + conn.hdel(feature_key, *fields_to_delete) if fields_to_delete.any? + end + end + def redis_sadd_returns_boolean? @sadd_returns_boolean end diff --git a/lib/flipper/adapters/rollout.rb b/lib/flipper/adapters/rollout.rb index 12c58dac3..1649f9eeb 100644 --- a/lib/flipper/adapters/rollout.rb +++ b/lib/flipper/adapters/rollout.rb @@ -50,8 +50,8 @@ def get(feature) percentage_of_actors: percentage_of_actors, percentage_of_time: nil, expression: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } end diff --git a/lib/flipper/adapters/sequel.rb b/lib/flipper/adapters/sequel.rb index afdf5adb4..9fa6ad7ec 100644 --- a/lib/flipper/adapters/sequel.rb +++ b/lib/flipper/adapters/sequel.rb @@ -155,7 +155,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear(feature) + clear_allow_gates(feature) when :integer set(feature, gate, thing) when :json @@ -175,6 +175,12 @@ def unsupported_data_type(data_type) raise "#{data_type} is not supported by this adapter" end + def clear_allow_gates(feature) + @gate_class.where(feature_key: feature.key.to_s) + .exclude(key: deny_gate_keys.map(&:to_s)) + .delete + end + def set(feature, gate, thing, options = {}) clear_feature = options.fetch(:clear, false) json_feature = options.fetch(:json, false) @@ -182,7 +188,7 @@ def set(feature, gate, thing, options = {}) raise VALUE_TO_TEXT_WARNING if json_feature && value_not_text? @gate_class.db.transaction do - clear(feature) if clear_feature + clear_allow_gates(feature) if clear_feature delete(feature, gate) begin diff --git a/lib/flipper/adapters/sync/feature_synchronizer.rb b/lib/flipper/adapters/sync/feature_synchronizer.rb index 3176dbe6a..f54473d20 100644 --- a/lib/flipper/adapters/sync/feature_synchronizer.rb +++ b/lib/flipper/adapters/sync/feature_synchronizer.rb @@ -17,6 +17,8 @@ class FeatureSynchronizer :local_percentage_of_actors def_delegator :@local_gate_values, :percentage_of_time, :local_percentage_of_time + def_delegator :@local_gate_values, :deny_actors, :local_deny_actors + def_delegator :@local_gate_values, :deny_groups, :local_deny_groups def_delegator :@remote_gate_values, :expression, :remote_expression def_delegator :@remote_gate_values, :boolean, :remote_boolean @@ -26,6 +28,8 @@ class FeatureSynchronizer :remote_percentage_of_actors def_delegator :@remote_gate_values, :percentage_of_time, :remote_percentage_of_time + def_delegator :@remote_gate_values, :deny_actors, :remote_deny_actors + def_delegator :@remote_gate_values, :deny_groups, :remote_deny_groups def initialize(feature, local_gate_values, remote_gate_values) @feature = feature @@ -35,11 +39,16 @@ def initialize(feature, local_gate_values, remote_gate_values) def call if remote_disabled? - return if local_disabled? - @feature.disable + unless local_disabled? + @feature.disable + sync_deny_actors + sync_deny_groups + end + return elsif remote_boolean_enabled? - return if local_boolean_enabled? - @feature.enable + @feature.enable unless local_boolean_enabled? + sync_deny_actors + sync_deny_groups else @feature.disable if local_boolean_enabled? sync_groups @@ -47,6 +56,8 @@ def call sync_expression sync_percentage_of_actors sync_percentage_of_time + sync_deny_actors + sync_deny_groups end end @@ -86,6 +97,30 @@ def sync_groups end end + def sync_deny_actors + remote_deny_actors_added = remote_deny_actors - local_deny_actors + remote_deny_actors_added.each do |flipper_id| + @feature.deny_actor Actor.new(flipper_id) + end + + remote_deny_actors_removed = local_deny_actors - remote_deny_actors + remote_deny_actors_removed.each do |flipper_id| + @feature.permit_actor Actor.new(flipper_id) + end + end + + def sync_deny_groups + remote_deny_groups_added = remote_deny_groups - local_deny_groups + remote_deny_groups_added.each do |group_name| + @feature.deny_group group_name + end + + remote_deny_groups_removed = local_deny_groups - remote_deny_groups + remote_deny_groups_removed.each do |group_name| + @feature.permit_group group_name + end + end + def sync_percentage_of_actors return if local_percentage_of_actors == remote_percentage_of_actors diff --git a/lib/flipper/api/middleware.rb b/lib/flipper/api/middleware.rb index 0f274b9ee..73ed284ea 100644 --- a/lib/flipper/api/middleware.rb +++ b/lib/flipper/api/middleware.rb @@ -19,8 +19,8 @@ def initialize(app, options = {}) @action_collection.add Api::V1::Actions::PercentageOfActorsGate @action_collection.add Api::V1::Actions::ActorsGate @action_collection.add Api::V1::Actions::GroupsGate - @action_collection.add Api::V1::Actions::BlockActorsGate - @action_collection.add Api::V1::Actions::BlockGroupsGate + @action_collection.add Api::V1::Actions::DenyActorsGate + @action_collection.add Api::V1::Actions::DenyGroupsGate @action_collection.add Api::V1::Actions::BooleanGate @action_collection.add Api::V1::Actions::ClearFeature @action_collection.add Api::V1::Actions::Actors diff --git a/lib/flipper/api/v1/actions/block_actors_gate.rb b/lib/flipper/api/v1/actions/deny_actors_gate.rb similarity index 83% rename from lib/flipper/api/v1/actions/block_actors_gate.rb rename to lib/flipper/api/v1/actions/deny_actors_gate.rb index 0fbb6ce34..760ef24d5 100644 --- a/lib/flipper/api/v1/actions/block_actors_gate.rb +++ b/lib/flipper/api/v1/actions/deny_actors_gate.rb @@ -5,16 +5,16 @@ module Flipper module Api module V1 module Actions - class BlockActorsGate < Api::Action + class DenyActorsGate < Api::Action include FeatureNameFromRoute - route %r{\A/features/(?.*)/block_actors/?\Z} + route %r{\A/features/(?.*)/deny_actors/?\Z} def post ensure_valid_params feature = flipper[feature_name] actor = Actor.new(flipper_id) - feature.block_actor(actor) + feature.deny_actor(actor) decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end @@ -23,7 +23,7 @@ def delete ensure_valid_params feature = flipper[feature_name] actor = Actor.new(flipper_id) - feature.unblock_actor(actor) + feature.permit_actor(actor) decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end diff --git a/lib/flipper/api/v1/actions/block_groups_gate.rb b/lib/flipper/api/v1/actions/deny_groups_gate.rb similarity index 86% rename from lib/flipper/api/v1/actions/block_groups_gate.rb rename to lib/flipper/api/v1/actions/deny_groups_gate.rb index a915b82dc..acb0c5ad6 100644 --- a/lib/flipper/api/v1/actions/block_groups_gate.rb +++ b/lib/flipper/api/v1/actions/deny_groups_gate.rb @@ -5,15 +5,15 @@ module Flipper module Api module V1 module Actions - class BlockGroupsGate < Api::Action + class DenyGroupsGate < Api::Action include FeatureNameFromRoute - route %r{\A/features/(?.*)/block_groups/?\Z} + route %r{\A/features/(?.*)/deny_groups/?\Z} def post ensure_valid_params feature = flipper[feature_name] - feature.block_group(group_name) + feature.deny_group(group_name) decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end @@ -21,7 +21,7 @@ def post def delete ensure_valid_params feature = flipper[feature_name] - feature.unblock_group(group_name) + feature.permit_group(group_name) decorated_feature = Decorators::Feature.new(feature) json_response(decorated_feature.as_json, 200) end diff --git a/lib/flipper/cli.rb b/lib/flipper/cli.rb index 3204c2be5..aea004fe2 100644 --- a/lib/flipper/cli.rb +++ b/lib/flipper/cli.rb @@ -70,6 +70,42 @@ def initialize(stdout: $stdout, stderr: $stderr, shell: Bundler::Thor::Base.shel end end + { 'deny' => 'deny', 'permit' => 'permit' }.each do |command_name, method_prefix| + command command_name do |c| + c.banner = "Usage: #{c.program_name} [options] " + c.description = "#{command_name.capitalize} actors or groups for a feature" + + values = [] + + c.on('-a id', '--actor=id', "#{command_name} an actor") do |id| + values << Actor.new(id) + end + c.on('-g name', '--group=name', "#{command_name} a group") do |name| + values << Types::Group.new(name) + end + + c.action do |feature| + f = Flipper.feature(feature) + + if values.empty? + ui.error "#{command_name} requires at least one -a or -g option" + exit 1 + end + + values.each do |value| + case value + when Actor + f.send("#{method_prefix}_actor", value) + when Types::Group + f.send("#{method_prefix}_group", value) + end + end + + ui.info feature_details(f) + end + end + end + command 'list' do |c| c.description = "List defined features" c.action do @@ -228,6 +264,11 @@ def feature_summary(features) end.join(', ') end + deny_parts = [] + deny_parts << pluralize(feature.deny_actors_value.size, 'denied actor', 'denied actors') if feature.deny_actors_value.any? + deny_parts << pluralize(feature.deny_groups_value.size, 'denied group', 'denied groups') if feature.deny_groups_value.any? + summary += ", " + colorize(deny_parts.join(', '), [:RED]) if deny_parts.any? + colorize("%-#{padding}s" % feature.key, [:BOLD, :WHITE]) + " is #{summary}" end.join("\n") end @@ -261,6 +302,20 @@ def feature_details(feature) indent(lines.flatten.join("\n"), 2) end + deny_lines = [] + if feature.deny_actors_value.any? + deny_lines << pluralize(feature.deny_actors_value.size, 'denied actor', 'denied actors') + feature.deny_actors_value.each { |actor| deny_lines << " - #{actor}" } + end + if feature.deny_groups_value.any? + deny_lines << pluralize(feature.deny_groups_value.size, 'denied group', 'denied groups') + feature.deny_groups_value.each { |group| deny_lines << " - #{group}" } + end + + if deny_lines.any? + summary += "\n" + colorize(" deny list:", [:RED]) + "\n" + indent(deny_lines.join("\n"), 4) + end + "#{colorize(feature.key, [:BOLD, :WHITE])} is #{summary}" end diff --git a/lib/flipper/dsl.rb b/lib/flipper/dsl.rb index 84582f077..1b5d8e447 100644 --- a/lib/flipper/dsl.rb +++ b/lib/flipper/dsl.rb @@ -181,20 +181,20 @@ def disable_percentage_of_actors(name) feature(name).disable_percentage_of_actors end - def block_actor(name, actor) - feature(name).block_actor(actor) + def deny_actor(name, actor) + feature(name).deny_actor(actor) end - def block_group(name, group) - feature(name).block_group(group) + def deny_group(name, group) + feature(name).deny_group(group) end - def unblock_actor(name, actor) - feature(name).unblock_actor(actor) + def permit_actor(name, actor) + feature(name).permit_actor(actor) end - def unblock_group(name, group) - feature(name).unblock_group(group) + def permit_group(name, group) + feature(name).permit_group(group) end # Public: Add a feature. diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index 6f6566722..6e3dbdf37 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -48,10 +48,7 @@ def enable(thing = true) payload[:gate_name] = gate.name payload[:thing] = wrapped_thing - saved_blocks = preserve_block_values if gate.data_type == :boolean - result = adapter.enable self, gate, wrapped_thing - restore_block_values(saved_blocks) if saved_blocks - result + adapter.enable self, gate, wrapped_thing end end @@ -67,51 +64,48 @@ def disable(thing = false) payload[:gate_name] = gate.name payload[:thing] = wrapped_thing - saved_blocks = preserve_block_values if gate.data_type == :boolean - result = adapter.disable self, gate, wrapped_thing - restore_block_values(saved_blocks) if saved_blocks - result + adapter.disable self, gate, wrapped_thing end end - # Public: Block an actor from this feature. + # Public: Deny an actor from this feature. # # actor - a Flipper::Types::Actor instance or an object that responds # to flipper_id. # # Returns result of Adapter#enable. - def block_actor(actor) - block gate(:block_actor), Types::Actor.wrap(actor) + def deny_actor(actor) + deny gate(:deny_actor), Types::Actor.wrap(actor) end - # Public: Block a group from this feature. + # Public: Deny a group from this feature. # # group - a Flipper::Types::Group instance or a String or Symbol name of a # registered group. # # Returns result of Adapter#enable. - def block_group(group) - block gate(:block_group), Types::Group.wrap(group) + def deny_group(group) + deny gate(:deny_group), Types::Group.wrap(group) end - # Public: Unblock an actor from this feature. + # Public: Remove an actor from the deny list for this feature. # # actor - a Flipper::Types::Actor instance or an object that responds # to flipper_id. # # Returns result of Adapter#disable. - def unblock_actor(actor) - unblock gate(:block_actor), Types::Actor.wrap(actor) + def permit_actor(actor) + permit gate(:deny_actor), Types::Actor.wrap(actor) end - # Public: Unblock a group from this feature. + # Public: Remove a group from the deny list for this feature. # # group - a Flipper::Types::Group instance or a String or Symbol name of a # registered group. # # Returns result of Adapter#disable. - def unblock_group(group) - unblock gate(:block_group), Types::Group.wrap(group) + def permit_group(group) + permit gate(:deny_group), Types::Group.wrap(group) end # Public: Adds this feature. @@ -303,7 +297,7 @@ def disable_percentage_of_actors def state values = gate_values boolean = gate(:boolean) - non_boolean_gates = gates - [boolean] + non_boolean_gates = gates.reject { |gate| gate == boolean || gate.deny? } if values.boolean || values.percentage_of_time == 100 :on @@ -397,18 +391,18 @@ def percentage_of_time_value gate_values.percentage_of_time end - # Public: Get the adapter value for the block actors gate. + # Public: Get the adapter value for the deny actors gate. # # Returns Set of String flipper_id's. - def block_actors_value - gate_values.block_actors + def deny_actors_value + gate_values.deny_actors end - # Public: Get the adapter value for the block groups gate. + # Public: Get the adapter value for the deny groups gate. # # Returns Set of String group names. - def block_groups_value - gate_values.block_groups + def deny_groups_value + gate_values.deny_groups end # Public: Get the gates that have been enabled for the feature. @@ -476,8 +470,8 @@ def gates_hash percentage_of_actors: Gates::PercentageOfActors.new, percentage_of_time: Gates::PercentageOfTime.new, group: Gates::Group.new, - block_actor: Gates::BlockActor.new, - block_group: Gates::BlockGroup.new, + deny_actor: Gates::DenyActor.new, + deny_group: Gates::DenyGroup.new, }.freeze end @@ -500,8 +494,8 @@ def gate_for(actor) private - def block(gate, thing) - instrument(:block) do |payload| + def deny(gate, thing) + instrument(:deny) do |payload| adapter.add self payload[:gate_name] = gate.name payload[:thing] = thing @@ -509,8 +503,8 @@ def block(gate, thing) end end - def unblock(gate, thing) - instrument(:unblock) do |payload| + def permit(gate, thing) + instrument(:permit) do |payload| adapter.add self payload[:gate_name] = gate.name payload[:thing] = thing @@ -518,26 +512,6 @@ def unblock(gate, thing) end end - def preserve_block_values - values = gate_values - block_actors = values.block_actors - block_groups = values.block_groups - return if block_actors.empty? && block_groups.empty? - { block_actors: block_actors, block_groups: block_groups } - end - - def restore_block_values(saved) - block_actor_gate = gate(:block_actor) - saved[:block_actors].each do |flipper_id| - adapter.enable self, block_actor_gate, Types::Actor.wrap(Flipper::Actor.new(flipper_id)) - end - - block_group_gate = gate(:block_group) - saved[:block_groups].each do |group_name| - adapter.enable self, block_group_gate, Types::Group.wrap(group_name) - end - end - # Private: Instrument a feature operation. def instrument(operation, initial_payload = {}) @instrumenter.instrument(InstrumentationName, initial_payload) do |payload| diff --git a/lib/flipper/gate.rb b/lib/flipper/gate.rb index b8523fd14..a98db8cb6 100644 --- a/lib/flipper/gate.rb +++ b/lib/flipper/gate.rb @@ -34,6 +34,12 @@ def blocks?(context) false end + # Internal: Whether this is a deny gate. Deny gate values are preserved + # when boolean enable/disable clears all gate values. + def deny? + false + end + # Internal: Check if a gate is protects an actor. Implemented in subclass. # # Returns true if gate protects actor, false if not. @@ -65,5 +71,5 @@ def inspect require 'flipper/gates/percentage_of_actors' require 'flipper/gates/percentage_of_time' require 'flipper/gates/expression' -require 'flipper/gates/block_actor' -require 'flipper/gates/block_group' +require 'flipper/gates/deny_actor' +require 'flipper/gates/deny_group' diff --git a/lib/flipper/gate_values.rb b/lib/flipper/gate_values.rb index e8d78c67d..996456d77 100644 --- a/lib/flipper/gate_values.rb +++ b/lib/flipper/gate_values.rb @@ -9,8 +9,8 @@ class GateValues attr_reader :expression attr_reader :percentage_of_actors attr_reader :percentage_of_time - attr_reader :block_actors - attr_reader :block_groups + attr_reader :deny_actors + attr_reader :deny_groups def initialize(adapter_values) @boolean = Typecast.to_boolean(adapter_values[:boolean]) @@ -19,8 +19,8 @@ def initialize(adapter_values) @expression = adapter_values[:expression] @percentage_of_actors = Typecast.to_number(adapter_values[:percentage_of_actors]) @percentage_of_time = Typecast.to_number(adapter_values[:percentage_of_time]) - @block_actors = Typecast.to_set(adapter_values[:block_actors]) - @block_groups = Typecast.to_set(adapter_values[:block_groups]) + @deny_actors = Typecast.to_set(adapter_values[:deny_actors]) + @deny_groups = Typecast.to_set(adapter_values[:deny_groups]) end def eql?(other) @@ -31,8 +31,8 @@ def eql?(other) expression == other.expression && percentage_of_actors == other.percentage_of_actors && percentage_of_time == other.percentage_of_time && - block_actors == other.block_actors && - block_groups == other.block_groups + deny_actors == other.deny_actors && + deny_groups == other.deny_groups end alias_method :==, :eql? end diff --git a/lib/flipper/gates/block_actor.rb b/lib/flipper/gates/deny_actor.rb similarity index 79% rename from lib/flipper/gates/block_actor.rb rename to lib/flipper/gates/deny_actor.rb index 9d69edc20..ca5284cec 100644 --- a/lib/flipper/gates/block_actor.rb +++ b/lib/flipper/gates/deny_actor.rb @@ -1,14 +1,14 @@ module Flipper module Gates - class BlockActor < Gate + class DenyActor < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name - :block_actor + :deny_actor end # Internal: Name converted to value safe for adapter. def key - :block_actors + :deny_actors end def data_type @@ -27,10 +27,14 @@ def blocks?(context) return false unless context.actors? context.actors.any? do |actor| - context.values.block_actors.include?(actor.value) + context.values.deny_actors.include?(actor.value) end end + def deny? + true + end + def wrap(actor) Types::Actor.wrap(actor) end diff --git a/lib/flipper/gates/block_group.rb b/lib/flipper/gates/deny_group.rb similarity index 82% rename from lib/flipper/gates/block_group.rb rename to lib/flipper/gates/deny_group.rb index fc38d22e1..660ac249f 100644 --- a/lib/flipper/gates/block_group.rb +++ b/lib/flipper/gates/deny_group.rb @@ -1,14 +1,14 @@ module Flipper module Gates - class BlockGroup < Gate + class DenyGroup < Gate # Internal: The name of the gate. Used for instrumentation, etc. def name - :block_group + :deny_group end # Internal: Name converted to value safe for adapter. def key - :block_groups + :deny_groups end def data_type @@ -26,13 +26,17 @@ def open?(context) def blocks?(context) return false unless context.actors? - context.values.block_groups.any? do |name| + context.values.deny_groups.any? do |name| context.actors.any? do |actor| Flipper.group(name).match?(actor, context) end end end + def deny? + true + end + def wrap(thing) Types::Group.wrap(thing) end diff --git a/lib/flipper/spec/shared_adapter_specs.rb b/lib/flipper/spec/shared_adapter_specs.rb index 328d45bcc..cf584fc29 100644 --- a/lib/flipper/spec/shared_adapter_specs.rb +++ b/lib/flipper/spec/shared_adapter_specs.rb @@ -10,8 +10,8 @@ let(:actor_gate) { feature.gate(:actor) } let(:actors_gate) { feature.gate(:percentage_of_actors) } let(:time_gate) { feature.gate(:percentage_of_time) } - let(:block_actor_gate) { feature.gate(:block_actor) } - let(:block_group_gate) { feature.gate(:block_group) } + let(:deny_actor_gate) { feature.gate(:deny_actor) } + let(:deny_group_gate) { feature.gate(:deny_group) } before do Flipper.register(:admins) do |actor| @@ -334,63 +334,63 @@ expect(subject.get(feature)).to eq(subject.default_config.merge(boolean: "true")) end - it 'can enable, disable and get value for block actor gate' do + it 'can enable, disable and get value for deny actor gate' do actor22 = Flipper::Actor.new('22') actor_asdf = Flipper::Actor.new('asdf') - expect(subject.enable(feature, block_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) - expect(subject.enable(feature, block_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) + expect(subject.enable(feature, deny_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) + expect(subject.enable(feature, deny_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) result = subject.get(feature) - expect(result[:block_actors]).to eq(Set['22', 'asdf']) + expect(result[:deny_actors]).to eq(Set['22', 'asdf']) - expect(subject.disable(feature, block_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) + expect(subject.disable(feature, deny_actor_gate, Flipper::Types::Actor.new(actor_asdf))).to eq(true) result = subject.get(feature) - expect(result[:block_actors]).to eq(Set['22']) + expect(result[:deny_actors]).to eq(Set['22']) - expect(subject.disable(feature, block_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) + expect(subject.disable(feature, deny_actor_gate, Flipper::Types::Actor.new(actor22))).to eq(true) result = subject.get(feature) - expect(result[:block_actors]).to eq(Set.new) + expect(result[:deny_actors]).to eq(Set.new) end - it 'can enable, disable and get value for block group gate' do - expect(subject.enable(feature, block_group_gate, flipper.group(:admins))).to eq(true) - expect(subject.enable(feature, block_group_gate, flipper.group(:early_access))).to eq(true) + it 'can enable, disable and get value for deny group gate' do + expect(subject.enable(feature, deny_group_gate, flipper.group(:admins))).to eq(true) + expect(subject.enable(feature, deny_group_gate, flipper.group(:early_access))).to eq(true) result = subject.get(feature) - expect(result[:block_groups]).to eq(Set['admins', 'early_access']) + expect(result[:deny_groups]).to eq(Set['admins', 'early_access']) - expect(subject.disable(feature, block_group_gate, flipper.group(:early_access))).to eq(true) + expect(subject.disable(feature, deny_group_gate, flipper.group(:early_access))).to eq(true) result = subject.get(feature) - expect(result[:block_groups]).to eq(Set['admins']) + expect(result[:deny_groups]).to eq(Set['admins']) - expect(subject.disable(feature, block_group_gate, flipper.group(:admins))).to eq(true) + expect(subject.disable(feature, deny_group_gate, flipper.group(:admins))).to eq(true) result = subject.get(feature) - expect(result[:block_groups]).to eq(Set.new) + expect(result[:deny_groups]).to eq(Set.new) end - it 'preserves block actors and groups when feature is enabled' do + it 'preserves deny actors and groups when feature is enabled' do actor = Flipper::Actor.new('blocked_user') - feature.block_actor(actor) - feature.block_group(:admins) + feature.deny_actor(actor) + feature.deny_group(:admins) feature.enable - expect(feature.block_actors_value).to eq(Set['blocked_user']) - expect(feature.block_groups_value).to eq(Set['admins']) + expect(feature.deny_actors_value).to eq(Set['blocked_user']) + expect(feature.deny_groups_value).to eq(Set['admins']) expect(feature.enabled?(actor)).to be(false) end - it 'preserves block actors and groups when feature is disabled' do + it 'preserves deny actors and groups when feature is disabled' do actor = Flipper::Actor.new('blocked_user') feature.enable_actor(actor) - feature.block_actor(actor) - feature.block_group(:admins) + feature.deny_actor(actor) + feature.deny_group(:admins) feature.disable - expect(feature.block_actors_value).to eq(Set['blocked_user']) - expect(feature.block_groups_value).to eq(Set['admins']) + expect(feature.deny_actors_value).to eq(Set['blocked_user']) + expect(feature.deny_groups_value).to eq(Set['admins']) end it 'can import and export' do diff --git a/lib/flipper/test/shared_adapter_test.rb b/lib/flipper/test/shared_adapter_test.rb index 2edbef69e..a36736dad 100644 --- a/lib/flipper/test/shared_adapter_test.rb +++ b/lib/flipper/test/shared_adapter_test.rb @@ -11,8 +11,8 @@ def setup @actor_gate = @feature.gate(:actor) @actors_gate = @feature.gate(:percentage_of_actors) @time_gate = @feature.gate(:percentage_of_time) - @block_actor_gate = @feature.gate(:block_actor) - @block_group_gate = @feature.gate(:block_group) + @deny_actor_gate = @feature.gate(:deny_actor) + @deny_group_gate = @feature.gate(:deny_group) Flipper.register(:admins) do |actor| actor.respond_to?(:admin?) && actor.admin? @@ -331,63 +331,63 @@ def test_clears_other_gate_values_on_enable assert_equal @adapter.default_config.merge(boolean: "true"), @adapter.get(@feature) end - def test_can_enable_disable_and_get_value_for_block_actor_gate + def test_can_enable_disable_and_get_value_for_deny_actor_gate actor22 = Flipper::Actor.new('22') actor_asdf = Flipper::Actor.new('asdf') - assert_equal true, @adapter.enable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor22)) - assert_equal true, @adapter.enable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor_asdf)) + assert_equal true, @adapter.enable(@feature, @deny_actor_gate, Flipper::Types::Actor.new(actor22)) + assert_equal true, @adapter.enable(@feature, @deny_actor_gate, Flipper::Types::Actor.new(actor_asdf)) result = @adapter.get(@feature) - assert_equal Set['22', 'asdf'], result[:block_actors] + assert_equal Set['22', 'asdf'], result[:deny_actors] - assert_equal true, @adapter.disable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor_asdf)) + assert_equal true, @adapter.disable(@feature, @deny_actor_gate, Flipper::Types::Actor.new(actor_asdf)) result = @adapter.get(@feature) - assert_equal Set['22'], result[:block_actors] + assert_equal Set['22'], result[:deny_actors] - assert_equal true, @adapter.disable(@feature, @block_actor_gate, Flipper::Types::Actor.new(actor22)) + assert_equal true, @adapter.disable(@feature, @deny_actor_gate, Flipper::Types::Actor.new(actor22)) result = @adapter.get(@feature) - assert_equal Set.new, result[:block_actors] + assert_equal Set.new, result[:deny_actors] end - def test_can_enable_disable_and_get_value_for_block_group_gate - assert_equal true, @adapter.enable(@feature, @block_group_gate, @flipper.group(:admins)) - assert_equal true, @adapter.enable(@feature, @block_group_gate, @flipper.group(:early_access)) + def test_can_enable_disable_and_get_value_for_deny_group_gate + assert_equal true, @adapter.enable(@feature, @deny_group_gate, @flipper.group(:admins)) + assert_equal true, @adapter.enable(@feature, @deny_group_gate, @flipper.group(:early_access)) result = @adapter.get(@feature) - assert_equal Set['admins', 'early_access'], result[:block_groups] + assert_equal Set['admins', 'early_access'], result[:deny_groups] - assert_equal true, @adapter.disable(@feature, @block_group_gate, @flipper.group(:early_access)) + assert_equal true, @adapter.disable(@feature, @deny_group_gate, @flipper.group(:early_access)) result = @adapter.get(@feature) - assert_equal Set['admins'], result[:block_groups] + assert_equal Set['admins'], result[:deny_groups] - assert_equal true, @adapter.disable(@feature, @block_group_gate, @flipper.group(:admins)) + assert_equal true, @adapter.disable(@feature, @deny_group_gate, @flipper.group(:admins)) result = @adapter.get(@feature) - assert_equal Set.new, result[:block_groups] + assert_equal Set.new, result[:deny_groups] end - def test_preserves_block_actors_and_groups_when_feature_is_enabled + def test_preserves_deny_actors_and_groups_when_feature_is_enabled actor = Flipper::Actor.new('blocked_user') - @feature.block_actor(actor) - @feature.block_group(:admins) + @feature.deny_actor(actor) + @feature.deny_group(:admins) @feature.enable - assert_equal Set['blocked_user'], @feature.block_actors_value - assert_equal Set['admins'], @feature.block_groups_value + assert_equal Set['blocked_user'], @feature.deny_actors_value + assert_equal Set['admins'], @feature.deny_groups_value refute @feature.enabled?(actor) end - def test_preserves_block_actors_and_groups_when_feature_is_disabled + def test_preserves_deny_actors_and_groups_when_feature_is_disabled actor = Flipper::Actor.new('blocked_user') @feature.enable_actor(actor) - @feature.block_actor(actor) - @feature.block_group(:admins) + @feature.deny_actor(actor) + @feature.deny_group(:admins) @feature.disable - assert_equal Set['blocked_user'], @feature.block_actors_value - assert_equal Set['admins'], @feature.block_groups_value + assert_equal Set['blocked_user'], @feature.deny_actors_value + assert_equal Set['admins'], @feature.deny_groups_value end def test_can_import_and_export diff --git a/lib/flipper/ui/actions/block_actors_gate.rb b/lib/flipper/ui/actions/deny_actors_gate.rb similarity index 73% rename from lib/flipper/ui/actions/block_actors_gate.rb rename to lib/flipper/ui/actions/deny_actors_gate.rb index 6bcd727fd..6cba86686 100644 --- a/lib/flipper/ui/actions/block_actors_gate.rb +++ b/lib/flipper/ui/actions/deny_actors_gate.rb @@ -5,10 +5,10 @@ module Flipper module UI module Actions - class BlockActorsGate < UI::Action + class DenyActorsGate < UI::Action include FeatureNameFromRoute - route %r{\A/features/(?.*)/block_actors/?\Z} + route %r{\A/features/(?.*)/deny_actors/?\Z} def post render_read_only if read_only? @@ -19,17 +19,17 @@ def post if values.empty? error = "#{value.inspect} is not a valid actor value." - redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/block_actors?error=#{Flipper::UI::Util.escape error}") + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/deny_actors?error=#{Flipper::UI::Util.escape error}") end values.each do |value| actor = Flipper::Actor.new(value) case params['operation'] - when 'block' - feature.block_actor actor - when 'unblock' - feature.unblock_actor actor + when 'deny' + feature.deny_actor actor + when 'permit' + feature.permit_actor actor end end diff --git a/lib/flipper/ui/actions/block_groups_gate.rb b/lib/flipper/ui/actions/deny_groups_gate.rb similarity index 69% rename from lib/flipper/ui/actions/block_groups_gate.rb rename to lib/flipper/ui/actions/deny_groups_gate.rb index 85a88d810..bfa7c333e 100644 --- a/lib/flipper/ui/actions/block_groups_gate.rb +++ b/lib/flipper/ui/actions/deny_groups_gate.rb @@ -4,10 +4,10 @@ module Flipper module UI module Actions - class BlockGroupsGate < UI::Action + class DenyGroupsGate < UI::Action include FeatureNameFromRoute - route %r{\A/features/(?.*)/block_groups/?\Z} + route %r{\A/features/(?.*)/deny_groups/?\Z} def post render_read_only if read_only? @@ -17,16 +17,16 @@ def post if Flipper.group_exists?(value) case params['operation'] - when 'block' - feature.block_group value - when 'unblock' - feature.unblock_group value + when 'deny' + feature.deny_group value + when 'permit' + feature.permit_group value end redirect_to("/features/#{Flipper::UI::Util.escape feature.key}") else error = "The group named #{value.inspect} has not been registered." - redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/block_groups?error=#{Flipper::UI::Util.escape error}") + redirect_to("/features/#{Flipper::UI::Util.escape feature.key}/deny_groups?error=#{Flipper::UI::Util.escape error}") end end end diff --git a/lib/flipper/ui/decorators/feature.rb b/lib/flipper/ui/decorators/feature.rb index a9c287fe0..262837e96 100644 --- a/lib/flipper/ui/decorators/feature.rb +++ b/lib/flipper/ui/decorators/feature.rb @@ -58,12 +58,12 @@ def gates_in_words end end - if feature.block_actors_value.count > 0 - statuses << %Q() + Util.pluralize(feature.block_actors_value.count, 'blocked actor', 'blocked actors') + "" + if feature.deny_actors_value.count > 0 + statuses << %Q() + Util.pluralize(feature.deny_actors_value.count, 'denied actor', 'denied actors') + "" end - if feature.block_groups_value.count > 0 - statuses << %Q() + Util.pluralize(feature.block_groups_value.count, 'blocked group', 'blocked groups') + "" + if feature.deny_groups_value.count > 0 + statuses << %Q() + Util.pluralize(feature.deny_groups_value.count, 'denied group', 'denied groups') + "" end Util.to_sentence(statuses) diff --git a/lib/flipper/ui/middleware.rb b/lib/flipper/ui/middleware.rb index 8e3a60de7..38e477cfa 100644 --- a/lib/flipper/ui/middleware.rb +++ b/lib/flipper/ui/middleware.rb @@ -20,8 +20,8 @@ def initialize(app, options = {}) @action_collection.add UI::Actions::AddFeature @action_collection.add UI::Actions::ActorsGate @action_collection.add UI::Actions::GroupsGate - @action_collection.add UI::Actions::BlockActorsGate - @action_collection.add UI::Actions::BlockGroupsGate + @action_collection.add UI::Actions::DenyActorsGate + @action_collection.add UI::Actions::DenyGroupsGate @action_collection.add UI::Actions::BooleanGate @action_collection.add UI::Actions::PercentageOfTimeGate @action_collection.add UI::Actions::PercentageOfActorsGate diff --git a/lib/flipper/ui/public/css/application.css b/lib/flipper/ui/public/css/application.css index 64f7db5b2..06f1db111 100644 --- a/lib/flipper/ui/public/css/application.css +++ b/lib/flipper/ui/public/css/application.css @@ -36,3 +36,23 @@ html { border-radius: 50%; display: inline-block; } + +.deny-list-chevron { + display: inline-block; + font-size: 0.65rem; + color: #999; + transition: transform 0.2s ease; +} + +[aria-expanded="true"] .deny-list-chevron { + transform: rotate(90deg); +} + +.deny-list-toggle { + cursor: pointer; + user-select: none; +} + +.deny-list-toggle:hover { + background: rgba(0, 0, 0, 0.02); +} diff --git a/lib/flipper/ui/views/feature.erb b/lib/flipper/ui/views/feature.erb index f540addc9..7f762b42f 100644 --- a/lib/flipper/ui/views/feature.erb +++ b/lib/flipper/ui/views/feature.erb @@ -298,146 +298,162 @@
+<% + deny_list_empty = @feature.deny_actors_value.empty? && @feature.deny_groups_value.empty? + deny_list_count = @feature.deny_actors_value.count + @feature.deny_groups_value.count +%>
-
-

Deny List

-

Blocked actors and groups are always denied access, even when the feature is fully enabled. These rules are not affected by enabling or disabling the feature.

-
- - <%# Blocked Actors Info and Form %> -
-
-
-
- "> - <% if @feature.block_actors_value.count > 0 %> - <%= Util.pluralize @feature.block_actors_value.count, 'blocked actor', 'blocked actors' %> - <% else %> - No blocked actors - <% end %> - -
-
- <% if write_allowed? %> -
- -
-
-
- <%== csrf_input_tag %> - -
- -
-
- -
-
-
-
- -
+
+
+ ▶ + Deny List + <% unless deny_list_empty %> + <%= deny_list_count %> <% end %>
- <%# Blocked Actors list %> - <% @feature.block_actors_value.each do |item| %> -
-
-
+
+ <%# Denied Actors Info and Form %> +
+
+
- <% if Flipper::UI::Util.present?(@feature.actor_names[item]) %> - <%== Sanitize.fragment("#{@feature.actor_names[item]} (#{item})", Sanitize::Config::BASIC) %> - <% else %> - <%= item %> - <% end %> + "> + <% if @feature.deny_actors_value.count > 0 %> + <%= Util.pluralize @feature.deny_actors_value.count, 'denied actor', 'denied actors' %> + <% else %> + No denied actors + <% end %> +
-
- <% if write_allowed? %> -
+ <% if write_allowed? %> +
+ +
+
+ <%== csrf_input_tag %> - - - + +
+ +
+
+ +
- <% end %> -
+
+
+ +
+ <% end %>
- <% end %> - <%# Blocked Groups Info and Form %> -
-
-
-
- "> - <% if @feature.block_groups_value.count > 0 %> - <%= Util.pluralize @feature.block_groups_value.count, 'blocked group', 'blocked groups' %> - <% else %> - No blocked groups + <%# Denied Actors list %> + <% @feature.deny_actors_value.each do |item| %> +
+
+
+
+ <% if Flipper::UI::Util.present?(@feature.actor_names[item]) %> + <%== Sanitize.fragment("#{@feature.actor_names[item]} (#{item})", Sanitize::Config::BASIC) %> + <% else %> + <%= item %> + <% end %> +
+
+
+ <% if write_allowed? %> +
+ <%== csrf_input_tag %> + + + +
<% end %> - -
-
- <% if write_allowed? %> -
- -
-
- <% if Flipper.groups.empty? %> - No groups available. - <% else %> -
- <%== csrf_input_tag %> - -
- -
-
-
- <% end %> +
-
- +
+ <% end %> + + <%# Denied Groups Info and Form %> +
+
+
+
+ "> + <% if @feature.deny_groups_value.count > 0 %> + <%= Util.pluralize @feature.deny_groups_value.count, 'denied group', 'denied groups' %> + <% else %> + No denied groups + <% end %> + +
- <% end %> + <% if write_allowed? %> +
+ +
+
+ <% deniable_groups = Flipper.groups.reject { |g| @feature.deny_groups_value.include?(g.name.to_s) } %> + <% if deniable_groups.empty? %> + All groups denied. + <% else %> +
+ <%== csrf_input_tag %> + +
+ +
+
+
+ <% end %> +
+
+ +
+ <% end %> +
-
- <%# Blocked Groups list %> - <% @feature.block_groups_value.each do |item| %> -
-
-
-
<%= item %>
-
-
- <% if write_allowed? %> -
- <%== csrf_input_tag %> - - - -
- <% end %> + <%# Denied Groups list %> + <% @feature.deny_groups_value.each do |item| %> +
+
+
+
<%= item %>
+
+
+ <% if write_allowed? %> +
+ <%== csrf_input_tag %> + + + +
+ <% end %> +
-
- <% end %> + <% end %> +
diff --git a/spec/flipper/adapter_spec.rb b/spec/flipper/adapter_spec.rb index 38f060b4e..8a5e891ca 100644 --- a/spec/flipper/adapter_spec.rb +++ b/spec/flipper/adapter_spec.rb @@ -9,8 +9,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } end diff --git a/spec/flipper/adapters/read_only_spec.rb b/spec/flipper/adapters/read_only_spec.rb index 0648b7245..af96061dd 100644 --- a/spec/flipper/adapters/read_only_spec.rb +++ b/spec/flipper/adapters/read_only_spec.rb @@ -63,8 +63,8 @@ }, percentage_of_actors: '25', percentage_of_time: '45', - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, }) end diff --git a/spec/flipper/adapters/rollout_spec.rb b/spec/flipper/adapters/rollout_spec.rb index a1b27c48e..277440495 100644 --- a/spec/flipper/adapters/rollout_spec.rb +++ b/spec/flipper/adapters/rollout_spec.rb @@ -36,8 +36,8 @@ expression: nil, percentage_of_actors: 20.0, percentage_of_time: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end @@ -52,8 +52,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end @@ -70,8 +70,8 @@ expression: nil, percentage_of_actors: nil, percentage_of_time: nil, - block_actors: Set.new, - block_groups: Set.new, + deny_actors: Set.new, + deny_groups: Set.new, } expect(source_adapter.get(feature)).to eq(expected) end diff --git a/spec/flipper/api/v1/actions/block_actors_gate_spec.rb b/spec/flipper/api/v1/actions/block_actors_gate_spec.rb deleted file mode 100644 index 1f07afadf..000000000 --- a/spec/flipper/api/v1/actions/block_actors_gate_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -RSpec.describe Flipper::Api::V1::Actions::BlockActorsGate do - let(:app) { build_api(flipper) } - let(:actor) { Flipper::Actor.new('1') } - - describe 'block' do - before do - post '/features/my_feature/block_actors', flipper_id: actor.flipper_id - end - - it 'blocks actor for feature' do - expect(last_response.status).to eq(200) - expect(flipper[:my_feature].block_actors_value).to include('1') - end - - it 'returns decorated feature with block_actors gate' do - gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } - expect(gate['value']).to eq(['1']) - end - end - - describe 'unblock' do - before do - flipper[:my_feature].block_actor(actor) - delete '/features/my_feature/block_actors', flipper_id: actor.flipper_id - end - - it 'unblocks actor for feature' do - expect(last_response.status).to eq(200) - expect(flipper[:my_feature].block_actors_value).to be_empty - end - - it 'returns decorated feature with empty block_actors gate' do - gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } - expect(gate['value']).to be_empty - end - end - - describe 'block with slash in feature name' do - before do - post '/features/my/feature/block_actors', flipper_id: actor.flipper_id - end - - it 'blocks actor for feature' do - expect(last_response.status).to eq(200) - expect(flipper["my/feature"].block_actors_value).to include('1') - end - - it 'returns decorated feature with block_actors gate' do - gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } - expect(gate['value']).to eq(['1']) - end - end - - describe 'block with space in feature name' do - before do - post '/features/sp%20ace/block_actors', flipper_id: actor.flipper_id - end - - it 'blocks actor for feature' do - expect(last_response.status).to eq(200) - expect(flipper["sp ace"].block_actors_value).to include('1') - end - - it 'returns decorated feature with block_actors gate' do - gate = json_response['gates'].find { |gate| gate['key'] == 'block_actors' } - expect(gate['value']).to eq(['1']) - end - end - - describe 'block missing flipper_id parameter' do - before do - post '/features/my_feature/block_actors' - end - - it 'returns correct error response' do - expect(last_response.status).to eq(422) - expect(json_response).to eq(api_flipper_id_is_missing_response) - end - end - - describe 'unblock missing flipper_id parameter' do - before do - delete '/features/my_feature/block_actors' - end - - it 'returns correct error response' do - expect(last_response.status).to eq(422) - expect(json_response).to eq(api_flipper_id_is_missing_response) - end - end - - describe 'block nil flipper_id parameter' do - before do - post '/features/my_feature/block_actors', flipper_id: nil - end - - it 'returns correct error response' do - expect(last_response.status).to eq(422) - expect(json_response).to eq(api_flipper_id_is_missing_response) - end - end - - describe 'unblock nil flipper_id parameter' do - before do - delete '/features/my_feature/block_actors', flipper_id: nil - end - - it 'returns correct error response' do - expect(last_response.status).to eq(422) - expect(json_response).to eq(api_flipper_id_is_missing_response) - end - end -end diff --git a/spec/flipper/api/v1/actions/deny_actors_gate_spec.rb b/spec/flipper/api/v1/actions/deny_actors_gate_spec.rb new file mode 100644 index 000000000..ecdaa14ad --- /dev/null +++ b/spec/flipper/api/v1/actions/deny_actors_gate_spec.rb @@ -0,0 +1,113 @@ +RSpec.describe Flipper::Api::V1::Actions::DenyActorsGate do + let(:app) { build_api(flipper) } + let(:actor) { Flipper::Actor.new('1') } + + describe 'deny' do + before do + post '/features/my_feature/deny_actors', flipper_id: actor.flipper_id + end + + it 'denies actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].deny_actors_value).to include('1') + end + + it 'returns decorated feature with deny_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'deny_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'permit' do + before do + flipper[:my_feature].deny_actor(actor) + delete '/features/my_feature/deny_actors', flipper_id: actor.flipper_id + end + + it 'undenies actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper[:my_feature].deny_actors_value).to be_empty + end + + it 'returns decorated feature with empty deny_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'deny_actors' } + expect(gate['value']).to be_empty + end + end + + describe 'deny with slash in feature name' do + before do + post '/features/my/feature/deny_actors', flipper_id: actor.flipper_id + end + + it 'denies actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper["my/feature"].deny_actors_value).to include('1') + end + + it 'returns decorated feature with deny_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'deny_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'deny with space in feature name' do + before do + post '/features/sp%20ace/deny_actors', flipper_id: actor.flipper_id + end + + it 'denies actor for feature' do + expect(last_response.status).to eq(200) + expect(flipper["sp ace"].deny_actors_value).to include('1') + end + + it 'returns decorated feature with deny_actors gate' do + gate = json_response['gates'].find { |gate| gate['key'] == 'deny_actors' } + expect(gate['value']).to eq(['1']) + end + end + + describe 'deny missing flipper_id parameter' do + before do + post '/features/my_feature/deny_actors' + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'permit missing flipper_id parameter' do + before do + delete '/features/my_feature/deny_actors' + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'deny nil flipper_id parameter' do + before do + post '/features/my_feature/deny_actors', flipper_id: nil + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end + + describe 'permit nil flipper_id parameter' do + before do + delete '/features/my_feature/deny_actors', flipper_id: nil + end + + it 'returns correct error response' do + expect(last_response.status).to eq(422) + expect(json_response).to eq(api_flipper_id_is_missing_response) + end + end +end diff --git a/spec/flipper/api/v1/actions/block_groups_gate_spec.rb b/spec/flipper/api/v1/actions/deny_groups_gate_spec.rb similarity index 53% rename from spec/flipper/api/v1/actions/block_groups_gate_spec.rb rename to spec/flipper/api/v1/actions/deny_groups_gate_spec.rb index ca00e4aa1..e28de728b 100644 --- a/spec/flipper/api/v1/actions/block_groups_gate_spec.rb +++ b/spec/flipper/api/v1/actions/deny_groups_gate_spec.rb @@ -1,47 +1,47 @@ -RSpec.describe Flipper::Api::V1::Actions::BlockGroupsGate do +RSpec.describe Flipper::Api::V1::Actions::DenyGroupsGate do let(:app) { build_api(flipper) } - describe 'block' do + describe 'deny' do before do Flipper.register(:admins) do |actor| actor.respond_to?(:admin?) && actor.admin? end - post '/features/my_feature/block_groups', name: 'admins' + post '/features/my_feature/deny_groups', name: 'admins' end - it 'blocks group for feature' do + it 'denies group for feature' do expect(last_response.status).to eq(200) - expect(flipper[:my_feature].block_groups_value).to include('admins') + expect(flipper[:my_feature].deny_groups_value).to include('admins') end - it 'returns decorated feature with block_groups gate' do - group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + it 'returns decorated feature with deny_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'deny_groups' } expect(group_gate['value']).to eq(['admins']) end end - describe 'block feature with slash in name' do + describe 'deny feature with slash in name' do before do Flipper.register(:admins) do |actor| actor.respond_to?(:admin?) && actor.admin? end - post '/features/my/feature/block_groups', name: 'admins' + post '/features/my/feature/deny_groups', name: 'admins' end - it 'blocks group for feature' do + it 'denies group for feature' do expect(last_response.status).to eq(200) - expect(flipper["my/feature"].block_groups_value).to include('admins') + expect(flipper["my/feature"].deny_groups_value).to include('admins') end - it 'returns decorated feature with block_groups gate' do - group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + it 'returns decorated feature with deny_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'deny_groups' } expect(group_gate['value']).to eq(['admins']) end end - describe 'block without name params' do + describe 'deny without name params' do before do - post '/features/my_feature/block_groups' + post '/features/my_feature/deny_groups' end it 'returns correct status code' do @@ -58,29 +58,29 @@ end end - describe 'unblock' do + describe 'permit' do before do Flipper.register(:admins) do |actor| actor.respond_to?(:admin?) && actor.admin? end - flipper[:my_feature].block_group(:admins) - delete '/features/my_feature/block_groups', name: 'admins' + flipper[:my_feature].deny_group(:admins) + delete '/features/my_feature/deny_groups', name: 'admins' end - it 'unblocks group for feature' do + it 'undenies group for feature' do expect(last_response.status).to eq(200) - expect(flipper[:my_feature].block_groups_value).to be_empty + expect(flipper[:my_feature].deny_groups_value).to be_empty end - it 'returns decorated feature with empty block_groups gate' do - group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + it 'returns decorated feature with empty deny_groups gate' do + group_gate = json_response['gates'].find { |m| m['key'] == 'deny_groups' } expect(group_gate['value']).to eq([]) end end - describe 'unblock for group not registered' do + describe 'permit for group not registered' do before do - delete '/features/my_feature/block_groups', name: 'admins' + delete '/features/my_feature/deny_groups', name: 'admins' end it '404s with correct error response when group not registered' do @@ -94,44 +94,44 @@ end end - describe 'block for group not registered when allow_unregistered_groups is true' do + describe 'deny for group not registered when allow_unregistered_groups is true' do before do Flipper.unregister_groups - post '/features/my_feature/block_groups', name: 'admins', allow_unregistered_groups: 'true' + post '/features/my_feature/deny_groups', name: 'admins', allow_unregistered_groups: 'true' end it 'responds successfully' do expect(last_response.status).to eq(200) end - it 'returns decorated feature with group in block_groups set' do - group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + it 'returns decorated feature with group in deny_groups set' do + group_gate = json_response['gates'].find { |m| m['key'] == 'deny_groups' } expect(group_gate['value']).to eq(['admins']) end - it 'blocks group' do - expect(flipper[:my_feature].block_groups_value).to eq(Set["admins"]) + it 'denies group' do + expect(flipper[:my_feature].deny_groups_value).to eq(Set["admins"]) end end - describe 'unblock for group not registered when allow_unregistered_groups is true' do + describe 'permit for group not registered when allow_unregistered_groups is true' do before do Flipper.unregister_groups - flipper[:my_feature].block_group(:admins) - delete '/features/my_feature/block_groups', name: 'admins', allow_unregistered_groups: 'true' + flipper[:my_feature].deny_group(:admins) + delete '/features/my_feature/deny_groups', name: 'admins', allow_unregistered_groups: 'true' end it 'responds successfully' do expect(last_response.status).to eq(200) end - it 'returns decorated feature with group not in block_groups set' do - group_gate = json_response['gates'].find { |m| m['key'] == 'block_groups' } + it 'returns decorated feature with group not in deny_groups set' do + group_gate = json_response['gates'].find { |m| m['key'] == 'deny_groups' } expect(group_gate['value']).to eq([]) end - it 'unblocks group' do - expect(flipper[:my_feature].block_groups_value).to be_empty + it 'undenies group' do + expect(flipper[:my_feature].deny_groups_value).to be_empty end end end diff --git a/spec/flipper/api/v1/actions/feature_spec.rb b/spec/flipper/api/v1/actions/feature_spec.rb index 4e64bc7d6..b85544b10 100644 --- a/spec/flipper/api/v1/actions/feature_spec.rb +++ b/spec/flipper/api/v1/actions/feature_spec.rb @@ -46,13 +46,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], @@ -105,13 +105,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], @@ -180,13 +180,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], @@ -239,13 +239,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], diff --git a/spec/flipper/api/v1/actions/features_spec.rb b/spec/flipper/api/v1/actions/features_spec.rb index 3e6e8861e..16d13a7e1 100644 --- a/spec/flipper/api/v1/actions/features_spec.rb +++ b/spec/flipper/api/v1/actions/features_spec.rb @@ -50,13 +50,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], @@ -80,8 +80,8 @@ {'key' => 'percentage_of_actors', 'value' => nil}, { 'key' => 'percentage_of_time', 'value' => nil}, { 'key' => 'groups', 'value' => []}, - { 'key' => 'block_actors', 'value' => []}, - { 'key' => 'block_groups', 'value' => []}, + { 'key' => 'deny_actors', 'value' => []}, + { 'key' => 'deny_groups', 'value' => []}, ], }, ], @@ -183,13 +183,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], @@ -249,13 +249,13 @@ 'value' => [], }, { - 'key' => 'block_actors', - 'name' => 'block_actor', + 'key' => 'deny_actors', + 'name' => 'deny_actor', 'value' => [], }, { - 'key' => 'block_groups', - 'name' => 'block_group', + 'key' => 'deny_groups', + 'name' => 'deny_group', 'value' => [], }, ], diff --git a/spec/flipper/cli_spec.rb b/spec/flipper/cli_spec.rb index df35253d6..44f86eedf 100644 --- a/spec/flipper/cli_spec.rb +++ b/spec/flipper/cli_spec.rb @@ -111,6 +111,48 @@ end end + describe "deny" do + describe "-a User;1 feature" do + it do + expect(subject).to have_attributes(status: 0, stdout: /feature.*deny.*User;1/m) + expect(Flipper.feature('feature').deny_actors_value).to include("User;1") + end + end + + describe "-g admins feature" do + it do + expect(subject).to have_attributes(status: 0, stdout: /feature.*deny.*admins/m) + expect(Flipper.feature('feature').deny_groups_value).to include("admins") + end + end + + describe "feature" do + it "errors without -a or -g" do + expect(subject).to have_attributes(status: 1, stderr: /requires at least one -a or -g/) + end + end + end + + describe "permit" do + describe "-a User;1 feature" do + before { Flipper.deny_actor(:feature, Flipper::Actor.new("User;1")) } + + it do + expect(subject).to have_attributes(status: 0) + expect(Flipper.feature('feature').deny_actors_value).not_to include("User;1") + end + end + + describe "-g admins feature" do + before { Flipper.deny_group(:feature, :admins) } + + it do + expect(subject).to have_attributes(status: 0) + expect(Flipper.feature('feature').deny_groups_value).not_to include("admins") + end + end + end + describe "list" do before do Flipper.enable :foo @@ -128,7 +170,7 @@ it { should have_attributes(status: 0, stdout: /Usage: flipper/) } it "should list subcommands" do - %w(enable disable list).each do |subcommand| + %w(enable disable deny permit list).each do |subcommand| expect(subject.stdout).to match(/#{subcommand}/) end end @@ -213,5 +255,15 @@ before { Flipper.enable_group :foo, :admins } it { should have_attributes(status: 0, stdout: /enabled.*admins/m) } end + + context "with denied actors" do + before { Flipper.deny_actor :foo, Flipper::Actor.new("User;1") } + it { should have_attributes(status: 0, stdout: /deny list.*User;1/m) } + end + + context "with denied groups" do + before { Flipper.deny_group :foo, :admins } + it { should have_attributes(status: 0, stdout: /deny list.*admins/m) } + end end end diff --git a/spec/flipper/cloud_spec.rb b/spec/flipper/cloud_spec.rb index 3aa6a46cc..669be7569 100644 --- a/spec/flipper/cloud_spec.rb +++ b/spec/flipper/cloud_spec.rb @@ -122,10 +122,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", deny_actors: Set.new, deny_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) @@ -148,10 +148,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", deny_actors: Set.new, deny_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) @@ -173,10 +173,10 @@ cloud_flipper = Flipper::Cloud.new(token: "asdf") get_all = { - "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", block_actors: Set.new, block_groups: Set.new}, - "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, + "logging" => {actors: Set.new, boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: "5", deny_actors: Set.new, deny_groups: Set.new}, + "search" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "stats" => {actors: Set["jnunemaker"], boolean: nil, groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "test" => {actors: Set.new, boolean: "true", groups: Set.new, expression: nil, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, } expect(flipper.adapter.get_all).to eq(get_all) diff --git a/spec/flipper/exporters/json/v1_spec.rb b/spec/flipper/exporters/json/v1_spec.rb index 3634acfaa..e11261a85 100644 --- a/spec/flipper/exporters/json/v1_spec.rb +++ b/spec/flipper/exporters/json/v1_spec.rb @@ -25,9 +25,9 @@ export = subject.call(adapter) expect(export.features).to eq({ - "google_analytics" => {actors: Set.new, boolean: nil, expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "plausible" => {actors: Set.new, boolean: "true", expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, block_actors: Set.new, block_groups: Set.new}, - "search" => {actors: Set["User;1", "User;100"], boolean: nil, expression: nil, groups: Set["admins", "employees"], percentage_of_actors: "10", percentage_of_time: "15", block_actors: Set.new, block_groups: Set.new}, + "google_analytics" => {actors: Set.new, boolean: nil, expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "plausible" => {actors: Set.new, boolean: "true", expression: nil, groups: Set.new, percentage_of_actors: nil, percentage_of_time: nil, deny_actors: Set.new, deny_groups: Set.new}, + "search" => {actors: Set["User;1", "User;100"], boolean: nil, expression: nil, groups: Set["admins", "employees"], percentage_of_actors: "10", percentage_of_time: "15", deny_actors: Set.new, deny_groups: Set.new}, }) end end diff --git a/spec/flipper/feature_spec.rb b/spec/flipper/feature_spec.rb index d76fcb7f0..7021e5d19 100644 --- a/spec/flipper/feature_spec.rb +++ b/spec/flipper/feature_spec.rb @@ -1214,8 +1214,8 @@ def actor.nil? :boolean, :group, :expression, - :block_actor, - :block_group, + :deny_actor, + :deny_group, ]) expect(subject.disabled_gate_names.to_set).to eq(Set[ @@ -1223,8 +1223,8 @@ def actor.nil? :boolean, :group, :expression, - :block_actor, - :block_group, + :deny_actor, + :deny_group, ]) end end diff --git a/spec/flipper/gates/block_actor_spec.rb b/spec/flipper/gates/deny_actor_spec.rb similarity index 82% rename from spec/flipper/gates/block_actor_spec.rb rename to spec/flipper/gates/deny_actor_spec.rb index 620324b20..486387c9c 100644 --- a/spec/flipper/gates/block_actor_spec.rb +++ b/spec/flipper/gates/deny_actor_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe Flipper::Gates::BlockActor do +RSpec.describe Flipper::Gates::DenyActor do let(:feature_name) { :search } subject do @@ -8,21 +8,21 @@ def context(set, actors: [Flipper::Types::Actor.new(Flipper::Actor.new('5'))]) Flipper::FeatureCheckContext.new( feature_name: feature_name, - values: Flipper::GateValues.new(block_actors: set), + values: Flipper::GateValues.new(deny_actors: set), actors: actors ) end describe '#blocks?' do - it 'returns true when actor is in blocked set' do + it 'returns true when actor is in denied set' do expect(subject.blocks?(context(Set['5']))).to be(true) end - it 'returns false when actor is not in blocked set' do + it 'returns false when actor is not in denied set' do expect(subject.blocks?(context(Set['10']))).to be(false) end - it 'returns false when blocked set is empty' do + it 'returns false when denied set is empty' do expect(subject.blocks?(context(Set.new))).to be(false) end diff --git a/spec/flipper/gates/block_group_spec.rb b/spec/flipper/gates/deny_group_spec.rb similarity index 86% rename from spec/flipper/gates/block_group_spec.rb rename to spec/flipper/gates/deny_group_spec.rb index aff24728b..f31df44b9 100644 --- a/spec/flipper/gates/block_group_spec.rb +++ b/spec/flipper/gates/deny_group_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe Flipper::Gates::BlockGroup do +RSpec.describe Flipper::Gates::DenyGroup do let(:feature_name) { :search } subject do @@ -8,7 +8,7 @@ def context(set) Flipper::FeatureCheckContext.new( feature_name: feature_name, - values: Flipper::GateValues.new(block_groups: set), + values: Flipper::GateValues.new(deny_groups: set), actors: [Flipper::Types::Actor.new(Flipper::Actor.new('5'))] ) end @@ -19,7 +19,7 @@ def context(set) Flipper.register(:staff) { |actor| true } end - it 'returns true when actor matches blocked group' do + it 'returns true when actor matches denied group' do expect(subject.blocks?(context(Set[:staff]))).to be(true) end end @@ -29,12 +29,12 @@ def context(set) Flipper.register(:staff) { |actor| false } end - it 'returns false when actor does not match blocked group' do + it 'returns false when actor does not match denied group' do expect(subject.blocks?(context(Set[:staff]))).to be(false) end end - it 'returns false when blocked set is empty' do + it 'returns false when denied set is empty' do expect(subject.blocks?(context(Set.new))).to be(false) end diff --git a/spec/flipper/ui/actions/block_actors_gate_spec.rb b/spec/flipper/ui/actions/deny_actors_gate_spec.rb similarity index 55% rename from spec/flipper/ui/actions/block_actors_gate_spec.rb rename to spec/flipper/ui/actions/deny_actors_gate_spec.rb index 0a50d84a3..0f9fba905 100644 --- a/spec/flipper/ui/actions/block_actors_gate_spec.rb +++ b/spec/flipper/ui/actions/deny_actors_gate_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe Flipper::UI::Actions::BlockActorsGate do +RSpec.describe Flipper::UI::Actions::DenyActorsGate do let(:token) do if Rack::Protection::AuthenticityToken.respond_to?(:random_token) Rack::Protection::AuthenticityToken.random_token @@ -10,30 +10,30 @@ { :csrf => token, 'csrf' => token, '_csrf_token' => token } end - describe 'POST /features/:feature/block_actors' do - context 'blocking an actor' do + describe 'POST /features/:feature/deny_actors' do + context 'denying an actor' do let(:value) { 'User;6' } let(:multi_value) { 'User;5, User;7, User;9, User;12' } before do - post 'features/search/block_actors', - { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + post 'features/search/deny_actors', + { 'value' => value, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session end - it 'adds item to blocked actors' do - expect(flipper[:search].block_actors_value).to include(value) + it 'adds item to denied actors' do + expect(flipper[:search].deny_actors_value).to include(value) end - it 'adds multiple items to blocked actors' do - post 'features/search/block_actors', - { 'value' => multi_value, 'operation' => 'block', 'authenticity_token' => token }, + it 'adds multiple items to denied actors' do + post 'features/search/deny_actors', + { 'value' => multi_value, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session - expect(flipper[:search].block_actors_value).to include('User;5') - expect(flipper[:search].block_actors_value).to include('User;7') - expect(flipper[:search].block_actors_value).to include('User;9') - expect(flipper[:search].block_actors_value).to include('User;12') + expect(flipper[:search].deny_actors_value).to include('User;5') + expect(flipper[:search].deny_actors_value).to include('User;7') + expect(flipper[:search].deny_actors_value).to include('User;9') + expect(flipper[:search].deny_actors_value).to include('User;12') end it 'redirects back to feature' do @@ -43,13 +43,13 @@ context 'when feature name contains space' do before do - post 'features/sp+ace/block_actors', - { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + post 'features/sp+ace/deny_actors', + { 'value' => value, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session end - it 'adds item to blocked actors' do - expect(flipper["sp ace"].block_actors_value).to include('User;6') + it 'adds item to denied actors' do + expect(flipper["sp ace"].deny_actors_value).to include('User;6') end it 'redirects back to feature' do @@ -62,7 +62,7 @@ let(:value) { ' User;6 ' } it 'adds item without whitespace' do - expect(flipper[:search].block_actors_value).to include('User;6') + expect(flipper[:search].deny_actors_value).to include('User;6') end end @@ -72,7 +72,7 @@ it 'redirects back with error' do expect(last_response.status).to be(302) - expect(last_response.headers['location']).to eq('/features/search/block_actors?error=%22%22+is+not+a+valid+actor+value.') + expect(last_response.headers['location']).to eq('/features/search/deny_actors?error=%22%22+is+not+a+valid+actor+value.') end end @@ -81,7 +81,7 @@ it 'redirects back with error' do expect(last_response.status).to be(302) - expect(last_response.headers['location']).to eq('/features/search/block_actors?error=%22%22+is+not+a+valid+actor+value.') + expect(last_response.headers['location']).to eq('/features/search/deny_actors?error=%22%22+is+not+a+valid+actor+value.') end end end @@ -94,28 +94,28 @@ allow(flipper).to receive(:read_only?) { true } end - it 'does not allow an actor to be blocked' do - post 'features/search/block_actors', - { 'value' => value, 'operation' => 'block', 'authenticity_token' => token }, + it 'does not allow an actor to be denied' do + post 'features/search/deny_actors', + { 'value' => value, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session - expect(flipper[:search].block_actors_value).not_to include('User;6') + expect(flipper[:search].deny_actors_value).not_to include('User;6') expect(last_response.body).to include("The UI is currently in read only mode.") end end - context 'unblocking an actor' do + context 'permitting an actor' do let(:value) { 'User;6' } before do - flipper[:search].block_actor Flipper::Actor.new(value) - post 'features/search/block_actors', - { 'value' => value, 'operation' => 'unblock', 'authenticity_token' => token }, + flipper[:search].deny_actor Flipper::Actor.new(value) + post 'features/search/deny_actors', + { 'value' => value, 'operation' => 'permit', 'authenticity_token' => token }, 'rack.session' => session end - it 'removes item from blocked actors' do - expect(flipper[:search].block_actors_value).not_to include(value) + it 'removes item from denied actors' do + expect(flipper[:search].deny_actors_value).not_to include(value) end it 'redirects back to feature' do @@ -127,7 +127,7 @@ let(:value) { ' User;6 ' } it 'removes item without whitespace' do - expect(flipper[:search].block_actors_value).not_to include('User;6') + expect(flipper[:search].deny_actors_value).not_to include('User;6') end end end diff --git a/spec/flipper/ui/actions/block_groups_gate_spec.rb b/spec/flipper/ui/actions/deny_groups_gate_spec.rb similarity index 64% rename from spec/flipper/ui/actions/block_groups_gate_spec.rb rename to spec/flipper/ui/actions/deny_groups_gate_spec.rb index d97ddeff9..e3e55cb03 100644 --- a/spec/flipper/ui/actions/block_groups_gate_spec.rb +++ b/spec/flipper/ui/actions/deny_groups_gate_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe Flipper::UI::Actions::BlockGroupsGate do +RSpec.describe Flipper::UI::Actions::DenyGroupsGate do let(:token) do if Rack::Protection::AuthenticityToken.respond_to?(:random_token) Rack::Protection::AuthenticityToken.random_token @@ -11,7 +11,7 @@ { :csrf => token, 'csrf' => token, '_csrf_token' => token } end - describe 'POST /features/:feature/block_groups' do + describe 'POST /features/:feature/deny_groups' do let(:group_name) { 'admins' } before do @@ -22,15 +22,15 @@ Flipper.unregister_groups end - context 'blocking a group' do + context 'denying a group' do before do - post 'features/search/block_groups', - { 'value' => group_name, 'operation' => 'block', 'authenticity_token' => token }, + post 'features/search/deny_groups', + { 'value' => group_name, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session end - it 'adds item to blocked groups' do - expect(flipper[:search].block_groups_value).to include('admins') + it 'adds item to denied groups' do + expect(flipper[:search].deny_groups_value).to include('admins') end it 'redirects back to feature' do @@ -40,13 +40,13 @@ context 'feature name contains space' do before do - post 'features/sp+ace/block_groups', - { 'value' => group_name, 'operation' => 'block', 'authenticity_token' => token }, + post 'features/sp+ace/deny_groups', + { 'value' => group_name, 'operation' => 'deny', 'authenticity_token' => token }, 'rack.session' => session end - it 'adds item to blocked groups' do - expect(flipper["sp ace"].block_groups_value).to include('admins') + it 'adds item to denied groups' do + expect(flipper["sp ace"].deny_groups_value).to include('admins') end it 'redirects back to feature' do @@ -59,7 +59,7 @@ let(:group_name) { ' admins ' } it 'adds item without whitespace' do - expect(flipper[:search].block_groups_value).to include('admins') + expect(flipper[:search].deny_groups_value).to include('admins') end end @@ -69,7 +69,7 @@ it 'redirects back with error' do expect(last_response.status).to be(302) - expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22not_here%22+has+not+been+registered.') + expect(last_response.headers['location']).to eq('/features/search/deny_groups?error=The+group+named+%22not_here%22+has+not+been+registered.') end end @@ -78,7 +78,7 @@ it 'redirects back with error' do expect(last_response.status).to be(302) - expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22%22+has+not+been+registered.') + expect(last_response.headers['location']).to eq('/features/search/deny_groups?error=The+group+named+%22%22+has+not+been+registered.') end end @@ -87,24 +87,24 @@ it 'redirects back with error' do expect(last_response.status).to be(302) - expect(last_response.headers['location']).to eq('/features/search/block_groups?error=The+group+named+%22%22+has+not+been+registered.') + expect(last_response.headers['location']).to eq('/features/search/deny_groups?error=The+group+named+%22%22+has+not+been+registered.') end end end end - context 'unblocking a group' do + context 'permitting a group' do let(:group_name) { 'admins' } before do - flipper[:search].block_group :admins - post 'features/search/block_groups', - { 'value' => group_name, 'operation' => 'unblock', 'authenticity_token' => token }, + flipper[:search].deny_group :admins + post 'features/search/deny_groups', + { 'value' => group_name, 'operation' => 'permit', 'authenticity_token' => token }, 'rack.session' => session end - it 'removes item from blocked groups' do - expect(flipper[:search].block_groups_value).not_to include('admins') + it 'removes item from denied groups' do + expect(flipper[:search].deny_groups_value).not_to include('admins') end it 'redirects back to feature' do @@ -116,7 +116,7 @@ let(:group_name) { ' admins ' } it 'removes item without whitespace' do - expect(flipper[:search].block_groups_value).not_to include('admins') + expect(flipper[:search].deny_groups_value).not_to include('admins') end end end diff --git a/spec/flipper/ui/actions/export_spec.rb b/spec/flipper/ui/actions/export_spec.rb index ce3f3d584..fa52ceb21 100644 --- a/spec/flipper/ui/actions/export_spec.rb +++ b/spec/flipper/ui/actions/export_spec.rb @@ -41,10 +41,10 @@ expect(last_response.headers['content-type']).to eq('application/json') expect(data['version']).to eq(1) expect(data['features']).to eq({ - "analytics" => {"boolean"=>nil, "expression"=>{"Equal"=>[{"Property"=>["plan"]}, "basic"]}, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, - "search"=> {"boolean"=>nil, "expression"=>nil, "groups"=>["admins", "employees"], "actors"=>["User;1", "User;100"], "percentage_of_actors"=>"10", "percentage_of_time"=>"15", "block_actors"=>[], "block_groups"=>[]}, - "plausible"=> {"boolean"=>"true", "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, - "google_analytics"=> {"boolean"=>nil, "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "block_actors"=>[], "block_groups"=>[]}, + "analytics" => {"boolean"=>nil, "expression"=>{"Equal"=>[{"Property"=>["plan"]}, "basic"]}, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "deny_actors"=>[], "deny_groups"=>[]}, + "search"=> {"boolean"=>nil, "expression"=>nil, "groups"=>["admins", "employees"], "actors"=>["User;1", "User;100"], "percentage_of_actors"=>"10", "percentage_of_time"=>"15", "deny_actors"=>[], "deny_groups"=>[]}, + "plausible"=> {"boolean"=>"true", "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "deny_actors"=>[], "deny_groups"=>[]}, + "google_analytics"=> {"boolean"=>nil, "expression"=>nil, "groups"=>[], "actors"=>[], "percentage_of_actors"=>nil, "percentage_of_time"=>nil, "deny_actors"=>[], "deny_groups"=>[]}, }) end end From 6d246e79c7722f35999e9a97cd274d8fb2814557 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sat, 11 Apr 2026 17:55:26 -0400 Subject: [PATCH 4/4] Rename clear_allow_gates to clear_permit_gates Align internal adapter method naming with the public deny/permit API vocabulary. Co-Authored-By: Claude Opus 4.6 --- lib/flipper/adapters/active_record.rb | 6 +++--- lib/flipper/adapters/memory.rb | 6 +++--- lib/flipper/adapters/moneta.rb | 6 +++--- lib/flipper/adapters/mongo.rb | 6 +++--- lib/flipper/adapters/pstore.rb | 6 +++--- lib/flipper/adapters/redis.rb | 6 +++--- lib/flipper/adapters/sequel.rb | 6 +++--- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index 6da82e2a3..bfb00255e 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -162,7 +162,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer set(feature, gate, thing) when :json @@ -187,7 +187,7 @@ def unsupported_data_type(data_type) private - def clear_allow_gates(feature) + def clear_permit_gates(feature) with_write_connection(@gate_class) do @gate_class.where(feature_key: feature.key) .where.not(key: deny_gate_keys.map(&:to_s)) @@ -203,7 +203,7 @@ def set(feature, gate, thing, options = {}) with_write_connection(@gate_class) do @gate_class.transaction(requires_new: true) do - clear_allow_gates(feature) if clear_feature + clear_permit_gates(feature) if clear_feature delete(feature, gate) begin @gate_class.create! do |g| diff --git a/lib/flipper/adapters/memory.rb b/lib/flipper/adapters/memory.rb index 5aa0e2deb..6c248f2e1 100644 --- a/lib/flipper/adapters/memory.rb +++ b/lib/flipper/adapters/memory.rb @@ -65,7 +65,7 @@ def enable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) @source[feature.key][gate.key] = thing.value.to_s when :integer @source[feature.key][gate.key] = thing.value.to_s @@ -88,7 +88,7 @@ def disable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer @source[feature.key][gate.key] = thing.value.to_s when :set @@ -122,7 +122,7 @@ def import(source) private - def clear_allow_gates(feature) + def clear_permit_gates(feature) existing = @source[feature.key] || default_config preserved = {} deny_gate_keys.each { |k| preserved[k] = existing[k] } diff --git a/lib/flipper/adapters/moneta.rb b/lib/flipper/adapters/moneta.rb index ad2046975..2927c89e3 100644 --- a/lib/flipper/adapters/moneta.rb +++ b/lib/flipper/adapters/moneta.rb @@ -54,7 +54,7 @@ def get(feature) def enable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) result = get(feature) result[gate.key] = thing.value.to_s moneta[key(feature.key)] = result @@ -84,7 +84,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer result = get(feature) result[gate.key] = thing.value.to_s @@ -103,7 +103,7 @@ def disable(feature, gate, thing) private - def clear_allow_gates(feature) + def clear_permit_gates(feature) existing = get(feature) preserved = {} deny_gate_keys.each { |k| preserved[k] = existing[k] } diff --git a/lib/flipper/adapters/mongo.rb b/lib/flipper/adapters/mongo.rb index 30fc30d5f..82bf139c8 100644 --- a/lib/flipper/adapters/mongo.rb +++ b/lib/flipper/adapters/mongo.rb @@ -66,7 +66,7 @@ def get_all(**kwargs) def enable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) update feature.key, '$set' => { gate.key.to_s => thing.value.to_s, } @@ -99,7 +99,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer update feature.key, '$set' => { gate.key.to_s => thing.value.to_s, @@ -121,7 +121,7 @@ def disable(feature, gate, thing) private - def clear_allow_gates(feature) + def clear_permit_gates(feature) doc = find(feature.key) return if doc.empty? diff --git a/lib/flipper/adapters/pstore.rb b/lib/flipper/adapters/pstore.rb index de98f45fb..381f67130 100644 --- a/lib/flipper/adapters/pstore.rb +++ b/lib/flipper/adapters/pstore.rb @@ -78,7 +78,7 @@ def enable(feature, gate, thing) @store.transaction do case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) write key(feature, gate), thing.value.to_s when :integer write key(feature, gate), thing.value.to_s @@ -98,7 +98,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - @store.transaction { clear_allow_gates(feature) } + @store.transaction { clear_permit_gates(feature) } when :integer @store.transaction do write key(feature, gate), thing.value.to_s @@ -136,7 +136,7 @@ def clear_gates(feature) end end - def clear_allow_gates(feature) + def clear_permit_gates(feature) feature.gates.each do |gate| next if gate.deny? delete key(feature, gate) diff --git a/lib/flipper/adapters/redis.rb b/lib/flipper/adapters/redis.rb index a65e92f7f..0a1a47475 100644 --- a/lib/flipper/adapters/redis.rb +++ b/lib/flipper/adapters/redis.rb @@ -92,7 +92,7 @@ def enable(feature, gate, thing) feature_key = key_for(feature.key) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } when :integer with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } @@ -118,7 +118,7 @@ def disable(feature, gate, thing) feature_key = key_for(feature.key) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer with_connection { |conn| conn.hset feature_key, gate.key, thing.value.to_s } when :set @@ -134,7 +134,7 @@ def disable(feature, gate, thing) private - def clear_allow_gates(feature) + def clear_permit_gates(feature) feature_key = key_for(feature.key) with_connection do |conn| all_fields = conn.hkeys(feature_key) diff --git a/lib/flipper/adapters/sequel.rb b/lib/flipper/adapters/sequel.rb index 9fa6ad7ec..1188a577d 100644 --- a/lib/flipper/adapters/sequel.rb +++ b/lib/flipper/adapters/sequel.rb @@ -155,7 +155,7 @@ def enable(feature, gate, thing) def disable(feature, gate, thing) case gate.data_type when :boolean - clear_allow_gates(feature) + clear_permit_gates(feature) when :integer set(feature, gate, thing) when :json @@ -175,7 +175,7 @@ def unsupported_data_type(data_type) raise "#{data_type} is not supported by this adapter" end - def clear_allow_gates(feature) + def clear_permit_gates(feature) @gate_class.where(feature_key: feature.key.to_s) .exclude(key: deny_gate_keys.map(&:to_s)) .delete @@ -188,7 +188,7 @@ def set(feature, gate, thing, options = {}) raise VALUE_TO_TEXT_WARNING if json_feature && value_not_text? @gate_class.db.transaction do - clear_allow_gates(feature) if clear_feature + clear_permit_gates(feature) if clear_feature delete(feature, gate) begin