From 8fb0e55a6bbcec5588e4c5a0827962a4dc552496 Mon Sep 17 00:00:00 2001 From: Nikitas Tampakis Date: Tue, 21 Nov 2017 17:24:24 -0500 Subject: [PATCH 1/3] upgrade to rails 5.1 --- .../javascripts/locations/application.js | 2 +- app/models/locations/delivery_location.rb | 2 +- app/models/locations/holding_location.rb | 6 +- ...738_create_locations_delivery_locations.rb | 2 +- ...150520130710_create_locations_libraries.rb | 2 +- ..._library_to_locations_delivery_location.rb | 2 +- ...2558_create_locations_holding_locations.rb | 2 +- ...s_library_to_locations_holding_location.rb | 2 +- ...00_add_locations_holdings_delivery_join.rb | 2 +- ...d_additional_fields_to_holding_location.rb | 2 +- ...18_add_pickup_attr_to_delivery_location.rb | 2 +- ...170114_create_locations_hours_locations.rb | 2 +- ...ours_loc_to_locations_holding_locations.rb | 2 +- ...d_digital_location_to_delivery_location.rb | 2 +- .../20160127155209_set_digital_location.rb | 2 +- ...rculates_to_locations_holding_locations.rb | 2 +- ...ng_library_to_locations_holding_library.rb | 2 +- .../20160610203622_create_locations_floors.rb | 2 +- ...d_locations_library_to_locations_floors.rb | 2 +- ...195737_add_order_to_locations_libraries.rb | 2 +- lib/generators/locations/install_generator.rb | 7 +- lib/locations/version.rb | 2 +- locations.gemspec | 9 +-- .../delivery_locations_controller_spec.rb | 52 +++++++-------- .../locations/floors_controller_spec.rb | 32 ++++----- .../holding_locations_controller_spec.rb | 60 ++++++++--------- .../locations/home_controller_spec.rb | 2 +- .../hours_locations_controller_spec.rb | 64 +++++++++--------- .../locations/libraries_controller_spec.rb | 66 +++++++++---------- .../locations/map_controller_spec.rb | 12 ++-- spec/factories/locations/floors.rb | 2 +- .../delivery_locations_json_view_spec.rb | 6 +- .../holding_locations_json_view_spec.rb | 10 +-- .../hours_locations_json_view_spec.rb | 7 +- .../locations/libraries_json_view_spec.rb | 6 +- spec/spec_helper.rb | 2 + 36 files changed, 195 insertions(+), 188 deletions(-) diff --git a/app/assets/javascripts/locations/application.js b/app/assets/javascripts/locations/application.js index b2ab96d..466924b 100644 --- a/app/assets/javascripts/locations/application.js +++ b/app/assets/javascripts/locations/application.js @@ -1,5 +1,5 @@ //= require jquery -//= require jquery_ujs +//= require rails-ujs //= require bootstrap-sprockets //= require jquery-tablesorter //= require jquery-tablesorter/widgets/widget-editable diff --git a/app/models/locations/delivery_location.rb b/app/models/locations/delivery_location.rb index a9e92e8..9b96fc0 100644 --- a/app/models/locations/delivery_location.rb +++ b/app/models/locations/delivery_location.rb @@ -3,7 +3,7 @@ class DeliveryLocation < ActiveRecord::Base include Locations::Labeled include Locations::WithLibrary - has_and_belongs_to_many :holding_locations, -> { uniq }, + has_and_belongs_to_many :holding_locations, -> { distinct }, class_name: 'Locations::HoldingLocation', join_table: 'locations_holdings_delivery', foreign_key: 'locations_holding_location_id', diff --git a/app/models/locations/holding_location.rb b/app/models/locations/holding_location.rb index 344ef66..64ed0fa 100644 --- a/app/models/locations/holding_location.rb +++ b/app/models/locations/holding_location.rb @@ -3,10 +3,10 @@ class HoldingLocation < ActiveRecord::Base include Locations::Coded include Locations::WithLibrary - belongs_to :hours_location, class_name: 'Locations::HoursLocation', foreign_key: :locations_hours_location_id - belongs_to :holding_library, class_name: 'Locations::Library', foreign_key: :holding_library_id + belongs_to :hours_location, class_name: 'Locations::HoursLocation', foreign_key: :locations_hours_location_id, optional: true + belongs_to :holding_library, class_name: 'Locations::Library', foreign_key: :holding_library_id, optional: true - has_and_belongs_to_many :delivery_locations, -> { uniq }, + has_and_belongs_to_many :delivery_locations, -> { distinct }, class_name: 'Locations::DeliveryLocation', join_table: 'locations_holdings_delivery', foreign_key: 'locations_delivery_location_id', diff --git a/db/migrate/20150519154738_create_locations_delivery_locations.rb b/db/migrate/20150519154738_create_locations_delivery_locations.rb index 26c75d7..5926236 100644 --- a/db/migrate/20150519154738_create_locations_delivery_locations.rb +++ b/db/migrate/20150519154738_create_locations_delivery_locations.rb @@ -1,4 +1,4 @@ -class CreateLocationsDeliveryLocations < ActiveRecord::Migration +class CreateLocationsDeliveryLocations < ActiveRecord::Migration[4.2] def change create_table :locations_delivery_locations do |t| t.string :label diff --git a/db/migrate/20150520130710_create_locations_libraries.rb b/db/migrate/20150520130710_create_locations_libraries.rb index 33a8389..c03211b 100644 --- a/db/migrate/20150520130710_create_locations_libraries.rb +++ b/db/migrate/20150520130710_create_locations_libraries.rb @@ -1,4 +1,4 @@ -class CreateLocationsLibraries < ActiveRecord::Migration +class CreateLocationsLibraries < ActiveRecord::Migration[4.2] def change create_table :locations_libraries do |t| t.string :label diff --git a/db/migrate/20150520175207_add_locations_library_to_locations_delivery_location.rb b/db/migrate/20150520175207_add_locations_library_to_locations_delivery_location.rb index f28af76..5e72994 100644 --- a/db/migrate/20150520175207_add_locations_library_to_locations_delivery_location.rb +++ b/db/migrate/20150520175207_add_locations_library_to_locations_delivery_location.rb @@ -1,4 +1,4 @@ -class AddLocationsLibraryToLocationsDeliveryLocation < ActiveRecord::Migration +class AddLocationsLibraryToLocationsDeliveryLocation < ActiveRecord::Migration[4.2] def change add_reference :locations_delivery_locations, :locations_library, index: true, foreign_key: true end diff --git a/db/migrate/20150521202558_create_locations_holding_locations.rb b/db/migrate/20150521202558_create_locations_holding_locations.rb index 958f3dc..a812f05 100644 --- a/db/migrate/20150521202558_create_locations_holding_locations.rb +++ b/db/migrate/20150521202558_create_locations_holding_locations.rb @@ -1,4 +1,4 @@ -class CreateLocationsHoldingLocations < ActiveRecord::Migration +class CreateLocationsHoldingLocations < ActiveRecord::Migration[4.2] def change create_table :locations_holding_locations do |t| t.string :label diff --git a/db/migrate/20150521203608_add_locations_library_to_locations_holding_location.rb b/db/migrate/20150521203608_add_locations_library_to_locations_holding_location.rb index 80b5f9c..630e522 100644 --- a/db/migrate/20150521203608_add_locations_library_to_locations_holding_location.rb +++ b/db/migrate/20150521203608_add_locations_library_to_locations_holding_location.rb @@ -1,4 +1,4 @@ -class AddLocationsLibraryToLocationsHoldingLocation < ActiveRecord::Migration +class AddLocationsLibraryToLocationsHoldingLocation < ActiveRecord::Migration[4.2] def change add_reference :locations_holding_locations, :locations_library, index: true, foreign_key: true end diff --git a/db/migrate/20150521221100_add_locations_holdings_delivery_join.rb b/db/migrate/20150521221100_add_locations_holdings_delivery_join.rb index b3a332d..1a44c36 100644 --- a/db/migrate/20150521221100_add_locations_holdings_delivery_join.rb +++ b/db/migrate/20150521221100_add_locations_holdings_delivery_join.rb @@ -1,4 +1,4 @@ -class AddLocationsHoldingsDeliveryJoin < ActiveRecord::Migration +class AddLocationsHoldingsDeliveryJoin < ActiveRecord::Migration[4.2] def change create_table :locations_holdings_delivery, id: false do |t| t.integer :locations_delivery_location_id, index: false diff --git a/db/migrate/20150522175704_add_additional_fields_to_holding_location.rb b/db/migrate/20150522175704_add_additional_fields_to_holding_location.rb index 4a24e1f..ba421e2 100644 --- a/db/migrate/20150522175704_add_additional_fields_to_holding_location.rb +++ b/db/migrate/20150522175704_add_additional_fields_to_holding_location.rb @@ -1,4 +1,4 @@ -class AddAdditionalFieldsToHoldingLocation < ActiveRecord::Migration +class AddAdditionalFieldsToHoldingLocation < ActiveRecord::Migration[4.2] def change add_column :locations_holding_locations, :aeon_location, :boolean, default: false add_column :locations_holding_locations, :recap_electronic_delivery_location, :boolean, default: false diff --git a/db/migrate/20150527211018_add_pickup_attr_to_delivery_location.rb b/db/migrate/20150527211018_add_pickup_attr_to_delivery_location.rb index 1feaa93..c13502d 100644 --- a/db/migrate/20150527211018_add_pickup_attr_to_delivery_location.rb +++ b/db/migrate/20150527211018_add_pickup_attr_to_delivery_location.rb @@ -1,4 +1,4 @@ -class AddPickupAttrToDeliveryLocation < ActiveRecord::Migration +class AddPickupAttrToDeliveryLocation < ActiveRecord::Migration[4.2] def change add_column :locations_delivery_locations, :gfa_pickup, :string add_column :locations_delivery_locations, :pickup_location, :boolean, default: false diff --git a/db/migrate/20150603170114_create_locations_hours_locations.rb b/db/migrate/20150603170114_create_locations_hours_locations.rb index 29ba130..b72be9e 100644 --- a/db/migrate/20150603170114_create_locations_hours_locations.rb +++ b/db/migrate/20150603170114_create_locations_hours_locations.rb @@ -1,4 +1,4 @@ -class CreateLocationsHoursLocations < ActiveRecord::Migration +class CreateLocationsHoursLocations < ActiveRecord::Migration[4.2] def change create_table :locations_hours_locations do |t| t.string :code diff --git a/db/migrate/20150603212708_add_hours_loc_to_locations_holding_locations.rb b/db/migrate/20150603212708_add_hours_loc_to_locations_holding_locations.rb index 3ada0f6..79ce34a 100644 --- a/db/migrate/20150603212708_add_hours_loc_to_locations_holding_locations.rb +++ b/db/migrate/20150603212708_add_hours_loc_to_locations_holding_locations.rb @@ -1,4 +1,4 @@ -class AddHoursLocToLocationsHoldingLocations < ActiveRecord::Migration +class AddHoursLocToLocationsHoldingLocations < ActiveRecord::Migration[4.2] def change add_reference :locations_holding_locations, :locations_hours_location, foreign_key: true end diff --git a/db/migrate/20160127140802_add_digital_location_to_delivery_location.rb b/db/migrate/20160127140802_add_digital_location_to_delivery_location.rb index 7fdd394..084db6e 100644 --- a/db/migrate/20160127140802_add_digital_location_to_delivery_location.rb +++ b/db/migrate/20160127140802_add_digital_location_to_delivery_location.rb @@ -1,4 +1,4 @@ -class AddDigitalLocationToDeliveryLocation < ActiveRecord::Migration +class AddDigitalLocationToDeliveryLocation < ActiveRecord::Migration[4.2] def change add_column :locations_delivery_locations, :digital_location, :boolean end diff --git a/db/migrate/20160127155209_set_digital_location.rb b/db/migrate/20160127155209_set_digital_location.rb index 1b92bc9..7633016 100644 --- a/db/migrate/20160127155209_set_digital_location.rb +++ b/db/migrate/20160127155209_set_digital_location.rb @@ -1,4 +1,4 @@ -class SetDigitalLocation < ActiveRecord::Migration +class SetDigitalLocation < ActiveRecord::Migration[4.2] def change Locations::DeliveryLocation.update_all("digital_location=pickup_location") end diff --git a/db/migrate/20160218152356_add_circulates_to_locations_holding_locations.rb b/db/migrate/20160218152356_add_circulates_to_locations_holding_locations.rb index bb7059b..7f36043 100644 --- a/db/migrate/20160218152356_add_circulates_to_locations_holding_locations.rb +++ b/db/migrate/20160218152356_add_circulates_to_locations_holding_locations.rb @@ -1,4 +1,4 @@ -class AddCirculatesToLocationsHoldingLocations < ActiveRecord::Migration +class AddCirculatesToLocationsHoldingLocations < ActiveRecord::Migration[4.2] def change add_column :locations_holding_locations, :circulates, :boolean, default: true reversible do |direction| diff --git a/db/migrate/20160526020836_add_holding_library_to_locations_holding_library.rb b/db/migrate/20160526020836_add_holding_library_to_locations_holding_library.rb index f677daa..4e73936 100644 --- a/db/migrate/20160526020836_add_holding_library_to_locations_holding_library.rb +++ b/db/migrate/20160526020836_add_holding_library_to_locations_holding_library.rb @@ -1,4 +1,4 @@ -class AddHoldingLibraryToLocationsHoldingLibrary < ActiveRecord::Migration +class AddHoldingLibraryToLocationsHoldingLibrary < ActiveRecord::Migration[4.2] def change add_column :locations_holding_locations, :holding_library_id, :integer, index: true add_foreign_key :locations_holding_locations, :locations_libraries, column: :holding_library_id diff --git a/db/migrate/20160610203622_create_locations_floors.rb b/db/migrate/20160610203622_create_locations_floors.rb index 5f367b9..2c1eb7f 100644 --- a/db/migrate/20160610203622_create_locations_floors.rb +++ b/db/migrate/20160610203622_create_locations_floors.rb @@ -1,4 +1,4 @@ -class CreateLocationsFloors < ActiveRecord::Migration +class CreateLocationsFloors < ActiveRecord::Migration[4.2] def change create_table :locations_floors do |t| t.string :label diff --git a/db/migrate/20160614154022_add_locations_library_to_locations_floors.rb b/db/migrate/20160614154022_add_locations_library_to_locations_floors.rb index ef8c2e0..6bf3814 100644 --- a/db/migrate/20160614154022_add_locations_library_to_locations_floors.rb +++ b/db/migrate/20160614154022_add_locations_library_to_locations_floors.rb @@ -1,4 +1,4 @@ -class AddLocationsLibraryToLocationsFloors < ActiveRecord::Migration +class AddLocationsLibraryToLocationsFloors < ActiveRecord::Migration[4.2] def change add_reference :locations_floors, :locations_library, index: true, foreign_key: true end diff --git a/db/migrate/20161228195737_add_order_to_locations_libraries.rb b/db/migrate/20161228195737_add_order_to_locations_libraries.rb index 36dddbb..3bbf59c 100644 --- a/db/migrate/20161228195737_add_order_to_locations_libraries.rb +++ b/db/migrate/20161228195737_add_order_to_locations_libraries.rb @@ -1,4 +1,4 @@ -class AddOrderToLocationsLibraries < ActiveRecord::Migration +class AddOrderToLocationsLibraries < ActiveRecord::Migration[4.2] def change add_column :locations_libraries, :order, :integer, default: 0 end diff --git a/lib/generators/locations/install_generator.rb b/lib/generators/locations/install_generator.rb index 6d94e9c..14953ed 100644 --- a/lib/generators/locations/install_generator.rb +++ b/lib/generators/locations/install_generator.rb @@ -6,7 +6,8 @@ class InstallGenerator < Rails::Generators::Base def add_gems gem 'bootstrap-sass' - gem 'yaml_db', '~> 0.3.0' + gem 'yaml_db', '~> 0.6' + gem 'jquery-rails' Bundler.with_clean_env do run "bundle install" end @@ -15,6 +16,10 @@ def add_gems def friendly_id gem 'friendly_id', '~> 5.1.0' generate 'friendly_id' + migration_file = Dir['db/migrate/*_create_friendly_id_slugs.rb'].first + inject_into_file migration_file, after: 'ActiveRecord::Migration' do + '[4.2]' + end gsub_file 'config/initializers/friendly_id.rb', 'new edit', 'create edit' end diff --git a/lib/locations/version.rb b/lib/locations/version.rb index 6f4fa3b..939005d 100644 --- a/lib/locations/version.rb +++ b/lib/locations/version.rb @@ -1,3 +1,3 @@ module Locations - VERSION = "0.5.0" + VERSION = "1.0.0" end diff --git a/locations.gemspec b/locations.gemspec index e7bf052..45568fa 100644 --- a/locations.gemspec +++ b/locations.gemspec @@ -17,22 +17,23 @@ Gem::Specification.new do |s| s.files = Dir['{app,config,db,lib}/**/*', 'MIT-LICENSE', 'Rakefile', 'README.md'] - s.add_dependency 'rails', '~> 4.2' + s.add_dependency 'rails', '~> 5.1' s.add_dependency 'bootstrap-sass', '~> 3.3' s.add_dependency 'friendly_id', '~> 5.1.0' - s.add_dependency 'yaml_db', '~> 0.3.0' + s.add_dependency 'yaml_db', '~> 0.6' s.add_dependency 'jquery-tablesorter', '~> 1.21' s.add_dependency 'rmagick' s.add_dependency 'carrierwave' s.add_development_dependency 'sqlite3' s.add_development_dependency 'rspec-rails', '~> 3.1' - s.add_development_dependency 'engine_cart', '~> 1.0' + s.add_development_dependency 'engine_cart', '~> 1.2' s.add_development_dependency 'factory_girl_rails', '~> 4.5.0' s.add_development_dependency 'faker', '~> 1.4.3' s.add_development_dependency 'database_cleaner', '~> 1.3' - s.add_development_dependency 'capybara', '~> 2.4.4' + s.add_development_dependency 'capybara', '~> 2.13' s.add_development_dependency 'poltergeist' s.add_development_dependency 'coveralls' s.add_development_dependency 'webmock' + s.add_development_dependency 'rails-controller-testing' end diff --git a/spec/controllers/locations/delivery_locations_controller_spec.rb b/spec/controllers/locations/delivery_locations_controller_spec.rb index 6b96c2f..3b615a2 100644 --- a/spec/controllers/locations/delivery_locations_controller_spec.rb +++ b/spec/controllers/locations/delivery_locations_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' module Locations - describe DeliveryLocationsController, type: :controller do + describe DeliveryLocationsController, type: :controller do routes { Locations::Engine.routes } let(:invalid_attributes) { @@ -15,15 +15,15 @@ module Locations it 'assigns all delivery_locations as @delivery_locations' do delivery_location = FactoryGirl.create(:delivery_location) - get :index, {}, valid_session + get :index expect(assigns(:delivery_locations)).to eq([delivery_location]) end it 'delivery_locations is active in navbar' do - get :index, {}, valid_session - expect(response.body.include?('
  • delivery_location.to_param, delivery_location: new_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: new_attributes } delivery_location.reload expect(delivery_location.label).to eq updated_label end it 'assigns the requested delivery_location as @delivery_location' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, { :id => delivery_location.to_param, delivery_location: new_attributes }, valid_session + put :update, params: { :id => delivery_location.to_param, delivery_location: new_attributes } expect(assigns(:delivery_location)).to eq(delivery_location) end it 'passes flash notice message' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, {:id => delivery_location.to_param, delivery_location: new_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: new_attributes } expect(flash[:notice]).to be_present end it 'redirects to the delivery_location' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, {:id => delivery_location.to_param, delivery_location: new_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: new_attributes } expect(response).to redirect_to(delivery_location) end end @@ -147,19 +147,19 @@ module Locations context 'with invalid params' do it 'assigns the delivery_location as @delivery_location' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, {:id => delivery_location.to_param, delivery_location: invalid_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: invalid_attributes } expect(assigns(:delivery_location)).to eq(delivery_location) end it 'passes a flash error message' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, {:id => delivery_location.to_param, delivery_location: invalid_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: invalid_attributes } expect(flash[:error]).to be_present end it 're-renders the "edit" template' do delivery_location = FactoryGirl.create(:delivery_location) - put :update, {:id => delivery_location.to_param, delivery_location: invalid_attributes }, valid_session + put :update, params: {:id => delivery_location.to_param, delivery_location: invalid_attributes } expect(response).to render_template('edit') end end @@ -169,19 +169,19 @@ module Locations it 'destroys the requested delivery_location' do delivery_location = FactoryGirl.create(:delivery_location) expect { - delete :destroy, { id: delivery_location.to_param }, valid_session + delete :destroy, params: { id: delivery_location.to_param } }.to change(DeliveryLocation, :count).by(-1) end it 'passes flash notice message' do delivery_location = FactoryGirl.create(:delivery_location) - delete :destroy, { id: delivery_location.to_param }, valid_session + delete :destroy, params: { id: delivery_location.to_param } expect(flash[:notice]).to be_present end it 'redirects to the delivery_locations list' do delivery_location = FactoryGirl.create(:delivery_location) - delete :destroy, { id: delivery_location.to_param }, valid_session + delete :destroy, params: { id: delivery_location.to_param } expect(response).to redirect_to(delivery_locations_path) end end diff --git a/spec/controllers/locations/floors_controller_spec.rb b/spec/controllers/locations/floors_controller_spec.rb index 72af7d2..3e780d9 100644 --- a/spec/controllers/locations/floors_controller_spec.rb +++ b/spec/controllers/locations/floors_controller_spec.rb @@ -48,7 +48,7 @@ module Locations it "assigns all floors as @floors" do floor = Floor.create! valid_attributes - get :index, {:library_id => floor.locations_library_id, :id => floor.to_param}, valid_session + get :index, params: {:library_id => floor.locations_library_id, :id => floor.to_param} expect(assigns(:floors)).to eq([floor]) end @@ -57,7 +57,7 @@ module Locations describe "GET #show" do it "assigns the requested floor as @floor" do floor = Floor.create! valid_attributes - get :show, {:library_id => floor.locations_library_id, :id => floor.to_param}, valid_session + get :show, params: {:library_id => floor.locations_library_id, :id => floor.to_param} expect(assigns(:floor)).to eq(floor) end end @@ -65,7 +65,7 @@ module Locations describe "GET #new" do it "assigns a new floor as @floor" do floor = Floor.create! valid_attributes - get :new, {:library_id => floor.locations_library_id, :id => floor.to_param}, valid_session + get :new, params: {:library_id => floor.locations_library_id, :id => floor.to_param} expect(assigns(:floor)).to be_a_new(Floor) end end @@ -73,7 +73,7 @@ module Locations describe "GET #edit" do it "assigns the requested floor as @floor" do floor = Floor.create! valid_attributes - get :edit, {:library_id => floor.locations_library_id, :id => floor.to_param}, valid_session + get :edit, params: {:library_id => floor.locations_library_id, :id => floor.to_param} expect(assigns(:floor)).to eq(floor) end end @@ -83,30 +83,30 @@ module Locations context "with valid params" do it "creates a new Floor" do expect { - post :create, {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes}, valid_session + post :create, params: {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes} }.to change(Floor, :count).by(1) end it "assigns a newly created floor as @floor" do - post :create, {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes}, valid_session + post :create, params: {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes} expect(assigns(:floor)).to be_a(Floor) expect(assigns(:floor)).to be_persisted end it "redirects to the created floor" do - post :create, {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes}, valid_session + post :create, params: {:library_id => valid_attributes[:locations_library_id], :floor => valid_attributes} expect(response).to redirect_to(library_floor_path(valid_attributes[:locations_library_id], Floor.last)) end end context "with invalid params" do it "assigns a newly created but unsaved floor as @floor" do - post :create, {:library_id => valid_attributes[:locations_library_id], :floor => invalid_attributes}, valid_session + post :create, params: {:library_id => valid_attributes[:locations_library_id], :floor => invalid_attributes} expect(assigns(:floor)).to be_a_new(Floor) end it "re-renders the 'new' template" do - post :create, {:library_id => valid_attributes[:locations_library_id], :floor => invalid_attributes}, valid_session + post :create, params: {:library_id => valid_attributes[:locations_library_id], :floor => invalid_attributes} expect(response).to render_template("new") end end @@ -124,19 +124,19 @@ module Locations it "updates the requested floor" do floor = Floor.create! valid_attributes - put :update, {:library_id => new_attributes[:locations_library_id], :id => floor.to_param, :floor => new_attributes}, valid_session + put :update, params: {:library_id => new_attributes[:locations_library_id], :id => floor.to_param, :floor => new_attributes} floor.reload end it "assigns the requested floor as @floor" do floor = Floor.create! valid_attributes - put :update, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => valid_attributes}, valid_session + put :update, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => valid_attributes} expect(assigns(:floor)).to eq(floor) end it "redirects to the floor" do floor = Floor.create! valid_attributes - put :update, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => valid_attributes}, valid_session + put :update, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => valid_attributes} expect(response).to redirect_to(library_floor_path(valid_attributes[:locations_library_id], floor)) end end @@ -144,13 +144,13 @@ module Locations context "with invalid params" do it "assigns the floor as @floor" do floor = Floor.create! valid_attributes - put :update, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => invalid_attributes}, valid_session + put :update, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => invalid_attributes} expect(assigns(:floor)).to eq(floor) end it "re-renders the 'edit' template" do floor = Floor.create! valid_attributes - put :update, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => invalid_attributes}, valid_session + put :update, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param, :floor => invalid_attributes} expect(response).to render_template("edit") end end @@ -160,13 +160,13 @@ module Locations it "destroys the requested floor" do floor = Floor.create! valid_attributes expect { - delete :destroy, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param}, valid_session + delete :destroy, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param} }.to change(Floor, :count).by(-1) end it "redirects to the floors list" do floor = Floor.create! valid_attributes - delete :destroy, {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param}, valid_session + delete :destroy, params: {:library_id => valid_attributes[:locations_library_id], :id => floor.to_param} expect(response).to redirect_to(library_floors_path(valid_attributes[:locations_library_id])) end end diff --git a/spec/controllers/locations/holding_locations_controller_spec.rb b/spec/controllers/locations/holding_locations_controller_spec.rb index b8538e0..d331606 100644 --- a/spec/controllers/locations/holding_locations_controller_spec.rb +++ b/spec/controllers/locations/holding_locations_controller_spec.rb @@ -16,28 +16,28 @@ module Locations it "assigns all holding_locations as @holding_locations" do holding_location = FactoryGirl.create(:holding_location) - get :index, {}, valid_session + get :index expect(assigns(:holding_locations)).to eq([holding_location]) end it 'holding_locations is active in navbar' do - get :index, {}, valid_session - expect(response.body.include?('
  • " library diff --git a/spec/requests/locations/delivery_locations_json_view_spec.rb b/spec/requests/locations/delivery_locations_json_view_spec.rb index 3b1efc5..3101e7c 100644 --- a/spec/requests/locations/delivery_locations_json_view_spec.rb +++ b/spec/requests/locations/delivery_locations_json_view_spec.rb @@ -4,7 +4,7 @@ module Locations describe 'DeliveryLocation json view', type: :request do it 'Renders the json template' do - get delivery_locations_path, format: :json + get delivery_locations_path, params: { format: :json } expect(response).to render_template(:index) expect(response.content_type).to eq 'application/json' end @@ -33,7 +33,7 @@ module Locations } expected << attrs end - get delivery_locations_path, format: :json + get delivery_locations_path, params: { format: :json } expect(response.body).to eq expected.to_json end @@ -55,7 +55,7 @@ module Locations order: delivery_location.library.order } } - get delivery_location_path(delivery_location), format: :json + get delivery_location_path(delivery_location), params: { format: :json } expect(response.body).to eq expected.to_json end diff --git a/spec/requests/locations/holding_locations_json_view_spec.rb b/spec/requests/locations/holding_locations_json_view_spec.rb index f5823d3..3471a76 100644 --- a/spec/requests/locations/holding_locations_json_view_spec.rb +++ b/spec/requests/locations/holding_locations_json_view_spec.rb @@ -4,7 +4,7 @@ module Locations describe 'HoldingLocation json view', type: :request do it 'Renders the json template' do - get holding_locations_path, format: :json + get holding_locations_path, params: { format: :json } expect(response).to render_template(:index) expect(response.content_type).to eq 'application/json' end @@ -66,7 +66,7 @@ module Locations } } expected << attrs - get holding_locations_path, format: :json + get holding_locations_path, params: { format: :json } expect(response.body).to eq expected.sort_by{ |k| k[:code] }.to_json end @@ -112,7 +112,7 @@ module Locations digital_location: dl.digital_location } end - get holding_location_path(holding_location), format: :json + get holding_location_path(holding_location), params: { format: :json } expect(response.body).to eq expected.to_json end @@ -161,7 +161,7 @@ module Locations digital_location: dl.digital_location } end - get holding_location_path(holding_location), format: :json + get holding_location_path(holding_location), params: { format: :json } expect(response.body).to eq expected.to_json end it "/holding_locations/{code} looks as we'd expect with holding_library" do @@ -210,7 +210,7 @@ module Locations digital_location: dl.digital_location } end - get holding_location_path(holding_location), format: :json + get holding_location_path(holding_location), params: { format: :json } expect(response.body).to eq expected.to_json end end diff --git a/spec/requests/locations/hours_locations_json_view_spec.rb b/spec/requests/locations/hours_locations_json_view_spec.rb index 499687c..5a73806 100644 --- a/spec/requests/locations/hours_locations_json_view_spec.rb +++ b/spec/requests/locations/hours_locations_json_view_spec.rb @@ -4,7 +4,7 @@ module Locations describe 'HoursLocation json view', type: :request do it 'Renders the json template' do - get hours_locations_path, format: :json + get hours_locations_path, params: { format: :json } expect(response).to render_template(:index) expect(response.content_type).to eq 'application/json' end @@ -22,9 +22,8 @@ module Locations } expected << attrs end - get hours_locations_path, format: :json + get hours_locations_path, params: { format: :json } expect(response.body).to eq expected.to_json - end it "/hours_locations/{code} looks as we'd expect" do @@ -33,7 +32,7 @@ module Locations label: hours_location.label, code: hours_location.code } - get hours_location_path(hours_location), format: :json + get hours_location_path(hours_location), params: { format: :json } expect(response.body).to eq expected.to_json end diff --git a/spec/requests/locations/libraries_json_view_spec.rb b/spec/requests/locations/libraries_json_view_spec.rb index 9df6103..06986de 100644 --- a/spec/requests/locations/libraries_json_view_spec.rb +++ b/spec/requests/locations/libraries_json_view_spec.rb @@ -4,7 +4,7 @@ module Locations describe 'Library json view', type: :request do it 'Renders the json template' do - get libraries_path, format: :json + get libraries_path, params: { format: :json } expect(response).to render_template(:index) expect(response.content_type).to eq 'application/json' end @@ -24,7 +24,7 @@ module Locations expected << attrs end sorted = expected.sort_by { |l| [l[:order], l[:label]] } - get libraries_path, format: :json + get libraries_path, params: { format: :json } expect(response.body).to eq sorted.to_json end @@ -36,7 +36,7 @@ module Locations code: library.code, order: library.order } - get library_path(library), format: :json + get library_path(library), params: { format: :json } expect(response.body).to eq expected.to_json end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a73dc74..0abad88 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,8 @@ require 'engine_cart' require 'database_cleaner' require 'capybara/poltergeist' +require 'rails-controller-testing' +Rails::Controller::Testing.install Capybara.javascript_driver = :poltergeist From 421c83f706f44d45543feecbc3f42c766261c8a3 Mon Sep 17 00:00:00 2001 From: Nikitas Tampakis Date: Wed, 22 Nov 2017 13:38:54 -0500 Subject: [PATCH 2/3] only call fetch voyager records for by title locations --- app/controllers/locations/map_controller.rb | 18 +++++----- app/models/locations/map.rb | 36 ++++++++++--------- .../locations/map_controller_spec.rb | 14 +++++--- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/app/controllers/locations/map_controller.rb b/app/controllers/locations/map_controller.rb index 8243c80..8668990 100644 --- a/app/controllers/locations/map_controller.rb +++ b/app/controllers/locations/map_controller.rb @@ -4,21 +4,19 @@ module Locations class MapController < ApplicationController def index - @map = Map.new(id: map_params[:id],loc: map_params[:loc]) - - if @map.valid? - unless @map.on_reserve? - redirect_to @map.url - end - else - render plain: "Invalid parameters.", status: 400 + @map = Map.new(id: map_params[:id],loc: map_params[:loc], callno: map_params[:callno]) + if @map.valid? + unless @map.on_reserve? + redirect_to @map.url end - + else + render plain: "Invalid parameters.", status: 400 + end end # Only allow a trusted parameter "white list" through. def map_params - params.permit(:id, :loc) + params.permit(:id, :loc, :callno) end end diff --git a/app/models/locations/map.rb b/app/models/locations/map.rb index eee5703..f3450fd 100644 --- a/app/models/locations/map.rb +++ b/app/models/locations/map.rb @@ -1,36 +1,42 @@ module Locations class Map - attr_reader :id, :loc + attr_reader :id, :loc, :cn - def initialize(id: nil, loc: nil) + def initialize(id: nil, loc: nil, callno: nil) @id = id @loc = loc + @cn = callno end def url - return locator_url if locator_url - return stackmap_url if stackmap_url - "https://pulsearch.princeton.edu/requests/#{id}" + if locator_libs.include? lib.code + locator_url + elsif stackmap_libs.include? lib.code + stackmap_url + else + "https://pulsearch.princeton.edu/requests/#{id}" + end end def locator_url - return false unless locator_libs.include? lib.code - "http://library.princeton.edu/locator/index.php?loc=#{loc}&id=#{id}" + "https://library.princeton.edu/locator/index.php?loc=#{loc}&id=#{id}" end def stackmap_url - return false unless stackmap_libs.include? lib.code - stackmap_url = 'http://princeton.stackmap.com' + stackmap_url = 'https://princeton.stackmap.com' URI.encode("#{stackmap_url}/view/?callno=#{callno}&location=#{loc}&library=#{lib.label}") end def callno - return bibrec['title_sort'].first if by_title_locations.include? loc - bibrec['call_number_browse_s'].first + if by_title_locations.include? loc + bibrec['title_sort'].first + else + cn || bibrec['call_number_browse_s'].first + end end # Need to include all non-stackmap libraries here to support the Main Catalog - # that displays the locator link on EVERY record. + # that displays the locator link on EVERY record. def locator_libs %w(firestone hrc annexa annexb mudd online rare recap) end @@ -64,13 +70,11 @@ def hours_location end def valid? - return true if !holding_location.nil? && !bibrec.nil? - false + !holding_location.nil? end def on_reserve? - return true if closed_stack_reserves.include? loc - false + closed_stack_reserves.include? loc end private diff --git a/spec/controllers/locations/map_controller_spec.rb b/spec/controllers/locations/map_controller_spec.rb index c448ed0..ceb8b65 100644 --- a/spec/controllers/locations/map_controller_spec.rb +++ b/spec/controllers/locations/map_controller_spec.rb @@ -18,21 +18,27 @@ module Locations it 'redirects to the locator url' do get :index, params - expect(response).to redirect_to("http://library.princeton.edu/locator/index.php?loc=#{map.loc}&id=#{map.id}") + expect(response).to redirect_to("https://library.princeton.edu/locator/index.php?loc=#{map.loc}&id=#{map.id}") end end context 'with stackmap params' do let(:id) { '9547751' } let(:loc) { holding_location.code } - let(:callno) { map.bibrec['call_number_display'].first } + let(:bibdata_callno) { map.bibrec['call_number_display'].first } + let(:callno) { 'B15' } let(:holding_location) { FactoryGirl.create(:holding_location_stackmap, library_args: { code: 'lewis' }) } before { stub_request(:get, map_bibdata).to_return(status: 200, body: fixture('stackmap_bibrec.json')) } - it 'redirects to the stackmap url' do + it 'redirects to the stackmap url with provided call number param' do + params[:params][:callno] = callno get :index, params - expect(response).to redirect_to(URI.encode("http://princeton.stackmap.com/view/?callno=#{callno}&location=#{map.loc}&library=#{map.lib.label}")) + expect(response).to redirect_to(URI.encode("https://princeton.stackmap.com/view/?callno=#{callno}&location=#{map.loc}&library=#{map.lib.label}")) + end + it 'redirects to the stackmap url with bibdata call number when not provided' do + get :index, params + expect(response).to redirect_to(URI.encode("https://princeton.stackmap.com/view/?callno=#{bibdata_callno}&location=#{map.loc}&library=#{map.lib.label}")) end end From cc8f79d5564a0cfcb835cb2e4c55ec1b15758f5b Mon Sep 17 00:00:00 2001 From: Nikitas Tampakis Date: Wed, 22 Nov 2017 14:02:21 -0500 Subject: [PATCH 3/3] update ruby version in travis for rails 5 compatibility --- .travis.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 469e265..73ba3cb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: ruby rvm: - - 2.1.5 - - 2.2.0 + - 2.3.5 script: bundle exec rake ci @@ -17,4 +16,4 @@ notifications: on_success: "change" on_failure: "always" template: - - "%{repository}//%{branch}@%{commit} by %{author}: %{message} - %{build_url}" \ No newline at end of file + - "%{repository}//%{branch}@%{commit} by %{author}: %{message} - %{build_url}"