-
Notifications
You must be signed in to change notification settings - Fork 141
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
Generic Object Definition CRUD #15
Generic Object Definition CRUD #15
Conversation
@@ -0,0 +1,286 @@ | |||
# rubocop:disable Style/WordArray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling as I prefer that the sample requests not use word arrays to better align with how the request will look.
@miq-bot remove_label wip |
hash_including('href' => a_string_matching(generic_object_definitions_url(object_def.compressed_id))) | ||
] | ||
} | ||
run_get(generic_object_definitions_url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does expand=resources
do the right thing? Could we add a test for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, tiny nit, but can you group all the verification code together and separately from the execution step(s)? 🍨
If you can in the PR comments, create and update examples with sample data showing the properties, methods, etc. Thanks!! |
Looks good @jntullo, minor updates noted. Thanks!! |
def fetch_generic_object_definition(id) | ||
klass = collection_class(:generic_object_definitions) | ||
klass.find_by(:name => id) || klass.find(id) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add Rbac.filtered_object before returning it.
|
||
def fetch_generic_object_definition(id) | ||
klass = collection_class(:generic_object_definitions) | ||
klass.find_by(:name => id) || klass.find(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leverage ApplicationRecord.compressed_id?(id) and leverage resource_search for id based.
|
||
def fetch_generic_object_definition(id) | ||
klass = collection_class(:generic_object_definitions) | ||
go_def = klass.find_by(:name => id) || klass.find(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want logic separation, i.e. check if id is compressed_id? to do a klass.find otherwise do the find_by :name, (just in case someone named one as a numeric string).
Checked commits jntullo/manageiq-api@55db55e~...25b9576 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Thanks @jntullo for this enhancement, LGTM!!. 😍 |
manageiq-api
version of ManageIQ/manageiq#15374This PR adds:
Example of create:
Editing a generic object definition. Properties will be updated as is - IE if you remove "methods" as in the below example, properties will no longer have methods and contain the attributes and associations passed in.
@miq-bot add_label enhancement, wip
@miq-bot assign @abellotti