Merge pull request #2446 from seek4science/citation-style-error-fix

Validate citation styles

Author: fbacall

app/controllers/citations_controller.rb

@@ -11,6 +11,6 @@ def fetch
   private
 
   def set_citation_style
-    session[:citation_style] = params[:style] if params[:style].present?
+    session[:citation_style] = params[:style] if params[:style].present? && Seek::Citations.valid_style?(params[:style])
   end
 end

app/helpers/citations_helper.rb

@@ -4,14 +4,18 @@
 module CitationsHelper
   def render_doi_citation(doi, style)
     Seek::Citations.from_doi(doi, style)
+  rescue Seek::Citations::InvalidStyleException
+    t('citations.errors.invalid_style')
   rescue JSON::ParserError, RestClient::Exception
-    'An error occurred whilst fetching the citation'
+    t('citations.errors.general')
   end
 
   def render_cff_citation(blob, style)
     Seek::Citations.from_cff(blob, style)
-  rescue StandardError
-    'An error occurred whilst fetching the citation'
+  rescue Seek::Citations::InvalidStyleException
+    t('citations.errors.invalid_style')
+  rescue JSON::ParserError, RestClient::Exception
+    t('citations.errors.general')
   end
 
   def citation_style_options(selected = nil)
@@ -20,6 +24,6 @@ def citation_style_options(selected = nil)
   end
 
   def selected_citation_style
-    session[:citation_style] || Seek::Citations::DEFAULT
+    session[:citation_style] || Seek::Config.default_citation_style
   end
 end

config/initializers/seek_configuration.rb

@@ -287,6 +287,7 @@ def load_seek_config_defaults!
   ]
 
   Seek::Config.default :reindex_all_batch_size, 50
+  Seek::Config.default :default_citation_style, 'apa'
 
   load_seek_testing_defaults! if Rails.env.test?
 end

config/locales/en.yml

@@ -351,4 +351,8 @@ you can choose to have a %{project} linked to a site managed %{programme} instea
 
   tooltips:
     batch_permission_changes_button: "You can change the sharing policy and permissions for your items as a batch. A preview of selected items will be given before you choose new permissions for them."
-    publish_your_items_button: "Publish your owned items as a batch. A preview will be given before publishing"
+    publish_your_items_button: "Publish your owned items as a batch. A preview will be given before publishing"
+  citations:
+    errors:
+      general: An error occurred whilst fetching the citation
+      invalid_style: Invalid citation style

lib/seek/citations.rb

@@ -4,14 +4,16 @@
 
 module Seek
   class Citations
-    DEFAULT = 'apa' # This could be a setting one day
-
     def self.from_doi(doi, style)
-      generate(doi_to_csl(doi), style)
+      validate_style(style)
+      csl = doi_to_csl(doi)
+      generate(csl, style)
     end
 
     def self.from_cff(blob, style)
-      generate(cff_to_csl(blob), style)
+      validate_style(style)
+      csl = cff_to_csl(blob)
+      generate(csl, style)
     end
 
     def self.generate(csl, style)
@@ -26,6 +28,10 @@ def self.style_pairs
       end
     end
 
+    def self.valid_styles
+      @valid_styles ||= Set.new(style_pairs.map(&:last))
+    end
+
     def self.doi_to_csl(doi)
       Rails.cache.fetch("citation-#{doi}") do
         resp = RestClient.get("https://doi.org/#{Addressable::URI.escape(doi)}", accept: 'application/vnd.citationstyles.csl+json')
@@ -47,5 +53,15 @@ def self.generate_style_pairs
     def self.style_dictionary_path
       Rails.root.join('config/default_data/csl_styles.yml')
     end
+
+    def self.valid_style?(style)
+      valid_styles.include?(style)
+    end
+
+    private
+
+    def self.validate_style(style)
+      raise Seek::Citations::InvalidStyleException unless valid_style?(style)
+    end
   end
 end

lib/seek/citations/invalid_style_exception.rb

@@ -0,0 +1 @@
+class Seek::Citations::InvalidStyleException < StandardError; end

lib/seek/config_setting_attributes.yml

@@ -278,3 +278,4 @@ max_filters:
   convert: :to_i
 reindex_all_batch_size:
   convert: :to_i
+default_citation_style:

lib/seek/renderers/citation_renderer.rb

@@ -12,7 +12,7 @@ def render_content
       private
 
       def style
-        params[:style] || Seek::Citations::DEFAULT
+        params[:style] || Seek::Config.default_citation_style
       end
     end
   end

test/integration/citation_style_test.rb

@@ -15,26 +15,52 @@ def setup
     get model_path(@model)
     # APA by default
     assert_select '#citation' do
-      assert_select 'div[data-citation-style=?]', Seek::Citations::DEFAULT, text: /Bacall, F/, count: 1
+      assert_select 'div[data-citation-style=?]', Seek::Config.default_citation_style, text: /Bacall, F/, count: 1
     end
     assert_select '#citation-style-select' do
-      assert_select "option[selected='selected'][value=?]", Seek::Citations::DEFAULT
+      assert_select "option[selected='selected'][value=?]", Seek::Config.default_citation_style
     end
 
     new_style = 'journal-of-infectious-diseases'
 
+    assert_nil session[:citation_style]
     get citation_path(@doi, style: new_style, format: :js), xhr: true
+    assert_equal new_style, session[:citation_style]
 
     get model_path(@model)
     assert_select '#citation' do
-      assert_select 'div[data-citation-style=?]', Seek::Citations::DEFAULT, count: 0
+      assert_select 'div[data-citation-style=?]', Seek::Config.default_citation_style, count: 0
       assert_select 'div[data-citation-style=?]', new_style, text: /Bacall F/, count: 1
     end
     assert_select '#citation-style-select' do
       assert_select "option[selected='selected'][value=?]", new_style
     end
   end
 
+  test 'does not remember style if invalid style chosen' do
+    assert_nil session[:citation_style]
+    get citation_path(@doi, style: 'ghdfgkhdfjkghdkj', format: :js), xhr: true
+    assert_nil session[:citation_style]
+  end
+
+  test 'handles invalid style selection' do
+    doi_citation_mock
+
+    get citation_path(@doi, style: 'fjkgdfhgkjdf123', format: :js), xhr: true
+
+    assert_includes response.body, 'Invalid citation style'
+  end
+
+  test 'handles CSL fetch error' do
+    stub_request(:get, /(https?:\/\/)?(dx\.)?doi\.org\/.+/)
+      .with(headers: { 'Accept' => 'application/vnd.citationstyles.csl+json' })
+      .to_return(body: 'Error!', status: 500)
+
+    get citation_path(@doi, style: 'apa', format: :js), xhr: true
+
+    assert_includes response.body, 'An error occurred whilst fetching the citation'
+  end
+
   private
 
   def doi_citation_mock