Merge pull request #692 from alphagov/fix_csv_preview

Add check to Dataset `organogram?` to return false for items without a schema id
alphagov · Oct 8, 2019 · 8b94660 · 8b94660
2 parents 46b29ef + 0359586
commit 8b94660
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 9 deletions.
diff --git a/app/controllers/map_previews_controller.rb b/app/controllers/map_previews_controller.rb
@@ -18,17 +18,17 @@ def getinfo
     base_wms_url = url_param.gsub(/;jsessionid=[a-z0-9]+/i, ';jsessionid=')
     response = URI(base_wms_url).read
     render xml: Nokogiri::XML(response)
-  rescue StandardError => exception
-    Raven.capture_exception(exception)
+  rescue StandardError => e
+    Raven.capture_exception(e)
     head :bad_request
   end
 
   def proxy
     url = correct_url(url_param)
     response = URI(url).read.force_encoding("ISO-8859-1").encode("UTF-8")
     render xml: Nokogiri::XML(response)
-  rescue StandardError => exception
-    Raven.capture_exception(exception)
+  rescue StandardError => e
+    Raven.capture_exception(e)
     head :bad_request
   end
 

diff --git a/app/models/dataset.rb b/app/models/dataset.rb
@@ -108,6 +108,8 @@ def editable?
   end
 
   def organogram?
+    return false unless @schema_id
+
     schema_id = @schema_id.gsub(/\["|"\]/, '')
     ORGANOGRAM_SCHEMA_IDS.include?(schema_id)
   end

diff --git a/app/models/ticket.rb b/app/models/ticket.rb
@@ -14,7 +14,7 @@ def initialize(ticket_details = {})
     @support = ticket_details[:support]
   end
 
-  def to_json
+  def to_json(*_args)
     { "requester": { "name": name, "email": email },
       "subject": support_queue + " Find open data - #{support} request",
       "comment": { "body": content } }

diff --git a/config/routes.rb b/config/routes.rb
@@ -56,8 +56,8 @@
   # Route everything else to CKAN
   if ENV["CKAN_REDIRECTION_URL"].present?
     match '*path',
-      to: redirect(domain: ENV['CKAN_REDIRECTION_URL'], subdomain: '', path: "/%{path}"),
-      via: :all,
-      constraints: { path: /(?!#{Regexp.quote(Rails.application.config.assets.prefix[1..-1])}).+/ }
+          to: redirect(domain: ENV['CKAN_REDIRECTION_URL'], subdomain: '', path: "/%{path}"),
+          via: :all,
+          constraints: { path: /(?!#{Regexp.quote(Rails.application.config.assets.prefix[1..-1])}).+/ }
   end
 end
diff --git a/spec/features/dataset_show_page_spec.rb b/spec/features/dataset_show_page_spec.rb
@@ -31,7 +31,7 @@
 
     scenario 'Link to licence with additional info' do
       dataset = build :dataset, :with_ogl_licence,
-                                licence_custom: 'Special case'
+                      licence_custom: 'Special case'
 
       index_and_visit(dataset)
 

diff --git a/spec/models/dataset_spec.rb b/spec/models/dataset_spec.rb
@@ -16,5 +16,10 @@
       dataset = build :dataset, schema_id: 'non-organogram'
       expect(dataset.organogram?).to be false
     end
+
+    it 'does not recognise nil ids as an organogram' do
+      dataset = build :dataset, schema_id: nil
+      expect(dataset.organogram?).to be false
+    end
   end
 end