Merge pull request #474 from latis-sw/develop

Cut down on pesky LDAP requests

app/controllers/application_controller.rb

@@ -9,7 +9,6 @@ class ApplicationController < ActionController::Base
   helper Starburst::AnnouncementsHelper
 
   before_action :set_paper_trail_whodunnit
-  before_action :ping_lookup_service
   before_action :expire_cache_headers
 
   before_action :set_notification
@@ -19,10 +18,6 @@ def set_notification
     # can be any key-value pairs
   end
 
-  def info_for_paper_trail
-    { whodunnit_name: current_user.internet_id, whodunnit_email: current_user.email } if signed_in?
-  end
-
   def ensure_signed_in
     redirect_to signin_path unless signed_in?
   end
@@ -50,13 +45,6 @@ def user_not_authorized
     redirect_to(request.referer || root_path)
   end
 
-  def ping_lookup_service
-    unless UserLookup.new.ping
-      flash.now[:error] = I18n.t 'controllers.application.lookup_service_down'
-      render file: 'public/lookup_service_down.html', layout: false
-    end
-  end
-
   def redirect_to_urls_if_logged_in
     redirect_to urls_path if signed_in?
   end

app/datatables/audit_datatable.rb

@@ -1,13 +1,11 @@
 class AuditDatatable < AjaxDatatablesRails::Base
-  def_delegators :@view, :display_whodunnit_email
+  def_delegators :@view, :display_whodunnit_internet_id
 
   def view_columns
     @view_columns ||= {
         item_type: { source: 'Audit.item_type' },
         event: { source: 'Audit.event' },
         whodunnit: { source: 'Audit.whodunnit' },
-        whodunnit_email: { source: 'Audit.whodunnit_email' },
-        whodunnit_name: { source: 'Audit.whodunnit_name' },
         audit_history: { source: 'Audit.version_history' },
         created_at: { source: 'Audit.created_at' }
     }
@@ -29,9 +27,7 @@ def data
           # example: record.attribute,
           item_type: record.item_type,
           event: record.event,
-          whodunnit: display_whodunnit_email(record),
-          whodunnit_email: record.whodunnit_email,
-          whodunnit_name: record.whodunnit_name,
+          whodunnit: display_whodunnit_internet_id(record),
           audit_history: record.version_history,
           created_at: record.created_at.to_s(:created_on_formatted),
           'DT_RowId' => "audit-#{record.id}"

app/helpers/audit_helper.rb

@@ -1,5 +1,5 @@
 module AuditHelper
-  def display_whodunnit_email(audited_thing)
-    audited_thing.whodunnit_name ? mail_to(audited_thing.whodunnit_email, audited_thing.whodunnit_name) : 'unknown'
+  def display_whodunnit_internet_id(audited_thing)
+    audited_thing.whodunnit ? User.find(audited_thing.whodunnit).internet_id : 'Unknown'
   end
 end

app/models/concerns/version_user.rb

@@ -0,0 +1,9 @@
+module VersionUser
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def version_user(version)
+      version.whodunnit.nil? ? 'N/A' : User.find(version.whodunnit).internet_id
+    end
+  end
+end

app/models/group.rb

@@ -10,6 +10,7 @@
 #
 # models/group.rb
 class Group < ApplicationRecord
+  include VersionUser
   has_paper_trail
   after_save :version_history
   before_destroy :version_history
@@ -65,7 +66,7 @@ def version_history
     self.versions.each do |v|
       g = v.reify unless v.event.equal? "create"
       h.concat "<b>What Happened: </b> #{v.event} <br/>"
-      h.concat "<b>Who Made It: </b>  #{v.whodunnit_name}<br/>"
+      h.concat "<b>Who Made It: </b>  #{self.class.version_user(v)}<br/>"
       h.concat "<b>Previous Name: </b>  #{g ? g.name : 'N/A'}<br/>"
       h.concat "<b>Previous Description: </b>  #{g ? g.description : 'N/A'}<br/>"
       h.concat "<b>Date of Change: </b>  #{g ? g.updated_at : 'N/A'}<br/>"

app/models/transfer_request.rb

@@ -12,6 +12,7 @@
 #
 
 class TransferRequest < ApplicationRecord
+  include VersionUser
   has_paper_trail
   after_save :version_history
   before_destroy :version_history
@@ -81,7 +82,7 @@ def version_history
     self.versions.each do |v|
       g = v.reify #unless v.event.equal? "create"
       h.concat "<b>What Happened: </b> #{v.event} <br/>"
-      h.concat "<b>Who Made It: </b>  #{v.whodunnit_name}<br/>"
+      h.concat "<b>Who Made It: </b>  #{self.class.version_user(v)}<br/>"
       h.concat "<b>Previous Status: </b>  #{g ? g.status : 'N/A'}<br/>"
       h.concat "<br/><br/>"
     end

app/models/url.rb

@@ -13,6 +13,7 @@
 #
 require 'uri'
 class Url < ApplicationRecord
+  include VersionUser
   has_paper_trail :ignore => [:total_clicks]
   after_save :version_history
   before_destroy :version_history
@@ -124,7 +125,7 @@ def version_history
     self.versions.each do |v|
       g = v.reify unless v.event.equal? "create"
       h.concat "<b>What Happened: </b> #{v.event} <br/>"
-      h.concat "<b>Who Made It: </b>  #{v.whodunnit_name}<br/>"
+      h.concat "<b>Who Made It: </b>  #{self.class.version_user(v)}<br/>"
       h.concat "<b>Previous URL: </b>  #{g ? g.url : 'N/A'}<br/>"
       h.concat "<b>Previous Keyword: </b>  #{g ? g.keyword : 'N/A'}<br/>"
       h.concat "<b>Previous Group Name: </b>  #{g && g.group ? g.group.name : 'N/A(Group doesnt exist)'}<br/>"

app/models/user.rb

@@ -31,8 +31,8 @@ class User < ApplicationRecord
   before_validation(on: :create) do
     if context_group_id.blank?
       new_context = Group.create(
-          name: internet_id,
-          description: uid
+        name: internet_id,
+        description: uid
       )
       groups << new_context
       self.context_group_id = new_context.id
@@ -57,40 +57,32 @@ def in_group?(group)
   end
 
   def display_name
-    if @display_name_loaded.nil?
-      load_user_data
-    end
+    load_user_data if @display_name_loaded.nil?
     @display_name_loaded
   end
 
   def email
-    if @internet_id_loaded.nil?
-      load_user_data
-    end
+    load_user_data if @internet_id_loaded.nil?
     "#{@internet_id_loaded}@umn.edu"
   end
 
   def internet_id
-    if @internet_id_loaded.nil?
-      load_user_data
-    end
+    load_user_data if @internet_id_loaded.nil?
     @internet_id_loaded
   end
 
-
   def load_user_data
-    # sets this objects UserData attrs
-    me = UserLookup.new(
+    me = Rails.cache.fetch("#{uid}/user_data", expires_in: 24.hours) do
+      UserLookup.new(
         query: uid,
         query_type: 'umndid'
-    ).search.try(:first)
+      ).search.try(:first)
+    end
 
     @display_name_loaded = 'Unknown'
     @internet_id_loaded = 'Unknown'
 
     if me.present?
-      # Sometimes this data is not present
-      # so we try for it
       @display_name_loaded = me[:display_name] || 'Name not available'
       @internet_id_loaded = me[:internet_id] || 'Unknown'
     end
@@ -114,8 +106,8 @@ def generate_authentication_token
   def user_belongs_to_context_group_validation
     unless new_record? || context_group.nil? || in_group?(context_group)
       errors.add(
-          :context_group,
-          "#{uid} is not a member of #{context_group.name}: #{context_group.description} and so cannot switch contexts to it."
+        :context_group,
+        "#{uid} is not a member of #{context_group.name}: #{context_group.description} and so cannot switch contexts to it."
       )
     end
   end

app/services/user_lookup_service.rb

@@ -10,15 +10,6 @@ def initialize(params = nil)
     @query_type = params[:query_type] if params
   end
 
-  def ping
-    begin
-      @connection.bind
-    rescue Net::LDAP::ConnectionRefusedError
-      return false
-    end
-    true
-  end
-
   def search
     return nil unless @query.present? && @query_type.present?
     results = nil

app/services/user_lookup_service_skeleton.rb

@@ -11,10 +11,6 @@ def initialize(params = nil)
     @query_type = params[:query_type] if params
   end
 
-  def ping
-    true
-  end
-
   def search
     return nil unless @query.present? && @query_type.present?
 

config/environments/development.rb

@@ -12,19 +12,7 @@
   # Show full error reports.
   config.consider_all_requests_local = true
 
-  # Enable/disable caching. By default caching is disabled.
-  if Rails.root.join('tmp/caching-dev.txt').exist?
-    config.action_controller.perform_caching = true
-
-    config.cache_store = :memory_store
-    config.public_file_server.headers = {
-      'Cache-Control' => 'public, max-age=172800'
-    }
-  else
-    config.action_controller.perform_caching = false
-
-    config.cache_store = :null_store
-  end
+  config.cache_store = :memory_store, { size: 32.megabytes }
 
   # Don't care if the mailer can't send.
   config.action_mailer.raise_delivery_errors = false

config/environments/production.rb

@@ -52,7 +52,7 @@
   config.log_tags = [:request_id]
 
   # Use a different cache store in production.
-  # config.cache_store = :mem_cache_store
+  config.cache_store = :memory_store, { size: 32.megabytes }
 
   # Use a real queuing backend for Active Job (and separate queues per environment)
   # config.active_job.queue_adapter     = :resque

config/environments/staging.rb

@@ -51,8 +51,8 @@
   # Prepend all log lines with the following tags.
   config.log_tags = [:request_id]
 
-  # Use a different cache store in production.
-  # config.cache_store = :mem_cache_store
+  # Use a different cache store in staging.
+  config.cache_store = :memory_store, { size: 32.megabytes }
 
   # Use a real queuing backend for Active Job (and separate queues per environment)
   # config.active_job.queue_adapter     = :resque