Fix ownership to work across multiple regions. #11992
Conversation
OwnershipMixin#user_or_group_owned was not region aware, and now it is.
Fryguy
force-pushed
the
evm_owner_regionally
branch
from
8c55167
to
9b897c3
Compare
|
It's okay that this doesn't take groups into account. We don't replicate I think we talked about it, but do we have a plan for the |
|
We decided it wasn't important at the moment. I don't want to change it because all of the virtual_attributes would be affected, and that is where @kbrock got the enormous performance savings. |
gtanzillo
changed the title
|
Marking as WIP until tests are added. |
|
Would be nice if we had a single user table across all regions. Or if we were able to link all of the same user together. I do wonder if having the same userid in multiple regions makes you the same user. |
gtanzillo
force-pushed
the
evm_owner_regionally
branch
from
57f4bc4
to
5715a01
Compare
gtanzillo
changed the title
|
@Fryguy I added some tests, as we discussed. Please review when you get a chance. |
| it "returns resource owned by user" do | ||
| my_region_number = ApplicationRecord.my_region_number | ||
| remote_region_number = my_region_number + 1 | ||
| remote_id = ApplicationRecord.region_to_range(remote_region_number).first |
There was a problem hiding this comment.
I would put these three lines in a let(:remote_id) block to DRY this up.
| remote_region_number = my_region_number + 1 | ||
| remote_id = ApplicationRecord.region_to_range(remote_region_number).first | ||
|
|
||
| remote_owning_user = FactoryGirl.create :user, :id => remote_id, :userid => "user_owner", :miq_groups => FactoryGirl.create_list(:miq_group, 1) |
There was a problem hiding this comment.
You could also make the remote_owning_user a let then assign the groups in this spec as that is what has actually changed.
…e and updated to enable generically testing ownership of Vm, Service and ServiceTemplate objects
gtanzillo
force-pushed
the
evm_owner_regionally
branch
from
e47145f
to
fd2a3da
Compare
|
@carbonin Thanks for the feedback. I've made the changes you suggested. |
|
Checked commits Fryguy/manageiq@9b897c3~...fd2a3da with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 app/models/mixins/ownership_mixin.rb
spec/support/examples_group/shared_examples_for_ownership_mixin.rb
|
|
👍 |
|
yay |
Fix ownership to work across multiple regions. (cherry picked from commit 76f78dc)
|
Euwe Backport details: $ git log -1
commit 9aa85b9777099729dc7be82920e169e603531af2
Author: Nick Carboni <ncarboni@redhat.com>
Date: Wed Oct 19 17:57:20 2016 -0400
Merge pull request #11992 from Fryguy/evm_owner_regionally
Fix ownership to work across multiple regions.
(cherry picked from commit 76f78dcad8d00f4d8e12239daf14b6373f431a5c) |
Includes the updates from ManageIQ#11992 and also includes the (minimal) necessary updates to allow it to work: updating the arel for the evm_owner_userid virtual_column, which was handled by virtual_delegates in the newer releases.
https://bugzilla.redhat.com/show_bug.cgi?id=1401912 Note, we were only downcasing the userids in the users table, so a mixed case logged in userid would never match. This would cause a logged in self service user with mixed case userid to not see vms they own. This was broken here: 9b897c3 As part of: ManageIQ#11992 We're fixing much like we did with groups here: ManageIQ#12114
https://bugzilla.redhat.com/show_bug.cgi?id=1401912 Note, we were only downcasing the userids in the users table, so a mixed case logged in userid would never match. This would cause a logged in self service user with mixed case userid to not see vms they own. This was broken here: 9b897c3 As part of: ManageIQ#11992 We're fixing much like we did with groups here: ManageIQ#12114
OwnershipMixin#user_or_group_owned was not region aware, and now it is.
This is only for the user...I did not make it work for the group yet.
@carbonin @gtanzillo Please review. I have not added tests yet, or even verified the existing tests covered it, but it works via rails c.
@kbrock Please also review. I totally ripped off your code from the
virtual_attribute :owned_by_current_user, and I'm wondering if there is a way to share the code so I am not duplicating it. I can't figure out how, since the virtual_attribute uses a block and build off of at(is that an arel_table?), but the code I wrote is a regular scope.