diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 991fdad65..2b52b354c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -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 diff --git a/app/controllers/pages/secondary_skip_controller.rb b/app/controllers/pages/secondary_skip_controller.rb index 79e56c838..dd9e9e815 100644 --- a/app/controllers/pages/secondary_skip_controller.rb +++ b/app/controllers/pages/secondary_skip_controller.rb @@ -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 diff --git a/app/presenters/route_summary_card_data_presenter.rb b/app/presenters/route_summary_card_data_presenter.rb index 075ca5c60..1e5804e6e 100644 --- a/app/presenters/route_summary_card_data_presenter.rb +++ b/app/presenters/route_summary_card_data_presenter.rb @@ -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, @@ -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") }, diff --git a/app/services/feature_service.rb b/app/services/feature_service.rb index a88d46229..54040572a 100644 --- a/app/services/feature_service.rb +++ b/app/services/feature_service.rb @@ -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) @@ -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 diff --git a/app/views/forms/_made_live_form.html.erb b/app/views/forms/_made_live_form.html.erb index ecbe2979f..009f5283e 100644 --- a/app/views/forms/_made_live_form.html.erb +++ b/app/views/forms/_made_live_form.html.erb @@ -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")) %> diff --git a/app/views/forms/show.html.erb b/app/views/forms/show.html.erb index 3f855fee4..f39613443 100644 --- a/app/views/forms/show.html.erb +++ b/app/views/forms/show.html.erb @@ -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")) %> diff --git a/app/views/pages/conditions/_routing_options.html.erb b/app/views/pages/conditions/_routing_options.html.erb index a22c5973f..1ee40d36c 100644 --- a/app/views/pages/conditions/_routing_options.html.erb +++ b/app/views/pages/conditions/_routing_options.html.erb @@ -11,7 +11,7 @@ <%= t("routing_page.body_routing_text") %>

- <% if FeatureService.enabled?(:branch_routing) %> + <% if FeatureService.new(group: form.group).enabled?(:branch_routing) %>

<%= t("routing_page.body_branching_text") %>

diff --git a/app/views/pages/conditions/routing_page.html.erb b/app/views/pages/conditions/routing_page.html.erb index 4ce46403e..609ea8c00 100644 --- a/app/views/pages/conditions/routing_page.html.erb +++ b/app/views/pages/conditions/routing_page.html.erb @@ -15,7 +15,7 @@ <%= t("routing_page.body_routing_text") %>

- <% if FeatureService.enabled?(:branch_routing) %> + <% if FeatureService.new(group: form.group).enabled?(:branch_routing) %>

<%= t("routing_page.body_branching_text") %>

diff --git a/config/settings.yml b/config/settings.yml index 6c92bdef7..a071c7f16 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -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 diff --git a/db/migrate/20250124094215_add_branching_enabled_to_groups.rb b/db/migrate/20250124094215_add_branching_enabled_to_groups.rb new file mode 100644 index 000000000..6e1ebafe8 --- /dev/null +++ b/db/migrate/20250124094215_add_branching_enabled_to_groups.rb @@ -0,0 +1,5 @@ +class AddBranchingEnabledToGroups < ActiveRecord::Migration[8.0] + def change + add_column :groups, :branch_routing_enabled, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index bc8ea9d4a..9135e4a8f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,9 +10,9 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_29_152103) do +ActiveRecord::Schema[8.0].define(version: 2025_01_24_094215) do # These are extensions that must be enabled in order to support this database - enable_extension "plpgsql" + enable_extension "pg_catalog.plpgsql" create_table "draft_questions", force: :cascade do |t| t.integer "form_id" @@ -55,6 +55,7 @@ t.bigint "creator_id" t.bigint "upgrade_requester_id" t.boolean "file_upload_enabled", default: false + t.boolean "branch_routing_enabled", default: false t.index ["creator_id"], name: "index_groups_on_creator_id" t.index ["external_id"], name: "index_groups_on_external_id", unique: true t.index ["name", "organisation_id"], name: "index_groups_on_name_and_organisation_id", unique: true diff --git a/lib/tasks/groups.rake b/lib/tasks/groups.rake index 4cbcb418f..12427d596 100644 --- a/lib/tasks/groups.rake +++ b/lib/tasks/groups.rake @@ -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[]".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[]".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:) diff --git a/spec/config/settings_spec.rb b/spec/config/settings_spec.rb index 732ee1578..631744235 100644 --- a/spec/config/settings_spec.rb +++ b/spec/config/settings_spec.rb @@ -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 diff --git a/spec/presenters/route_summary_card_data_presenter_spec.rb b/spec/presenters/route_summary_card_data_presenter_spec.rb index ea9ca195d..4b43fb8a3 100644 --- a/spec/presenters/route_summary_card_data_presenter_spec.rb +++ b/spec/presenters/route_summary_card_data_presenter_spec.rb @@ -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:) diff --git a/spec/services/feature_service_spec.rb b/spec/services/feature_service_spec.rb index b25043a85..09b249394 100644 --- a/spec/services/feature_service_spec.rb +++ b/spec/services/feature_service_spec.rb @@ -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" } @@ -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 }) @@ -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 diff --git a/spec/views/pages/conditions/routing_page.html.erb_spec.rb b/spec/views/pages/conditions/routing_page.html.erb_spec.rb index 5e1a49df6..64e182199 100644 --- a/spec/views/pages/conditions/routing_page.html.erb_spec.rb +++ b/spec/views/pages/conditions/routing_page.html.erb_spec.rb @@ -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