Skip to content

Commit

Permalink
[WEBDEV-776][WEBDEV-768] Fix url generation and standardise page meta
Browse files Browse the repository at this point in the history
* Refactored serialisers to build meta information from the request object
* Refactored controllers to send the request event
* Updated all fixtures
  • Loading branch information
mattrayner committed Oct 3, 2018
1 parent 2a72b8d commit 4410d4d
Show file tree
Hide file tree
Showing 70 changed files with 220 additions and 187 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@
/doc
/.yardoc

.DS_Store
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ GEM
parliament-grom-decorators (0.31.1)
parliament-ntriple (0.4.0)
grom (~> 1.0)
parliament-opensearch (0.6.0)
parliament-opensearch (0.7.0)
activesupport (>= 5.0.0.1)
feedjira (~> 2.1, >= 2.1.2)
parliament-routes (0.6.13)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ApplicationController < ActionController::API

attr_reader :app_insights_request_id

before_action :populate_request_id, :reset_alternates
before_action :reset_alternates

# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
Expand All @@ -35,7 +35,7 @@ def render_page(serializer, response_parameter = response)
render json: serializer.to_h
end

def populate_request_id
@app_insights_request_id = request.env['ApplicationInsights.request.id']
def default_url_options
{ port: nil, protocol: 'https' }
end
end
6 changes: 3 additions & 3 deletions app/controllers/concerns/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ def data_url
self.class::ROUTE_MAP[params[:action].to_sym] || raise(StandardError, "You must provide a ROUTE_MAP proc for #{params[:controller]}##{params[:action]}")
end

# Populates @request with a data url which can be used within controllers.
# Populates @api_request with a data url which can be used within controllers.
def build_request
@request = data_url.call(params)
@api_request = data_url.call(params)

populate_alternates(@request.query_url)
populate_alternates(@api_request.query_url)
end

# Populates @alternates with a list of data formats and corresponding urls
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class GroupsController < ApplicationController
}.freeze

def index
@groups = FilterHelper.filter(@request, 'Group')
@groups = FilterHelper.filter(@api_request, 'Group')

list_components = @groups.map do |group|
paragraph_content = [].tap do |content|
Expand All @@ -22,16 +22,16 @@ def index
).build_card
end

serializer = PageSerializer::ListPageSerializer.new(page_title: 'groups.index.title', list_components: list_components, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::ListPageSerializer.new(request: request, page_title: 'groups.index.title', list_components: list_components, data_alternates: @alternates)

render_page(serializer)
end

def show
@group = FilterHelper.filter(@request, 'Group')
@group = FilterHelper.filter(@api_request, 'Group')
@group = @group.first

serializer = PageSerializer::GroupsShowPageSerializer.new(group: @group, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::GroupsShowPageSerializer.new(request: request, group: @group, data_alternates: @alternates)

render_page(serializer)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/home_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class HomeController < ApplicationController
# This controller renders the home page serializer. It does not currently query the data base.
def index
render_page(PageSerializer::HomePageSerializer.new(request_id: app_insights_request_id, request_original_url: request.original_url))
render_page(PageSerializer::HomePageSerializer.new(request: request))
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ProposedNegativeStatutoryInstrumentsController < ApplicationController
}.freeze

def index
@proposed_negative_statutory_instruments = FilterHelper.filter(@request, 'ProposedNegativeStatutoryInstrumentPaper')
@proposed_negative_statutory_instruments = FilterHelper.filter(@api_request, 'ProposedNegativeStatutoryInstrumentPaper')

list_components = @proposed_negative_statutory_instruments.map do |proposed_negative_statutory_instrument|
CardFactory.new(
Expand All @@ -17,16 +17,16 @@ def index
).build_card
end

serializer = PageSerializer::ListPageSerializer.new(page_title: 'proposed-negative-statutory-instruments.index.title', list_components: list_components, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::ListPageSerializer.new(request: request, page_title: 'proposed-negative-statutory-instruments.index.title', list_components: list_components, data_alternates: @alternates)

render_page(serializer)
end

def show
@proposed_negative_statutory_instrument = FilterHelper.filter(@request, 'ProposedNegativeStatutoryInstrumentPaper')
@proposed_negative_statutory_instrument = FilterHelper.filter(@api_request, 'ProposedNegativeStatutoryInstrumentPaper')
@proposed_negative_statutory_instrument = @proposed_negative_statutory_instrument.first

serializer = PageSerializer::ProposedNegativeStatutoryInstrumentsShowPageSerializer.new(proposed_negative_statutory_instrument: @proposed_negative_statutory_instrument, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::ProposedNegativeStatutoryInstrumentsShowPageSerializer.new(request: request, proposed_negative_statutory_instrument: @proposed_negative_statutory_instrument, data_alternates: @alternates)

render_page(serializer)
end
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/search_controller.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
class SearchController < ApplicationController
def index
# Show the index page if there is no query or an empty string is passed
return render_page(PageSerializer::SearchPage::LandingPageSerializer.new(opensearch_description_url: opensearch_description_url, flash_message: search_service.flash_message, request_id: app_insights_request_id, request_original_url: request.original_url)) unless search_service.sanitised_query.present?
return render_page(PageSerializer::SearchPage::LandingPageSerializer.new(request: request, flash_message: search_service.flash_message)) unless search_service.sanitised_query.present?

search_service.fetch_description

begin
serialiser = PageSerializer::SearchPage::ResultsPageSerializer.new(opensearch_description_url: opensearch_description_url, query: search_service.sanitised_query, results: search_service.results, pagination_hash: search_service.pagination_hash, request_id: app_insights_request_id, request_original_url: request.original_url)
serialiser = PageSerializer::SearchPage::ResultsPageSerializer.new(request: request, query: search_service.sanitised_query, results: search_service.results, pagination_hash: search_service.pagination_hash)

return render_page(serialiser)
rescue Parliament::ServerError => e
logger.warn "Server error caught from search request: #{e.message}"
serialiser = PageSerializer::SearchPage::ResultsPageSerializer.new(opensearch_description_url: opensearch_description_url, query: search_service.sanitised_query, request_id: app_insights_request_id, request_original_url: request.original_url)
serialiser = PageSerializer::SearchPage::ResultsPageSerializer.new(request: request, query: search_service.sanitised_query)

return render_page(serialiser)
end
Expand All @@ -34,6 +34,6 @@ def opensearch
private

def search_service
@search_service ||= SearchService.new(app_insights_request_id, search_path, params)
@search_service ||= SearchService.new(request.env['ApplicationInsights.request.id'], search_path, params)
end
end
8 changes: 4 additions & 4 deletions app/controllers/statutory_instruments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class StatutoryInstrumentsController < ApplicationController
}.freeze

def index
@statutory_instruments = FilterHelper.filter(@request, 'StatutoryInstrumentPaper')
@statutory_instruments = FilterHelper.filter(@api_request, 'StatutoryInstrumentPaper')

list_components = @statutory_instruments.map do |statutory_instrument|
CardFactory.new(
Expand All @@ -17,16 +17,16 @@ def index
).build_card
end

serializer = PageSerializer::ListPageSerializer.new(page_title: 'statutory-instruments.index.title', list_components: list_components, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::ListPageSerializer.new(request: request, page_title: 'statutory-instruments.index.title', list_components: list_components, data_alternates: @alternates)

render_page(serializer)
end

def show
@statutory_instrument = FilterHelper.filter(@request, 'StatutoryInstrumentPaper')
@statutory_instrument = FilterHelper.filter(@api_request, 'StatutoryInstrumentPaper')
@statutory_instrument = @statutory_instrument.first

serializer = PageSerializer::StatutoryInstrumentsShowPageSerializer.new(statutory_instrument: @statutory_instrument, request_id: app_insights_request_id, data_alternates: @alternates, request_original_url: request.original_url)
serializer = PageSerializer::StatutoryInstrumentsShowPageSerializer.new(request: request, statutory_instrument: @statutory_instrument, data_alternates: @alternates)

render_page(serializer)
end
Expand Down
37 changes: 26 additions & 11 deletions app/serializers/page_serializer/base_page_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
module PageSerializer
class BasePageSerializer < BaseSerializer
include OpenGraphHelper
attr_reader :request_id, :data_alternates
attr_reader :request, :request_id, :data_alternates, :request_original_url
include ActionDispatch::Routing::UrlFor

def initialize(request_id: nil, data_alternates: nil, request_original_url: nil)
@request_id = request_id if request_id
# @param [ActionDispatch::Request] request the current request object
def initialize(request: nil, data_alternates: nil)
@request = request
@request_id = request.try(:env)&.fetch('ApplicationInsights.request.id', nil) if request
@data_alternates = data_alternates
@request_original_url = request_original_url
@request_original_url = generate_original_url
end

# Creates a hash of the serializer's content
Expand Down Expand Up @@ -54,19 +57,31 @@ def footer_section(hash)
end
end

def opensearch_description_url
raise 'You must implement #opensearch_description_url'
end

def meta(title: nil, image_id: nil)
{}.tap do |meta|
meta[:title] = title
meta[:request_id] = @request_id if @request_id
meta[:data_alternates] = @data_alternates
meta[:open_graph] = OpenGraphHelper.information(page_title: title, request_original_url: @request_original_url, image_id: image_id)
meta[:request_id] = request_id if request_id
meta[:data_alternates] = data_alternates if data_alternates

meta[:open_graph] = OpenGraphHelper.information(page_title: title, request_original_url: request_original_url, image_id: image_id)

meta[:opensearch_description_url] = url_for(host: request.try(:host), protocol: 'https', port: nil, controller: 'search', action: 'opensearch') if request.try(:host)
end
end

def generate_original_url
begin
uri_object = URI.parse(request.try(:original_url))
rescue URI::InvalidURIError
return nil
end

uri_object.port = nil
uri_object.scheme = 'https'

uri_object.to_s
end

# Sets the default for including global search in the header to true.
def include_global_search
true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ module PageSerializer
class GroupsShowPageSerializer < PageSerializer::BasePageSerializer
# Initialise a Groups show page serializer.
#
# @param [ActionDispatch::Request] request the current request object.
# @param [<Grom::Node>] group a Grom::Node object of type StatutoryInstrumentPaper.
# @param [String] request_id AppInsights request id
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls
# @param [String] request_original_url original url of the request
def initialize(group: nil, request_id: nil, data_alternates: nil, request_original_url: nil)
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls.
def initialize(request: nil, group: nil, data_alternates: nil)
@group = group

super(request_id: request_id, data_alternates: data_alternates, request_original_url: request_original_url)
super(request: request, data_alternates: data_alternates)
end

private
Expand Down
12 changes: 0 additions & 12 deletions app/serializers/page_serializer/home_page_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
module PageSerializer
class HomePageSerializer < PageSerializer::BasePageSerializer
# This serializer includes HousesHelper
# Initialise a Home index page serializer.
#
# @param [String] opensearch_description_url a description url for the search.
def initialize(opensearch_description_url: nil, request_id: nil, request_original_url: nil)
@opensearch_description_url = opensearch_description_url

super(request_id: request_id, request_original_url: request_original_url)
end

private

attr_reader :opensearch_description_url

def meta
super(title: 'beta.parliament.uk')
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@ module PageSerializer
class LaidThingShowPageSerializer < PageSerializer::BasePageSerializer
# Initialise a Laid Thing show page serializer.
#
# @param [ActionDispatch::Request] request the current request object.
# @param [<Grom::Node>] laid_thing a Grom::Node object of type LaidThing.
# @param [String] request_id AppInsights request id
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls
# @param [String] request_original_url original url of the request
def initialize(laid_thing:, request_id: nil, data_alternates: nil, request_original_url: nil)
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls.
def initialize(request: nil, laid_thing:, data_alternates: nil)
@laid_thing = laid_thing
@work_package = @laid_thing.work_package
@laying_body = @laid_thing&.laying&.body
@laying_person = @laid_thing&.laying&.person
@procedure = @work_package&.procedure

super(request_id: request_id, data_alternates: data_alternates, request_original_url: request_original_url)
super(request: request, data_alternates: data_alternates)
end

private
Expand Down
9 changes: 4 additions & 5 deletions app/serializers/page_serializer/list_page_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ module PageSerializer
class ListPageSerializer < PageSerializer::BasePageSerializer
# Initialise a list page serializer.
#
# @param [ActionDispatch::Request] request the current request object
# @param [String] page_title title of the page
# @param [Array<Hash>] list_components an array of components to be passed into the list
# @param [String] request_id AppInsights request id
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls
# @param [String] request_original_url original url of the request
def initialize(page_title: nil, list_components: nil, request_id: nil, data_alternates: nil, request_original_url: nil)
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data url
def initialize(request: nil, page_title: nil, list_components: nil, data_alternates: nil)
@page_title = page_title
@list_components = list_components
@data_alternates = data_alternates

super(request_id: request_id, data_alternates: data_alternates, request_original_url: request_original_url)
super(request: request, data_alternates: data_alternates)
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ module PageSerializer
class ProposedNegativeStatutoryInstrumentsShowPageSerializer < LaidThingShowPageSerializer
# Initialise a Proposed Negative Statutory Instruments show page serializer.
#
# @param [ActionDispatch::Request] request the current request object.
# @param [<Grom::Node>] proposed_negative_statutory_instrument a Grom::Node object of type ProposedNegativeStatutoryInstrumentPaper.
# @param [String] request_id AppInsights request id
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls
# @param [String] request_original_url original url of the request
def initialize(proposed_negative_statutory_instrument:, request_id: nil, data_alternates: nil, request_original_url: nil)
def initialize(request: nil, proposed_negative_statutory_instrument:, data_alternates: nil)
@proposed_negative_statutory_instrument = proposed_negative_statutory_instrument
@following_statutory_instruments = @proposed_negative_statutory_instrument.statutory_instrument_papers

super(laid_thing: @proposed_negative_statutory_instrument, request_id: request_id, data_alternates: data_alternates, request_original_url: request_original_url)
super(request: request, laid_thing: @proposed_negative_statutory_instrument, data_alternates: data_alternates)
end

private
Expand Down
7 changes: 3 additions & 4 deletions app/serializers/page_serializer/search_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,20 @@ module PageSerializer
class SearchPage < PageSerializer::BasePageSerializer
# Initialise a Search index page serializer.
#
# @param [ActionDispatch::Request] request the current request object.
# @param [String] opensearch_description_url a description url for the search.
# @param [String] query a query string used for the search.
# @param [Array<Object>] results an array of objects used for displaying results.
# @param [Hash] pagination_hash a hash containing data used for pagination.
# @param [String] flash_message a translation block that is evaluated into a flash message.
# @param [String] request_id AppInsights request id
# @param [String] request_original_url original url of the request
def initialize(opensearch_description_url: nil, query: nil, results: nil, pagination_hash: nil, flash_message: nil, request_id: nil, request_original_url: nil)
def initialize(request: nil, opensearch_description_url: nil, query: nil, results: nil, pagination_hash: nil, flash_message: nil)
@opensearch_description_url = opensearch_description_url
@query = query
@results = results
@pagination_helper = PaginationHelper.new(pagination_hash) if pagination_hash
@flash_message = flash_message

super(request_id: request_id, data_alternates: nil, request_original_url: request_original_url)
super(request: request, data_alternates: nil)
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ class LandingPageSerializer < PageSerializer::SearchPage
private

def meta
super(title: title).tap do |meta|
meta[:opensearch_description_url] = opensearch_description_url if opensearch_description_url
end
super(title: title)
end

def title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ module PageSerializer
class StatutoryInstrumentsShowPageSerializer < LaidThingShowPageSerializer
# Initialise a Statutory Instruments show page serializer.
#
# @param [ActionDispatch::Request] request the current request object.
# @param [<Grom::Node>] statutory_instrument a Grom::Node object of type StatutoryInstrumentPaper.
# @param [String] request_id AppInsights request id
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls
# @param [String] request_original_url original url of the request
def initialize(statutory_instrument:, request_id: nil, data_alternates: nil, request_original_url: nil)
# @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls.
def initialize(request: nil, statutory_instrument:, data_alternates: nil)
@statutory_instrument = statutory_instrument
@preceding_proposed_negative_statutory_instruments = @statutory_instrument.proposed_negative_statutory_instrument_papers

super(laid_thing: @statutory_instrument, request_id: request_id, data_alternates: data_alternates, request_original_url: request_original_url)
super(request: request, laid_thing: @statutory_instrument, data_alternates: data_alternates)
end

private
Expand Down
Loading

0 comments on commit 4410d4d

Please sign in to comment.