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

configuration_script_sources subcollection #15070

Merged
merged 1 commit into from
May 17, 2017
Merged

configuration_script_sources subcollection #15070

merged 1 commit into from
May 17, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented May 11, 2017

For this BZ, it is necessary to filter on region_number of playbooks that belong to a repository.

While this can be achieved via

GET http://localhost:3000/api/configuration_script_payloads?expand=resources&filter[]=configuration_script_source_id=10000000000001&filter[]=region_number=10

adding it as a subcollection makes it possible to filter on the configuration_script_sources collection directly:

GET http://localhost:3000/api/configuration_script_sources/10000000000001/configuration_script_payloads?expand=resources&filter[]=region_number=10

@miq-bot add_label enhancement, api
@miq-bot assign @abellotti

cc: @h-kataria @gmcculloug

module Subcollections
module ConfigurationScriptPayloads
def configuration_script_payloads_query_resource(object)
object.respond_to?(:configuration_script_payloads) ? object.configuration_script_payloads : []
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not all ConfigurationScripts respond to configuration_script_payloads? If they don't, can you add a test showing that this check is necessary?

Copy link
Contributor

@imtayadeway imtayadeway left a comment

Choose a reason for hiding this comment

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

LGTM. It might be good to provide an example query/response so we could see how this solves the problem outlined in the BZ, if it's not too much trouble

@miq-bot
Copy link
Member

miq-bot commented May 11, 2017

Checked commit jntullo@111b23e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

@jntullo
Copy link
Author

jntullo commented May 16, 2017

@miq-bot add_label fine/yes

@abellotti
Copy link
Member

This LGTM!! Thanks @jntullo for the enhancement. 👍

@abellotti abellotti merged commit 632c2d0 into ManageIQ:master May 17, 2017
@abellotti abellotti added this to the Sprint 61 Ending May 22, 2017 milestone May 17, 2017
simaishi pushed a commit that referenced this pull request Jun 8, 2017
@simaishi
Copy link
Contributor

simaishi commented Jun 8, 2017

Fine backport details:

$ git log -1
commit 5f807ccfd0d01dbec2169e4062b8a343cb3ee1bd
Author: Alberto Bellotti <[email protected]>
Date:   Wed May 17 11:23:03 2017 -0400

    Merge pull request #15070 from jntullo/enhancement/csp_subcollection
    
    configuration_script_sources subcollection
    (cherry picked from commit 632c2d0c272e13fcd3f91d8334f4e402dcab6f32)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1459986

@jntullo jntullo deleted the enhancement/csp_subcollection branch November 28, 2017 19:42
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.

5 participants