Get the tenant user information for Host which does not have a connected provider. #6464
Conversation
|
@gmcculloug please review |
|
@mkanoor @gmcculloug Please review |
|
<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush. |
lfu
force-pushed
the
default_tenant_1302687
branch
2 times, most recently
from
5197ad3
to
77336c6
Compare
| it "has tenant from root tenant" do | ||
| host = FactoryGirl.create(:host, :ext_management_system => ems) | ||
| ems.destroy | ||
| host.reload |
There was a problem hiding this comment.
@lfu Is there any reason to create the host with an EMS and then delete the EMS? You should be able to just create the host and not pass any EMS and avoid the destroy and reload.
There was a problem hiding this comment.
@gmcculloug Was recreating the issue as reported 😄
lfu
force-pushed
the
default_tenant_1302687
branch
2 times, most recently
from
e15f7e1
to
b5956ca
Compare
|
@lfu We should add tests for |
lfu
force-pushed
the
default_tenant_1302687
branch
from
b5956ca
to
612621d
Compare
|
@gmcculloug Updated. |
| :miq_group_id => ems.tenant.default_miq_group.id, | ||
| :tenant_id => ems.tenant.id | ||
| } | ||
| expect(MiqAeEngine).to receive(:deliver_queue).with(hash_including(args), anything) |
There was a problem hiding this comment.
@lfu You do not need to test through MiqAeEngine on each model. I would expect much simpler tests that call tenant_identify with the two configurations (with provider/without provider) and validate that they returns the expected User object configuration.
lfu
force-pushed
the
default_tenant_1302687
branch
from
612621d
to
e3638fb
Compare
| @@ -213,4 +213,30 @@ | |||
| expect(result).to be_falsey | |||
| end | |||
| end | |||
|
|
|||
| context "#tenant_identity" do | |||
| # admin user is needed to process Events | |||
There was a problem hiding this comment.
@lfu This comment does not apply anymore and should be removed. Same with the other tests.
There was a problem hiding this comment.
@gmcculloug User admin is still needed for these classes.
There was a problem hiding this comment.
Correct, but it is not being used "to process events". I do not think the comment line is needed anymore as the use of admin in the tests below is clear.
There was a problem hiding this comment.
To clarify, I am not saying get rid of admin, I am saying the comment is no longer needed.
lfu
force-pushed
the
default_tenant_1302687
branch
from
e3638fb
to
d36f26a
Compare
|
Checked commits lfu/manageiq@67a0795~...d36f26a with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0 |
Get the tenant user information for Host which does not have a connected provider.
Get the tenant user information for Host which does not have a connected provider. Manually merged [Refactor MiqAeEvent#automate_user_ids ManageIQ#6201](ManageIQ#6201). Clean cherry-pick [Remove the call to User#groups_include? which has been removed. ManageIQ#6610](ManageIQ#6610). Manually merged [Get the tenant user information for Host which does not have a connected provider. ManageIQ#6464](ManageIQ#6464). https://bugzilla.redhat.com/show_bug.cgi?id=1308748 See merge request !796
The default user/group/tenant to start with.
https://bugzilla.redhat.com/show_bug.cgi?id=1302687