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

convert from AvailabilityMixin to SupportsFeatureMixin #21991

Merged
merged 1 commit into from
Jul 15, 2022

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Jul 13, 2022

part of #21179

@@ -179,12 +179,15 @@ class Host < ApplicationRecord
before_create :make_smart
after_save :process_events


supports :check_compliance_queue
Copy link
Member

Choose a reason for hiding this comment

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

@kbrock think we should drop the _queue from the feature name

Copy link
Member Author

@kbrock kbrock Jul 15, 2022

Choose a reason for hiding this comment

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

ok, I was translating it as-is.
I know we do have host_button_operation - which again, makes me worried about the whole generic button

Copy link
Member Author

Choose a reason for hiding this comment

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

note: this also pertains to scan_and_check_compliance_queue a few lines down

Copy link
Member Author

Choose a reason for hiding this comment

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

Overview

So the places that we do reference these methods are using supports in a way we like. It is the generic button that has me concerned, though I do not know if that is used for checking compliance. Localization seems to use the name without Queue.

Details

ok, ci_processing using 1 and 2 does convert the action check_compliance_queue to :check_compliance. (not sure if the scan one is viable here)

host_actions does reference these 2 methods, and it converts them to use different privileges (i.e.: host_check_compliance, host_analyze_check_compliance)

But generic_feature_button calls with the feature name directly. I do not know if these methods could be called from a generic button. But this is my concern, which may be not valid.

In a few spots, we convert check_compliance_queue to Check Compliance for display. So at least we don't have that as a concern.

Copy link
Member

Choose a reason for hiding this comment

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

Okay I think _queue in the feature name is wrong but we can follow-up later

@kbrock
Copy link
Member Author

kbrock commented Jul 15, 2022

update:

  • removed adhoc
  • fixed cop

@miq-bot
Copy link
Member

miq-bot commented Jul 15, 2022

Checked commit kbrock@9bcb970 with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
8 files checked, 0 offenses detected
Everything looks fine. 🏆

@agrare agrare merged commit 10cf07e into ManageIQ:master Jul 15, 2022
@kbrock kbrock deleted the supports_validate branch July 15, 2022 20:23
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