Add support for Ruby 3.x -> 4.0, Rails 6.1 -> 8.1

.github/workflows/main.yml

@@ -12,7 +12,7 @@ jobs:
       matrix:
         database: [ mysql, postgresql ]
         extension: [ core, dragonfly, images, pages, resources ]
-        ruby: [ 2.7, 2.6 ]
+        ruby: [ 3.1]
       fail-fast: false
       max-parallel: 20
     runs-on: ubuntu-latest

.gitignore

@@ -97,3 +97,4 @@ Gemfile.lock
 
 # rspec failures
 .rspec_failures
+Refinery Review.md

Gemfile

@@ -1,20 +1,26 @@
 source 'https://rubygems.org'
 
+ruby "3.1.2"
 gemspec
 
+gem 'rails', "~>6.1"
+gem 'net-smtp', require: false
+gem 'net-imap', require: false
+gem 'net-pop', require: false
+
+
 path "./" do
   gem "refinerycms-core"
+  gem "refinerycms-dragonfly"
   gem "refinerycms-images"
   gem "refinerycms-pages"
   gem "refinerycms-resources"
 end
 
-gem 'refinerycms-i18n', github: 'refinery/refinerycms-i18n', branch: 'master'
+gem 'refinerycms-i18n', git: 'https://github.com/refinery/refinerycms-i18n', branch: 'ruby3'
 
 # Add support for refinerycms-acts-as-indexed
-gem 'refinerycms-acts-as-indexed', ['~> 4.0', '>= 4.0.0'],
-  git: 'https://github.com/refinery/refinerycms-acts-as-indexed',
-  branch: 'master'
+gem 'refinerycms-acts-as-indexed', ['~> 4.0', '>= 4.0.0'], git: 'https://github.com/refinery/refinerycms-acts-as-indexed', branch: 'master'
 
 # Add the default visual editor, for now.
 gem 'refinerycms-wymeditor', ['~> 3.0', '>= 3.0.0']
@@ -43,6 +49,7 @@ group :development, :test do
   gem 'activejob'
   gem 'bootsnap', require: false
   gem 'listen', '~> 3.0'
+  gem 'rspec-rails', '~> 6.0.0.rc1'
 end
 
 group :test do
@@ -53,9 +60,6 @@ group :test do
   gem 'rspec-retry'
   gem 'falcon'
   gem 'falcon-capybara'
-
-  # TODO: Use beta source for Rails 6 support
-  gem 'rspec-rails', '~> 4.0.0.beta3'
 end
 
 # Load local gems according to Refinery developer preference.

core/app/helpers/refinery/site_bar_helper.rb

@@ -3,24 +3,24 @@ module SiteBarHelper
 
     # Generates the link to determine where the site bar switch button returns to.
     def site_bar_switch_link
-      link_to_if(admin?, t('.switch_to_your_website', site_bar_translate_locale_args),
+      link_to_if(admin?, t('.switch_to_your_website', **site_bar_translate_locale_args),
                          refinery.root_path(site_bar_translate_locale_args),
                          'data-turbolinks' => false) do
-        link_to t('.switch_to_your_website_editor', site_bar_translate_locale_args),
+        link_to t('.switch_to_your_website_editor', **site_bar_translate_locale_args),
                 Refinery::Core.backend_path, 'data-turbolinks' => false
       end
     end
 
     def site_bar_edit_link
       return nil if admin? || @page.nil?
-      link_to t('refinery.admin.pages.edit', site_bar_translate_locale_args),
+      link_to t('refinery.admin.pages.edit', **site_bar_translate_locale_args),
               refinery.admin_edit_page_path(@page.nested_url,
               :switch_locale => (@page.translations.first.locale unless @page.translated_to_default_locale?)),
               'data-turbolinks' => false
     end
 
     def site_bar_translate_locale_args
-      { :locale => Refinery::I18n.current_locale }
+      { locale: Refinery::I18n.current_locale }
     end
 
     def display_site_bar?

core/app/helpers/refinery/translation_helper.rb

@@ -3,7 +3,7 @@ module TranslationHelper
 
     # Overrides Rails' core I18n.t() function to produce a more helpful error message.
     # The default one wreaks havoc with CSS and makes it hard to understand the problem.
-    def t(key, options = {})
+    def t(key, **options)
       if (val = super) =~ /class.+?translation_missing/
         val = val.to_s.gsub(/<span[^>]*>/, 'i18n: ').gsub('</span>', '').gsub(', ', '.')
       end

core/app/views/refinery/_site_bar.html.erb

@@ -17,12 +17,13 @@
 
       <div id='site_bar_branding'>
         <span id='site_bar_company_name'>
-          <%= Refinery::Core.site_name %>
+          <% Rails.logger.debug Refinery::Core.site_name %>
+          <%#= Refinery::Core.site_name %>
         </span>
 
-        <%= link_to t('.log_out', site_bar_translate_locale_args),
+        <%= link_to t('.log_out', **site_bar_translate_locale_args),
                     ::Refinery::Core.refinery_logout_path,
-                    :id => 'logout' if ::Refinery::Core.refinery_logout_path.present? %>
+                    id: 'logout' if ::Refinery::Core.refinery_logout_path.present? %>
       </div>
     </div>
   </div>

core/app/views/refinery/admin/_error_messages.html.erb

@@ -3,12 +3,12 @@
   <p><%= t('.problems_in_following_fields') %>:</p>
   <ul>
   <% if defined?(include_object_name) and include_object_name %>
-    <% object.errors.full_messages.each do |value| %>
-      <li><%= value %></li>
+    <% object.errors.each do |error| %>
+      <li><%= error.message %></li>
     <% end %>
   <% else %>
-    <% object.errors.each do |key, value| %>
-      <li><%= value %></li>
+    <% object.errors.each do |error| %>
+      <li><%= error.message %></li>
     <% end %>
   <% end %>
   </ul>

core/lib/refinery/core/engine.rb

@@ -45,11 +45,22 @@ def refinery_inclusion!
       end
 
       initializer "refinery.mobility" do
-        Mobility.configure do |config|
-          config.default_backend = :table
-          config.accessor_method = :translates
-          config.query_method    = :i18n
-          config.default_options[:dirty] = true
+        Mobility.configure do
+          plugins do
+            backend :table
+            reader                             # Explicitly declare readers,
+            writer                             # writers, and
+            backend_reader                     # backend reader (post.title_backend, etc).
+            active_record                      # You must now also explicitly ask for ActiveRecord (or Sequel)
+            query                              # i18n is the default scope
+            cache                              # previously implicit
+            fallbacks
+            presence                           # previously implicit
+            default
+            attribute_methods                  # uncomment this to get methods like `translated_attributes`
+            dirty
+          end
+          # accessor_method not available in v1.0
         end
       end
 

core/lib/refinery/version.rb

@@ -13,7 +13,7 @@ def to_s
       end
 
       def required_ruby_version
-        '>= 2.5.5'
+        '>= 3.1'
       end
     end
   end

core/spec/helpers/refinery/translation_helper_spec.rb

@@ -15,8 +15,9 @@ module Refinery
 
       before do
         Mobility.with_locale(:en) do
-          page.title = "draft"
-          page.save!
+          # page.title = "draft"
+          # page.save!
+          page.update!({title: 'draft'})
         end
 
         Mobility.with_locale(:lv) do

core/spec/presenters/refinery/translated_field_presenter_spec.rb

@@ -6,13 +6,11 @@ module Refinery
 
     before do
       Mobility.with_locale(:en) do
-        page.title = "draft"
-        page.save!
+        page.update!( title: "draft")
       end
 
       Mobility.with_locale(:lv) do
-        page.title = "melnraksts"
-        page.save!
+        page.update!(title: "melnraksts")
       end
     end
 

core/spec/system/refinery/admin/xhr_paging_spec.rb

@@ -8,8 +8,8 @@ module Refinery
       # Refinery::Admin::ImagesController specifies :order => 'created_at DESC' in crudify
       let(:first_image) { Image.order('created_at DESC').first }
       let(:last_image) { Image.order('created_at DESC').last }
-      let!(:image_1) { FactoryBot.create :image }
-      let!(:image_2) { FactoryBot.create :image }
+      let!(:image_1) { FactoryBot.create(:image) }
+      let!(:image_2) { FactoryBot.create(:image) }
 
       before do
         allow(Image).to receive(:per_page).and_return(1)

core/spec/system/refinery/site_bar_spec.rb

@@ -10,10 +10,10 @@ module Refinery
       context "when set" do
         before do
           allow(Refinery::Core).to receive(:refinery_logout_path).and_return(logout_path)
-          visit Refinery::Core.backend_path
         end
 
         it "is present" do
+          visit Refinery::Core.backend_path
           expect(page).to have_selector("a[href='#{logout_path}']")
           expect(page).to have_content("Log out")
         end

images/lib/refinery/images/validators/image_size_validator.rb

@@ -2,17 +2,16 @@ module Refinery
   module Images
     module Validators
       class ImageSizeValidator < ActiveModel::Validator
-
         def validate(record)
-          image = record.image
-
-          if image.respond_to?(:length) && image.length > Images.max_image_size
-            record.errors[:image] << ::I18n.t('too_big',
-                                             :scope => 'activerecord.errors.models.refinery/image',
-                                             :size => Images.max_image_size)
-          end
+          record.errors.add(:image, ::I18n.t('too_big',
+                                             scope: 'activerecord.errors.models.refinery/image',
+                                             size: Images.max_image_size)) if too_big(record.image)
         end
 
+        private
+        def too_big(image)
+          image.respond_to?(:length) && image.length > Images.max_image_size
+        end
       end
     end
   end

images/lib/refinery/images/validators/image_update_validator.rb

@@ -7,7 +7,7 @@ def validate(record)
           if record.image_name_changed?
             record.errors.add :image_name,
               ::I18n.t("different_file_name",
-                       :scope => "activerecord.errors.models.refinery/image")
+                       scope: "activerecord.errors.models.refinery/image")
           end
         end
 

images/spec/support/shared_examples/image_deleter.rb

@@ -1,27 +1,26 @@
-shared_examples_for 'deletes an image' do
+shared_examples_for 'Delete' do
   before do
     raise "please set let(:initial_path)" if initial_path.blank?
     ensure_on(initial_path)
   end
 
-  let(:image_count) {[Refinery::Image.count, Refinery::Images.pages_per_admin_index].min}
-  let(:deleting_an_image) {
-    -> {
-      first("#records li").click_link(::I18n.t('delete', scope: 'refinery.admin.images'))
-    }
-  }
+  def deleting_an_image
+    first("#records li").click_link(::I18n.t('delete', scope: 'refinery.admin.images'))
+  end
+
+  let(:image_count) { [Refinery::Image.count, Refinery::Images.pages_per_admin_index].min }
 
   it 'has a delete image link for each image' do
     expect(page).to have_selector('#records.images li a.confirm-delete', count: image_count)
   end
 
   it "removes an image" do
-    expect(deleting_an_image).to change(Refinery::Image, :count).by(-1)
+    expect { deleting_an_image }.to change(Refinery::Image, :count).by(-1)
   end
 
   it 'says the image has been removed' do
     image_title = page.has_selector?('#image_grid') ? find("#image_grid li:first").first("img")[:title] :
-                                                      find("#image_list li:first").first("span.title").text
+      find("#image_list li:first").first("span.title").text
 
     expect(image_title).to be_present
 

images/spec/support/shared_examples/image_editor.rb

@@ -1,3 +1,3 @@
-shared_examples 'edits an image' do
+shared_examples 'Edit' do
   pending
-end
+end

images/spec/support/shared_examples/image_indexer.rb

@@ -1,4 +1,4 @@
-shared_examples_for 'indexes images' do
+shared_examples_for 'Index' do
 
   let(:image_count) {[Refinery::Image.count, Refinery::Images.pages_per_admin_index].min}
 
@@ -20,7 +20,7 @@
 
 end # image index
 
-shared_examples_for 'shows list and grid views' do
+shared_examples_for 'Index Views' do
 
   let(:image_count) {[Refinery::Image.count, Refinery::Images.pages_per_admin_index].min}
 
@@ -70,11 +70,10 @@
 
       expect(page).to have_content(::I18n.t('switch_to', view_name: 'grid', scope: 'refinery.admin.images.index.view'))
     end
-
   end # list view
 end
 
-shared_examples_for 'paginates the list of images' do
+shared_examples_for 'Index Pagination' do
 
   let(:image_count) {[Refinery::Image.count, Refinery::Images.pages_per_admin_index].min}
 

images/spec/support/shared_examples/image_inserter.rb

@@ -1,5 +1,5 @@
 
-shared_examples 'inserts images' do
+shared_examples 'Insert' do
   before do
     raise "please set let(:initial_path)" if initial_path.blank?
     ensure_on(initial_path)

images/spec/support/shared_examples/image_previewer.rb

@@ -9,7 +9,7 @@ def preview_image
   preview_window.close
 end
 
-shared_examples 'shows an image preview' do
+shared_examples 'Preview' do
   before do
     raise "please set let(:initial_path)" if initial_path.blank?
     ensure_on(initial_path)

images/spec/support/shared_examples/image_translator.rb

@@ -1,4 +1,4 @@
-shared_examples 'translates an image' do
+shared_examples 'Translate' do
   before do
     allow(Refinery::I18n).to receive(:frontend_locales).and_return([:en, :fr])
     ensure_on(initial_path)

images/spec/support/shared_examples/image_uploader.rb

@@ -1,34 +1,32 @@
-shared_examples 'uploads images' do
+shared_examples 'Upload' do
   before do
     raise "please set let(:initial_path)" if initial_path.blank?
     ensure_on(initial_path)
     initialize_context
   end
 
-  let(:uploading_an_image) {
-  ->{
+  def uploading_an_image
     open_upload_dialog
     page.within_frame(dialog_frame_id) do
       select_upload
       attach_file 'image_image', image_path
-      fill_in  'image_image_title', with: 'Image With Dashes'
-      fill_in  'image_image_alt', with: "Alt description for image"
+      fill_in 'image_image_title', with: 'Image With Dashes'
+      fill_in 'image_image_alt', with: "Alt description for image"
       click_button ::I18n.t('save', scope: 'refinery.admin.form_actions')
     end
-    }
-  }
+  end
 
   context 'when the image type is acceptable' do
-    let(:image_path) {Refinery.roots('refinery/images').join("spec/fixtures/image-with-dashes.jpg")}
+    let(:image_path) { Refinery.roots('refinery/images').join("spec/fixtures/image-with-dashes.jpg") }
     it 'the image is uploaded', :js => true do
-      expect(uploading_an_image).to change(Refinery::Image, :count).by(1)
+      expect { uploading_an_image }.to change(Refinery::Image, :count).by(1)
     end
   end
 
   context 'when the image type is not acceptable' do
-    let(:image_path) {Refinery.roots('refinery/images').join("spec/fixtures/cape-town-tide-table.pdf")}
+    let(:image_path) { Refinery.roots('refinery/images').join("spec/fixtures/cape-town-tide-table.pdf") }
     it 'the image is rejected', :js => true do
-      expect(uploading_an_image).to_not change(Refinery::Image, :count)
+      expect { uploading_an_image }.to_not change(Refinery::Image, :count)
       page.within_frame(dialog_frame_id) do
         expect(page).to have_content(::I18n.t('incorrect_format',
                                               :scope => 'activerecord.errors.models.refinery/image',