API for vm_infras in reconfigure form #22980
Conversation
jaisejose1123
force-pushed
the
api_vm_infra
branch
from
070bf88
to
35542c6
Compare
| # Vm Infra | ||
| - :name: Vm Infras | ||
| :description: Every thing to Vm infras | ||
| :feature_type: node | ||
| :identifier: vm_infra | ||
| :children: | ||
| - :name: View | ||
| :description: View Vm Infras | ||
| :feature_type: view | ||
| :identifier: vm_infra_view |
There was a problem hiding this comment.
Confused why we're adding a new top level node when we already have Reconfigure VMs / vm_reconfigure_all https://github.com/ManageIQ/manageiq/blob/master/db/fixtures/miq_product_features.yml#L5949-L5973
There was a problem hiding this comment.
Do we need infra specific features?
Can we use 1 set of privileges for both views?
jaisejose1123
force-pushed
the
api_vm_infra
branch
2 times, most recently
from
6fbab91
to
1a3a156
Compare
jaisejose1123
force-pushed
the
api_vm_infra
branch
from
6e6a5ac
to
74cd750
Compare
|
Checked commit jaisejose1123@74cd750 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint lib/rbac/filterer.rb
|
| elsif targets.first.kind_of?(Hash) | ||
| target_ids = targets.first[:objectIds] |
There was a problem hiding this comment.
targets should be passed in as a scope Vm.all or maybe an array of ids [1,2,3].
Don't add this here.
And to be honest, probably should not need to make changes in here unless we are not properly filtering a class.
| @@ -367,7 +380,7 @@ def search(options = {}) | |||
| end | |||
|
|
|||
| # Preserve sort order of incoming target_ids | |||
| if !target_ids.nil? && !order | |||
| if !target_ids.nil? && !order && !targets.kind_of?(Array) | |||
There was a problem hiding this comment.
please share reason for this change.
The implications of this change will impact many many places of the application.
I do agree, that this sorting is called in a few places that I wish it were not.
The solution I find is to pass in a scope and that solves this problem.
|
In reviewing this again, I would like to avoid all the rbac changes, and probably the feature changes. |
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
1 similar comment
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
Related PR
ManageIQ/manageiq-api#1253