Add security group subcollection to providers, cloud tenants, and vms #81
Conversation
|
Required by ManageIQ/manageiq-ui-classic#1997 |
tzumainn
force-pushed
the
security-group-subcollection
branch
from
bec9298 to
7a60ed2
Compare
tzumainn
changed the title
| module Subcollections | ||
| module SecurityGroups | ||
| def security_groups_query_resource(object) | ||
| object.respond_to?(:security_groups) ? object.security_groups : [] |
There was a problem hiding this comment.
Can you add a test that executes the case when the object does not respond to :security_groups?
|
@tzumainn when you get a chance, can you add tests for accessing security_groups subcollection for vms and cloud_tenants, similar to what you did for providers ? Thanks. |
|
@abellotti Thanks for taking a look! I've added the tests; they depend on ManageIQ/manageiq#16127 being merged in manageiq, though. |
|
Thanks @tzumainn for the update, I'll look at both. |
|
@tzumainn can you fix the Style/Align rubocop issue and repush ? Thanks. |
|
@abellotti Done! (I think - I'm a bit uncertain about the frozen string comment, but I guess I'll see). |
| @@ -2,7 +2,7 @@ module Api | |||
| module Subcollections | |||
| module SecurityGroups | |||
| def security_groups_query_resource(object) | |||
| object.respond_to?(:security_groups) ? object.security_groups : [] | |||
| object.respond_to?(:security_groups) ? object.security_groups : [].freeze | |||
There was a problem hiding this comment.
this .freeze is not needed. can you remove it/repush ? Thanks. the frozen literal rubocop is unrelated.
|
@abellotti Ah, that's good to know. I've removed it! |
|
Checked commits tzumainn/manageiq-api@6fcfe7e~...fed2730 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 app/controllers/api/subcollections/security_groups.rb
|
|
Thanks @tzumainn for the API Enhancement. LGTM!! 👍 |
|
Looks like create & update (& delete) will also be needed for ManageIQ/manageiq-ui-classic#1997 . |
No description provided.