API Enhancement to support filtering on id attributes by compressed id's #13645
Conversation
abellotti
changed the title
- This enhancement allows filtering on id attribute or any of the *_id
named attributes, i.e. storage_id, ems_id, etc. by compressed id,
i.e.
GET /api/vms?expand=resources&attributes=name,vendor&filter=id='2r77'
GET /api/instances?expand=resources&attributes=name,storage_id&filter[]=storage_id=1r32
- Added specs
Fixes: ManageIQ#11636
abellotti
force-pushed
the
api_filters_compressed_ids
branch
from
a6d14d1
to
b7124ff
Compare
| @@ -1,6 +1,8 @@ | |||
| module Api | |||
| class BaseController | |||
| module Parameters | |||
| include CompressedIds | |||
There was a problem hiding this comment.
Wondering if moving CompressedIds to the top of the includes in the BaseController removes the need to include it (again) here? (Which I believe might also be the solution to the weird problems Jillian was running into)
There was a problem hiding this comment.
Agreed, let's do that in a separate PR, removing all other includes of CompressedIds too.
There was a problem hiding this comment.
@imtayadeway just tried adding it to the top BaseController in my issue to see if it made a difference, but it didn't 😞
There was a problem hiding this comment.
@abellotti I think this can just be removed. Explanation here: #13671, tl;dr is these methods from this module are already available, you just can't reference any constants from the module unqualified from another module such as this.
| ems2 = FactoryGirl.create(:ems_vmware, :zone => zone) | ||
| host = FactoryGirl.create(:host) | ||
|
|
||
| _vm = FactoryGirl.create(:vm_vmware, :host => host, :ems_id => ems1.id, :raw_power_state => "poweredOn") |
There was a problem hiding this comment.
I believe setting association ids explicitly can cause some callbacks to fail..... does :ext_management_system => ems1 work?
There was a problem hiding this comment.
we do the same in vms_spec.rb, never had an issue with them. Yes, ext_management_system => ems1 works too.
There was a problem hiding this comment.
just updated and re-push (added more lines to keep rubocop happy).
| @@ -100,6 +102,8 @@ def parse_filter(filter, operators) | |||
| end | |||
| end | |||
|
|
|||
| filter_value = from_cid(filter_value) if filter_attr =~ /[_]?id$/ && cid?(filter_value) | |||
There was a problem hiding this comment.
Are the [ and ] needed in this regex?
There was a problem hiding this comment.
they are optional, I tend to use them regardless, one or more character being matched for consistency.
There was a problem hiding this comment.
WDYT about pulling this out into a named constant?
There was a problem hiding this comment.
Meh, it's only used here for now, up to you. it will eventually be needed by https://www.pivotaltracker.com/story/show/121850107
|
@imtayadeway any other changes here ? |
No need to include CompressedIds
abellotti
force-pushed
the
api_filters_compressed_ids
branch
from
2c76329
to
64b8f19
Compare
|
Checked commits abellotti/manageiq@b7124ff~...64b8f19 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
This enhancement allows filtering on id attributes or any of the *_id
named attributes, i.e. storage_id, ems_id, etc. by compressed id,
i.e.
GET /api/vms?expand=resources&attributes=name,vendor&filter[]=id='2r77'
GET /api/instances?expand=resources&attributes=name,storage_id&filter[]=storage_id=1r32
Added specs
Fixes: #11636