Support sysprep for windows templates #2479
Conversation
pkliczewski
changed the title
|
@martinpovolny please review |
|
@miq-bot assign @martinpovolny |
|
This PR depends on ManageIQ/manageiq#16263 |
| @@ -297,7 +335,7 @@ | |||
| :label => _("Custom Specification"), | |||
| :prefix => "/miq_request/", | |||
| :keys => keys}) | |||
| - if (!@options && @edit && @edit[:new] && @edit[:new][:sysprep_enabled] && @edit[:new][:sysprep_enabled][0] && @edit[:new][:sysprep_enabled][0] != "disabled") || (!@edit && @options && @options[:sysprep_enabled] && @options[:sysprep_enabled][0] && @options[:sysprep_enabled][0] != "disabled") | |||
| - if ((!@options && @edit && @edit[:new] && @edit[:new][:sysprep_enabled] && @edit[:new][:sysprep_enabled][0] && @edit[:new][:sysprep_enabled][0] != "disabled") || (!@edit && @options && @options[:sysprep_enabled] && @options[:sysprep_enabled][0] && @options[:sysprep_enabled][0] != "disabled")) && !wf.kind_of?(ManageIQ::Providers::Redhat::InfraManager::ProvisionWorkflow) | |||
There was a problem hiding this comment.
this line is getting even longer than before
can you extract that to a (helper) method?
There was a problem hiding this comment.
@martinpovolny Where do you propose to extract this condition to? This condition needs to be checked for ovirt and vmware provider.
|
@pkliczewski : can you get someone from your group to tests this? (in the browser) |
| @@ -180,7 +180,7 @@ | |||
| :prefix => "/miq_request/", | |||
| :keys => keys}) | |||
| - when :customize | |||
| - if wf.kind_of?(ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow) && !wf.supports_pxe? && !wf.supports_iso? | |||
| - if (wf.kind_of?(ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow) || wf.kind_of?(ManageIQ::Providers::Redhat::InfraManager::ProvisionWorkflow)) && !wf.supports_pxe? && !wf.supports_iso? | |||
There was a problem hiding this comment.
this looks like a pretty complex condition to be in a template, can you extract that to a (helper) method, please?
There was a problem hiding this comment.
btw does this condition represent some property of the provider? or the workflow? maybe it shoudl be pushed even deeper in the model?
There was a problem hiding this comment.
This condition checks only the type of the workflow. We want to have similar customize tab as there is for vmware.
|
The travis failure is unrelated. |
|
any chance of adding at least a trivial spec for this? |
|
Will check whether there is existing spec for this and if so I will add new one. |
We want to let the user to upload sysprep xml directly from the customize tab in the same way as we do for vmware. Bug-Url: https://bugzilla.redhat.com/1323289
|
Checked commit pkliczewski@861edec with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 app/controllers/catalog_controller.rb
app/controllers/miq_request_controller.rb
app/views/shared/views/_prov_dialog.html.haml |
|
Looks good!. The CC issue can be ignored. Can be merged once tested. I don't have the capacity to test this right now :-( |
|
I tested this, in the UI, it works as expected. |
We want to let the user to upload sysprep xml directly from
the customize tab in the same way as we do for vmware.
Bug-Url:
https://bugzilla.redhat.com/1323289