Merge pull request #194 from ukparliament/development

Development merge

app/controllers/content_objects_controller.rb

@@ -21,28 +21,32 @@ def show
         raise 'unknown error occurred'
       end
     elsif @response['docs'].blank?
-      # API responds with success but no results - show a 404
       render template: 'layouts/shared/error/404', locals: { status: 404, message: 'Page not found' }
     else
       object_data = @response['docs'].first
-      @object = ContentObject.generate(object_data)
 
-      associated_object_query = AssociatedObjects.new(@object).data
-      @associated_object_data = associated_object_query[:object_data]
+      if object_data.dig('type_ses').blank?
+        render template: 'layouts/shared/error/404', locals: { status: 404, message: 'Page not found' }
+      else
+        @object = ContentObject.generate(object_data)
 
-      # Combine SES IDs from associated objects if any
-      associated_ses_ids = associated_object_query&.dig(:ses_ids)
-      all_ses_ids = associated_ses_ids.blank? ? @object.ses_lookup_ids : @object.ses_lookup_ids + associated_ses_ids
+        associated_object_query = AssociatedObjects.new(@object).data
+        @associated_object_data = associated_object_query[:object_data]
 
-      # Single SES lookup using all IDs
-      @ses_data = SesLookup.new(all_ses_ids).data
+        # Combine SES IDs from associated objects if any
+        associated_ses_ids = associated_object_query&.dig(:ses_ids)
+        all_ses_ids = associated_ses_ids.blank? ? @object.ses_lookup_ids : @object.ses_lookup_ids + associated_ses_ids
 
-      @page_title = @object.object_title
+        # Single SES lookup using all IDs
+        @ses_data = SesLookup.new(all_ses_ids).data
 
-      if @ses_data&.has_key?(:error)
-        render template: 'layouts/shared/error/500', locals: { status: 500, message: 'There was an error resolving names using the SES service' }
-      else
-        render template: @object.template, :locals => { :object => @object }
+        @page_title = @object.object_title
+
+        if @ses_data&.has_key?(:error)
+          render template: 'layouts/shared/error/500', locals: { status: 500, message: 'There was an error resolving names using the SES service' }
+        else
+          render template: @object.template, :locals => { :object => @object }
+        end
       end
     end
 

app/models/api_call.rb

@@ -12,7 +12,7 @@ def initialize(params)
   end
 
   def object_data
-    all_data['response']['docs']
+    all_data['response']['docs']&.reject{|h| h.dig('type_ses').blank?}
   end
 
   def all_data

app/models/search_data.rb

@@ -49,7 +49,7 @@ def error_partial_path
   def object_data
     return unless search
 
-    search.dig(:data, 'response', 'docs')
+    search.dig(:data, 'response', 'docs')&.reject { |h| h.dig('type_ses').blank? }
   end
 
   def objects

app/views/search/index.html.erb

@@ -90,7 +90,7 @@
               of
               <strong><%= number_to_delimited(@search_data.number_of_results, separator: ",") %></strong> in
               <strong>
-                <%= "#{(Time.now - @start_time).round(3)} seconds" %>.
+                <%= "#{(Time.now - @start_time).round(2)} seconds" %>.
               </strong>
             </p>
 

coverage/.last_run.json

@@ -1,5 +1,5 @@
 {
   "result": {
-    "line": 80.23
+    "line": 80.25
   }
 }

spec/models/search_data_spec.rb

@@ -18,9 +18,9 @@
                                "numFound" => 3,
                                "start" => 0,
                                "docs" => [
-                                 { 'test_string' => 'test string 1', 'uri' => 'test1' },
-                                 { 'test_string' => 'test string 2', 'uri' => 'test2' },
-                                 { 'test_string' => 'test string 3', 'uri' => 'test3' },
+                                 { 'type_ses' => [12345], 'test_string' => 'test string 1', 'uri' => 'test1' },
+                                 { 'type_ses' => [23456], 'test_string' => 'test string 2', 'uri' => 'test2' },
+                                 { 'type_ses' => [34567], 'test_string' => 'test string 3', 'uri' => 'test3' },
                                ]
                              },
                              "highlighting" => { "test_url" => {} },
@@ -93,9 +93,38 @@
     context 'where data is present' do
       it 'returns the array of docs' do
         expect(search_data.object_data).to eq([
-                                                { 'test_string' => 'test string 1', 'uri' => 'test1' },
-                                                { 'test_string' => 'test string 2', 'uri' => 'test2' },
-                                                { 'test_string' => 'test string 3', 'uri' => 'test3' },
+                                                { 'type_ses' => [12345], 'test_string' => 'test string 1', 'uri' => 'test1' },
+                                                { 'type_ses' => [23456], 'test_string' => 'test string 2', 'uri' => 'test2' },
+                                                { 'type_ses' => [34567], 'test_string' => 'test string 3', 'uri' => 'test3' },
+                                              ])
+      end
+    end
+    context 'where type_ses is missing' do
+      let!(:search_output) { { search_parameters: { filter: ['topic_ses:12345'], query: 'horse' },
+                               data: {
+                                 "responseHeader" => {
+                                   "status" => 0,
+                                   "QTime" => 4,
+                                   "params" => { "q" => "externalLocation_uri:\"test_external_location_uri\"", "wt" => "json" }
+                                 },
+                                 "response" => {
+                                   "numFound" => 3,
+                                   "start" => 0,
+                                   "docs" => [
+                                     { 'type_ses' => [12345], 'test_string' => 'test string 1', 'uri' => 'test1' },
+                                     { 'test_string' => 'test string 2', 'uri' => 'test2' },
+                                     { 'type_ses' => [34567], 'test_string' => 'test string 3', 'uri' => 'test3' },
+                                   ]
+                                 },
+                                 "highlighting" => { "test_url" => {} },
+                                 "facets" => facet_data
+                               },
+      } }
+
+      it 'returns only data that has type_ses present' do
+        expect(search_data.object_data).to eq([
+                                                { 'type_ses' => [12345], 'test_string' => 'test string 1', 'uri' => 'test1' },
+                                                { 'type_ses' => [34567], 'test_string' => 'test string 3', 'uri' => 'test3' },
                                               ])
       end
     end

spec/models/solr_multi_query_spec.rb

@@ -11,19 +11,19 @@
     "response" => {
       "numFound" => 1,
       "start" => 0,
-      "docs" => [{ 'test_string' => 'test1', 'uri' => 'test_uri1', 'all_ses' => [123, 456] },
-                 { 'test_string' => 'test2', 'uri' => 'test_uri2', 'all_ses' => [456, 789] },
-                 { 'test_string' => 'test3', 'uri' => 'test_uri3', 'all_ses' => [234, 567] },
+      "docs" => [{ 'test_string' => 'test1', 'uri' => 'test_uri1', 'all_ses' => [123, 456], 'type_ses' => [12345] },
+                 { 'test_string' => 'test2', 'uri' => 'test_uri2', 'all_ses' => [456, 789], 'type_ses' => [23456] },
+                 { 'test_string' => 'test3', 'uri' => 'test_uri3', 'all_ses' => [234, 567], 'type_ses' => [34567] },
       ] },
     "highlighting" => { "test_url" => {} }
   } }
 
   describe 'object_data' do
     it 'returns the data for the object' do
       allow(api_call).to receive(:evaluated_response).and_return(mock_response)
-      expect(api_call.object_data).to match_array([{ 'test_string' => 'test1', 'uri' => 'test_uri1', 'all_ses' => [123, 456] },
-                                                   { 'test_string' => 'test2', 'uri' => 'test_uri2', 'all_ses' => [456, 789] },
-                                                   { 'test_string' => 'test3', 'uri' => 'test_uri3', 'all_ses' => [234, 567] },
+      expect(api_call.object_data).to match_array([{ 'test_string' => 'test1', 'uri' => 'test_uri1', 'all_ses' => [123, 456], 'type_ses' => [12345] },
+                                                   { 'test_string' => 'test2', 'uri' => 'test_uri2', 'all_ses' => [456, 789], 'type_ses' => [23456] },
+                                                   { 'test_string' => 'test3', 'uri' => 'test_uri3', 'all_ses' => [234, 567], 'type_ses' => [34567] },
                                                   ])
     end
   end

spec/models/solr_query_spec.rb

@@ -11,15 +11,15 @@
     "response" => {
       "numFound" => 1,
       "start" => 0,
-      "docs" => [{ test_string: 'test' }]
+      "docs" => [{ 'type_ses' => [12345] }]
     },
     "highlighting" => { "test_url" => {} }
   } }
 
   describe 'object_data' do
     it 'returns the data for the object' do
       allow(api_call).to receive(:evaluated_response).and_return(mock_response)
-      expect(api_call.object_data).to eq({ test_string: 'test' })
+      expect(api_call.object_data).to eq({ 'type_ses' => [12345] })
     end
   end
 end

spec/requests/content_objects_spec.rb

@@ -6,14 +6,14 @@
 
     context 'success' do
       it 'returns http success' do
-        allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
+        allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
         allow_any_instance_of(SesLookup).to receive(:data).and_return({})
         allow(ContentObject).to receive(:generate).and_return(edm_instance)
         get '/objects', params: { :object => 'test_string' }
         expect(response).to have_http_status(:ok)
       end
       it 'renders the footer' do
-        allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
+        allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
         allow_any_instance_of(SesLookup).to receive(:data).and_return({})
         allow(ContentObject).to receive(:generate).and_return(edm_instance)
         get '/objects', params: { :object => 'test_string' }
@@ -35,11 +35,22 @@
     end
 
     context '404 error' do
-      it 'renders the error page' do
-        allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { 'code' => 404 } })
-        allow(ContentObject).to receive(:generate).and_return(edm_instance)
-        get '/objects', params: { :object => 'test_string' }
-        expect(response.body).to include("We can't find what you are looking for")
+      context 'when receiving a 404 error code from SolrQuery' do
+        it 'renders the error page' do
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { 'code' => 404 } })
+          allow(ContentObject).to receive(:generate).and_return(edm_instance)
+          get '/objects', params: { :object => 'test_string' }
+          expect(response.body).to include("We can't find what you are looking for")
+        end
+      end
+
+      context 'when receiving valid data from SolrQuery but type_ses is missing' do
+        it 'renders the error page' do
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'not_type_ses' => [12345] }] } })
+          allow(ContentObject).to receive(:generate).and_return(edm_instance)
+          get '/objects', params: { :object => 'test_string' }
+          expect(response.body).to include("We can't find what you are looking for")
+        end
       end
     end
   end

spec/requests/edm_spec.rb

@@ -5,7 +5,7 @@
     let!(:edm_instance) { Edm.new('type_ses' => [12345]) }
 
     it 'returns http success' do
-      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
+      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
       allow_any_instance_of(SesLookup).to receive(:data).and_return({})
       allow(ContentObject).to receive(:generate).and_return(edm_instance)
       get '/objects', params: { :object => 'test_string' }
@@ -45,7 +45,7 @@
             end
           end
 
-          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
           allow(ContentObject).to receive(:generate).and_return(edm_instance)
           allow_any_instance_of(SesLookup).to receive(:data).and_return(test_ses_data)
 

spec/requests/research_briefing_spec.rb

@@ -5,8 +5,8 @@
     let!(:research_briefing_instance) { ResearchBriefing.new('test') }
 
     it 'returns http success' do
-      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
-      allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([])
+      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
+      allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([{ 'type_ses' => [12345] }])
       allow_any_instance_of(SesLookup).to receive(:data).and_return({})
       allow(ContentObject).to receive(:generate).and_return(research_briefing_instance)
       get '/objects', params: { :object => 'test_string' }
@@ -47,8 +47,8 @@
             end
           end
 
-          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
-          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([])
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
+          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([{ 'type_ses' => [12345] }])
           allow(ContentObject).to receive(:generate).and_return(research_briefing_instance)
           allow_any_instance_of(SesLookup).to receive(:data).and_return(test_ses_data)
 

spec/requests/written_question_spec.rb

@@ -5,8 +5,8 @@
     let!(:written_question_instance) { WrittenQuestion.new('type_ses' => [12345]) }
 
     it 'returns http success' do
-      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
-      allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([])
+      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { 'docs' => [{ 'type_ses' => [12345] }] } })
+      allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([{ 'type_ses' => [12345] }])
       allow_any_instance_of(SesLookup).to receive(:data).and_return({})
       allow(ContentObject).to receive(:generate).and_return(written_question_instance)
       allow(written_question_instance).to receive(:tabled?).and_return(true)
@@ -41,8 +41,8 @@
             end
           end
 
-          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
-          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([])
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
+          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([{ 'type_ses' => [12345] }])
           allow(ContentObject).to receive(:generate).and_return(written_question_instance)
           allow(written_question_instance).to receive(:tabled?).and_return(true)
           allow_any_instance_of(SesLookup).to receive(:data).and_return(test_ses_data)

spec/requests/written_statement_spec.rb

@@ -5,7 +5,7 @@
     let!(:written_statement_instance) { WrittenStatement.new('type_ses' => [12345]) }
 
     it 'returns http success' do
-      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
+      allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
       allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return({})
       allow_any_instance_of(SesLookup).to receive(:data).and_return({})
       allow(ContentObject).to receive(:generate).and_return(written_statement_instance)
@@ -41,8 +41,8 @@
             end
           end
 
-          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => ['test'] } })
-          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([])
+          allow_any_instance_of(SolrQuery).to receive(:all_data).and_return({ 'response' => { "docs" => [{ 'type_ses' => [12345] }] } })
+          allow_any_instance_of(SolrMultiQuery).to receive(:object_data).and_return([{ 'type_ses' => [12345] }])
           allow(ContentObject).to receive(:generate).and_return(written_statement_instance)
           allow_any_instance_of(SesLookup).to receive(:data).and_return(test_ses_data)