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

Move supports :create out of base Flavor model #20332

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

agrare
Copy link
Member

@agrare agrare commented Jul 6, 2020

Currently the supports :create/:delete features are enabled at the base Flavor model, however only Openstack implements these operations

This was added by #15552 along with validate_create_flavor and validate_delete_flavor methods, however the UI doesn't appear to use these and instead just checks ems.class::Flavor.supports?(:create)

The result of this is that you can add a flavor on other Cloud providers such as Amazon and Azure but nothing actually happens when you do so.

Cross Repo Tests: ManageIQ/manageiq-cross_repo-tests#149

Depends on:

Before: Able to get to the Add a Flavor screen with an AWS provider
Screenshot from 2020-07-06 12-53-16

After: Configuration dropdown is disabled
Screenshot from 2020-07-06 12-55-05

Currently the supports :create/:delete features are enabled at the base
Flavor model, however only Openstack implements these operations
@agrare
Copy link
Member Author

agrare commented Jul 6, 2020

I'd like to delete validate_create_flavor and validate_delete_flavor but need to do more checking that they aren't used anywhere else

@miq-bot
Copy link
Member

miq-bot commented Jul 6, 2020

Checked commits agrare/manageiq@ecbc1d1~...f3eb604 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@agrare
Copy link
Member Author

agrare commented Jul 6, 2020

I don't see validate_create_flavor or validate_delete_flavor used anywhere, also running cross repo test of UI and API ManageIQ/manageiq-cross_repo-tests#149

The validations didn't even make any sense, the create_flavor validated that the flavor was connected to an EMS but we are trying to create one.

I'm going to leave these for this bugfix and follow-up later with a refactoring PR to remove the validates_* methods after more time confirming that they aren't used

@agrare agrare force-pushed the move_supports_create_flavor branch from f3eb604 to ecbc1d1 Compare July 6, 2020 17:48
@chessbyte chessbyte merged commit 61df10e into ManageIQ:master Jul 7, 2020
@agrare agrare deleted the move_supports_create_flavor branch July 7, 2020 16:27
simaishi pushed a commit that referenced this pull request Jul 7, 2020
Move supports :create out of base Flavor model

(cherry picked from commit 61df10e)
@simaishi
Copy link
Contributor

simaishi commented Jul 7, 2020

Jansa backport details:

$ git log -1
commit 4d39829b12168bb1f58476b458174a87daf3c256
Author: Oleg Barenboim <chessbyte@gmail.com>
Date:   Tue Jul 7 12:24:13 2020 -0400

    Merge pull request #20332 from agrare/move_supports_create_flavor

    Move supports :create out of base Flavor model

    (cherry picked from commit 61df10e30a647d8b55ef4261b8e79058372bd90d)

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