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.

Sign up for GitHub

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

Jump to bottom

Fix ordering an Ansible Playbook catalog items #2991

Merged
merged 2 commits into from
Dec 11, 2017
Merged

Fix ordering an Ansible Playbook catalog items #2991

merged 2 commits into from
Dec 11, 2017

Conversation

lgalis
Copy link
Contributor

@lgalis lgalis commented Dec 8, 2017 •
edited
Loading

Use service template in api_submit_endpoint for service template subclasses, as there are no collections for subclasses

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1523580

Before:
screenshot from 2017-12-08 16-56-46

After
screenshot from 2017-12-08 16-53-41

@lgalis
Copy link
Contributor Author

lgalis commented Dec 8, 2017

@miq-bot add_label bug, gaprindashvili/yes, blocker

@lgalis
Copy link
Contributor Author

lgalis commented Dec 8, 2017

@eclarizio , @dclarizio - please review

@dclarizio dclarizio self-assigned this Dec 8, 2017
@lgalis lgalis force-pushed the use_service_template_base_class_for_srvc_dialog_api branch from d4391e5 to d84d09b Compare December 9, 2017 16:11
@lgalis lgalis changed the title [WIP] Fix ordering an Ansible Playbook catalog items Fix ordering an Ansible Playbook catalog items Dec 11, 2017
@miq-bot miq-bot removed the wip label Dec 11, 2017
eclarizio
eclarizio suggested changes Dec 11, 2017
View reviewed changes
Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

The code seems like it should work but the spec is passing with the incorrect target_type.

spec/services/dialog_local_service_spec.rb Outdated
target_ansible = instance_double("ServiceTemplateAnsiblePlaybook", :class => ServiceTemplateAnsiblePlaybook, :id => 987, :service_template_catalog_id => 798)
expect(service.determine_dialog_locals_for_svc_catalog_provision(resource_action, target_ansible, finish_submit_endpoint)).to eq(:resource_action_id => 456,
:target_id => 987,
:target_type => 'service_template_ansible_playbook',
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the target type end up being just service_template and not service_template_ansible_playbook? Otherwise this doesn't fix the issue, because the API doesn't know what a service_template_ansible_playbook collection is.

Also it seems weird to me that rubocop didn't flag the line length issue, but you could put the hash on a new line so it looks something like:

expect(method_call).to eq(
  :resource_action_id => "123",
  :target_id          => "321",
  # etc
)

@lgalis lgalis force-pushed the use_service_template_base_class_for_srvc_dialog_api branch from d84d09b to b63a6ce Compare December 11, 2017 19:04
@lgalis
Copy link
Contributor Author

lgalis commented Dec 11, 2017

@eclarizio - fixed the spec - the instance_double requires the kind_of to be specifically allowed to return the real base class - please re-review

@lgalis lgalis force-pushed the use_service_template_base_class_for_srvc_dialog_api branch from b63a6ce to 0be177f Compare December 11, 2017 20:20
@lgalis lgalis force-pushed the use_service_template_base_class_for_srvc_dialog_api branch from 0be177f to da71436 Compare December 11, 2017 20:36
@miq-bot
Copy link
Member

miq-bot commented Dec 11, 2017

Checked commits lgalis/manageiq-ui-classic@c1de2a0~...da71436 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

eclarizio
eclarizio approved these changes Dec 11, 2017
View reviewed changes
Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

👍

@dclarizio dclarizio merged commit 37e15d2 into ManageIQ:master Dec 11, 2017
@dclarizio dclarizio added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 11, 2017
simaishi pushed a commit that referenced this pull request Dec 12, 2017
@simaishi
Merge pull request #2991 from lgalis/use_service_template_base_class_…
b57b8ec 
…for_srvc_dialog_api

Fix ordering an Ansible Playbook catalog items
(cherry picked from commit 37e15d2)

https://bugzilla.redhat.com/show_bug.cgi?id=1525077
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit b57b8ec36c339b9f5666cf8f738591a7d232f85d
Author: Dan Clarizio <dclarizi@redhat.com>
Date:   Mon Dec 11 14:52:31 2017 -0800

    Merge pull request #2991 from lgalis/use_service_template_base_class_for_srvc_dialog_api
    
    Fix ordering an Ansible Playbook catalog items
    (cherry picked from commit 37e15d2062ff3e5f81f3a2866cfcb7153bb680e1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1525077

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eclarizio eclarizio eclarizio approved these changes

Assignees

@dclarizio dclarizio

Labels
blocker bug gaprindashvili/backported services
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lgalis @miq-bot @simaishi @eclarizio @dclarizio