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 25, 2019
1 parent c691be6 commit 846d013
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 18 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
17 changes: 14 additions & 3 deletions app/models/concerns/thredded/topic_common.rb
Expand Up @@ -53,9 +53,20 @@ 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))))
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
scope = Thredded.rails_gte_600_rc_2? ? merge(unread_scope) : self

scope = scope.joins(
topics.join(reads, Arel::Nodes::OuterJoin)
.on(topics[:id].eq(reads[:postable_id]).and(reads[:user_id].eq(user.id))).join_sources
)

# Work around https://github.com/rails/rails/issues/36761
scope = scope.merge(unread_scope) unless Thredded.rails_gte_600_rc_2?

scope
end

private
Expand Down
34 changes: 26 additions & 8 deletions app/models/thredded/messageboard.rb
Expand Up @@ -128,14 +128,32 @@ 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(

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(
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
)
else
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
)
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 846d013

Please sign in to comment.