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

Retire Task deliver_to_automate now uses tenant_identity #18104

Merged
merged 1 commit into from
Oct 25, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Oct 17, 2018

In app/models/miq_retire_task.rb
Change hardcoded user reference to the request user and tenant

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1640618

@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 17, 2018

@tinaafitz

@d-m-u d-m-u changed the title Er, this shouldn't be hard coded... [WIP] Er, this shouldn't be hard coded... Oct 17, 2018
@miq-bot miq-bot added the wip label Oct 17, 2018
@d-m-u d-m-u force-pushed the fixing_retirement_task_user branch from 6496fd5 to f870703 Compare October 25, 2018 14:13
@d-m-u d-m-u changed the title [WIP] Er, this shouldn't be hard coded... [WIP] Retire Task deliver_to_automate should use tenancy to set the user Oct 25, 2018
@d-m-u d-m-u force-pushed the fixing_retirement_task_user branch from f870703 to 9558cd6 Compare October 25, 2018 14:16
@d-m-u d-m-u changed the title [WIP] Retire Task deliver_to_automate should use tenancy to set the user [WIP] Retire Task deliver_to_automate should maaaaybe use tenant_identity to set the user Oct 25, 2018
Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookin good

:miq_group_id => 2,
:tenant_id => 1,
:user_id => tenant_identity.id,
:miq_group_id => tenant_identity.miq_groups.first.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:miq_group_id => tenant_identity.miq_groups.first.id,
:miq_group_id => tenant_identity.current_group.id,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically, this could be current_group_id, but you need to download current_group for the next line, so probably close enough

:tenant_id => 1,
:user_id => tenant_identity.id,
:miq_group_id => tenant_identity.miq_groups.first.id,
:tenant_id => tenant_identity.miq_groups.first.tenant.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:tenant_id => tenant_identity.miq_groups.first.tenant.id,
:tenant_id => tenant_identity.current_group.tenant_id,

@kbrock kbrock self-assigned this Oct 25, 2018
@d-m-u d-m-u force-pushed the fixing_retirement_task_user branch from 9558cd6 to 48c6d5e Compare October 25, 2018 15:45
@d-m-u
Copy link
Contributor Author

d-m-u commented Oct 25, 2018

@miq-bot remove_label wip
@miq-bot add_label bug, hammer/yes
@miq-bot add_reviewer @tinaafitz

@d-m-u d-m-u changed the title [WIP] Retire Task deliver_to_automate should maaaaybe use tenant_identity to set the user Retire Task deliver_to_automate should maaaaybe use tenant_identity to set the user Oct 25, 2018
@miq-bot miq-bot requested a review from tinaafitz October 25, 2018 15:49
@d-m-u d-m-u force-pushed the fixing_retirement_task_user branch from 48c6d5e to 567b40d Compare October 25, 2018 17:39
@d-m-u d-m-u force-pushed the fixing_retirement_task_user branch from 567b40d to a396837 Compare October 25, 2018 17:40
@miq-bot
Copy link
Member

miq-bot commented Oct 25, 2018

Checked commit d-m-u@a396837 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@kbrock kbrock added this to the Sprint 98 Ending Nov 5, 2018 milestone Oct 25, 2018
@kbrock kbrock changed the title Retire Task deliver_to_automate should maaaaybe use tenant_identity to set the user Retire Task deliver_to_automate now uses tenant_identity Oct 25, 2018
@kbrock kbrock merged commit af80b91 into ManageIQ:master Oct 25, 2018
@d-m-u d-m-u deleted the fixing_retirement_task_user branch October 25, 2018 18:32
simaishi pushed a commit that referenced this pull request Oct 26, 2018
Retire Task deliver_to_automate now uses tenant_identity

(cherry picked from commit af80b91)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1640618
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit ec64282145d29bb47923b72db4fdbfaeffb2773d
Author: Keenan Brock <[email protected]>
Date:   Thu Oct 25 14:32:28 2018 -0400

    Merge pull request #18104 from d-m-u/fixing_retirement_task_user
    
    Retire Task deliver_to_automate now uses tenant_identity
    
    (cherry picked from commit af80b9174dbfabf874974891282cd2477b04faa4)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1640618

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.

5 participants