Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local content item support #4366

Merged
merged 6 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"name": "Frontend",
"repository": "https://github.com/alphagov/frontend",
"env": {
"ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE": {
"value": "true"
},
"GOVUK_APP_DOMAIN": {
"value": "www.gov.uk"
},
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/calendar_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ def division

helper_method :calendar

def content_item_path
"/#{params[:slug]}"
end

def set_cors_headers
headers["Access-Control-Allow-Origin"] = "*"
end
Expand Down
8 changes: 2 additions & 6 deletions app/controllers/content_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,17 @@ def publication_class
end

def content_item
@content_item ||= ContentItemFactory.build(request.env[:content_item] || request_content_item(content_item_slug || "/#{params[:slug]}"))
@content_item ||= ContentItemFactory.build(ContentItemLoader.load(content_item_path))
end

def content_item_slug
def content_item_path
request.path
end

def content_item_hash
@content_item_hash ||= content_item.to_h
end

def request_content_item(base_path = "/#{params[:slug]}")
GdsApi.content_store.content_item(base_path)
end

# NOTE: Frontend honours the max-age directive provided
# in Content Store's Cache-Control response header.
def set_expiry(expiry = nil)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/electoral_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def publication_class
LocalTransactionPresenter
end

def content_item_slug
def content_item_path
BASE_PATH_OF_EXISTING_CONTACT_LOCAL_ERO_SERVICE
end

Expand Down
6 changes: 4 additions & 2 deletions app/controllers/error_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class ErrorController < ApplicationController
def handler
# defer any errors to be handled in ApplicationController
raise request.env[:content_item_error]
# We know at this point that the ContentItemLoader has stored
# an exception to deal with, so just retrieve it and raise it
# to be handled in ApplicationController
raise ContentItemLoader.load(request.path)
end
end
2 changes: 1 addition & 1 deletion app/controllers/find_local_council_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def result

private

def content_item_slug
def content_item_path
BASE_PATH
end

Expand Down
6 changes: 0 additions & 6 deletions app/controllers/help_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,4 @@ def sign_in
},
]
end

private

def content_item_slug
request.path
end
end
7 changes: 0 additions & 7 deletions app/controllers/help_page_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,2 @@
class HelpPageController < ContentItemsController
def show; end

private

def content_item_slug
request.path
end
end
13 changes: 10 additions & 3 deletions app/controllers/landing_page_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ class LandingPageController < ContentItemsController

# SCAFFOLDING: can be removed when basic content items are available
# from content-store
def request_content_item(_base_path)
GdsApi.content_store.content_item(request.path).to_h
rescue StandardError
def content_item
@content_item ||= ContentItemFactory.build(old_scaffolding_content_item)
end

# SCAFFOLDING: can be removed when basic content items are available
# from content-store
def old_scaffolding_content_item
result = ContentItemLoader.load(request.path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that you've fixed this. I think it would be good to add a test for this scaffolding too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The good news is that the existing request tests for landing_page already cover the old scaffolding (tests written... checks blame ... one @leenagupte)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can't have been thorough enough because nothing caught the attempted call to ContentItemLoaderGdsApi.content_store.content_item(request.path).to_h which doesn't exist!

return result.to_h if result.is_a?(GdsApi::Response)

fake_data
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/licence_transaction_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def redirect_to_continuation_licence
end
end

def content_item_slug
def content_item_path
"/find-licences/#{params[:slug]}"
end

Expand Down
4 changes: 4 additions & 0 deletions app/controllers/local_transaction_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ def translated_slug
local_authority_slug
end

def content_item_path
"/#{params[:slug]}"
end

def publication_class
LocalTransactionPresenter
end
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/simple_smart_answers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def flow
:smart_answer_path_for_responses,
)

def content_item_path
"/#{params[:slug]}"
end

def publication_class
SimpleSmartAnswerPresenter
end
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/transaction_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ def show

private

def content_item_path
"/#{params[:slug]}"
end

def publication_class
TransactionPresenter
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/travel_advice_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def index
# These objects duplicate roles (see `presenter || @publication`) in views.
def publication; end

def content_item_slug
def content_item_path
"/#{FOREIGN_TRAVEL_ADVICE_SLUG}"
end
end
11 changes: 11 additions & 0 deletions docs/local-content-items.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Local Content Items

For local development and preview apps, it is sometimes desirable to have the ability to load content items that are specified locally rather than pointing to production/integration or having the content store running locally.

To support this, set:

`ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE=true`
leenagupte marked this conversation as resolved.
Show resolved Hide resolved

...in your environment.

With that environment variable set, the ContentItemLoader will add an additional check when loading a content item from the content store. Before making the GdsApi content store call, it will look in config/local-content-items/ for a JSON or YAML file matching the path of the content item (for instance, if you're looking for /find-licences/my-licence, it will look for config/local-content-items/find-licences/my-licence.json, then config/local-content-items/find-licences/my-licence.yml)
2 changes: 1 addition & 1 deletion lib/api_error_routing_constraint.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class ApiErrorRoutingConstraint
def matches?(request)
request.env[:content_item_error].present?
ContentItemLoader.load(request.path).is_a?(StandardError)
end
end
53 changes: 53 additions & 0 deletions lib/content_item_loader.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
require "ostruct"

class ContentItemLoader
LOCAL_ITEMS_PATH = "lib/data/local-content-items".freeze

@cache = {}

class << self
attr_reader :cache

def load(base_path)
cache[base_path] ||= if use_local_file? && File.exist?(yaml_filename(base_path))
Rails.logger.debug("Loading content item #{base_path} from #{yaml_filename(base_path)}")
load_yaml_file(base_path)
elsif use_local_file? && File.exist?(json_filename(base_path))
Rails.logger.debug("Loading content item #{base_path} from #{json_filename(base_path)}")
load_json_file(base_path)
else
begin
GdsApi.content_store.content_item(base_path)
rescue GdsApi::HTTPErrorResponse, GdsApi::InvalidUrl => e
e
end
end
end

private

def use_local_file?
ENV["ALLOW_LOCAL_CONTENT_ITEM_OVERRIDE"] == "true"
end

def yaml_filename(base_path)
Rails.root.join("#{LOCAL_ITEMS_PATH}#{base_path}.yml")
end

def load_yaml_file(base_path)
GdsApi::Response.new(OpenStruct.new(code: 200, body: YAML.load(File.read(yaml_filename(base_path))).to_json, headers:))
end

def json_filename(base_path)
Rails.root.join("#{LOCAL_ITEMS_PATH}#{base_path}.json")
end

def load_json_file(base_path)
GdsApi::Response.new(OpenStruct.new(code: 200, body: File.read(json_filename(base_path)), headers:))
end

def headers
{ cache_control: "max-age=0, public", expires: "" }
end
end
end
19 changes: 2 additions & 17 deletions lib/format_routing_constraint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,12 @@ def initialize(format)
end

def matches?(request)
content_item = set_content_item(request)
@format == content_item&.[]("schema_name")
end

def set_content_item(request)
return request.env[:content_item] if already_cached?(request)

begin
request.env[:content_item] = GdsApi.content_store.content_item(key(request))
rescue GdsApi::HTTPErrorResponse, GdsApi::InvalidUrl => e
request.env[:content_item_error] = e
nil
end
content_item = ContentItemLoader.load(key(request))
content_item.is_a?(GdsApi::Response) && content_item["schema_name"] == @format
end

private

def already_cached?(request)
request.env.include?(:content_item) || request.env.include?(:content_item_error)
end

def key(request)
"/#{request.params.fetch(:slug)}"
end
Expand Down
4 changes: 0 additions & 4 deletions lib/full_path_format_routing_constraint.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
class FullPathFormatRoutingConstraint < FormatRoutingConstraint
private

def already_cached?(_request)
false
end

def key(request)
request.path
end
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/local-content-items/my-json-item.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"schema_name": "json_page"
}
1 change: 1 addition & 0 deletions spec/fixtures/local-content-items/my-yaml-item.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
schema_name: yaml_page
6 changes: 6 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
config.infer_spec_type_from_file_location!
config.include FactoryBot::Syntax::Methods

config.include ContentItemLoaderHelpers

config.before(:each) do
clear_content_item_loader_cache
end

config.include ContentStoreHelpers, type: :request
config.include ContentStoreHelpers, type: :system

Expand Down
5 changes: 5 additions & 0 deletions spec/support/content_item_loader_helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module ContentItemLoaderHelpers
def clear_content_item_loader_cache
ContentItemLoader.cache.each_key { |key| ContentItemLoader.cache.delete(key) }
end
end
2 changes: 2 additions & 0 deletions spec/support/meta_tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
)
example_doc["links"]["available_translations"] = []

clear_content_item_loader_cache
stub_content_store_has_item(path, example_doc.to_json)
end

Expand All @@ -28,6 +29,7 @@
)
example_doc["links"]["available_translations"] = []

clear_content_item_loader_cache
stub_content_store_has_item(path, example_doc.to_json)
end

Expand Down
17 changes: 7 additions & 10 deletions spec/unit/api_error_routing_constraint_spec.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
require "api_error_routing_constraint"

RSpec.describe ApiErrorRoutingConstraint do
include ContentStoreHelpers

let(:subject) { described_class.new }
let(:request) { double(path: "/slug") }

it "returns true if there's a cached error" do
request = double(env: { content_item_error: StandardError.new })
stub_content_store_does_not_have_item("/slug")

expect(subject.matches?(request)).to be true
end

it "returns false if there was no error in API calls" do
request = double(env: {})
stub_content_store_has_item("/slug")

expect(subject.matches?(request)).to be false
end

private

def subject
ApiErrorRoutingConstraint.new
end
end
Loading