From 4e92118f6bcac2a5dbcefa40dc9e5a9584002085 Mon Sep 17 00:00:00 2001 From: Karl Baker Date: Thu, 20 Jun 2019 10:03:42 +0100 Subject: [PATCH] Display new recommended related links This commit adds support for the new recommended related links to be shown based on the existence of a HTTP header set by our CDN. To achieve this we add support in general for feature flags via a `FeatureToggler` class. Should the header exist, the new recommended related links will only be shown if there are no existing `ordered_related_items` in the content item, to ensure that we do not override manually curated links. Solo: @karlbaker02 --- app/controllers/content_items_controller.rb | 5 ++ app/lib/services.rb | 6 ++ app/models/feature_toggler.rb | 12 ++++ app/models/http_feature_flags.rb | 17 ++++++ config/initializers/feature_flags.rb | 7 +++ .../content_items_controller_test.rb | 32 ++++++++++ test/models/feature_toggler_test.rb | 59 +++++++++++++++++++ test/models/http_feature_flags_test.rb | 55 +++++++++++++++++ 8 files changed, 193 insertions(+) create mode 100644 app/models/feature_toggler.rb create mode 100644 app/models/http_feature_flags.rb create mode 100644 config/initializers/feature_flags.rb create mode 100644 test/models/feature_toggler_test.rb create mode 100644 test/models/http_feature_flags_test.rb diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index a3d0eb42d..809c5530c 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -56,6 +56,11 @@ 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 = PresenterBuilder.new(content_item, content_item_path).presenter end diff --git a/app/lib/services.rb b/app/lib/services.rb index 7c1b2fa84..7315084ce 100644 --- a/app/lib/services.rb +++ b/app/lib/services.rb @@ -10,4 +10,10 @@ def self.content_store disable_cache: true, ) end + + def self.feature_toggler + @feature_toggler ||= FeatureToggler.new( + HttpFeatureFlags.instance + ) + end end diff --git a/app/models/feature_toggler.rb b/app/models/feature_toggler.rb new file mode 100644 index 000000000..47dc3949f --- /dev/null +++ b/app/models/feature_toggler.rb @@ -0,0 +1,12 @@ +class FeatureToggler + 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? && + @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 new file mode 100644 index 000000000..f85f43a81 --- /dev/null +++ b/app/models/http_feature_flags.rb @@ -0,0 +1,17 @@ +class HttpFeatureFlags + def initialize + @feature_flags = {} + end + + def add_http_feature_flag(header_name, val) + @feature_flags[header_name] = val + end + + def feature_enabled?(header_name, request_headers) + @feature_flags.has_key?(header_name) && @feature_flags[header_name] == request_headers[header_name] + end + + def self.instance + @instance ||= HttpFeatureFlags.new + end +end diff --git a/config/initializers/feature_flags.rb b/config/initializers/feature_flags.rb new file mode 100644 index 000000000..64278180c --- /dev/null +++ b/config/initializers/feature_flags.rb @@ -0,0 +1,7 @@ +module FeatureFlagNames + def self.recommended_related_links + 'HTTP_GOVUK_USE_RECOMMENDED_RELATED_LINKS' + end +end + +HttpFeatureFlags.instance.add_http_feature_flag(FeatureFlagNames.recommended_related_links, 'true') diff --git a/test/controllers/content_items_controller_test.rb b/test/controllers/content_items_controller_test.rb index 12d5a4e16..85d96cb29 100644 --- a/test/controllers/content_items_controller_test.rb +++ b/test/controllers/content_items_controller_test.rb @@ -130,6 +130,38 @@ 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 + 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 + request.headers[FeatureFlagNames.recommended_related_links] = 'true' + + content_item = content_store_has_schema_example('guide', 'guide') + + get :show, params: { path: path_for(content_item) } + assert_response :success + refute_empty content_item['links']['ordered_related_items'], 'Content item should have existing related links' + refute_empty content_item['links']['suggested_ordered_related_items'], 'Content item should have existing suggested related links' + 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 replaces ordered_related_items when feature flag header is specified and there are no existing links" do + request.headers[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_empty content_item['links']['ordered_related_items'], 'Content item should not have existing related links' + refute_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'], content_item['links']['suggested_ordered_related_items'] + end + test "sets the expiry as sent by content-store" do content_item = content_store_has_schema_example('coming_soon', 'coming_soon') 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 new file mode 100644 index 000000000..e2df0288c --- /dev/null +++ b/test/models/feature_toggler_test.rb @@ -0,0 +1,59 @@ +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 + + 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 new file mode 100644 index 000000000..c0bb70fac --- /dev/null +++ b/test/models/http_feature_flags_test.rb @@ -0,0 +1,55 @@ +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', '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', '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', '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', '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', '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', 'USE_MAGIC' => 'only_at_weekends') + assert_equal(true, feature_enabled) + end +end