Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruby3 #3518

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Ruby3 #3518

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,4 @@ Gemfile.lock

# rspec failures
.rspec_failures
Refinery Review.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Refinery Review.md

18 changes: 11 additions & 7 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -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']
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions core/app/helpers/refinery/site_bar_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion core/app/helpers/refinery/translation_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions core/app/views/refinery/_site_bar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions core/app/views/refinery/admin/_error_messages.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
21 changes: 16 additions & 5 deletions core/lib/refinery/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion core/lib/refinery/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def to_s
end

def required_ruby_version
'>= 2.5.5'
'>= 3.1'
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions core/spec/helpers/refinery/translation_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions core/spec/system/refinery/admin/xhr_paging_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion core/spec/system/refinery/site_bar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions images/lib/refinery/images/validators/image_size_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 8 additions & 9 deletions images/spec/support/shared_examples/image_deleter.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
4 changes: 2 additions & 2 deletions images/spec/support/shared_examples/image_editor.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
shared_examples 'edits an image' do
shared_examples 'Edit' do
pending
end
end
7 changes: 3 additions & 4 deletions images/spec/support/shared_examples/image_indexer.rb
Original file line number Diff line number Diff line change
@@ -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}

Expand All @@ -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}

Expand Down Expand Up @@ -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}

Expand Down
2 changes: 1 addition & 1 deletion images/spec/support/shared_examples/image_inserter.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 1 addition & 1 deletion images/spec/support/shared_examples/image_previewer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion images/spec/support/shared_examples/image_translator.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
20 changes: 9 additions & 11 deletions images/spec/support/shared_examples/image_uploader.rb
Original file line number Diff line number Diff line change
@@ -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',
Expand Down
Loading