Skip to content

Commit

Permalink
Merge pull request #1737 from alphagov/add-fflag-groups-enabled
Browse files Browse the repository at this point in the history
Add fflag groups enabled
  • Loading branch information
thomasiles authored Jan 30, 2025
2 parents a6b4e1e + 1e57741 commit d7dd719
Show file tree
Hide file tree
Showing 16 changed files with 97 additions and 16 deletions.
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def current_form
end

def groups_enabled
@groups_enabled ||= current_user.present? && FeatureService.new(current_user).enabled?(:groups)
@groups_enabled ||= current_user.present? && FeatureService.new(user: current_user).enabled?(:groups)
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/pages/secondary_skip_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def delete_secondary_skip_input_params
end

def ensure_branch_routing_feature_enabled
raise ActionController::RoutingError, "branch_routing feature not enabled" unless Settings.features.branch_routing
raise ActionController::RoutingError, "branch_routing feature not enabled" unless FeatureService.new(group: current_form.group).enabled?(:branch_routing)
end

def ensure_page_has_skip_condition
Expand Down
4 changes: 2 additions & 2 deletions app/presenters/route_summary_card_data_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def conditional_route_card(routing_condition, index)
def default_route_card(index)
continue_to_name = page.has_next_page? ? page_name(page.next_page) : end_page_name

actions = if FeatureService.enabled?(:branch_routing) && all_routes.find(&:secondary_skip?).present?
actions = if FeatureService.new(group: form.group).enabled?(:branch_routing) && all_routes.find(&:secondary_skip?).present?
[
edit_secondary_skip_link,
delete_secondary_skip_link,
Expand Down Expand Up @@ -101,7 +101,7 @@ def secondary_skip_rows
secondary_skip = all_routes.find(&:secondary_skip?)

if secondary_skip.blank?
if FeatureService.enabled?(:branch_routing)
if FeatureService.new(group: form.group).enabled?(:branch_routing)
return [
{
key: { text: I18n.t("page_route_card.then") },
Expand Down
14 changes: 12 additions & 2 deletions app/services/feature_service.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
class FeatureService
class UserRequiredError < StandardError; end
class GroupRequiredError < StandardError; end

attr_reader :group

class << self
def enabled?(...)
FeatureService.new(nil).enabled?(...)
FeatureService.new.enabled?(...)
end
end

def initialize(user)
def initialize(user: nil, group: nil)
@user = user
@group = group
end

def enabled?(feature_name)
Expand All @@ -28,6 +32,12 @@ def enabled?(feature_name)
end
end

if feature.enabled_by_group.present? && feature.enabled_by_group
raise GroupRequiredError, "Feature #{feature_name} requires group to be provided" if group.blank?

return group.send(:"#{feature_name}_enabled?")
end

feature.enabled
end
end
2 changes: 1 addition & 1 deletion app/views/forms/_made_live_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% set_page_title(form.name) %>

<% if FeatureService.new(@current_user).enabled?(:groups) && form.group.present? %>
<% if FeatureService.new(user: @current_user).enabled?(:groups) && form.group.present? %>
<% content_for :back_link, govuk_back_link_to(group_path(form.group), t("back_link.group", group_name: form.group.name)) %>
<% else %>
<% content_for :back_link, govuk_back_link_to(root_path, t("back_link.forms")) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/forms/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<% content_for :back_link, govuk_back_link_to(live_form_path(current_form.id), t("back_link.form_view")) %>
<% elsif current_form.is_archived? %>
<% content_for :back_link, govuk_back_link_to(archived_form_path(current_form.id), t("back_link.form_view")) %>
<% elsif FeatureService.new(@current_user).enabled?(:groups) && current_form.group.present? %>
<% elsif FeatureService.new(user: @current_user).enabled?(:groups) && current_form.group.present? %>
<% content_for :back_link, govuk_back_link_to(group_path(current_form.group), t("back_link.group", group_name: current_form.group.name)) %>
<% else %>
<% content_for :back_link, govuk_back_link_to(root_path, t("back_link.forms")) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/pages/conditions/_routing_options.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<%= t("routing_page.body_routing_text") %>
</p>

<% if FeatureService.enabled?(:branch_routing) %>
<% if FeatureService.new(group: form.group).enabled?(:branch_routing) %>
<p>
<%= t("routing_page.body_branching_text") %>
</p>
Expand Down
2 changes: 1 addition & 1 deletion app/views/pages/conditions/routing_page.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<%= t("routing_page.body_routing_text") %>
</p>

<% if FeatureService.enabled?(:branch_routing) %>
<% if FeatureService.new(group: form.group).enabled?(:branch_routing) %>
<p>
<%= t("routing_page.body_branching_text") %>
</p>
Expand Down
3 changes: 2 additions & 1 deletion config/settings.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Used to add feature flags in the app to control access to certain features.
features:
groups: true # Do not switch off!
branch_routing: false
branch_routing:
enabled_by_group: true

forms_api:
# Authentication key to authenticate with forms-api
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20250124094215_add_branching_enabled_to_groups.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddBranchingEnabledToGroups < ActiveRecord::Migration[8.0]
def change
add_column :groups, :branch_routing_enabled, :boolean, default: false
end
end
5 changes: 3 additions & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions lib/tasks/groups.rake
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,24 @@ namespace :groups do
Group.find_by(external_id: args[:group_id]).update!(file_upload_enabled: false)
Rails.logger.info("Updated file_upload_enabled to false for group #{args[:group_id]}")
end

desc "Enable branch_routing feature for group"
task :enable_branch_routing, %i[group_id] => :environment do |_, args|
usage_message = "usage: rake groups:enable_branch_routing[<group_external_id>]".freeze
abort usage_message if args[:group_id].blank?

Group.find_by(external_id: args[:group_id]).update!(branch_routing_enabled: true)
Rails.logger.info("Updated branch_routing_enabled to true for group #{args[:group_id]}")
end

desc "Disable branch_routing feature for group"
task :disable_branch_routing, %i[group_id] => :environment do |_, args|
usage_message = "usage: rake groups:disable_branch_routing[<group_external_id>]".freeze
abort usage_message if args[:group_id].blank?

Group.find_by(external_id: args[:group_id]).update!(branch_routing_enabled: false)
Rails.logger.info("Updated branch_routing_enabled to false for group #{args[:group_id]}")
end
end

def run_task(task_name, args, rollback:)
Expand Down
2 changes: 1 addition & 1 deletion spec/config/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
features = settings[:features]

include_examples expected_value_test, :groups, features, true
include_examples expected_value_test, :branch_routing, features, false
include_examples expected_value_test, :branch_routing, features, { "enabled_by_group" => true }
end

describe "forms_api" do
Expand Down
4 changes: 4 additions & 0 deletions spec/presenters/route_summary_card_data_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@

let(:next_page_routing_conditions) { [] }

before do
allow(form).to receive(:group).and_return(build(:group))
end

describe ".call" do
it "instantiates and returns a new instance" do
service = described_class.call(form:, page: current_page, pages:)
Expand Down
45 changes: 43 additions & 2 deletions spec/services/feature_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
describe FeatureService do
describe "#enabled?" do
subject :feature_service do
described_class.new(user)
described_class.new(user:)
end

let(:organisation) { build :organisation, id: 1, slug: "a-test-org" }
Expand Down Expand Up @@ -103,7 +103,7 @@
end

context "when a key exists for the organisation overriding the feature and the user has not been provided to the service" do
let(:feature_service) { described_class.new(nil) }
let(:feature_service) { described_class.new }

before do
Settings.features[:some_feature] = Config::Options.new(enabled: false, organisations: { a_test_org: true })
Expand Down Expand Up @@ -144,6 +144,47 @@
expect(feature_service).not_to be_enabled(:some_feature)
end
end

context "when feature is enabled_by_group" do
subject(:feature_service) { described_class.new(group:) }

let(:group) { double }

before do
Settings.features[:test_feature] = Config::Options.new(enabled_by_group: true)
end

it "raises GroupRequiredError if group is not provided" do
service = described_class.new

expect {
service.enabled?(:test_feature)
}.to raise_error(
described_class::GroupRequiredError,
"Feature test_feature requires group to be provided",
)
end

it "calls the corresponding enabled? method on the group" do
allow(group).to receive(:test_feature_enabled?).and_return(true)

expect(feature_service.enabled?(:test_feature)).to be true
end

it "returns false when group method returns false" do
allow(group).to receive(:test_feature_enabled?).and_return(false)

expect(feature_service.enabled?(:test_feature)).to be false
end

it "raises NoMethodError when group does not respond to the feature method" do
allow(group).to receive(:method_missing).and_raise(NoMethodError)

expect {
feature_service.enabled?(:test_feature)
}.to raise_error(NoMethodError)
end
end
end
end

Expand Down
1 change: 1 addition & 0 deletions spec/views/pages/conditions/routing_page.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

allow(view).to receive_messages(form_pages_path: "/forms/1/pages", routing_page_path: "/forms/1/new-condition", set_routing_page_path: "/forms/1/new-condition")
allow(form).to receive_messages(qualifying_route_pages: pages, has_no_remaining_routes_available?: all_routes_created)
allow(form).to receive(:group).and_return(build(:group))

render template: "pages/conditions/routing_page", locals: { form:, routing_page_input: }
end
Expand Down

0 comments on commit d7dd719

Please sign in to comment.