diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index 76ee10035..7d62d34bc 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -31,7 +31,6 @@ def show elsif is_history_page? show_history_page else - set_use_recommended_related_links_header set_access_control_allow_origin_header if request.format.atom? set_guide_draft_access_token if @content_item.is_a?(GuidePresenter) render_template @@ -139,9 +138,8 @@ def set_guide_draft_access_token def load_content_item content_item = Services.content_store.content_item(content_item_path) - if Services.feature_toggler.use_recommended_related_links?(content_item["links"], request.headers) - content_item["links"]["ordered_related_items"] = content_item["links"].fetch("suggested_ordered_related_items", []) - end + + content_item["links"]["ordered_related_items"] = ordered_related_items(content_item["links"]) if content_item["links"] @content_item = PresenterBuilder.new( content_item, @@ -150,6 +148,14 @@ def load_content_item ).presenter end + def ordered_related_items(links) + return [] if links["ordered_related_items_overrides"].present? + + links["ordered_related_items"].presence || links.fetch( + "suggested_ordered_related_items", [] + ) + end + def format_banner_links(links, type) links.each.with_index(1).map do |(title, base_path), index| view_context.link_to( @@ -205,14 +211,6 @@ def set_access_control_allow_origin_header response.headers["Access-Control-Allow-Origin"] = "*" end - def set_use_recommended_related_links_header - response.headers["Vary"] = [response.headers["Vary"], FeatureFlagNames.recommended_related_links].compact.join(", ") - - related_links_request_header = RequestHelper.get_header(FeatureFlagNames.recommended_related_links, request.headers) - required_header_value = Services.feature_toggler.feature_flags.get_feature_flag(FeatureFlagNames.recommended_related_links) - response.headers[FeatureFlagNames.recommended_related_links] = (related_links_request_header == required_header_value).to_s - end - def set_expiry expires_in( @content_item.cache_control_max_age(request.format), diff --git a/app/lib/services.rb b/app/lib/services.rb index 97629a385..cfeae95fb 100644 --- a/app/lib/services.rb +++ b/app/lib/services.rb @@ -12,12 +12,6 @@ def self.content_store ) end - def self.feature_toggler - @feature_toggler ||= FeatureToggler.new( - HttpFeatureFlags.instance, - ) - end - def self.publishing_api @publishing_api ||= GdsApi::PublishingApi.new( Plek.find("publishing-api"), diff --git a/app/models/feature_toggler.rb b/app/models/feature_toggler.rb deleted file mode 100644 index 2c3e2f94f..000000000 --- a/app/models/feature_toggler.rb +++ /dev/null @@ -1,15 +0,0 @@ -class FeatureToggler - attr_reader :feature_flags - - def initialize(feature_flags) - @feature_flags = feature_flags - end - - def use_recommended_related_links?(content_item_links, request_headers) - return false if content_item_links.nil? - - content_item_links.fetch("ordered_related_items", []).empty? && - content_item_links.fetch("ordered_related_items_overrides", []).empty? && - @feature_flags.feature_enabled?(FeatureFlagNames.recommended_related_links, request_headers) - end -end diff --git a/app/models/http_feature_flags.rb b/app/models/http_feature_flags.rb deleted file mode 100644 index 4a50273d0..000000000 --- a/app/models/http_feature_flags.rb +++ /dev/null @@ -1,21 +0,0 @@ -class HttpFeatureFlags - def initialize - @feature_flags = {} - end - - def add_http_feature_flag(feature_flag_name, val) - @feature_flags[RequestHelper.headerise(feature_flag_name)] = val - end - - def get_feature_flag(feature_flag_name) - @feature_flags[RequestHelper.headerise(feature_flag_name)] - end - - def feature_enabled?(feature_flag_name, request_headers) - get_feature_flag(feature_flag_name) == RequestHelper.get_header(feature_flag_name, request_headers) - end - - def self.instance - @instance ||= HttpFeatureFlags.new - end -end diff --git a/config/initializers/feature_flags.rb b/config/initializers/feature_flags.rb deleted file mode 100644 index 5e9b0353b..000000000 --- a/config/initializers/feature_flags.rb +++ /dev/null @@ -1,9 +0,0 @@ -module FeatureFlagNames - def self.recommended_related_links - "Govuk-Use-Recommended-Related-Links" - end -end - -Rails.application.reloader.to_prepare do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") -end diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb index e21d129b1..8cd57808a 100644 --- a/test/controllers/content_items_controller_test.rb +++ b/test/controllers/content_items_controller_test.rb @@ -132,19 +132,7 @@ class ContentItemsControllerTest < ActionController::TestCase assert_equal content_item["title"], assigns[:content_item].title end - test "gets item from content store and keeps existing ordered_related_items when feature flag header not specified" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - content_item = content_store_has_schema_example("case_study", "case_study") - - get :show, params: { path: path_for(content_item) } - assert_response :success - assert_equal content_item["links"]["ordered_related_items"], assigns[:content_item].content_item["links"]["ordered_related_items"] - end - - test "gets item from content store and keep existing ordered_related_items when feature flag header is specified but links already exist" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "true" - + test "gets item from content store and keeps existing ordered_related_items when links already exist" do content_item = content_store_has_schema_example("guide", "guide") get :show, params: { path: path_for(content_item) } @@ -154,10 +142,7 @@ class ContentItemsControllerTest < ActionController::TestCase assert_equal content_item["links"]["ordered_related_items"], assigns[:content_item].content_item["links"]["ordered_related_items"] end - test "gets item from content store and does not change ordered_related_items when feature flag header is specified but link overrides exist" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "true" - + test "gets item from content store and does not change ordered_related_items when link overrides exist" do content_item = content_store_has_schema_example("guide", "guide-with-related-link-overrides") get :show, params: { path: path_for(content_item) } @@ -168,23 +153,7 @@ class ContentItemsControllerTest < ActionController::TestCase assert_nil content_item["links"]["ordered_related_items"] end - test "gets item from content store and keeps ordered_related_items when feature flag header is specified but recommended links turned off" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "false") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "true" - - content_item = content_store_has_schema_example("case_study", "case_study") - - get :show, params: { path: path_for(content_item) } - assert_response :success - assert_empty content_item["links"]["ordered_related_items"], "Content item should have existing related links" - assert_not_empty content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links" - assert_equal [], assigns[:content_item].content_item["links"]["ordered_related_items"] - end - - test "gets item from content store and replaces ordered_related_items when feature flag header is specified and there are no existing links or overrides" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "true" - + test "gets item from content store and replaces ordered_related_items there are no existing links or overrides" do content_item = content_store_has_schema_example("case_study", "case_study") get :show, params: { path: path_for(content_item) } @@ -194,38 +163,6 @@ class ContentItemsControllerTest < ActionController::TestCase assert_equal assigns[:content_item].content_item["links"]["ordered_related_items"], content_item["links"]["suggested_ordered_related_items"] end - test "sets the Govuk-Use-Recommended-Links response header to false when request header is not set" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - content_item = content_store_has_schema_example("case_study", "case_study") - - get :show, params: { path: path_for(content_item) } - - assert_includes response.headers["Vary"], FeatureFlagNames.recommended_related_links - assert_equal "false", response.headers[FeatureFlagNames.recommended_related_links] - end - - test "sets the Govuk-Use-Recommended-Links response header to false when request header is set to false" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "false" - content_item = content_store_has_schema_example("case_study", "case_study") - - get :show, params: { path: path_for(content_item) } - - assert_includes response.headers["Vary"], FeatureFlagNames.recommended_related_links - assert_equal "false", response.headers[FeatureFlagNames.recommended_related_links] - end - - test "sets the Govuk-Use-Recommended-Links response header to true when request header is set to true" do - HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, "true") - request.headers["HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS"] = "true" - content_item = content_store_has_schema_example("case_study", "case_study") - - get :show, params: { path: path_for(content_item) } - - assert_includes response.headers["Vary"], FeatureFlagNames.recommended_related_links - assert_equal "true", response.headers[FeatureFlagNames.recommended_related_links] - end - test "sets the expiry as sent by content-store" do content_item = content_store_has_schema_example("case_study", "case_study") stub_content_store_has_item(content_item["base_path"], content_item, max_age: 20) diff --git a/test/models/feature_toggler_test.rb b/test/models/feature_toggler_test.rb deleted file mode 100644 index bcd633f90..000000000 --- a/test/models/feature_toggler_test.rb +++ /dev/null @@ -1,66 +0,0 @@ -require "test_helper" - -class FeatureTogglerTest < ActiveSupport::TestCase - test "use_recommended_related_links returns false when content item has no links attribute" do - content_item = {} - instance = setup_feature_toggler_with_feature_enabled(true) - - use_recommended_links = instance.use_recommended_related_links?(content_item["links"], @request_headers) - - assert_equal(false, use_recommended_links) - end - - test "use_recommended_related_links returns false when content item has existing ordered_related_items" do - content_item = { "links" => { "ordered_related_items" => [{ "content_id" => "1234" }] } } - instance = setup_feature_toggler_with_feature_enabled(true) - - use_recommended_links = instance.use_recommended_related_links?(content_item["links"], @request_headers) - - assert_equal(false, use_recommended_links) - end - - test "use_recommended_related_links returns false when headers are not correct" do - content_item = { "links" => {} } - instance = setup_feature_toggler_with_feature_enabled(false) - - use_recommended_links = instance.use_recommended_related_links?(content_item["links"], {}) - - assert_equal(false, use_recommended_links) - end - - test "use_recommended_related_links returns true when content item has no ordered_related_items attribute and headers are correct" do - content_item = { "links" => {} } - instance = setup_feature_toggler_with_feature_enabled(true) - - use_recommended_links = instance.use_recommended_related_links?(content_item["links"], @request_headers) - - assert_equal(true, use_recommended_links) - end - - test "use_recommended_related_links returns true when content item has no items in ordered_related_items attribute and headers are correct" do - content_item = { "links" => { "ordered_related_items" => [] } } - instance = setup_feature_toggler_with_feature_enabled(true) - - use_recommended_links = instance.use_recommended_related_links?(content_item["links"], @request_headers) - - assert_equal(true, use_recommended_links) - end - - test "feature_flags attr_reader delegates to instance of feature_flags" do - feature_flags = HttpFeatureFlags.new - feature_toggler = FeatureToggler.new(feature_flags) - - assert_equal feature_flags, feature_toggler.feature_flags - end - - def setup - @request_headers = { 'HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS': "true" } - end - - def setup_feature_toggler_with_feature_enabled(feature_enabled) - feature_flags = HttpFeatureFlags.new - feature_flags.stubs(:feature_enabled?).returns(feature_enabled) - - FeatureToggler.new(feature_flags) - end -end diff --git a/test/models/http_feature_flags_test.rb b/test/models/http_feature_flags_test.rb deleted file mode 100644 index 3abe9848c..000000000 --- a/test/models/http_feature_flags_test.rb +++ /dev/null @@ -1,72 +0,0 @@ -require "test_helper" - -class HttpFeatureFlagsTest < ActiveSupport::TestCase - test "instance should create a singleton instance" do - instance = HttpFeatureFlags.instance - instance.add_http_feature_flag("TEST_HEADER", "show") - - new_instance = HttpFeatureFlags.instance - feature_enabled = new_instance.feature_enabled?("TEST_HEADER", "HTTP_TEST_HEADER" => "show") - - assert_equal(true, feature_enabled) - end - - test "add_http_feature_flag should set a new feature flag" do - instance = HttpFeatureFlags.new - - feature_enabled = instance.feature_enabled?("USE_MAGIC", "HTTP_USE_MAGIC" => "only_at_weekends") - assert_equal(false, feature_enabled) - - instance.add_http_feature_flag("USE_MAGIC", "only_at_weekends") - feature_enabled = instance.feature_enabled?("USE_MAGIC", "HTTP_USE_MAGIC" => "only_at_weekends") - assert_equal(true, feature_enabled) - end - - test "feature_enabled? should return false when feature flag has not been set" do - instance = HttpFeatureFlags.new - - feature_enabled = instance.feature_enabled?("USE_MAGIC", "HTTP_USE_MAGIC" => "only_at_weekends") - assert_equal(false, feature_enabled) - end - - test "feature_enabled? should return false when header has not been set" do - instance = HttpFeatureFlags.new - - instance.add_http_feature_flag("USE_MAGIC", "only_at_weekends") - feature_enabled = instance.feature_enabled?("USE_MAGIC", {}) - assert_equal(false, feature_enabled) - end - - test "feature_enabled? should return false when header has been set but does not match specified value" do - instance = HttpFeatureFlags.new - - instance.add_http_feature_flag("USE_MAGIC", "only_at_weekends") - feature_enabled = instance.feature_enabled?("USE_MAGIC", "HTTP_USE_MAGIC" => "all_the_time") - assert_equal(false, feature_enabled) - end - - test "feature_enabled? should return true when headers has been set and matches specified value" do - instance = HttpFeatureFlags.new - - instance.add_http_feature_flag("USE_MAGIC", "only_at_weekends") - feature_enabled = instance.feature_enabled?("USE_MAGIC", "HTTP_USE_MAGIC" => "only_at_weekends") - assert_equal(true, feature_enabled) - end - - test "get_feature_flag returns nil when feature flag does not exist" do - instance = HttpFeatureFlags.new - - feature_flag_value = instance.get_feature_flag("USE_MAGIC") - - assert_nil feature_flag_value - end - - test "get_feature_flag returns feature flag value when feature flag exists" do - instance = HttpFeatureFlags.new - - instance.add_http_feature_flag("USE_MAGIC", "only_at_weekends") - feature_flag_value = instance.get_feature_flag("USE_MAGIC") - - assert_equal "only_at_weekends", feature_flag_value - end -end