From caee8627241d02e6856135b0bae9e0a89b22afff Mon Sep 17 00:00:00 2001 From: Chris Beer Date: Mon, 1 Aug 2016 10:04:58 -0700 Subject: [PATCH] Support Rails 5 (#1483) * Provide default images for all image uploaders * Support Rails 5 * Begin fixing compatibility with Rails 4 tests --- .rubocop.yml | 4 +++ .travis.yml | 5 ++++ Gemfile | 2 ++ .../concerns/spotlight/controller.rb | 3 +- .../spotlight/catalog_controller.rb | 2 +- .../spotlight/searches_controller.rb | 6 +++- app/models/concerns/spotlight/ar_light.rb | 8 ++++++ .../concerns/spotlight/solr_document.rb | 1 - .../spotlight/blacklight_configuration.rb | 4 ++- app/models/spotlight/filter.rb | 13 ++++++++- app/models/spotlight/resource.rb | 12 ++++++-- app/models/spotlight/search.rb | 4 +++ .../spotlight/attachment_uploader.rb | 4 +-- app/uploaders/spotlight/avatar_uploader.rb | 4 +++ .../spotlight/featured_image_uploader.rb | 4 +++ app/uploaders/spotlight/item_uploader.rb | 4 +++ app/uploaders/spotlight/masthead_uploader.rb | 4 +++ .../spotlight/pages/_view_type_group.html.erb | 2 +- blacklight-spotlight.gemspec | 7 +++-- lib/generators/spotlight/install_generator.rb | 4 +-- lib/spotlight/engine.rb | 10 ++++++- .../spotlight/appearances_controller_spec.rb | 28 +++++++++++++------ .../spotlight/solr_controller_spec.rb | 16 ++++++++--- spec/spec_helper.rb | 9 +++++- .../helpers/controller_level_helpers.rb | 12 ++++++++ spec/support/views/test_view_helpers.rb | 4 +++ spec/test_app_templates/Gemfile.extra | 2 ++ 27 files changed, 147 insertions(+), 31 deletions(-) create mode 100644 spec/test_app_templates/Gemfile.extra diff --git a/.rubocop.yml b/.rubocop.yml index ec2a1f44b..1c332a883 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -46,3 +46,7 @@ Metrics/ClassLength: Exclude: - 'app/models/spotlight/resource.rb' - 'lib/generators/spotlight/**/*' # Generators tend to have longer class lengths due to their lengthy public API + +Style/PredicateName: + Exclude: + - 'app/models/concerns/spotlight/ar_light.rb' \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 859423cbc..17b4e317d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,11 @@ rvm: - 2.3.1 - 2.2.5 +matrix: + include: + - rvm: 2.3.1 + env: "RAILS_VERSION=5.0.0" + notifications: irc: "irc.freenode.org#blacklight" diff --git a/Gemfile b/Gemfile index 037e17132..3f1f75778 100644 --- a/Gemfile +++ b/Gemfile @@ -47,3 +47,5 @@ else end end # END ENGINE_CART BLOCK + +eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__)) diff --git a/app/controllers/concerns/spotlight/controller.rb b/app/controllers/concerns/spotlight/controller.rb index 3faf7a014..0a4802014 100644 --- a/app/controllers/concerns/spotlight/controller.rb +++ b/app/controllers/concerns/spotlight/controller.rb @@ -82,8 +82,7 @@ def exhibit_search_action_url(*args) def exhibit_search_facet_url(*args) options = args.extract_options! - options = params.merge(options).except(:exhibit_id, :only_path) - + options = params.to_unsafe_h.merge(options).except(:exhibit_id, :only_path) spotlight.facet_exhibit_catalog_url(current_exhibit, *args, options) end end diff --git a/app/controllers/spotlight/catalog_controller.rb b/app/controllers/spotlight/catalog_controller.rb index 5eb608536..5d3297c14 100644 --- a/app/controllers/spotlight/catalog_controller.rb +++ b/app/controllers/spotlight/catalog_controller.rb @@ -191,7 +191,7 @@ def redirect_to_exhibit_home_without_search_params! end def add_breadcrumb_with_search_params - add_breadcrumb t(:'spotlight.catalog.breadcrumb.index'), request.fullpath if has_search_parameters? + add_breadcrumb t(:'spotlight.catalog.breadcrumb.index'), spotlight.search_exhibit_catalog_path(params.to_unsafe_h) if has_search_parameters? end # rubocop:disable Metrics/AbcSize diff --git a/app/controllers/spotlight/searches_controller.rb b/app/controllers/spotlight/searches_controller.rb index 4c9d355d5..c7c4b2c52 100644 --- a/app/controllers/spotlight/searches_controller.rb +++ b/app/controllers/spotlight/searches_controller.rb @@ -13,7 +13,7 @@ class SearchesController < Spotlight::ApplicationController def create @search.attributes = search_params - @search.query_params = params.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? } + @search.query_params = query_params if @search.save redirect_to :back, notice: t(:'helpers.submit.search.created', model: @search.class.model_name.human.downcase) @@ -103,6 +103,10 @@ def search_params ) end + def query_params + params.to_unsafe_h.with_indifferent_access.except(:exhibit_id, :search, *blacklisted_search_session_params).reject { |_k, v| v.blank? } + end + def featured_image_attributes [:display, :source, :image, :remote_image_url, :document_global_id, :image_crop_x, :image_crop_y, :image_crop_w, :image_crop_h] end diff --git a/app/models/concerns/spotlight/ar_light.rb b/app/models/concerns/spotlight/ar_light.rb index 3704e2b3a..a4bb565bc 100644 --- a/app/models/concerns/spotlight/ar_light.rb +++ b/app/models/concerns/spotlight/ar_light.rb @@ -13,6 +13,14 @@ module ArLight ## # Mock activerecord class-level methods module ClassMethods + def has_attribute?(*_args) + false + end + + def columns_hash + {} + end + def base_class self end diff --git a/app/models/concerns/spotlight/solr_document.rb b/app/models/concerns/spotlight/solr_document.rb index 10db714fa..612e9e41b 100644 --- a/app/models/concerns/spotlight/solr_document.rb +++ b/app/models/concerns/spotlight/solr_document.rb @@ -10,7 +10,6 @@ module SolrDocument include GlobalID::Identification included do - extend ActsAsTaggableOn::Compatibility extend ActsAsTaggableOn::Taggable acts_as_taggable diff --git a/app/models/spotlight/blacklight_configuration.rb b/app/models/spotlight/blacklight_configuration.rb index a509385e8..ff61c7cde 100644 --- a/app/models/spotlight/blacklight_configuration.rb +++ b/app/models/spotlight/blacklight_configuration.rb @@ -336,7 +336,9 @@ def field_weight(fields, index) end def value_to_boolean(v) - if defined? ActiveRecord::Type + if defined? ActiveModel::Type::Boolean + ActiveModel::Type::Boolean.new.cast v + elsif defined? ActiveRecord::Type::Boolean # Rails 4.2+ ActiveRecord::Type::Boolean.new.type_cast_from_database v else diff --git a/app/models/spotlight/filter.rb b/app/models/spotlight/filter.rb index fd8a807cf..f917a4463 100644 --- a/app/models/spotlight/filter.rb +++ b/app/models/spotlight/filter.rb @@ -17,10 +17,21 @@ def cast_value return value unless field if field.ends_with? Spotlight::Engine.config.solr_fields.boolean_suffix - ActiveRecord::Type::Boolean.new.type_cast_from_database(value) + value_to_boolean(value) else value end end + + def value_to_boolean(v) + if defined? ActiveModel::Type::Boolean + ActiveModel::Type::Boolean.new.cast v + elsif defined? ActiveRecord::Type::Boolean + # Rails 4.2+ + ActiveRecord::Type::Boolean.new.type_cast_from_database v + else + ActiveRecord::ConnectionAdapters::Column.value_to_boolean v + end + end end end diff --git a/app/models/spotlight/resource.rb b/app/models/spotlight/resource.rb index 7be6b5635..aa0d27f63 100644 --- a/app/models/spotlight/resource.rb +++ b/app/models/spotlight/resource.rb @@ -49,11 +49,19 @@ def waiting! end def enqueued_at - ActiveRecord::Type::DateTime.new.type_cast_from_database(super) + if defined? ActiveModel::Type::DateTime + ActiveModel::Type::DateTime.new.cast(super) + else + ActiveRecord::Type::DateTime.new.type_cast_from_database(super) + end end def last_indexed_finished - ActiveRecord::Type::DateTime.new.type_cast_from_database(super) + if defined? ActiveModel::Type::DateTime + ActiveModel::Type::DateTime.new.cast(super) + else + ActiveRecord::Type::DateTime.new.type_cast_from_database(super) + end end def document_model diff --git a/app/models/spotlight/search.rb b/app/models/spotlight/search.rb index 5a88e713b..e80bdf920 100644 --- a/app/models/spotlight/search.rb +++ b/app/models/spotlight/search.rb @@ -48,6 +48,10 @@ def documents end end + def count + documents.size + end + delegate :blacklight_config, to: :exhibit def display_masthead? diff --git a/app/uploaders/spotlight/attachment_uploader.rb b/app/uploaders/spotlight/attachment_uploader.rb index 6386cf6bb..bad5c7078 100644 --- a/app/uploaders/spotlight/attachment_uploader.rb +++ b/app/uploaders/spotlight/attachment_uploader.rb @@ -30,9 +30,9 @@ def store_dir # Provide a default URL as a default if there hasn't been a file uploaded: # def default_url # # For Rails 3.1+ asset pipeline compatibility: - # # ActionController::Base.helpers.asset_path("fallback/" + [version_name, "default.png"].compact.join('_')) + # # ActionController::Base.helpers.asset_path('fallback/' + [version_name, 'default.png'].compact.join('_')) # - # "/images/fallback/" + [version_name, "default.png"].compact.join('_') + # "/images/fallback/" + [version_name, 'default.png'].compact.join('_') # end # Process files as they are uploaded: diff --git a/app/uploaders/spotlight/avatar_uploader.rb b/app/uploaders/spotlight/avatar_uploader.rb index 4639fc1d6..5e99dde28 100644 --- a/app/uploaders/spotlight/avatar_uploader.rb +++ b/app/uploaders/spotlight/avatar_uploader.rb @@ -16,5 +16,9 @@ class AvatarUploader < CarrierWave::Uploader::Base def store_dir "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end + + def default_url + ActionController::Base.helpers.asset_path('fallback/' + [version_name, 'default.png'].compact.join('_')) + end end end diff --git a/app/uploaders/spotlight/featured_image_uploader.rb b/app/uploaders/spotlight/featured_image_uploader.rb index 346e08ee7..92dde277f 100644 --- a/app/uploaders/spotlight/featured_image_uploader.rb +++ b/app/uploaders/spotlight/featured_image_uploader.rb @@ -21,5 +21,9 @@ class FeaturedImageUploader < CarrierWave::Uploader::Base def store_dir "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end + + def default_url + ActionController::Base.helpers.asset_path('fallback/' + [version_name, 'default.png'].compact.join('_')) + end end end diff --git a/app/uploaders/spotlight/item_uploader.rb b/app/uploaders/spotlight/item_uploader.rb index ea233a6fd..11325ee83 100644 --- a/app/uploaders/spotlight/item_uploader.rb +++ b/app/uploaders/spotlight/item_uploader.rb @@ -17,5 +17,9 @@ def extension_white_list def store_dir "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end + + def default_url + ActionController::Base.helpers.asset_path('fallback/' + [version_name, 'default.png'].compact.join('_')) + end end end diff --git a/app/uploaders/spotlight/masthead_uploader.rb b/app/uploaders/spotlight/masthead_uploader.rb index bff49a5aa..3ea2bfd91 100644 --- a/app/uploaders/spotlight/masthead_uploader.rb +++ b/app/uploaders/spotlight/masthead_uploader.rb @@ -14,5 +14,9 @@ class MastheadUploader < CarrierWave::Uploader::Base def store_dir "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" end + + def default_url + ActionController::Base.helpers.asset_path('fallback/' + [version_name, 'default.png'].compact.join('_')) + end end end diff --git a/app/views/spotlight/pages/_view_type_group.html.erb b/app/views/spotlight/pages/_view_type_group.html.erb index 0a1e69c9f..96f607ef7 100644 --- a/app/views/spotlight/pages/_view_type_group.html.erb +++ b/app/views/spotlight/pages/_view_type_group.html.erb @@ -4,7 +4,7 @@ <%= t('blacklight.search.view_title') %>
<% views.each do |view, config| %> - <%= link_to url_for(params.merge(:view => view)), :title => t("blacklight.search.view_title.#{view}", default: t("blacklight.search.view.#{view}", default: blacklight_config.view[view].title)), :class => "btn btn-default view-type-#{ view.to_s.parameterize } #{"active" if block_document_index_view_type(block) == view}" do %> + <%= link_to url_for(search_state.to_h.merge(view: view)), :title => t("blacklight.search.view_title.#{view}", default: t("blacklight.search.view.#{view}", default: blacklight_config.view[view].title)), :class => "btn btn-default view-type-#{ view.to_s.parameterize } #{"active" if block_document_index_view_type(block) == view}" do %> <%= render_view_type_group_icon view %> <%= t("blacklight.search.view.#{view}") %> <% end %> diff --git a/blacklight-spotlight.gemspec b/blacklight-spotlight.gemspec index 267664d87..163383ff6 100644 --- a/blacklight-spotlight.gemspec +++ b/blacklight-spotlight.gemspec @@ -19,7 +19,7 @@ these collections.) s.required_ruby_version = '~> 2.2' - s.add_dependency 'rails', '~> 4.0', '>= 4.2.0' + s.add_dependency 'rails', '>= 4.2.0', '< 6' s.add_dependency 'blacklight', '~> 6.3' s.add_dependency 'autoprefixer-rails' s.add_dependency 'cancancan' @@ -28,8 +28,8 @@ these collections.) s.add_dependency 'carrierwave-crop' s.add_dependency 'mini_magick' s.add_dependency 'bootstrap_form', '~> 2.2' - s.add_dependency 'acts-as-taggable-on', '~> 3.5' - s.add_dependency 'friendly_id', '~> 5.1.0' + s.add_dependency 'acts-as-taggable-on', '>= 4.0.0.pre' + s.add_dependency 'friendly_id', '>= 5.2.0.beta.1', '< 6' s.add_dependency 'breadcrumbs_on_rails', '~> 2.3.0' s.add_dependency 'social-share-button', '~> 0.3' s.add_dependency 'blacklight-gallery', '>= 0.3.0' @@ -55,6 +55,7 @@ these collections.) s.add_development_dependency 'rspec-its' s.add_development_dependency 'rspec-activemodel-mocks' s.add_development_dependency 'rspec-collection_matchers' + s.add_development_dependency 'rails-controller-testing' s.add_development_dependency 'capybara', '>= 2.5.0' s.add_development_dependency 'rubocop', '~> 0.41', '>= 0.41.2' s.add_development_dependency 'rubocop-rspec' diff --git a/lib/generators/spotlight/install_generator.rb b/lib/generators/spotlight/install_generator.rb index 82a05a393..a0e52fb21 100644 --- a/lib/generators/spotlight/install_generator.rb +++ b/lib/generators/spotlight/install_generator.rb @@ -17,7 +17,7 @@ def inject_spotlight_routes end def friendly_id - gem 'friendly_id' + gem 'friendly_id', github: 'norman/friendly_id' generate 'friendly_id' end @@ -119,7 +119,7 @@ def add_mailer_defaults end def generate_social_share_button_initializer - gem 'social-share-button' + gem 'social-share-button', github: 'cbeer/social-share-button', branch: 'on_load' directory 'config' end diff --git a/lib/spotlight/engine.rb b/lib/spotlight/engine.rb index 8d2e4a4d9..8ec5ff9d7 100644 --- a/lib/spotlight/engine.rb +++ b/lib/spotlight/engine.rb @@ -17,7 +17,15 @@ module Spotlight class Engine < ::Rails::Engine isolate_namespace Spotlight # Breadcrumbs on rails must be required outside of an initializer or it doesn't get loaded. - require 'breadcrumbs_on_rails' + require 'breadcrumbs_on_rails/breadcrumbs' + require 'breadcrumbs_on_rails/action_controller' + + initializer 'breadcrumbs_on_rails.initialize' do + ActiveSupport.on_load(:action_controller) do + include BreadcrumbsOnRails::ActionController + end + end + require 'carrierwave' require 'carrierwave/crop' require 'social-share-button' diff --git a/spec/controllers/spotlight/appearances_controller_spec.rb b/spec/controllers/spotlight/appearances_controller_spec.rb index 4a391ca5f..c85d1399b 100644 --- a/spec/controllers/spotlight/appearances_controller_spec.rb +++ b/spec/controllers/spotlight/appearances_controller_spec.rb @@ -41,15 +41,27 @@ end describe 'PATCH update' do + let(:first_nav) { exhibit.main_navigations.first } + let(:last_nav) { exhibit.main_navigations.last } it 'updates the navigation' do - first_nav = exhibit.main_navigations.first - last_nav = exhibit.main_navigations.last - patch :update, exhibit_id: exhibit, exhibit: { - main_navigations_attributes: [ - { id: first_nav.id, label: 'Some Label', weight: 500 }, - { id: last_nav.id, display: false } - ] - } + if Rails::VERSION::MAJOR >= 5 + patch :update, params: { + exhibit_id: exhibit, + exhibit: { + main_navigations_attributes: { + 0 => { id: first_nav.id, label: 'Some Label', weight: 500 }, + 1 => { id: last_nav.id, display: false } + } + } + } + else + patch :update, exhibit_id: exhibit, exhibit: { + main_navigations_attributes: [ + { id: first_nav.id, label: 'Some Label', weight: 500 }, + { id: last_nav.id, display: false } + ] + } + end expect(flash[:notice]).to eq 'The exhibit was successfully updated.' expect(response).to redirect_to edit_exhibit_appearance_path(exhibit) assigns[:exhibit].tap do |saved| diff --git a/spec/controllers/spotlight/solr_controller_spec.rb b/spec/controllers/spotlight/solr_controller_spec.rb index 4a972697a..a7942bea5 100644 --- a/spec/controllers/spotlight/solr_controller_spec.rb +++ b/spec/controllers/spotlight/solr_controller_spec.rb @@ -29,7 +29,7 @@ doc = arr.first end - post :update, { a: 1 }.to_json, content_type: :json, exhibit_id: exhibit + post_update_with_json_body(exhibit, a: 1) expect(response).to be_successful expect(doc).to include 'a' => 1 @@ -41,7 +41,7 @@ end it 'raises an error' do - post :update, { a: 1 }.to_json, content_type: :json, exhibit_id: exhibit + post_update_with_json_body(exhibit, a: 1) expect(response.code).to eq '409' end @@ -53,7 +53,7 @@ doc = arr.first end - post :update, { a: 1 }.to_json, content_type: :json, exhibit_id: exhibit + post_update_with_json_body(exhibit, a: 1) expect(response).to be_successful expect(doc).to include exhibit.solr_data @@ -67,11 +67,19 @@ allow_any_instance_of(SolrDocument).to receive(:to_solr).and_return(b: 1) - post :update, { a: 1 }.to_json, content_type: :json, exhibit_id: exhibit + post_update_with_json_body(exhibit, a: 1) expect(response).to be_successful expect(doc).to include b: 1 end end end + + def post_update_with_json_body(exhibit, hash) + if Rails::VERSION::MAJOR >= 5 + post :update, body: hash.to_json, params: { exhibit_id: exhibit }, as: :json + else + post :update, hash.to_json, content_type: :json, exhibit_id: exhibit + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 53ba887e6..50766157c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,11 +1,13 @@ ENV['RAILS_ENV'] ||= 'test' - require 'factory_girl' require 'database_cleaner' require 'devise' require 'engine_cart' EngineCart.load_application! +Internal::Application.config.active_job.queue_adapter = :inline + +require 'rails-controller-testing' if Rails::VERSION::MAJOR >= 5 require 'rspec/collection_matchers' require 'rspec/its' require 'rspec/rails' @@ -47,6 +49,7 @@ RSpec.configure do |config| config.infer_spec_type_from_file_location! + config.filter_rails_from_backtrace! config.use_transactional_fixtures = false @@ -88,6 +91,10 @@ config.after(:each, type: :feature) { Warden.test_reset! } config.include Controllers::EngineHelpers, type: :controller config.include Capybara::DSL + if Rails::VERSION::MAJOR >= 5 + config.include ::Rails.application.routes.url_helpers + config.include ::Rails.application.routes.mounted_helpers + end config.include Spotlight::TestFeaturesHelpers, type: :feature end diff --git a/spec/support/helpers/controller_level_helpers.rb b/spec/support/helpers/controller_level_helpers.rb index e59d027d3..71e0730b8 100644 --- a/spec/support/helpers/controller_level_helpers.rb +++ b/spec/support/helpers/controller_level_helpers.rb @@ -3,11 +3,23 @@ def search_state @search_state ||= Blacklight::SearchState.new(params, blacklight_config) end + def current_site + Spotlight::Site.instance + end + def blacklight_configuration_context @blacklight_configuration_context ||= Blacklight::Configuration::Context.new(controller) end def initialize_controller_helpers(helper) helper.extend ControllerLevelHelpers + initialize_routing_helpers(helper) + end + + def initialize_routing_helpers(helper) + return unless Rails::VERSION::MAJOR >= 5 + + helper.class.include ::Rails.application.routes.url_helpers + helper.class.include ::Rails.application.routes.mounted_helpers if ::Rails.application.routes.respond_to?(:mounted_helpers) end end diff --git a/spec/support/views/test_view_helpers.rb b/spec/support/views/test_view_helpers.rb index e07d29704..4d2e3d235 100644 --- a/spec/support/views/test_view_helpers.rb +++ b/spec/support/views/test_view_helpers.rb @@ -4,8 +4,12 @@ module TestViewHelpers included do before do + view.send(:extend, Spotlight::MainAppHelpers) view.send(:extend, Spotlight::CrudLinkHelpers) view.send(:extend, Spotlight::TitleHelper) + view.send(:extend, Spotlight::NavbarHelper) + view.send(:extend, Blacklight::ComponentHelperBehavior) + view.send(:extend, BreadcrumbsOnRails::ActionController::HelperMethods) end end end diff --git a/spec/test_app_templates/Gemfile.extra b/spec/test_app_templates/Gemfile.extra new file mode 100644 index 000000000..e84bc55af --- /dev/null +++ b/spec/test_app_templates/Gemfile.extra @@ -0,0 +1,2 @@ +gem 'friendly_id', github: 'norman/friendly_id' +gem 'social-share-button', github: 'cbeer/social-share-button', branch: 'on_load'