Commit 90716733 authored by blackst0ne's avatar blackst0ne

[Rails5] Fix `User#manageable_groups`

In `arel 7.0` (`7.1.4` version is used for rails5) there were introduced
some changes that break our code in the `User#manageable_groups` method.

The problem is that `arel_table[:id].in(Arel::Nodes::SqlLiteral)` generates
wrong `IN ()` construction. The selection for `IN` is missing:

=> "\"namespaces\".\"id\" IN (0)"

That caused such spec errors for the `rails5` branch:

```
4) User groups with child groups #manageable_groups does not include duplicates if a membership was added for the subgroup
    Failure/Error: expect(user.manageable_groups).to contain_exactly(group, subgroup)

      expected collection contained:  [#<Group id:232 @group29>, #<Group id:234 @group29/group30>]
      actual collection contained:    []
      the missing elements were:      [#<Group id:232 @group29>, #<Group id:234 @group29/group30>]
    # ./spec/models/user_spec.rb:699:in `block (5 levels) in <top (required)>'
    # ./spec/spec_helper.rb:188:in `block (2 levels) in <top (required)>'
    # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:112:in `block in run'
    # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `loop'
    # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `run'
    # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
    # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:30:in `block (2 levels) in setup'
```

This commit changes `User#manageable_groups` in the way to drop the usage of
`Arel::Nodes::SqlLiteral` and adds usage of raw SQL query.
This change should be updated when we're migrated to Rails 5.2 because arel
was fixed in `9.0.0` (which is used in Rails 5.2).
parent 947ed8b8
...@@ -947,10 +947,13 @@ class User < ActiveRecord::Base ...@@ -947,10 +947,13 @@ class User < ActiveRecord::Base
end end
def manageable_groups def manageable_groups
union = Gitlab::SQL::Union.new([owned_groups.select(:id), union_sql = Gitlab::SQL::Union.new([owned_groups.select(:id), masters_groups.select(:id)]).to_sql
masters_groups.select(:id)])
arel_union = Arel::Nodes::SqlLiteral.new(union.to_sql) # Update this line to not use raw SQL when migrated to Rails 5.2.
owned_and_master_groups = Group.where(Group.arel_table[:id].in(arel_union)) # Either ActiveRecord or Arel constructions are fine.
# This was replaced with the raw SQL construction because of bugs in the arel gem.
# Bugs were fixed in arel 9.0.0 (Rails 5.2).
owned_and_master_groups = Group.where("namespaces.id IN (#{union_sql})") # rubocop:disable GitlabSecurity/SqlInjection
Gitlab::GroupHierarchy.new(owned_and_master_groups).base_and_descendants Gitlab::GroupHierarchy.new(owned_and_master_groups).base_and_descendants
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment