Skip to content

Commit

Permalink
Rails 6.2.0.rc2 compatibility
Browse files Browse the repository at this point in the history
Rails 6.2.0.rc2 changed join order when combining Arel and Rails joins.
Worked around by using only Arel joins in the affected query.
See rails/rails#36761

Fixes #822
  • Loading branch information
glebm committed Jul 26, 2019
1 parent c691be6 commit 67630c1
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 19 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -2,7 +2,7 @@

source 'https://rubygems.org'

gem 'rails', '~> 6.0.0.rc1'
gem 'rails', '~> 6.0.0.rc2'
gem 'rails-i18n', '~> 6.0.0.beta1'

# https://github.com/rails/rails/blob/v6.0.0.rc1/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L12
Expand Down
16 changes: 13 additions & 3 deletions app/models/concerns/thredded/topic_common.rb
Expand Up @@ -53,9 +53,19 @@ def unread(user)
topics = arel_table
reads_class = reflect_on_association(:user_read_states).klass
reads = reads_class.arel_table
joins(topics.join(reads, Arel::Nodes::OuterJoin)
.on(topics[:id].eq(reads[:postable_id]).and(reads[:user_id].eq(user.id))).join_sources)
.merge(reads_class.where(reads[:id].eq(nil).or(reads[:unread_posts_count].not_eq(0))))

joins_reads =
topics.outer_join(reads)
.on(topics[:id].eq(reads[:postable_id]).and(reads[:user_id].eq(user.id))).join_sources

unread_scope = reads_class.where(reads[:id].eq(nil).or(reads[:unread_posts_count].not_eq(0)))

# Work around https://github.com/rails/rails/issues/36761
if Thredded.rails_gte_600_rc_2?
merge(unread_scope).joins(joins_reads)
else
joins(joins_reads).merge(unread_scope)
end
end

private
Expand Down
32 changes: 23 additions & 9 deletions app/models/thredded/messageboard.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Thredded
class Messageboard < ActiveRecord::Base
class Messageboard < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
extend FriendlyId
friendly_id :slug_candidates,
use: %i[slugged reserved],
Expand Down Expand Up @@ -128,14 +128,28 @@ def unread_topics_counts(user:, topics_scope: Thredded::Topic.all)
messageboards = arel_table
read_states = Thredded::UserTopicReadState.arel_table
topics = topics_scope.arel_table
joins(:topics).merge(topics_scope).joins(
messageboards.outer_join(read_states).on(
messageboards[:id].eq(read_states[:messageboard_id])
.and(read_states[:postable_id].eq(topics[:id]))
.and(read_states[:user_id].eq(user.id))
.and(read_states[:unread_posts_count].eq(0))
).join_sources
).group(messageboards[:id]).pluck(

read_states_join_cond =
messageboards[:id].eq(read_states[:messageboard_id])
.and(read_states[:postable_id].eq(topics[:id]))
.and(read_states[:user_id].eq(user.id))
.and(read_states[:unread_posts_count].eq(0))

scope =
# Work around https://github.com/rails/rails/issues/36761
if Thredded.rails_gte_600_rc_2?
merge(topics_scope).joins(
messageboards.join(topics)
.on(topics[:messageboard_id].eq(messageboards[:id]))
.outer_join(read_states).on(read_states_join_cond).join_sources
)
else
joins(:topics).merge(topics_scope).joins(
messageboards.outer_join(read_states).on(read_states_join_cond).join_sources
)
end

scope.group(messageboards[:id]).pluck(
:id,
Arel::Nodes::Subtraction.new(topics[:id].count, read_states[:id].count)
).to_h
Expand Down
7 changes: 7 additions & 0 deletions lib/thredded.rb
Expand Up @@ -251,6 +251,13 @@ def rails_gte_51?
@rails_gte_51
end

# @api private
# Mainly to work around https://github.com/rails/rails/issues/36761
def rails_gte_600_rc_2?
@rails_gte_600_rc_2 = (Rails.gem_version >= Gem::Version.new('6.0.0.rc2')) if @rails_gte_600_rc_2.nil?
@rails_gte_600_rc_2
end

# @api private
def rails_supports_csp_nonce?
@rails_supports_csp_nonce = (Rails.gem_version >= Gem::Version.new('5.2.0')) if @rails_supports_csp_nonce.nil?
Expand Down
11 changes: 6 additions & 5 deletions lib/thredded/db_tools.rb
Expand Up @@ -9,13 +9,14 @@ class << self
def migrate(paths:, quiet:, &filter)
verbose_was = ActiveRecord::Migration.verbose
ActiveRecord::Migration.verbose = !quiet
migrate = -> {
if Rails::VERSION::STRING >= '5.2'
ActiveRecord::MigrationContext.new(paths).migrate(nil, &filter)
migrate =
if Rails.gem_version >= Gem::Version.new('6.0.0.rc2')
-> { ActiveRecord::MigrationContext.new(paths, ActiveRecord::SchemaMigration).migrate(nil, &filter) }
elsif Rails::VERSION::STRING >= '5.2'
-> { ActiveRecord::MigrationContext.new(paths).migrate(nil, &filter) }
else
ActiveRecord::Migrator.migrate(paths, &filter)
-> { ActiveRecord::Migrator.migrate(paths, &filter) }
end
}
if quiet
silence_active_record(&migrate)
else
Expand Down
2 changes: 1 addition & 1 deletion spec/gemfiles/rails_6_0.gemfile
Expand Up @@ -4,7 +4,7 @@ source 'https://rubygems.org'
gemspec path: '../../'
eval_gemfile '../../shared.gemfile'

gem 'rails', '~> 6.0.0.rc1'
gem 'rails', '~> 6.0.0.rc2'
gem 'rails-i18n', '~> 6.0.0.beta1'

# https://github.com/rails/rails/blob/v6.0.0.rc1/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L12
Expand Down

0 comments on commit 67630c1

Please sign in to comment.