summaryrefslogtreecommitdiff
path: root/app
diff options
context:
space:
mode:
authorblackst0ne <blackst0ne.ru@gmail.com>2018-04-19 15:42:50 +1100
committerblackst0ne <blackst0ne.ru@gmail.com>2018-04-19 15:42:50 +1100
commit90716733522691e964680539e231d3677bafbc51 (patch)
treeecd0c96876b106fc2eb43fa10747ffb51e69a7aa /app
parent947ed8b88281a618530ae30bc0fe1e3d61a3c1d9 (diff)
downloadgitlab-ce-90716733522691e964680539e231d3677bafbc51.tar.gz
[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).
Diffstat (limited to 'app')
-rw-r--r--app/models/user.rb11
1 files changed, 7 insertions, 4 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index d5c5c0964c5..cddc0b8d2e9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -947,10 +947,13 @@ class User < ActiveRecord::Base
end
def manageable_groups
- union = Gitlab::SQL::Union.new([owned_groups.select(:id),
- masters_groups.select(:id)])
- arel_union = Arel::Nodes::SqlLiteral.new(union.to_sql)
- owned_and_master_groups = Group.where(Group.arel_table[:id].in(arel_union))
+ union_sql = Gitlab::SQL::Union.new([owned_groups.select(:id), masters_groups.select(:id)]).to_sql
+
+ # Update this line to not use raw SQL when migrated to Rails 5.2.
+ # 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
end