Fixes for remaining brakeman issues

app/controllers/concerns/paginable.rb

@@ -126,13 +126,12 @@ def refine_query(scope)
     scope = scope.search(@args[:search]) if @args[:search].present?
     # Can raise NoMethodError if the scope does not define a search method
     if @args[:sort_field].present?
-      unless @args[:sort_field][SORT_COLUMN_FORMAT]
-        raise ArgumentError, "sort_field param looks unsafe"
-      end
+      frmt = @args[:sort_field][SORT_COLUMN_FORMAT]
+      raise ArgumentError, "sort_field param looks unsafe" unless frmt
 
       # Can raise ActiveRecord::StatementInvalid (e.g. column does not
       # exist, ambiguity on column, etc)
-      scope = scope.order(@args[:sort_field].to_sym => sort_direction)
+      scope = scope.order(@args[:sort_field].to_sym => sort_direction.to_s)
     end
     if @args[:page] != "ALL"
       # Can raise error if page is not a number
@@ -142,7 +141,6 @@ def refine_query(scope)
   end
 
   def sort_direction
-    @args = @args.with_indifferent_access
     @sort_direction ||= SortDirection.new(@args[:sort_direction])
   end
 

app/helpers/identifier_helper.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module IdentifierHelper
+
+  def id_for_display(id:, with_scheme_name: true)
+    return _("None defined") if id.new_record? || id.value.blank?
+
+    without = id.value_without_scheme_prefix
+    prefix = with_scheme_name ? id.identifier_scheme.description + ": " : ""
+    return prefix + id.value unless without != id.value && !without.starts_with?("http")
+
+    link_to "#{prefix} #{without}", id.value, class: "has-new-window-popup-info"
+  end
+
+end

app/models/concerns/date_rangeable.rb

@@ -16,7 +16,8 @@ def date_range?(term:)
     def by_date_range(field, term)
       date = Date.parse(term) if term[0..1].match(/[0-9]{2}/).present?
       date = Date.parse("1st #{term}") unless date.present?
-      where("#{table_name}.#{field} BETWEEN ? AND ?", date, date.end_of_month)
+      query = "%{table}.%{field} BETWEEN ? AND ?" % { table: table_name, field: field }
+      where(query, date, date.end_of_month)
     end
 
   end

app/presenters/identifier_presenter.rb

@@ -23,17 +23,6 @@ def scheme_by_name(name:)
     schemes.select { |scheme| scheme.name.downcase == name.downcase }
   end
 
-  def id_for_display(id:, with_scheme_name: true)
-    return _("None defined") if id.new_record? || id.value.blank?
-
-    without = id.value_without_scheme_prefix
-    prefix = with_scheme_name ? id.identifier_scheme.description + ": " : ""
-    return prefix + id.value unless without != id.value && !without.starts_with?("http")
-
-    "<a href=\"#{id.value}\" class=\"has-new-window-popup-info\"> " \
-      "#{prefix} #{without}</a>"
-  end
-
   private
 
   def load_schemes

app/views/org_admin/users/edit.html.erb

@@ -58,9 +58,8 @@
 
     <% if @user.identifiers.present? %>
       <h2>Identifiers</h2>
-      <% presenter = IdentifierPresenter.new(identifiable: @user) %>
       <% presenter.identifiers.each do |identifier| %>
-        <p><%= presenter.id_for_display(id: identifier, with_scheme_name: true).html_safe %></p>
+        <p><%= id_for_display(id: identifier, with_scheme_name: true) %></p>
       <% end %>
     <% end %>
 

app/views/orgs/_external_identifiers.html.erb

@@ -17,7 +17,7 @@
         <div class="form-group col-xs-10">
           <% id = presenter.id_for_scheme(scheme: scheme) %>
           <span class="bold"><%= scheme.description %>:</span>
-          <%= presenter.id_for_display(id: id).html_safe %>
+          <%= id_for_display(id: id) %>
         </div>
       </div>
     <% end %>
@@ -62,7 +62,7 @@
       <div class="form-group col-xs-10">
         <% id = presenter.id_for_scheme(scheme: scheme) %>
         <span class="bold"><%= scheme.description %>:</span>
-        <%= presenter.id_for_display(id: id).html_safe %>
+        <%= id_for_display(id: id) %>
       </div>
     </div>
   <% end %>

app/views/paginable/contributors/_index.html.erb

@@ -38,9 +38,8 @@
             <%= contributor.org&.name %>
             <% ror = contributor.org.identifier_for_scheme(scheme: ror_scheme) %>
             <% if ror.present? %>
-              <% id_presenter = IdentifierPresenter.new(identifiable: contributor.org) %>
               <br/>
-              <%= id_presenter.id_for_display(id: ror).html_safe %>
+              <%= id_for_display(id: ror) %>
             <% end %>
           <% end %>
         </td>

app/views/plans/_project_details.html.erb

@@ -100,7 +100,7 @@
         identifier = id_presenter.id_for_scheme(scheme: scheme)
         %>
         <div class="col-md-8">
-          <li><%= id_presenter.id_for_display(id: identifier).html_safe %></li>
+          <li><%= id_for_display(id: identifier) %></li>
         </div>
       <% end %>
     </ul>

app/views/super_admin/users/edit.html.erb

@@ -70,7 +70,7 @@
       <h2>Identifiers</h2>
       <% presenter = IdentifierPresenter.new(identifiable: @user) %>
       <% presenter.identifiers.each do |identifier| %>
-        <p><%= presenter.id_for_display(id: identifier, with_scheme_name: true).html_safe %></p>
+        <p><%= id_for_display(id: identifier, with_scheme_name: true) %></p>
       <% end %>
     <% end %>
 

package.json

@@ -33,6 +33,10 @@
     "bootstrap-select": "^1.13.18",
     "chart.js": "^2.9.3",
     "core-js": "^3.6.5",
+    "eslint": "^7.1.0",
+    "eslint-config-airbnb-base": "^14.1.0",
+    "eslint-loader": "^4.0.2",
+    "eslint-plugin-import": "^2.21.1",
     "jquery": "^3.5.1",
     "jquery-ui": "^1.12.1",
     "jquery-ui-sass": "^0.0.1",
@@ -42,12 +46,8 @@
     "rails-erb-loader": "^5.5.2",
     "regenerator-runtime": "^0.13.5",
     "timeago.js": "^4.0.2",
-    "tinymce": "^4.9.10",
-    "turbolinks": "^5.2.0",
-    "eslint": "^7.1.0",
-    "eslint-config-airbnb-base": "^14.1.0",
-    "eslint-loader": "^4.0.2",
-    "eslint-plugin-import": "^2.21.1"
+    "tinymce": "4.9.10",
+    "turbolinks": "^5.2.0"
   },
   "devDependencies": {
     "eslint": "^7.1.0",

spec/helpers/identifier_helper_spec.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe IdentifierHelper do
+  include IdentifierHelper
+
+  before(:each) do
+    @user_scheme = create(:identifier_scheme, for_users: true)
+  end
+
+  describe "#id_for_display(id:, with_scheme_name)" do
+    before(:each) do
+      @none = _("None defined")
+      url = Faker::Internet.url
+      @user_scheme.identifier_prefix = url
+      val = "#{url}/#{Faker::Lorem.word}"
+      @identifier = create(:identifier, identifier_scheme: @user_scheme,
+                                        value: val)
+    end
+
+    it "defaults to showing the scheme name" do
+      rslt = id_for_display(id: @identifier)
+      expect(rslt.include?(@user_scheme.identifier_prefix)).to eql(true)
+    end
+    it "does not display the scheme name if flag is set" do
+      rslt = id_for_display(id: @identifier, with_scheme_name: false)
+      expect(rslt.include?(@user_scheme.name)).to eql(false)
+    end
+    it "returns the correct text when the identifier is new" do
+      id = build(:identifier)
+      rslt = id_for_display(id: id)
+      expect(rslt).to eql(@none)
+    end
+    it "returns the correct text when the identifier is blank" do
+      @identifier.value = ""
+      rslt = id_for_display(id: @identifier)
+      expect(rslt).to eql(@none)
+    end
+    it "returns the value when the scheme has no identifier_prefix" do
+      val = Faker::Lorem.word
+      @user_scheme.identifier_prefix = nil
+      @user_scheme.save
+      @identifier.value = val
+      rslt = id_for_display(id: @identifier)
+      expect(rslt).to eql(@user_scheme.description + ": " + val)
+    end
+    it "returns the value as a link when the scheme has a identifier_prefix" do
+      rslt = id_for_display(id: @identifier)
+      expect(rslt.include?(@identifier.value)).to eql(true)
+    end
+  end
+
+end

spec/presenters/identifier_presenter_spec.rb

@@ -49,50 +49,6 @@
     end
   end
 
-  describe "#id_for_display(id:, with_scheme_name)" do
-    before(:each) do
-      @none = _("None defined")
-      @presenter = described_class.new(identifiable: @user)
-
-      url = Faker::Internet.url
-      @user_scheme.identifier_prefix = url
-      val = "#{url}/#{Faker::Lorem.word}"
-      @identifier = create(:identifier, identifier_scheme: @user_scheme,
-                                        value: val)
-    end
-
-    it "defaults to showing the scheme name" do
-      rslt = @presenter.id_for_display(id: @identifier)
-      expect(rslt.include?(@user_scheme.identifier_prefix)).to eql(true)
-    end
-    it "does not display the scheme name if flag is set" do
-      rslt = @presenter.id_for_display(id: @identifier, with_scheme_name: false)
-      expect(rslt.include?(@user_scheme.name)).to eql(false)
-    end
-    it "returns the correct text when the identifier is new" do
-      id = build(:identifier)
-      rslt = @presenter.id_for_display(id: id)
-      expect(rslt).to eql(@none)
-    end
-    it "returns the correct text when the identifier is blank" do
-      @identifier.value = ""
-      rslt = @presenter.id_for_display(id: @identifier)
-      expect(rslt).to eql(@none)
-    end
-    it "returns the value when the scheme has no identifier_prefix" do
-      val = Faker::Lorem.word
-      @user_scheme.identifier_prefix = nil
-      @user_scheme.save
-      @identifier.value = val
-      rslt = @presenter.id_for_display(id: @identifier)
-      expect(rslt).to eql(@user_scheme.description + ": " + val)
-    end
-    it "returns the value as a link when the scheme has a identifier_prefix" do
-      rslt = @presenter.id_for_display(id: @identifier)
-      expect(rslt.include?(@identifier.value)).to eql(true)
-    end
-  end
-
   context "#schemes" do
     describe "when the identifiable object is an Org" do
       before(:each) do

yarn.lock

@@ -7974,10 +7974,10 @@ timsort@^0.3.0:
   resolved "https://registry.yarnpkg.com/timsort/-/timsort-0.3.0.tgz#405411a8e7e6339fe64db9a234de11dc31e02bd4"
   integrity sha1-QFQRqOfmM5/mTbmiNN4R3DHgK9Q=
 
-tinymce@^4.9.10:
-  version "4.9.11"
-  resolved "https://registry.yarnpkg.com/tinymce/-/tinymce-4.9.11.tgz#e3dae099722294c5b8d84ba7ef18dd126de6b582"
-  integrity sha512-nkSLsax+VY5DBRjMFnHFqPwTnlLEGHCco82FwJF2JNH6W+5/ClvNC1P4uhD5lXPDNiDykSHR0XJdEh7w/ICHzA==
+tinymce@4.9.10:
+  version "4.9.10"
+  resolved "https://registry.yarnpkg.com/tinymce/-/tinymce-4.9.10.tgz#47bd7b4d27d80d53a464356eb7c72b97e5c3aadd"
+  integrity sha512-vyzGG04Q44Y7zWIKA4c+G7MxMCsed6JkrhU+k0TaDs9XKAiS+e+D3Fzz5OIJ7p5keF7lbRK5czgI8T1JtouZqw==
 
 tmp@0.2.1:
   version "0.2.1"