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

Changes to AnsibleTowerJobTemplateDialogService to support both job and workflow template. #4157

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

lfu
Copy link
Member

@lfu lfu commented Jun 18, 2018

The dialog code for Ansible Tower job template can be used for workflow template as well.

  • Add Service Name field in Options group so the dialog would have at least one box.

  • Limit option is supported only by Ansible Job Template. Ansible Workflow Template seems does not support it.

Depends on ManageIQ/manageiq-providers-ansible_tower#103.

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

Before:
screen shot 2018-08-13 at 6 01 02 pm

After:
screen shot 2018-08-13 at 6 01 36 pm

New dialog for workflow job template:
screen shot 2018-08-13 at 6 01 47 pm

@lfu lfu changed the title Refactoring of AnsibleTowerJobTemplateDialogService. [WIP] Refactoring of AnsibleTowerJobTemplateDialogService. Jun 28, 2018
@miq-bot miq-bot added the wip label Jun 28, 2018
@lfu lfu force-pushed the ansible_workflow_1590975 branch from 808ddd6 to ee1a3de Compare June 29, 2018 16:18
@lfu lfu changed the title [WIP] Refactoring of AnsibleTowerJobTemplateDialogService. Changes to AnsibleTowerJobTemplateDialogService to support both job and workflow template. Jun 29, 2018
@miq-bot miq-bot removed the wip label Jun 29, 2018
@lfu lfu force-pushed the ansible_workflow_1590975 branch from ee1a3de to c9fdbb6 Compare July 2, 2018 18:29
@mzazrivec
Copy link
Contributor

@lfu Please remove the pending core label once the provider PR (and everything else needed in the core) is merged. Thanks.

@d-m-u
Copy link
Contributor

d-m-u commented Jul 24, 2018

Does this need tests?

@gmcculloug
Copy link
Member

@lfu Please review test failure.

@lfu lfu force-pushed the ansible_workflow_1590975 branch 2 times, most recently from 76a17f8 to 2d6e9c7 Compare August 13, 2018 16:14
@lfu
Copy link
Member Author

lfu commented Aug 13, 2018

@miq-bot remove_label "pending core"

@miq-bot
Copy link
Member

miq-bot commented Aug 13, 2018

@lfu Cannot remove the following label because they are not recognized: "pending core"

@lfu
Copy link
Member Author

lfu commented Aug 13, 2018

@miq-bot remove_label pending core

@lfu
Copy link
Member Author

lfu commented Aug 13, 2018

Please review.

@gmcculloug
Copy link
Member

@lfu Please add screenshots of the before/after dialogs for JobTemplates and include the new dialog for Workflows to the description.

@h-kataria Please review.

@h-kataria
Copy link
Contributor

@lfu looks like code is very similar in these methods add_service_name_field and add_limit_field if that can be merged into single method that will address codeclimate error

Add service_name field so the dialog would have at least one tab.

https://bugzilla.redhat.com/show_bug.cgi?id=1590975
@miq-bot
Copy link
Member

miq-bot commented Aug 14, 2018

Checked commit lfu@c40c18c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@h-kataria h-kataria self-assigned this Aug 14, 2018
@h-kataria h-kataria added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 14, 2018
@h-kataria h-kataria merged commit a8b34de into ManageIQ:master Aug 14, 2018
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.

6 participants