Merge pull request #73 from mattrayner/master

[WEBDEV-776][WEBDEV-768] Fix url generation and standardise page meta

.gitignore

@@ -38,3 +38,4 @@
 /doc
 /.yardoc
 
+.DS_Store

Gemfile.lock

@@ -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)

app/controllers/application_controller.rb

@@ -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.
@@ -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

app/controllers/concerns/application_helper.rb

@@ -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

app/controllers/groups_controller.rb

@@ -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|
@@ -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

app/controllers/home_controller.rb

@@ -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

app/controllers/proposed_negative_statutory_instruments_controller.rb

@@ -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(
@@ -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

app/controllers/search_controller.rb

@@ -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
@@ -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

app/controllers/statutory_instruments_controller.rb

@@ -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(
@@ -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

app/serializers/page_serializer/base_page_serializer.rb

@@ -1,12 +1,16 @@
 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.
+    # @param [Array<Hash>] data_alternates array containing the href and type of the alternative data urls.
+    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
@@ -54,19 +58,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

app/serializers/page_serializer/groups_show_page_serializer.rb

@@ -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

app/serializers/page_serializer/home_page_serializer.rb

@@ -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

app/serializers/page_serializer/laid_thing_show_page_serializer.rb

@@ -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

app/serializers/page_serializer/list_page_serializer.rb

@@ -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

app/serializers/page_serializer/proposed_negative_statutory_instruments_show_page_serializer.rb

@@ -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

app/serializers/page_serializer/search_page.rb

@@ -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

app/serializers/page_serializer/search_page/landing_page_serializer.rb

@@ -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

app/serializers/page_serializer/statutory_instruments_show_page_serializer.rb

@@ -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