Refactor code

app/controllers/holds_controller.rb

@@ -10,7 +10,6 @@ class HoldsController < ApplicationController
   end
 
   def index
-    LogWrapper.log('DEBUG', {'message' => 'index.start', 'method' => 'app/controllers/holds_controller.rb.index'})
     redirect_to root_url
   end
 
@@ -20,8 +19,6 @@ def index
   # their holds history.  In routing terms, responds to a GET request on the
   # /holds/[hold id].json url.
   def show
-    LogWrapper.log('DEBUG', {'message' => 'show.start', 'method' => 'app/controllers/holds_controller.rb.show'})
-
     @hold = Hold.find_by_access_key(params[:id])
     head 401 if @hold.nil?
 
@@ -35,7 +32,6 @@ def show
 
   # GET /holds/new.json
   def new
-    LogWrapper.log('DEBUG', {'message' => 'new.start', 'method' => 'app/controllers/holds_controller.rb.new'})
     @hold = Hold.new
     @hold.teacher_set = TeacherSet.find params[:teacher_set_id]
     render json: {
@@ -47,7 +43,6 @@ def new
 
   # GET /holds/1/cancel.json
   def cancel
-    LogWrapper.log('DEBUG', {'message' => 'cancel.start', 'method' => 'app/controllers/holds_controller.rb.cancel'})
     @hold = Hold.find_by_access_key(params[:id])
     head 401 if @hold.nil?
     render json: {
@@ -62,7 +57,6 @@ def cancel
   # Create holds and update quantity column in holds.
   # Calculate available copies from quantity saves in teacherset table.
   def create
-    LogWrapper.log('DEBUG', {'message' => 'create.start', 'method' => 'app/controllers/holds_controller.rb.create'})
     begin
       # If user's school is inactive, then display an error message and redirect to teacher set detail page.
       is_school_active = current_user.school_id.present? ? School.find(current_user.school_id).active : false
@@ -109,7 +103,6 @@ def create
 
 
   def error_message(exception)
-    LogWrapper.log('ERROR', 'message' => exception.message)
     # Note: don't need an explicit return here
     respond_to do |format|
       format.html {}
@@ -123,7 +116,6 @@ def error_message(exception)
 
   # Here calculate the teacher-set available_copies based on the current-user holds than saves in teacher-set table and cancel the current-user holds.
   def update
-    LogWrapper.log('DEBUG', {'message' => 'update.start', 'method' => 'app/controllers/holds_controller.rb.update'})
     @hold = Hold.find_by_access_key(params[:id])
 
     unless (c = params[:hold_change]).nil?
@@ -151,7 +143,6 @@ def update
   protected
 
   def check_ownership
-    LogWrapper.log('DEBUG', {'message' => 'check_ownership.start', 'method' => 'app/controllers/holds_controller.rb.check_ownership'})
     @hold = Hold.find_by_access_key(params[:id])
 
     if not user_signed_in?

app/controllers/registrations_controller.rb

@@ -13,7 +13,6 @@ class RegistrationsController < Devise::RegistrationsController
   # In addition, overriding the method allows us to validate the incoming data from the form, send the data
   # to the microservice, and create a record on MyLibraryNYC depending on if the microservice is working properly.
   def create
-    LogWrapper.log('INFO','message' => "Creating new user record")
     build_resource(sign_up_params)
     if resource.valid?
       begin

app/controllers/teacher_sets_controller.rb

@@ -11,47 +11,44 @@ class TeacherSetsController < ApplicationController
   # Called on loading the teacher set list page and when the user selects
   # a facet to filter by.
   def index
-    LogWrapper.log('DEBUG', {'message' => 'index.start', 'method' => 'app/controllers/teacher_sets_controller.rb.index'})
-    begin
-      # Feature flag: 'teacherset.data.from.elasticsearch.enabled = true' means gets teacher-set documents from elastic search.
-      # teacherset.data.from.elasticsearch.enabled = false means gets teacher-set data from database.
-      if MlnConfigurationController.new.feature_flag_config('teacherset.data.from.elasticsearch.enabled')
-        LogWrapper.log('INFO', {'message' => "Calling elastic search to get teacher-sets", 
-                                'method' => 'app/controllers/teacher_sets_controller.rb.index'})
-
-        # Get teachersets and facets from elastic search
-        teacher_sets, @facets = ElasticSearch.new.get_teacher_sets_from_es(params)
-        @teacher_sets = teacher_sets_from_elastic_search_doc(teacher_sets)
-      else
-        LogWrapper.log('INFO', {'message' => "Calling database to get teacher-sets", 
-                                'method' => 'app/controllers/teacher_sets_controller.rb.index'})
-        @teacher_sets = TeacherSet.for_query params
-        @facets = TeacherSet.facets_for_query @teacher_sets
-      end
-      # Determine what facets are selected based on query string
-      @facets.each do |f|
-        f[:items].each do |v|
-          k = f[:label].underscore
-          v[:selected] = params.keys.include?(k) && params[k].include?(v[:value].to_s)
-        end
+    # Feature flag: 'teacherset.data.from.elasticsearch.enabled = true' means gets teacher-set documents from elastic search.
+    # teacherset.data.from.elasticsearch.enabled = false means gets teacher-set data from database.
+    if MlnConfigurationController.new.feature_flag_config('teacherset.data.from.elasticsearch.enabled')
+      LogWrapper.log('INFO', {'message' => "Calling elastic search to get teacher-sets", 
+                              'method' => 'app/controllers/teacher_sets_controller.rb.index'})
+
+      # Get teachersets and facets from elastic search
+      teacher_sets, @facets = ElasticSearch.new.get_teacher_sets_from_es(params)
+      @teacher_sets = teacher_sets_from_elastic_search_doc(teacher_sets)
+    else
+      LogWrapper.log('INFO', {'message' => "Calling database to get teacher-sets", 
+                              'method' => 'app/controllers/teacher_sets_controller.rb.index'})
+      @teacher_sets = TeacherSet.for_query params
+      @facets = TeacherSet.facets_for_query @teacher_sets
+    end
+    # Determine what facets are selected based on query string
+    @facets.each do |f|
+      f[:items].each do |v|
+        k = f[:label].underscore
+        v[:selected] = params.keys.include?(k) && params[k].include?(v[:value].to_s)
       end
+    end
 
-      @facets = teacher_set_facets(params)
-      # Attach custom :q param to each facet with query params to be applied to that link
-      if MlnConfigurationController.new.feature_flag_config('teacherset.data.from.elasticsearch.enabled')
-        render json: { teacher_sets: @teacher_sets, facets: @facets }
-      else
-        render json: { teacher_sets: @teacher_sets, facets: @facets }, serializer: SearchSerializer, include_books: false, include_contents: false
-      end
-    rescue StandardError => e
-      LogWrapper.log('DEBUG', {'message' => "Error occured in teacherset controller. Error: #{e.message}, backtrace: #{e.backtrace}", 
-                               'method' => 'app/controllers/teacher_sets_controller.rb.index'})
-      render json: {
-        errors: {error_message: "We've encountered an error. Please try again later or email [email protected] for assistance."},
-        teacher_sets: {},
-        facets: {}
-      }, serializer: SearchSerializer, include_books: false, include_contents: false
+    @facets = teacher_set_facets(params)
+    # Attach custom :q param to each facet with query params to be applied to that link
+    if MlnConfigurationController.new.feature_flag_config('teacherset.data.from.elasticsearch.enabled')
+      render json: { teacher_sets: @teacher_sets, facets: @facets }
+    else
+      render json: { teacher_sets: @teacher_sets, facets: @facets }, serializer: SearchSerializer, include_books: false, include_contents: false
     end
+  rescue StandardError => e
+    LogWrapper.log('ERROR', {'message' => "Error occured in teacherset controller. Error: #{e.message}, backtrace: #{e.backtrace}", 
+                             'method' => 'app/controllers/teacher_sets_controller.rb.index'})
+    render json: {
+      errors: {error_message: "We've encountered an error. Please try again later or email [email protected] for assistance."},
+      teacher_sets: {},
+      facets: {}
+    }, serializer: SearchSerializer, include_books: false, include_contents: false
   end
 
 
@@ -92,8 +89,6 @@ def teacher_set_facets(params)
 
   # GET /teacher_sets/1.json
   def show
-    LogWrapper.log('DEBUG', {'message' => 'show.start', 'method' => 'app/controllers/teacher_sets_controller.rb.show'})
-
     # Stores the current location in session, so if an un-authenticated user
     # tries to order this teacher set, we can ask the user to sign in, and
     # then redirect back to this teacher_set detail page.
@@ -139,7 +134,6 @@ def show
 
   # Gets current user teacherset holds from database.
   def teacher_set_holds
-    LogWrapper.log('DEBUG', {'message' => 'teacher_set_holds.start', 'method' => 'app/controllers/teacher_sets_controller.rb.teacher_set_holds'})
     @set = TeacherSet.find(params[:id])
     @holds = @set.holds_for_user(current_user)
   end

app/helpers/teacher_sets_helper.rb

@@ -116,4 +116,21 @@ def fixed_field(marc_tag)
   rescue StandardError
     nil
   end
+
+
+  # Parses out the items duedate, items code is '-' which determines if an item is available or not.
+  # Calculates the total number of items in the list, the number of items that are
+  # available to lend.
+  def parse_items_available_and_total_count(response)
+    available_count = 0
+    total_count = 0
+    status_codes = %w[w m k u]
+    response['data'].each do |item|
+      total_count += 1 unless item['status']['code'].present? && status_codes.include?(item['status']['code'])
+      available_count += 1 if item['status']['code'].present? && item['status']['code'] == '-' && !item['status']['duedate'].present?
+    end
+    LogWrapper.log('INFO','message' => "TeacherSet available_count: #{available_count}, total_count: #{total_count}")
+
+    [total_count, available_count]
+  end
 end

app/mailers/admin_mailer.rb

@@ -13,10 +13,6 @@ def failed_bibs_controller_api_request(request_body, error_code_and_message, act
       @request_body = request_body
       @error_code_and_message = error_code_and_message
       emails = AdminUser.where(email_notifications:true).pluck(:email)
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send failed_bibs_controller_api_request email.\n@error_code_and_message: #{@error_code_and_message || 'nil'}",
-        'method' => 'AdminMailer.failed_bibs_controller_api_request'
-      })
       mail(:to => emails, :subject => "Problem occurred updating bib from Sierra")
     rescue => exception
       LogWrapper.log('ERROR', {
@@ -35,10 +31,6 @@ def teacher_set_update_missing_required_fields(bnumber, title, physical_descript
       @title = title
       @physical_description = physical_description
       emails = AdminUser.where(email_notifications:true).pluck(:email)
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send failed_bibs_controller_api_request email",
-        'method' => 'AdminMailer teacher_set_update_missing_required_fields'
-      })
       mail(:to => emails, :subject => "New teacher set bib in Sierra is missing required fields")
     rescue => exception
       LogWrapper.log('ERROR', {
@@ -54,10 +46,6 @@ def teacher_set_update_missing_required_fields(bnumber, title, physical_descript
   def failed_items_controller_api_request(error_code_and_message)
     begin
       emails = AdminUser.where(email_notifications:true).pluck(:email)
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send failed_items_controller_api_request email.\n@error_code_and_message: #{@error_code_and_message || 'nil'}",
-        'method' => 'AdminMailer.failed_items_controller_api_request'
-      })
       mail(:to => emails, :subject => "Problem occurred updating bib availability")
     rescue => exception
       LogWrapper.log('ERROR', {

app/mailers/hold_mailer.rb

@@ -14,10 +14,6 @@ def admin_notification(hold)
       @user = hold.user
       @teacher_set = hold.teacher_set
       emails = AdminUser.where(email_notifications:true).pluck(:email)
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send hold order admin notification email on #{@teacher_set.title or 'unknown'} to #{@user.email or 'unknown'}",
-        'method' => 'HoldMailer admin_notification'
-      })
       mail(:to => emails, :subject => "New Order from #{@user.email or 'unknown'} for #{@teacher_set.title or 'unknown'}")
     rescue => exception
       LogWrapper.log('ERROR', {
@@ -37,10 +33,6 @@ def confirmation(hold)
       @hold = hold
       @user = hold.user
       @teacher_set = hold.teacher_set
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send patron hold order notification email to #{@user.email}",
-        'method' => 'HoldMailer confirmation'
-      })
       mail(:to => @user.contact_email, :subject => "Your order confirmation for #{@teacher_set.title}")
     rescue => exception
       LogWrapper.log('ERROR', {
@@ -59,10 +51,6 @@ def status_change(hold, status, details)
       @user = hold.user
       @teacher_set = hold.teacher_set
       @details = details
-      LogWrapper.log('DEBUG', {
-        'message' => "status_change: About to send status_change notification email to #{@user.email}",
-        'method' => 'HoldMailer status_change'
-      })
       mail(:to => @user.contact_email, :subject => "Order #{@hold.status} | Your teacher set order for #{@teacher_set.title}")
     rescue => exception
       LogWrapper.log('ERROR', {

app/mailers/user_mailer.rb

@@ -12,10 +12,6 @@ class UserMailer < ActionMailer::Base
   def unsubscribe(user)
     begin
       @user = user
-      LogWrapper.log('DEBUG', {
-        'message' => "About to send unsubscribe confirmation email to #{@user.email or 'unknown'}",
-        'method' => 'UserMailer unsubscribe'
-      })
       mail(:to => @user.contact_email, :subject => "You have now unsubscribed from MyLibraryNyc.")
     rescue => exception
       # something went wrong.  perhaps the user isn't set properly, or maybe the email couldn't be sent out.

app/models/book.rb

@@ -194,12 +194,6 @@ def update_from_isbn
 
   # Sends a request to the bibs microservice.
   def send_request_to_bibs_microservice
-    LogWrapper.log('DEBUG', {
-       'message' => 'Request sent to bibs service',
-       'method' => 'send_request_to_bibs_microservice',
-       'status' => 'start',
-       'dataSent' => ENV['BIBS_MICROSERVICE_URL_V01'] + "?standardNumber=#{isbn}"
-      })
     response = HTTParty.get(
       ENV['BIBS_MICROSERVICE_URL_V01'] + "?standardNumber=#{isbn}",
       headers:
@@ -208,7 +202,6 @@ def send_request_to_bibs_microservice
       timeout: 10
     )
 
-
     case response.code
     when 200
       @book_found = true