Skip to content

Commit

Permalink
Merge pull request #18257 from kbrock/fix-miq_request-ownership
Browse files Browse the repository at this point in the history
Fix miq request ownership

(cherry picked from commit aa6fba5)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1656170
  • Loading branch information
gtanzillo authored and simaishi committed Dec 14, 2018
1 parent 7ed95ff commit dc417f3
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 2 deletions.
8 changes: 8 additions & 0 deletions app/models/miq_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@ def ordered_widget_sets
end
end

def regional_groups
self.class.regional_groups(self)
end

def self.regional_groups(group)
where(arel_table.grouping(Arel::Nodes::NamedFunction.new("LOWER", [arel_attribute(:description)]).eq(group.description.downcase)))
end

def self.create_tenant_group(tenant)
tenant_full_name = (tenant.ancestors.map(&:name) + [tenant.name]).join("/")

Expand Down
4 changes: 2 additions & 2 deletions app/models/miq_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@ def self.user_or_group_owned(user, miq_group)
end

def self.user_owned(user)
where(:requester_id => user.id)
where(:requester_id => user.regional_users.select(:id))
end

def self.group_owned(miq_group)
where(:requester_id => miq_group.user_ids)
where(:requester_id => miq_group.regional_groups.joins(:users).select("users.id"))
end

# Supports old-style requests where specific request was a seperate table connected as a resource
Expand Down
8 changes: 8 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,14 @@ def accessible_vms
end
end

def regional_users
self.class.regional_users(self)
end

def self.regional_users(user)
where(arel_table.grouping(Arel::Nodes::NamedFunction.new("LOWER", [arel_attribute(:userid)]).eq(user.userid.downcase)))
end

def self.super_admin
in_my_region.find_by_userid("admin")
end
Expand Down
13 changes: 13 additions & 0 deletions spec/models/miq_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -546,4 +546,17 @@
expect(User.find_by(:id => user2).current_group.id).to eq(testgroup3.id)
end
end

describe "#regional_groups" do
let(:other_id) { ApplicationRecord.id_in_region(1, ApplicationRecord.my_region_number + 1) }
let(:group) { FactoryGirl.create(:miq_group) }
let!(:regional_group) { FactoryGirl.create(:miq_group, :description => group.description.upcase, :id => other_id) }

it "finds regional groups" do
FactoryGirl.create(:miq_group) # ensure these doen't come back
FactoryGirl.create(:miq_group, :id => other_id + 1)

expect(group.regional_groups).to match_array([group, regional_group])
end
end
end
28 changes: 28 additions & 0 deletions spec/models/miq_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -476,4 +476,32 @@
expect(request.options[:abc]).to eq(1)
end
end

describe ".user_owned" do
let(:regional_id) { ApplicationRecord.id_in_region(1, ApplicationRecord.my_region_number + 1) }
let(:user) { FactoryGirl.create(:user) }
let!(:regional_user) { FactoryGirl.create(:user, :userid => user.userid.upcase, :id => regional_id) }
let!(:request) { FactoryGirl.create(:vm_migrate_request, :requester => user) }
let!(:regional_request) { FactoryGirl.create(:vm_migrate_request, :requester => regional_user) }
it "finds request for cross region users" do
FactoryGirl.create(:vm_migrate_request, :requester => FactoryGirl.create(:user))
FactoryGirl.create(:vm_migrate_request, :requester => FactoryGirl.create(:user), :id => regional_id + 1)
expect(MiqRequest.user_owned(regional_user)).to match_array([request, regional_request])
end
end

describe ".group_owned" do
let(:regional_id) { ApplicationRecord.id_in_region(1, ApplicationRecord.my_region_number + 1) }
let(:group) { FactoryGirl.create(:miq_group) }
let(:regional_group) { FactoryGirl.create(:miq_group, :id => regional_id) }
let(:user) { FactoryGirl.create(:user, :miq_groups => [group]) }
let!(:regional_user) { FactoryGirl.create(:user, :userid => user.userid.upcase, :id => regional_id, :miq_groups => [regional_group]) }
let!(:request) { FactoryGirl.create(:vm_migrate_request, :requester => user) }
let!(:regional_request) { FactoryGirl.create(:vm_migrate_request, :requester => regional_user) }
it "finds request for cross region groups" do
FactoryGirl.create(:vm_migrate_request, :requester => FactoryGirl.create(:user))
FactoryGirl.create(:vm_migrate_request, :requester => FactoryGirl.create(:user), :id => regional_id + 1)
expect(MiqRequest.user_owned(regional_user)).to match_array([request, regional_request])
end
end
end
13 changes: 13 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -591,4 +591,17 @@
expect(User.authorize_user("admin")).to be_nil
end
end

describe "#regional_users" do
let(:other_id) { ApplicationRecord.id_in_region(1, ApplicationRecord.my_region_number + 1) }
let(:user) { FactoryGirl.create(:user) }
let!(:regional_user) { FactoryGirl.create(:user, :userid => user.userid.upcase, :id => other_id) }

it "finds regional users" do
FactoryGirl.create(:user) # ensure these doen't come back
FactoryGirl.create(:user, :id => other_id + 1)

expect(user.regional_users).to match_array([user, regional_user])
end
end
end

0 comments on commit dc417f3

Please sign in to comment.