Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return empty AR relation instead of nil for ::InfraManager#cloud_tenants #184

Merged
merged 1 commit into from
Jan 4, 2018

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Jan 3, 2018

InfraProvider don't have relation to cloud_tenants - there was
created only empty method returning nil to preserve consitency - but on
some places is expected AR relation object.

For example screen add new subnet, there are listed network managers and
their parent manager's cloud_tenants but they can be infra manager whithout
cloud_tenants.

Such place, where was expected AR Relation object is here:
https://github.com/ManageIQ/manageiq-api/blob/master/app/controllers/api/subcollections/cloud_tenants.rb#L5

which was reached for example by this request:

http://localhost:3000/api/providers/:id/cloud_tenants but it fails later
when object has been infra manager and the method cloud_tenants_query_resource
was returning nil.

@miq-bot add_label graprindashvili/yes, bug

cc @gtanzillo
@miq-bot assign @aufi

Reproducer

zytkbnyvnw

Links

it was found during BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1463422

InfraProvider don't have relation to cloud_tenants - there was
created only empty method returning nil to preserve consitency - but on
some places is expected AR relation object.

For example screen add new subnet, there are listed network managers and
their parent manager's cloud_tenants but they can be infra manager whithout
cloud_tenants.

Such place, where was expected AR Relation object is here:
https://github.com/ManageIQ/manageiq-api/blob/master/app/controllers/api/subcollections/cloud_tenants.rb#L5

which was reached for example by this request:

http://localhost:3000/api/providers/:id/cloud_tenants but it fails later
when object has been infra manager and the method cloud_tenants_query_resource
was returning nil.
@miq-bot
Copy link
Member

miq-bot commented Jan 3, 2018

@lpichler Cannot apply the following label because they are not recognized: graprindashvili/yes

@miq-bot miq-bot added the bug label Jan 3, 2018
@lpichler lpichler changed the title Return empty AR relation instead of nil Return empty AR relation instead of nil for ::Openstack::InfraManager#cloud_tenants Jan 3, 2018
@lpichler lpichler changed the title Return empty AR relation instead of nil for ::Openstack::InfraManager#cloud_tenants Return empty AR relation instead of nil for ::InfraManager#cloud_tenants Jan 3, 2018
@miq-bot
Copy link
Member

miq-bot commented Jan 3, 2018

Checked commit lpichler@a39cb4a with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@aufi
Copy link
Member

aufi commented Jan 4, 2018

Thanks for PR @lpichler! LGTM, merging.

@aufi aufi merged commit 37acd49 into ManageIQ:master Jan 4, 2018
@aufi aufi added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 4, 2018
@lpichler lpichler deleted the return_empty_for_cloud_tenants branch January 4, 2018 09:38
simaishi pushed a commit that referenced this pull request Jan 4, 2018
Return empty AR relation instead of nil for ::InfraManager#cloud_tenants
(cherry picked from commit 37acd49)
@simaishi
Copy link
Contributor

simaishi commented Jan 4, 2018

Gaprindashvili backport details:

$ git log -1
commit 8c59c3a3cfdc376db5b3fdeedf1b43ee0547580b
Author: Marek Aufart <[email protected]>
Date:   Thu Jan 4 10:29:48 2018 +0100

    Merge pull request #184 from lpichler/return_empty_for_cloud_tenants
    
    Return empty AR relation instead of nil for ::InfraManager#cloud_tenants
    (cherry picked from commit 37acd49c820af306d5ed152ffbfcf1c6609deaad)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants