Skip to content

Commit

Permalink
Remove HTTP feature flags
Browse files Browse the repository at this point in the history
The rendering of recommended related links was conditional on a feature flag,
which is set via a header within our Fastly VCL.

As an outcome of RFC-163, we are trying to simplify our CDN configuration by
removing functionality that’s no longer used, or by moving logic to other parts
of the stack.

This change was suggested in a comment on RFC-163:

  "I think the flag for Whitehall recommended links should be removed. It was put
  in as a safety mechanism for the introduction of the links, in case something
  really inappropriate was found. There are ways to manually override the links
  now, and we've never used the feature flag."

Related content is component is defined in govuk_publishing_components gem and
this is where the overriding is happening (see:
https://github.com/alphagov/govuk_publishing_components/blob/5d713ff637de14bec6abc784c35ee63a0cbc2c75/lib/govuk_publishing_components/presenters/content_item.rb#L70-L74)

It's now recommended to create feature flags as environment variables in helm
charts. See the docs:
https://github.com/alphagov/govuk-developer-docs/blob/33f1f0434aa988419df0687a829367e3f769816f/source/kubernetes/manage-app/set-env-var/index.html.md?plain=1#L7-L21

The value can then be read in Rails config:

```
config.unreleased_features = ENV["UNRELEASED_FEATURES"].present?
```
  • Loading branch information
AgaDufrat committed Sep 7, 2023
1 parent dae4e22 commit ba01552
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 267 deletions.
22 changes: 10 additions & 12 deletions app/controllers/content_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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),
Expand Down
6 changes: 0 additions & 6 deletions app/lib/services.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
15 changes: 0 additions & 15 deletions app/models/feature_toggler.rb

This file was deleted.

21 changes: 0 additions & 21 deletions app/models/http_feature_flags.rb

This file was deleted.

9 changes: 0 additions & 9 deletions config/initializers/feature_flags.rb

This file was deleted.

69 changes: 3 additions & 66 deletions test/controllers/content_items_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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) }
Expand All @@ -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) }
Expand All @@ -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)
Expand Down
66 changes: 0 additions & 66 deletions test/models/feature_toggler_test.rb

This file was deleted.

72 changes: 0 additions & 72 deletions test/models/http_feature_flags_test.rb

This file was deleted.

0 comments on commit ba01552

Please sign in to comment.