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

Only show catalog types for supported EMSs #20039

Merged

Conversation

@agrare agrare requested review from Fryguy and kbrock as code owners April 2, 2020 13:45
@agrare agrare added the wip label Apr 2, 2020
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from 8072098 to 1eeddac Compare April 2, 2020 13:46
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from 1eeddac to c6c6340 Compare April 2, 2020 14:51
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch 4 times, most recently from fea624f to a9aafaa Compare April 3, 2020 13:11
@Fryguy
Copy link
Member

Fryguy commented Apr 3, 2020

Aside, I'm concerned about the supported prefix, especially because this introduced the supported? method, which might conflict with the supports feature stuff. For now it's fine, but we may want to consider renaming to permitted prefix (e.g. permitted_subclasses, permitted_types_for_create, etc.)

@Fryguy
Copy link
Member

Fryguy commented Apr 3, 2020

@agrare Can you also verify that the API makes its way here as well? I'm concerned the API might present/allow things it shouldn't.

@Fryguy Fryguy self-assigned this Apr 3, 2020
@agrare agrare mentioned this pull request Apr 3, 2020
48 tasks
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from a9aafaa to c8123d9 Compare April 3, 2020 13:39
@agrare
Copy link
Member Author

agrare commented Apr 3, 2020

Yeah that's true let me rename the methods I added to permitted and we can follow up with the existing supported_subclasses / supported_types

@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from c8123d9 to 150dfeb Compare April 3, 2020 13:57
@agrare agrare requested a review from gtanzillo as a code owner April 3, 2020 13:57
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from 150dfeb to 16adfcb Compare April 3, 2020 14:16
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from 16adfcb to 6b1ea4b Compare April 3, 2020 16:32
If an EMS isn't supported by disabling it in the Vmdb::PermissionStore
we shouldn't show it as an available catalog type

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1746084
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from 6b1ea4b to 4514e30 Compare April 3, 2020 17:29
@agrare
Copy link
Member Author

agrare commented Apr 3, 2020

@Fryguy cross-repo tests are green (at least on traivs .org) ManageIQ/manageiq-cross_repo-tests#106

@Fryguy
Copy link
Member

Fryguy commented Apr 6, 2020

I noticed that the network manager also has this method, which feels like it should be refactored as part of this PR.

@agrare
Copy link
Member Author

agrare commented Apr 6, 2020

I noticed that the network manager also has this method, which feels like it should be refactored as part of this PR.

https://github.com/ManageIQ/manageiq/pull/20039/files#diff-43ca61dfc6415fdd839b6468b7c20a37R71-R73

agrare added a commit to agrare/manageiq-cross_repo-tests that referenced this pull request Apr 6, 2020
@agrare agrare force-pushed the only_show_supported_emss_catalog_types branch from ee836b6 to 227ad29 Compare April 6, 2020 22:44
@miq-bot
Copy link
Member

miq-bot commented Apr 6, 2020

Checked commits agrare/manageiq@4514e30~...227ad29 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
7 files checked, 2 offenses detected

app/models/service_template.rb

agrare added a commit to agrare/manageiq-cross_repo-tests that referenced this pull request Apr 6, 2020
@agrare agrare changed the title [WIP] Only show catalog types for supported EMSs Only show catalog types for supported EMSs Apr 8, 2020
@miq-bot miq-bot removed the wip label Apr 8, 2020
@Fryguy
Copy link
Member

Fryguy commented Apr 9, 2020

Merging on red since cross-repo is green

@Fryguy Fryguy merged commit ac4f13a into ManageIQ:master Apr 9, 2020
@agrare agrare deleted the only_show_supported_emss_catalog_types branch April 9, 2020 17:43
simaishi pushed a commit that referenced this pull request Apr 16, 2020
…g_types

Only show catalog types for supported EMSs

(cherry picked from commit ac4f13a)
@simaishi
Copy link
Contributor

Jansa backport details:

$ git log -1
commit 92c8b5e3d46aebd124fb143f21ff4e736ec9d00c
Author: Jason Frey <[email protected]>
Date:   Thu Apr 9 13:37:11 2020 -0400

    Merge pull request #20039 from agrare/only_show_supported_emss_catalog_types

    Only show catalog types for supported EMSs

    (cherry picked from commit ac4f13a0be15ce047805b828317022530027970d)

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.

6 participants