Merge pull request #856 from NYPL/qa

Merging from qa to master

app/admin/dashboard.rb

@@ -6,10 +6,8 @@
 
   content :title => proc{ I18n.t("active_admin.dashboard") } do
     order_param = params[:order]
-
     order = nil
     pending_order = nil
-
     if !order_param.nil? && order_param[0] == 'p'
       pending_order = case(order_param)
       when 'p_school_desc'
@@ -78,7 +76,6 @@
       end
     end
 
-
     if (order.nil?)
       holds = Hold.unseen
     elsif order_param == 'call_number_desc' or
@@ -89,7 +86,6 @@
     else
       holds = Hold.unseen.joins('JOIN users on users.id = holds.user_id').order(order)
     end
-
     if (pending_order.nil?)
       pending_holds = Hold.pending
     elsif order_param == 'p_call_number_desc' or

app/admin/hold_changes.rb

@@ -49,7 +49,10 @@ def create
       f.input :comment, :label => 'Note to Requester', :input_html => { :class => 'message' }
       f.input :hold_id, :as => :hidden # :input_html => { :disabled => true } 
     end
-    f.actions
+    f.actions do
+      f.action :submit
+      f.action :cancel, :label => 'Cancel'
+    end
   end
 
   controller do

app/admin/holds.rb

@@ -140,8 +140,10 @@ def scoped_collection
 
   member_action :close, :method => :put do
     hold = Hold.find(params[:id])
-    user.close!
-    redirect_to action: :show, notice: "Hold closed!"
+    if hold.present? && hold.user.present?
+      hold.close!(hold.user.first_name) 
+      redirect_to action: :show, notice: "Hold closed!"
+    end
   end
 
   i = 0

app/assets/stylesheets/active_admin.css.scss

@@ -101,3 +101,31 @@ body.active_admin.logged_out {
   }
 }
 
+body.active_admin {
+  #hold_change_cancel_action {
+    margin-right: 10px;
+    border-radius: 200px;
+    display: inline-block;
+    font-weight: bold;
+    font-size: 1.0em;
+    font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif;
+    line-height: 12px;
+    margin-right: 3px;
+    padding: 7px 16px 6px;
+    text-decoration: none;
+    background-color: #838a90;
+    background-image: linear-gradient(180deg, #838a90, #414549);
+    text-shadow: #000 0 1px 0;
+    border: solid 1px #484e53;
+    border-color: #484e53;
+    border-top-color: #616a71;
+    border-right-color: #484e53;
+    border-bottom-color: #363b3f;
+    border-left-color: #484e53;
+    color: #efefef;
+    a, a:link {
+      color: #fff !important;
+      text-decoration: none !important;
+    }
+  }
+}

app/controllers/api/v01/bibs_controller.rb

@@ -119,7 +119,7 @@ def create_or_update_teacher_sets
         end
       end
       saved_teacher_sets << teacher_set
-      LogWrapper.log('INFO', {'message' => "create_or_update_teacher_sets:finished making teacher set.
+      LogWrapper.log('INFO', {'message' => "create_or_update_teacher_sets:finished making teacher set. bibId: #{bnumber}
                      Teacher set availableCount: #{ts_items_info[:available_count]}, totalCount: #{ts_items_info[:total_count]}",
                      'method' => "bibs_controller.create_or_update_teacher_sets"})
     end

app/models/hold.rb

@@ -45,8 +45,8 @@ def cancel!(reason)
 
 
   def close!(by_whom)
-    status = 'closed'
     puts "closing hold: #{self.inspect}"
+    status = 'closed'
   end
 
 

app/models/teacher_set.rb

@@ -825,11 +825,17 @@ def update_included_book_list(teacher_set_record)
   def update_subjects_via_api(subject_name_array)
     LogWrapper.log('DEBUG', {'message' => 'update_subjects_via_api.start','method' => 'teacher_set.update_subjects_via_api'})
 
-    # teacher_set.rb facets_for_query uses cached results of each query
-    Rails.cache.clear unless Rails.env.test?
-
     return if subject_name_array.blank?
 
+    # If Elastic search feature-flag is enabled, teacher-set data saves in elastic-search cluster.
+    # Other wise teacher-set data saves in database and rails cache.
+    # If Elastic search feature-flag is enabled no need to clear the rails cache.
+    unless MlnConfigurationController.new.feature_flag_config('teacherset.data.from.elasticsearch.enabled')
+      # teacher_set.rb facets_for_query uses cached results of each query
+      Rails.cache.clear unless Rails.env.test?
+    end
+
+
     # record the list of current teacher set <--> subject associations,
     # so we can prune the subjects later.
     old_subjects = Array.new

config/database.yml

@@ -4,14 +4,16 @@ require 'cgi'
 require 'uri'
 
 begin
-  # NOTE: remove the logging once launch on production, and comment out for now unless need it.
-  # Not a good idea to declare private stuff in log files.
-  # LogWrapper.log('WARNING', {'message' => 'DATABASE_URL: before parsing the URL', 'method' => 'database.yml load'})
   uri = URI.parse(ENV["DATABASE_URL"])
-  # LogWrapper.log('WARNING', {'message' => "DATABASE_URL: after parsing the URL=[#{uri or 'empty_uri'}]", 'method' => 'database.yml load'})
-
 rescue URI::InvalidURIError
-  raise "Invalid DATABASE_URL: hello#{ENV['DATABASE_URL']}world"
+  # NOTE: Not specifying the actual DATABASE_URL is intentional --
+  # wouldn't be a good idea to declare private stuff in production log files.
+  error_message = 'Invalid DATABASE_URL.'
+  if ENV['DATABASE_URL'].nil? || ENV['DATABASE_URL'].blank?
+    error_message = error_message + '  No DATABASE_URL present'
+  end
+  LogWrapper.log('ERROR', {'message' => "#{error_message}]", 'method' => 'database.yml load'})
+  raise "Invalid DATABASE_URL: [#{error_message}]"
 end
 
 raise "No RACK_ENV or RAILS_ENV found" unless ENV["RAILS_ENV"] || ENV["RACK_ENV"]

config/initializers/devise.rb

@@ -12,7 +12,7 @@
   # Configure the e-mail address which will be shown in Devise::Mailer,
   # note that it will be overwritten if you use your own mailer class
   # with default "from" parameter.
-  config.mailer_sender = '[email protected]'
+  config.mailer_sender = '[email protected]'
 
   # Configure the class responsible to send e-mails.
   # config.mailer = 'Devise::Mailer'

lib/tasks/combined_tests.rake

@@ -37,6 +37,7 @@ namespace :combined_tests do
       'test/functional/s3_controller_test.rb',
       'test/functional/teacher_sets_test.rb',
       'test/functional/*.rb',
+      'test/functional/admin/*.rb',
       'test/unit/user_test.rb', 'test/unit/*.rb',
       'test/unit/elastic_search_test.rb',
       'test/unit/encrypt_decrypt_string_test.rb',

test/fixtures/admin_users.yml

@@ -5,3 +5,7 @@
 # below each fixture, per the syntax in the comments below
 #
 
+one:
+  email: [email protected]
+two:
+  email: [email protected]

test/fixtures/books.yml

@@ -3,7 +3,9 @@
 books_one:
   title: title
   details_url: details_url
+  cover_uri: "http://contentcafe2.btol.com/ContentCafe/Jacket.aspx?&userID=NYPL49807"
 
 books_two:
   title: title1
-  details_url: details_url1
+  details_url: details_url1
+  cover_uri: "http://contentcafe2.btol.com/ContentCafe/Jacket.aspx?&userID=NYPL49807"

test/fixtures/faqs.yml

@@ -4,8 +4,12 @@
 # model remove the '{}' from the fixture names and add the columns immediately
 # below each fixture, per the syntax in the comments below
 #
-one: {}
+one:
+  question: "text"
+  answer: "text"
+  position: 1
 # column: value
 #
-two: {}
-# column: value
+two: 
+  question: "text"
+  answer: "text"

test/fixtures/hold_changes.yml

@@ -0,0 +1,7 @@
+# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/Fixtures.html
+
+hold_changes1:
+  hold_id: hold1.id
+  admin_user_id: 1
+  status: new
+  hold: hold1

test/fixtures/holds.yml

@@ -6,16 +6,14 @@ hold1:
   quantity: 2
   teacher_set: teacher_set_one
   user: user1
-
-
+
 hold2:
   access_key: access_key2
   status: new
   quantity: 1
   teacher_set: teacher_set_one
   user: user1
 
-
 hold3:
   access_key: access_key3
   status: trouble
@@ -26,4 +24,28 @@ hold3:
 hold4:
   access_key: access_key4
   status: cancelled
-  teacher_set: teacher_set_four
+  teacher_set: teacher_set_four
+
+hold5:
+  access_key: access_key5
+  status: pending
+  teacher_set: teacher_set_four
+  user: user1
+
+hold6:
+  access_key: access_key6
+  status: transit
+  teacher_set: teacher_set_four
+  user: user1
+
+hold7:
+  access_key: access_key7
+  status: trouble
+  teacher_set: teacher_set_four
+  user: user1
+
+hold8:
+  access_key: access_key8
+  status: n_a
+  teacher_set: teacher_set_four
+  user: user1

test/fixtures/subjects.yml

@@ -0,0 +1,7 @@
+# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/Fixtures.html
+
+subjects_one:
+  title: "Authors Russiantest"
+
+subjects_two:
+  title: "Social"

test/fixtures/teacher_sets.yml

@@ -5,20 +5,26 @@ teacher_set_one:
   title: MyString
   available_copies: 2
   bnumber: '123'
+  books: books_one
+  subjects: [subjects_one, subjects_two]
 teacher_set_two:
   description: MyText1
   title: MyString1
   available_copies: 2
   bnumber: '12345'
+  books: books_two
+  subjects: subjects_two
 teacher_set_three:
   description: MyText1
   title: MyString1
   available_copies: 2
   bnumber: '123416'
   set_type: 'Book Club Set'
+  subjects: school_one
 teacher_set_four:
   description: MyText1
   title: MyString1
   available_copies: 2
   bnumber: '123418'
   set_type: 'Topic Set'
+  subjects: school_one

test/functional/admin/admin_users_controller_test.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+module Admin
+  class AdminUsersControllerTest < ActionController::TestCase
+    setup do
+      @admin_user = admin_users(:one)
+      sign_in AdminUser.create!(email: '[email protected]', password: 'password')
+    end
+
+    test "test index method" do
+      response = get :index
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    test "test form method" do
+      response = get :edit, id: @admin_user.id
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    test 'test enable_or_disable' do
+      response = put :enable_or_disable, params: {id: @admin_user.id}
+      assert_equal("E-mail notification change made!", flash[:notice])
+      assert_equal("302", response.code)
+      assert_response :redirect
+    end
+  end
+end

test/functional/admin/books_controller_test.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+module Admin
+  class BooksControllerTest < ActionController::TestCase
+    setup do
+      @book = books(:books_two)
+      sign_in AdminUser.create!(email: '[email protected]', password: 'password')
+    end
+
+    test "test index method" do
+      get :index
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    test "test show method" do
+      get :show, params: { id: @book.id }
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    test "test show method with version param" do
+      create_book_version
+      get :show, params: { id: @book.id, version: 1 }
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    test "test history method" do
+      create_book_version
+      get :history, params: { id: @book.id }
+      assert_equal("200", response.code)
+      assert_response :success
+    end
+
+    private
+
+    def create_book_version
+      book = Book.find(@book.id)
+      book.details_url = 'details_url'
+      book.save!
+    end
+  end
+end