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

Make Taggable of AutomationManager's authentications/playbooks/repos #17049

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

@agrare agrare self-assigned this Feb 26, 2018
@jameswnl
Copy link
Contributor Author

@miq-bot add_labels enhancement, providers/ansible_tower

@miq-bot
Copy link
Member

miq-bot commented Feb 26, 2018

Checked commit jameswnl@8ab4bbf with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

@agrare
Copy link
Member

agrare commented Feb 26, 2018

@jameswnl probably should add acts_as_miq_taggable to the base models not just the ansible ones

@jameswnl
Copy link
Contributor Author

jameswnl commented Feb 26, 2018

@agrare I actually thought about adding to the base models. But the BZs ask for scope of Ansible. Now I've added to AutomationManager.

I don't have specific reason opposing to tag the base models. But If we do want to, essentially we are making all Authentication taggable. Is that Ok?

@agrare
Copy link
Member

agrare commented Feb 27, 2018

@jameswnl okay that's fine with me, @h-kataria do you know if not having these at the base model will impact the UI RBAC work?

@h-kataria
Copy link
Contributor

@agrare @jameswnl these changes don't look harmful to me, we will need these to turn on Tagging support in UI for these models.

@agrare
Copy link
Member

agrare commented Feb 27, 2018

Thanks @h-kataria

@agrare agrare merged commit dd90df8 into ManageIQ:master Feb 27, 2018
@jameswnl
Copy link
Contributor Author

@h-kataria what abstraction level does UI call on? Does UI call on ManageIQ::Providers::AutomationManager? Or it call on the base models?

@agrare agrare added this to the Sprint 81 Ending Mar 12, 2018 milestone Feb 28, 2018
@simaishi
Copy link
Contributor

simaishi commented Mar 7, 2018

gaprindashvili/yes ?

@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 7, 2018

@miq-bot add_labels gaprindashvili/yes

@simaishi
Copy link
Contributor

simaishi commented Mar 9, 2018

Gaprindashvili backport details:

$ git log -1
commit 71b010697829b6ba2451b14dcae1a9ab2d8987c8
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Feb 27 14:54:27 2018 -0500

    Merge pull request #17049 from jameswnl/taggable
    
    Make Taggable of AutomationManager's authentications/playbooks/repos
    (cherry picked from commit dd90df8863441ef705f6dfc1e6f3f0b338ef58a5)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1553393
    https://bugzilla.redhat.com/show_bug.cgi?id=1553768
    https://bugzilla.redhat.com/show_bug.cgi?id=1553396

@hstastna
Copy link

hstastna commented Apr 3, 2018

@jameswnl I had to move acts_as_miq_taggable to base classes to fix errors when editing Ansible Credential/Repository (#17244)

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.

7 participants